User search with LDAP federation not consistent

Closes #10195
This commit is contained in:
vramik 2023-05-16 17:38:15 +02:00 committed by Hynek Mlnařík
parent 588265e463
commit bdbbd2959d
4 changed files with 61 additions and 16 deletions

View file

@ -497,15 +497,23 @@ public class LDAPStorageProvider implements UserStorageProvider,
try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) { try (LDAPQuery ldapQuery = LDAPUtils.createQueryForUserSearch(this, realm)) {
LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder(); LDAPQueryConditionsBuilder conditionsBuilder = new LDAPQueryConditionsBuilder();
List<Condition> conditions = new LinkedList<>();
for (String s : search.split("\\s+")) { for (String s : search.split("\\s+")) {
List<Condition> conditions = new LinkedList<>();
if (s.startsWith("\"") && s.endsWith("\"")) {
// exact search
s = s.substring(1, s.length() - 1);
} else if (!s.endsWith("*")) {
// default to prefix search
s += "*";
}
conditions.add(conditionsBuilder.equal(UserModel.USERNAME, s.trim().toLowerCase(), EscapeStrategy.NON_ASCII_CHARS_ONLY)); conditions.add(conditionsBuilder.equal(UserModel.USERNAME, s.trim().toLowerCase(), EscapeStrategy.NON_ASCII_CHARS_ONLY));
conditions.add(conditionsBuilder.equal(UserModel.EMAIL, s.trim().toLowerCase(), EscapeStrategy.NON_ASCII_CHARS_ONLY)); conditions.add(conditionsBuilder.equal(UserModel.EMAIL, s.trim().toLowerCase(), EscapeStrategy.NON_ASCII_CHARS_ONLY));
conditions.add(conditionsBuilder.equal(UserModel.FIRST_NAME, s, EscapeStrategy.NON_ASCII_CHARS_ONLY)); conditions.add(conditionsBuilder.equal(UserModel.FIRST_NAME, s, EscapeStrategy.NON_ASCII_CHARS_ONLY));
conditions.add(conditionsBuilder.equal(UserModel.LAST_NAME, s, EscapeStrategy.NON_ASCII_CHARS_ONLY)); conditions.add(conditionsBuilder.equal(UserModel.LAST_NAME, s, EscapeStrategy.NON_ASCII_CHARS_ONLY));
}
ldapQuery.addWhereCondition(conditionsBuilder.orCondition(conditions.toArray(Condition[]::new))); ldapQuery.addWhereCondition(conditionsBuilder.orCondition(conditions.toArray(Condition[]::new)));
}
return paginatedSearchLDAP(ldapQuery, firstResult, maxResults); return paginatedSearchLDAP(ldapQuery, firstResult, maxResults);
} }

View file

@ -1005,7 +1005,7 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest {
Assert.assertNull(UserStoragePrivateUtil.userLocalStorage(session).getUserByUsername(appRealm, "username4")); Assert.assertNull(UserStoragePrivateUtil.userLocalStorage(session).getUserByUsername(appRealm, "username4"));
// search by username (we use a terminal operation on the stream to ensure it is consumed) // search by username (we use a terminal operation on the stream to ensure it is consumed)
Assert.assertEquals(1, 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"); 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) // search by email (we use a terminal operation on the stream to ensure it is consumed)

View file

@ -25,6 +25,7 @@ import java.util.TreeSet;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasItems;
import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.not;
@ -96,4 +97,40 @@ public class LDAPSearchForUsersPaginationTest extends AbstractLDAPTest {
assertThat(search, not(contains(importedUsers.get(2)))); assertThat(search, not(contains(importedUsers.get(2))));
assertThat(search, hasItems(importedUsers.get(3), importedUsers.get(4))); assertThat(search, hasItems(importedUsers.get(3), importedUsers.get(4)));
} }
@Test
public void testSearchLDAPMatchesLocalDBTwoKeywords() {
assertLDAPSearchMatchesLocalDB("Some Some");
}
@Test
public void testSearchLDAPMatchesLocalDBExactSearch() {
assertLDAPSearchMatchesLocalDB("\"Some\"");
}
@Test
public void testSearchLDAPMatchesLocalDBInfixSearch() {
assertLDAPSearchMatchesLocalDB("*ohn*");
}
@Test
public void testSearchLDAPMatchesLocalDBPrefixSearch() {
assertLDAPSearchMatchesLocalDB("john*");
}
@Test
public void testSearchLDAPMatchesLocalDBDefaultPrefixSearch() {
// default search is prefix search
assertLDAPSearchMatchesLocalDB("john");
}
private void assertLDAPSearchMatchesLocalDB(String searchString) {
//this call should import some users into local database
List<String> importedUsers = adminClient.realm(TEST_REALM_NAME).users().search(searchString, null, null).stream().map(UserRepresentation::getUsername).collect(Collectors.toList());
//this should query local db
List<String> search = adminClient.realm(TEST_REALM_NAME).users().search(searchString, null, null).stream().map(UserRepresentation::getUsername).collect(Collectors.toList());
assertThat(search, containsInAnyOrder(importedUsers.toArray()));
}
} }

