dimas-b commented on code in PR #499:
URL: https://github.com/apache/polaris/pull/499#discussion_r1866384350
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/PolarisAuthorizer.java:
##########
@@ -19,26 +19,28 @@
package org.apache.polaris.core.auth;
import jakarta.annotation.Nonnull;
-import jakarta.annotation.Nullable;
-import java.util.List;
-import java.util.Set;
-import org.apache.polaris.core.entity.PolarisBaseEntity;
-import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
+import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifest;
/** Interface for invoking authorization checks. */
public interface PolarisAuthorizer {
+ /**
+ * Validates whether the requested operation is permitted based on the
collection of entities
+ * (including principals, roles, and catalog objects) that are affected by
the operation.
+ *
+ * <p>"activated" entities, "targets" and "secondaries" are contained within
the provided
+ * manifest. The extra selector parameters merely define what sub-set of
objects from the manifest
+ * should be considered as "targets", etc.
+ *
+ * <p>The effective principal information is also provided in the manifest.
+ *
+ * @param manifest defines the input for authorization checks.
+ * @param operation the operation being authorized.
+ * @param considerCatalogRoles whether catalog roles should be considered
({@code true}) or only
+ * principal roles ({@code false}).
+ */
void authorizeOrThrow(
- @Nonnull AuthenticatedPolarisPrincipal authenticatedPrincipal,
- @Nonnull Set<PolarisBaseEntity> activatedEntities,
- @Nonnull PolarisAuthorizableOperation authzOp,
- @Nullable PolarisResolvedPathWrapper target,
- @Nullable PolarisResolvedPathWrapper secondary);
-
- void authorizeOrThrow(
- @Nonnull AuthenticatedPolarisPrincipal authenticatedPrincipal,
- @Nonnull Set<PolarisBaseEntity> activatedEntities,
- @Nonnull PolarisAuthorizableOperation authzOp,
- @Nullable List<PolarisResolvedPathWrapper> targets,
- @Nullable List<PolarisResolvedPathWrapper> secondaries);
+ @Nonnull PolarisResolutionManifest manifest,
Review Comment:
I do believe that this change actually decouples the Authorizer
_implementation_ from entity resolution. That is because Authrorizer _inputs_
in this PR are self-sufficient when the authrorization SPI is invoked. The
implementation does not have to access the model. The default implementation
certainly can function purely on the data from the authrorization SPI method
parameters.
I suppose you meant it to be approached the other way, when the Authorizer
SPI is decoupled from entity resolution, but the Authorizer implementation
resolves what is needs to resolve against the storage data model inside the
authorization calls.
The latter approach is certainly possible, but it will lose strong
correlation with public API inputs. My reading of current code made me think,
that the Authorizer was expected to act exactly on the same state of data that
the API implementations use for producing API outputs. Is that so?
In that case, and if the Authorizer has to perform extra resolutions, it
will complicate the contract of the Persistence layer, which will then have to
ensure consistency across calls from multiple services. WDYT?
--
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]
