OIDC: extract claims from JWT access token

[mmoayyed]

When OIDC OP produces an access token as JWT, pac4j must be able to extract those claims as well, after verifying the AT. Currently, only ID token claims and profile claims are collected into the profile.

[mmoayyed]

Updated PR with minor adjustments and tests.

[haster]

Hey all,

after upgrading to pac4j 5.4.4 we had some test failures, which we were eventually able to trace back to this pull request. After looking at it we think there are some issues with the current implementation.

1. Credentials without an accesstoken (only an idtoken), despite being valid in certain flows, result in an NPE wrapped in a TechnicalException being thrown due to line

   pac4j/pac4j-oidc/src/main/java/org/pac4j/oidc/profile/creator/OidcProfileCreator.java

   Line 156 in 778c6a2

   var accessTokenJwt = JWTParser.parse(credentials.getAccessToken().getValue());

2. Accesstokens do not have to be JWTs. In fact, by default there is no restriction on the format of an accesstoken. Accesstokens can be JWTs, but in our opinion it is only valid to use the claims from accesstokens supplied by IDPs that implement RFC 9068. Such tokens should be validated in accordance with said RFC, specifically chapter 4: https://www.rfc-editor.org/rfc/rfc9068#name-validating-jwt-access-token

3. The accesstoken is currently validated by the TokenValidator returned by configuration.findTokenValidator(), which is explicitly a validator for IDTokens and which seems ill suited to accesstokens. This should probably be replaced by some kind of logic validating accesstokens in accordance with RFC 9068.

This feature seems a bit large for such a minor upgrade (5.4.3 -> 5.4.4) and due to the above issues, we think that it might be better to revert this pull request and reimplement the feature at a later date.

[mmoayyed]

Thanks much for the details. Reverting is not possible; we have this feature used in production in 1-2 places. You're welcome to submit a pull request to make improvements.

[mmoayyed]

Also, please do not comment on merged pull requests. There are mailing lists reserved for discussions and evaluations, if necessary (which sounds like it might not be in this case, and we just need to apply 1-2 patches from you)

[leleuj]

1. OK. It's just a NPE on getAccessToken().getValue(), right? Can you test with 5.4.5-SNAPSHOT? It should be fixed.

2. and 3. What are the difference(s) between the ID token validation and the access token validation? We may end up with two different validation processes but I found it was a good first implementation to rely on the IDToken validation. If things are missing, please submit a PR.

[leleuj]

Reading https://www.rfc-editor.org/rfc/rfc8725, a security risk could be JWT substitution attacks.

I don't see how this could be possible between clients given that the ID token validation (https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation) checks the iss and aud claims as well as the signature, all being different between clients.
But maybe for the implicit flow, we could try to rewrite URLs and pass the ID token instead of the access token.

@mmoayyed for JWT access tokens generated by the CAS server, does it respect that the typ header value is "at+jwt" or "application/at+jwt"?

[leleuj]

@mmoayyed I just did the test by myself ;-) and it's not the case (the type is "JWT"): we should certainly change that for the incoming CAS v6.6.0? What do you think?

[mmoayyed]

That sounds like a good idea, sure.

[papegaaij]

