dimas-b commented on code in PR #4409:
URL: https://github.com/apache/polaris/pull/4409#discussion_r3236435324
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/AuthorizationRequest.java:
##########
@@ -19,100 +19,56 @@
package org.apache.polaris.core.auth;
import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
import java.util.List;
-import java.util.Objects;
-import java.util.stream.Collectors;
import org.apache.polaris.core.entity.PolarisEntityType;
-import org.apache.polaris.immutables.PolarisImmutable;
-import org.immutables.value.Value;
/**
* Authorization request inputs for pre-authorization and core authorization.
*
- * <p>This wrapper keeps authorization inputs together and conveys the intent
to be authorized via
- * {@link AuthorizationTargetBinding} target bindings.
+ * <p>This hierarchy makes the target shape explicit on the request itself
while preserving the
+ * normalized compatibility accessors used by current authorizer
implementations.
*/
-@PolarisImmutable
-public interface AuthorizationRequest {
+public sealed interface AuthorizationRequest
+ permits UntargetedAuthorizationRequest,
+ SingleTargetAuthorizationRequest,
+ PairwiseTargetAuthorizationRequest {
+ static AuthorizationRequest of(@Nonnull PolarisAuthorizableOperation
operation) {
+ return new UntargetedAuthorizationRequest(operation);
+ }
+
static AuthorizationRequest of(
- @Nonnull PolarisPrincipal principal,
- @Nonnull PolarisAuthorizableOperation operation,
- @Nonnull List<AuthorizationTargetBinding> targetBindings) {
- return ImmutableAuthorizationRequest.builder()
- .principal(principal)
- .operation(operation)
- .targetBindings(targetBindings)
- .build();
+ @Nonnull PolarisAuthorizableOperation operation, @Nonnull
PolarisSecurable target) {
+ return new SingleTargetAuthorizationRequest(operation, target);
}
- /** Returns the principal requesting authorization. */
- @Nonnull
- PolarisPrincipal getPrincipal();
+ static AuthorizationRequest of(
+ @Nonnull PolarisAuthorizableOperation operation,
+ @Nullable PolarisSecurable target,
+ @Nullable PolarisSecurable secondary) {
+ return new PairwiseTargetAuthorizationRequest(operation, target,
secondary);
+ }
/** Returns the operation being authorized. */
@Nonnull
PolarisAuthorizableOperation getOperation();
- /** Returns the target/secondary target bindings. */
+ /** Returns the primary target securables, if any. */
@Nonnull
- List<AuthorizationTargetBinding> getTargetBindings();
+ List<PolarisSecurable> getTargets();
- /**
- * Returns the primary target securables, if any.
- *
- * <p>Compatibility accessor derived from {@link #getTargetBindings()}.
- */
+ /** Returns secondary securables, if any. */
@Nonnull
- @Value.Derived
- default List<PolarisSecurable> getTargets() {
- return getTargetBindings().stream()
- .map(AuthorizationTargetBinding::getTarget)
- .filter(Objects::nonNull)
- .toList();
- }
-
- /**
- * Returns secondary securables, if any.
- *
- * <p>Compatibility accessor derived from {@link #getTargetBindings()}.
- */
- @Nonnull
- @Value.Derived
- default List<PolarisSecurable> getSecondaries() {
- return getTargetBindings().stream()
- .map(AuthorizationTargetBinding::getSecondary)
- .filter(Objects::nonNull)
- .toList();
- }
-
- /**
- * Returns a stable debug string for authorization messages.
- *
- * <p>Includes the operation, principal name, formatted targets, and
formatted secondaries.
- */
- @Nonnull
- default String formatForAuthorizationMessage() {
- return String.format(
- "operation=%s principal=%s targets=%s secondaries=%s",
- getOperation(),
- getPrincipal().getName(),
- formatSecurables(getTargets()),
- formatSecurables(getSecondaries()));
- }
-
- private static String formatSecurables(List<PolarisSecurable> securables) {
- return securables.stream()
- .map(PolarisSecurable::formatForAuthorizationMessage)
- .collect(Collectors.joining(", ", "[", "]"));
- }
+ List<PolarisSecurable> getSecondaries();
Review Comment:
On a second thought, I think we might be over-designing this 😅
I guess we do not have any existing use cases for multiple
targets/secondaries per `AuthorizationRequest`.
I personally gravitate towards starting simple and just have one triplet:
<operation, target, secondary> per `AuthorizationRequest`, the latter two
parameters being optional.
I think, for the sake of making progress, it might be best to have a simple
SPI now and worry about more complex case later, if we ever have to. WDYT?
##########
polaris-core/src/main/java/org/apache/polaris/core/auth/AuthorizationRequest.java:
##########
@@ -19,100 +19,56 @@
package org.apache.polaris.core.auth;
import jakarta.annotation.Nonnull;
+import jakarta.annotation.Nullable;
import java.util.List;
-import java.util.Objects;
-import java.util.stream.Collectors;
import org.apache.polaris.core.entity.PolarisEntityType;
-import org.apache.polaris.immutables.PolarisImmutable;
-import org.immutables.value.Value;
/**
* Authorization request inputs for pre-authorization and core authorization.
*
- * <p>This wrapper keeps authorization inputs together and conveys the intent
to be authorized via
- * {@link AuthorizationTargetBinding} target bindings.
+ * <p>This hierarchy makes the target shape explicit on the request itself
while preserving the
+ * normalized compatibility accessors used by current authorizer
implementations.
*/
-@PolarisImmutable
-public interface AuthorizationRequest {
+public sealed interface AuthorizationRequest
+ permits UntargetedAuthorizationRequest,
+ SingleTargetAuthorizationRequest,
+ PairwiseTargetAuthorizationRequest {
+ static AuthorizationRequest of(@Nonnull PolarisAuthorizableOperation
operation) {
+ return new UntargetedAuthorizationRequest(operation);
+ }
+
static AuthorizationRequest of(
- @Nonnull PolarisPrincipal principal,
- @Nonnull PolarisAuthorizableOperation operation,
- @Nonnull List<AuthorizationTargetBinding> targetBindings) {
- return ImmutableAuthorizationRequest.builder()
- .principal(principal)
- .operation(operation)
- .targetBindings(targetBindings)
- .build();
+ @Nonnull PolarisAuthorizableOperation operation, @Nonnull
PolarisSecurable target) {
+ return new SingleTargetAuthorizationRequest(operation, target);
}
- /** Returns the principal requesting authorization. */
- @Nonnull
- PolarisPrincipal getPrincipal();
+ static AuthorizationRequest of(
+ @Nonnull PolarisAuthorizableOperation operation,
+ @Nullable PolarisSecurable target,
+ @Nullable PolarisSecurable secondary) {
+ return new PairwiseTargetAuthorizationRequest(operation, target,
secondary);
+ }
/** Returns the operation being authorized. */
@Nonnull
PolarisAuthorizableOperation getOperation();
- /** Returns the target/secondary target bindings. */
+ /** Returns the primary target securables, if any. */
@Nonnull
- List<AuthorizationTargetBinding> getTargetBindings();
+ List<PolarisSecurable> getTargets();
- /**
- * Returns the primary target securables, if any.
- *
- * <p>Compatibility accessor derived from {@link #getTargetBindings()}.
- */
+ /** Returns secondary securables, if any. */
@Nonnull
- @Value.Derived
- default List<PolarisSecurable> getTargets() {
- return getTargetBindings().stream()
- .map(AuthorizationTargetBinding::getTarget)
- .filter(Objects::nonNull)
- .toList();
- }
-
- /**
- * Returns secondary securables, if any.
- *
- * <p>Compatibility accessor derived from {@link #getTargetBindings()}.
- */
- @Nonnull
- @Value.Derived
- default List<PolarisSecurable> getSecondaries() {
- return getTargetBindings().stream()
- .map(AuthorizationTargetBinding::getSecondary)
- .filter(Objects::nonNull)
- .toList();
- }
-
- /**
- * Returns a stable debug string for authorization messages.
- *
- * <p>Includes the operation, principal name, formatted targets, and
formatted secondaries.
- */
- @Nonnull
- default String formatForAuthorizationMessage() {
- return String.format(
- "operation=%s principal=%s targets=%s secondaries=%s",
- getOperation(),
- getPrincipal().getName(),
- formatSecurables(getTargets()),
- formatSecurables(getSecondaries()));
- }
-
- private static String formatSecurables(List<PolarisSecurable> securables) {
- return securables.stream()
- .map(PolarisSecurable::formatForAuthorizationMessage)
- .collect(Collectors.joining(", ", "[", "]"));
- }
+ List<PolarisSecurable> getSecondaries();
Review Comment:
On a second thought, I think we might be over-designing this 😅
I guess we do not have any existing use cases for multiple
targets/secondaries per `AuthorizationRequest`.
I personally gravitate towards starting simple and just have one triplet:
<operation, target, secondary> per `AuthorizationRequest`, the latter two
parameters being optional.
I think, for the sake of making progress, it might be best to have a simple
SPI now and worry about more complex cases later, if we ever have to. 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]
