From 5063a98cdf8724eb076e131a712c796d57785e24 Mon Sep 17 00:00:00 2001 From: Andreas Fleig Date: Tue, 13 Dec 2022 10:33:04 +0100 Subject: [PATCH 1/5] Enable JUnit 5 test runner in samples --- .../samples-custom-consent-authorizationserver.gradle | 4 ++++ .../samples-default-authorizationserver.gradle | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/samples/custom-consent-authorizationserver/samples-custom-consent-authorizationserver.gradle b/samples/custom-consent-authorizationserver/samples-custom-consent-authorizationserver.gradle index 8036588ad..42a0c3be4 100644 --- a/samples/custom-consent-authorizationserver/samples-custom-consent-authorizationserver.gradle +++ b/samples/custom-consent-authorizationserver/samples-custom-consent-authorizationserver.gradle @@ -8,6 +8,10 @@ group = project.rootProject.group version = project.rootProject.version sourceCompatibility = "17" +test { + useJUnitPlatform() +} + repositories { mavenCentral() maven { url 'https://repo.spring.io/milestone' } diff --git a/samples/default-authorizationserver/samples-default-authorizationserver.gradle b/samples/default-authorizationserver/samples-default-authorizationserver.gradle index c0d0124dd..adfb0f1d2 100644 --- a/samples/default-authorizationserver/samples-default-authorizationserver.gradle +++ b/samples/default-authorizationserver/samples-default-authorizationserver.gradle @@ -8,6 +8,10 @@ group = project.rootProject.group version = project.rootProject.version sourceCompatibility = "17" +test { + useJUnitPlatform() +} + repositories { mavenCentral() maven { url 'https://repo.spring.io/milestone' } From 5eb94584d3668f230157b894bf8e302e0268d67e Mon Sep 17 00:00:00 2001 From: Andreas Fleig Date: Fri, 16 Dec 2022 15:29:55 +0100 Subject: [PATCH 2/5] DefaultAuthorizationServerApplicationTests: check for redirect URI in authorization response The redirect URI used in the test contains a URL-encoded parameter and is similar to redirect URIs of the popular Yii PHP framework. Also document how state is currently decoded and re-encoded, modifying it in the process. --- .../config/AuthorizationServerConfig.java | 1 + ...ltAuthorizationServerApplicationTests.java | 27 ++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/samples/default-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java b/samples/default-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java index f1d9b7e4f..b07a0a2e0 100644 --- a/samples/default-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java +++ b/samples/default-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java @@ -87,6 +87,7 @@ public RegisteredClientRepository registeredClientRepository(JdbcTemplate jdbcTe .authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN) .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) .redirectUri("http://127.0.0.1:8080/login/oauth2/code/messaging-client-oidc") + .redirectUri("http://127.0.0.1:8080/login/oauth2/code/messaging-client-oidc?param=is%2Fencoded") .redirectUri("http://127.0.0.1:8080/authorized") .scope(OidcScopes.OPENID) .scope(OidcScopes.PROFILE) diff --git a/samples/default-authorizationserver/src/test/java/sample/DefaultAuthorizationServerApplicationTests.java b/samples/default-authorizationserver/src/test/java/sample/DefaultAuthorizationServerApplicationTests.java index d9a0b2acd..fb995f6e7 100644 --- a/samples/default-authorizationserver/src/test/java/sample/DefaultAuthorizationServerApplicationTests.java +++ b/samples/default-authorizationserver/src/test/java/sample/DefaultAuthorizationServerApplicationTests.java @@ -16,6 +16,9 @@ package sample; import java.io.IOException; +import java.net.URLDecoder; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import com.gargoylesoftware.htmlunit.Page; import com.gargoylesoftware.htmlunit.WebClient; @@ -33,6 +36,7 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.http.HttpStatus; import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; import static org.assertj.core.api.Assertions.assertThat; @@ -46,15 +50,18 @@ @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @AutoConfigureMockMvc public class DefaultAuthorizationServerApplicationTests { - private static final String REDIRECT_URI = "http://127.0.0.1:8080/login/oauth2/code/messaging-client-oidc"; + private static final String REDIRECT_URI = "http://127.0.0.1:8080/login/oauth2/code/messaging-client-oidc?param=is%2Fencoded"; + public static final String STATE = "some+%20encoded%2Fstate"; + public static final String REENCODED_STATE = "some%20%20encoded%2Fstate"; private static final String AUTHORIZATION_REQUEST = UriComponentsBuilder .fromPath("/oauth2/authorize") .queryParam("response_type", "code") .queryParam("client_id", "messaging-client") .queryParam("scope", "openid") - .queryParam("state", "some-state") - .queryParam("redirect_uri", REDIRECT_URI) + .queryParam("state", STATE) + .queryParam("redirect_uri", URLEncoder.encode(REDIRECT_URI, StandardCharsets.UTF_8)) + .build() .toUriString(); @Autowired @@ -67,6 +74,16 @@ public void setUp() { this.webClient.getCookieManager().clearCookies(); // log out } + @Test + void authorizationRequestProperlyEncoded() { + UriComponents uriComponents = UriComponentsBuilder.fromUriString(AUTHORIZATION_REQUEST).build(); + + assertThat(uriComponents.getQueryParams().getFirst("state")).isEqualTo(STATE); + + String redirectUri = uriComponents.getQueryParams().getFirst("redirect_uri"); + assertThat(URLDecoder.decode(redirectUri, StandardCharsets.UTF_8)).isEqualTo(REDIRECT_URI); + } + @Test public void whenLoginSuccessfulThenDisplayNotFoundError() throws IOException { HtmlPage page = this.webClient.getPage("/"); @@ -108,6 +125,10 @@ public void whenLoggingInAndRequestingTokenThenRedirectsToClientApplication() th assertThat(response.getStatusCode()).isEqualTo(HttpStatus.MOVED_PERMANENTLY.value()); String location = response.getResponseHeaderValue("location"); + + String state = UriComponentsBuilder.fromHttpUrl(location).build().getQueryParams().getFirst("state"); + assertThat(state).isEqualTo(REENCODED_STATE); + assertThat(location).startsWith(REDIRECT_URI); assertThat(location).contains("code="); } From 1b1f4d36dce988bf925aaf83fea7248d5d525bba Mon Sep 17 00:00:00 2001 From: Andreas Fleig Date: Fri, 16 Dec 2022 16:10:05 +0100 Subject: [PATCH 3/5] Fix: do not re-encode redirect URI query parameters in authorization response Redirect URIs can contain query parameters and these can be URI encoded. UriComponentsBuilder does not decode query parameters in fromUriString(). They become part of the template which by default is encoded (again). This commit configures a UriBuilder that does not encode the template but only provided values. --- .../web/OAuth2AuthorizationEndpointFilter.java | 10 +++++++--- .../authorization/client/TestRegisteredClients.java | 12 ++++++++++++ .../web/OAuth2AuthorizationEndpointFilterTests.java | 12 ++++++------ 3 files changed, 25 insertions(+), 9 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 7bf292b68..db0886aac 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 @@ -66,6 +66,8 @@ 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.UriComponentsBuilder; /** @@ -296,8 +298,10 @@ private void sendAuthorizationResponse(HttpServletRequest request, HttpServletRe OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = (OAuth2AuthorizationCodeRequestAuthenticationToken) authentication; - UriComponentsBuilder uriBuilder = UriComponentsBuilder - .fromUriString(authorizationCodeRequestAuthentication.getRedirectUri()) + DefaultUriBuilderFactory uriBuilderFactory = new DefaultUriBuilderFactory(); + uriBuilderFactory.setEncodingMode(DefaultUriBuilderFactory.EncodingMode.VALUES_ONLY); + UriBuilder uriBuilder = uriBuilderFactory + .uriString(authorizationCodeRequestAuthentication.getRedirectUri()) .queryParam(OAuth2ParameterNames.CODE, authorizationCodeRequestAuthentication.getAuthorizationCode().getTokenValue()); String redirectUri; if (StringUtils.hasText(authorizationCodeRequestAuthentication.getState())) { @@ -306,7 +310,7 @@ private void sendAuthorizationResponse(HttpServletRequest request, HttpServletRe 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); } diff --git a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java index bf2c26403..c829ab470 100644 --- a/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java +++ b/oauth2-authorization-server/src/test/java/org/springframework/security/oauth2/server/authorization/client/TestRegisteredClients.java @@ -39,6 +39,18 @@ public static RegisteredClient.Builder registeredClient() { .scope("scope1"); } + public static RegisteredClient.Builder registeredClientEncodedUri() { + return RegisteredClient.withId("registration-1") + .clientId("client-1") + .clientIdIssuedAt(Instant.now().truncatedTo(ChronoUnit.SECONDS)) + .clientSecret("secret-1") + .authorizationGrantType(AuthorizationGrantType.AUTHORIZATION_CODE) + .authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN) + .clientAuthenticationMethod(ClientAuthenticationMethod.CLIENT_SECRET_BASIC) + .redirectUri("https://example.com?param=is%2Fencoded") + .scope("scope1"); + } + public static RegisteredClient.Builder registeredClient2() { return RegisteredClient.withId("registration-2") .clientId("client-2") 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 417f4848d..ffa4598b0 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 @@ -83,7 +83,7 @@ public class OAuth2AuthorizationEndpointFilterTests { private static final String DEFAULT_AUTHORIZATION_ENDPOINT_URI = "/oauth2/authorize"; private static final String AUTHORIZATION_URI = "https://provider.com/oauth2/authorize"; - private static final String STATE = "state"; + private static final String STATE = "previously encoded/state"; private static final String REMOTE_ADDRESS = "remote-address"; private AuthenticationManager authenticationManager; private OAuth2AuthorizationEndpointFilter filter; @@ -299,7 +299,7 @@ public void doFilterWhenAuthorizationRequestAuthenticationExceptionThenErrorResp verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?error=errorCode&error_description=errorDescription&error_uri=errorUri&state=state"); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?error=errorCode&error_description=errorDescription&error_uri=errorUri&state=previously%20encoded%2Fstate"); assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(this.principal); } @@ -459,7 +459,7 @@ public void doFilterWhenAuthorizationRequestConsentRequiredWithCustomConsentUriT verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("http://localhost/oauth2/custom-consent?scope=scope1%20scope2&client_id=client-1&state=state"); + assertThat(response.getRedirectedUrl()).isEqualTo("http://localhost/oauth2/custom-consent?scope=scope1%20scope2&client_id=client-1&state=previously%20encoded/state"); } @Test @@ -535,7 +535,7 @@ public void doFilterWhenAuthorizationRequestConsentRequiredWithPreviouslyApprove @Test public void doFilterWhenAuthorizationRequestAuthenticatedThenAuthorizationResponse() throws Exception { - RegisteredClient registeredClient = TestRegisteredClients.registeredClient().build(); + RegisteredClient registeredClient = TestRegisteredClients.registeredClientEncodedUri().build(); OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthenticationResult = new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, this.authorizationCode, @@ -560,7 +560,7 @@ public void doFilterWhenAuthorizationRequestAuthenticatedThenAuthorizationRespon .extracting(WebAuthenticationDetails::getRemoteAddress) .isEqualTo(REMOTE_ADDRESS); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=state"); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?param=is%2Fencoded&code=code&state=previously%20encoded%2Fstate"); } @Test @@ -591,7 +591,7 @@ public void doFilterWhenAuthenticationRequestAuthenticatedThenAuthorizationRespo verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=state"); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?code=code&state=previously%20encoded%2Fstate"); } private void doFilterWhenAuthorizationRequestInvalidParameterThenError(RegisteredClient registeredClient, From ca9e58c93008d2fad3ba08c191b63900dd9bbd74 Mon Sep 17 00:00:00 2001 From: Andreas Fleig Date: Fri, 16 Dec 2022 16:55:17 +0100 Subject: [PATCH 4/5] Fix: do not re-encode redirect URI query parameters in error response --- .../OAuth2AuthorizationEndpointFilter.java | 28 +++++++++++-------- ...Auth2AuthorizationEndpointFilterTests.java | 4 +-- ...efaultAuthorizationServerConsentTests.java | 25 +++++++++++++++-- 3 files changed, 40 insertions(+), 17 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 db0886aac..3c60d70fd 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 @@ -68,6 +68,7 @@ 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; /** @@ -298,9 +299,7 @@ private void sendAuthorizationResponse(HttpServletRequest request, HttpServletRe OAuth2AuthorizationCodeRequestAuthenticationToken authorizationCodeRequestAuthentication = (OAuth2AuthorizationCodeRequestAuthenticationToken) authentication; - DefaultUriBuilderFactory uriBuilderFactory = new DefaultUriBuilderFactory(); - uriBuilderFactory.setEncodingMode(DefaultUriBuilderFactory.EncodingMode.VALUES_ONLY); - UriBuilder uriBuilder = uriBuilderFactory + UriBuilder uriBuilder = valuesOnlyEncodingUriBuilderFactory() .uriString(authorizationCodeRequestAuthentication.getRedirectUri()) .queryParam(OAuth2ParameterNames.CODE, authorizationCodeRequestAuthentication.getAuthorizationCode().getTokenValue()); String redirectUri; @@ -334,27 +333,32 @@ private void sendErrorResponse(HttpServletRequest request, HttpServletResponse r this.logger.trace("Redirecting to client with error"); } - UriComponentsBuilder uriBuilder = UriComponentsBuilder - .fromUriString(authorizationCodeRequestAuthentication.getRedirectUri()) + UriBuilder uriBuilder = valuesOnlyEncodingUriBuilderFactory() + .uriString(authorizationCodeRequestAuthentication.getRedirectUri()) .queryParam(OAuth2ParameterNames.ERROR, error.getErrorCode()); + Map queryParams = new HashMap<>(); if (StringUtils.hasText(error.getDescription())) { - uriBuilder.queryParam(OAuth2ParameterNames.ERROR_DESCRIPTION, error.getDescription()); + uriBuilder.queryParam(OAuth2ParameterNames.ERROR_DESCRIPTION, "{error_description}"); + queryParams.put(OAuth2ParameterNames.ERROR_DESCRIPTION, error.getDescription()); } if (StringUtils.hasText(error.getUri())) { - uriBuilder.queryParam(OAuth2ParameterNames.ERROR_URI, error.getUri()); + uriBuilder.queryParam(OAuth2ParameterNames.ERROR_URI, "{error_uri}"); + queryParams.put(OAuth2ParameterNames.ERROR_URI, error.getUri()); } - 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(); } + String redirectUri = uriBuilder.build(queryParams).toString(); 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 ffa4598b0..d3d1102d3 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 @@ -285,7 +285,7 @@ public void doFilterWhenAuthorizationRequestAuthenticationExceptionThenErrorResp new OAuth2AuthorizationCodeRequestAuthenticationToken( AUTHORIZATION_URI, registeredClient.getClientId(), principal, registeredClient.getRedirectUris().iterator().next(), STATE, registeredClient.getScopes(), null); - OAuth2Error error = new OAuth2Error("errorCode", "errorDescription", "errorUri"); + OAuth2Error error = new OAuth2Error("errorCode", "errorDescription", "errorUri#section"); when(this.authenticationManager.authenticate(any())) .thenThrow(new OAuth2AuthorizationCodeRequestAuthenticationException(error, authorizationCodeRequestAuthentication)); @@ -299,7 +299,7 @@ public void doFilterWhenAuthorizationRequestAuthenticationExceptionThenErrorResp verifyNoInteractions(filterChain); assertThat(response.getStatus()).isEqualTo(HttpStatus.FOUND.value()); - assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?error=errorCode&error_description=errorDescription&error_uri=errorUri&state=previously%20encoded%2Fstate"); + assertThat(response.getRedirectedUrl()).isEqualTo("https://example.com?error=errorCode&error_description=errorDescription&error_uri=errorUri%23section&state=previously%20encoded%2Fstate"); assertThat(SecurityContextHolder.getContext().getAuthentication()).isSameAs(this.principal); } diff --git a/samples/default-authorizationserver/src/test/java/sample/DefaultAuthorizationServerConsentTests.java b/samples/default-authorizationserver/src/test/java/sample/DefaultAuthorizationServerConsentTests.java index 641b63fac..43d589e8a 100644 --- a/samples/default-authorizationserver/src/test/java/sample/DefaultAuthorizationServerConsentTests.java +++ b/samples/default-authorizationserver/src/test/java/sample/DefaultAuthorizationServerConsentTests.java @@ -16,6 +16,9 @@ package sample; import java.io.IOException; +import java.net.URLDecoder; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; @@ -36,6 +39,7 @@ import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService; import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; import static org.assertj.core.api.Assertions.assertThat; @@ -58,15 +62,18 @@ public class DefaultAuthorizationServerConsentTests { @MockBean private OAuth2AuthorizationConsentService authorizationConsentService; - private final String redirectUri = "http://127.0.0.1/login/oauth2/code/messaging-client-oidc"; + private final String redirectUri = "http://127.0.0.1:8080/login/oauth2/code/messaging-client-oidc?param=is%2Fencoded"; + public final String state = "some+%20encoded%2Fstate"; + public final String reencodedState = "some%20%20encoded%2Fstate"; private final String authorizationRequestUri = UriComponentsBuilder .fromPath("/oauth2/authorize") .queryParam("response_type", "code") .queryParam("client_id", "messaging-client") .queryParam("scope", "openid message.read message.write") - .queryParam("state", "state") - .queryParam("redirect_uri", this.redirectUri) + .queryParam("state", this.state) + .queryParam("redirect_uri", URLEncoder.encode(this.redirectUri, StandardCharsets.UTF_8)) + .build() .toUriString(); @BeforeEach @@ -77,6 +84,15 @@ public void setUp() { when(this.authorizationConsentService.findById(any(), any())).thenReturn(null); } + @Test + void authorizationRequestProperlyEncoded() { + UriComponents uriComponents = UriComponentsBuilder.fromUriString(authorizationRequestUri).build(); + + assertThat(uriComponents.getQueryParams().getFirst("state")).isEqualTo(state); + String redirectUri = uriComponents.getQueryParams().getFirst("redirect_uri"); + assertThat(URLDecoder.decode(redirectUri, StandardCharsets.UTF_8)).isEqualTo(this.redirectUri); + } + @Test @WithMockUser("user1") public void whenUserConsentsToAllScopesThenReturnAuthorizationCode() throws IOException { @@ -121,6 +137,9 @@ public void whenUserCancelsConsentThenReturnAccessDeniedError() throws IOExcepti String location = cancelConsentResponse.getResponseHeaderValue("location"); assertThat(location).startsWith(this.redirectUri); assertThat(location).contains("error=access_denied"); + + String state = UriComponentsBuilder.fromHttpUrl(location).build().getQueryParams().getFirst("state"); + assertThat(state).isEqualTo(reencodedState); } } From 1dfa38c3593085b44a2210c23db0784265a7fed3 Mon Sep 17 00:00:00 2001 From: Andreas Fleig Date: Fri, 16 Dec 2022 17:02:37 +0100 Subject: [PATCH 5/5] CustomConsentAuthorizationServerTests: check redirect URI and state encoding --- .../config/AuthorizationServerConfig.java | 1 + ...CustomConsentAuthorizationServerTests.java | 26 ++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/samples/custom-consent-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java b/samples/custom-consent-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java index a2cf209b6..ce4e33b96 100644 --- a/samples/custom-consent-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java +++ b/samples/custom-consent-authorizationserver/src/main/java/sample/config/AuthorizationServerConfig.java @@ -93,6 +93,7 @@ public RegisteredClientRepository registeredClientRepository() { .authorizationGrantType(AuthorizationGrantType.REFRESH_TOKEN) .authorizationGrantType(AuthorizationGrantType.CLIENT_CREDENTIALS) .redirectUri("http://127.0.0.1:8080/login/oauth2/code/messaging-client-oidc") + .redirectUri("http://127.0.0.1:8080/login/oauth2/code/messaging-client-oidc?param=is%2Fencoded") .redirectUri("http://127.0.0.1:8080/authorized") .scope(OidcScopes.OPENID) .scope(OidcScopes.PROFILE) diff --git a/samples/custom-consent-authorizationserver/src/test/java/sample/CustomConsentAuthorizationServerTests.java b/samples/custom-consent-authorizationserver/src/test/java/sample/CustomConsentAuthorizationServerTests.java index 762ccf811..232d6f3f9 100644 --- a/samples/custom-consent-authorizationserver/src/test/java/sample/CustomConsentAuthorizationServerTests.java +++ b/samples/custom-consent-authorizationserver/src/test/java/sample/CustomConsentAuthorizationServerTests.java @@ -16,6 +16,9 @@ package sample; import java.io.IOException; +import java.net.URLDecoder; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; @@ -36,6 +39,7 @@ import org.springframework.security.oauth2.server.authorization.OAuth2AuthorizationConsentService; import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.web.util.UriComponents; import org.springframework.web.util.UriComponentsBuilder; import static org.assertj.core.api.Assertions.assertThat; @@ -58,15 +62,18 @@ public class CustomConsentAuthorizationServerTests { @MockBean private OAuth2AuthorizationConsentService authorizationConsentService; - private final String redirectUri = "http://127.0.0.1/login/oauth2/code/messaging-client-oidc"; + private final String redirectUri = "http://127.0.0.1:8080/login/oauth2/code/messaging-client-oidc?param=is%2Fencoded"; + public final String state = "some+%20encoded%2Fstate"; + public final String reencodedState = "some%20%20encoded%2Fstate"; private final String authorizationRequestUri = UriComponentsBuilder .fromPath("/oauth2/authorize") .queryParam("response_type", "code") .queryParam("client_id", "messaging-client") .queryParam("scope", "openid message.read message.write") - .queryParam("state", "state") - .queryParam("redirect_uri", this.redirectUri) + .queryParam("state", this.state) + .queryParam("redirect_uri", URLEncoder.encode(this.redirectUri, StandardCharsets.UTF_8)) + .build() .toUriString(); @BeforeEach @@ -77,6 +84,15 @@ public void setUp() { when(this.authorizationConsentService.findById(any(), any())).thenReturn(null); } + @Test + void authorizationRequestProperlyEncoded() { + UriComponents uriComponents = UriComponentsBuilder.fromUriString(authorizationRequestUri).build(); + + assertThat(uriComponents.getQueryParams().getFirst("state")).isEqualTo(state); + String redirectUri = uriComponents.getQueryParams().getFirst("redirect_uri"); + assertThat(URLDecoder.decode(redirectUri, StandardCharsets.UTF_8)).isEqualTo(this.redirectUri); + } + @Test @WithMockUser("user1") public void whenUserConsentsToAllScopesThenReturnAuthorizationCode() throws IOException { @@ -105,6 +121,8 @@ public void whenUserConsentsToAllScopesThenReturnAuthorizationCode() throws IOEx String location = approveConsentResponse.getResponseHeaderValue("location"); assertThat(location).startsWith(this.redirectUri); assertThat(location).contains("code="); + String state = UriComponentsBuilder.fromHttpUrl(location).build().getQueryParams().getFirst("state"); + assertThat(state).isEqualTo(this.reencodedState); } @Test @@ -121,6 +139,8 @@ public void whenUserCancelsConsentThenReturnAccessDeniedError() throws IOExcepti String location = cancelConsentResponse.getResponseHeaderValue("location"); assertThat(location).startsWith(this.redirectUri); assertThat(location).contains("error=access_denied"); + String state = UriComponentsBuilder.fromHttpUrl(location).build().getQueryParams().getFirst("state"); + assertThat(state).isEqualTo(this.reencodedState); } }