Skip to content

Commit

Permalink
Revert "URL encode client credentials"
Browse files Browse the repository at this point in the history 
This reverts commit c020051.

Issue gh-9610 gh-9863
Closes gh-10018
  • Loading branch information
@sjohnr
sjohnr committed Jul 20, 2021
1 parent 8bb69c4 commit e1b6a7b
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 177 deletions.
23 changes: 4 additions & 19 deletions ...framework/security/oauth2/client/endpoint/OAuth2AuthorizationGrantRequestEntityUtils.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -15,18 +15,15 @@
*/
package org.springframework.security.oauth2.client.endpoint;

import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Collections;

import org.springframework.core.convert.converter.Converter;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.http.RequestEntity;
import org.springframework.security.oauth2.client.registration.ClientRegistration;
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;

import java.util.Collections;

import static org.springframework.http.MediaType.APPLICATION_FORM_URLENCODED_VALUE;

/**
Expand All @@ -47,23 +44,11 @@ static HttpHeaders getTokenRequestHeaders(ClientRegistration clientRegistration)
HttpHeaders headers = new HttpHeaders();
headers.addAll(DEFAULT_TOKEN_REQUEST_HEADERS);
if (ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
String clientId = encodeClientCredential(clientRegistration.getClientId());
String clientSecret = encodeClientCredential(clientRegistration.getClientSecret());
headers.setBasicAuth(clientId, clientSecret);
headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());
}
return headers;
}

private static String encodeClientCredential(String clientCredential) {
try {
return URLEncoder.encode(clientCredential, StandardCharsets.UTF_8.toString());
}
catch (UnsupportedEncodingException ex) {
// Will not happen since UTF-8 is a standard charset
throw new IllegalArgumentException(ex);
}
}

private static HttpHeaders getDefaultTokenRequestHeaders() {
HttpHeaders headers = new HttpHeaders();
headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON_UTF8));
Expand Down
23 changes: 3 additions & 20 deletions ...ecurity/oauth2/client/endpoint/WebClientReactiveAuthorizationCodeTokenResponseClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@
*/
package org.springframework.security.oauth2.client.endpoint;

import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;

import reactor.core.publisher.Mono;

import org.springframework.http.MediaType;
import org.springframework.security.oauth2.client.registration.ClientRegistration;
import org.springframework.security.oauth2.core.AuthorizationGrantType;
Expand All @@ -30,9 +24,10 @@
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationResponse;
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
import org.springframework.security.oauth2.core.endpoint.PkceParameterNames;
import org.springframework.util.Assert;
import org.springframework.web.reactive.function.BodyInserters;
import org.springframework.web.reactive.function.client.WebClient;
import org.springframework.util.Assert;
import reactor.core.publisher.Mono;

import static org.springframework.security.oauth2.core.web.reactive.function.OAuth2BodyExtractors.oauth2AccessTokenResponse;

Expand Down Expand Up @@ -79,9 +74,7 @@ public Mono<OAuth2AccessTokenResponse> getTokenResponse(OAuth2AuthorizationCodeG
.accept(MediaType.APPLICATION_JSON)
.headers(headers -> {
if (ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
String clientId = encodeClientCredential(clientRegistration.getClientId());
String clientSecret = encodeClientCredential(clientRegistration.getClientSecret());
headers.setBasicAuth(clientId, clientSecret);
headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());
}
})
.body(body)
Expand All @@ -98,16 +91,6 @@ public Mono<OAuth2AccessTokenResponse> getTokenResponse(OAuth2AuthorizationCodeG
});
}

private static String encodeClientCredential(String clientCredential) {
try {
return URLEncoder.encode(clientCredential, StandardCharsets.UTF_8.toString());
}
catch (UnsupportedEncodingException ex) {
// Will not happen since UTF-8 is a standard charset
throw new IllegalArgumentException(ex);
}
}

