From 624a8fb252f35ca939966b2c908b89fe5ac06652 Mon Sep 17 00:00:00 2001 From: Max Batischev Date: Thu, 19 Dec 2024 18:17:16 +0300 Subject: [PATCH] Add Alerting About Deprecated Authorize Config Closes gh-16213 --- .../WebSecurityFilterChainValidator.java | 34 +++++++++++++++++++ .../http/DefaultFilterChainValidator.java | 26 ++++++++++++++ .../WebSecurityFilterChainValidatorTests.java | 10 ++++++ .../DefaultFilterChainValidatorTests.java | 6 ++++ 4 files changed, 76 insertions(+) diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java index 5e069a446e..b0bf43fb3b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidator.java @@ -18,10 +18,16 @@ package org.springframework.security.config.annotation.web.builders; 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 @@ import org.springframework.security.web.util.matcher.AnyRequestMatcher; */ final class WebSecurityFilterChainValidator implements FilterChainProxy.FilterChainValidator { + private final Log logger = LogFactory.getLog(getClass()); + @Override public void validate(FilterChainProxy filterChainProxy) { List chains = filterChainProxy.getFilterChains(); checkForAnyRequestRequestMatcher(chains); checkForDuplicateMatchers(chains); + checkAuthorizationFilters(chains); } private void checkForAnyRequestRequestMatcher(List chains) { @@ -76,4 +85,29 @@ final class WebSecurityFilterChainValidator implements FilterChainProxy.FilterCh } } + private void checkAuthorizationFilters(List 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/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java index 74b6fbb7ca..8f2baeb4c6 100644 --- a/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java +++ b/config/src/main/java/org/springframework/security/config/http/DefaultFilterChainValidator.java @@ -69,6 +69,7 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain } checkPathOrder(new ArrayList<>(fcp.getFilterChains())); checkForDuplicateMatchers(new ArrayList<>(fcp.getFilterChains())); + checkAuthorizationFilters(new ArrayList<>(fcp.getFilterChains())); } private void checkPathOrder(List filterChains) { @@ -107,6 +108,31 @@ public class DefaultFilterChainValidator implements FilterChainProxy.FilterChain } } + private void checkAuthorizationFilters(List 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 getFilter(Class type, List filters) { for (Filter f : filters) { diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidatorTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidatorTests.java index edc0be90a6..2e6984ab9b 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidatorTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityFilterChainValidatorTests.java @@ -35,6 +35,7 @@ import org.springframework.security.web.util.matcher.AntPathRequestMatcher; 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/config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java b/config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java index f120b9d058..d75ce815d5 100644 --- a/config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java +++ b/config/src/test/java/org/springframework/security/config/http/DefaultFilterChainValidatorTests.java @@ -49,6 +49,7 @@ 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; @@ -103,6 +104,11 @@ public class DefaultFilterChainValidatorTests { ReflectionTestUtils.setField(this.validator, "logger", this.logger); } + @Test + void validateWhenFilterSecurityInterceptorConfiguredThenValidates() { + assertThatNoException().isThrownBy(() -> this.validator.validate(this.chain)); + } + // SEC-1878 @SuppressWarnings("unchecked") @Test