This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 410d670cdb [#12591] refactor(authz): Extract protocol-neutral metadata
authorization (#12593)
410d670cdb is described below
commit 410d670cdbb3a05cb02a76088b3b36b6224639d0
Author: Qi Yu <[email protected]>
AuthorDate: Thu Aug 27 09:26:03 2026 +0800
[#12591] refactor(authz): Extract protocol-neutral metadata authorization
(#12593)
### What changes were proposed in this pull request?
This pull request:
- Moves the shared metadata authorization pipeline from the Iceberg REST
module to `server-common`.
- Keeps AOP Alliance adaptation inside each REST protocol module, so
`server-common` gains no interception-framework dependency.
- Introduces a protocol-neutral authorization target containing both
metadata identifiers and the directly addressed entity type.
- Centralizes user validation, active-role validation, custom handlers,
expression evaluation, and authorization failure handling.
- Adds protocol hooks for request target resolution and error response
mapping.
- Migrates Iceberg REST authorization to the shared pipeline without
changing its authorization behavior.
- Adds a shared schema-probe expression that permits `CREATE_SCHEMA`
while preserving deny precedence.
### Why are the changes needed?
Metadata REST protocols otherwise need to duplicate the same
authorization workflow and may implement user, role, and expression
checks inconsistently.
The shared pipeline also provides the dynamic entity type required by
`CAN_ACCESS_METADATA`, allowing later protocol implementations to reuse
the existing authorization expressions.
Fix: #12591
### Does this PR introduce _any_ user-facing change?
No. The Iceberg REST authorization behavior and error responses are
preserved.
### How was this patch tested?
- Added comprehensive unit tests for dynamic target resolution, user
validation, active roles, handler short-circuiting, protocol error
mapping, and operation error mapping.
- Added real-expression tests for schema probes, including
`CREATE_SCHEMA` and deny precedence.
- Verified the shared pipeline with 100% line and branch coverage.
- Ran the existing Iceberg metadata authorization interceptor tests.
- Ran the remaining Iceberg REST server tests excluding local
database-container-dependent tests.
- Verified the runtime dependency tree contains only the AOP Alliance
classes already provided by Jersey/HK2.
All test commands were run with proxy environment variables disabled.
---
.../service/rest/IcebergNamespaceOperations.java | 3 +-
...bergMetadataAuthorizationMethodInterceptor.java | 35 +-
.../AuthorizationExpressionConstants.java | 10 +
...BaseMetadataAuthorizationMethodInterceptor.java | 204 ++++++++---
.../TestAuthorizationExpressionEvaluator.java | 61 ++++
...BaseMetadataAuthorizationMethodInterceptor.java | 404 +++++++++++++++++++++
6 files changed, 659 insertions(+), 58 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java
index 959b5a39ec..0e0c2cf2dd 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/rest/IcebergNamespaceOperations.java
@@ -177,8 +177,7 @@ public class IcebergNamespaceOperations {
@Timed(name = "namespace-exists." + MetricNames.HTTP_PROCESS_DURATION,
absolute = true)
@ResponseMetered(name = "namespace-exists", absolute = true)
@AuthorizationExpression(
- expression =
- "ANY(OWNER, METALAKE, CATALOG) || ANY_USE_CATALOG && (SCHEMA::OWNER
|| ANY_USE_SCHEMA || ANY_CREATE_SCHEMA)",
+ expression =
AuthorizationExpressionConstants.PROBE_SCHEMA_AUTHORIZATION_EXPRESSION,
accessMetadataType = MetadataObject.Type.SCHEMA)
public Response namespaceExists(
@AuthorizationMetadata(type = Entity.EntityType.CATALOG)
@PathParam("prefix") String prefix,
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/IcebergMetadataAuthorizationMethodInterceptor.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/IcebergMetadataAuthorizationMethodInterceptor.java
index 17f5076eb6..31275cc75e 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/IcebergMetadataAuthorizationMethodInterceptor.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/IcebergMetadataAuthorizationMethodInterceptor.java
@@ -24,12 +24,15 @@ import java.lang.reflect.Parameter;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
+import org.aopalliance.intercept.MethodInterceptor;
+import org.aopalliance.intercept.MethodInvocation;
import org.apache.gravitino.Entity;
import org.apache.gravitino.Entity.EntityType;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.iceberg.common.ops.IcebergCatalogWrapper;
import org.apache.gravitino.iceberg.service.IcebergCatalogWrapperManager;
+import org.apache.gravitino.iceberg.service.IcebergExceptionMapper;
import org.apache.gravitino.iceberg.service.IcebergRESTUtils;
import
org.apache.gravitino.iceberg.service.authorization.IcebergRESTServerContext;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
@@ -46,10 +49,31 @@ import org.apache.iceberg.rest.RESTUtil;
* metadata authorization.
*/
public class IcebergMetadataAuthorizationMethodInterceptor
- extends BaseMetadataAuthorizationMethodInterceptor {
+ extends BaseMetadataAuthorizationMethodInterceptor implements
MethodInterceptor {
private final String metalakeName =
IcebergRESTServerContext.getInstance().metalakeName();
@Override
+ public Object invoke(MethodInvocation methodInvocation) throws Throwable {
+ return authorizeMethod(
+ methodInvocation.getMethod(), methodInvocation.getArguments(),
methodInvocation::proceed);
+ }
+
+ @Override
+ protected AuthorizationTarget resolveAuthorizationTarget(
+ Method method, AuthorizationExpression annotation, Parameter[]
parameters, Object[] args) {
+ Map<Entity.EntityType, NameIdentifier> nameIdentifierMap =
+ extractNameIdentifierFromParameters(parameters, args);
+ return new AuthorizationTarget(
+ nameIdentifierMap,
EntityType.valueOf(annotation.accessMetadataType().name()));
+ }
+
+ /**
+ * Extracts Gravitino identifiers from Iceberg REST path parameters.
+ *
+ * @param parameters invoked method parameters
+ * @param args invoked method arguments
+ * @return identifiers keyed by entity type
+ */
protected Map<Entity.EntityType, NameIdentifier>
extractNameIdentifierFromParameters(
Parameter[] parameters, Object[] args) {
Map<Entity.EntityType, NameIdentifier> nameIdentifierMap = new HashMap<>();
@@ -107,6 +131,11 @@ public class IcebergMetadataAuthorizationMethodInterceptor
return nameIdentifierMap;
}
+ @Override
+ protected Object toErrorResponse(Method method, Object[] args, Throwable
throwable) {
+ return IcebergExceptionMapper.toRESTResponse(throwable);
+ }
+
/**
* Creates an authorization handler for Iceberg-specific operations that
require custom logic
* beyond standard annotation-based authorization.
@@ -141,8 +170,8 @@ public class IcebergMetadataAuthorizationMethodInterceptor
}
@Override
- protected boolean isExceptionPropagate(Exception e) {
- return e.getClass().getName().startsWith("org.apache.iceberg.exceptions");
+ protected boolean isExceptionPropagate(Exception exception) {
+ return
exception.getClass().getName().startsWith("org.apache.iceberg.exceptions");
}
/**
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConstants.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConstants.java
index 7086f7e125..60374880b8 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConstants.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationExpressionConstants.java
@@ -26,6 +26,16 @@ public class AuthorizationExpressionConstants {
ANY_USE_CATALOG && (SCHEMA::OWNER || ANY_USE_SCHEMA)
""";
+ /**
+ * Authorizes a schema existence probe. CREATE_SCHEMA is intentionally
included because clients
+ * commonly check whether a schema exists before attempting to create it.
+ */
+ public static final String PROBE_SCHEMA_AUTHORIZATION_EXPRESSION =
+ """
+ ANY(OWNER, METALAKE, CATALOG) ||
+ ANY_USE_CATALOG && (SCHEMA::OWNER || ANY_USE_SCHEMA ||
ANY_CREATE_SCHEMA)
+ """;
+
public static final String LOAD_MODEL_AUTHORIZATION_EXPRESSION =
"""
ANY(OWNER, METALAKE, CATALOG) ||
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
b/server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
similarity index 51%
rename from
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
rename to
server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
index c4b7b13c52..dd22014fb1 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
@@ -22,35 +22,72 @@ package org.apache.gravitino.server.web.filter;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
-import org.aopalliance.intercept.MethodInterceptor;
-import org.aopalliance.intercept.MethodInvocation;
import org.apache.gravitino.Entity;
-import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.auth.ActiveRoles;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.AuthorizationUtils;
-import org.apache.gravitino.iceberg.service.IcebergExceptionMapper;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
import org.apache.gravitino.server.web.Utils;
import org.apache.gravitino.utils.PrincipalUtils;
-import org.apache.iceberg.exceptions.ForbiddenException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * Through dynamic proxy, obtain the annotations on the method and parameter
list to perform
- * metadata authorization.
+ * Runs the metadata authorization steps shared by REST protocols.
+ *
+ * <p>Protocol implementations only resolve request parameters into an {@link
AuthorizationTarget}
+ * and map failures to their response format. This class consistently
validates the user and active
+ * roles, runs any request-specific handler, and evaluates the standard
authorization expression.
*/
@SuppressWarnings("FormatStringAnnotation")
-public abstract class BaseMetadataAuthorizationMethodInterceptor implements
MethodInterceptor {
+public abstract class BaseMetadataAuthorizationMethodInterceptor {
private static final Logger LOG =
LoggerFactory.getLogger(BaseMetadataAuthorizationMethodInterceptor.class);
+ /** The metadata identifiers and entity type resolved from one protocol
request. */
+ protected static class AuthorizationTarget {
+ private final Map<Entity.EntityType, NameIdentifier> nameIdentifiers;
+ private final Entity.EntityType entityType;
+
+ /**
+ * Creates an authorization target.
+ *
+ * @param nameIdentifiers identifiers needed by the authorization
expression
+ * @param entityType the entity type directly addressed by this request
+ */
+ public AuthorizationTarget(
+ Map<Entity.EntityType, NameIdentifier> nameIdentifiers,
Entity.EntityType entityType) {
+ this.nameIdentifiers = Objects.requireNonNull(nameIdentifiers,
"nameIdentifiers");
+ this.entityType = Objects.requireNonNull(entityType, "entityType");
+ }
+
+ /**
+ * Returns the identifiers needed by the authorization expression. The map
remains mutable so a
+ * request-specific handler can add an identifier found in a request body.
+ *
+ * @return the identifiers keyed by entity type
+ */
+ public Map<Entity.EntityType, NameIdentifier> nameIdentifiers() {
+ return nameIdentifiers;
+ }
+
+ /**
+ * Returns the entity type directly addressed by this request.
+ *
+ * @return the target entity type
+ */
+ public Entity.EntityType entityType() {
+ return entityType;
+ }
+ }
+
/**
* Handler for request-specific authorization processing that cannot be
handled by standard
* annotation-based expressions. Implementations can enrich identifiers,
validate requests, and/or
@@ -67,10 +104,9 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
* </ul>
*
* @param nameIdentifierMap Name identifier map (can be modified to add
identifiers)
- * @throws ForbiddenException if authorization or validation fails
+ * @throws Exception if authorization or validation fails
*/
- void process(Map<Entity.EntityType, NameIdentifier> nameIdentifierMap)
- throws ForbiddenException;
+ void process(Map<Entity.EntityType, NameIdentifier> nameIdentifierMap)
throws Exception;
/**
* Whether this handler has completed full authorization. Called after
{@link #process} to
@@ -82,8 +118,41 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
boolean authorizationCompleted();
}
- protected abstract Map<Entity.EntityType, NameIdentifier>
extractNameIdentifierFromParameters(
- Parameter[] parameters, Object[] args);
+ /** Invokes the protocol operation after authorization succeeds. */
+ @FunctionalInterface
+ protected interface MethodInvoker {
+ /**
+ * Invokes the intercepted protocol operation.
+ *
+ * @return the operation result
+ * @throws Throwable if the operation fails
+ */
+ Object proceed() throws Throwable;
+ }
+
+ /**
+ * Resolves the metadata identifiers and the directly addressed entity type
from a protocol
+ * request. The entity type is kept separately because some protocols encode
catalog and schema
+ * requests in the same path parameter.
+ *
+ * @param method invoked protocol method
+ * @param annotation authorization annotation on the invoked method
+ * @param parameters invoked method parameters
+ * @param args invoked method arguments
+ * @return the resolved authorization target
+ */
+ protected abstract AuthorizationTarget resolveAuthorizationTarget(
+ Method method, AuthorizationExpression annotation, Parameter[]
parameters, Object[] args);
+
+ /**
+ * Maps an authorization or operation failure to the response format
required by a protocol.
+ *
+ * @param method invoked protocol method
+ * @param args invoked method arguments
+ * @param throwable failure to map
+ * @return the protocol response
+ */
+ protected abstract Object toErrorResponse(Method method, Object[] args,
Throwable throwable);
/**
* Create an authorization handler for this request, if special handling is
needed beyond standard
@@ -103,13 +172,23 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
return Optional.empty();
}
- protected boolean isExceptionPropagate(Exception e) {
+ /**
+ * Returns whether an exception should be returned as a protocol error
without being wrapped as an
+ * internal authorization failure.
+ *
+ * @param exception exception raised while authorizing the request
+ * @return {@code true} to preserve the original exception
+ */
+ protected boolean isExceptionPropagate(Exception exception) {
return false;
}
/**
- * Hook for subclasses to skip standard authorization before user validation
and expression
- * evaluation.
+ * Returns whether the complete local authorization pipeline should be
skipped. This is intended
+ * for a protocol proxy whose downstream Gravitino server authorizes the
same request.
+ *
+ * @param nameIdentifierMap identifiers resolved from the request
+ * @return {@code true} to skip user validation, handlers, and expression
evaluation
*/
protected boolean shouldSkipAuthorization(
Map<Entity.EntityType, NameIdentifier> nameIdentifierMap) {
@@ -117,54 +196,74 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
}
/**
- * Determine whether authorization is required and the rules via the
authorization annotation ,
- * and obtain the metadata ID that requires authorization via the
authorization annotation.
+ * Hook for requests that must validate the current user and active roles
but do not have a
+ * metadata object on which to evaluate an expression. A protocol root-list
request is a typical
+ * example: its returned children are filtered separately.
+ *
+ * @param target resolved authorization target
+ * @return {@code true} to skip only expression evaluation
+ */
+ protected boolean shouldSkipExpressionEvaluation(AuthorizationTarget target)
{
+ return false;
+ }
+
+ /**
+ * Authorizes a protocol method and maps all failures through the protocol
hook.
+ *
+ * <p>The protocol-specific interceptor adapts its invocation framework to
these plain Java
+ * arguments. Keeping that adapter outside this shared pipeline avoids
coupling server-common to
+ * the interception framework used by each REST server.
*
- * @param methodInvocation methodInvocation with the Method object
- * @return the return result of the original method.
- * @throws Throwable throw an exception when authorization fails.
+ * @param method method to authorize
+ * @param args method arguments
+ * @param methodInvoker operation to invoke after authorization succeeds
+ * @return the mapped error response, or the result of the invoked method
+ * @throws Throwable if the invocation infrastructure itself cannot run
*/
- @Override
- public Object invoke(MethodInvocation methodInvocation) throws Throwable {
+ protected final Object authorizeMethod(Method method, Object[] args,
MethodInvoker methodInvoker)
+ throws Throwable {
try {
- Method method = methodInvocation.getMethod();
Parameter[] parameters = method.getParameters();
AuthorizationExpression expressionAnnotation =
method.getAnnotation(AuthorizationExpression.class);
if (expressionAnnotation != null) {
String expression = expressionAnnotation.expression();
- Object[] args = methodInvocation.getArguments();
- Map<Entity.EntityType, NameIdentifier> nameIdentifierMap =
- extractNameIdentifierFromParameters(parameters, args);
+ AuthorizationTarget target =
+ resolveAuthorizationTarget(method, expressionAnnotation,
parameters, args);
+ Map<Entity.EntityType, NameIdentifier> nameIdentifierMap =
target.nameIdentifiers();
boolean skipStandardCheck = shouldSkipAuthorization(nameIdentifierMap);
- // Check if current user exists in the metalake.
NameIdentifier metalakeIdent =
nameIdentifierMap.get(Entity.EntityType.METALAKE);
AuthorizationRequestContext authorizationRequestContext = new
AuthorizationRequestContext();
if (!skipStandardCheck && metalakeIdent != null) {
String currentUser = PrincipalUtils.getCurrentUserName();
+ // Reuse this request context so user, role, and privilege checks
see exactly the same
+ // active-role selection and can share cached authorization results.
try {
- AuthorizationUtils.checkCurrentUser(metalakeIdent.name(),
currentUser);
- } catch (org.apache.gravitino.exceptions.ForbiddenException ex) {
+ AuthorizationUtils.checkCurrentUser(
+ metalakeIdent.name(), currentUser,
authorizationRequestContext);
+ } catch (ForbiddenException exception) {
LOG.info(
"User validation failed - User: '{}', Metalake: '{}', Reason:
{}",
currentUser,
metalakeIdent.name(),
- ex.getMessage());
- return IcebergExceptionMapper.toRESTResponse(ex);
- } catch (Exception ex) {
+ exception.getMessage());
+ throw exception;
+ } catch (Exception exception) {
+ // User lookup failures are different from a missing user.
Preserve the existing
+ // protocol behavior by returning an internal error instead of a
403 denial.
LOG.error(
"Unexpected error during user validation - User: '{}',
Metalake: '{}'",
currentUser,
metalakeIdent.name(),
- ex);
- return IcebergExceptionMapper.toRESTResponse(
- new RuntimeException("Failed to validate user", ex));
+ exception);
+ return toErrorResponse(
+ method, args, new RuntimeException("Failed to validate user",
exception));
}
- // Role assumption: reject a NAMED declaration that names roles the
caller does not hold
- // (403); ALL/NONE need no membership check.
+ // ALL and NONE already describe a complete role selection. NAMED is
different: every
+ // requested role must be checked so a caller cannot assume somebody
else's role.
ActiveRoles activeRoles =
authorizationRequestContext.getActiveRoles();
if (activeRoles.mode() == ActiveRoles.Mode.NAMED) {
Set<String> unheldRoles =
@@ -181,12 +280,11 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
"User '%s' cannot assume active role(s) that are not
held: %s",
currentUser, unheldRoles);
LOG.info(message);
- return IcebergExceptionMapper.toRESTResponse(new
ForbiddenException(message));
+ throw new ForbiddenException(message);
}
}
}
- // Process custom authorization if handler exists
Optional<AuthorizationHandler> handler =
createAuthorizationHandler(method, parameters, args);
@@ -196,18 +294,18 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
skipStandardCheck = authzHandler.authorizationCompleted();
}
- // Perform standard authorization check if custom handler didn't
complete it
- if (!skipStandardCheck) {
+ if (!skipStandardCheck && !shouldSkipExpressionEvaluation(target)) {
Map<String, Object> pathParams =
Utils.extractPathParamsFromParameters(parameters, args);
AuthorizationExpressionEvaluator authorizationExpressionEvaluator =
new AuthorizationExpressionEvaluator(expression);
boolean authorizeResult =
authorizationExpressionEvaluator.evaluate(
- nameIdentifierMap, pathParams, authorizationRequestContext,
Optional.empty());
+ nameIdentifierMap,
+ pathParams,
+ authorizationRequestContext,
+ Optional.of(target.entityType().name()));
if (!authorizeResult) {
- MetadataObject.Type type =
expressionAnnotation.accessMetadataType();
- NameIdentifier accessMetadataName =
- nameIdentifierMap.get(Entity.EntityType.valueOf(type.name()));
+ NameIdentifier accessMetadataName =
nameIdentifierMap.get(target.entityType());
String currentUser = PrincipalUtils.getCurrentUserName();
String methodName = method.getName();
String notAuthzMessage =
@@ -215,28 +313,28 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
"User '%s' is not authorized to perform operation '%s' on
metadata '%s' with expression '%s'",
currentUser, methodName, accessMetadataName, expression);
LOG.info(notAuthzMessage);
- return IcebergExceptionMapper.toRESTResponse(new
ForbiddenException(notAuthzMessage));
+ throw new ForbiddenException(notAuthzMessage);
}
}
}
} catch (Exception ex) {
- if (isExceptionPropagate(ex)) {
- return IcebergExceptionMapper.toRESTResponse(ex);
+ if (ex instanceof ForbiddenException || isExceptionPropagate(ex)) {
+ return toErrorResponse(method, args, ex);
}
String currentUser = PrincipalUtils.getCurrentUserName();
- String methodName = methodInvocation.getMethod().getName();
+ String methodName = method.getName();
String errorMessage =
String.format(
"Authorization failed due to system internal error, User: '%s',
Operation: '%s'",
currentUser, methodName);
LOG.info(errorMessage, ex);
- return IcebergExceptionMapper.toRESTResponse(new
RuntimeException(errorMessage, ex));
+ return toErrorResponse(method, args, new RuntimeException(errorMessage,
ex));
}
try {
- return methodInvocation.proceed();
+ return methodInvoker.proceed();
} catch (Throwable e) {
- return IcebergExceptionMapper.toRESTResponse(e);
+ return toErrorResponse(method, args, e);
}
}
}
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/expression/TestAuthorizationExpressionEvaluator.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/expression/TestAuthorizationExpressionEvaluator.java
index 03f2549933..88cd841cf7 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/expression/TestAuthorizationExpressionEvaluator.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/expression/TestAuthorizationExpressionEvaluator.java
@@ -17,13 +17,16 @@
package org.apache.gravitino.server.authorization.expression;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.when;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -36,6 +39,8 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.UserPrincipal;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
+import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
import org.apache.gravitino.server.authorization.MockGravitinoAuthorizer;
import org.apache.gravitino.utils.NameIdentifierUtil;
@@ -219,6 +224,62 @@ public class TestAuthorizationExpressionEvaluator {
}
}
+ @Test
+ public void testProbeSchemaExpressionAllowsCreateAndHonorsDeny() {
+ GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class);
+ Set<Privilege.Name> allowed = Set.of(Privilege.Name.USE_CATALOG,
Privilege.Name.CREATE_SCHEMA);
+ Set<Privilege.Name> denied = new HashSet<>();
+ when(authorizer.authorize(any(), any(), any(), any(), any()))
+ .thenAnswer(invocation -> allowed.contains(invocation.getArgument(3)));
+ when(authorizer.deny(any(), any(), any(), any(), any()))
+ .thenAnswer(invocation -> denied.contains(invocation.getArgument(3)));
+ when(authorizer.isOwner(any(), any(), any(), any())).thenReturn(false);
+ UserPrincipal principal = new UserPrincipal("tester");
+ Map<Entity.EntityType, NameIdentifier> metadataNames =
metadataNames(false);
+
+ Assertions.assertTrue(
+ evaluate(
+
AuthorizationExpressionConstants.PROBE_SCHEMA_AUTHORIZATION_EXPRESSION,
+ authorizer,
+ principal,
+ metadataNames));
+ Assertions.assertFalse(
+ evaluate(
+
AuthorizationExpressionConstants.LOAD_SCHEMA_AUTHORIZATION_EXPRESSION,
+ authorizer,
+ principal,
+ metadataNames),
+ "CREATE_SCHEMA must permit only the existence probe, not loading
schema metadata");
+
+ denied.add(Privilege.Name.CREATE_SCHEMA);
+ Assertions.assertFalse(
+ evaluate(
+
AuthorizationExpressionConstants.PROBE_SCHEMA_AUTHORIZATION_EXPRESSION,
+ authorizer,
+ principal,
+ metadataNames),
+ "A CREATE_SCHEMA deny must override the corresponding allow");
+
+ denied.clear();
+ denied.add(Privilege.Name.USE_CATALOG);
+ Assertions.assertFalse(
+ evaluate(
+
AuthorizationExpressionConstants.PROBE_SCHEMA_AUTHORIZATION_EXPRESSION,
+ authorizer,
+ principal,
+ metadataNames),
+ "A USE_CATALOG deny on the required parent path must reject the
probe");
+ }
+
+ private static boolean evaluate(
+ String expression,
+ GravitinoAuthorizer authorizer,
+ UserPrincipal principal,
+ Map<Entity.EntityType, NameIdentifier> metadataNames) {
+ return new AuthorizationExpressionEvaluator(expression, authorizer)
+ .evaluate(metadataNames, new AuthorizationRequestContext(), principal,
Optional.empty());
+ }
+
private static Map<Entity.EntityType, NameIdentifier> metadataNames(boolean
authorized) {
Map<Entity.EntityType, NameIdentifier> metadataNames = new HashMap<>();
metadataNames.put(Entity.EntityType.METALAKE,
NameIdentifierUtil.ofMetalake("testMetalake"));
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/web/filter/TestBaseMetadataAuthorizationMethodInterceptor.java
b/server-common/src/test/java/org/apache/gravitino/server/web/filter/TestBaseMetadataAuthorizationMethodInterceptor.java
new file mode 100644
index 0000000000..de3afcb917
--- /dev/null
+++
b/server-common/src/test/java/org/apache/gravitino/server/web/filter/TestBaseMetadataAuthorizationMethodInterceptor.java
@@ -0,0 +1,404 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.web.filter;
+
+import static
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants.CAN_ACCESS_METADATA;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.lang.reflect.Method;
+import java.lang.reflect.Parameter;
+import java.util.EnumMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.ActiveRoles;
+import org.apache.gravitino.authorization.AuthorizationRequestContext;
+import org.apache.gravitino.authorization.AuthorizationUtils;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
+import org.apache.gravitino.authorization.Privilege;
+import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
+import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
+import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.PrincipalUtils;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+
+/** Tests for {@link BaseMetadataAuthorizationMethodInterceptor}. */
+public class TestBaseMetadataAuthorizationMethodInterceptor {
+
+ private UserPrincipal principal;
+ private GravitinoAuthorizer authorizer;
+ private MockedStatic<PrincipalUtils> principalUtils;
+ private MockedStatic<AuthorizationUtils> authorizationUtils;
+ private MockedStatic<GravitinoAuthorizerProvider> authorizerProvider;
+
+ @BeforeEach
+ public void setUp() {
+ principal = new UserPrincipal("tester");
+ authorizer = mock(GravitinoAuthorizer.class);
+ GravitinoAuthorizerProvider provider =
mock(GravitinoAuthorizerProvider.class);
+
+ principalUtils = mockStatic(PrincipalUtils.class);
+
principalUtils.when(PrincipalUtils::getCurrentPrincipal).thenReturn(principal);
+
principalUtils.when(PrincipalUtils::getCurrentUserName).thenReturn(principal.getName());
+
+ authorizationUtils = mockStatic(AuthorizationUtils.class);
+ authorizerProvider = mockStatic(GravitinoAuthorizerProvider.class);
+
authorizerProvider.when(GravitinoAuthorizerProvider::getInstance).thenReturn(provider);
+ when(provider.getGravitinoAuthorizer()).thenReturn(authorizer);
+ when(authorizer.deny(any(), any(), any(), any(), any())).thenReturn(false);
+ when(authorizer.isOwner(any(), any(), any(), any())).thenReturn(false);
+ when(authorizer.findUnheldRoles(any(), any(), any(),
any())).thenReturn(Set.of());
+ }
+
+ @AfterEach
+ public void tearDown() {
+ authorizerProvider.close();
+ authorizationUtils.close();
+ principalUtils.close();
+ }
+
+ @Test
+ public void testDynamicTargetTypeDrivesCanAccessMetadata() throws Throwable {
+ when(authorizer.authorize(any(), any(), any(), any(), any()))
+ .thenAnswer(
+ invocation -> {
+ Privilege.Name privilege = invocation.getArgument(3);
+ return privilege == Privilege.Name.USE_CATALOG
+ || privilege == Privilege.Name.USE_SCHEMA;
+ });
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ TestInvocation invocation = invocation("canAccessMetadata", "authorized");
+
+ assertEquals("authorized", interceptor.invoke(invocation));
+ assertEquals("canAccessMetadata", interceptor.resolvedMethod.getName());
+ verify(invocation).proceed();
+ authorizationUtils.verify(
+ () ->
+ AuthorizationUtils.checkCurrentUser(
+ eq("metalake"), eq("tester"),
any(AuthorizationRequestContext.class)));
+ }
+
+ @Test
+ public void testUserValidationFailureUsesProtocolMapper() throws Throwable {
+ ForbiddenException failure = new ForbiddenException("not a metalake user");
+ authorizationUtils
+ .when(
+ () ->
+ AuthorizationUtils.checkCurrentUser(
+ eq("metalake"), eq("tester"),
any(AuthorizationRequestContext.class)))
+ .thenThrow(failure);
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ TestInvocation invocation = invocation("canAccessMetadata", "authorized");
+
+ assertSame(failure, interceptor.invoke(invocation));
+ verify(invocation, never()).proceed();
+ }
+
+ @Test
+ public void testUserValidationSystemFailureRemainsInternalError() throws
Throwable {
+ IllegalStateException failure = new IllegalStateException("user store
unavailable");
+ authorizationUtils
+ .when(
+ () ->
+ AuthorizationUtils.checkCurrentUser(
+ eq("metalake"), eq("tester"),
any(AuthorizationRequestContext.class)))
+ .thenThrow(failure);
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ TestInvocation invocation = invocation("canAccessMetadata", "authorized");
+
+ RuntimeException response =
+ assertInstanceOf(RuntimeException.class,
interceptor.invoke(invocation));
+ assertEquals("Failed to validate user", response.getMessage());
+ assertSame(failure, response.getCause());
+ verify(invocation, never()).proceed();
+ }
+
+ @Test
+ public void testExpressionDenialUsesResolvedTarget() throws Throwable {
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ TestInvocation invocation = invocation("canAccessMetadata", "authorized");
+
+ Object response = interceptor.invoke(invocation);
+
+ ForbiddenException failure = assertInstanceOf(ForbiddenException.class,
response);
+ assertTrue(failure.getMessage().contains("metalake.catalog.schema"));
+ verify(invocation, never()).proceed();
+ }
+
+ @Test
+ public void testUnheldActiveRoleUsesProtocolMapper() throws Throwable {
+ principal = principal.withActiveRoles(ActiveRoles.of(List.of("admin")));
+
principalUtils.when(PrincipalUtils::getCurrentPrincipal).thenReturn(principal);
+ when(authorizer.findUnheldRoles(any(), any(), any(),
any())).thenReturn(Set.of("admin"));
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ TestInvocation invocation = invocation("canAccessMetadata", "authorized");
+
+ Object response = interceptor.invoke(invocation);
+
+ ForbiddenException failure = assertInstanceOf(ForbiddenException.class,
response);
+ assertTrue(failure.getMessage().contains("admin"));
+ verify(invocation, never()).proceed();
+ }
+
+ @Test
+ public void testHeldActiveRoleContinuesAuthorization() throws Throwable {
+ principal = principal.withActiveRoles(ActiveRoles.of(List.of("analyst")));
+
principalUtils.when(PrincipalUtils::getCurrentPrincipal).thenReturn(principal);
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.METALAKE);
+ interceptor.skipExpressionEvaluation = true;
+ TestInvocation invocation = invocation("alwaysDenied", "authorized");
+
+ assertEquals("authorized", interceptor.invoke(invocation));
+ verify(authorizer)
+ .findUnheldRoles(
+ eq(principal),
+ eq("metalake"),
+ eq(Set.of("analyst")),
+ any(AuthorizationRequestContext.class));
+ }
+
+ @Test
+ public void testCompletedHandlerSkipsExpressionEvaluation() throws Throwable
{
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ interceptor.handler =
+ Optional.of(
+ new
BaseMetadataAuthorizationMethodInterceptor.AuthorizationHandler() {
+ @Override
+ public void process(Map<Entity.EntityType, NameIdentifier>
nameIdentifierMap) {}
+
+ @Override
+ public boolean authorizationCompleted() {
+ return true;
+ }
+ });
+ TestInvocation invocation = invocation("alwaysDenied", "authorized by
handler");
+
+ assertEquals("authorized by handler", interceptor.invoke(invocation));
+ verify(authorizer, never()).authorize(any(), any(), any(), any(), any());
+ }
+
+ @Test
+ public void testExpressionOnlySkipStillValidatesUser() throws Throwable {
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.METALAKE);
+ interceptor.skipExpressionEvaluation = true;
+ TestInvocation invocation = invocation("alwaysDenied", "root list");
+
+ assertEquals("root list", interceptor.invoke(invocation));
+ authorizationUtils.verify(
+ () ->
+ AuthorizationUtils.checkCurrentUser(
+ eq("metalake"), eq("tester"),
any(AuthorizationRequestContext.class)));
+ verify(authorizer, never()).authorize(any(), any(), any(), any(), any());
+ }
+
+ @Test
+ public void testFullSkipBypassesLocalAuthorization() throws Throwable {
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ interceptor.skipAuthorization = true;
+ TestInvocation invocation = invocation("alwaysDenied", "proxied");
+
+ assertEquals("proxied", interceptor.invoke(invocation));
+ authorizationUtils.verifyNoInteractions();
+ verify(authorizer, never()).authorize(any(), any(), any(), any(), any());
+ }
+
+ @Test
+ public void testMissingMetalakeIdentifierSkipsUserValidation() throws
Throwable {
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.METALAKE);
+ interceptor.includeMetalake = false;
+ interceptor.skipExpressionEvaluation = true;
+ TestInvocation invocation = invocation("alwaysDenied", "authorized");
+
+ assertEquals("authorized", interceptor.invoke(invocation));
+ authorizationUtils.verifyNoInteractions();
+ }
+
+ @Test
+ public void testProtocolExceptionFromHandlerIsPreserved() throws Throwable {
+ TestProtocolException failure = new TestProtocolException("bad protocol
input");
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ interceptor.handler =
+ Optional.of(
+ new
BaseMetadataAuthorizationMethodInterceptor.AuthorizationHandler() {
+ @Override
+ public void process(Map<Entity.EntityType, NameIdentifier>
nameIdentifierMap) {
+ throw failure;
+ }
+
+ @Override
+ public boolean authorizationCompleted() {
+ return false;
+ }
+ });
+ TestInvocation invocation = invocation("canAccessMetadata", "authorized");
+
+ assertSame(failure, interceptor.invoke(invocation));
+ verify(invocation, never()).proceed();
+ }
+
+ @Test
+ public void testInternalAuthorizationFailureIsWrapped() throws Throwable {
+ IllegalStateException failure = new IllegalStateException("resolver
failed");
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ interceptor.resolutionFailure = failure;
+ TestInvocation invocation = invocation("canAccessMetadata", "authorized");
+
+ RuntimeException response =
+ assertInstanceOf(RuntimeException.class,
interceptor.invoke(invocation));
+ assertSame(failure, response.getCause());
+ assertTrue(response.getMessage().contains("system internal error"));
+ verify(invocation, never()).proceed();
+ }
+
+ @Test
+ public void testOperationFailureUsesProtocolMapper() throws Throwable {
+ IllegalStateException failure = new IllegalStateException("operation
failed");
+ TestInterceptor interceptor = new
TestInterceptor(Entity.EntityType.SCHEMA);
+ TestInvocation invocation = invocation("unannotated", null);
+ when(invocation.proceed()).thenThrow(failure);
+
+ assertSame(failure, interceptor.invoke(invocation));
+ }
+
+ private static TestInvocation invocation(String methodName, Object result)
throws Throwable {
+ Method method = TestOperations.class.getDeclaredMethod(methodName);
+ TestInvocation invocation = mock(TestInvocation.class);
+ when(invocation.getMethod()).thenReturn(method);
+ when(invocation.getArguments()).thenReturn(new Object[0]);
+ when(invocation.proceed()).thenReturn(result);
+ return invocation;
+ }
+
+ private static class TestInterceptor extends
BaseMetadataAuthorizationMethodInterceptor {
+ private final Entity.EntityType targetType;
+ private Optional<AuthorizationHandler> handler = Optional.empty();
+ private Method resolvedMethod;
+ private RuntimeException resolutionFailure;
+ private boolean includeMetalake = true;
+ private boolean skipAuthorization;
+ private boolean skipExpressionEvaluation;
+
+ private TestInterceptor(Entity.EntityType targetType) {
+ this.targetType = targetType;
+ }
+
+ private Object invoke(TestInvocation invocation) throws Throwable {
+ return authorizeMethod(
+ invocation.getMethod(), invocation.getArguments(),
invocation::proceed);
+ }
+
+ @Override
+ protected AuthorizationTarget resolveAuthorizationTarget(
+ Method method, AuthorizationExpression annotation, Parameter[]
parameters, Object[] args) {
+ resolvedMethod = method;
+ if (resolutionFailure != null) {
+ throw resolutionFailure;
+ }
+ Map<Entity.EntityType, NameIdentifier> identifiers = new
EnumMap<>(Entity.EntityType.class);
+ if (includeMetalake) {
+ identifiers.put(Entity.EntityType.METALAKE,
NameIdentifierUtil.ofMetalake("metalake"));
+ }
+ identifiers.put(
+ Entity.EntityType.CATALOG, NameIdentifierUtil.ofCatalog("metalake",
"catalog"));
+ identifiers.put(
+ Entity.EntityType.SCHEMA, NameIdentifierUtil.ofSchema("metalake",
"catalog", "schema"));
+ return new AuthorizationTarget(identifiers, targetType);
+ }
+
+ @Override
+ protected Object toErrorResponse(Method method, Object[] args, Throwable
throwable) {
+ return throwable;
+ }
+
+ @Override
+ protected Optional<AuthorizationHandler> createAuthorizationHandler(
+ Method method, Parameter[] parameters, Object[] args) {
+ return handler.isPresent()
+ ? handler
+ : super.createAuthorizationHandler(method, parameters, args);
+ }
+
+ @Override
+ protected boolean isExceptionPropagate(Exception exception) {
+ return exception instanceof TestProtocolException ||
super.isExceptionPropagate(exception);
+ }
+
+ @Override
+ protected boolean shouldSkipAuthorization(
+ Map<Entity.EntityType, NameIdentifier> nameIdentifierMap) {
+ return skipAuthorization ||
super.shouldSkipAuthorization(nameIdentifierMap);
+ }
+
+ @Override
+ protected boolean shouldSkipExpressionEvaluation(AuthorizationTarget
target) {
+ return skipExpressionEvaluation ||
super.shouldSkipExpressionEvaluation(target);
+ }
+ }
+
+ private static class TestProtocolException extends RuntimeException {
+ private TestProtocolException(String message) {
+ super(message);
+ }
+ }
+
+ private interface TestInvocation {
+ Method getMethod();
+
+ Object[] getArguments();
+
+ Object proceed() throws Throwable;
+ }
+
+ private static class TestOperations {
+ @AuthorizationExpression(
+ expression = CAN_ACCESS_METADATA,
+ accessMetadataType = MetadataObject.Type.METALAKE)
+ private String canAccessMetadata() {
+ return "unused";
+ }
+
+ @AuthorizationExpression(expression = "SCHEMA::CREATE_SCHEMA")
+ private String alwaysDenied() {
+ return "unused";
+ }
+
+ private String unannotated() {
+ return "unused";
+ }
+ }
+}
