authorization_uri Uses UriComponentsBuilder

Because of this, authorization_uri can now be a fully-qualified url.

Fixes: gh-5760

config/src/test/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2ClientConfigurerTests.java

@@ -135,7 +135,10 @@ public class OAuth2ClientConfigurerTests {
 		MvcResult mvcResult = this.mockMvc.perform(get("/oauth2/authorization/registration-1"))
 			.andExpect(status().is3xxRedirection())
 			.andReturn();
-		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?response_type=code&client_id=client-1&scope=user&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Fclient-1");
+		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
+				"response_type=code&client_id=client-1&" +
+				"scope=user&state=.{15,}&" +
+				"redirect_uri=http://localhost/client-1");
 	}
 
 	@Test
@@ -184,7 +187,10 @@ public class OAuth2ClientConfigurerTests {
 		MvcResult mvcResult = this.mockMvc.perform(get("/resource1").with(user("user1")))
 				.andExpect(status().is3xxRedirection())
 				.andReturn();
-		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?response_type=code&client_id=client-1&scope=user&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Fclient-1");
+		assertThat(mvcResult.getResponse().getRedirectedUrl()).matches("https://provider.com/oauth2/authorize\\?" +
+				"response_type=code&client_id=client-1&" +
+				"scope=user&state=.{15,}&" +
+				"redirect_uri=http://localhost/client-1");
 
 		verify(requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
 	}

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/DefaultOAuth2AuthorizationRequestResolverTests.java

@@ -107,7 +107,11 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		assertThat(authorizationRequest.getState()).isNotNull();
 		assertThat(authorizationRequest.getAdditionalParameters())
 				.containsExactly(entry(OAuth2ParameterNames.REGISTRATION_ID, clientRegistration.getRegistrationId()));
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Flogin%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" +
+						"response_type=code&client_id=client-id&" +
+						"scope=read:user&state=.{15,}&" +
+						"redirect_uri=http://localhost/login/oauth2/code/registration-id");
 	}
 
 	@Test
@@ -164,7 +168,11 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		request.setServletPath(requestUri);
 
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Fexample.com%2Flogin%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" +
+						"response_type=code&client_id=client-id&" +
+						"scope=read:user&state=.{15,}&" +
+						"redirect_uri=http://example.com/login/oauth2/code/registration-id");
 	}
 
 	@Test
@@ -178,7 +186,11 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		request.setServletPath(requestUri);
 
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=https%3A%2F%2Fexample.com%2Flogin%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" +
+						"response_type=code&client_id=client-id&" +
+						"scope=read:user&state=.{15,}&" +
+						"redirect_uri=https://example.com/login/oauth2/code/registration-id");
 	}
 
 	@Test
@@ -189,7 +201,11 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		request.setServletPath(requestUri);
 
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request, clientRegistration.getRegistrationId());
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Fauthorize%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" +
+						"response_type=code&client_id=client-id&" +
+						"scope=read:user&state=.{15,}&" +
+						"redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
 	}
 
 	@Test
@@ -200,7 +216,11 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		request.setServletPath(requestUri);
 
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id-2&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Flogin%2Foauth2%2Fcode%2Fregistration-id-2");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" +
+						"response_type=code&client_id=client-id-2&" +
+						"scope=read:user&state=.{15,}&" +
+						"redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
 	}
 
 	@Test
@@ -212,7 +232,11 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		request.setServletPath(requestUri);
 
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Fauthorize%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" +
+						"response_type=code&client_id=client-id&" +
+						"scope=read:user&state=.{15,}&" +
+						"redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
 	}
 
 	@Test
@@ -224,6 +248,10 @@ public class DefaultOAuth2AuthorizationRequestResolverTests {
 		request.setServletPath(requestUri);
 
 		OAuth2AuthorizationRequest authorizationRequest = this.resolver.resolve(request);
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id-2&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Flogin%2Foauth2%2Fcode%2Fregistration-id-2");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.matches("https://example.com/login/oauth/authorize\\?" +
+						"response_type=code&client_id=client-id-2&" +
+						"scope=read:user&state=.{15,}&" +
+						"redirect_uri=http://localhost/login/oauth2/code/registration-id-2");
 	}
 }

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/OAuth2AuthorizationRequestRedirectFilterTests.java

