OAuth2ClientAuthenticationToken should not be persisted across requests

[mbarringer]

Describe the bug
Sometimes the server returns a 500 error with the message:

java.lang.IllegalArgumentException: The class with org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken and name of org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken is not in the allowlist. If you believe this class is safe to deserialize, please provide an explicit mapping using Jackson annotations or by providing a Mixin. If the serialization is only done by a trusted source, you can also enable default typing. See spring-projects/spring-security#4370 for details

To Reproduce

I encountered the bug while testing a resource server in conjunction with a new Spring Authorization Server's authorization code flow. The API tool I use automatically does the OAuth2 authentication, using the same session cookie from last time when it gets a new token, and that then stops working due to the 500 error. This is a contrived example, but it does trigger the bug reliably:

1. Modify the sample authorization server, adding .redirectUri("https://oidcdebugger.com/debug") to the RegisteredClient and then run the server.

2. Open a browser, go to https://oidcdebugger.com/debug, open the network tab of the browser inspector.

3. Fill the form in, log-in, and get an authorization code.

4. Look in the browser inspector to get the JSESSIONID cookie from /oauth2/authorize.

5. I'm not sure how quickly the next steps need to be done, but now POST directly to the /oauth2/token endpoint. Run curl or your preferred API client:

export AUTHCODE="YOUR CODE HERE" && curl -X "POST" "http://127.0.0.1:9000/oauth2/token" \
-H 'Cookie: JSESSIONID=COOKIE FROM BROWSER' \
-H 'Content-Type: application/x-www-form-urlencoded; charset=utf-8' \
-u 'messaging-client:secret' \
--data-urlencode "grant_type=authorization_code" \
--data-urlencode "code=$AUTHCODE" \
--data-urlencode "redirect_uri=https://oidcdebugger.com/debug"

5. A token should be correctly returned.

6. Click "Start Over" in the browser, which will hopefully cause the browser to send the same JSESSIONID with the new call to /oauth2/authorize, and get the new authorization code. Try the POST again with the new code and the same JSESSIONID. It should return a 500 error, and in the logs will be the exception.

Expected behavior

I expected to get a new token, and if not, an error message rather than an exception.

[sjohnr]

Thanks for the report, @mbarringer.

I was able to reproduce this with the existing sample, and created a reproducible collection in postman. I don't yet have a reason for the issue, so more research is needed. I feel this is an issue we will need to solve because it seems like it could happen in a public client like a single page application from the browser, which would conceivably be reusing cookies on every request as your test does.

[edwardzjl]

I ran into the same issue.

I found that there's some difference on the attributes column of the oauth2_authorization table between the fresh request of an access token (no session yet, and will succeed) and the following requests (which will fail).

The fresh request will persist attibures with java.security.Principal of org.springframework.security.authentication.UsernamePasswordAuthenticationToken, and the following requests will persist attributes with java.security.Principal of org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken, which then causes the OAuth2ClientAuthenticationToken is not in the allowlist error.

[edwardzjl]

It seems that the attributes column is not properly (serialized and) deserialized. I'm interested in making a PR of this issue.

[sjohnr]

Thanks @edwardzjl. I believe that is likely the explanation for what's going on. Ultimately, there may be something we need to check to ensure consistency between requests, but I think it's reasonable for now to add the OAuth2ClientAuthenticationToken as a class that can be serialized. Take it away!

[colin-riddell]

Hi folks. Using @sjohnr's JpaOAuth2AuthorizationService gist I'm getting a similar, but slightly different issue (de)serializing OAuth2Authorization objects:

org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationToken is not in the allowlist. 

edit:

I'm using oauth2ResourceServer(OAuth2ResourceServerConfigurer::jwt) in my chain and looks like it's being passed through and into one of the OAuth2Authorization's attributes under the key java.security.Principal. The value is of course an instance of JwtAuthenticationToken which it's having trouble deserializing.

toObject takes the authorization that's been queried from the db and tries to re-build the OAuth2Authorization object. That's where it comes across the JwtAuthenticationToken in the attributes and fails to deserialize.

Is this something you want more info on in a separate issue @sjohnr - if it's something the core team believes would be useful to be able to serialize, am happy to contribute that mixin.

[edwardzjl]

Thanks @edwardzjl. I believe that is likely the explanation for what's going on. Ultimately, there may be something we need to check to ensure consistency between requests, but I think it's reasonable for now to add the OAuth2ClientAuthenticationToken as a class that can be serialized. Take it away!

Thanks for the feedback. I opened #507 with code working in my project, but it may contain unnecessary mixins which the core team don't want. Once I have your guidance I'll improve the code and get this done. Thanks!

[shanzhaozhen]

