Expected Behavior

If I have a WebClient with OAuth2 support, based on AuthorizedClientServiceOAuth2AuthorizedClientManager, I would expect the client_credentials-based OAuth token to be only retrieved once, even if I operate from a servlet request. This seems to make sense, as the client_credentials would be the same for every user.

Current Behavior

Currently, ServletOAuth2AuthorizedClientExchangeFilterFunction retrieves the principal from the attributes, which get transparently set by SecurityReactorContextConfiguration imported by @EnableWebSecurity. This principal is then used when retrieving/storing the token and any token retrieved is tied to the principal doing the request, thus the OAuth2 token endpoint is called for every user separately, even when using client_credentials.

Context

In the current situation, tens of thousands of unnecessary calls would be done to the token endpoint, as each user retrieves the token for itself. I could probably override the getAuthentication() in ServletOAuth2AuthorizedClientExchangeFilterFunction, but that seems like a hack for something that I think should be default behaviour. I don't really get why client_credentials-based tokens are scoped to the principal.

It seems the same would be the case for DefaultOAuth2AuthorizedClientManager by the way, it's just that I want to be able to use the WebClient in and outside of a servlet request.

Comment From: sjohnr

@MikeN123, thanks for the enhancement request.

It seems very easy to fall into the case you describe, where the ServletOAuth2AuthorizedClientExchangeFilterFunction uses the principal from the request.

The following section of the docs has tips on resolving an authorized client:

However, there seems to be one gap in the docs, which is the ServletOAuth2AuthorizedClientExchangeFilterFunction.authentication(...) helper method.

For example:

    @GetMapping("/hello")
    public Map<String, String> hello() {
        AnonymousAuthenticationToken anonymousAuthentication = new AnonymousAuthenticationToken(
                "test-client", "test-client", AuthorityUtils.createAuthorityList("ROLE_ANONYMOUS"));
        return this.webClient.get()
                .uri("http://localhost:8081/hello") // uri of a resource server
                .attributes(clientRegistrationId("test-client"))
                .attributes(authentication(anonymousAuthentication))
                .retrieve()
                .bodyToMono(new ParameterizedTypeReference<Map<String, String>>() {})
                .block();
    }

In my testing, this seems to cover the case you're describing, by instructing the filter function to ignore the current authentication and scope the authorized client based on the provided one. Can you verify that this has an impact on your application's behavior? If not, can you provide a minimal, reproducible sample for your case?

Comment From: MikeN123

Hi, many thanks for your elaborate response.

Judging from the code, this would indeed work. I'm still wondering what the use case for tying together the current authentication with the client_credentials flow is though, I can't think of any. It doesn't seem like a sane default and is prone to overrequesting OAuth tokens. At the same time, after digging through the code extensively, I do not really have a suggestion on how to make things better either, as both the ServletOAuth2AuthorizedClientExchangeFilterFunction and the AuthorizedClientServiceOAuth2AuthorizedClientManager are not client_credentials-specific and can't assume the authentication is not relevant.

So maybe this boils down to a documentation improvement. It still feels a bit 'clumsy' that one should make sure to set both the client_id and an anonymous authentication token when using a client_credentials flow, but maybe that's just the way it is.

Comment From: sjohnr

Thanks for the response, @MikeN123. I agree that this seems counter-intuitive coming from the direction of grant-type specific functionality. I think for the moment, I'd like to focus on the docs improvement. I'll open a specific enhancement to the docs, and close this as a duplicate.

Comment From: sjohnr

Closing in favor of gh-10120.