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 5b640f9504 [#13691] fix(authorization): report deny while a role's
policies are incomplete (#13707)
5b640f9504 is described below
commit 5b640f9504b7299b75d5579698f313ed273e34f2
Author: dennismdejong <[email protected]>
AuthorDate: Fri Nov 6 11:51:39 2026 +0100
[#13691] fix(authorization): report deny while a role's policies are
incomplete (#13707)
### What changes were proposed in this pull request?
`JcasbinAuthorizer` now tracks which roles currently hold incomplete
policies in a new `incompleteRolePolicyIds` set. While a role is in that
set, `hasDenyPolicy` reports a possible deny for any request that holds
it — directly or via a group, including inactive roles, since DENY
remains global — so list authorization falls back to per-object checks
instead of the parent-scope short-circuit.
The check runs inside the existing `evaluateWithLoadedRolePolicies`
callback and inspects the request's held-role subjects
(`roleSubjects(requestContext, false)`), so it follows the
request-scoped role model introduced by #13747 rather than the shared
JCasbin grouping rows.
### Why are the changes needed?
`hasDenyPolicy` answers an existence question — "can a DENY hide one of
the listed objects?" — by scanning the deny enforcer's rows for the
request's roles. A role load that cannot resolve a securable object
installs only the rows it could resolve, so the missing deny row makes
the scan return `false`, and `MetadataAuthzHelper` may then take the
parent-scope short-circuit.
`partialRoleLoadBackoff` was the only available evidence of an
incomplete load, and it cannot carry that answer: it is a retry throttle
whose expiry is unrelated to the installed rows, and it is not even
re-armed when the retry cannot read the role entity. The new marker is
added and removed only together with the policy rows it describes, so
the two lifetimes always coincide.
Deliberately conservative: the marker is role-scoped, not
privilege-scoped, so a role that stays unresolvable (e.g. it still
references a dropped table) keeps the list short-circuit disabled for
its holders.
Fix: #13691
### Does this PR introduce _any_ user-facing change?
Yes, but the guarantee is limited: a request holding a partially loaded
role no longer takes the parent-scope list short-circuit and falls back
to per-object authorization, which may be slower.
This does **not** by itself make a not-yet-resolved DENY effective. The
deny row is still absent from the enforcer, so an object that DENY is
meant to hide can remain in the list until the role reloads completely
and installs the row. Today the list result is the same as with the
short-circuit, because the per-object path reads the same deny rows. The
marker keeps `hasDenyPolicy` truthful, so a future fix for the gap
(#14379) cannot be bypassed by the short-circuit.
Note on the #13691 reproduction: since #13699, a transient normalization
failure already installs a catalog-scoped deny guard that the existing
scan sees, and an entity-store failure fails closed. The marker
therefore only matters when a DENY object genuinely resolves to
NotFound, for example after a concurrent rename (#14379).
### How was this patch tested?
Added regression tests in `TestJcasbinAuthorizer`:
- `testHasDenyPolicyReportsMissingDenyOfIncompleteRoleLoad`: initial
partial load; asserts the deny row is genuinely absent from the deny
enforcer; then expires the retry throttle and makes the retry
unreadable, so nothing re-arms the throttle — the possible deny must
still be reported.
- `testHasDenyPolicyRecoversAfterCompleteRoleLoad`: recovery after a
complete load (marker cleared, answer goes back to `false` for an
ALLOW-only role), and a partial reload over a complete load re-arms the
marker while the resolvable rows stay installed.
- `testCompleteResultAfterPartialLoadClearsIncompleteMarker`:
complete-after-partial replacement, plus explicit
`handleRolePrivilegeChange` invalidation clearing the marker.
-
`testListStillIncludesObjectDeniedByIncompleteRoleUntilCompleteReload`:
drives the real `MetadataAuthzHelper.filterByExpression`. Documents the
actual list result — the catalog stays listed while the deny row is
missing (even though the short-circuit is disabled) and is excluded only
after a complete reload installs the row.
- `testPartialResultCannotOverwriteCompletedConcurrentLoad` (extended):
a late partial result must not mark a role that a concurrent complete
load already finished.
`./gradlew :server-common:test --tests
"org.apache.gravitino.server.authorization.jcasbin.*" -PskipITs` passes
(181 tests, 0 failures), and `:server-common:spotlessCheck` passes. The
branch is rebased onto current `main`.
---------
Co-authored-by: yuqi <[email protected]>
---
.../authorization/jcasbin/JcasbinAuthorizer.java | 68 +++-
.../jcasbin/TestJcasbinAuthorizer.java | 364 +++++++++++++++++++++
2 files changed, 429 insertions(+), 3 deletions(-)
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 695b61c299..0b8e8eae34 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
@@ -120,6 +120,13 @@ import org.slf4j.LoggerFactory;
* role id also deletes that role's policies from both enforcers. A request
that loaded the role
* before such an eviction reloads it before its next check; see {@link
* #evaluateWithLoadedRolePolicies}.
+ *
+ * <p>A role whose securable objects could not all be resolved is recorded in
{@link
+ * #incompleteRolePolicyIds} for as long as its partial rows are installed.
{@link #hasDenyPolicy}
+ * then reports a possible deny for any request holding that role: the deny
enforcer's rows are
+ * missing the unresolved objects, so they cannot prove that no DENY hides one
of the listed
+ * objects. This only disables the list short-circuit; per-object evaluation
is unchanged, so an
+ * unresolvable DENY still cannot hide an object it never resolved to.
*/
public class JcasbinAuthorizer implements GravitinoAuthorizer {
@@ -243,6 +250,25 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
*/
private GravitinoCache<Long, Boolean> partialRoleLoadBackoff;
+ /**
+ * Role ids (the same string form used as the {@code sub} field of a policy
row) whose currently
+ * installed policies are known to be incomplete, i.e. at least one
securable object of the role
+ * could not be resolved to a metadata id during its last load.
+ *
+ * <p>An entry means the role's rows in {@link #denyEnforcer} cannot prove
the absence of a DENY:
+ * the missing object may have carried one. {@link #hasDenyPolicy} therefore
reports a possible
+ * deny for any request holding such a role, disabling the list
short-circuit. It does not make
+ * the missing DENY itself effective; per-object evaluation is unchanged.
+ *
+ * <p>Unlike {@link #partialRoleLoadBackoff} — a retry throttle that expires
on its own — an entry
+ * is added and removed only together with the policy set it describes:
added by {@link
+ * #replaceRolePolicies} when a partial result is applied, and removed when
a complete result
+ * replaces it or when {@link #clearRolePoliciesWithoutLock} drops the
role's rows. Guarded by
+ * {@link #rolePolicyLock}; the lifetimes of the marker and of the partial
policies therefore
+ * always coincide.
+ */
+ private final Set<String> incompleteRolePolicyIds = new HashSet<>();
+
// ---- Eventual consistency caches (poller-driven) ----
/** Path-based metadata object key -> entity id. Evicted by entity change
poller. */
@@ -488,6 +514,29 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
metalake,
requestContext,
() -> {
+ // The request holds these roles (direct + group-inherited);
inactive roles are
+ // included because a DENY remains in effect regardless of
active-role narrowing.
+ Set<String> heldRoleIds = roleSubjects(requestContext, false);
+
+ // A role whose securable objects could not all be resolved
installs only the rows it
+ // could resolve, so its deny rows may be missing. This method
answers an existence
+ // question — "can a DENY on these privileges hide one of the
listed objects?" — and
+ // missing rows can never prove the absence of a DENY, so a held
role with incomplete
+ // policies must be reported as possibly holding one. The caller
then falls back to
+ // per-object checks instead of the parent-scope short-circuit.
Note that this only
+ // disables that short-circuit: per-object evaluation is
unchanged, so an unresolvable
+ // DENY still cannot hide an object it never resolved to.
+ for (String roleId : heldRoleIds) {
+ if (incompleteRolePolicyIds.contains(roleId)) {
+ LOG.debug(
+ "Reporting a possible deny for user {} because role {}
holds incomplete"
+ + " policies",
+ userId,
+ roleId);
+ return true;
+ }
+ }
+
// 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
@@ -496,7 +545,7 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
// 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 : roleSubjects(requestContext, false)) {
+ for (String roleId : heldRoleIds) {
// 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
@@ -1557,8 +1606,16 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
if (resolved.isComplete()) {
loadedRoles.put(roleId, dbUpdatedAt);
partialRoleLoadBackoff.invalidate(roleId);
+ // The installed rows are now the role's complete policy set, so the
deny enforcer proves
+ // the absence of a DENY on these privileges again. Defensive: a
previous partial load
+ // leaves no loadedRoles marker, so clearRolePoliciesWithoutLock above
has normally removed
+ // the entry already.
+ incompleteRolePolicyIds.remove(String.valueOf(roleId));
} else {
partialRoleLoadBackoff.put(roleId, Boolean.TRUE);
+ // The rows applied above omit every securable object that did not
resolve; remember that
+ // this role may be hiding a DENY so hasDenyPolicy cannot clear the
list short-circuit.
+ incompleteRolePolicyIds.add(String.valueOf(roleId));
}
return true;
} finally {
@@ -1628,6 +1685,10 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
String roleIdStr = String.valueOf(roleId);
boolean allowRemoved = allowEnforcer.removeFilteredPolicy(0, roleIdStr);
boolean denyRemoved = denyEnforcer.removeFilteredPolicy(0, roleIdStr);
+ // The marker describes these rows, so it must go with them. Leaving it
behind would keep the
+ // list short-circuit disabled for every request holding this role even
though nothing is
+ // missing.
+ incompleteRolePolicyIds.remove(roleIdStr);
return allowRemoved || denyRemoved;
}
@@ -1676,8 +1737,9 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
*
* <p>Earlier decisions keep their existing request-scoped results. Each new
policy evaluation
* runs inside one read-locked section without holding the lock across
metadata or DB lookups.
- * Partial role-policy resolution keeps its existing per-object behavior;
reliable completeness
- * tracking for the deny-existence scan is handled separately in issue
#13691.
+ * Partial role-policy resolution keeps its existing per-object behavior;
the deny-existence scan
+ * only learns that such a role may hold a missing DENY through {@link
#incompleteRolePolicyIds}
+ * (see {@link #hasDenyPolicy}).
*
* @return the evaluation result, or empty when the request's roles kept
being cleared or could
* not be reloaded; the caller must then fail closed
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 380b557f3d..949dc53900 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
@@ -1210,6 +1210,362 @@ public class TestJcasbinAuthorizer {
assertFalse(
getAllowEnforcer(jcasbinAuthorizer).getFilteredPolicy(0,
roleIdStr).isEmpty(),
"the completed load's policies must survive a late partial result");
+ assertFalse(
+ getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(roleIdStr),
+ "a late partial result must not mark a completely loaded role as
incomplete");
+ }
+
+ @Test
+ public void testCompleteResultAfterPartialLoadClearsIncompleteMarker()
throws Exception {
+ // Mirror of testPartialResultCannotOverwriteCompletedConcurrentLoad with
the opposite ordering:
+ // the partial result wins first, then a complete result for the same
version replaces it.
+ String roleIdStr = String.valueOf(ALLOW_ROLE_ID);
+ String[] policyRow =
+ new String[] {
+ roleIdStr,
+ MetadataObject.Type.CATALOG.name(),
+ String.valueOf(CATALOG_ID),
+ USE_CATALOG.name(),
+ AuthConstants.ALLOW
+ };
+ ResolvedRolePolicies complete =
+ new ResolvedRolePolicies(
+ ImmutableList.<String[]>of(policyRow),
+ Collections.emptyList(),
+ Collections.emptyList());
+ ResolvedRolePolicies partial =
+ new ResolvedRolePolicies(
+ Collections.emptyList(),
+ Collections.emptyList(),
+ ImmutableList.of("CATALOG:testCatalog"));
+ long roleVersion = 43L;
+
+ assertTrue(invokeReplaceRolePolicies(jcasbinAuthorizer, ALLOW_ROLE_ID,
roleVersion, partial));
+ assertTrue(
+ getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(roleIdStr),
+ "a partial load must mark the role's policies as incomplete");
+ assertFalse(
+
getLoadedRolesCache(jcasbinAuthorizer).getIfPresent(ALLOW_ROLE_ID).isPresent(),
+ "a partial load must stay out of loadedRoles");
+
+ // The complete result replaces the partial rows, so nothing can be
missing any longer.
+ assertTrue(invokeReplaceRolePolicies(jcasbinAuthorizer, ALLOW_ROLE_ID,
roleVersion, complete));
+ assertFalse(
+ getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(roleIdStr),
+ "a complete load must clear the incomplete marker");
+ assertFalse(
+ getAllowEnforcer(jcasbinAuthorizer).getFilteredPolicy(0,
roleIdStr).isEmpty(),
+ "the complete load's policies must be installed");
+
+ // An explicit invalidation drops the rows, so the marker must not outlive
them. Re-arm the
+ // marker first with a newer partial load that still installs a row;
otherwise the assertion
+ // below would pass even if clearing the rows left the marker behind.
+ ResolvedRolePolicies partialWithRow =
+ new ResolvedRolePolicies(
+ ImmutableList.<String[]>of(policyRow),
+ Collections.emptyList(),
+ ImmutableList.of("CATALOG:testCatalog"));
+ assertTrue(
+ invokeReplaceRolePolicies(
+ jcasbinAuthorizer, ALLOW_ROLE_ID, roleVersion + 1,
partialWithRow));
+ assertTrue(
+ getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(roleIdStr),
+ "a newer partial load must re-arm the incomplete marker");
+
+ jcasbinAuthorizer.handleRolePrivilegeChange(ALLOW_ROLE_ID);
+ assertTrue(
+ getAllowEnforcer(jcasbinAuthorizer).getFilteredPolicy(0,
roleIdStr).isEmpty(),
+ "an explicit invalidation must drop the partial rows");
+ assertFalse(
+ getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(roleIdStr),
+ "clearing a role's policies must also clear its incomplete marker");
+ }
+
+ @Test
+ public void testHasDenyPolicyReportsMissingDenyOfIncompleteRoleLoad() throws
Exception {
+ Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
+
+ // The user holds a role that DENIES USE_CATALOG on a parent scope, but
the role's securable
+ // object cannot be resolved, so no deny row is installed. hasDenyPolicy
answers an existence
+ // question ("can a DENY hide one of the listed objects?"), and a row that
was never installed
+ // cannot prove there is none. Reporting "no deny" would let list
authorization take the
+ // parent-scope short-circuit; reporting one forces the per-object
evaluation, which is the
+ // strongest guarantee this fix provides (it does not make the missing
DENY effective; see
+ // testListStillIncludesObjectDeniedByIncompleteRoleUntilCompleteReload).
+ 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()));
+
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenReturn(Optional.empty());
+ try {
+ assertTrue(
+ jcasbinAuthorizer.hasDenyPolicy(
+ currentPrincipal,
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ new AuthorizationRequestContext()),
+ "an incomplete role load must be reported as possibly holding a
DENY");
+ assertTrue(
+ getDenyEnforcer(jcasbinAuthorizer)
+ .getFilteredNamedPolicy("p", 0, String.valueOf(DENY_ROLE_ID))
+ .isEmpty(),
+ "the deny row must really be absent, so the answer cannot come from
the policy scan");
+ } finally {
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenReturn(Optional.of(CATALOG_ID));
+ }
+
+ // The retry throttle is a TTL cache and lapses independently of the
partial policies. In
+ // production it can expire after loadRolePrivilege and before the
evaluation, while the partial
+ // rows are still installed. The null entity below is only a test stand-in
for that window: it
+ // keeps the retry from reloading the role or re-arming the throttle. (A
role that really cannot
+ // be read throws instead, is marked unreadable, and hasDenyPolicy fails
closed.) The marker,
+ // not the throttle, must carry the answer.
+ getPartialRoleLoadBackoffCache(jcasbinAuthorizer).invalidate(DENY_ROLE_ID);
+ when(entityStore.get(
+ eq(NameIdentifierUtil.ofRole(METALAKE, "denyRole")),
+ eq(Entity.EntityType.ROLE),
+ eq(RoleEntity.class)))
+ .thenReturn(null);
+
+ assertTrue(
+ jcasbinAuthorizer.hasDenyPolicy(
+ currentPrincipal,
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ new AuthorizationRequestContext()),
+ "the deny must still be reported after the retry throttle expired");
+ assertFalse(
+
getPartialRoleLoadBackoffCache(jcasbinAuthorizer).getIfPresent(DENY_ROLE_ID).isPresent(),
+ "the retry must not have re-armed the throttle, proving the marker
carries the answer");
+ assertTrue(
+
getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(String.valueOf(DENY_ROLE_ID)),
+ "the incomplete marker must survive the retry throttle");
+ }
+
+ @Test
+ public void testHasDenyPolicyRecoversAfterCompleteRoleLoad() throws
Exception {
+ Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
+
+ // An ALLOW-only role so that the deny enforcer never holds a row: every
hasDenyPolicy answer
+ // below is driven solely by policy completeness, which makes the recovery
observable.
+ RoleEntity allowRole =
+ mockRoleInStore(ALLOW_ROLE_ID, "allowRole",
ImmutableList.of(getAllowSecurableObject()));
+ mockDirectUserRoles(allowRole);
+
+ try {
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenReturn(Optional.empty());
+ assertTrue(
+ jcasbinAuthorizer.hasDenyPolicy(
+ currentPrincipal,
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ new AuthorizationRequestContext()),
+ "no policy could be installed, so the DENY enforcer cannot be
trusted yet");
+ assertTrue(
+
getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(String.valueOf(ALLOW_ROLE_ID)));
+
+ // 1. Recovery: once every object resolves the role loads completely,
the marker goes away and
+ // the deny enforcer is authoritative again.
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenReturn(Optional.of(CATALOG_ID));
+
getPartialRoleLoadBackoffCache(jcasbinAuthorizer).invalidate(ALLOW_ROLE_ID);
+ assertFalse(
+ jcasbinAuthorizer.hasDenyPolicy(
+ currentPrincipal,
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ new AuthorizationRequestContext()),
+ "a completely loaded ALLOW-only role holds no deny");
+ assertFalse(
+
getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(String.valueOf(ALLOW_ROLE_ID)));
+
assertTrue(getLoadedRolesCache(jcasbinAuthorizer).getIfPresent(ALLOW_ROLE_ID).isPresent());
+
+ // 2. Partial reloads: a newer version of an already complete role that
fails to resolve one
+ // securable object must re-arm the marker.
+ RoleEntity partiallyResolvable =
+ getRoleEntity(
+ ALLOW_ROLE_ID,
+ "allowRole",
+ ImmutableList.of(
+ getAllowSecurableObject(),
+ buildSecurableObject(
+ ALLOW_ROLE_ID,
+ MetadataObject.Type.CATALOG,
+ "unresolvableCatalog",
+ USE_CATALOG,
+ "ALLOW")));
+ when(entityStore.get(
+ eq(NameIdentifierUtil.ofRole(METALAKE, "allowRole")),
+ eq(Entity.EntityType.ROLE),
+ eq(RoleEntity.class)))
+ .thenReturn(partiallyResolvable);
+ mockedRoleVersions.put(
+ ALLOW_ROLE_ID, new RoleUpdatedAt(ALLOW_ROLE_ID, "allowRole",
nextRoleVersion()));
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenAnswer(
+ invocation -> {
+ MetadataObject object = invocation.getArgument(0);
+ return object.fullName().contains("unresolvableCatalog")
+ ? Optional.empty()
+ : Optional.of(CATALOG_ID);
+ });
+
getPartialRoleLoadBackoffCache(jcasbinAuthorizer).invalidate(ALLOW_ROLE_ID);
+
+ assertTrue(
+ jcasbinAuthorizer.hasDenyPolicy(
+ currentPrincipal,
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ new AuthorizationRequestContext()),
+ "a partial reload must re-arm the marker even though the previous
load was complete");
+ assertTrue(
+
getIncompleteRolePolicyIds(jcasbinAuthorizer).contains(String.valueOf(ALLOW_ROLE_ID)));
+ assertFalse(
+ getAllowEnforcer(jcasbinAuthorizer)
+ .getFilteredPolicy(0, String.valueOf(ALLOW_ROLE_ID))
+ .isEmpty(),
+ "the resolvable securable object must still install its row");
+ } finally {
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenReturn(Optional.of(CATALOG_ID));
+ }
+ }
+
+ /**
+ * Documents the limit of the incomplete-role marker through the real list
filter: disabling the
+ * parent-scope short-circuit does not make an unresolved DENY hide its
object. Per-object
+ * evaluation is unchanged, so the object stays listed until the role
reloads completely and
+ * installs the deny row. The remaining authorization gap is tracked
separately.
+ */
+ @Test
+ public void
testListStillIncludesObjectDeniedByIncompleteRoleUntilCompleteReload()
+ throws Exception {
+ Field executorField =
MetadataAuthzHelper.class.getDeclaredField("executor");
+ executorField.setAccessible(true);
+ Object previousExecutor = executorField.get(null);
+ executorField.set(null, (Executor) Runnable::run);
+ Config previousConfig = gravitinoEnv.config();
+ Config config = mock(Config.class);
+ when(config.get(eq(Configs.ENABLE_AUTHORIZATION))).thenReturn(true);
+ when(gravitinoEnv.config()).thenReturn(config);
+ try {
+ // A metalake-level ALLOW USE_CATALOG satisfies the catalog-list
parent-scope path, and a
+ // catalog-level DENY USE_CATALOG on the same catalog must invalidate it.
+ RoleEntity allowRole =
+ mockRoleInStore(
+ ALLOW_ROLE_ID,
+ "listAllowRole",
+ ImmutableList.of(
+ buildSecurableObject(
+ ALLOW_ROLE_ID,
+ MetadataObject.Type.METALAKE,
+ METALAKE,
+ USE_CATALOG,
+ "ALLOW")));
+ RoleEntity denyRole =
+ mockRoleInStore(
+ DENY_ROLE_ID,
+ "listDenyRole",
+ ImmutableList.of(
+ buildSecurableObject(
+ DENY_ROLE_ID,
+ MetadataObject.Type.CATALOG,
+ "testCatalog",
+ USE_CATALOG,
+ "DENY")));
+ mockDirectUserRoles(allowRole, denyRole);
+
+ // The DENY's catalog cannot be resolved while the role loads, so only
the ALLOW role
+ // installs rows and the deny enforcer has no row for this catalog.
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenAnswer(
+ invocation -> {
+ MetadataObject object = invocation.getArgument(0);
+ return object.type() == MetadataObject.Type.CATALOG
+ && "testCatalog".equals(object.fullName())
+ ? Optional.empty()
+ : Optional.of(CATALOG_ID);
+ });
+
+ AuthorizationRequestContext context = new AuthorizationRequestContext();
+ assertTrue(
+ jcasbinAuthorizer.hasDenyPolicy(
+ PrincipalUtils.getCurrentPrincipal(),
+ METALAKE,
+ ImmutableSet.of(USE_CATALOG),
+ context),
+ "the incomplete role must be reported as possibly holding a DENY");
+ assertTrue(
+ getDenyEnforcer(jcasbinAuthorizer)
+ .getFilteredNamedPolicy("p", 0, String.valueOf(DENY_ROLE_ID))
+ .isEmpty(),
+ "the deny row must really be absent");
+
+ // The per-object path can resolve the catalog now, but the deny row is
still missing, so the
+ // listed catalog is still returned. This is the documented limit of the
fix.
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenReturn(Optional.of(CATALOG_ID));
+
+ GravitinoAuthorizerProvider provider =
mock(GravitinoAuthorizerProvider.class);
+ when(provider.getGravitinoAuthorizer()).thenReturn(jcasbinAuthorizer);
+ NameIdentifier deniedCatalog = NameIdentifier.of(METALAKE,
"testCatalog");
+ try (MockedStatic<GravitinoAuthorizerProvider> providerMock =
+ mockStatic(GravitinoAuthorizerProvider.class);
+ AuthorizationRequestScope scope = AuthorizationRequestScope.open()) {
+
providerMock.when(GravitinoAuthorizerProvider::getInstance).thenReturn(provider);
+ scope.bind(METALAKE, context);
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ METALAKE,
+
AuthorizationExpressionConstants.LOAD_CATALOG_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.CATALOG,
+ new NameIdentifier[] {deniedCatalog});
+ assertEquals(
+ 1,
+ filtered.length,
+ "the unresolved DENY row cannot hide the catalog: per-object
evaluation still allows it");
+ }
+
+ // A complete reload of the same DENY installs the row, and the
per-object path now excludes
+ // the catalog.
+
getPartialRoleLoadBackoffCache(jcasbinAuthorizer).invalidate(DENY_ROLE_ID);
+ AuthorizationRequestContext reloadedContext = new
AuthorizationRequestContext();
+ try (MockedStatic<GravitinoAuthorizerProvider> providerMock =
+ mockStatic(GravitinoAuthorizerProvider.class);
+ AuthorizationRequestScope scope = AuthorizationRequestScope.open()) {
+
providerMock.when(GravitinoAuthorizerProvider::getInstance).thenReturn(provider);
+ scope.bind(METALAKE, reloadedContext);
+ NameIdentifier[] filtered =
+ MetadataAuthzHelper.filterByExpression(
+ METALAKE,
+
AuthorizationExpressionConstants.LOAD_CATALOG_AUTHORIZATION_EXPRESSION,
+ Entity.EntityType.CATALOG,
+ new NameIdentifier[] {deniedCatalog});
+ assertEquals(0, filtered.length, "a completely loaded DENY must hide
the catalog again");
+ }
+ assertTrue(
+ getIncompleteRolePolicyIds(jcasbinAuthorizer).isEmpty(),
+ "the complete reload must clear the incomplete marker");
+ } finally {
+ executorField.set(null, previousExecutor);
+ when(gravitinoEnv.config()).thenReturn(previousConfig);
+ metadataIdConverterMockedStatic
+ .when(() -> MetadataIdConverter.getID(any(), eq(METALAKE)))
+ .thenReturn(Optional.of(CATALOG_ID));
+ }
}
@Test
@@ -4463,6 +4819,14 @@ public class TestJcasbinAuthorizer {
return (GravitinoCache<Long, Boolean>) field.get(authorizer);
}
+ @SuppressWarnings("unchecked")
+ private static Set<String> getIncompleteRolePolicyIds(JcasbinAuthorizer
authorizer)
+ throws Exception {
+ Field field =
JcasbinAuthorizer.class.getDeclaredField("incompleteRolePolicyIds");
+ field.setAccessible(true);
+ return (Set<String>) field.get(authorizer);
+ }
+
@SuppressWarnings("unchecked")
private static GravitinoCache<Long, Optional<OwnerInfo>> getOwnerRelCache(
JcasbinAuthorizer authorizer) throws Exception {