When can it be updated to the release version? I look forward to it

[sjohnr]

Is this something you want more info on in a separate issue @sjohnr - if it's something the core team believes would be useful to be able to serialize, am happy to contribute that mixin.

Hi @colin-riddell. Since that is not something that's happening out of the box (e.g. in the existing sample), probably not at this time. However, it may be required in the future. As a current example, when using the UserInfo endpoint you need to enable oauth2ResourceServer().jwt(). So it's very close to a core requirement, but not enabled out-of-the-box as of yet.

Thanks for the feedback. I opened #507 with code working in my project, but it may contain unnecessary mixins which the core team don't want. Once I have your guidance I'll improve the code and get this done. Thanks!

Awesome. Thanks for the heads up! I will review it as soon as I can and get back to you with that feedback.

[jgrandja]

@mbarringer I was not able to reproduce the issue based on your detailed steps. Each time I clicked "Start Over", I was able obtain a token each time.

I really don't see how OAuth2ClientAuthenticationToken is saved in the OAuth2Authorization. This is the only place where Principal.class.getName() is saved as an OAuth2Authorization.attribute:

spring-authorization-server/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/authentication/OAuth2AuthorizationCodeRequestAuthenticationProvider.java

Line 422 in 8defe2e

.attribute(Principal.class.getName(), principal)

The principal instance in the sample would be UsernamePasswordAuthenticationToken and for any custom application it would be a user-based principal instance NOT a client-based principal instance, which OAuth2ClientAuthenticationToken is. The OAuth2ClientAuthenticationToken is only meant to be used for client authentication and it should never be persisted or serialized. It's meant to be transient and only used to authenticate the client in the current flow. I'm very curious how OAuth2ClientAuthenticationToken is being serialized in the flow you're testing. Are you able to provide more detail? Can you provide a full stacktrace of the exception?

@edwardzjl

... and the following requests will persist attributes with java.security.Principal of org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken, which then causes the OAuth2ClientAuthenticationToken is not in the allowlist error.

Can you also provide more detail on your flow and how OAuth2ClientAuthenticationToken is being saved to OAuth2Authorization.attribute.

[jgrandja]

@mbarringer @edwardzjl
I was able to reproduce with the help of @sjohnr. This is an interesting find !

I'll get back to you after I find a fix.

[fushengruomengzhang]

add Annotation com.fasterxml.jackson.databind.annotation.JsonSerialize

[jgrandja]

@mbarringer @edwardzjl @colin-riddell This issue is related to a bug in Spring Security. See spring-security#9993. There is a fix in place and will be released in 5.7.0 GA, which is a way out.

FYI, the OAuth2ClientAuthenticationToken and JwtAuthenticationToken are marked as @Transient and therefore should NOT be persisted across requests. However, it is being persisted across requests, resulting in this bug.

Since Spring Security 5.7.0 GA is a way out, I've applied a temporary fix via d0e1107

[colin-riddell]

Thanks @jgrandja. Does this mean that the java.security.Principal in OAuth2Authorization's attributes should never be a JwtAuthenticationToken object or indeed OAuth2ClientAuthenticationToken - Or the entire entry shouldn't exist?

If so, would a valid work-around in the mean time be to drop that from the attributes map?

[jgrandja]

@colin-riddell

Does this mean that the java.security.Principal in OAuth2Authorization's attributes should never be a JwtAuthenticationToken object or indeed OAuth2ClientAuthenticationToken

Correct.

If so, would a valid work-around in the mean time be to drop that from the attributes map?

Not sure I understand? If you have a persisted OAuth2Authorization with JwtAuthenticationToken or OAuth2ClientAuthenticationToken as java.security.Principal, then yes the OAuth2Authorization should be removed since it's not in the correct state.

With the current fix merged, all new flows will persist in the correct state.

[mbarringer]

Great, thank you @jgrandja and @sjohnr! The temporary fix works perfectly.

[sjohnr]

@jgrandja @colin-riddell

I believe what @colin-riddell is referring to (since we discussed this together offline) is a custom scenario using a different authentication mechanism other than form login, for example using JWT authentication. In this case, it seems possible that JwtAuthenticationToken would be placed in the java.security.Principal attribute as it is actually the principal representing the user's authentication.

Our mixin inherits from Spring Security the ability to persist a UsernamePasswordAuthenticationToken into attributes. But in case of a different authentication, there would still be the need for an application to provide their own mixins to ensure the OAuth2Authorization can be persisted via JSON in the database. I feel this would be another example where a How-to guide could capture how this can be customized, and potentially such a mixin (that Colin has already developed a draft of) could be part of that guide.

Hope that helps.

[colin-riddell]

