This is an automated email from the ASF dual-hosted git repository.
bharos 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 a03467c5e5 [#11967] feat(auth): narrow allow enforcement to active
roles (deny stays global) (#12044)
a03467c5e5 is described below
commit a03467c5e552b4093f88c093068a5f4106ca7bb3
Author: Bharath Krishna <[email protected]>
AuthorDate: Sun Jul 19 21:28:00 2026 -0700
[#11967] feat(auth): narrow allow enforcement to active roles (deny stays
global) (#12044)
### What changes were proposed in this pull request?
Phase 1 role-assumption **enforcement** in `JcasbinAuthorizer`. When a
request declares a subset of the caller's roles as active, allow
policies are evaluated against only that subset; deny stays global.
- `AuthorizationRequestContext`: carry an immutable `ActiveRoles`
(defaults to `ALL`, so an unset value is byte-for-byte today's
behavior).
- `JcasbinAuthorizer`: only the allow enforcer narrows. When a subset is
declared, `enforceNarrowed`:
- applies deny **globally** first (over the caller's full role union) —
a deny on any held role always applies, keeping narrowing subtractive;
- then ORs allow across the active set = `(declared names) ∩ (caller's
effective roles)`.
- `NONE` activates no role; `ALL` / absent is unchanged.
- Ownership is deliberately not narrowed: access a caller derives from
*owning* an object comes from the ownership grant, not a role, so
`isOwner` is untouched and owners keep that access regardless of which
roles are active. This is the design's default (Option A in
`design-docs/gravitino-role-assumption.md` §5).
This is intentionally scoped to enforcement so it can be reviewed on its
own. Carrying the value from the `X-Gravitino-Active-Roles` header onto
the request and the `400`/`403` mapping follow in later PRs.
### Why are the changes needed?
Implements the enforcement step of the approved design
[`design-docs/gravitino-role-assumption.md`](https://github.com/apache/gravitino/blob/main/design-docs/gravitino-role-assumption.md)
(discussion
[#10894](https://github.com/apache/gravitino/discussions/10894)). A
caller needs authorization to be narrowed to a declared subset of its
roles; this PR is the server-side narrowing the rest of the feature
builds on.
Part of #11965. Closes #11967.
### Does this PR introduce any user-facing change?
No. The narrowing path only activates when an `ActiveRoles` subset is
set on the request; nothing populates it yet, so behavior is unchanged.
### How was this patch tested?
New unit tests in `TestJcasbinAuthorizer`: allow-narrowing to a named
role, `NONE`, deny-stays-global when the allow is narrowed,
group-inherited active role, and unheld-role-activates-nothing
(subtractive). Verified locally:
- `./gradlew :server-common:test --tests
"org.apache.gravitino.server.authorization.jcasbin.TestJcasbinAuthorizer"`
- `./gradlew :core:test --tests
"org.apache.gravitino.authorization.TestAuthorizationRequestContext"`
- `./gradlew :server-common:spotlessCheck :core:spotlessCheck`
---
.../authorization/AuthorizationRequestContext.java | 27 ++++
.../authorization/jcasbin/JcasbinAuthorizer.java | 86 +++++++++++--
.../jcasbin/TestJcasbinAuthorizer.java | 137 +++++++++++++++++++++
3 files changed, 242 insertions(+), 8 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationRequestContext.java
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationRequestContext.java
index d30f15bee1..d676f101b5 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AuthorizationRequestContext.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AuthorizationRequestContext.java
@@ -30,6 +30,7 @@ import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.auth.ActiveRoles;
import org.apache.gravitino.storage.relational.po.auth.GroupUpdatedAt;
import org.apache.gravitino.storage.relational.po.auth.OwnerInfo;
import org.apache.gravitino.storage.relational.po.auth.RoleUpdatedAt;
@@ -84,6 +85,12 @@ public class AuthorizationRequestContext {
private volatile String originalAuthorizationExpression;
+ /**
+ * The roles the caller has declared active for this request (role
assumption). Defaults to {@link
+ * ActiveRoles#all()}, which evaluates every role the caller holds (no
narrowing).
+ */
+ private volatile ActiveRoles activeRoles = ActiveRoles.all();
+
/**
* check allow
*
@@ -213,6 +220,26 @@ public class AuthorizationRequestContext {
this.prefetchedRoleVersions = prefetchedRoleVersions;
}
+ /**
+ * Returns the roles declared active for this request. Never {@code null};
defaults to {@link
+ * ActiveRoles#all()}.
+ *
+ * @return the active-role declaration
+ */
+ public ActiveRoles getActiveRoles() {
+ return activeRoles;
+ }
+
+ /**
+ * Sets the roles declared active for this request. Narrowing is
subtractive: only roles the
+ * caller actually holds are ever consulted, regardless of what is declared
here.
+ *
+ * @param activeRoles the active-role declaration; must not be {@code null}
+ */
+ public void setActiveRoles(ActiveRoles activeRoles) {
+ this.activeRoles = Objects.requireNonNull(activeRoles, "activeRoles must
not be null");
+ }
+
/**
* Composite key for {@link #allowAuthorizerCache} / {@link
#denyAuthorizerCache}. Immutable —
* mutating any field after construction would silently corrupt the {@link
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 47108b3799..ef355dea0c 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
@@ -24,6 +24,7 @@ import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.security.Principal;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
@@ -46,6 +47,7 @@ import org.apache.gravitino.MetadataObjects;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.UserGroup;
import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.ActiveRoles;
import org.apache.gravitino.auth.AuthConstants;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.AuthorizationUtils;
@@ -192,9 +194,9 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
// Initialize enforcers before caches that reference them in removal
listeners
allowEnforcer = new SyncedEnforcer(getModel("/jcasbin_model.conf"), new
GravitinoAdapter());
- allowInternalAuthorizer = new InternalAuthorizer(allowEnforcer);
+ allowInternalAuthorizer = new InternalAuthorizer(allowEnforcer, true);
denyEnforcer = new SyncedEnforcer(getModel("/jcasbin_model.conf"), new
GravitinoAdapter());
- denyInternalAuthorizer = new InternalAuthorizer(denyEnforcer);
+ denyInternalAuthorizer = new InternalAuthorizer(denyEnforcer, false);
// loadedRoles: roleId -> updated_at.
// When evicted, we must clean up the corresponding JCasbin policies.
@@ -696,8 +698,16 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
Enforcer enforcer;
- public InternalAuthorizer(Enforcer enforcer) {
+ /**
+ * When {@code true}, evaluation considers only the request's active roles
instead of every role
+ * the caller holds. Enabled for the allow authorizer so role assumption
can drop allows; the
+ * deny authorizer leaves it {@code false} so denies always apply. See
{@link #enforceNarrowed}.
+ */
+ private final boolean narrowByActiveRoles;
+
+ public InternalAuthorizer(Enforcer enforcer, boolean narrowByActiveRoles) {
this.enforcer = enforcer;
+ this.narrowByActiveRoles = narrowByActiveRoles;
}
private boolean authorizeInternal(
@@ -809,11 +819,71 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
return ownerMatchesUserOrGroups(
owner, PrincipalUtils.getCurrentPrincipal(), metalake,
requestContext);
}
- return enforcer.enforce(
- String.valueOf(userId),
- String.valueOf(metadataObject.type()),
- String.valueOf(metadataId),
- privilege);
+
+ String metadataType = String.valueOf(metadataObject.type());
+ String metadataIdStr = String.valueOf(metadataId);
+
+ // Role assumption: if the caller activated only a subset of their
roles, check this allow
+ // against just those roles (enforceNarrowed). ALL or an absent header
falls through to the
+ // normal check over every role the caller holds.
+ ActiveRoles activeRoles = requestContext.getActiveRoles();
+ if (narrowByActiveRoles && !activeRoles.isAll()) {
+ return enforceNarrowed(
+ userId, metadataType, metadataIdStr, privilege, activeRoles,
requestContext);
+ }
+
+ return enforcer.enforce(String.valueOf(userId), metadataType,
metadataIdStr, privilege);
+ }
+
+ /**
+ * Enforces one object/privilege against only the active roles the caller
actually holds, so
+ * narrowing can never grant access the caller lacks. {@link
ActiveRoles#none()} activates no
+ * role and denies every role-derived privilege.
+ *
+ * <p>Deny stays global: denyEnforcer is checked over the caller's full
role union first, so a
+ * deny on any held role still applies even when that role is inactive.
+ */
+ private boolean enforceNarrowed(
+ long userId,
+ String metadataType,
+ String metadataIdStr,
+ String privilege,
+ ActiveRoles activeRoles,
+ AuthorizationRequestContext requestContext) {
+ String userIdStr = String.valueOf(userId);
+ if (denyEnforcer.enforce(userIdStr, metadataType, metadataIdStr,
privilege)) {
+ return false;
+ }
+ if (activeRoles.isNone()) {
+ return false;
+ }
+ for (String roleId : activeRoleIds(userId, activeRoles, requestContext))
{
+ if (enforcer.enforce(roleId, metadataType, metadataIdStr, privilege)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * Returns the ids of the caller's roles whose names are in the active
set. Only roles the
+ * caller actually holds are considered, so naming a role the caller lacks
activates nothing.
+ */
+ private Set<String> activeRoleIds(
+ long userId, ActiveRoles activeRoles, AuthorizationRequestContext
requestContext) {
+ Map<Long, RoleUpdatedAt> roleVersions =
requestContext.getPrefetchedRoleVersions();
+ if (roleVersions == null || roleVersions.isEmpty()) {
+ return Collections.emptySet();
+ }
+ Set<String> activeNames = activeRoles.roleNames();
+ Set<String> result = new HashSet<>();
+ for (String roleId : enforcer.getRolesForUser(String.valueOf(userId))) {
+ RoleUpdatedAt roleInfo = roleVersions.get(Long.parseLong(roleId));
+ if (roleInfo != null && activeNames.contains(roleInfo.getRoleName())) {
+ result.add(roleId);
+ }
+ }
+ return result;
}
}
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 d13437d5d9..7eb04c08cb 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
@@ -64,6 +64,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.SupportsRelationOperations;
import org.apache.gravitino.UserGroup;
import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.auth.ActiveRoles;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.authorization.SecurableObject;
@@ -1294,6 +1295,131 @@ public class TestJcasbinAuthorizer {
getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
}
+ /**
+ * Role assumption narrows allows to the active subset. The caller holds a
granting and a
+ * non-granting role; the per-assertion comments below cover each case.
+ */
+ @Test
+ public void testActiveRolesNarrowAllowToNamedRole() throws Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ Principal currentPrincipal = setCurrentPrincipalWithGroup(null);
+
+ RoleEntity grantingRole =
+ mockRoleInStore(ALLOW_ROLE_ID, "grantingRole",
ImmutableList.of(getAllowSecurableObject()));
+ RoleEntity nonGrantingRole = mockRoleInStore(30L, "nonGrantingRole",
ImmutableList.of());
+ mockDirectUserRoles(grantingRole, nonGrantingRole);
+
+ // Default (ALL) evaluates every role -> allowed.
+ assertTrue(doAuthorizeWithActiveRoles(currentPrincipal,
ActiveRoles.all()));
+
+ // Narrowing to the non-granting role removes the granting role's allow ->
denied.
+ assertFalse(
+ doAuthorizeWithActiveRoles(
+ currentPrincipal,
ActiveRoles.of(ImmutableList.of("nonGrantingRole"))));
+
+ // Narrowing to the granting role keeps the allow -> allowed.
+ assertTrue(
+ doAuthorizeWithActiveRoles(
+ currentPrincipal,
ActiveRoles.of(ImmutableList.of("grantingRole"))));
+
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
+ /** {@code NONE} activates no role, so every role-derived allow is dropped.
*/
+ @Test
+ public void testActiveRolesNoneDeniesRoleDerivedAccess() throws Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ Principal currentPrincipal = setCurrentPrincipalWithGroup(null);
+
+ RoleEntity grantingRole =
+ mockRoleInStore(
+ ALLOW_ROLE_ID, "noneGrantingRole",
ImmutableList.of(getAllowSecurableObject()));
+ mockDirectUserRoles(grantingRole);
+
+ assertTrue(doAuthorizeWithActiveRoles(currentPrincipal,
ActiveRoles.all()));
+ assertFalse(doAuthorizeWithActiveRoles(currentPrincipal,
ActiveRoles.none()));
+
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
+ /**
+ * Deny stays global: a deny carried by a role that is <em>not</em> in the
active set still blocks
+ * access. Here only the granting role is active, but the caller also holds
a deny role, so the
+ * request is denied.
+ */
+ @Test
+ public void testActiveRolesDenyStaysGlobalWhenAllowNarrowed() throws
Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ Principal currentPrincipal = setCurrentPrincipalWithGroup(null);
+
+ RoleEntity grantingRole =
+ mockRoleInStore(
+ ALLOW_ROLE_ID, "denyGlobalAllowRole",
ImmutableList.of(getAllowSecurableObject()));
+ RoleEntity denyRole =
+ mockRoleInStore(
+ DENY_ROLE_ID, "denyGlobalDenyRole",
ImmutableList.of(getDenySecurableObject()));
+ mockDirectUserRoles(grantingRole, denyRole);
+
+ assertFalse(
+ doAuthorizeWithActiveRoles(
+ currentPrincipal,
ActiveRoles.of(ImmutableList.of("denyGlobalAllowRole"))));
+
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
+ /** A group-inherited role can be activated by name exactly like a
directly-granted role. */
+ @Test
+ public void testActiveRolesNarrowingCoversGroupInheritedRole() throws
Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+
+ UserPrincipal groupPrincipal = setCurrentPrincipalWithGroup(GROUP_NAME);
+
+ Long groupRoleId = 31L;
+ RoleEntity groupRole =
+ mockRoleInStore(
+ groupRoleId, "activeGroupRole",
ImmutableList.of(getAllowSecurableObject()));
+ mockNoDirectUserRoles();
+ mockGroupWithRoles(
+ GROUP_NAME, ImmutableList.of(groupRoleId),
ImmutableList.of(groupRole.name()));
+
+ assertTrue(
+ doAuthorizeWithActiveRoles(
+ groupPrincipal,
ActiveRoles.of(ImmutableList.of("activeGroupRole"))));
+
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
+ /**
+ * Narrowing is subtractive: naming only a role the caller does not hold
activates nothing. The
+ * {@code 403} rejection for unheld roles happens earlier in the request
pipeline, not here.
+ */
+ @Test
+ public void testActiveRolesUnheldRoleActivatesNothing() throws Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ Principal currentPrincipal = setCurrentPrincipalWithGroup(null);
+
+ RoleEntity grantingRole =
+ mockRoleInStore(
+ ALLOW_ROLE_ID, "heldGrantingRole",
ImmutableList.of(getAllowSecurableObject()));
+ mockDirectUserRoles(grantingRole);
+
+ assertFalse(
+ doAuthorizeWithActiveRoles(
+ currentPrincipal,
ActiveRoles.of(ImmutableList.of("roleTheUserDoesNotHold"))));
+
+ 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.
@@ -1407,6 +1533,17 @@ public class TestJcasbinAuthorizer {
new AuthorizationRequestContext());
}
+ private Boolean doAuthorizeWithActiveRoles(Principal currentPrincipal,
ActiveRoles activeRoles) {
+ AuthorizationRequestContext requestContext = new
AuthorizationRequestContext();
+ requestContext.setActiveRoles(activeRoles);
+ return jcasbinAuthorizer.authorize(
+ currentPrincipal,
+ "testMetalake",
+ MetadataObjects.of(null, "testCatalog", MetadataObject.Type.CATALOG),
+ USE_CATALOG,
+ requestContext);
+ }
+
private Boolean doAuthorizeOwner(Principal currentPrincipal) {
AuthorizationRequestContext authorizationRequestContext = new
AuthorizationRequestContext();
return jcasbinAuthorizer.isOwner(
