Automated Test: feature-groups-authz-implementation #315
+831
−126
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ation implementation Closes #35562 Signed-off-by: vramik <vramik@redhat.com>
📝 WalkthroughWalkthroughThis change introduces fine-grained group-based authorization support across Keycloak's admin API. It adds group resource types to the authorization schema, expands permission evaluators with granular access checks, replaces GroupModel references with group IDs in permission logic, and introduces a V2 implementation layer for group permissions evaluation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Admin Client
participant Endpoint as GroupsResource
participant Auth as GroupPermissions
participant ResourceStore as Resource Store
participant PolicyStore as Policy Store
Client->>Endpoint: searchForGroups(with view check)
Endpoint->>Auth: canView()
Auth->>Auth: Check admin roles (MANAGE_USERS/VIEW_USERS)
alt Has admin role
Auth-->>Endpoint: true
else No admin role
Auth->>ResourceStore: Fetch group resources
Auth->>PolicyStore: Evaluate policies
Auth-->>Endpoint: filtered groups with IDs
end
Endpoint->>Endpoint: Filter results by per-group visibility
Endpoint-->>Client: Filtered group list
sequenceDiagram
participant User as User Request
participant UsersResource as UsersResource
participant GroupPerms as GroupPermissions
participant Session as Session/Groups API
participant Search as Search Stream
User->>UsersResource: searchForUser(with group filters)
UsersResource->>GroupPerms: getGroupIdsWithViewPermission()
alt Has global view permission
GroupPerms-->>UsersResource: empty set (global view)
else Per-group permissions
GroupPerms->>Session: Evaluate per-group visibility
GroupPerms-->>UsersResource: Set of group IDs with permission
end
UsersResource->>Session: Set GROUPS attribute with IDs
UsersResource->>Search: searchForUserStream(with group IDs)
Search-->>UsersResource: Filtered user stream
UsersResource-->>User: User representations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
🚥 Pre-merge checks | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/src/main/java/org/keycloak/services/resources/admin/permissions/AdminPermissions.java (1)
73-98:⚠️ Potential issue | 🟠 MajorEnsure listener also registers when FGAP V2 is enabled.
The
registerListenermethod gates onADMIN_FINE_GRAINED_AUTHZonly, but V2-only deployments won't register the cleanup listener for deleted roles, clients, and groups. This is inconsistent with other methods in this class (likeevaluatorandrealms) which check both V1 and V2 flags. SeeMgmtPermissions.javafor the same both-flags pattern.Suggested fix
- if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ)) { + if (Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ) + || Profile.isFeatureEnabled(Profile.Feature.ADMIN_FINE_GRAINED_AUTHZ_V2)) {
🤖 Fix all issues with AI agents
In
`@services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java`:
- Around line 142-147: The Javadoc for
GroupPermissionEvaluator#getGroupIdsWithViewPermission incorrectly refers to a
"Stream" while the method returns a Set; update the Javadoc to say it returns a
Set (e.g., "Set of IDs of groups with view permission") and ensure the
description and `@return` tag match the actual return type, keeping the reference
to UserPermissionEvaluator#canView() intact.
- Around line 132-135: Update the Javadoc for the requireManageMembers method in
GroupPermissionEvaluator to reference the correct predicate name
canManageMembers (not canManageMembership); change the `@link` target in the
comment above requireManageMembers so it points to canManageMembers to match the
actual predicate/method name.
In
`@services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.java`:
- Around line 105-128: In getGroupIdsWithViewPermission(), use the resource name
(groupResource.getName()) instead of the internal DB id (groupResource.getId())
when calling hasPermission and when adding to the granted set: update the
hasPermission(...) call and the granted.add(...) to pass
groupResource.getName(), since hasPermission uses findByName() and
AdminPermissionsSchema stores the group ID as the resource name.
In
`@tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java`:
- Around line 138-187: The testManageAllGroups method creates a group
(testGroupId via ApiUtil.handleCreatedResponse) but doesn't remove it; add
cleanup so the created group is deleted after the test to avoid test-order
leakage. Wrap the portion that creates/uses the group (creation, update,
subgroup add, role mapping) in a try/finally or append a finally-like cleanup
that calls
realmAdminClient.realm(realm.getName()).groups().group(testGroupId).remove() (or
equivalent removal API) and handle/remove only if testGroupId is non-null to
ensure the group is removed even if assertions throw.
🧹 Nitpick comments (1)
server-spi-private/src/main/java/org/keycloak/authorization/AdminPermissionsSchema.java (1)
55-90: Consider resolving GROUPS resource names to group display names.
With the new GROUPS resource type,getResourceName()currently returns raw IDs for groups. Mapping to group names (and "All groups") improves admin UI readability.♻️ Suggested addition to getResourceName()
if (supportsAuthorizationSchema(session, resourceServer)) { String resourceType = policy.getResourceType(); if (USERS.getType().equals(resourceType)) { if (resource.getName().equals(USERS_RESOURCE_TYPE)) { return "All users"; } @@ return user.getUsername(); } + if (GROUPS.getType().equals(resourceType)) { + if (resource.getName().equals(GROUPS_RESOURCE_TYPE)) { + return "All groups"; + } + GroupModel group = session.groups().getGroupById(session.getContext().getRealm(), resource.getName()); + if (group == null) { + throw new ModelIllegalStateException("Group not found for resource [" + resource.getId() + "]"); + } + return group.getName(); + } }Also applies to: 334-346
Comment on lines
+132
to
135
| /** | ||
| * Throws ForbiddenException if {@link #canManageMembership(GroupModel)} returns {@code false}. | ||
| */ | ||
| void requireManageMembers(GroupModel group); |
There was a problem hiding this comment.
Javadoc points to the wrong predicate for requireManageMembers.
Line 132 should reference canManageMembers, not canManageMembership.
📝 Javadoc fix
- * Throws ForbiddenException if {`@link` `#canManageMembership`(GroupModel)} returns {`@code` false}.
+ * Throws ForbiddenException if {`@link` `#canManageMembers`(GroupModel)} returns {`@code` false}.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * Throws ForbiddenException if {@link #canManageMembership(GroupModel)} returns {@code false}. | |
| */ | |
| void requireManageMembers(GroupModel group); | |
| /** | |
| * Throws ForbiddenException if {`@link` `#canManageMembers`(GroupModel)} returns {`@code` false}. | |
| */ | |
| void requireManageMembers(GroupModel group); |
🤖 Prompt for AI Agents
In
`@services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java`
around lines 132 - 135, Update the Javadoc for the requireManageMembers method
in GroupPermissionEvaluator to reference the correct predicate name
canManageMembers (not canManageMembership); change the `@link` target in the
comment above requireManageMembers so it points to canManageMembers to match the
actual predicate/method name.
Comment on lines
+142
to
+147
| /** | ||
| * If {@link UserPermissionEvaluator#canView()} evaluates to {@code true}, returns empty set. | ||
| * | ||
| * @return Stream of IDs of groups with view permission. | ||
| */ | ||
| Set<String> getGroupIdsWithViewPermission(); |
There was a problem hiding this comment.
Return type in Javadoc should match Set.
Line 145 mentions “Stream” but the method returns a Set.
📝 Javadoc fix
- * `@return` Stream of IDs of groups with view permission.
+ * `@return` Set of IDs of groups with view permission.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * If {@link UserPermissionEvaluator#canView()} evaluates to {@code true}, returns empty set. | |
| * | |
| * @return Stream of IDs of groups with view permission. | |
| */ | |
| Set<String> getGroupIdsWithViewPermission(); | |
| /** | |
| * If {`@link` UserPermissionEvaluator#canView()} evaluates to {`@code` true}, returns empty set. | |
| * | |
| * `@return` Set of IDs of groups with view permission. | |
| */ | |
| Set<String> getGroupIdsWithViewPermission(); |
🤖 Prompt for AI Agents
In
`@services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionEvaluator.java`
around lines 142 - 147, The Javadoc for
GroupPermissionEvaluator#getGroupIdsWithViewPermission incorrectly refers to a
"Stream" while the method returns a Set; update the Javadoc to say it returns a
Set (e.g., "Set of IDs of groups with view permission") and ensure the
description and `@return` tag match the actual return type, keeping the reference
to UserPermissionEvaluator#canView() intact.
Comment on lines
+105
to
+128
| @Override | ||
| public Set<String> getGroupIdsWithViewPermission() { | ||
| if (root.users().canView()) return Collections.emptySet(); | ||
|
|
||
| if (!root.isAdminSameRealm()) { | ||
| return Collections.emptySet(); | ||
| } | ||
|
|
||
| ResourceServer server = root.realmResourceServer(); | ||
|
|
||
| if (server == null) { | ||
| return Collections.emptySet(); | ||
| } | ||
|
|
||
| Set<String> granted = new HashSet<>(); | ||
|
|
||
| resourceStore.findByType(server, AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, groupResource -> { | ||
| if (hasPermission(groupResource.getId(), AdminPermissionsSchema.VIEW_MEMBERS, AdminPermissionsSchema.MANAGE_MEMBERS)) { | ||
| granted.add(groupResource.getId()); | ||
| } | ||
| }); | ||
|
|
||
| return granted; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the file and check its structure
find . -name "GroupPermissionsV2.java" -type fRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 185
🏁 Script executed:
# Also search for the hasPermission method to understand what it expects
rg -n "hasPermission" services/src/main/java/org/keycloak/services/resources/admin/permissions/ -A 3 | head -80Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 8945
🏁 Script executed:
# Check AdminPermissionsSchema for GROUPS_RESOURCE_TYPE
find . -name "AdminPermissionsSchema.java" -type fRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 176
🏁 Script executed:
# Read the GroupPermissionsV2.java file to understand the context
cat -n services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.java | head -200Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 8105
🏁 Script executed:
# Look for the hasPermission method definition in GroupPermissionsV2
rg -n "hasPermission" services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.java -B 2 -A 10Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 2882
🏁 Script executed:
# Check AdminPermissionsSchema for GROUPS_RESOURCE_TYPE
cat -n server-spi-private/src/main/java/org/keycloak/authorization/AdminPermissionsSchema.javaRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 17918
🏁 Script executed:
# Check if there are other methods in GroupPermissionsV2 that iterate resources
rg -n "resourceStore\.findByType|findByName" services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.java -A 5Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 838
🏁 Script executed:
# Check lines 130-165 (also mentioned as affected) to see if there's another method
sed -n '130,200p' services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.java | cat -nRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 2924
🏁 Script executed:
# Search for similar patterns in other files to understand the naming convention
rg -n "groupResource\.|\.getName\(\)|\.getId\(\)" services/src/main/java/org/keycloak/services/resources/admin/permissions/ -B 2 -A 2 | head -100Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 13098
🏁 Script executed:
# Check if there are other methods in GroupPermissionsV2 that might have the same issue
wc -l services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.javaRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 187
🏁 Script executed:
# Look for other methods that iterate resources and might have the same pattern
rg -n "public.*getGroupIds" services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.javaRepository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 147
🏁 Script executed:
# Check the entire file to look for more methods iterating over resources
cat services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.java | tail -100Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 3271
🏁 Script executed:
# Check if UserPermissionsV2 has a similar method that iterates resources
rg -n "getUsersWithPermission|resourceStore\.findByType" services/src/main/java/org/keycloak/services/resources/admin/permissions/UserPermissionsV2.java -A 5Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 69
🏁 Script executed:
# Look at how Resource objects are used elsewhere to understand the pattern better
rg -n "\.findByType\(" services/src/main/java/org/keycloak/services/resources/admin/permissions/ -B 2 -A 8Repository: admin-coderabbit/coderabbit-keycloak-mirror
Length of output: 7277
Use resource name instead of resource ID when checking group permissions.
Line 122–123 incorrectly use groupResource.getId() (the internal Resource database ID). Since hasPermission() calls findByName() expecting a resource name, and AdminPermissionsSchema sets the resource name to the group ID, this should use groupResource.getName() instead. The current code fails the per-group resource lookup and falls back to checking only the all-groups resource.
🛠️ Suggested fix
resourceStore.findByType(server, AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, groupResource -> {
- if (hasPermission(groupResource.getId(), AdminPermissionsSchema.VIEW_MEMBERS, AdminPermissionsSchema.MANAGE_MEMBERS)) {
- granted.add(groupResource.getId());
+ String groupId = groupResource.getName();
+ if (hasPermission(groupId, AdminPermissionsSchema.VIEW_MEMBERS, AdminPermissionsSchema.MANAGE_MEMBERS)) {
+ granted.add(groupId);
}
});🤖 Prompt for AI Agents
In
`@services/src/main/java/org/keycloak/services/resources/admin/permissions/GroupPermissionsV2.java`
around lines 105 - 128, In getGroupIdsWithViewPermission(), use the resource
name (groupResource.getName()) instead of the internal DB id
(groupResource.getId()) when calling hasPermission and when adding to the
granted set: update the hasPermission(...) call and the granted.add(...) to pass
groupResource.getName(), since hasPermission uses findByName() and
AdminPermissionsSchema stores the group ID as the resource name.
Comment on lines
+138
to
+187
| @Test | ||
| public void testManageAllGroups() { | ||
| // myadmin shouldn't be able to create groups just yet | ||
| try (Response response = realmAdminClient.realm(realm.getName()).groups().add(new GroupRepresentation())) { | ||
| assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus()); | ||
| } | ||
|
|
||
| // myadmin shouldn't be able to add child for a group | ||
| try (Response response = realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).subGroup(new GroupRepresentation())) { | ||
| assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus()); | ||
| } | ||
|
|
||
| // myadmin shouldn't be able to map roles for group | ||
| try { | ||
| realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).roles().realmLevel().add(List.of()); | ||
| fail("Expected Exception wasn't thrown."); | ||
| } catch (Exception ex) { | ||
| assertThat(ex, instanceOf(ForbiddenException.class)); | ||
| } | ||
|
|
||
| //create all-groups permission for "myadmin" (so that myadmin can manage all groups in the realm) | ||
| UserPolicyRepresentation policy = createUserPolicy(realm, client, "Only My Admin User Policy", realm.admin().users().search("myadmin").get(0).getId()); | ||
| createAllGroupsPermission(policy, Set.of(MANAGE)); | ||
|
|
||
| // creating group requires manage scope | ||
| GroupRepresentation group = new GroupRepresentation(); | ||
| group.setName("testGroup"); | ||
| String testGroupId = ApiUtil.handleCreatedResponse(realmAdminClient.realm(realm.getName()).groups().add(group)); | ||
| group.setId(testGroupId); | ||
|
|
||
| // it should be possible to update the group due to fallback to all-groups permission | ||
| group.setName("newGroup"); | ||
| realmAdminClient.realm(realm.getName()).groups().group(testGroupId).update(group); | ||
| assertEquals("newGroup", realmAdminClient.realm(realm.getName()).groups().group(testGroupId).toRepresentation().getName()); | ||
|
|
||
| // it should be possible to add the child to the group now | ||
| try (Response response = realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).subGroup(group)) { | ||
| assertEquals(Response.Status.NO_CONTENT.getStatusCode(), response.getStatus()); | ||
| } | ||
|
|
||
| // it should be possible to map roles now | ||
| // trying with non existent role as we need to test manage permission for groups (not `auth.roles().requireMapRole(roleModel);`) | ||
| // expecting NotFoundException | ||
| try { | ||
| realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).roles().realmLevel().add(List.of(new RoleRepresentation("non_existent", null, false))); | ||
| fail("Expected Exception wasn't thrown."); | ||
| } catch (Exception ex) { | ||
| assertThat(ex, instanceOf(NotFoundException.class)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add cleanup for the group created in this test.
testManageAllGroups() creates testGroup but never removes it, which can make later tests order-dependent if the realm isn’t reset between tests.
🧹 Suggested cleanup
String testGroupId = ApiUtil.handleCreatedResponse(realmAdminClient.realm(realm.getName()).groups().add(group));
group.setId(testGroupId);
+ realm.cleanup().add(r -> r.groups().group(testGroupId).remove());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @Test | |
| public void testManageAllGroups() { | |
| // myadmin shouldn't be able to create groups just yet | |
| try (Response response = realmAdminClient.realm(realm.getName()).groups().add(new GroupRepresentation())) { | |
| assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus()); | |
| } | |
| // myadmin shouldn't be able to add child for a group | |
| try (Response response = realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).subGroup(new GroupRepresentation())) { | |
| assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus()); | |
| } | |
| // myadmin shouldn't be able to map roles for group | |
| try { | |
| realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).roles().realmLevel().add(List.of()); | |
| fail("Expected Exception wasn't thrown."); | |
| } catch (Exception ex) { | |
| assertThat(ex, instanceOf(ForbiddenException.class)); | |
| } | |
| //create all-groups permission for "myadmin" (so that myadmin can manage all groups in the realm) | |
| UserPolicyRepresentation policy = createUserPolicy(realm, client, "Only My Admin User Policy", realm.admin().users().search("myadmin").get(0).getId()); | |
| createAllGroupsPermission(policy, Set.of(MANAGE)); | |
| // creating group requires manage scope | |
| GroupRepresentation group = new GroupRepresentation(); | |
| group.setName("testGroup"); | |
| String testGroupId = ApiUtil.handleCreatedResponse(realmAdminClient.realm(realm.getName()).groups().add(group)); | |
| group.setId(testGroupId); | |
| // it should be possible to update the group due to fallback to all-groups permission | |
| group.setName("newGroup"); | |
| realmAdminClient.realm(realm.getName()).groups().group(testGroupId).update(group); | |
| assertEquals("newGroup", realmAdminClient.realm(realm.getName()).groups().group(testGroupId).toRepresentation().getName()); | |
| // it should be possible to add the child to the group now | |
| try (Response response = realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).subGroup(group)) { | |
| assertEquals(Response.Status.NO_CONTENT.getStatusCode(), response.getStatus()); | |
| } | |
| // it should be possible to map roles now | |
| // trying with non existent role as we need to test manage permission for groups (not `auth.roles().requireMapRole(roleModel);`) | |
| // expecting NotFoundException | |
| try { | |
| realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).roles().realmLevel().add(List.of(new RoleRepresentation("non_existent", null, false))); | |
| fail("Expected Exception wasn't thrown."); | |
| } catch (Exception ex) { | |
| assertThat(ex, instanceOf(NotFoundException.class)); | |
| } | |
| } | |
| `@Test` | |
| public void testManageAllGroups() { | |
| // myadmin shouldn't be able to create groups just yet | |
| try (Response response = realmAdminClient.realm(realm.getName()).groups().add(new GroupRepresentation())) { | |
| assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus()); | |
| } | |
| // myadmin shouldn't be able to add child for a group | |
| try (Response response = realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).subGroup(new GroupRepresentation())) { | |
| assertEquals(Response.Status.FORBIDDEN.getStatusCode(), response.getStatus()); | |
| } | |
| // myadmin shouldn't be able to map roles for group | |
| try { | |
| realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).roles().realmLevel().add(List.of()); | |
| fail("Expected Exception wasn't thrown."); | |
| } catch (Exception ex) { | |
| assertThat(ex, instanceOf(ForbiddenException.class)); | |
| } | |
| //create all-groups permission for "myadmin" (so that myadmin can manage all groups in the realm) | |
| UserPolicyRepresentation policy = createUserPolicy(realm, client, "Only My Admin User Policy", realm.admin().users().search("myadmin").get(0).getId()); | |
| createAllGroupsPermission(policy, Set.of(MANAGE)); | |
| // creating group requires manage scope | |
| GroupRepresentation group = new GroupRepresentation(); | |
| group.setName("testGroup"); | |
| String testGroupId = ApiUtil.handleCreatedResponse(realmAdminClient.realm(realm.getName()).groups().add(group)); | |
| group.setId(testGroupId); | |
| realm.cleanup().add(r -> r.groups().group(testGroupId).remove()); | |
| // it should be possible to update the group due to fallback to all-groups permission | |
| group.setName("newGroup"); | |
| realmAdminClient.realm(realm.getName()).groups().group(testGroupId).update(group); | |
| assertEquals("newGroup", realmAdminClient.realm(realm.getName()).groups().group(testGroupId).toRepresentation().getName()); | |
| // it should be possible to add the child to the group now | |
| try (Response response = realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).subGroup(group)) { | |
| assertEquals(Response.Status.NO_CONTENT.getStatusCode(), response.getStatus()); | |
| } | |
| // it should be possible to map roles now | |
| // trying with non existent role as we need to test manage permission for groups (not `auth.roles().requireMapRole(roleModel);`) | |
| // expecting NotFoundException | |
| try { | |
| realmAdminClient.realm(realm.getName()).groups().group(topGroup.getId()).roles().realmLevel().add(List.of(new RoleRepresentation("non_existent", null, false))); | |
| fail("Expected Exception wasn't thrown."); | |
| } catch (Exception ex) { | |
| assertThat(ex, instanceOf(NotFoundException.class)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeEvaluationTest.java`
around lines 138 - 187, The testManageAllGroups method creates a group
(testGroupId via ApiUtil.handleCreatedResponse) but doesn't remove it; add
cleanup so the created group is deleted after the test to avoid test-order
leakage. Wrap the portion that creates/uses the group (creation, update,
subgroup add, role mapping) in a try/finally or append a finally-like cleanup
that calls
realmAdminClient.realm(realm.getName()).groups().group(testGroupId).remove() (or
equivalent removal API) and handle/remove only if testGroupId is non-null to
ensure the group is removed even if assertions throw.
Sign in
to join this conversation on GitHub.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
New Features
Bug Fixes