From 26205a2d8df0f1c610ab2b6a380c61499c064c48 Mon Sep 17 00:00:00 2001 From: Andreas Fleig Date: Tue, 13 Dec 2022 10:33:04 +0100 Subject: [PATCH 1/2] Preserve encoding for authorization request redirect_uri parameter Closes gh-1011 --- .../web/OAuth2AuthorizationEndpointFilter.java | 17 +++++++++++++---- .../OAuth2AuthorizationEndpointFilterTests.java | 9 +++++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java index c5c63bf7..428e10ba 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2020-2022 the original author or authors. + * Copyright 2020-2023 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. @@ -66,6 +66,9 @@ import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.filter.OncePerRequestFilter; +import org.springframework.web.util.DefaultUriBuilderFactory; +import org.springframework.web.util.UriBuilder; +import org.springframework.web.util.UriBuilderFactory; import org.springframework.web.util.UriComponentsBuilder; /** @@ -296,8 +299,8 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = (OAuth2AuthorizationCodeRequestAuthenticationToken) authentication; - UriComponentsBuilder uriBuilder = UriComponentsBuilder - .fromUriString(authorizationCodeRequestAuthentication.getRedirectUri()) + UriBuilder uriBuilder = valuesOnlyEncodingUriBuilderFactory() + .uriString(authorizationCodeRequestAuthentication.getRedirectUri()) .queryParam(OAuth2ParameterNames.CODE, authorizationCodeRequestAuthentication.getAuthorizationCode().getTokenValue()); String redirectUri; if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) { @@ -306,7 +309,7 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); redirectUri = uriBuilder.build(queryParams).toString(); } else { - redirectUri = uriBuilder.toUriString(); + redirectUri = uriBuilder.build().toString(); } this.redirectStrategy.sendRedirect(request, response, redirectUri); } @@ -351,6 +354,12 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte this.redirectStrategy.sendRedirect(request, response, redirectUri); } + private UriBuilderFactory valuesOnlyEncodingUriBuilderFactory() { + DefaultUriBuilderFactory uriBuilderFactory = new DefaultUriBuilderFactory(); + uriBuilderFactory.setEncodingMode(DefaultUriBuilderFactory.EncodingMode.VALUES_ONLY); + return uriBuilderFactory; + } + /** * For internal use only. */ diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java index ae76f3a1..3597131d 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilterTests.java @@ -537,7 +537,12 @@ public class OAuth2AuthorizationEndpointFilterTests { @Test public void doFilterWhenAuthorizationRequestAuthenticatedThenAuthorizationResponse() throws Exception { - RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + RegisteredClient registeredClient = TestRegisteredClients.registeredClient() + .redirectUris(redirectUris -> { + redirectUris.clear(); + redirectUris.add("https://example.com?param=encoded%20parameter%20value"); + }) + .build(); OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, this.authorizationCode, @@ -563,7 +568,7 @@ public class OAuth2AuthorizationEndpointFilterTests { .isEqualTo(REMOTE_ADDRESS); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); assertThat(response.getRedirectedUrl()).isEqualTo( - request.getParameter(OAuth2ParameterNames.REDIRECT_URI) + "?code=code&state=state"); + "https://example.com?param=encoded%20parameter%20value&code=code&state=state"); } @Test From 30927ad5e736ceb6720d41dfc63e400942d1c729 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Wed, 15 Feb 2023 15:25:49 -0500 Subject: [PATCH 2/2] Polish gh-1011 --- .../OAuth2AuthorizationEndpointFilter.java | 36 ++++++------------- .../OAuth2AuthorizationCodeGrantTests.java | 12 +++++-- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java index 428e10ba..76bcbe6a 100644 --- a/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java +++ b/oauth2-authorization-server/src/main/java/org/springframework/security/oauth2/server/authorization/web/OAuth2AuthorizationEndpointFilter.java @@ -18,9 +18,7 @@ package org.springframework.security.oauth2.server.authorization.web; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Arrays; -import java.util.HashMap; import java.util.HashSet; -import java.util.Map; import java.util.Set; import javax.servlet.FilterChain; @@ -66,10 +64,8 @@ import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.util.StringUtils; import org.springframework.web.filter.OncePerRequestFilter; -import org.springframework.web.util.DefaultUriBuilderFactory; -import org.springframework.web.util.UriBuilder; -import org.springframework.web.util.UriBuilderFactory; import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.util.UriUtils; /** * A {@code Filter} for the OAuth 2.0 Authorization Code Grant, @@ -299,18 +295,16 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = (OAuth2AuthorizationCodeRequestAuthenticationToken) authentication; - UriBuilder uriBuilder = valuesOnlyEncodingUriBuilderFactory() - .uriString(authorizationCodeRequestAuthentication.getRedirectUri()) + UriComponentsBuilder uriBuilder = UriComponentsBuilder + .fromUriString(authorizationCodeRequestAuthentication.getRedirectUri()) .queryParam(OAuth2ParameterNames.CODE, authorizationCodeRequestAuthentication.getAuthorizationCode().getTokenValue()); String redirectUri; if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) { - uriBuilder.queryParam(OAuth2ParameterNames.STATE, "{state}"); - Map queryParams = new HashMap<>(); - queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); - redirectUri = uriBuilder.build(queryParams).toString(); - } else { - redirectUri = uriBuilder.build().toString(); + uriBuilder.queryParam( + OAuth2ParameterNames.STATE, + UriUtils.encode(authorizationCodeRequestAuthentication.getState(), StandardCharsets.UTF_8)); } + redirectUri = uriBuilder.build(true).toUriString(); // build(true) -> Components are explicitly encoded this.redirectStrategy.sendRedirect(request, response, redirectUri); } @@ -344,22 +338,14 @@ public final class OAuth2AuthorizationEndpointFilter extends OncePerRequestFilte } String redirectUri; if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) { - uriBuilder.queryParam(OAuth2ParameterNames.STATE, "{state}"); - Map queryParams = new HashMap<>(); - queryParams.put(OAuth2ParameterNames.STATE, authorizationCodeRequestAuthentication.getState()); - redirectUri = uriBuilder.build(queryParams).toString(); - } else { - redirectUri = uriBuilder.toUriString(); + uriBuilder.queryParam( + OAuth2ParameterNames.STATE, + UriUtils.encode(authorizationCodeRequestAuthentication.getState(), StandardCharsets.UTF_8)); } + redirectUri = uriBuilder.build(true).toUriString(); // build(true) -> Components are explicitly encoded this.redirectStrategy.sendRedirect(request, response, redirectUri); } - private UriBuilderFactory valuesOnlyEncodingUriBuilderFactory() { - DefaultUriBuilderFactory uriBuilderFactory = new DefaultUriBuilderFactory(); - uriBuilderFactory.setEncodingMode(DefaultUriBuilderFactory.EncodingMode.VALUES_ONLY); - return uriBuilderFactory; - } - /** * For internal use only. */ diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java index c794097f..7f75750d 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/config/annotation/web/configurers/OAuth2AuthorizationCodeGrantTests.java @@ -286,7 +286,12 @@ public class OAuth2AuthorizationCodeGrantTests { } private void assertAuthorizationRequestRedirectsToClient(String authorizationEndpointUri) throws Exception { - RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + RegisteredClient registeredClient = TestRegisteredClients.registeredClient() + .redirectUris(redirectUris -> { + redirectUris.clear(); + redirectUris.add("https://example.com/callback-1?param=encoded%20parameter%20value"); // gh-1011 + }) + .build(); this.registeredClientRepository.save(registeredClient); MultiValueMap authorizationRequestParameters = getAuthorizationRequestParameters(registeredClient); @@ -296,8 +301,9 @@ public class OAuth2AuthorizationCodeGrantTests { .andExpect(status().is3xxRedirection()) .andReturn(); String redirectedUrl = mvcResult.getResponse().getRedirectedUrl(); - String expectedRedirectUri = authorizationRequestParameters.getFirst(OAuth2ParameterNames.REDIRECT_URI); - assertThat(redirectedUrl).matches(expectedRedirectUri + "\\?code=.{15,}&state=" + STATE_URL_ENCODED); + String redirectUri = authorizationRequestParameters.getFirst(OAuth2ParameterNames.REDIRECT_URI); + String code = extractParameterFromRedirectUri(redirectedUrl, "code"); + assertThat(redirectedUrl).isEqualTo(redirectUri + "&code=" + code + "&state=" + STATE_URL_ENCODED); String authorizationCode = extractParameterFromRedirectUri(redirectedUrl, "code"); OAuth2Authorization authorization = this.authorizationService.findByToken(authorizationCode, AUTHORIZATION_CODE_TOKEN_TYPE);