Strip Prefix breaking OAuth Flow - spring-cloud-gateway-server-mvc-4.3.0-M3

[joaquinjsb]

Describe the bug
The stripPrefix filter is interfering with redirect URLs in my Spring Authorization Server OAuth flow, specifically because the state parameter, which includes an equals sign (=), is being incorrectly processed.

for example:
/{route}/oauth2/authorize?state=EKAqtHTLsbuIH2P6jZAZhuRqzUfdTU2OnrVvDPKdcZ0%3D&redirect_uri=....

here's the exception:

java.lang.IllegalArgumentException: Invalid character '=' for QUERY_PARAM in "EKAqtHTLsbuIH2P6jZAZhuRqzUfdTU2OnrVvDPKdcZ0="
	at org.springframework.web.util.HierarchicalUriComponents.verifyUriComponent(HierarchicalUriComponents.java:422) ~[spring-web-6.2.5.jar:6.2.5]
	at org.springframework.web.util.HierarchicalUriComponents.lambda$verify$4(HierarchicalUriComponents.java:390) ~[spring-web-6.2.5.jar:6.2.5]
	at org.springframework.util.UnmodifiableMultiValueMap.lambda$forEach$0(UnmodifiableMultiValueMap.java:115) ~[spring-core-6.2.5.jar:6.2.5]
	at java.base/java.util.LinkedHashMap.forEach(LinkedHashMap.java:986) ~[na:na]
	at org.springframework.util.MultiValueMapAdapter.forEach(MultiValueMapAdapter.java:179) ~[spring-core-6.2.5.jar:6.2.5]
	at org.springframework.util.UnmodifiableMultiValueMap.forEach(UnmodifiableMultiValueMap.java:115) ~[spring-core-6.2.5.jar:6.2.5]
	at org.springframework.web.util.HierarchicalUriComponents.verify(HierarchicalUriComponents.java:387) ~[spring-web-6.2.5.jar:6.2.5]
	at org.springframework.web.util.HierarchicalUriComponents.<init>(HierarchicalUriComponents.java:146) ~[spring-web-6.2.5.jar:6.2.5]
	at org.springframework.web.util.UriComponentsBuilder.buildInternal(UriComponentsBuilder.java:346) ~[spring-web-6.2.5.jar:6.2.5]
	at org.springframework.web.util.UriComponentsBuilder.build(UriComponentsBuilder.java:334) ~[spring-web-6.2.5.jar:6.2.5]
	at org.springframework.cloud.gateway.server.mvc.filter.BeforeFilterFunctions.lambda$stripPrefix$32(BeforeFilterFunctions.java:438) ~[spring-cloud-gateway-server-mvc-4.3.0-M3.jar:4.3.0-M3

[raccoonback]

@joaquinjsb
Hello,
As far as I understand, Spring Cloud Gateway MVC internally uses ServletServerHttpRequest to handle request URIs, and decoding is automatically performed during that process.
Therefore, sending query parameters in an encoded format should allow the request to be processed correctly in stripPrefix(). (4.3.0-M3 version)

When I send a request using

spring:
  cloud:
    gateway:
      mvc:
        routes:
          - id: test
            uri: {routed-domain}
            predicates:
              - Path=/prefix/**
            filters:
              - StripPrefix=1

curl --location 'http://localhost:8080/prefix/oauth2/authorize?state=EKAqtHTLsbuIH2P6jZAZhuRqzUfdTU2OnrVvDPKdcZ0%3D&redirect_uri=test'

it is routed to

http://{routed-domain}/oauth2/authorize?state=EKAqtHTLsbuIH2P6jZAZhuRqzUfdTU2OnrVvDPKdcZ0%3D&redirect_uri=test

[joaquinjsb]

@joaquinjsb Hello, As far as I understand, Spring Cloud Gateway MVC internally uses ServletServerHttpRequest to handle request URIs, and decoding is automatically performed during that process. Therefore, sending query parameters in an encoded format should allow the request to be processed correctly in stripPrefix(). (4.3.0-M3 version)

When I send a request using

spring:
cloud:
gateway:
mvc:
routes:
- id: test
uri: {routed-domain}
predicates:
- Path=/prefix/**
filters:
- StripPrefix=1
curl --location 'http://localhost:8080/prefix/oauth2/authorize?state=EKAqtHTLsbuIH2P6jZAZhuRqzUfdTU2OnrVvDPKdcZ0%3D&redirect_uri=test'
it is routed to

http://{routed-domain}/oauth2/authorize?state=EKAqtHTLsbuIH2P6jZAZhuRqzUfdTU2OnrVvDPKdcZ0%3D&redirect_uri=test

can you try the test here ?
#3769

@Test
void prefixEncodedPath() {
	MockHttpServletRequest servletRequest = MockMvcRequestBuilders
			.get("http://localhost/get/é").buildRequest(null);
	ServerRequest request = ServerRequest.create(servletRequest,
			Collections.emptyList());
	ServerRequest modified = BeforeFilterFunctions.prefixPath("/pre fix")
			.apply(request);
	assertThat(modified.uri().getRawPath()).isEqualTo("/pre%20fix/get/%C3%A9");
}

[raccoonback]

@joaquinjsb

I tested it and found that the path is double-encoded, resulting in a route like /pre%2520fix/get/%25C3%25A9 when applying prefixPath().

@Test
void prefixEncodedPath() {
	MockHttpServletRequest servletRequest = MockMvcRequestBuilders
			.get("http://localhost/get/é").buildRequest(null);
	ServerRequest request = ServerRequest.create(servletRequest,
			Collections.emptyList());
	ServerRequest modified = BeforeFilterFunctions.prefixPath("/pre fix")
			.apply(request);
	assertThat(modified.uri().getRawPath()).isEqualTo("/pre%20fix/get/%C3%A9");
}

Unlike prefixPath(), it seems that this issue does not occur with stripPrefix().
Are you saying that the issue lies with prefixPath(), rather than with stripPrefix() as mentioned in this issue?

[joaquinjsb]

the problem I'm facing is still on the StripPrefix, I'm still working on the MCVE, it's leading me to believe that maybe the issue could be within the SavedRequest (Spring Security) at this point?

UPDATE:
@raccoonback
it's still not clear to me yet, but at some point, this URI is arriving to the filter, which is already encoded, I'll keep you posted.

so your reproducer would be something like

http://{routed-domain}/oauth2/authorize?state=EKAqtHTLsbuIH2P6jZAZhuRqzUfdTU2OnrVvDPKdcZ0=&redirect_uri=test

[raccoonback]

@joaquinjsb
It seems that only the value of the state parameter has been decoded.

There might be a place where the state is processed in the middle, or the request may have been sent without being properly encoded.

[joaquinjsb]

yes, I do agree, my route though, doesn't have anything besides a strip prefix.

this is the test:

@Test
    void prefixPath() {
        MockHttpServletRequest servletRequest = MockMvcRequestBuilders
                .get("http://localhost/auth/oauth2/authorize?state=kx67vfupe3OHkkLi0hsmGx6jPdbze74qZkzJa6Bd14U=&redirect_uri=test").buildRequest(null);

        servletRequest.setQueryString("state=kx67vfupe3OHkkLi0hsmGx6jPdbze74qZkzJa6Bd14U=&redirect_uri=test");

        ServerRequest request = ServerRequest.create(servletRequest,
                Collections.emptyList());

        ServerRequest modified = BeforeFilterFunctions.stripPrefix(1)
                .apply(request);

        assertThat(modified.uri().getPath()).isEqualTo("/oauth2/authorize");
        assertThat(modified.uri().getQuery()).isEqualTo("state=kx67vfupe3OHkkLi0hsmGx6jPdbze74qZkzJa6Bd14U=&redirect_uri=test");
    }

[raccoonback]

@joaquinjsb

I think the state query parameter should be encoded when testing.

	@Test
	void prefixPath() {
		MockHttpServletRequest servletRequest = MockMvcRequestBuilders.get(
				"http://localhost/auth/oauth2/authorize?state=kx67vfupe3OHkkLi0hsmGx6jPdbze74qZkzJa6Bd14U%3D&redirect_uri=test")
			.buildRequest(null);
		
		ServerRequest request = ServerRequest.create(servletRequest, Collections.emptyList());

		ServerRequest modified = BeforeFilterFunctions.stripPrefix(1).apply(request);

		assertThat(modified.uri().getPath()).isEqualTo("/oauth2/authorize");
		assertThat(modified.param("state")).isPresent().hasValue("kx67vfupe3OHkkLi0hsmGx6jPdbze74qZkzJa6Bd14U%3D");
		assertThat(modified.param("redirect_uri")).isPresent().hasValue("test");
	}

[joaquinjsb]

@raccoonback I've finished a deep dive debugging, here's what I've found out:

having a request like this:

/gwroute/oauth2/authorize?response_type=code&client_id=masked-client&scope=openid%20profile&state=Pp4Pkqa9mnkwcdacdSh1D4cO1i0hAanXzpaanRc7LeU%3D&redirect_uri=http://domaint/login/oauth2/code/example&nonce=m6v5lPUK8TWwaZu_aXmgBWNpspZH1eQdn38Wl-qikp8

when it reaches org.springframework.cloud.gateway.server.mvc.handler.ProxyExchangeHandlerFunction#handle,

the encoded flag is set to false because partial encoded urls are being treated as not, which ends up creating partially encoded urls, that then the stripPrefix can't decode afterwards when the redirect starts from the location header.

here's a test case reproducing the issue:

@Test
    void proxyExchange() throws InvocationTargetException, IllegalAccessException {

        MockHttpServletRequest servletRequest = MockMvcRequestBuilders
                .get(UriComponentsBuilder.fromUriString("http://localhost/oauth2/authorize?scope=openid%20profile&state=Pp4Pkqa9mnkwcdacdSh1D4cO1i0hAanXzpaanRc7LeU%3D").build(true).toUri()).buildRequest(null);

        servletRequest.setQueryString("scope=openid%20profile&state=Pp4Pkqa9mnkwcdacdSh1D4cO1i0hAanXzpaanRc7LeU%3D");

        ServerRequest request = ServerRequest.create(servletRequest,
                Collections.emptyList());

        final ServerRequest modified = BeforeFilterFunctions.stripPrefix().apply(request);

        URI uri = URI.create("http://downstream-service:8080/");

        Method method = Arrays.stream(ProxyExchangeHandlerFunction.class.getDeclaredMethods()).filter(m -> m.getName().equals("containsEncodedQuery")).findFirst().orElse(null);

        Assertions.assertNotNull(method);

        method.setAccessible(true);

        boolean encoded = (boolean) method.invoke(ProxyExchangeHandlerFunction.class, modified.uri(), modified.params());
        // @formatter:off
        URI url = UriComponentsBuilder.fromUri(modified.uri())
                .scheme(uri.getScheme())
                .host(uri.getHost())
                .port(uri.getPort())
                .replaceQueryParams(modified.params())
                .build(encoded)
                .toUri();

        ProxyExchange.Request proxyRequest = proxyExchange.request(modified).uri(url)
                .build();


     assertAll(
                () -> assertThat(proxyRequest.getUri().toString()).isEqualTo("http://downstream-service:8080/authorize?scope=openid%20profile&state=Pp4Pkqa9mnkwcdacdSh1D4cO1i0hAanXzpaanRc7LeU%3D"),
                () -> assertDoesNotThrow(() -> BeforeFilterFunctions.stripPrefix().apply(ServerRequest.create(MockMvcRequestBuilders.get(proxyRequest.getUri()).buildRequest(null), List.of())))
        );
    }

[raccoonback]

@joaquinjsb
As you mentioned, it appears that the query parameters replaced in ProxyExchangeHandlerFunction.handle() are not being encoded.

I do have one question.
ProxyExchangeHandlerFunction.handle() is being called after BeforeFilterFunctions.stripPrefix().
Could this issue be related to routing?
If you could clarify the exact behavior you're observing, it would help greatly with troubleshooting.

[joaquinjsb]

@joaquinjsb As you mentioned, it appears that the query parameters replaced in ProxyExchangeHandlerFunction.handle() are not being encoded.

I do have one question. ProxyExchangeHandlerFunction.handle() is being called after BeforeFilterFunctions.stripPrefix(). Could this issue be related to routing? If you could clarify the exact behavior you're observing, it would help greatly with troubleshooting.

probably the stripPrefix initial hint was maybe misleading, that maybe is just a consequence, this could be a routing issue, with the oauth flow.

the Authorization server (Spring A.S) is routed through my spring cloud gateway:

GatewayRouterFunctions.route("authService")
      .route(host(gatewayHostname).and(path("/auth/**")), HandlerFunctions.http(uri))
      .before(BeforeFilterFunctions.stripPrefix(1))
      .build()

what happens is that when spring security in SCG, generates the redirect url for the Idp, it doesn't route properly the redirection URI, which is the one I mentioned before:

http://localhost/auth/oauth2/authorize?scope=openid%20profile&state=Pp4Pkqa9mnkwcdacdSh1D4cO1i0hAanXzpaanRc7LeU%3D

when then the route is sending the request to the route, the stripPrefix is having the mentioned issue.

[raccoonback]

@joaquinjsb
I've created a PR to fix the issue in the corresponding component.
ref. #3789