From 4bd3534b7d91f922ad903a75beb19b6bdca39e5c Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 13 Jul 2022 08:31:56 +0100 Subject: [PATCH] Adapt to change in Security's filtering behavior Spring Security now filters every dispatch by default and not only once-per-request. Security configuration has been updated in a number of places to restore the old behavior as needed for the tests to pass. gh-31703 has been opened to review this and to investigate if we can now remove the error page security filter and rely on the filtering of every dispatch instead. In addition to switching to once-per-request filtering where needed, this commit also restructures the configuration of the error page security filter. The restructuring was necessary to ensure that the privilege evaluator bean has been defined before the conditions on the error page security filter are evaluated. Without the change, the filter was no longer being configured as the privilege evaluator hadn't been defined before the on bean condition was evaluated. We may want to back port this change as the ordering doesn't appear to have been defined before and we were just getting lucky. See gh-31622 See spring-projects/spring-security#11466 --- .../ErrorPageSecurityFilterConfiguration.java | 51 +++++++++++++++++++ .../servlet/SecurityAutoConfiguration.java | 3 +- .../SpringBootWebSecurityConfiguration.java | 25 --------- .../customsecurity/SecurityConfiguration.java | 1 + .../SampleMethodSecurityApplication.java | 6 ++- .../CustomServletPathErrorPageTests.java | 1 + .../smoketest/web/secure/ErrorPageTests.java | 1 + .../web/secure/NoSessionErrorPageTests.java | 1 + 8 files changed, 61 insertions(+), 28 deletions(-) create mode 100644 spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/ErrorPageSecurityFilterConfiguration.java diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/ErrorPageSecurityFilterConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/ErrorPageSecurityFilterConfiguration.java new file mode 100644 index 0000000000..c86c47e81e --- /dev/null +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/ErrorPageSecurityFilterConfiguration.java @@ -0,0 +1,51 @@ +/* + * Copyright 2012-2022 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.boot.autoconfigure.security.servlet; + +import jakarta.servlet.DispatcherType; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; +import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type; +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator; + +/** + * Configures the {@link ErrorPageSecurityFilter}. + * + * @author Madhura Bhave + */ +@Configuration(proxyBeanMethods = false) +@ConditionalOnClass(WebInvocationPrivilegeEvaluator.class) +@ConditionalOnBean(WebInvocationPrivilegeEvaluator.class) +@ConditionalOnWebApplication(type = Type.SERVLET) +class ErrorPageSecurityFilterConfiguration { + + @Bean + FilterRegistrationBean errorPageSecurityFilter(ApplicationContext context) { + FilterRegistrationBean registration = new FilterRegistrationBean<>( + new ErrorPageSecurityFilter(context)); + registration.setDispatcherTypes(DispatcherType.ERROR); + return registration; + } + +} diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfiguration.java index d5490ebeb1..313bf58985 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfiguration.java @@ -40,7 +40,8 @@ import org.springframework.security.authentication.DefaultAuthenticationEventPub @AutoConfiguration @ConditionalOnClass(DefaultAuthenticationEventPublisher.class) @EnableConfigurationProperties(SecurityProperties.class) -@Import({ SpringBootWebSecurityConfiguration.class, SecurityDataConfiguration.class }) +@Import({ SpringBootWebSecurityConfiguration.class, ErrorPageSecurityFilterConfiguration.class, + SecurityDataConfiguration.class }) public class SecurityAutoConfiguration { @Bean diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SpringBootWebSecurityConfiguration.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SpringBootWebSecurityConfiguration.java index 710510c957..ad7750bec5 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SpringBootWebSecurityConfiguration.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SpringBootWebSecurityConfiguration.java @@ -16,18 +16,12 @@ package org.springframework.boot.autoconfigure.security.servlet; -import jakarta.servlet.DispatcherType; - -import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication.Type; import org.springframework.boot.autoconfigure.security.ConditionalOnDefaultWebSecurity; import org.springframework.boot.autoconfigure.security.SecurityProperties; -import org.springframework.boot.web.servlet.FilterRegistrationBean; -import org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter; -import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.annotation.Order; @@ -35,7 +29,6 @@ import org.springframework.security.config.BeanIds; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.web.SecurityFilterChain; -import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator; /** * {@link Configuration @Configuration} class securing servlet applications. @@ -69,24 +62,6 @@ class SpringBootWebSecurityConfiguration { } - /** - * Configures the {@link ErrorPageSecurityFilter}. - */ - @Configuration(proxyBeanMethods = false) - @ConditionalOnClass(WebInvocationPrivilegeEvaluator.class) - @ConditionalOnBean(WebInvocationPrivilegeEvaluator.class) - static class ErrorPageSecurityFilterConfiguration { - - @Bean - FilterRegistrationBean errorPageSecurityFilter(ApplicationContext context) { - FilterRegistrationBean registration = new FilterRegistrationBean<>( - new ErrorPageSecurityFilter(context)); - registration.setDispatcherTypes(DispatcherType.ERROR); - return registration; - } - - } - /** * Adds the {@link EnableWebSecurity @EnableWebSecurity} annotation if Spring Security * is on the classpath. This will make sure that the annotation is present with diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-custom-security/src/main/java/smoketest/actuator/customsecurity/SecurityConfiguration.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-custom-security/src/main/java/smoketest/actuator/customsecurity/SecurityConfiguration.java index 42a9396683..11276948d4 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-custom-security/src/main/java/smoketest/actuator/customsecurity/SecurityConfiguration.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-custom-security/src/main/java/smoketest/actuator/customsecurity/SecurityConfiguration.java @@ -62,6 +62,7 @@ public class SecurityConfiguration { .hasRole("ACTUATOR"); requests.requestMatchers(PathRequest.toStaticResources().atCommonLocations()).permitAll(); requests.antMatchers("/foo").permitAll(); + requests.antMatchers("/error").permitAll(); requests.antMatchers("/**").hasRole("USER"); }); http.cors(Customizer.withDefaults()); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-method-security/src/main/java/smoketest/security/method/SampleMethodSecurityApplication.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-method-security/src/main/java/smoketest/security/method/SampleMethodSecurityApplication.java index 2f77338b65..462e359e81 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-method-security/src/main/java/smoketest/security/method/SampleMethodSecurityApplication.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-method-security/src/main/java/smoketest/security/method/SampleMethodSecurityApplication.java @@ -69,7 +69,8 @@ public class SampleMethodSecurityApplication implements WebMvcConfigurer { @Bean SecurityFilterChain configure(HttpSecurity http) throws Exception { http.csrf().disable(); - http.authorizeRequests((requests) -> requests.anyRequest().fullyAuthenticated()); + http.authorizeRequests((requests) -> requests.anyRequest().fullyAuthenticated() + .filterSecurityInterceptorOncePerRequest(true)); http.formLogin((form) -> form.loginPage("/login").permitAll()); http.exceptionHandling((exceptions) -> exceptions.accessDeniedPage("/access")); return http.build(); @@ -85,7 +86,8 @@ public class SampleMethodSecurityApplication implements WebMvcConfigurer { SecurityFilterChain actuatorSecurity(HttpSecurity http) throws Exception { http.csrf().disable(); http.requestMatcher(EndpointRequest.toAnyEndpoint()); - http.authorizeRequests((requests) -> requests.anyRequest().authenticated()); + http.authorizeRequests( + (requests) -> requests.anyRequest().authenticated().filterSecurityInterceptorOncePerRequest(true)); http.httpBasic(); return http.build(); } diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/CustomServletPathErrorPageTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/CustomServletPathErrorPageTests.java index 142d59d69f..e642bc05f8 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/CustomServletPathErrorPageTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/CustomServletPathErrorPageTests.java @@ -47,6 +47,7 @@ class CustomServletPathErrorPageTests extends AbstractErrorPageTests { http.authorizeRequests((requests) -> { requests.antMatchers("/custom/servlet/path/public/**").permitAll(); requests.anyRequest().fullyAuthenticated(); + requests.filterSecurityInterceptorOncePerRequest(true); }); http.httpBasic(); http.formLogin((form) -> form.loginPage("/custom/servlet/path/login").permitAll()); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/ErrorPageTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/ErrorPageTests.java index 99c10a4d42..de2fe2a2fd 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/ErrorPageTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/ErrorPageTests.java @@ -46,6 +46,7 @@ class ErrorPageTests extends AbstractErrorPageTests { http.authorizeRequests((requests) -> { requests.antMatchers("/public/**").permitAll(); requests.anyRequest().fullyAuthenticated(); + requests.filterSecurityInterceptorOncePerRequest(true); }); http.httpBasic(); http.formLogin((form) -> form.loginPage("/login").permitAll()); diff --git a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/NoSessionErrorPageTests.java b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/NoSessionErrorPageTests.java index 0e9ad9043c..1ae28b04bf 100644 --- a/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/NoSessionErrorPageTests.java +++ b/spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/NoSessionErrorPageTests.java @@ -48,6 +48,7 @@ class NoSessionErrorPageTests extends AbstractErrorPageTests { .authorizeRequests((requests) -> { requests.antMatchers("/public/**").permitAll(); requests.anyRequest().authenticated(); + requests.filterSecurityInterceptorOncePerRequest(true); }); http.httpBasic(); return http.build();