Merge pull request #650 from mposolda/master

More Federation & LDAP fixes

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";

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)
+        return LDAPUtils.getUserByEmail(identityManager, email);
-                .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");
-        }
     }
 
 

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);

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.
 

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);
+        }
+    }
 }

pom.xml

@@ -18,7 +18,7 @@
         <resteasy.version>2.3.7.Final</resteasy.version>
         <resteasy.version.latest>3.0.8.Final</resteasy.version.latest>
         <undertow.version>1.0.15.Final</undertow.version>
-        <picketlink.version>2.7.0.Beta1-20140731</picketlink.version>
+        <picketlink.version>2.7.0.Beta1</picketlink.version>
         <picketbox.ldap.version>1.0.2.Final</picketbox.ldap.version>
         <mongo.driver.version>2.11.3</mongo.driver.version>
         <jboss.logging.version>3.1.4.GA</jboss.logging.version>

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";

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"));

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");
         }
     }

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