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

Regression with URL encode client credentials #10018 #11435

Closed
scizeron opened this issue Jun 24, 2022 · 1 comment
Closed

Regression with URL encode client credentials #10018 #11435

scizeron opened this issue Jun 24, 2022 · 1 comment
Assignees
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: declined A suggestion or change that we don't feel we should currently apply

Comments

@scizeron
Copy link

scizeron commented Jun 24, 2022

Describe the bug
This bug is described here "Regression with URL encode client credentials #10018", it was fixed in 5.5.2 but it appears again in
5.6.x
https://github.com/spring-projects/spring-security/blob/5.6.0/oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/endpoint/OAuth2AuthorizationGrantRequestEntityUtils.java

BUG:

  String clientId = **encodeClientCredential**(clientRegistration.getClientId());
  String clientSecret = **encodeClientCredential**(clientRegistration.getClientSecret());
  headers.setBasicAuth(clientId, clientSecret);

with

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

FIX:

  headers.setBasicAuth(clientRegistration.getClientId(), clientRegistration.getClientSecret());

with setBasicAuth :

		String **credentialsString = username + ":" + password**;
		byte[] encodedBytes = Base64.getEncoder().encode(credentialsString.getBytes(charset));

To Reproduce
Use spring boot 2.6 and oauth2 client credentials. If client Secret contains specific characters => URL encoding

Expected behavior
Restores the 5.5.2 fix please

Sample

A link to a GitHub repository with a minimal, reproducible sample.

Reports that include a sample will take priority over reports that do not.
At times, we may require a sample, so it is good to try and include a sample up front.

@scizeron scizeron added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Jun 24, 2022
@rwinch rwinch self-assigned this Jun 24, 2022
@rwinch rwinch added the in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) label Jun 24, 2022
@rwinch
Copy link
Member

rwinch commented Jun 24, 2022

Thank you for getting in touch. This is not a bug in Spring Security OAuth. It is a bit hard to follow everything that happened in the thread, but I'll give you a summary:

The issue gh-10018 was a fix because the OAuth specification states that client credentials and secret should be encoded using the "application/x-www-form-urlencoded"
encoding algorithm
.

Given this broke quite a few users, the fix was reverted from the 5.2.x 5.3.x 5.4.x and 5.5.x but was kept for 5.6.x and forward. This is likely why you believe that it was fixed in 5.5.x (the commit was reverted and the actual fix does not appear until 5.6.x).

If you have an authorization server that is not URL encoding the client and secret it is not following the specification. You can work around that using the links on this comment.

Given the number of people using Authorization Servers that do not properly URL Encode the client id and secret, I created gh-11440 to simplify the configuration.

@rwinch rwinch closed this as completed Jun 24, 2022
@rwinch rwinch added for: stackoverflow A question that's better suited to stackoverflow.com status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged type: bug A general bug for: stackoverflow A question that's better suited to stackoverflow.com labels Jun 24, 2022
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: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

2 participants