diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java index 7f6bc3541eda..7dd23a6afdb8 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java @@ -182,6 +182,7 @@ public Stream getSubGroups( @Parameter(description = "The maximum number of results that are to be returned. Defaults to 10") @QueryParam("max") @DefaultValue("10") Integer max, @Parameter(description = "Boolean which defines whether brief groups representations are returned or not (default: false)") @QueryParam("briefRepresentation") @DefaultValue("false") Boolean briefRepresentation, @Parameter(description = "Boolean which defines whether to return the count of subgroups for each subgroup of this group (default: true") @QueryParam("subGroupsCount") @DefaultValue("true") Boolean subGroupsCount) { + this.auth.groups().requireList(); this.auth.groups().requireView(group); Stream stream = group.getSubGroupsStream(search, exact, -1, -1); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java index f087588f3143..b8ecd22559af 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java @@ -48,6 +48,7 @@ import org.keycloak.testframework.injection.LifeCycle; import org.keycloak.testframework.realm.ManagedClient; import org.keycloak.testframework.realm.ManagedRealm; +import org.keycloak.testframework.util.ApiUtil; public abstract class AbstractPermissionTest { @@ -225,4 +226,15 @@ protected ScopePermissionRepresentation createPermission(ManagedClient client, S protected ScopePermissionRepresentation createGroupPermission(GroupRepresentation group, Set scopes, AbstractPolicyRepresentation... policies) { return createPermission(client, group.getId(), AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, scopes, policies); } + + protected GroupRepresentation createGroup(String name) { + GroupRepresentation group = new GroupRepresentation(); + + group.setName(name); + + try (Response response = realm.admin().groups().add(group)) { + group.setId(ApiUtil.getCreatedId(response)); + return group; + } + } } diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/FineGrainedPermissionsUsersTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/FineGrainedPermissionsUsersTest.java index 75f4406d5cf7..36d2620c93d5 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/FineGrainedPermissionsUsersTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/FineGrainedPermissionsUsersTest.java @@ -259,13 +259,4 @@ private List setupUsersInGroups() { return groups; } - - private GroupRepresentation createGroup(String name) { - GroupRepresentation grp = new GroupRepresentation(); - grp.setName(name); - String groupId = ApiUtil.getCreatedId(realm.admin().groups().add(grp)); - grp.setId(groupId); - realm.cleanup().add(r -> r.groups().group(groupId).remove()); - return grp; - } } diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java index 4822fab9d107..da2c03781635 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java @@ -39,13 +39,17 @@ import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.Response; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.GroupsResource; import org.keycloak.authorization.fgap.AdminPermissionsSchema; +import org.keycloak.models.AdminRoles; +import org.keycloak.models.Constants; import org.keycloak.representations.idm.GroupRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.representations.idm.UserRepresentation; @@ -516,6 +520,94 @@ public void testImpersonateMembersFromChildGroups() { realmAdminClient.tokenManager().logout(); } + @Test + public void testChildrenEndpointRequiresQueryRole() { + GroupRepresentation parentGroup = createGroup("parent-group"); + + GroupRepresentation hiddenChildGroup = new GroupRepresentation(); + hiddenChildGroup.setName("hidden-child-group"); + try (Response response = realm.admin().groups().group(parentGroup.getId()).subGroup(hiddenChildGroup)) { + hiddenChildGroup.setId(ApiUtil.getCreatedId(response)); + } + hiddenChildGroup.setAttributes(Map.of("hidden", List.of("child-secret-attr"))); + realm.admin().groups().group(hiddenChildGroup.getId()).update(hiddenChildGroup); + + UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0); + + // remove all query roles from myadmin + String realmMgmtId = realm.admin().clients().findByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID).get(0).getId(); + List queryRoles = Stream.of(AdminRoles.QUERY_USERS, AdminRoles.QUERY_GROUPS, AdminRoles.QUERY_CLIENTS) + .map(roleName -> realm.admin().clients().get(realmMgmtId).roles().get(roleName).toRepresentation()) + .toList(); + realm.admin().users().get(myadmin.getId()).roles().clientLevel(realmMgmtId).remove(queryRoles); + + UserPolicyRepresentation policy = createUserPolicy(realm, client, "Only My Admin User Policy", myadmin.getId()); + createGroupPermission(parentGroup, Set.of(VIEW), policy); + + // limited admin can still view parent group directly (requireView passes via FGAP) + realmAdminClient.realm(realm.getName()).groups().group(parentGroup.getId()).toRepresentation(); + + // children endpoint requires query role — should be forbidden without it + try { + realmAdminClient.realm(realm.getName()).groups().group(parentGroup.getId()).getSubGroups(-1, -1, false); + fail("Should not be able to list children without query role"); + } catch (Exception ex) { + assertThat(ex, instanceOf(ForbiddenException.class)); + } + + try { + realmAdminClient.realm(realm.getName()).groups().group(parentGroup.getId()).getSubGroups("hidden-child-group", true, -1, -1, false); + fail("Should not be able to search children without query role"); + } catch (Exception ex) { + assertThat(ex, instanceOf(ForbiddenException.class)); + } + } + + @Test + public void testChildrenEndpointFilteredByGroupPermissions() { + GroupRepresentation parentGroup = createGroup("parent-group"); + + GroupRepresentation hiddenChildGroup = new GroupRepresentation(); + hiddenChildGroup.setName("hidden-child-group"); + try (Response response = realm.admin().groups().group(parentGroup.getId()).subGroup(hiddenChildGroup)) { + hiddenChildGroup.setId(ApiUtil.getCreatedId(response)); + } + hiddenChildGroup.setAttributes(Map.of("hidden", List.of("child-secret-attr"))); + realm.admin().groups().group(hiddenChildGroup.getId()).update(hiddenChildGroup); + + UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0); + UserPolicyRepresentation policy = createUserPolicy(realm, client, "Only My Admin User Policy", myadmin.getId()); + createGroupPermission(parentGroup, Set.of(VIEW), policy); + + // limited admin can view parent group + realmAdminClient.realm(realm.getName()).groups().group(parentGroup.getId()).toRepresentation(); + + // limited admin cannot view hidden child group directly + try { + realmAdminClient.realm(realm.getName()).groups().group(hiddenChildGroup.getId()).toRepresentation(); + fail("Should not be able to access hidden child group directly"); + } catch (Exception ex) { + assertThat(ex, instanceOf(ForbiddenException.class)); + } + + // searching for hidden child group by name returns empty + List search = realmAdminClient.realm(realm.getName()).groups().groups("hidden-child-group", -1, -1, false); + assertTrue(search.isEmpty()); + + // children endpoint must not return hidden child group (partial evaluator filters it) + List children = realmAdminClient.realm(realm.getName()).groups().group(parentGroup.getId()).getSubGroups(-1, -1, false); + assertTrue(children.isEmpty()); + + // exact name search via children endpoint must not return hidden child group + children = realmAdminClient.realm(realm.getName()).groups().group(parentGroup.getId()).getSubGroups("hidden-child-group", true, -1, -1, false); + assertTrue(children.isEmpty()); + + // full admin still sees the child + List adminChildren = realm.admin().groups().group(parentGroup.getId()).getSubGroups(-1, -1, false); + assertThat(adminChildren, hasSize(1)); + assertEquals("hidden-child-group", adminChildren.get(0).getName()); + } + private ScopePermissionRepresentation createAllGroupsPermission(UserPolicyRepresentation policy, Set scopes) { return createAllPermission(client, AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, policy, scopes); }