Browse Source

Remove AuthorizationCodeAuthenticationFilter.AuthorizationResponseMatcher

Fixes gh-4654
Joe Grandja 7 years ago
parent
commit
8e3a2a7123

+ 10 - 8
config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/AuthorizationCodeGrantConfigurer.java

@@ -126,7 +126,10 @@ public class AuthorizationCodeGrantConfigurer<B extends HttpSecurityBuilder<B>>
 		http.authenticationProvider(this.postProcess(oidcAuthorizationCodeAuthenticationProvider));
 
 		this.authorizationRequestFilter = new AuthorizationRequestRedirectFilter(
-			this.getAuthorizationRequestBaseUri(), this.getClientRegistrationRepository());
+			this.authorizationRequestBaseUri != null ?
+				this.authorizationRequestBaseUri :
+				AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI,
+			this.getClientRegistrationRepository());
 		if (this.authorizationRequestUriBuilder != null) {
 			this.authorizationRequestFilter.setAuthorizationRequestUriBuilder(this.authorizationRequestUriBuilder);
 		}
@@ -134,7 +137,10 @@ public class AuthorizationCodeGrantConfigurer<B extends HttpSecurityBuilder<B>>
 			this.authorizationRequestFilter.setAuthorizationRequestRepository(this.authorizationRequestRepository);
 		}
 
