From 09994d1730e3650b89f7a510380d5b8a9b71d961 Mon Sep 17 00:00:00 2001 From: mposolda Date: Sat, 27 Jun 2015 22:49:40 +0200 Subject: [PATCH] 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",