Skip to content
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

OAuth2AuthorizedClient doesn't get removed when 403 returned by Resource Server #13437

Open
trung opened this issue Jun 29, 2023 · 2 comments
Open
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: feedback-provided Feedback has been provided type: enhancement A general enhancement

Comments

@trung
Copy link

trung commented Jun 29, 2023

Describe the bug
when configuring WebClient using ServerOAuth2AuthorizedClientExchangeFilterFunction with AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager, if the resource server returns 403, the OAuth2AuthorizedClient doesn't get removed.

To Reproduce

@Bean
public WebClient oauth2WebClient(
      final WebClient.Builder webClientBuilder,
      final ReactiveClientRegistrationRepository registrationRepository,
      final ReactiveOAuth2AuthorizedClientService authorizedClientService,
      final String clientRegistrationId) {

    final ServerOAuth2AuthorizedClientExchangeFilterFunction secureExchangeFilterFunction =
        new ServerOAuth2AuthorizedClientExchangeFilterFunction(
            new AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager(
                registrationRepository, authorizedClientService));
    
    secureExchangeFilterFunction.setAuthorizationFailureHandler(
        new RemoveAuthorizedClientReactiveOAuth2AuthorizationFailureHandler(
            (clientRegistrationId, principal, attributes) ->
                authorizedClientService.removeAuthorizedClient(
                    clientRegistrationId, principal.getName())
        ));
    secureExchangeFilterFunction.setDefaultClientRegistrationId(clientRegistrationId);

    return webClientBuilder.clone().filter(secureExchangeFilterFunction).build();
}

when using the above oauth2WebClient to make a call to a resource server, the resource server returns 403.
A subsequence call to the resource server uses the old access token.

Expected behavior
when using the above oauth2WebClient to make a call to a resource server, the resource server returns 403.
A subsequence call to the resource server should retrieve a new access token from the authorization server.

Initial thoughts

RemoveAuthorizedClientReactiveOAuth2AuthorizationFailureHandler is initialized with DEFAULT_REMOVE_AUTHORIZED_CLIENT_ERROR_CODES which contains only INVALID_TOKEN and INVALID_GRANT. However, with 403 returned by the resource server, ServerOAuth2AuthorizedClientExchangeFilterFunction#AuthorizationFailureForwarder maps 403 to INSUFFICIENT_SCOPE. This causes condition hasRemovalErrorCode() in RemoveAuthorizedClientReactiveOAuth2AuthorizationFailureHandler#onAuthorizationFailure not satisfy hence Oauth2AuthorizedClient doesn't get removed.

public Mono<Void> onAuthorizationFailure(OAuth2AuthorizationException authorizationException,
Authentication principal, Map<String, Object> attributes) {
if (authorizationException instanceof ClientAuthorizationException clientAuthorizationException
&& hasRemovalErrorCode(authorizationException)) {
return this.delegate.removeAuthorizedClient(clientAuthorizationException.getClientRegistrationId(),
principal, attributes);
}
return Mono.empty();

Work Around

@Bean
public WebClient oauth2WebClient(
      final WebClient.Builder webClientBuilder,
      final ReactiveClientRegistrationRepository registrationRepository,
      final ReactiveOAuth2AuthorizedClientService authorizedClientService,
      final String clientRegistrationId) {

    final ServerOAuth2AuthorizedClientExchangeFilterFunction secureExchangeFilterFunction =
        new ServerOAuth2AuthorizedClientExchangeFilterFunction(
            new AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager(
                registrationRepository, authorizedClientService));
    
    Set<String> removeAuthorizedClientErrorCodes =
        new HashSet<>(DEFAULT_REMOVE_AUTHORIZED_CLIENT_ERROR_CODES);
    removeAuthorizedClientErrorCodes.add(INSUFFICIENT_SCOPE); // 403

    secureExchangeFilterFunction.setAuthorizationFailureHandler(
        new RemoveAuthorizedClientReactiveOAuth2AuthorizationFailureHandler(
            (clientRegistrationId, principal, attributes) ->
                authorizedClientService.removeAuthorizedClient(
                    clientRegistrationId, principal.getName()),
             removeAuthorizedClientErrorCodes
        ));
    secureExchangeFilterFunction.setDefaultClientRegistrationId(clientRegistrationId);

    return webClientBuilder.clone().filter(secureExchangeFilterFunction).build();
}
@trung trung added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 29, 2023
@jzheaux
Copy link
Contributor

jzheaux commented Jul 10, 2023

Hi, @trung. I don't find it quite straightforward to have the filter function query for another token if the first is valid, just with insufficient privileges. In most cases, I think that would be an unending loop.

For example, if your client has a token with a scope of message:read and presents it to the resource server. If it responds with a 403 and then you request a new access token, isn't it most likely that the new access token's scope will still be message:read?

Please elaborate if I'm missing something. Otherwise, I'd encourage you to contribute this use case to #11783 to potentially simplify your configuration.

@jzheaux jzheaux self-assigned this Jul 10, 2023
@jzheaux jzheaux added status: waiting-for-feedback We need additional information before we can continue type: enhancement A general enhancement 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 type: bug A general bug labels Jul 10, 2023
@trung
Copy link
Author

trung commented Jul 11, 2023

Hi @jzheaux, the configuration is for machine-to-machine communication. If there were missing privileges, we would update the client scopes in our authorization server. In that case, the old token should be invalidated instantly without the need of waiting for token to expire or restarting the server.

Let me take a look at the ticket and see if this could be configurable.

@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 labels Jul 11, 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: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants