diff --git a/core/src/main/java/org/apache/gravitino/Configs.java b/core/src/main/java/org/apache/gravitino/Configs.java index 1c50d74402e..a0b4ca5c894 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 @@ private Configs() {} public static final ConfigEntry 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/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedGroupRoles.java b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedGroupRoles.java new file mode 100644 index 00000000000..442f9cdfbad --- /dev/null +++ b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedGroupRoles.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 assignments. 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 list is stale and must be reloaded. + */ +final class CachedGroupRoles { + + private final long groupId; + private final long updatedAt; + private final List roleIds; + + CachedGroupRoles(long groupId, long updatedAt, List roleIds) { + this.groupId = groupId; + this.updatedAt = updatedAt; + this.roleIds = roleIds; + } + + long getGroupId() { + return groupId; + } + + long getUpdatedAt() { + return updatedAt; + } + + List getRoleIds() { + return roleIds; + } +} diff --git a/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedUserRoles.java b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedUserRoles.java new file mode 100644 index 00000000000..083e7366b86 --- /dev/null +++ b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/CachedUserRoles.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 assignments. 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 list is stale and must be reloaded. + */ +final class CachedUserRoles { + + private final long userId; + private final long updatedAt; + private final List roleIds; + + CachedUserRoles(long userId, long updatedAt, List roleIds) { + this.userId = userId; + this.updatedAt = updatedAt; + this.roleIds = roleIds; + } + + long getUserId() { + return userId; + } + + long getUpdatedAt() { + return updatedAt; + } + + List 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 00000000000..28723d3cb4f --- /dev/null +++ b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationCacheKeys.java @@ -0,0 +1,70 @@ +/* + * 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.google.common.annotations.VisibleForTesting; +import org.apache.gravitino.MetadataObject; + +/** Cache key factory for JCasbin authorization caches. */ +final class JcasbinAuthorizationCacheKeys { + + /** Unit Separator for internal cache keys. */ + static final String SEPARATOR = "\u001F"; + + private JcasbinAuthorizationCacheKeys() {} + + /** + * Builds a path-based key for the metadata id cache. + * + *

Container objects end with the internal separator so prefix invalidation can remove both the + * container and entries under the same name path. Leaf objects include the type suffix to avoid + * collisions between objects that share the same name path. + */ + static String metadataObjectKey(String metalake, MetadataObject metadataObject) { + if (metadataObject.type() == MetadataObject.Type.METALAKE) { + return metalake + SEPARATOR; + } + + StringBuilder sb = new StringBuilder(metalake); + sb.append(SEPARATOR); + sb.append(String.join(SEPARATOR, metadataObject.fullName().split("\\."))); + if (isMetadataContainer(metadataObject.type())) { + sb.append(SEPARATOR); + } else { + sb.append(SEPARATOR); + sb.append(metadataObject.type().name()); + } + return sb.toString(); + } + + static String userRoleKey(String metalake, String username) { + return "USER" + SEPARATOR + metalake + SEPARATOR + username; + } + + static String groupRoleKey(String metalake, String groupname) { + return "GROUP" + SEPARATOR + metalake + SEPARATOR + groupname; + } + + @VisibleForTesting + static boolean isMetadataContainer(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/JcasbinAuthorizationLookups.java b/server-common/src/main/java/org/apache/gravitino/server/authorization/jcasbin/JcasbinAuthorizationLookups.java index 1877e3e1da9..8fb20eb37c3 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 @@ */ public class JcasbinAuthorizationLookups { - /** Unit Separator for internal path-based cache keys. */ - static final String KEY_SEP = "\u001F"; - private final GravitinoCache metadataIdCache; private final GravitinoCache> 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( @@ -72,7 +68,7 @@ public JcasbinAuthorizationLookups( */ public Optional resolveMetadataId( MetadataObject metadataObject, String metalake, AuthorizationRequestContext requestContext) { - String cacheKey = buildCacheKey(metalake, metadataObject); + String cacheKey = JcasbinAuthorizationCacheKeys.metadataObjectKey(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 +112,4 @@ public Optional resolveOwnerId( return ownerInfo == null ? Optional.empty() : Optional.of(ownerInfo); })); } - - /** Underlying metadata-id cache; exposed for invalidation by the change hooks and the poller. */ - public GravitinoCache metadataIdCache() { - return metadataIdCache; - } - - /** Underlying owner cache; exposed for invalidation by the change hooks and the poller. */ - public GravitinoCache> 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. - * - *

Examples: {@code metalake}, {@code metalakecatalog}, {@code - * metalakecatalogschema}, {@code - * metalakecatalogschematableTABLE}. - */ - @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 1f6b28aa85d..2c302c20216 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,10 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; +import java.util.Locale; 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 +53,18 @@ 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 +74,41 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -/** The Jcasbin implementation of GravitinoAuthorizer. */ +/** + * The Jcasbin implementation of {@link GravitinoAuthorizer}. + * + *

Cache architecture

+ * + *

Authorization decisions are read-mostly and run on the hot path, so this class layers three + * cache families with different consistency models: + * + *

    + *
  1. Per-request dedup — 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. + *
  2. Version-validated shared caches (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}. + *
  3. Eventual-consistency caches — {@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. + *
+ * + *

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). + * + *

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 +125,39 @@ public class JcasbinAuthorizer implements GravitinoAuthorizer { /** deny internal authorizer */ private InternalAuthorizer denyInternalAuthorizer; + // ---- Version-validated caches (strong consistency) ---- + + /** + * userRoleCache: per-(metalake, userName) -> CachedUserRoles. Version-validated per request via + * user_meta.updated_at. + */ + private GravitinoCache 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) -> CachedGroupRoles. Version-validated per request + * via group_meta.updated_at. */ - private Cache loadedRoles; + private GravitinoCache groupRoleCache; + + /** + * loadedRoles: roleId -> updated_at. If the DB updated_at is newer, evict and reload policies. + */ + private GravitinoCache loadedRoles; + + // ---- Eventual consistency caches (poller-driven) ---- - /** Path-based {@code metalake::catalog::schema::object::TYPE} → entity id. */ + /** Path-based metadata object key -> entity id. Evicted by entity change poller. */ private GravitinoCache metadataIdCache; - /** {@code metadataObjectId} → {@link Optional} of {@link OwnerInfo}. */ + /** ownerRelCache: metadataObjectId -> Optional(owner). Evicted by owner change poller. */ private GravitinoCache> 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 +179,18 @@ public void initialize() { 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 +199,6 @@ public void initialize() { 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 +213,10 @@ private Model getModel(String modelFilePath) { return model; } + // --------------------------------------------------------------------------- + // Authorize / deny / isOwner + // --------------------------------------------------------------------------- + @Override public boolean authorize( Principal principal, @@ -300,7 +335,7 @@ private boolean isOwnerOfObject( } Optional owner = lookups.resolveOwnerId(metadataId.get(), metadataObject.type(), requestContext); - return ownerMatchesUserOrGroups(owner, principal, metalake); + return ownerMatchesUserOrGroups(owner, principal, metalake, requestContext); } @Override @@ -316,14 +351,10 @@ public boolean isMetalakeUser(String metalake, AuthorizationRequestContext reque 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 +414,13 @@ public boolean hasSetOwnerPermission( 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 +442,6 @@ public boolean hasSetOwnerPermission( || 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 +468,6 @@ public boolean hasSetOwnerPermission( } return true; } - // metadata parent owner can set owner. } while ((metadataObject = MetadataObjects.parent(metadataObject)) != null); return false; } @@ -448,17 +476,12 @@ public boolean hasSetOwnerPermission( 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 chain = new ArrayList<>(); for (MetadataObject obj = MetadataObjects.parse(fullName, metadataType); obj != null; @@ -476,6 +499,10 @@ public boolean hasMetadataPrivilegePermission( return hasSetOwnerPermission(metalake, type, fullName, requestContext); } + // --------------------------------------------------------------------------- + // Cache invalidation hooks (called from service layer) + // --------------------------------------------------------------------------- + @Override public void handleRolePrivilegeChange(Long roleId) { loadedRoles.invalidate(roleId); @@ -487,7 +514,8 @@ public void handleMetadataOwnerChange( 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.metadataObjectKey(metalake, metadataObject)); try { MetadataIdConverter.getID(metadataObject, metalake).ifPresent(ownerRelCache::invalidate); } catch (RuntimeException e) { @@ -499,9 +527,9 @@ public void handleMetadataOwnerChange( 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.metadataObjectKey(metalake, metadataObject); + if (JcasbinAuthorizationCacheKeys.isMetadataContainer(metadataObject.type())) { + // Prefix invalidation removes a container and all children under the same name path. metadataIdCache.invalidateByPrefix(cacheKey); } else { metadataIdCache.invalidate(cacheKey); @@ -513,19 +541,14 @@ public void close() throws IOException { 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 +580,10 @@ private List buildSchemaInheritanceChain(MetadataObject schemaOb return ImmutableList.copyOf(chain); } + // --------------------------------------------------------------------------- + // Internal authorizer + // --------------------------------------------------------------------------- + private class InternalAuthorizer { Enforcer enforcer; @@ -581,15 +608,25 @@ private boolean loadPrivilegeAndAuthorize( 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 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 +655,7 @@ private boolean loadPrivilegeAndAuthorize( * {@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 +670,21 @@ private boolean authorizeObject( } 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 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 +694,205 @@ private boolean authorizeByJcasbin( } } - 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 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 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 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 entities; - try { - entities = - entityStore - .relationOperations() - .listEntitiesByRelation( - SupportsRelationOperations.Type.ROLE_USER_REL, - userNameIdentifier, - Entity.EntityType.USER); - List> loadRoleFutures = new ArrayList<>(); - Set 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 userDirectRoleIds = loadUserRoles(metalake, username, userId, userInfo); + + // 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 groupInheritedRoleIds = new ArrayList<>(); + for (String groupname : currentPrincipalGroupNames()) { + groupInheritedRoleIds.addAll( + loadGroupRoles(metalake, groupname, userId, requestContext)); + } - // Load roles inherited from the user's groups. - for (GroupEntity groupEntity : resolveCurrentUserGroups(metalake, entityStore)) { - List roleIds = groupEntity.roleIds(); - List 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); - } + // 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 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); } + } - CompletableFuture.allOf(loadRoleFutures.toArray(new CompletableFuture[0])).join(); - - // 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); - } - } - } 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 allRoleIds = new ArrayList<>(userDirectRoleIds); + allRoleIds.addAll(groupInheritedRoleIds); + if (!allRoleIds.isEmpty()) { + versionCheckAndLoadRoles(metalake, allRoleIds, requestContext); } }); } + private List loadUserRoles( + String metalake, String username, long userId, UserUpdatedAt userInfo) { + String userCacheKey = JcasbinAuthorizationCacheKeys.userRoleKey(metalake, username); + Optional 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. + CachedUserRoles cached = cachedOpt.get(); + bindUserRoles(userId, cached.getRoleIds()); + return cached.getRoleIds(); + } + + // Cache miss or stale — reload from DB + List rolePOs = + SessionUtils.getWithoutCommit(RoleMetaMapper.class, m -> m.listRolesByUserId(userId)); + List roleIds = rolePOs.stream().map(RolePO::getRoleId).collect(Collectors.toList()); + + userRoleCache.put(userCacheKey, new CachedUserRoles(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 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 loadGroupRoles( + String metalake, String groupname, long userId, AuthorizationRequestContext requestContext) { + Optional 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 cachedOpt = groupRoleCache.getIfPresent(groupCacheKey); + + if (cachedOpt.isPresent()) { + CachedGroupRoles cached = cachedOpt.get(); + if (cached.getGroupId() == groupId && cached.getUpdatedAt() >= groupInfo.getUpdatedAt()) { + bindUserRoles(userId, cached.getRoleIds()); + return cached.getRoleIds(); + } + } + + List rolePOs = + SessionUtils.getWithoutCommit(RoleMetaMapper.class, m -> m.listRolesByGroupId(groupId)); + List roleIds = rolePOs.stream().map(RolePO::getRoleId).collect(Collectors.toList()); + + groupRoleCache.put( + groupCacheKey, new CachedGroupRoles(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 currentPrincipalGroupNames() { + Principal principal = PrincipalUtils.getCurrentPrincipal(); + if (!(principal instanceof UserPrincipal)) { + return new ArrayList<>(); + } + List 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 resolveCurrentUserGroups(String metalake, EntityStore entityStore) { Principal principal = PrincipalUtils.getCurrentPrincipal(); @@ -756,47 +910,65 @@ private List resolveCurrentUserGroups(String metalake, EntityStore 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> 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) { - return; - } - CompletableFuture 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); + private void versionCheckAndLoadRoles( + String metalake, List roleIds, AuthorizationRequestContext requestContext) { + // Step 3: batch fetch (roleId, roleName, updated_at) for all role IDs — 1 query + List uniqueRoleIds = roleIds.stream().distinct().collect(Collectors.toList()); + List roleVersions = + SessionUtils.getWithoutCommit( + RoleMetaMapper.class, m -> m.batchGetRoleUpdatedAt(uniqueRoleIds)); + + for (RoleUpdatedAt rv : roleVersions) { + long roleId = rv.getRoleId(); + long dbUpdatedAt = rv.getUpdatedAt(); + Optional cachedUpdatedAt = loadedRoles.getIfPresent(roleId); + + if (cachedUpdatedAt.isPresent() && cachedUpdatedAt.get() >= dbUpdatedAt) { + // Role policies are still current + continue; + } + + // Load full role entity using roleName from the batch query (no extra DB scan) + RoleEntity roleEntity; + try { + EntityStore entityStore = GravitinoEnv.getInstance().entityStore(); + roleEntity = + entityStore.get( + NameIdentifierUtil.ofRole(metalake, rv.getRoleName()), + Entity.EntityType.ROLE, + RoleEntity.class); + } catch (Exception e) { + LOG.warn("Failed to load role policies for roleId {}", roleId, e); + continue; + } + + // 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 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 +990,9 @@ private void loadPolicyByRoleEntity( 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 +1000,9 @@ private void loadPolicyByRoleEntity( 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 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 groups = ((UserPrincipal) principal).getGroups(); - if (groups.isEmpty()) { - return false; - } - try { - List groupIdents = - groups.stream() - .map(g -> NameIdentifierUtil.ofGroup(metalake, g.getGroupname())) - .collect(Collectors.toList()); - List 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 611cc9616bb..4b26b3ae405 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 @@ private synchronized void pollEntityChanges() { } MetadataObject mdObj = metadataObjectFromChangeLog(metalake, fullName, mdType); - String cacheKey = JcasbinAuthorizationLookups.buildCacheKey(metalake, mdObj); + String cacheKey = JcasbinAuthorizationCacheKeys.metadataObjectKey(metalake, mdObj); - if (JcasbinAuthorizationLookups.isContainerType(mdType)) { + if (JcasbinAuthorizationCacheKeys.isMetadataContainer(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 00000000000..0205ec1608e --- /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). + * + *

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 { + + private final Cache 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 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 00000000000..b88723231f7 --- /dev/null +++ b/server-common/src/test/java/org/apache/gravitino/server/authorization/jcasbin/TestJcasbinAuthorizationCacheKeys.java @@ -0,0 +1,142 @@ +/* + * 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 testMetadataObjectKeyMetalake() { + MetadataObject obj = + MetadataObjects.of(Collections.singletonList("ml1"), MetadataObject.Type.METALAKE); + Assertions.assertEquals( + key("ml1", ""), JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", obj)); + } + + @Test + void testMetadataObjectKeyCatalog() { + MetadataObject obj = + MetadataObjects.of(Collections.singletonList("cat1"), MetadataObject.Type.CATALOG); + Assertions.assertEquals( + key("ml1", "cat1", ""), JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", obj)); + } + + @Test + void testMetadataObjectKeySchema() { + MetadataObject obj = + MetadataObjects.of(Arrays.asList("cat1", "sch1"), MetadataObject.Type.SCHEMA); + Assertions.assertEquals( + key("ml1", "cat1", "sch1", ""), + JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", obj)); + } + + @Test + void testMetadataObjectKeyLeafTypesGetTypeSuffix() { + MetadataObject table = + MetadataObjects.of(Arrays.asList("cat1", "sch1", "tbl1"), MetadataObject.Type.TABLE); + Assertions.assertEquals( + key("ml1", "cat1", "sch1", "tbl1", "TABLE"), + JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", table)); + + MetadataObject view = + MetadataObjects.of(Arrays.asList("cat1", "sch1", "v1"), MetadataObject.Type.VIEW); + Assertions.assertEquals( + key("ml1", "cat1", "sch1", "v1", "VIEW"), + JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", view)); + + MetadataObject fileset = + MetadataObjects.of(Arrays.asList("cat1", "sch1", "fs1"), MetadataObject.Type.FILESET); + Assertions.assertEquals( + key("ml1", "cat1", "sch1", "fs1", "FILESET"), + JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", fileset)); + } + + @Test + void testPrincipalRoleKeysAreTyped() { + Assertions.assertEquals( + key("USER", "ml1", "alice"), JcasbinAuthorizationCacheKeys.userRoleKey("ml1", "alice")); + Assertions.assertEquals( + key("GROUP", "ml1", "admins"), JcasbinAuthorizationCacheKeys.groupRoleKey("ml1", "admins")); + } + + @Test + void testPrincipalRoleKeysAreDistinctFromMetadataKeys() { + MetadataObject metalake = + MetadataObjects.of(Collections.singletonList("ml1"), MetadataObject.Type.METALAKE); + String metalakeKey = JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", metalake); + String userKey = JcasbinAuthorizationCacheKeys.userRoleKey("ml1", "alice"); + String groupKey = JcasbinAuthorizationCacheKeys.groupRoleKey("ml1", "alice"); + + Assertions.assertNotEquals(metalakeKey, userKey); + Assertions.assertNotEquals(metalakeKey, groupKey); + Assertions.assertNotEquals(userKey, groupKey); + } + + @Test + void testIsMetadataContainerContainerTypes() { + Assertions.assertTrue( + JcasbinAuthorizationCacheKeys.isMetadataContainer(MetadataObject.Type.METALAKE)); + Assertions.assertTrue( + JcasbinAuthorizationCacheKeys.isMetadataContainer(MetadataObject.Type.CATALOG)); + Assertions.assertTrue( + JcasbinAuthorizationCacheKeys.isMetadataContainer(MetadataObject.Type.SCHEMA)); + } + + @Test + void testIsMetadataContainerLeafTypes() { + Assertions.assertFalse( + JcasbinAuthorizationCacheKeys.isMetadataContainer(MetadataObject.Type.TABLE)); + Assertions.assertFalse( + JcasbinAuthorizationCacheKeys.isMetadataContainer(MetadataObject.Type.VIEW)); + Assertions.assertFalse( + JcasbinAuthorizationCacheKeys.isMetadataContainer(MetadataObject.Type.FILESET)); + Assertions.assertFalse( + JcasbinAuthorizationCacheKeys.isMetadataContainer(MetadataObject.Type.TOPIC)); + } + + @Test + void testPrefixInvalidationCoversContainerPath() { + MetadataObject catalog = + MetadataObjects.of(Collections.singletonList("cat1"), MetadataObject.Type.CATALOG); + String catalogKey = JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", catalog); + + MetadataObject schema = + MetadataObjects.of(Arrays.asList("cat1", "sch1"), MetadataObject.Type.SCHEMA); + String schemaKey = JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", schema); + + MetadataObject table = + MetadataObjects.of(Arrays.asList("cat1", "sch1", "tbl1"), MetadataObject.Type.TABLE); + String tableKey = JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", table); + + Assertions.assertTrue(schemaKey.startsWith(catalogKey)); + Assertions.assertTrue(tableKey.startsWith(catalogKey)); + Assertions.assertTrue(tableKey.startsWith(schemaKey)); + } + + private static String key(String... parts) { + return String.join(JcasbinAuthorizationCacheKeys.SEPARATOR, 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 afbf9f36855..b41035c8572 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.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 @@ void testResolveOwnerIdUsesAtomicSharedCacheAndRequestDedup() { Assertions.assertEquals(0, ownerRelCache.putCount); } - private static String key(String... parts) { - return String.join(JcasbinAuthorizationLookups.KEY_SEP, parts); - } - private static class CountingCache implements GravitinoCache { 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 e1dea19922a..b3bec108973 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 @@ 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.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.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 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 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 static void setup() throws IOException { invocation -> { Class mapperClass = invocation.getArgument(0); Function 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 static void setup() throws IOException { 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 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 static void setup() throws IOException { 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 static void setup() throws IOException { @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 void resetSharedState() throws Exception { 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 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 void testAuthorize() throws Exception { 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 void testAuthorize() throws Exception { 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 void testAuthorize() throws Exception { 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 void testAuthorizeByOwner() throws Exception { 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 void testAuthorizeByGroupOwner() throws Exception { .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 void testAuthorizeByGroupOwner() throws Exception { 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 void testAuthorizeByGroupRole() throws Exception { 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 void testAuthorizeByDirectAndGroupRoles() throws Exception { 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 void testIsSelfRoleViaGroup() throws Exception { 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 void testStaleGroupSkippedWhenNotInStore() throws Exception { 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 void testGroupRoleRevokedDeniesAccess() throws Exception { 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 void testRoleSharedByUserAndGroupSurvivesGroupRevocation() throws Excepti 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 void testRoleSharedByUserAndGroupSurvivesUserRevocation() throws Exceptio 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 void testMultipleGroupsPartialRevocationRetainsAccess() throws Exception 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 void testDenyRoleOnUserOverridesAllowFromGroup() throws Exception { 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 void testDenyRoleFromGroupOverridesAllowOnUser() throws Exception { 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 @@ private static void restoreDefaultPrincipal() { .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 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 securableObjects) throws IOException { RoleEntity role = getRoleEntity(roleId, roleName, securableObjects); @@ -865,18 +977,28 @@ private static RoleEntity mockRoleInStore( 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 roleIds, List roleNames) throws IOException { + return mockGroupWithRoles(GROUP_ID, groupName, roleIds, roleNames); + } + + private static GroupEntity mockGroupWithRoles( + Long groupId, String groupName, List roleIds, List 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 @@ private static GroupEntity mockGroupWithRoles( 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 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 @@ private static SecurableObject getDenySecurableObject() { "testCatalog2", getDenySecurableObjectPO(), MetadataObject.Type.CATALOG); } - private static void makeCompletableFutureUseCurrentThread(JcasbinAuthorizer jcasbinAuthorizer) { - try { - Executor currentThread = Runnable::run; - Class 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 void testRoleCacheInvalidation() throws Exception { makeCompletableFutureUseCurrentThread(jcasbinAuthorizer); // Get the loadedRoles cache via reflection - Cache loadedRoles = getLoadedRolesCache(jcasbinAuthorizer); + GravitinoCache 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> ownerRelCache = getOwnerRelCache(jcasbinAuthorizer); + // Get the ownerRel cache via reflection + GravitinoCache> 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 void testOwnerCacheInvalidation() throws Exception { 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 void testOwnerChangeBestEffortWhenMetadataIdLookupFails() throws Exceptio GravitinoCache> ownerRelCache = getOwnerRelCache(jcasbinAuthorizer); NameIdentifier catalogIdent = NameIdentifierUtil.ofCatalog(METALAKE, "testCatalog"); String cacheKey = - JcasbinAuthorizationLookups.buildCacheKey( + JcasbinAuthorizationCacheKeys.metadataObjectKey( METALAKE, NameIdentifierUtil.toMetadataObject(catalogIdent, Entity.EntityType.CATALOG)); metadataIdCache.put(cacheKey, CATALOG_ID); @@ -1072,43 +1201,6 @@ public void testOwnerChangeBestEffortWhenMetadataIdLookupFails() throws Exceptio } } - @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 void testRoleCacheSynchronousRemovalListenerDeletesPolicy() throws Except Enforcer denyEnforcer = getDenyEnforcer(jcasbinAuthorizer); // Get the loadedRoles cache - Cache loadedRoles = getLoadedRolesCache(jcasbinAuthorizer); + GravitinoCache loadedRoles = getLoadedRolesCache(jcasbinAuthorizer); // Add a role and its policy to the enforcer Long testRoleId = 300L; @@ -1129,7 +1221,7 @@ public void testRoleCacheSynchronousRemovalListenerDeletesPolicy() throws Except 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 void testRoleCacheSynchronousRemovalListenerDeletesPolicy() throws Except assertFalse(denyEnforcer.hasPolicy(roleIdStr, "CATALOG", "999", "USE_CATALOG", "allow")); } + @Test + public void testRoleCacheReplacementDoesNotDeletePolicy() throws Exception { + Enforcer allowEnforcer = getAllowEnforcer(jcasbinAuthorizer); + Enforcer denyEnforcer = getDenyEnforcer(jcasbinAuthorizer); + GravitinoCache 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 loadedRoles = getLoadedRolesCache(jcasbinAuthorizer); + GravitinoCache loadedRolesCache = getLoadedRolesCache(jcasbinAuthorizer); GravitinoCache> 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 void testHasMetadataPrivilegePermission() throws Exception { 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 void testHasMetadataPrivilegePermission() throws Exception { 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 void testHasMetadataPrivilegePermission() throws Exception { 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 @@ private static SecurableObject buildManageGrantsSecurableObject( } @SuppressWarnings("unchecked") - private static Cache getLoadedRolesCache(JcasbinAuthorizer authorizer) + private static GravitinoCache getLoadedRolesCache(JcasbinAuthorizer authorizer) throws Exception { Field field = JcasbinAuthorizer.class.getDeclaredField("loadedRoles"); field.setAccessible(true); - return (Cache) field.get(authorizer); + return (GravitinoCache) field.get(authorizer); } @SuppressWarnings("unchecked") @@ -1311,6 +1429,46 @@ private static GravitinoCache> getOwnerRelCache( return (GravitinoCache>) 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 getMetadataIdCache(JcasbinAuthorizer authorizer) throws Exception { @@ -1330,25 +1488,4 @@ private static Enforcer getDenyEnforcer(JcasbinAuthorizer authorizer) throws Exc 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 00000000000..a7551bd60e5 --- /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 snapshot classes used by the version-validated role caches. */ +public class TestJcasbinAuthorizerCacheHelpers { + + @Test + void testCachedUserRoles() { + List roleIds = Arrays.asList(10L, 20L, 30L); + CachedUserRoles cur = new CachedUserRoles(1L, 1000L, roleIds); + Assertions.assertEquals(1L, cur.getUserId()); + Assertions.assertEquals(1000L, cur.getUpdatedAt()); + Assertions.assertEquals(roleIds, cur.getRoleIds()); + } + + @Test + void testCachedGroupRoles() { + List roleIds = Arrays.asList(100L, 200L); + CachedGroupRoles cgr = new CachedGroupRoles(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 867a8626032..40e557ad45a 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,21 @@ void testChangeLogFullNameStripsLeadingMetalakeForChildTypes() { JcasbinChangePoller.metadataObjectFromChangeLog( "ml1", "ml1.cat1", MetadataObject.Type.CATALOG); Assertions.assertEquals( - key("ml1", "cat1", ""), JcasbinAuthorizationLookups.buildCacheKey("ml1", catalog)); + key("ml1", "cat1", ""), JcasbinAuthorizationCacheKeys.metadataObjectKey("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", "cat1", "sch1", ""), + JcasbinAuthorizationCacheKeys.metadataObjectKey("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)); + JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", table)); } @Test @@ -85,7 +86,7 @@ void testChangeLogFullNameForMetalakeKeepsItself() { MetadataObject metalake = JcasbinChangePoller.metadataObjectFromChangeLog("ml1", "ml1", MetadataObject.Type.METALAKE); Assertions.assertEquals( - key("ml1", ""), JcasbinAuthorizationLookups.buildCacheKey("ml1", metalake)); + key("ml1", ""), JcasbinAuthorizationCacheKeys.metadataObjectKey("ml1", metalake)); } @Test @@ -139,7 +140,7 @@ void testPollCursorAdvancementIsSynchronized() throws NoSuchMethodException { } private static String key(String... parts) { - return String.join(JcasbinAuthorizationLookups.KEY_SEP, parts); + return String.join(JcasbinAuthorizationCacheKeys.SEPARATOR, parts); } private static EntityChangeRecord change(long id, MetadataObject.Type type, String fullName) {