flyrain commented on code in PR #4409:
URL: https://github.com/apache/polaris/pull/4409#discussion_r3238072492
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -38,26 +39,85 @@ public interface PolarisAuthorizer {
* <p>This method should not perform authorization decisions directly.
*/
void resolveAuthorizationInputs(
- @Nonnull AuthorizationState authzState, @Nonnull AuthorizationRequest
request);
+ @Nonnull AuthorizationState authzState,
+ @Nonnull PolarisPrincipal polarisPrincipal,
Review Comment:
I'd push back on lifting `PolarisPrincipal` out of `AuthorizationRequest`.
`(subject, action, resource)` is the canonical tuple for authorization, the
principal is the "who" in "who did what on what" and belongs with the operation
and target as part of one authorization question, not as a parallel argument
threaded alongside.
Batching feels like an internal concern of the request model, not a reason
to split principal out. The targeted shapes are really `(action, resource)`.
They don't need to know about the subject. So I'd factor that into its own
interface and let AuthorizationRequest compose principal on top:
```
// (action, resource)
public sealed interface AuthorizationIntent {
PolarisAuthorizableOperation operation();
record Untargeted(PolarisAuthorizableOperation operation)
implements AuthorizationIntent {}
record SingleTarget(PolarisAuthorizableOperation operation,
PolarisSecurable target)
implements AuthorizationIntent {}
record PairwiseTarget(
PolarisAuthorizableOperation operation,
@Nullable PolarisSecurable target,
@Nullable PolarisSecurable secondary)
implements AuthorizationIntent {}
}
// (subject, intents), single shape, batching is just N >= 1
record AuthorizationRequest(
PolarisPrincipal principal,
List<AuthorizationIntent> intents) {}
```
If a future flow ever needs mixed-principal batches, that's a new sealed
variant rather than an SPI signature change.
I think it's worth getting this right before handlers migrate, since walking
the SPI shape back later is much harder than now. Curious what you and others
think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
