HonahX commented on code in PR #2767:
URL: https://github.com/apache/polaris/pull/2767#discussion_r2412187884
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -41,4 +41,18 @@ void authorizeOrThrow(
@Nonnull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries);
+
+ void authorizeOrThrow(
+ @Nonnull PolarisPrincipal polarisPrincipal,
+ @Nonnull Set<PolarisBaseEntity> activatedEntities,
+ @Nonnull Set<PolarisAuthorizableOperation> authzOps,
+ @Nullable PolarisResolvedPathWrapper target,
+ @Nullable PolarisResolvedPathWrapper secondary);
+
+ void authorizeOrThrow(
+ @Nonnull PolarisPrincipal polarisPrincipal,
+ @Nonnull Set<PolarisBaseEntity> activatedEntities,
+ @Nonnull Set<PolarisAuthorizableOperation> authzOps,
+ @Nullable List<PolarisResolvedPathWrapper> targets,
+ @Nullable List<PolarisResolvedPathWrapper> secondaries);
Review Comment:
Sounds good, will start a discussion!
IMHO, batch AuthorizableOperations should mainly be used for fine-grained
cases like `updateTable`, where a single endpoint handles multiple types of
updates that each require distinct authorization controls. The original
intention behind `AuthorizableOperation` was that each operation corresponds to
one API call, and `updateTable` is a special exception that needs finer-grained
handling.
In the updateTable case, there’s a sort of internal inheritance — for
example, a fine-grained operation like TABLE_ADD_SNAPSHOT conceptually extends
a broader one such as TABLE_WRITE_PROPERTIES. All these fine-grained operations
should share the same set of primary and secondary securable entities; it’s
just that this “inheritance” relationship hasn’t been reflected in the code yet.
If two operations involve different sets of securables, they should ideally
be authorized through separate API calls, each likely corresponding to its own
endpoint.
If in the future the above assumption does not hold, I assume we will need
large change anyway and thus a new set of apis will be introduced.
But the above are just my thoughts, let's discuss in dev mailing list to
collect thoughts on the new api and see what we want to support : )
--
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]
