Polish AuthorizationCodeAuthenticationFilter

Fixes gh-4599

config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/AuthorizationCodeGrantConfigurer.java

@@ -45,7 +45,6 @@ import org.springframework.security.oauth2.core.user.OAuth2User;
 import org.springframework.security.oauth2.oidc.client.authentication.OidcAuthorizationCodeAuthenticator;
 import org.springframework.security.oauth2.oidc.client.user.OidcUserService;
 import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy;
-import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.Assert;
 
 import java.net.URI;
@@ -65,13 +64,13 @@ public class AuthorizationCodeGrantConfigurer<B extends HttpSecurityBuilder<B>>
 
 	// ***** Authorization Request members
 	private AuthorizationCodeRequestRedirectFilter authorizationRequestFilter;
-	private String authorizationRequestBaseUri = AuthorizationCodeRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI;
+	private String authorizationRequestBaseUri;
 	private AuthorizationRequestUriBuilder authorizationRequestBuilder;
 	private AuthorizationRequestRepository authorizationRequestRepository;
 
 	// ***** Authorization Response members
 	private AuthorizationCodeAuthenticationFilter authorizationResponseFilter;
-	private RequestMatcher authorizationResponseMatcher;
+	private String authorizationResponseBaseUri;
 	private AuthorizationGrantAuthenticator<AuthorizationCodeAuthenticationToken> authorizationCodeAuthenticator;
 	private AuthorizationGrantTokenExchanger<AuthorizationCodeAuthenticationToken> authorizationCodeTokenExchanger;
 	private SecurityTokenRepository<AccessToken> accessTokenRepository;
@@ -98,9 +97,9 @@ public class AuthorizationCodeGrantConfigurer<B extends HttpSecurityBuilder<B>>
 		return this;
 	}
 
-	public AuthorizationCodeGrantConfigurer<B> authorizationResponseMatcher(RequestMatcher authorizationResponseMatcher) {
-		Assert.notNull(authorizationResponseMatcher, "authorizationResponseMatcher cannot be null");
-		this.authorizationResponseMatcher = authorizationResponseMatcher;
+	public AuthorizationCodeGrantConfigurer<B> authorizationResponseBaseUri(String authorizationResponseBaseUri) {
+		Assert.hasText(authorizationResponseBaseUri, "authorizationResponseBaseUri cannot be empty");
+		this.authorizationResponseBaseUri = authorizationResponseBaseUri;
 		return this;
 	}
 
@@ -183,7 +182,7 @@ public class AuthorizationCodeGrantConfigurer<B extends HttpSecurityBuilder<B>>
 		//
 		// 	-> AuthorizationCodeRequestRedirectFilter
 		this.authorizationRequestFilter = new AuthorizationCodeRequestRedirectFilter(
-			this.authorizationRequestBaseUri, this.getClientRegistrationRepository());
+			this.getAuthorizationRequestBaseUri(), this.getClientRegistrationRepository());
 		if (this.authorizationRequestBuilder != null) {
 			this.authorizationRequestFilter.setAuthorizationUriBuilder(this.authorizationRequestBuilder);
 		}
@@ -192,11 +191,8 @@ public class AuthorizationCodeGrantConfigurer<B extends HttpSecurityBuilder<B>>
 		}
 
 		// 	-> AuthorizationCodeAuthenticationFilter
-		this.authorizationResponseFilter = new AuthorizationCodeAuthenticationFilter();
+		this.authorizationResponseFilter = new AuthorizationCodeAuthenticationFilter(this.getAuthorizationResponseBaseUri());
 		this.authorizationResponseFilter.setClientRegistrationRepository(this.getClientRegistrationRepository());
-		if (this.authorizationResponseMatcher != null) {
-			this.authorizationResponseFilter.setAuthorizationResponseMatcher(this.authorizationResponseMatcher);
-		}
 		if (this.authorizationRequestRepository != null) {
 			this.authorizationResponseFilter.setAuthorizationRequestRepository(this.authorizationRequestRepository);
 		}
@@ -219,15 +215,15 @@ public class AuthorizationCodeGrantConfigurer<B extends HttpSecurityBuilder<B>>
 	}
 
 	String getAuthorizationRequestBaseUri() {
-		return this.authorizationRequestBaseUri;
-	}
-
-	AuthorizationCodeAuthenticationFilter getAuthorizationResponseFilter() {
-		return this.authorizationResponseFilter;
+		return this.authorizationRequestBaseUri != null ?
+			this.authorizationRequestBaseUri :
+			AuthorizationCodeRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI;
 	}
 