@@ -151,7 +151,10 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 
 		verifyZeroInteractions(filterChain);
 
-		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Flogin%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
+				"response_type=code&client_id=client-id&" +
+				"scope=read:user&state=.{15,}&" +
+				"redirect_uri=http://localhost/login/oauth2/code/registration-id");
 	}
 
 	@Test
@@ -187,7 +190,10 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 
 		verifyZeroInteractions(filterChain);
 
-		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?response_type=token&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Fauthorize%2Foauth2%2Fimplicit%2Fregistration-3");
+		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
+				"response_type=token&client_id=client-id&" +
+				"scope=read:user&state=.{15,}&" +
+				"redirect_uri=http://localhost/authorize/oauth2/implicit/registration-3");
 	}
 
 	@Test
@@ -225,7 +231,10 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 
 		verifyZeroInteractions(filterChain);
 
-		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Flogin%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
+				"response_type=code&client_id=client-id&" +
+				"scope=read:user&state=.{15,}&" +
+				"redirect_uri=http://localhost/login/oauth2/code/registration-id");
 	}
 
 	@Test
@@ -243,7 +252,10 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 
 		verify(filterChain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
 
-		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Fauthorize%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
+				"response_type=code&client_id=client-id&" +
+				"scope=read:user&state=.{15,}&" +
+				"redirect_uri=http://localhost/authorize/oauth2/code/registration-id");
 		verify(this.requestCache).saveRequest(any(HttpServletRequest.class), any(HttpServletResponse.class));
 	}
 
@@ -298,7 +310,11 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 
 		verifyZeroInteractions(filterChain);
 
-		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Flogin%2Foauth2%2Fcode%2Fregistration-id&idp=https%3A%2F%2Fother.provider.com");
+		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
+				"response_type=code&client_id=client-id&" +
+				"scope=read:user&state=.{15,}&" +
+				"redirect_uri=http://localhost/login/oauth2/code/registration-id&" +
+				"idp=https://other.provider.com");
 	}
 
 	// gh-4911, gh-5244
@@ -339,6 +355,10 @@ public class OAuth2AuthorizationRequestRedirectFilterTests {
 
 		verifyZeroInteractions(filterChain);
 
-		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.{15,}&redirect_uri=http%3A%2F%2Flocalhost%2Flogin%2Foauth2%2Fcode%2Fregistration-id&login_hint=user@provider\\.com");
+		assertThat(response.getRedirectedUrl()).matches("https://example.com/login/oauth/authorize\\?" +
+				"response_type=code&client_id=client-id&" +
+				"scope=read:user&state=.{15,}&" +
+				"redirect_uri=http://localhost/login/oauth2/code/registration-id&" +
+				"login_hint=user@provider\\.com");
 	}
 }

oauth2/oauth2-client/src/test/java/org/springframework/security/oauth2/client/web/server/DefaultServerOAuth2AuthorizationRequestResolverTests.java

