libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Avoid returning the same entity multiple times from separate searches

Browse source 
Closes #15604
This commit is contained in:
Alexander Schwartz 2023-02-28 11:40:52 +01:00 committed by Hynek Mlnařík
parent b28bde542f
commit 1e4401f521
3 changed files with 51 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPStorageProvider.java
View file

@ -436,7 +436,12 @@ public class LDAPStorageProvider implements UserStorageProvider,
protected List<LDAPObject> searchLDAP(RealmModel realm, Map<String, String> attributes) {
List<LDAPObject> results = new ArrayList<LDAPObject>();
// return a stable ordered result to the caller
List<LDAPObject> results = new ArrayList<>();
// a set to ensure fast uniqueness checks based on equals/hashCode of LDAPObject
Set<LDAPObject> unique = new HashSet<>();
if (attributes.containsKey(UserModel.USERNAME)) {
try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) {
LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
@ -447,6 +452,7 @@ public class LDAPStorageProvider implements UserStorageProvider,
List<LDAPObject> ldapObjects = ldapQuery.getResultList();
results.addAll(ldapObjects);
unique.addAll(ldapObjects);
}
}
@ -459,7 +465,13 @@ public class LDAPStorageProvider implements UserStorageProvider,
ldapQuery.addWhereCondition(emailCondition);
List<LDAPObject> ldapObjects = ldapQuery.getResultList();
results.addAll(ldapObjects);
ldapObjects.forEach(ldapObject -> {
// ensure that no entity is listed twice and still preserve the order of returned entities
if (!unique.contains(ldapObject)) {
results.add(ldapObject);
unique.add(ldapObject);
}
});
}
}
@ -476,7 +488,13 @@ public class LDAPStorageProvider implements UserStorageProvider,
}
List<LDAPObject> ldapObjects = ldapQuery.getResultList();
results.addAll(ldapObjects);
ldapObjects.forEach(ldapObject -> {
// ensure that no entity is listed twice and still preserve the order of returned entities
if (!unique.contains(ldapObject)) {
results.add(ldapObject);
unique.add(ldapObject);
}
});
}
}

13
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java
View file

@ -997,6 +997,7 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest {
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "username2", "John2", "Doel2", "user2@email.org", null, "122");
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "username3", "John3", "Doel3", "user3@email.org", null, "123");
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "username4", "John4", "Doel4", "user4@email.org", null, "124");
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "username11", "John11", "Doel11", "user11@email.org", null, "124");
// Users are not at local store at this moment
Assert.assertNull(UserStoragePrivateUtil.userLocalStorage(session).getUserByUsername(appRealm, "username1"));
@ -1005,20 +1006,24 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest {
Assert.assertNull(UserStoragePrivateUtil.userLocalStorage(session).getUserByUsername(appRealm, "username4"));
// search by username (we use a terminal operation on the stream to ensure it is consumed)
session.users().searchForUserStream(appRealm, "username1").count();
Assert.assertEquals(1, session.users().searchForUserStream(appRealm, "username1").count());
LDAPTestAsserts.assertUserImported(UserStoragePrivateUtil.userLocalStorage(session), appRealm, "username1", "John1", "Doel1", "user1@email.org", "121");
// search by email (we use a terminal operation on the stream to ensure it is consumed)
session.users().searchForUserStream(appRealm, "user2@email.org").count();
Assert.assertEquals(1, session.users().searchForUserStream(appRealm, "user2@email.org").count());
LDAPTestAsserts.assertUserImported(UserStoragePrivateUtil.userLocalStorage(session), appRealm, "username2", "John2", "Doel2", "user2@email.org", "122");
// search by lastName (we use a terminal operation on the stream to ensure it is consumed)
session.users().searchForUserStream(appRealm, "Doel3").count();
Assert.assertEquals(1, session.users().searchForUserStream(appRealm, "Doel3").count());
LDAPTestAsserts.assertUserImported(UserStoragePrivateUtil.userLocalStorage(session), appRealm, "username3", "John3", "Doel3", "user3@email.org", "123");
// search by firstName + lastName (we use a terminal operation on the stream to ensure it is consumed)
session.users().searchForUserStream(appRealm, "John4 Doel4").count();
Assert.assertEquals(1, session.users().searchForUserStream(appRealm, "John4 Doel4").count());
LDAPTestAsserts.assertUserImported(UserStoragePrivateUtil.userLocalStorage(session), appRealm, "username4", "John4", "Doel4", "user4@email.org", "124");
// search by a string that matches multiple fields. Should still return the one entity it matches.
Assert.assertEquals(1, session.users().searchForUserStream(appRealm, "*11*").count());
LDAPTestAsserts.assertUserImported(UserStoragePrivateUtil.userLocalStorage(session), appRealm, "username11", "John11", "Doel11", "user11@email.org", "124");
});
}

25
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/noimport/LDAPProvidersIntegrationNoImportTest.java
View file

@ -18,6 +18,8 @@
package org.keycloak.testsuite.federation.ldap.noimport;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;
import javax.ws.rs.BadRequestException;
import javax.ws.rs.core.Response;
@ -96,22 +98,37 @@ public class LDAPProvidersIntegrationNoImportTest extends LDAPProvidersIntegrati
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "username2", "John2", "Doel2", "user2@email.org", null, "122");
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "username3", "John3", "Doel3", "user3@email.org", null, "123");
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "username4", "John4", "Doel4", "user4@email.org", null, "124");
LDAPTestUtils.addLDAPUser(ctx.getLdapProvider(), appRealm, "username8", "John8", "Doel8", "user8@email.org", null, "124");
// search by username
UserModel user = session.users().searchForUserStream(appRealm, "username1").findFirst().get();
List<UserModel> users = session.users().searchForUserStream(appRealm, "username1").collect(Collectors.toList());
Assert.assertEquals(1, users.size());
UserModel user = users.get(0);
LDAPTestAsserts.assertLoaded(user, "username1", "John1", "Doel1", "user1@email.org", "121");
// search by email
user = session.users().searchForUserStream(appRealm, "user2@email.org").findFirst().get();
users = session.users().searchForUserStream(appRealm, "user2@email.org").collect(Collectors.toList());
Assert.assertEquals(1, users.size());
user = users.get(0);
LDAPTestAsserts.assertLoaded(user, "username2", "John2", "Doel2", "user2@email.org", "122");
// search by lastName
user = session.users().searchForUserStream(appRealm, "Doel3").findFirst().get();
users = session.users().searchForUserStream(appRealm, "Doel3").collect(Collectors.toList());
Assert.assertEquals(1, users.size());
user = users.get(0);
LDAPTestAsserts.assertLoaded(user, "username3", "John3", "Doel3", "user3@email.org", "123");
// search by firstName + lastName
user = session.users().searchForUserStream(appRealm, "John4 Doel4").findFirst().get();
users = session.users().searchForUserStream(appRealm, "John4 Doel4").collect(Collectors.toList());
Assert.assertEquals(1, users.size());
user = users.get(0);
LDAPTestAsserts.assertLoaded(user, "username4", "John4", "Doel4", "user4@email.org", "124");
// search by a string that matches multiple fields. Should still return the one entity it matches
users = session.users().searchForUserStream(appRealm, "*8*").collect(Collectors.toList());
Assert.assertEquals(1, users.size());
user = users.get(0);
LDAPTestAsserts.assertLoaded(user, "username8", "John8", "Doel8", "user8@email.org", "124");
});
}