Merge pull request #824 from mposolda/master

KEYCLOAK-808 Issue when DB search is case-sensitive byt LDAP search is n...

federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPFederationProvider.java

@@ -211,6 +211,12 @@ public class LDAPFederationProvider implements UserFederationProvider {
                 return null;
             }
 
+            // KEYCLOAK-808: Should we allow case-sensitivity to be configurable?
+            if (!username.equals(picketlinkUser.getLoginName())) {
+                logger.warnf("User found in LDAP but with different username. LDAP username: %s, Searched username: %s", username, picketlinkUser.getLoginName());
+                return null;
+            }
+
             return importUserFromPicketlink(realm, picketlinkUser);
         } catch (IdentityManagementException ie) {
             throw convertIDMException(ie);
@@ -223,6 +229,11 @@ public class LDAPFederationProvider implements UserFederationProvider {
 
     protected UserModel importUserFromPicketlink(RealmModel realm, User picketlinkUser) {
         String email = (picketlinkUser.getEmail() != null && picketlinkUser.getEmail().trim().length() > 0) ? picketlinkUser.getEmail() : null;
+
+        if (picketlinkUser.getLoginName() == null) {
+            throw new ModelException("User returned from LDAP has null username! Check configuration of your LDAP mappings. ID of user from LDAP: " + picketlinkUser.getId());
+        }
+
         UserModel imported = session.userStorage().addUser(realm, picketlinkUser.getLoginName());
         imported.setEnabled(true);
         imported.setEmail(email);
@@ -247,6 +258,13 @@ public class LDAPFederationProvider implements UserFederationProvider {
             if (picketlinkUser == null) {
                 return null;
             }
+
+            // KEYCLOAK-808: Should we allow case-sensitivity to be configurable?
+            if (!email.equals(picketlinkUser.getEmail())) {
+                logger.warnf("User found in LDAP but with different email. LDAP email: %s, Searched email: %s", email, picketlinkUser.getEmail());
+                return null;
+            }
+
             return importUserFromPicketlink(realm, picketlinkUser);
         } catch (IdentityManagementException ie) {
             throw convertIDMException(ie);

testsuite/integration/src/test/java/org/keycloak/testsuite/forms/FederationProvidersIntegrationTest.java

@@ -147,6 +147,15 @@ public class FederationProvidersIntegrationTest {
         Assert.assertEquals("john@email.org", profilePage.getEmail());
     }
 
+    @Test
+    public void loginLdapWithEmail() {
+        loginPage.open();
+        loginPage.login("john@email.org", "Password1");
+
+        Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+        Assert.assertNotNull(oauth.getCurrentQuery().get(OAuth2Constants.CODE));
+    }
+
     @Test
     public void XdeleteLink() {
         loginLdap();
@@ -200,6 +209,11 @@ public class FederationProvidersIntegrationTest {
         loginPage.open();
         loginPage.login("johnkeycloak", "New-password1");
         Assert.assertEquals(AppPage.RequestType.AUTH_RESPONSE, appPage.getRequestType());
+
+        // Change password back to previous value
+        changePasswordPage.open();
+        changePasswordPage.changePassword("New-password1", "Password1", "Password1");
+        Assert.assertEquals("Your password has been updated", profilePage.getSuccess());
     }
 
     @Test
@@ -370,7 +384,7 @@ public class FederationProvidersIntegrationTest {
             Assert.assertTrue(session.users().validCredentials(appRealm, user, cred));
 
             // LDAP password is still unchanged
-            Assert.assertTrue(LDAPUtils.validatePassword(getPartitionManager(session, model), "johnkeycloak", "New-password1"));
+            Assert.assertTrue(LDAPUtils.validatePassword(getPartitionManager(session, model), "johnkeycloak", "Password1"));
 
             // ATM it's not permitted to delete user in unsynced mode. Should be user deleted just locally instead?
             Assert.assertFalse(session.users().removeUser(appRealm, user));
@@ -387,6 +401,18 @@ public class FederationProvidersIntegrationTest {
         }
     }
 
+    @Test
+    public void testCaseSensitiveSearch() {
+        loginPage.open();
+
+        // This should fail for now due to case-sensitivity
+        loginPage.login("johnKeycloak", "Password1");
+        Assert.assertEquals("Invalid username or password.", loginPage.getError());
+
+        loginPage.login("John@email.org", "Password1");
+        Assert.assertEquals("Invalid username or password.", loginPage.getError());
+    }
+
     static PartitionManager getPartitionManager(KeycloakSession keycloakSession, UserFederationProviderModel ldapFedModel) {
         PartitionManagerProvider partitionManagerProvider = keycloakSession.getProvider(PartitionManagerProvider.class);
         return partitionManagerProvider.getPartitionManager(ldapFedModel);