Skip to content

Serialize Non-Standard Claims On OidcIdToken To java.util Types Instead Of com.nimbusds.jose.shaded.json #12108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
adase11 opened this issue Oct 29, 2022 · 3 comments
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement

Comments

@adase11
Copy link

adase11 commented Oct 29, 2022

Expected Behavior
When converting the claims from the ID token in an OAuth2AccessTokenResponse it would be useful, for those that want to store the token in the session and use Jackson to serialize/deserialize the session, if any non-standard claims were converted to base java.util types, rather than left as com.nimbusds.jose.shaded.json types. I think this could be done in ClaimTypeConverter and MappedJwtClaimSetConverter by adjusting the loops in the convert methods.

Current Behavior
Currently non-standard claims are ignored and passed on in a map of Map<String, Object> and therefore retain the type they were given when the getJWTClaimsSet method calls payload.toJSONObject().

Context

This is similar to (or at least related to) #9210 but the current behavior is not a bug but rather is acting to the specification; however, I think this would be a useful way to treat non-standard claims which is why I'm proposing this as an enhancement.

Each of the implementations of com.nimbusds.jwt.JWT use getPayload().toJSONObject(); to get the com.nimbusds.jwt.JWTClaimsSet and ClaimTypeConverter (or MappedJwtClaimSetConverter ) converts only standard claims non-standard claims are not modified.
If/when the token is later serialized by Jackson, for example when using GenericJackson2JsonRedisSerializer with RedisIndexedSessionRepository), non-standard claims that are ArrayList or HashMap types retain their com.nimbusds.jose.shaded.json type (either JSONArray or JSONObject), this causes issues later with deserialization because they are not able to be deserialized by Jackson and SecurityJackson2Modules does not include a module for those types. I think this would be an easy change to make because JSONArray and JSONObject extend ArrayList and HashMap respectively.

Alternatively a module declaring serialization/deserialization strategies for the nimbusds types could be added to SecurityJackson2Modules.

@adase11 adase11 added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Oct 29, 2022
@sjohnr sjohnr added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Nov 2, 2022
@adase11
Copy link
Author

adase11 commented Dec 8, 2022

A quick update, it looks like depending on which version of nimbus-jose-jwt that's being used this could be either com.nimbusds.jose.shaded.json objects or com.nimbusds.jose.shaded.gson objects. I was using Spring Boot 2.7.6 (Spring Security 5.7) and getting nimbus-jose-jwt:jar:9.22 - now when upgrading to Spring Security 5.8 in preparation for a Spring Boot 3.0 upgrade I'm getting nimbus-jose-jwt:jar:9.24. It looks like the change from JSON Smart to GSon happened in v9.24 - see this commit - I'll revisit this once I'm on Spring Security v6.

@jgrandja
Copy link
Contributor

@adase11 We do not want to reference Nimbus libraries (com.nimbusds.*) in ClaimTypeConverter or MappedJwtClaimSetConverter.

When converting the claims from the ID token in an OAuth2AccessTokenResponse it would be useful, for those that want to store the token in the session and use Jackson to serialize/deserialize the session, if any non-standard claims were converted to base java.util types, rather than left as com.nimbusds.jose.shaded.json types

This would be the applications responsibility to register the appropriate mixins for any custom claim type. The framework cannot account for all cases of types and in reality would be a maintenance overhead. Case in point:

it looks like depending on which version of nimbus-jose-jwt that's being used this could be either com.nimbusds.jose.shaded.json objects or com.nimbusds.jose.shaded.gson objects. I was using Spring Boot 2.7.6 (Spring Security 5.7) and getting nimbus-jose-jwt:jar:9.22 - now when upgrading to Spring Security 5.8 in preparation for a Spring Boot 3.0 upgrade I'm getting nimbus-jose-jwt:jar:9.24. It looks like the change from JSON Smart to GSon happened in v9.24

I'm going to close this based on the explanation provided.

@jgrandja jgrandja added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 14, 2022
@adase11
Copy link
Author

adase11 commented Dec 15, 2022

Makes sense. Thanks for considering @jgrandja

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants