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 5f5e2b346a [#10772] feat(authorization): refactor JcasbinAuthorizer
with version-validated cache (#10996)
5f5e2b346a is described below
commit 5f5e2b346a71ff049b8a7c3044953533d2ebe63d
Author: Qi Yu <[email protected]>
AuthorDate: Fri May 22 09:35:20 2026 +0800
[#10772] feat(authorization): refactor JcasbinAuthorizer with
version-validated cache (#10996)
### What changes were proposed in this pull request?
> **Prerequisite:** #11117 (eventual-consistency invalidation
infrastructure) must land first. This PR will be rebased on `main` after
that, at which point the diff narrows to the role-loading rewrite
described below.
This is the final piece of the #10812 split. It rewrites
`JcasbinAuthorizer`'s strong-consistency caching layer on top of the
lookups + change-poller infrastructure introduced in #11117:
- **Per-request dedup** via `AuthorizationRequestContext`: user
identity, group identity and the role-load step are deduplicated within
a single HTTP request.
- **Version-validated shared caches** (`userRoleCache`,
`groupRoleCache`, `loadedRoles`): cache hits revalidate against
`user_meta.updated_at` / `group_meta.updated_at` /
`role_meta.updated_at` before being trusted, replacing TTL-only
invalidation.
- **`JcasbinRoleLoader`** (new): owns the 4-step role load —
version-checked user-direct roles, version-checked group-inherited
roles, prune stale g-rows, then batch version-check + reload of
`role_meta`.
- **`JcasbinLoadedRolesCache`** (new): wraps a `GravitinoCache` with a
removal listener that synchronously deletes the role's JCasbin policies,
so eviction and policy state stay in sync.
- Removes the `Executor` + `CompletableFuture` async role-load path;
role loading is now synchronous via
`AuthorizationRequestContext.loadRole(Runnable)`.
- `isMetalakeUser` now takes `AuthorizationRequestContext` so the
per-request `UserUpdatedAt` cache populated by `authorize`/`isOwner` is
reused, removing a redundant DB round-trip.
### Why are the changes needed?
Fix: #10772
### Does this PR introduce _any_ user-facing change?
No. New configs (`gravitino.authorization.jcasbin.*`) have safe
defaults.
### How was this patch tested?
- 12 new unit tests across `TestGravitinoCache` (TTL, max-size eviction,
prefix edge cases) and `TestAuthorizationRequestContext` (request-scoped
dedup for user/owner/metadata-id, including absent-result caching).
- New `TestJcasbinAuthorizer` group-role / version-validated cases:
group-inherited role, IdP group removal, multi-group partial revocation,
deny-wins over group-inherited allow, role-shared-by-user-and-group
survives single-side revocation.
- All existing `TestJcasbinAuthorizer`,
`TestJcasbinAuthorizerCacheHelpers` and `TestPassThroughAuthorizer`
suites still pass.
---
.../main/java/org/apache/gravitino/Configs.java | 5 +-
.../apache/gravitino/cache/TestGravitinoCache.java | 2 +-
.../authorization/jcasbin/CachedGroupRoleRels.java | 51 ++
.../authorization/jcasbin/CachedUserRoleRels.java | 51 ++
.../jcasbin/JcasbinAuthorizationCacheKeys.java | 125 ++++
.../jcasbin/JcasbinAuthorizationLookups.java | 62 +-
.../authorization/jcasbin/JcasbinAuthorizer.java | 651 +++++++++++--------
.../authorization/jcasbin/JcasbinChangePoller.java | 4 +-
.../jcasbin/JcasbinLoadedRolesCache.java | 94 +++
.../jcasbin/TestJcasbinAuthorizationCacheKeys.java | 117 ++++
.../jcasbin/TestJcasbinAuthorizationLookups.java | 94 +--
.../jcasbin/TestJcasbinAuthorizer.java | 699 ++++++++++++---------
.../jcasbin/TestJcasbinAuthorizerCacheHelpers.java | 46 ++
.../jcasbin/TestJcasbinChangePoller.java | 22 +-
14 files changed, 1331 insertions(+), 692 deletions(-)
diff --git a/core/src/main/java/org/apache/gravitino/Configs.java
b/core/src/main/java/org/apache/gravitino/Configs.java
index 1c50d74402..a0b4ca5c89 100644
--- a/core/src/main/java/org/apache/gravitino/Configs.java
+++ b/core/src/main/java/org/apache/gravitino/Configs.java
@@ -316,7 +316,10 @@ public class Configs {
public static final ConfigEntry<Long>
GRAVITINO_AUTHORIZATION_ROLE_CACHE_SIZE =
new ConfigBuilder("gravitino.authorization.jcasbin.roleCacheSize")
- .doc("The maximum size of the role cache for authorization")
+ .doc(
+ "The maximum size of the role-related caches used by the
JcasbinAuthorizer. "
+ + "Shared by the user-role, group-role, and loaded-role
caches, so the "
+ + "effective memory footprint is up to roughly 3x this
value.")
.version(ConfigConstants.VERSION_1_1_1)
.longConf()
.createWithDefault(DEFAULT_GRAVITINO_AUTHORIZATION_ROLE_CACHE_SIZE);
diff --git
a/core/src/test/java/org/apache/gravitino/cache/TestGravitinoCache.java
b/core/src/test/java/org/apache/gravitino/cache/TestGravitinoCache.java
index 8cbf4e7232..0f5fa08678 100644
--- a/core/src/test/java/org/apache/gravitino/cache/TestGravitinoCache.java
+++ b/core/src/test/java/org/apache/gravitino/cache/TestGravitinoCache.java
@@ -238,7 +238,7 @@ public class TestGravitinoCache {
void testCaffeineInvalidateByPrefix() {
CaffeineGravitinoCache<String, Long> cache = new
CaffeineGravitinoCache<>(60_000L, 1000L);
try {
- // Simulate hierarchical keys: metalake::catalog::schema::
+ // Simulate name-path keys: metalake::catalog::schema::
cache.put("lake1::cat1::", 1L);
cache.put("lake1::cat1::s1::", 2L);
cache.put("lake1::cat1::s1::t1::TABLE", 3L);
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedGroupRoleRels.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedGroupRoleRels.java
new file mode 100644
index 0000000000..38573c96e1
--- /dev/null
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedGroupRoleRels.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.authorization.jcasbin;
+
+import java.util.List;
+
+/**
+ * Cached snapshot of a group's role relations. The {@code updatedAt}
timestamp corresponds to the
+ * {@code group_meta.updated_at} column and is used as a version sentinel: if
the DB value is newer,
+ * the cached role relation list is stale and must be reloaded.
+ */
+final class CachedGroupRoleRels {
+
+ private final long groupId;
+ private final long updatedAt;
+ private final List<Long> roleIds;
+
+ CachedGroupRoleRels(long groupId, long updatedAt, List<Long> roleIds) {
+ this.groupId = groupId;
+ this.updatedAt = updatedAt;
+ this.roleIds = roleIds;
+ }
+
+ long getGroupId() {
+ return groupId;
+ }
+
+ long getUpdatedAt() {
+ return updatedAt;
+ }
+
+ List<Long> getRoleIds() {
+ return roleIds;
+ }
+}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedUserRoleRels.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedUserRoleRels.java
new file mode 100644
index 0000000000..10e5cf43d3
--- /dev/null
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedUserRoleRels.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.authorization.jcasbin;
+
+import java.util.List;
+
+/**
+ * Cached snapshot of a user's direct role relations. The {@code updatedAt}
timestamp corresponds to
+ * the {@code user_meta.updated_at} column and is used as a version sentinel:
if the DB value is
+ * newer, the cached role relation list is stale and must be reloaded.
+ */
+final class CachedUserRoleRels {
+
+ private final long userId;
+ private final long updatedAt;
+ private final List<Long> roleIds;
+
+ CachedUserRoleRels(long userId, long updatedAt, List<Long> roleIds) {
+ this.userId = userId;
+ this.updatedAt = updatedAt;
+ this.roleIds = roleIds;
+ }
+
+ long getUserId() {
+ return userId;
+ }
+
+ long getUpdatedAt() {
+ return updatedAt;
+ }
+
+ List<Long> getRoleIds() {
+ return roleIds;
+ }
+}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationCacheKeys.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationCacheKeys.java
new file mode 100644
index 0000000000..c6d0228186
--- /dev/null
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationCacheKeys.java
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.authorization.jcasbin;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.gravitino.MetadataObject;
+
+/** Cache key factory for JCasbin authorization caches. */
+final class JcasbinAuthorizationCacheKeys {
+
+ private static final String KEY_SEPARATOR = "\u001F";
+
+ static final String USER_ROLE_REL = "USER_ROLE_REL";
+ static final String GROUP_ROLE_REL = "GROUP_ROLE_REL";
+
+ private JcasbinAuthorizationCacheKeys() {}
+
+ static String metadataIdCacheKey(String metalake, MetadataObject
metadataObject) {
+ List<String> parts = new ArrayList<>();
+ parts.add(metalake);
+
+ String[] names = metadataObject.fullName().split("\\.");
+ switch (metadataObject.type()) {
+ case METALAKE:
+ parts.add(MetadataObject.Type.METALAKE.name());
+ break;
+ case CATALOG:
+ parts.add(MetadataObject.Type.CATALOG.name());
+ parts.add(names[0]);
+ break;
+ case SCHEMA:
+ appendCatalogAndSchemas(parts, names);
+ break;
+ case TABLE:
+ appendCatalogSchemasAndLeaf(parts, names, MetadataObject.Type.TABLE);
+ break;
+ case COLUMN:
+ appendCatalogAndSchemas(parts, names, names.length - 2);
+ parts.add(MetadataObject.Type.TABLE.name());
+ parts.add(names[names.length - 2]);
+ parts.add(MetadataObject.Type.COLUMN.name());
+ parts.add(names[names.length - 1]);
+ break;
+ case VIEW:
+ case TOPIC:
+ case FILESET:
+ case MODEL:
+ case FUNCTION:
+ appendCatalogSchemasAndLeaf(parts, names, metadataObject.type());
+ break;
+ default:
+ parts.add(metadataObject.type().name());
+ parts.add(metadataObject.fullName());
+ break;
+ }
+
+ String key = String.join(KEY_SEPARATOR, parts);
+ if (hasNestedMetadataObjects(metadataObject.type())) {
+ key += KEY_SEPARATOR;
+ }
+ return key;
+ }
+
+ static String userRoleKey(String metalake, String username) {
+ return String.join(KEY_SEPARATOR, metalake, USER_ROLE_REL, username);
+ }
+
+ static String groupRoleKey(String metalake, String groupname) {
+ return String.join(KEY_SEPARATOR, metalake, GROUP_ROLE_REL, groupname);
+ }
+
+ static boolean hasNestedMetadataObjects(MetadataObject.Type type) {
+ return type == MetadataObject.Type.METALAKE
+ || type == MetadataObject.Type.CATALOG
+ || type == MetadataObject.Type.SCHEMA
+ || type == MetadataObject.Type.TABLE;
+ }
+
+ static String joinKeyParts(String... parts) {
+ return String.join(KEY_SEPARATOR, parts);
+ }
+
+ private static void appendCatalogAndSchemas(List<String> parts, String[]
names) {
+ parts.add(MetadataObject.Type.CATALOG.name());
+ parts.add(names[0]);
+ for (int i = 1; i < names.length; i++) {
+ parts.add(MetadataObject.Type.SCHEMA.name());
+ parts.add(names[i]);
+ }
+ }
+
+ private static void appendCatalogSchemasAndLeaf(
+ List<String> parts, String[] names, MetadataObject.Type leafType) {
+ appendCatalogAndSchemas(parts, names, names.length - 1);
+ parts.add(leafType.name());
+ parts.add(names[names.length - 1]);
+ }
+
+ private static void appendCatalogAndSchemas(
+ List<String> parts, String[] names, int exclusiveEnd) {
+ parts.add(MetadataObject.Type.CATALOG.name());
+ parts.add(names[0]);
+ for (int i = 1; i < exclusiveEnd; i++) {
+ parts.add(MetadataObject.Type.SCHEMA.name());
+ parts.add(names[i]);
+ }
+ }
+}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationLookups.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationLookups.java
index 1877e3e1da..6a1b30c643 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationLookups.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationLookups.java
@@ -18,7 +18,6 @@
*/
package org.apache.gravitino.server.authorization.jcasbin;
-import com.google.common.annotations.VisibleForTesting;
import java.util.Optional;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.authorization.AuthorizationRequestContext;
@@ -43,9 +42,6 @@ import
org.apache.gravitino.storage.relational.utils.SessionUtils;
*/
public class JcasbinAuthorizationLookups {
- /** Unit Separator for internal path-based cache keys. */
- static final String KEY_SEP = "\u001F";
-
private final GravitinoCache<String, Long> metadataIdCache;
private final GravitinoCache<Long, Optional<OwnerInfo>> ownerRelCache;
@@ -53,7 +49,7 @@ public class JcasbinAuthorizationLookups {
* Creates a new lookups facade around the supplied caches. The caches are
owned by the caller and
* remain accessible for invalidation by other components (poller, change
hooks).
*
- * @param metadataIdCache path-based {@code
metalake::catalog::schema::object::TYPE} → entity id
+ * @param metadataIdCache path-based metadata object key → entity id
* @param ownerRelCache {@code metadataObjectId} → {@link Optional} of
{@link OwnerInfo}
*/
public JcasbinAuthorizationLookups(
@@ -67,12 +63,14 @@ public class JcasbinAuthorizationLookups {
* Two-tier name→id lookup: the per-request map in {@code requestContext}
dedups calls within the
* same HTTP request; on a miss, the long-lived {@code metadataIdCache} is
consulted, and finally
* we fall back to a DB query via {@link MetadataIdConverter#getID}. Returns
{@link
- * Optional#empty()} when the metadata object does not exist so callers can
deny authorization; a
- * missing object is never cached as a negative result.
+ * Optional#empty()} when the metadata object does not exist so callers can
deny authorization.
+ * Missing metadata objects are never cached as negative results: a later
create for the same name
+ * can be observed without waiting for cache eviction. Existing objects are
invalidated by local
+ * name-id mapping hooks and by the change-log poller on peer nodes.
*/
public Optional<Long> resolveMetadataId(
MetadataObject metadataObject, String metalake,
AuthorizationRequestContext requestContext) {
- String cacheKey = buildCacheKey(metalake, metadataObject);
+ String cacheKey =
JcasbinAuthorizationCacheKeys.metadataIdCacheKey(metalake, metadataObject);
try {
// Both cache tiers load atomically and forbid caching null, so a
missing object is signalled
// by throwing through the loaders and translated back to
Optional.empty() here. This caches
@@ -116,52 +114,4 @@ public class JcasbinAuthorizationLookups {
return ownerInfo == null ? Optional.empty() :
Optional.of(ownerInfo);
}));
}
-
- /** Underlying metadata-id cache; exposed for invalidation by the change
hooks and the poller. */
- public GravitinoCache<String, Long> metadataIdCache() {
- return metadataIdCache;
- }
-
- /** Underlying owner cache; exposed for invalidation by the change hooks and
the poller. */
- public GravitinoCache<Long, Optional<OwnerInfo>> ownerRelCache() {
- return ownerRelCache;
- }
-
- /**
- * Builds a path-based cache key for the metadataIdCache. Container objects
end with the internal
- * separator so a prefix invalidation can remove the container and all
entries under the same name
- * path.
- *
- * <p>Examples: {@code metalake<sep>}, {@code metalake<sep>catalog<sep>},
{@code
- * metalake<sep>catalog<sep>schema<sep>}, {@code
- * metalake<sep>catalog<sep>schema<sep>table<sep>TABLE}.
- */
- @VisibleForTesting
- public static String buildCacheKey(String metalake, MetadataObject
metadataObject) {
- if (metadataObject.type() == MetadataObject.Type.METALAKE) {
- return metalake + KEY_SEP;
- }
- StringBuilder sb = new StringBuilder(metalake);
- sb.append(KEY_SEP);
- // fullName uses '.' as separator, e.g. "catalog1.schema1.table1"
- String[] parts = metadataObject.fullName().split("\\.");
- sb.append(String.join(KEY_SEP, parts));
- if (isContainerType(metadataObject.type())) {
- // Trailing separator enables prefix-based invalidation.
- sb.append(KEY_SEP);
- } else {
- // Leaf nodes get the type suffix to avoid collisions
- sb.append(KEY_SEP);
- sb.append(metadataObject.type().name());
- }
- return sb.toString();
- }
-
- /** Returns true for entity types that can contain children (metalake,
catalog, schema). */
- @VisibleForTesting
- public static boolean isContainerType(MetadataObject.Type type) {
- return type == MetadataObject.Type.METALAKE
- || type == MetadataObject.Type.CATALOG
- || type == MetadataObject.Type.SCHEMA;
- }
}
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 1f6b28aa85..da67d85eea 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
@@ -17,8 +17,6 @@
package org.apache.gravitino.server.authorization.jcasbin;
-import com.github.benmanes.caffeine.cache.Cache;
-import com.github.benmanes.caffeine.cache.Caffeine;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
@@ -29,13 +27,11 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
+import java.util.Locale;
+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;
-import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import org.apache.commons.io.IOUtils;
@@ -58,12 +54,18 @@ import org.apache.gravitino.authorization.Privilege;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.cache.CaffeineGravitinoCache;
import org.apache.gravitino.cache.GravitinoCache;
-import org.apache.gravitino.exceptions.NoSuchUserException;
import org.apache.gravitino.meta.GroupEntity;
import org.apache.gravitino.meta.RoleEntity;
-import org.apache.gravitino.meta.UserEntity;
import org.apache.gravitino.server.authorization.MetadataIdConverter;
+import org.apache.gravitino.storage.relational.mapper.GroupMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.RoleMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.UserMetaMapper;
+import org.apache.gravitino.storage.relational.po.RolePO;
+import org.apache.gravitino.storage.relational.po.auth.GroupUpdatedAt;
import org.apache.gravitino.storage.relational.po.auth.OwnerInfo;
+import org.apache.gravitino.storage.relational.po.auth.RoleUpdatedAt;
+import org.apache.gravitino.storage.relational.po.auth.UserUpdatedAt;
+import org.apache.gravitino.storage.relational.utils.SessionUtils;
import org.apache.gravitino.utils.HierarchicalSchemaUtil;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.PrincipalUtils;
@@ -73,7 +75,41 @@ import org.casbin.jcasbin.model.Model;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-/** The Jcasbin implementation of GravitinoAuthorizer. */
+/**
+ * The Jcasbin implementation of {@link GravitinoAuthorizer}.
+ *
+ * <h2>Cache architecture</h2>
+ *
+ * <p>Authorization decisions are read-mostly and run on the hot path, so this
class layers three
+ * cache families with different consistency models:
+ *
+ * <ol>
+ * <li><b>Per-request dedup</b> — fields on {@link
AuthorizationRequestContext} (user info, group
+ * info, name→id, owner). A fresh context is created for every HTTP
request; every underlying
+ * DB query runs at most once per request even when the same
authorize/isOwner pair is
+ * evaluated repeatedly for a single authorization expression.
+ * <li><b>Version-validated shared caches</b> (strong consistency) — {@link
#userRoleCache},
+ * {@link #groupRoleCache}, {@link #loadedRoles}. Each cached entry
carries the {@code
+ * *_meta.updated_at} value it was loaded against; every read issues a
lightweight version
+ * probe and discards the entry if the DB sentinel has advanced. No TTL
is relied on for
+ * correctness — TTL eviction only bounds memory. User/group role
snapshots use write-based
+ * TTLs through {@link CaffeineGravitinoCache}; loaded role policies use
access-based TTLs
+ * through {@link JcasbinLoadedRolesCache}.
+ * <li><b>Eventual-consistency caches</b> — {@link #metadataIdCache} and
{@link #ownerRelCache}. A
+ * single background poller ({@link #changePoller}) drains {@code
entity_change_log} and
+ * {@code owner_meta} change rows since a high-water-mark cursor and
invalidates the affected
+ * keys. Other Gravitino nodes therefore observe ALTER/DROP and owner
changes within one poll
+ * interval.
+ * </ol>
+ *
+ * <p>The pollers are best-effort and intentionally cheap; see {@link
JcasbinChangePoller} for the
+ * contracts they rely on (most notably that {@code
entity_change_log.full_name} is the pre-mutation
+ * name).
+ *
+ * <p>JCasbin enforcer state ({@link #allowEnforcer}/{@link #denyEnforcer}) is
kept in sync with
+ * {@link #loadedRoles} via the removal listener inside {@link
JcasbinLoadedRolesCache} — evicting a
+ * role id also deletes that role's policies from both enforcers.
+ */
public class JcasbinAuthorizer implements GravitinoAuthorizer {
private static final Logger LOG =
LoggerFactory.getLogger(JcasbinAuthorizer.class);
@@ -90,26 +126,39 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
/** deny internal authorizer */
private InternalAuthorizer denyInternalAuthorizer;
+ // ---- Version-validated caches (strong consistency) ----
+
+ /**
+ * userRoleCache: per-(metalake, userName) -> CachedUserRoleRels.
Version-validated per request
+ * via user_meta.updated_at.
+ */
+ private GravitinoCache<String, CachedUserRoleRels> userRoleCache;
+
/**
- * loadedRoles is used to cache roles that have loaded permissions. When the
permissions of a role
- * are updated, they should be removed from it.
+ * groupRoleCache: per-(metalake, groupName) -> CachedGroupRoleRels.
Version-validated per request
+ * via group_meta.updated_at.
*/
- private Cache<Long, Boolean> loadedRoles;
+ private GravitinoCache<String, CachedGroupRoleRels> groupRoleCache;
- /** Path-based {@code metalake::catalog::schema::object::TYPE} → entity id.
*/
+ /**
+ * loadedRoles: roleId -> updated_at. If the DB updated_at is newer, evict
and reload policies.
+ */
+ private GravitinoCache<Long, Long> loadedRoles;
+
+ // ---- Eventual consistency caches (poller-driven) ----
+
+ /** Path-based metadata object key -> entity id. Evicted by entity change
poller. */
private GravitinoCache<String, Long> metadataIdCache;
- /** {@code metadataObjectId} → {@link Optional} of {@link OwnerInfo}. */
+ /** ownerRelCache: metadataObjectId -> Optional(owner). Evicted by owner
change poller. */
private GravitinoCache<Long, Optional<OwnerInfo>> ownerRelCache;
- /** Two-tier lookup facade (per-request dedup + shared cache + DB fallback).
*/
+ /** Two-tier lookup facade for metadata-id / owner (per-request dedup +
Caffeine + DB). */
private JcasbinAuthorizationLookups lookups;
/** Background HA invalidator for {@link #metadataIdCache} and {@link
#ownerRelCache}. */
private JcasbinChangePoller changePoller;
- private Executor executor = null;
-
@Override
public void initialize() {
long cacheExpirationSecs =
@@ -131,25 +180,18 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
long ttlMs = TimeUnit.SECONDS.toMillis(cacheExpirationSecs);
- // Initialize enforcers before the caches that reference them in removal
listeners
+ // Initialize enforcers before caches that reference them in removal
listeners
allowEnforcer = new SyncedEnforcer(getModel("/jcasbin_model.conf"), new
GravitinoAdapter());
allowInternalAuthorizer = new InternalAuthorizer(allowEnforcer);
denyEnforcer = new SyncedEnforcer(getModel("/jcasbin_model.conf"), new
GravitinoAdapter());
denyInternalAuthorizer = new InternalAuthorizer(denyEnforcer);
- loadedRoles =
- Caffeine.newBuilder()
- .expireAfterAccess(cacheExpirationSecs, TimeUnit.SECONDS)
- .maximumSize(roleCacheSize)
- .executor(Runnable::run)
- .removalListener(
- (roleId, value, cause) -> {
- if (roleId != null) {
- allowEnforcer.deleteRole(String.valueOf(roleId));
- denyEnforcer.deleteRole(String.valueOf(roleId));
- }
- })
- .build();
+ // loadedRoles: roleId -> updated_at.
+ // When evicted, we must clean up the corresponding JCasbin policies.
+ loadedRoles = new JcasbinLoadedRolesCache(ttlMs, roleCacheSize,
allowEnforcer, denyEnforcer);
+
+ userRoleCache = new CaffeineGravitinoCache<>(ttlMs, roleCacheSize);
+ groupRoleCache = new CaffeineGravitinoCache<>(ttlMs, roleCacheSize);
// The change poller is the primary HA invalidation path. These
write-based TTLs bound the
// stale window if a poll cycle misses a change; access-based TTLs could
keep hot stale entries
// alive indefinitely.
@@ -158,16 +200,6 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
lookups = new JcasbinAuthorizationLookups(metadataIdCache, ownerRelCache);
changePoller = new JcasbinChangePoller(metadataIdCache, ownerRelCache,
pollIntervalSecs);
changePoller.start();
- executor =
- Executors.newFixedThreadPool(
- GravitinoEnv.getInstance()
- .config()
- .get(Configs.GRAVITINO_AUTHORIZATION_THREAD_POOL_SIZE),
- runnable -> {
- Thread thread = new Thread(runnable);
- thread.setName("GravitinoAuthorizer-ThreadPool-" +
thread.getId());
- return thread;
- });
}
private Model getModel(String modelFilePath) {
@@ -182,6 +214,10 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
return model;
}
+ //
---------------------------------------------------------------------------
+ // Authorize / deny / isOwner
+ //
---------------------------------------------------------------------------
+
@Override
public boolean authorize(
Principal principal,
@@ -300,7 +336,7 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
}
Optional<OwnerInfo> owner =
lookups.resolveOwnerId(metadataId.get(), metadataObject.type(),
requestContext);
- return ownerMatchesUserOrGroups(owner, principal, metalake);
+ return ownerMatchesUserOrGroups(owner, principal, metalake,
requestContext);
}
@Override
@@ -316,14 +352,10 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
if (StringUtils.isBlank(currentUserName)) {
return false;
}
-
- try {
- return
GravitinoEnv.getInstance().accessControlDispatcher().getUser(metalake,
currentUserName)
- != null;
- } catch (NoSuchUserException e) {
- LOG.warn("Can not get user {} in metalake {}", currentUserName,
metalake, e);
- return false;
- }
+ // Reuse the per-request UserUpdatedAt cache populated by
authorize/isOwner. Presence of a
+ // UserUpdatedAt entry for (metalake, user) already implies the user
exists in that metalake,
+ // so we avoid a second accessControlDispatcher().getUser() DB round-trip
per request.
+ return loadUserInfo(metalake, currentUserName, requestContext).isPresent();
}
@Override
@@ -383,14 +415,13 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
if (isOwner(currentPrincipal, metalake, metalakeObject, requestContext)) {
return true;
}
- MetadataObject.Type metadataType =
MetadataObject.Type.valueOf(type.toUpperCase());
+ MetadataObject.Type metadataType =
MetadataObject.Type.valueOf(type.toUpperCase(Locale.ROOT));
MetadataObject metadataObject =
MetadataObjects.of(Arrays.asList(fullName.split("\\.")), metadataType);
do {
if (isOwner(currentPrincipal, metalake, metadataObject, requestContext))
{
MetadataObject.Type tempType = metadataObject.type();
if (tempType == MetadataObject.Type.SCHEMA) {
- // schema owner need use catalog privilege
boolean hasCatalogUseCatalog =
authorize(
currentPrincipal,
@@ -412,7 +443,6 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
|| tempType == MetadataObject.Type.TOPIC
|| tempType == MetadataObject.Type.FILESET
|| tempType == MetadataObject.Type.MODEL) {
- // table owner need use_catalog and use_schema privileges
boolean hasMetalakeUseSchema =
authorize(
currentPrincipal,
@@ -439,7 +469,6 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
}
return true;
}
- // metadata parent owner can set owner.
} while ((metadataObject = MetadataObjects.parent(metadataObject)) !=
null);
return false;
}
@@ -448,17 +477,12 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
public boolean hasMetadataPrivilegePermission(
String metalake, String type, String fullName,
AuthorizationRequestContext requestContext) {
Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
- // Check whether the principal holds MANAGE_GRANTS on the target object or
any ancestor.
- // A grant at a broader level (e.g. CATALOG or SCHEMA) implicitly covers
all objects beneath it.
MetadataObject.Type metadataType;
try {
- metadataType = MetadataObject.Type.valueOf(type.toUpperCase());
+ metadataType =
MetadataObject.Type.valueOf(type.toUpperCase(Locale.ROOT));
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("Unknown metadata object type: " +
type, e);
}
- // Build the full ancestor chain from the target object up to and
including the metalake.
- // MetadataObjects.parent(CATALOG) returns null (CATALOG is a root in the
parent API), so the
- // metalake is appended manually at the end.
List<MetadataObject> chain = new ArrayList<>();
for (MetadataObject obj = MetadataObjects.parse(fullName, metadataType);
obj != null;
@@ -476,6 +500,10 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
return hasSetOwnerPermission(metalake, type, fullName, requestContext);
}
+ //
---------------------------------------------------------------------------
+ // Cache invalidation hooks (called from service layer)
+ //
---------------------------------------------------------------------------
+
@Override
public void handleRolePrivilegeChange(Long roleId) {
loadedRoles.invalidate(roleId);
@@ -487,7 +515,8 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
MetadataObject metadataObject =
NameIdentifierUtil.toMetadataObject(nameIdentifier, type);
// Owner mutations may happen after drop/recreate with the same name.
Invalidate the
// name->id mapping as well to prevent using a stale metadataId from
metadataIdCache.
-
metadataIdCache.invalidate(JcasbinAuthorizationLookups.buildCacheKey(metalake,
metadataObject));
+ metadataIdCache.invalidate(
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey(metalake,
metadataObject));
try {
MetadataIdConverter.getID(metadataObject,
metalake).ifPresent(ownerRelCache::invalidate);
} catch (RuntimeException e) {
@@ -499,9 +528,9 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
public void handleEntityNameIdMappingChange(
String metalake, NameIdentifier nameIdentifier, Entity.EntityType type) {
MetadataObject metadataObject =
NameIdentifierUtil.toMetadataObject(nameIdentifier, type);
- String cacheKey = JcasbinAuthorizationLookups.buildCacheKey(metalake,
metadataObject);
- if (JcasbinAuthorizationLookups.isContainerType(metadataObject.type())) {
- // Prefix invalidation: metalake::catalog:: removes catalog + all
children.
+ String cacheKey =
JcasbinAuthorizationCacheKeys.metadataIdCacheKey(metalake, metadataObject);
+ if
(JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(metadataObject.type()))
{
+ // Prefix invalidation removes the object and all nested objects under
the same name path.
metadataIdCache.invalidateByPrefix(cacheKey);
} else {
metadataIdCache.invalidate(cacheKey);
@@ -513,19 +542,14 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
if (changePoller != null) {
changePoller.close();
}
- if (executor != null) {
- if (executor instanceof ThreadPoolExecutor) {
- ThreadPoolExecutor threadPoolExecutor = (ThreadPoolExecutor) executor;
- threadPoolExecutor.shutdown();
- try {
- if (!threadPoolExecutor.awaitTermination(5, TimeUnit.SECONDS)) {
- threadPoolExecutor.shutdownNow();
- }
- } catch (InterruptedException e) {
- threadPoolExecutor.shutdownNow();
- Thread.currentThread().interrupt();
- }
- }
+ if (userRoleCache != null) {
+ userRoleCache.close();
+ }
+ if (groupRoleCache != null) {
+ groupRoleCache.close();
+ }
+ if (loadedRoles != null) {
+ loadedRoles.close();
}
if (metadataIdCache != null) {
metadataIdCache.close();
@@ -557,6 +581,10 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
return ImmutableList.copyOf(chain);
}
+ //
---------------------------------------------------------------------------
+ // Internal authorizer
+ //
---------------------------------------------------------------------------
+
private class InternalAuthorizer {
Enforcer enforcer;
@@ -581,15 +609,25 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
MetadataObject metadataObject,
String privilege,
AuthorizationRequestContext requestContext) {
- Long userId;
+ long userId;
+ UserUpdatedAt userInfo;
try {
- UserEntity userEntity = getUserEntity(username, metalake);
- userId = userEntity.id();
+ // Step 1a: lightweight query — get userId + user.updated_at (version
sentinel).
+ // Per-request dedup: only the first authorize() call for
this user hits DB.
+ Optional<UserUpdatedAt> userInfoOpt = loadUserInfo(metalake, username,
requestContext);
+ if (!userInfoOpt.isPresent()) {
+ LOG.debug("User {} not found in metalake {}", username, metalake);
+ return false;
+ }
+ userInfo = userInfoOpt.get();
+ userId = userInfo.getUserId();
} catch (Exception e) {
LOG.debug("Can not get entity id", e);
return false;
}
- loadRolePrivilege(metalake, username, userId, requestContext);
+
+ // Steps 1b→3: version-validated role loading — pass userInfo to avoid
re-query
+ loadRolePrivilege(metalake, username, userId, userInfo, requestContext);
// For requests such as CREATE SCHEMA, the metadata object may be null.
This method
// performs object-scoped authorization, so without a metadata object it
cannot evaluate
@@ -618,7 +656,7 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
* {@link #authorizeByJcasbin}. A missing object is treated as "not
authorized".
*/
private boolean authorizeObject(
- Long userId,
+ long userId,
String metalake,
MetadataObject metadataObject,
String privilege,
@@ -633,16 +671,21 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
}
private boolean authorizeByJcasbin(
- Long userId,
+ long userId,
String metalake,
MetadataObject metadataObject,
Long metadataId,
String privilege,
AuthorizationRequestContext requestContext) {
+ // Step 4: JCasbin enforce (pure in-memory) — except OWNER, which is
resolved via the
+ // owner cache rather than g-rows.
if (AuthConstants.OWNER.equals(privilege)) {
+ // Cold-path: resolveOwnerId loads from DB when neither the
per-request nor the shared
+ // Caffeine cache has the entry, ensuring the first OWNER check
doesn't spuriously deny.
Optional<OwnerInfo> owner =
lookups.resolveOwnerId(metadataId, metadataObject.type(),
requestContext);
- return ownerMatchesUserOrGroups(owner,
PrincipalUtils.getCurrentPrincipal(), metalake);
+ return ownerMatchesUserOrGroups(
+ owner, PrincipalUtils.getCurrentPrincipal(), metalake,
requestContext);
}
return enforcer.enforce(
String.valueOf(userId),
@@ -652,93 +695,206 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
}
}
- private static UserEntity getUserEntity(String username, String metalake)
throws IOException {
+ //
---------------------------------------------------------------------------
+ // User info / ownership helpers
+ //
---------------------------------------------------------------------------
+
+ /**
+ * Per-request {@link UserUpdatedAt} lookup. The underlying {@code
user_meta} query is issued at
+ * most once per (metalake, username) within a single request.
+ */
+ private Optional<UserUpdatedAt> loadUserInfo(
+ String metalake, String username, AuthorizationRequestContext
requestContext) {
+ String cacheKey = JcasbinAuthorizationCacheKeys.userRoleKey(metalake,
username);
+ return requestContext.computeUserInfoIfAbsent(
+ cacheKey,
+ k ->
+ Optional.ofNullable(
+ SessionUtils.getWithoutCommit(
+ UserMetaMapper.class, m -> m.getUserUpdatedAt(metalake,
username))));
+ }
+
+ /**
+ * Returns true when the cached owner type and ID match the given principal
or one of the
+ * principal's groups. The user id is resolved via the version-validated
{@link #loadUserInfo}
+ * cache so back-to-back ownership checks in the same request do not
re-query {@code user_meta}.
+ */
+ private boolean ownerMatchesUserOrGroups(
+ Optional<OwnerInfo> owner,
+ Principal principal,
+ String metalake,
+ AuthorizationRequestContext requestContext) {
+ if (!owner.isPresent()) {
+ return false;
+ }
+ OwnerInfo ownerInfo = owner.get();
+ if
(Entity.EntityType.USER.name().equalsIgnoreCase(ownerInfo.getOwnerType())) {
+ Optional<UserUpdatedAt> userInfo =
+ loadUserInfo(metalake, principal.getName(), requestContext);
+ return userInfo.isPresent() && userInfo.get().getUserId() ==
ownerInfo.getOwnerId();
+ }
+ if
(!Entity.EntityType.GROUP.name().equalsIgnoreCase(ownerInfo.getOwnerType())) {
+ return false;
+ }
EntityStore entityStore = GravitinoEnv.getInstance().entityStore();
- UserEntity userEntity =
- entityStore.get(
- NameIdentifierUtil.ofUser(metalake, username),
- Entity.EntityType.USER,
- UserEntity.class);
- return userEntity;
+ for (GroupEntity groupEntity : resolveCurrentUserGroups(metalake,
entityStore)) {
+ if (Objects.equals(groupEntity.id(), ownerInfo.getOwnerId())) {
+ return true;
+ }
+ }
+ return false;
}
+ //
---------------------------------------------------------------------------
+ // 4-step role loading with version validation
+ //
---------------------------------------------------------------------------
+
private void loadRolePrivilege(
- String metalake, String username, Long userId,
AuthorizationRequestContext requestContext) {
+ String metalake,
+ String username,
+ long userId,
+ UserUpdatedAt userInfo,
+ AuthorizationRequestContext requestContext) {
requestContext.loadRole(
() -> {
- EntityStore entityStore = GravitinoEnv.getInstance().entityStore();
- NameIdentifier userNameIdentifier =
NameIdentifierUtil.ofUser(metalake, username);
- List<RoleEntity> entities;
- try {
- entities =
- entityStore
- .relationOperations()
- .listEntitiesByRelation(
- SupportsRelationOperations.Type.ROLE_USER_REL,
- userNameIdentifier,
- Entity.EntityType.USER);
- List<CompletableFuture<Void>> loadRoleFutures = new ArrayList<>();
- Set<String> desiredRoleIds = new HashSet<>();
- for (RoleEntity role : entities) {
- desiredRoleIds.add(String.valueOf(role.id()));
- addRoleForUserAndLoadPolicies(
- userId,
- metalake,
- role.id(),
- role.name(),
- loadRoleFutures,
- entityStore,
- requestContext);
- }
+ // Step 1a: version-validated user-direct roles via cache.
+ List<Long> userDirectRoleIds = loadUserRoles(metalake, username,
userId, userInfo);
- // Load roles inherited from the user's groups.
- for (GroupEntity groupEntity : resolveCurrentUserGroups(metalake,
entityStore)) {
- List<Long> roleIds = groupEntity.roleIds();
- List<String> roleNames = groupEntity.roleNames();
- if (roleIds == null || roleNames == null) {
- continue;
- }
- if (roleIds.size() != roleNames.size()) {
- LOG.warn(
- "Group {} has mismatched roleIds ({}) and roleNames ({})
-- skipping",
- groupEntity.name(),
- roleIds.size(),
- roleNames.size());
- continue;
- }
- for (int i = 0; i < roleIds.size(); i++) {
- desiredRoleIds.add(String.valueOf(roleIds.get(i)));
- addRoleForUserAndLoadPolicies(
- userId,
- metalake,
- roleIds.get(i),
- roleNames.get(i),
- loadRoleFutures,
- entityStore,
- requestContext);
- }
- }
-
- CompletableFuture.allOf(loadRoleFutures.toArray(new
CompletableFuture[0])).join();
+ // Step 1b: version-validated group-inherited roles via cache. Group
membership comes
+ // from the IdP-pushed UserPrincipal; for each group we load its
roles via the same
+ // version-validated path as users (group_meta.updated_at as the
staleness sentinel).
+ List<Long> groupInheritedRoleIds = new ArrayList<>();
+ for (String groupname : currentPrincipalGroupNames()) {
+ groupInheritedRoleIds.addAll(
+ loadGroupRoles(metalake, groupname, userId, requestContext));
+ }
- // Prune stale g-rows: remove role mappings that are no longer
valid
- // (e.g. user was removed from a group at the IdP level).
- String userIdStr = String.valueOf(userId);
- for (String currentRole :
allowEnforcer.getRolesForUser(userIdStr)) {
- if (!desiredRoleIds.contains(currentRole)) {
- allowEnforcer.deleteRoleForUser(userIdStr, currentRole);
- denyEnforcer.deleteRoleForUser(userIdStr, currentRole);
- }
+ // Prune stale g-rows: any role currently bound but no longer in the
desired
+ // set (e.g. user removed from a group at the IdP, or role
unassigned).
+ Set<String> desiredRoleIds = new HashSet<>();
+ for (Long id : userDirectRoleIds) {
+ desiredRoleIds.add(String.valueOf(id));
+ }
+ for (Long id : groupInheritedRoleIds) {
+ desiredRoleIds.add(String.valueOf(id));
+ }
+ String userIdStr = String.valueOf(userId);
+ for (String currentRole : allowEnforcer.getRolesForUser(userIdStr)) {
+ if (!desiredRoleIds.contains(currentRole)) {
+ allowEnforcer.deleteRoleForUser(userIdStr, currentRole);
+ denyEnforcer.deleteRoleForUser(userIdStr, currentRole);
}
- } catch (IOException e) {
- throw new RuntimeException(e);
+ }
+
+ // Step 3: batch version-check all role IDs (direct +
group-inherited),
+ // load stale ones (1 query for the version probe).
+ List<Long> allRoleIds = new ArrayList<>(userDirectRoleIds);
+ allRoleIds.addAll(groupInheritedRoleIds);
+ if (!allRoleIds.isEmpty()) {
+ versionCheckAndLoadRoles(metalake, allRoleIds, requestContext);
}
});
}
+ private List<Long> loadUserRoles(
+ String metalake, String username, long userId, UserUpdatedAt userInfo) {
+ String userCacheKey = JcasbinAuthorizationCacheKeys.userRoleKey(metalake,
username);
+ Optional<CachedUserRoleRels> cachedOpt =
userRoleCache.getIfPresent(userCacheKey);
+
+ if (cachedOpt.isPresent()
+ && cachedOpt.get().getUserId() == userId
+ && cachedOpt.get().getUpdatedAt() >= userInfo.getUpdatedAt()) {
+ // Cache is still valid. The user id check prevents reusing roles after
deleting and
+ // recreating the same username with a new entity id.
+ CachedUserRoleRels cached = cachedOpt.get();
+ bindUserRoles(userId, cached.getRoleIds());
+ return cached.getRoleIds();
+ }
+
+ // Cache miss or stale — reload from DB
+ List<RolePO> rolePOs =
+ SessionUtils.getWithoutCommit(RoleMetaMapper.class, m ->
m.listRolesByUserId(userId));
+ List<Long> roleIds =
rolePOs.stream().map(RolePO::getRoleId).collect(Collectors.toList());
+
+ userRoleCache.put(
+ userCacheKey, new CachedUserRoleRels(userId, userInfo.getUpdatedAt(),
roleIds));
+ bindUserRoles(userId, roleIds);
+ return roleIds;
+ }
+
+ /**
+ * Per-request {@link GroupUpdatedAt} lookup, mirroring {@link
#loadUserInfo}. The {@code
+ * group_meta} probe runs at most once per (metalake, groupname) within a
single request.
+ */
+ private Optional<GroupUpdatedAt> loadGroupInfo(
+ String metalake, String groupname, AuthorizationRequestContext
requestContext) {
+ String cacheKey = JcasbinAuthorizationCacheKeys.groupRoleKey(metalake,
groupname);
+ return requestContext.computeGroupInfoIfAbsent(
+ cacheKey,
+ k ->
+ Optional.ofNullable(
+ SessionUtils.getWithoutCommit(
+ GroupMetaMapper.class, m -> m.getGroupUpdatedAt(metalake,
groupname))));
+ }
+
+ /**
+ * Version-validated group-role load, mirroring {@link #loadUserRoles}. A
cached snapshot is valid
+ * only when it belongs to the current group id and is at least as fresh as
{@code
+ * group_meta.updated_at}; the group id check prevents reusing stale roles
after a
+ * delete-and-create of the same group name. In both cases the resulting
role IDs are bound to the
+ * user's jcasbin g-rows so that the enforcer sees inherited privileges.
Groups missing from the
+ * DB return an empty list.
+ */
+ private List<Long> loadGroupRoles(
+ String metalake, String groupname, long userId,
AuthorizationRequestContext requestContext) {
+ Optional<GroupUpdatedAt> groupInfoOpt = loadGroupInfo(metalake, groupname,
requestContext);
+ if (!groupInfoOpt.isPresent()) {
+ return new ArrayList<>();
+ }
+ GroupUpdatedAt groupInfo = groupInfoOpt.get();
+ long groupId = groupInfo.getGroupId();
+ String groupCacheKey =
JcasbinAuthorizationCacheKeys.groupRoleKey(metalake, groupname);
+ Optional<CachedGroupRoleRels> cachedOpt =
groupRoleCache.getIfPresent(groupCacheKey);
+
+ if (cachedOpt.isPresent()) {
+ CachedGroupRoleRels cached = cachedOpt.get();
+ if (cached.getGroupId() == groupId && cached.getUpdatedAt() >=
groupInfo.getUpdatedAt()) {
+ bindUserRoles(userId, cached.getRoleIds());
+ return cached.getRoleIds();
+ }
+ }
+
+ List<RolePO> rolePOs =
+ SessionUtils.getWithoutCommit(RoleMetaMapper.class, m ->
m.listRolesByGroupId(groupId));
+ List<Long> roleIds =
rolePOs.stream().map(RolePO::getRoleId).collect(Collectors.toList());
+
+ groupRoleCache.put(
+ groupCacheKey, new CachedGroupRoleRels(groupId,
groupInfo.getUpdatedAt(), roleIds));
+ bindUserRoles(userId, roleIds);
+ return roleIds;
+ }
+
+ /**
+ * Returns the current principal's group names as carried by the IdP-pushed
{@link UserPrincipal}.
+ * Returns an empty list when the principal is not a {@link UserPrincipal}
(e.g. service tokens)
+ * or has no groups.
+ */
+ private List<String> currentPrincipalGroupNames() {
+ Principal principal = PrincipalUtils.getCurrentPrincipal();
+ if (!(principal instanceof UserPrincipal)) {
+ return new ArrayList<>();
+ }
+ List<UserGroup> groups = ((UserPrincipal) principal).getGroups();
+ if (groups.isEmpty()) {
+ return new ArrayList<>();
+ }
+ return
groups.stream().map(UserGroup::getGroupname).collect(Collectors.toList());
+ }
+
/**
* Resolves GroupEntity objects for the current principal's groups, skipping
any that are stale or
- * not found in the store.
+ * not found in the store. Used by {@link #isSelf} (ROLE branch) and owner
checks that need full
+ * group entities instead of only group names.
*/
private List<GroupEntity> resolveCurrentUserGroups(String metalake,
EntityStore entityStore) {
Principal principal = PrincipalUtils.getCurrentPrincipal();
@@ -756,47 +912,96 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
return entityStore.batchGet(groupIdents, Entity.EntityType.GROUP,
GroupEntity.class);
}
- /**
- * Adds a role mapping for the given user in both enforcers and
asynchronously loads the role's
- * policies if they are not already cached. When a role needs loading, the
resulting {@link
- * CompletableFuture} is appended to {@code loadRoleFutures} so the caller
can join all futures
- * after processing both direct and group-inherited roles.
- */
- private void addRoleForUserAndLoadPolicies(
- Long userId,
- String metalake,
- Long roleId,
- String roleName,
- List<CompletableFuture<Void>> loadRoleFutures,
- EntityStore entityStore,
- AuthorizationRequestContext requestContext) {
- allowEnforcer.addRoleForUser(String.valueOf(userId),
String.valueOf(roleId));
- denyEnforcer.addRoleForUser(String.valueOf(userId),
String.valueOf(roleId));
- if (loadedRoles.getIfPresent(roleId) != null) {
+ private void versionCheckAndLoadRoles(
+ String metalake, List<Long> roleIds, AuthorizationRequestContext
requestContext) {
+ // Step 3: batch fetch (roleId, roleName, updated_at) for all role IDs — 1
query
+ List<Long> uniqueRoleIds =
roleIds.stream().distinct().collect(Collectors.toList());
+ List<RoleUpdatedAt> roleVersions =
+ SessionUtils.getWithoutCommit(
+ RoleMetaMapper.class, m -> m.batchGetRoleUpdatedAt(uniqueRoleIds));
+
+ List<RoleUpdatedAt> staleRoleVersions = new ArrayList<>();
+ for (RoleUpdatedAt rv : roleVersions) {
+ Optional<Long> cachedUpdatedAt =
loadedRoles.getIfPresent(rv.getRoleId());
+
+ if (cachedUpdatedAt.isPresent() && cachedUpdatedAt.get() >=
rv.getUpdatedAt()) {
+ // Role policies are still current
+ continue;
+ }
+ staleRoleVersions.add(rv);
+ }
+
+ if (staleRoleVersions.isEmpty()) {
return;
}
- CompletableFuture<Void> loadRoleFuture =
- CompletableFuture.supplyAsync(
- () -> {
- try {
- return entityStore.get(
- NameIdentifierUtil.ofRole(metalake, roleName),
- Entity.EntityType.ROLE,
- RoleEntity.class);
- } catch (Exception e) {
- throw new RuntimeException("Failed to load role: " +
roleName, e);
- }
- },
- executor)
- .thenAcceptAsync(
- roleEntity -> {
- loadPolicyByRoleEntity(roleEntity, requestContext);
- loadedRoles.put(roleId, true);
- },
- executor);
- loadRoleFutures.add(loadRoleFuture);
+
+ EntityStore entityStore = GravitinoEnv.getInstance().entityStore();
+ List<NameIdentifier> roleIdents =
+ staleRoleVersions.stream()
+ .map(rv -> NameIdentifierUtil.ofRole(metalake, rv.getRoleName()))
+ .collect(Collectors.toList());
+ List<RoleEntity> roleEntities;
+ try {
+ roleEntities = entityStore.batchGet(roleIdents, Entity.EntityType.ROLE,
RoleEntity.class);
+ } catch (Exception e) {
+ LOG.warn("Failed to batch load stale role policies for roleIds {}",
staleRoleVersions, e);
+ roleEntities = new ArrayList<>();
+ }
+ if (roleEntities == null) {
+ roleEntities = new ArrayList<>();
+ }
+ if (roleEntities.isEmpty()) {
+ for (RoleUpdatedAt rv : staleRoleVersions) {
+ try {
+ roleEntities.add(
+ entityStore.get(
+ NameIdentifierUtil.ofRole(metalake, rv.getRoleName()),
+ Entity.EntityType.ROLE,
+ RoleEntity.class));
+ } catch (Exception e) {
+ LOG.warn("Failed to load role policies for roleId {}",
rv.getRoleId(), e);
+ }
+ }
+ }
+
+ Map<Long, RoleUpdatedAt> staleRoleVersionById =
+
staleRoleVersions.stream().collect(Collectors.toMap(RoleUpdatedAt::getRoleId,
rv -> rv));
+ for (RoleEntity roleEntity : roleEntities) {
+ RoleUpdatedAt rv = staleRoleVersionById.get(roleEntity.id());
+ if (rv == null) {
+ continue;
+ }
+ long roleId = rv.getRoleId();
+ long dbUpdatedAt = rv.getUpdatedAt();
+ Optional<Long> cachedUpdatedAt = loadedRoles.getIfPresent(roleId);
+
+ // Stale or missing: refresh only permission policies. Do not call
deleteRole here because it
+ // also removes the current user's freshly bound grouping links.
+ if (cachedUpdatedAt.isPresent()) {
+ clearRolePolicies(roleId);
+ }
+ loadPolicyByRoleEntity(roleEntity, requestContext);
+ loadedRoles.put(roleId, dbUpdatedAt);
+ }
+ }
+
+ private void clearRolePolicies(long roleId) {
+ String roleIdStr = String.valueOf(roleId);
+ allowEnforcer.removeFilteredPolicy(0, roleIdStr);
+ denyEnforcer.removeFilteredPolicy(0, roleIdStr);
}
+ private void bindUserRoles(long userId, List<Long> roleIds) {
+ for (Long roleId : roleIds) {
+ allowEnforcer.addRoleForUser(String.valueOf(userId),
String.valueOf(roleId));
+ denyEnforcer.addRoleForUser(String.valueOf(userId),
String.valueOf(roleId));
+ }
+ }
+
+ //
---------------------------------------------------------------------------
+ // Policy loading from role entity
+ //
---------------------------------------------------------------------------
+
private void loadPolicyByRoleEntity(
RoleEntity roleEntity, AuthorizationRequestContext requestContext) {
String metalake =
NameIdentifierUtil.getMetalake(roleEntity.nameIdentifier());
@@ -818,16 +1023,9 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
String.valueOf(metadataId.get()),
AuthorizationUtils.replaceLegacyPrivilegeName(privilege.name())
.name()
- .toUpperCase(java.util.Locale.ROOT),
+ .toUpperCase(Locale.ROOT),
AuthConstants.ALLOW);
}
- // Since different roles of a user may simultaneously hold both
"allow" and "deny"
- // permissions
- // for the same privilege on a given MetadataObject, the allowEnforcer
must also incorporate
- // the "deny" privilege to ensure that the authorize method correctly
returns false in such
- // cases. For example, if role1 has an "allow" privilege for
SELECT_TABLE on table1, while
- // role2 has a "deny" privilege for the same action on table1, then a
user assigned both
- // roles should receive a false result when calling the authorize
method.
allowEnforcer.addPolicy(
String.valueOf(roleEntity.id()),
@@ -835,56 +1033,9 @@ public class JcasbinAuthorizer implements
GravitinoAuthorizer {
String.valueOf(metadataId.get()),
AuthorizationUtils.replaceLegacyPrivilegeName(privilege.name())
.name()
- .toUpperCase(java.util.Locale.ROOT),
- condition.name().toLowerCase(java.util.Locale.ROOT));
+ .toUpperCase(Locale.ROOT),
+ condition.name().toLowerCase(Locale.ROOT));
}
}
}
-
- /**
- * Returns true when the resolved owner matches the current user (by id) or
one of the user's
- * groups. We compare by entity id rather than name so the cached snapshot
survives a
- * delete-then-recreate-with-same-name scenario without spuriously granting
ownership to the new
- * entity.
- */
- private boolean ownerMatchesUserOrGroups(
- Optional<OwnerInfo> owner, Principal principal, String metalake) {
- if (!owner.isPresent()) {
- return false;
- }
- OwnerInfo ownerInfo = owner.get();
- if
(Entity.EntityType.USER.name().equalsIgnoreCase(ownerInfo.getOwnerType())) {
- try {
- UserEntity userEntity = getUserEntity(principal.getName(), metalake);
- return Objects.equals(userEntity.id(), ownerInfo.getOwnerId());
- } catch (Exception e) {
- LOG.debug("Can not get user entity for ownership check", e);
- return false;
- }
- }
- if
(!Entity.EntityType.GROUP.name().equalsIgnoreCase(ownerInfo.getOwnerType())) {
- return false;
- }
- if (!(principal instanceof UserPrincipal)) {
- return false;
- }
- List<UserGroup> groups = ((UserPrincipal) principal).getGroups();
- if (groups.isEmpty()) {
- return false;
- }
- try {
- List<NameIdentifier> groupIdents =
- groups.stream()
- .map(g -> NameIdentifierUtil.ofGroup(metalake, g.getGroupname()))
- .collect(Collectors.toList());
- List<GroupEntity> groupEntities =
- GravitinoEnv.getInstance()
- .entityStore()
- .batchGet(groupIdents, Entity.EntityType.GROUP,
GroupEntity.class);
- return groupEntities.stream().anyMatch(ge -> Objects.equals(ge.id(),
ownerInfo.getOwnerId()));
- } catch (Exception e) {
- LOG.debug("Can not get group entities for ownership check", e);
- return false;
- }
- }
}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangePoller.java
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangePoller.java
index 611cc9616b..5399e45f24 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangePoller.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinChangePoller.java
@@ -242,9 +242,9 @@ public class JcasbinChangePoller implements AutoCloseable {
}
MetadataObject mdObj = metadataObjectFromChangeLog(metalake, fullName,
mdType);
- String cacheKey = JcasbinAuthorizationLookups.buildCacheKey(metalake,
mdObj);
+ String cacheKey =
JcasbinAuthorizationCacheKeys.metadataIdCacheKey(metalake, mdObj);
- if (JcasbinAuthorizationLookups.isContainerType(mdType)) {
+ if (JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(mdType)) {
addCoalescedPrefix(containerPrefixes, cacheKey);
} else {
leafKeys.add(cacheKey);
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
new file mode 100644
index 0000000000..0205ec1608
--- /dev/null
+++
b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinLoadedRolesCache.java
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.authorization.jcasbin;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import com.github.benmanes.caffeine.cache.RemovalCause;
+import java.util.Optional;
+import java.util.concurrent.TimeUnit;
+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.
+ */
+class JcasbinLoadedRolesCache implements GravitinoCache<Long, Long> {
+
+ private final Cache<Long, Long> cache;
+
+ JcasbinLoadedRolesCache(long ttlMs, long maxSize, Enforcer allowEnforcer,
Enforcer denyEnforcer) {
+ this.cache =
+ Caffeine.newBuilder()
+ .expireAfterAccess(ttlMs, TimeUnit.MILLISECONDS)
+ .maximumSize(maxSize)
+ .executor(Runnable::run)
+ .removalListener(
+ (Long roleId, Long value, RemovalCause cause) -> {
+ if (roleId != null && cause != RemovalCause.REPLACED) {
+ allowEnforcer.deleteRole(String.valueOf(roleId));
+ denyEnforcer.deleteRole(String.valueOf(roleId));
+ }
+ })
+ .build();
+ }
+
+ @Override
+ public Optional<Long> getIfPresent(Long key) {
+ return Optional.ofNullable(cache.getIfPresent(key));
+ }
+
+ @Override
+ public void put(Long key, Long value) {
+ cache.put(key, value);
+ }
+
+ @Override
+ public void invalidate(Long key) {
+ cache.invalidate(key);
+ }
+
+ @Override
+ public void invalidateAll() {
+ cache.invalidateAll();
+ }
+
+ @Override
+ public void invalidateByPrefix(String prefix) {
+ // Role ids are Long keys, so prefix invalidation is not meaningful for
this cache.
+ }
+
+ @Override
+ public long size() {
+ cache.cleanUp();
+ return cache.estimatedSize();
+ }
+
+ @Override
+ public void close() {
+ cache.invalidateAll();
+ cache.cleanUp();
+ }
+}
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationCacheKeys.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationCacheKeys.java
new file mode 100644
index 0000000000..1bc0afc38d
--- /dev/null
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationCacheKeys.java
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.authorization.jcasbin;
+
+import java.util.Arrays;
+import java.util.Collections;
+import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.MetadataObjects;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Tests for {@link JcasbinAuthorizationCacheKeys}. */
+public class TestJcasbinAuthorizationCacheKeys {
+
+ @Test
+ void testMetadataIdCacheKey() {
+ MetadataObject metalake =
+ MetadataObjects.of(Collections.singletonList("ml1"),
MetadataObject.Type.METALAKE);
+ Assertions.assertEquals(
+ key("ml1", "METALAKE", ""),
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1", metalake));
+
+ MetadataObject catalog =
+ MetadataObjects.of(Collections.singletonList("cat1"),
MetadataObject.Type.CATALOG);
+ String catalogKey =
JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1", catalog);
+ Assertions.assertEquals(key("ml1", "CATALOG", "cat1", ""), catalogKey);
+
+ MetadataObject schema =
+ MetadataObjects.of(Arrays.asList("cat1", "sch1"),
MetadataObject.Type.SCHEMA);
+ String schemaKey = JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1",
schema);
+ Assertions.assertEquals(key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1",
""), schemaKey);
+
+ MetadataObject table =
+ MetadataObjects.of(Arrays.asList("cat1", "sch1", "tbl1"),
MetadataObject.Type.TABLE);
+ String tableKey = JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1",
table);
+ Assertions.assertEquals(
+ key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1", "TABLE", "tbl1", ""),
tableKey);
+
+ MetadataObject column =
+ MetadataObjects.of(
+ Arrays.asList("cat1", "sch1", "tbl1", "col1"),
MetadataObject.Type.COLUMN);
+ String columnKey = JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1",
column);
+ Assertions.assertEquals(
+ key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1", "TABLE", "tbl1",
"COLUMN", "col1"),
+ columnKey);
+
+ MetadataObject view =
+ MetadataObjects.of(Arrays.asList("cat1", "sch1", "tbl1"),
MetadataObject.Type.VIEW);
+ String viewKey = JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1",
view);
+ Assertions.assertEquals(
+ key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1", "VIEW", "tbl1"),
viewKey);
+
+ Assertions.assertTrue(schemaKey.startsWith(catalogKey));
+ Assertions.assertTrue(tableKey.startsWith(schemaKey));
+ Assertions.assertTrue(columnKey.startsWith(tableKey));
+ Assertions.assertFalse(viewKey.startsWith(tableKey));
+ }
+
+ @Test
+ void testHasNestedMetadataObjects() {
+ Assertions.assertTrue(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.METALAKE));
+ Assertions.assertTrue(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.CATALOG));
+ Assertions.assertTrue(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.SCHEMA));
+ Assertions.assertTrue(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.TABLE));
+
+ Assertions.assertFalse(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.VIEW));
+ Assertions.assertFalse(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.FILESET));
+ Assertions.assertFalse(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.TOPIC));
+ Assertions.assertFalse(
+
JcasbinAuthorizationCacheKeys.hasNestedMetadataObjects(MetadataObject.Type.COLUMN));
+ }
+
+ @Test
+ void testPrincipalRoleKeysAreTyped() {
+ Assertions.assertEquals(
+ key("ml1", JcasbinAuthorizationCacheKeys.USER_ROLE_REL, "alice"),
+ JcasbinAuthorizationCacheKeys.userRoleKey("ml1", "alice"));
+ Assertions.assertEquals(
+ key("ml1", JcasbinAuthorizationCacheKeys.GROUP_ROLE_REL, "admins"),
+ JcasbinAuthorizationCacheKeys.groupRoleKey("ml1", "admins"));
+ }
+
+ @Test
+ void testPrincipalRoleKeysAreDistinct() {
+ String userKey = JcasbinAuthorizationCacheKeys.userRoleKey("ml1", "alice");
+ String groupKey = JcasbinAuthorizationCacheKeys.groupRoleKey("ml1",
"alice");
+
+ Assertions.assertNotEquals(userKey, groupKey);
+ }
+
+ private static String key(String... parts) {
+ return JcasbinAuthorizationCacheKeys.joinKeyParts(parts);
+ }
+}
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationLookups.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationLookups.java
index afbf9f3685..b41035c857 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationLookups.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationLookups.java
@@ -19,7 +19,6 @@
package org.apache.gravitino.server.authorization.jcasbin;
import java.util.Arrays;
-import java.util.Collections;
import java.util.Optional;
import java.util.function.Function;
import org.apache.gravitino.MetadataObject;
@@ -30,96 +29,9 @@ import
org.apache.gravitino.storage.relational.po.auth.OwnerInfo;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
-/** Tests for {@link JcasbinAuthorizationLookups} static helpers. */
+/** Tests for {@link JcasbinAuthorizationLookups}. */
public class TestJcasbinAuthorizationLookups {
- // ---------- buildCacheKey ----------
-
- @Test
- void testBuildCacheKeyMetalake() {
- MetadataObject obj =
- MetadataObjects.of(Collections.singletonList("ml1"),
MetadataObject.Type.METALAKE);
- Assertions.assertEquals(key("ml1", ""),
JcasbinAuthorizationLookups.buildCacheKey("ml1", obj));
- }
-
- @Test
- void testBuildCacheKeyCatalog() {
- MetadataObject obj =
- MetadataObjects.of(Collections.singletonList("cat1"),
MetadataObject.Type.CATALOG);
- Assertions.assertEquals(
- key("ml1", "cat1", ""),
JcasbinAuthorizationLookups.buildCacheKey("ml1", obj));
- }
-
- @Test
- void testBuildCacheKeySchema() {
- MetadataObject obj =
- MetadataObjects.of(Arrays.asList("cat1", "sch1"),
MetadataObject.Type.SCHEMA);
- Assertions.assertEquals(
- key("ml1", "cat1", "sch1", ""),
JcasbinAuthorizationLookups.buildCacheKey("ml1", obj));
- }
-
- @Test
- void testBuildCacheKeyLeafTypesGetTypeSuffix() {
- MetadataObject table =
- MetadataObjects.of(Arrays.asList("cat1", "sch1", "tbl1"),
MetadataObject.Type.TABLE);
- Assertions.assertEquals(
- key("ml1", "cat1", "sch1", "tbl1", "TABLE"),
- JcasbinAuthorizationLookups.buildCacheKey("ml1", table));
-
- MetadataObject view =
- MetadataObjects.of(Arrays.asList("cat1", "sch1", "v1"),
MetadataObject.Type.VIEW);
- Assertions.assertEquals(
- key("ml1", "cat1", "sch1", "v1", "VIEW"),
- JcasbinAuthorizationLookups.buildCacheKey("ml1", view));
-
- MetadataObject fileset =
- MetadataObjects.of(Arrays.asList("cat1", "sch1", "fs1"),
MetadataObject.Type.FILESET);
- Assertions.assertEquals(
- key("ml1", "cat1", "sch1", "fs1", "FILESET"),
- JcasbinAuthorizationLookups.buildCacheKey("ml1", fileset));
- }
-
- // ---------- isContainerType ----------
-
- @Test
- void testIsContainerTypeContainerTypes() {
- Assertions.assertTrue(
-
JcasbinAuthorizationLookups.isContainerType(MetadataObject.Type.METALAKE));
-
Assertions.assertTrue(JcasbinAuthorizationLookups.isContainerType(MetadataObject.Type.CATALOG));
-
Assertions.assertTrue(JcasbinAuthorizationLookups.isContainerType(MetadataObject.Type.SCHEMA));
- }
-
- @Test
- void testIsContainerTypeLeafTypes() {
-
Assertions.assertFalse(JcasbinAuthorizationLookups.isContainerType(MetadataObject.Type.TABLE));
-
Assertions.assertFalse(JcasbinAuthorizationLookups.isContainerType(MetadataObject.Type.VIEW));
- Assertions.assertFalse(
-
JcasbinAuthorizationLookups.isContainerType(MetadataObject.Type.FILESET));
-
Assertions.assertFalse(JcasbinAuthorizationLookups.isContainerType(MetadataObject.Type.TOPIC));
- }
-
- // ---------- Prefix invalidation ----------
-
- @Test
- void testPrefixInvalidationCoversContainerPath() {
- // Dropping a catalog should use a prefix that covers all schemas and
tables below it.
- MetadataObject catalog =
- MetadataObjects.of(Collections.singletonList("cat1"),
MetadataObject.Type.CATALOG);
- String catalogKey = JcasbinAuthorizationLookups.buildCacheKey("ml1",
catalog);
-
- MetadataObject schema =
- MetadataObjects.of(Arrays.asList("cat1", "sch1"),
MetadataObject.Type.SCHEMA);
- String schemaKey = JcasbinAuthorizationLookups.buildCacheKey("ml1",
schema);
-
- MetadataObject table =
- MetadataObjects.of(Arrays.asList("cat1", "sch1", "tbl1"),
MetadataObject.Type.TABLE);
- String tableKey = JcasbinAuthorizationLookups.buildCacheKey("ml1", table);
-
- Assertions.assertTrue(schemaKey.startsWith(catalogKey));
- Assertions.assertTrue(tableKey.startsWith(catalogKey));
- Assertions.assertTrue(tableKey.startsWith(schemaKey));
- }
-
@Test
void testResolveMetadataIdUsesAtomicSharedCacheAndRequestDedup() {
MetadataObject table =
@@ -158,10 +70,6 @@ public class TestJcasbinAuthorizationLookups {
Assertions.assertEquals(0, ownerRelCache.putCount);
}
- private static String key(String... parts) {
- return String.join(JcasbinAuthorizationLookups.KEY_SEP, parts);
- }
-
private static class CountingCache<K, V> implements GravitinoCache<K, V> {
private final V value;
private int getCount;
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 e1dea19922..dba8697088 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
@@ -20,35 +20,35 @@ package org.apache.gravitino.server.authorization.jcasbin;
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;
-import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
-import com.github.benmanes.caffeine.cache.Cache;
import com.google.common.collect.ImmutableList;
import java.io.IOException;
import java.lang.reflect.Field;
+import java.lang.reflect.Method;
import java.security.Principal;
+import java.util.ArrayList;
import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
-import java.util.concurrent.Executor;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Function;
import java.util.stream.Collectors;
-import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
@@ -72,9 +72,16 @@ import org.apache.gravitino.meta.UserEntity;
import org.apache.gravitino.server.ServerConfig;
import org.apache.gravitino.server.authorization.MetadataIdConverter;
import org.apache.gravitino.storage.relational.mapper.EntityChangeLogMapper;
+import org.apache.gravitino.storage.relational.mapper.GroupMetaMapper;
import org.apache.gravitino.storage.relational.mapper.OwnerMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.RoleMetaMapper;
+import org.apache.gravitino.storage.relational.mapper.UserMetaMapper;
+import org.apache.gravitino.storage.relational.po.RolePO;
import org.apache.gravitino.storage.relational.po.SecurableObjectPO;
+import org.apache.gravitino.storage.relational.po.auth.GroupUpdatedAt;
import org.apache.gravitino.storage.relational.po.auth.OwnerInfo;
+import org.apache.gravitino.storage.relational.po.auth.RoleUpdatedAt;
+import org.apache.gravitino.storage.relational.po.auth.UserUpdatedAt;
import org.apache.gravitino.storage.relational.service.OwnerMetaService;
import org.apache.gravitino.storage.relational.utils.POConverters;
import org.apache.gravitino.storage.relational.utils.SessionUtils;
@@ -83,6 +90,7 @@ import org.apache.gravitino.utils.NamespaceUtil;
import org.apache.gravitino.utils.PrincipalUtils;
import org.casbin.jcasbin.main.Enforcer;
import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
@@ -127,11 +135,38 @@ public class TestJcasbinAuthorizer {
private static MockedStatic<SessionUtils> sessionUtilsMockedStatic;
+ private static UserMetaMapper userMetaMapper = mock(UserMetaMapper.class);
+
+ private static GroupMetaMapper groupMetaMapper = mock(GroupMetaMapper.class);
+
+ private static RoleMetaMapper roleMetaMapper = mock(RoleMetaMapper.class);
+
private static OwnerMetaMapper ownerMetaMapper = mock(OwnerMetaMapper.class);
private static EntityChangeLogMapper entityChangeLogMapper =
mock(EntityChangeLogMapper.class);
- private static JcasbinAuthorizer jcasbinAuthorizer;
+ /**
+ * Tracks roles registered via {@link #mockRoleInStore} so {@code
+ * roleMetaMapper.batchGetRoleUpdatedAt} can return their versions on demand.
+ */
+ private static final Map<Long, RoleUpdatedAt> mockedRoleVersions = new
HashMap<>();
+
+ /**
+ * Monotonic counter for {@code group_meta.updated_at} mocks so that
successive {@link
+ * #mockGroupWithRoles} calls always advance the version, forcing the
groupRoleCache to miss even
+ * when the wall clock hasn't advanced.
+ */
+ private static final AtomicLong groupVersionCounter = new AtomicLong(1L);
+
+ private static final AtomicLong roleVersionCounter = new AtomicLong(1L);
+
+ private static final AtomicLong userVersionCounter = new AtomicLong(1L);
+
+ /**
+ * Recreated per test in {@link #createAuthorizer()} so each case starts
with empty enforcer state
+ * and a fresh cache; the previous static instance leaked g-rows and cache
entries across cases.
+ */
+ private JcasbinAuthorizer jcasbinAuthorizer;
private static ObjectMapper objectMapper = new ObjectMapper();
@@ -156,7 +191,13 @@ public class TestJcasbinAuthorizer {
invocation -> {
Class<?> mapperClass = invocation.getArgument(0);
Function<Object, Object> func = invocation.getArgument(1);
- if (mapperClass == OwnerMetaMapper.class) {
+ if (mapperClass == UserMetaMapper.class) {
+ return func.apply(userMetaMapper);
+ } else if (mapperClass == GroupMetaMapper.class) {
+ return func.apply(groupMetaMapper);
+ } else if (mapperClass == RoleMetaMapper.class) {
+ return func.apply(roleMetaMapper);
+ } else if (mapperClass == OwnerMetaMapper.class) {
return func.apply(ownerMetaMapper);
}
if (mapperClass == EntityChangeLogMapper.class) {
@@ -165,6 +206,28 @@ public class TestJcasbinAuthorizer {
return null;
});
+ // Default mock: getUserInfo returns a valid user
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, 1000L));
+
+ // Default: no roles assigned initially
+
when(roleMetaMapper.listRolesByUserId(eq(USER_ID))).thenReturn(ImmutableList.of());
+ // Default answer pulls versions from mockedRoleVersions, populated by
mockRoleInStore.
+ // Use doAnswer to avoid eager invocation of any previous stub when
re-stubbing.
+ doAnswer(
+ invocation -> {
+ List<Long> ids = invocation.getArgument(0);
+ if (ids == null) {
+ return ImmutableList.of();
+ }
+ return ids.stream()
+ .map(mockedRoleVersions::get)
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList());
+ })
+ .when(roleMetaMapper)
+ .batchGetRoleUpdatedAt(any());
+
gravitinoEnvMockedStatic = mockStatic(GravitinoEnv.class);
gravitinoEnvMockedStatic.when(GravitinoEnv::getInstance).thenReturn(gravitinoEnv);
when(gravitinoEnv.config()).thenReturn(new ServerConfig());
@@ -185,8 +248,6 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.USER),
eq(UserEntity.class)))
.thenReturn(getUserEntity());
- jcasbinAuthorizer = new JcasbinAuthorizer();
- jcasbinAuthorizer.initialize();
BaseMetalake baseMetalake =
BaseMetalake.builder()
.withId(USER_METALAKE_ID)
@@ -203,34 +264,30 @@ public class TestJcasbinAuthorizer {
@AfterAll
public static void stop() {
- if (jcasbinAuthorizer != null) {
- try {
- jcasbinAuthorizer.close();
- } catch (IOException e) {
- throw new RuntimeException("Failed to close JcasbinAuthorizer", e);
- }
- }
+ // jcasbinAuthorizer is per-test (see @AfterEach); only static mocks need
cleanup here.
if (principalUtilsMockedStatic != null) {
principalUtilsMockedStatic.close();
}
if (metadataIdConverterMockedStatic != null) {
metadataIdConverterMockedStatic.close();
}
- if (ownerMetaServiceMockedStatic != null) {
- ownerMetaServiceMockedStatic.close();
- }
if (sessionUtilsMockedStatic != null) {
sessionUtilsMockedStatic.close();
}
+ if (ownerMetaServiceMockedStatic != null) {
+ ownerMetaServiceMockedStatic.close();
+ }
if (gravitinoEnvMockedStatic != null) {
gravitinoEnvMockedStatic.close();
}
}
@BeforeEach
- public void resetSharedState() throws Exception {
- // Reset shared enforcer and cache state to prevent test ordering
contamination.
- getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ public void createAuthorizer() throws Exception {
+ // Build a fresh authorizer per test so enforcer g-rows and
version-validated cache state can
+ // never bleed across cases regardless of the JUnit execution order.
+ jcasbinAuthorizer = new JcasbinAuthorizer();
+ jcasbinAuthorizer.initialize();
restoreDefaultPrincipal();
// Reset role-user relation mock to return empty list (no roles) by
default; individual tests
// can override as needed.
@@ -240,13 +297,39 @@ public class TestJcasbinAuthorizer {
eq(userNameIdentifier),
eq(Entity.EntityType.USER)))
.thenReturn(ImmutableList.of());
+ // Reset role version map and re-stub the answer to keep tests isolated
from each other.
+ mockedRoleVersions.clear();
+ doAnswer(
+ invocation -> {
+ List<Long> ids = invocation.getArgument(0);
+ if (ids == null) {
+ return ImmutableList.of();
+ }
+ return ids.stream()
+ .map(mockedRoleVersions::get)
+ .filter(Objects::nonNull)
+ .collect(Collectors.toList());
+ })
+ .when(roleMetaMapper)
+ .batchGetRoleUpdatedAt(any());
+ }
+
+ @AfterEach
+ public void closeAuthorizer() throws IOException {
+ if (jcasbinAuthorizer != null) {
+ jcasbinAuthorizer.close();
+ jcasbinAuthorizer = null;
+ }
}
@Test
public void testAuthorize() throws Exception {
makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
+ // No roles assigned — should fail
assertFalse(doAuthorize(currentPrincipal));
+
+ // Set up allowRole
RoleEntity allowRole =
getRoleEntity(ALLOW_ROLE_ID, "allowRole",
ImmutableList.of(getAllowSecurableObject()));
when(entityStore.get(
@@ -254,14 +337,19 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.ROLE),
eq(RoleEntity.class)))
.thenReturn(allowRole);
- NameIdentifier userNameIdentifier = NameIdentifierUtil.ofUser(METALAKE,
USERNAME);
- // Mock adds roles to users.
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(allowRole));
+
+ // Mock mapper: user has allowRole
+ long roleVersion = nextRoleVersion();
+ RolePO allowRolePO = buildRolePO(ALLOW_ROLE_ID, "allowRole");
+
when(roleMetaMapper.listRolesByUserId(eq(USER_ID))).thenReturn(ImmutableList.of(allowRolePO));
+ when(roleMetaMapper.batchGetRoleUpdatedAt(any()))
+ .thenReturn(ImmutableList.of(new RoleUpdatedAt(ALLOW_ROLE_ID,
"allowRole", roleVersion)));
+ // Bump user version to invalidate userRoleCache
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
+
assertTrue(doAuthorize(currentPrincipal));
+
// Test role cache.
// When the user's role changes to one with no privileges, the prune step
removes
// the stale role's g-rows from the enforcer, so authorization fails
immediately.
@@ -272,24 +360,28 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.ROLE),
eq(RoleEntity.class)))
.thenReturn(tempNewRole);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(tempNewRole));
+ RolePO tempNewRolePO = buildRolePO(newRoleId, "tempNewRole");
+
when(roleMetaMapper.listRolesByUserId(eq(USER_ID))).thenReturn(ImmutableList.of(tempNewRolePO));
+ long roleVersion2 = nextRoleVersion();
+ when(roleMetaMapper.batchGetRoleUpdatedAt(any()))
+ .thenReturn(ImmutableList.of(new RoleUpdatedAt(newRoleId,
"tempNewRole", roleVersion2)));
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
+ // tempNewRole has no privileges; prune step removes stale allowRole
g-row, so authz fails.
assertFalse(doAuthorize(currentPrincipal));
- // When the user is re-assigned the correct role, the authorization will
succeed.
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(allowRole));
- when(entityStore.get(
- eq(NameIdentifierUtil.ofRole(METALAKE, allowRole.name())),
- eq(Entity.EntityType.ROLE),
- eq(RoleEntity.class)))
- .thenReturn(allowRole);
+
+ // After clearing the role policy cache, the next authorize forces a
reload.
+ jcasbinAuthorizer.handleRolePrivilegeChange(ALLOW_ROLE_ID);
+
+ // Re-assign allowRole, the authorization will succeed
+
when(roleMetaMapper.listRolesByUserId(eq(USER_ID))).thenReturn(ImmutableList.of(allowRolePO));
+ long roleVersion3 = nextRoleVersion();
+ when(roleMetaMapper.batchGetRoleUpdatedAt(any()))
+ .thenReturn(ImmutableList.of(new RoleUpdatedAt(ALLOW_ROLE_ID,
"allowRole", roleVersion3)));
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
assertTrue(doAuthorize(currentPrincipal));
+
// Test deny
RoleEntity denyRole =
getRoleEntity(DENY_ROLE_ID, "denyRole",
ImmutableList.of(getDenySecurableObject()));
@@ -298,20 +390,49 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.ROLE),
eq(RoleEntity.class)))
.thenReturn(denyRole);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(allowRole, denyRole));
+ RolePO denyRolePO = buildRolePO(DENY_ROLE_ID, "denyRole");
+ when(roleMetaMapper.listRolesByUserId(eq(USER_ID)))
+ .thenReturn(ImmutableList.of(allowRolePO, denyRolePO));
+ long roleVersion4 = nextRoleVersion();
+ when(roleMetaMapper.batchGetRoleUpdatedAt(any()))
+ .thenReturn(
+ ImmutableList.of(
+ new RoleUpdatedAt(ALLOW_ROLE_ID, "allowRole", roleVersion4),
+ new RoleUpdatedAt(DENY_ROLE_ID, "denyRole", roleVersion4)));
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
+ assertFalse(doAuthorize(currentPrincipal));
+ }
+
+ @Test
+ public void testUserRoleCacheDoesNotReuseRolesAfterUsernameRecreate() throws
Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
+
+ RoleEntity allowRole =
+ mockRoleInStore(
+ ALLOW_ROLE_ID,
+ "allowRoleBeforeUserRecreate",
+ ImmutableList.of(getAllowSecurableObject()));
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, 1000L));
+ when(roleMetaMapper.listRolesByUserId(eq(USER_ID)))
+ .thenReturn(ImmutableList.of(buildRolePO(allowRole.id(),
allowRole.name())));
+
+ assertTrue(doAuthorize(currentPrincipal));
+
+ long recreatedUserId = USER_ID + 1000L;
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(recreatedUserId, 0L));
+
when(roleMetaMapper.listRolesByUserId(eq(recreatedUserId))).thenReturn(ImmutableList.of());
assertFalse(doAuthorize(currentPrincipal));
+ verify(roleMetaMapper).listRolesByUserId(eq(recreatedUserId));
}
@Test
public void testAuthorizeByOwner() throws Exception {
Principal currentPrincipal = PrincipalUtils.getCurrentPrincipal();
- NameIdentifier catalogIdent = NameIdentifierUtil.ofCatalog(METALAKE,
"testCatalog");
-
// No owner set — should fail
when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(CATALOG_ID),
eq("CATALOG")))
.thenReturn(null);
@@ -319,14 +440,23 @@ public class TestJcasbinAuthorizer {
assertFalse(doAuthorizeOwner(currentPrincipal));
// Set owner to current user
+ OwnerInfo ownerInfo = new OwnerInfo(USER_ID, "USER");
when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(CATALOG_ID),
eq("CATALOG")))
- .thenReturn(new OwnerInfo(USER_ID, "USER"));
+ .thenReturn(ownerInfo);
getOwnerRelCache(jcasbinAuthorizer).invalidateAll();
assertTrue(doAuthorizeOwner(currentPrincipal));
- // Remove owner via change hook
+ // Matching ID with a GROUP owner type must not grant user ownership.
+ OwnerInfo collidingGroupOwnerInfo = new OwnerInfo(USER_ID, "GROUP");
+ when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(CATALOG_ID),
eq("CATALOG")))
+ .thenReturn(collidingGroupOwnerInfo);
+ getOwnerRelCache(jcasbinAuthorizer).invalidateAll();
+ assertFalse(doAuthorizeOwner(currentPrincipal));
+
+ // Remove owner
when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(CATALOG_ID),
eq("CATALOG")))
.thenReturn(null);
+ NameIdentifier catalogIdent = NameIdentifierUtil.ofCatalog(METALAKE,
"testCatalog");
jcasbinAuthorizer.handleMetadataOwnerChange(
METALAKE, USER_ID, catalogIdent, Entity.EntityType.CATALOG);
assertFalse(doAuthorizeOwner(currentPrincipal));
@@ -363,7 +493,7 @@ public class TestJcasbinAuthorizer {
.withAuditInfo(AuditInfo.EMPTY)
.build()));
- // Mock owner_meta returning a GROUP-typed owner with GROUP_ID
+ // Mock owner_meta lookup returning a GROUP-typed OwnerInfo (the owner is
GROUP_ID).
OwnerInfo groupOwnerInfo = new OwnerInfo(GROUP_ID, "GROUP");
when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(CATALOG_ID),
eq("CATALOG")))
.thenReturn(groupOwnerInfo);
@@ -386,6 +516,7 @@ public class TestJcasbinAuthorizer {
principalUtilsMockedStatic
.when(PrincipalUtils::getCurrentPrincipal)
.thenReturn(nonMemberPrincipal);
+ // Re-populate the owner cache with the group owner
when(ownerMetaMapper.selectOwnerByMetadataObjectIdAndType(eq(CATALOG_ID),
eq("CATALOG")))
.thenReturn(groupOwnerInfo);
getOwnerRelCache(jcasbinAuthorizer).invalidateAll();
@@ -424,6 +555,26 @@ public class TestJcasbinAuthorizer {
restoreDefaultPrincipal();
}
+ @Test
+ public void testGroupRoleSkippedWhenRoleIdsAndNamesMismatch() throws
Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+
+ String mismatchGroupName = "mismatchGroup";
+ UserPrincipal groupPrincipal =
setCurrentPrincipalWithGroup(mismatchGroupName);
+
+ mockNoDirectUserRoles();
+ // Mismatched roleIds (1) and roleNames (2) -- the whole group should be
skipped
+ mockGroupWithRoles(
+ mismatchGroupName, ImmutableList.of(101L), ImmutableList.of("roleA",
"roleB"));
+
+ // Authorization denied -- the mismatched group is skipped, so no role is
loaded
+ assertFalse(doAuthorize(groupPrincipal));
+
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
@Test
public void testAuthorizeByDirectAndGroupRoles() throws Exception {
makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
@@ -439,13 +590,7 @@ public class TestJcasbinAuthorizer {
mockRoleInStore(
groupRoleId, "groupCatalogRole",
ImmutableList.of(getAllowSecurableObject()));
- NameIdentifier userNameIdentifier = NameIdentifierUtil.ofUser(METALAKE,
USERNAME);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(directRole));
-
+ mockDirectUserRoles(directRole);
mockGroupWithRoles(
GROUP_NAME, ImmutableList.of(groupRoleId),
ImmutableList.of(groupRole.name()));
@@ -477,26 +622,6 @@ public class TestJcasbinAuthorizer {
restoreDefaultPrincipal();
}
- @Test
- public void testGroupRoleSkippedWhenRoleIdsAndNamesMismatch() throws
Exception {
- makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
- getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
-
- String mismatchGroupName = "mismatchGroup";
- UserPrincipal groupPrincipal =
setCurrentPrincipalWithGroup(mismatchGroupName);
-
- mockNoDirectUserRoles();
- // Mismatched roleIds (1) and roleNames (2) -- the whole group should be
skipped
- mockGroupWithRoles(
- mismatchGroupName, ImmutableList.of(101L), ImmutableList.of("roleA",
"roleB"));
-
- // Authorization denied -- the mismatched group is skipped, so no role is
loaded
- assertFalse(doAuthorize(groupPrincipal));
-
- restoreDefaultPrincipal();
- getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
- }
-
@Test
public void testStaleGroupSkippedWhenNotInStore() throws Exception {
makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
@@ -506,12 +631,8 @@ public class TestJcasbinAuthorizer {
UserPrincipal groupPrincipal =
setCurrentPrincipalWithGroup(staleGroupName);
mockNoDirectUserRoles();
- // batchGet silently skips missing entities -- the stale group returns an
empty list
- when(entityStore.batchGet(
- eq(ImmutableList.of(NameIdentifierUtil.ofGroup(METALAKE,
staleGroupName))),
- eq(Entity.EntityType.GROUP),
- eq(GroupEntity.class)))
- .thenReturn(ImmutableList.of());
+ // group_meta lookup returns null -- the stale group has no row in the DB.
+ when(groupMetaMapper.getGroupUpdatedAt(eq(METALAKE),
eq(staleGroupName))).thenReturn(null);
// Authorization denied without throwing -- the stale group is silently
skipped
assertFalse(doAuthorize(groupPrincipal));
@@ -551,6 +672,40 @@ public class TestJcasbinAuthorizer {
getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
}
+ @Test
+ public void testRecreatedGroupWithSameNameDoesNotReuseOldRoleCache() throws
Exception {
+ makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+
+ Long oldGroupId = 201L;
+ Long newGroupId = 202L;
+ Long oldGroupRoleId = 203L;
+ RoleEntity oldGroupRole =
+ mockRoleInStore(
+ oldGroupRoleId, "oldGroupRole",
ImmutableList.of(getAllowSecurableObject()));
+ UserPrincipal groupPrincipal = setCurrentPrincipalWithGroup(GROUP_NAME);
+
+ mockNoDirectUserRoles();
+ mockGroupWithRoles(
+ oldGroupId,
+ GROUP_NAME,
+ ImmutableList.of(oldGroupRoleId),
+ ImmutableList.of(oldGroupRole.name()));
+
+ assertTrue(doAuthorize(groupPrincipal));
+
+ // The group is deleted and recreated with the same name but a new id.
Keep updated_at lower
+ // than the old cache snapshot to verify the group id, not only
updated_at, controls reuse.
+ when(groupMetaMapper.getGroupUpdatedAt(eq(METALAKE), eq(GROUP_NAME)))
+ .thenReturn(new GroupUpdatedAt(newGroupId, 0L));
+
when(roleMetaMapper.listRolesByGroupId(eq(newGroupId))).thenReturn(ImmutableList.of());
+
+ assertFalse(doAuthorize(groupPrincipal));
+
+ restoreDefaultPrincipal();
+ getLoadedRolesCache(jcasbinAuthorizer).invalidateAll();
+ }
+
@Test
public void testRoleSharedByUserAndGroupSurvivesGroupRevocation() throws
Exception {
makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
@@ -563,12 +718,7 @@ public class TestJcasbinAuthorizer {
RoleEntity sharedRole =
mockRoleInStore(sharedRoleId, "sharedRole",
ImmutableList.of(getAllowSecurableObject()));
- NameIdentifier userNameIdentifier = NameIdentifierUtil.ofUser(METALAKE,
USERNAME);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(sharedRole));
+ mockDirectUserRoles(sharedRole);
mockGroupWithRoles(
GROUP_NAME, ImmutableList.of(sharedRoleId),
ImmutableList.of(sharedRole.name()));
@@ -604,24 +754,16 @@ public class TestJcasbinAuthorizer {
mockRoleInStore(
sharedRoleId, "sharedRoleForUserRevoke",
ImmutableList.of(getAllowSecurableObject()));
- NameIdentifier userNameIdentifier = NameIdentifierUtil.ofUser(METALAKE,
USERNAME);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(sharedRole));
+ mockDirectUserRoles(sharedRole);
mockGroupWithRoles(
GROUP_NAME, ImmutableList.of(sharedRoleId),
ImmutableList.of(sharedRole.name()));
// Authorization succeeds (role via both direct and group)
assertTrue(doAuthorize(groupPrincipal));
- // User loses the role directly; group still has it
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of());
+ // User loses the role directly; group still has it. Bumping the user
version forces the
+ // userRoleCache to miss and reload the (now-empty) direct role list.
+ mockDirectUserRoles();
jcasbinAuthorizer.handleRolePrivilegeChange(sharedRoleId);
// Authorization should still succeed -- role is retained via group
inheritance
@@ -700,54 +842,17 @@ public class TestJcasbinAuthorizer {
mockNoDirectUserRoles();
- GroupEntity groupAEntity =
- GroupEntity.builder()
- .withId(groupAId)
- .withName(groupA)
- .withNamespace(Namespace.of(METALAKE, "group"))
- .withAuditInfo(AuditInfo.EMPTY)
- .withRoleNames(ImmutableList.of(roleA.name()))
- .withRoleIds(ImmutableList.of(roleAId))
- .build();
- GroupEntity groupBEntity =
- GroupEntity.builder()
- .withId(groupBId)
- .withName(groupB)
- .withNamespace(Namespace.of(METALAKE, "group"))
- .withAuditInfo(AuditInfo.EMPTY)
- .withRoleNames(ImmutableList.of(roleB.name()))
- .withRoleIds(ImmutableList.of(roleBId))
- .build();
- when(entityStore.batchGet(
- eq(
- ImmutableList.of(
- NameIdentifierUtil.ofGroup(METALAKE, groupA),
- NameIdentifierUtil.ofGroup(METALAKE, groupB))),
- eq(Entity.EntityType.GROUP),
- eq(GroupEntity.class)))
- .thenReturn(ImmutableList.of(groupAEntity, groupBEntity));
+ mockGroupWithRoles(groupAId, groupA, ImmutableList.of(roleAId),
ImmutableList.of(roleA.name()));
+ mockGroupWithRoles(groupBId, groupB, ImmutableList.of(roleBId),
ImmutableList.of(roleB.name()));
// Authorization succeeds
assertTrue(doAuthorize(multiGroupPrincipal));
- // Revoke roleA from groupA; groupB still has roleB
- GroupEntity groupANoRoles =
- GroupEntity.builder()
- .withId(groupAId)
- .withName(groupA)
- .withNamespace(Namespace.of(METALAKE, "group"))
- .withAuditInfo(AuditInfo.EMPTY)
- .withRoleNames(ImmutableList.of())
- .withRoleIds(ImmutableList.of())
- .build();
- when(entityStore.batchGet(
- eq(
- ImmutableList.of(
- NameIdentifierUtil.ofGroup(METALAKE, groupA),
- NameIdentifierUtil.ofGroup(METALAKE, groupB))),
- eq(Entity.EntityType.GROUP),
- eq(GroupEntity.class)))
- .thenReturn(ImmutableList.of(groupANoRoles, groupBEntity));
+ // Revoke roleA from groupA; groupB still has roleB. Bumping the
group_meta version forces
+ // the groupRoleCache to miss and reload the now-empty role list for
groupA.
+ when(groupMetaMapper.getGroupUpdatedAt(eq(METALAKE), eq(groupA)))
+ .thenReturn(new GroupUpdatedAt(groupAId,
groupVersionCounter.incrementAndGet()));
+
when(roleMetaMapper.listRolesByGroupId(eq(groupAId))).thenReturn(ImmutableList.of());
jcasbinAuthorizer.handleRolePrivilegeChange(roleAId);
// Authorization should still succeed via groupB's role
@@ -779,12 +884,7 @@ public class TestJcasbinAuthorizer {
Long denyRoleId = 19L;
RoleEntity denyRole =
mockRoleInStore(denyRoleId, "userDenyRole",
ImmutableList.of(getDenySecurableObject()));
- NameIdentifier userNameIdentifier = NameIdentifierUtil.ofUser(METALAKE,
USERNAME);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(denyRole));
+ mockDirectUserRoles(denyRole);
// Deny should win -- user has explicit deny even though group provides
allow
assertFalse(doAuthorize(groupPrincipal));
@@ -811,12 +911,7 @@ public class TestJcasbinAuthorizer {
Long allowRoleId = 21L;
RoleEntity allowRole =
mockRoleInStore(allowRoleId, "userAllowRole",
ImmutableList.of(getAllowSecurableObject()));
- NameIdentifier userNameIdentifier = NameIdentifierUtil.ofUser(METALAKE,
USERNAME);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(allowRole));
+ mockDirectUserRoles(allowRole);
// Deny should win -- group provides deny even though user has direct allow
assertFalse(doAuthorize(groupPrincipal));
@@ -846,17 +941,34 @@ public class TestJcasbinAuthorizer {
.thenReturn(new UserPrincipal(USERNAME));
}
- /** Mocks the user as having no direct (ROLE_USER_REL) role assignments. */
+ /**
+ * Mocks the user as having no directly-assigned roles. Bumps
userMetaMapper.getUserUpdatedAt to
+ * force the userRoleCache to miss and re-read from the (empty)
roleMetaMapper.listRolesByUserId.
+ */
private static void mockNoDirectUserRoles() throws IOException {
- NameIdentifier userNameIdentifier = NameIdentifierUtil.ofUser(METALAKE,
USERNAME);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of());
+
when(roleMetaMapper.listRolesByUserId(eq(USER_ID))).thenReturn(ImmutableList.of());
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
}
- /** Builds a {@link RoleEntity} and registers it in the mocked entity store.
*/
+ /**
+ * Mocks the user as having the given roles directly assigned via the
version-validated cache
+ * path. Bumps {@code userMetaMapper.getUserUpdatedAt} to force a
userRoleCache miss.
+ */
+ private static void mockDirectUserRoles(RoleEntity... roles) {
+ List<RolePO> rolePOs = new ArrayList<>();
+ for (RoleEntity role : roles) {
+ rolePOs.add(buildRolePO(role.id(), role.name()));
+ }
+ when(roleMetaMapper.listRolesByUserId(eq(USER_ID))).thenReturn(rolePOs);
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
+ }
+
+ /**
+ * Builds a {@link RoleEntity}, registers it in the mocked entity store, and
records its version
+ * so {@code batchGetRoleUpdatedAt} returns it during version-check.
+ */
private static RoleEntity mockRoleInStore(
Long roleId, String roleName, List<SecurableObject> securableObjects)
throws IOException {
RoleEntity role = getRoleEntity(roleId, roleName, securableObjects);
@@ -865,18 +977,28 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.ROLE),
eq(RoleEntity.class)))
.thenReturn(role);
+ mockedRoleVersions.put(roleId, new RoleUpdatedAt(roleId, roleName,
nextRoleVersion()));
return role;
}
/**
- * Builds a {@link GroupEntity} with the given role ids/names and registers
it in the mocked
- * entity store via batchGet.
+ * Mocks the group as carrying the given roles via the version-validated
cache path. The {@code
+ * group_meta.updated_at} sentinel is bumped so that the groupRoleCache
misses and re-reads from
+ * {@code roleMetaMapper.listRolesByGroupId}. The {@code
entityStore.batchGet} mock is also wired
+ * because {@code isSelf(ROLE)} and {@code ownerMatchesUserOrGroups} still
resolve full group
+ * entities through the relation store.
*/
private static GroupEntity mockGroupWithRoles(
String groupName, List<Long> roleIds, List<String> roleNames) throws
IOException {
+ return mockGroupWithRoles(GROUP_ID, groupName, roleIds, roleNames);
+ }
+
+ private static GroupEntity mockGroupWithRoles(
+ Long groupId, String groupName, List<Long> roleIds, List<String>
roleNames)
+ throws IOException {
GroupEntity group =
GroupEntity.builder()
- .withId(GROUP_ID)
+ .withId(groupId)
.withName(groupName)
.withNamespace(Namespace.of(METALAKE, "group"))
.withAuditInfo(AuditInfo.EMPTY)
@@ -888,6 +1010,17 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.GROUP),
eq(GroupEntity.class)))
.thenReturn(ImmutableList.of(group));
+
+ // Version-validated path: group_meta.updated_at sentinel +
listRolesByGroupId.
+ // Use a monotonic counter so successive calls always advance the version.
+ when(groupMetaMapper.getGroupUpdatedAt(eq(METALAKE), eq(groupName)))
+ .thenReturn(new GroupUpdatedAt(groupId,
groupVersionCounter.incrementAndGet()));
+ List<RolePO> rolePOs = new ArrayList<>();
+ for (int i = 0; i < roleIds.size(); i++) {
+ String name = i < roleNames.size() ? roleNames.get(i) : "role" +
roleIds.get(i);
+ rolePOs.add(buildRolePO(roleIds.get(i), name));
+ }
+ when(roleMetaMapper.listRolesByGroupId(eq(groupId))).thenReturn(rolePOs);
return group;
}
@@ -988,16 +1121,11 @@ public class TestJcasbinAuthorizer {
"testCatalog2", getDenySecurableObjectPO(),
MetadataObject.Type.CATALOG);
}
- private static void makeCompletableFutureUseCurrentThread(JcasbinAuthorizer
jcasbinAuthorizer) {
- try {
- Executor currentThread = Runnable::run;
- Class<JcasbinAuthorizer> jcasbinAuthorizerClass =
JcasbinAuthorizer.class;
- Field field = jcasbinAuthorizerClass.getDeclaredField("executor");
- field.setAccessible(true);
- FieldUtils.writeField(field, jcasbinAuthorizer, currentThread);
- } catch (Exception e) {
- throw new RuntimeException(e);
- }
+ @SuppressWarnings("UnusedVariable")
+ private static void makeCompletableFutureUseCurrentThread(
+ @SuppressWarnings("unused") JcasbinAuthorizer jcasbinAuthorizer) {
+ // No-op: the executor field was removed during cache refactoring.
+ // Role loading is now synchronous via requestContext.loadRole().
}
@Test
@@ -1005,31 +1133,32 @@ public class TestJcasbinAuthorizer {
makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
// Get the loadedRoles cache via reflection
- Cache<Long, Boolean> loadedRoles = getLoadedRolesCache(jcasbinAuthorizer);
+ GravitinoCache<Long, Long> loadedRoles =
getLoadedRolesCache(jcasbinAuthorizer);
// Manually add a role to the cache
Long testRoleId = 100L;
- loadedRoles.put(testRoleId, true);
+ loadedRoles.put(testRoleId, System.currentTimeMillis());
// Verify it's in the cache
- assertNotNull(loadedRoles.getIfPresent(testRoleId));
+ assertTrue(loadedRoles.getIfPresent(testRoleId).isPresent());
// Call handleRolePrivilegeChange which should invalidate the cache entry
jcasbinAuthorizer.handleRolePrivilegeChange(testRoleId);
// Verify it's removed from the cache
- assertNull(loadedRoles.getIfPresent(testRoleId));
+ assertFalse(loadedRoles.getIfPresent(testRoleId).isPresent());
}
@Test
public void testOwnerCacheInvalidation() throws Exception {
- GravitinoCache<Long, Optional<OwnerInfo>> ownerRelCache =
getOwnerRelCache(jcasbinAuthorizer);
+ // Get the ownerRel cache via reflection
+ GravitinoCache<Long, Optional<OwnerInfo>> ownerRel =
getOwnerRelCache(jcasbinAuthorizer);
// Manually add an owner relation to the cache
- ownerRelCache.put(CATALOG_ID, Optional.of(new OwnerInfo(USER_ID, "USER")));
+ ownerRel.put(CATALOG_ID, Optional.of(new OwnerInfo(USER_ID, "USER")));
// Verify it's in the cache
- assertTrue(ownerRelCache.getIfPresent(CATALOG_ID).isPresent());
+ assertTrue(ownerRel.getIfPresent(CATALOG_ID).isPresent());
// Create a mock NameIdentifier for the metadata object
NameIdentifier catalogIdent = NameIdentifierUtil.ofCatalog(METALAKE,
"testCatalog");
@@ -1039,7 +1168,7 @@ public class TestJcasbinAuthorizer {
METALAKE, USER_ID, catalogIdent, Entity.EntityType.CATALOG);
// Verify it's removed from the cache
- assertFalse(ownerRelCache.getIfPresent(CATALOG_ID).isPresent());
+ assertFalse(ownerRel.getIfPresent(CATALOG_ID).isPresent());
}
@Test
@@ -1048,7 +1177,7 @@ public class TestJcasbinAuthorizer {
GravitinoCache<Long, Optional<OwnerInfo>> ownerRelCache =
getOwnerRelCache(jcasbinAuthorizer);
NameIdentifier catalogIdent = NameIdentifierUtil.ofCatalog(METALAKE,
"testCatalog");
String cacheKey =
- JcasbinAuthorizationLookups.buildCacheKey(
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey(
METALAKE, NameIdentifierUtil.toMetadataObject(catalogIdent,
Entity.EntityType.CATALOG));
metadataIdCache.put(cacheKey, CATALOG_ID);
@@ -1072,43 +1201,6 @@ public class TestJcasbinAuthorizer {
}
}
- @Test
- public void testCloseAwaitsRoleLoadExecutorTermination() throws Exception {
- RecordingThreadPoolExecutor executor = new RecordingThreadPoolExecutor();
-
- Field executorField = JcasbinAuthorizer.class.getDeclaredField("executor");
- Field changePollerField =
JcasbinAuthorizer.class.getDeclaredField("changePoller");
- Field metadataIdCacheField =
JcasbinAuthorizer.class.getDeclaredField("metadataIdCache");
- Field ownerRelCacheField =
JcasbinAuthorizer.class.getDeclaredField("ownerRelCache");
-
- Executor originalExecutor =
- (Executor) FieldUtils.readField(executorField, jcasbinAuthorizer,
true);
- Object originalPoller = FieldUtils.readField(changePollerField,
jcasbinAuthorizer, true);
- Object originalMetadataIdCache =
- FieldUtils.readField(metadataIdCacheField, jcasbinAuthorizer, true);
- Object originalOwnerRelCache =
- FieldUtils.readField(ownerRelCacheField, jcasbinAuthorizer, true);
-
- try {
- FieldUtils.writeField(executorField, jcasbinAuthorizer, executor, true);
- // Detach shared resources so close() only exercises the executor
shutdown branch and does
- // not leave the shared poller / caches in a closed state for subsequent
tests.
- FieldUtils.writeField(changePollerField, jcasbinAuthorizer, null, true);
- FieldUtils.writeField(metadataIdCacheField, jcasbinAuthorizer, null,
true);
- FieldUtils.writeField(ownerRelCacheField, jcasbinAuthorizer, null, true);
-
- jcasbinAuthorizer.close();
-
- assertTrue(executor.shutdownCalled.get());
- assertTrue(executor.awaitTerminationCalled.get());
- } finally {
- FieldUtils.writeField(executorField, jcasbinAuthorizer,
originalExecutor, true);
- FieldUtils.writeField(changePollerField, jcasbinAuthorizer,
originalPoller, true);
- FieldUtils.writeField(metadataIdCacheField, jcasbinAuthorizer,
originalMetadataIdCache, true);
- FieldUtils.writeField(ownerRelCacheField, jcasbinAuthorizer,
originalOwnerRelCache, true);
- }
- }
-
@Test
public void testRoleCacheSynchronousRemovalListenerDeletesPolicy() throws
Exception {
makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
@@ -1118,7 +1210,7 @@ public class TestJcasbinAuthorizer {
Enforcer denyEnforcer = getDenyEnforcer(jcasbinAuthorizer);
// Get the loadedRoles cache
- Cache<Long, Boolean> loadedRoles = getLoadedRolesCache(jcasbinAuthorizer);
+ GravitinoCache<Long, Long> loadedRoles =
getLoadedRolesCache(jcasbinAuthorizer);
// Add a role and its policy to the enforcer
Long testRoleId = 300L;
@@ -1129,7 +1221,7 @@ public class TestJcasbinAuthorizer {
denyEnforcer.addPolicy(roleIdStr, "CATALOG", "999", "USE_CATALOG",
"allow");
// Add role to cache
- loadedRoles.put(testRoleId, true);
+ loadedRoles.put(testRoleId, System.currentTimeMillis());
// Verify role exists in enforcer (has policy)
assertTrue(allowEnforcer.hasPolicy(roleIdStr, "CATALOG", "999",
"USE_CATALOG", "allow"));
@@ -1144,28 +1236,66 @@ public class TestJcasbinAuthorizer {
assertFalse(denyEnforcer.hasPolicy(roleIdStr, "CATALOG", "999",
"USE_CATALOG", "allow"));
}
+ @Test
+ public void testRoleCacheReplacementDoesNotDeletePolicy() throws Exception {
+ Enforcer allowEnforcer = getAllowEnforcer(jcasbinAuthorizer);
+ Enforcer denyEnforcer = getDenyEnforcer(jcasbinAuthorizer);
+ GravitinoCache<Long, Long> loadedRoles =
getLoadedRolesCache(jcasbinAuthorizer);
+
+ Long testRoleId = 301L;
+ String roleIdStr = String.valueOf(testRoleId);
+ loadedRoles.put(testRoleId, 1L);
+
+ allowEnforcer.addPolicy(roleIdStr, "CATALOG", "999", "USE_CATALOG",
"allow");
+ denyEnforcer.addPolicy(roleIdStr, "CATALOG", "999", "USE_CATALOG",
"allow");
+
+ loadedRoles.put(testRoleId, 2L);
+
+ assertTrue(allowEnforcer.hasPolicy(roleIdStr, "CATALOG", "999",
"USE_CATALOG", "allow"));
+ assertTrue(denyEnforcer.hasPolicy(roleIdStr, "CATALOG", "999",
"USE_CATALOG", "allow"));
+ }
+
+ @Test
+ public void testClearRolePoliciesPreservesUserRoleBindings() throws
Exception {
+ Enforcer allowEnforcer = getAllowEnforcer(jcasbinAuthorizer);
+ Enforcer denyEnforcer = getDenyEnforcer(jcasbinAuthorizer);
+
+ Long testRoleId = 302L;
+ String roleIdStr = String.valueOf(testRoleId);
+ String userIdStr = String.valueOf(USER_ID);
+ allowEnforcer.addRoleForUser(userIdStr, roleIdStr);
+ denyEnforcer.addRoleForUser(userIdStr, roleIdStr);
+ allowEnforcer.addPolicy(roleIdStr, "CATALOG", "999", "USE_CATALOG",
"allow");
+ denyEnforcer.addPolicy(roleIdStr, "CATALOG", "999", "USE_CATALOG",
"allow");
+
+ Method clearRolePolicies =
+ JcasbinAuthorizer.class.getDeclaredMethod("clearRolePolicies",
long.class);
+ clearRolePolicies.setAccessible(true);
+ clearRolePolicies.invoke(jcasbinAuthorizer, testRoleId);
+
+ 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
public void testCacheInitialization() throws Exception {
// Verify that caches are initialized
- Cache<Long, Boolean> loadedRoles = getLoadedRolesCache(jcasbinAuthorizer);
+ GravitinoCache<Long, Long> loadedRolesCache =
getLoadedRolesCache(jcasbinAuthorizer);
GravitinoCache<Long, Optional<OwnerInfo>> ownerRelCache =
getOwnerRelCache(jcasbinAuthorizer);
- assertNotNull(loadedRoles, "loadedRoles cache should be initialized");
- assertNotNull(ownerRelCache, "ownerRelCache should be initialized");
+ assertNotNull(loadedRolesCache, "loadedRoles cache should be initialized");
+ assertNotNull(ownerRelCache, "ownerRel cache should be initialized");
}
/** Tests {@link JcasbinAuthorizer#hasMetadataPrivilegePermission} hierarchy
walk */
@Test
public void testHasMetadataPrivilegePermission() throws Exception {
makeCompletableFutureUseCurrentThread(jcasbinAuthorizer);
- NameIdentifier userNameIdentifier = NameIdentifierUtil.ofUser(METALAKE,
USERNAME);
// --- Case 1: no MANAGE_GRANTS anywhere → false ---
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of());
+ mockUserRoles();
assertFalse(
jcasbinAuthorizer.hasMetadataPrivilegePermission(
METALAKE,
@@ -1188,11 +1318,7 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.ROLE),
eq(RoleEntity.class)))
.thenReturn(metalakeGrantRole);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(metalakeGrantRole));
+ mockUserRoles(metalakeGrantRoleId, "metalakeGrantRole");
assertTrue(
jcasbinAuthorizer.hasMetadataPrivilegePermission(
METALAKE,
@@ -1215,11 +1341,7 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.ROLE),
eq(RoleEntity.class)))
.thenReturn(catalogGrantRole);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(catalogGrantRole));
+ mockUserRoles(catalogGrantRoleId, "catalogGrantRole");
assertTrue(
jcasbinAuthorizer.hasMetadataPrivilegePermission(
METALAKE,
@@ -1248,11 +1370,7 @@ public class TestJcasbinAuthorizer {
eq(Entity.EntityType.ROLE),
eq(RoleEntity.class)))
.thenReturn(tableGrantRole);
- when(supportsRelationOperations.listEntitiesByRelation(
- eq(SupportsRelationOperations.Type.ROLE_USER_REL),
- eq(userNameIdentifier),
- eq(Entity.EntityType.USER)))
- .thenReturn(ImmutableList.of(tableGrantRole));
+ mockUserRoles(tableGrantRoleId, "tableGrantRole");
assertTrue(
jcasbinAuthorizer.hasMetadataPrivilegePermission(
METALAKE,
@@ -1296,11 +1414,11 @@ public class TestJcasbinAuthorizer {
}
@SuppressWarnings("unchecked")
- private static Cache<Long, Boolean> getLoadedRolesCache(JcasbinAuthorizer
authorizer)
+ private static GravitinoCache<Long, Long>
getLoadedRolesCache(JcasbinAuthorizer authorizer)
throws Exception {
Field field = JcasbinAuthorizer.class.getDeclaredField("loadedRoles");
field.setAccessible(true);
- return (Cache<Long, Boolean>) field.get(authorizer);
+ return (GravitinoCache<Long, Long>) field.get(authorizer);
}
@SuppressWarnings("unchecked")
@@ -1311,6 +1429,46 @@ public class TestJcasbinAuthorizer {
return (GravitinoCache<Long, Optional<OwnerInfo>>) field.get(authorizer);
}
+ /** Mock mapper to assign zero roles. Bumps user version to invalidate
cache. */
+ private static void mockUserRoles() {
+
when(roleMetaMapper.listRolesByUserId(eq(USER_ID))).thenReturn(ImmutableList.of());
+
when(roleMetaMapper.batchGetRoleUpdatedAt(any())).thenReturn(ImmutableList.of());
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
+ }
+
+ /** Mock mapper to assign a single role. Bumps user version to invalidate
cache. */
+ private static void mockUserRoles(Long roleId, String roleName) {
+ long roleVersion = nextRoleVersion();
+ when(roleMetaMapper.listRolesByUserId(eq(USER_ID)))
+ .thenReturn(ImmutableList.of(buildRolePO(roleId, roleName)));
+ when(roleMetaMapper.batchGetRoleUpdatedAt(any()))
+ .thenReturn(ImmutableList.of(new RoleUpdatedAt(roleId, roleName,
roleVersion)));
+ when(userMetaMapper.getUserUpdatedAt(eq(METALAKE), eq(USERNAME)))
+ .thenReturn(new UserUpdatedAt(USER_ID, nextUserVersion()));
+ }
+
+ private static long nextRoleVersion() {
+ return roleVersionCounter.incrementAndGet();
+ }
+
+ private static long nextUserVersion() {
+ return userVersionCounter.incrementAndGet();
+ }
+
+ private static RolePO buildRolePO(Long roleId, String roleName) {
+ return RolePO.builder()
+ .withRoleId(roleId)
+ .withRoleName(roleName)
+ .withMetalakeId(USER_METALAKE_ID)
+ .withProperties("{}")
+ .withAuditInfo("{}")
+ .withCurrentVersion(1L)
+ .withLastVersion(1L)
+ .withDeletedAt(0L)
+ .build();
+ }
+
@SuppressWarnings("unchecked")
private static GravitinoCache<String, Long>
getMetadataIdCache(JcasbinAuthorizer authorizer)
throws Exception {
@@ -1330,25 +1488,4 @@ public class TestJcasbinAuthorizer {
field.setAccessible(true);
return (Enforcer) field.get(authorizer);
}
-
- private static class RecordingThreadPoolExecutor extends ThreadPoolExecutor {
- private final AtomicBoolean shutdownCalled = new AtomicBoolean();
- private final AtomicBoolean awaitTerminationCalled = new AtomicBoolean();
-
- private RecordingThreadPoolExecutor() {
- super(1, 1, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>());
- }
-
- @Override
- public void shutdown() {
- shutdownCalled.set(true);
- super.shutdown();
- }
-
- @Override
- public boolean awaitTermination(long timeout, TimeUnit unit) throws
InterruptedException {
- awaitTerminationCalled.set(true);
- return super.awaitTermination(timeout, unit);
- }
- }
}
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizerCacheHelpers.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizerCacheHelpers.java
new file mode 100644
index 0000000000..922039ad43
--- /dev/null
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizerCacheHelpers.java
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.authorization.jcasbin;
+
+import java.util.Arrays;
+import java.util.List;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Tests for the cached role relation snapshot classes used by the
version-validated caches. */
+public class TestJcasbinAuthorizerCacheHelpers {
+
+ @Test
+ void testCachedUserRoleRels() {
+ List<Long> roleIds = Arrays.asList(10L, 20L, 30L);
+ CachedUserRoleRels cur = new CachedUserRoleRels(1L, 1000L, roleIds);
+ Assertions.assertEquals(1L, cur.getUserId());
+ Assertions.assertEquals(1000L, cur.getUpdatedAt());
+ Assertions.assertEquals(roleIds, cur.getRoleIds());
+ }
+
+ @Test
+ void testCachedGroupRoleRels() {
+ List<Long> roleIds = Arrays.asList(100L, 200L);
+ CachedGroupRoleRels cgr = new CachedGroupRoleRels(5L, 2000L, roleIds);
+ Assertions.assertEquals(5L, cgr.getGroupId());
+ Assertions.assertEquals(2000L, cgr.getUpdatedAt());
+ Assertions.assertEquals(roleIds, cgr.getRoleIds());
+ }
+}
diff --git
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java
index 867a862603..e436fdd9bf 100644
---
a/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java
+++
b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinChangePoller.java
@@ -64,20 +64,22 @@ public class TestJcasbinChangePoller {
JcasbinChangePoller.metadataObjectFromChangeLog(
"ml1", "ml1.cat1", MetadataObject.Type.CATALOG);
Assertions.assertEquals(
- key("ml1", "cat1", ""),
JcasbinAuthorizationLookups.buildCacheKey("ml1", catalog));
+ key("ml1", "CATALOG", "cat1", ""),
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1", catalog));
MetadataObject schema =
JcasbinChangePoller.metadataObjectFromChangeLog(
"ml1", "ml1.cat1.sch1", MetadataObject.Type.SCHEMA);
Assertions.assertEquals(
- key("ml1", "cat1", "sch1", ""),
JcasbinAuthorizationLookups.buildCacheKey("ml1", schema));
+ key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1", ""),
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1", schema));
MetadataObject table =
JcasbinChangePoller.metadataObjectFromChangeLog(
"ml1", "ml1.cat1.sch1.tbl1", MetadataObject.Type.TABLE);
Assertions.assertEquals(
- key("ml1", "cat1", "sch1", "tbl1", "TABLE"),
- JcasbinAuthorizationLookups.buildCacheKey("ml1", table));
+ key("ml1", "CATALOG", "cat1", "SCHEMA", "sch1", "TABLE", "tbl1", ""),
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1", table));
}
@Test
@@ -85,7 +87,8 @@ public class TestJcasbinChangePoller {
MetadataObject metalake =
JcasbinChangePoller.metadataObjectFromChangeLog("ml1", "ml1",
MetadataObject.Type.METALAKE);
Assertions.assertEquals(
- key("ml1", ""), JcasbinAuthorizationLookups.buildCacheKey("ml1",
metalake));
+ key("ml1", "METALAKE", ""),
+ JcasbinAuthorizationCacheKeys.metadataIdCacheKey("ml1", metalake));
}
@Test
@@ -124,9 +127,12 @@ public class TestJcasbinChangePoller {
poller.pollChanges();
}
- Assertions.assertEquals(List.of(key("ml1", "cat1", "")),
metadataIdCache.invalidatedPrefixes);
Assertions.assertEquals(
- List.of(key("ml1", "cat2", "sch1", "tbl1", "TABLE")),
metadataIdCache.invalidatedKeys);
+ List.of(
+ key("ml1", "CATALOG", "cat1", ""),
+ key("ml1", "CATALOG", "cat2", "SCHEMA", "sch1", "TABLE", "tbl1",
"")),
+ metadataIdCache.invalidatedPrefixes);
+ Assertions.assertEquals(List.of(), metadataIdCache.invalidatedKeys);
}
@Test
@@ -139,7 +145,7 @@ public class TestJcasbinChangePoller {
}
private static String key(String... parts) {
- return String.join(JcasbinAuthorizationLookups.KEY_SEP, parts);
+ return JcasbinAuthorizationCacheKeys.joinKeyParts(parts);
}
private static EntityChangeRecord change(long id, MetadataObject.Type type,
String fullName) {
