Introduce Reactive OAuth2Authorization success/failure handlers

[philsttr]

Summary

When an OAuth2 Client sends a token to a downstream OAuth2 Resource Server, and the OAuth2 Resource Server returns an HTTP 401 response, I would like to prevent the OAuth2 Client from sending that same token in future requests (and instead retrieve a new token for future requests).

Currently, spring-security-oauth2-client provides good support for re-retrieving/refreshing tokens before they expire. However, spring-security-oauth2-client does not provide good support for re-retrieving/refreshing access tokens when they have become invalid for reasons other than expiration. A good signal that a token is no longer valid is when an OAuth2 Resource Server returns an HTTP 401 response to the client.

Both ServerOAuth2AuthorizedClientRepository and ReactiveOAuth2AuthorizedClientService provide a removeAuthorizedClient method to remove the cached OAuth2AuthorizedClient. It would be nice if spring-security-oauth2-client provided something that actually called that method when an HTTP 401 response is received from an OAuth2 Resource Server.

A possible implementation might be:

1. the DefaultReactiveOAuth2AuthorizedClientManager (and the future AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager from Reactive Implementation of AuthorizedClientServiceOAuth2AuthorizedCli… #7589) could put a Mono<Void> in the subscriber context that will remove the token from the cache when subscribed
   • DefaultReactiveOAuth2AuthorizedClientManager would put a Mono<Void> that invokes its ServerOAuth2AuthorizedClientRepository.removeAuthorizedClient
   • AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager would put a Mono<Void> that invokes its ReactiveOAuth2AuthorizedClientService.removeAuthorizedClient
2. A new ExchangeFilterFunction (or one of the existing ones) checks for HTTP 401 responses, and chains the Mono<Void> from step 1 if it exists
   • usage of the subscriber context would decouple the ExchangeFilterFunction from the implementation of how to remove the cached authorized client (i.e. separate filter function implementations would not be needed depending on whether a ServerOAuth2AuthorizedClientRepository or ReactiveOAuth2AuthorizedClientService is being used to persist OAuth2AuthorizedClient s)
   • It would also allow other places downstream (e.g. non-WebClient related functionality, like WebSocketClient) to potentially invalidate the authorized client, without direct knowledge of exactly how it is persisted, other than just subscribing to a Mono<Void>)

(After looking at this a bit more, I realize the above approach probably won't work, since anything put into the subscriber context by a ReactiveOAuth2AuthorizedClientManager wouldn't actually be visible downstream in an ExchangeFilterFunction . So, something else is needed. Perhaps something in ServerOAuth2AuthorizedClientExchangeFilterFunction)

Actual Behavior

spring-security-oauth2-client continues to reuse an OAuth2 token, even after an OAuth2 Resource server has returned a 401 response, which usually indicates the token is invalid.

Expected Behavior

spring-security-oauth2-client removes the cached token after an OAuth2 Resource server has returned a 401 response. Future requests will trigger a re-retrieval of a new token.

Configuration

• spring-security-oauth2-client
• using either DefaultReactiveOAuth2AuthorizedClientManager or AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager from Reactive Implementation of AuthorizedClientServiceOAuth2AuthorizedCli… #7589
• WebClient or WebSocketClient usage downstream

Version

5.2.1.RELEASE

[jgrandja]

@philsttr This sounds like a valuable enhancement. Would you be interested in submitting a PR for this?

[philsttr]

Yeah, I could tackle the implementation, once we settle on an approach. I'd like your input on what approach to take, before I get started.

My current thinking is to:

1. expose a new method in ReactiveOAuth2AuthorizedClientManager for invalidating an OAuth2AuthorizedClient. (named invalidate ?)
   • This would have to be a default method for backwards compatibility, with a default implementation that just returns Mono.empty().
   • DefaultReactiveOAuth2AuthorizedClientManager would delegate to its ServerOAuth2AuthorizedClientRepository.removeAuthorizedClient
   • AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager would delegate to its ReactiveOAuth2AuthorizedClientService.removeAuthorizedClient
