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

Simplify customizing ReactiveOAuth2AccessTokenResponseClient to workaround urlencoding of oauth clientid/secrets #10042

Closed
vboulaye opened this issue Jul 2, 2021 · 6 comments
Assignees
Labels
status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@vboulaye
Copy link
Contributor

vboulaye commented Jul 2, 2021

Expected Behavior
Hello,
This is kind of a follow up to #10018.
While trying to upgrade to spring security 5.5.1, I found out that the oauth2 clientId and secret are now URL encoded in AbstractWebClientReactiveOAuth2AccessTokenResponseClient which my token provider does not support.
I understand this is seen as a bug fix from your side #9610), but in my case I cannot make the token provider change its behavior easily.

So I tried to find a way to work around this.

The documentation link provided in the previous issue does not seem to apply when you use a WebClient configuration (which is my case).

In order to workaround the clientid/secret encoding I had to copy most of the existing code from AbstractWebClientReactiveOAuth2AccessTokenResponseClient to customize the WebClientReactiveClientCredentialsTokenResponseClient because most of it has private/default visibility.

I'd like to know if there is a better way to do this, or if you could provide one ?

(Otherwise I'll have to try my luck and get a secret with only url-compliant characters!)

Current Behavior

Here is what I ended up doing, just to remove the commented url encoding call:

    clientManager.setAuthorizedClientProvider(
                    ReactiveOAuth2AuthorizedClientProviderBuilder.builder()
                            .clientCredentials(configurer -> configurer
                                    .accessTokenResponseClient(new WebClientReactiveClientCredentialsTokenResponseClient() {
                                        @Override
                                        public Mono<OAuth2AccessTokenResponse> getTokenResponse(OAuth2ClientCredentialsGrantRequest grantRequest) {
                                            Assert.notNull(grantRequest, "grantRequest cannot be null");
                                            WebClient webClient = WebClient.builder().build();
                                            setWebClient(webClient);
                                            // @formatter:off
                                            return Mono.defer(() -> webClient.post()
                                                    .uri(clientRegistration(grantRequest).getProviderDetails().getTokenUri())
                                                    .headers((headers) -> populateTokenRequestHeaders(grantRequest, headers))
                                                    .body(createTokenRequestBody(grantRequest))
                                                    .exchange()
                                                    .flatMap((response) -> readTokenResponse(grantRequest, response))
                                            );
                                            // @formatter:on
                                        }

                                        ClientRegistration clientRegistration(OAuth2ClientCredentialsGrantRequest grantRequest) {
                                            return grantRequest.getClientRegistration();
                                        }
                                        private void populateTokenRequestHeaders(OAuth2ClientCredentialsGrantRequest grantRequest, HttpHeaders headers) {
                                            ClientRegistration clientRegistration = clientRegistration(grantRequest);
                                            headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
                                            headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));
                                            if (ClientAuthenticationMethod.CLIENT_SECRET_BASIC.equals(clientRegistration.getClientAuthenticationMethod())
                                                    || ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
                                                String clientId = clientRegistration.getClientId();// encodeClientCredential(clientRegistration.getClientId());
                                                String clientSecret = clientRegistration.getClientSecret(); // encodeClientCredential(clientRegistration.getClientSecret());
                                                headers.setBasicAuth(clientId, clientSecret);
                                            }
                                        }

                                        private BodyInserters.FormInserter<String> createTokenRequestBody(OAuth2ClientCredentialsGrantRequest grantRequest) {
                                            BodyInserters.FormInserter<String> body = BodyInserters.fromFormData(OAuth2ParameterNames.GRANT_TYPE,
                                                    grantRequest.getGrantType().getValue());
                                            return populateTokenRequestBody(grantRequest, body);
                                        }

                                        BodyInserters.FormInserter<String> populateTokenRequestBody(OAuth2ClientCredentialsGrantRequest grantRequest,
                                                                                                    BodyInserters.FormInserter<String> body) {
                                            ClientRegistration clientRegistration = clientRegistration(grantRequest);
                                            if (!ClientAuthenticationMethod.CLIENT_SECRET_BASIC.equals(clientRegistration.getClientAuthenticationMethod())
                                                    && !ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
                                                body.with(OAuth2ParameterNames.CLIENT_ID, clientRegistration.getClientId());
                                            }
                                            if (ClientAuthenticationMethod.CLIENT_SECRET_POST.equals(clientRegistration.getClientAuthenticationMethod())
                                                    || ClientAuthenticationMethod.POST.equals(clientRegistration.getClientAuthenticationMethod())) {
                                                body.with(OAuth2ParameterNames.CLIENT_SECRET, clientRegistration.getClientSecret());
                                            }
                                            Set<String> scopes = scopes(grantRequest);
                                            if (!CollectionUtils.isEmpty(scopes)) {
                                                body.with(OAuth2ParameterNames.SCOPE, StringUtils.collectionToDelimitedString(scopes, " "));
                                            }
                                            return body;
                                        }
                                        Set<String> scopes(OAuth2ClientCredentialsGrantRequest grantRequest) {
                                            return grantRequest.getClientRegistration().getScopes();
                                        }

                                        private Mono<OAuth2AccessTokenResponse> readTokenResponse(OAuth2ClientCredentialsGrantRequest grantRequest, ClientResponse response) {
                                            return response.body(OAuth2BodyExtractors.oauth2AccessTokenResponse())
                                                    .map((tokenResponse) -> populateTokenResponse(grantRequest, tokenResponse));
                                        }


                                        OAuth2AccessTokenResponse populateTokenResponse(OAuth2ClientCredentialsGrantRequest grantRequest, OAuth2AccessTokenResponse tokenResponse) {
                                            if (CollectionUtils.isEmpty(tokenResponse.getAccessToken().getScopes())) {
                                                Set<String> defaultScopes = defaultScopes(grantRequest);
                                                // @formatter:off
                                                tokenResponse = OAuth2AccessTokenResponse
                                                        .withResponse(tokenResponse)
                                                        .scopes(defaultScopes)
                                                        .build();
                                                // @formatter:on
                                            }
                                            return tokenResponse;
                                        }

                                        Set<String> defaultScopes(OAuth2ClientCredentialsGrantRequest grantRequest) {
                                            return scopes(grantRequest);
                                        }
                                    })
                            .build());

