From 04af3af170f4878b7ac1d72ada07467815004c9e Mon Sep 17 00:00:00 2001
From: Tobias Genannt <tobias.genannt@gmail.com>
Date: Sun, 5 Mar 2023 13:40:42 +0100
Subject: [PATCH] fix: get_group_by_path uses Keycloak API to load (#417)

Keycloak provides an API endpoint to directly load a group by its path.
This commit changes get_group_by_path to not filter the groups locally
and instead uses the API endpoint.
---
 src/keycloak/keycloak_admin.py | 24 +++++-------------------
 src/keycloak/urls_patterns.py  |  1 +
 tests/test_keycloak_admin.py   | 14 ++++----------
 3 files changed, 10 insertions(+), 29 deletions(-)

diff --git a/src/keycloak/keycloak_admin.py b/src/keycloak/keycloak_admin.py
index 91d2c50..0db13e4 100644
--- a/src/keycloak/keycloak_admin.py
+++ b/src/keycloak/keycloak_admin.py
@@ -1155,36 +1155,22 @@ class KeycloakAdmin:
 
         return self.__fetch_all(url, query)
 
-    def get_group_by_path(self, path, search_in_subgroups=False):
+    def get_group_by_path(self, path):
         """Get group id based on name or path.
 
-        A straight name or path match with a top-level group will return first.
-        Subgroups are traversed, the first to match path (or name with path) is returned.
+        Returns full group details for a group defined by path
 
         GroupRepresentation
         https://www.keycloak.org/docs-api/18.0/rest-api/#_grouprepresentation
 
         :param path: group path
         :type path: str
-        :param search_in_subgroups: True if want search in the subgroups
-        :type search_in_subgroups: bool
         :return: Keycloak server response (GroupRepresentation)
         :rtype: dict
         """
-        groups = self.get_groups()
-
-        # TODO: Review this code is necessary
-        for group in groups:
-            if group["path"] == path:
-                return group
-            elif search_in_subgroups and group["subGroups"]:
-                for group in group["subGroups"]:
-                    if group["path"] == path:
-                        return group
-                    res = self.get_subgroups(group, path)
-                    if res is not None:
-                        return res
-        return None
+        params_path = {"realm-name": self.realm_name, "path": path}
+        data_raw = self.raw_get(urls_patterns.URL_ADMIN_GROUP_BY_PATH.format(**params_path))
+        return raise_error_from_response(data_raw, KeycloakGetError)
 
     def create_group(self, payload, parent=None, skip_exists=False):
         """Create a group in the Realm.
diff --git a/src/keycloak/urls_patterns.py b/src/keycloak/urls_patterns.py
index 4fedee1..055ce5c 100644
--- a/src/keycloak/urls_patterns.py
+++ b/src/keycloak/urls_patterns.py
@@ -78,6 +78,7 @@ URL_ADMIN_SERVER_INFO = "admin/serverinfo"
 
 URL_ADMIN_GROUPS = "admin/realms/{realm-name}/groups"
 URL_ADMIN_GROUP = "admin/realms/{realm-name}/groups/{id}"
+URL_ADMIN_GROUP_BY_PATH = "admin/realms/{realm-name}/group-by-path/{path}"
 URL_ADMIN_GROUP_CHILD = "admin/realms/{realm-name}/groups/{id}/children"
 URL_ADMIN_GROUP_PERMISSIONS = "admin/realms/{realm-name}/groups/{id}/management/permissions"
 URL_ADMIN_GROUP_MEMBERS = "admin/realms/{realm-name}/groups/{id}/members"
diff --git a/tests/test_keycloak_admin.py b/tests/test_keycloak_admin.py
index 5b1cf06..43e2d2a 100644
--- a/tests/test_keycloak_admin.py
+++ b/tests/test_keycloak_admin.py
@@ -606,20 +606,14 @@ def test_groups(admin: KeycloakAdmin, user: str):
 
     # Test get group by path
     res = admin.get_group_by_path(path="/main-group/subgroup-1")
-    assert res is None, res
-
-    res = admin.get_group_by_path(path="/main-group/subgroup-1", search_in_subgroups=True)
     assert res is not None, res
     assert res["id"] == subgroup_id_1, res
 
-    res = admin.get_group_by_path(
-        path="/main-group/subgroup-2/subsubgroup-1/test", search_in_subgroups=True
-    )
-    assert res is None, res
+    with pytest.raises(KeycloakGetError) as err:
+        admin.get_group_by_path(path="/main-group/subgroup-2/subsubgroup-1/test")
+    assert err.match('404: b\'{"error":"Group path does not exist"}\'')
 
-    res = admin.get_group_by_path(
-        path="/main-group/subgroup-2/subsubgroup-1", search_in_subgroups=True
-    )
+    res = admin.get_group_by_path(path="/main-group/subgroup-2/subsubgroup-1")
     assert res is not None, res
     assert res["id"] == subsubgroup_id_1