Skip to content

Decoded JWT Token results in invalid content in newer versions #12329

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
madduci opened this issue Dec 2, 2022 · 7 comments
Closed

Decoded JWT Token results in invalid content in newer versions #12329

madduci opened this issue Dec 2, 2022 · 7 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

Comments

@madduci
Copy link

madduci commented Dec 2, 2022

Describe the bug
We use Spring Security for OAuth2 login using Keycloak. Until Spring Security 5.7.5 the JWT Token validation has worked perfectly.
Since the update at 5.8.0, but also using the newer version, 6.0.0, it stopped working, producing an odd format.

Spring Security 5.7.5:
jwt.getClaimAsString("resource_access") produces {"notification-entry-service":{"roles":["disease-notification-sender"]}}

Spring Security 5.8.0+:
jwt.getClaimAsString("resource_access") produces {notification-entry-service={roles=[disease-notification-sender]}}

This happens by simply updating the dependency in the project, without touching/modifying the existing code.

To Reproduce
Setup Spring Security 5.8. 0with OAuth2 and JWT.

Expected behavior
The JWT parsing should not change behaviour (not even reported in the Changelog here - https://docs.spring.io/spring-security/reference/5.8/whats-new.html)

Sample

Could not be provided

@madduci madduci added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Dec 2, 2022
@madduci madduci changed the title JWT Claim has an invalid content Decoded JWT Token results in invalid content in newer versions Dec 2, 2022
@jzheaux jzheaux self-assigned this Dec 5, 2022
@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 5, 2022
@jzheaux jzheaux added this to the 5.8.1 milestone Dec 5, 2022
@jzheaux
Copy link
Contributor

jzheaux commented Dec 5, 2022

@madduci, thanks for the report. This feels like it could be a regression. I'll take a closer look shortly and prioritize any needed fixes for the next maintenance release.

@michaelweidmann
Copy link

michaelweidmann commented Dec 13, 2022

Hi @jzheaux,

we had a look on it and we found out that the issue is caused by a dependency upgrade from the com.nimbusds:nimbus-jose-jwt library (see this commit). The library switched the JSON parser from json-smart to GSON (see here).

The call to parse the token looks like this and from there on the library does its job (see here):

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

The stacktrace looks like this:
stacktrace nimbusds library

With json-smart a complex claim value was parsed to a JSONObject and with GSON the result is a LinkedTreeMap.

Up to 5.7:
source type spring security 5.7

From 5.8 on:
source type spring security 5.8 and upwards

This leads to the wrong conversion of the value from an object to the string in the class ObjectToStringConverter (see here):

@Override
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
	return (source != null) ? source.toString() : null;
}
  • in Spring Security 5.7: input is a JSONObject which gets converted to a JSON object string
  • in Spring Security 5.8: input is a LinkedTreeMap which gets converted to a key-value string

How to proceed with this?

@marcusdacoregio marcusdacoregio modified the milestones: 5.8.1, 5.8.2 Dec 16, 2022
@adase11
Copy link

adase11 commented Dec 16, 2022

If it helps I noticed this as well in this issue #12108 (comment)

@jzheaux
Copy link
Contributor

jzheaux commented Dec 21, 2022

The JWT parsing should not change behaviour

Since JSONObject and LinkedTreeMap both implement the Map interface, I guess I'm not clear yet on what the problem is. Could you clarify? It sounds like your code could be relying on Nimbus implementation details. I'll leave it to the Nimbus team to say whether they support that.

Either way, Spring Security merely wraps Nimbus, so I'd recommend filing an issue with them if you'd like to see changes in its parsing behavior. The only parsing that we have dedicated code for are standard claims. Since resource_access is a non-standard claim, there is not much that Spring Security can do in the way of adapting changes in third-party code.

If you end up filing an issue with Nimbus, please consider posting the link here so that folks can follow that discussion.

In the meantime, you can use a custom claim converter to convert any given claim to the representation that you need:

@Bean 
JwtDecoderFactory<ClientRegistration> jwtDecoderFactory() {
    OidcIdTokenDecoderFactory factory = new OidcIdTokenDecoderFactory();
    Map<String, Converter<Object, ?>> converters = OidcIdTokenDecoderFactory
            .createDefaultClaimTypeConverters();
    converters.put("resource_access", (object) -> {
        // ... reformulate as a JSONObject
    });
    ClaimTypeConverter claimTypeConverter = new ClaimTypeConverter(converters);
    factory.setClaimTypeConverterFactory((registration) -> claimTypeConverter);
    return factory;
}

For completeness, I'll also mention that getClaimAsMap is preferred given that the value is a Map. If the string representation is needed for serialization, note that Jwt#getTokenValue has the original serialized value.

not even reported in the Changelog here

I'll take this as feedback for the future, thank you. I believe the release notes do say that Nimbus was updated, though our release notes aren't linked in the What's New section.

@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue and removed type: bug A general bug labels Dec 21, 2022
@spring-projects-issues
Copy link

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Dec 28, 2022
@madduci
Copy link
Author

madduci commented Dec 29, 2022

Hi

Thank you very much for the support and help debugging this. I will report the problem to Nimbus developers and post here updates if I'll receive any.

For sure, this was a weird one.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Dec 29, 2022
@jzheaux
Copy link
Contributor

jzheaux commented Jan 5, 2023

Sounds great, @madduci. Given that, I'll close the issue for now. If it becomes clear that Spring Security should somehow change, we can reopen.

@jzheaux jzheaux closed this as completed Jan 5, 2023
@jzheaux jzheaux added status: declined A suggestion or change that we don't feel we should currently apply and removed status: feedback-provided Feedback has been provided labels Jan 5, 2023
@jzheaux jzheaux removed this from the 5.8.2 milestone Jan 5, 2023
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
Projects
None yet
Development

No branches or pull requests

6 participants