sungwy commented on code in PR #4409:
URL: https://github.com/apache/polaris/pull/4409#discussion_r3237723242
##########
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,
+ @Nonnull AuthorizationRequest request);
+
+ /**
+ * Resolve authorizer-specific inputs for a batch of authorization requests
that share one
+ * principal.
+ *
+ * <p>Implementations must define their own batch pre-resolution behavior
explicitly because
+ * manifest registration is authorizer-specific.
+ */
+ void resolveAuthorizationInputs(
+ @Nonnull AuthorizationState authzState,
+ @Nonnull PolarisPrincipal polarisPrincipal,
+ @Nonnull List<AuthorizationRequest> requests);
/**
* Core authorization entry point for the new SPI.
*
* <p>Implementations should rely on any required state in {@link
AuthorizationState} and the
- * intent captured by {@link AuthorizationRequest} (principal, operation,
and target securables).
+ * intent captured by {@link AuthorizationRequest} (operation and target
securables), together
+ * with the explicit {@link PolarisPrincipal} argument.
*/
@Nonnull
AuthorizationDecision authorize(
- @Nonnull AuthorizationState authzState, @Nonnull AuthorizationRequest
request);
+ @Nonnull AuthorizationState authzState,
+ @Nonnull PolarisPrincipal polarisPrincipal,
+ @Nonnull AuthorizationRequest request);
+
+ /**
+ * Core authorization entry point for a batch of requests that share one
principal.
+ *
+ * <p>The default behavior preserves semantics by evaluating requests
independently in order and
+ * returning the first denial. Implementations may override this to use a
single batched
+ * downstream authorization call.
+ */
+ @Nonnull
+ default AuthorizationDecision authorize(
+ @Nonnull AuthorizationState authzState,
+ @Nonnull PolarisPrincipal polarisPrincipal,
+ @Nonnull List<AuthorizationRequest> requests) {
+ Preconditions.checkArgument(
+ !requests.isEmpty(), "Authorization request batch must contain at
least one request");
+ for (AuthorizationRequest request : requests) {
+ AuthorizationDecision decision = authorize(authzState, polarisPrincipal,
request);
+ if (!decision.isAllowed()) {
+ return decision;
Review Comment:
I agree. I don’t think the current batch `authorize` shape is the right
model for list filtering, since that likely needs a different response shape
entirely rather than a binary decision.
I’d prefer to leave that out of scope for this PR and introduce it as a
separate dedicated API. However, if we think we need to reach consensus on that
before moving this SPI forward, I’m okay slowing this down and thinking it
through first. My current view, though, is that `authorize` and `filterTargets`
should be separate SPIs.
--
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]
