dimas-b commented on code in PR #4409:
URL: https://github.com/apache/polaris/pull/4409#discussion_r3237956437
##########
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:
With throwing the caller has to examine the request's sub-type to avoid
hitting those exceptions... this might promote a lot of if/else/switch code
:thinking: Optional will allow more generic processing relying only on the
presence of data.... just sharing my view :)
--
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]