@@ -77,7 +77,10 @@ public class DefaultServerOAuth2AuthorizationRequestResolverTests {
 
 		OAuth2AuthorizationRequest request = resolve("/oauth2/authorization/not-found-id");
 
-		assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?response_type=code&client_id=client-id&scope=read%3Auser&state=.*?&redirect_uri=%2Flogin%2Foauth2%2Fcode%2Fregistration-id");
+		assertThat(request.getAuthorizationRequestUri()).matches("https://example.com/login/oauth/authorize\\?" +
+				"response_type=code&client_id=client-id&" +
+				"scope=read:user&state=.*?&" +
+				"redirect_uri=/login/oauth2/code/registration-id");
 	}
 
 	private OAuth2AuthorizationRequest resolve(String path) {

oauth2/oauth2-core/spring-security-oauth2-core.gradle

@@ -3,6 +3,7 @@ apply plugin: 'io.spring.convention.spring-module'
 dependencies {
 	compile project(':spring-security-core')
 	compile springCoreDependency
+	compile 'org.springframework:spring-web'
 
 	optional 'com.fasterxml.jackson.core:jackson-databind'
 	optional 'com.nimbusds:oauth2-oidc-sdk'

oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequest.java

@@ -15,24 +15,25 @@
  */
 package org.springframework.security.oauth2.core.endpoint;
 
-import org.springframework.security.core.SpringSecurityCoreVersion;
-import org.springframework.security.oauth2.core.AuthorizationGrantType;
-import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
-import org.springframework.util.StringUtils;
-
 import java.io.Serializable;
-import java.io.UnsupportedEncodingException;
-import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
-import java.util.StringJoiner;
 import java.util.stream.Collectors;
 
+import org.springframework.security.core.SpringSecurityCoreVersion;
+import org.springframework.security.oauth2.core.AuthorizationGrantType;
+import org.springframework.util.Assert;
+import org.springframework.util.CollectionUtils;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
+import org.springframework.util.StringUtils;
+import org.springframework.web.util.UriComponentsBuilder;
+
 /**
  * A representation of an OAuth 2.0 Authorization Request
  * for the authorization code grant type or implicit grant type.
@@ -336,34 +337,30 @@ public final class OAuth2AuthorizationRequest implements Serializable {
 		}
 
 		private String buildAuthorizationRequestUri() {
-			Map<String, String> parameters = new LinkedHashMap<>();
-			parameters.put(OAuth2ParameterNames.RESPONSE_TYPE, this.responseType.getValue());
-			parameters.put(OAuth2ParameterNames.CLIENT_ID, this.clientId);
+			MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
+			parameters.set(OAuth2ParameterNames.RESPONSE_TYPE, this.responseType.getValue());
+			parameters.set(OAuth2ParameterNames.CLIENT_ID, this.clientId);
 			if (!CollectionUtils.isEmpty(this.scopes)) {
-				parameters.put(OAuth2ParameterNames.SCOPE,
+				parameters.set(OAuth2ParameterNames.SCOPE,
 						StringUtils.collectionToDelimitedString(this.scopes, " "));
 			}
 			if (this.state != null) {
-				parameters.put(OAuth2ParameterNames.STATE, this.state);
+				parameters.set(OAuth2ParameterNames.STATE, this.state);
 			}
 			if (this.redirectUri != null) {
-				parameters.put(OAuth2ParameterNames.REDIRECT_URI, this.redirectUri);
+				parameters.set(OAuth2ParameterNames.REDIRECT_URI, this.redirectUri);
 			}
 			if (!CollectionUtils.isEmpty(this.additionalParameters)) {
 				this.additionalParameters.entrySet().stream()
 						.filter(e -> !e.getKey().equals(OAuth2ParameterNames.REGISTRATION_ID))
-						.forEach(e -> parameters.put(e.getKey(), e.getValue().toString()));
+						.forEach(e -> parameters.set(e.getKey(), e.getValue().toString()));
 			}
 
-			try {
-				StringJoiner queryParams = new StringJoiner("&");
-				for (String paramName : parameters.keySet()) {
-					queryParams.add(paramName + "=" + URLEncoder.encode(parameters.get(paramName), "UTF-8"));
-				}
-				return this.authorizationUri + "?" + queryParams.toString();
-			} catch (UnsupportedEncodingException ex) {
-				throw new IllegalArgumentException("Unable to build authorization request uri: " + ex.getMessage(), ex);
-			}
+			return UriComponentsBuilder.fromHttpUrl(this.authorizationUri)
+					.queryParams(parameters)
+					.encode(StandardCharsets.UTF_8)
+					.build()
+					.toUriString();
 		}
 	}
 }

oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/endpoint/OAuth2AuthorizationRequestTests.java

@@ -15,16 +15,19 @@
  */
 package org.springframework.security.oauth2.core.endpoint;
 
-import org.junit.Test;
-import org.springframework.security.oauth2.core.AuthorizationGrantType;
-
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
 
-import static org.assertj.core.api.Assertions.*;
+import org.junit.Test;
+
+import org.springframework.security.oauth2.core.AuthorizationGrantType;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /**
  * Tests for {@link OAuth2AuthorizationRequest}.
@@ -194,7 +197,11 @@ public class OAuth2AuthorizationRequestTests {
 				.state(STATE)
 				.build();
 
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).isEqualTo("https://provider.com/oauth2/authorize?response_type=token&client_id=client-id&scope=scope1+scope2&state=state&redirect_uri=http%3A%2F%2Fexample.com");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.isEqualTo("https://provider.com/oauth2/authorize?" +
+						"response_type=token&client_id=client-id&" +
+						"scope=scope1%20scope2&state=state&" +
+						"redirect_uri=http://example.com");
 	}
 
 	@Test
@@ -226,7 +233,11 @@ public class OAuth2AuthorizationRequestTests {
 				.build();
 
 		assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull();
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).isEqualTo("https://provider.com/oauth2/authorize?response_type=code&client_id=client-id&scope=scope1+scope2&state=state&redirect_uri=http%3A%2F%2Fexample.com&param1=value1&param2=value2");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.isEqualTo("https://provider.com/oauth2/authorize?" +
+						"response_type=code&client_id=client-id&" +
+						"scope=scope1%20scope2&state=state&" +
+						"redirect_uri=http://example.com&param1=value1&param2=value2");
 	}
 
 	@Test
@@ -248,13 +259,17 @@ public class OAuth2AuthorizationRequestTests {
 		OAuth2AuthorizationRequest authorizationRequest = OAuth2AuthorizationRequest.authorizationCode()
 				.authorizationUri(AUTHORIZATION_URI)
 				.clientId(CLIENT_ID)
-				.redirectUri(REDIRECT_URI)
+				.redirectUri(REDIRECT_URI + "?rparam1=rvalue1&rparam2=rvalue2")
 				.scopes(SCOPES)
 				.state(STATE)
 				.additionalParameters(additionalParameters)
 				.build();
 
-		assertThat(authorizationRequest.getAuthorizationRequestUri()).isEqualTo("https://provider.com/oauth2/authorize?response_type=code&client_id=client-id&scope=scope1+scope2&state=state&redirect_uri=http%3A%2F%2Fexample.com&param1=value1");
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.isEqualTo("https://provider.com/oauth2/authorize?" +
+						"response_type=code&client_id=client-id&" +
+						"scope=scope1%20scope2&state=state&" +
+						"redirect_uri=http://example.com?rparam1%3Drvalue1%26rparam2%3Drvalue2&param1=value1");
 	}
 
 	@Test
@@ -290,4 +305,19 @@ public class OAuth2AuthorizationRequestTests {
 		assertThat(authorizationRequestCopy.getAdditionalParameters()).isEqualTo(authorizationRequest.getAdditionalParameters());
 		assertThat(authorizationRequestCopy.getAuthorizationRequestUri()).isEqualTo(authorizationRequest.getAuthorizationRequestUri());
 	}
+
+	@Test
+	public void buildWhenAuthorizationUriIncludesQueryParameterThenAuthorizationRequestUrlIncludesIt() {
+		OAuth2AuthorizationRequest authorizationRequest =
+				TestOAuth2AuthorizationRequests.request()
+						.authorizationUri(AUTHORIZATION_URI +
+								"?param1=value1&param2=value2").build();
+
+		assertThat(authorizationRequest.getAuthorizationRequestUri()).isNotNull();
+		assertThat(authorizationRequest.getAuthorizationRequestUri())
+				.isEqualTo("https://provider.com/oauth2/authorize?" +
+						"param1=value1&param2=value2&" +
+						"response_type=code&client_id=client-id&state=state&" +
+						"redirect_uri=https://example.com/authorize/oauth2/code/registration-id");
+	}
 }