KEYCLOAK-1571 Error when the value of UUID LDAP attribute is the same of the Username LDAP attribute

federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java

@@ -10,6 +10,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
+import java.util.TreeSet;
 
 import javax.naming.NamingEnumeration;
 import javax.naming.NamingException;
@@ -130,7 +131,7 @@ public class LDAPIdentityStore implements IdentityStore {
                                 .lookupById(baseDN, equalCondition.getValue().toString(), identityQuery.getReturningLdapAttributes());
 
                         if (search != null) {
-                            results.add(populateAttributedType(search, identityQuery.getReturningReadOnlyLdapAttributes()));
+                            results.add(populateAttributedType(search, identityQuery));
                         }
                     }
 
@@ -150,7 +151,7 @@ public class LDAPIdentityStore implements IdentityStore {
 
             for (SearchResult result : search) {
                 if (!result.getNameInNamespace().equalsIgnoreCase(baseDN)) {
-                    results.add(populateAttributedType(result, identityQuery.getReturningReadOnlyLdapAttributes()));
+                    results.add(populateAttributedType(result, identityQuery));
                 }
             }
         } catch (Exception e) {
@@ -371,7 +372,13 @@ public class LDAPIdentityStore implements IdentityStore {
     }
 
 
-    private LDAPObject populateAttributedType(SearchResult searchResult, Collection<String> readOnlyAttrNames) {
+    private LDAPObject populateAttributedType(SearchResult searchResult, LDAPQuery ldapQuery) {
+        Set<String> readOnlyAttrNames = ldapQuery.getReturningReadOnlyLdapAttributes();
+        Set<String> lowerCasedAttrNames = new TreeSet<>();
+        for (String attrName : ldapQuery.getReturningLdapAttributes()) {
+            lowerCasedAttrNames.add(attrName.toLowerCase());
+        }
+
         try {
             String entryDN = searchResult.getNameInNamespace();
             Attributes attributes = searchResult.getAttributes();
@@ -397,7 +404,10 @@ public class LDAPIdentityStore implements IdentityStore {
                 if (ldapAttributeName.equalsIgnoreCase(getConfig().getUuidLDAPAttributeName())) {
                     Object uuidValue = ldapAttribute.get();
                     ldapObject.setUuid(this.operationManager.decodeEntryUUID(uuidValue));
-                } else {
+                }
+
+                // Note: UUID is normally not populated here. It's populated just in case that it's used for name of other attribute as well
+                if (!ldapAttributeName.equalsIgnoreCase(getConfig().getUuidLDAPAttributeName()) || (lowerCasedAttrNames.contains(ldapAttributeName.toLowerCase()))) {
                     Set<String> attrValues = new LinkedHashSet<>();
                     NamingEnumeration<?> enumm = ldapAttribute.getAll();
                     while (enumm.hasMoreElements()) {

federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java

@@ -240,7 +240,7 @@ public class LDAPOperationManager {
 
                 filter = "(&(objectClass=*)(" + getUuidAttributeName() + LDAPConstants.EQUAL + LDAPUtil.convertObjectGUIToByteString(objectGUID) + "))";
             } catch (NamingException ne) {
-                return filter;
+                filter = null;
             }
         }
 
@@ -435,7 +435,7 @@ public class LDAPOperationManager {
     public String decodeEntryUUID(final Object entryUUID) {
         String id;
 
-        if (this.config.isActiveDirectory()) {
+        if (this.config.isActiveDirectory() && entryUUID instanceof byte[]) {
             id = LDAPUtil.decodeObjectGUID((byte[]) entryUUID);
         } else {
             id = entryUUID.toString();

testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java

@@ -7,6 +7,7 @@ import org.junit.Test;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 import org.junit.runners.MethodSorters;
+import org.keycloak.federation.ldap.LDAPConfig;
 import org.keycloak.federation.ldap.LDAPFederationProvider;
 import org.keycloak.federation.ldap.LDAPFederationProviderFactory;
 import org.keycloak.federation.ldap.idm.model.LDAPObject;
@@ -17,9 +18,12 @@ import org.keycloak.models.RealmModel;
 import org.keycloak.models.UserFederationProvider;
 import org.keycloak.models.UserFederationProviderModel;
 import org.keycloak.models.UserFederationSyncResult;
+import org.keycloak.models.UserModel;
 import org.keycloak.models.UserProvider;
+import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.services.managers.RealmManager;
 import org.keycloak.services.managers.UsersSyncManager;
+import org.keycloak.testsuite.rule.AbstractKeycloakRule;
 import org.keycloak.testsuite.rule.KeycloakRule;
 import org.keycloak.testsuite.rule.LDAPRule;
 import org.keycloak.testsuite.DummyUserFederationProviderFactory;
@@ -213,6 +217,68 @@ public class SyncProvidersTest {
         }
     }
 
+    // KEYCLOAK-1571
+    @Test
+    public void test03SameUUIDAndUsernameSync() {
+        KeycloakSession session = keycloakRule.startSession();
+        String origUuidAttrName;
+
+        try {
+            RealmModel testRealm = session.realms().getRealm("test");
+
+            // Remove all users from model
+            for (UserModel user : session.userStorage().getUsers(testRealm)) {
+                session.userStorage().removeUser(testRealm, user);
+            }
+
+            UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
+
+            // Change name of UUID attribute to same like usernameAttribute
+            LDAPFederationProvider ldapFedProvider = FederationTestUtils.getLdapProvider(session, ldapModel);
+            String uidAttrName = ldapFedProvider.getLdapIdentityStore().getConfig().getUsernameLdapAttribute();
+            origUuidAttrName = providerModel.getConfig().get(LDAPConstants.UUID_LDAP_ATTRIBUTE);
+            providerModel.getConfig().put(LDAPConstants.UUID_LDAP_ATTRIBUTE, uidAttrName);
+
+            // Need to change this due to ApacheDS pagination bug (For other LDAP servers, pagination works fine) TODO: Remove once ApacheDS upgraded and pagination is fixed
+            providerModel.getConfig().put(LDAPConstants.BATCH_SIZE_FOR_SYNC, "10");
+            testRealm.updateUserFederationProvider(providerModel);
+
+        } finally {
+            keycloakRule.stopSession(session, true);
+        }
+
+        session = keycloakRule.startSession();
+        try {
+            RealmModel testRealm = session.realms().getRealm("test");
+            UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
+
+            KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
+            UserFederationSyncResult syncResult = new UsersSyncManager().syncAllUsers(sessionFactory, "test", providerModel);
+            Assert.assertEquals(0, syncResult.getFailed());
+
+        } finally {
+            keycloakRule.stopSession(session, false);
+        }
+
+        session = keycloakRule.startSession();
+        try {
+            RealmModel testRealm = session.realms().getRealm("test");
+
+            // Assert users imported with correct LDAP_ID
+            FederationTestUtils.assertUserImported(session.users(), testRealm, "user1", "User1FN", "User1LN", "user1@email.org", "121");
+            FederationTestUtils.assertUserImported(session.users(), testRealm, "user2", "User2FN", "User2LN", "user2@email.org", "122");
+            UserModel user1 = session.users().getUserByUsername("user1", testRealm);
+            Assert.assertEquals("user1",  user1.getFirstAttribute(LDAPConstants.LDAP_ID));
+
+            // Revert config changes
+            UserFederationProviderModel providerModel = KeycloakModelUtils.findUserFederationProviderByDisplayName(ldapModel.getDisplayName(), testRealm);
+            providerModel.getConfig().put(LDAPConstants.UUID_LDAP_ATTRIBUTE, origUuidAttrName);
+            testRealm.updateUserFederationProvider(providerModel);
+        } finally {
+            keycloakRule.stopSession(session, true);
+        }
+    }
+
     @Test
     public void testPeriodicSync() {
         KeycloakSession session = keycloakRule.startSession();