Add support checking same security matchers

config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java

@@ -18,28 +18,45 @@
 
 import java.util.List;
 
+import jakarta.servlet.Filter;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 import org.springframework.security.web.DefaultSecurityFilterChain;
 import org.springframework.security.web.FilterChainProxy;
 import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.UnreachableFilterChainException;
+import org.springframework.security.web.access.intercept.AuthorizationFilter;
+import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
 import org.springframework.security.web.util.matcher.AnyRequestMatcher;
 
 /**
  * A filter chain validator for filter chains built by {@link WebSecurity}
  *
+ * @author Josh Cummings
+ * @author Max Batischev
  * @since 6.5
  */
 final class WebSecurityFilterChainValidator implements FilterChainProxy.FilterChainValidator {
 
+	private final Log logger = LogFactory.getLog(getClass());
+
 	@Override
 	public void validate(FilterChainProxy filterChainProxy) {
 		List<SecurityFilterChain> chains = filterChainProxy.getFilterChains();
+		checkForAnyRequestRequestMatcher(chains);
+		checkForDuplicateMatchers(chains);
+		checkAuthorizationFilters(chains);
+	}
+
+	private void checkForAnyRequestRequestMatcher(List<SecurityFilterChain> chains) {
 		DefaultSecurityFilterChain anyRequestFilterChain = null;
 		for (SecurityFilterChain chain : chains) {
 			if (anyRequestFilterChain != null) {
 				String message = "A filter chain that matches any request [" + anyRequestFilterChain
 						+ "] has already been configured, which means that this filter chain [" + chain
 						+ "] will never get invoked. Please use `HttpSecurity#securityMatcher` to ensure that there is only one filter chain configured for 'any request' and that the 'any request' filter chain is published last.";
-				throw new IllegalArgumentException(message);
+				throw new UnreachableFilterChainException(message, anyRequestFilterChain, chain);
 			}
 			if (chain instanceof DefaultSecurityFilterChain defaultChain) {
 				if (defaultChain.getRequestMatcher() instanceof AnyRequestMatcher) {
@@ -49,4 +66,48 @@ public void validate(FilterChainProxy filterChainProxy) {
 		}
 	}
 
+	private void checkForDuplicateMatchers(List<SecurityFilterChain> chains) {
+		DefaultSecurityFilterChain filterChain = null;
+		for (SecurityFilterChain chain : chains) {
+			if (filterChain != null) {
+				if (chain instanceof DefaultSecurityFilterChain defaultChain) {
+					if (defaultChain.getRequestMatcher().equals(filterChain.getRequestMatcher())) {
+						throw new UnreachableFilterChainException(
+								"The FilterChainProxy contains two filter chains using the" + " matcher "
+										+ defaultChain.getRequestMatcher(),
+								filterChain, defaultChain);
+					}
+				}
+			}
+			if (chain instanceof DefaultSecurityFilterChain defaultChain) {
+				filterChain = defaultChain;
+			}
+		}
+	}
+
+	private void checkAuthorizationFilters(List<SecurityFilterChain> chains) {
+		Filter authorizationFilter = null;
+		Filter filterSecurityInterceptor = null;
+		for (SecurityFilterChain chain : chains) {
+			for (Filter filter : chain.getFilters()) {
+				if (filter instanceof AuthorizationFilter) {
+					authorizationFilter = filter;
+				}
+				if (filter instanceof FilterSecurityInterceptor) {
+					filterSecurityInterceptor = filter;
+				}
+			}
+			if (authorizationFilter != null && filterSecurityInterceptor != null) {
+				this.logger.warn(
+						"It is not recommended to use authorizeRequests in the configuration. Please only use authorizeHttpRequests");
+			}
+			if (filterSecurityInterceptor != null) {
+				this.logger.warn(
+						"Usage of authorizeRequests is deprecated. Please use authorizeHttpRequests in the configuration");
+			}
+			authorizationFilter = null;
+			filterSecurityInterceptor = null;
+		}
+	}
+
 }

config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java

@@ -39,6 +39,7 @@
 import org.springframework.security.web.FilterChainProxy;
 import org.springframework.security.web.FilterInvocation;
 import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.UnreachableFilterChainException;
 import org.springframework.security.web.access.ExceptionTranslationFilter;
 import org.springframework.security.web.access.intercept.AuthorizationFilter;
 import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
@@ -53,7 +54,6 @@
 import org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter;
 import org.springframework.security.web.session.SessionManagementFilter;
 import org.springframework.security.web.util.matcher.AnyRequestMatcher;
-import org.springframework.security.web.util.matcher.RequestMatcher;
 
 public class DefaultFilterChainValidator implements FilterChainProxy.FilterChainValidator {
 
@@ -69,31 +69,67 @@ public void validate(FilterChainProxy fcp) {
 		}
 		checkPathOrder(new ArrayList<>(fcp.getFilterChains()));
 		checkForDuplicateMatchers(new ArrayList<>(fcp.getFilterChains()));
+		checkAuthorizationFilters(new ArrayList<>(fcp.getFilterChains()));
 	}
 
 	private void checkPathOrder(List<SecurityFilterChain> filterChains) {
 		// Check that the universal pattern is listed at the end, if at all
 		Iterator<SecurityFilterChain> chains = filterChains.iterator();
 		while (chains.hasNext()) {
-			RequestMatcher matcher = ((DefaultSecurityFilterChain) chains.next()).getRequestMatcher();
-			if (AnyRequestMatcher.INSTANCE.equals(matcher) && chains.hasNext()) {
-				throw new IllegalArgumentException("A universal match pattern ('/**') is defined "
-						+ " before other patterns in the filter chain, causing them to be ignored. Please check the "
-						+ "ordering in your <security:http> namespace or FilterChainProxy bean configuration");
+			if (chains.next() instanceof DefaultSecurityFilterChain securityFilterChain) {
+				if (AnyRequestMatcher.INSTANCE.equals(securityFilterChain.getRequestMatcher()) && chains.hasNext()) {
+					throw new UnreachableFilterChainException("A universal match pattern ('/**') is defined "
+							+ " before other patterns in the filter chain, causing them to be ignored. Please check the "
+							+ "ordering in your <security:http> namespace or FilterChainProxy bean configuration",
+							securityFilterChain, chains.next());
+				}
 			}
 		}
 	}
 
 	private void checkForDuplicateMatchers(List<SecurityFilterChain> chains) {
-		while (chains.size() > 1) {
-			DefaultSecurityFilterChain chain = (DefaultSecurityFilterChain) chains.remove(0);
-			for (SecurityFilterChain test : chains) {
-				if (chain.getRequestMatcher().equals(((DefaultSecurityFilterChain) test).getRequestMatcher())) {
-					throw new IllegalArgumentException("The FilterChainProxy contains two filter chains using the"
-							+ " matcher " + chain.getRequestMatcher() + ". If you are using multiple <http> namespace "
-							+ "elements, you must use a 'pattern' attribute to define the request patterns to which they apply.");
+		DefaultSecurityFilterChain filterChain = null;
+		for (SecurityFilterChain chain : chains) {
+			if (filterChain != null) {
+				if (chain instanceof DefaultSecurityFilterChain defaultChain) {
+					if (defaultChain.getRequestMatcher().equals(filterChain.getRequestMatcher())) {
+						throw new UnreachableFilterChainException(
+								"The FilterChainProxy contains two filter chains using the" + " matcher "
+										+ defaultChain.getRequestMatcher()
+										+ ". If you are using multiple <http> namespace "
+										+ "elements, you must use a 'pattern' attribute to define the request patterns to which they apply.",
+								defaultChain, chain);
+					}
 				}
 			}
+			if (chain instanceof DefaultSecurityFilterChain defaultChain) {
+				filterChain = defaultChain;
+			}
+		}
+	}
+
+	private void checkAuthorizationFilters(List<SecurityFilterChain> chains) {
+		Filter authorizationFilter = null;
+		Filter filterSecurityInterceptor = null;
+		for (SecurityFilterChain chain : chains) {
+			for (Filter filter : chain.getFilters()) {
+				if (filter instanceof AuthorizationFilter) {
+					authorizationFilter = filter;
+				}
+				if (filter instanceof FilterSecurityInterceptor) {
+					filterSecurityInterceptor = filter;
+				}
+			}
+			if (authorizationFilter != null && filterSecurityInterceptor != null) {
+				this.logger.warn(
+						"It is not recommended to use authorizeRequests in the configuration. Please only use authorizeHttpRequests");
+			}
+			if (filterSecurityInterceptor != null) {
+				this.logger.warn(
+						"Usage of authorizeRequests is deprecated. Please use authorizeHttpRequests in the configuration");
+			}
+			authorizationFilter = null;
+			filterSecurityInterceptor = null;
 		}
 	}
 

config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidatorTests.java

@@ -0,0 +1,119 @@
+/*
+ * Copyright 2002-2024 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.security.config.annotation.web.builders;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import org.springframework.security.web.DefaultSecurityFilterChain;
+import org.springframework.security.web.FilterChainProxy;
+import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.UnreachableFilterChainException;
+import org.springframework.security.web.access.ExceptionTranslationFilter;
+import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
+import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
+import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
+import org.springframework.security.web.util.matcher.AnyRequestMatcher;
+import org.springframework.security.web.util.matcher.RequestMatcher;
+import org.springframework.security.web.util.matcher.RequestMatchers;
+
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+
+/**
+ * Tests for {@link WebSecurityFilterChainValidator}
+ *
+ * @author Max Batischev
+ */
+@ExtendWith(MockitoExtension.class)
+public class WebSecurityFilterChainValidatorTests {
+
+	private final WebSecurityFilterChainValidator validator = new WebSecurityFilterChainValidator();
+
+	@Mock
+	private AnonymousAuthenticationFilter authenticationFilter;
+
+	@Mock
+	private ExceptionTranslationFilter exceptionTranslationFilter;
+
+	@Mock
+	private FilterSecurityInterceptor authorizationInterceptor;
+
+	@Test
+	void validateWhenFilterSecurityInterceptorConfiguredThenValidates() {
+		SecurityFilterChain chain = new DefaultSecurityFilterChain(AntPathRequestMatcher.antMatcher("/api"),
+				this.authenticationFilter, this.exceptionTranslationFilter, this.authorizationInterceptor);
+		FilterChainProxy proxy = new FilterChainProxy(List.of(chain));
+
+		assertThatNoException().isThrownBy(() -> this.validator.validate(proxy));
+	}
+
+	@Test
+	void validateWhenAnyRequestMatcherIsPresentThenUnreachableFilterChainException() {
+		SecurityFilterChain chain1 = new DefaultSecurityFilterChain(AntPathRequestMatcher.antMatcher("/api"),
+				this.authenticationFilter, this.exceptionTranslationFilter, this.authorizationInterceptor);
+		SecurityFilterChain chain2 = new DefaultSecurityFilterChain(AnyRequestMatcher.INSTANCE,
+				this.authenticationFilter, this.exceptionTranslationFilter, this.authorizationInterceptor);
+		List<SecurityFilterChain> chains = new ArrayList<>();
+		chains.add(chain2);
+		chains.add(chain1);
+		FilterChainProxy proxy = new FilterChainProxy(chains);
+
+		assertThatExceptionOfType(UnreachableFilterChainException.class)
+			.isThrownBy(() -> this.validator.validate(proxy));
+	}
+
+	@Test
+	void validateWhenSameRequestMatchersArePresentThenUnreachableFilterChainException() {
+		SecurityFilterChain chain1 = new DefaultSecurityFilterChain(AntPathRequestMatcher.antMatcher("/api"),
+				this.authenticationFilter, this.exceptionTranslationFilter, this.authorizationInterceptor);
+		SecurityFilterChain chain2 = new DefaultSecurityFilterChain(AntPathRequestMatcher.antMatcher("/api"),
+				this.authenticationFilter, this.exceptionTranslationFilter, this.authorizationInterceptor);
+		List<SecurityFilterChain> chains = new ArrayList<>();
+		chains.add(chain2);
+		chains.add(chain1);
+		FilterChainProxy proxy = new FilterChainProxy(chains);
+
+		assertThatExceptionOfType(UnreachableFilterChainException.class)
+			.isThrownBy(() -> this.validator.validate(proxy));
+	}
+
+	@Test
+	void validateWhenSameComposedRequestMatchersArePresentThenUnreachableFilterChainException() {
+		RequestMatcher matcher1 = RequestMatchers.anyOf(RequestMatchers.allOf(AntPathRequestMatcher.antMatcher("/api"),
+				AntPathRequestMatcher.antMatcher("*.do")), AntPathRequestMatcher.antMatcher("/admin"));
+		RequestMatcher matcher2 = RequestMatchers.anyOf(RequestMatchers.allOf(AntPathRequestMatcher.antMatcher("/api"),
+				AntPathRequestMatcher.antMatcher("*.do")), AntPathRequestMatcher.antMatcher("/admin"));
+		SecurityFilterChain chain1 = new DefaultSecurityFilterChain(matcher1, this.authenticationFilter,
+				this.exceptionTranslationFilter, this.authorizationInterceptor);
+		SecurityFilterChain chain2 = new DefaultSecurityFilterChain(matcher2, this.authenticationFilter,
+				this.exceptionTranslationFilter, this.authorizationInterceptor);
+		List<SecurityFilterChain> chains = new ArrayList<>();
+		chains.add(chain2);
+		chains.add(chain1);
+		FilterChainProxy proxy = new FilterChainProxy(chains);
+
+		assertThatExceptionOfType(UnreachableFilterChainException.class)
+			.isThrownBy(() -> this.validator.validate(proxy));
+	}
+
+}

config/src/test/java/org/springframework/security/config/annotation/web/configuration/WebSecurityConfigurationTests.java

@@ -323,7 +323,7 @@ public void loadConfigWhenTwoSecurityFilterChainsPresentAndSecondWithAnyRequestT
 		assertThatExceptionOfType(BeanCreationException.class)
 			.isThrownBy(() -> this.spring.register(MultipleAnyRequestSecurityFilterChainConfig.class).autowire())
 			.havingRootCause()
-			.isExactlyInstanceOf(IllegalArgumentException.class);
+			.isInstanceOf(IllegalArgumentException.class);
 	}
 
 	private void assertAnotherUserPermission(WebInvocationPrivilegeEvaluator privilegeEvaluator) {

config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java

@@ -16,7 +16,9 @@
 
 package org.springframework.security.config.http;
 
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 
 import jakarta.servlet.http.HttpServletRequest;
 import org.apache.commons.logging.Log;
@@ -33,16 +35,21 @@
 import org.springframework.security.web.AuthenticationEntryPoint;
 import org.springframework.security.web.DefaultSecurityFilterChain;
 import org.springframework.security.web.FilterChainProxy;
+import org.springframework.security.web.SecurityFilterChain;
+import org.springframework.security.web.UnreachableFilterChainException;
 import org.springframework.security.web.access.ExceptionTranslationFilter;
 import org.springframework.security.web.access.intercept.AuthorizationFilter;
 import org.springframework.security.web.access.intercept.DefaultFilterInvocationSecurityMetadataSource;
 import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
 import org.springframework.security.web.access.intercept.FilterSecurityInterceptor;
 import org.springframework.security.web.authentication.AnonymousAuthenticationFilter;
 import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint;
+import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
 import org.springframework.security.web.util.matcher.AnyRequestMatcher;
 import org.springframework.test.util.ReflectionTestUtils;
 
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.BDDMockito.willThrow;
@@ -97,6 +104,11 @@ public void setUp() {
 		ReflectionTestUtils.setField(this.validator, "logger", this.logger);
 	}
 
+	@Test
+	void validateWhenFilterSecurityInterceptorConfiguredThenValidates() {
+		assertThatNoException().isThrownBy(() -> this.validator.validate(this.chain));
+	}
+
 	// SEC-1878
 	@SuppressWarnings("unchecked")
 	@Test
@@ -130,4 +142,21 @@ public void validateCustomMetadataSource() {
 		verify(customMetaDataSource, atLeastOnce()).getAttributes(any());
 	}
 
+	@Test
+	void validateWhenSameRequestMatchersArePresentThenUnreachableFilterChainException() {
+		AnonymousAuthenticationFilter authenticationFilter = mock(AnonymousAuthenticationFilter.class);
+		ExceptionTranslationFilter exceptionTranslationFilter = mock(ExceptionTranslationFilter.class);
+		SecurityFilterChain chain1 = new DefaultSecurityFilterChain(AntPathRequestMatcher.antMatcher("/api"),
+				authenticationFilter, exceptionTranslationFilter, this.authorizationInterceptor);
+		SecurityFilterChain chain2 = new DefaultSecurityFilterChain(AntPathRequestMatcher.antMatcher("/api"),
+				authenticationFilter, exceptionTranslationFilter, this.authorizationInterceptor);
+		List<SecurityFilterChain> chains = new ArrayList<>();
+		chains.add(chain2);
+		chains.add(chain1);
+		FilterChainProxy proxy = new FilterChainProxy(chains);
+
+		assertThatExceptionOfType(UnreachableFilterChainException.class)
+			.isThrownBy(() -> this.validator.validate(proxy));
+	}
+
 }