Expose role enrichment helper for cross-client authorization evaluation by hakdogan · Pull Request #49049 · keycloak/keycloak

hakdogan · 2026-05-17T10:04:52Z

Summary

Extracts the role enrichment loop from PolicyEvaluationService.createIdentity() into a public static helper, org.keycloak.authorization.common.TokenIdentityEnricher.addAllUserRoles(AccessToken, UserModel). PolicyEvaluationService now delegates to the helper. The change is additive — no existing public API is modified.

The motivation, scope, and the rationale for choosing a minimum-surface helper over the originally proposed fromUser(...) factory are in the linked issue.

Changes

File	Change
`services/.../common/TokenIdentityEnricher.java`	New public static helper with Javadoc and null-argument guards
`services/.../admin/PolicyEvaluationService.java`	`createIdentity()` delegates the role-projection loop to the helper; admin-console behavior preserved
`services/.../common/TokenIdentityEnricherTest.java`	New unit tests covering the null-argument contract
`testsuite/.../authz/KeycloakIdentityCrossClientRoleTest.java`	New integration tests: admin-console parity + helper proof end-to-end

Test plan

Verified locally with the auth-server-quarkus profile:

./mvnw test -pl services -Dtest=TokenIdentityEnricherTest — 2/2 pass
./mvnw test -Pauth-server-quarkus -pl testsuite/integration-arquillian/tests/base -Dtest=KeycloakIdentityCrossClientRoleTest,PolicyEvaluationCompositeRoleTest,PolicyEvaluationTest -Ddocker.database.skip=true -Ddocker.infinispan.skip=true
- KeycloakIdentityCrossClientRoleTest: 2/2 pass (admin-console parity + enriched-helper proof)
- PolicyEvaluationCompositeRoleTest: 1/1 pass (regression guard)
- PolicyEvaluationTest: 14/14 pass (regression guard)
./mvnw spotless:check — clean

Copilot

Pull request overview

Exposes reusable token role enrichment for cross-client authorization while preserving admin-console evaluation behavior.

Changes:

Adds TokenIdentityEnricher.addAllUserRoles.
Delegates policy evaluation enrichment to the helper.
Adds unit and integration coverage.

Reviewed changes

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

File	Description
`TokenIdentityEnricher.java`	Implements role projection.
`PolicyEvaluationService.java`	Delegates existing enrichment.
`TokenIdentityEnricherTest.java`	Tests null arguments.
`KeycloakIdentityCrossClientRoleTest.java`	Tests cross-client evaluation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

hakdogan · 2026-08-12T09:13:12Z

+        String roleValues = "[{\"id\":\"" + role.getId() + "\",\"required\": true}]";
+        Map<String, String> config = new HashMap<>();
+        config.put("roles", roleValues);
+        config.put("fetchRoles", Boolean.TRUE.toString());


Good catch — fixed in the latest push.

fetchRoles is gone from the role policy, so RolePolicyProvider no longer reloads the subject and now resolves the role from the identity's token claims, which is the path the helper actually feeds.

To make the coverage self-checking rather than merely passing, I also added a negative test, plainTokenIdentity_missesCrossClientRole_yieldsDeny: an identity built from the raw client token is denied, because the client-b role never reaches the token under fullScopeAllowed=false. Together with the existing PERMIT test, removing or breaking addAllUserRoles now turns the pair red.

All three tests pass locally against the embedded Undertow auth server.

hakdogan · 2026-08-12T09:13:14Z