View file

@ -32,8 +32,6 @@ import org.keycloak.testsuite.federation.ldap.LDAPTestContext;
import org.keycloak.testsuite.util.LDAPRule; import org.keycloak.testsuite.util.LDAPRule;
import org.keycloak.testsuite.util.LDAPTestUtils; import org.keycloak.testsuite.util.LDAPTestUtils;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.not;
@ -89,8 +87,8 @@ public class LDAPSearchForUsersPaginationNoImportTest extends AbstractLDAPTest {
@Test @Test
public void testPagination() { public void testPagination() {
//tests LDAPStorageProvider.searchLDAP(... //tests LDAPStorageProvider.searchLDAP(...
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John Some Doe", 0, 15), hasSize(15)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("jo*", 0, 15), hasSize(15));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John Some Doe", 7, 10), hasSize(8)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John Some Doe", 0, 15), hasSize(0));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("*", null, null), hasSize(15)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("*", null, null), hasSize(15));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("*", null, null), hasSize(15)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("*", null, null), hasSize(15));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("*", 10, 8), hasSize(5)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("*", 10, 8), hasSize(5));
@ -100,14 +98,16 @@ public class LDAPSearchForUsersPaginationNoImportTest extends AbstractLDAPTest {
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("*", 14, 2), hasSize(1)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("*", 14, 2), hasSize(1));
//tests LDAPStorageProvider.searchLDAP(... //tests LDAPStorageProvider.searchLDAP(...
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John", null, null), hasSize(11)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John", null, null), hasSize(15));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John", 10, 8), hasSize(1)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John*", null, null), hasSize(15));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John", 0, 10), hasSize(10)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("\"John\"", null, null), hasSize(11));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John", 0, 5), hasSize(5)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("\"John\"", 10, 8), hasSize(1));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John", 2, 10), hasSize(9)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("\"John\"", 0, 10), hasSize(10));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("John", 0, 8), hasSize(8)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("\"John\"", 0, 5), hasSize(5));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("Some", 0, 20), hasSize(10)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("\"John\"", 2, 10), hasSize(9));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("Some", 10, 20), hasSize(0)); assertThat(adminClient.realm(TEST_REALM_NAME).users().search("\"John\"", 0, 8), hasSize(8));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("\"Some\"", 0, 20), hasSize(10));
assertThat(adminClient.realm(TEST_REALM_NAME).users().search("\"Some\"", 10, 20), hasSize(0));
//tests LDAPStorageProvider.searchLDAPByAttributes(... //tests LDAPStorageProvider.searchLDAPByAttributes(...
assertThat(adminClient.realm(TEST_REALM_NAME).users().list(), hasSize(15)); assertThat(adminClient.realm(TEST_REALM_NAME).users().list(), hasSize(15));