dimas-b commented on code in PR #2767:
URL: https://github.com/apache/polaris/pull/2767#discussion_r2412093796
##########
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(
Review Comment:
Would it be possible to add `default` (possibly inefficient) implementations
to the new methods to simplify transition in downstream projects? This is just
a matter of convenience, so optional.
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizerImpl.java:
##########
@@ -774,33 +775,63 @@ public void authorizeOrThrow(
@Nonnull PolarisAuthorizableOperation authzOp,
@Nullable List<PolarisResolvedPathWrapper> targets,
@Nullable List<PolarisResolvedPathWrapper> secondaries) {
+ authorizeOrThrow(
+ polarisPrincipal, activatedEntities, EnumSet.of(authzOp), targets,
secondaries);
+ }
+
+ @Override
+ public void authorizeOrThrow(
+ @Nonnull PolarisPrincipal polarisPrincipal,
+ @Nonnull Set<PolarisBaseEntity> activatedEntities,
+ @Nonnull Set<PolarisAuthorizableOperation> authzOps,
+ @Nullable PolarisResolvedPathWrapper target,
+ @Nullable PolarisResolvedPathWrapper secondary) {
+ authorizeOrThrow(
+ polarisPrincipal,
+ activatedEntities,
+ authzOps,
+ target == null ? null : List.of(target),
+ secondary == null ? null : List.of(secondary));
+ }
+
+ @Override
+ public void authorizeOrThrow(
+ @Nonnull PolarisPrincipal polarisPrincipal,
+ @Nonnull Set<PolarisBaseEntity> activatedEntities,
+ @Nonnull Set<PolarisAuthorizableOperation> authzOps,
+ @Nullable List<PolarisResolvedPathWrapper> targets,
+ @Nullable List<PolarisResolvedPathWrapper> secondaries) {
boolean enforceCredentialRotationRequiredState =
realmConfig.getConfig(
FeatureConfiguration.ENFORCE_PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_CHECKING);
boolean isRoot = getRootPrincipalName().equals(polarisPrincipal.getName());
- if (enforceCredentialRotationRequiredState
- && polarisPrincipal
- .getProperties()
-
.containsKey(PolarisEntityConstants.PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE)
- && authzOp != PolarisAuthorizableOperation.ROTATE_CREDENTIALS) {
- throw new ForbiddenException(
- "Principal '%s' is not authorized for op %s due to
PRINCIPAL_CREDENTIAL_ROTATION_REQUIRED_STATE",
- polarisPrincipal.getName(), authzOp);
- } else if (authzOp == PolarisAuthorizableOperation.RESET_CREDENTIALS) {
- if (!isRoot) {
- throw new ForbiddenException("Only Root principal(service-admin) can
perform %s", authzOp);
+ for (PolarisAuthorizableOperation authzOp : authzOps) {
Review Comment:
This loop could be done in a `default` impl. of the new method (redirecting
to old methods). Performance-wise it would be the same, as far as I can tell,
but it will have an easier migration path downstream.
--
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]
