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 9696477781 [#11775] improvement(authz): Short-circuit list
authorization via parent-scope grant (#11778)
9696477781 is described below
commit 96964777819dc92a60f58cfe2fd400dc5f9d6108
Author: Qi Yu <[email protected]>
AuthorDate: Wed Jul 1 09:12:56 2026 +0800
[#11775] improvement(authz): Short-circuit list authorization via
parent-scope grant (#11778)
### What changes were proposed in this pull request?
When authorization is enabled, list operations evaluated one per-object
permission check per result, producing an N+1 explosion. This PR
evaluates the **ancestor-only portion** of the filter expression
**once** per list request: if a privilege is granted (or the object is
owned) at a parent scope (metalake/catalog/schema), every child is
visible unless an object-level deny overrides it.
- New parent-scope expressions in `AuthorizationExpressionConstants` for
`TABLE`/`SCHEMA`/`CATALOG`.
- `MetadataAuthzHelper.filterByExpression(…, NameIdentifier[])` attempts
the short-circuit before the per-object loop. One insertion point covers
both Gravitino REST (`listTables`/`listSchemas`/`listCatalogs`) and
Iceberg REST (`listTable`/`listNamespace`), since they all route through
this helper.
- New SPI method `GravitinoAuthorizer#hasDenyPolicyOnType` (conservative
default `true`); `JcasbinAuthorizer` implements it as an in-memory scan
of the current user's deny-enforcer policies. The short-circuit is taken
only when no object-level deny may exist for the relevant
type/privileges, otherwise it falls back to the existing per-object
filtering.
### Why are the changes needed?
Per #11775, with authorization on, listing a schema of ~10,496 tables
took ~134s (vs ~4s with auth off) — a ~70x regression — because
permission was checked once per table. Owner/grant at a parent scope
makes every child visible (OR semantics); only an object-level deny can
subtract, so the ancestor part is a loop-invariant that can be lifted
out of the per-object loop. Ancestor-level denies are already handled
inside the parent-scope expression's `!ANY(DENY_..., METALAKE, CATALOG,
SCHEMA)` terms.
Fix: #11775
### Does this PR introduce _any_ user-facing change?
No. The new method on the `GravitinoAuthorizer` SPI is a
backward-compatible default method (default returns `true`, i.e. no
short-circuit), and authorization decisions are unchanged.
### How was this patch tested?
New unit tests:
- `TestMetadataAuthzHelper`: parent grant + no deny returns the whole
list; parent grant + possible deny falls back and excludes the denied
table; schema list short-circuits via a catalog-level grant; catalog
list short-circuits via a metalake-level grant.
- `TestJcasbinAuthorizer#testHasDenyPolicyOnType`: detects an
object-level deny for the matching type/privilege and ignores
non-matching type/privilege.
Full `:server-common` test suite passes; `:server`,
`:iceberg:iceberg-rest-server`, and `:core` compile clean.
---
.../authorization/GravitinoAuthorizer.java | 32 +++
.../server/authorization/MetadataAuthzHelper.java | 92 ++++++
.../AuthorizationExpressionConstants.java | 42 +++
.../authorization/jcasbin/JcasbinAuthorizer.java | 55 ++++
.../authorization/TestMetadataAuthzHelper.java | 314 +++++++++++++++++++++
.../jcasbin/TestJcasbinAuthorizer.java | 65 +++++
6 files changed, 600 insertions(+)
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 f072c15684..beb9a50c41 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.Set;
import javax.annotation.Nullable;
import org.apache.gravitino.Entity;
import org.apache.gravitino.GravitinoEnv;
@@ -107,6 +108,37 @@ public interface GravitinoAuthorizer extends Closeable {
*/
boolean isMetalakeUser(String metalake, AuthorizationRequestContext
requestContext);
+ /**
+ * Determines whether the given principal may have any {@code DENY} policy,
for any of the given
+ * privileges, at any scope (metalake, catalog, schema or the object itself)
within the metalake.
+ *
+ * <p>This supports list-authorization short-circuiting: when a privilege is
granted at a parent
+ * scope (metalake, catalog or schema), every child object is visible
<em>unless</em> a {@code
+ * DENY} overrides it at some scope. A {@code false} return therefore lets
the caller skip
+ * per-object authorization for the whole list and return every identifier;
a {@code true} return
+ * forces the caller to fall back to per-object authorization.
+ *
+ * <p>The query is scope-agnostic on purpose: a deny granted at a parent
scope hides the whole
+ * subtree, while a deny granted on a single object hides just that object.
Both cases must
+ * disable the short-circuit, so the privilege match is not restricted to a
metadata type.
+ *
+ * <p>The default implementation conservatively returns {@code true} so that
authorizers which
+ * cannot answer the question never enable an unsafe short-circuit.
+ *
+ * @param principal the user principal
+ * @param metalake the metalake
+ * @param privileges the privileges whose denies would affect visibility
+ * @param requestContext authorization request context
+ * @return whether a deny for any of the given privileges may exist
+ */
+ default boolean hasDenyPolicy(
+ Principal principal,
+ String metalake,
+ Set<Privilege.Name> privileges,
+ AuthorizationRequestContext requestContext) {
+ return true;
+ }
+
/**
* Determine whether the user can set owner
*
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
index b63251fd96..b71fb460ba 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/MetadataAuthzHelper.java
@@ -26,6 +26,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.CompletableFuture;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
@@ -42,6 +43,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.SupportsRelationOperations;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
import org.apache.gravitino.authorization.GravitinoAuthorizer;
+import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.dto.tag.MetadataObjectDTO;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants;
import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionEvaluator;
@@ -87,6 +89,43 @@ public class MetadataAuthzHelper {
private static final List<Entity.EntityType> REQUIRE_SCHEMA_EXISTS =
Arrays.asList(Entity.EntityType.TABLE, Entity.EntityType.TOPIC);
+ /**
+ * Registry of list-authorization short-circuits keyed by the listed
object's entity type. Each
+ * entry pairs the per-object filter expression it applies to with the
parent-scope expression to
+ * evaluate once and the privileges whose object-level denies would defeat
the short-circuit.
+ */
+ private static final Map<Entity.EntityType, ListShortCircuit>
LIST_SHORT_CIRCUITS =
+ Map.of(
+ Entity.EntityType.TABLE,
+ new ListShortCircuit(
+
AuthorizationExpressionConstants.FILTER_TABLE_AUTHORIZATION_EXPRESSION,
+
AuthorizationExpressionConstants.TABLE_LIST_PARENT_SCOPE_AUTHORIZATION_EXPRESSION,
+ Set.of(Privilege.Name.SELECT_TABLE,
Privilege.Name.MODIFY_TABLE)),
+ Entity.EntityType.SCHEMA,
+ new ListShortCircuit(
+
AuthorizationExpressionConstants.FILTER_SCHEMA_AUTHORIZATION_EXPRESSION,
+
AuthorizationExpressionConstants.SCHEMA_LIST_PARENT_SCOPE_AUTHORIZATION_EXPRESSION,
+ Set.of(Privilege.Name.USE_SCHEMA)),
+ Entity.EntityType.CATALOG,
+ new ListShortCircuit(
+
AuthorizationExpressionConstants.LOAD_CATALOG_AUTHORIZATION_EXPRESSION,
+
AuthorizationExpressionConstants.CATALOG_LIST_PARENT_SCOPE_AUTHORIZATION_EXPRESSION,
+ Set.of(Privilege.Name.USE_CATALOG)));
+
+ /** Immutable description of a single list-authorization short-circuit. */
+ private static final class ListShortCircuit {
+ private final String filterExpression;
+ private final String parentScopeExpression;
+ private final Set<Privilege.Name> denyPrivileges;
+
+ private ListShortCircuit(
+ String filterExpression, String parentScopeExpression,
Set<Privilege.Name> denyPrivileges) {
+ this.filterExpression = filterExpression;
+ this.parentScopeExpression = parentScopeExpression;
+ this.denyPrivileges = denyPrivileges;
+ }
+ }
+
private MetadataAuthzHelper() {}
public static Metalake[] filterMetalakes(Metalake[] metalakes, String
expression) {
@@ -167,11 +206,64 @@ public class MetadataAuthzHelper {
String expression,
Entity.EntityType entityType,
NameIdentifier[] nameIdentifiers) {
+ if (enableAuthorization()
+ && nameIdentifiers.length > 0
+ && allVisibleViaParentScope(metalake, expression, entityType,
nameIdentifiers)) {
+ // A privilege granted at a parent scope (metalake/catalog/schema) makes
every object in the
+ // list visible, and no object-level deny exists, so the per-object
authorization loop is
+ // skipped entirely. See
AuthorizationExpressionConstants.*_LIST_PARENT_SCOPE_*.
+ return nameIdentifiers;
+ }
preloadToCache(entityType, nameIdentifiers);
preloadOwner(entityType, nameIdentifiers);
return filterByExpression(metalake, expression, entityType,
nameIdentifiers, e -> e);
}
+ /**
+ * Attempts the list-authorization short-circuit: when the listed objects
all share one parent and
+ * the matching filter expression is granted at a parent scope, the whole
list is visible unless
+ * an object-level deny may exist. Returns {@code true} only when it is safe
to return every
+ * identifier without per-object authorization.
+ */
+ private static boolean allVisibleViaParentScope(
+ String metalake,
+ String expression,
+ Entity.EntityType entityType,
+ NameIdentifier[] nameIdentifiers) {
+ ListShortCircuit spec = LIST_SHORT_CIRCUITS.get(entityType);
+ if (spec == null || !spec.filterExpression.equals(expression)) {
+ return false;
+ }
+
+ // The short-circuit reasons about a single parent scope, so every
identifier must share it.
+ Namespace parent = nameIdentifiers[0].namespace();
+ for (NameIdentifier ident : nameIdentifiers) {
+ if (!ident.namespace().equals(parent)) {
+ return false;
+ }
+ }
+
+ Principal principal = PrincipalUtils.getCurrentPrincipal();
+ GravitinoAuthorizer authorizer =
+ GravitinoAuthorizerProvider.getInstance().getGravitinoAuthorizer();
+ AuthorizationRequestContext requestContext = new
AuthorizationRequestContext();
+
requestContext.setOriginalAuthorizationExpression(spec.parentScopeExpression);
+ Map<Entity.EntityType, NameIdentifier> metadataNames =
+ NameIdentifierUtil.splitNameIdentifier(metalake, entityType,
nameIdentifiers[0]);
+
+ boolean parentGrantsAccess =
+ new AuthorizationExpressionEvaluator(spec.parentScopeExpression,
authorizer)
+ .evaluate(metadataNames, requestContext, principal,
Optional.empty());
+ if (!parentGrantsAccess) {
+ return false;
+ }
+
+ // Parent scope grants access to every object; the only thing that can
still hide one is a
+ // deny on these privileges (at the parent scope or on an individual
object), so the
+ // short-circuit is only safe when no such deny may exist.
+ return !authorizer.hasDenyPolicy(principal, metalake, spec.denyPrivileges,
requestContext);
+ }
+
/**
* Call {@link AuthorizationExpressionEvaluator} to check access
*
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 122bdfcd01..c8fd3d07d1 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
@@ -166,6 +166,48 @@ public class AuthorizationExpressionConstants {
ANY_MODIFY_TABLE
""";
+ /**
+ * Parent-scope expression used to short-circuit table-list authorization.
It is the ancestor-only
+ * portion of {@link #FILTER_TABLE_AUTHORIZATION_EXPRESSION}: ownership of,
or a SELECT/MODIFY
+ * grant at, the metalake/catalog/schema scope means every table in the
schema is visible.
+ * Evaluated once per list request instead of once per table.
+ *
+ * <p>This expression only checks the positive parent-scope grant; deny
coverage (at any scope) is
+ * handled separately by {@link
+ * org.apache.gravitino.authorization.GravitinoAuthorizer#hasDenyPolicy} so
the short-circuit
+ * falls back to per-object authorization whenever a relevant deny may exist.
+ */
+ public static final String TABLE_LIST_PARENT_SCOPE_AUTHORIZATION_EXPRESSION =
+ """
+ ANY(OWNER, METALAKE, CATALOG, SCHEMA) ||
+ ANY(SELECT_TABLE, METALAKE, CATALOG, SCHEMA) ||
+ ANY(MODIFY_TABLE, METALAKE, CATALOG, SCHEMA)
+ """;
+
+ /**
+ * Parent-scope expression used to short-circuit schema-list authorization.
Ancestor-only portion
+ * of {@link #FILTER_SCHEMA_AUTHORIZATION_EXPRESSION}: ownership of, or a
USE_SCHEMA grant at, the
+ * metalake/catalog scope means every schema in the catalog is visible. Deny
coverage is handled
+ * by {@link
org.apache.gravitino.authorization.GravitinoAuthorizer#hasDenyPolicy}.
+ */
+ public static final String SCHEMA_LIST_PARENT_SCOPE_AUTHORIZATION_EXPRESSION
=
+ """
+ ANY(OWNER, METALAKE, CATALOG) ||
+ ANY(USE_SCHEMA, METALAKE, CATALOG)
+ """;
+
+ /**
+ * Parent-scope expression used to short-circuit catalog-list authorization.
Ancestor-only portion
+ * of {@link #LOAD_CATALOG_AUTHORIZATION_EXPRESSION}: metalake ownership, or
a USE_CATALOG grant
+ * at the metalake scope, means every catalog is visible. Deny coverage is
handled by {@link
+ * org.apache.gravitino.authorization.GravitinoAuthorizer#hasDenyPolicy}.
+ */
+ public static final String
CATALOG_LIST_PARENT_SCOPE_AUTHORIZATION_EXPRESSION =
+ """
+ METALAKE::OWNER ||
+ ANY(USE_CATALOG, METALAKE)
+ """;
+
public static final String FILTER_VIEW_AUTHORIZATION_EXPRESSION =
"""
ANY(OWNER, METALAKE, CATALOG, SCHEMA, VIEW) ||
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 347900e606..231f092bbf 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
@@ -115,6 +115,16 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
private static final Logger LOG =
LoggerFactory.getLogger(JcasbinAuthorizer.class);
+ /**
+ * Field index of {@code sub} (the role/user/group id) in a jcasbin {@code
p} policy row. See the
+ * {@code policy_definition} in {@code jcasbin_model.conf}: {@code p = sub,
metadataType,
+ * metadataId, act, eft}.
+ */
+ private static final int POLICY_SUBJECT_FIELD_INDEX = 0;
+
+ /** Field index of {@code act} (the privilege) in a jcasbin {@code p} policy
row. */
+ private static final int POLICY_ACTION_FIELD_INDEX = 3;
+
/** Jcasbin enforcer is used for metadata authorization. */
private Enforcer allowEnforcer;
@@ -363,6 +373,51 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
return loadUserInfo(metalake, currentUserName, requestContext).isPresent();
}
+ @Override
+ public boolean hasDenyPolicy(
+ Principal principal,
+ String metalake,
+ Set<Privilege.Name> privileges,
+ AuthorizationRequestContext requestContext) {
+ Optional<UserUpdatedAt> userInfoOpt =
+ loadUserInfo(metalake, principal.getName(), requestContext);
+ if (!userInfoOpt.isPresent()) {
+ // An unknown user holds no roles and therefore no deny policies.
+ return false;
+ }
+ UserUpdatedAt userInfo = userInfoOpt.get();
+ long userId = userInfo.getUserId();
+ // Bind the user's (direct + group-inherited) roles into the enforcers so
the in-memory scan
+ // below sees every policy the user can carry. Idempotent within a request.
+ loadRolePrivilege(metalake, principal.getName(), userId, userInfo,
requestContext);
+
+ Set<String> privilegeNames =
privileges.stream().map(Enum::name).collect(Collectors.toSet());
+ String userIdStr = String.valueOf(userId);
+ // This is an existence query, not a per-object check: it answers "does
any deny on these
+ // privileges exist for the user's roles, at any scope?" The standard
enforce path needs a
+ // concrete metadataId, so reusing it would mean iterating every listed
object and defeat the
+ // short-circuit. Filtering the deny enforcer's policies by role keeps the
scan bounded by the
+ // user's role/policy count, never by the number of listed objects. The
match is intentionally
+ // scope-agnostic (no metadataType filter): a parent-scope deny hides the
whole subtree and an
+ // object-scope deny hides one object, and both must disable the
short-circuit.
+ for (String roleId : denyEnforcer.getRolesForUser(userIdStr)) {
+ // getFilteredNamedPolicy returns every "p" row (p = sub, metadataType,
metadataId, act, eft)
+ // whose field at POLICY_SUBJECT_FIELD_INDEX (sub) equals roleId, i.e.
all rules carried by
+ // this role. denyEnforcer is a dedicated enforcer that is only ever
loaded with privileges
+ // whose condition is DENY (see loadPolicyByRoleEntity), so every row
here represents a deny
+ // regardless of its stored eft string. Each returned row is the list of
those five fields,
+ // so we read field POLICY_ACTION_FIELD_INDEX (act) to compare the
denied privilege.
+ for (List<String> policy :
+ denyEnforcer.getFilteredNamedPolicy("p", POLICY_SUBJECT_FIELD_INDEX,
roleId)) {
+ if (policy.size() > POLICY_ACTION_FIELD_INDEX
+ && privilegeNames.contains(policy.get(POLICY_ACTION_FIELD_INDEX)))
{
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
@Override
public boolean isSelf(
Entity.EntityType type,
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataAuthzHelper.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataAuthzHelper.java
index 81e7d858ca..a5019085cb 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataAuthzHelper.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/TestMetadataAuthzHelper.java
@@ -18,12 +18,15 @@
package org.apache.gravitino.server.authorization;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anySet;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.when;
import java.lang.reflect.Field;
+import java.util.Arrays;
import java.util.concurrent.Executor;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
@@ -32,7 +35,10 @@ import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.UserPrincipal;
+import org.apache.gravitino.authorization.GravitinoAuthorizer;
+import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.dto.tag.MetadataObjectDTO;
+import
org.apache.gravitino.server.authorization.expression.AuthorizationExpressionConstants;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.PrincipalUtils;
import org.junit.jupiter.api.AfterAll;
@@ -190,6 +196,314 @@ public class TestMetadataAuthzHelper {
}
}
+ /**
+ * Builds three table identifiers under the same schema, where a parent
(schema) level
+ * SELECT_TABLE grant exists and the middle table additionally carries a
table-level deny. The
+ * deny gate is controlled separately so each branch of the list
short-circuit can be exercised.
+ */
+ private GravitinoAuthorizer mockTableListAuthorizer(boolean owner, boolean
denyGate) {
+ GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class);
+ // Parent-scope SELECT_TABLE grant at the schema level (applies to every
table in the schema).
+ lenient()
+ .when(authorizer.authorize(any(), eq("testMetalake"), any(), any(),
any()))
+ .thenAnswer(
+ invocation -> {
+ MetadataObject object = invocation.getArgument(2);
+ Privilege.Name privilege = invocation.getArgument(3);
+ return object.type() == MetadataObject.Type.SCHEMA
+ && privilege == Privilege.Name.SELECT_TABLE;
+ });
+ // The middle table (t2) has a table-level deny on SELECT_TABLE.
+ lenient()
+ .when(authorizer.deny(any(), eq("testMetalake"), any(), any(), any()))
+ .thenAnswer(
+ invocation -> {
+ MetadataObject object = invocation.getArgument(2);
+ Privilege.Name privilege = invocation.getArgument(3);
+ return object.type() == MetadataObject.Type.TABLE
+ && "t2".equals(object.name())
+ && privilege == Privilege.Name.SELECT_TABLE;
+ });
+ lenient().when(authorizer.isOwner(any(), eq("testMetalake"), any(),
any())).thenReturn(owner);
+ lenient()
+ .when(authorizer.hasDenyPolicy(any(), eq("testMetalake"), anySet(),
any()))
+ .thenReturn(denyGate);
+ return authorizer;
+ }
+
+ /**
+ * Builds an authorizer that grants {@code grantPrivilege} at a single
ancestor scope ({@code
+ * grantType}) and reports no object-level deny, so a list of children under
that ancestor is
+ * fully visible via the parent-scope short-circuit.
+ */
+ private GravitinoAuthorizer mockParentGrantAuthorizer(
+ MetadataObject.Type grantType, Privilege.Name grantPrivilege) {
+ GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class);
+ lenient()
+ .when(authorizer.authorize(any(), eq("testMetalake"), any(), any(),
any()))
+ .thenAnswer(
+ invocation -> {
+ MetadataObject object = invocation.getArgument(2);
+ Privilege.Name privilege = invocation.getArgument(3);
+ return object.type() == grantType && privilege == grantPrivilege;
+ });
+ lenient()
+ .when(authorizer.deny(any(), eq("testMetalake"), any(), any(), any()))
+ .thenReturn(false);
+ lenient().when(authorizer.isOwner(any(), eq("testMetalake"), any(),
any())).thenReturn(false);
+ lenient()
+ .when(authorizer.hasDenyPolicy(any(), eq("testMetalake"), anySet(),
any()))
+ .thenReturn(false);
+ return authorizer;
+ }
+
+ @Test
+ public void testListShortCircuitSchemaViaCatalogGrant() {
+ makeCompletableFutureUseCurrentThread();
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> mockStatic =
+ mockStatic(GravitinoAuthorizerProvider.class)) {
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+ principalUtilsMocked.when(() -> PrincipalUtils.doAs(any(),
any())).thenCallRealMethod();
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ GravitinoAuthorizer authorizer =
+ mockParentGrantAuthorizer(MetadataObject.Type.CATALOG,
Privilege.Name.USE_SCHEMA);
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer);
+
+ NameIdentifier[] schemas =
+ new NameIdentifier[] {
+ NameIdentifierUtil.ofSchema("testMetalake", "testCatalog", "s1"),
+ NameIdentifierUtil.ofSchema("testMetalake", "testCatalog", "s2")
+ };
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake",
+
AuthorizationExpressionConstants.FILTER_SCHEMA_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.SCHEMA,
+ schemas);
+
+ Assertions.assertEquals(2, filtered.length);
+ }
+ }
+
+ @Test
+ public void testListShortCircuitCatalogViaMetalakeGrant() {
+ makeCompletableFutureUseCurrentThread();
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> mockStatic =
+ mockStatic(GravitinoAuthorizerProvider.class)) {
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+ principalUtilsMocked.when(() -> PrincipalUtils.doAs(any(),
any())).thenCallRealMethod();
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ GravitinoAuthorizer authorizer =
+ mockParentGrantAuthorizer(MetadataObject.Type.METALAKE,
Privilege.Name.USE_CATALOG);
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer);
+
+ NameIdentifier[] catalogs =
+ new NameIdentifier[] {
+ NameIdentifierUtil.ofCatalog("testMetalake", "c1"),
+ NameIdentifierUtil.ofCatalog("testMetalake", "c2")
+ };
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake",
+
AuthorizationExpressionConstants.LOAD_CATALOG_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.CATALOG,
+ catalogs);
+
+ Assertions.assertEquals(2, filtered.length);
+ }
+ }
+
+ /**
+ * Builds an authorizer that grants {@code SELECT_TABLE} at the schema
scope, but only for the
+ * schema whose simple name equals {@code grantedSchema}. Used to prove the
short-circuit never
+ * applies one parent's grant to siblings under a different parent.
+ */
+ private GravitinoAuthorizer mockSchemaScopedTableAuthorizer(String
grantedSchema) {
+ GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class);
+ lenient()
+ .when(authorizer.authorize(any(), eq("testMetalake"), any(), any(),
any()))
+ .thenAnswer(
+ invocation -> {
+ MetadataObject object = invocation.getArgument(2);
+ Privilege.Name privilege = invocation.getArgument(3);
+ return object.type() == MetadataObject.Type.SCHEMA
+ && grantedSchema.equals(object.name())
+ && privilege == Privilege.Name.SELECT_TABLE;
+ });
+ lenient()
+ .when(authorizer.deny(any(), eq("testMetalake"), any(), any(), any()))
+ .thenReturn(false);
+ lenient().when(authorizer.isOwner(any(), eq("testMetalake"), any(),
any())).thenReturn(false);
+ lenient()
+ .when(authorizer.hasDenyPolicy(any(), eq("testMetalake"), anySet(),
any()))
+ .thenReturn(false);
+ return authorizer;
+ }
+
+ /**
+ * Builds an authorizer that grants no parent-scope privilege at all and
only owns a single table
+ * ({@code ownedTable}). No ancestor grant means the short-circuit must not
trigger, falling back
+ * to per-object filtering that exposes only the owned table.
+ */
+ private GravitinoAuthorizer mockTableOwnerOnlyAuthorizer(String ownedTable) {
+ GravitinoAuthorizer authorizer = mock(GravitinoAuthorizer.class);
+ lenient()
+ .when(authorizer.authorize(any(), eq("testMetalake"), any(), any(),
any()))
+ .thenReturn(false);
+ lenient()
+ .when(authorizer.deny(any(), eq("testMetalake"), any(), any(), any()))
+ .thenReturn(false);
+ lenient()
+ .when(authorizer.isOwner(any(), eq("testMetalake"), any(), any()))
+ .thenAnswer(
+ invocation -> {
+ MetadataObject object = invocation.getArgument(2);
+ return object.type() == MetadataObject.Type.TABLE &&
ownedTable.equals(object.name());
+ });
+ lenient()
+ .when(authorizer.hasDenyPolicy(any(), eq("testMetalake"), anySet(),
any()))
+ .thenReturn(false);
+ return authorizer;
+ }
+
+ @Test
+ public void testListShortCircuitDoesNotLeakAcrossParents() {
+ makeCompletableFutureUseCurrentThread();
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> mockStatic =
+ mockStatic(GravitinoAuthorizerProvider.class)) {
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+ principalUtilsMocked.when(() -> PrincipalUtils.doAs(any(),
any())).thenCallRealMethod();
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ // Grant SELECT_TABLE only at schema s1; t2 lives under a different
schema s2.
+ GravitinoAuthorizer authorizer = mockSchemaScopedTableAuthorizer("s1");
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer);
+
+ NameIdentifier[] tables =
+ new NameIdentifier[] {
+ NameIdentifierUtil.ofTable("testMetalake", "testCatalog", "s1",
"t1"),
+ NameIdentifierUtil.ofTable("testMetalake", "testCatalog", "s2",
"t2")
+ };
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake",
+
AuthorizationExpressionConstants.FILTER_TABLE_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.TABLE,
+ tables);
+
+ // Identifiers span two schemas, so the single-parent short-circuit must
not fire. Per-object
+ // filtering keeps only t1 (granted via s1) and never leaks t2 from the
ungranted schema s2.
+ Assertions.assertEquals(1, filtered.length);
+ Assertions.assertEquals("t1", filtered[0].name());
+ }
+ }
+
+ @Test
+ public void testListNoParentGrantFallsBackToPerObject() {
+ makeCompletableFutureUseCurrentThread();
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> mockStatic =
+ mockStatic(GravitinoAuthorizerProvider.class)) {
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+ principalUtilsMocked.when(() -> PrincipalUtils.doAs(any(),
any())).thenCallRealMethod();
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ GravitinoAuthorizer authorizer = mockTableOwnerOnlyAuthorizer("t1");
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer);
+
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake",
+
AuthorizationExpressionConstants.FILTER_TABLE_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.TABLE,
+ threeTables());
+
+ // No ancestor grant exists, so the short-circuit is skipped and
per-object filtering returns
+ // only the single table the user owns.
+ Assertions.assertEquals(1, filtered.length);
+ Assertions.assertEquals("t1", filtered[0].name());
+ }
+ }
+
+ private static NameIdentifier[] threeTables() {
+ return new NameIdentifier[] {
+ NameIdentifierUtil.ofTable("testMetalake", "testCatalog", "testSchema",
"t1"),
+ NameIdentifierUtil.ofTable("testMetalake", "testCatalog", "testSchema",
"t2"),
+ NameIdentifierUtil.ofTable("testMetalake", "testCatalog", "testSchema",
"t3")
+ };
+ }
+
+ @Test
+ public void testListShortCircuitParentGrantWithoutDenyReturnsAll() {
+ makeCompletableFutureUseCurrentThread();
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> mockStatic =
+ mockStatic(GravitinoAuthorizerProvider.class)) {
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+ principalUtilsMocked.when(() -> PrincipalUtils.doAs(any(),
any())).thenCallRealMethod();
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ GravitinoAuthorizer authorizer = mockTableListAuthorizer(false, false);
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer);
+
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake",
+
AuthorizationExpressionConstants.FILTER_TABLE_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.TABLE,
+ threeTables());
+
+ // The schema-level grant covers every table and no object-level deny is
reported, so the
+ // whole list is returned without consulting the per-table deny on t2.
+ Assertions.assertEquals(3, filtered.length);
+ }
+ }
+
+ @Test
+ public void testListShortCircuitFallsBackWhenDenyMayExist() {
+ makeCompletableFutureUseCurrentThread();
+ try (MockedStatic<PrincipalUtils> principalUtilsMocked =
mockStatic(PrincipalUtils.class);
+ MockedStatic<GravitinoAuthorizerProvider> mockStatic =
+ mockStatic(GravitinoAuthorizerProvider.class)) {
+ principalUtilsMocked
+ .when(PrincipalUtils::getCurrentPrincipal)
+ .thenReturn(new UserPrincipal("tester"));
+ principalUtilsMocked.when(() -> PrincipalUtils.doAs(any(),
any())).thenCallRealMethod();
+ GravitinoAuthorizerProvider mockedProvider =
mock(GravitinoAuthorizerProvider.class);
+
mockStatic.when(GravitinoAuthorizerProvider::getInstance).thenReturn(mockedProvider);
+ GravitinoAuthorizer authorizer = mockTableListAuthorizer(false, true);
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer);
+
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake",
+
AuthorizationExpressionConstants.FILTER_TABLE_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.TABLE,
+ threeTables());
+
+ // A deny may exist, so the short-circuit is disabled and per-table
filtering excludes t2.
+ Assertions.assertEquals(2, filtered.length);
+ Assertions.assertTrue(
+ Arrays.stream(filtered).noneMatch(id -> "t2".equals(id.name())),
+ "t2 must be filtered out by its table-level deny");
+ }
+ }
+
private static void makeCompletableFutureUseCurrentThread() {
try {
Executor currentThread = Runnable::run;
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 5f28bfc7dd..9932a73858 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
@@ -17,6 +17,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.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -37,6 +38,7 @@ import static org.mockito.Mockito.when;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
@@ -460,6 +462,69 @@ public class TestJcasbinAuthorizer {
assertFalse(doAuthorize(currentPrincipal));
}
+ @Test
+ public void testHasDenyPolicy() throws Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
+
+ // With no roles assigned, the user can hold no deny policy.
+ assertFalse(
+ jcasbinAuthorizer.hasDenyPolicy(
+ currentPrincipal,
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ new AuthorizationRequestContext()));
+
+ // Assign a role that DENIES USE_CATALOG at the catalog scope.
+ mockRoleInStore(DENY_ROLE_ID, "denyRole",
ImmutableList.of(getDenySecurableObject()));
+ when(roleMetaMapper.listRolesByUserId(eq(USER_ID)))
+ .thenReturn(ImmutableList.of(buildRolePO(DENY_ROLE_ID, "denyRole")));
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
+
+ // The deny on USE_CATALOG is detected. The match is scope-agnostic: the
deny lives on a
+ // catalog, which is a parent scope for a schema/catalog list, so it must
be reported and
+ // disable the short-circuit.
+ assertTrue(
+ jcasbinAuthorizer.hasDenyPolicy(
+ currentPrincipal,
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ new AuthorizationRequestContext()));
+
+ // A deny on USE_CATALOG must not be reported when querying a different
privilege.
+ assertFalse(
+ jcasbinAuthorizer.hasDenyPolicy(
+ currentPrincipal,
+ METALAKE,
+ ImmutableSet.of(SELECT_TABLE),
+ new AuthorizationRequestContext()));
+ }
+
+ @Test
+ public void testHasDenyPolicyDetectsGroupInheritedDeny() throws Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+
+ // The user holds no direct roles; the deny role is only reachable through
group membership.
+ UserPrincipal groupPrincipal = setCurrentPrincipalWithGroup(GROUP_NAME);
+ mockRoleInStore(DENY_ROLE_ID, "denyRole",
ImmutableList.of(getDenySecurableObject()));
+ mockNoDirectUserRoles();
+ mockGroupWithRoles(GROUP_NAME, ImmutableList.of(DENY_ROLE_ID),
ImmutableList.of("denyRole"));
+
+ // A deny inherited via a group must still be detected, otherwise the list
short-circuit would
+ // over-expose objects that a group-level deny is meant to hide.
+ assertTrue(
+ jcasbinAuthorizer.hasDenyPolicy(
+ groupPrincipal,
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ new AuthorizationRequestContext()));
+
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
@Test
public void testUserRoleCacheDoesNotReuseRolesAfterUsernameRecreate() throws
Exception {
makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
