diff --git a/keycloak-plugin/README.md b/keycloak-plugin/README.md index da983ae8d..da6cc0c00 100644 --- a/keycloak-plugin/README.md +++ b/keycloak-plugin/README.md @@ -27,8 +27,8 @@ This plugin doesn't come with Role and Group management, because Entando Core ro >- `keycloak.authenticated.user.default.authorizations`: **[OPTIONAL]** Use if you want to automatically assign `group:role` to any user that logs in, comma separated. Example: `administrators:admin,readers` ## Environment variables ->- `KC_CONFIG_REFRESH`: specifies the refresh period -in cron style!- of the dynamic configuration used to assign authorizations to the loggin-in users. The default is `0 * * * * *` ->- `KC_SYNC_CLEAN`: Specify the periodicity of the internal synchronization table cleanup. The default is `0 0 0/4 * * *` +>- `KC_CONFIG_REFRESH`: specifies the refresh period -in cron style!- of the dynamic configuration used to assign authorizations to the loggin-in users. The default is `0 0/15 * * * *` (every 15 minutes) +>- `KC_SYNC_CLEAN`: specifies the periodicity -in cron style!- of the internal synchronization table cleanup. **Disabled by default** (`0 0 0 31 2 *`, a date that never occurs); set a real cron expression to enable it. >- `KC_SYNC_BATCH_SIZE`: Specify the batch size for the internal synchronization table cleanup. The default is `100` diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManager.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManager.java index 1208f1598..4fe19b29d 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManager.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakAuthorizationManager.java @@ -87,7 +87,7 @@ public KeycloakAuthorizationManager(final KeycloakConfiguration configuration, /** * Immutable list of mapping elements that are currently active. The configuration is constantly updated * either by reloading the global configuration or after a certain amount of time (by default, - * one minute) + * 15 minutes — see {@code KC_CONFIG_REFRESH}) */ private final transient Map config = new ConcurrentHashMap<>(); @@ -102,7 +102,6 @@ public void initTenantAware() throws Exception { List profileMappings = new ArrayList<>(); List jwtMappings = new ArrayList<>(); - List ignore = new ArrayList<>(); List roles = new ArrayList<>(); List groups = new ArrayList<>(); List excludeUsers = new ArrayList<>(); @@ -115,6 +114,7 @@ public void initTenantAware() throws Exception { DynamicMapping dynConf = xmlMapper.readValue(xml, DynamicMapping.class); if (dynConf != null) { if (dynConf.mapping != null) { + dynConf.mapping.forEach(this::normalizeMappingElement); final Map> partitioned = dynConf.mapping.stream() @@ -128,14 +128,9 @@ public void initTenantAware() throws Exception { log.debug("{} dynamic auth mapping found, {} getConfig().profileMappings", jwtMappings.size(), profileMappings.size()); } - ignore = ofNullable(dynConf.exclusions) - .orElse(List.of()); - roles = ofNullable(dynConf.roles) - .orElseGet(List::of); - groups = ofNullable(dynConf.groups) - .orElse(List.of()); - excludeUsers = ofNullable(dynConf.excludeUsers) - .orElse(List.of()); + roles = normalizeNames(dynConf.roles); + groups = normalizeNames(dynConf.groups); + excludeUsers = normalizeNames(dynConf.excludeUsers); enabled = ofNullable(dynConf.enabled) .orElse(false); persist = ofNullable(dynConf.persist) @@ -149,12 +144,12 @@ public void initTenantAware() throws Exception { jwtMappings.forEach(m -> log.debug("jwt mapping active: {}", m.toString())); } // finally - KeycloakImportConfig cfg = new KeycloakImportConfig(profileMappings, jwtMappings, ignore, roles, groups, enabled, persist, excludeUsers); + KeycloakImportConfig cfg = new KeycloakImportConfig(profileMappings, jwtMappings, roles, groups, enabled, persist, excludeUsers); setImportConfiguration(cfg); } catch (Exception e) { log.error("Error initializing KeycloakAuthorizationManager", e); - KeycloakImportConfig cfg = new KeycloakImportConfig(List.of(), List.of(), List.of(), List.of(), List.of(), false, PersistKind.NONE, List.of()); + KeycloakImportConfig cfg = new KeycloakImportConfig(List.of(), List.of(), List.of(), List.of(), false, PersistKind.NONE, List.of()); setImportConfiguration(cfg); throw e; @@ -190,6 +185,43 @@ public void cleanSyncData() { } } + /** + * Normalize a configured list of role, group or user names: surrounding whitespace is stripped, + * blank entries are dropped and duplicates are collapsed. Names are compared against the values + * extracted from the JWT or the user profile, which are normalized the same way, so that a + * stray space in the configuration cannot silently disable an entry. + * + * @param names the raw configured names, possibly null + * @return an immutable, normalized list; never null + */ + private List normalizeNames(List names) { + return ofNullable(names) + .orElseGet(List::of) + .stream() + .filter(Objects::nonNull) + .map(String::trim) + .filter(StringUtils::isNotBlank) + .distinct() + .toList(); + } + + /** + * Trim the free-text fields of a mapping element in place. A stray leading/trailing space or + * newline pasted into {@code }, {@code } or {@code } would otherwise + * never match the real Keycloak attribute key, JWT claim path or role-group token, silently + * turning the whole mapping into a no-op instead of a validation failure. + * + * @param elem the mapping element to normalize, possibly null + */ + private void normalizeMappingElement(DynamicMappingElement elem) { + if (elem == null) { + return; + } + elem.attribute = StringUtils.trim(elem.attribute); + elem.path = StringUtils.trim(elem.path); + elem.separator = StringUtils.trim(elem.separator); + } + /** * Check whether the dynamic configuration element provided is valid * @param elem the single dynamic mapping element to validate @@ -236,7 +268,8 @@ public void processNewUser(final UserDetails user, final String token, final boo readLock.lock(); try { final List excludedUsers = getImportConfiguration().getExcludeUsers(); - if (excludedUsers != null && user.getUsername() != null && excludedUsers.contains(user.getUsername())) { + if (excludedUsers != null && user.getUsername() != null + && excludedUsers.contains(user.getUsername().trim())) { log.debug("user {} is in the excludeUsers list, skipping dynamic sync", user.getUsername()); return; } @@ -576,30 +609,27 @@ private Authorization finalizeAssociation(KeycloakUser user, DynamicMappingEleme return finalizeAssociation(user, roleName, groupName, true); } - private boolean isIgnored(String name) { - if (getImportConfiguration().getIgnore() == null || StringUtils.isBlank(name)) { - return false; - } - return getImportConfiguration().getIgnore().contains(name.trim()); - } - private Authorization finalizeAssociation(KeycloakUser user, String roleName, String groupName, boolean createRoleIfMissing) { - // is it excluded? - if (isIgnored(roleName) || isIgnored(groupName)) { - log.info("Role {} or Group {} is in the exclusions list. Skipping assignment for user {}", roleName, groupName, user.getUsername()); + // names are normalized here so that every source (JWT claim, profile attribute, role-group + // token) is compared against the allowlists on the same terms + final String role = StringUtils.trimToNull(roleName); + final String group = StringUtils.trimToNull(groupName); + + if (role == null && group == null) { + log.warn("Blank role and group extracted for user {}, discarding", user.getUsername()); return null; } // are they managed? - if (StringUtils.isNotBlank(roleName) && !getImportConfiguration().getRoles().contains(roleName)) { - log.warn("Role {} is not in the roles allowlist. Skipping assignment for user {}", roleName, user.getUsername()); + if (StringUtils.isNotBlank(role) && !getImportConfiguration().getRoles().contains(role)) { + log.warn("Role {} is not in the roles allowlist. Skipping assignment for user {}", role, user.getUsername()); return null; } - if (StringUtils.isNotBlank(groupName) && !getImportConfiguration().getGroups().contains(groupName)) { - log.warn("Group {} is not in the groups allowlist. Skipping assignment for user {}", groupName, user.getUsername()); + if (StringUtils.isNotBlank(group) && !getImportConfiguration().getGroups().contains(group)) { + log.warn("Group {} is not in the groups allowlist. Skipping assignment for user {}", group, user.getUsername()); return null; } - return createAuthorization(roleName, groupName, createRoleIfMissing); + return createAuthorization(role, group, createRoleIfMissing); } private Authorization createAuthorization(String roleName, String groupName, boolean createRoleIfMissing) { diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakImportConfig.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakImportConfig.java index d1d64d839..a89ce08f1 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakImportConfig.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/KeycloakImportConfig.java @@ -15,7 +15,6 @@ public class KeycloakImportConfig { private transient List profileMappings = new ArrayList<>(); private transient List jwtMappings = new ArrayList<>(); - private transient List ignore; private transient List roles; private transient List groups; private transient Boolean enabled; diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMapping.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMapping.java index afdaff265..67a83b9a8 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMapping.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMapping.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlElementWrapper; +import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlProperty; import com.fasterxml.jackson.dataformat.xml.annotation.JacksonXmlRootElement; import java.util.List; @@ -10,21 +11,27 @@ public class DynamicMapping { @JacksonXmlElementWrapper(localName = "mappings") + @JacksonXmlProperty(localName = "mapping") public List mapping; - @JacksonXmlElementWrapper(localName = "exclusions") - public List exclusions; - @JacksonXmlElementWrapper(localName = "roles") + @JacksonXmlProperty(localName = "role") public List roles; @JacksonXmlElementWrapper(localName = "groups") + @JacksonXmlProperty(localName = "group") public List groups; @JacksonXmlElementWrapper(localName = "excludeUsers") + @JacksonXmlProperty(localName = "excludeUser") public List excludeUsers; public Boolean enabled; public PersistKind persist; + @Deprecated(forRemoval = true) + @JacksonXmlElementWrapper(localName = "exclusions") + @JacksonXmlProperty(localName = "exclusion") + public List exclusions; + } diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMappingKind.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMappingKind.java index d682702b0..7a2888331 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMappingKind.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/DynamicMappingKind.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.annotation.JsonValue; import java.util.Arrays; import lombok.Getter; +import org.apache.commons.lang3.StringUtils; public enum DynamicMappingKind { @@ -30,8 +31,9 @@ public String getXmlValue() { @JsonCreator public static DynamicMappingKind fromValue(String value) { + final String trimmed = StringUtils.trim(value); return Arrays.stream(values()) - .filter(k -> k.kind.equalsIgnoreCase(value)) + .filter(k -> k.kind.equalsIgnoreCase(trimmed)) .findFirst() .orElseThrow(() -> new IllegalArgumentException("Unknown DynamicMappingKind: " + value)); diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/PersistKind.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/PersistKind.java index 71026a97e..131254309 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/PersistKind.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/mapping/PersistKind.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonValue; import java.util.Arrays; +import org.apache.commons.lang3.StringUtils; public enum PersistKind { NONE("none"), @@ -24,8 +25,9 @@ public String getXmlValue() { @JsonCreator public static PersistKind fromValue(String value) { + final String trimmed = StringUtils.trim(value); return Arrays.stream(values()) - .filter(k -> k.kind.equalsIgnoreCase(value)) + .filter(k -> k.kind.equalsIgnoreCase(trimmed)) .findFirst() .orElseThrow(() -> new IllegalArgumentException("Unknown PersistKind: " + value)); diff --git a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/oidc/OidcMappingHelper.java b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/oidc/OidcMappingHelper.java index d09d1b3e3..21f5d568f 100644 --- a/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/oidc/OidcMappingHelper.java +++ b/keycloak-plugin/src/main/java/org/entando/entando/keycloak/services/oidc/OidcMappingHelper.java @@ -8,7 +8,6 @@ import java.util.Base64; import java.util.Collections; import java.util.List; -import java.util.stream.Collectors; import org.entando.entando.ent.util.EntLogging.EntLogFactory; import org.entando.entando.ent.util.EntLogging.EntLogger; import org.entando.entando.keycloak.services.mapping.DynamicMappingElement; @@ -100,21 +99,26 @@ private static List extractAuthorizationsFromNode(JsonNode authNode, Dyn if (authNode.isArray()) { return extractFromArrayNode(authNode); } - if (authNode.isTextual()) { - return List.of(authNode.asText()); - } - if (authNode.isNumber()) { - return List.of(authNode.asText()); + if (authNode.isTextual() || authNode.isNumber()) { + final String value = authNode.asText().trim(); + return value.isEmpty() ? Collections.emptyList() : List.of(value); } log.warn("Unsupported node type for path '{}' in JWT: {}", claimMapper.path, authNode.getNodeType()); return Collections.emptyList(); } + /** + * Collect the textual elements of a claim array, discarding blank ones. A blank name cannot be + * matched against the allowlists, so keeping it would only produce an empty authorization. + * + * @param arrayNode the claim array node + * @return the trimmed, non-blank values + */ private static List extractFromArrayNode(JsonNode arrayNode) { List authorizations = new ArrayList<>(); for (JsonNode node : arrayNode) { - if (node.isTextual()) { - authorizations.add(node.asText()); + if (node.isTextual() && !node.asText().isBlank()) { + authorizations.add(node.asText().trim()); } } return authorizations; diff --git a/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/changeSetPort.xml b/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/changeSetPort.xml index 02b9537b7..389d22903 100644 --- a/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/changeSetPort.xml +++ b/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/changeSetPort.xml @@ -6,5 +6,6 @@ http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.4.xsd"> + diff --git a/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/port/00000000000004_dataPort_production_removeExclusions.xml b/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/port/00000000000004_dataPort_production_removeExclusions.xml new file mode 100644 index 000000000..15e859df8 --- /dev/null +++ b/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/port/00000000000004_dataPort_production_removeExclusions.xml @@ -0,0 +1,99 @@ + + + + + + + + false + FULL + + + true + groups + GROUPCLAIM + + + true + realm_access.roles + ROLECLAIM + + + false + realm_access.roles + ROLEGROUPCLAIM + _SEP_ + + + false + AD_ROLE + ROLE + + + false + AD_GROUP + GROUP + + + false + AD_GROUPROLE + ROLEGROUP + _r_ + + + + default-roles-entando-development + offline_access + uma_authorization + + + imported_role + imported_role2 + + + imported_group + imported_group2 + + +' + ]]> + + + + version = 'production' AND item = 'dynamicAuthMapping' + + + + diff --git a/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/port/clob/production/sysconfig_kc_no_exclusions.xml b/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/port/clob/production/sysconfig_kc_no_exclusions.xml new file mode 100644 index 000000000..f552f4809 --- /dev/null +++ b/keycloak-plugin/src/main/resources/liquibase/entando-keycloak-auth/port/clob/production/sysconfig_kc_no_exclusions.xml @@ -0,0 +1,47 @@ + + + false + FULL + + + true + groups + GROUPCLAIM + + + true + realm_access.roles + ROLECLAIM + + + false + realm_access.roles + ROLEGROUPCLAIM + _SEP_ + + + false + AD_ROLE + ROLE + + + false + AD_GROUP + GROUP + + + false + AD_GROUPROLE + ROLEGROUP + _r_ + + + + imported_role + imported_role2 + + + imported_group + imported_group2 + + diff --git a/keycloak-plugin/src/main/resources/spring/plugins/keycloak/aps/keycloak.xml b/keycloak-plugin/src/main/resources/spring/plugins/keycloak/aps/keycloak.xml index 303eaa7f1..c2fa802d9 100644 --- a/keycloak-plugin/src/main/resources/spring/plugins/keycloak/aps/keycloak.xml +++ b/keycloak-plugin/src/main/resources/spring/plugins/keycloak/aps/keycloak.xml @@ -10,11 +10,11 @@ - + + cron="${KC_CONFIG_REFRESH:0 0/15 * * * *}" /> a.getRole().getName().equals("roleB") && a.getGroup().getName().equals("groupB")); } + /** + * The retired {@code } element must not break parsing of pre-existing configurations: + * a name that is only in {@code } is still discarded, but by the roles allowlist. + */ @Test - void testExclusions() throws Exception { + void testLegacyExclusionsElementStillParses() throws Exception { String xml = "" + " true" + " none" @@ -299,6 +303,287 @@ void testExclusions() throws Exception { assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("role1"); } + /** + * Behaviour change: a name present in both the retired {@code } element and the + * allowlist used to be discarded (and revoked). The allowlist is now the only filter, so it is + * assigned. + */ + @Test + void testLegacyExclusionsElementIsNotEnforced() throws Exception { + String xml = "" + + " true" + + " none" + + " " + + " " + + " true" + + " kc_roles" + + " role" + + " " + + " " + + " " + + " role1" + + " " + + " " + + " role1" + + " " + + ""; + setMappingConfig(xml); + + KeycloakUser user = createKeycloakUser("test-user", "kc_roles", List.of("role1")); + + manager.processNewUser(user, null, false); + + assertThat(user.getAuthorizations()).hasSize(1); + assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("role1"); + } + + /** + * Allowlist entries are matched after trimming on both sides, so padding in the configuration + * cannot silently disable an entry. + */ + @Test + void testAllowlistNamesAreTrimmed() throws Exception { + String xml = "" + + " true" + + " none" + + " " + + " " + + " true" + + " realm_access.roles" + + " roleclaim" + + " " + + " " + + " " + + " role1 " + + " " + + " " + + ""; + setMappingConfig(xml); + + String token = createToken("{\"iat\":123, \"realm_access\":{\"roles\":[\" role1 \"]}}"); + KeycloakUser user = createKeycloakUser("test-user", null, null); + + manager.processNewUser(user, token, true); + + assertThat(user.getAuthorizations()).hasSize(1); + assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("role1"); + } + + /** + * Regression test: {@code DynamicMappingKind.fromValue} used to compare the raw XML text with + * no trimming, so a stray trailing newline in {@code } (easy to introduce via copy/paste + * or templating) threw during deserialization. That exception was only caught by the top-level + * handler in {@code initTenantAware}, which discarded the *entire* configuration for *every* + * mapping and every user, not just the offending one. The value must now be trimmed before + * matching, so the mapping keeps working. + */ + @Test + void testMappingKindWithTrailingNewlineDoesNotBreakConfig() throws Exception { + String xml = "" + + " true" + + " none" + + " " + + " " + + " true" + + " kc_roles" + + " role\n" + + " " + + " " + + " " + + " role1" + + " " + + ""; + setMappingConfig(xml); + + KeycloakUser user = createKeycloakUser("test-user", "kc_roles", List.of("role1")); + + manager.processNewUser(user, null, false); + + assertThat(user.getAuthorizations()).hasSize(1); + assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("role1"); + } + + /** + * Same regression as {@link #testMappingKindWithTrailingNewlineDoesNotBreakConfig}, but for + * {@code PersistKind.fromValue} and the top-level {@code } element. + */ + @Test + void testPersistKindWithTrailingNewlineDoesNotBreakConfig() throws Exception { + String xml = "" + + " true" + + " full\n" + + " " + + " " + + " true" + + " kc_roles" + + " role" + + " " + + " " + + " " + + " role1" + + " " + + ""; + setMappingConfig(xml); + + Role role1 = new Role(); + role1.setName("role1"); + when(roleManager.getRole("role1")).thenReturn(role1); + + KeycloakUser user = createKeycloakUser("test-user", "kc_roles", List.of("role1")); + + manager.processNewUser(user, null, false); + + assertThat(user.getAuthorizations()).hasSize(1); + assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("role1"); + } + + /** + * {@code } is a plain string used verbatim as a Keycloak profile-attribute key + * ({@code Map.containsKey}). Unlike {@code }/{@code } this never throws, but + * without trimming, surrounding whitespace makes the key lookup fail silently and the whole + * mapping produces no authorizations for any user. + */ + @Test + void testMappingAttributeWithSurroundingWhitespaceIsTrimmed() throws Exception { + String xml = """ + + true + none + + + true + + kc_roles + + role + + + + role1 + + + """; + setMappingConfig(xml); + + KeycloakUser user = createKeycloakUser("test-user", "kc_roles", List.of("role1")); + + manager.processNewUser(user, null, false); + + assertThat(user.getAuthorizations()).hasSize(1); + assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("role1"); + } + + /** + * Same as {@link #testMappingAttributeWithSurroundingWhitespaceIsTrimmed} but for {@code } + * on a JWT-claim mapping kind: an untrimmed path never resolves to a JSON pointer that exists in + * the token, so the claim lookup silently misses for every user. + */ + @Test + void testMappingPathWithSurroundingWhitespaceIsTrimmed() throws Exception { + String xml = """ + + true + none + + + true + + realm_access.roles + + roleclaim + + + + jwt-role1 + + + """; + setMappingConfig(xml); + + String token = createToken("{\"iat\":123, \"realm_access\":{\"roles\":[\"jwt-role1\"]}}"); + KeycloakUser user = createKeycloakUser("test-user", null, null); + + manager.processNewUser(user, token, true); + + assertThat(user.getAuthorizations()).hasSize(1); + assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("jwt-role1"); + } + + /** + * Same as {@link #testMappingAttributeWithSurroundingWhitespaceIsTrimmed} but for + * {@code }: an untrimmed separator never matches inside a real token, so every + * rolegroup value falls through as unsplittable. + */ + @Test + void testMappingSeparatorWithSurroundingWhitespaceIsTrimmed() throws Exception { + String xml = "" + + " true" + + " full" + + " " + + " " + + " true" + + " kc_rolegroups" + + " rolegroup" + + " _SEP_ " + + " " + + " " + + " " + + " role1" + + " " + + " " + + " group1" + + " " + + ""; + setMappingConfig(xml); + + KeycloakUser user = createKeycloakUser("test-user", "kc_rolegroups", List.of("role1_SEP_group1")); + + Group group1 = new Group(); + group1.setName("group1"); + Role role1 = new Role(); + role1.setName("role1"); + when(groupManager.getGroup("group1")).thenReturn(group1); + when(roleManager.getRole("role1")).thenReturn(role1); + + manager.processNewUser(user, null, false); + + assertThat(user.getAuthorizations()).hasSize(1); + assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("role1"); + assertThat(user.getAuthorizations().get(0).getGroup().getName()).isEqualTo("group1"); + } + + /** + * Blank claim values must not survive extraction: they cannot match an allowlist and would + * otherwise yield an authorization carrying neither a role nor a group. + */ + @Test + void testBlankClaimValuesProduceNoAuthorization() throws Exception { + String xml = "" + + " true" + + " none" + + " " + + " " + + " true" + + " realm_access.roles" + + " roleclaim" + + " " + + " " + + " " + + " role1" + + " " + + ""; + setMappingConfig(xml); + + String token = createToken("{\"iat\":123, \"realm_access\":{\"roles\":[\"\", \" \", \"role1\"]}}"); + KeycloakUser user = createKeycloakUser("test-user", null, null); + + manager.processNewUser(user, token, true); + + assertThat(user.getAuthorizations()).hasSize(1); + assertThat(user.getAuthorizations().get(0).getRole().getName()).isEqualTo("role1"); + assertThat(user.getAuthorizations()).noneMatch(a -> a.getRole() == null && a.getGroup() == null); + } + @Test void testPersistAuthorizations() throws Exception { String xml = ""