Skip to content

URL encode client credentials #9791

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 1, 2021
Merged

URL encode client credentials #9791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions ...rity/oauth2/client/endpoint/AbstractWebClientReactiveOAuth2AccessTokenResponseClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2021 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,6 +16,9 @@

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.Set;

Expand Down Expand Up @@ -97,7 +100,19 @@ private void populateTokenRequestHeaders(T grantRequest, HttpHeaders headers) {
headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));
if (ClientAuthenticationMethod.CLIENT_SECRET_BASIC.equals(clientRegistration.getClientAuthenticationMethod())
|| ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());
String clientId = encodeClientCredential(clientRegistration.getClientId());
String clientSecret = encodeClientCredential(clientRegistration.getClientSecret());
headers.setBasicAuth(clientId, clientSecret);
}
}

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);
}
}

Expand Down
19 changes: 17 additions & 2 deletions ...framework/security/oauth2/client/endpoint/OAuth2AuthorizationGrantRequestEntityUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2021 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,6 +16,9 @@

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;
Expand Down Expand Up @@ -48,11 +51,23 @@ static HttpHeaders getTokenRequestHeaders(ClientRegistration clientRegistration)
headers.addAll(DEFAULT_TOKEN_REQUEST_HEADERS);
if (ClientAuthenticationMethod.CLIENT_SECRET_BASIC.equals(clientRegistration.getClientAuthenticationMethod())
|| ClientAuthenticationMethod.BASIC.equals(clientRegistration.getClientAuthenticationMethod())) {
headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());
String clientId = encodeClientCredential(clientRegistration.getClientId());
String clientSecret = encodeClientCredential(clientRegistration.getClientSecret());
headers.setBasicAuth(clientId, clientSecret);
}
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
38 changes: 38 additions & 0 deletions ...urity/oauth2/client/endpoint/OAuth2ClientCredentialsGrantRequestEntityConverterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@

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.mockito.InOrder;
Expand Down Expand Up @@ -128,4 +133,37 @@ public void convertWhenGrantRequestValidThenConverts() {
assertThat(formParameters.getFirst(OAuth2ParameterNames.SCOPE)).contains(clientRegistration.getScopes());
}

// 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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjohnr Rather than compare it to exactly the same code, actually specify the expected encoded version. e.g. with the case from the RFC:

assertEquals("+%25%26%2B%C2%A3%E2%82%AC", encodeClientCredential(" %&+£€"));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OrangeDog that's a good idea too. However, instead of testing the private method, we could probably test the end result with hard-coded characters. Would you be interested in submitting a PR with an additional test using characters like those (not from the ansi keyboard, as mine was), and asserting like:

assertThat(actualRequest.getHeader(HttpHeaders.AUTHORIZATION)).isEqualTo("Basic <hard-coded encoded credentials here>");

Note: I was mostly concerned with using the test case to ensure that the code complied with the spec, not ensuring specific characters are encoded correctly with Java's encoding mechanism, hence re-using the URLEncoder class.

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());
}

}
35 changes: 34 additions & 1 deletion ...ty/oauth2/client/endpoint/WebClientReactiveClientCredentialsTokenResponseClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2021 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,6 +16,10 @@

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;
Expand Down Expand Up @@ -89,6 +93,35 @@ 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