2. have ServerOAuth2AuthorizedClientExchangeFilterFunction look for HTTP 401 responses, and invoke the new invalidate method on ReactiveOAuth2AuthorizedClientManager.

We could also combine this approach somewhat with the approach I mentioned in the issue description, where ServerOAuth2AuthorizedClientExchangeFilterFunction would put a Mono<Void> into subscriber context, and then anything downstream could subscribe to that mono to perform the invalidation. (e.g. another optional filter function, or whatever) This would be a little more complex, but more flexible.

Your thoughts?

(It would also be great if #7702 was merged before I start on this, so I can modify AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager)

[jgrandja]

@philsttr I'm not sure if ReactiveOAuth2AuthorizedClientManager.invalidate() makes sense.

I'm leaning towards introducing 2 new strategies similar to ServerAuthenticationSuccessHandler and ServerAuthenticationFailureHandler, which is used by AuthenticationWebFilter. This design would align the authorization flow with the authentication flow to provide some consistency.

For example:

public interface ServerOAuth2AuthorizationSuccessHandler {
	Mono<Void> onAuthorizationSuccess(OAuth2AuthorizedClient authorizedClient, ...);
}

public interface ServerOAuth2AuthorizationFailureHandler {
	Mono<Void> onAuthorizationFailure(OAuth2AuthorizationException exception, ...);
}

We could consider extending OAuth2AuthorizationException, similar to ClientAuthorizationRequiredException, that captures the details the ServerOAuth2AuthorizationFailureHandler requires to do it's thing, eg. remove the OAuth2AuthorizedClient from the repository. However, I think I would keep OAuth2AuthorizationException as the type parameter.

The default implementation of ServerOAuth2AuthorizationSuccessHandler could simply do what DefaultReactiveOAuth2AuthorizedClientManager.saveAuthorizedClient() does today.

Thoughts?

[jgrandja]

Related #7583

[philsttr]

A few questions/comments:

What would have a reference to the handlers, detect authentication success/failure, and call the handlers? ServerOAuth2AuthorizedClientExchangeFilterFunction?

Since ServerOAuth2AuthorizedClientExchangeFilterFunction has a ReactiveOAuth2AuthorizedClientManager (either a DefaultReactiveOAuth2AuthorizedClientManager or AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager), are you saying there would be two implementations of the failure handler... one would call ServerOAuth2AuthorizedClientRepository.removeAuthorizedClient and the other would call ReactiveOAuth2AuthorizedClientService.removeAuthorizedClient ?

This is where I was thinking that ReactiveOAuth2AuthorizedClientManager.invalidate would be good... so that the ReactiveOAuth2AuthorizedClientManager implementation could decide how to handle the invalidation. The main complexity that I'm trying to solve here is that there are two possible ways to remove cached authorized clients (depending on which ReactiveOAuth2AuthorizedClientManager is being used):

1. DefaultReactiveOAuth2AuthorizedClientManager
   -> ServerOAuth2AuthorizedClientRepository.removeAuthorizedClient
2. AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager -> ReactiveOAuth2AuthorizedClientService.removeAuthorizedClient

Also, is "authorization" is the proper term here? I was thinking "authentication" is more correct. When the token is no longer valid, that is an authentication failure and a 401 returned. If the token was valid, but did not have the proper scopes, that would be an authorization failure, and a 403 would be returned. The case being handled here is the former.

[jgrandja]

What would have a reference to the handlers

The ReactiveOAuth2AuthorizedClientManager implementations would have these handlers with associated setters so applications can customize. This would handle the following cases:

1. Client is not authorized and the authorization request fails, eg. client requests a scope that they are not allowed to request.
2. Client is authorized but the token is expired and the refresh token request fails, eg. refresh token is not valid anymore RefreshTokenOAuth2AuthorizedClientProvider does not handle expired refresh token #7583
3. Client is authorized but the token is expired and the token renewal request fails, eg. client_credentials client had it's credentials leaked so it was rotated on provider

We also would need to handle 401 errors in ServerOAuth2AuthorizedClientExchangeFilterFunction so I believe we would need to reuse the failure handler here to. Assuming the OAuth2AuthorizedClient is returned from the manager, the WebClient will initiate the request but it fails with 401 as a result of the token being revoked on provider.

are you saying there would be two implementations of the failure handler... one would call ServerOAuth2AuthorizedClientRepository.removeAuthorizedClient and the other would call ReactiveOAuth2AuthorizedClientService.removeAuthorizedClient

Yes, there would need to be two since they call different collaborators.

Another advantage of having these handlers is that user's can customize the behaviour (via setters) above the standard behaviour of simply removing the authorized client from the repository/service.

Also, is "authorization" is the proper term here? I was thinking "authentication" is more correct.

Yes, authentication is more correct when a 401 occurs during a protected resource request because the token is no longer valid (expired/revoked). However, what about the cases when the client attempts to obtain a new token or renew/refresh an existing expired token? For these cases the authorization fails for the client and typically a 400 is returned as per spec. I'm going with the term authorization to suit all cases. Makes sense?

[philsttr]

Ok, I think I'm following you now. I was only thinking about using these handlers when communicating with a resource server. But you make an excellent point that these same handlers could be used when communicating with the authorization server to retrieve/refresh a token (your cases 1-3)

We also would need to handle 401 errors in ServerOAuth2AuthorizedClientExchangeFilterFunction so I believe we would need to reuse the failure handler here too.

Yeah, this was the original use case for this issue. I'd like to have ServerOAuth2AuthorizedClientExchangeFilterFunction 's default behavior be to invalidate the client when it receives a 401. In order to do that, what do you think about:

• making ReactiveOAuth2AuthorizedClientManager extend ServerOAuth2AuthorizationFailureHandler, and provide a default implementation of onAuthorizationFailure that does nothing
• modify DefaultReactiveOAuth2AuthorizedClientManager and AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager to delegate the onAuthorizationFailure call to their handlers (which default to removing the authorized client appropriately)
• Have ServerOAuth2AuthorizedClientExchangeFilterFunction's default failure handler call clientManager.onAuthenticationFailure on receiving a 401 response from the resource server

Without doing the above, I don't see how I can make ServerOAuth2AuthorizedClientExchangeFilterFunction 's default behavior be to invalidate the client when it receives a 401. I could make the default behavior of ServerOAuth2AuthorizedClientExchangeFilterFunction do nothing on a 401, and therefore require the user explicitly configure a failure handler based on which type of client manager is being used, but that is not a good out-of-the-box experience for such a common use case IMHO.

authentication vs authorization

Fair point. I'm on board with using "authorization" since we're also now including use cases for handling responses from the authorization server. I was originally only looking at the use case for handling responses from a resource server.

[jgrandja]

what do you think about: making ReactiveOAuth2AuthorizedClientManager extends ServerOAuth2AuthorizationFailureHandler

I'd rather keep the ReactiveOAuth2AuthorizedClientManager contract minimal (functional). As well, I see the handling of errors an implementation detail so I feel it makes sense to define this in the implementation rather than the interface. We currently provide setters for these type of handlers in AuthenticationWebFilter or AbstractAuthenticationProcessingFilter so we would follow the same pattern in the 2x ReactiveOAuth2AuthorizedClientManager implementations.

Without doing the above, I don't see how I can make ServerOAuth2AuthorizedClientExchangeFilterFunction 's default behavior be to invalidate the client when it receives a 401

The default ReactiveOAuth2AuthorizedClientManager is DefaultReactiveOAuth2AuthorizedClientManager via ServerOAuth2AuthorizedClientExchangeFilterFunction.createDefaultAuthorizedClientManager(). Could we not update createDefaultAuthorizedClientManager() with defaultReactiveOAuth2AuthorizedClientManager.setAuthorizationFailureHandler()?

If the application uses the constructor ServerOAuth2AuthorizedClientExchangeFilterFunction(ReactiveOAuth2AuthorizedClientManager authorizedClientManager), than the application will be responsible for configuring the ServerOAuth2AuthorizationFailureHandler.

[philsttr]

Ok, that works for me. I think I have enough to start on an implementation. I have a couple other things on my plate, but should be able to start soon.

[jgrandja]

Sounds good and thanks for all your great work @philsttr