diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java index dea8cab938f..42a46839831 100755 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java @@ -93,7 +93,6 @@ import org.keycloak.storage.user.ImportedUserValidation; import org.keycloak.storage.user.UserLookupProvider; import org.keycloak.storage.user.UserQueryMethodsProvider; import org.keycloak.storage.user.UserRegistrationProvider; -import org.keycloak.userprofile.AttributeContext; import org.keycloak.userprofile.AttributeGroupMetadata; import org.keycloak.userprofile.AttributeMetadata; import org.keycloak.userprofile.UserProfileDecorator; @@ -296,7 +295,15 @@ public class LDAPStorageProvider implements UserStorageProvider, } } - return ldapObjects.stream().map(ldapUser -> importUserFromLDAP(session, realm, ldapUser)); + return ldapObjects.stream().map(ldapUser -> { + String ldapUsername = LDAPUtils.getUsername(ldapUser, this.ldapIdentityStore.getConfig()); + UserModel localUser = UserStoragePrivateUtil.userLocalStorage(session).getUserByUsername(realm, ldapUsername); + if (localUser == null) { + return importUserFromLDAP(session, realm, ldapUser); + } else { + return proxy(realm, localUser, ldapUser, false); + } + }); } public boolean synchronizeRegistrations() { @@ -565,16 +572,16 @@ public class LDAPStorageProvider implements UserStorageProvider, } /** - * Searches LDAP using logical disjunction of params. It supports + * Searches LDAP using logical disjunction of params. It supports * - * + * * It uses multiple LDAP calls and results are combined together with respect to firstResult and maxResults - * + * * This method serves for {@code search} param of {@link org.keycloak.services.resources.admin.UsersResource#getUsers} */ private Stream searchLDAP(RealmModel realm, String search, Integer firstResult, Integer maxResults) { @@ -1044,11 +1051,11 @@ public class LDAPStorageProvider implements UserStorageProvider, /** * This method leverages existing pagination support in {@link LDAPQuery#getResultList()}. It sets the limit for the query * based on {@code firstResult}, {@code maxResults} and {@link LDAPConfig#getBatchSizeForSync()}. - * + * *

- * Internally it uses {@link Stream#iterate(java.lang.Object, java.util.function.Predicate, java.util.function.UnaryOperator)} - * to ensure there will be obtained required number of users considering a fact that some of the returned ldap users could be - * filtered out (as they might be already imported in local storage). The returned {@code Stream} will be filled + * Internally it uses {@link Stream#iterate(java.lang.Object, java.util.function.Predicate, java.util.function.UnaryOperator)} + * to ensure there will be obtained required number of users considering a fact that some of the returned ldap users could be + * filtered out (as they might be already imported in local storage). The returned {@code Stream} will be filled * "on demand". */ private Stream paginatedSearchLDAP(LDAPQuery ldapQuery, Integer firstResult, Integer maxResults) { @@ -1071,7 +1078,7 @@ public class LDAPStorageProvider implements UserStorageProvider, } } - return Stream.iterate(ldapQuery, + return Stream.iterate(ldapQuery, query -> { //the very 1st page - Pagination context might not yet be present if (query.getPaginationContext() == null) try { @@ -1082,7 +1089,7 @@ public class LDAPStorageProvider implements UserStorageProvider, throw new ModelException("Querying of LDAP failed " + query, e); } return query.getPaginationContext().hasNextPage(); - }, + }, query -> query ).flatMap(query -> { query.setLimit(limit); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSearchForUsersPaginationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSearchForUsersPaginationTest.java index 5851b9c3f50..7c9f1645e2c 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSearchForUsersPaginationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPSearchForUsersPaginationTest.java @@ -37,7 +37,11 @@ import org.junit.Test; import org.junit.runners.MethodSorters; import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; +import org.keycloak.models.UserModel; +import org.keycloak.models.UserProvider; import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.storage.DatastoreProvider; +import org.keycloak.storage.datastore.DefaultDatastoreProvider; import org.keycloak.testsuite.util.LDAPRule; import org.keycloak.testsuite.util.LDAPTestUtils; @@ -180,6 +184,48 @@ public class LDAPSearchForUsersPaginationTest extends AbstractLDAPTest { Assert.assertEquals(Set.of("john"), usernames); } + @Test + public void testSearchByUserAttributeDoesNotTriggerUserReimport() { + + testingClient.server().run(session -> { + // add a new user for testing that searching by attributes should not cause the user to be re-imported. + LDAPTestContext ctx = LDAPTestContext.init(session); + RealmModel appRealm = ctx.getRealm(); + LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "bwayne", "Bruce", "Wayne", "bwayne@waynecorp.com", "Gotham Avenue", "666"); + }); + + testingClient.server(TEST_REALM_NAME).run(session -> { + // check the user doesn't yet exist in Keycloak + UserProvider localProvider = ((DefaultDatastoreProvider) session.getProvider(DatastoreProvider.class)).userLocalStorage(); + UserModel user = localProvider.getUserByUsername(session.getContext().getRealm(), "bwayne"); + Assert.assertNull(user); + + // import the user by searching for its username, and check it has the timestamp set by one of the LDAP mappers. + user = session.users().getUserByUsername(session.getContext().getRealm(), "bwayne"); + Assert.assertNotNull(user); + Assert.assertNotNull(user.getAttributes().get("createTimestamp")); + + // remove the create timestamp from the user. + user.removeAttribute("createTimestamp"); + user = localProvider.getUserByUsername(session.getContext().getRealm(), "bwayne"); + Assert.assertNull(user.getAttributes().get("createTimestamp")); + }); + + testingClient.server(TEST_REALM_NAME).run(session -> { + // search users by user attribute - the existing user SHOULD NOT be re-imported (GHI #32870) + List users = session.users().searchForUserByUserAttributeStream(session.getContext().getRealm(), "street", "Gotham Avenue").collect(Collectors.toList()); + Assert.assertEquals(1, users.size()); + UserModel user = users.get(0); + // create timestamp won't be null because it is provided directly from the LDAP mapper, so it should still be visible. + Assert.assertNotNull(user.getAttributes().get("createTimestamp")); + + // however, the local stored attribute should not have been updated (i.e. user should not have been fully re-imported). + UserProvider localProvider = ((DefaultDatastoreProvider) session.getProvider(DatastoreProvider.class)).userLocalStorage(); + user = localProvider.getUserByUsername(session.getContext().getRealm(), "bwayne"); + Assert.assertNull(user.getAttributes().get("createTimestamp")); + }); + } + private void assertLDAPSearchMatchesLocalDB(String searchString) { //this call should import some users into local database List importedUsers = adminClient.realm(TEST_REALM_NAME).users().search(searchString, null, null).stream().map(UserRepresentation::getUsername).collect(Collectors.toList());