diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java index 5c10438d1b..f6490355ea 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java +++ b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -23,6 +23,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.util.UriUtils; import java.io.Serializable; import java.nio.charset.StandardCharsets; @@ -376,29 +377,34 @@ public final class OAuth2AuthorizationRequest implements Serializable { private String buildAuthorizationRequestUri() { MultiValueMap parameters = new LinkedMultiValueMap<>(); - parameters.set(OAuth2ParameterNames.RESPONSE_TYPE, this.responseType.getValue()); - parameters.set(OAuth2ParameterNames.CLIENT_ID, this.clientId); + parameters.set(OAuth2ParameterNames.RESPONSE_TYPE, encodeQueryParam(this.responseType.getValue())); + parameters.set(OAuth2ParameterNames.CLIENT_ID, encodeQueryParam(this.clientId)); if (!CollectionUtils.isEmpty(this.scopes)) { parameters.set(OAuth2ParameterNames.SCOPE, - StringUtils.collectionToDelimitedString(this.scopes, " ")); + encodeQueryParam(StringUtils.collectionToDelimitedString(this.scopes, " "))); } if (this.state != null) { - parameters.set(OAuth2ParameterNames.STATE, this.state); + parameters.set(OAuth2ParameterNames.STATE, encodeQueryParam(this.state)); } if (this.redirectUri != null) { - parameters.set(OAuth2ParameterNames.REDIRECT_URI, this.redirectUri); + parameters.set(OAuth2ParameterNames.REDIRECT_URI, encodeQueryParam(this.redirectUri)); } if (!CollectionUtils.isEmpty(this.additionalParameters)) { - this.additionalParameters.forEach((k, v) -> parameters.set(k, v.toString())); + this.additionalParameters.forEach((k, v) -> + parameters.set(encodeQueryParam(k), encodeQueryParam(v.toString()))); } return UriComponentsBuilder.fromHttpUrl(this.authorizationUri) .queryParams(parameters) - .encode(StandardCharsets.UTF_8) .build() .toUriString(); } + // Encode query parameter value according to RFC 3986 + private static String encodeQueryParam(String value) { + return UriUtils.encodeQueryParam(value, StandardCharsets.UTF_8); + } + private LinkedHashSet toLinkedHashSet(String... scope) { LinkedHashSet result = new LinkedHashSet<>(); Collections.addAll(result, scope); diff --git a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequestTests.java b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequestTests.java index caa1da5fbe..6480442f53 100644 --- a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequestTests.java +++ b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequestTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -307,4 +307,38 @@ public class OAuth2AuthorizationRequestTests { "response_type=code&client_id=client-id&state=state&" + "redirect_uri=https://example.com/authorize/oauth2/code/registration-id"); } + + @Test + public void buildWhenAuthorizationUriIncludesEscapedQueryParameterThenAuthorizationRequestUrlIncludesIt() { + OAuth2AuthorizationRequest authorizationRequest = + TestOAuth2AuthorizationRequests.request() + .authorizationUri(AUTHORIZATION_URI + + "?claims=%7B%22userinfo%22%3A%7B%22email_verified%22%3A%7B%22essential%22%3Atrue%7D%7D%7D").build(); + + assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull(); + assertThat(authorizationRequest.getAuthorizationRequestUri()) + .isEqualTo("https://provider.com/oauth2/authorize?" + + "claims=%7B%22userinfo%22%3A%7B%22email_verified%22%3A%7B%22essential%22%3Atrue%7D%7D%7D&" + + "response_type=code&client_id=client-id&state=state&" + + "redirect_uri=https://example.com/authorize/oauth2/code/registration-id"); + } + + @Test + public void buildWhenNonAsciiAdditionalParametersThenProperlyEncoded() { + Map additionalParameters = new HashMap<>(); + additionalParameters.put("item amount", "19.95" + '\u20ac'); + additionalParameters.put("item name", "H" + '\u00c5' + "M" + '\u00d6'); + additionalParameters.put('\u00e2' + "ge", "4" + '\u00bd'); + OAuth2AuthorizationRequest authorizationRequest = + TestOAuth2AuthorizationRequests.request() + .additionalParameters(additionalParameters) + .build(); + + assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull(); + assertThat(authorizationRequest.getAuthorizationRequestUri()) + .isEqualTo("https://example.com/login/oauth/authorize?" + + "response_type=code&client_id=client-id&state=state&" + + "redirect_uri=https://example.com/authorize/oauth2/code/registration-id&" + + "item%20amount=19.95%E2%82%AC&%C3%A2ge=4%C2%BD&item%20name=H%C3%85M%C3%96"); + } }