sungwy commented on code in PR #3999:
URL: https://github.com/apache/polaris/pull/3999#discussion_r2937819734
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java:
##########
@@ -747,15 +755,75 @@ public PolarisAuthorizerImpl(RealmConfig realmConfig) {
@Override
public void resolveAuthorizationInputs(
@Nonnull AuthorizationState authzState, @Nonnull AuthorizationRequest
request) {
- throw new UnsupportedOperationException(
- "resolveAuthorizationInputs is not implemented yet for
PolarisAuthorizerImpl");
+ PolarisResolutionManifest resolutionManifest =
authzState.getResolutionManifest();
+ resolutionManifest.resolveAll();
}
@Override
+ @Nonnull
public AuthorizationDecision authorize(
@Nonnull AuthorizationState authzState, @Nonnull AuthorizationRequest
request) {
- throw new UnsupportedOperationException(
- "authorize is not implemented yet for PolarisAuthorizerImpl");
+ PolarisResolutionManifest resolutionManifest =
authzState.getResolutionManifest();
+ RbacOperationSemantics semantics =
RbacOperationSemantics.forOperation(request.getOperation());
+ // Delegate to legacy authorizeOrThrow() to preserve existing RBAC
privilege
+ // evaluation semantics. A later cleanup can refactor internals to a
+ // decision-first approach and remove this exception-to-decision
adaptation.
+ try {
+ List<PolarisResolvedPathWrapper> resolvedSecondaries =
+ !semantics.hasSecondaryPrivileges() ||
request.getSecondaries().isEmpty()
+ ? null
+ : getResolvedSecurables(
+ resolutionManifest, semantics, request.getSecondaries(),
TargetType.SECONDARY);
+ authorizeOrThrow(
+ request.getPrincipal(),
+ resolutionManifest.getAllActivatedCatalogRoleAndPrincipalRoles(),
+ request.getOperation(),
+ getResolvedSecurables(
+ resolutionManifest, semantics, request.getTargets(),
TargetType.TARGET),
+ resolvedSecondaries);
+ return AuthorizationDecision.allow();
+ } catch (ForbiddenException e) {
+ return AuthorizationDecision.deny(e.getMessage());
+ }
+ }
+
+ private List<PolarisResolvedPathWrapper> getResolvedSecurables(
+ PolarisResolutionManifest resolutionManifest,
+ RbacOperationSemantics semantics,
+ List<PolarisSecurable> securables,
+ TargetType targetType) {
+ boolean prependRootContainer =
+ switch (targetType) {
+ case TARGET -> semantics.targetScope() == PathEvaluationScope.ROOT;
Review Comment:
Hi @jbonofre - thanks for the early feedback on the PR :)
I was confused initially as well, so your confusion isn't unfounded. the
`ROOT` scope here refers to whether `ROOT` container should be prepended to the
resolved path, and hence whether the `ROOT` container resource should be
included for privilege evaluation.
For example, `PolarisAuthorizableOperation.LIST_NAMESPACES` prepends root
container:
https://github.com/apache/polaris/blob/00061ff0dc44570eba8d3ce72835ab66758baa7f/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java#L135
`PolarisAuthorizableOperation.GET_APPLICABLE_POLICIES_ON_CATALOG` sets
prependRootContainer to `false`.
https://github.com/apache/polaris/blob/00061ff0dc44570eba8d3ce72835ab66758baa7f/runtime/service/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalogHandler.java#L195
I think the name `PathEvaluationScope` may be misleading. I'll think of a
better name for this enum
--
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]