+            throw new IllegalArgumentException("user must not be null");
+        }
+
+        user.getRoleMappingsStream().forEach(roleModel -> {


The observation is correct: getRoleMappingsStream() returns direct assignments only, while token issuance projects RoleUtils.getDeepUserRoleMappings(user).

I have deliberately not changed the semantics here, because this PR is a behavior-preserving extraction: the loop is lifted verbatim out of PolicyEvaluationService.createIdentity(), where it has always used the direct mappings. Switching to the deep mappings would change what the admin console's Evaluate tab reports for group- and composite-derived roles — a user-visible behavior change that seems out of scope for a refactoring PR and one I would not want to slip in unannounced.

What I did change is the promise: the class and method Javadoc now state that only directly assigned mappings are projected, and point at RoleUtils#getDeepUserRoleMappings(UserModel) as the deeper alternative used during token issuance, so the helper no longer over-promises.

That leaves a real question for maintainers: should the admin-console enrichment (and therefore this helper) project deep role mappings? If yes, I am happy to do it in this PR, with coverage for both the group and the composite inheritance path; if you would rather keep this one purely mechanical, I will open a follow-up issue for it. Leaving this thread open until there is a call either way.

hakdogan · 2026-08-12T09:13:37Z

Rebased onto current main and pushed the review fixes. Three things worth calling out:

The tests now actually exercise the enrichment. The role policy no longer sets fetchRoles, and a negative test (plainTokenIdentity_missesCrossClientRole_yieldsDeny) pins the gap itself, so the pair fails if addAllUserRoles regresses. Details in the thread.
The helper's Javadoc no longer over-promises. It now states that only directly assigned role mappings are projected — matching the behavior the extraction preserves — and points at RoleUtils#getDeepUserRoleMappings(UserModel). Whether the admin-console enrichment should move to deep mappings is a genuine question I have left open for maintainers in the corresponding thread.
setup() now associates the scope with the resource before creating the scope permission. This is required after Validate scopes are associated with resources in scope permissions #50181 (merged on 11 Aug): the server now rejects a scope permission whose scope is not associated with any of the permission's resources, so without it the setup fails with ModelValidationException. This surfaced only on the rebase — worth knowing for any other older PR carrying a similar test fixture.

Verified locally against the embedded Undertow auth server: KeycloakIdentityCrossClientRoleTest 3/3 green.

This PR has been open since May without a human review — a look from team/core-iam would be much appreciated, including on the scoping question in point 2.

A KeycloakIdentity constructed from a bearer access token derives its role attributes from the token's realm_access and resource_access claims. When a resource is protected by a policy referencing a role defined in a different client, and that client is not part of the requesting client's scope, the role is absent from the token and the policy evaluates to DENY even when the user has been granted the role. PolicyEvaluationService.createIdentity() already compensates for this internally by projecting all of the user's role mappings onto the access token before constructing the identity. This commit promotes that enrichment to a public utility, TokenIdentityEnricher.addAllUserRoles(token, user), located alongside KeycloakIdentity in the services module. PolicyEvaluationService now delegates to the same helper, eliminating the duplication noted in discussion keycloak#46661. Closes keycloak#49048 Signed-off-by: Hüseyin Akdoğan <huseyin@keymate.io>

Copilot

Pull request overview

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

Suppressed comments (2)

services/src/main/java/org/keycloak/authorization/common/TokenIdentityEnricher.java:90

No test exercises the realm-role path or the new null-realm_access initialization here; the functional integration scenario assigns only CLIENT_B_ROLE. Add a case with a directly assigned realm role and an AccessToken lacking realm access so this public contract is regression-protected.

                AccessToken.Access realmAccess = token.getRealmAccess();
                if (realmAccess == null) {
                    realmAccess = new AccessToken.Access();
                    token.setRealmAccess(realmAccess);

tests/base/src/test/java/org/keycloak/tests/authz/services/KeycloakIdentityCrossClientRoleTest.java:96

The test plan claims this regression test was run with -pl testsuite/integration-arquillian/tests/base, but this class is in the separate tests/base module and the legacy module does not depend on or scan it. Update the verification command and results to target tests/base; the documented command did not execute this class.

@KeycloakIntegrationTest
public class KeycloakIdentityCrossClientRoleTest {

hakdogan · 2026-08-12T09:37:03Z

Migrated the integration test out of the deprecated testsuite, which the Testsuite Deprecation Check was rightly failing on (adding a new file under testsuite/ is forbidden). It now lives at tests/base/src/test/java/org/keycloak/tests/authz/services/KeycloakIdentityCrossClientRoleTest.java and is written against the new test framework:

@KeycloakIntegrationTest with @InjectRealm(config = ...), so the two clients, the client role and the user are declared through ClientBuilder/UserBuilder instead of being created imperatively;
@InjectRunOnServer for the two server-side evaluations, replacing testingClient.server().run(...);
realm.admin() for the admin-console evaluation path.

The authorization artifacts (resource server, scope, resource, role policy, scope permission) are still created server-side in a @BeforeEach, guarded to stay idempotent.

Verified locally: mvn -pl tests/base test -Dtest=KeycloakIdentityCrossClientRoleTest → 3/3 green. Testsuite Deprecation Check is green on this head.

hakdogan requested review from a team as code owners May 17, 2026 10:04

keycloak-github-bot Bot added team/cloud-native team/core-clients team/core-iam labels May 17, 2026

stianst removed team/cloud-native team/core-clients labels May 21, 2026

hakdogan force-pushed the tokenidentityenricher-helper branch from 06352da to 08471ca Compare June 15, 2026 07:26

jordandunmire97-ai approved these changes Jun 28, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings August 11, 2026 06:54

Copilot started reviewing on behalf of hakdogan August 11, 2026 06:55 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

hakdogan force-pushed the tokenidentityenricher-helper branch from 0e0a3a3 to a8c0089 Compare August 12, 2026 09:11

Copilot AI review requested due to automatic review settings August 12, 2026 09:26

hakdogan force-pushed the tokenidentityenricher-helper branch from a8c0089 to 2ee1bc7 Compare August 12, 2026 09:26

Copilot started reviewing on behalf of hakdogan August 12, 2026 09:29 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

Navigation Menu

Uh oh!

Expose role enrichment helper for cross-client authorization evaluation - #49049

Expose role enrichment helper for cross-client authorization evaluation#49049
hakdogan wants to merge 1 commit into
keycloak:mainfrom
hakdogan:tokenidentityenricher-helper

hakdogan commented May 17, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

hakdogan Aug 12, 2026

Uh oh!

hakdogan Aug 12, 2026

Uh oh!

hakdogan commented Aug 12, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

hakdogan commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

hakdogan commented May 17, 2026

Summary

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

hakdogan Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

hakdogan Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

hakdogan commented Aug 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

hakdogan commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants