Skip to content

Commit 2598bf8

Browse files
sjohnrsjohnr
committed
Polish gh-14859 
1 parent d0adb2a commit 2598bf8

File tree

6 files changed

+173
-87
lines changed

6 files changed

+173
-87
lines changed

Diff for: ‎docs/modules/ROOT/pages/servlet/oauth2/client/client-authentication.adoc

-3
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,6 @@ val tokenResponseClient = DefaultAuthorizationCodeTokenResponseClient()
9292
tokenResponseClient.setRequestEntityConverter(requestEntityConverter)
9393
----
9494
======
95-
[NOTE]
96-
If you're using the `client-authentication-method: client_secret_basic` and you need to skip URL encoding,
97-
create a new `DefaultOAuth2TokenRequestHeadersConverter` and set it in the Request Entity Converter above.
9895

9996
=== Authenticate using `client_secret_jwt`
10097

Diff for: ‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/AbstractOAuth2AuthorizationGrantRequestEntityConverter.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,7 @@
4242
abstract class AbstractOAuth2AuthorizationGrantRequestEntityConverter<T extends AbstractOAuth2AuthorizationGrantRequest>
4343
implements Converter<T, RequestEntity<?>> {
4444

45-
private Converter<T, HttpHeaders> headersConverter = DefaultOAuth2TokenRequestHeadersConverter
46-
.historicalConverter();
45+
private Converter<T, HttpHeaders> headersConverter = DefaultOAuth2TokenRequestHeadersConverter.withCharsetUtf8();
4746

4847
private Converter<T, MultiValueMap<String, String>> parametersConverter = this::createParameters;
4948

Diff for: ‎oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/DefaultOAuth2TokenRequestHeadersConverter.java

+42-43
Original file line numberDiff line numberDiff line change
@@ -16,94 +16,93 @@
1616

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

19+
import java.net.URLEncoder;
20+
import java.nio.charset.StandardCharsets;
21+
import java.util.List;
22+
1923
import org.springframework.core.convert.converter.Converter;
2024
import org.springframework.http.HttpHeaders;
2125
import org.springframework.http.MediaType;
2226
import org.springframework.http.RequestEntity;
2327
import org.springframework.security.oauth2.client.registration.ClientRegistration;
2428
import org.springframework.security.oauth2.core.ClientAuthenticationMethod;
2529

26-
import java.net.URLEncoder;
27-
import java.nio.charset.StandardCharsets;
28-
import java.util.Collections;
29-
3030
/**
3131
* Default {@link Converter} used to convert an
32-
* {@link AbstractOAuth2AuthorizationGrantRequest} to the {@link HttpHeaders} of aKk
32+
* {@link AbstractOAuth2AuthorizationGrantRequest} to the {@link HttpHeaders} of a
3333
* {@link RequestEntity} representation of an OAuth 2.0 Access Token Request for the
3434
* specific Authorization Grant.
3535
*
3636
* @author Peter Eastham
37-
* @author Joe Grandja
38-
* @see AbstractOAuth2AuthorizationGrantRequestEntityConverter
37+
* @author Steve Riesenberg
3938
* @since 6.3
39+
* @see AbstractOAuth2AuthorizationGrantRequestEntityConverter
4040
*/
4141
public final class DefaultOAuth2TokenRequestHeadersConverter<T extends AbstractOAuth2AuthorizationGrantRequest>
4242
implements Converter<T, HttpHeaders> {
4343

44-
private MediaType accept = MediaType.APPLICATION_JSON;
44+
private static final MediaType APPLICATION_JSON_UTF8 = new MediaType(MediaType.APPLICATION_JSON,
45+
StandardCharsets.UTF_8);
46+
47+
private static final MediaType APPLICATION_FORM_URLENCODED_UTF8 = new MediaType(
48+
MediaType.APPLICATION_FORM_URLENCODED, StandardCharsets.UTF_8);
49+
50+
private List<MediaType> accept = List.of(MediaType.APPLICATION_JSON);
4551

4652
private MediaType contentType = MediaType.APPLICATION_FORM_URLENCODED;
4753

48-
private boolean encodeClientCredentialsIfRequired = true;
54+
private boolean encodeClientCredentials = true;
4955

5056
/**
51-
* Populates the headers for the token request.
52-
* @param grantRequest the grant request
57+
* Populates the default headers for the token request.
58+
* @param grantRequest the authorization grant request
5359
* @return the headers populated for the token request
5460
*/
5561
@Override
5662
public HttpHeaders convert(T grantRequest) {
5763
HttpHeaders headers = new HttpHeaders();
58-
headers.setAccept(Collections.singletonList(accept));
59-
headers.setContentType(contentType);
64+
headers.setAccept(this.accept);
65+
headers.setContentType(this.contentType);
6066
ClientRegistration clientRegistration = grantRequest.getClientRegistration();
6167
if (ClientAuthenticationMethod.CLIENT_SECRET_BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
62-
String clientId = encodeClientCredential(clientRegistration.getClientId());
63-
String clientSecret = encodeClientCredential(clientRegistration.getClientSecret());
68+
String clientId = encodeClientCredentialIfRequired(clientRegistration.getClientId());
69+
String clientSecret = encodeClientCredentialIfRequired(clientRegistration.getClientSecret());
6470
headers.setBasicAuth(clientId, clientSecret);
6571
}
6672
return headers;
6773
}
6874

69-
private String encodeClientCredential(String clientCredential) {
70-
String encodedCredential = clientCredential;
71-
if (this.encodeClientCredentialsIfRequired) {
72-
encodedCredential = URLEncoder.encode(clientCredential, StandardCharsets.UTF_8);
75+
private String encodeClientCredentialIfRequired(String clientCredential) {
76+
if (!this.encodeClientCredentials) {
77+
return clientCredential;
7378
}
74-
return encodedCredential;
75-
}
76-
77-
/**
78-
* Sets the behavior for if this URL Encoding the Client Credentials during the
79-
* conversion.
80-
* @param encodeClientCredentialsIfRequired if false, no URL encoding will happen
81-
*/
82-
public void setEncodeClientCredentials(boolean encodeClientCredentialsIfRequired) {
83-
this.encodeClientCredentialsIfRequired = encodeClientCredentialsIfRequired;
79+
return URLEncoder.encode(clientCredential, StandardCharsets.UTF_8);
8480
}
8581

8682
/**
87-
* MediaType to set for the Accept header. Default is application/json
88-
* @param accept MediaType to use for the Accept header
83+
* Sets whether the client credentials of the {@code Authorization} header will be
84+
* encoded using the {@code application/x-www-form-urlencoded} encoding algorithm
85+
* according to RFC 6749. Default is {@code true}.
86+
* @param encodeClientCredentials whether the client credentials will be encoded
87+
* @see <a target="_blank" href=
88+
* "https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1">2.3.1 Client
89+
* Password</a>
8990
*/
90-
private void setAccept(MediaType accept) {
91-
this.accept = accept;
91+
public void setEncodeClientCredentials(boolean encodeClientCredentials) {
92+
this.encodeClientCredentials = encodeClientCredentials;
9293
}
9394

9495
/**
95-
* MediaType to set for the Content Type header. Default is
96-
* application/x-www-form-urlencoded
97-
* @param contentType MediaType to use for the Content Type header
96+
* Creates a {@link DefaultOAuth2TokenRequestHeadersConverter} that populates default
97+
* {@link HttpHeaders} that includes {@code charset=UTF-8} on both the {@code Accept}
98+
* and {@code Content-Type} headers to provide backwards compatibility for
99+
* {@link AbstractOAuth2AuthorizationGrantRequestEntityConverter}.
100+
* @return the default headers converter
98101
*/
99-
private void setContentType(MediaType contentType) {
100-
this.contentType = contentType;
101-
}
102-
103-
static <T extends AbstractOAuth2AuthorizationGrantRequest> DefaultOAuth2TokenRequestHeadersConverter<T> historicalConverter() {
102+
static <T extends AbstractOAuth2AuthorizationGrantRequest> DefaultOAuth2TokenRequestHeadersConverter<T> withCharsetUtf8() {
104103
DefaultOAuth2TokenRequestHeadersConverter<T> converter = new DefaultOAuth2TokenRequestHeadersConverter<>();
105-
converter.setAccept(MediaType.APPLICATION_JSON_UTF8);
106-
converter.setContentType(MediaType.valueOf(MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=UTF-8"));
104+
converter.accept = List.of(APPLICATION_JSON_UTF8);
105+
converter.contentType = APPLICATION_FORM_URLENCODED_UTF8;
107106
return converter;
108107
}
109108

Diff for: ‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/DefaultOAuth2TokenRequestHeadersConverterTests.java

+119
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.security.oauth2.client.endpoint;
18+
19+
import java.nio.charset.StandardCharsets;
20+
21+
import org.junit.jupiter.api.BeforeEach;
22+
import org.junit.jupiter.api.Test;
23+
24+
import org.springframework.http.HttpHeaders;
25+
import org.springframework.http.MediaType;
26+
import org.springframework.security.oauth2.client.registration.ClientRegistration;
27+
import org.springframework.security.oauth2.client.registration.TestClientRegistrations;
28+
29+
import static org.assertj.core.api.Assertions.assertThat;
30+
31+
/**
32+
* Tests for {@link DefaultOAuth2TokenRequestHeadersConverter}.
33+
*
34+
* @author Steve Riesenberg
35+
*/
36+
public class DefaultOAuth2TokenRequestHeadersConverterTests {
37+
38+
private static final MediaType APPLICATION_JSON_UTF8 = new MediaType(MediaType.APPLICATION_JSON,
39+
StandardCharsets.UTF_8);
40+
41+
private static final MediaType APPLICATION_FORM_URLENCODED_UTF8 = new MediaType(
42+
MediaType.APPLICATION_FORM_URLENCODED, StandardCharsets.UTF_8);
43+
44+
private DefaultOAuth2TokenRequestHeadersConverter<OAuth2ClientCredentialsGrantRequest> converter;
45+
46+
@BeforeEach
47+
public void setUp() {
48+
this.converter = new DefaultOAuth2TokenRequestHeadersConverter<>();
49+
}
50+
51+
@Test
52+
public void convertWhenEncodeClientCredentialsTrueThenConvertsWithUrlEncoding() {
53+
// @formatter:off
54+
ClientRegistration clientRegistration = TestClientRegistrations.clientCredentials()
55+
.clientId("clientId")
56+
.clientSecret("clientSecret=")
57+
.build();
58+
// @formatter:on
59+
OAuth2ClientCredentialsGrantRequest grantRequest = new OAuth2ClientCredentialsGrantRequest(clientRegistration);
60+
HttpHeaders defaultHeaders = this.converter.convert(grantRequest);
61+
assertThat(defaultHeaders.getAccept()).containsExactly(MediaType.APPLICATION_JSON);
62+
assertThat(defaultHeaders.getContentType()).isEqualTo(MediaType.APPLICATION_FORM_URLENCODED);
63+
assertThat(defaultHeaders.getFirst(HttpHeaders.AUTHORIZATION))
64+
.isEqualTo("Basic Y2xpZW50SWQ6Y2xpZW50U2VjcmV0JTNE");
65+
}
66+
67+
@Test
68+
public void convertWhenEncodeClientCredentialsFalseThenConvertsWithoutUrlEncoding() {
69+
this.converter.setEncodeClientCredentials(false);
70+
// @formatter:off
71+
ClientRegistration clientRegistration = TestClientRegistrations.clientCredentials()
72+
.clientId("clientId")
73+
.clientSecret("clientSecret=")
74+
.build();
75+
// @formatter:on
76+
OAuth2ClientCredentialsGrantRequest grantRequest = new OAuth2ClientCredentialsGrantRequest(clientRegistration);
77+
HttpHeaders defaultHeaders = this.converter.convert(grantRequest);
78+
assertThat(defaultHeaders.getAccept()).containsExactly(MediaType.APPLICATION_JSON);
79+
assertThat(defaultHeaders.getContentType()).isEqualTo(MediaType.APPLICATION_FORM_URLENCODED);
80+
assertThat(defaultHeaders.getFirst(HttpHeaders.AUTHORIZATION))
81+
.isEqualTo("Basic Y2xpZW50SWQ6Y2xpZW50U2VjcmV0PQ==");
82+
}
83+
84+
@Test
85+
public void convertWhenWithCharsetUtf8AndEncodeClientCredentialsTrueThenConvertsWithUrlEncoding() {
86+
this.converter = DefaultOAuth2TokenRequestHeadersConverter.withCharsetUtf8();
87+
// @formatter:off
88+
ClientRegistration clientRegistration = TestClientRegistrations.clientCredentials()
89+
.clientId("clientId")
90+
.clientSecret("clientSecret=")
91+
.build();
92+
// @formatter:on
93+
OAuth2ClientCredentialsGrantRequest grantRequest = new OAuth2ClientCredentialsGrantRequest(clientRegistration);
94+
HttpHeaders defaultHeaders = this.converter.convert(grantRequest);
95+
assertThat(defaultHeaders.getAccept()).containsExactly(APPLICATION_JSON_UTF8);
96+
assertThat(defaultHeaders.getContentType()).isEqualTo(APPLICATION_FORM_URLENCODED_UTF8);
97+
assertThat(defaultHeaders.getFirst(HttpHeaders.AUTHORIZATION))
98+
.isEqualTo("Basic Y2xpZW50SWQ6Y2xpZW50U2VjcmV0JTNE");
99+
}
100+
101+
@Test
102+
public void convertWhenWithCharsetUtf8EncodeClientCredentialsFalseThenConvertsWithoutUrlEncoding() {
103+
this.converter = DefaultOAuth2TokenRequestHeadersConverter.withCharsetUtf8();
104+
this.converter.setEncodeClientCredentials(false);
105+
// @formatter:off
106+
ClientRegistration clientRegistration = TestClientRegistrations.clientCredentials()
107+
.clientId("clientId")
108+
.clientSecret("clientSecret=")
109+
.build();
110+
// @formatter:on
111+
OAuth2ClientCredentialsGrantRequest grantRequest = new OAuth2ClientCredentialsGrantRequest(clientRegistration);
112+
HttpHeaders defaultHeaders = this.converter.convert(grantRequest);
113+
assertThat(defaultHeaders.getAccept()).containsExactly(APPLICATION_JSON_UTF8);
114+
assertThat(defaultHeaders.getContentType()).isEqualTo(APPLICATION_FORM_URLENCODED_UTF8);
115+
assertThat(defaultHeaders.getFirst(HttpHeaders.AUTHORIZATION))
116+
.isEqualTo("Basic Y2xpZW50SWQ6Y2xpZW50U2VjcmV0PQ==");
117+
}
118+
119+
}

Diff for: ‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/OAuth2AuthorizationCodeGrantRequestEntityConverterTests.java

+8-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -122,7 +122,12 @@ public void convertWhenParametersConverterSetThenCalled() {
122122
@SuppressWarnings("unchecked")
123123
@Test
124124
public void convertWhenGrantRequestValidThenConverts() {
125-
ClientRegistration clientRegistration = TestClientRegistrations.clientRegistration().build();
125+
// @formatter:off
126+
ClientRegistration clientRegistration = TestClientRegistrations.clientRegistration()
127+
.clientId("clientId")
128+
.clientSecret("clientSecret=")
129+
.build();
130+
// @formatter:on
126131
OAuth2AuthorizationExchange authorizationExchange = TestOAuth2AuthorizationExchanges.success();
127132
OAuth2AuthorizationRequest authorizationRequest = authorizationExchange.getAuthorizationRequest();
128133
OAuth2AuthorizationResponse authorizationResponse = authorizationExchange.getAuthorizationResponse();
@@ -136,7 +141,7 @@ public void convertWhenGrantRequestValidThenConverts() {
136141
assertThat(headers.getAccept()).contains(MediaType.APPLICATION_JSON_UTF8);
137142
assertThat(headers.getContentType())
138143
.isEqualTo(MediaType.valueOf(MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=UTF-8"));
139-
assertThat(headers.getFirst(HttpHeaders.AUTHORIZATION)).startsWith("Basic ");
144+
assertThat(headers.getFirst(HttpHeaders.AUTHORIZATION)).isEqualTo("Basic Y2xpZW50SWQ6Y2xpZW50U2VjcmV0JTNE");
140145
MultiValueMap<String, String> formParameters = (MultiValueMap<String, String>) requestEntity.getBody();
141146
assertThat(formParameters.getFirst(OAuth2ParameterNames.GRANT_TYPE))
142147
.isEqualTo(AuthorizationGrantType.AUTHORIZATION_CODE.getValue());

Diff for: ‎oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/endpoint/OAuth2PasswordGrantRequestEntityConverterTests.java

+3-36
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2021 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -110,10 +110,7 @@ public void convertWhenParametersConverterSetThenCalled() {
110110
@SuppressWarnings("unchecked")
111111
@Test
112112
public void convertWhenGrantRequestValidThenConverts() {
113-
ClientRegistration clientRegistration = TestClientRegistrations.password()
114-
.clientId("clientId")
115-
.clientSecret("clientSecret=")
116-
.build();
113+
ClientRegistration clientRegistration = TestClientRegistrations.password().build();
117114
OAuth2PasswordGrantRequest passwordGrantRequest = new OAuth2PasswordGrantRequest(clientRegistration, "user1",
118115
"password");
119116
RequestEntity<?> requestEntity = this.converter.convert(passwordGrantRequest);
@@ -124,7 +121,7 @@ public void convertWhenGrantRequestValidThenConverts() {
124121
assertThat(headers.getAccept()).contains(MediaType.APPLICATION_JSON_UTF8);
125122
assertThat(headers.getContentType())
126123
.isEqualTo(MediaType.valueOf(MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=UTF-8"));
127-
assertThat(headers.getFirst(HttpHeaders.AUTHORIZATION)).isEqualTo("Basic Y2xpZW50SWQ6Y2xpZW50U2VjcmV0JTNE");
124+
assertThat(headers.getFirst(HttpHeaders.AUTHORIZATION)).startsWith("Basic ");
128125
MultiValueMap<String, String> formParameters = (MultiValueMap<String, String>) requestEntity.getBody();
129126
assertThat(formParameters.getFirst(OAuth2ParameterNames.GRANT_TYPE))
130127
.isEqualTo(AuthorizationGrantType.PASSWORD.getValue());
@@ -133,34 +130,4 @@ public void convertWhenGrantRequestValidThenConverts() {
133130
assertThat(formParameters.getFirst(OAuth2ParameterNames.SCOPE)).contains(clientRegistration.getScopes());
134131
}
135132

136-
@SuppressWarnings("unchecked")
137-
@Test
138-
public void convertWhenGrantRequestValidThenConvertsWithoutUrlEncoding() {
139-
ClientRegistration clientRegistration = TestClientRegistrations.password()
140-
.clientId("clientId")
141-
.clientSecret("clientSecret=")
142-
.build();
143-
OAuth2PasswordGrantRequest passwordGrantRequest = new OAuth2PasswordGrantRequest(clientRegistration, "user1",
144-
"password=");
145-
DefaultOAuth2TokenRequestHeadersConverter<OAuth2PasswordGrantRequest> headersConverter = DefaultOAuth2TokenRequestHeadersConverter
146-
.historicalConverter();
147-
headersConverter.setEncodeClientCredentials(false);
148-
this.converter.setHeadersConverter(headersConverter);
149-
RequestEntity<?> requestEntity = this.converter.convert(passwordGrantRequest);
150-
assertThat(requestEntity.getMethod()).isEqualTo(HttpMethod.POST);
151-
assertThat(requestEntity.getUrl().toASCIIString())
152-
.isEqualTo(clientRegistration.getProviderDetails().getTokenUri());
153-
HttpHeaders headers = requestEntity.getHeaders();
154-
assertThat(headers.getAccept()).contains(MediaType.APPLICATION_JSON_UTF8);
155-
assertThat(headers.getContentType())
156-
.isEqualTo(MediaType.valueOf(MediaType.APPLICATION_FORM_URLENCODED_VALUE + ";charset=UTF-8"));
157-
assertThat(headers.getFirst(HttpHeaders.AUTHORIZATION)).isEqualTo("Basic Y2xpZW50SWQ6Y2xpZW50U2VjcmV0PQ==");
158-
MultiValueMap<String, String> formParameters = (MultiValueMap<String, String>) requestEntity.getBody();
159-
assertThat(formParameters.getFirst(OAuth2ParameterNames.GRANT_TYPE))
160-
.isEqualTo(AuthorizationGrantType.PASSWORD.getValue());
161-
assertThat(formParameters.getFirst(OAuth2ParameterNames.USERNAME)).isEqualTo("user1");
162-
assertThat(formParameters.getFirst(OAuth2ParameterNames.PASSWORD)).isEqualTo("password=");
163-
assertThat(formParameters.getFirst(OAuth2ParameterNames.SCOPE)).contains(clientRegistration.getScopes());
164-
}
165-
166133
}

0 commit comments

Comments
 (0)