Query parameters in authorization-url are double-encoded

[manuelbl]

Summary

If the autorization URL of a OAuth 2.0 provider contains query parameters with URL encoded characters, they are encoded again and the request is rejected by the IdP.

Example: If the authorization URL in application.yml is configured as:

authorization-uri: 'https://iam.abc.com/idp/oauth2/authorize?claims=%7B%22urn%3Aabc%3Aplace_of_birth%22%3Anull%7D

Then the resulting URL becomes (somewhat shortened):

https://iam.abc.com/idp/oauth2/authorize?acr_values=quo1&claims=%257B%2522urn%253Aabc%253Aplace_of_birth%2522%253Anull%257D&response_type=code&client_id=c1&scope=openid%20profile%20email&state=83l9MUgoab0w&redirect_uri=http://localhost:8081/login/oauth2/code/abc&nonce=w4VgoHfH9f71'

The claims parameter is now double encoded. The IdP doesn't understand it anymore and rejects the authorization request.

Actual Behavior

The query parameter value is double encoded and no longer valid for the IdP.

Correct value in configuration:

claims=%7B%22urn%3Aabc%3Aplace_of_birth%22%3Anull%7D

Invalid value in request:

claims=claims=%257B%2522urn%253Aabc%253Aplace_of_birth%2522%253Anull%257D

Expected Behavior

No double encoding should be applied. The query parameter value should be retained without changes.

Correct value in configuration:

claims=%7B%22urn%3Aabc%3Aplace_of_birth%22%3Anull%7D

Correct value in request:

claims=%7B%22urn%3Aabc%3Aplace_of_birth%22%3Anull%7D

Notes

If an invalid URL without URL encoding is used::

claims={"urn:abc:place_of_birth":null}

the resulting URL is invalid as well (and rejected by the IdP):

claims={%22urn:abc:date_of_birth%22:null}

Furthermore, if URLs were to be entered without encoding the query parameters, many parameter values could not be entered as the URL would no longer be parseable.

I am aware of issue #5760 and the associated PR #6299. Unfortunately, it fixes the problem only partially.

At first look, it seems that the root cause is at OAuth2AuthorizationRequest.java, line 395 where UriComponentsBuilder.fromHttpUrl is called. This method is for parsing URL templates that contain substitutable components and has documented limitations regarding query parameters. Since substitutable components are not needed, a more suitable method should be used instead.

Configuration

application.yml

server:
  port: 8081

spring:
  security:
    oauth2:
      client:
        registration:
          abcfund:
            client-id: abc
            client-secret: 0123456789abcdef
            clientAuthenticationMethod: post
            provider: my-idp
            scope:
              - openid
              - profile
              - email
        provider:
          my-idp:
            issuer-uri: https://iam.abc.com/idp/oauth2
            authorization-uri: 'https://iam.abc.com/idp/oauth2/authorize?acr_values=quo1&claims=%7B%22urn%3Aabc%3Aplace_of_birth%22%3Anull%7D'

build.gradle

plugins {
    id 'java'
    id 'org.springframework.boot' version '2.2.4.RELEASE'
    id 'io.spring.dependency-management' version '1.0.9.RELEASE'
}

group = 'com.example'
version = '0.0.1-SNAPSHOT'
sourceCompatibility = '11'

repositories {
    mavenCentral()
}

dependencies {
    implementation 'org.springframework.boot:spring-boot-starter-web'
    implementation 'org.springframework.boot:spring-boot-starter-oauth2-client'
}

Version

• Spring Security 5.2.1
• Spring Boot 2.2.4

Sample

In addition to the above configurations, the following code completes a sample. There is no custom WebSecurityConfigurerAdapter. The out-of-the-box configuration sufficient.

DemoApplication.java

package demo;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;

@SpringBootApplication
public class DemoApplication {
    public static void main(String[] args) {
        SpringApplication.run(DemoApplication.class, args);
    }
}

HomeController.java

package demo;

import org.springframework.security.core.Authentication;
import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;

import java.util.Map;

@RestController
public class HomeController {

    @GetMapping("/")
    Map<String, Object> hello(Authentication authentication) {
        OAuth2AuthenticationToken token = (OAuth2AuthenticationToken)authentication;
        return token.getPrincipal().getAttributes();
    }

}

[manuelbl]

A further analysis reveals two problems:

• UriComponentsBuilder.fromHttpUrl (OAuth2AuthorizationRequest.java, line 395) parses the authorization URL without URL decoding the query parameters. Then further query parameters are added (nonce, redirect_uri etc.). The result is a mixture of encoded and unencoded parameters without any change to generate a correct URL in complex cases.
• With a reference to non-compliant web servers (see authorization_uri Supports Query Parameters #6299 (comment)) regarding the / and ? characters, template encoding instead of full URL encoding is chosen (builder.encode().build() instead of builder.build(true)). However, the encodings differ in many characters, not just / and ?. And worst of all, the template encoding has special treatment of { and } as it expects a template like https://host/customers/{customer-id}. Yet the authorization URL is no template at all.

The correct way forward should be:

• parse the authorization URL such that query parameters are properly decoded
• add the additional parameters (nonce, redirect_uri etc.)
• fully encode all parameters.

At no point should the authorization URL be treated as a template URL.

The implication (aside from fixing the double encoding) would be that the redirect_uri is encoded differently. Instead of:

...Ugoab0w&redirect_uri=http://localhost:8081/login/oauth2/code/abc&nonce=w4Vg...

it would become:

...Ugoab0w&redirect_uri=http%3A%2F%2Flocalhost%3A8081%2Flogin%2Foauth2%2Fcode%2Fabc&nonce=w4Vg...

This is correct according to the standard (RFC 3986). The referred backward compatiblity issue most likely has never been revelant for IdP implementations. And for a security relevant process like OAuth 2.0, correctness should be valued higher than compatiblity with broken software.

[manuelbl]

A small update: Before PR #6299, the redirect_uri was fully encoded. So for this particular aspect, the proposed approach would restore the old behavior without breaking what the PR was supposed to fix in the first place (support for query parameters).

[pthorson]

The referred backward compatiblity issue most likely has never been revelant for IdP implementations. And for a security relevant process like OAuth 2.0, correctness should be valued higher than compatiblity with broken software.

Even if there is some need for backward compatibility, I'd suggest the correct behavior described above be the default and any special cases require customization.

[manuelbl]

BTW: This fix is important to support the OIDC claims parameter (see https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter). It mandates a properly encoded JSON data structure.

[manuelbl]

It seems that I misunderstood the backward compatibility issue:

The characters / and : do not need to be escaped in query parameters as web servers are expected to first separate the query part and then slice the path. Broken software first searches for / and is then confused by these characters in the query part.

So fixing this issue and being fully standard's compliant does not change the current encoding of the redirect_uri parameter.