From 09994d1730e3650b89f7a510380d5b8a9b71d961 Mon Sep 17 00:00:00 2001 From: mposolda Date: Sat, 27 Jun 2015 22:49:40 +0200 Subject: [PATCH 1/2] KEYCLOAK-1487 Support for multiple values of one UserModel attribute. LDAP multivalued attribute support --- .../AbstractJsonUserAttributeMapper.java | 2 +- .../oidc/mappers/UserAttributeMapper.java | 10 +- .../saml/mappers/UserAttributeMapper.java | 9 +- .../META-INF/jpa-changelog-1.4.0.xml | 9 ++ .../mongo/api/types/MapperContext.java | 7 +- .../mongo/impl/types/BasicDBListMapper.java | 2 +- .../impl/types/BasicDBListToSetMapper.java | 2 +- .../mongo/impl/types/BasicDBObjectMapper.java | 13 +- .../idm/UserRepresentation.java | 24 ++- .../kerberos/KerberosFederationProvider.java | 6 +- .../ldap/LDAPFederationProvider.java | 26 +-- .../ldap/LDAPFederationProviderFactory.java | 12 +- .../keycloak/federation/ldap/LDAPUtils.java | 10 +- .../federation/ldap/idm/model/LDAPObject.java | 46 +++--- ...{LDAPIdentityQuery.java => LDAPQuery.java} | 26 +-- .../ldap/idm/store/IdentityStore.java | 6 +- .../idm/store/ldap/LDAPIdentityStore.java | 45 +++--- .../idm/store/ldap/LDAPOperationManager.java | 4 +- .../mappers/FullNameLDAPFederationMapper.java | 16 +- .../ldap/mappers/LDAPFederationMapper.java | 4 +- .../mappers/RoleLDAPFederationMapper.java | 61 ++++---- .../UserAttributeLDAPFederationMapper.java | 148 ++++++++++++++---- .../account/freemarker/model/AccountBean.java | 20 ++- .../org/keycloak/freemarker/LocaleHelper.java | 8 +- .../admin/resources/js/controllers/users.js | 27 +++- .../java/org/keycloak/models/UserModel.java | 24 ++- .../keycloak/models/entities/UserEntity.java | 8 +- .../models/utils/ModelToRepresentation.java | 3 +- .../models/utils/RepresentationToModel.java | 14 +- .../models/utils/UserModelDelegate.java | 18 ++- .../models/file/adapter/UserAdapter.java | 35 ++++- .../keycloak/models/cache/UserAdapter.java | 26 ++- .../models/cache/entities/CachedUser.java | 5 +- .../org/keycloak/models/jpa/UserAdapter.java | 58 ++++++- .../jpa/entities/UserAttributeEntity.java | 60 ++----- .../mongo/keycloak/adapters/UserAdapter.java | 37 ++++- .../keycloak/protocol/saml/SamlProtocol.java | 4 +- .../mappers/UserAttributeStatementMapper.java | 4 +- .../protocol/oidc/mappers/AddressMapper.java | 10 +- .../oidc/mappers/UserAttributeMapper.java | 3 +- .../resources/AttributeFormDataProcessor.java | 23 ++- .../resources/admin/UsersResource.java | 5 +- .../testsuite/account/ProfileTest.java | 13 +- .../keycloak/testsuite/admin/UserTest.java | 50 +++--- .../broker/AbstractIdentityProviderTest.java | 4 +- .../FederationProvidersIntegrationTest.java | 16 +- .../federation/FederationTestUtils.java | 16 +- .../LDAPMultipleAttributesTest.java | 112 +++++++++++++ .../federation/LDAPRoleMappingsTest.java | 4 +- .../federation/SyncProvidersTest.java | 4 +- .../keycloak/testsuite/i18n/EmailTest.java | 4 +- .../keycloak/testsuite/model/AdapterTest.java | 3 +- .../keycloak/testsuite/model/ImportTest.java | 16 ++ .../testsuite/model/UserModelTest.java | 56 +++++++ .../testsuite/oauth/AccessTokenTest.java | 12 +- .../src/test/resources/ldap/users.ldif | 1 + .../src/test/resources/model/testrealm.json | 9 ++ 57 files changed, 841 insertions(+), 359 deletions(-) rename federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/{LDAPIdentityQuery.java => LDAPQuery.java} (85%) create mode 100644 testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPMultipleAttributesTest.java diff --git a/broker/oidc/src/main/java/org/keycloak/broker/oidc/mappers/AbstractJsonUserAttributeMapper.java b/broker/oidc/src/main/java/org/keycloak/broker/oidc/mappers/AbstractJsonUserAttributeMapper.java index 9f5085e273..6d3f8db6f2 100755 --- a/broker/oidc/src/main/java/org/keycloak/broker/oidc/mappers/AbstractJsonUserAttributeMapper.java +++ b/broker/oidc/src/main/java/org/keycloak/broker/oidc/mappers/AbstractJsonUserAttributeMapper.java @@ -110,7 +110,7 @@ public abstract class AbstractJsonUserAttributeMapper extends AbstractIdentityPr String value = getJsonValue(mapperModel, context); if (value != null) { - user.setAttribute(attribute, value); + user.setSingleAttribute(attribute, value); } } diff --git a/broker/oidc/src/main/java/org/keycloak/broker/oidc/mappers/UserAttributeMapper.java b/broker/oidc/src/main/java/org/keycloak/broker/oidc/mappers/UserAttributeMapper.java index 61500a731e..d9cb07900f 100755 --- a/broker/oidc/src/main/java/org/keycloak/broker/oidc/mappers/UserAttributeMapper.java +++ b/broker/oidc/src/main/java/org/keycloak/broker/oidc/mappers/UserAttributeMapper.java @@ -3,16 +3,14 @@ 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.broker.provider.IdentityBrokerException; -import org.keycloak.models.ClientModel; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import org.keycloak.models.RoleModel; import org.keycloak.models.UserModel; import org.keycloak.provider.ProviderConfigProperty; import java.util.ArrayList; +import java.util.Collection; import java.util.List; /** @@ -76,7 +74,7 @@ public class UserAttributeMapper extends AbstractClaimMapper { String attribute = mapperModel.getConfig().get(USER_ATTRIBUTE); Object value = getClaimValue(mapperModel, context); if (value != null) { - user.setAttribute(attribute, value.toString()); + user.setSingleAttribute(attribute, value.toString()); } } @@ -84,9 +82,9 @@ public class UserAttributeMapper extends AbstractClaimMapper { public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { String attribute = mapperModel.getConfig().get(USER_ATTRIBUTE); Object value = getClaimValue(mapperModel, context); - String current = user.getAttribute(attribute); + String current = user.getFirstAttribute(attribute); if (value != null && !value.equals(current)) { - user.setAttribute(attribute, value.toString()); + user.setSingleAttribute(attribute, value.toString()); } else if (value == null) { user.removeAttribute(attribute); } diff --git a/broker/saml/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java b/broker/saml/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java index b5f9770431..a1feac8148 100755 --- a/broker/saml/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java +++ b/broker/saml/src/main/java/org/keycloak/broker/saml/mappers/UserAttributeMapper.java @@ -2,17 +2,14 @@ package org.keycloak.broker.saml.mappers; import org.keycloak.broker.provider.AbstractIdentityProviderMapper; import org.keycloak.broker.provider.BrokeredIdentityContext; -import org.keycloak.broker.provider.IdentityBrokerException; import org.keycloak.broker.saml.SAMLEndpoint; import org.keycloak.broker.saml.SAMLIdentityProviderFactory; 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.ClientModel; import org.keycloak.models.IdentityProviderMapperModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; -import org.keycloak.models.RoleModel; import org.keycloak.models.UserModel; import org.keycloak.provider.ProviderConfigProperty; @@ -87,7 +84,7 @@ public class UserAttributeMapper extends AbstractIdentityProviderMapper { String attribute = mapperModel.getConfig().get(USER_ATTRIBUTE); Object value = getAttribute(mapperModel, context); if (value != null) { - user.setAttribute(attribute, value.toString()); + user.setSingleAttribute(attribute, value.toString()); } } @@ -115,9 +112,9 @@ public class UserAttributeMapper extends AbstractIdentityProviderMapper { public void updateBrokeredUser(KeycloakSession session, RealmModel realm, UserModel user, IdentityProviderMapperModel mapperModel, BrokeredIdentityContext context) { String attribute = mapperModel.getConfig().get(USER_ATTRIBUTE); Object value = getAttribute(mapperModel, context); - String current = user.getAttribute(attribute); + String current = user.getFirstAttribute(attribute); if (value != null && !value.equals(current)) { - user.setAttribute(attribute, value.toString()); + user.setSingleAttribute(attribute, value.toString()); } else if (value == null) { user.removeAttribute(attribute); } diff --git a/connections/jpa-liquibase/src/main/resources/META-INF/jpa-changelog-1.4.0.xml b/connections/jpa-liquibase/src/main/resources/META-INF/jpa-changelog-1.4.0.xml index ff0311dfa9..f5f01954d9 100755 --- a/connections/jpa-liquibase/src/main/resources/META-INF/jpa-changelog-1.4.0.xml +++ b/connections/jpa-liquibase/src/main/resources/META-INF/jpa-changelog-1.4.0.xml @@ -2,6 +2,7 @@ + @@ -22,6 +23,12 @@ + + + + + + @@ -110,6 +117,8 @@ + + diff --git a/connections/mongo/src/main/java/org/keycloak/connections/mongo/api/types/MapperContext.java b/connections/mongo/src/main/java/org/keycloak/connections/mongo/api/types/MapperContext.java index 094f506ad7..30bd9f2970 100644 --- a/connections/mongo/src/main/java/org/keycloak/connections/mongo/api/types/MapperContext.java +++ b/connections/mongo/src/main/java/org/keycloak/connections/mongo/api/types/MapperContext.java @@ -1,5 +1,6 @@ package org.keycloak.connections.mongo.api.types; +import java.lang.reflect.Type; import java.util.List; /** @@ -14,9 +15,9 @@ public class MapperContext { private final Class expectedReturnType; // in case that expected return type is generic type (like "List"), then genericTypes could contain list of expected generic arguments - private final List> genericTypes; + private final List genericTypes; - public MapperContext(T objectToConvert, Class expectedReturnType, List> genericTypes) { + public MapperContext(T objectToConvert, Class expectedReturnType, List genericTypes) { this.objectToConvert = objectToConvert; this.expectedReturnType = expectedReturnType; this.genericTypes = genericTypes; @@ -30,7 +31,7 @@ public class MapperContext { return expectedReturnType; } - public List> getGenericTypes() { + public List getGenericTypes() { return genericTypes; } } diff --git a/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBListMapper.java b/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBListMapper.java index cc229c6969..d435e1eef0 100755 --- a/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBListMapper.java +++ b/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBListMapper.java @@ -24,7 +24,7 @@ public class BasicDBListMapper implements Mapper { public List convertObject(MapperContext context) { BasicDBList dbList = context.getObjectToConvert(); ArrayList appObjects = new ArrayList(); - Class expectedListElementType = context.getGenericTypes().get(0); + Class expectedListElementType = (Class) context.getGenericTypes().get(0); for (Object dbObject : dbList) { MapperContext newContext = new MapperContext(dbObject, expectedListElementType, null); diff --git a/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBListToSetMapper.java b/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBListToSetMapper.java index d43781a225..eea2ee9278 100644 --- a/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBListToSetMapper.java +++ b/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBListToSetMapper.java @@ -23,7 +23,7 @@ public class BasicDBListToSetMapper implements Mapper { public Set convertObject(MapperContext context) { BasicDBList dbList = context.getObjectToConvert(); Set appObjects = new HashSet(); - Class expectedListElementType = context.getGenericTypes().get(0); + Class expectedListElementType = (Class) context.getGenericTypes().get(0); for (Object dbObject : dbList) { MapperContext newContext = new MapperContext(dbObject, expectedListElementType, null); diff --git a/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBObjectMapper.java b/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBObjectMapper.java index 102cf21d45..e592df9573 100644 --- a/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBObjectMapper.java +++ b/connections/mongo/src/main/java/org/keycloak/connections/mongo/impl/types/BasicDBObjectMapper.java @@ -14,6 +14,7 @@ import org.keycloak.util.reflections.Types; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; /** @@ -87,10 +88,14 @@ public class BasicDBObjectMapper implements Mapper { ParameterizedType parameterized = (ParameterizedType) type; Type[] genericTypeArguments = parameterized.getActualTypeArguments(); - List> genericTypes = new ArrayList>(); - for (Type genericType : genericTypeArguments) { - genericTypes.add((Class)genericType); - } + List genericTypes = Arrays.asList(genericTypeArguments); + /*for (Type genericType : genericTypeArguments) { + if (genericType instanceof Class) { + genericTypes.add((Class) genericType); + } else { + System.out.println("foo"); + } + }*/ Class expectedReturnType = (Class)parameterized.getRawType(); context = new MapperContext(valueFromDB, expectedReturnType, genericTypes); diff --git a/core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java index 747b64cc0b..8607264d04 100755 --- a/core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java +++ b/core/src/main/java/org/keycloak/representations/idm/UserRepresentation.java @@ -1,10 +1,14 @@ package org.keycloak.representations.idm; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; +import org.codehaus.jackson.annotate.JsonIgnore; +import org.keycloak.util.MultivaluedHashMap; + /** * @author Bill Burke * @version $Revision: 1 $ @@ -21,7 +25,9 @@ public class UserRepresentation { protected String lastName; protected String email; protected String federationLink; - protected Map attributes; + + // Currently there is Map> but for backwards compatibility, we also need to support Map + protected Map attributes; protected List credentials; protected List requiredActions; protected List federatedIdentities; @@ -106,17 +112,23 @@ public class UserRepresentation { this.emailVerified = emailVerified; } - public Map getAttributes() { + public Map getAttributes() { return attributes; } - public void setAttributes(Map attributes) { + // This method can be removed once we can remove backwards compatibility with Keycloak 1.3 (then getAttributes() can be changed to return Map> ) + @JsonIgnore + public Map> getAttributesAsListValues() { + return (Map) attributes; + } + + public void setAttributes(Map attributes) { this.attributes = attributes; } - public UserRepresentation attribute(String name, String value) { - if (this.attributes == null) attributes = new HashMap(); - attributes.put(name, value); + public UserRepresentation singleAttribute(String name, String value) { + if (this.attributes == null) attributes = new HashMap<>(); + attributes.put(name, Arrays.asList(value)); return this; } diff --git a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java index c43b58bc9b..6c39849fe9 100644 --- a/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java +++ b/federation/kerberos/src/main/java/org/keycloak/federation/kerberos/KerberosFederationProvider.java @@ -110,7 +110,7 @@ public class KerberosFederationProvider implements UserFederationProvider { // KerberosUsernamePasswordAuthenticator.isUserAvailable is an overhead, so avoid it for now String kerberosPrincipal = local.getUsername() + "@" + kerberosConfig.getKerberosRealm(); - return kerberosPrincipal.equals(local.getAttribute(KERBEROS_PRINCIPAL)); + return kerberosPrincipal.equals(local.getFirstAttribute(KERBEROS_PRINCIPAL)); } @Override @@ -229,7 +229,7 @@ public class KerberosFederationProvider implements UserFederationProvider { return proxied; } else { logger.warn("User with username " + username + " already exists and is linked to provider [" + model.getDisplayName() + - "] but kerberos principal is not correct. Kerberos principal on user is: " + user.getAttribute(KERBEROS_PRINCIPAL)); + "] but kerberos principal is not correct. Kerberos principal on user is: " + user.getFirstAttribute(KERBEROS_PRINCIPAL)); logger.warn("Will re-create user"); session.userStorage().removeUser(realm, user); } @@ -249,7 +249,7 @@ public class KerberosFederationProvider implements UserFederationProvider { user.setEnabled(true); user.setEmail(email); user.setFederationLink(model.getId()); - user.setAttribute(KERBEROS_PRINCIPAL, username + "@" + kerberosConfig.getKerberosRealm()); + user.setSingleAttribute(KERBEROS_PRINCIPAL, username + "@" + kerberosConfig.getKerberosRealm()); if (kerberosConfig.isUpdateProfileFirstLogin()) { user.addRequiredAction(UserModel.RequiredAction.UPDATE_PROFILE); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java index cd857ff63d..66665bf8f7 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java @@ -6,7 +6,7 @@ import org.keycloak.federation.kerberos.impl.SPNEGOAuthenticator; import org.keycloak.federation.ldap.idm.model.LDAPObject; import org.keycloak.federation.ldap.idm.query.Condition; import org.keycloak.federation.ldap.idm.query.QueryParameter; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.federation.ldap.idm.query.internal.LDAPQueryConditionsBuilder; import org.keycloak.federation.ldap.idm.store.ldap.LDAPIdentityStore; import org.keycloak.federation.ldap.kerberos.LDAPProviderKerberosConfig; @@ -51,7 +51,7 @@ public class LDAPFederationProvider implements UserFederationProvider { protected EditMode editMode; protected LDAPProviderKerberosConfig kerberosConfig; - protected final Set supportedCredentialTypes = new HashSet(); + protected final Set supportedCredentialTypes = new HashSet<>(); public LDAPFederationProvider(LDAPFederationProviderFactory factory, KeycloakSession session, UserFederationProviderModel model, LDAPIdentityStore ldapIdentityStore) { this.factory = factory; @@ -145,8 +145,8 @@ public class LDAPFederationProvider implements UserFederationProvider { if (!synchronizeRegistrations()) throw new IllegalStateException("Registration is not supported by this ldap server"); LDAPObject ldapObject = LDAPUtils.addUserToLDAP(this, realm, user); - user.setAttribute(LDAPConstants.LDAP_ID, ldapObject.getUuid()); - user.setAttribute(LDAPConstants.LDAP_ENTRY_DN, ldapObject.getDn().toString()); + user.setSingleAttribute(LDAPConstants.LDAP_ID, ldapObject.getUuid()); + user.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, ldapObject.getDn().toString()); return proxy(realm, user, ldapObject); } @@ -202,7 +202,7 @@ public class LDAPFederationProvider implements UserFederationProvider { } if (attributes.containsKey(FIRST_NAME) || attributes.containsKey(LAST_NAME)) { - LDAPIdentityQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); + LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); // Mapper should replace parameter with correct LDAP mapped attributes @@ -229,10 +229,10 @@ public class LDAPFederationProvider implements UserFederationProvider { if (ldapUser == null) { return null; } - if (ldapUser.getUuid().equals(local.getAttribute(LDAPConstants.LDAP_ID))) { + if (ldapUser.getUuid().equals(local.getFirstAttribute(LDAPConstants.LDAP_ID))) { return ldapUser; } else { - logger.warnf("LDAP User invalid. ID doesn't match. ID from LDAP [%s], LDAP ID from local DB: [%s]", ldapUser.getUuid(), local.getAttribute(LDAPConstants.LDAP_ID)); + logger.warnf("LDAP User invalid. ID doesn't match. ID from LDAP [%s], LDAP ID from local DB: [%s]", ldapUser.getUuid(), local.getFirstAttribute(LDAPConstants.LDAP_ID)); return null; } } @@ -271,8 +271,8 @@ public class LDAPFederationProvider implements UserFederationProvider { String userDN = ldapUser.getDn().toString(); imported.setFederationLink(model.getId()); - imported.setAttribute(LDAPConstants.LDAP_ID, ldapUser.getUuid()); - imported.setAttribute(LDAPConstants.LDAP_ENTRY_DN, userDN); + imported.setSingleAttribute(LDAPConstants.LDAP_ID, ldapUser.getUuid()); + imported.setSingleAttribute(LDAPConstants.LDAP_ENTRY_DN, userDN); logger.debugf("Imported new user from LDAP to Keycloak DB. Username: [%s], Email: [%s], LDAP_ID: [%s], LDAP Entry DN: [%s]", imported.getUsername(), imported.getEmail(), ldapUser.getUuid(), userDN); @@ -280,7 +280,7 @@ public class LDAPFederationProvider implements UserFederationProvider { } protected LDAPObject queryByEmail(RealmModel realm, String email) { - LDAPIdentityQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); + LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); // Mapper should replace "email" in parameter name with correct LDAP mapped attribute @@ -395,7 +395,7 @@ public class LDAPFederationProvider implements UserFederationProvider { importUserFromLDAP(realm, ldapUser); syncResult.increaseAdded(); } else { - if ((fedModel.getId().equals(currentUser.getFederationLink())) && (ldapUser.getUuid().equals(currentUser.getAttribute(LDAPConstants.LDAP_ID)))) { + if ((fedModel.getId().equals(currentUser.getFederationLink())) && (ldapUser.getUuid().equals(currentUser.getFirstAttribute(LDAPConstants.LDAP_ID)))) { // Update keycloak user Set federationMappers = realm.getUserFederationMappersByFederationProvider(model.getId()); @@ -435,7 +435,7 @@ public class LDAPFederationProvider implements UserFederationProvider { return proxy(realm, user, ldapObject); } else { logger.warnf("User with username [%s] aready exists and is linked to provider [%s] but is not valid. Stale LDAP_ID on local user is: %s", - username, model.getDisplayName(), user.getAttribute(LDAPConstants.LDAP_ID)); + username, model.getDisplayName(), user.getFirstAttribute(LDAPConstants.LDAP_ID)); logger.warn("Will re-create user"); session.userStorage().removeUser(realm, user); } @@ -448,7 +448,7 @@ public class LDAPFederationProvider implements UserFederationProvider { } public LDAPObject loadLDAPUserByUsername(RealmModel realm, String username) { - LDAPIdentityQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); + LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm); LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); String usernameMappedAttribute = this.ldapIdentityStore.getConfig().getUsernameLdapAttribute(); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java index 98ee688663..2b60ff83b9 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProviderFactory.java @@ -9,7 +9,7 @@ import org.keycloak.federation.kerberos.impl.SPNEGOAuthenticator; import org.keycloak.federation.ldap.idm.model.LDAPObject; import org.keycloak.federation.ldap.idm.query.Condition; import org.keycloak.federation.ldap.idm.query.QueryParameter; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.federation.ldap.idm.query.internal.LDAPQueryConditionsBuilder; import org.keycloak.federation.ldap.idm.store.ldap.LDAPIdentityStore; import org.keycloak.federation.ldap.mappers.FullNameLDAPFederationMapper; @@ -184,7 +184,7 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi public UserFederationSyncResult syncAllUsers(KeycloakSessionFactory sessionFactory, final String realmId, final UserFederationProviderModel model) { logger.infof("Sync all users from LDAP to local store: realm: %s, federation provider: %s", realmId, model.getDisplayName()); - LDAPIdentityQuery userQuery = createQuery(sessionFactory, realmId, model); + LDAPQuery userQuery = createQuery(sessionFactory, realmId, model); UserFederationSyncResult syncResult = syncImpl(sessionFactory, userQuery, realmId, model); // TODO: Remove all existing keycloak users, which have federation links, but are not in LDAP. Perhaps don't check users, which were just added or updated during this sync? @@ -203,7 +203,7 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi Condition modifyCondition = conditionsBuilder.greaterThanOrEqualTo(new QueryParameter(LDAPConstants.MODIFY_TIMESTAMP), lastSync); Condition orCondition = conditionsBuilder.orCondition(createCondition, modifyCondition); - LDAPIdentityQuery userQuery = createQuery(sessionFactory, realmId, model); + LDAPQuery userQuery = createQuery(sessionFactory, realmId, model); userQuery.where(orCondition); UserFederationSyncResult result = syncImpl(sessionFactory, userQuery, realmId, model); @@ -211,7 +211,7 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi return result; } - protected UserFederationSyncResult syncImpl(KeycloakSessionFactory sessionFactory, LDAPIdentityQuery userQuery, final String realmId, final UserFederationProviderModel fedModel) { + protected UserFederationSyncResult syncImpl(KeycloakSessionFactory sessionFactory, LDAPQuery userQuery, final String realmId, final UserFederationProviderModel fedModel) { final UserFederationSyncResult syncResult = new UserFederationSyncResult(); @@ -254,9 +254,9 @@ public class LDAPFederationProviderFactory extends UserFederationEventAwareProvi return syncResult; } - private LDAPIdentityQuery createQuery(KeycloakSessionFactory sessionFactory, final String realmId, final UserFederationProviderModel model) { + private LDAPQuery createQuery(KeycloakSessionFactory sessionFactory, final String realmId, final UserFederationProviderModel model) { class QueryHolder { - LDAPIdentityQuery query; + LDAPQuery query; } final QueryHolder queryHolder = new QueryHolder(); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java index 27e8df3850..c731330576 100755 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPUtils.java @@ -4,7 +4,7 @@ import java.util.Set; import org.keycloak.federation.ldap.idm.model.LDAPDn; import org.keycloak.federation.ldap.idm.model.LDAPObject; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.federation.ldap.idm.store.ldap.LDAPIdentityStore; import org.keycloak.federation.ldap.mappers.LDAPFederationMapper; import org.keycloak.models.ModelException; @@ -44,8 +44,8 @@ public class LDAPUtils { return ldapUser; } - public static LDAPIdentityQuery createQueryForUserSearch(LDAPFederationProvider ldapProvider, RealmModel realm) { - LDAPIdentityQuery ldapQuery = new LDAPIdentityQuery(ldapProvider); + public static LDAPQuery createQueryForUserSearch(LDAPFederationProvider ldapProvider, RealmModel realm) { + LDAPQuery ldapQuery = new LDAPQuery(ldapProvider); LDAPConfig config = ldapProvider.getLdapIdentityStore().getConfig(); ldapQuery.setSearchScope(config.getSearchScope()); ldapQuery.setSearchDn(config.getUsersDn()); @@ -60,7 +60,7 @@ public class LDAPUtils { // ldapUser has filled attributes, but doesn't have filled dn. private static void computeAndSetDn(LDAPConfig config, LDAPObject ldapUser) { String rdnLdapAttrName = config.getRdnLdapAttribute(); - String rdnLdapAttrValue = ldapUser.getAttributeAsStringCaseInsensitive(rdnLdapAttrName); + String rdnLdapAttrValue = ldapUser.getAttributeAsString(rdnLdapAttrName); if (rdnLdapAttrValue == null) { throw new ModelException("RDN Attribute [" + rdnLdapAttrName + "] is not filled. Filled attributes: " + ldapUser.getAttributes()); } @@ -72,6 +72,6 @@ public class LDAPUtils { public static String getUsername(LDAPObject ldapUser, LDAPConfig config) { String usernameAttr = config.getUsernameLdapAttribute(); - return ldapUser.getAttributeAsStringCaseInsensitive(usernameAttr); + return ldapUser.getAttributeAsString(usernameAttr); } } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java index c449484cef..81f058d24c 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPObject.java @@ -2,9 +2,12 @@ package org.keycloak.federation.ldap.idm.model; import java.util.Collection; import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import org.jboss.logging.Logger; @@ -24,10 +27,10 @@ public class LDAPObject { // NOTE: names of read-only attributes are lower-cased to avoid case sensitivity issues private final List readOnlyAttributeNames = new LinkedList<>(); - private final Map attributes = new HashMap<>(); + private final Map> attributes = new HashMap<>(); // Copy of "attributes" containing lower-cased keys - private final Map lowerCasedAttributes = new HashMap<>(); + private final Map> lowerCasedAttributes = new HashMap<>(); public String getUuid() { @@ -71,32 +74,37 @@ public class LDAPObject { this.rdnAttributeName = rdnAttributeName; } - public void setAttribute(String attributeName, Object attributeValue) { + public void setSingleAttribute(String attributeName, String attributeValue) { + Set asSet = new LinkedHashSet<>(); + asSet.add(attributeValue); + setAttribute(attributeName, asSet); + } + + public void setAttribute(String attributeName, Set attributeValue) { attributes.put(attributeName, attributeValue); lowerCasedAttributes.put(attributeName.toLowerCase(), attributeValue); } - public Object getAttributeCaseInsensitive(String name) { - return lowerCasedAttributes.get(name.toLowerCase()); - } - - public String getAttributeAsStringCaseInsensitive(String name) { - Object attrValue = lowerCasedAttributes.get(name.toLowerCase()); - if (attrValue != null && !(attrValue instanceof String)) { - logger.warnf("Expected String but attribute '%s' has value '%s' of type '%s' ", name, attrValue, attrValue.getClass().getName()); - - if (attrValue instanceof Collection) { - Collection attrValues = (Collection) attrValue; - attrValue = attrValues.iterator().next(); - logger.warnf("Returning just first founded value '%s' from the collection", attrValue); - } + // Case-insensitive + public String getAttributeAsString(String name) { + Set attrValue = lowerCasedAttributes.get(name.toLowerCase()); + if (attrValue == null || attrValue.size() == 0) { + return null; + } else if (attrValue.size() > 1) { + logger.warnf("Expected String but attribute '%s' has more values '%s' on object '%s' . Returning just first value", name, attrValue, dn); } - return (String) attrValue; + return attrValue.iterator().next(); + } + + // Case-insensitive. Return null if there is not value of attribute with given name or set with all values otherwise + public Set getAttributeAsSet(String name) { + Set values = lowerCasedAttributes.get(name.toLowerCase()); + return (values == null) ? null : new LinkedHashSet<>(values); } - public Map getAttributes() { + public Map> getAttributes() { return attributes; } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPIdentityQuery.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPQuery.java similarity index 85% rename from federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPIdentityQuery.java rename to federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPQuery.java index ee292b17e7..e9f30f85a3 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPIdentityQuery.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/query/internal/LDAPQuery.java @@ -26,7 +26,7 @@ import static java.util.Collections.unmodifiableSet; * * @author Shane Bryzak */ -public class LDAPIdentityQuery { +public class LDAPQuery { private final LDAPFederationProvider ldapFedProvider; @@ -48,46 +48,46 @@ public class LDAPIdentityQuery { private int searchScope = SearchControls.SUBTREE_SCOPE; - public LDAPIdentityQuery(LDAPFederationProvider ldapProvider) { + public LDAPQuery(LDAPFederationProvider ldapProvider) { this.ldapFedProvider = ldapProvider; } - public LDAPIdentityQuery where(Condition... condition) { + public LDAPQuery where(Condition... condition) { this.conditions.addAll(Arrays.asList(condition)); return this; } - public LDAPIdentityQuery sortBy(Sort... sorts) { + public LDAPQuery sortBy(Sort... sorts) { this.ordering.addAll(Arrays.asList(sorts)); return this; } - public LDAPIdentityQuery setSearchDn(String searchDn) { + public LDAPQuery setSearchDn(String searchDn) { this.searchDn = searchDn; return this; } - public LDAPIdentityQuery addObjectClasses(Collection objectClasses) { + public LDAPQuery addObjectClasses(Collection objectClasses) { this.objectClasses.addAll(objectClasses); return this; } - public LDAPIdentityQuery addReturningLdapAttribute(String ldapAttributeName) { + public LDAPQuery addReturningLdapAttribute(String ldapAttributeName) { this.returningLdapAttributes.add(ldapAttributeName); return this; } - public LDAPIdentityQuery addReturningReadOnlyLdapAttribute(String ldapAttributeName) { + public LDAPQuery addReturningReadOnlyLdapAttribute(String ldapAttributeName) { this.returningReadOnlyLdapAttributes.add(ldapAttributeName.toLowerCase()); return this; } - public LDAPIdentityQuery addMappers(Collection mappers) { + public LDAPQuery addMappers(Collection mappers) { this.mappers.addAll(mappers); return this; } - public LDAPIdentityQuery setSearchScope(int searchScope) { + public LDAPQuery setSearchScope(int searchScope) { this.searchScope = searchScope; return this; } @@ -170,17 +170,17 @@ public class LDAPIdentityQuery { return ldapFedProvider.getLdapIdentityStore().countQueryResults(this); } - public LDAPIdentityQuery setOffset(int offset) { + public LDAPQuery setOffset(int offset) { this.offset = offset; return this; } - public LDAPIdentityQuery setLimit(int limit) { + public LDAPQuery setLimit(int limit) { this.limit = limit; return this; } - public LDAPIdentityQuery setPaginationContext(byte[] paginationContext) { + public LDAPQuery setPaginationContext(byte[] paginationContext) { this.paginationContext = paginationContext; return this; } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/IdentityStore.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/IdentityStore.java index 01d85a62ad..23c6d99eb5 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/IdentityStore.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/IdentityStore.java @@ -4,7 +4,7 @@ import java.util.List; import org.keycloak.federation.ldap.LDAPConfig; import org.keycloak.federation.ldap.idm.model.LDAPObject; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; /** * IdentityStore representation providing minimal SPI @@ -48,9 +48,9 @@ public interface IdentityStore { // Identity query - List fetchQueryResults(LDAPIdentityQuery LDAPIdentityQuery); + List fetchQueryResults(LDAPQuery LDAPQuery); - int countQueryResults(LDAPIdentityQuery LDAPIdentityQuery); + int countQueryResults(LDAPQuery LDAPQuery); // // Relationship query // diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java index 64087ad139..67e92c7e2c 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java @@ -28,7 +28,7 @@ import org.keycloak.federation.ldap.idm.model.LDAPObject; import org.keycloak.federation.ldap.idm.query.Condition; import org.keycloak.federation.ldap.idm.query.QueryParameter; import org.keycloak.federation.ldap.idm.query.internal.BetweenCondition; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.federation.ldap.idm.query.internal.EqualCondition; import org.keycloak.federation.ldap.idm.query.internal.GreaterThanCondition; import org.keycloak.federation.ldap.idm.query.internal.InCondition; @@ -108,7 +108,7 @@ public class LDAPIdentityStore implements IdentityStore { @Override - public List fetchQueryResults(LDAPIdentityQuery identityQuery) { + public List fetchQueryResults(LDAPQuery identityQuery) { if (identityQuery.getSorting() != null && !identityQuery.getSorting().isEmpty()) { throw new ModelException("LDAP Identity Store does not yet support sorted queries."); } @@ -160,7 +160,7 @@ public class LDAPIdentityStore implements IdentityStore { } @Override - public int countQueryResults(LDAPIdentityQuery identityQuery) { + public int countQueryResults(LDAPQuery identityQuery) { int limit = identityQuery.getLimit(); int offset = identityQuery.getOffset(); @@ -247,7 +247,7 @@ public class LDAPIdentityStore implements IdentityStore { // ************ END CREDENTIALS AND USER SPECIFIC STUFF - protected StringBuilder createIdentityTypeSearchFilter(final LDAPIdentityQuery identityQuery) { + protected StringBuilder createIdentityTypeSearchFilter(final LDAPQuery identityQuery) { StringBuilder filter = new StringBuilder(); for (Condition condition : identityQuery.getConditions()) { @@ -400,18 +400,14 @@ public class LDAPIdentityStore implements IdentityStore { Set attrValues = new LinkedHashSet<>(); NamingEnumeration enumm = ldapAttribute.getAll(); while (enumm.hasMoreElements()) { - String attrVal = enumm.next().toString(); + String attrVal = enumm.next().toString().trim(); attrValues.add(attrVal); } if (ldapAttributeName.equalsIgnoreCase(LDAPConstants.OBJECT_CLASS)) { ldapObject.setObjectClasses(attrValues); } else { - if (attrValues.size() == 1) { - ldapObject.setAttribute(ldapAttributeName, attrValues.iterator().next()); - } else { - ldapObject.setAttribute(ldapAttributeName, attrValues); - } + ldapObject.setAttribute(ldapAttributeName, attrValues); // readOnlyAttrNames are lower-cased if (readOnlyAttrNames.contains(ldapAttributeName.toLowerCase())) { @@ -435,30 +431,25 @@ public class LDAPIdentityStore implements IdentityStore { protected BasicAttributes extractAttributes(LDAPObject ldapObject, boolean isCreate) { BasicAttributes entryAttributes = new BasicAttributes(); - for (Map.Entry attrEntry : ldapObject.getAttributes().entrySet()) { + for (Map.Entry> attrEntry : ldapObject.getAttributes().entrySet()) { String attrName = attrEntry.getKey(); - Object attrValue = attrEntry.getValue(); + Set attrValue = attrEntry.getValue(); // ldapObject.getReadOnlyAttributeNames() are lower-cased if (!ldapObject.getReadOnlyAttributeNames().contains(attrName.toLowerCase()) && (isCreate || !ldapObject.getRdnAttributeName().equalsIgnoreCase(attrName))) { - - if (String.class.isInstance(attrValue)) { - if (attrValue.toString().trim().length() == 0) { - attrValue = LDAPConstants.EMPTY_ATTRIBUTE_VALUE; - } - entryAttributes.put(attrName, attrValue); - } else if (Collection.class.isInstance(attrValue)) { - BasicAttribute attr = new BasicAttribute(attrName); - Collection valueCollection = (Collection) attrValue; - for (String val : valueCollection) { + BasicAttribute attr = new BasicAttribute(attrName); + if (attrValue == null) { + // Adding empty value as we don't know if attribute is mandatory in LDAP + attr.add(LDAPConstants.EMPTY_ATTRIBUTE_VALUE); + } else { + for (String val : attrValue) { + if (val == null || val.toString().trim().length() == 0) { + val = LDAPConstants.EMPTY_ATTRIBUTE_VALUE; + } attr.add(val); } - entryAttributes.put(attr); - } else if (attrValue == null || attrValue.toString().trim().length() == 0) { - entryAttributes.put(attrName, LDAPConstants.EMPTY_ATTRIBUTE_VALUE); - } else { - throw new ModelException("Unexpected type of value of argument " + attrName + ". Value is " + attrValue); } + entryAttributes.put(attr); } } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java index 8d934f3a11..18b8a8673a 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java @@ -31,7 +31,7 @@ import javax.naming.ldap.PagedResultsResponseControl; import org.jboss.logging.Logger; import org.keycloak.federation.ldap.LDAPConfig; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.models.LDAPConstants; import org.keycloak.models.ModelException; @@ -165,7 +165,7 @@ public class LDAPOperationManager { } } - public List searchPaginated(final String baseDN, final String filter, final LDAPIdentityQuery identityQuery) throws NamingException { + public List searchPaginated(final String baseDN, final String filter, final LDAPQuery identityQuery) throws NamingException { final List result = new ArrayList(); final SearchControls cons = getSearchControls(identityQuery.getReturningLdapAttributes(), identityQuery.getSearchScope()); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java index 483cc06b68..a29581d68e 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/FullNameLDAPFederationMapper.java @@ -8,7 +8,7 @@ import org.keycloak.federation.ldap.idm.model.LDAPObject; import org.keycloak.federation.ldap.idm.query.Condition; import org.keycloak.federation.ldap.idm.query.QueryParameter; import org.keycloak.federation.ldap.idm.query.internal.EqualCondition; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; import org.keycloak.models.UserFederationMapperModel; @@ -28,9 +28,13 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper { @Override public void onImportUserFromLDAP(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, UserModel user, RealmModel realm, boolean isCreate) { String ldapFullNameAttrName = getLdapFullNameAttrName(mapperModel); - String fullName = ldapUser.getAttributeAsStringCaseInsensitive(ldapFullNameAttrName); + String fullName = ldapUser.getAttributeAsString(ldapFullNameAttrName); + if (fullName == null) { + return; + } + fullName = fullName.trim(); - if (fullName != null && !fullName.trim().isEmpty()) { + if (!fullName.isEmpty()) { int lastSpaceIndex = fullName.lastIndexOf(" "); if (lastSpaceIndex == -1) { user.setLastName(fullName); @@ -45,7 +49,7 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper { public void onRegisterUserToLDAP(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, UserModel localUser, RealmModel realm) { String ldapFullNameAttrName = getLdapFullNameAttrName(mapperModel); String fullName = getFullName(localUser.getFirstName(), localUser.getLastName()); - ldapUser.setAttribute(ldapFullNameAttrName, fullName); + ldapUser.setSingleAttribute(ldapFullNameAttrName, fullName); if (isReadOnly(mapperModel)) { ldapUser.addReadOnlyAttributeName(ldapFullNameAttrName); @@ -80,7 +84,7 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper { ensureTransactionStarted(); String ldapFullNameAttrName = getLdapFullNameAttrName(mapperModel); - ldapUser.setAttribute(ldapFullNameAttrName, fullName); + ldapUser.setSingleAttribute(ldapFullNameAttrName, fullName); } }; @@ -92,7 +96,7 @@ public class FullNameLDAPFederationMapper extends AbstractLDAPFederationMapper { } @Override - public void beforeLDAPQuery(UserFederationMapperModel mapperModel, LDAPIdentityQuery query) { + public void beforeLDAPQuery(UserFederationMapperModel mapperModel, LDAPQuery query) { String ldapFullNameAttrName = getLdapFullNameAttrName(mapperModel); query.addReturningLdapAttribute(ldapFullNameAttrName); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/LDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/LDAPFederationMapper.java index 4676a6b198..8d5ab4ea86 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/LDAPFederationMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/LDAPFederationMapper.java @@ -2,7 +2,7 @@ package org.keycloak.federation.ldap.mappers; import org.keycloak.federation.ldap.LDAPFederationProvider; import org.keycloak.federation.ldap.idm.model.LDAPObject; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.models.RealmModel; import org.keycloak.mappers.UserFederationMapper; import org.keycloak.models.UserFederationMapperModel; @@ -58,5 +58,5 @@ public interface LDAPFederationMapper extends UserFederationMapper { * @param mapperModel * @param query */ - void beforeLDAPQuery(UserFederationMapperModel mapperModel, LDAPIdentityQuery query); + void beforeLDAPQuery(UserFederationMapperModel mapperModel, LDAPQuery query); } diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java index bc9fb05160..4565f885ca 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/RoleLDAPFederationMapper.java @@ -12,7 +12,7 @@ import org.keycloak.federation.ldap.idm.model.LDAPDn; import org.keycloak.federation.ldap.idm.model.LDAPObject; import org.keycloak.federation.ldap.idm.query.Condition; import org.keycloak.federation.ldap.idm.query.QueryParameter; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.federation.ldap.idm.query.internal.LDAPQueryConditionsBuilder; import org.keycloak.models.ClientModel; import org.keycloak.models.LDAPConstants; @@ -58,7 +58,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { // List of IDs of UserFederationMapperModels where syncRolesFromLDAP was already called in this KeycloakSession. This is to improve performance // TODO: Rather address this with caching at LDAPIdentityStore level? - private Set rolesSyncedModels = new TreeSet(); + private Set rolesSyncedModels = new TreeSet<>(); @Override public void onImportUserFromLDAP(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, UserModel user, RealmModel realm, boolean isCreate) { @@ -74,7 +74,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { // Import role mappings from LDAP into Keycloak DB String roleNameAttr = getRoleNameLdapAttribute(mapperModel); for (LDAPObject ldapRole : ldapRoles) { - String roleName = ldapRole.getAttributeAsStringCaseInsensitive(roleNameAttr); + String roleName = ldapRole.getAttributeAsString(roleNameAttr); RoleContainerModel roleContainer = getTargetRoleContainer(mapperModel, realm); RoleModel role = roleContainer.getRole(roleName); @@ -95,7 +95,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { if (!rolesSyncedModels.contains(mapperModel.getId())) { logger.debugf("Syncing roles from LDAP into Keycloak DB. Mapper is [%s], LDAP provider is [%s]", mapperModel.getName(), ldapProvider.getModel().getDisplayName()); - LDAPIdentityQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); + LDAPQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); // Send query List ldapRoles = ldapQuery.getResultList(); @@ -103,7 +103,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { RoleContainerModel roleContainer = getTargetRoleContainer(mapperModel, realm); String rolesRdnAttr = getRoleNameLdapAttribute(mapperModel); for (LDAPObject ldapRole : ldapRoles) { - String roleName = ldapRole.getAttributeAsStringCaseInsensitive(rolesRdnAttr); + String roleName = ldapRole.getAttributeAsString(rolesRdnAttr); if (roleContainer.getRole(roleName) == null) { logger.infof("Syncing role [%s] from LDAP to keycloak DB", roleName); @@ -115,8 +115,8 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { } } - public LDAPIdentityQuery createRoleQuery(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider) { - LDAPIdentityQuery ldapQuery = new LDAPIdentityQuery(ldapProvider); + public LDAPQuery createRoleQuery(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider) { + LDAPQuery ldapQuery = new LDAPQuery(ldapProvider); // For now, use same search scope, which is configured "globally" and used for user's search. ldapQuery.setSearchScope(ldapProvider.getLdapIdentityStore().getConfig().getSearchScope()); @@ -178,7 +178,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { } String[] objClasses = objectClasses.split(","); - Set trimmed = new HashSet(); + Set trimmed = new HashSet<>(); for (String objectClass : objClasses) { objectClass = objectClass.trim(); if (objectClass.length() > 0) { @@ -202,7 +202,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { String roleNameAttribute = getRoleNameLdapAttribute(mapperModel); ldapObject.setRdnAttributeName(roleNameAttribute); ldapObject.setObjectClasses(getRoleObjectClasses(mapperModel, ldapProvider)); - ldapObject.setAttribute(roleNameAttribute, roleName); + ldapObject.setSingleAttribute(roleNameAttribute, roleName); LDAPDn roleDn = LDAPDn.fromString(getRolesDn(mapperModel)); roleDn.addFirst(roleNameAttribute, roleName); @@ -220,6 +220,15 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { } Set memberships = getExistingMemberships(mapperModel, ldapRole); + + // Remove membership placeholder if present + for (String membership : memberships) { + if (membership.trim().length() == 0) { + memberships.remove(membership); + break; + } + } + memberships.add(ldapUser.getDn().toString()); ldapRole.setAttribute(getMembershipLdapAttribute(mapperModel), memberships); @@ -240,7 +249,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { } public LDAPObject loadLDAPRoleByName(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, String roleName) { - LDAPIdentityQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); + LDAPQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); Condition roleNameCondition = new LDAPQueryConditionsBuilder().equal(new QueryParameter(getRoleNameLdapAttribute(mapperModel)), roleName); ldapQuery.where(roleNameCondition); return ldapQuery.getFirstResult(); @@ -248,29 +257,15 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { protected Set getExistingMemberships(UserFederationMapperModel mapperModel, LDAPObject ldapRole) { String memberAttrName = getMembershipLdapAttribute(mapperModel); - Set memberships = new TreeSet(); - Object existingMemberships = ldapRole.getAttributeCaseInsensitive(memberAttrName); - - if (existingMemberships != null) { - if (existingMemberships instanceof String) { - String existingMembership = existingMemberships.toString().trim(); - if (existingMemberships != null && existingMembership.length() > 0) { - memberships.add(existingMembership); - } - } else if (existingMemberships instanceof Collection) { - Collection exMemberships = (Collection) existingMemberships; - for (String membership : exMemberships) { - if (membership.trim().length() > 0) { - memberships.add(membership); - } - } - } + Set memberships = ldapRole.getAttributeAsSet(memberAttrName); + if (memberships == null) { + memberships = new HashSet<>(); } return memberships; } protected List getLDAPRoleMappings(UserFederationMapperModel mapperModel, LDAPFederationProvider ldapProvider, LDAPObject ldapUser) { - LDAPIdentityQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); + LDAPQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); String membershipAttr = getMembershipLdapAttribute(mapperModel); Condition membershipCondition = new LDAPQueryConditionsBuilder().equal(new QueryParameter(membershipAttr), ldapUser.getDn().toString()); ldapQuery.where(membershipCondition); @@ -290,7 +285,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { } @Override - public void beforeLDAPQuery(UserFederationMapperModel mapperModel, LDAPIdentityQuery query) { + public void beforeLDAPQuery(UserFederationMapperModel mapperModel, LDAPQuery query) { } @@ -389,7 +384,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { if (mode == Mode.LDAP_ONLY) { // For LDAP-only we want to retrieve role mappings of target container just from LDAP - Set modelRolesCopy = new HashSet(modelRoleMappings); + Set modelRolesCopy = new HashSet<>(modelRoleMappings); for (RoleModel role : modelRolesCopy) { if (role.getContainer().equals(targetRoleContainer)) { modelRoleMappings.remove(role); @@ -408,10 +403,10 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { List ldapRoles = getLDAPRoleMappings(mapperModel, ldapProvider, ldapUser); - Set roles = new HashSet(); + Set roles = new HashSet<>(); String roleNameLdapAttr = getRoleNameLdapAttribute(mapperModel); for (LDAPObject role : ldapRoles) { - String roleName = role.getAttributeAsStringCaseInsensitive(roleNameLdapAttr); + String roleName = role.getAttributeAsString(roleNameLdapAttr); RoleModel modelRole = roleContainer.getRole(roleName); if (modelRole == null) { // Add role to local DB @@ -430,7 +425,7 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper { RoleContainerModel roleContainer = getTargetRoleContainer(mapperModel, realm); if (role.getContainer().equals(roleContainer)) { - LDAPIdentityQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); + LDAPQuery ldapQuery = createRoleQuery(mapperModel, ldapProvider); LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); Condition roleNameCondition = conditionsBuilder.equal(new QueryParameter(getRoleNameLdapAttribute(mapperModel)), role.getName()); Condition membershipCondition = conditionsBuilder.equal(new QueryParameter(getMembershipLdapAttribute(mapperModel)), ldapUser.getDn().toString()); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java index c372769575..24bf4509e5 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/UserAttributeLDAPFederationMapper.java @@ -1,14 +1,20 @@ package org.keycloak.federation.ldap.mappers; import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; +import java.util.Set; +import org.jboss.logging.Logger; import org.keycloak.federation.ldap.LDAPFederationProvider; import org.keycloak.federation.ldap.idm.model.LDAPObject; import org.keycloak.federation.ldap.idm.query.Condition; import org.keycloak.federation.ldap.idm.query.QueryParameter; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.models.RealmModel; import org.keycloak.models.UserFederationMapperModel; import org.keycloak.models.UserFederationProvider; @@ -23,10 +29,12 @@ import org.keycloak.models.utils.reflection.PropertyQueries; */ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMapper { + private static final Logger logger = Logger.getLogger(UserAttributeLDAPFederationMapper.class); + private static final Map> userModelProperties; static { - userModelProperties = PropertyQueries.createQuery(UserModel.class).addCriteria(new PropertyCriteria() { + Map> userModelProps = PropertyQueries.createQuery(UserModel.class).addCriteria(new PropertyCriteria() { @Override public boolean methodMatches(Method m) { @@ -38,6 +46,12 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap } }).getResultList(); + + // Convert to be keyed by lower-cased attribute names + userModelProperties = new HashMap<>(); + for (Map.Entry> entry : userModelProps.entrySet()) { + userModelProperties.put(entry.getKey().toLowerCase(), entry.getValue()); + } } public static final String USER_MODEL_ATTRIBUTE = "user.model.attribute"; @@ -51,16 +65,21 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap String userModelAttrName = mapperModel.getConfig().get(USER_MODEL_ATTRIBUTE); String ldapAttrName = mapperModel.getConfig().get(LDAP_ATTRIBUTE); - Object ldapAttrValue = ldapUser.getAttributeCaseInsensitive(ldapAttrName); - if (ldapAttrValue != null && !ldapAttrValue.toString().trim().isEmpty()) { - Property userModelProperty = userModelProperties.get(userModelAttrName); + Property userModelProperty = userModelProperties.get(userModelAttrName.toLowerCase()); - if (userModelProperty != null) { - // we have java property on UserModel - userModelProperty.setValue(user, ldapAttrValue); + if (userModelProperty != null) { + + // we have java property on UserModel + String ldapAttrValue = ldapUser.getAttributeAsString(ldapAttrName); + setPropertyOnUserModel(userModelProperty, user, ldapAttrValue); + } else { + + // we don't have java property. Let's set attribute + Set ldapAttrValue = ldapUser.getAttributeAsSet(ldapAttrName); + if (ldapAttrValue != null) { + user.setAttribute(userModelAttrName, new ArrayList<>(ldapAttrValue)); } else { - // we don't have java property. Let's just setAttribute - user.setAttribute(userModelAttrName, (String) ldapAttrValue); + user.removeAttribute(userModelAttrName); } } } @@ -70,18 +89,26 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap String userModelAttrName = mapperModel.getConfig().get(USER_MODEL_ATTRIBUTE); String ldapAttrName = mapperModel.getConfig().get(LDAP_ATTRIBUTE); - Property userModelProperty = userModelProperties.get(userModelAttrName); + Property userModelProperty = userModelProperties.get(userModelAttrName.toLowerCase()); - Object attrValue; if (userModelProperty != null) { - // we have java property on UserModel - attrValue = userModelProperty.getValue(localUser); + + // we have java property on UserModel. Assuming we support just properties of simple types + Object attrValue = userModelProperty.getValue(localUser); + String valueAsString = (attrValue == null) ? null : attrValue.toString(); + ldapUser.setSingleAttribute(ldapAttrName, valueAsString); } else { - // we don't have java property. Let's just setAttribute - attrValue = localUser.getAttribute(userModelAttrName); + + // we don't have java property. Let's set attribute + List attrValues = localUser.getAttribute(userModelAttrName); + + if (attrValues.size() == 0) { + ldapUser.setAttribute(ldapAttrName, null); + } else { + ldapUser.setAttribute(ldapAttrName, new LinkedHashSet<>(attrValues)); + } } - ldapUser.setAttribute(ldapAttrName, attrValue); if (isReadOnly(mapperModel)) { ldapUser.addReadOnlyAttributeName(ldapAttrName); } @@ -99,9 +126,21 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap delegate = new TxAwareLDAPUserModelDelegate(delegate, ldapProvider, ldapUser) { @Override - public void setAttribute(String name, String value) { + public void setSingleAttribute(String name, String value) { setLDAPAttribute(name, value); - super.setAttribute(name, value); + super.setSingleAttribute(name, value); + } + + @Override + public void setAttribute(String name, List values) { + setLDAPAttribute(name, values); + super.setAttribute(name, values); + } + + @Override + public void removeAttribute(String name) { + setLDAPAttribute(name, null); + super.removeAttribute(name); } @Override @@ -122,15 +161,22 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap super.setFirstName(firstName); } - protected void setLDAPAttribute(String modelAttrName, String value) { + protected void setLDAPAttribute(String modelAttrName, Object value) { if (modelAttrName.equalsIgnoreCase(userModelAttrName)) { if (logger.isTraceEnabled()) { - logger.tracef("Pushing user attribute to LDAP. Model attribute name: %s, LDAP attribute name: %s, Attribute value: %s", modelAttrName, ldapAttrName, value); + logger.tracef("Pushing user attribute to LDAP. username: %s, Model attribute name: %s, LDAP attribute name: %s, Attribute value: %s", getUsername(), modelAttrName, ldapAttrName, value); } ensureTransactionStarted(); - ldapUser.setAttribute(ldapAttrName, value); + if (value == null) { + ldapUser.setAttribute(ldapAttrName, null); + } else if (value instanceof String) { + ldapUser.setSingleAttribute(ldapAttrName, (String) value); + } else { + List asList = (List) value; + ldapUser.setAttribute(ldapAttrName, new LinkedHashSet<>(asList)); + } } } @@ -144,32 +190,48 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap delegate = new UserModelDelegate(delegate) { @Override - public String getAttribute(String name) { + public String getFirstAttribute(String name) { if (name.equalsIgnoreCase(userModelAttrName)) { - // TODO: Support different types than strings as well... - return ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName); + return ldapUser.getAttributeAsString(ldapAttrName); + } else { + return super.getFirstAttribute(name); + } + } + + @Override + public List getAttribute(String name) { + if (name.equalsIgnoreCase(userModelAttrName)) { + Collection ldapAttrValue = ldapUser.getAttributeAsSet(ldapAttrName); + if (ldapAttrValue == null) { + return null; + } else { + return new ArrayList<>(ldapAttrValue); + } } else { return super.getAttribute(name); } } @Override - public Map getAttributes() { - Map attrs = new HashMap<>(super.getAttributes()); + public Map> getAttributes() { + Map> attrs = new HashMap<>(super.getAttributes()); - // Ignore properties - if (UserModel.EMAIL.equalsIgnoreCase(userModelAttrName) || UserModel.FIRST_NAME.equalsIgnoreCase(userModelAttrName) || UserModel.LAST_NAME.equalsIgnoreCase(userModelAttrName)) { + // Ignore UserModel properties + if (userModelProperties.get(userModelAttrName.toLowerCase()) != null) { return attrs; } - attrs.put(userModelAttrName, ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName)); + Set allLdapAttrValues = ldapUser.getAttributeAsSet(ldapAttrName); + if (allLdapAttrValues != null) { + attrs.put(userModelAttrName, new ArrayList<>(allLdapAttrValues)); + } return attrs; } @Override public String getEmail() { if (UserModel.EMAIL.equalsIgnoreCase(userModelAttrName)) { - return ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName); + return ldapUser.getAttributeAsString(ldapAttrName); } else { return super.getEmail(); } @@ -178,7 +240,7 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap @Override public String getLastName() { if (UserModel.LAST_NAME.equalsIgnoreCase(userModelAttrName)) { - return ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName); + return ldapUser.getAttributeAsString(ldapAttrName); } else { return super.getLastName(); } @@ -187,7 +249,7 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap @Override public String getFirstName() { if (UserModel.FIRST_NAME.equalsIgnoreCase(userModelAttrName)) { - return ldapUser.getAttributeAsStringCaseInsensitive(ldapAttrName); + return ldapUser.getAttributeAsString(ldapAttrName); } else { return super.getFirstName(); } @@ -200,7 +262,7 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap } @Override - public void beforeLDAPQuery(UserFederationMapperModel mapperModel, LDAPIdentityQuery query) { + public void beforeLDAPQuery(UserFederationMapperModel mapperModel, LDAPQuery query) { String userModelAttrName = mapperModel.getConfig().get(USER_MODEL_ATTRIBUTE); String ldapAttrName = mapperModel.getConfig().get(LDAP_ATTRIBUTE); @@ -222,4 +284,22 @@ public class UserAttributeLDAPFederationMapper extends AbstractLDAPFederationMap private boolean isReadOnly(UserFederationMapperModel mapperModel) { return parseBooleanParameter(mapperModel, READ_ONLY); } + + + protected void setPropertyOnUserModel(Property userModelProperty, UserModel user, String ldapAttrValue) { + if (ldapAttrValue == null) { + userModelProperty.setValue(user, null); + } else { + Class clazz = userModelProperty.getJavaClass(); + + if (String.class.equals(clazz)) { + userModelProperty.setValue(user, ldapAttrValue); + } else if (Boolean.class.equals(clazz) || boolean.class.equals(clazz)) { + Boolean boolVal = Boolean.valueOf(ldapAttrValue); + userModelProperty.setValue(user, boolVal); + } else { + logger.warnf("Don't know how to set the property '%s' on user '%s' . Value of LDAP attribute is '%s' ", userModelProperty.getName(), user.getUsername(), ldapAttrValue.toString()); + } + } + } } diff --git a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountBean.java b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountBean.java index 20323705fe..c152e11143 100755 --- a/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountBean.java +++ b/forms/account-freemarker/src/main/java/org/keycloak/account/freemarker/model/AccountBean.java @@ -1,9 +1,12 @@ package org.keycloak.account.freemarker.model; +import org.jboss.logging.Logger; import org.keycloak.models.UserModel; +import org.keycloak.util.MultivaluedHashMap; import javax.ws.rs.core.MultivaluedMap; import java.util.HashMap; +import java.util.List; import java.util.Map; /** @@ -11,14 +14,29 @@ import java.util.Map; */ public class AccountBean { + private static final Logger logger = Logger.getLogger(AccountBean.class); + private final UserModel user; private final MultivaluedMap profileFormData; + + // TODO: More proper multi-value attribute support private final Map attributes = new HashMap<>(); public AccountBean(UserModel user, MultivaluedMap profileFormData) { this.user = user; this.profileFormData = profileFormData; - attributes.putAll(user.getAttributes()); + + for (Map.Entry> attr : user.getAttributes().entrySet()) { + List attrValue = attr.getValue(); + if (attrValue.size() > 0) { + attributes.put(attr.getKey(), attrValue.get(0)); + } + + if (attrValue.size() > 1) { + logger.warnf("There are more values for attribute '%s' of user '%s' . Will display just first value", attr.getKey(), user.getUsername()); + } + } + if (profileFormData != null) { for (String key : profileFormData.keySet()) { if (key.startsWith("user.attributes.")) { diff --git a/forms/common-freemarker/src/main/java/org/keycloak/freemarker/LocaleHelper.java b/forms/common-freemarker/src/main/java/org/keycloak/freemarker/LocaleHelper.java index 0029dfd245..fc249d56bf 100644 --- a/forms/common-freemarker/src/main/java/org/keycloak/freemarker/LocaleHelper.java +++ b/forms/common-freemarker/src/main/java/org/keycloak/freemarker/LocaleHelper.java @@ -35,7 +35,7 @@ public class LocaleHelper { Locale locale = findLocale(realm.getSupportedLocales(), localeString); if(locale != null){ if(user != null){ - user.setAttribute(UserModel.LOCALE, locale.toLanguageTag()); + user.setSingleAttribute(UserModel.LOCALE, locale.toLanguageTag()); } return locale; }else{ @@ -48,8 +48,8 @@ public class LocaleHelper { String localeString = httpHeaders.getCookies().get(LOCALE_COOKIE).getValue(); Locale locale = findLocale(realm.getSupportedLocales(), localeString); if(locale != null){ - if(user != null && user.getAttribute(UserModel.LOCALE) == null){ - user.setAttribute(UserModel.LOCALE, locale.toLanguageTag()); + if(user != null && user.getFirstAttribute(UserModel.LOCALE) == null){ + user.setSingleAttribute(UserModel.LOCALE, locale.toLanguageTag()); } return locale; }else{ @@ -59,7 +59,7 @@ public class LocaleHelper { //2. User profile if(user != null && user.getAttributes().containsKey(UserModel.LOCALE)){ - String localeString = user.getAttribute(UserModel.LOCALE); + String localeString = user.getFirstAttribute(UserModel.LOCALE); Locale locale = findLocale(realm.getSupportedLocales(), localeString); if(locale != null){ diff --git a/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js b/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js index 2ff1273816..f4059944b8 100755 --- a/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js +++ b/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js @@ -206,6 +206,8 @@ module.controller('UserDetailCtrl', function($scope, realm, user, User, UserFede if (!user.attributes) { user.attributes = {} } + convertAttributeValuesToString(user); + $scope.user = angular.copy(user); if(user.federationLink) { console.log("federationLink is not null"); @@ -252,13 +254,15 @@ module.controller('UserDetailCtrl', function($scope, realm, user, User, UserFede }, true); $scope.save = function() { + convertAttributeValuesToLists(); + if ($scope.create) { User.save({ realm: realm.realm }, $scope.user, function (data, headers) { $scope.changed = false; + convertAttributeValuesToString($scope.user); user = angular.copy($scope.user); - var l = headers().location; console.debug("Location == " + l); @@ -275,12 +279,33 @@ module.controller('UserDetailCtrl', function($scope, realm, user, User, UserFede userId: $scope.user.id }, $scope.user, function () { $scope.changed = false; + convertAttributeValuesToString($scope.user); user = angular.copy($scope.user); Notifications.success("Your changes have been saved to the user."); }); } }; + function convertAttributeValuesToLists() { + var attrs = $scope.user.attributes; + for (var attribute in attrs) { + if (typeof attrs[attribute] === "string") { + var attrVals = attrs[attribute].split("##"); + attrs[attribute] = attrVals; + } + } + } + + function convertAttributeValuesToString(user) { + var attrs = user.attributes; + for (var attribute in attrs) { + if (typeof attrs[attribute] === "object") { + var attrVals = attrs[attribute].join("##"); + attrs[attribute] = attrVals; + } + } + } + $scope.reset = function() { $scope.user = angular.copy(user); $scope.changed = false; diff --git a/model/api/src/main/java/org/keycloak/models/UserModel.java b/model/api/src/main/java/org/keycloak/models/UserModel.java index 645250e91b..dea9e7b039 100755 --- a/model/api/src/main/java/org/keycloak/models/UserModel.java +++ b/model/api/src/main/java/org/keycloak/models/UserModel.java @@ -27,13 +27,31 @@ public interface UserModel { void setEnabled(boolean enabled); - void setAttribute(String name, String value); + /** + * Set single value of specified attribute. Remove all other existing values + * + * @param name + * @param value + */ + void setSingleAttribute(String name, String value); + + void setAttribute(String name, List values); void removeAttribute(String name); - String getAttribute(String name); + /** + * @param name + * @return null if there is not any value of specified attribute or first value otherwise. Don't throw exception if there are more values of the attribute + */ + String getFirstAttribute(String name); - Map getAttributes(); + /** + * @param name + * @return list of all attribute values or empty list if there are not any values. Never return null + */ + List getAttribute(String name); + + Map> getAttributes(); Set getRequiredActions(); diff --git a/model/api/src/main/java/org/keycloak/models/entities/UserEntity.java b/model/api/src/main/java/org/keycloak/models/entities/UserEntity.java index 064697b00a..d21d20310d 100755 --- a/model/api/src/main/java/org/keycloak/models/entities/UserEntity.java +++ b/model/api/src/main/java/org/keycloak/models/entities/UserEntity.java @@ -1,7 +1,5 @@ package org.keycloak.models.entities; -import org.keycloak.models.UserModel; - import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -23,7 +21,7 @@ public class UserEntity extends AbstractIdentifiableEntity { private List roleIds; - private Map attributes; + private Map> attributes; private List requiredActions; private List credentials = new ArrayList(); private List federatedIdentities; @@ -101,11 +99,11 @@ public class UserEntity extends AbstractIdentifiableEntity { this.roleIds = roleIds; } - public Map getAttributes() { + public Map> getAttributes() { return attributes; } - public void setAttributes(Map attributes) { + public void setAttributes(Map> attributes) { this.attributes = attributes; } diff --git a/model/api/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/model/api/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 467040f448..3bf412c93e 100755 --- a/model/api/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/model/api/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -31,6 +31,7 @@ import org.keycloak.representations.idm.UserFederationMapperRepresentation; import org.keycloak.representations.idm.UserFederationProviderRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.UserSessionRepresentation; +import org.keycloak.util.MultivaluedHashMap; import org.keycloak.util.Time; import java.util.ArrayList; @@ -67,7 +68,7 @@ public class ModelToRepresentation { rep.setRequiredActions(reqActions); if (user.getAttributes() != null && !user.getAttributes().isEmpty()) { - Map attrs = new HashMap(); + Map attrs = new HashMap<>(); attrs.putAll(user.getAttributes()); rep.setAttributes(attrs); } diff --git a/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index 245e6c6e8b..217da6714d 100755 --- a/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/model/api/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -43,6 +43,7 @@ import org.keycloak.util.UriUtils; import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; @@ -805,8 +806,17 @@ public class RepresentationToModel { user.setFederationLink(userRep.getFederationLink()); user.setTotp(userRep.isTotp()); if (userRep.getAttributes() != null) { - for (Map.Entry entry : userRep.getAttributes().entrySet()) { - user.setAttribute(entry.getKey(), entry.getValue()); + for (Map.Entry entry : userRep.getAttributes().entrySet()) { + Object value = entry.getValue(); + + if (value instanceof Collection) { + Collection colVal = (Collection) value; + user.setAttribute(entry.getKey(), new ArrayList<>(colVal)); + } else if (value instanceof String) { + // TODO: This is here just for backwards compatibility with KC 1.3 and earlier + String stringVal = (String) value; + user.setSingleAttribute(entry.getKey(), stringVal); + } } } if (userRep.getRequiredActions() != null) { diff --git a/model/api/src/main/java/org/keycloak/models/utils/UserModelDelegate.java b/model/api/src/main/java/org/keycloak/models/utils/UserModelDelegate.java index 7123c3e1d5..699a38e308 100755 --- a/model/api/src/main/java/org/keycloak/models/utils/UserModelDelegate.java +++ b/model/api/src/main/java/org/keycloak/models/utils/UserModelDelegate.java @@ -53,8 +53,13 @@ public class UserModelDelegate implements UserModel { } @Override - public void setAttribute(String name, String value) { - delegate.setAttribute(name, value); + public void setSingleAttribute(String name, String value) { + delegate.setSingleAttribute(name, value); + } + + @Override + public void setAttribute(String name, List values) { + delegate.setAttribute(name, values); } @Override @@ -63,12 +68,17 @@ public class UserModelDelegate implements UserModel { } @Override - public String getAttribute(String name) { + public String getFirstAttribute(String name) { + return delegate.getFirstAttribute(name); + } + + @Override + public List getAttribute(String name) { return delegate.getAttribute(name); } @Override - public Map getAttributes() { + public Map> getAttributes() { return delegate.getAttributes(); } diff --git a/model/file/src/main/java/org/keycloak/models/file/adapter/UserAdapter.java b/model/file/src/main/java/org/keycloak/models/file/adapter/UserAdapter.java index 39024c1100..5c33375b5a 100755 --- a/model/file/src/main/java/org/keycloak/models/file/adapter/UserAdapter.java +++ b/model/file/src/main/java/org/keycloak/models/file/adapter/UserAdapter.java @@ -158,12 +158,23 @@ public class UserAdapter implements UserModel, Comparable { } @Override - public void setAttribute(String name, String value) { + public void setSingleAttribute(String name, String value) { if (user.getAttributes() == null) { - user.setAttributes(new HashMap()); + user.setAttributes(new HashMap>()); } - user.getAttributes().put(name, value); + List attrValues = new ArrayList<>(); + attrValues.add(value); + user.getAttributes().put(name, attrValues); + } + + @Override + public void setAttribute(String name, List values) { + if (user.getAttributes() == null) { + user.setAttributes(new HashMap>()); + } + + user.getAttributes().put(name, values); } @Override @@ -174,13 +185,23 @@ public class UserAdapter implements UserModel, Comparable { } @Override - public String getAttribute(String name) { - return user.getAttributes()==null ? null : user.getAttributes().get(name); + public String getFirstAttribute(String name) { + if (user.getAttributes()==null) return null; + + List attrValues = user.getAttributes().get(name); + return (attrValues==null || attrValues.isEmpty()) ? null : attrValues.get(0); } @Override - public Map getAttributes() { - return user.getAttributes()==null ? Collections.emptyMap() : Collections.unmodifiableMap(user.getAttributes()); + public List getAttribute(String name) { + if (user.getAttributes()==null) return Collections.emptyList(); + List attrValues = user.getAttributes().get(name); + return (attrValues == null) ? Collections.emptyList() : Collections.unmodifiableList(attrValues); + } + + @Override + public Map> getAttributes() { + return user.getAttributes()==null ? Collections.>emptyMap() : Collections.unmodifiableMap((Map)user.getAttributes()); } @Override diff --git a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/UserAdapter.java b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/UserAdapter.java index aa80a25fc1..fb516c60f4 100755 --- a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/UserAdapter.java +++ b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/UserAdapter.java @@ -11,6 +11,7 @@ import org.keycloak.models.UserCredentialValueModel; import org.keycloak.models.UserModel; import org.keycloak.models.cache.entities.CachedUser; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -78,9 +79,15 @@ public class UserAdapter implements UserModel { } @Override - public void setAttribute(String name, String value) { + public void setSingleAttribute(String name, String value) { getDelegateForUpdate(); - updated.setAttribute(name, value); + updated.setSingleAttribute(name, value); + } + + @Override + public void setAttribute(String name, List values) { + getDelegateForUpdate(); + updated.setAttribute(name, values); } @Override @@ -90,13 +97,20 @@ public class UserAdapter implements UserModel { } @Override - public String getAttribute(String name) { - if (updated != null) return updated.getAttribute(name); - return cached.getAttributes().get(name); + public String getFirstAttribute(String name) { + if (updated != null) return updated.getFirstAttribute(name); + return cached.getAttributes().getFirst(name); } @Override - public Map getAttributes() { + public List getAttribute(String name) { + if (updated != null) return updated.getAttribute(name); + List result = cached.getAttributes().get(name); + return (result == null) ? Collections.emptyList() : result; + } + + @Override + public Map> getAttributes() { if (updated != null) return updated.getAttributes(); return cached.getAttributes(); } diff --git a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/entities/CachedUser.java b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/entities/CachedUser.java index bbef81b759..d7824cc85b 100755 --- a/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/entities/CachedUser.java +++ b/model/invalidation-cache/model-adapters/src/main/java/org/keycloak/models/cache/entities/CachedUser.java @@ -4,6 +4,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserCredentialValueModel; import org.keycloak.models.UserModel; +import org.keycloak.util.MultivaluedHashMap; import java.io.Serializable; import java.util.HashMap; @@ -29,7 +30,7 @@ public class CachedUser implements Serializable { private boolean enabled; private boolean totp; private String federationLink; - private Map attributes = new HashMap<>(); + private MultivaluedHashMap attributes = new MultivaluedHashMap<>(); private Set requiredActions = new HashSet<>(); private Set roleMappings = new HashSet(); @@ -93,7 +94,7 @@ public class CachedUser implements Serializable { return totp; } - public Map getAttributes() { + public MultivaluedHashMap getAttributes() { return attributes; } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java index 670f5f039b..3719b9dd9a 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java @@ -22,6 +22,7 @@ import org.keycloak.models.jpa.entities.UserRequiredActionEntity; import org.keycloak.models.jpa.entities.UserRoleMappingEntity; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.Pbkdf2PasswordEncoder; +import org.keycloak.util.MultivaluedHashMap; import org.keycloak.util.Time; import javax.persistence.EntityManager; @@ -92,14 +93,46 @@ public class UserAdapter implements UserModel { } @Override - public void setAttribute(String name, String value) { + public void setSingleAttribute(String name, String value) { + boolean found = false; + List toRemove = new ArrayList<>(); for (UserAttributeEntity attr : user.getAttributes()) { if (attr.getName().equals(name)) { - attr.setValue(value); - return; + if (!found) { + attr.setValue(value); + found = true; + } else { + toRemove.add(attr); + } } } + + for (UserAttributeEntity attr : toRemove) { + em.remove(attr); + user.getAttributes().remove(attr); + } + + if (found) { + return; + } + + persistAttributeValue(name, value); + } + + @Override + public void setAttribute(String name, List values) { + // Remove all existing + removeAttribute(name); + + // Put all new + for (String value : values) { + persistAttributeValue(name, value); + } + } + + private void persistAttributeValue(String name, String value) { UserAttributeEntity attr = new UserAttributeEntity(); + attr.setId(KeycloakModelUtils.generateId()); attr.setName(name); attr.setValue(value); attr.setUser(user); @@ -120,7 +153,7 @@ public class UserAdapter implements UserModel { } @Override - public String getAttribute(String name) { + public String getFirstAttribute(String name) { for (UserAttributeEntity attr : user.getAttributes()) { if (attr.getName().equals(name)) { return attr.getValue(); @@ -130,10 +163,21 @@ public class UserAdapter implements UserModel { } @Override - public Map getAttributes() { - Map result = new HashMap(); + public List getAttribute(String name) { + List result = new ArrayList<>(); for (UserAttributeEntity attr : user.getAttributes()) { - result.put(attr.getName(), attr.getValue()); + if (attr.getName().equals(name)) { + result.add(attr.getValue()); + } + } + return result; + } + + @Override + public Map> getAttributes() { + MultivaluedHashMap result = new MultivaluedHashMap<>(); + for (UserAttributeEntity attr : user.getAttributes()) { + result.add(attr.getName(), attr.getValue()); } return result; } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserAttributeEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserAttributeEntity.java index 2454f969fa..51352b4bf6 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserAttributeEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserAttributeEntity.java @@ -1,6 +1,8 @@ package org.keycloak.models.jpa.entities; +import javax.persistence.CollectionTable; import javax.persistence.Column; +import javax.persistence.ElementCollection; import javax.persistence.Entity; import javax.persistence.FetchType; import javax.persistence.Id; @@ -11,6 +13,8 @@ import javax.persistence.NamedQueries; import javax.persistence.NamedQuery; import javax.persistence.Table; import java.io.Serializable; +import java.util.HashSet; +import java.util.Set; /** * @author Bill Burke @@ -22,20 +26,29 @@ import java.io.Serializable; }) @Table(name="USER_ATTRIBUTE") @Entity -@IdClass(UserAttributeEntity.Key.class) public class UserAttributeEntity { @Id + @Column(name="ID", length = 36) + protected String id; + @ManyToOne(fetch= FetchType.LAZY) @JoinColumn(name = "USER_ID") protected UserEntity user; - @Id @Column(name = "NAME") protected String name; @Column(name = "VALUE") protected String value; + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + public String getName() { return name; } @@ -60,47 +73,4 @@ public class UserAttributeEntity { this.user = user; } - public static class Key implements Serializable { - - protected UserEntity user; - - protected String name; - - public Key() { - } - - public Key(UserEntity user, String name) { - this.user = user; - this.name = name; - } - - public UserEntity getUser() { - return user; - } - - public String getName() { - return name; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - - Key key = (Key) o; - - if (name != null ? !name.equals(key.name) : key.name != null) return false; - if (user != null ? !user.getId().equals(key.user != null ? key.user.getId() : null) : key.user != null) return false; - - return true; - } - - @Override - public int hashCode() { - int result = user != null ? user.getId().hashCode() : 0; - result = 31 * result + (name != null ? name.hashCode() : 0); - return result; - } - } - } diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java index 79a6260b9d..dc858ef982 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java @@ -19,7 +19,6 @@ import org.keycloak.models.UserCredentialValueModel; import org.keycloak.models.UserModel; import org.keycloak.models.entities.CredentialEntity; import org.keycloak.models.entities.UserConsentEntity; -import org.keycloak.models.mongo.keycloak.entities.MongoRoleEntity; import org.keycloak.models.mongo.keycloak.entities.MongoUserConsentEntity; import org.keycloak.models.mongo.keycloak.entities.MongoUserEntity; import org.keycloak.models.mongo.utils.MongoModelUtils; @@ -127,12 +126,24 @@ public class UserAdapter extends AbstractMongoAdapter implement } @Override - public void setAttribute(String name, String value) { + public void setSingleAttribute(String name, String value) { if (user.getAttributes() == null) { - user.setAttributes(new HashMap()); + user.setAttributes(new HashMap>()); } - user.getAttributes().put(name, value); + List attrValues = new ArrayList<>(); + attrValues.add(value); + user.getAttributes().put(name, attrValues); + updateUser(); + } + + @Override + public void setAttribute(String name, List values) { + if (user.getAttributes() == null) { + user.setAttributes(new HashMap>()); + } + + user.getAttributes().put(name, values); updateUser(); } @@ -145,13 +156,23 @@ public class UserAdapter extends AbstractMongoAdapter implement } @Override - public String getAttribute(String name) { - return user.getAttributes()==null ? null : user.getAttributes().get(name); + public String getFirstAttribute(String name) { + if (user.getAttributes()==null) return null; + + List attrValues = user.getAttributes().get(name); + return (attrValues==null || attrValues.isEmpty()) ? null : attrValues.get(0); } @Override - public Map getAttributes() { - return user.getAttributes()==null ? Collections.emptyMap() : Collections.unmodifiableMap(user.getAttributes()); + public List getAttribute(String name) { + if (user.getAttributes()==null) return Collections.emptyList(); + List attrValues = user.getAttributes().get(name); + return (attrValues == null) ? Collections.emptyList() : Collections.unmodifiableList(attrValues); + } + + @Override + public Map> getAttributes() { + return user.getAttributes()==null ? Collections.>emptyMap() : Collections.unmodifiableMap((Map)user.getAttributes()); } public MongoUserEntity getUser() { diff --git a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java index d5630bd54a..6dcef8157f 100755 --- a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java +++ b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java @@ -238,11 +238,11 @@ public class SamlProtocol implements LoginProtocol { // generate a persistent user id specifically for each client. UserModel user = userSession.getUser(); String name = SAML_PERSISTENT_NAME_ID_FOR + "." + clientSession.getClient().getClientId(); - String samlPersistentId = user.getAttribute(name); + String samlPersistentId = user.getFirstAttribute(name); if (samlPersistentId != null) return samlPersistentId; // "G-" stands for "generated" samlPersistentId = "G-" + UUID.randomUUID().toString(); - user.setAttribute(name, samlPersistentId); + user.setSingleAttribute(name, samlPersistentId); return samlPersistentId; } else if(nameIdFormat.equals(JBossSAMLURIConstants.NAMEID_FORMAT_UNSPECIFIED.get())){ // TODO: Support for persistent NameID (pseudo-random identifier persisted in user object) diff --git a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/mappers/UserAttributeStatementMapper.java b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/mappers/UserAttributeStatementMapper.java index 671abb2daf..b5f59adcc2 100755 --- a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/mappers/UserAttributeStatementMapper.java +++ b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/mappers/UserAttributeStatementMapper.java @@ -13,7 +13,7 @@ import java.util.ArrayList; import java.util.List; /** - * Mappings UserModel property (the property name of a getter method) to an AttributeStatement. + * Mappings UserModel attribute (not property name of a getter method) to an AttributeStatement. * * @author Bill Burke * @version $Revision: 1 $ @@ -62,7 +62,7 @@ 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 = user.getAttribute(attributeName); + String attributeValue = user.getFirstAttribute(attributeName); if (attributeValue == null) return; AttributeStatementHelper.addAttribute(attributeStatement, mappingModel, attributeValue); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/mappers/AddressMapper.java b/services/src/main/java/org/keycloak/protocol/oidc/mappers/AddressMapper.java index 5ebff0d8ef..64fc4cdc53 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/mappers/AddressMapper.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/mappers/AddressMapper.java @@ -118,11 +118,11 @@ public class AddressMapper extends AbstractOIDCProtocolMapper implements OIDCAcc protected void setClaim(IDToken token, UserSessionModel userSession) { UserModel user = userSession.getUser(); AddressClaimSet addressSet = new AddressClaimSet(); - addressSet.setStreetAddress(user.getAttribute("street")); - addressSet.setLocality(user.getAttribute("locality")); - addressSet.setRegion(user.getAttribute("region")); - addressSet.setPostalCode(user.getAttribute("postal_code")); - addressSet.setCountry(user.getAttribute("country")); + addressSet.setStreetAddress(user.getFirstAttribute("street")); + addressSet.setLocality(user.getFirstAttribute("locality")); + addressSet.setRegion(user.getFirstAttribute("region")); + addressSet.setPostalCode(user.getFirstAttribute("postal_code")); + addressSet.setCountry(user.getFirstAttribute("country")); token.getOtherClaims().put("address", addressSet); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/mappers/UserAttributeMapper.java b/services/src/main/java/org/keycloak/protocol/oidc/mappers/UserAttributeMapper.java index b8543510d6..164f7a5f92 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/mappers/UserAttributeMapper.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/mappers/UserAttributeMapper.java @@ -3,7 +3,6 @@ package org.keycloak.protocol.oidc.mappers; import org.keycloak.models.ClientSessionModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ProtocolMapperModel; -import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; import org.keycloak.protocol.ProtocolMapperUtils; @@ -77,7 +76,7 @@ public class UserAttributeMapper extends AbstractOIDCProtocolMapper implements O protected void setClaim(IDToken token, ProtocolMapperModel mappingModel, UserSessionModel userSession) { UserModel user = userSession.getUser(); String attributeName = mappingModel.getConfig().get(ProtocolMapperUtils.USER_ATTRIBUTE); - String attributeValue = user.getAttribute(attributeName); + String attributeValue = user.getFirstAttribute(attributeName); if (attributeValue == null) return; OIDCAttributeMapperHelper.mapClaim(token, mappingModel, attributeValue); } diff --git a/services/src/main/java/org/keycloak/services/resources/AttributeFormDataProcessor.java b/services/src/main/java/org/keycloak/services/resources/AttributeFormDataProcessor.java index e426b10fe3..10ce8db87f 100755 --- a/services/src/main/java/org/keycloak/services/resources/AttributeFormDataProcessor.java +++ b/services/src/main/java/org/keycloak/services/resources/AttributeFormDataProcessor.java @@ -1,5 +1,8 @@ package org.keycloak.services.resources; +import java.util.ArrayList; +import java.util.List; + import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; @@ -21,8 +24,26 @@ public class AttributeFormDataProcessor { for (String key : formData.keySet()) { if (!key.startsWith("user.attributes.")) continue; String attribute = key.substring("user.attributes.".length()); - user.setAttribute(attribute, formData.getFirst(key)); + + // Need to handle case when attribute has multiple values, but in UI was displayed just first value + List modelValue = new ArrayList<>(user.getAttribute(attribute)); + + int index = 0; + for (String value : formData.get(key)) { + addOrSetValue(modelValue, index, value); + index++; + } + + user.setAttribute(attribute, modelValue); } } + + private static void addOrSetValue(List list, int index, String value) { + if (list.size() > index) { + list.set(index, value); + } else { + list.add(value); + } + } } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java index b27e45353d..b6f1fb179e 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java @@ -5,7 +5,6 @@ import org.jboss.resteasy.annotations.cache.NoCache; import org.jboss.resteasy.spi.BadRequestException; import org.jboss.resteasy.spi.NotFoundException; import org.keycloak.ClientConnection; -import org.keycloak.authentication.RequiredActionFactory; import org.keycloak.authentication.RequiredActionProvider; import org.keycloak.email.EmailException; import org.keycloak.email.EmailProvider; @@ -228,8 +227,8 @@ public class UsersResource { } } - if (rep.getAttributes() != null) { - for (Map.Entry attr : rep.getAttributes().entrySet()) { + if (rep.getAttributesAsListValues() != null) { + for (Map.Entry> attr : rep.getAttributesAsListValues().entrySet()) { user.setAttribute(attr.getKey(), attr.getValue()); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/ProfileTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/ProfileTest.java index 526813bc6b..dd572d722e 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/ProfileTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/ProfileTest.java @@ -6,6 +6,7 @@ import org.apache.http.HttpResponse; import org.apache.http.client.HttpClient; import org.apache.http.client.methods.HttpGet; import org.apache.http.impl.client.DefaultHttpClient; +import org.json.JSONArray; import org.json.JSONObject; import org.junit.ClassRule; import org.junit.Rule; @@ -51,8 +52,8 @@ public class ProfileTest { UserModel user = manager.getSession().users().getUserByUsername("test-user@localhost", appRealm); user.setFirstName("First"); user.setLastName("Last"); - user.setAttribute("key1", "value1"); - user.setAttribute("key2", "value2"); + user.setSingleAttribute("key1", "value1"); + user.setSingleAttribute("key2", "value2"); ClientModel accountApp = appRealm.getClientByClientId(org.keycloak.models.Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); @@ -114,8 +115,12 @@ public class ProfileTest { assertEquals("Last", profile.getString("lastName")); JSONObject attributes = profile.getJSONObject("attributes"); - assertEquals("value1", attributes.getString("key1")); - assertEquals("value2", attributes.getString("key2")); + JSONArray attrValue = attributes.getJSONArray("key1"); + assertEquals(1, attrValue.length()); + assertEquals("value1", attrValue.get(0)); + attrValue = attributes.getJSONArray("key2"); + assertEquals(1, attrValue.length()); + assertEquals("value2", attrValue.get(0)); } @Test diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java index 42dd464011..ab644cdcdb 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -12,6 +12,8 @@ import org.keycloak.representations.idm.UserRepresentation; import javax.ws.rs.ClientErrorException; import javax.ws.rs.core.Response; + +import java.util.ArrayList; import java.util.List; import static org.junit.Assert.assertEquals; @@ -271,8 +273,8 @@ public class UserTest extends AbstractClientTest { public void attributes() { UserRepresentation user1 = new UserRepresentation(); user1.setUsername("user1"); - user1.attribute("attr1", "value1user1"); - user1.attribute("attr2", "value2user1"); + user1.singleAttribute("attr1", "value1user1"); + user1.singleAttribute("attr2", "value2user1"); Response response = realm.users().create(user1); String user1Id = ApiUtil.getCreatedId(response); @@ -280,40 +282,45 @@ public class UserTest extends AbstractClientTest { UserRepresentation user2 = new UserRepresentation(); user2.setUsername("user2"); - user2.attribute("attr1", "value1user2"); - user2.attribute("attr2", "value2user2"); + user2.singleAttribute("attr1", "value1user2"); + List vals = new ArrayList<>(); + vals.add("value2user2"); + vals.add("value2user2_2"); + user2.getAttributesAsListValues().put("attr2", vals); response = realm.users().create(user2); String user2Id = ApiUtil.getCreatedId(response); response.close(); user1 = realm.users().get(user1Id).toRepresentation(); - assertEquals(2, user1.getAttributes().size()); - assertEquals("value1user1", user1.getAttributes().get("attr1")); - assertEquals("value2user1", user1.getAttributes().get("attr2")); + assertEquals(2, user1.getAttributesAsListValues().size()); + assertAttributeValue("value1user1", user1.getAttributesAsListValues().get("attr1")); + assertAttributeValue("value2user1", user1.getAttributesAsListValues().get("attr2")); user2 = realm.users().get(user2Id).toRepresentation(); - assertEquals(2, user2.getAttributes().size()); - assertEquals("value1user2", user2.getAttributes().get("attr1")); - assertEquals("value2user2", user2.getAttributes().get("attr2")); + assertEquals(2, user2.getAttributesAsListValues().size()); + assertAttributeValue("value1user2", user2.getAttributesAsListValues().get("attr1")); + vals = user2.getAttributesAsListValues().get("attr2"); + assertEquals(2, vals.size()); + assertTrue(vals.contains("value2user2") && vals.contains("value2user2_2")); - user1.attribute("attr1", "value3user1"); - user1.attribute("attr3", "value4user1"); + user1.singleAttribute("attr1", "value3user1"); + user1.singleAttribute("attr3", "value4user1"); realm.users().get(user1Id).update(user1); user1 = realm.users().get(user1Id).toRepresentation(); - assertEquals(3, user1.getAttributes().size()); - assertEquals("value3user1", user1.getAttributes().get("attr1")); - assertEquals("value2user1", user1.getAttributes().get("attr2")); - assertEquals("value4user1", user1.getAttributes().get("attr3")); + assertEquals(3, user1.getAttributesAsListValues().size()); + assertAttributeValue("value3user1", user1.getAttributesAsListValues().get("attr1")); + assertAttributeValue("value2user1", user1.getAttributesAsListValues().get("attr2")); + assertAttributeValue("value4user1", user1.getAttributesAsListValues().get("attr3")); user1.getAttributes().remove("attr1"); realm.users().get(user1Id).update(user1); user1 = realm.users().get(user1Id).toRepresentation(); - assertEquals(2, user1.getAttributes().size()); - assertEquals("value2user1", user1.getAttributes().get("attr2")); - assertEquals("value4user1", user1.getAttributes().get("attr3")); + assertEquals(2, user1.getAttributesAsListValues().size()); + assertAttributeValue("value2user1", user1.getAttributesAsListValues().get("attr2")); + assertAttributeValue("value4user1", user1.getAttributesAsListValues().get("attr3")); user1.getAttributes().clear(); realm.users().get(user1Id).update(user1); @@ -322,6 +329,11 @@ public class UserTest extends AbstractClientTest { assertNull(user1.getAttributes()); } + private void assertAttributeValue(String expectedValue, List attrValues) { + assertEquals(1, attrValues.size()); + assertEquals(expectedValue, attrValues.get(0)); + } + @Test public void sendResetPasswordEmail() { UserRepresentation userRep = new UserRepresentation(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java index 3f4ffa5957..ba6d44bcf0 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java @@ -141,7 +141,7 @@ public abstract class AbstractIdentityProviderTest { identityProviderModel.setUpdateProfileFirstLoginMode(IdentityProviderRepresentation.UPFLM_ON); UserModel user = assertSuccessfulAuthentication(identityProviderModel, "test-user", "new@email.com", true); - Assert.assertEquals("617-666-7777", user.getAttribute("mobile")); + Assert.assertEquals("617-666-7777", user.getFirstAttribute("mobile")); } @Test @@ -304,7 +304,7 @@ public abstract class AbstractIdentityProviderTest { identityProviderModel.setTrustEmail(true); UserModel user = assertSuccessfulAuthenticationWithEmailVerification(identityProviderModel, "test-user", "new@email.com", true); - Assert.assertEquals("617-666-7777", user.getAttribute("mobile")); + Assert.assertEquals("617-666-7777", user.getFirstAttribute("mobile")); } finally { identityProviderModel.setTrustEmail(false); getRealm().setVerifyEmail(false); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java index 1643aa0743..c4a028c033 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationProvidersIntegrationTest.java @@ -279,7 +279,7 @@ public class FederationProvidersIntegrationTest { // Fetch user from LDAP and check that postalCode is filled UserModel user = session.users().getUserByUsername("johnzip", appRealm); - String postalCode = user.getAttribute("postal_code"); + String postalCode = user.getFirstAttribute("postal_code"); Assert.assertEquals("12398", postalCode); } finally { @@ -299,21 +299,21 @@ public class FederationProvidersIntegrationTest { // Fetch user from LDAP and check that postalCode is filled UserModel user = session.users().getUserByUsername("johndirect", appRealm); - String postalCode = user.getAttribute("postal_code"); + String postalCode = user.getFirstAttribute("postal_code"); Assert.assertEquals("12399", postalCode); // Directly update user in LDAP - johnDirect.setAttribute(LDAPConstants.POSTAL_CODE, "12400"); - johnDirect.setAttribute(LDAPConstants.SN, "DirectLDAPUpdated"); + johnDirect.setSingleAttribute(LDAPConstants.POSTAL_CODE, "12400"); + johnDirect.setSingleAttribute(LDAPConstants.SN, "DirectLDAPUpdated"); ldapFedProvider.getLdapIdentityStore().update(johnDirect); // Verify that postalCode is still the same as we read it's value from Keycloak DB user = session.users().getUserByUsername("johndirect", appRealm); - postalCode = user.getAttribute("postal_code"); + postalCode = user.getFirstAttribute("postal_code"); Assert.assertEquals("12399", postalCode); // Check user.getAttributes() - postalCode = user.getAttributes().get("postal_code"); + postalCode = user.getAttributes().get("postal_code").get(0); Assert.assertEquals("12399", postalCode); // LastName is new as lastName mapper will read the value from LDAP @@ -339,11 +339,11 @@ public class FederationProvidersIntegrationTest { // Verify that postalCode is read from LDAP now UserModel user = session.users().getUserByUsername("johndirect", appRealm); - String postalCode = user.getAttribute("postal_code"); + String postalCode = user.getFirstAttribute("postal_code"); Assert.assertEquals("12400", postalCode); // Check user.getAttributes() - postalCode = user.getAttributes().get("postal_code"); + postalCode = user.getAttributes().get("postal_code").get(0); Assert.assertEquals("12400", postalCode); Assert.assertFalse(user.getAttributes().containsKey(UserModel.LAST_NAME)); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java index 1a78875ff4..56c4a70b1b 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/FederationTestUtils.java @@ -1,5 +1,7 @@ package org.keycloak.testsuite.federation; +import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.junit.Assert; @@ -7,7 +9,7 @@ import org.keycloak.federation.ldap.LDAPFederationProvider; import org.keycloak.federation.ldap.LDAPFederationProviderFactory; import org.keycloak.federation.ldap.LDAPUtils; import org.keycloak.federation.ldap.idm.model.LDAPObject; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.federation.ldap.idm.store.ldap.LDAPIdentityStore; import org.keycloak.federation.ldap.mappers.RoleLDAPFederationMapper; import org.keycloak.federation.ldap.mappers.RoleLDAPFederationMapperFactory; @@ -69,11 +71,11 @@ class FederationTestUtils { } @Override - public String getAttribute(String name) { + public List getAttribute(String name) { if ("postal_code".equals(name)) { - return postalCode; + return Arrays.asList(postalCode); } else { - return null; + return Collections.emptyList(); } } }; @@ -91,7 +93,7 @@ class FederationTestUtils { Assert.assertEquals(expectedFirstName, user.getFirstName()); Assert.assertEquals(expectedLastName, user.getLastName()); Assert.assertEquals(expectedEmail, user.getEmail()); - Assert.assertEquals(expectedPostalCode, user.getAttribute("postal_code")); + Assert.assertEquals(expectedPostalCode, user.getFirstAttribute("postal_code")); } public static void addZipCodeLDAPMapper(RealmModel realm, UserFederationProviderModel providerModel) { @@ -138,7 +140,7 @@ class FederationTestUtils { public static void removeAllLDAPUsers(LDAPFederationProvider ldapProvider, RealmModel realm) { LDAPIdentityStore ldapStore = ldapProvider.getLdapIdentityStore(); - LDAPIdentityQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm); + LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(ldapProvider, realm); List allUsers = ldapQuery.getResultList(); for (LDAPObject ldapUser : allUsers) { @@ -149,7 +151,7 @@ class FederationTestUtils { public static void removeAllLDAPRoles(KeycloakSession session, RealmModel appRealm, UserFederationProviderModel ldapModel, String mapperName) { UserFederationMapperModel mapperModel = appRealm.getUserFederationMapperByName(ldapModel.getId(), mapperName); LDAPFederationProvider ldapProvider = FederationTestUtils.getLdapProvider(session, ldapModel); - LDAPIdentityQuery roleQuery = new RoleLDAPFederationMapper().createRoleQuery(mapperModel, ldapProvider); + LDAPQuery roleQuery = new RoleLDAPFederationMapper().createRoleQuery(mapperModel, ldapProvider); List ldapRoles = roleQuery.getResultList(); for (LDAPObject ldapRole : ldapRoles) { ldapProvider.getLdapIdentityStore().remove(ldapRole); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPMultipleAttributesTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPMultipleAttributesTest.java new file mode 100644 index 0000000000..ed9d3380e9 --- /dev/null +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPMultipleAttributesTest.java @@ -0,0 +1,112 @@ +package org.keycloak.testsuite.federation; + +import java.util.List; +import java.util.Map; + +import org.junit.Assert; +import org.junit.ClassRule; +import org.junit.FixMethodOrder; +import org.junit.Test; +import org.junit.rules.RuleChain; +import org.junit.rules.TestRule; +import org.junit.runners.MethodSorters; +import org.keycloak.federation.ldap.LDAPFederationProvider; +import org.keycloak.federation.ldap.LDAPFederationProviderFactory; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.LDAPConstants; +import org.keycloak.models.RealmModel; +import org.keycloak.models.UserFederationProvider; +import org.keycloak.models.UserFederationProviderModel; +import org.keycloak.models.UserModel; +import org.keycloak.services.managers.RealmManager; +import org.keycloak.testsuite.rule.KeycloakRule; +import org.keycloak.testsuite.rule.LDAPRule; + +/** + * @author Marek Posolda + */ +@FixMethodOrder(MethodSorters.NAME_ASCENDING) +public class LDAPMultipleAttributesTest { + + private static LDAPRule ldapRule = new LDAPRule(); + + private static UserFederationProviderModel ldapModel = null; + + private static KeycloakRule keycloakRule = new KeycloakRule(new KeycloakRule.KeycloakSetup() { + + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + Map ldapConfig = ldapRule.getConfig(); + ldapConfig.put(LDAPConstants.EDIT_MODE, UserFederationProvider.EditMode.WRITABLE.toString()); + + ldapModel = appRealm.addUserFederationProvider(LDAPFederationProviderFactory.PROVIDER_NAME, ldapConfig, 0, "test-ldap", -1, -1, 0); + FederationTestUtils.addZipCodeLDAPMapper(appRealm, ldapModel); + } + }); + + @ClassRule + public static TestRule chain = RuleChain + .outerRule(ldapRule) + .around(keycloakRule); + + @Test + public void testModel() { + KeycloakSession session = keycloakRule.startSession(); + try { + RealmModel appRealm = session.realms().getRealmByName("test"); + LDAPFederationProvider ldapProvider = FederationTestUtils.getLdapProvider(session, ldapModel); + + FederationTestUtils.assertUserImported(session.users(), appRealm, "jbrown", "James", "Brown", "jbrown@keycloak.org", "88441"); + + UserModel user = session.users().getUserByUsername("bwilson", appRealm); + Assert.assertEquals("bwilson@keycloak.org", user.getEmail()); + Assert.assertEquals("Bruce", user.getFirstName()); + + // There are 2 lastnames in ldif + Assert.assertTrue("Wilson".equals(user.getLastName()) || "Schneider".equals(user.getLastName())); + + // Actually there are 2 postalCodes + List postalCodes = user.getAttribute("postal_code"); + assertPostalCodes(postalCodes, "88441", "77332"); + + postalCodes.remove("77332"); + user.setAttribute("postal_code", postalCodes); + + } finally { + keycloakRule.stopSession(session, true); + } + + session = keycloakRule.startSession(); + try { + RealmModel appRealm = session.realms().getRealmByName("test"); + UserModel user = session.users().getUserByUsername("bwilson", appRealm); + List postalCodes = user.getAttribute("postal_code"); + assertPostalCodes(postalCodes, "88441"); + + postalCodes.add("77332"); + user.setAttribute("postal_code", postalCodes); + assertPostalCodes(user.getAttribute("postal_code"), "88441", "77332"); + } finally { + keycloakRule.stopSession(session, true); + } + } + + private void assertPostalCodes(List postalCodes, String... expectedPostalCodes) { + if (expectedPostalCodes == null && postalCodes.isEmpty()) { + return; + } + + + Assert.assertEquals(expectedPostalCodes.length, postalCodes.size()); + for (String expected : expectedPostalCodes) { + if (!postalCodes.contains(expected)) { + Assert.fail("postalCode '" + expected + "' not in postalCodes: " + postalCodes); + } + } + } + + + +} + + diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPRoleMappingsTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPRoleMappingsTest.java index f8a6944232..4802105107 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPRoleMappingsTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPRoleMappingsTest.java @@ -16,7 +16,7 @@ import org.keycloak.federation.ldap.LDAPFederationProviderFactory; import org.keycloak.federation.ldap.idm.model.LDAPObject; import org.keycloak.federation.ldap.idm.query.Condition; import org.keycloak.federation.ldap.idm.query.QueryParameter; -import org.keycloak.federation.ldap.idm.query.internal.LDAPIdentityQuery; +import org.keycloak.federation.ldap.idm.query.internal.LDAPQuery; import org.keycloak.federation.ldap.idm.query.internal.LDAPQueryConditionsBuilder; import org.keycloak.federation.ldap.mappers.RoleLDAPFederationMapper; import org.keycloak.models.AccountRoles; @@ -329,7 +329,7 @@ public class LDAPRoleMappingsTest { } private void deleteRoleMappingsInLDAP(UserFederationMapperModel roleMapperModel, RoleLDAPFederationMapper roleMapper, LDAPFederationProvider ldapProvider, LDAPObject ldapUser, String roleName) { - LDAPIdentityQuery ldapQuery = roleMapper.createRoleQuery(roleMapperModel, ldapProvider); + LDAPQuery ldapQuery = roleMapper.createRoleQuery(roleMapperModel, ldapProvider); LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); Condition roleNameCondition = conditionsBuilder.equal(new QueryParameter(LDAPConstants.CN), roleName); ldapQuery.where(roleNameCondition); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java index bab44c146c..b463bcdfbd 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java @@ -126,8 +126,8 @@ public class SyncProvidersTest { FederationTestUtils.addLDAPUser(ldapFedProvider, testRealm, "user6", "User6FN", "User6LN", "user6@email.org", "126"); LDAPObject ldapUser5 = ldapFedProvider.loadLDAPUserByUsername(testRealm, "user5"); // NOTE: Changing LDAP attributes directly here - ldapUser5.setAttribute(LDAPConstants.EMAIL, "user5Updated@email.org"); - ldapUser5.setAttribute(LDAPConstants.POSTAL_CODE, "521"); + ldapUser5.setSingleAttribute(LDAPConstants.EMAIL, "user5Updated@email.org"); + ldapUser5.setSingleAttribute(LDAPConstants.POSTAL_CODE, "521"); ldapFedProvider.getLdapIdentityStore().update(ldapUser5); // Assert still old users in local provider diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/i18n/EmailTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/i18n/EmailTest.java index b8e2453d27..af6750f62a 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/i18n/EmailTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/i18n/EmailTest.java @@ -54,7 +54,7 @@ public class EmailTest { UserModel user = manager.getSession().users().addUser(appRealm, "login-test"); user.setEmail("login@test.com"); user.setEnabled(true); - user.setAttribute(UserModel.LOCALE, "de"); + user.setSingleAttribute(UserModel.LOCALE, "de"); UserCredentialModel creds = new UserCredentialModel(); creds.setType(CredentialRepresentation.PASSWORD); @@ -91,7 +91,7 @@ public class EmailTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - manager.getSession().users().getUserByUsername("login-test", appRealm).setAttribute(UserModel.LOCALE, "en"); + manager.getSession().users().getUserByUsername("login-test", appRealm).setSingleAttribute(UserModel.LOCALE, "en"); } }); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AdapterTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AdapterTest.java index 8640d8e44a..57b4ce9135 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AdapterTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/AdapterTest.java @@ -25,7 +25,6 @@ import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -159,7 +158,7 @@ public class AdapterTest extends AbstractModelTest { test1CreateRealm(); UserModel user = realmManager.getSession().users().addUser(realmModel, "bburke"); - user.setAttribute("attr1", "val1"); + user.setSingleAttribute("attr1", "val1"); user.addRequiredAction(UserModel.RequiredAction.UPDATE_PASSWORD); RoleModel testRole = realmModel.addRole("test"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java index 276698de93..e28d2b39aa 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java @@ -140,6 +140,22 @@ public class ImportTest extends AbstractModelTest { Assert.assertEquals(1, appRoles.size()); Assert.assertEquals("app-admin", appRoles.iterator().next().getName()); + // Test attributes + Map> attrs = wburke.getAttributes(); + Assert.assertEquals(1, attrs.size()); + List attrVals = attrs.get("email"); + Assert.assertEquals(1, attrVals.size()); + Assert.assertEquals("bburke@redhat.com", attrVals.get(0)); + + attrs = admin.getAttributes(); + Assert.assertEquals(2, attrs.size()); + attrVals = attrs.get("key1"); + Assert.assertEquals(1, attrVals.size()); + Assert.assertEquals("val1", attrVals.get(0)); + attrVals = attrs.get("key2"); + Assert.assertEquals(2, attrVals.size()); + Assert.assertTrue(attrVals.contains("val21") && attrVals.contains("val22")); + // Test client ClientModel oauthClient = realm.getClientByClientId("oauthclient"); Assert.assertEquals("clientpassword", oauthClient.getSecret()); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserModelTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserModelTest.java index d9e1aa5d0e..ff7429303b 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserModelTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserModelTest.java @@ -8,6 +8,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserModel.RequiredAction; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -137,6 +138,61 @@ public class UserModelTest extends AbstractModelTest { Assert.assertTrue(user.getRequiredActions().isEmpty()); } + @Test + public void testUserMultipleAttributes() throws Exception { + RealmModel realm = realmManager.createRealm("original"); + UserModel user = session.users().addUser(realm, "user"); + + user.setSingleAttribute("key1", "value1"); + List attrVals = new ArrayList<>(Arrays.asList( "val21", "val22" )); + user.setAttribute("key2", attrVals); + + commit(); + + // Test read attributes + realm = realmManager.getRealmByName("original"); + user = session.users().getUserByUsername("user", realm); + + attrVals = user.getAttribute("key1"); + Assert.assertEquals(1, attrVals.size()); + Assert.assertEquals("value1", attrVals.get(0)); + Assert.assertEquals("value1", user.getFirstAttribute("key1")); + + attrVals = user.getAttribute("key2"); + Assert.assertEquals(2, attrVals.size()); + Assert.assertTrue(attrVals.contains("val21")); + Assert.assertTrue(attrVals.contains("val22")); + + attrVals = user.getAttribute("key3"); + Assert.assertTrue(attrVals.isEmpty()); + Assert.assertNull(user.getFirstAttribute("key3")); + + Map> allAttrVals = user.getAttributes(); + Assert.assertEquals(2, allAttrVals.size()); + Assert.assertEquals(allAttrVals.get("key1"), user.getAttribute("key1")); + Assert.assertEquals(allAttrVals.get("key2"), user.getAttribute("key2")); + + // Test searching + Map attributes = new HashMap(); + attributes.put("key2", "val22"); + List users = session.users().searchForUserByAttributes(attributes, realm); + Assert.assertEquals(1, users.size()); + Assert.assertEquals(users.get(0), user); + + // Test remove and rewrite attribute + user.removeAttribute("key1"); + user.setSingleAttribute("key2", "val23"); + + commit(); + + realm = realmManager.getRealmByName("original"); + user = session.users().getUserByUsername("user", realm); + Assert.assertNull(user.getFirstAttribute("key1")); + attrVals = user.getAttribute("key2"); + Assert.assertEquals(1, attrVals.size()); + Assert.assertEquals("val23", attrVals.get(0)); + } + public static void assertEquals(UserModel expected, UserModel actual) { Assert.assertEquals(expected.getUsername(), actual.getUsername()); Assert.assertEquals(expected.getFirstName(), actual.getFirstName()); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java index 0920be51ae..c92c1624e1 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java @@ -616,12 +616,12 @@ public class AccessTokenTest { KeycloakSession session = keycloakRule.startSession(); RealmModel realm = session.realms().getRealmByName("test"); UserModel user = session.users().getUserByUsername("test-user@localhost", realm); - user.setAttribute("street", "5 Yawkey Way"); - user.setAttribute("locality", "Boston"); - user.setAttribute("region", "MA"); - user.setAttribute("postal_code", "02115"); - user.setAttribute("country", "USA"); - user.setAttribute("phone", "617-777-6666"); + user.setSingleAttribute("street", "5 Yawkey Way"); + user.setSingleAttribute("locality", "Boston"); + user.setSingleAttribute("region", "MA"); + user.setSingleAttribute("postal_code", "02115"); + user.setSingleAttribute("country", "USA"); + user.setSingleAttribute("phone", "617-777-6666"); ClientModel app = realm.getClientByClientId("test-app"); ProtocolMapperModel mapper = AddressMapper.createAddressMapper(true, true); app.addProtocolMapper(mapper); diff --git a/testsuite/integration/src/test/resources/ldap/users.ldif b/testsuite/integration/src/test/resources/ldap/users.ldif index de41e19420..b04f081414 100644 --- a/testsuite/integration/src/test/resources/ldap/users.ldif +++ b/testsuite/integration/src/test/resources/ldap/users.ldif @@ -38,6 +38,7 @@ objectclass: inetOrgPerson uid: bwilson cn: Bruce sn: Wilson +sn: Schneider mail: bwilson@keycloak.org postalCode: 88441 postalCode: 77332 diff --git a/testsuite/integration/src/test/resources/model/testrealm.json b/testsuite/integration/src/test/resources/model/testrealm.json index 0e41313879..00336c82ea 100755 --- a/testsuite/integration/src/test/resources/model/testrealm.json +++ b/testsuite/integration/src/test/resources/model/testrealm.json @@ -82,6 +82,15 @@ { "username": "admin", "enabled": true, + "attributes": { + "key1": [ + "val1" + ], + "key2": [ + "val21", + "val22" + ] + }, "credentials": [ { "type": "password", From 605c88a029c56fb35a8f678d9a2567e748acdaef Mon Sep 17 00:00:00 2001 From: mposolda Date: Mon, 29 Jun 2015 19:25:27 +0200 Subject: [PATCH 2/2] KEYCLOAK-1487 Multivalued support for UserAttribute protocol mapper. End-to-end LDAP example test including application --- .../org/keycloak/models/LDAPConstants.java | 1 + .../protocol/ProtocolMapperUtils.java | 3 + .../mappers/OIDCAttributeMapperHelper.java | 31 ++++++- .../oidc/mappers/UserAttributeMapper.java | 20 ++++- .../federation/LDAPExampleServlet.java | 58 +++++++++++++ .../LDAPMultipleAttributesTest.java | 81 +++++++++++++++++++ .../testsuite/oauth/AccessTokenTest.java | 15 +++- .../resources/ldap/ldap-app-keycloak.json | 10 +++ .../src/test/resources/ldap/users.ldif | 3 + 9 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPExampleServlet.java create mode 100644 testsuite/integration/src/test/resources/ldap/ldap-app-keycloak.json diff --git a/model/api/src/main/java/org/keycloak/models/LDAPConstants.java b/model/api/src/main/java/org/keycloak/models/LDAPConstants.java index 0d97664029..1a32097dce 100644 --- a/model/api/src/main/java/org/keycloak/models/LDAPConstants.java +++ b/model/api/src/main/java/org/keycloak/models/LDAPConstants.java @@ -60,6 +60,7 @@ public class LDAPConstants { public static final String SAM_ACCOUNT_NAME = "sAMAccountName"; public static final String EMAIL = "mail"; public static final String POSTAL_CODE = "postalCode"; + public static final String STREET = "street"; public static final String MEMBER = "member"; public static final String MEMBER_OF = "memberOf"; public static final String OBJECT_CLASS = "objectclass"; diff --git a/services/src/main/java/org/keycloak/protocol/ProtocolMapperUtils.java b/services/src/main/java/org/keycloak/protocol/ProtocolMapperUtils.java index b91b7542a0..457084ac57 100755 --- a/services/src/main/java/org/keycloak/protocol/ProtocolMapperUtils.java +++ b/services/src/main/java/org/keycloak/protocol/ProtocolMapperUtils.java @@ -15,12 +15,15 @@ import java.util.List; public class ProtocolMapperUtils { public static final String USER_ATTRIBUTE = "user.attribute"; public static final String USER_SESSION_NOTE = "user.session.note"; + public static final String MULTIVALUED = "multivalued"; public static final String USER_MODEL_PROPERTY_LABEL = "User Property"; public static final String USER_MODEL_PROPERTY_HELP_TEXT = "Name of the property method in the UserModel interface. For example, a value of 'email' would reference the UserModel.getEmail() method."; public static final String USER_MODEL_ATTRIBUTE_LABEL = "User Attribute"; public static final String USER_MODEL_ATTRIBUTE_HELP_TEXT = "Name of stored user attribute which is the name of an attribute within the UserModel.attribute map."; public static final String USER_SESSION_MODEL_NOTE_LABEL = "User Session Note"; public static final String USER_SESSION_MODEL_NOTE_HELP_TEXT = "Name of stored user session note within the UserSessionModel.note map."; + public static final String MULTIVALUED_LABEL = "Multivalued"; + public static final String MULTIVALUED_HELP_TEXT = "Indicates if attribute supports multiple values. If true, then the list of all values of this attribute will be set as claim. If false, then just first value will be set as claim"; public static String getUserModelValue(UserModel user, String propertyName) { 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 bae8c08abd..9e53b22a1a 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 @@ -1,5 +1,6 @@ package org.keycloak.protocol.oidc.mappers; +import org.jboss.logging.Logger; import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.RealmModel; import org.keycloak.protocol.ProtocolMapper; @@ -19,6 +20,8 @@ import java.util.Map; * @version $Revision: 1 $ */ public class OIDCAttributeMapperHelper { + private static final Logger logger = Logger.getLogger(OIDCAttributeMapperHelper.class); + public static final String TOKEN_CLAIM_NAME = "claim.name"; public static final String TOKEN_CLAIM_NAME_LABEL = "Token Claim Name"; public static final String JSON_TYPE = "Claim JSON Type"; @@ -31,6 +34,26 @@ public class OIDCAttributeMapperHelper { public static Object mapAttributeValue(ProtocolMapperModel mappingModel, Object attributeValue) { if (attributeValue == null) return null; + + if (attributeValue instanceof List) { + List valueAsList = (List) attributeValue; + if (valueAsList.size() == 0) return null; + + if (isMultivalued(mappingModel)) { + List result = new ArrayList<>(); + for (Object valueItem : valueAsList) { + result.add(mapAttributeValue(mappingModel, valueItem)); + } + return result; + } else { + if (valueAsList.size() > 1) { + logger.warnf("Multiple values found '%s' for protocol mapper '%s' but expected just single value", attributeValue.toString(), mappingModel.getName()); + } + + attributeValue = valueAsList.get(0); + } + } + String type = mappingModel.getConfig().get(JSON_TYPE); if (type == null) return attributeValue; if (type.equals("boolean")) { @@ -53,8 +76,9 @@ public class OIDCAttributeMapperHelper { } public static void mapClaim(IDToken token, ProtocolMapperModel mappingModel, Object attributeValue) { - if (attributeValue == null) return; attributeValue = mapAttributeValue(mappingModel, attributeValue); + if (attributeValue == null) return; + String protocolClaim = mappingModel.getConfig().get(TOKEN_CLAIM_NAME); String[] split = protocolClaim.split("\\."); Map jsonObject = token.getOtherClaims(); @@ -102,6 +126,11 @@ public class OIDCAttributeMapperHelper { return "true".equals(mappingModel.getConfig().get(INCLUDE_IN_ACCESS_TOKEN)); } + + public static boolean isMultivalued(ProtocolMapperModel mappingModel) { + return "true".equals(mappingModel.getConfig().get(ProtocolMapperUtils.MULTIVALUED)); + } + public static void addAttributeConfig(List configProperties) { ProviderConfigProperty property; property = new ProviderConfigProperty(); diff --git a/services/src/main/java/org/keycloak/protocol/oidc/mappers/UserAttributeMapper.java b/services/src/main/java/org/keycloak/protocol/oidc/mappers/UserAttributeMapper.java index 164f7a5f92..b4c9c7632e 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/mappers/UserAttributeMapper.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/mappers/UserAttributeMapper.java @@ -1,5 +1,6 @@ package org.keycloak.protocol.oidc.mappers; +import org.jboss.logging.Logger; import org.keycloak.models.ClientSessionModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ProtocolMapperModel; @@ -35,6 +36,13 @@ public class UserAttributeMapper extends AbstractOIDCProtocolMapper implements O configProperties.add(property); OIDCAttributeMapperHelper.addAttributeConfig(configProperties); + property = new ProviderConfigProperty(); + property.setName(ProtocolMapperUtils.MULTIVALUED); + property.setLabel(ProtocolMapperUtils.MULTIVALUED_LABEL); + property.setHelpText(ProtocolMapperUtils.MULTIVALUED_HELP_TEXT); + property.setType(ProviderConfigProperty.BOOLEAN_TYPE); + configProperties.add(property); + } public static final String PROVIDER_ID = "oidc-usermodel-attribute-mapper"; @@ -76,7 +84,7 @@ public class UserAttributeMapper extends AbstractOIDCProtocolMapper implements O protected void setClaim(IDToken token, ProtocolMapperModel mappingModel, UserSessionModel userSession) { UserModel user = userSession.getUser(); String attributeName = mappingModel.getConfig().get(ProtocolMapperUtils.USER_ATTRIBUTE); - String attributeValue = user.getFirstAttribute(attributeName); + List attributeValue = user.getAttribute(attributeName); if (attributeValue == null) return; OIDCAttributeMapperHelper.mapClaim(token, mappingModel, attributeValue); } @@ -92,12 +100,18 @@ public class UserAttributeMapper extends AbstractOIDCProtocolMapper implements O String userAttribute, String tokenClaimName, String claimType, boolean consentRequired, String consentText, - boolean accessToken, boolean idToken) { - return OIDCAttributeMapperHelper.createClaimMapper(name, userAttribute, + boolean accessToken, boolean idToken, boolean multivalued) { + ProtocolMapperModel mapper = OIDCAttributeMapperHelper.createClaimMapper(name, userAttribute, tokenClaimName, claimType, consentRequired, consentText, accessToken, idToken, PROVIDER_ID); + + if (multivalued) { + mapper.getConfig().put(ProtocolMapperUtils.MULTIVALUED, "true"); + } + + return mapper; } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPExampleServlet.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPExampleServlet.java new file mode 100644 index 0000000000..a629ddbca8 --- /dev/null +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPExampleServlet.java @@ -0,0 +1,58 @@ +package org.keycloak.testsuite.federation; + +import java.io.IOException; +import java.io.PrintWriter; +import java.util.List; +import java.util.Map; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.keycloak.KeycloakSecurityContext; +import org.keycloak.representations.IDToken; + +/** + * @author Marek Posolda + */ +public class LDAPExampleServlet extends HttpServlet { + + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + KeycloakSecurityContext securityContext = (KeycloakSecurityContext) req.getAttribute(KeycloakSecurityContext.class.getName()); + IDToken idToken = securityContext.getIdToken(); + + PrintWriter out = resp.getWriter(); + out.println("LDAP Portal"); + out.println(""); + + out.printf("", "preferred_username", idToken.getPreferredUsername()); + out.println(); + out.printf("", "name", idToken.getName()); + out.println(); + out.printf("", "email", idToken.getEmail()); + out.println(); + + for (Map.Entry claim : idToken.getOtherClaims().entrySet()) { + Object value = claim.getValue(); + + if (value instanceof List) { + List asList = (List) value; + StringBuilder result = new StringBuilder(); + for (String item : asList) { + result.append(item + "
"); + } + value = result.toString(); + } + + out.printf("", claim.getKey(), value); + out.println(); + } + + out.println("
Attribute nameAttribute values
%s%s
%s%s
%s%s
%s%s
"); + out.flush(); + } + +} diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPMultipleAttributesTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPMultipleAttributesTest.java index ed9d3380e9..e1e9aff5c7 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPMultipleAttributesTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/LDAPMultipleAttributesTest.java @@ -1,26 +1,40 @@ package org.keycloak.testsuite.federation; +import java.net.URL; import java.util.List; import java.util.Map; +import javax.ws.rs.core.UriBuilder; + import org.junit.Assert; import org.junit.ClassRule; import org.junit.FixMethodOrder; +import org.junit.Rule; import org.junit.Test; import org.junit.rules.RuleChain; import org.junit.rules.TestRule; import org.junit.runners.MethodSorters; +import org.keycloak.OAuth2Constants; import org.keycloak.federation.ldap.LDAPFederationProvider; import org.keycloak.federation.ldap.LDAPFederationProviderFactory; +import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; import org.keycloak.models.UserFederationProvider; import org.keycloak.models.UserFederationProviderModel; import org.keycloak.models.UserModel; +import org.keycloak.protocol.oidc.OIDCLoginProtocolService; +import org.keycloak.protocol.oidc.mappers.UserAttributeMapper; import org.keycloak.services.managers.RealmManager; +import org.keycloak.testsuite.OAuthClient; +import org.keycloak.testsuite.adapter.AdapterTest; +import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.rule.KeycloakRule; import org.keycloak.testsuite.rule.LDAPRule; +import org.keycloak.testsuite.rule.WebResource; +import org.keycloak.testsuite.rule.WebRule; +import org.openqa.selenium.WebDriver; /** * @author Marek Posolda @@ -28,6 +42,9 @@ import org.keycloak.testsuite.rule.LDAPRule; @FixMethodOrder(MethodSorters.NAME_ASCENDING) public class LDAPMultipleAttributesTest { + protected String APP_SERVER_BASE_URL = "http://localhost:8081"; + protected String LOGIN_URL = OIDCLoginProtocolService.authUrl(UriBuilder.fromUri(APP_SERVER_BASE_URL + "/auth")).build("test").toString(); + private static LDAPRule ldapRule = new LDAPRule(); private static UserFederationProviderModel ldapModel = null; @@ -41,6 +58,24 @@ public class LDAPMultipleAttributesTest { ldapModel = appRealm.addUserFederationProvider(LDAPFederationProviderFactory.PROVIDER_NAME, ldapConfig, 0, "test-ldap", -1, -1, 0); FederationTestUtils.addZipCodeLDAPMapper(appRealm, ldapModel); + FederationTestUtils.addUserAttributeMapper(appRealm, ldapModel, "streetMapper", "street", LDAPConstants.STREET); + + // Create ldap-portal client + ClientModel ldapClient = appRealm.addClient("ldap-portal"); + ldapClient.addRedirectUri("/ldap-portal"); + ldapClient.addRedirectUri("/ldap-portal/*"); + ldapClient.setManagementUrl("/ldap-portal"); + ldapClient.addProtocolMapper(UserAttributeMapper.createClaimMapper("postalCode", "postal_code", "postal_code", "String", true, "", true, true, true)); + ldapClient.addProtocolMapper(UserAttributeMapper.createClaimMapper("street", "street", "street", "String", true, "", true, true, false)); + ldapClient.addScopeMapping(appRealm.getRole("user")); + ldapClient.setSecret("password"); + + // Deploy ldap-portal client + URL url = getClass().getResource("/ldap/ldap-app-keycloak.json"); + keycloakRule.createApplicationDeployment() + .name("ldap-portal").contextPath("/ldap-portal") + .servletClass(LDAPExampleServlet.class).adapterConfigPath(url.getPath()) + .role("user").deployApplication(); } }); @@ -49,6 +84,18 @@ public class LDAPMultipleAttributesTest { .outerRule(ldapRule) .around(keycloakRule); + @Rule + public WebRule webRule = new WebRule(this); + + @WebResource + protected WebDriver driver; + + @WebResource + protected OAuthClient oauth; + + @WebResource + protected LoginPage loginPage; + @Test public void testModel() { KeycloakSession session = keycloakRule.startSession(); @@ -105,6 +152,40 @@ public class LDAPMultipleAttributesTest { } } + @Test + public void ldapPortalEndToEndTest() { + // Login as bwilson + driver.navigate().to(APP_SERVER_BASE_URL + "/ldap-portal"); + Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); + loginPage.login("bwilson", "password"); + Assert.assertTrue(driver.getCurrentUrl().startsWith(APP_SERVER_BASE_URL + "/ldap-portal")); + String pageSource = driver.getPageSource(); + System.out.println(pageSource); + Assert.assertTrue(pageSource.contains("bwilson") && pageSource.contains("Bruce")); + Assert.assertTrue(pageSource.contains("street") && pageSource.contains("Elm 5")); + Assert.assertTrue(pageSource.contains("postal_code") && pageSource.contains("88441") && pageSource.contains("77332")); + + // Logout + String logoutUri = OIDCLoginProtocolService.logoutUrl(UriBuilder.fromUri(APP_SERVER_BASE_URL + "/auth")) + .queryParam(OAuth2Constants.REDIRECT_URI, APP_SERVER_BASE_URL + "/ldap-portal").build("test").toString(); + driver.navigate().to(logoutUri); + + // Login as jbrown + driver.navigate().to(APP_SERVER_BASE_URL + "/ldap-portal"); + Assert.assertTrue(driver.getCurrentUrl().startsWith(LOGIN_URL)); + loginPage.login("jbrown", "password"); + Assert.assertTrue(driver.getCurrentUrl().startsWith(APP_SERVER_BASE_URL + "/ldap-portal")); + pageSource = driver.getPageSource(); + System.out.println(pageSource); + Assert.assertTrue(pageSource.contains("jbrown") && pageSource.contains("James Brown")); + Assert.assertFalse(pageSource.contains("street")); + Assert.assertTrue(pageSource.contains("postal_code") && pageSource.contains("88441")); + Assert.assertFalse(pageSource.contains("77332")); + + // Logout + driver.navigate().to(logoutUri); + } + } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java index c92c1624e1..89d353b089 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AccessTokenTest.java @@ -69,7 +69,9 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; import java.io.IOException; import java.net.URI; +import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import static org.hamcrest.Matchers.*; @@ -622,13 +624,16 @@ public class AccessTokenTest { user.setSingleAttribute("postal_code", "02115"); user.setSingleAttribute("country", "USA"); user.setSingleAttribute("phone", "617-777-6666"); + List departments = Arrays.asList("finance", "development"); + user.setAttribute("departments", departments); ClientModel app = realm.getClientByClientId("test-app"); ProtocolMapperModel mapper = AddressMapper.createAddressMapper(true, true); app.addProtocolMapper(mapper); app.addProtocolMapper(HardcodedClaim.create("hard", "hard", "coded", "String", false, null, true, true)); app.addProtocolMapper(HardcodedClaim.create("hard-nested", "nested.hard", "coded-nested", "String", false, null, true, true)); - app.addProtocolMapper(UserAttributeMapper.createClaimMapper("custom phone", "phone", "home_phone", "String", true, "", true, true)); - app.addProtocolMapper(UserAttributeMapper.createClaimMapper("nested phone", "phone", "home.phone", "String", true, "", true, true)); + app.addProtocolMapper(UserAttributeMapper.createClaimMapper("custom phone", "phone", "home_phone", "String", true, "", true, true, false)); + app.addProtocolMapper(UserAttributeMapper.createClaimMapper("nested phone", "phone", "home.phone", "String", true, "", true, true, false)); + app.addProtocolMapper(UserAttributeMapper.createClaimMapper("departments", "departments", "department", "String", true, "", true, true, true)); app.addProtocolMapper(HardcodedRole.create("hard-realm", "hardcoded")); app.addProtocolMapper(HardcodedRole.create("hard-app", "app.hardcoded")); app.addProtocolMapper(RoleNameMapper.create("rename-app-role", "test-app.customer-user", "realm-user")); @@ -655,6 +660,9 @@ public class AccessTokenTest { Assert.assertEquals("coded-nested", nested.get("hard")); nested = (Map)idToken.getOtherClaims().get("home"); Assert.assertEquals("617-777-6666", nested.get("phone")); + List departments = (List)idToken.getOtherClaims().get("department"); + Assert.assertEquals(2, departments.size()); + Assert.assertTrue(departments.contains("finance") && departments.contains("development")); AccessToken accessToken = getAccessToken(tokenResponse); Assert.assertEquals(accessToken.getName(), "Tom Brady"); @@ -671,6 +679,9 @@ public class AccessTokenTest { Assert.assertEquals("coded-nested", nested.get("hard")); nested = (Map)accessToken.getOtherClaims().get("home"); Assert.assertEquals("617-777-6666", nested.get("phone")); + departments = (List)idToken.getOtherClaims().get("department"); + Assert.assertEquals(2, departments.size()); + Assert.assertTrue(departments.contains("finance") && departments.contains("development")); Assert.assertTrue(accessToken.getRealmAccess().getRoles().contains("hardcoded")); Assert.assertTrue(accessToken.getRealmAccess().getRoles().contains("realm-user")); Assert.assertFalse(accessToken.getResourceAccess("test-app").getRoles().contains("customer-user")); diff --git a/testsuite/integration/src/test/resources/ldap/ldap-app-keycloak.json b/testsuite/integration/src/test/resources/ldap/ldap-app-keycloak.json new file mode 100644 index 0000000000..e082558d2f --- /dev/null +++ b/testsuite/integration/src/test/resources/ldap/ldap-app-keycloak.json @@ -0,0 +1,10 @@ +{ + "realm": "test", + "resource": "ldap-portal", + "realm-public-key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQCrVrCuTtArbgaZzL1hvh0xtL5mc7o0NqPVnYXkLvgcwiC3BjLGw1tGEGoJaXDuSaRllobm53JBhjx33UNv+5z/UMG4kytBWxheNVKnL6GgqlNabMaFfPLPCF8kAgKnsi79NMo+n6KnSY8YeUmec/p2vjO2NjsSAVcWEQMVhJ31LwIDAQAB", + "auth-server-url": "http://localhost:8081/auth", + "ssl-required" : "external", + "credentials": { + "secret": "password" + } +} \ No newline at end of file diff --git a/testsuite/integration/src/test/resources/ldap/users.ldif b/testsuite/integration/src/test/resources/ldap/users.ldif index b04f081414..4d6d87ee0c 100644 --- a/testsuite/integration/src/test/resources/ldap/users.ldif +++ b/testsuite/integration/src/test/resources/ldap/users.ldif @@ -29,6 +29,7 @@ cn: James sn: Brown mail: jbrown@keycloak.org postalCode: 88441 +userPassword: password dn: uid=bwilson,ou=People,dc=keycloak,dc=org objectclass: top @@ -42,3 +43,5 @@ sn: Schneider mail: bwilson@keycloak.org postalCode: 88441 postalCode: 77332 +street: Elm 5 +userPassword: password