Add actor to impersonation tokens and details to event logs - #51492
Conversation
abstractj
left a comment
There was a problem hiding this comment.
Two things I noticed while reviewing:
- act claim placement relative to protocol mappers
setActClaimFromImpersonator runs before the mapper transformation pipeline — inside initToken for the access token and before transformIDToken for the ID token.
This means a custom protocol mapper could overwrite or remove the act claim, which would break the audit trail. The release notes say the claim "is always present and cannot be disabled," so it might be worth moving the call to after transformAccessToken / transformIDToken to make sure mappers can't interfere with it.
- act.sub inconsistency with the introspection endpoint
The new code sets act.sub to the impersonator's user ID, but the pre-existing introspection provider in AccessTokenIntrospectionProvider sets act.sub to the username.
A resource server decoding the JWT sees a UUID, but calling introspection for the same token sees a username. Since the JWT didn't have an act claim before, there was nothing to be inconsistent with, but now there is.
It would be worth aligning both to use the same identifier so consumers get a consistent contract regardless of how they validate the token.
Neither of these is a deal breaker to merge, but aligning them now would be cleaner than fixing them later.
rmartinc
left a comment
There was a problem hiding this comment.
Thanks @mabartos! It's a good idea IMO. The only thing that maybe it's better to use a mapper (like the sub one) in the basic client scope instead of using direct mapping. We will need an upgrade class to add the mapper to the basic scope and probably to also modify the introspection part. Just an idea but maybe it's better integrated.
|
@abstractj @rmartinc Thanks for your inputs!
Nice catch! Done.
Done. We just go the standard way the same as with the TE delegation (act.sub:uuid, act.preffered_username:username) and added a small notable change for the 26.8 release.
As discussed in the offline thread, we're going the way of hardcoded |
Closes keycloak#51489 Signed-off-by: Martin Bartoš <mabartos@redhat.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (6)
services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java:1409
- The linked issue explicitly lists adding
actto ID tokens as a non-goal, but this adds it to every generated ID token for an impersonation session. Limit the new claim to access tokens and update the corresponding test and release documentation accordingly.
setActClaimFromImpersonator(idToken, userSession);
server-spi-private/src/main/java/org/keycloak/events/EventBuilder.java:150
- Enrichment only occurs for callers using the model overload, but several token-event paths still load a
UserSessionModeland retain the string overload—for example introspection (AccessTokenIntrospectionProvider.java:201,209), user info (UserInfoEndpoint.java:247,254), and revocation (TokenRevocationEndpoint.java:130,241). Those events therefore still omit impersonator details, contrary to the stated complete audit trail; route their validated models through this overload too.
String impersonatorId = session.getNote(ImpersonationSessionNote.IMPERSONATOR_ID.toString());
if (StringUtil.isNotBlank(impersonatorId)) {
detail(Details.IMPERSONATOR_ID, impersonatorId);
detail(Details.IMPERSONATOR, session.getNote(ImpersonationSessionNote.IMPERSONATOR_USERNAME.toString()));
services/src/main/java/org/keycloak/protocol/oidc/grants/device/DeviceGrantType.java:312
- This delays recording the session until after lookup and user resolution, so failures such as
USER_NOT_FOUNDno longer retain the session ID that was previously recorded before lookup. Keep the string session ID for early error events, then call the model overload after successful lookup to add impersonator details.
event.session(userSession);
tests/base/src/test/java/org/keycloak/tests/admin/ImpersonationTest.java:368
- This test enforces an
actclaim in the ID token even though the linked issue explicitly scopes the feature to access tokens and names ID tokens as a non-goal. Remove the ID-token assertions when limiting the implementation to access tokens.
IDToken idToken = oauth.verifyToken(tokenResponse.getIdToken(), IDToken.class);
Map<String, Object> idTokenAct = (Map<String, Object>) idToken.getOtherClaims().get("act");
assertThat(idTokenAct, notNullValue());
assertThat((String) idTokenAct.get("preferred_username"), is(expectedUsername));
assertThat((String) idTokenAct.get("sub"), notNullValue());
docs/documentation/release_notes/topics/26_8_0.adoc:10
- The linked issue explicitly excludes ID tokens from this change, so documenting
actas present in ID tokens describes behavior outside the accepted scope. Document access tokens only, consistent with the stated RFC 8693 goal.
Additionally, tokens issued from impersonation sessions now include the `act` (actor) claim (https://datatracker.ietf.org/doc/html/rfc8693#section-4.1[RFC 8693 Section 4.1]) in both access tokens and ID tokens. This allows downstream resource servers to identify the impersonator for audit purposes. The claim is always present and cannot be disabled.
docs/documentation/upgrading/topics/changes/changes-26_8_0.adoc:43
- This migration note says the new JWT claim is in ID tokens, but the linked issue explicitly limits it to access tokens. Remove the ID-token reference so consumers are not told to rely on out-of-scope behavior.
The token introspection endpoint previously set the `act.sub` field to the impersonator's username. It now uses the impersonator's user ID to be consistent with the `act` claim in JWT access tokens and ID tokens. The `preferred_username` field has been added to the introspection `act` object for consumers that need the impersonator's username.
|
Should be ready for a review |
Demo:
Screencast.From.2026-08-06.12-43-03.mp4