From 7e00961ee1d12c8aa43b49595a434e57dab7fd9a Mon Sep 17 00:00:00 2001 From: Pedro Ruivo Date: Tue, 17 Feb 2026 11:45:37 +0000 Subject: [PATCH] Cache evaluation of client roles with dots for role mapper Closes #43726 Signed-off-by: Pedro Ruivo <1492066+pruivo@users.noreply.github.com> Co-authored-by: Pedro Ruivo <1492066+pruivo@users.noreply.github.com> --- .../HardcodedLDAPRoleStorageMapper.java | 2 +- ...HardcodedLDAPRoleStorageMapperFactory.java | 12 +- .../AbstractIdentityProviderMapper.java | 9 ++ .../cache/AlternativeLookupProvider.java | 28 +++++ .../models/utils/KeycloakModelUtils.java | 92 ++++----------- .../models/KeycloakModelUtilsTest.java | 2 +- .../ConditionalOtpFormAuthenticator.java | 93 ++++++--------- ...onditionalOtpFormAuthenticatorFactory.java | 8 ++ .../ConditionalRoleAuthenticator.java | 2 +- .../mappers/AbstractClaimToRoleMapper.java | 18 +-- .../broker/provider/HardcodedRoleMapper.java | 12 +- .../AbstractAttributeToRoleMapper.java | 18 +-- .../java/org/keycloak/cache/CachedValue.java | 96 +++++++++++++++ .../DefaultAlternativeLookupProvider.java | 109 ++++++++++++++++-- ...faultAlternativeLookupProviderFactory.java | 6 +- .../ClientUpdaterSourceRolesCondition.java | 2 +- .../model/AlternativeLookupProviderTest.java | 28 +++++ 17 files changed, 368 insertions(+), 169 deletions(-) create mode 100644 services/src/main/java/org/keycloak/cache/CachedValue.java diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapper.java index 8101d325455..80c1fccdb65 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapper.java @@ -124,7 +124,7 @@ public class HardcodedLDAPRoleStorageMapper extends AbstractLDAPStorageMapper { private RoleModel getRole(RealmModel realm) { String roleName = mapperModel.getConfig().getFirst(HardcodedLDAPRoleStorageMapper.ROLE); - RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); + RoleModel role = KeycloakModelUtils.getRoleFromString(ldapProvider.getSession(), realm, roleName); if (role == null) { logger.warnf("Hardcoded role '%s' configured in mapper '%s' is not available anymore"); } diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapperFactory.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapperFactory.java index 093eaac6615..df4a9f22f6d 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapperFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapperFactory.java @@ -19,13 +19,16 @@ package org.keycloak.storage.ldap.mappers; import java.util.ArrayList; import java.util.List; +import java.util.Set; +import org.keycloak.cache.AlternativeLookupProvider; import org.keycloak.component.ComponentModel; import org.keycloak.component.ComponentValidationException; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.provider.Provider; import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.storage.ldap.LDAPStorageProvider; @@ -35,7 +38,7 @@ import org.keycloak.storage.ldap.LDAPStorageProvider; public class HardcodedLDAPRoleStorageMapperFactory extends AbstractLDAPStorageMapperFactory { public static final String PROVIDER_ID = "hardcoded-ldap-role-mapper"; - protected static final List configProperties = new ArrayList(); + protected static final List configProperties = new ArrayList<>(); static { ProviderConfigProperty roleAttr = createConfigProperty(HardcodedLDAPRoleStorageMapper.ROLE, @@ -62,13 +65,18 @@ public class HardcodedLDAPRoleStorageMapperFactory extends AbstractLDAPStorageMa return PROVIDER_ID; } + @Override + public Set> dependsOn() { + return Set.of(AlternativeLookupProvider.class); // for caching + } + @Override public void validateConfiguration(KeycloakSession session, RealmModel realm, ComponentModel config) throws ComponentValidationException { String roleName = config.getConfig().getFirst(HardcodedLDAPRoleStorageMapper.ROLE); if (roleName == null) { throw new ComponentValidationException("Role can't be null"); } - RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); + RoleModel role = KeycloakModelUtils.getRoleFromString(session, realm, roleName); if (role == null) { throw new ComponentValidationException("There is no role corresponding to configured value"); } diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderMapper.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderMapper.java index e9bd2072ebd..db1ad57b082 100755 --- a/server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderMapper.java +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/AbstractIdentityProviderMapper.java @@ -17,12 +17,16 @@ package org.keycloak.broker.provider; +import java.util.Set; + import org.keycloak.broker.provider.mappersync.ConfigSyncEventListener; +import org.keycloak.cache.AlternativeLookupProvider; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; +import org.keycloak.provider.Provider; import org.jboss.logging.Logger; @@ -92,4 +96,9 @@ public abstract class AbstractIdentityProviderMapper implements IdentityProvider public void updateBrokeredUserLegacy(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { updateBrokeredUser(session, realm, user, mapperModel, context); } + + @Override + public Set> dependsOn() { + return Set.of(AlternativeLookupProvider.class); //for caching + } } diff --git a/server-spi-private/src/main/java/org/keycloak/cache/AlternativeLookupProvider.java b/server-spi-private/src/main/java/org/keycloak/cache/AlternativeLookupProvider.java index 377d24e0d13..443dc9212ad 100644 --- a/server-spi-private/src/main/java/org/keycloak/cache/AlternativeLookupProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/cache/AlternativeLookupProvider.java @@ -5,6 +5,8 @@ import java.util.Map; import org.keycloak.models.ClientModel; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.RoleModel; import org.keycloak.provider.Provider; public interface AlternativeLookupProvider extends Provider { @@ -13,4 +15,30 @@ public interface AlternativeLookupProvider extends Provider { ClientModel lookupClientFromClientAttributes(KeycloakSession session, Map attributes); + /** + * Looks up a role from its string representation, supporting both realm and client roles. + *

+ * The method interprets the {@code roleName} parameter as follows: + *

    + *
  • For realm roles: the role name directly (e.g., {@code "admin"})
    • + *
  • For client roles: the format {@code "client-id.role-name"} where the client ID and role name + * are separated by a dot separator
    • + *
+ *

+ * Since client IDs can contain dots, the method attempts multiple splits from right to left to resolve ambiguous + * role names. For example, {@code "my.client.app.role"} will first try to look up + * client {@code "my.client.app"} with role {@code "role"}, then client {@code "my.client"} with role + * {@code "app.role"}, and so on. + *

+ * The lookup uses caching to reduce database load. If a role is not found in the cache, the method + * performs a database lookup and caches the result for subsequent calls. + * + * @param realm the realm in which to look up the role + * @param roleName the string representation of the role name, which can be a realm role name or a client role in + * the format {@code "client-id.role-name"}. May be {@code null}. + * @return the corresponding {@link RoleModel} if found, or {@code null} if the role does not exist or if + * {@code roleName} is {@code null} + */ + RoleModel lookupRoleFromString(RealmModel realm, String roleName); + } diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java index 70e5ebea5fd..c94a9275e52 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java @@ -49,6 +49,7 @@ import org.keycloak.Config; import org.keycloak.Config.Scope; import org.keycloak.broker.social.SocialIdentityProvider; import org.keycloak.broker.social.SocialIdentityProviderFactory; +import org.keycloak.cache.AlternativeLookupProvider; import org.keycloak.common.util.CertificateUtils; import org.keycloak.common.util.KeyUtils; import org.keycloak.common.util.PemUtils; @@ -109,7 +110,7 @@ public final class KeycloakModelUtils { public static final String GROUP_PATH_SEPARATOR = "/"; public static final String GROUP_PATH_ESCAPE = "~"; - private static final char CLIENT_ROLE_SEPARATOR = '.'; + public static final char CLIENT_ROLE_SEPARATOR = '.'; public static final int MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE = 25; @@ -695,7 +696,7 @@ public final class KeycloakModelUtils { public static Collection resolveAttribute(UserModel user, String name, boolean aggregateAttrs) { List values = user.getAttributeStream(name).collect(Collectors.toList()); - Set aggrValues = new HashSet(); + Set aggrValues = new HashSet<>(); if (!values.isEmpty()) { if (!aggregateAttrs) { return values; @@ -717,70 +718,13 @@ public final class KeycloakModelUtils { } - private static GroupModel findSubGroup(String[] segments, int index, GroupModel parent) { - return parent.getSubGroupsStream().map(group -> { - String groupName = group.getName(); - String[] pathSegments = formatPathSegments(segments, index, groupName); - - if (groupName.equals(pathSegments[index])) { - if (pathSegments.length == index + 1) { - return group; - } else { - if (index + 1 < pathSegments.length) { - GroupModel found = findSubGroup(pathSegments, index + 1, group); - if (found != null) return found; - } - } - } - return null; - }).filter(Objects::nonNull).findFirst().orElse(null); - } - - /** - * Given the {@code pathParts} of a group with the given {@code groupName}, format the {@code segments} in order to ignore - * group names containing a {@code /} character. - * - * @param segments the path segments - * @param index the index pointing to the position to start looking for the group name - * @param groupName the groupName - * @return a new array of strings with the correct segments in case the group has a name containing slashes - */ - private static String[] formatPathSegments(String[] segments, int index, String groupName) { - String[] nameSegments = groupName.split(GROUP_PATH_SEPARATOR); - - if (nameSegments.length > 1 && segments.length >= nameSegments.length) { - for (int i = 0; i < nameSegments.length; i++) { - if (!nameSegments[i].equals(segments[index + i])) { - return segments; - } - } - - int numMergedIndexes = nameSegments.length - 1; - String[] newPath = new String[segments.length - numMergedIndexes]; - - for (int i = 0; i < newPath.length; i++) { - if (i == index) { - newPath[i] = groupName; - } else if (i > index) { - newPath[i] = segments[i + numMergedIndexes]; - } else { - newPath[i] = segments[i]; - } - } - - return newPath; - } - - return segments; - } - /** * Helper to get from the session if group path slashes should be escaped or not. * @param session The session * @return true or false */ public static boolean escapeSlashesInGroupPath(KeycloakSession session) { - GroupProviderFactory fact = (GroupProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(GroupProvider.class); + GroupProviderFactory fact = (GroupProviderFactory) session.getKeycloakSessionFactory().getProviderFactory(GroupProvider.class); return fact.escapeSlashesInGroupPath(); } @@ -993,8 +937,24 @@ public final class KeycloakModelUtils { Objects.equals(client.getId(), role.getContainer().getId())); } - // Used in various role mappers + /** + * @deprecated for removal. Use {@link #getRoleFromString(KeycloakSession, RealmModel, String)} instead. + */ + @Deprecated(forRemoval = true, since = "26.6") public static RoleModel getRoleFromString(RealmModel realm, String roleName) { + return getRoleFromString(KeycloakSessionUtil.getKeycloakSession(), realm, roleName); + } + + public static RoleModel getRoleFromString(KeycloakSession session, RealmModel realm, String roleName) { + if (session == null) { + return getRoleFromStringNoCaching(realm, roleName); + } + return session.getProvider(AlternativeLookupProvider.class) + .lookupRoleFromString(realm, roleName); + } + + // Used in various role mappers + private static RoleModel getRoleFromStringNoCaching(RealmModel realm, String roleName) { if (roleName == null) { return null; } @@ -1028,11 +988,9 @@ public final class KeycloakModelUtils { if (scopeIndex > -1) { String appName = role.substring(0, scopeIndex); role = role.substring(scopeIndex + 1); - String[] rtn = {appName, role}; - return rtn; + return new String[]{appName, role}; } else { - String[] rtn = {null, role}; - return rtn; + return new String[]{null, role}; } } @@ -1222,7 +1180,7 @@ public final class KeycloakModelUtils { return displayName; } - SocialIdentityProviderFactory providerFactory = (SocialIdentityProviderFactory) session.getKeycloakSessionFactory() + SocialIdentityProviderFactory providerFactory = (SocialIdentityProviderFactory) session.getKeycloakSessionFactory() .getProviderFactory(SocialIdentityProvider.class, provider.getProviderId()); if (providerFactory != null) { return providerFactory.getName(); @@ -1258,7 +1216,7 @@ public final class KeycloakModelUtils { * @throws RuntimeException if a group does not exist */ public static void setDefaultGroups(KeycloakSession session, RealmModel realm, Stream groups) { - realm.getDefaultGroupsStream().collect(Collectors.toList()).forEach(realm::removeDefaultGroup); + realm.getDefaultGroupsStream().toList().forEach(realm::removeDefaultGroup); groups.forEach(path -> { GroupModel found = KeycloakModelUtils.findGroupByPath(session, realm, path); if (found == null) throw new RuntimeException("default group in realm rep doesn't exist: " + path); diff --git a/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java b/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java index 78abaa6089b..e2eeed3d8d0 100644 --- a/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java +++ b/server-spi-private/src/test/java/org/keycloak/models/KeycloakModelUtilsTest.java @@ -92,7 +92,7 @@ public class KeycloakModelUtilsTest { } Assert.assertEquals(65536, badRoleName.length()); - Assert.assertNull(KeycloakModelUtils.getRoleFromString(realm, badRoleName)); + Assert.assertNull(KeycloakModelUtils.getRoleFromString(null, realm, badRoleName)); Assert.assertEquals(KeycloakModelUtils.MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE, counter.get()); } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java index 73d40bf1918..7b2546a99fb 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java @@ -20,9 +20,7 @@ package org.keycloak.authentication.authenticators.browser; import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.regex.Pattern; -import java.util.stream.Collectors; import jakarta.ws.rs.core.MultivaluedMap; @@ -117,7 +115,7 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { return; } - if (tryConcludeBasedOn(voteForUserRole(context.getRealm(), context.getUser(), config), context)) { + if (tryConcludeBasedOn(voteForUserRole(context.getSession(), context.getRealm(), context.getUser(), config), context)) { return; } @@ -138,46 +136,29 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { return ABSTAIN; } - switch (config.get(DEFAULT_OTP_OUTCOME)) { - case SKIP: - return SKIP_OTP; - case FORCE: - return SHOW_OTP; - default: - return ABSTAIN; - } + return switch (config.get(DEFAULT_OTP_OUTCOME)) { + case SKIP -> SKIP_OTP; + case FORCE -> SHOW_OTP; + default -> ABSTAIN; + }; } private boolean tryConcludeBasedOn(OtpDecision state, AuthenticationFlowContext context) { - - switch (state) { - - case SHOW_OTP: + return switch (state) { + case SHOW_OTP -> { showOtpForm(context); - return true; - - case SKIP_OTP: + yield true; + } + case SKIP_OTP -> { context.success(); - return true; - - default: - return false; - } + yield true; + } + default -> false; + }; } - private boolean tryConcludeBasedOn(OtpDecision state) { - - switch (state) { - - case SHOW_OTP: - return true; - - case SKIP_OTP: - return false; - - default: - return false; - } + private static boolean tryConcludeBasedOn(OtpDecision state) { + return state == SHOW_OTP; } private void showOtpForm(AuthenticationFlowContext context) { @@ -195,19 +176,13 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { return ABSTAIN; } - Optional value = user.getAttributeStream(attributeName).findFirst(); - if (!value.isPresent()) { - return ABSTAIN; - } - - switch (value.get().trim()) { - case SKIP: - return SKIP_OTP; - case FORCE: - return SHOW_OTP; - default: - return ABSTAIN; - } + return user.getAttributeStream(attributeName) + .findFirst() + .map(s -> switch (s.trim()) { + case SKIP -> SKIP_OTP; + case FORCE -> SHOW_OTP; + default -> ABSTAIN; + }).orElse(ABSTAIN); } private OtpDecision voteForHttpHeaderMatchesPattern(MultivaluedMap requestHeaders, Map config) { @@ -257,30 +232,30 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { return false; } - private OtpDecision voteForUserRole(RealmModel realm, UserModel user, Map config) { + private OtpDecision voteForUserRole(KeycloakSession session, RealmModel realm, UserModel user, Map config) { if (!config.containsKey(SKIP_OTP_ROLE) && !config.containsKey(FORCE_OTP_ROLE)) { return ABSTAIN; } - if (userHasRole(realm, user, config.get(SKIP_OTP_ROLE))) { + if (userHasRole(session, realm, user, config.get(SKIP_OTP_ROLE))) { return SKIP_OTP; } - if (userHasRole(realm, user, config.get(FORCE_OTP_ROLE))) { + if (userHasRole(session, realm, user, config.get(FORCE_OTP_ROLE))) { return SHOW_OTP; } return ABSTAIN; } - private boolean userHasRole(RealmModel realm, UserModel user, String roleName) { + private boolean userHasRole(KeycloakSession session, RealmModel realm, UserModel user, String roleName) { if (roleName == null) { return false; } - RoleModel role = getRoleFromString(realm, roleName); + RoleModel role = getRoleFromString(session, realm, roleName); if (role != null) { return user.hasRole(role); } @@ -290,8 +265,8 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { private boolean isOTPRequired(KeycloakSession session, RealmModel realm, UserModel user) { MultivaluedMap requestHeaders = session.getContext().getRequestHeaders().getRequestHeaders(); List> configs = realm.getAuthenticatorConfigsStream().map(AuthenticatorConfigModel::getConfig) - .filter(this::containsConditionalOtpConfig) - .collect(Collectors.toList()); + .filter(ConditionalOtpFormAuthenticator::containsConditionalOtpConfig) + .toList(); if (configs.isEmpty()) { // no configuration at all means it is configured return true; @@ -300,7 +275,7 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { if (tryConcludeBasedOn(voteForUserOtpControlAttribute(user, config))) { return true; } - if (tryConcludeBasedOn(voteForUserRole(realm, user, config))) { + if (tryConcludeBasedOn(voteForUserRole(session, realm, user, config))) { return true; } if (tryConcludeBasedOn(voteForHttpHeaderMatchesPattern(requestHeaders, config))) { @@ -312,13 +287,13 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { return true; } return voteForUserOtpControlAttribute(user, config) == ABSTAIN - && voteForUserRole(realm, user, config) == ABSTAIN + && voteForUserRole(session, realm, user, config) == ABSTAIN && voteForHttpHeaderMatchesPattern(requestHeaders, config) == ABSTAIN && (voteForDefaultFallback(config) == SHOW_OTP || voteForDefaultFallback(config) == ABSTAIN); }); } - private boolean containsConditionalOtpConfig(Map config) { + private static boolean containsConditionalOtpConfig(Map config) { return config.containsKey(OTP_CONTROL_USER_ATTRIBUTE) || config.containsKey(SKIP_OTP_ROLE) || config.containsKey(FORCE_OTP_ROLE) diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java index 0ff23c08cd1..2901c7477d2 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java @@ -18,14 +18,17 @@ package org.keycloak.authentication.authenticators.browser; import java.util.List; +import java.util.Set; import org.keycloak.Config; import org.keycloak.authentication.Authenticator; import org.keycloak.authentication.AuthenticatorFactory; +import org.keycloak.cache.AlternativeLookupProvider; import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.credential.OTPCredentialModel; +import org.keycloak.provider.Provider; import org.keycloak.provider.ProviderConfigProperty; import static java.util.Arrays.asList; @@ -157,4 +160,9 @@ public class ConditionalOtpFormAuthenticatorFactory implements AuthenticatorFact return asList(forceOtpUserAttribute, skipOtpRole, forceOtpRole, skipOtpForHttpHeader, forceOtpForHttpHeader, defaultOutcome); } + + @Override + public Set> dependsOn() { + return Set.of(AlternativeLookupProvider.class); //for caching + } } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalRoleAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalRoleAuthenticator.java index b53d50e5765..640be84f1ec 100644 --- a/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalRoleAuthenticator.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/conditional/ConditionalRoleAuthenticator.java @@ -22,7 +22,7 @@ public class ConditionalRoleAuthenticator implements ConditionalAuthenticator { if (user != null && authConfig!=null && authConfig.getConfig()!=null) { String requiredRole = authConfig.getConfig().get(ConditionalRoleAuthenticatorFactory.CONDITIONAL_USER_ROLE); boolean negateOutput = Boolean.parseBoolean(authConfig.getConfig().get(ConditionalRoleAuthenticatorFactory.CONF_NEGATE)); - RoleModel role = KeycloakModelUtils.getRoleFromString(realm, requiredRole); + RoleModel role = KeycloakModelUtils.getRoleFromString(context.getSession(), realm, requiredRole); if (role == null) { logger.errorv("Invalid role name submitted: {0}", requiredRole); return false; diff --git a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToRoleMapper.java b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToRoleMapper.java index c5ee04ca625..8d161ad2eda 100644 --- a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToRoleMapper.java +++ b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimToRoleMapper.java @@ -40,7 +40,7 @@ public abstract class AbstractClaimToRoleMapper extends AbstractClaimMapper { @Override public void importNewUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - RoleModel role = getRole(realm, mapperModel); + RoleModel role = getRole(session, realm, mapperModel); if (role == null) { return; } @@ -52,7 +52,7 @@ public abstract class AbstractClaimToRoleMapper extends AbstractClaimMapper { @Override public void updateBrokeredUserLegacy(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - RoleModel role = getRole(realm, mapperModel); + RoleModel role = getRole(session, realm, mapperModel); if (role == null) { return; } @@ -64,7 +64,7 @@ public abstract class AbstractClaimToRoleMapper extends AbstractClaimMapper { @Override public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - RoleModel role = getRole(realm, mapperModel); + RoleModel role = getRole(session, realm, mapperModel); if (role == null) { return; } @@ -94,17 +94,17 @@ public abstract class AbstractClaimToRoleMapper extends AbstractClaimMapper { /** * Obtains the {@link RoleModel} corresponding the role configured in the specified - * {@link IdentityProviderMapperModel}. - * If the role doesn't correspond to one of the realm's client roles or to one of the realm's roles, this method - * returns {@code null}. + * {@link IdentityProviderMapperModel}. If the role doesn't correspond to one of the realm's client roles or to one + * of the realm's roles, this method returns {@code null}. * - * @param realm a reference to the realm. + * @param session the {@link KeycloakSession}. + * @param realm a reference to the realm. * @param mapperModel a reference to the {@link IdentityProviderMapperModel} containing the configured role. * @return the {@link RoleModel} that corresponds to the mapper model role; {@code null}, when role was not found */ - private RoleModel getRole(final RealmModel realm, final IdentityProviderMapperModel mapperModel) { + private RoleModel getRole(KeycloakSession session, final RealmModel realm, final IdentityProviderMapperModel mapperModel) { String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE); - RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); + RoleModel role = KeycloakModelUtils.getRoleFromString(session, realm, roleName); if (role == null) { LOG.warnf("Unable to find role '%s' referenced by mapper '%s' on realm '%s'.", roleName, diff --git a/services/src/main/java/org/keycloak/broker/provider/HardcodedRoleMapper.java b/services/src/main/java/org/keycloak/broker/provider/HardcodedRoleMapper.java index f8a76cc1a2d..59e1de858d7 100755 --- a/services/src/main/java/org/keycloak/broker/provider/HardcodedRoleMapper.java +++ b/services/src/main/java/org/keycloak/broker/provider/HardcodedRoleMapper.java @@ -93,19 +93,19 @@ public class HardcodedRoleMapper extends AbstractIdentityProviderMapper { @Override public void importNewUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - grantUserRole(realm, user, mapperModel); + grantUserRole(session, realm, user, mapperModel); } - private void grantUserRole(RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel) { - RoleModel role = getRole(realm, mapperModel); + private void grantUserRole(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel) { + RoleModel role = getRole(session, realm, mapperModel); if (role != null) { user.grantRole(role); } } - private RoleModel getRole(final RealmModel realm, final IdentityProviderMapperModel mapperModel) { + private RoleModel getRole(KeycloakSession session, final RealmModel realm, final IdentityProviderMapperModel mapperModel) { String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE); - RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); + RoleModel role = KeycloakModelUtils.getRoleFromString(session, realm, roleName); if (role == null) { LOG.warnf("Unable to find role '%s' referenced by mapper '%s' on realm '%s'.", roleName, @@ -117,7 +117,7 @@ public class HardcodedRoleMapper extends AbstractIdentityProviderMapper { @Override public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - grantUserRole(realm, user, mapperModel); + grantUserRole(session, realm, user, mapperModel); } @Override diff --git a/services/src/main/java/org/keycloak/broker/saml/mappers/AbstractAttributeToRoleMapper.java b/services/src/main/java/org/keycloak/broker/saml/mappers/AbstractAttributeToRoleMapper.java index 874a90569e2..0300674db90 100644 --- a/services/src/main/java/org/keycloak/broker/saml/mappers/AbstractAttributeToRoleMapper.java +++ b/services/src/main/java/org/keycloak/broker/saml/mappers/AbstractAttributeToRoleMapper.java @@ -41,7 +41,7 @@ public abstract class AbstractAttributeToRoleMapper extends AbstractIdentityProv @Override public void importNewUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - RoleModel role = this.getRole(realm, mapperModel); + RoleModel role = this.getRole(session, realm, mapperModel); if (role == null) { return; } @@ -53,7 +53,7 @@ public abstract class AbstractAttributeToRoleMapper extends AbstractIdentityProv @Override public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - RoleModel role = this.getRole(realm, mapperModel); + RoleModel role = this.getRole(session, realm, mapperModel); if (role == null) { return; } @@ -88,18 +88,18 @@ public abstract class AbstractAttributeToRoleMapper extends AbstractIdentityProv /** * Obtains the {@link RoleModel} corresponding the role configured in the specified - * {@link IdentityProviderMapperModel}. - * If the role doesn't correspond to one of the realm's client roles or to one of the realm's roles, this method - * returns {@code null}. + * {@link IdentityProviderMapperModel}. If the role doesn't correspond to one of the realm's client roles or to one + * of the realm's roles, this method returns {@code null}. * - * @param realm a reference to the realm. + * @param session the {@link KeycloakSession}. + * @param realm a reference to the realm. * @param mapperModel a reference to the {@link IdentityProviderMapperModel} containing the configured role. * @return the {@link RoleModel} that corresponds to the mapper model role or {@code null}, if the role could not be - * found + * found */ - private RoleModel getRole(final RealmModel realm, final IdentityProviderMapperModel mapperModel) { + private RoleModel getRole(KeycloakSession session, final RealmModel realm, final IdentityProviderMapperModel mapperModel) { String roleName = mapperModel.getConfig().get(ConfigConstants.ROLE); - RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); + RoleModel role = KeycloakModelUtils.getRoleFromString(session, realm, roleName); if (role == null) { LOG.warnf("Unable to find role '%s' for mapper '%s' on realm '%s'.", roleName, mapperModel.getName(), realm.getName()); diff --git a/services/src/main/java/org/keycloak/cache/CachedValue.java b/services/src/main/java/org/keycloak/cache/CachedValue.java new file mode 100644 index 00000000000..69449b37d42 --- /dev/null +++ b/services/src/main/java/org/keycloak/cache/CachedValue.java @@ -0,0 +1,96 @@ +/* + * Copyright 2026 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed 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.keycloak.cache; + +import java.util.Objects; + +/** + * Internal representation of cached lookup values. + *

+ * This is an implementation detail used by {@link DefaultAlternativeLookupProvider} to cache alternative lookups and + * reduce database load. The interface provides type-safe wrappers for different kinds of cached values used in the + * lookup cache. + */ +interface CachedValue { + + /** + * Creates a cached value wrapping a simple identifier string. + * + * @param value the non-null identifier value to cache + * @return a cached string value + */ + static CachedString ofId(String value) { + return new CachedString(value); + } + + /** + * Creates a cached value for a client role lookup. + * + * @param clientId the non-null client identifier + * @param roleName the non-null role name + * @return a cached role qualifier for a client role + */ + static CachedRoleQualifier ofClientRole(String clientId, String roleName) { + return new CachedRoleQualifier(Objects.requireNonNull(clientId), roleName); + } + + /** + * Creates a cached value for a realm role lookup. + * + * @param roleName the non-null role name + * @return a cached role qualifier for a realm role + */ + static CachedRoleQualifier ofRealmRole(String roleName) { + return new CachedRoleQualifier(null, roleName); + } + + /** + * A cached value wrapping a simple string identifier. + * + * @param value the non-null identifier value + */ + record CachedString(String value) implements CachedValue { + public CachedString { + Objects.requireNonNull(value); + } + } + + /** + * A cached value wrapping role qualifier information. + *

+ * For client roles, both {@code clientId} and {@code roleName} are present. For realm roles, {@code clientId} is + * {@code null}. + * + * @param clientId the client identifier, or {@code null} for realm roles + * @param roleName the non-null role name + */ + record CachedRoleQualifier(String clientId, String roleName) implements CachedValue { + public CachedRoleQualifier { + Objects.requireNonNull(roleName); + } + + /** + * Checks if this qualifier represents a realm role. + * + * @return {@code true} if this is a realm role (clientId is null), {@code false} otherwise + */ + public boolean isRealmRole() { + return clientId == null; + } + } +} diff --git a/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProvider.java b/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProvider.java index 55aac81c67f..6ead0bd2929 100644 --- a/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProvider.java +++ b/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProvider.java @@ -7,21 +7,30 @@ import org.keycloak.models.ClientModel; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.IdentityProviderQuery; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.RoleModel; + +import org.jboss.logging.Logger; + +import static org.keycloak.models.utils.KeycloakModelUtils.CLIENT_ROLE_SEPARATOR; +import static org.keycloak.models.utils.KeycloakModelUtils.MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE; public class DefaultAlternativeLookupProvider implements AlternativeLookupProvider { - private final LocalCache lookupCache; + private static final Logger logger = Logger.getLogger(DefaultAlternativeLookupProvider.class); + private final LocalCache lookupCache; - public DefaultAlternativeLookupProvider(LocalCache lookupCache) { + DefaultAlternativeLookupProvider(LocalCache lookupCache) { this.lookupCache = lookupCache; } + @Override public IdentityProviderModel lookupIdentityProviderFromIssuer(KeycloakSession session, String issuerUrl) { String alternativeKey = ComputedKey.computeKey(session.getContext().getRealm().getId(), "idp", issuerUrl); - String cachedIdpAlias = lookupCache.get(alternativeKey); - if (cachedIdpAlias != null) { - IdentityProviderModel idp = session.identityProviders().getByAlias(cachedIdpAlias); + CachedValue cachedIdpAlias = lookupCache.get(alternativeKey); + if (cachedIdpAlias instanceof CachedValue.CachedString cachedString) { + IdentityProviderModel idp = session.identityProviders().getByAlias(cachedString.value()); if (idp != null && issuerUrl.equals(idp.getConfig().get(IdentityProviderModel.ISSUER))) { return idp; } else { @@ -37,7 +46,7 @@ public class DefaultAlternativeLookupProvider implements AlternativeLookupProvid if (idps.size() == 1) { idp = idps.get(0); if (idp.getAlias() != null) { - lookupCache.put(alternativeKey, idp.getAlias()); + lookupCache.put(alternativeKey, CachedValue.ofId(idp.getAlias())); } } else if (idps.size() > 1) { throw new RuntimeException("Multiple IDPs match the same issuer: " + idps.stream().map(IdentityProviderModel::getAlias).toList()); @@ -46,12 +55,13 @@ public class DefaultAlternativeLookupProvider implements AlternativeLookupProvid return idp; } + @Override public ClientModel lookupClientFromClientAttributes(KeycloakSession session, Map attributes) { String alternativeKey = ComputedKey.computeKey(session.getContext().getRealm().getId(), "client", attributes); - String cachedClientId = lookupCache.get(alternativeKey); - if (cachedClientId != null) { - ClientModel client = session.clients().getClientByClientId(session.getContext().getRealm(), cachedClientId); + CachedValue cachedClientId = lookupCache.get(alternativeKey); + if (cachedClientId instanceof CachedValue.CachedString cachedString) { + ClientModel client = session.clients().getClientByClientId(session.getContext().getRealm(), cachedString.value()); boolean match = client != null; if (match) { for (Map.Entry e : attributes.entrySet()) { @@ -72,7 +82,7 @@ public class DefaultAlternativeLookupProvider implements AlternativeLookupProvid List clients = session.clients().searchClientsByAttributes(session.getContext().getRealm(), attributes, 0, 2).toList(); if (clients.size() == 1) { client = clients.get(0); - lookupCache.put(alternativeKey, client.getClientId()); + lookupCache.put(alternativeKey, CachedValue.ofId(client.getClientId())); } else if (clients.size() > 1) { throw new RuntimeException("Multiple clients matches attributes"); } @@ -80,7 +90,86 @@ public class DefaultAlternativeLookupProvider implements AlternativeLookupProvid return client; } + @Override + public RoleModel lookupRoleFromString(RealmModel realm, String roleName) { + if (roleName == null) { + return null; + } + + var roleModel = findRoleInCache(realm, roleName); + if (roleModel != null) { + return roleModel; + } + + // Check client roles for all possible splits by dot + int counter = 0; + int scopeIndex = roleName.lastIndexOf(CLIENT_ROLE_SEPARATOR); + while (scopeIndex >= 0 && counter < MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE) { + counter++; + String appName = roleName.substring(0, scopeIndex); + ClientModel client = realm.getClientByClientId(appName); + if (client != null) { + return storeClientRoleInCache(client, roleName, roleName.substring(scopeIndex + 1), counter); + } + + scopeIndex = roleName.lastIndexOf(CLIENT_ROLE_SEPARATOR, scopeIndex - 1); + } + if (counter >= MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE) { + logger.warnf("Not able to retrieve role model from the role name '%s'. Please use shorter role names with the limited amount of dots, roleName", roleName.length() > 100 ? roleName.substring(0, 100) + "..." : roleName); + return null; + } + + return storeRealmRoleInCache(realm, roleName); + } + @Override public void close() { } + + private RoleModel findRoleInCache(RealmModel realm, String roleName) { + var cachedRole = lookupCache.get(roleName); + if (!(cachedRole instanceof CachedValue.CachedRoleQualifier cachedRoleQualifier)) { + return null; + } + if (cachedRoleQualifier.isRealmRole()) { + var role = realm.getRole(cachedRoleQualifier.roleName()); + if (role == null) { + lookupCache.invalidate(roleName); + } + return role; + } + + var client = realm.getClientByClientId(cachedRoleQualifier.clientId()); + if (client == null) { + lookupCache.invalidate(roleName); + return null; + } + + var role = client.getRole(cachedRoleQualifier.roleName()); + if (role == null) { + lookupCache.invalidate(roleName); + } + return role; + } + + private RoleModel storeClientRoleInCache(ClientModel client, String cacheKey, String roleName, int dotCount) { + // If dotCount is equals to 1, we skip caching. + // It means, we have the following format, client-id.role-name. + // Both realm.getClientByClientId and client.getRole methods already use an internal cache. + var roleModel = client.getRole(roleName); + if (roleModel != null && dotCount > 1) { + lookupCache.put(cacheKey, CachedValue.ofClientRole(client.getClientId(), roleName)); + } + return roleModel; + } + + private RoleModel storeRealmRoleInCache(RealmModel realm, String roleName) { + // determine if roleName is a realm role + var roleModel = realm.getRole(roleName); + if (roleModel != null) { + // only cache if the role is present + lookupCache.put(roleName, CachedValue.ofRealmRole(roleName)); + } + return roleModel; + } } diff --git a/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProviderFactory.java b/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProviderFactory.java index 7dcfc4527d7..f38e1729df7 100644 --- a/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProviderFactory.java +++ b/services/src/main/java/org/keycloak/cache/DefaultAlternativeLookupProviderFactory.java @@ -8,8 +8,8 @@ import org.keycloak.models.KeycloakSessionFactory; public class DefaultAlternativeLookupProviderFactory implements AlternativeLookupProviderFactory { - private LocalCacheConfiguration cacheConfig; - private LocalCache lookupCache; + private LocalCacheConfiguration cacheConfig; + private LocalCache lookupCache; @Override public String getId() { @@ -26,7 +26,7 @@ public class DefaultAlternativeLookupProviderFactory implements AlternativeLooku Integer maximumSize = config.getInt("maximumSize", 1000); Integer expireAfter = config.getInt("expireAfter", 60); - cacheConfig = LocalCacheConfiguration.builder() + cacheConfig = LocalCacheConfiguration.builder() .name("lookup") .expiration(Duration.ofMinutes(expireAfter)) .maxSize(maximumSize) diff --git a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceRolesCondition.java b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceRolesCondition.java index 7198f29a6a8..d3e04efd212 100644 --- a/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceRolesCondition.java +++ b/services/src/main/java/org/keycloak/services/clientpolicy/condition/ClientUpdaterSourceRolesCondition.java @@ -139,7 +139,7 @@ public class ClientUpdaterSourceRolesCondition extends AbstractClientPolicyCondi RealmModel realm = session.getContext().getRealm(); for (String roleName : expectedRoles) { - RoleModel role = KeycloakModelUtils.getRoleFromString(realm, roleName); + RoleModel role = KeycloakModelUtils.getRoleFromString(session, realm, roleName); if (role == null) continue; if (user.hasRole(role)) return true; } diff --git a/tests/base/src/test/java/org/keycloak/tests/model/AlternativeLookupProviderTest.java b/tests/base/src/test/java/org/keycloak/tests/model/AlternativeLookupProviderTest.java index e6de8b88c90..262def8a8c0 100644 --- a/tests/base/src/test/java/org/keycloak/tests/model/AlternativeLookupProviderTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/model/AlternativeLookupProviderTest.java @@ -17,9 +17,15 @@ package org.keycloak.tests.model; +import java.util.concurrent.atomic.AtomicInteger; + import org.keycloak.cache.AlternativeLookupProvider; +import org.keycloak.models.ClientModel; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.utils.KeycloakModelUtils; +import org.keycloak.models.utils.RealmModelDelegate; import org.keycloak.testframework.annotations.InjectRealm; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; import org.keycloak.testframework.realm.ManagedRealm; @@ -55,6 +61,28 @@ public class AlternativeLookupProviderTest { } } + @TestOnServer + public void testLimitCountOfClientLookupsDuringGetRoleFromString(KeycloakSession session) { + AtomicInteger counter = new AtomicInteger(0); + + RealmModel realm = new RealmModelDelegate(null) { + @Override + public ClientModel getClientByClientId(String clientId) { + counter.incrementAndGet(); + return null; + } + }; + + String badRoleName = "."; + for (int i = 0 ; i < 16 ; i++) { + badRoleName = badRoleName + badRoleName; + } + Assertions.assertEquals(65536, badRoleName.length()); + + Assertions.assertNull(KeycloakModelUtils.getRoleFromString(session, realm, badRoleName)); + Assertions.assertEquals(KeycloakModelUtils.MAX_CLIENT_LOOKUPS_DURING_ROLE_RESOLVE, counter.get()); + } + protected IdentityProviderModel createModel(String alias, String providerId) { return createModel(alias, providerId,true);