Add Alerting About Deprecated Authorize Config

Closes spring-projectsgh-16213
franticticktick · Dec 19, 2024 · 6088267 · 6088267
1 parent 6d4bd07
commit 6088267
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 0 deletions.
diff --git a/...ingframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java b/...ingframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java
@@ -18,10 +18,16 @@
 
 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;
 
 /**
@@ -33,11 +39,14 @@
  */
 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) {
@@ -76,4 +85,29 @@ private void checkForDuplicateMatchers(List<SecurityFilterChain> chains) {
 		}
 	}
 
+	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;
+		}
+	}
+
 }
diff --git a/...g/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java b/...g/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java
@@ -69,6 +69,7 @@ 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) {
@@ -107,6 +108,31 @@ private void checkForDuplicateMatchers(List<SecurityFilterChain> chains) {
 		}
 	}
 
+	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;
+		}
+	}
+
 	@SuppressWarnings({ "unchecked" })
 	private <F extends Filter> F getFilter(Class<F> type, List<Filter> filters) {
 		for (Filter f : filters) {

diff --git a/...amework/security/config/annotation/web/builders/WebSecurityFilterChainValidatorTests.java b/...amework/security/config/annotation/web/builders/WebSecurityFilterChainValidatorTests.java
@@ -35,6 +35,7 @@
 import org.springframework.security.web.util.matcher.AnyRequestMatcher;
 
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 
 /**
  * Tests for {@link WebSecurityFilterChainValidator}
@@ -55,6 +56,15 @@ public class WebSecurityFilterChainValidatorTests {
 	@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"),

diff --git a/.../test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java b/.../test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java
@@ -49,6 +49,7 @@
 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;
@@ -103,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