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

OAuth2 / WebClient: support client credentials not scoped to principal #10083

Closed
MikeN123 opened this issue Jul 15, 2021 · 4 comments
Closed
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue

Comments

@MikeN123
Copy link
Contributor

MikeN123 commented Jul 15, 2021

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.

@MikeN123 MikeN123 added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Jul 15, 2021
@sjohnr sjohnr added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Jul 16, 2021
@sjohnr sjohnr self-assigned this Jul 16, 2021
@sjohnr sjohnr removed the status: waiting-for-triage An issue we've not yet triaged label Jul 16, 2021
@sjohnr
Copy link
Member

sjohnr commented Jul 19, 2021

@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?

@sjohnr sjohnr added the status: waiting-for-feedback We need additional information before we can continue label Jul 19, 2021
@MikeN123
Copy link
Contributor Author

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.

@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 20, 2021
@sjohnr
Copy link
Member

sjohnr commented Jul 20, 2021

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.

@sjohnr
Copy link
Member

sjohnr commented Jul 20, 2021

Closing in favor of gh-10120.

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: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants