diff --git a/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java b/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java index f7a26e9899..f2c8a7aad7 100755 --- a/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java +++ b/server-spi-private/src/main/java/org/keycloak/broker/provider/BrokeredIdentityContext.java @@ -161,6 +161,10 @@ public class BrokeredIdentityContext { getContextData().put(Constants.USER_ATTRIBUTES_PREFIX + attributeName, list); } + public void setUserAttribute(String attributeName, List attributeValues) { + getContextData().put(Constants.USER_ATTRIBUTES_PREFIX + attributeName, attributeValues); + } + public String getUserAttribute(String attributeName) { List userAttribute = (List) getContextData().get(Constants.USER_ATTRIBUTES_PREFIX + attributeName); if (userAttribute == null || userAttribute.isEmpty()) { diff --git a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimMapper.java b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimMapper.java index feb51b67c9..f2b52882fe 100755 --- a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimMapper.java +++ b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractClaimMapper.java @@ -76,7 +76,7 @@ public abstract class AbstractClaimMapper extends AbstractIdentityProviderMapper { // Search the OIDC UserInfo claim set (if any) JsonNode profileJsonNode = (JsonNode) context.getContextData().get(OIDCIdentityProvider.USER_INFO); - String value = AbstractJsonUserAttributeMapper.getJsonValue(profileJsonNode, claim); + Object value = AbstractJsonUserAttributeMapper.getJsonValue(profileJsonNode, claim); if (value != null) return value; } return null; diff --git a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractJsonUserAttributeMapper.java b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractJsonUserAttributeMapper.java index e718a0d7f7..49e145cf0b 100755 --- a/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractJsonUserAttributeMapper.java +++ b/services/src/main/java/org/keycloak/broker/oidc/mappers/AbstractJsonUserAttributeMapper.java @@ -35,7 +35,7 @@ import java.util.List; * Abstract class for Social Provider mappers which allow mapping of JSON user profile field into Keycloak user * attribute. Concrete mapper classes with own ID and provider mapping must be implemented for each social provider who * uses {@link JsonNode} user profile. - * + * * @author Vlastimil Elias (velias at redhat dot com) */ public abstract class AbstractJsonUserAttributeMapper extends AbstractIdentityProviderMapper { @@ -81,8 +81,8 @@ public abstract class AbstractJsonUserAttributeMapper extends AbstractIdentityPr } /** - * Store used profile JsonNode into user context for later use by this mapper. Profile data are dumped into special logger if enabled also to allow investigation of the structure. - * + * Store used profile JsonNode into user context for later use by this mapper. Profile data are dumped into special logger if enabled also to allow investigation of the structure. + * * @param user context to store profile data into * @param profile to store into context * @param provider identification of social provider to be used in log dump @@ -125,9 +125,13 @@ public abstract class AbstractJsonUserAttributeMapper extends AbstractIdentityPr } attribute = attribute.trim(); - String value = getJsonValue(mapperModel, context); + Object value = getJsonValue(mapperModel, context); if (value != null) { - context.setUserAttribute(attribute, value); + if (value instanceof List) { + context.setUserAttribute(attribute, (List) value); + } else { + context.setUserAttribute(attribute, value.toString()); + } } } @@ -136,7 +140,7 @@ public abstract class AbstractJsonUserAttributeMapper extends AbstractIdentityPr // we do not update user profile from social provider } - protected static String getJsonValue(IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { + protected static Object getJsonValue(IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { String jsonField = mapperModel.getConfig().get(CONF_JSON_FIELD); if (jsonField == null || jsonField.trim().isEmpty()) { @@ -152,7 +156,7 @@ public abstract class AbstractJsonUserAttributeMapper extends AbstractIdentityPr JsonNode profileJsonNode = (JsonNode) context.getContextData().get(CONTEXT_JSON_NODE); - String value = getJsonValue(profileJsonNode, jsonField); + Object value = getJsonValue(profileJsonNode, jsonField); if (value == null) { logger.debugf("User profile JSON value '%s' is not available.", jsonField); @@ -161,7 +165,7 @@ public abstract class AbstractJsonUserAttributeMapper extends AbstractIdentityPr return value; } - public static String getJsonValue(JsonNode baseNode, String fieldPath) { + public static Object getJsonValue(JsonNode baseNode, String fieldPath) { logger.debug("Going to process JsonNode path " + fieldPath + " on data " + baseNode); if (baseNode != null) { @@ -206,6 +210,20 @@ public abstract class AbstractJsonUserAttributeMapper extends AbstractIdentityPr } if (idx < 0) { + if (currentNode.isArray()) { + List values = new ArrayList<>(); + for (JsonNode childNode : currentNode) { + if (childNode.isTextual()) { + values.add(childNode.textValue()); + } else { + logger.warn("JsonNode in array is not text value " + childNode); + } + } + if (values.isEmpty()) { + return null; + } + return arrayIndex == idx? values : null; + } if (!currentNode.isValueNode()) { logger.debug("JsonNode is not value node for name " + currentFieldName); return null; diff --git a/services/src/main/java/org/keycloak/broker/oidc/mappers/UserAttributeMapper.java b/services/src/main/java/org/keycloak/broker/oidc/mappers/UserAttributeMapper.java index 52ad5de33f..2e2abcae29 100755 --- a/services/src/main/java/org/keycloak/broker/oidc/mappers/UserAttributeMapper.java +++ b/services/src/main/java/org/keycloak/broker/oidc/mappers/UserAttributeMapper.java @@ -20,14 +20,20 @@ package org.keycloak.broker.oidc.mappers; import org.keycloak.broker.oidc.KeycloakOIDCIdentityProviderFactory; import org.keycloak.broker.oidc.OIDCIdentityProviderFactory; import org.keycloak.broker.provider.BrokeredIdentityContext; +import org.keycloak.common.util.CollectionUtil; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.saml.common.util.StringUtil; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Objects; +import java.util.function.Consumer; +import java.util.stream.Collectors; /** * @author Bill Burke @@ -37,9 +43,12 @@ public class UserAttributeMapper extends AbstractClaimMapper { public static final String[] COMPATIBLE_PROVIDERS = {KeycloakOIDCIdentityProviderFactory.PROVIDER_ID, OIDCIdentityProviderFactory.PROVIDER_ID}; - private static final List configProperties = new ArrayList(); + private static final List configProperties = new ArrayList<>(); public static final String USER_ATTRIBUTE = "user.attribute"; + public static final String EMAIL = "email"; + public static final String FIRST_NAME = "firstName"; + public static final String LAST_NAME = "lastName"; static { ProviderConfigProperty property; @@ -88,37 +97,63 @@ public class UserAttributeMapper extends AbstractClaimMapper { @Override public void preprocessFederatedIdentity(KeycloakSession session, RealmModel realm, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { String attribute = mapperModel.getConfig().get(USER_ATTRIBUTE); - Object value = getClaimValue(mapperModel, context); - if (value != null) { - if (attribute.equalsIgnoreCase("email")) { - context.setEmail(value.toString()); - } else if (attribute.equalsIgnoreCase("firstName")) { - context.setFirstName(value.toString()); - } else if (attribute.equalsIgnoreCase("lastName")) { - context.setLastName(value.toString()); - } else { - context.setUserAttribute(attribute, value.toString()); - } + if(StringUtil.isNullOrEmpty(attribute)){ + return; } + Object value = getClaimValue(mapperModel, context); + List values = toList(value); + + if (EMAIL.equalsIgnoreCase(attribute)) { + setIfNotEmpty(context::setEmail, values); + } else if (FIRST_NAME.equalsIgnoreCase(attribute)) { + setIfNotEmpty(context::setFirstName, values); + } else if (LAST_NAME.equalsIgnoreCase(attribute)) { + setIfNotEmpty(context::setLastName, values); + } else { + List valuesToString = values.stream() + .filter(Objects::nonNull) + .map(Object::toString) + .collect(Collectors.toList()); + + context.setUserAttribute(attribute, valuesToString); + } + } + + private void setIfNotEmpty(Consumer consumer, List values) { + if (values != null && !values.isEmpty()) { + consumer.accept(values.get(0)); + } + } + + private List toList(Object value) { + List values = (value instanceof List) + ? (List) value + : Collections.singletonList(value); + return values.stream() + .filter(Objects::nonNull) + .map(Object::toString) + .collect(Collectors.toList()); } @Override public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { String attribute = mapperModel.getConfig().get(USER_ATTRIBUTE); + if(StringUtil.isNullOrEmpty(attribute)){ + return; + } Object value = getClaimValue(mapperModel, context); - String stringValue = null; - if (value != null) stringValue = value.toString(); - if (attribute.equalsIgnoreCase("email")) { - user.setEmail(stringValue); - } else if (attribute.equalsIgnoreCase("firstName")) { - user.setFirstName(stringValue); - } else if (attribute.equalsIgnoreCase("lastName")) { - user.setLastName(stringValue); + List values = toList(value); + if (EMAIL.equalsIgnoreCase(attribute)) { + setIfNotEmpty(user::setEmail, values); + } else if (FIRST_NAME.equalsIgnoreCase(attribute)) { + setIfNotEmpty(user::setFirstName, values); + } else if (LAST_NAME.equalsIgnoreCase(attribute)) { + setIfNotEmpty(user::setLastName, values); } else { - String current = user.getFirstAttribute(attribute); - if (stringValue != null && !stringValue.equals(current)) { - user.setSingleAttribute(attribute, stringValue); - } else if (value == null) { + List current = user.getAttribute(attribute); + if (!CollectionUtil.collectionEquals(values, current)) { + user.setAttribute(attribute, values); + } else if (values.isEmpty()) { user.removeAttribute(attribute); } } diff --git a/services/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java b/services/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java index 70cae078e5..8a71493fe5 100755 --- a/services/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java +++ b/services/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java @@ -21,17 +21,18 @@ import org.keycloak.broker.provider.AbstractIdentityProviderMapper; import org.keycloak.broker.provider.BrokeredIdentityContext; import org.keycloak.broker.saml.SAMLEndpoint; import org.keycloak.broker.saml.SAMLIdentityProviderFactory; +import org.keycloak.common.util.CollectionUtil; import org.keycloak.dom.saml.v2.assertion.AssertionType; import org.keycloak.dom.saml.v2.assertion.AttributeStatementType; import org.keycloak.dom.saml.v2.assertion.AttributeType; -import org.keycloak.models.IdentityProviderMapperModel; -import org.keycloak.models.KeycloakSession; -import org.keycloak.models.RealmModel; -import org.keycloak.models.UserModel; +import org.keycloak.models.*; import org.keycloak.provider.ProviderConfigProperty; +import org.keycloak.saml.common.util.StringUtil; -import java.util.ArrayList; -import java.util.List; +import java.util.*; +import java.util.function.Consumer; +import java.util.function.Predicate; +import java.util.stream.Collectors; /** * @author Bill Burke @@ -41,11 +42,14 @@ public class UserAttributeMapper extends AbstractIdentityProviderMapper { public static final String[] COMPATIBLE_PROVIDERS = {SAMLIdentityProviderFactory.PROVIDER_ID}; - private static final List configProperties = new ArrayList(); + private static final List configProperties = new ArrayList<>(); public static final String ATTRIBUTE_NAME = "attribute.name"; public static final String ATTRIBUTE_FRIENDLY_NAME = "attribute.friendly.name"; public static final String USER_ATTRIBUTE = "user.attribute"; + private static final String EMAIL = "email"; + private static final String FIRST_NAME = "firstName"; + private static final String LAST_NAME = "lastName"; static { ProviderConfigProperty property; @@ -99,61 +103,84 @@ public class UserAttributeMapper extends AbstractIdentityProviderMapper { @Override public void preprocessFederatedIdentity(KeycloakSession session, RealmModel realm, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { String attribute = mapperModel.getConfig().get(USER_ATTRIBUTE); - String value = getAttribute(mapperModel, context); - if (value != null) { - if (attribute.equalsIgnoreCase("email")) { - context.setEmail(value); - } else if (attribute.equalsIgnoreCase("firstName")) { - context.setFirstName(value); - } else if (attribute.equalsIgnoreCase("lastName")) { - context.setLastName(value); + if (StringUtil.isNullOrEmpty(attribute)) { + return; + } + String attributeName = getAttributeNameFromMapperModel(mapperModel); + + List attributeValuesInContext = findAttributeValuesInContext(attributeName, context); + if (!attributeValuesInContext.isEmpty()) { + if (attribute.equalsIgnoreCase(EMAIL)) { + setIfNotEmpty(context::setEmail, attributeValuesInContext); + } else if (attribute.equalsIgnoreCase(FIRST_NAME)) { + setIfNotEmpty(context::setFirstName, attributeValuesInContext); + } else if (attribute.equalsIgnoreCase(LAST_NAME)) { + setIfNotEmpty(context::setLastName, attributeValuesInContext); } else { - context.setUserAttribute(attribute, value); + context.setUserAttribute(attribute, attributeValuesInContext); } } } - protected String getAttribute(IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { - String name = mapperModel.getConfig().get(ATTRIBUTE_NAME); - if (name != null && name.trim().equals("")) name = null; - String friendly = mapperModel.getConfig().get(ATTRIBUTE_FRIENDLY_NAME); - if (friendly != null && friendly.trim().equals("")) friendly = null; - AssertionType assertion = (AssertionType)context.getContextData().get(SAMLEndpoint.SAML_ASSERTION); - for (AttributeStatementType statement : assertion.getAttributeStatements()) { - for (AttributeStatementType.ASTChoiceType choice : statement.getAttributes()) { - AttributeType attr = choice.getAttribute(); - if (name != null && !name.equals(attr.getName())) continue; - if (friendly != null && !friendly.equals(attr.getFriendlyName())) continue; - - List attributeValue = attr.getAttributeValue(); - if (attributeValue == null || attributeValue.isEmpty()) return null; - return attributeValue.get(0).toString(); - } + private String getAttributeNameFromMapperModel(IdentityProviderMapperModel mapperModel) { + String attributeName = mapperModel.getConfig().get(ATTRIBUTE_NAME); + if (attributeName == null) { + attributeName = mapperModel.getConfig().get(ATTRIBUTE_FRIENDLY_NAME); } - return null; + return attributeName; + } + + private void setIfNotEmpty(Consumer consumer, List values) { + if (values != null && !values.isEmpty()) { + consumer.accept(values.get(0)); + } + } + + private Predicate elementWith(String attributeName) { + return attributeType -> { + AttributeType attribute = attributeType.getAttribute(); + return Objects.equals(attribute.getName(), attributeName) + || Objects.equals(attribute.getFriendlyName(), attributeName); + }; + } + + + private List findAttributeValuesInContext(String attributeName, BrokeredIdentityContext context) { + AssertionType assertion = (AssertionType) context.getContextData().get(SAMLEndpoint.SAML_ASSERTION); + + return assertion.getAttributeStatements().stream() + .flatMap(statement -> statement.getAttributes().stream()) + .filter(elementWith(attributeName)) + .flatMap(attributeType -> attributeType.getAttribute().getAttributeValue().stream()) + .filter(Objects::nonNull) + .map(Object::toString) + .collect(Collectors.toList()); } @Override public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { String attribute = mapperModel.getConfig().get(USER_ATTRIBUTE); - String value = getAttribute(mapperModel, context); - if (attribute.equalsIgnoreCase("email")) { - user.setEmail(value); - } else if (attribute.equalsIgnoreCase("firstName")) { - user.setFirstName(value); - } else if (attribute.equalsIgnoreCase("lastName")) { - user.setLastName(value); + if (StringUtil.isNullOrEmpty(attribute)) { + return; + } + String attributeName = getAttributeNameFromMapperModel(mapperModel); + List attributeValuesInContext = findAttributeValuesInContext(attributeName, context); + if (attribute.equalsIgnoreCase(EMAIL)) { + setIfNotEmpty(user::setEmail, attributeValuesInContext); + } else if (attribute.equalsIgnoreCase(FIRST_NAME)) { + setIfNotEmpty(user::setFirstName, attributeValuesInContext); + } else if (attribute.equalsIgnoreCase(LAST_NAME)) { + setIfNotEmpty(user::setLastName, attributeValuesInContext); } else { - String current = user.getFirstAttribute(attribute); - if (value != null && !value.equals(current)) { - user.setSingleAttribute(attribute, value.toString()); - } else if (value == null) { + List currentAttributeValues = user.getAttributes().get(attribute); + if (attributeValuesInContext != null + && currentAttributeValues != null + && !CollectionUtil.collectionEquals(attributeValuesInContext, currentAttributeValues)) { + user.setAttribute(attribute, attributeValuesInContext); + } else if (attributeValuesInContext == null) { user.removeAttribute(attribute); } } - - - } @Override diff --git a/services/src/main/java/org/keycloak/protocol/oidc/mappers/OIDCAttributeMapperHelper.java b/services/src/main/java/org/keycloak/protocol/oidc/mappers/OIDCAttributeMapperHelper.java index 8b64aef891..7b8ba36716 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/mappers/OIDCAttributeMapperHelper.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/mappers/OIDCAttributeMapperHelper.java @@ -25,10 +25,9 @@ import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.representations.IDToken; import org.keycloak.services.ServicesLogger; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; +import java.util.function.Function; +import java.util.stream.Collectors; /** * @author Bill Burke @@ -70,29 +69,78 @@ public class OIDCAttributeMapperHelper { ServicesLogger.LOGGER.multipleValuesForMapper(attributeValue.toString(), mappingModel.getName()); } - attributeValue = valueAsList.get(0); + attributeValue = valueAsList; } } String type = mappingModel.getConfig().get(JSON_TYPE); + Object converted = convertToType(type, attributeValue); + return converted != null ? converted : attributeValue; + } + + private static List transform(List attributeValue, Function mapper) { + return attributeValue.stream() + .filter(Objects::nonNull) + .map(mapper) + .collect(Collectors.toList()); + } + + private static Object convertToType(String type, Object attributeValue) { if (type == null) return attributeValue; - if (type.equals("boolean")) { - if (attributeValue instanceof Boolean) return attributeValue; - if (attributeValue instanceof String) return Boolean.valueOf((String)attributeValue); - throw new RuntimeException("cannot map type for token claim"); - } else if (type.equals("String")) { - if (attributeValue instanceof String) return attributeValue; - return attributeValue.toString(); - } else if (type.equals("long")) { - if (attributeValue instanceof Long) return attributeValue; - if (attributeValue instanceof String) return Long.valueOf((String)attributeValue); - throw new RuntimeException("cannot map type for token claim"); - } else if (type.equals("int")) { - if (attributeValue instanceof Integer) return attributeValue; - if (attributeValue instanceof String) return Integer.valueOf((String)attributeValue); - throw new RuntimeException("cannot map type for token claim"); + switch (type) { + case "boolean": + Boolean booleanObject = getBoolean(attributeValue); + if (booleanObject != null) return booleanObject; + if (attributeValue instanceof List) { + return transform((List) attributeValue, OIDCAttributeMapperHelper::getBoolean); + } + throw new RuntimeException("cannot map type for token claim"); + case "String": + if (attributeValue instanceof String) return attributeValue; + if (attributeValue instanceof List) { + return transform((List) attributeValue, OIDCAttributeMapperHelper::getString); + } + return attributeValue.toString(); + case "long": + Long longObject = getLong(attributeValue); + if (longObject != null) return longObject; + if (attributeValue instanceof List) { + return transform((List) attributeValue, OIDCAttributeMapperHelper::getLong); + } + throw new RuntimeException("cannot map type for token claim"); + case "int": + Integer intObject = getInteger(attributeValue); + if (intObject != null) return intObject; + if (attributeValue instanceof List) { + return transform((List) attributeValue, OIDCAttributeMapperHelper::getInteger); + } + throw new RuntimeException("cannot map type for token claim"); + default: + return null; } - return attributeValue; + } + + private static String getString(Object attributeValue) { + return attributeValue.toString(); + } + + + private static Long getLong(Object attributeValue) { + if (attributeValue instanceof Long) return (Long) attributeValue; + if (attributeValue instanceof String) return Long.valueOf((String) attributeValue); + return null; + } + + private static Integer getInteger(Object attributeValue) { + if (attributeValue instanceof Integer) return (Integer) attributeValue; + if (attributeValue instanceof String) return Integer.valueOf((String) attributeValue); + return null; + } + + private static Boolean getBoolean(Object attributeValue) { + if (attributeValue instanceof Boolean) return (Boolean) attributeValue; + if (attributeValue instanceof String) return Boolean.valueOf((String) attributeValue); + return null; } public static void mapClaim(IDToken token, ProtocolMapperModel mappingModel, Object attributeValue) { diff --git a/services/src/main/java/org/keycloak/protocol/saml/mappers/AttributeStatementHelper.java b/services/src/main/java/org/keycloak/protocol/saml/mappers/AttributeStatementHelper.java index c1cf9c4b73..423cab9b94 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/mappers/AttributeStatementHelper.java +++ b/services/src/main/java/org/keycloak/protocol/saml/mappers/AttributeStatementHelper.java @@ -52,6 +52,15 @@ public class AttributeStatementHelper { attributeStatement.addAttribute(new AttributeStatementType.ASTChoiceType(attribute)); } + public static void addAttributes(AttributeStatementType attributeStatement, ProtocolMapperModel mappingModel, + List attributeValues) { + + AttributeType attribute = createAttributeType(mappingModel); + attributeValues.forEach(attribute::addAttributeValue); + + attributeStatement.addAttribute(new AttributeStatementType.ASTChoiceType(attribute)); + } + public static AttributeType createAttributeType(ProtocolMapperModel mappingModel) { String attributeName = mappingModel.getConfig().get(SAML_ATTRIBUTE_NAME); AttributeType attribute = new AttributeType(attributeName); diff --git a/services/src/main/java/org/keycloak/protocol/saml/mappers/UserAttributeStatementMapper.java b/services/src/main/java/org/keycloak/protocol/saml/mappers/UserAttributeStatementMapper.java index 2340191c51..f29d972234 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/mappers/UserAttributeStatementMapper.java +++ b/services/src/main/java/org/keycloak/protocol/saml/mappers/UserAttributeStatementMapper.java @@ -80,10 +80,9 @@ public class UserAttributeStatementMapper extends AbstractSAMLProtocolMapper imp public void transformAttributeStatement(AttributeStatementType attributeStatement, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, ClientSessionModel clientSession) { UserModel user = userSession.getUser(); String attributeName = mappingModel.getConfig().get(ProtocolMapperUtils.USER_ATTRIBUTE); - String attributeValue = KeycloakModelUtils.resolveFirstAttribute(user, attributeName); - if (attributeValue == null) return; - AttributeStatementHelper.addAttribute(attributeStatement, mappingModel, attributeValue); - + List attributeValues = KeycloakModelUtils.resolveAttribute(user, attributeName); + if (attributeValues.isEmpty()) return; + AttributeStatementHelper.addAttributes(attributeStatement, mappingModel, attributeValues); } public static ProtocolMapperModel createAttributeMapper(String name, String userAttribute, diff --git a/services/src/test/java/org/keycloak/test/broker/oidc/mappers/AbstractJsonUserAttributeMapperTest.java b/services/src/test/java/org/keycloak/test/broker/oidc/mappers/AbstractJsonUserAttributeMapperTest.java index 24c4cebcd6..786a528c43 100755 --- a/services/src/test/java/org/keycloak/test/broker/oidc/mappers/AbstractJsonUserAttributeMapperTest.java +++ b/services/src/test/java/org/keycloak/test/broker/oidc/mappers/AbstractJsonUserAttributeMapperTest.java @@ -24,10 +24,11 @@ import org.junit.Test; import org.keycloak.broker.oidc.mappers.AbstractJsonUserAttributeMapper; import java.io.IOException; +import java.util.Arrays; /** * Unit test for {@link org.keycloak.broker.oidc.mappers.AbstractJsonUserAttributeMapper} - * + * * @author Vlastimil Elias (velias at redhat dot com) */ public class AbstractJsonUserAttributeMapperTest { @@ -58,7 +59,7 @@ public class AbstractJsonUserAttributeMapperTest { //unknown field returns null Assert.assertEquals(null, AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_unknown")); - + // we check value is trimmed also! Assert.assertEquals("v1", AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value1")); Assert.assertEquals(null, AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_empty")); @@ -95,14 +96,14 @@ public class AbstractJsonUserAttributeMapperTest { public void getJsonValue_simpleArray() throws JsonProcessingException, IOException { // array field itself returns null if no index is provided - Assert.assertEquals(null, AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_array")); + Assert.assertEquals(Arrays.asList("a1", "a2"), AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_array")); // outside index returns null Assert.assertEquals(null, AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_array[2]")); //corect index Assert.assertEquals("a1", AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_array[0]")); Assert.assertEquals("a2", AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_array[1]")); - + //incorrect array constructs Assert.assertNull(AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_array[]")); Assert.assertNull(AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "value_array]")); @@ -125,7 +126,7 @@ public class AbstractJsonUserAttributeMapperTest { Assert.assertEquals(null, AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "nesta.a.av1")); Assert.assertEquals(null, AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "nesta.a].av1")); Assert.assertEquals(null, AbstractJsonUserAttributeMapper.getJsonValue(getJsonNode(), "nesta.a[.av1")); - + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupMappersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupMappersTest.java index d61a517ce6..a1e6b0ad36 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupMappersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupMappersTest.java @@ -32,10 +32,10 @@ import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; -import java.util.HashMap; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; +import java.util.*; + +import static org.hamcrest.CoreMatchers.hasItems; +import static org.hamcrest.MatcherAssert.assertThat; /** * @author Marko Strukelj @@ -119,7 +119,7 @@ public class GroupMappersTest extends AbstractGroupTest { Assert.assertNotNull(groups); Assert.assertTrue(groups.size() == 1); Assert.assertEquals("topGroup", groups.get(0)); - Assert.assertEquals("true", token.getOtherClaims().get("topAttribute")); + Assert.assertEquals(Collections.singletonList("true"), token.getOtherClaims().get("topAttribute")); } { UserRepresentation user = realm.users().search("level2GroupUser", -1, -1).get(0); @@ -132,8 +132,8 @@ public class GroupMappersTest extends AbstractGroupTest { Assert.assertNotNull(groups); Assert.assertTrue(groups.size() == 1); Assert.assertEquals("level2group", groups.get(0)); - Assert.assertEquals("true", token.getOtherClaims().get("topAttribute")); - Assert.assertEquals("true", token.getOtherClaims().get("level2Attribute")); + Assert.assertEquals(Collections.singletonList("true"), token.getOtherClaims().get("topAttribute")); + Assert.assertEquals(Collections.singletonList("true"), token.getOtherClaims().get("level2Attribute")); } } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractUserAttributeMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractUserAttributeMapperTest.java index 2e5c4c6789..f5e9d2e732 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractUserAttributeMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractUserAttributeMapperTest.java @@ -238,7 +238,6 @@ public abstract class AbstractUserAttributeMapperTest extends AbstractBaseBroker } @Test - @Ignore("Unignore to test KEYCLOAK-3648") public void testBasicMappingMultipleValues() { testValueMapping(ImmutableMap.>builder() .put(ATTRIBUTE_TO_MAP_NAME, ImmutableList.builder().add("value 1").add("value 2").build()) diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java index 7ffcfdcad7..b896588852 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/OIDCProtocolMappersTest.java @@ -141,12 +141,12 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { assertNull(idToken.getAddress().getCountry()); // Null because we changed userAttribute name to "country_some", but user contains "country" assertEquals(idToken.getAddress().getFormattedAddress(), "6 Foo Street"); assertNotNull(idToken.getOtherClaims().get("home_phone")); - assertEquals("617-777-6666", idToken.getOtherClaims().get("home_phone")); + assertThat((List) idToken.getOtherClaims().get("home_phone"), hasItems("617-777-6666")); assertEquals("coded", idToken.getOtherClaims().get("hard")); Map nested = (Map) idToken.getOtherClaims().get("nested"); assertEquals("coded-nested", nested.get("hard")); nested = (Map) idToken.getOtherClaims().get("home"); - assertEquals("617-777-6666", nested.get("phone")); + assertThat((List) nested.get("phone"), hasItems("617-777-6666")); List departments = (List) idToken.getOtherClaims().get("department"); assertEquals(2, departments.size()); assertTrue(departments.contains("finance") && departments.contains("development")); @@ -161,12 +161,12 @@ public class OIDCProtocolMappersTest extends AbstractKeycloakTest { assertNull(idToken.getAddress().getCountry()); // Null because we changed userAttribute name to "country_some", but user contains "country" assertEquals(idToken.getAddress().getFormattedAddress(), "6 Foo Street"); assertNotNull(accessToken.getOtherClaims().get("home_phone")); - assertEquals("617-777-6666", accessToken.getOtherClaims().get("home_phone")); + assertThat((List) accessToken.getOtherClaims().get("home_phone"), hasItems("617-777-6666")); assertEquals("coded", accessToken.getOtherClaims().get("hard")); nested = (Map) accessToken.getOtherClaims().get("nested"); assertEquals("coded-nested", nested.get("hard")); nested = (Map) accessToken.getOtherClaims().get("home"); - assertEquals("617-777-6666", nested.get("phone")); + assertThat((List) nested.get("phone"), hasItems("617-777-6666")); departments = (List) idToken.getOtherClaims().get("department"); assertEquals(2, departments.size()); assertTrue(departments.contains("finance") && departments.contains("development"));