From ca6b78b730f9f3e9642ec1403151fce2ef97a96d Mon Sep 17 00:00:00 2001
From: Sebastian Rose <sebastian.rose@codecentric.de>
Date: Wed, 9 Jun 2021 14:36:15 +0200
Subject: [PATCH] KEYCLOAK-18390 GroupProvider search implementation of JPA and
 Map delivers different results

---
 .../models/map/group/MapGroupProvider.java    |  9 ++++-
 .../storage/group/GroupLookupProvider.java    |  8 +++-
 .../testsuite/admin/group/GroupTest.java      | 37 +++++++++++++++++++
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java
index f04be591df..8cf672379a 100644
--- a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java
+++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java
@@ -178,8 +178,15 @@ public class MapGroupProvider<K> implements GroupProvider {
           (ModelCriteriaBuilder<GroupModel> mcb) -> mcb.compare(SearchableFields.NAME, Operator.ILIKE, "%" + search + "%")
         );
 
+        final Stream<String> groups = paginatedStream(groupModelStream.map(GroupModel::getId), firstResult, maxResults);
 
-        return paginatedStream(groupModelStream, firstResult, maxResults);
+        return groups.map(id -> {
+            GroupModel groupById = session.groups().getGroupById(realm,id);
+            while (Objects.nonNull(groupById.getParentId())) {
+                groupById = session.groups().getGroupById(realm, groupById.getParentId());
+            }
+            return groupById;
+        }).sorted(GroupModel.COMPARE_BY_NAME).distinct();
     }
 
     @Override
diff --git a/server-spi/src/main/java/org/keycloak/storage/group/GroupLookupProvider.java b/server-spi/src/main/java/org/keycloak/storage/group/GroupLookupProvider.java
index c7c239e71e..33de690bab 100644
--- a/server-spi/src/main/java/org/keycloak/storage/group/GroupLookupProvider.java
+++ b/server-spi/src/main/java/org/keycloak/storage/group/GroupLookupProvider.java
@@ -50,13 +50,17 @@ public interface GroupLookupProvider {
     }
 
     /**
-     * Returns groups with the given string in name for the given realm.
+     * Returns the group hierarchy with the given string in name for the given realm.
+     *
+     * For a matching group node the parent group is fetched by id (with all children) and added to the result stream.
+     * This is done until the group node does not have a parent (root group)
      *
      * @param realm Realm.
      * @param search Case sensitive searched string.
      * @param firstResult First result to return. Ignored if negative or {@code null}.
      * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}.
-     * @return Stream of groups with the given string in name. Never returns {@code null}.
+     * @return Stream of root groups that have the given string in their name themself or a group in their child-collection has.
+     * The returned hierarchy contains siblings that do not necessarily have a matching name. Never returns {@code null}.
      */
     Stream<GroupModel> searchForGroupByNameStream(RealmModel realm, String search, Integer firstResult, Integer maxResults);
 
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
index 52c20240f2..b1169d586c 100755
--- 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
@@ -1078,4 +1078,41 @@ public class GroupTest extends AbstractGroupTest {
             user.remove();
         }
     }
+
+    /**
+     * Verifies that the group search works the same across group provider implementations for hierarchies
+     * @link https://issues.jboss.org/browse/KEYCLOAK-18390
+     */
+    @Test
+    public void searchGroupsOnGroupHierarchies() throws Exception {
+        final RealmResource realm = this.adminClient.realms().realm("test");
+
+        final String searchFor = UUID.randomUUID().toString();
+
+        final GroupRepresentation g1 = new GroupRepresentation();
+        g1.setName("g1");
+        final GroupRepresentation g1_1 = new GroupRepresentation();
+        g1_1.setName("g1.1-" + searchFor);
+
+        createGroup(realm, g1);
+        addSubGroup(realm, g1, g1_1);
+
+        final GroupRepresentation expectedRootGroup = realm.groups().group(g1.getId()).toRepresentation();
+        final GroupRepresentation expectedChildGroup = realm.groups().group(g1_1.getId()).toRepresentation();
+
+        final List<GroupRepresentation> searchResultGroups = realm.groups().groups(searchFor, 0, 10);
+
+        Assert.assertFalse(searchResultGroups.isEmpty());
+        Assert.assertEquals(expectedRootGroup.getId(), searchResultGroups.get(0).getId());
+        Assert.assertEquals(expectedRootGroup.getName(), searchResultGroups.get(0).getName());
+
+        List<GroupRepresentation> searchResultSubGroups = searchResultGroups.get(0).getSubGroups();
+        Assert.assertEquals(expectedChildGroup.getId(), searchResultSubGroups.get(0).getId());
+        Assert.assertEquals(expectedChildGroup.getName(), searchResultSubGroups.get(0).getName());
+
+        searchResultSubGroups.remove(0);
+        Assert.assertTrue(searchResultSubGroups.isEmpty());
+        searchResultGroups.remove(0);
+        Assert.assertTrue(searchResultGroups.isEmpty());
+    }
 }