Fix Device Code session propagation and OIDC claims

[dairoca90]

Fix Device Code session propagation and OIDC claims

Description

This PR updates the Device Code flow to improve session propagation and OpenID Connect claim handling.

Changes

• Propagated the user Session so it is available in the Access Token Modifier Script. Previously, the session was not exposed to the script.
• Added support for including the acr and amr values in Device Code tokens.
• Updated the Device Code response so that when the openid scope is requested, the OIDC-related information is included when a valid user session exists at the time the Device Code is generated.
• Ensured the session information is preserved throughout the Device Code flow so it can be consumed by downstream token customization logic.

Result

The Device Code flow now provides the necessary session context for token modification and correctly propagates acr and amr authentication information when applicable.

[maximthomas]

Diagnosis looks right - the flow really wasn't issuing an id_token. I'd drop the session restore though; I don't think it's needed, and it brings problems.

Why it breaks:

• isValidToken at 120 guards only request.setSession(). On failure: warn, then fall through to additionalDataToReturnFromTokenEndpoint at 150 anyway.
• Line 121 is dead weight regardless. OpenIDTokenIssuer:80 overwrites the request session with accessToken.getSessionId() -> extraData["ssoTokenId"], i.e. line 141.
• Session gone -> validate() with null session -> ResourceOwnerSessionValidator:207 -> ResourceOwnerAuthenticationRequired -> ServerException -> 500. finally at 164 already deleted the device code, so no retry - the user restarts the whole authorization.
• Client with Default max age set: ResourceOwnerSessionValidator:188 -> authenticationRequired(request, token) -> destroyToken() at :345. A back-channel poll logs the user out of their browser. Deterministic, not a race.
• setSessionId() is new here, so device codes authorized by the currently deployed build have a null sessionId and 500 on first poll after upgrade.

The flow doesn't need a session. ResourceOwnerSessionValidator:201-206 already handles this for password/client_credentials - suggest adding the grant type:

} else if (TokenEndpoint.PASSWORD.equals(request.getParameter(GRANT_TYPE))
        || TokenEndpoint.CLIENT_CREDENTIALS.equals(request.getParameter(GRANT_TYPE))
        || TokenEndpoint.DEVICE_CODE.equals(request.getParameter(GRANT_TYPE))) {
    return getResourceOwner(request.getToken(AccessToken.class));
}

Everything it needs is already there. Access token is on the request by line 150 (StatefulTokenStore:566, StatelessTokenStore:305). auth_time is on the device code - DeviceCode.setAuthorized() stamps AUTH_INSTANT, GrantTypeAccessTokenGenerator:68-72 reads it. Both on master already, nothing exercises them yet.

The block then collapses to:

AccessToken accessToken = accessTokenGenerator.generateAccessToken(providerSettings, grantType,
        clientId, resourceOwnerId, null, scope, validatedClaims, null,
        deviceCode.getNonce(), request);

providerSettings.additionalDataToReturnFromTokenEndpoint(accessToken, request);
return accessToken;

That drops the SSOTokenManager dep, the ssoTokenId set/unset dance, and the second isValidToken at 140 - that one resets session idle time, and a device poll shouldn't extend the user's browser session.

Also fixes the nonce: addExtraData(NONCE, ...) at 135 is never read. OpenIDTokenIssuer reads accessToken.getNonce(), set from the nonce arg to createAccessToken - currently null, so the id_token ships without a nonce claim.

Minor: line 127 logs the raw SSO token ID - worth dropping.

Two things this doesn't cover:

• amr/acr still don't reach the id_token on a default install - OpenAMTokenStore.createOpenIDToken (:78-82) doesn't route on statelessCheck. Details in the comment on StatelessTokenStore:250. I'd keep it in this PR; once the resource owner comes from the access token it's a two-line addition rather than a port.
• getOps() -> accessToken.getSessionId() -> null, so no ops claim and no OIDC session management on device tokens. Probably correct for this flow - a TV token shouldn't die when the user closes a tab - but worth making deliberate. If you agree, deviceCode.setSessionId() becomes unused and can go.

Could we get a test for poll-after-session-expiry? That's the unrecoverable case.

[maximthomas]

Both DeviceCode branches - this one and createRefreshToken:559 - are stateless-only, and StatefulTokenStore didn't get an equivalent. I think that means amr/acr don't reach the id_token on a default install.

OpenAMTokenStore routes every method on statelessCheck.byRequest(request) except this one:

// OpenAMTokenStore:78-82
public OpenIdConnectToken createOpenIDToken(...) {
    return statefulTokenStore.createOpenIDToken(resourceOwner, clientId, authorizationParty, nonce, ops, request);
}

No branch - createOpenIDToken only exists on StatefulTokenStore; StatelessTokenStore implements TokenStore, not OpenIdConnectTokenStore. So the id_token is always built by the stateful store, and claims come from getAMRFromAuthModules:404 / getAuthenticationContextClassReference:432. Both are AuthorizationCode -> RefreshToken -> SSO-cookie ladders. Auth code is null on a device poll, so it comes down to the refresh token.

statelessTokensEnabled	issueRefreshToken	id_token amr/acr
false (default)	true (default)	absent - StatefulTokenStore.createRefreshToken:639 has no device branch, refresh token carries null
false	false	absent - :413 reads the cookie off the HTTP request; a device poll has none
true	true	present - the only path this PR exercises
true	false	absent - same :413 fallback

Defaults are statelessTokensEnabled=false (OAuth2Provider.xml:88) and issueRefreshToken=true (:134), so the working combination is the one nobody has out of the box.

Third condition on top: getAMRFromAuthModules:415-425 only emits amr when getAMRAuthModuleMappings() is non-empty, and forgerock-oauth2-provider-amr-mappings (OAuth2Provider.xml:552) is optional with no default. Pre-existing, but it means amr needs stateless and refresh tokens and configured mappings.

Relevant to the other comment: in the default config getAMRFromAuthModules takes the RefreshToken branch at :411 and never reaches :413, so the restored session contributes nothing to amr/acr. It only buys passage through ResourceOwnerSessionValidator.

Separate point in this method - :255 is if, not else if:

} else if (deviceCode != null) {      // :250
    authModules = deviceCode.getAuthModules();
    acr = deviceCode.getAcrValues();
}
if (currentRefreshToken != null) {    // :255 - overwrites unconditionally
    authModules = currentRefreshToken.getAuthModules();
    acr = currentRefreshToken.getAuthenticationContextClassReference();
}

GrantTypeAccessTokenGenerator:77-80 creates the refresh token first and puts it on the request, so :250 is always overwritten when refresh tokens are on. Same values today (the refresh token got them from the same DeviceCode at :559), so nothing breaks - but the access-token branch only runs with refresh off and the id_token fix only works with refresh on. The two halves never exercise together. else if would match the intent.

realm_access.roles staying stateless-only seems right - nowhere to put it on an opaque CTS token.

[maximthomas]

Hi @dairoca90
Thanks for the contribution! A few issues need to be addressed before this can be merged.

[maximthomas]

Hi @dairoca90,
Please resolve merge conflicts

[maximthomas]

Hi @dairoca90
Thanks for the quick fix!
Please see the PR feeback below:

The device-code acr/amr plumbing works. The problem is that the PR also renames the stateless JWT claim authModules → amr for all grants — not mentioned in the descriptio — and that rename breaks two things outside the device-code flow.

---

issue (blocking): amr is written under a key nothing reads back, so it is lost on every stateless refresh

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/StatelessTokenStore.java:581

// createRefreshToken — writes "amr"
claimsSetBuilder.claim("amr", getAMRFromAuthModules(authModules, providerSettings));

// StatelessRefreshToken.java:63-64 — reads "authModules". Untouched by this PR.
return jwt.getClaimsSet().getClaim(AUTH_MODULES, String.class);   // AUTH_MODULES == "authModules"

On grant_type=refresh_token, readRefreshToken (:719-726) rebuilds the token, createAccessToken
(:257) takes the REFRESH_TOKEN branch, getAuthModules() returns null, and the authModules != null
guard at :272 is skipped. The reissued access token has no amr, and :580 drops it from the reissued
refresh token too. amr survives zero refreshes. At base the write and read keys matched.

Suggest writing both keys for one release rather than only changing the read side — refresh tokens
minted before the upgrade are still in the wild.

---

issue (blocking): in a default install the authModules claim silently disappears from all stateless tokens

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/StatelessTokenStore.java:272-274 (same at :580-581)

if (authModules != null) {                                        // guards on the raw String…
    claimsSetBuilder.claim("amr", getAMRFromAuthModules(authModules, providerSettings));
}                                                                 // …but writes the mapped List

getAMRFromAuthModules returns null unless getAMRAuthModuleMappings() is non-empty. That mapping
is optional with no default — OAuth2Provider.xml:552-565 has <IsOptional/> and no <DefaultValues>,
and AgentOAuth2ProviderSettings:277 returns an empty map unconditionally. JWObject.put drops null
values, so the key is simply absent.

Net effect on a stock install: a token that carried "authModules":"DataStore" now carries neither
authModules nor amr. Affects authorization_code and refresh_token, not just device code.

Suggest falling back to the raw authModules value when the mapping is empty.

---

issue (blocking): one query parameter turns /oauth2/authorize into a 500 for anonymous callers

openam-oauth2/src/main/java/org/forgerock/oauth2/core/ResourceOwnerSessionValidator.java:203

} else if (/* password || client_credentials || */ DEVICE_CODE.equals(request.getParameter(GRANT_TYPE))) {
    return getResourceOwner(request.getToken(AccessToken.class));   // :207 — may be null
}
// :253
return new ResourceOwner(token.getResourceOwnerId(), ...);          // NPE, not an OAuth2Exception

OAuth2Request.getParameter falls back to query params (:93-101, :162), so:

GET /oauth2/authorize?client_id=X&response_type=code&redirect_uri=…&scope=openid
    &grant_type=urn:ietf:params:oauth:grant-type:device_code

with no SSO session reaches AuthorizationService:163 → validate() → this branch. Nothing sets an
AccessToken on an /authorize request, so it NPEs. Previously it redirected to login.

The token-endpoint path is fine — both stores call setToken(AccessToken.class, …) first. Suggest
gating the branch on the token actually being present.

---

issue (non-blocking): the script-facing amr changes type, breaking existing customer scripts

openam-oauth2/src/main/java/org/forgerock/openam/oauth2/StatelessTokenStore.java:294 (and :604)

accessTokenContext.put("amr", authModules);                        // base  — pipe-joined String
accessTokenContext.put("amr", getAMRFromAuthModules(authModules, providerSettings));  // head — List<String> or null

That map is bound as ScriptParams.ACCESS_TOKEN and read by access-token-modification.groovy via
getField("amr"), which returns the raw object with no coercion. A script doing amr.split("\\|")
gets a MissingMethodException at token-issuance time.

Needs a release note, and openam-scripting/src/main/groovy/access-token-modification.groovy (header
comment at line 30) still documents the old contract.

---

issue (non-blocking): a failed id_token leaves orphaned tokens and destroys the device code

openam-oauth2/src/main/java/org/forgerock/oauth2/core/DeviceCodeGrantTypeHandler.java:108

try {
    accessToken = generateAccessToken(...);                                          // :105 — already in CTS
    providerSettings.additionalDataToReturnFromTokenEndpoint(accessToken, request);  // :108 — can throw
    return accessToken;
} finally {
    if (deviceCode.isAuthorized() || ...) {
        tokenStore.deleteDeviceCode(clientId, code, request);                        // :118-124 — runs anyway
    }
}

createOpenIDToken throws ServerException on any CoreTokenException. Result for an openid-scoped
device grant during a CTS blip: access + refresh tokens live in CTS but never returned, device code
gone, client gets a 500, and every retry poll then reports invalid_grant (:90).

Widens a pre-existing window rather than creating one. Moving the call outside the try is enough.

---

question (blocking): was the Session propagation descoped, or dropped?

The first bullet of the description says the user Session is propagated to the Access Token
Modifier Script. I can't find it implemented. Three pieces of scaffolding, none wired up:

// StatelessTokenStore.java:208 — hoisted to method scope, still only assigned inside `if (authCode != null)`
String sessionId = null;

// StatelessTokenStore.java:894 — no call site in this class; sole reason for the new ServletUtils import
private String getAuthModulesFromSSOToken(OAuth2Request request) { ... }

accessTokenContext (:287-295) gains no session entry. What DeviceCodeVerificationResource actually
propagates is one String:

deviceCode.setAuthModules(token.getProperty(ISAuthConstants.AUTH_TYPE));

If it was descoped, could the description be trimmed and the dead method plus hoisted variable removed?
If it was meant to land here, the PR looks incomplete.

---

note (non-blocking): id_token is now issued for openid device grants unconditionally

OpenAMScopeValidator:492 issues on scope.contains(OPENID) alone — there is no check that a session
existed when the device code was generated, as the description states. Almost certainly fine in
practice; worth correcting the wording and noting the response-shape change for existing clients.

---

suggestion (non-blocking): two tests would pin the riskiest parts

Would you consider adding:

1. A mint-then-refresh case. The tests here stub RefreshToken directly, so they can't see what
   createRefreshToken wrote. Since it returns new StatelessRefreshToken(jwt, jwt.build())
   (StatelessTokenStore.java:622), asserting getAuthModules() on the returned object is enough to
   catch the first issue above — no readRefreshToken round-trip needed.
2. An empty AMR-mapping case. That's the shipped default, and the assertion that used to guard it
   (doesNotContainKey("authModules")) was removed with the rename.

The new device_code branches in both stores, the new branch in ResourceOwnerSessionValidator, and
the acr/session block in DeviceCodeVerificationResource have no coverage at all — a follow-up is fine.

---

nitpick (non-blocking): the new acr/session block is copy-pasted into both branches

openam-oauth2/src/main/java/org/forgerock/oauth2/restlet/DeviceCodeVerificationResource.java:174-179 and :191-196

deviceCode.setAcrValues(getAuthenticationContextClassReferenceFromRequest(request));
SSOToken token = resourceOwnerSessionValidator.getResourceOwnerSession(request);
if (token != null) {
    populateAuthenticationInfo(deviceCode, token);
}

Verbatim in both. Worth a private helper — the adjacent authorize block was already duplicated, so
this grows it from 3 to 7 lines.

---

Checked and found fine

• New authModules key on DeviceCode survives the CTS round-trip — OAuthAdapter serialises the
  whole JsonValue map into the blob, no field whitelist.
• acr recorded at verification is the matched value, not the raw requested acr_values;
  setCurrentAcr does run, and unmatched values become "0", consistent with authorization_code.
• No "amr": null is emitted — JWObject.put drops null values.
• The three modified assertions in StatelessTokenStoreTest hold at head.