keycloak-memoizeit

KEYCLOAK-643 Handle flows with adding user with existing username

8/27/2014 5:37:06 AM

Details

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 04be394..d6a5be2 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 1813e8a..eed9912 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<User> 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 f862d03..51c5529 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<User> 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 a1df56a..a171b1a 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/services/src/main/java/org/keycloak/services/messages/Messages.java b/services/src/main/java/org/keycloak/services/messages/Messages.java
index 740d971..329d12a 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/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
index 13de4df..8f0b0bf 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/services/src/main/java/org/keycloak/services/resources/TokenService.java b/services/src/main/java/org/keycloak/services/resources/TokenService.java
index c247dc6..0907b16 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/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java
index 3caae8b..868092b 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