diff --git a/audit/api/src/main/java/org/keycloak/audit/Errors.java b/audit/api/src/main/java/org/keycloak/audit/Errors.java index 04be394937..d6a5be2396 100755 --- a/audit/api/src/main/java/org/keycloak/audit/Errors.java +++ b/audit/api/src/main/java/org/keycloak/audit/Errors.java @@ -19,6 +19,7 @@ public interface Errors { String USERNAME_MISSING = "username_missing"; String USERNAME_IN_USE = "username_in_use"; + String EMAIL_IN_USE = "email_in_use"; String INVALID_REDIRECT_URI = "invalid_redirect_uri"; String INVALID_CODE = "invalid_code"; 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 1813e8ab1a..eed99122cc 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 @@ -162,7 +162,6 @@ public class LDAPFederationProvider implements UserFederationProvider { User user = BasicModel.getUser(identityManager, attributes.get(USERNAME)); if (user != null) { results.put(user.getLoginName(), user); - return results; } } @@ -170,7 +169,6 @@ public class LDAPFederationProvider implements UserFederationProvider { User user = queryByEmail(identityManager, attributes.get(EMAIL)); if (user != null) { results.put(user.getLoginName(), user); - return results; } } @@ -236,16 +234,7 @@ public class LDAPFederationProvider implements UserFederationProvider { } protected User queryByEmail(IdentityManager identityManager, String email) throws IdentityManagementException { - List agents = identityManager.createIdentityQuery(User.class) - .setParameter(User.EMAIL, email).getResultList(); - - if (agents.isEmpty()) { - return null; - } else if (agents.size() == 1) { - return agents.get(0); - } else { - throw new IdentityManagementException("Error - multiple Agent objects found with same email"); - } + return LDAPUtils.getUserByEmail(identityManager, email); } 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 f862d03ef2..51c5529d7a 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 @@ -1,5 +1,7 @@ package org.keycloak.federation.ldap; +import org.keycloak.models.ModelDuplicateException; +import org.picketlink.idm.IdentityManagementException; import org.picketlink.idm.IdentityManager; import org.picketlink.idm.PartitionManager; import org.picketlink.idm.credential.Credentials; @@ -19,13 +21,21 @@ import java.util.List; public class LDAPUtils { public static User addUser(PartitionManager partitionManager, String username, String firstName, String lastName, String email) { - IdentityManager idmManager = getIdentityManager(partitionManager); + IdentityManager identityManager = getIdentityManager(partitionManager); + + if (BasicModel.getUser(identityManager, username) != null) { + throw new ModelDuplicateException("User with same username already exists"); + } + if (getUserByEmail(identityManager, email) != null) { + throw new ModelDuplicateException("User with same email already exists"); + } + User picketlinkUser = new User(username); picketlinkUser.setFirstName(firstName); picketlinkUser.setLastName(lastName); picketlinkUser.setEmail(email); picketlinkUser.setAttribute(new Attribute("fullName", getFullName(username, firstName, lastName))); - idmManager.add(picketlinkUser); + identityManager.add(picketlinkUser); return picketlinkUser; } @@ -64,6 +74,20 @@ public class LDAPUtils { return BasicModel.getUser(idmManager, username); } + + public static User getUserByEmail(IdentityManager idmManager, String email) throws IdentityManagementException { + List agents = idmManager.createIdentityQuery(User.class) + .setParameter(User.EMAIL, email).getResultList(); + + if (agents.isEmpty()) { + return null; + } else if (agents.size() == 1) { + return agents.get(0); + } else { + throw new IdentityManagementException("Error - multiple users found with same email"); + } + } + public static boolean removeUser(PartitionManager partitionManager, String username) { IdentityManager idmManager = getIdentityManager(partitionManager); User picketlinkUser = BasicModel.getUser(idmManager, username); diff --git a/forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties b/forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties index a1df56a075..a171b1aeb1 100755 --- a/forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties +++ b/forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties @@ -50,6 +50,7 @@ successTotp=Google authenticator configured. successTotpRemoved=Google authenticator removed. usernameExists=Username already exists +emailExists=Email already exists socialEmailExists=User with email already exists. Please login to account management to link the account. diff --git a/picketlink/keycloak-picketlink-ldap/src/main/java/org/keycloak/picketlink/idm/LDAPKeycloakCredentialHandler.java b/picketlink/keycloak-picketlink-ldap/src/main/java/org/keycloak/picketlink/idm/LDAPKeycloakCredentialHandler.java index 7844e8e19f..dea12085e3 100644 --- a/picketlink/keycloak-picketlink-ldap/src/main/java/org/keycloak/picketlink/idm/LDAPKeycloakCredentialHandler.java +++ b/picketlink/keycloak-picketlink-ldap/src/main/java/org/keycloak/picketlink/idm/LDAPKeycloakCredentialHandler.java @@ -1,7 +1,15 @@ package org.keycloak.picketlink.idm; +import javax.naming.directory.SearchResult; + import org.picketlink.idm.IdentityManager; +import org.picketlink.idm.config.LDAPMappingConfiguration; +import org.picketlink.idm.credential.UsernamePasswordCredentials; +import org.picketlink.idm.credential.storage.CredentialStorage; +import org.picketlink.idm.ldap.internal.LDAPIdentityStore; +import org.picketlink.idm.ldap.internal.LDAPOperationManager; import org.picketlink.idm.ldap.internal.LDAPPlainTextPasswordCredentialHandler; +import org.picketlink.idm.model.Account; import org.picketlink.idm.model.basic.BasicModel; import org.picketlink.idm.model.basic.User; import org.picketlink.idm.spi.IdentityContext; @@ -24,4 +32,33 @@ public class LDAPKeycloakCredentialHandler extends LDAPPlainTextPasswordCredenti return BasicModel.getUser(identityManager, loginName); } + + + @Override + protected boolean validateCredential(IdentityContext context, CredentialStorage credentialStorage, UsernamePasswordCredentials credentials, LDAPIdentityStore ldapIdentityStore) { + Account account = getAccount(context, credentials.getUsername()); + char[] password = credentials.getPassword().getValue(); + String userDN = getDNOfUser(ldapIdentityStore, account); + if (CREDENTIAL_LOGGER.isDebugEnabled()) { + CREDENTIAL_LOGGER.debugf("Using DN [%s] for authentication of user [%s]", userDN, credentials.getUsername()); + } + + if (ldapIdentityStore.getOperationManager().authenticate(userDN, new String(password))) { + return true; + } + + return false; + } + + protected String getDNOfUser(LDAPIdentityStore ldapIdentityStore, Account user) { + LDAPMappingConfiguration userMappingConfig = ldapIdentityStore.getConfig().getMappingConfig(User.class); + SearchResult sr = ldapIdentityStore.getOperationManager().lookupById(userMappingConfig.getBaseDN(), user.getId(), userMappingConfig); + + if (sr != null) { + return sr.getNameInNamespace(); + } else { + // Fallback + return ldapIdentityStore.getBindingDN(user, true); + } + } } diff --git a/pom.xml b/pom.xml index 07cb8cb0b9..b78caca039 100755 --- a/pom.xml +++ b/pom.xml @@ -18,7 +18,7 @@ 2.3.7.Final 3.0.8.Final 1.0.15.Final - 2.7.0.Beta1-20140731 + 2.7.0.Beta1 1.0.2.Final 2.11.3 3.1.4.GA diff --git a/services/src/main/java/org/keycloak/services/messages/Messages.java b/services/src/main/java/org/keycloak/services/messages/Messages.java index 740d971e65..329d12a08e 100755 --- a/services/src/main/java/org/keycloak/services/messages/Messages.java +++ b/services/src/main/java/org/keycloak/services/messages/Messages.java @@ -59,6 +59,8 @@ public class Messages { public static final String USERNAME_EXISTS = "usernameExists"; + public static final String EMAIL_EXISTS = "emailExists"; + public static final String ACTION_WARN_TOTP = "actionTotpWarning"; public static final String ACTION_WARN_PROFILE = "actionProfileWarning"; diff --git a/services/src/main/java/org/keycloak/services/resources/TokenService.java b/services/src/main/java/org/keycloak/services/resources/TokenService.java index c247dc6168..0907b16495 100755 --- a/services/src/main/java/org/keycloak/services/resources/TokenService.java +++ b/services/src/main/java/org/keycloak/services/resources/TokenService.java @@ -635,12 +635,18 @@ public class TokenService { return Flows.forms(session, realm, client, uriInfo).setError(error).setFormData(formData).createRegistration(); } - // Validate that user with this username doesn't exist in realm or any authentication provider + // Validate that user with this username doesn't exist in realm or any federation provider if (session.users().getUserByUsername(username, realm) != null) { audit.error(Errors.USERNAME_IN_USE); return Flows.forms(session, realm, client, uriInfo).setError(Messages.USERNAME_EXISTS).setFormData(formData).createRegistration(); } + // Validate that user with this email doesn't exist in realm or any federation provider + if (session.users().getUserByEmail(email, realm) != null) { + audit.error(Errors.EMAIL_IN_USE); + return Flows.forms(session, realm, client, uriInfo).setError(Messages.EMAIL_EXISTS).setFormData(formData).createRegistration(); + } + UserModel user = session.users().addUser(realm, username); user.setEnabled(true); user.setFirstName(formData.getFirst("firstName")); 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 13de4df0c4..8f0b0bf93a 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 @@ -20,6 +20,7 @@ import org.keycloak.models.SocialLinkModel; import org.keycloak.models.UserCredentialModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; +import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.RepresentationToModel; import org.keycloak.representations.adapters.action.UserStats; @@ -136,6 +137,14 @@ public class UsersResource { public Response createUser(final @Context UriInfo uriInfo, final UserRepresentation rep) { auth.requireManage(); + // Double-check duplicated username and email here due to federation + if (session.users().getUserByUsername(rep.getUsername(), realm) != null) { + return Flows.errors().exists("User exists with same username"); + } + if (session.users().getUserByEmail(rep.getEmail(), realm) != null) { + return Flows.errors().exists("User exists with same email"); + } + try { UserModel user = session.users().addUser(realm, rep.getUsername()); updateUserFromRep(user, rep); @@ -146,6 +155,9 @@ public class UsersResource { return Response.created(uriInfo.getAbsolutePathBuilder().path(user.getUsername()).build()).build(); } catch (ModelDuplicateException e) { + if (session.getTransaction().isActive()) { + session.getTransaction().setRollbackOnly(); + } return Flows.errors().exists("User exists with same username or email"); } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java index 3caae8bc06..868092bbba 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java @@ -209,10 +209,15 @@ public class FederationProvidersIntegrationTest { loginPage.clickRegister(); registerPage.assertCurrent(); + // check existing username registerPage.register("firstName", "lastName", "email", "existing", "password", "password"); - registerPage.assertCurrent(); Assert.assertEquals("Username already exists", registerPage.getError()); + + // Check existing email + registerPage.register("firstName", "lastName", "existing@email.org", "nonExisting", "password", "password"); + registerPage.assertCurrent(); + Assert.assertEquals("Email already exists", registerPage.getError()); } @Test