We are using spring-security-oauth2-client with client credentials flow in a batch application to call a service that is protected with oauth2.

The access token has a lifetime of 5 minutes. When the token expires (respectively 1 minute before because of default clock skew) the access token is replaced by the AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager, which calls the ClientCredentialsReactiveOAuth2AuthorizedClientProvider . The problem is that more or less twenty requests are simultaneously fired. If the token expires in that moment, the IDPs token endpoint is not called once but in worse case 20 times.

Describe the bug Token is renewed multiple times instead of once.

Expected behavior The token endpoint should be called only once if the token expries.

Comment From: sjohnr

@marbon87 thanks for reaching out. Have you tried scoping the OAuth2AuthorizedClient to the application instead of per-principal (the default)? See this comment for recent discussion on this. As mentioned in that comment, I am actually enhancing the docs as we speak to outline this case. The docs did have a Servlet example, but sadly the commit (49f3c0ce534254ea4b5f5a674c5afb8322c1736c) was lost during our docs update to Antora.

See also gh-11461 for discussion on a similar topic.

If that doesn't help, please provide additional details of your issue and/or a minimal, reproducible sample.

Comment From: marbon87

Hi @sjohnr,

thanks a lot for your quick response.

Unfortunanetly this does not fix the problem because we are not using the ServletOAuth2AuthorizedClientExchangeFilterFunction. The reason is that we are providing the logic in a company internal (spring based ❤️) framework, that uses the token as a default. The default mechanism from ServletOAuth2AuthorizedClientExchangeFilterFunction#setDefaultClientRegistrationId is not flexible enough because it always sets the token as also documented in the ref doc (Be cautious with this feature, since all HTTP requests receive the access token.). So we implemented a custom matcher that decides when the token is used.

The interceptor for the webclient looks like this:

public class OAuth2AppTokenExchangeFilterFunction implements ExchangeFilterFunction {
    private final Predicate<URI> uriMatcher;
    private final Supplier<Mono<OAuth2AuthorizedClient>> oAuth2AuthorizedClientSupplier;

    public OAuth2AppTokenExchangeFilterFunction(@NonNull Predicate<URI> uriMatcher,
                                                @NonNull Supplier<Mono<OAuth2AuthorizedClient>> oAuth2AuthorizedClientSupplier) {
        this.uriMatcher = uriMatcher;
        this.oAuth2AuthorizedClientSupplier = oAuth2AuthorizedClientSupplier;
    }

    @Override
    public Mono<ClientResponse> filter(ClientRequest request, ExchangeFunction next) {
        return Mono.just(request)
            .flatMap(r -> {
                if (uriMatcher.test(r.url())) {
                    return oAuth2AuthorizedClientSupplier.get()
                        .map(oAuth2AuthorizedClient -> {
                                return ClientRequest
                                    .from(request)
                                    .headers(headers -> headers.setBearerAuth(oAuth2AuthorizedClient.getAccessToken().getTokenValue()))
                                    .build();
                            }
                        ).defaultIfEmpty(r);
                }
                return Mono.just(r);
            })
            .flatMap(next::exchange);
    }
}

and the default oAuth2AuthorizedClientSupplier is

() -> authorizedClientManager.authorize(authorizeRequest)

with 

authorizeRequest = OAuth2AuthorizeRequest.withClientRegistrationId("default-client-registration-id")
            .principal("appName")
            .build()

and

public ReactiveOAuth2AuthorizedClientManager authorizedClientManager(ReactiveClientRegistrationRepository clientRegistrationRepository,
                                                                     ReactiveOAuth2AuthorizedClientService authorizedClientService) {
    AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager authorizedClientServiceReactiveOAuth2AuthorizedClientManager =
        new AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager(clientRegistrationRepository, authorizedClientService);

    // Default wird um Refresh-Token-Unterstützung ergänzt
    authorizedClientServiceReactiveOAuth2AuthorizedClientManager.setAuthorizedClientProvider(
        ReactiveOAuth2AuthorizedClientProviderBuilder
            .builder()
            .refreshToken()
            .clientCredentials()
            .build()
    );

    return authorizedClientServiceReactiveOAuth2AuthorizedClientManager;
}

So we are saving the token only once for the app because we are using the appname (spring.application.name) as the principal name.

Comment From: marbon87

Maybe it isn't the ClientCredentialsReactiveOAuth2AuthorizedClientProvider that should be thread-safe but the AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager

Comment From: sjohnr

Thanks for the additional info @marbon87. I am still not seeing information about how this works in your application, who is calling it, how many instances of the application are running, and timing details on why there would be 20 simultaneous requests, etc.

Regardless, when you say either the ClientCredentialsReactiveOAuth2AuthorizedClientProvider or AuthorizedClientServiceReactiveOAuth2AuthorizedClientManager should be thread-safe, calling the token endpoint on the authz server multiple times does not indicate the class is not thread-safe. In fact, it seems quite thread safe (as calling on multiple threads works fine). However, I agree that it does not achieve the goal you have for the system which is a single atomic call to the token endpoint.

This is not really something the framework can provide anyway, since this code could be running on multiple instances of the application and would quickly run into the same problem again even if we find a way to prevent single-instance multi-threaded requests. If you want to only allow a single thread to fetch a token, you are welcome to wrap calls to these classes in synchronized blocks or any other means at your disposal to achieve the goal.

Having said that, it doesn't seem like the ReactiveOAuth2AuthorizedClientManager is really well-suited to your use case if you truly need to make only a single token request per expiration interval. I would instead recommend that you interact directly with the ReactiveOAuth2AuthorizedClientService to load tokens for the interceptor, and only use ReactiveOAuth2AuthorizedClientManager to fetch tokens in a background thread once per interval.

I'm going to close this issue, as I don't believe the framework can or should solve this case for you. Let me know if you feel I am misunderstanding anything.