This is an automated email from the ASF dual-hosted git repository.
roryqi 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 551c23378b [#12095] feat(auth): reject assumption of unheld active
roles with 403 (#12167)
551c23378b is described below
commit 551c23378b92729a69b31c40be68b90253252526
Author: Bharath Krishna <[email protected]>
AuthorDate: Mon Aug 3 18:30:38 2026 -0700
[#12095] feat(auth): reject assumption of unheld active roles with 403
(#12167)
### What changes were proposed in this pull request?
Validate the `X-Gravitino-Active-Roles` header against the roles the
caller actually holds. A request that names an active role the caller
does **not** hold (directly or via a group) is now rejected with `403
Forbidden` before authorization, instead of the previous behavior of
silently dropping the unheld name. This validates the *declaration* only
(like SQL `SET ROLE`): you can activate only roles you were granted —
the object-level access decision is unchanged.
- `GravitinoAuthorizer`: add `findUnheldRoles(...)`, defaulting to empty
so authorizers without role membership (e.g. `PassThroughAuthorizer`
when authorization is disabled) never block role assumption.
- `JcasbinAuthorizer`: resolve the caller's held role ids (direct +
group-inherited) and return the declared names not held; fails closed if
membership can't be resolved.
- `GravitinoInterceptionService` (native REST) and
`BaseMetadataAuthorizationMethodInterceptor` (Iceberg REST): reject
`NAMED` declarations with unheld roles (`403`). `ALL`/`NONE` name
nothing and need no check.
### Why are the changes needed?
#11967 added the narrowing enforcement and #12096 wired the header onto
the principal, but a declared role the caller does not hold was silently
dropped. This completes subtask #12095 by rejecting it with `403`.
### Does this PR introduce any user-facing change?
Yes. Sending `X-Gravitino-Active-Roles` with a role the caller does not
hold now returns `403 Forbidden` (previously ignored). An absent header,
`ALL`, and `NONE` are unaffected; when authorization is disabled, role
assumption is never rejected.
### How was this patch tested?
New unit tests:
- `TestJcasbinAuthorizer`: `findUnheldRoles` — all declared roles held
(direct + group-inherited), a declared-but-unassigned role, and a
non-existent role name.
- `TestGravitinoInterceptionService` and
`TestIcebergMetadataAuthorizationMethodInterceptor`: a `NAMED`
declaration with an unheld role returns `403` and does not proceed to
the method.
Verified `test` + `spotlessCheck` for `core`, `server-common`, `server`,
and `iceberg-rest-server`.
Part of #11965.
---
.../authorization/GravitinoAuthorizer.java | 22 +++++
...BaseMetadataAuthorizationMethodInterceptor.java | 31 ++++++-
...bergMetadataAuthorizationMethodInterceptor.java | 56 +++++++++++++
.../authorization/jcasbin/JcasbinAuthorizer.java | 44 ++++++++++
.../jcasbin/TestJcasbinAuthorizer.java | 95 ++++++++++++++++++++++
.../web/filter/GravitinoInterceptionService.java | 24 ++++++
.../filter/TestGravitinoInterceptionService.java | 50 ++++++++++++
7 files changed, 318 insertions(+), 4 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/GravitinoAuthorizer.java
b/core/src/main/java/org/apache/gravitino/authorization/GravitinoAuthorizer.java
index 5b121f5abf..1b28ab2062 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/GravitinoAuthorizer.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/GravitinoAuthorizer.java
@@ -19,6 +19,7 @@ package org.apache.gravitino.authorization;
import java.io.Closeable;
import java.security.Principal;
+import java.util.Collections;
import java.util.Set;
import javax.annotation.Nullable;
import org.apache.gravitino.Entity;
@@ -139,6 +140,27 @@ public interface GravitinoAuthorizer extends Closeable {
return true;
}
+ /**
+ * Returns the declared active-role names that the given principal does not
hold (directly or via
+ * a group) in the metalake; empty when the caller holds all of them.
+ *
+ * <p>The default returns an empty set, so authorizers without role
membership never reject role
+ * assumption.
+ *
+ * @param principal the user principal
+ * @param metalake the metalake
+ * @param declaredRoleNames the active-role names the caller declared
+ * @param requestContext authorization request context
+ * @return the declared role names the caller does not hold; empty when all
are held
+ */
+ default Set<String> findUnheldRoles(
+ Principal principal,
+ String metalake,
+ Set<String> declaredRoleNames,
+ AuthorizationRequestContext requestContext) {
+ return Collections.emptySet();
+ }
+
/**
* Determine whether the user can set owner
*
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
index d3ad8f0c73..c4b7b13c52 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/server/web/filter/BaseMetadataAuthorizationMethodInterceptor.java
@@ -23,14 +23,17 @@ import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.Map;
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.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;
@@ -137,6 +140,7 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
// 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();
@@ -158,6 +162,28 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
return IcebergExceptionMapper.toRESTResponse(
new RuntimeException("Failed to validate user", ex));
}
+
+ // Role assumption: reject a NAMED declaration that names roles the
caller does not hold
+ // (403); ALL/NONE need no membership check.
+ ActiveRoles activeRoles =
authorizationRequestContext.getActiveRoles();
+ if (activeRoles.mode() == ActiveRoles.Mode.NAMED) {
+ Set<String> unheldRoles =
+ GravitinoAuthorizerProvider.getInstance()
+ .getGravitinoAuthorizer()
+ .findUnheldRoles(
+ PrincipalUtils.getCurrentPrincipal(),
+ metalakeIdent.name(),
+ activeRoles.roleNames(),
+ authorizationRequestContext);
+ if (!unheldRoles.isEmpty()) {
+ String message =
+ String.format(
+ "User '%s' cannot assume active role(s) that are not
held: %s",
+ currentUser, unheldRoles);
+ LOG.info(message);
+ return IcebergExceptionMapper.toRESTResponse(new
ForbiddenException(message));
+ }
+ }
}
// Process custom authorization if handler exists
@@ -177,10 +203,7 @@ public abstract class
BaseMetadataAuthorizationMethodInterceptor implements Meth
new AuthorizationExpressionEvaluator(expression);
boolean authorizeResult =
authorizationExpressionEvaluator.evaluate(
- nameIdentifierMap,
- pathParams,
- new AuthorizationRequestContext(),
- Optional.empty());
+ nameIdentifierMap, pathParams, authorizationRequestContext,
Optional.empty());
if (!authorizeResult) {
MetadataObject.Type type =
expressionAnnotation.accessMetadataType();
NameIdentifier accessMetadataName =
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/server/web/filter/TestIcebergMetadataAuthorizationMethodInterceptor.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/server/web/filter/TestIcebergMetadataAuthorizationMethodInterceptor.java
index cd0be14d34..c51052b3c3 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/server/web/filter/TestIcebergMetadataAuthorizationMethodInterceptor.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/server/web/filter/TestIcebergMetadataAuthorizationMethodInterceptor.java
@@ -27,6 +27,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
+import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import javax.ws.rs.core.Response;
@@ -37,15 +38,20 @@ import org.apache.gravitino.Configs;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.ActiveRoles;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.iceberg.service.CatalogWrapperForREST;
import org.apache.gravitino.iceberg.service.IcebergCatalogWrapperManager;
import org.apache.gravitino.iceberg.service.IcebergRESTUtils;
import
org.apache.gravitino.iceberg.service.authorization.IcebergRESTServerContext;
import org.apache.gravitino.iceberg.service.provider.IcebergConfigProvider;
+import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationMetadata;
import org.apache.gravitino.utils.NameIdentifierUtil;
+import org.apache.gravitino.utils.PrincipalUtils;
import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.exceptions.ForbiddenException;
@@ -55,6 +61,7 @@ import org.apache.iceberg.rest.responses.ErrorResponse;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
+import org.mockito.MockedStatic;
import org.mockito.Mockito;
/** Test for {@link IcebergMetadataAuthorizationMethodInterceptor}. */
@@ -474,6 +481,55 @@ public class
TestIcebergMetadataAuthorizationMethodInterceptor {
errorResponse.message().contains("Authorization failed due to system
internal error"));
}
+ @Test
+ public void testInvokeRejectsUnheldActiveRolesWith403() throws Throwable {
+ // Standard authorization runs (not proxying to a REST backend), so the
membership check
+ // applies.
+ resetContext(null, false);
+
+ Method method =
+ TestOperations.class.getMethod(
+ "testTableOperationWithAuthorizationExpression",
+ String.class,
+ String.class,
+ String.class);
+ MethodInvocation invocation = Mockito.mock(MethodInvocation.class);
+ Mockito.when(invocation.getMethod()).thenReturn(method);
+ Mockito.when(invocation.getArguments())
+ .thenReturn(new Object[] {TEST_CATALOG + "/", TEST_SCHEMA, "tbl"});
+
+ // The caller declares an active role via the header; the authorizer
reports it as unheld.
+ UserPrincipal principal =
+ new UserPrincipal("tester")
+
.withActiveRoles(ActiveRoles.of(Collections.singletonList("ghostRole")));
+ GravitinoAuthorizer authorizer = Mockito.mock(GravitinoAuthorizer.class);
+ Mockito.when(
+ authorizer.findUnheldRoles(
+ Mockito.any(), Mockito.eq(TEST_METALAKE), Mockito.any(),
Mockito.any()))
+ .thenReturn(Collections.singleton("ghostRole"));
+
+ try (MockedStatic<PrincipalUtils> principalUtils =
Mockito.mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> providerStatic =
+ Mockito.mockStatic(GravitinoAuthorizerProvider.class)) {
+
principalUtils.when(PrincipalUtils::getCurrentPrincipal).thenReturn(principal);
+
principalUtils.when(PrincipalUtils::getCurrentUserName).thenReturn("tester");
+ GravitinoAuthorizerProvider provider =
Mockito.mock(GravitinoAuthorizerProvider.class);
+
providerStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(provider);
+ Mockito.when(provider.getGravitinoAuthorizer()).thenReturn(authorizer);
+
+ IcebergMetadataAuthorizationMethodInterceptor interceptor =
+ new IcebergMetadataAuthorizationMethodInterceptor();
+ Object result = interceptor.invoke(invocation);
+
+ assertTrue(result instanceof Response);
+ Response response = (Response) result;
+ assertEquals(403, response.getStatus());
+ ErrorResponse errorResponse = (ErrorResponse) response.getEntity();
+ assertTrue(errorResponse.message().contains("ghostRole"));
+ Mockito.verify(invocation, Mockito.never()).proceed();
+ }
+ }
+
/** Test operations class to provide method annotations for testing. */
@SuppressWarnings("unused")
public static class TestOperations {
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
index 66583526e1..a5bd48bee7 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizer.java
@@ -484,6 +484,50 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
throw new UnsupportedOperationException("Unsupported Entity Type.");
}
+ @Override
+ public Set<String> findUnheldRoles(
+ Principal principal,
+ String metalake,
+ Set<String> declaredRoleNames,
+ AuthorizationRequestContext requestContext) {
+ if (declaredRoleNames == null || declaredRoleNames.isEmpty()) {
+ return new LinkedHashSet<>();
+ }
+ String username = principal.getName();
+ Set<String> heldRoleNames;
+ try {
+ List<String> groupNames = principalGroupNames(principal);
+ // Prime the role caches and versions that the downstream authorize()
call will reuse.
+ Optional<UserUpdatedAt> userInfoOpt =
+ prefetchUserAndGroupInfo(metalake, username, groupNames,
requestContext);
+ if (!userInfoOpt.isPresent()) {
+ // No user record => the caller holds no roles, so every declared role
is unheld.
+ return new LinkedHashSet<>(declaredRoleNames);
+ }
+ Map<Long, RoleUpdatedAt> roleVersions =
requestContext.getPrefetchedRoleVersions();
+ heldRoleNames =
+ roleVersions.values().stream()
+ .map(RoleUpdatedAt::getRoleName)
+ .collect(Collectors.toSet());
+ } catch (Exception e) {
+ // Fail closed: if membership cannot be resolved, treat every declared
role as unheld.
+ LOG.warn(
+ "Failed to resolve held roles for user {} in metalake {}; rejecting
role assumption",
+ username,
+ metalake,
+ e);
+ return new LinkedHashSet<>(declaredRoleNames);
+ }
+
+ Set<String> unheldRoles = new LinkedHashSet<>();
+ for (String roleName : declaredRoleNames) {
+ if (!heldRoleNames.contains(roleName)) {
+ unheldRoles.add(roleName);
+ }
+ }
+ return unheldRoles;
+ }
+
@Override
public boolean hasSetOwnerPermission(
String metalake, String type, String fullName,
AuthorizationRequestContext requestContext) {
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java
index 177fda2d7b..e38aa7a349 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizer.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.server.authorization.jcasbin;
import static org.apache.gravitino.authorization.Privilege.Name.SELECT_TABLE;
import static org.apache.gravitino.authorization.Privilege.Name.USE_CATALOG;
import static org.apache.gravitino.authorization.Privilege.Name.USE_SCHEMA;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -51,6 +52,7 @@ import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Function;
import java.util.stream.Collectors;
@@ -1460,6 +1462,99 @@ public class TestJcasbinAuthorizer {
getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
}
+ /** All declared active roles are held (directly), so nothing is unheld. */
+ @Test
+ public void testFindUnheldRolesEmptyWhenAllRolesHeld() throws Exception {
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ Principal principal = setCurrentPrincipalWithGroup(null);
+ RoleEntity held = mockRoleInStore(ALLOW_ROLE_ID, "heldRole",
ImmutableList.of());
+ mockDirectUserRoles(held);
+ Mockito.clearInvocations(userMetaMapper);
+ metadataIdConverterMockedStatic.clearInvocations();
+
+ AuthorizationRequestContext requestContext = new
AuthorizationRequestContext();
+ Set<String> unheld =
+ jcasbinAuthorizer.findUnheldRoles(
+ principal, METALAKE, ImmutableSet.of("heldRole"), requestContext);
+
+ assertTrue(unheld.isEmpty());
+ metadataIdConverterMockedStatic.verify(
+ () -> MetadataIdConverter.getID(any(), eq(METALAKE)), Mockito.never());
+ jcasbinAuthorizer.authorize(
+ principal,
+ METALAKE,
+ MetadataObjects.of(null, "testCatalog", MetadataObject.Type.CATALOG),
+ USE_CATALOG,
+ requestContext);
+ verify(userMetaMapper).batchGetAuthSubjectsForUser(eq(METALAKE),
eq(USERNAME), anyList());
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
+ /** A declared role that exists but is not assigned to the caller is
reported as unheld. */
+ @Test
+ public void testFindUnheldRolesReturnsRolesNotAssigned() throws Exception {
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ Principal principal = setCurrentPrincipalWithGroup(null);
+ RoleEntity held = mockRoleInStore(ALLOW_ROLE_ID, "heldRole",
ImmutableList.of());
+ mockDirectUserRoles(held);
+
+ Set<String> unheld =
+ jcasbinAuthorizer.findUnheldRoles(
+ principal,
+ METALAKE,
+ ImmutableSet.of("heldRole", "otherRole"),
+ new AuthorizationRequestContext());
+
+ assertEquals(ImmutableSet.of("otherRole"), unheld);
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
+ /** A declared role that does not exist (no id) is treated as unheld. */
+ @Test
+ public void testFindUnheldRolesTreatsNonExistentRoleAsUnheld() throws
Exception {
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ Principal principal = setCurrentPrincipalWithGroup(null);
+ RoleEntity held = mockRoleInStore(ALLOW_ROLE_ID, "heldRole",
ImmutableList.of());
+ mockDirectUserRoles(held);
+
+ Set<String> unheld =
+ jcasbinAuthorizer.findUnheldRoles(
+ principal, METALAKE, ImmutableSet.of("ghostRole"), new
AuthorizationRequestContext());
+
+ assertEquals(ImmutableSet.of("ghostRole"), unheld);
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
+ /** A group-inherited role counts as held, so it is not reported as unheld.
*/
+ @Test
+ public void testFindUnheldRolesCoversGroupInheritedRole() throws Exception {
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ UserPrincipal groupPrincipal = setCurrentPrincipalWithGroup(GROUP_NAME);
+ Long groupRoleId = 31L;
+ mockRoleInStore(groupRoleId, "groupRole", ImmutableList.of());
+ mockNoDirectUserRoles();
+ mockGroupWithRoles(GROUP_NAME, ImmutableList.of(groupRoleId),
ImmutableList.of("groupRole"));
+ Mockito.clearInvocations(userMetaMapper);
+ metadataIdConverterMockedStatic.clearInvocations();
+
+ Set<String> unheld =
+ jcasbinAuthorizer.findUnheldRoles(
+ groupPrincipal,
+ METALAKE,
+ ImmutableSet.of("groupRole"),
+ new AuthorizationRequestContext());
+
+ assertTrue(unheld.isEmpty());
+ verify(userMetaMapper).batchGetAuthSubjectsForUser(eq(METALAKE),
eq(USERNAME), anyList());
+ metadataIdConverterMockedStatic.verify(
+ () -> MetadataIdConverter.getID(any(), eq(METALAKE)), Mockito.never());
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
/**
* Sets the current principal mock to a {@link UserPrincipal} with the given
group, or with no
* groups when {@code groupName} is null. Returns the principal for use in
assertions.
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
b/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
index 82135aa463..e3e057ffc4 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/filter/GravitinoInterceptionService.java
@@ -41,11 +41,13 @@ import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
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.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import
org.apache.gravitino.listener.api.event.server.AuthorizationDenialFailureEvent;
+import org.apache.gravitino.server.authorization.GravitinoAuthorizerProvider;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationExpression;
import
org.apache.gravitino.server.authorization.annotations.AuthorizationRequest;
import
org.apache.gravitino.server.authorization.annotations.ExpressionCondition;
@@ -189,6 +191,28 @@ public class GravitinoInterceptionService implements
InterceptionService {
ex);
return Utils.internalError("Failed to validate user", ex);
}
+
+ // Role assumption: reject a NAMED declaration that names roles
the caller does not
+ // hold (403); ALL/NONE need no membership check.
+ ActiveRoles activeRoles =
authorizationRequestContext.getActiveRoles();
+ if (activeRoles.mode() == ActiveRoles.Mode.NAMED) {
+ Set<String> unheldRoles =
+ GravitinoAuthorizerProvider.getInstance()
+ .getGravitinoAuthorizer()
+ .findUnheldRoles(
+ PrincipalUtils.getCurrentPrincipal(),
+ metalakeIdent.name(),
+ activeRoles.roleNames(),
+ authorizationRequestContext);
+ if (!unheldRoles.isEmpty()) {
+ dispatchAuthzDenialEvent(currentUser, metalakeIdent,
method.getName(), expression);
+ return Utils.forbidden(
+ String.format(
+ "User '%s' cannot assume active role(s) that are not
held: %s",
+ currentUser, unheldRoles),
+ null);
+ }
+ }
}
// If expression is empty, skip authorization check (method handles
its own filtering)
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
b/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
index 3439287d8a..3a74305ce5 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/filter/TestGravitinoInterceptionService.java
@@ -39,6 +39,7 @@ import org.apache.gravitino.GravitinoEnv;
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;
@@ -121,6 +122,55 @@ public class TestGravitinoInterceptionService {
}
}
+ @Test
+ public void testRejectsUnheldActiveRolesWith403() throws Throwable {
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> mockStatic =
+ mockStatic(GravitinoAuthorizerProvider.class);
+ MockedStatic<GravitinoEnv> envMocked = mockStatic(GravitinoEnv.class);
+ MockedStatic<MetalakeManager> metalakeManagerMocked =
mockStatic(MetalakeManager.class)) {
+ // The caller declares an active role via the header; the authorizer
reports it as unheld.
+ UserPrincipal principal =
+ new UserPrincipal("tester")
+
.withActiveRoles(ActiveRoles.of(Collections.singletonList("ghostRole")));
+
principalUtilsMocked.when(PrincipalUtils::getCurrentPrincipal).thenReturn(principal);
+
principalUtilsMocked.when(PrincipalUtils::getCurrentUserName).thenReturn("tester");
+
+ MethodInvocation methodInvocation = mock(MethodInvocation.class);
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class);
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer);
+ when(authorizer.findUnheldRoles(
+ ArgumentMatchers.any(),
+ ArgumentMatchers.eq("testMetalake"),
+ ArgumentMatchers.any(),
+ ArgumentMatchers.any()))
+ .thenReturn(Collections.singleton("ghostRole"));
+
+ GravitinoEnv mockEnv = mock(GravitinoEnv.class);
+ EntityStore mockStore = mock(EntityStore.class);
+ envMocked.when(GravitinoEnv::getInstance).thenReturn(mockEnv);
+ when(mockEnv.entityStore()).thenReturn(mockStore);
+ metalakeManagerMocked
+ .when(() -> MetalakeManager.checkMetalake(ArgumentMatchers.any(),
ArgumentMatchers.any()))
+ .thenAnswer(invocation -> null);
+
+ GravitinoInterceptionService service = new
GravitinoInterceptionService();
+ Method testMethod = TestOperations.class.getMethods()[0];
+ MethodInterceptor interceptor =
service.getMethodInterceptors(testMethod).get(0);
+ when(methodInvocation.getMethod()).thenReturn(testMethod);
+ when(methodInvocation.getArguments()).thenReturn(new Object[]
{"testMetalake"});
+
+ Response response = (Response) interceptor.invoke(methodInvocation);
+
+ assertEquals(Response.Status.FORBIDDEN.getStatusCode(),
response.getStatus());
+ Assertions.assertTrue(
+ ((ErrorResponse)
response.getEntity()).getMessage().contains("ghostRole"));
+ verify(methodInvocation, never()).proceed();
+ }
+ }
+
@Test
public void testSystemInternalErrorHandling() throws Throwable {
try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
