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 b3593b8697 [#12837] fix(authz): Evaluate list denies by access path
(#12838)
b3593b8697 is described below
commit b3593b8697f357ebf7d2d6cb9bcc3d0891f385f6
Author: Qi Yu <[email protected]>
AuthorDate: Wed Sep 2 16:43:46 2026 +0800
[#12837] fix(authz): Evaluate list denies by access path (#12838)
### What changes were proposed in this pull request?
- Model list-authorization short-circuits as independent parent-scope
access paths.
- Check deny policies only for the privilege used by each access path.
- Support both table filter and table-like list authorization
expressions.
- Preserve the `USE_CATALOG` and `USE_SCHEMA` requirements for
table-like listing.
### Why are the changes needed?
The existing short-circuit checks `SELECT_TABLE` and `MODIFY_TABLE`
denies together. A deny on one privilege therefore disables an
independent, deny-free access path and falls back to per-object
authorization.
For large schemas, this causes thousands of unnecessary authorization
checks even when all tables are visible through another parent-scope
privilege.
Fix: #12837
### Does this PR introduce _any_ user-facing change?
No API or authorization-result changes. Eligible list operations avoid
unnecessary per-object authorization checks.
### How was this patch tested?
- `./gradlew :server-common:spotlessApply`
- `./gradlew :server-common:test --tests
org.apache.gravitino.server.authorization.TestMetadataAuthzHelper
-PskipITs -PskipDockerTests=false`
- `./gradlew :server-common:check -PskipITs -PskipDockerTests=false`
- `git diff --check`
---
.../server/authorization/MetadataAuthzHelper.java | 151 +++++++++++-------
.../authorization/TestMetadataAuthzHelper.java | 176 +++++++++++++++++++++
2 files changed, 271 insertions(+), 56 deletions(-)
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 cd6178258f..f2c4d097c9 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
@@ -89,45 +89,77 @@ public class MetadataAuthzHelper {
private static final List<Entity.EntityType> REQUIRE_SCHEMA_EXISTS =
Arrays.asList(Entity.EntityType.TABLE, Entity.EntityType.TOPIC);
+ private static final String TABLE_PARENT_SCOPES = "METALAKE, CATALOG,
SCHEMA";
+ private static final String SCHEMA_PARENT_SCOPES = "METALAKE, CATALOG";
+ private static final String CATALOG_PARENT_SCOPES = "METALAKE";
+
/**
* 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.
+ * entry pairs a per-object filter expression with the alternative
parent-scope access paths that
+ * can make every listed object visible. Each path tracks only the deny
privileges that can
+ * invalidate that path, so a deny on one path does not disable an
independent path.
*/
- 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 static final Map<Entity.EntityType, Map<String,
List<ParentScopeAccessPath>>>
+ LIST_SHORT_CIRCUITS =
+ Map.of(
+ Entity.EntityType.TABLE,
+ Map.of(
+
AuthorizationExpressionConstants.FILTER_TABLE_AUTHORIZATION_EXPRESSION,
+ List.of(
+ parentOwnerPath(TABLE_PARENT_SCOPES),
+ parentPrivilegePath(Privilege.Name.SELECT_TABLE,
TABLE_PARENT_SCOPES),
+ parentPrivilegePath(Privilege.Name.MODIFY_TABLE,
TABLE_PARENT_SCOPES)),
+
AuthorizationExpressionConstants.LIST_TABLE_LIKE_AUTHORIZATION_EXPRESSION,
+ List.of(
+ parentOwnerPath(TABLE_PARENT_SCOPES),
+
tableLikeParentPrivilegePath(Privilege.Name.PROBE_TABLE_LIKE),
+
tableLikeParentPrivilegePath(Privilege.Name.SELECT_TABLE),
+
tableLikeParentPrivilegePath(Privilege.Name.MODIFY_TABLE),
+
tableLikeParentPrivilegePath(Privilege.Name.CREATE_TABLE),
+
tableLikeParentPrivilegePath(Privilege.Name.CREATE_VIEW))),
+ Entity.EntityType.SCHEMA,
+ Map.of(
+
AuthorizationExpressionConstants.FILTER_SCHEMA_AUTHORIZATION_EXPRESSION,
+ List.of(
+ parentOwnerPath(SCHEMA_PARENT_SCOPES),
+ parentPrivilegePath(Privilege.Name.USE_SCHEMA,
SCHEMA_PARENT_SCOPES))),
+ Entity.EntityType.CATALOG,
+ Map.of(
+
AuthorizationExpressionConstants.LOAD_CATALOG_AUTHORIZATION_EXPRESSION,
+ List.of(
+ parentOwnerPath(CATALOG_PARENT_SCOPES),
+ parentPrivilegePath(Privilege.Name.USE_CATALOG,
CATALOG_PARENT_SCOPES))));
+
+ /** A sufficient parent-scope access path and the deny privileges that can
invalidate it. */
+ private static final class ParentScopeAccessPath {
+ private final String expression;
private final Set<Privilege.Name> denyPrivileges;
- private ListShortCircuit(
- String filterExpression, String parentScopeExpression,
Set<Privilege.Name> denyPrivileges) {
- this.filterExpression = filterExpression;
- this.parentScopeExpression = parentScopeExpression;
+ private ParentScopeAccessPath(String expression, Set<Privilege.Name>
denyPrivileges) {
+ this.expression = expression;
this.denyPrivileges = denyPrivileges;
}
}
private MetadataAuthzHelper() {}
+ private static ParentScopeAccessPath parentOwnerPath(String parentScopes) {
+ return new ParentScopeAccessPath("ANY(OWNER, " + parentScopes + ")",
Set.of());
+ }
+
+ private static ParentScopeAccessPath parentPrivilegePath(
+ Privilege.Name privilege, String parentScopes) {
+ return new ParentScopeAccessPath(
+ String.format("ANY(%s, %s)", privilege.name(), parentScopes),
Set.of(privilege));
+ }
+
+ private static ParentScopeAccessPath
tableLikeParentPrivilegePath(Privilege.Name privilege) {
+ ParentScopeAccessPath privilegePath = parentPrivilegePath(privilege,
TABLE_PARENT_SCOPES);
+ return new ParentScopeAccessPath(
+ "ANY_USE_CATALOG && ANY_USE_SCHEMA && (" + privilegePath.expression +
")",
+ privilegePath.denyPrivileges);
+ }
+
public static Metalake[] filterMetalakes(Metalake[] metalakes, String
expression) {
AuthorizationRequestContext authorizationRequestContext = new
AuthorizationRequestContext();
return doFilter(
@@ -221,17 +253,20 @@ public class MetadataAuthzHelper {
Entity.EntityType entityType,
NameIdentifier[] nameIdentifiers) {
Principal principal = PrincipalUtils.getCurrentPrincipal();
- ListShortCircuit spec = LIST_SHORT_CIRCUITS.get(entityType);
- if (spec == null || !spec.filterExpression.equals(expression)) {
+ Map<String, List<ParentScopeAccessPath>> entityShortCircuits =
+ LIST_SHORT_CIRCUITS.get(entityType);
+ List<ParentScopeAccessPath> accessPaths =
+ entityShortCircuits == null ? null :
entityShortCircuits.get(expression);
+ if (accessPaths == null) {
LOG.debug(
"Parent-scope short-circuit unavailable for principal {}, entity
type {} under metalake "
+ "{}: {}.",
principal.getName(),
entityType,
metalake,
- spec == null
+ entityShortCircuits == null
? "no short-circuit spec is registered for this entity type"
- : "the requested filter expression does not match the registered
short-circuit "
+ : "the requested filter expression does not match a registered
short-circuit "
+ "expression");
return false;
}
@@ -255,39 +290,43 @@ public class MetadataAuthzHelper {
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) {
- LOG.debug(
- "Parent-scope short-circuit skipped for entity type {} under
metalake {}: principal {} "
- + "is not granted access at the parent scope, so per-object
authorization is "
- + "required.",
- entityType,
- metalake,
- principal.getName());
- return false;
- }
+ for (ParentScopeAccessPath accessPath : accessPaths) {
+ requestContext.setOriginalAuthorizationExpression(accessPath.expression);
+ boolean parentGrantsAccess =
+ new AuthorizationExpressionEvaluator(accessPath.expression,
authorizer)
+ .evaluate(metadataNames, requestContext, principal,
Optional.empty());
+ if (!parentGrantsAccess) {
+ continue;
+ }
+
+ boolean hasDeny =
+ !accessPath.denyPrivileges.isEmpty()
+ && authorizer.hasDenyPolicy(
+ principal, metalake, accessPath.denyPrivileges,
requestContext);
+ if (!hasDeny) {
+ return true;
+ }
- // 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.
- boolean hasDeny =
- authorizer.hasDenyPolicy(principal, metalake, spec.denyPrivileges,
requestContext);
- if (hasDeny) {
LOG.debug(
- "Parent-scope short-circuit disabled for entity type {} under
metalake {}: principal {} "
- + "holds a deny policy on {}, so per-object authorization is
required.",
+ "Parent-scope access path {} disabled for entity type {} under
metalake {}: principal "
+ + "{} holds a deny policy on {}.",
+ accessPath.expression,
entityType,
metalake,
principal.getName(),
- spec.denyPrivileges);
+ accessPath.denyPrivileges);
}
- return !hasDeny;
+
+ LOG.debug(
+ "Parent-scope short-circuit skipped for entity type {} under metalake
{}: principal {} "
+ + "has no deny-free parent access path, so per-object
authorization is required.",
+ entityType,
+ metalake,
+ principal.getName());
+ return false;
}
/**
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 15783d818a..024443f568 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
@@ -29,6 +29,7 @@ import static org.mockito.Mockito.when;
import java.lang.reflect.Field;
import java.util.Arrays;
+import java.util.Set;
import java.util.concurrent.Executor;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
@@ -47,6 +48,8 @@ import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.EnumSource;
import org.mockito.MockedStatic;
/** Test of {@link MetadataAuthzHelper} */
@@ -233,6 +236,61 @@ public class TestMetadataAuthzHelper {
return authorizer;
}
+ /**
+ * Builds an authorizer with table privileges granted at the schema scope.
Optional use grants
+ * satisfy the catalog and schema gates in the table-like list expression,
while deny discovery
+ * reports a deny only when the short-circuit asks about one of {@code
deniedPrivileges}.
+ */
+ private GravitinoAuthorizer mockTableListRouteAuthorizer(
+ Set<Privilege.Name> grantedPrivileges,
+ Set<Privilege.Name> deniedPrivileges,
+ boolean grantUsePrivileges) {
+ 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);
+ if (object.type() == MetadataObject.Type.CATALOG) {
+ return grantUsePrivileges && privilege ==
Privilege.Name.USE_CATALOG;
+ }
+ if (object.type() == MetadataObject.Type.SCHEMA) {
+ return grantedPrivileges.contains(privilege)
+ || grantUsePrivileges && privilege ==
Privilege.Name.USE_SCHEMA;
+ }
+ return false;
+ });
+ 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()))
+ .thenAnswer(
+ invocation -> {
+ Set<Privilege.Name> privileges = invocation.getArgument(2);
+ return privileges.stream().anyMatch(deniedPrivileges::contains);
+ });
+ return authorizer;
+ }
+
+ private void withAuthorizer(GravitinoAuthorizer authorizer, Runnable
assertions) {
+ 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);
+ when(mockedProvider.getGravitinoAuthorizer()).thenReturn(authorizer);
+ assertions.run();
+ }
+ }
+
/**
* 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
@@ -559,6 +617,124 @@ public class TestMetadataAuthzHelper {
}
}
+ @ParameterizedTest
+ @EnumSource(
+ value = Privilege.Name.class,
+ names = {"PROBE_TABLE_LIKE", "SELECT_TABLE", "MODIFY_TABLE",
"CREATE_TABLE", "CREATE_VIEW"})
+ public void
testListTableLikeShortCircuitSupportsEveryParentPrivilege(Privilege.Name
privilege) {
+ GravitinoAuthorizer authorizer =
+ mockTableListRouteAuthorizer(Set.of(privilege), Set.of(), true);
+ withAuthorizer(
+ authorizer,
+ () -> {
+ NameIdentifier[] tables = threeTables();
+ NameIdentifier[] filtered = filterTableLike(tables);
+
+ Assertions.assertSame(tables, filtered);
+ verify(authorizer, times(1))
+ .hasDenyPolicy(any(), eq("testMetalake"), eq(Set.of(privilege)),
any());
+ });
+ }
+
+ @Test
+ public void testListTableLikeShortCircuitIgnoresDenyOnIndependentPath() {
+ GravitinoAuthorizer authorizer =
+ mockTableListRouteAuthorizer(
+ Set.of(Privilege.Name.SELECT_TABLE, Privilege.Name.MODIFY_TABLE),
+ Set.of(Privilege.Name.MODIFY_TABLE),
+ true);
+ withAuthorizer(
+ authorizer,
+ () -> {
+ NameIdentifier[] tables = threeTables();
+ NameIdentifier[] filtered = filterTableLike(tables);
+
+ Assertions.assertSame(
+ tables,
+ filtered,
+ "A MODIFY_TABLE deny must not disable the independent
SELECT_TABLE access path");
+ verify(authorizer, times(1))
+ .hasDenyPolicy(
+ any(), eq("testMetalake"),
eq(Set.of(Privilege.Name.SELECT_TABLE)), any());
+ verify(authorizer, times(0))
+ .hasDenyPolicy(
+ any(), eq("testMetalake"),
eq(Set.of(Privilege.Name.MODIFY_TABLE)), any());
+ });
+ }
+
+ @Test
+ public void testListTableLikeShortCircuitTriesNextPathAfterDeny() {
+ GravitinoAuthorizer authorizer =
+ mockTableListRouteAuthorizer(
+ Set.of(Privilege.Name.SELECT_TABLE, Privilege.Name.MODIFY_TABLE),
+ Set.of(Privilege.Name.SELECT_TABLE),
+ true);
+ withAuthorizer(
+ authorizer,
+ () -> {
+ NameIdentifier[] tables = threeTables();
+ NameIdentifier[] filtered = filterTableLike(tables);
+
+ Assertions.assertSame(
+ tables,
+ filtered,
+ "A denied SELECT_TABLE path must not hide a deny-free
MODIFY_TABLE path");
+ verify(authorizer, times(1))
+ .hasDenyPolicy(
+ any(), eq("testMetalake"),
eq(Set.of(Privilege.Name.SELECT_TABLE)), any());
+ verify(authorizer, times(1))
+ .hasDenyPolicy(
+ any(), eq("testMetalake"),
eq(Set.of(Privilege.Name.MODIFY_TABLE)), any());
+ });
+ }
+
+ @Test
+ public void testListTableLikeShortCircuitRequiresUsePrivileges() {
+ GravitinoAuthorizer authorizer =
+ mockTableListRouteAuthorizer(Set.of(Privilege.Name.SELECT_TABLE),
Set.of(), false);
+ withAuthorizer(
+ authorizer,
+ () -> {
+ NameIdentifier[] filtered = filterTableLike(threeTables());
+
+ verify(authorizer, times(0)).hasDenyPolicy(any(),
eq("testMetalake"), anySet(), any());
+ Assertions.assertEquals(
+ 0,
+ filtered.length,
+ "The short-circuit must not bypass the table-like expression's
use privileges");
+ });
+ }
+
+ @Test
+ public void testListShortCircuitOwnerIgnoresPrivilegeDenies() {
+ GravitinoAuthorizer authorizer = mockTableListAuthorizer(true, true);
+ withAuthorizer(
+ authorizer,
+ () -> {
+ NameIdentifier[] tables = threeTables();
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ "testMetalake",
+
AuthorizationExpressionConstants.FILTER_TABLE_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.TABLE,
+ tables);
+
+ Assertions.assertSame(
+ tables,
+ filtered,
+ "Privilege denies do not invalidate an independent
ancestor-owner access path");
+ verify(authorizer, times(0)).hasDenyPolicy(any(),
eq("testMetalake"), anySet(), any());
+ });
+ }
+
+ private static NameIdentifier[] filterTableLike(NameIdentifier[] tables) {
+ return MetadataAuthzHelper.filterByExpression(
+ "testMetalake",
+
AuthorizationExpressionConstants.LIST_TABLE_LIKE_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.TABLE,
+ tables);
+ }
+
private static void makeCompletableFutureUseCurrentThread() {
try {
Executor currentThread = Runnable::run;
