AuthorizationCodeRequestRedirectFilter -> always expand redirectUri

Fixes gh-4444

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeAuthenticationProcessingFilter.java

@@ -31,15 +31,12 @@ import org.springframework.security.web.authentication.AbstractAuthenticationPro
 import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 
-import static org.springframework.security.oauth2.client.authentication.AuthorizationCodeRequestRedirectFilter.isDefaultRedirectUri;
-
 /**
  * An implementation of an {@link AbstractAuthenticationProcessingFilter} that handles
  * the processing of an <i>OAuth 2.0 Authorization Response</i> for the authorization code grant flow.
@@ -136,12 +133,16 @@ public class AuthorizationCodeAuthenticationProcessingFilter extends AbstractAut
 		ClientRegistration clientRegistration = this.getClientRegistrationRepository().getRegistrationByClientId(
 				matchingAuthorizationRequest.getClientId());
 
-		// If clientRegistration.redirectUri is the default one (with Uri template variables)
-		// then use matchingAuthorizationRequest.redirectUri instead
-		if (isDefaultRedirectUri(clientRegistration)) {
-			clientRegistration = new ClientRegistrationBuilderWithUriOverrides(
-				clientRegistration, matchingAuthorizationRequest.getRedirectUri()).build();
-		}
+		// The clientRegistration.redirectUri may contain Uri template variables, whether it's configured by
+		// the user or configured by default. In these cases, the redirectUri will be expanded and ultimately changed
+		// (by AuthorizationCodeRequestRedirectFilter) before setting it in the authorization request.
+		// The resulting redirectUri used for the authorization request and saved within the AuthorizationRequestRepository
+		// MUST BE the same one used to complete the authorization code flow.
+		// Therefore, we'll create a copy of the clientRegistration and override the redirectUri
+		// with the one contained in matchingAuthorizationRequest.
+		clientRegistration = new ClientRegistration.Builder(clientRegistration)
+			.redirectUri(matchingAuthorizationRequest.getRedirectUri())
+			.build();
 
 		AuthorizationCodeAuthorizationResponseAttributes authorizationCodeResponseAttributes =
 				this.authorizationCodeResponseConverter.apply(request);
@@ -203,24 +204,4 @@ public class AuthorizationCodeAuthenticationProcessingFilter extends AbstractAut
 			throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
 		}
 	}
-
-	private static class ClientRegistrationBuilderWithUriOverrides extends ClientRegistration.Builder {
-
-		private ClientRegistrationBuilderWithUriOverrides(ClientRegistration clientRegistration, String redirectUri) {
-			super(clientRegistration.getClientId());
-			this.clientSecret(clientRegistration.getClientSecret());
-			this.clientAuthenticationMethod(clientRegistration.getClientAuthenticationMethod());
-			this.authorizedGrantType(clientRegistration.getAuthorizedGrantType());
-			this.redirectUri(redirectUri);
-			if (!CollectionUtils.isEmpty(clientRegistration.getScopes())) {
-				this.scopes(clientRegistration.getScopes().stream().toArray(String[]::new));
-			}
-			this.authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri());
-			this.tokenUri(clientRegistration.getProviderDetails().getTokenUri());
-			this.userInfoUri(clientRegistration.getProviderDetails().getUserInfoUri());
-			this.jwkSetUri(clientRegistration.getProviderDetails().getJwkSetUri());
-			this.clientName(clientRegistration.getClientName());
-			this.clientAlias(clientRegistration.getClientAlias());
-		}
-	}
 }

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/AuthorizationCodeRequestRedirectFilter.java

@@ -32,6 +32,8 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.net.URI;
+import java.util.HashMap;
+import java.util.Map;
 
 import static org.springframework.security.oauth2.client.authentication.AuthorizationCodeAuthenticationProcessingFilter.AUTHORIZE_BASE_URI;
 
@@ -61,7 +63,6 @@ public class AuthorizationCodeRequestRedirectFilter extends OncePerRequestFilter
 	public static final String AUTHORIZATION_BASE_URI = "/oauth2/authorization/code";
 	private static final String CLIENT_ALIAS_VARIABLE_NAME = "clientAlias";
 	private static final String AUTHORIZATION_URI = AUTHORIZATION_BASE_URI + "/{" + CLIENT_ALIAS_VARIABLE_NAME + "}";
-	private static final String DEFAULT_REDIRECT_URI_TEMPLATE = "{scheme}://{serverName}:{serverPort}{baseAuthorizeUri}/{clientAlias}";
 	private final AntPathRequestMatcher authorizationRequestMatcher;
 	private final ClientRegistrationRepository clientRegistrationRepository;
 	private final AuthorizationRequestUriBuilder authorizationUriBuilder;
@@ -114,12 +115,7 @@ public class AuthorizationCodeRequestRedirectFilter extends OncePerRequestFilter
 			throw new IllegalArgumentException("Invalid Client Identifier (Alias): " + clientAlias);
 		}
 
-		String redirectUriStr;
-		if (isDefaultRedirectUri(clientRegistration)) {
-			redirectUriStr = this.expandDefaultRedirectUri(request, clientRegistration);
-		} else {
-			redirectUriStr = clientRegistration.getRedirectUri();
-		}
+		String redirectUriStr = this.expandRedirectUri(request, clientRegistration);
 
 		AuthorizationRequestAttributes authorizationRequestAttributes =
 			AuthorizationRequestAttributes.withAuthorizationCode()
@@ -145,15 +141,16 @@ public class AuthorizationCodeRequestRedirectFilter extends OncePerRequestFilter
 		response.sendError(HttpServletResponse.SC_BAD_REQUEST, failed.getMessage());
 	}
 
-	static boolean isDefaultRedirectUri(ClientRegistration clientRegistration) {
-		return DEFAULT_REDIRECT_URI_TEMPLATE.equals(clientRegistration.getRedirectUri());
-	}
+	private String expandRedirectUri(HttpServletRequest request, ClientRegistration clientRegistration) {
+		Map<String, String> uriVariables = new HashMap<>();
+		uriVariables.put("scheme", request.getScheme());
+		uriVariables.put("serverName", request.getServerName());
+		uriVariables.put("serverPort", String.valueOf(request.getServerPort()));
+		uriVariables.put("baseAuthorizeUri", AUTHORIZE_BASE_URI);
+		uriVariables.put("clientAlias", clientRegistration.getClientAlias());
 
-	private String expandDefaultRedirectUri(HttpServletRequest request, ClientRegistration clientRegistration) {
-		return UriComponentsBuilder.fromUriString(DEFAULT_REDIRECT_URI_TEMPLATE)
-			.buildAndExpand(request.getScheme(), request.getServerName(), request.getServerPort(),
-				AUTHORIZE_BASE_URI, clientRegistration.getClientAlias())
-			.encode()
+		return UriComponentsBuilder.fromUriString(clientRegistration.getRedirectUri())
+			.buildAndExpand(uriVariables)
 			.toUriString();
 	}
 }

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/registration/ClientRegistration.java

@@ -195,6 +195,23 @@ public class ClientRegistration {
 			this.clientAlias(clientRegistrationProperties.getClientAlias());
 		}
 
+		public Builder(ClientRegistration clientRegistration) {
+			this(clientRegistration.getClientId());
+			this.clientSecret(clientRegistration.getClientSecret());
+			this.clientAuthenticationMethod(clientRegistration.getClientAuthenticationMethod());
+			this.authorizedGrantType(clientRegistration.getAuthorizedGrantType());
+			this.redirectUri(clientRegistration.getRedirectUri());
+			if (!CollectionUtils.isEmpty(clientRegistration.getScopes())) {
+				this.scopes(clientRegistration.getScopes().stream().toArray(String[]::new));
+			}
+			this.authorizationUri(clientRegistration.getProviderDetails().getAuthorizationUri());
+			this.tokenUri(clientRegistration.getProviderDetails().getTokenUri());
+			this.userInfoUri(clientRegistration.getProviderDetails().getUserInfoUri());
+			this.jwkSetUri(clientRegistration.getProviderDetails().getJwkSetUri());
+			this.clientName(clientRegistration.getClientName());
+			this.clientAlias(clientRegistration.getClientAlias());
+		}
+
 		public Builder clientSecret(String clientSecret) {
 			this.clientSecret = clientSecret;
 			return this;