Context

I am using spring security to call a Oauth2 resource using a WebClient.
And I'd like to upgrade it to the latest version.
It is working with version 5.4.2.

@sjohnr
Copy link
Member

sjohnr commented Jul 8, 2021

@vboulaye thanks for the suggestion. It is indeed tough to customize this behavior on the reactive side. There seem to be two sides to this issue:

  1. A question around how customize the Authorization header of the token request in the meantime.
  2. A request to enhance AbstractWebClientReactiveOAuth2AccessTokenResponseClient to allow for the headers to be easily customized.

I think I have an answer for 1. We should probably take that half over to stackoverflow. Would you mind opening that question and linking it from here? I'll be happy to post what I have so far.

As for number 2, I'll keep this as an open enhancement request, and we can look into adding a Consumer<HttpHeaders> for the private method in AbstractWebClientReactiveOAuth2AccessTokenResponseClient, which matches signature of the headers(...) method on the WebClient.post() call chain.

Would you be interested in submitting a PR for that?

@vboulaye
Copy link
Contributor Author

vboulaye commented Jul 8, 2021

Hi,
I created a SO question regarding the Authorization header cusomtiation.

I'll try to prepare a PR in the coming days.

Thanks for your help

@jgrandja jgrandja removed the status: waiting-for-triage An issue we've not yet triaged label Jul 9, 2021
@sjohnr
Copy link
Member

sjohnr commented Jul 13, 2021

@vboulaye Any chance you've had time to look into a change for this? No problem if not.

@sjohnr
Copy link
Member

sjohnr commented Jul 20, 2021

Closing in favor of gh-10130 which is more specific around customizing headers of the request.

@sjohnr sjohnr closed this as completed Jul 20, 2021
@sjohnr sjohnr added the status: duplicate A duplicate of another issue label Jul 20, 2021
@sjohnr sjohnr removed this from the 5.6.x milestone Jul 20, 2021
@vboulaye
Copy link
Contributor Author

Hi, sorry for the late reply, I was just looking at this again today.
I 'll try to propose a PR for the new issue, but I am not sure how to do this properly!

@sjohnr
Copy link
Member

sjohnr commented Jul 20, 2021

@vboulaye no problem, the new issue I opened is where we can discuss it. I'll post some thoughts for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants