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.