Spring-security: OAuth2AuthorizedClientArgumentResolver should get new access token when expired and client_credentials

[jgrandja]

If the OAuth2AuthorizedClient.accessToken is expired for a client_credentials OAuth2AuthorizedClient.clientRegistration than the OAuth2AuthorizedClientArgumentResolver should handle getting a new access token.

This functionality already exists in ServletOAuth2AuthorizedClientExchangeFilterFunction.authorizeWithClientCredentials() (Servlet) and ServerOAuth2AuthorizedClientExchangeFilterFunction.authorizeWithClientCredentials() (Reactive).

NOTE: This functionality needs to be implemented in both the Servlet and Reactive OAuth2AuthorizedClientArgumentResolver.

Related #5893

[mkheck]

I'll take this on. Thanks Joe!

[jgrandja]

Great, thanks @mkheck. Let me know if you have any questions along the way.

[fritzdj]

@mkheck and @jgrandja, a couple notes on this:

1. Would it make sense for there to be a buffer period (that can be configurable by the client) when the token would get refreshed? i.e. if the token is within milliseconds of being expired, it's possible the call to an endpoint would still fail because it is the destination that is doing token validation. Internal processing + latency needs to be taken into account here.
2. I am not a big fan of how we _need_ to use OAuth2AuthorizedClientArgumentResolver to get an access token using the client credentials flow. This is because we end up needing to pass that access token around to different classes, causing unnecessary refactoring. What are your thoughts on creating an autowired service that goes through this same logic? It could be autowired in by a client class because the HTTP request and response are request-scoped beans.

[jgrandja]

@fritzdj

Would it make sense for there to be a buffer period...

You will notice the accessTokenExpiresSkew member in both ServletOAuth2AuthorizedClientExchangeFilterFunction and ServerOAuth2AuthorizedClientExchangeFilterFunction...this accounts for the buffer / latency that can occur. This same logic needs to be implemented here.

This is because we end up needing to pass that access token around to different classes, causing unnecessary refactoring

After OAuth2AuthorizedClientArgumentResolver resolves the OAuth2AuthorizedClient, you don't necessarily need to pass OAuth2AuthorizedClient down your application stack/tiers. For example, you can auto-wire OAuth2AuthorizedClientService in a component in the service/data tier to lookup the OAuth2AuthorizedClient. Note: OAuth2AuthorizedClientRepository is meant to be used in web-tier and OAuth2AuthorizedClientService in service/data tier.

Does this approach work for you?

[fritzdj]

Thanks @jgrandja, this clears things up. I do still think we should be able to call the logic within that resolver using a bean on demand if desired though. This way you could create clients that are more self-contained and wouldn't need to add the resolver to the controller. Could we potentially have shared logic being used by both options?

[jgrandja]

@fritzdj I'm not sure I'm completely following your idea/suggestion. Can you provide more detail please.

[fritzdj]

For example, you can auto-wire OAuth2AuthorizedClientService in a component in the service/data tier to lookup the OAuth2AuthorizedClient

@jgrandja This makes sense to me, but you would still need to use OAuth2AuthorizedClientArgumentResolver to resolve OAuth2AuthorizedClient in web-tier first. My suggestion is to make things more self-contained in service/data tier by allowing OAuth2AuthorizedClientService to get new access tokens if needed. I think Spring should allow new access tokens to be retrieved either way (in service/data tier _OR_ web tier). We tried this with a custom service bean and it is working using some of the logic in OAuth2AuthorizedClientArgumentResolver. Sorry for getting a little off topic with this particular issue.

[jgrandja]

@fritzdj

We tried this with a custom service bean and it is working using some of the logic in OAuth2AuthorizedClientArgumentResolver. Sorry for getting a little off topic with this particular issue.

Can you please open a new issue so we can continue this there. Also, can you share the code in the new issue so we can get more into the details. Thanks.

[jgrandja]

I re-evaluated this issue and the requirements are not correct. The OAuth2AuthorizedClientArgumentResolver is not intended to act in the role of an OAuth Client, therefore, it is not responsible for refreshing/renewing expired access tokens. This responsibility belongs to the OAuth Client - to check the existing access token for expiry before it attempts to request a protected resource.

NOTE: This capability is implemented in ServletOAuth2AuthorizedClientExchangeFilterFunction and ServerOAuth2AuthorizedClientExchangeFilterFunction, which is used by WebClient and acts in the role of the OAuth Client.

The only responsibility for the OAuth2AuthorizedClientArgumentResolver is to resolve the existing OAuth2AuthorizedClient from the OAuth2AuthorizedClientRepository, if not available, than initiate the authorization grant flow to resolve a new OAuth2AuthorizedClient. _NOTE:_ This functionality already exists.

Given this, I'm going to close this ticket as invalid.

[kalyaniyerra]

6610 is still valuable. I actually use the @RegisteredOAuth2AuthorizedClient in my controller and use the accesstoken to pass it on to a different client. I am not using the WebClient. I have a need to use 3rd party HTTP client that underneath uses the okHTTP client. In this scenario, it would be beneficial for adding the logic to retrieve the access token again if it is expired since the 3rd party client I am using doesn't allow to add the ServletOAuth2AuthorizedClientExchangeFilterFunction.

[jgrandja]

@kalyaniyerra Please see #6811 as the goal there is to allow better reuse of the existing logic in ServletOAuth2AuthorizedClientExchangeFilterFunction.