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 a33fa0d1d7 [#12169] fix(auth): clear only role policies on cache
eviction (#12172)
a33fa0d1d7 is described below
commit a33fa0d1d7fd5ee4e1010c14008662e901eef401
Author: jarred0214 <[email protected]>
AuthorDate: Fri Jul 24 16:44:50 2026 +0800
[#12169] fix(auth): clear only role policies on cache eviction (#12172)
### What changes were proposed in this pull request?
This PR updates `JcasbinLoadedRolesCache` to clear only the loaded role
policies when a role cache entry is evicted.
Instead of calling `deleteRole(roleId)`, the cache eviction now
delegates to `JcasbinAuthorizer#clearRolePolicies`, which removes only
`p(roleId, ...)` policies from the allow and deny enforcers.
The test is also updated to verify that cache invalidation removes the
role policies but preserves the user-role bindings.
### Why are the changes needed?
`loadedRoles` tracks whether a role's permission policies have been
loaded into jCasbin. It should only own role policy cleanup.
Calling `deleteRole(roleId)` also removes `g(user/group, roleId)` role
bindings. This can cause a transient authorization failure when a
request has already bound user roles, then the loaded role cache entry
expires and removes the binding before enforcement. In that case,
jCasbin may return `Hit Policy: []`, while a later retry succeeds after
the binding is added again.
Using `clearRolePolicies` keeps role policy refresh behavior while
avoiding accidental removal of user/group role bindings.
Fix: #12169
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
```bash
./gradlew spotlessApply :server-common:test --tests
org.apache.gravitino.server.authorization.jcasbin.TestJcasbinAuthorizer
-PskipITs -PskipDockerTests=false
---
.../server/authorization/jcasbin/JcasbinAuthorizer.java | 2 +-
.../authorization/jcasbin/JcasbinLoadedRolesCache.java | 14 +++++++-------
.../authorization/jcasbin/TestJcasbinAuthorizer.java | 14 +++++++++++---
3 files changed, 19 insertions(+), 11 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 ef355dea0c..2535e8e336 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
@@ -200,7 +200,7 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
// loadedRoles: roleId -> updated_at.
// When evicted, we must clean up the corresponding JCasbin policies.
- loadedRoles = new JcasbinLoadedRolesCache(ttlMs, roleCacheSize,
allowEnforcer, denyEnforcer);
+ loadedRoles = new JcasbinLoadedRolesCache(ttlMs, roleCacheSize,
this::clearRolePolicies);
userRoleCache = new CaffeineGravitinoCache<>(ttlMs, roleCacheSize);
groupRoleCache = new CaffeineGravitinoCache<>(ttlMs, roleCacheSize);
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinLoadedRolesCache.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinLoadedRolesCache.java
index 6799fe7dcc..75997b95a1 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinLoadedRolesCache.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinLoadedRolesCache.java
@@ -23,8 +23,8 @@ import com.github.benmanes.caffeine.cache.Caffeine;
import com.github.benmanes.caffeine.cache.RemovalCause;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
+import java.util.function.LongConsumer;
import org.apache.gravitino.cache.GravitinoCache;
-import org.casbin.jcasbin.main.Enforcer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -33,9 +33,10 @@ import org.slf4j.LoggerFactory;
* JCasbin policies from both enforcers when a key is evicted (by TTL, size,
or explicit
* invalidate).
*
- * <p>Uses a raw Caffeine cache internally so it can attach a removal listener
with {@code
- * executor(Runnable::run)} — eviction and policy cleanup must happen on the
same thread, so the
- * {@link JcasbinAuthorizer} never sees a role bound in the enforcer without a
backing policy.
+ * <p>This cache owns role permission policies only. Therefore, eviction must
clear only {@code
+ * p(roleId, ...)} policies and must not delete the role itself, because
JCasbin's {@code
+ * deleteRole(roleId)} also removes {@code g(user/group, roleId)} bindings
that are managed
+ * separately by {@link JcasbinAuthorizer}.
*/
class JcasbinLoadedRolesCache implements GravitinoCache<Long, Long> {
@@ -43,7 +44,7 @@ class JcasbinLoadedRolesCache implements GravitinoCache<Long,
Long> {
private final Cache<Long, Long> cache;
- JcasbinLoadedRolesCache(long ttlMs, long maxSize, Enforcer allowEnforcer,
Enforcer denyEnforcer) {
+ JcasbinLoadedRolesCache(long ttlMs, long maxSize, LongConsumer
rolePolicyCleaner) {
this.cache =
Caffeine.newBuilder()
.expireAfterAccess(ttlMs, TimeUnit.MILLISECONDS)
@@ -56,8 +57,7 @@ class JcasbinLoadedRolesCache implements GravitinoCache<Long,
Long> {
roleId,
cause);
if (roleId != null && cause != RemovalCause.REPLACED) {
- allowEnforcer.deleteRole(String.valueOf(roleId));
- denyEnforcer.deleteRole(String.valueOf(roleId));
+ rolePolicyCleaner.accept(roleId);
}
})
.build();
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 7eb04c08cb..624c547908 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
@@ -1726,25 +1726,33 @@ public class TestJcasbinAuthorizer {
// Add a role and its policy to the enforcer
Long testRoleId = 300L;
String roleIdStr = String.valueOf(testRoleId);
+ String userIdStr = String.valueOf(USER_ID);
- // Add a policy for this role
+ // Add a policy and a user-role binding for this role.
+ allowEnforcer.addRoleForUser(userIdStr, roleIdStr);
+ denyEnforcer.addRoleForUser(userIdStr, roleIdStr);
allowEnforcer.addPolicy(roleIdStr, "CATALOG", "999", "USE_CATALOG",
"allow");
denyEnforcer.addPolicy(roleIdStr, "CATALOG", "999", "USE_CATALOG",
"allow");
// Add role to cache
loadedRoles.put(testRoleId, System.currentTimeMillis());
- // Verify role exists in enforcer (has policy)
+ // Verify role exists in enforcer (has policy and grouping).
assertTrue(allowEnforcer.hasPolicy(roleIdStr, "CATALOG", "999",
"USE_CATALOG", "allow"));
assertTrue(denyEnforcer.hasPolicy(roleIdStr, "CATALOG", "999",
"USE_CATALOG", "allow"));
+ assertTrue(allowEnforcer.getRolesForUser(userIdStr).contains(roleIdStr));
+ assertTrue(denyEnforcer.getRolesForUser(userIdStr).contains(roleIdStr));
// Invalidate the cache entry - this triggers the synchronous removal
listener
// (using executor(Runnable::run) to ensure synchronous execution)
loadedRoles.invalidate(testRoleId);
- // Verify the role's policies have been deleted from enforcers
(synchronous, no need to wait)
+ // Verify the role's policies have been deleted from enforcers
(synchronous, no need to wait),
+ // but user-role bindings are preserved because loadedRoles owns role
policies only.
assertFalse(allowEnforcer.hasPolicy(roleIdStr, "CATALOG", "999",
"USE_CATALOG", "allow"));
assertFalse(denyEnforcer.hasPolicy(roleIdStr, "CATALOG", "999",
"USE_CATALOG", "allow"));
+ assertTrue(allowEnforcer.getRolesForUser(userIdStr).contains(roleIdStr));
+ assertTrue(denyEnforcer.getRolesForUser(userIdStr).contains(roleIdStr));
}
@Test
