Regression with URL encode client credentials

[petergphillips]

Describe the bug
We are using https://spring.io/projects/spring-security-oauth as an oauth2 provider.
In separate projects we are then using the spring security webclient provided by this project.
We tried upgrading to Spring Boot 2.5.2 from 2.5.0, which bundled Spring Security 5.5.1 instead of 5.5.0 and noticed that our applications broke and our client authentication attempts were rejected by our oauth2 provider.

It appears that the change in e6c268a now causes both the client id and client secret to be url encoded. This means that any client secrets containing those special characters then don't work any more as the oauth2 provider doesn't url decode the client id / secret before testing them.

To Reproduce
Upgrade to Spring Security 5.5.1 with a client secret with special characters (e.g. space, quote) that will be url encoded. The basic authentication header will then be sent with the client id / secret url encoded as well as base64 encoded. This breaks oauth2 providers that aren't expecting the basic authentication header elements to be url encoded too.

Expected behavior
We expect that the client id and client secret are not url encoded before being transmitted. That is what the spring security oauth provider is expecting and is current behaviour.
https://datatracker.ietf.org/doc/html/rfc2617#section-2 specifies that the client id and secret are base64 encoded before being transmitted anyway, so it seems odd to url encode them and then base64 encode them immediately afterwards anyway.

The equivalent ticket for our oauth2 provider in spring-attic/spring-security-oauth#1826 is still open, indicating that there hasn't been a fix there for the server. Reading the comment in #9610 (comment):

applying the fix now may break existing applications that are using non-compliant providers.

which is exactly what has happened to us.

[petergphillips]

I guess at the very least, there should be an option in the configuration to choose whether the oauth2 provider is expecting the client id / secret to be url encoded before being base64 encoded so that we can turn it off for our provider.

[rainerfrey-inxmail]

See also #9610 (comment)

We'll come up with a strategy that does not break existing applications.

[rpaasche]

BTW. Same for 5.4.7.

[jgrandja]

@petergphillips @rainerfrey-inxmail @rpaasche

As per spec, in Section 2.3.1. Client Password:

Clients in possession of a client password, also known as a client
secret, MAY use the HTTP Basic authentication scheme as defined in
[RFC2617] to authenticate with the authorization server. The client
identifier is encoded using the "application/x-www-form-urlencoded"
encoding algorithm per Appendix B, and the encoded value is used as
the username; the client secret is encoded using the same algorithm
and used as the password. The authorization server MUST support the
HTTP Basic authentication scheme for authenticating clients that were
issued a client secret.

This bug was reported in gh-9610 and was fixed in 5.5 and backported to older branches.

Regarding my comment:

We're going to apply this fix in 5.5 since this is a bug. We'll come up with a strategy that does not break existing applications.

Unfortunately, this fix has broken your existing applications but this is a bug and needed to be fixed on the client. The provider is non-compliant and should be fixed there.

In order to get the client working with a non-compliant provider, you can customize the Token Request.
See the reference for customizing the DefaultAuthorizationCodeTokenResponseClient.

You would supply a custom Converter<OAuth2AuthorizationCodeGrantRequest, RequestEntity<?>> to DefaultAuthorizationCodeTokenResponseClient.setRequestEntityConverter(). Also see OAuth2AuthorizationCodeGrantRequestEntityConverter.setHeadersConverter().

I'm going to close this since the issue is with the non-compliant provider.