Allow duplicated keys in advanced claim mappers

Closes https://github.com/keycloak/keycloak/issues/22638
This commit is contained in:
rmartinc 2023-09-11 15:45:31 +02:00 committed by Pedro Igor
parent 227b841c4a
commit 743bb696d9
11 changed files with 109 additions and 73 deletions

View file

@ -22,6 +22,7 @@ import org.keycloak.util.JsonSerialization;
import java.io.IOException;
import java.io.Serializable;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
@ -93,12 +94,16 @@ public class IdentityProviderMapperModel implements Serializable {
this.config = config;
}
public Map<String, String> getConfigMap(String configKey) {
public Map<String, List<String>> getConfigMap(String configKey) {
String configMap = config.get(configKey);
try {
List<StringPair> map = JsonSerialization.readValue(configMap, MAP_TYPE_REPRESENTATION);
return map.stream().collect(Collectors.toMap(StringPair::getKey, StringPair::getValue));
return map.stream().collect(
Collectors.collectingAndThen(
Collectors.groupingBy(StringPair::getKey,
Collectors.mapping(StringPair::getValue, Collectors.toUnmodifiableList())),
Collections::unmodifiableMap));
} catch (IOException e) {
throw new RuntimeException("Could not deserialize json: " + configMap, e);
}

View file

@ -100,15 +100,16 @@ public class AdvancedClaimToGroupMapper extends AbstractClaimToGroupMapper {
@Override
protected boolean applies(IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) {
Map<String, String> claims = mapperModel.getConfigMap(CLAIM_PROPERTY_NAME);
Map<String, List<String>> claims = mapperModel.getConfigMap(CLAIM_PROPERTY_NAME);
boolean areClaimValuesRegex = Boolean.parseBoolean(mapperModel.getConfig().get(ARE_CLAIM_VALUES_REGEX_PROPERTY_NAME));
for (Map.Entry<String, String> claim : claims.entrySet()) {
Object value = getClaimValue(context, claim.getKey());
boolean claimValuesMismatch = !(areClaimValuesRegex ? valueMatchesRegex(claim.getValue(), value) : valueEquals(claim.getValue(), value));
if (claimValuesMismatch) {
return false;
for (Map.Entry<String, List<String>> claim : claims.entrySet()) {
Object claimValue = getClaimValue(context, claim.getKey());
for (String value : claim.getValue()) {
boolean claimValuesMismatch = !(areClaimValuesRegex ? valueMatchesRegex(value, claimValue) : valueEquals(value, claimValue));
if (claimValuesMismatch) {
return false;
}
}
}

View file

@ -46,7 +46,7 @@ public class AdvancedClaimToRoleMapper extends AbstractClaimToRoleMapper {
public static final String[] COMPATIBLE_PROVIDERS = {KeycloakOIDCIdentityProviderFactory.PROVIDER_ID, OIDCIdentityProviderFactory.PROVIDER_ID};
private static final Set<IdentityProviderSyncMode> IDENTITY_PROVIDER_SYNC_MODES = new HashSet<>(Arrays.asList(IdentityProviderSyncMode.values()));
private static final List<ProviderConfigProperty> configProperties = new ArrayList<ProviderConfigProperty>();
private static final List<ProviderConfigProperty> configProperties = new ArrayList<>();
static {
ProviderConfigProperty claimsProperty = new ProviderConfigProperty();
@ -108,15 +108,16 @@ public class AdvancedClaimToRoleMapper extends AbstractClaimToRoleMapper {
@Override
protected boolean applies(IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) {
Map<String, String> claims = mapperModel.getConfigMap(CLAIM_PROPERTY_NAME);
Map<String, List<String>> claims = mapperModel.getConfigMap(CLAIM_PROPERTY_NAME);
boolean areClaimValuesRegex = Boolean.parseBoolean(mapperModel.getConfig().get(ARE_CLAIM_VALUES_REGEX_PROPERTY_NAME));
for (Map.Entry<String, String> claim : claims.entrySet()) {
Object value = getClaimValue(context, claim.getKey());
boolean claimValuesMismatch = !(areClaimValuesRegex ? valueMatchesRegex(claim.getValue(), value) : valueEquals(claim.getValue(), value));
if (claimValuesMismatch) {
return false;
for (Map.Entry<String, List<String>> claim : claims.entrySet()) {
Object claimValue = getClaimValue(context, claim.getKey());
for (String value : claim.getValue()) {
boolean claimValuesMismatch = !(areClaimValuesRegex ? valueMatchesRegex(value, claimValue) : valueEquals(value, claimValue));
if (claimValuesMismatch) {
return false;
}
}
}

View file

@ -104,30 +104,31 @@ public class ClaimToUserSessionNoteMapper extends AbstractClaimMapper {
}
private void addClaimsToSessionNote(IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) {
Map<String, String> claims = mapperModel.getConfigMap(CLAIMS_PROPERTY_NAME);
Map<String, List<String>> claims = mapperModel.getConfigMap(CLAIMS_PROPERTY_NAME);
boolean areClaimValuesRegex =
Boolean.parseBoolean(mapperModel.getConfig().get(ARE_CLAIM_VALUES_REGEX_PROPERTY_NAME));
for (Map.Entry<String, String> claim : claims.entrySet()) {
Object valueObj = getClaimValue(context, claim.getKey());
for (Map.Entry<String, List<String>> claim : claims.entrySet()) {
Object claimValueObj = getClaimValue(context, claim.getKey());
for (String value : claim.getValue()) {
if (claimValueObj != null) {
if (!(claimValueObj instanceof String)) {
LOG.warnf(
"Claim '%s' does not contain a string value for user with brokerUserId '%s'. "
+ "Actual value is of type '%s': %s",
claim.getKey(),
context.getBrokerUserId(), claimValueObj.getClass(), claimValueObj);
continue;
}
if (valueObj != null) {
if (!(valueObj instanceof String)) {
LOG.warnf(
"Claim '%s' does not contain a string value for user with brokerUserId '%s'. " +
"Actual value is of type '%s': %s",
claim.getKey(),
context.getBrokerUserId(), valueObj.getClass(), valueObj);
continue;
}
String claimValue = (String) claimValueObj;
String value = (String) valueObj;
boolean claimValuesMatch = areClaimValuesRegex ? valueMatchesRegex(value, claimValue)
: valueEquals(value, claimValue);
boolean claimValuesMatch = areClaimValuesRegex ? valueMatchesRegex(claim.getValue(), value)
: valueEquals(claim.getValue(), value);
if (claimValuesMatch) {
context.getAuthenticationSession().setUserSessionNote(claim.getKey(), value);
if (claimValuesMatch) {
context.getAuthenticationSession().setUserSessionNote(claim.getKey(), claimValue);
}
}
}
}

View file

@ -17,7 +17,6 @@
package org.keycloak.broker.saml.mappers;
import org.keycloak.broker.provider.BrokeredIdentityContext;
import org.keycloak.broker.provider.ConfigConstants;
import org.keycloak.broker.saml.SAMLEndpoint;
@ -114,8 +113,9 @@ public class AdvancedAttributeToGroupMapper extends AbstractAttributeToGroupMapp
return "If all attributes exists, assign the user to the specified group.";
}
@Override
protected boolean applies(final IdentityProviderMapperModel mapperModel, final BrokeredIdentityContext context) {
Map<String, String> attributes = mapperModel.getConfigMap(ATTRIBUTE_PROPERTY_NAME);
Map<String, List<String>> attributes = mapperModel.getConfigMap(ATTRIBUTE_PROPERTY_NAME);
boolean areAttributeValuesRegexes = Boolean.parseBoolean(mapperModel.getConfig().get(ARE_ATTRIBUTE_VALUES_REGEX_PROPERTY_NAME));
AssertionType assertion = (AssertionType) context.getContextData().get(SAMLEndpoint.SAML_ASSERTION);
@ -124,19 +124,21 @@ public class AdvancedAttributeToGroupMapper extends AbstractAttributeToGroupMapp
return false;
}
for (Map.Entry<String, String> attribute : attributes.entrySet()) {
String attributeKey = attribute.getKey();
List<Object> attributeValues = attributeAssertions.stream()
.flatMap(statements -> statements.getAttributes().stream())
.filter(choiceType -> attributeKey.equals(choiceType.getAttribute().getName())
|| attributeKey.equals(choiceType.getAttribute().getFriendlyName()))
// Several statements with same name are treated like one with several values
.flatMap(choiceType -> choiceType.getAttribute().getAttributeValue().stream())
.collect(Collectors.toList());
for (Map.Entry<String, List<String>> entry : attributes.entrySet()) {
String attributeKey = entry.getKey();
for (String value : entry.getValue()) {
List<Object> attributeValues = attributeAssertions.stream()
.flatMap(statements -> statements.getAttributes().stream())
.filter(choiceType -> attributeKey.equals(choiceType.getAttribute().getName())
|| attributeKey.equals(choiceType.getAttribute().getFriendlyName()))
// Several statements with same name are treated like one with several values
.flatMap(choiceType -> choiceType.getAttribute().getAttributeValue().stream())
.collect(Collectors.toList());
boolean attributeValueMatch = areAttributeValuesRegexes ? valueMatchesRegex(attribute.getValue(), attributeValues) : attributeValues.contains(attribute.getValue());
if (!attributeValueMatch) {
return false;
boolean attributeValueMatch = areAttributeValuesRegexes ? valueMatchesRegex(value, attributeValues) : attributeValues.contains(value);
if (!attributeValueMatch) {
return false;
}
}
}

View file

@ -17,7 +17,6 @@
package org.keycloak.broker.saml.mappers;
import org.keycloak.broker.provider.BrokeredIdentityContext;
import org.keycloak.broker.provider.ConfigConstants;
import org.keycloak.broker.saml.SAMLEndpoint;
@ -122,8 +121,9 @@ public class AdvancedAttributeToRoleMapper extends AbstractAttributeToRoleMapper
return "If the set of attributes exists and can be matched, grant the user the specified realm or client role.";
}
@Override
protected boolean applies(final IdentityProviderMapperModel mapperModel, final BrokeredIdentityContext context) {
Map<String, String> attributes = mapperModel.getConfigMap(ATTRIBUTE_PROPERTY_NAME);
Map<String, List<String>> attributes = mapperModel.getConfigMap(ATTRIBUTE_PROPERTY_NAME);
boolean areAttributeValuesRegexes = Boolean.parseBoolean(mapperModel.getConfig().get(ARE_ATTRIBUTE_VALUES_REGEX_PROPERTY_NAME));
AssertionType assertion = (AssertionType) context.getContextData().get(SAMLEndpoint.SAML_ASSERTION);
@ -132,19 +132,21 @@ public class AdvancedAttributeToRoleMapper extends AbstractAttributeToRoleMapper
return false;
}
for (Map.Entry<String, String> attribute : attributes.entrySet()) {
String attributeKey = attribute.getKey();
List<Object> attributeValues = attributeAssertions.stream()
.flatMap(statements -> statements.getAttributes().stream())
.filter(choiceType -> attributeKey.equals(choiceType.getAttribute().getName())
|| attributeKey.equals(choiceType.getAttribute().getFriendlyName()))
// Several statements with same name are treated like one with several values
.flatMap(choiceType -> choiceType.getAttribute().getAttributeValue().stream())
.collect(Collectors.toList());
for (Map.Entry<String, List<String>> entry : attributes.entrySet()) {
String attributeKey = entry.getKey();
for (String value : entry.getValue()) {
List<Object> attributeValues = attributeAssertions.stream()
.flatMap(statements -> statements.getAttributes().stream())
.filter(choiceType -> attributeKey.equals(choiceType.getAttribute().getName())
|| attributeKey.equals(choiceType.getAttribute().getFriendlyName()))
// Several statements with same name are treated like one with several values
.flatMap(choiceType -> choiceType.getAttribute().getAttributeValue().stream())
.collect(Collectors.toList());
boolean attributeValueMatch = areAttributeValuesRegexes ? valueMatchesRegex(attribute.getValue(), attributeValues) : attributeValues.contains(attribute.getValue());
if (!attributeValueMatch) {
return false;
boolean attributeValueMatch = areAttributeValuesRegexes ? valueMatchesRegex(value, attributeValues) : attributeValues.contains(value);
if (!attributeValueMatch) {
return false;
}
}
}

View file

@ -27,6 +27,10 @@ public abstract class AbstractAdvancedRoleMapperTest extends AbstractRoleMapperT
" \"value\": \"value 1\"\n" +
" },\n" +
" {\n" +
" \"key\": \"" + KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME + "\",\n" +
" \"value\": \"value 2\"\n" +
" },\n" +
" {\n" +
" \"key\": \"" + KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2 + "\",\n" +
" \"value\": \"value 2\"\n" +
" }\n" +
@ -152,7 +156,7 @@ public abstract class AbstractAdvancedRoleMapperTest extends AbstractRoleMapperT
UserRepresentation user = findUser(bc.providerRealmName(), bc.getUserLogin(), bc.getUserEmail());
ImmutableMap<String, List<String>> matchingAttributes = ImmutableMap.<String, List<String>> builder()
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME,
ImmutableList.<String> builder().add("value 1").build())
ImmutableList.<String> builder().add("value 1").add("value 2").build())
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2,
ImmutableList.<String> builder().add(newValueForAttribute2).build())
.put("some.other.attribute", ImmutableList.<String> builder().add("some value").build())
@ -174,7 +178,7 @@ public abstract class AbstractAdvancedRoleMapperTest extends AbstractRoleMapperT
private static Map<String, List<String>> createMatchingUserConfig() {
return ImmutableMap.<String, List<String>> builder()
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME,
ImmutableList.<String> builder().add("value 1").build())
ImmutableList.<String> builder().add("value 1").add("value 2").build())
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2,
ImmutableList.<String> builder().add("value 2").build())
.build();

View file

@ -36,6 +36,10 @@ public abstract class AbstractGroupBrokerMapperTest extends AbstractGroupMapperT
" \"value\": \"value 1\"\n" +
" },\n" +
" {\n" +
" \"key\": \"" + KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME + "\",\n" +
" \"value\": \"value 2\"\n" +
" },\n" +
" {\n" +
" \"key\": \"" + KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2 + "\",\n" +
" \"value\": \"value 2\"\n" +
" }\n" +
@ -69,7 +73,7 @@ public abstract class AbstractGroupBrokerMapperTest extends AbstractGroupMapperT
protected void updateUser() {
UserRepresentation user = findUser(bc.providerRealmName(), bc.getUserLogin(), bc.getUserEmail());
ImmutableMap<String, List<String>> matchingAttributes = ImmutableMap.<String, List<String>>builder()
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.<String>builder().add("value 1").build())
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME, ImmutableList.<String>builder().add("value 1").add("value 2").build())
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2, ImmutableList.<String>builder().add(newValueForAttribute2).build())
.put("some.other.attribute", ImmutableList.<String>builder().add("some value").build())
.build();
@ -112,7 +116,7 @@ public abstract class AbstractGroupBrokerMapperTest extends AbstractGroupMapperT
protected static Map<String, List<String>> createMatchingAttributes() {
return ImmutableMap.<String, List<String>> builder()
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME,
ImmutableList.<String> builder().add("value 1").build())
ImmutableList.<String> builder().add("value 1").add("value 2").build())
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2,
ImmutableList.<String> builder().add("value 2").build())
.build();

View file

@ -28,6 +28,9 @@ public class KcSamlAdvancedAttributeToGroupMapperTest extends AbstractGroupBroke
" {\n" +
" \"key\": \"" + ATTRIBUTE_TO_MAP_FRIENDLY_NAME + "\",\n" +
" \"value\": \"value 1\"\n" +
" },\n" +" {\n" +
" \"key\": \"" + ATTRIBUTE_TO_MAP_FRIENDLY_NAME + "\",\n" +
" \"value\": \"value 2\"\n" +
" },\n" +
" {\n" +
" \"key\": \"" + KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2 + "\",\n" +
@ -65,7 +68,7 @@ public class KcSamlAdvancedAttributeToGroupMapperTest extends AbstractGroupBroke
public void attributeFriendlyNameGetsConsideredAndMatchedToGroup() {
createAdvancedGroupMapper(ATTRIBUTES, false,KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2);
createUserInProviderRealm(ImmutableMap.<String, List<String>> builder()
.put(ATTRIBUTE_TO_MAP_FRIENDLY_NAME, ImmutableList.<String> builder().add("value 1").build())
.put(ATTRIBUTE_TO_MAP_FRIENDLY_NAME, ImmutableList.<String> builder().add("value 1").add("value 2").build())
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2,
ImmutableList.<String> builder().add("value 2").build())
.build());

View file

@ -26,6 +26,10 @@ public class KcSamlAdvancedAttributeToRoleMapperTest extends AbstractAdvancedRol
" \"value\": \"value 1\"\n" +
" },\n" +
" {\n" +
" \"key\": \"" + ATTRIBUTE_TO_MAP_FRIENDLY_NAME + "\",\n" +
" \"value\": \"value 2\"\n" +
" },\n" +
" {\n" +
" \"key\": \"" + KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2 + "\",\n" +
" \"value\": \"value 2\"\n" +
" }\n" +
@ -58,7 +62,7 @@ public class KcSamlAdvancedAttributeToRoleMapperTest extends AbstractAdvancedRol
public void attributeFriendlyNameGetsConsideredAndMatchedToRole() {
createAdvancedRoleMapper(ATTRIBUTES, false);
createUserInProviderRealm(ImmutableMap.<String, List<String>> builder()
.put(ATTRIBUTE_TO_MAP_FRIENDLY_NAME, ImmutableList.<String> builder().add("value 1").build())
.put(ATTRIBUTE_TO_MAP_FRIENDLY_NAME, ImmutableList.<String> builder().add("value 1").add("value 2").build())
.put(KcOidcBrokerConfiguration.ATTRIBUTE_TO_MAP_NAME_2,
ImmutableList.<String> builder().add("value 2").build())
.build());

View file

@ -123,7 +123,7 @@ public class OidcClaimToUserSessionNoteMapperTest extends AbstractIdentityProvid
@Test
public void claimIsNotPropagatedWhenNameDoesNotMatch() {
createUserSessionNoteIdpMapper(IdentityProviderMapperSyncMode.IMPORT, "something-unexpected");
createUserSessionNoteIdpMapper(IdentityProviderMapperSyncMode.IMPORT, "something-unexpected-1", "something-unexpected-2");
AccessToken accessToken = login();
@ -157,7 +157,7 @@ public class OidcClaimToUserSessionNoteMapperTest extends AbstractIdentityProvid
}
private IdentityProviderMapperRepresentation createUserSessionNoteIdpMapper(IdentityProviderMapperSyncMode syncMode,
String matchingValue) {
String... matchingValue) {
IdentityProviderMapperRepresentation mapper = new IdentityProviderMapperRepresentation();
mapper.setName("User Session Note Idp Mapper");
mapper.setIdentityProviderMapper(ClaimToUserSessionNoteMapper.PROVIDER_ID);
@ -170,8 +170,17 @@ public class OidcClaimToUserSessionNoteMapperTest extends AbstractIdentityProvid
return persistMapper(mapper);
}
private String createClaimsConfig(String matchingValue) {
return "[{\"key\":\"" + CLAIM_NAME + "\",\"value\":\"" + matchingValue + "\"}]";
private String createClaimsConfig(String... matchingValue) {
StringBuilder sb = new StringBuilder();
sb.append("[");
if (matchingValue != null) {
for (String value : matchingValue) {
sb.append("{\"key\":\"").append(CLAIM_NAME).append("\",\"value\":\"").append(value).append("\"},");
}
sb.setLength(sb.length() - 1);
}
sb.append("]");
return sb.toString();
}
private void updateProviderHardcodedClaimMapper(String value) {