This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new db124a5318 [Cherry-pick to branch-1.3] [#12169] fix(auth): clear only
role policies on cache eviction (#12172) (#12178)
db124a5318 is described below
commit db124a5318af508d0e522ec441918b32fc68ddab
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Jul 24 20:47:03 2026 +0800
[Cherry-pick to branch-1.3] [#12169] fix(auth): clear only role policies on
cache eviction (#12172) (#12178)
**Cherry-pick Information:**
- Original commit: a33fa0d1d7fd5ee4e1010c14008662e901eef401
- Target branch: `branch-1.3`
- Status: ✅ **Conflicts resolved**
The conflict in `JcasbinLoadedRolesCache` was resolved by keeping the
`branch-1.3` cache implementation and applying the original
`LongConsumer` policy-cleanup callback. The logging-only imports from
`main`, which are not used in `branch-1.3`, were not backported.
**Validation:**
```bash
./gradlew spotlessApply
./gradlew :server-common:test --tests
'org.apache.gravitino.server.authorization.jcasbin.TestJcasbinAuthorizer'
-PskipITs
```
Co-authored-by: jarred0214 <[email protected]>
---
.../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 25494e548a..5f3d299cc0 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
@@ -188,7 +188,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 0205ec1608..07e730761e 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,23 +23,24 @@ 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;
/**
* A {@link GravitinoCache} of {@code roleId -> updated_at} that synchronously
deletes the role's
* 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> {
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)
@@ -48,8 +49,7 @@ class JcasbinLoadedRolesCache implements GravitinoCache<Long,
Long> {
.removalListener(
(Long roleId, Long value, RemovalCause 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 4cd5bb4654..2a043e2391 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
@@ -1524,25 +1524,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
