Prevent double-escaping of authorize URL parameters

If the authorization URL in the OAuth2 provider configuration contained query parameters with escaped characters, these characters were escaped a second time. This commit fixes it.

It is relevant to support the OIDC claims parameter (see https://openid.net/specs/openid-connect-core-1_0.html#ClaimsParameter).

Fixes gh-7871

Author: manuelbl

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.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 OAuth2AuthorizationRequest build() {
 
 		private String buildAuthorizationRequestUri() {
 			MultiValueMap<String, String> 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<String> toLinkedHashSet(String... scope) {
 			LinkedHashSet<String> result = new LinkedHashSet<>();
 			Collections.addAll(result, scope);

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 void buildWhenAuthorizationUriIncludesQueryParameterThenAuthorizationRequ
 						"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<String, Object> 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");
+	}
 }