upstream/keycloak
1
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-06-08 16:42:13 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 16

fix: reducing the cost of clear admin composite role cache

Browse source 
closes: #47139

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Co-authored-by: Alexander Schwartz <alexander.schwartz@ibm.com>
This commit is contained in:
Steven Hawkins 2026-03-23 18:59:44 -04:00 committed by GitHub
parent ce672801af
commit 3c0c94f1d6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 125 additions and 74 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

93
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java vendored
View file

@ -26,12 +26,10 @@ import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.keycloak.Config;
import org.keycloak.client.clienttype.ClientTypeManager;
import org.keycloak.cluster.ClusterProvider;
import org.keycloak.common.Profile;
import org.keycloak.component.ComponentModel;
import org.keycloak.models.AdminRoles;
import org.keycloak.models.ClientInitialAccessModel;
import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientProvider;
@ -483,12 +481,6 @@ public class RealmCacheSession implements CacheRealmProvider {
return adapter;
}
/**
* Cache that we've triggered an admin role invalidation in this session already.
* This avoids loading the admin role again from the database
*/
private boolean adminRoleInvalidated;
private RealmAdapter prepareCachedRealm(String id, KeycloakSession session) {
CachedRealm cached = cache.get(id, CachedRealm.class);
RealmAdapter adapter;
@ -499,18 +491,6 @@ public class RealmCacheSession implements CacheRealmProvider {
return null;
}
// To accommodate imports of new realms in a separate process, always evict the admin role.
// As an alternative, one could iterate through all composite roles of the Admin role, but that would
// load about 20 roles per realm to the cache which is also inefficient.
if (!adminRoleInvalidated && !model.getName().equals(Config.getAdminRealm())) {
RealmModel adminRealm = session.realms().getRealmByName(Config.getAdminRealm());
if (adminRealm != null) {
RoleModel adminRole = adminRealm.getRole(AdminRoles.ADMIN);
invalidateRole(adminRole.getId());
adminRoleInvalidated = true;
}
}
cached = new CachedRealm(loaded, model);
cache.addRevisioned(cached, startupRevision);
adapter = new RealmAdapter(session, cached, this);
@ -559,11 +539,9 @@ public class RealmCacheSession implements CacheRealmProvider {
}
query = new RealmListQuery(loaded, cacheKey, model.getId());
cache.addRevisioned(query, startupRevision);
return model;
} else {
String realmId = query.getRealms().iterator().next();
return getRealm(realmId);
}
String realmId = query.getRealms().iterator().next();
return getRealm(realmId);
}
static String getRealmByNameCacheKey(String name) {
@ -757,7 +735,6 @@ public class RealmCacheSession implements CacheRealmProvider {
query = new RoleListQuery(loaded, cacheKey, realm, ids);
logger.tracev("adding realm roles cache miss: realm {0} key {1}", realm.getName(), cacheKey);
cache.addRevisioned(query, startupRevision);
return model.stream();
}
Set<RoleModel> list = new HashSet<>();
for (String id : query.getRoles()) {
@ -792,7 +769,6 @@ public class RealmCacheSession implements CacheRealmProvider {
query = new RoleListQuery(loaded, cacheKey, client.getRealm(), ids, client.getClientId());
logger.tracev("adding client roles cache miss: client {0} key {1}", client.getClientId(), cacheKey);
cache.addRevisioned(query, startupRevision);
return model.stream();
}
Set<RoleModel> list = new HashSet<>();
for (String id : query.getRoles()) {
@ -877,7 +853,6 @@ public class RealmCacheSession implements CacheRealmProvider {
}
logger.tracev("adding realm role cache miss: client {0} key {1}", realm.getName(), cacheKey);
cache.addRevisioned(query, startupRevision);
return model;
}
String roleId = query.getRole();
if (roleId == null) {
@ -915,7 +890,6 @@ public class RealmCacheSession implements CacheRealmProvider {
}
logger.tracev("adding client role cache miss: client {0} key {1}", client.getClientId(), cacheKey);
cache.addRevisioned(query, startupRevision);
return model;
}
String roleId = query.getRole();
if (roleId == null) {
@ -952,6 +926,22 @@ public class RealmCacheSession implements CacheRealmProvider {
@Override
public RoleModel getRoleById(RealmModel realm, String id) {
if (invalidations.contains(id)) {
return getRoleDelegate().getRoleById(realm, id);
} else if (managedRoles.containsKey(id)) {
return managedRoles.get(id);
}
CachedRole cached = getCachedRole(realm, id);
if (cached == null) {
return null;
}
RoleAdapter adapter = new RoleAdapter(cached,this, realm);
managedRoles.put(id, adapter);
return adapter;
}
protected CachedRole getCachedRole(RealmModel realm, String id) {
CachedRole cached = cache.get(id, CachedRole.class);
if (cached != null && !cached.getRealm().equals(realm.getId())) {
cached = null;
@ -961,22 +951,14 @@ public class RealmCacheSession implements CacheRealmProvider {
long loaded = cache.getCurrentRevision(id);
RoleModel model = getRoleDelegate().getRoleById(realm, id);
if (model == null) return null;
if (invalidations.contains(id)) return model;
if (model.isClientRole()) {
cached = new CachedClientRole(loaded, model.getContainerId(), model, realm);
} else {
cached = new CachedRealmRole(loaded, model, realm);
}
cache.addRevisioned(cached, startupRevision);
} else if (invalidations.contains(id)) {
return getRoleDelegate().getRoleById(realm, id);
} else if (managedRoles.containsKey(id)) {
return managedRoles.get(id);
}
RoleAdapter adapter = new RoleAdapter(cached,this, realm);
managedRoles.put(id, adapter);
return adapter;
return cached;
}
@Override
@ -1343,9 +1325,6 @@ public class RealmCacheSession implements CacheRealmProvider {
query = new ClientListQuery(loaded, cacheKey, realm, id);
logger.tracev("adding client by name cache miss: {0}", clientId);
cache.addRevisioned(query, startupRevision);
if (invalidations.contains(model.getId())) {
return model;
}
} else {
id = query.getClients().iterator().next();
}
@ -1607,4 +1586,38 @@ public class RealmCacheSession implements CacheRealmProvider {
listInvalidations.add(realm.getId());
getGroupDelegate().preRemove(realm);
}
@Override
public boolean refreshMasterAdminRole(RoleModel masterAdminRole, String clientId) {
if (!(masterAdminRole instanceof RoleAdapter)) {
return false;
}
RoleAdapter role = (RoleAdapter)masterAdminRole;
if (role.isUpdated() || role.cached.getCachedComposites().clientContainerIds().contains(clientId)) {
return false;
}
CachedRole inCache = getCachedRole(role.realm, role.getId());
if (inCache == null) {
return false;
}
if (!inCache.getComposites(session, role::getRoleModel).clientContainerIds().contains(clientId)) {
// we suspect that the cache entry is out-of-date, lock to prevent multiple invalidations
synchronized (inCache) {
inCache = getCachedRole(role.realm, role.getId());
if (inCache == null) {
return false;
}
if (!inCache.getComposites(session, role::getRoleModel).clientContainerIds().contains(clientId)) {
cache.invalidateObject(role.getId());
inCache = getCachedRole(role.realm, role.getId());
}
}
}
if (inCache != null) {
role.composites = null;
role.cached = inCache;
return true;
}
return false;
}
}

6
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RoleAdapter.java vendored
View file

@ -142,7 +142,7 @@ public class RoleAdapter implements RoleModel {
if (composites == null) {
composites = new HashSet<>();
for (String id : cached.getComposites(session, modelSupplier)) {
for (String id : cached.getComposites(session, modelSupplier).ids()) {
RoleModel role = realm.getRoleById(id);
if (role == null) {
// chance that composite role was removed, so invalidate this entry and fallback to delegate
@ -160,7 +160,7 @@ public class RoleAdapter implements RoleModel {
public Stream<RoleModel> getCompositesStream(String search, Integer first, Integer max) {
if (isUpdated()) return updated.getCompositesStream(search, first, max);
return cacheSession.getRoleDelegate().getRolesStream(realm, cached.getComposites(session, modelSupplier).stream(), search, first, max);
return cacheSession.getRoleDelegate().getRolesStream(realm, cached.getComposites(session, modelSupplier).ids().stream(), search, first, max);
}
@Override
@ -243,7 +243,7 @@ public class RoleAdapter implements RoleModel {
return cached.getAttributes(session, modelSupplier);
}
private RoleModel getRoleModel() {
protected RoleModel getRoleModel() {
return cacheSession.getRoleDelegate().getRoleById(realm, cached.getId());
}

27
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedRole.java vendored
View file

@ -17,10 +17,10 @@
package org.keycloak.models.cache.infinispan.entities;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.models.KeycloakSession;
@ -35,15 +35,17 @@ import org.keycloak.models.cache.infinispan.LazyLoader;
*/
public class CachedRole extends AbstractRevisioned implements InRealm {
public record CompositeRolesRecord (Set<String> clientContainerIds, Set<String> ids) {}
final protected String name;
final protected String realm;
final protected String description;
final protected LazyLoader<RoleModel, Set<String>> composites;
final protected LazyLoader<RoleModel, CompositeRolesRecord> composites;
/**
* Use this so the cache invalidation can retrieve any previously cached role mappings to determine if this
* items should be evicted.
*/
private Set<String> cachedComposites = new HashSet<>();
private volatile CompositeRolesRecord cachedComposites = new CompositeRolesRecord(Set.of(), Set.of());
private final LazyLoader<RoleModel, MultivaluedHashMap<String, String>> attributes;
public CachedRole(long revision, RoleModel model, RealmModel realm) {
@ -51,7 +53,17 @@ public class CachedRole extends AbstractRevisioned implements InRealm {
description = model.getDescription();
name = model.getName();
this.realm = realm.getId();
composites = new DefaultLazyLoader<>(roleModel -> roleModel.getCompositesStream().map(RoleModel::getId).collect(Collectors.toSet()), HashSet::new);
composites = new DefaultLazyLoader<>(roleModel -> {
Set<String> ids = new HashSet<>();
Set<String> clientContainerIds = new HashSet<>();
roleModel.getCompositesStream().forEach(r -> {
ids.add(r.getId());
if (r.isClientRole()) {
clientContainerIds.add(r.getContainerId());
}
});
return new CompositeRolesRecord(Collections.unmodifiableSet(clientContainerIds), Collections.unmodifiableSet(ids));
}, null);
attributes = new DefaultLazyLoader<>(roleModel -> new MultivaluedHashMap<>(roleModel.getAttributes()), MultivaluedHashMap::new);
}
@ -59,6 +71,7 @@ public class CachedRole extends AbstractRevisioned implements InRealm {
return name;
}
@Override
public String getRealm() {
return realm;
}
@ -68,10 +81,10 @@ public class CachedRole extends AbstractRevisioned implements InRealm {
}
public boolean isComposite(KeycloakSession session, Supplier<RoleModel> roleModel) {
return !getComposites(session, roleModel).isEmpty();
return !getComposites(session, roleModel).ids().isEmpty();
}
public Set<String> getComposites(KeycloakSession session, Supplier<RoleModel> roleModel) {
public CompositeRolesRecord getComposites(KeycloakSession session, Supplier<RoleModel> roleModel) {
cachedComposites = composites.get(session, roleModel);
return cachedComposites;
}
@ -80,7 +93,7 @@ public class CachedRole extends AbstractRevisioned implements InRealm {
* Use this so the cache invalidation can retrieve any previously cached role mappings to determine if this
* items should be evicted. Will return an empty list if it hasn't been cached yet (and then no invalidation is necessary)
*/
public Set<String> getCachedComposites() {
public CompositeRolesRecord getCachedComposites() {
return cachedComposites;
}

2
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/stream/HasRolePredicate.java vendored
View file

@ -43,7 +43,7 @@ public class HasRolePredicate implements Predicate<Map.Entry<String, Revisioned>
@Override
public boolean test(Map.Entry<String, Revisioned> entry) {
Object value = entry.getValue();
return (value instanceof CachedRole cachedRole && cachedRole.getCachedComposites().contains(role)) ||
return (value instanceof CachedRole cachedRole && cachedRole.getCachedComposites().ids().contains(role)) ||
(value instanceof CachedGroup cachedGroup && cachedGroup.getCachedRoleMappings().contains(role)) ||
(value instanceof RoleQuery roleQuery && roleQuery.getRoles().contains(role)) ||
(value instanceof CachedClient cachedClient && cachedClient.getScope().contains(role)) ||

3
model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java
View file

@ -116,7 +116,8 @@ public class RoleAdapter implements RoleModel, JpaModel<RoleEntity> {
@Override
public Stream<RoleModel> getCompositesStream() {
Stream<RoleModel> composites = getChildRoles().map(c -> new RoleAdapter(session, realm, em, c));
// look up the roles via the session to allow returning cached entries
Stream<RoleModel> composites = getChildRoles().map(c -> session.roles().getRoleById(realm, c.getId()));
return composites.filter(Objects::nonNull);
}

4
model/storage-private/src/main/java/org/keycloak/models/cache/CacheRealmProvider.java vendored
View file

@ -21,6 +21,7 @@ import org.keycloak.models.ClientProvider;
import org.keycloak.models.ClientScopeProvider;
import org.keycloak.models.GroupProvider;
import org.keycloak.models.RealmProvider;
import org.keycloak.models.RoleModel;
import org.keycloak.models.RoleProvider;
/**
@ -40,4 +41,7 @@ public interface CacheRealmProvider extends RealmProvider, ClientProvider, Clien
void registerGroupInvalidation(String id);
void registerInvalidation(String id);
default boolean refreshMasterAdminRole(RoleModel masterAdminRole, String clientId) {
return false;
}
}

29
quarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/ImportDistTest.java vendored
View file

@ -35,6 +35,7 @@ import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;
import static org.junit.Assert.assertFalse;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
@ -78,6 +79,16 @@ public class ImportDistTest {
@Test
void testImportNewRealm(KeycloakDistribution dist) throws IOException {
dist.setEnvVar("MY_SECRET", "admin123");
RawKeycloakDistribution rawDist = dist.unwrap(RawKeycloakDistribution.class);
CLIResult result = rawDist.run("bootstrap-admin", "service", "--db=dev-file", "--client-id=admin", "--client-secret:env=MY_SECRET");
assertTrue(result.getErrorOutput().isEmpty(), result.getErrorOutput());
rawDist.setManualStop(true);
result = rawDist.run("start-dev", "--db-url-properties=;AUTO_SERVER=TRUE;DB_CLOSE_ON_EXIT=TRUE");
File file = new File("target/realm.json");
RealmRepresentation newRealm=new RealmRepresentation();
@ -90,20 +101,14 @@ public class ImportDistTest {
mapper.writeValue(fos, newRealm);
}
var cliResult = dist.run("import", "--file=" + file.getAbsolutePath());
CLIResult adminResult = rawDist.kcadm("get", "realms", "--server", "http://localhost:8080", "--realm", "master", "--client", "admin", "--secret", "admin123");
assertEquals(0, adminResult.exitCode());
assertFalse(adminResult.getOutput().contains("anotherRealm"));
var cliResult = rawDist.kc("import", "--db-url-properties=;AUTO_SERVER=TRUE;DB_CLOSE_ON_EXIT=TRUE", "--file=" + file.getAbsolutePath());
cliResult.assertMessage("Realm 'anotherRealm' imported");
dist.setEnvVar("MY_SECRET", "admin123");
RawKeycloakDistribution rawDist = dist.unwrap(RawKeycloakDistribution.class);
CLIResult result = rawDist.run("bootstrap-admin", "service", "--db=dev-file", "--client-id=admin", "--client-secret:env=MY_SECRET");
assertTrue(result.getErrorOutput().isEmpty(), result.getErrorOutput());
rawDist.setManualStop(true);
rawDist.run("start-dev");
CLIResult adminResult = rawDist.kcadm("get", "realms", "--server", "http://localhost:8080", "--realm", "master", "--client", "admin", "--secret", "admin123");
adminResult = rawDist.kcadm("get", "realms", "--server", "http://localhost:8080", "--realm", "master", "--client", "admin", "--secret", "admin123");
assertEquals(0, adminResult.exitCode());
assertTrue(adminResult.getOutput().contains("anotherRealm"));

24
quarkus/tests/junit5/src/main/java/org/keycloak/it/utils/RawKeycloakDistribution.java
View file

@ -134,14 +134,26 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
return this;
}
public CLIResult kc(String... arguments) throws IOException {
return kc(Arrays.asList(arguments));
}
public CLIResult kc(List<String> arguments) throws IOException {
return invoke(SCRIPT_CMD, arguments);
}
public CLIResult kcadm(String... arguments) throws IOException {
return kcadm(Arrays.asList(arguments));
}
public CLIResult kcadm(List<String> arguments) throws IOException {
return invoke(SCRIPT_KCADM_CMD, arguments);
}
private CLIResult invoke(String script, List<String> arguments) throws IOException {
List<String> allArgs = new ArrayList<>();
invoke(allArgs, SCRIPT_KCADM_CMD);
invoke(allArgs, script);
if (this.isDebug()) {
allArgs.add("-x");
@ -159,12 +171,12 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
builder.environment().putAll(envVars);
Process kcadm = builder.start();
Process proc = builder.start();
DefaultOutputConsumer outputConsumer = new DefaultOutputConsumer();
readOutput(kcadm, outputConsumer);
readOutput(proc, outputConsumer);
int exitValue = kcadm.exitValue();
int exitValue = proc.exitValue();
return CLIResult.create(outputConsumer.getStdOut(), outputConsumer.getErrOut(), exitValue);
}
@ -257,7 +269,7 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
if (descendants.isEmpty()) {
return;
}
LOG.debugf("Found %d descendant processes to terminate", descendants.size());
CompletableFuture<?> allProcesses = CompletableFuture.completedFuture(null);
@ -498,7 +510,7 @@ public final class RawKeycloakDistribution implements KeycloakDistribution {
}
private Path inDistZipDirectory(File distFile) throws Exception{
try (ZipFile zipFile = new ZipFile(distFile)) {
Optional<? extends ZipEntry> e = zipFile.stream().filter(ZipEntry::isDirectory).findFirst();
if (e.isPresent()) {

11
services/src/main/java/org/keycloak/services/resources/admin/fgap/MgmtPermissions.java
View file

@ -45,6 +45,7 @@ import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.cache.CacheRealmProvider;
import org.keycloak.protocol.oidc.mappers.AbstractOIDCProtocolMapper;
import org.keycloak.representations.AccessToken;
import org.keycloak.representations.idm.authorization.Permission;
@ -192,11 +193,13 @@ class MgmtPermissions implements AdminPermissionEvaluator, AdminPermissionManage
if (!admin.hasRole(masterAdminRole)) {
return false;
}
CacheRealmProvider cache = session.getProvider(CacheRealmProvider.class);
if (cache == null || !cache.refreshMasterAdminRole(masterAdminRole, clientId)) {
return false;
}
Set<String> roleNames = Set.of(adminRoles);
ClientModel clientModel = masterRealm.getClientByClientId(clientId);
return clientModel != null && masterAdminRole.getCompositesStream()
.anyMatch(r -> (r.isClientRole() && r.getContainerId().equals(clientModel.getId())
&& roleNames.contains(r.getName())));
return masterAdminRole.getCompositesStream().anyMatch(r -> (r.isClientRole()
&& r.getContainerId().equals(clientId) && roleNames.contains(r.getName())));
}
public boolean isAdminSameRealm() {