-	RequestMatcher getAuthorizationResponseMatcher() {
-		return this.authorizationResponseMatcher;
+	String getAuthorizationResponseBaseUri() {
+		return this.authorizationResponseBaseUri != null ?
+			this.authorizationResponseBaseUri :
+			AuthorizationCodeAuthenticationFilter.DEFAULT_AUTHORIZATION_RESPONSE_BASE_URI;
 	}
 
 	AuthorizationRequestRepository getAuthorizationRequestRepository() {

config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java

@@ -149,9 +149,9 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 		private RedirectionEndpointConfig() {
 		}
 
-		public RedirectionEndpointConfig requestMatcher(RequestMatcher authorizationResponseMatcher) {
-			Assert.notNull(authorizationResponseMatcher, "authorizationResponseMatcher cannot be null");
-			authorizationCodeGrantConfigurer.authorizationResponseMatcher(authorizationResponseMatcher);
+		public RedirectionEndpointConfig baseUri(String authorizationResponseBaseUri) {
+			Assert.hasText(authorizationResponseBaseUri, "authorizationResponseBaseUri cannot be empty");
+			authorizationCodeGrantConfigurer.authorizationResponseBaseUri(authorizationResponseBaseUri);
 			return this;
 		}
 
@@ -209,9 +209,7 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 
 	@Override
 	protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingUrl) {
-		return (this.authorizationCodeGrantConfigurer.getAuthorizationResponseMatcher() != null ?
-			this.authorizationCodeGrantConfigurer.getAuthorizationResponseMatcher() :
-			this.getAuthenticationFilter().getAuthorizationResponseMatcher());
+		return this.getAuthenticationFilter().getAuthorizationResponseMatcher();
 	}
 
 	private ClientRegistrationRepository getClientRegistrationRepository() {
@@ -244,9 +242,11 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 		}
 
 		Map<String, String> authenticationUrlToClientName = new HashMap<>();
-		clientRegistrations.forEach(registration -> authenticationUrlToClientName.put(
-			authorizationCodeGrantConfigurer.getAuthorizationRequestBaseUri() + "/" + registration.getRegistrationId(),
-			registration.getClientName()));
+		clientRegistrations.forEach(registration -> {
+			authenticationUrlToClientName.put(
+				authorizationCodeGrantConfigurer.getAuthorizationRequestBaseUri() + "/" + registration.getRegistrationId(),
+				registration.getClientName());
+		});
 		loginPageGeneratingFilter.setOauth2LoginEnabled(true);
 		loginPageGeneratingFilter.setOauth2AuthenticationUrlToClientName(authenticationUrlToClientName);
 		loginPageGeneratingFilter.setLoginPageUrl(this.getLoginPage());
@@ -261,10 +261,8 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 
 			AuthorizationCodeAuthenticationFilter authorizationResponseFilter = getAuthenticationFilter();
 			authorizationResponseFilter.setClientRegistrationRepository(getClientRegistrationRepository());
-			if (authorizationCodeGrantConfigurer.getAuthorizationResponseMatcher() != null) {
-				authorizationResponseFilter.setAuthorizationResponseMatcher(
-					authorizationCodeGrantConfigurer.getAuthorizationResponseMatcher());
-			}
+			authorizationResponseFilter.setAuthorizationResponseBaseUri(
+				authorizationCodeGrantConfigurer.getAuthorizationResponseBaseUri());
 			if (authorizationCodeGrantConfigurer.getAuthorizationRequestRepository() != null) {
 				authorizationResponseFilter.setAuthorizationRequestRepository(
 					authorizationCodeGrantConfigurer.getAuthorizationRequestRepository());

oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/web/AuthorizationCodeAuthenticationFilter.java

@@ -83,30 +83,41 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio
 	public static final String DEFAULT_AUTHORIZATION_RESPONSE_BASE_URI = "/oauth2/authorize/code";
 	private static final String AUTHORIZATION_REQUEST_NOT_FOUND_ERROR_CODE = "authorization_request_not_found";
 	private final AuthorizationResponseConverter authorizationResponseConverter = new AuthorizationResponseConverter();
+	private final ClientRegistrationIdentifierStrategy<String> providerIdentifierStrategy = new ProviderIdentifierStrategy();
+	private RequestMatcher authorizationResponseMatcher;
 	private ClientRegistrationRepository clientRegistrationRepository;
-	private RequestMatcher authorizationResponseMatcher = new AuthorizationResponseMatcher();
 	private AuthorizationRequestRepository authorizationRequestRepository = new HttpSessionAuthorizationRequestRepository();
-	private final ClientRegistrationIdentifierStrategy<String> providerIdentifierStrategy = new ProviderIdentifierStrategy();
 
 	public AuthorizationCodeAuthenticationFilter() {
-		super(new AuthorizationResponseMatcher());
+		this(DEFAULT_AUTHORIZATION_RESPONSE_BASE_URI);
+	}
+
+	public AuthorizationCodeAuthenticationFilter(String authorizationResponseBaseUri) {
+		super(new AuthorizationResponseMatcher(authorizationResponseBaseUri));
+		this.authorizationResponseMatcher = new AuthorizationResponseMatcher(authorizationResponseBaseUri);
+	}
+
+	@Override
+	public void afterPropertiesSet() {
+		super.afterPropertiesSet();
+		Assert.notNull(this.clientRegistrationRepository, "clientRegistrationRepository cannot be null");
 	}
 
 	@Override
 	public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response)
 			throws AuthenticationException, IOException, ServletException {
 
-		AuthorizationRequest authorizationRequest = this.getAuthorizationRequestRepository().loadAuthorizationRequest(request);
+		AuthorizationRequest authorizationRequest = this.authorizationRequestRepository.loadAuthorizationRequest(request);
 		if (authorizationRequest == null) {
 			OAuth2Error oauth2Error = new OAuth2Error(AUTHORIZATION_REQUEST_NOT_FOUND_ERROR_CODE);
 			throw new OAuth2AuthenticationException(oauth2Error, oauth2Error.toString());
 		}
-		this.getAuthorizationRequestRepository().removeAuthorizationRequest(request);
+		this.authorizationRequestRepository.removeAuthorizationRequest(request);
 
 		AuthorizationResponse authorizationResponse = this.authorizationResponseConverter.apply(request);
 
 		String registrationId = (String)authorizationRequest.getAdditionalParameters().get(OAuth2Parameter.REGISTRATION_ID);
-		ClientRegistration clientRegistration = this.getClientRegistrationRepository().findByRegistrationId(registrationId);
+		ClientRegistration clientRegistration = this.clientRegistrationRepository.findByRegistrationId(registrationId);
 
 		// 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
@@ -142,18 +153,14 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio
 		return oauth2UserAuthentication;
 	}
 
-	public RequestMatcher getAuthorizationResponseMatcher() {
+	public final RequestMatcher getAuthorizationResponseMatcher() {
 		return this.authorizationResponseMatcher;
 	}
 
-	public final <T extends RequestMatcher> void setAuthorizationResponseMatcher(T authorizationResponseMatcher) {
-		Assert.notNull(authorizationResponseMatcher, "authorizationResponseMatcher cannot be null");
-		this.authorizationResponseMatcher = authorizationResponseMatcher;
-		this.setRequiresAuthenticationRequestMatcher(authorizationResponseMatcher);
-	}
-
-	protected ClientRegistrationRepository getClientRegistrationRepository() {
-		return this.clientRegistrationRepository;
+	public final void setAuthorizationResponseBaseUri(String authorizationResponseBaseUri) {
+		Assert.hasText(authorizationResponseBaseUri, "authorizationResponseBaseUri cannot be empty");
+		this.authorizationResponseMatcher = new AuthorizationResponseMatcher(authorizationResponseBaseUri);
+		this.setRequiresAuthenticationRequestMatcher(this.authorizationResponseMatcher);
 	}
 
 	public final void setClientRegistrationRepository(ClientRegistrationRepository clientRegistrationRepository) {
@@ -161,10 +168,6 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio
 		this.clientRegistrationRepository = clientRegistrationRepository;
 	}
 
-	protected AuthorizationRequestRepository getAuthorizationRequestRepository() {
-		return this.authorizationRequestRepository;
-	}
-
 	public final void setAuthorizationRequestRepository(AuthorizationRequestRepository authorizationRequestRepository) {
 		Assert.notNull(authorizationRequestRepository, "authorizationRequestRepository cannot be null");
 		this.authorizationRequestRepository = authorizationRequestRepository;
@@ -215,10 +218,17 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio
 	}
 
 	private static class AuthorizationResponseMatcher implements RequestMatcher {
+		private final String baseUri;
+
+		private AuthorizationResponseMatcher(String baseUri) {
+			Assert.hasText(baseUri, "baseUri cannot be empty");
+			this.baseUri = baseUri;
+		}
 
 		@Override
 		public boolean matches(HttpServletRequest request) {
-			return this.successResponse(request) || this.errorResponse(request);
+			return request.getRequestURI().startsWith(this.baseUri) &&
+				(this.successResponse(request) || this.errorResponse(request));
 		}
 
 		private boolean successResponse(HttpServletRequest request) {