-		this.authorizationResponseFilter = new AuthorizationCodeAuthenticationFilter(this.getAuthorizationResponseBaseUri());
+		this.authorizationResponseFilter = new AuthorizationCodeAuthenticationFilter(
+			this.authorizationResponseBaseUri != null ?
+				this.authorizationResponseBaseUri :
+				AuthorizationCodeAuthenticationFilter.DEFAULT_AUTHORIZATION_RESPONSE_BASE_URI);
 		this.authorizationResponseFilter.setClientRegistrationRepository(this.getClientRegistrationRepository());
 		if (this.authorizationRequestRepository != null) {
 			this.authorizationResponseFilter.setAuthorizationRequestRepository(this.authorizationRequestRepository);
@@ -158,15 +164,11 @@ public class AuthorizationCodeGrantConfigurer<B extends HttpSecurityBuilder<B>>
 	}
 
 	String getAuthorizationRequestBaseUri() {
-		return this.authorizationRequestBaseUri != null ?
-			this.authorizationRequestBaseUri :
-			AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI;
+		return this.authorizationRequestBaseUri;
 	}
 
 	String getAuthorizationResponseBaseUri() {
-		return this.authorizationResponseBaseUri != null ?
-			this.authorizationResponseBaseUri :
-			AuthorizationCodeAuthenticationFilter.DEFAULT_AUTHORIZATION_RESPONSE_BASE_URI;
+		return this.authorizationResponseBaseUri;
 	}
 
 	AuthorizationRequestRepository getAuthorizationRequestRepository() {

+ 16 - 10
config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java

@@ -32,6 +32,7 @@ import org.springframework.security.oauth2.client.registration.ClientRegistratio
 import org.springframework.security.oauth2.client.registration.InMemoryClientRegistrationRepository;
 import org.springframework.security.oauth2.client.token.SecurityTokenRepository;
 import org.springframework.security.oauth2.client.web.AuthorizationCodeAuthenticationFilter;
+import org.springframework.security.oauth2.client.web.AuthorizationRequestRedirectFilter;
 import org.springframework.security.oauth2.client.web.AuthorizationRequestRepository;
 import org.springframework.security.oauth2.client.web.AuthorizationRequestUriBuilder;
 import org.springframework.security.oauth2.core.AccessToken;
@@ -39,6 +40,7 @@ import org.springframework.security.oauth2.core.user.OAuth2User;
 import org.springframework.security.oauth2.oidc.client.authentication.userinfo.OidcUserAuthenticationProvider;
 import org.springframework.security.oauth2.oidc.client.authentication.userinfo.OidcUserService;
 import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter;
+import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.Assert;
 
@@ -58,6 +60,7 @@ import java.util.Map;
 public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> extends
 	AbstractAuthenticationFilterConfigurer<B, OAuth2LoginConfigurer<B>, AuthorizationCodeAuthenticationFilter> {
 
+	private static final String DEFAULT_LOGIN_PROCESSING_URI = "/login/oauth2/authorize/code/*";
 	private final AuthorizationCodeGrantConfigurer<B> authorizationCodeGrantConfigurer = new AuthorizationCodeGrantLoginConfigurer();
 	private final AuthorizationEndpointConfig authorizationEndpointConfig = new AuthorizationEndpointConfig();
 	private final TokenEndpointConfig tokenEndpointConfig = new TokenEndpointConfig();
@@ -65,8 +68,7 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 	private final UserInfoEndpointConfig userInfoEndpointConfig = new UserInfoEndpointConfig();
 
 	public OAuth2LoginConfigurer() {
-		super(new AuthorizationCodeAuthenticationFilter(), null);
-		this.authorizationCodeGrantConfigurer.authorizationResponseBaseUri("/login/oauth2/authorize/code");
+		super(new AuthorizationCodeAuthenticationFilter(DEFAULT_LOGIN_PROCESSING_URI), DEFAULT_LOGIN_PROCESSING_URI);
 	}
 
 	public OAuth2LoginConfigurer<B> clients(ClientRegistration... clientRegistrations) {
@@ -248,7 +250,7 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 
 	@Override
 	protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingUrl) {
-		return this.getAuthenticationFilter().getAuthorizationResponseMatcher();
+		return new AntPathRequestMatcher(loginProcessingUrl);
 	}
 
 	private ClientRegistrationRepository getClientRegistrationRepository() {
@@ -280,12 +282,14 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 			return;
 		}
 
+		String authorizationRequestBaseUri = authorizationCodeGrantConfigurer.getAuthorizationRequestBaseUri() != null ?
+			authorizationCodeGrantConfigurer.getAuthorizationRequestBaseUri() :
+			AuthorizationRequestRedirectFilter.DEFAULT_AUTHORIZATION_REQUEST_BASE_URI;
 		Map<String, String> authenticationUrlToClientName = new HashMap<>();
-		clientRegistrations.forEach(registration -> {
-			authenticationUrlToClientName.put(
-				authorizationCodeGrantConfigurer.getAuthorizationRequestBaseUri() + "/" + registration.getRegistrationId(),
-				registration.getClientName());
-		});
+
+		clientRegistrations.forEach(registration -> authenticationUrlToClientName.put(
+			authorizationRequestBaseUri + "/" + registration.getRegistrationId(),
+			registration.getClientName()));
 		loginPageGeneratingFilter.setOauth2LoginEnabled(true);
 		loginPageGeneratingFilter.setOauth2AuthenticationUrlToClientName(authenticationUrlToClientName);
 		loginPageGeneratingFilter.setLoginPageUrl(this.getLoginPage());
@@ -300,8 +304,10 @@ public final class OAuth2LoginConfigurer<B extends HttpSecurityBuilder<B>> exten
 
 			AuthorizationCodeAuthenticationFilter authorizationResponseFilter = getAuthenticationFilter();
 			authorizationResponseFilter.setClientRegistrationRepository(getClientRegistrationRepository());
-			authorizationResponseFilter.setAuthorizationResponseBaseUri(
-				authorizationCodeGrantConfigurer.getAuthorizationResponseBaseUri());
+			if (authorizationCodeGrantConfigurer.getAuthorizationResponseBaseUri() != null) {
+				authorizationResponseFilter.setAuthorizationResponseBaseUri(
+					authorizationCodeGrantConfigurer.getAuthorizationResponseBaseUri());
+			}
 			if (authorizationCodeGrantConfigurer.getAuthorizationRequestRepository() != null) {
 				authorizationResponseFilter.setAuthorizationRequestRepository(
 					authorizationCodeGrantConfigurer.getAuthorizationRequestRepository());

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

@@ -29,7 +29,6 @@ import org.springframework.security.oauth2.core.endpoint.AuthorizationRequest;
 import org.springframework.security.oauth2.core.endpoint.AuthorizationResponse;
 import org.springframework.security.oauth2.core.endpoint.OAuth2Parameter;
 import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter;
-import org.springframework.security.web.util.matcher.RequestMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
@@ -73,9 +72,8 @@ import java.io.IOException;
  * @see <a target="_blank" href="https://tools.ietf.org/html/rfc6749#section-4.1.2">Section 4.1.2 Authorization Response</a>
  */
 public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticationProcessingFilter {
-	public static final String DEFAULT_AUTHORIZATION_RESPONSE_BASE_URI = "/oauth2/authorize/code";
+	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 AuthorizationResponseMatcher authorizationResponseMatcher;
 	private ClientRegistrationRepository clientRegistrationRepository;
 	private AuthorizationRequestRepository authorizationRequestRepository = new HttpSessionAuthorizationRequestRepository();
 
@@ -84,8 +82,7 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio
 	}
 
 	public AuthorizationCodeAuthenticationFilter(String authorizationResponseBaseUri) {
-		super(new AuthorizationResponseMatcher(authorizationResponseBaseUri));
-		this.authorizationResponseMatcher = new AuthorizationResponseMatcher(authorizationResponseBaseUri);
+		super(authorizationResponseBaseUri);
 	}
 
 	@Override
@@ -131,14 +128,9 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio
 		return this.getAuthenticationManager().authenticate(clientAuthentication);
 	}
 
-	public final RequestMatcher getAuthorizationResponseMatcher() {
-		return this.authorizationResponseMatcher;
-	}
-
 	public final void setAuthorizationResponseBaseUri(String authorizationResponseBaseUri) {
 		Assert.hasText(authorizationResponseBaseUri, "authorizationResponseBaseUri cannot be empty");
-		this.authorizationResponseMatcher = new AuthorizationResponseMatcher(authorizationResponseBaseUri);
-		this.setRequiresAuthenticationRequestMatcher(this.authorizationResponseMatcher);
+		this.setFilterProcessesUrl(authorizationResponseBaseUri);
 	}
 
 	public final void setClientRegistrationRepository(ClientRegistrationRepository clientRegistrationRepository) {
@@ -152,7 +144,7 @@ public class AuthorizationCodeAuthenticationFilter extends AbstractAuthenticatio
 	}
 
 	private AuthorizationResponse convert(HttpServletRequest request) {
-		if (!this.getAuthorizationResponseMatcher().matches(request)) {
+		if (!this.authorizationResponseSuccess(request) && !this.authorizationResponseError(request)) {
 			return null;
 		}
 
@@ -178,28 +170,13 @@ 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 request.getRequestURI().startsWith(this.baseUri) &&
-				(this.successResponse(request) || this.errorResponse(request));
-		}
-
-		private boolean successResponse(HttpServletRequest request) {
-			return StringUtils.hasText(request.getParameter(OAuth2Parameter.CODE)) &&
-				StringUtils.hasText(request.getParameter(OAuth2Parameter.STATE));
-		}
+	private boolean authorizationResponseSuccess(HttpServletRequest request) {
+		return StringUtils.hasText(request.getParameter(OAuth2Parameter.CODE)) &&
+			StringUtils.hasText(request.getParameter(OAuth2Parameter.STATE));
+	}
 
-		private boolean errorResponse(HttpServletRequest request) {
-			return StringUtils.hasText(request.getParameter(OAuth2Parameter.ERROR)) &&
-				StringUtils.hasText(request.getParameter(OAuth2Parameter.STATE));
-		}
+	private boolean authorizationResponseError(HttpServletRequest request) {
+		return StringUtils.hasText(request.getParameter(OAuth2Parameter.ERROR)) &&
+			StringUtils.hasText(request.getParameter(OAuth2Parameter.STATE));
 	}
 }