[CVE-2026-16105] Missing per-role authorization on RoleContainerResou… by martin-kanis · Pull Request #51084 · keycloak/keycloak

martin-kanis · 2026-07-22T09:31:28Z

…rce composite endpoints

Closes #51002

Copilot

Pull request overview

Adds per-role authorization to name-based composite role endpoints, addressing CVE-2026-16105.

Changes:

Requires manage permission on the target role when adding or deleting composites.
Adds integration tests confirming unauthorized operations return forbidden without modifying composites.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java`	Enforces target-role manage authorization.
`tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/InternalClientManagementTest.java`	Covers unauthorized composite additions and removals.

sguilhen · 2026-07-23T00:21:40Z

Hi @martin-kanis! The fix looks good overall, I just noticed something that might be worth handling now or at least as a follow up. The same container-only auth pattern also exists on the GET side. The name-based read endpoints in RoleContainerResource check requireView(roleContainer) but not requireView(role) - see getRoleComposites, getRealmRoleComposites, and getClientRoleComposites.

Their ID-based equivalents in RoleByIdResource all check requireView(role). I think we should follow the same pattern and add auth.roles().requireView(role) after the role lookup. Do you think we should also handle this as part of this PR or do you prefer to keep it focused and we address the GET side on a follow up?

vramik · 2026-07-23T10:09:24Z

Thanks for the PR. Before we proceed, I want to share some analysis of what this change actually does under both RBAC and FGAP evaluation, because I think we need to align on whether this addresses a real security boundary.

No `view` or `manage` scopes exist for roles in FGAP

The Roles resource type in AdminPermissionsSchema only defines three scopes: map-role, map-role-client-scope, and map-role-composite. There is no view or manage scope. This means under FGAP, requireView(role) and requireManage(role) have no per-resource scope to evaluate — they cannot provide any finer-grained check than the container-level calls. Should these scopes be added in the future, the PR would become meaningful as defense-in-depth.

Under classic RBAC, `requireView(role)` is equivalent to `requireView(roleContainer)`

canView(RoleModel role) delegates directly to the container:

Realm role → canViewRealm() → requires view-realm
Client role → canView(client) → requires view-clients

This is identical to canView(RoleContainerModel). So @sguilhen's suggestion to add requireView(role) to the GET endpoints would have no effect under either authorization model today. Again probably meaningful defense-in-depth addition.

Under classic RBAC, `requireManage(role)` differs from `requireManage(roleContainer)` only by the `isRealmAdminRole` guard

canManage(RoleModel role) for realm roles checks canManageRealm() && !isRealmAdminRole(role), while canManage(RoleContainerModel) checks only canManageRealm(). The sole difference is the isRealmAdminRole guard. This is what the PR relies on — a manage-realm holder would be blocked from modifying composites on the built-in admin role.

However, `manage-realm` is effectively equivalent to `realm-admin` (still to be implemented)

So the reported scenario (a manage-realm delegate stripping composites from the admin role) is not a real privilege boundary violation — the attacker already has equivalent power.

The PR is valid as defense-in-depth and for consistency with RoleByIdResource, so +1 for having it, but it does not fix a real exploitable vulnerability.

I would like to hear some thoughts, how would you like to proceed with it. For instance, we can merge it and adjust when we are merging the two roles, not sure what to do with the CVE though.

sguilhen · 2026-07-23T20:03:13Z

@vramik do you think this should have not been assigned a CVE?

…rce composite endpoints Closes keycloak#51002 Signed-off-by: Martin Kanis <mkanis@ibm.com> # Conflicts: # services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/InternalClientManagementTest.java:136

composites.iterator().next() will throw NoSuchElementException if the admin role has no composites in a given environment/configuration, causing a test failure unrelated to the authorization behavior. Consider asserting the set is non-empty (with a clear failure message), or making the test self-contained by creating a temporary role and ensuring it is added as a composite (via adminClient) before attempting the forbidden delete.

            Set<RoleRepresentation> composites = client.realm(masterRealm.getName()).roles()
                    .get(AdminRoles.ADMIN).getRoleComposites();
            int compositesBefore = composites.size();
            RoleRepresentation targetComposite = composites.iterator().next();

tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/InternalClientManagementTest.java:156

The test creates TEMP_ROLE_NAME but does not delete it. If the test fails mid-flight or the suite is rerun against a shared/dirty realm, this can cause interference (e.g., role already exists) and make the suite flaky. Add a cleanup step (preferably in a finally block inside runAs) to delete the temp role even when assertions fail.

            adminClient.realm(masterRealm.getName()).roles().create(
                    new RoleRepresentation(TEMP_ROLE_NAME, "temp", false));
            RoleRepresentation tempRole = adminClient.realm(masterRealm.getName()).roles()
                    .get(TEMP_ROLE_NAME).toRepresentation();

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/InternalClientManagementTest.java:136

composites.iterator().next() will throw NoSuchElementException if the admin role has no composites in a given environment/config. To make the test robust, explicitly create/attach a known composite (via adminClient) before selecting it, or assert the set is non-empty with a clear failure message before calling next().

            RoleRepresentation targetComposite = composites.iterator().next();

tests/base/src/test/java/org/keycloak/tests/admin/authz/rbac/InternalClientManagementTest.java:156

This test creates a realm role but doesn’t clean it up, which can cause cross-test pollution and intermittent failures when tests are re-run against the same realm. Consider deleting TEMP_ROLE_NAME in a finally block (or using existing test utilities/hooks for cleanup), and/or pre-deleting if it already exists before creation.

            adminClient.realm(masterRealm.getName()).roles().create(
                    new RoleRepresentation(TEMP_ROLE_NAME, "temp", false));
            RoleRepresentation tempRole = adminClient.realm(masterRealm.getName()).roles()
                    .get(TEMP_ROLE_NAME).toRepresentation();

Copilot AI review requested due to automatic review settings July 22, 2026 09:31

martin-kanis requested a review from a team as a code owner July 22, 2026 09:31

keycloak-github-bot Bot added the team/core-iam label Jul 22, 2026

Copilot started reviewing on behalf of martin-kanis July 22, 2026 09:33 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

keycloak-github-bot Bot added the flaky-test label Jul 22, 2026

vramik self-requested a review July 23, 2026 09:06

pedroigor approved these changes Jul 28, 2026

View reviewed changes

[CVE-2026-16105] Missing per-role authorization on RoleContainerResou…

bd85b9e

…rce composite endpoints Closes keycloak#51002 Signed-off-by: Martin Kanis <mkanis@ibm.com> # Conflicts: # services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java

pedroigor previously approved these changes Jul 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 28, 2026 11:27

pedroigor dismissed their stale review via bd85b9e July 28, 2026 11:27

pedroigor force-pushed the issue-51002 branch from eb364b9 to bd85b9e Compare July 28, 2026 11:27

pedroigor approved these changes Jul 28, 2026

View reviewed changes

Copilot AI reviewed Jul 28, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 28, 2026 11:43

Copilot AI reviewed Jul 28, 2026

View reviewed changes

pedroigor added the status/hold PR should not be merged. On hold for later. label Jul 28, 2026

Copilot started reviewing on behalf of pedroigor July 28, 2026 13:29 View session

Copilot started reviewing on behalf of martin-kanis July 28, 2026 13:44 View session

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CVE-2026-16105] Missing per-role authorization on RoleContainerResou… - #51084

[CVE-2026-16105] Missing per-role authorization on RoleContainerResou…#51084
martin-kanis wants to merge 1 commit into
keycloak:mainfrom
martin-kanis:issue-51002

martin-kanis commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

sguilhen commented Jul 23, 2026

Uh oh!

vramik commented Jul 23, 2026

Uh oh!

sguilhen commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

martin-kanis commented Jul 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sguilhen commented Jul 23, 2026

Uh oh!

vramik commented Jul 23, 2026

No view or manage scopes exist for roles in FGAP

Under classic RBAC, requireView(role) is equivalent to requireView(roleContainer)

Under classic RBAC, requireManage(role) differs from requireManage(roleContainer) only by the isRealmAdminRole guard

However, manage-realm is effectively equivalent to realm-admin (still to be implemented)

Uh oh!

sguilhen commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

No `view` or `manage` scopes exist for roles in FGAP

Under classic RBAC, `requireView(role)` is equivalent to `requireView(roleContainer)`

Under classic RBAC, `requireManage(role)` differs from `requireManage(roleContainer)` only by the `isRealmAdminRole` guard

However, `manage-realm` is effectively equivalent to `realm-admin` (still to be implemented)