KEYCLOAK-18380 Fix Groups search by name returns unwanted groups

thomasdarimont · hmlnarik · commit 17da3ee8d947 · 2021-08-05T11:43:56.000+02:00
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.
diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java
@@ -38,6 +38,7 @@
 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 static GroupRepresentation toRepresentation(GroupModel group, boolean ful
 
     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 static Stream<GroupRepresentation> toGroupHierarchy(UserModel user, boole
     }
 
     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());
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java
@@ -60,6 +60,7 @@
 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 void adminEndpointAccessibleWhenAdminRoleAssignedToGroup() {
         }
     }
 
+    /**
+     * 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.