Hi @jgrandja and @sjohnr apologies for dragging this up again but I just need to clarify what should happen here as I'm not 100% on what the outcome was.

Should the JwtAuthenticationToken be persisted as part of the OAuth2Authorization's attributes or not?

If it doesn't need to be persisted I'll need to have a the deserializer I've written just ignore it for now, presumably, until it's fixed elsewhere and is no longer being incorrectly included as that attribute? Or should I be checking for the transient flag as part of my custom serialization?

Thanks

[jgrandja]

@colin-riddell

The answer has been provided by @sjohnr in this comment

[colin-riddell]

hey @jgrandja - my question wasn't so much "how do we work around this issue?" as we've established that the JwtAuthenticationToken needs to have its own mixins to be serialized as part of the OAuth2Authorization otherwise auth flows simply will not work. I've understood that and implemented this 👌🏻

My question was more "Does it NEED to be?". Reason I'm asking is because I sensed some uncertainty between what you said in an earlier post and what @sjohnr said.

you said..

JwtAuthenticationToken are marked as @transient and therefore should NOT be persisted across requests. However, it is being persisted across requests, resulting in this bug.

In other words by persisting the JwtAuthenticationToken am I covering up a bug / working around it? Or is this correct behaviour? I hope you can now appreciate my need to clarify.

[sjohnr]

@colin-riddell - You're good to go with the way you've done it. This thread was actually specific to OAuth2ClientAuthenticationToken and the out of the box setup. Your setup is for a custom use-case, so is a different situation.

[akuma8]

Our mixin inherits from Spring Security the ability to persist a UsernamePasswordAuthenticationToken into attributes. But in case of a different authentication, there would still be the need for an application to provide their own mixins to ensure the OAuth2Authorization can be persisted via JSON in the database. I feel this would be another example where a How-to guide could capture how this can be customized, and potentially such a mixin (that Colin has already developed a draft of) could be part of that guide.

@sjohnr I just encoutered the issue when deserializing OAuth2ClientAuthenticationToken. I am implemententing a custom grant type (the password grant type) but when I try to refresh the token I get this error:

The class with org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken and name of org.springframework.security.oauth2.server.authorization.authentication.OAuth2ClientAuthenticationToken is not in the allowlist.

The issue comes from OAuth2RefreshTokenAuthenticationProvider where we have to provide authorization's attribute.
To provide one I registered it like this:

@Override
    public Authentication authenticate(Authentication authentication) throws AuthenticationException {
          ...
        OAuth2ClientAuthenticationToken clientPrincipal = OAuth2AuthenticationProviderUtils.getAuthenticatedClientElseThrowInvalidClient((Authentication) passwordGrantAuthenticationToken.getPrincipal());
        ...
        OAuth2Authorization authorization = OAuth2Authorization.withRegisteredClient(registeredClient)
                .principalName(userDetails.getUsername())
                .authorizedScopes(authorizedScopes)
                .authorizationGrantType(PASSWORD_GRANT_TYPE)
                .attribute(Principal.class.getName(), clientPrincipal) //===== here
                .token(accessToken, (metadata) -> metadata.put(OAuth2Authorization.Token.CLAIMS_METADATA_NAME, tokenMetadata))
                .token(refreshToken, (metadata) -> metadata.put(OAuth2Authorization.Token.CLAIMS_METADATA_NAME, refreshTokenMetadata))
                .build();
}

The serialization seems to work but not the deserialization.
You mentionned a How-to guide in your comment, where can I find a reference guide or should I define a custom mixin?
Thank you

[sjohnr]

@akuma8

You mentionned a How-to guide in your comment, where can I find a reference guide

We don't yet have a guide specific to this, and I'm not sure we will at this point.

should I define a custom mixin

Yes.

[NuwanSameera]

@akuma8 I am also faced exact same issue when I am implementing custom grant type. Access token and refresh token return by custom grant type. But cannot get new access token from refresh token.

Then I try to implement custom mixin. But I couldn't. Did you solve your problem ?

[NuwanSameera]

I found a solution for that issue. Issue is saved Principal.class.getName() is OAuth2ClientAuthenticationToken. It cannot deserialize. Then I debug authrization_code flow. In that flow initially saved UsernamePasswordAuthenticationToken as Principal.class.getName(). So I did the same in my custom grant type.

         UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("", "");

        // Initialize the OAuth2Authorization
        OAuth2Authorization.Builder authorizationBuilder = OAuth2Authorization.withRegisteredClient(registeredClient)
                .principalName(clientPrincipal.getName())
                .authorizationGrantType(otpCodeGrantAuthentication.getGrantType())
                .attribute(Principal.class.getName(), token);

Is this accepted solution.