private static BodyInserters.FormInserter<String> body(OAuth2AuthorizationExchange authorizationExchange, ClientRegistration clientRegistration) {
OAuth2AuthorizationResponse authorizationResponse = authorizationExchange.getAuthorizationResponse();
BodyInserters.FormInserter<String> body = BodyInserters
Expand Down
26 changes: 5 additions & 21 deletions ...ecurity/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,6 @@
*/
package org.springframework.security.oauth2.client.endpoint;

import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Set;
import java.util.function.Consumer;

import reactor.core.publisher.Mono;

import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferUtils;
import org.springframework.http.HttpHeaders;
Expand All @@ -38,6 +30,10 @@
import org.springframework.web.reactive.function.BodyInserters;
import org.springframework.web.reactive.function.client.WebClient;
import org.springframework.web.reactive.function.client.WebClientResponseException;
import reactor.core.publisher.Mono;

import java.util.Set;
import java.util.function.Consumer;

import static org.springframework.security.oauth2.core.web.reactive.function.OAuth2BodyExtractors.oauth2AccessTokenResponse;

Expand Down Expand Up @@ -102,23 +98,11 @@ private Consumer<HttpHeaders> headers(ClientRegistration clientRegistration) {
return headers -> {
headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
if (ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
String clientId = encodeClientCredential(clientRegistration.getClientId());
String clientSecret = encodeClientCredential(clientRegistration.getClientSecret());
headers.setBasicAuth(clientId, clientSecret);
headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());
}
};
}

private static String encodeClientCredential(String clientCredential) {
try {
return URLEncoder.encode(clientCredential, StandardCharsets.UTF_8.toString());
}
catch (UnsupportedEncodingException ex) {
// Will not happen since UTF-8 is a standard charset
throw new IllegalArgumentException(ex);
}
}

private static BodyInserters.FormInserter<String> body(OAuth2ClientCredentialsGrantRequest authorizationGrantRequest) {
ClientRegistration clientRegistration = authorizationGrantRequest.getClientRegistration();
BodyInserters.FormInserter<String> body = BodyInserters
Expand Down
26 changes: 5 additions & 21 deletions ...amework/security/oauth2/client/endpoint/WebClientReactivePasswordTokenResponseClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,6 @@
*/
package org.springframework.security.oauth2.client.endpoint;

import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.function.Consumer;

import reactor.core.publisher.Mono;

import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferUtils;
import org.springframework.http.HttpHeaders;
Expand All @@ -40,6 +32,10 @@
import org.springframework.util.StringUtils;
import org.springframework.web.reactive.function.BodyInserters;
import org.springframework.web.reactive.function.client.WebClient;
import reactor.core.publisher.Mono;

import java.util.Collections;
import java.util.function.Consumer;

import static org.springframework.security.oauth2.core.web.reactive.function.OAuth2BodyExtractors.oauth2AccessTokenResponse;

Expand Down Expand Up @@ -104,23 +100,11 @@ private static Consumer<HttpHeaders> tokenRequestHeaders(ClientRegistration clie
headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));
if (ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
String clientId = encodeClientCredential(clientRegistration.getClientId());
String clientSecret = encodeClientCredential(clientRegistration.getClientSecret());
headers.setBasicAuth(clientId, clientSecret);
headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());
}
};
}

private static String encodeClientCredential(String clientCredential) {
try {
return URLEncoder.encode(clientCredential, StandardCharsets.UTF_8.toString());
}
catch (UnsupportedEncodingException ex) {
// Will not happen since UTF-8 is a standard charset
throw new IllegalArgumentException(ex);
}
}

private static BodyInserters.FormInserter<String> tokenRequestBody(OAuth2PasswordGrantRequest passwordGrantRequest) {
ClientRegistration clientRegistration = passwordGrantRequest.getClientRegistration();
BodyInserters.FormInserter<String> body = BodyInserters.fromFormData(
Expand Down
26 changes: 5 additions & 21 deletions ...ork/security/oauth2/client/endpoint/WebClientReactiveRefreshTokenTokenResponseClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,6 @@
*/
package org.springframework.security.oauth2.client.endpoint;

import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.function.Consumer;

import reactor.core.publisher.Mono;

import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferUtils;
import org.springframework.http.HttpHeaders;
Expand All @@ -40,6 +32,10 @@
import org.springframework.util.StringUtils;
import org.springframework.web.reactive.function.BodyInserters;
import org.springframework.web.reactive.function.client.WebClient;
import reactor.core.publisher.Mono;

import java.util.Collections;
import java.util.function.Consumer;

import static org.springframework.security.oauth2.core.web.reactive.function.OAuth2BodyExtractors.oauth2AccessTokenResponse;

Expand Down Expand Up @@ -92,23 +88,11 @@ private static Consumer<HttpHeaders> tokenRequestHeaders(ClientRegistration clie
headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);
headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));
if (ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
String clientId = encodeClientCredential(clientRegistration.getClientId());
String clientSecret = encodeClientCredential(clientRegistration.getClientSecret());
headers.setBasicAuth(clientId, clientSecret);
headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());
}
};
}

private static String encodeClientCredential(String clientCredential) {
try {
return URLEncoder.encode(clientCredential, StandardCharsets.UTF_8.toString());
}
catch (UnsupportedEncodingException ex) {
// Will not happen since UTF-8 is a standard charset
throw new IllegalArgumentException(ex);
}
}

private static BodyInserters.FormInserter<String> tokenRequestBody(OAuth2RefreshTokenGrantRequest refreshTokenGrantRequest) {
ClientRegistration clientRegistration = refreshTokenGrantRequest.getClientRegistration();
BodyInserters.FormInserter<String> body = BodyInserters.fromFormData(
Expand Down
40 changes: 0 additions & 40 deletions ...urity/oauth2/client/endpoint/OAuth2ClientCredentialsGrantRequestEntityConverterTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,13 @@
*/
package org.springframework.security.oauth2.client.endpoint;

import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Base64;

import org.junit.Before;
import org.junit.Test;

import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.MediaType;
import org.springframework.http.RequestEntity;
import org.springframework.security.oauth2.client.registration.ClientRegistration;
import org.springframework.security.oauth2.client.registration.TestClientRegistrations;
import org.springframework.security.oauth2.core.AuthorizationGrantType;
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
import org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames;
Expand Down Expand Up @@ -81,37 +74,4 @@ public void convertWhenGrantRequestValidThenConverts() {
AuthorizationGrantType.CLIENT_CREDENTIALS.getValue());
assertThat(formParameters.getFirst(OAuth2ParameterNames.SCOPE)).isEqualTo("read write");
}

// gh-9610
@SuppressWarnings("unchecked")
@Test
public void convertWhenSpecialCharactersThenConvertsWithEncodedClientCredentials()
throws UnsupportedEncodingException {
String clientCredentialWithAnsiKeyboardSpecialCharacters = "~!@#$%^&*()_+{}|:\"<>?`-=[]\\;',./ ";
// @formatter:off
ClientRegistration clientRegistration = TestClientRegistrations.clientCredentials()
.clientId(clientCredentialWithAnsiKeyboardSpecialCharacters)
.clientSecret(clientCredentialWithAnsiKeyboardSpecialCharacters)
.build();
// @formatter:on
OAuth2ClientCredentialsGrantRequest clientCredentialsGrantRequest = new OAuth2ClientCredentialsGrantRequest(
clientRegistration);
RequestEntity<?> requestEntity = this.converter.convert(clientCredentialsGrantRequest);
assertThat(requestEntity.getMethod()).isEqualTo(HttpMethod.POST);
assertThat(requestEntity.getUrl().toASCIIString())
.isEqualTo(clientRegistration.getProviderDetails().getTokenUri());
HttpHeaders headers = requestEntity.getHeaders();
assertThat(headers.getAccept()).contains(MediaType.APPLICATION_JSON_UTF8);
assertThat(headers.getContentType())
.isEqualTo(MediaType.valueOf(MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=UTF-8"));
String urlEncodedClientCredential = URLEncoder.encode(clientCredentialWithAnsiKeyboardSpecialCharacters,
StandardCharsets.UTF_8.toString());
String clientCredentials = Base64.getEncoder().encodeToString(
(urlEncodedClientCredential + ":" + urlEncodedClientCredential).getBytes(StandardCharsets.UTF_8));
assertThat(headers.getFirst(HttpHeaders.AUTHORIZATION)).isEqualTo("Basic " + clientCredentials);
MultiValueMap<String, String> formParameters = (MultiValueMap<String, String>) requestEntity.getBody();
assertThat(formParameters.getFirst(OAuth2ParameterNames.GRANT_TYPE))
.isEqualTo(AuthorizationGrantType.CLIENT_CREDENTIALS.getValue());
assertThat(formParameters.getFirst(OAuth2ParameterNames.SCOPE)).contains(clientRegistration.getScopes());
}
}
36 changes: 1 addition & 35 deletions ...ty/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClientTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2019 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,17 +16,12 @@

package org.springframework.security.oauth2.client.endpoint;

import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Base64;

import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.security.oauth2.client.registration.ClientRegistration;
Expand Down Expand Up @@ -87,35 +82,6 @@ public void getTokenResponseWhenHeaderThenSuccess() throws Exception {
assertThat(body).isEqualTo("grant_type=client_credentials&scope=read%3Auser");
}

// gh-9610
@Test
public void getTokenResponseWhenSpecialCharactersThenSuccessWithEncodedClientCredentials() throws Exception {
// @formatter:off
enqueueJson("{\n"
+ " \"access_token\":\"MTQ0NjJkZmQ5OTM2NDE1ZTZjNGZmZjI3\",\n"
+ " \"token_type\":\"bearer\",\n"
+ " \"expires_in\":3600,\n"
+ " \"refresh_token\":\"IwOGYzYTlmM2YxOTQ5MGE3YmNmMDFkNTVk\",\n"
+ " \"scope\":\"create\"\n"
+ "}");
// @formatter:on
String clientCredentialWithAnsiKeyboardSpecialCharacters = "~!@#$%^&*()_+{}|:\"<>?`-=[]\\;',./ ";
OAuth2ClientCredentialsGrantRequest request = new OAuth2ClientCredentialsGrantRequest(
this.clientRegistration.clientId(clientCredentialWithAnsiKeyboardSpecialCharacters)
.clientSecret(clientCredentialWithAnsiKeyboardSpecialCharacters).build());
OAuth2AccessTokenResponse response = this.client.getTokenResponse(request).block();
RecordedRequest actualRequest = this.server.takeRequest();
String body = actualRequest.getBody().readUtf8();
assertThat(response.getAccessToken()).isNotNull();
String urlEncodedClientCredentialecret = URLEncoder.encode(clientCredentialWithAnsiKeyboardSpecialCharacters,
StandardCharsets.UTF_8.toString());
String clientCredentials = Base64.getEncoder()
.encodeToString((urlEncodedClientCredentialecret + ":" + urlEncodedClientCredentialecret)
.getBytes(StandardCharsets.UTF_8));
assertThat(actualRequest.getHeader(HttpHeaders.AUTHORIZATION)).isEqualTo("Basic " + clientCredentials);
assertThat(body).isEqualTo("grant_type=client_credentials&scope=read%3Auser");
}

@Test
public void getTokenResponseWhenPostThenSuccess() throws Exception {
ClientRegistration registration = this.clientRegistration
Expand Down

0 comments on commit e1b6a7b

Please sign in to comment.