[#243] Do not check the id_token nonce in getProfile for OAUTH providers

[vharseko]

Fixes #243

The problem

OAuthHttpClient.getProfile, which social registration and account linking use, checked the nonce
of any id_token the token endpoint returned, whatever the provider type. getAuthToken, which login
uses, checks it only for OPENID_CONNECT (OAuthHttpClient.java:215-225). checkNonce also fails
when the claim is absent (:130).

So an OAUTH provider whose token endpoint returns an id_token without the request nonce passes
login but fails registration and account linking with 400 "Nonce provided does not match claim".
LinkedIn becomes such a provider with #231: its catalogue entry is OAUTH with the openid scope,
which /v2/userinfo needs, and Keycloak's LinkedIn OIDC provider turns the nonce check off because
LinkedIn does not echo the request nonce (raised in the review of #231).

The change

• getProfile skips the nonce check for OAUTH only (OAuthHttpClient.java:165-167). Every other
  type is still checked as before, so a null or hand-written type, which getAuthToken rejects with
  a 500 at login, keeps failing closed at registration and linking too. For an OAUTH provider the
  id_token claims are still read: they are the profile when the provider has no
  userinfo_endpoint, as before (:180).
• New OAuthHttpClientTest. The provider is an in-process Handler answering the token and userinfo
  endpoints; the id_token is signed with JwtBuilderFactory (HS256) and parsed by a real
  JwtReconstruction. It covers:
  • getProfile, OAUTH, id_token without nonce → the userinfo profile;
  • getProfile, OAUTH without a userinfo endpoint, id_token with another nonce → the profile
    from the id_token claims;
  • getProfile, OPENID_CONNECT, nonce missing or different → BadRequestException;
  • getProfile, OPENID_CONNECT, matching nonce → the profile from the id_token claims;
  • getProfile, a type that is neither (openid_connect), different nonce → BadRequestException;
  • getAuthToken: OAUTH → the access token; OPENID_CONNECT with a missing nonce →
    BadRequestException, with a matching nonce → the id_token itself, to pin that login is
    unchanged.

Verification

• Before the fix testGetProfileIgnoresNonceForOAuth failed with
  BadRequest Nonce provided does not match claim.
• After the fix mvn -o -pl openidm-identity-provider test: 13 tests, 0 failures
  (the nine in OAuthHttpClientTest and IdentityProviderServiceTest).
• Six mutants each turn exactly one OAuthHttpClientTest case red: in getProfile, nulling the
  claims for OAUTH, reading the claims only inside the guard, checking the nonce for OAUTH
  whenever the claim is present, and the earlier OPENID_CONNECT.equals(...) guard; in
  getAuthToken, checking against a null nonce and an unconditional 400 on the OpenID Connect
  branch.

Not verified: a real registration against a LinkedIn application (needs a registered app).

[maximthomas]

praise: The fix is in the right place, and a test that fails without it pins it.

• getProfile now applies the same OPENID_CONNECT test as getAuthToken (OAuthHttpClient.java:164, :214-220), so registration, linking and login agree on when the nonce is required.
• testGetProfileIgnoresNonceForOAuth goes red under the BASE behaviour (unconditional checkNonce). The in-process Handler with an HS256 id_token and a real JwtReconstruction exercises the real parsing path offline.

---

suggestion (non-blocking): No test covers an OAUTH provider without a userinfo_endpoint, though the description says that road is preserved.

openidm-identity-provider/src/test/java/org/forgerock/openidm/idp/client/OAuthHttpClientTest.java:53-60, openidm-identity-provider/src/main/java/org/forgerock/openidm/idp/client/OAuthHttpClient.java:160-181

The only OAUTH getProfile case sets USERINFO_ENDPOINT and an id_token with no nonce claim, so the profile always comes from userinfo. Only OPENID_CONNECT cases reach the id_token-claims-as-profile road at :177. On a HEAD export three mutants stay green: nulling jwtClaimSet for OAUTH (7/7), moving getClaims inside the OPENID_CONNECT guard (6/6), and checking the nonce for OAUTH whenever the claim is present (6/6). Under the first two, such a provider gets 500 "No means available for getting profile data". Under the third, a provider that returns some other nonce gets back the 400 this PR fixes.

@Test
public void testGetProfileReadsIdTokenClaimsForOAuthWithoutUserInfo() throws Exception {
    final OAuthHttpClient client = newClient("OAUTH", null, idToken("other-nonce"));

    final JsonValue profile = client.getProfile(new JwtReconstruction(), "code", NONCE, REDIRECT_URI);

    assertThat(profile.get("sub").asString()).isEqualTo("id-token-subject");
}

Pin: a mismatched nonce and no userinfo endpoint. The case fails at BASE and under all three mutants (traced, not run).

---

suggestion (non-blocking): No getAuthToken case covers a successful OpenID Connect login.

openidm-identity-provider/src/test/java/org/forgerock/openidm/idp/client/OAuthHttpClientTest.java:85-98, openidm-identity-provider/src/main/java/org/forgerock/openidm/idp/client/OAuthHttpClient.java:217-220

The description says the getAuthToken cases "pin that login is unchanged", but the only OPENID_CONNECT case is a rejection. Two mutants stay green (6/6) on a HEAD export: passing null as the nonce at :219, and an unconditional BadRequestException on that branch. Both make every OIDC login fail with 400.

@Test
public void testGetAuthTokenAcceptsMatchingNonceForOpenIdConnect() throws Exception {
    final String idToken = idToken(NONCE);
    final OAuthHttpClient client = newClient("OPENID_CONNECT", USERINFO_ENDPOINT, idToken);

    assertThat(client.getAuthToken(new JwtReconstruction(), "code", NONCE, REDIRECT_URI).getOrThrow())
            .isEqualTo(idToken);
}

Pin: with a matching nonce, getAuthToken returns the id_token itself. Both mutants turn this case red (traced, not run).

---

suggestion (non-blocking): Exempt OAUTH rather than require OPENID_CONNECT, so that an unknown type keeps the nonce check.

openidm-identity-provider/src/main/java/org/forgerock/openidm/idp/client/OAuthHttpClient.java:164

OPENID_CONNECT.equals(config.getType()) skips checkNonce for every type except that exact string, including a null type and a hand-written "openid_connect". BASE checked those, and getAuthToken's default branch (:221-223) rejects them with a 500. So on such a type login fails closed, while registration and linking now fail open. I did not show that such a provider can reach getProfile: AuthenticationService rejects non-enum types when SOCIAL_PROVIDERS is configured, and the Admin UI writes only the two known types. With the narrower guard all seven tests stay green.

// every provider type but OAUTH must echo the request nonce
if (!OAUTH.equals(config.getType())) {
    checkNonce(jwtClaimSet, nonce);
}

[vharseko]

@maximthomas all three suggestions are taken in 6d779c8; the branch is also rebased onto the current master.

OAUTH without a userinfo_endpoint. Added testGetProfileReadsIdTokenClaimsForOAuthWithoutUserInfo as you proposed: an OAUTH provider with no userinfo endpoint and an id_token carrying another nonce gets its profile from the id_token claims. Your three mutants (nulling the claims for OAUTH, reading them only inside the guard, checking the nonce for OAUTH whenever the claim is present) each turn it red: run, not just traced.

Successful OpenID Connect login. Added testGetAuthTokenAcceptsMatchingNonceForOpenIdConnect: with a matching nonce getAuthToken returns the id_token itself. Both mutants (a null nonce at the checkNonce call, an unconditional 400 on that branch) turn it red.

Exempt OAUTH rather than require OPENID_CONNECT. Agreed: the guard is now !OAUTH.equals(config.getType()) (OAuthHttpClient.java:165), so only OAUTH changes against the old behaviour and any other type string keeps the nonce check at registration and linking. testGetProfileRejectsWrongNonceForUnknownType pins it with openid_connect and a different nonce; it fails under the previous OPENID_CONNECT.equals(...) guard. The PR title and description now describe the exemption instead of the requirement.

mvn -o -pl openidm-identity-provider test: 13 tests, 0 failures.

[maximthomas]

praise: All three round-1 items are addressed, and the guard now fails closed for every type except the one the PR targets.

• getProfile checks the nonce under if (!OAUTH.equals(config.getType())) (OAuthHttpClient.java:165), so null and hand-written types still get the nonce check on registration and linking.
• testGetProfileReadsIdTokenClaimsForOAuthWithoutUserInfo and testGetAuthTokenAcceptsMatchingNonceForOpenIdConnect cover the two roads that had no case in round 1.