KEYCLOAK-18380 Fix Groups search by name returns unwanted groups

Previously the group search did not apply a given search query as filter
for groups along the group path.

We now filter the found groups with the given group search query if present.
This commit is contained in:
Thomas Darimont 2021-06-08 10:53:20 +02:00 committed by Hynek Mlnařík
parent bd55694903
commit 17da3ee8d9
2 changed files with 79 additions and 2 deletions

View file

@ -38,6 +38,7 @@ import org.keycloak.provider.ProviderConfigProperty;
import org.keycloak.representations.idm.*;
import org.keycloak.representations.idm.authorization.*;
import org.keycloak.storage.StorageId;
import org.keycloak.utils.StringUtil;
import java.util.ArrayList;
import java.util.HashMap;
@ -144,7 +145,7 @@ public class ModelToRepresentation {
public static Stream<GroupRepresentation> searchForGroupByName(RealmModel realm, boolean full, String search, Integer first, Integer max) {
return realm.searchForGroupByNameStream(search, first, max)
.map(g -> toGroupHierarchy(g, full));
.map(g -> toGroupHierarchy(g, full, search));
}
public static Stream<GroupRepresentation> searchForGroupByName(UserModel user, boolean full, String search, Integer first, Integer max) {
@ -173,13 +174,28 @@ public class ModelToRepresentation {
}
public static GroupRepresentation toGroupHierarchy(GroupModel group, boolean full) {
return toGroupHierarchy(group, full, null);
}
public static GroupRepresentation toGroupHierarchy(GroupModel group, boolean full, String search) {
GroupRepresentation rep = toRepresentation(group, full);
List<GroupRepresentation> subGroups = group.getSubGroupsStream()
.map(subGroup -> toGroupHierarchy(subGroup, full)).collect(Collectors.toList());
.filter(g -> groupMatchesSearchOrIsPathElement(g, search))
.map(subGroup -> toGroupHierarchy(subGroup, full, search)).collect(Collectors.toList());
rep.setSubGroups(subGroups);
return rep;
}
private static boolean groupMatchesSearchOrIsPathElement(GroupModel group, String search) {
if (StringUtil.isBlank(search)) {
return true;
}
if (group.getName().contains(search)) {
return true;
}
return group.getSubGroupsStream().findAny().isPresent();
}
public static UserRepresentation toRepresentation(KeycloakSession session, RealmModel realm, UserModel user) {
UserRepresentation rep = new UserRepresentation();
rep.setId(user.getId());

View file

@ -60,6 +60,7 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.stream.Collectors;
import javax.ws.rs.ClientErrorException;
import javax.ws.rs.core.Response.Status;
import static org.hamcrest.Matchers.*;
@ -839,6 +840,66 @@ public class GroupTest extends AbstractGroupTest {
}
}
/**
* Groups search with query returns unwanted groups
* @link https://issues.redhat.com/browse/KEYCLOAK-18380
*/
@Test
public void searchForGroupsShouldOnlyReturnMatchingElementsOrIntermediatePaths() {
/*
* /g1/g1.1-bubu
* /g1/g1.2-test1234
* /g2-test1234
* /g3/g3.1-test1234/g3.1.1
*/
String needle = "test1234";
GroupRepresentation g1 = GroupBuilder.create().name("g1").build();
GroupRepresentation g1_1 = GroupBuilder.create().name("g1.1-bubu").build();
GroupRepresentation g1_2 = GroupBuilder.create().name("g1.2-" + needle).build();
GroupRepresentation g2 = GroupBuilder.create().name("g2-" + needle).build();
GroupRepresentation g3 = GroupBuilder.create().name("g3").build();
GroupRepresentation g3_1 = GroupBuilder.create().name("g3.1-" + needle).build();
GroupRepresentation g3_1_1 = GroupBuilder.create().name("g3.1.1").build();
String realmName = AuthRealm.TEST;
RealmResource realm = adminClient.realms().realm(realmName);
createGroup(realm, g1);
createGroup(realm, g2);
createGroup(realm, g3);
addSubGroup(realm, g1, g1_1);
addSubGroup(realm, g1, g1_2);
addSubGroup(realm, g3, g3_1);
addSubGroup(realm, g3_1, g3_1_1);
try {
// we search for "test1234" and expect only /g1/g1.2-test1234, /g2-test1234 and /g3/g3.1-test1234 as a result
List<GroupRepresentation> result = realm.groups().groups(needle, 0, 100);
// ensure stable sorting to make tests pass
result = result.stream().sorted(Comparator.comparing(GroupRepresentation::getName)).collect(Collectors.toList());
assertEquals(3, result.size());
assertEquals("g1", result.get(0).getName());
assertEquals(1, result.get(0).getSubGroups().size());
assertEquals("g1.2-" + needle, result.get(0).getSubGroups().get(0).getName());
assertEquals("g2-" + needle, result.get(1).getName());
assertEquals("g3", result.get(2).getName());
assertEquals(1, result.get(2).getSubGroups().size());
assertEquals("g3.1-" + needle, result.get(2).getSubGroups().get(0).getName());
} finally {
if (g1.getId() != null) {
realm.groups().group(g1.getId()).remove();
}
if (g2.getId() != null) {
realm.groups().group(g2.getId()).remove();
}
if (g3.getId() != null) {
realm.groups().group(g3.getId()).remove();
}
}
}
/**
* Verifies that the role assigned to a user's group is correctly handled by Keycloak Admin endpoint.