OidcIdToken cannot be serialized to JSON if token contains claim of type JSONArray

[mengelbrecht]

Describe the bug
If an IdP sends an ID token with claim amr, the Jackson ObjectMapper with SecurityJackson2Modules cannot serialize the ID token to JSON (related: #4370).
The amr claim in the ID token has the type com.nimbusds.jose.shaded.json.JSONArray for which there is no default mixin.

Tested with Spring-Security 5.4.1.

To Reproduce
These steps resemble a normal oauth2Login configuration where additionally the ID token is serialized to JSON.

1. Include an amr claim in the ID token
2. Decode the string token value using an JwtDecoder created by OidcIdTokenDecoderFactory to a Jwt.
3. Create an OidcIdToken from the Jwt.
4. Serialize the OidcIdToken to a JSON string using an ObjectMapper with the SecurityJackson2Modules.

Expected behavior
The amr claim should be an ArrayList instead of JSONArray.

Workaround
Define a mixin for the JSONArray class.

[jgrandja]

Thanks for the report @mengelbrecht. The default converter for the amr claim is Collection<String> in OidcIdTokenDecoderFactory.createDefaultClaimTypeConverters().

Can you put together a test that reproduces the issue as I'm not seeing how the amr claim is decoded as a com.nimbusds.jose.shaded.json.JSONArray.

[mengelbrecht]

I could not create a test because the JWT has to be signed and the validator wants to fetch the jwks which fails in my test.

After a little more digging I could reproduce it using this Kotlin snippet. The code outputs com.nimbusds.jose.shaded.json.JSONArray as type for the converted object.

    val jsonArray = com.nimbusds.jose.shaded.json.JSONArray().apply { add("test") }
    val converted = ClaimConversionService.getSharedInstance().convert(
        jsonArray,
        TypeDescriptor.valueOf(Any::class.java),
        TypeDescriptor.collection(Collection::class.java, TypeDescriptor.valueOf(String::class.java))
    )
    println(converted.javaClass.name)

When NimbusJwtProcessor processes the parsed JWT the JWTClaimsSet contains the JSONArray for the amr claim:

spring-security/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java

Line 147 in 2abf59b

JWTClaimsSet jwtClaimsSet = this.jwtProcessor.process(parsedJwt, null);

Just as you mentioned the ObjectToListStringConverter then tries to convert it to a Collection<String> here:

spring-security/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/NimbusJwtDecoder.java

Line 149 in 2abf59b

Map<String, Object> claims = this.claimSetConverter.convert(jwtClaimsSet.getClaims());

However, since JSONArray subclasses List<Object> and the first element is of type String the converter decides that there is nothing to do and just returns the source which then still is a JSONArray:

spring-security/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java

Line 62 in ef3b4d4

return source;

[jgrandja]

Good catch @mengelbrecht !

So the issue is in ObjectToListStringConverter:

spring-security/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java

Line 62 in 2abf59b

return source;

Instead of returning the source as-is, it should instead convert to a new ArrayList<String>().

Would you be interested in submitting this fix?

[mengelbrecht]

@jgrandja unfortunately I don't have the time at the moment, sorry

[jgrandja]

No worries @mengelbrecht. Thanks for reporting this!

[ghost]

hi @jgrandja . I have some spare time and I can submit a PR for this today or tomorrow.

[jgrandja]

That would be great @ovidiupopa91. Thank you!

[jgrandja]

@ovidiupopa91 Looks like we could run into the same problem with ObjectToMapStringObjectConverter:

spring-security/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToMapStringObjectConverter.java

Line 57 in 2abf59b

return source;

It returns the Map as-is, which could be a com.nimbusds.jose.shaded.json.JSONObject. Could you also update this to return a new Map, as part of the PR. Thanks!