diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java index 141ae387d7..43057d1d5c 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPIdentityStore.java +++ b/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 readOnlyAttrNames) { + private LDAPObject populateAttributedType(SearchResult searchResult, LDAPQuery ldapQuery) { + Set readOnlyAttrNames = ldapQuery.getReturningReadOnlyLdapAttributes(); + Set 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 attrValues = new LinkedHashSet<>(); NamingEnumeration enumm = ldapAttribute.getAll(); while (enumm.hasMoreElements()) { diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java index 18b8a8673a..3e28dfe767 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java +++ b/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(); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java index c13c9bc250..fd1c28dbb7 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/SyncProvidersTest.java +++ b/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();