@leleuj The validation of access tokens according to RFC 9068 (https://www.rfc-editor.org/rfc/rfc9068.html#name-validating-jwt-access-token) and ID tokens according to the OIDC specification (https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation) is quite different. To summarize:

• Access tokens must have a typ header containing at+jwt, ID tokens don't.
• The aud claim for an access token must contain a resource indicator value corresponding to an identifier the resource server expects for itself, for ID tokens it must contain the client_id.
• Access tokens don't specify the azp claim.
• ID tokens must contain the nonce, if sent in the authorization request, access tokens don't.
• ID tokens can contain an auth_time, which must be verified when a max_age was requested. Access tokens do not have this claim.

[leleuj]

@papegaaij Thanks for summarizing both validation processes. Very clear.

It looks like we will need a specific access token validation eventually.

For the ID token validation, we rely on the com.nimbusds.openid.connect.sdk.validators.IDTokenValidator class from the Nimbus OAuth OIDC SDK.
For the access token validation, I only see the com.nimbusds.openid.connect.sdk.validators.AccessTokenValidator class which only checks the hash.

The RFC 9068 is not currently supported: https://connect2id.com/products/nimbus-oauth-openid-connect-sdk#specs / https://bitbucket.org/connect2id/oauth-2.0-sdk-with-openid-connect-extensions/issues?status=new&status=open

I think it should be implemented in the Nimbus SDK instead of pac4j.

What do you think?

[papegaaij]

Yes, that would make sense. However, supporting RFC 9068 without the Nimbus SDK providing a specific implementation it, isn't very hard. For pac4j, being a client library, only the part about validation would apply. All the components needed to implement this validation are already included in the Nimbus SDK.

[leleuj]

The current implementation does not comply to RFC 9068, but it works for the JWT access tokens generated by the CAS server (they have the "nonce" claim for example).
I will keep it "as is" as long as it does not bring any security concern or break things in other use cases.

I will cut the 5.4.5 release to fix the NPE.

If someone needs a true RFC9068 validation, a PR will be welcome.

[papegaaij]

The risk lies in the fact that a consumer of a profile has no way of knowing whether a profile attribute originates from the verifiable and validated ID token or was just some random claim from the access token. The OAuth2 spec states that access tokens should be opaque to the client; that is, the client should not make any assumptions on the format or contents of such a token. RFC 9068 (https://www.rfc-editor.org/rfc/rfc9068.html#name-privacy-considerations) goes even further in this regard:

The client MUST NOT inspect the content of the access token: the authorization server and the resource server might decide to change the token format at any time (for example, by switching from this profile to opaque tokens); hence, any logic in the client relying on the ability to read the access token content would break without recourse. The OAuth 2.0 framework assumes that access tokens are treated as opaque by clients. Administrators of authorization servers should also take into account that the content of an access token is visible to the client. Whenever client access to the access token content presents privacy issues for a given scenario, the authorization server needs to take explicit steps to prevent them.

Therefore any RFC 9068 compliant client implementation MUST NOT inspect the contents of the access token.

The JWT RFC 7519 describes the validation of the aud claim. Each principal intended to process the JWT MUST identify itself with a value in the audience claim. RFC 9068 states that access tokens should use the resource URI of the resource server for this claim. This can never be an identifier for the client. Thus, an authorization server correctly implementing RFC 9068 will never produce access tokens that pass JWT validation of a client. This is caused by the simple fact that the access token is intended to be consumed by the resource server, not the client. The resource server is the audience of the token and the client must not inspect it. To pass claims to the client, either the ID token or userinfo endpoint should be used.

[mmoayyed]

Access tokens can be issued as JWTs. They are not always opaque and they can be made into a JWT using a variety of ways, supported by many IDPs and in many cases required by many frameworks and consumed in a variety of ways including client applications that may or may not be treated as resource servers. You can customize this bit as you like with options and settings, but the overall core functionality must remain in place, and there is no literature that would invalidate this, as evidenced by many many certified OIDC implementations.

[mmoayyed]

To clarify things better:

• If you'd rather work with an IDP that does not produce access tokens as JWT, you're of course welcome to do so.
• If you would rather always work with access tokens that are opaque and not JWTs and yet your IDP is doing so, you're welcome to have a discussion with the IDP and teach them about your requirements.
• If you have an IDP that produces opaque access tokens and not JWTs, the code here should simply ignore that and move on. (with a few changes that Jérôme has made)
• If you have an IDP that produces JWTs for access tokens and you'd like the processing of claims inside the access token to stop for whatever reason, you're welcome to send a PR that makes this bit optional.

[papegaaij]

@mmoayyed It is true that access tokens can be JWTs, that's what RFC 9068 describes. However, even though the client is technically able to inspect such a token, RFC 9068 clearly states that it MUST NOT do so. It must still treat the token as opaque. The OAuth2.1 draft also explains this: The string is considered opaque to the client, even if it has a structure.

IMHO pac4j should not enable such processing by default. I do not have an issue with pac4j providing functionality to do this processing, but it should be disabled by default. Processing of claims in an access token should only be done if there is an agreement between the client and the authorization server about the format of the token and the presence of certain claims. And in that case, the client must also perform all validation required by the JWT spec (which includes checking the audience of the token). I can prepare a PR to make this processing optional and disabled by default.

[mmoayyed]

I have no problem with making this behavior optional and making it be the default. Please do so with a PR. Thank you.