bharos commented on code in PR #12167:
URL: https://github.com/apache/gravitino/pull/12167#discussion_r3707890992
##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java:
##########
@@ -158,6 +162,28 @@ public Object invoke(MethodInvocation methodInvocation)
throws Throwable {
return IcebergExceptionMapper.toRESTResponse(
new RuntimeException("Failed to validate user", ex));
}
+
+ // Role assumption: reject a NAMED declaration that names roles the
caller does not hold
+ // (403); ALL/NONE need no membership check.
+ ActiveRoles activeRoles =
authorizationRequestContext.getActiveRoles();
+ if (activeRoles.mode() == ActiveRoles.Mode.NAMED) {
+ Set<String> unheldRoles =
+ GravitinoAuthorizerProvider.getInstance()
+ .getGravitinoAuthorizer()
+ .findUnheldRoles(
Review Comment:
Updated the code now.
`findUnheldRoles` now reuses the existing `prefetchUserAndGroupInfo` which
is the same single `batchGetAuthSubjectsForUser` query the authorize path
already issues. The interceptor now passes one `AuthorizationRequestContext` to
both the membership check and the expression evaluator (it previously created a
fresh one), so the prefetch is shared and the second call hits the per-request
cache.
NAMED header + expression: 0 extra queries.
NAMED header, no expression: 1 query - intrinsic, we have to read role
bindings to verify the claim.
ALL / NONE / no header: 0 extra queries, returns immediately.
`testFindUnheldRolesEmptyWhenAllRolesHeld` asserts exactly one
`batchGetAuthSubjectsForUser` and zero `MetadataIdConverter.getID` calls across
the membership check plus authorize.
--
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]
