Remove error page security filter

Spring Security now re-applies the authorization rules
to the error page by default. Additionally, it configures
RequestAttributeSecurityContextRepository as the default for
stateless applications allowing those applications to have access
to the original authentication during an error dispatch.

Closes gh-31703
This commit is contained in:
Madhura Bhave 2022-10-18 21:17:21 -07:00
parent 8621be6bba
commit cedd553b83
10 changed files with 1 additions and 376 deletions

View File

@ -1,51 +0,0 @@
/*
* 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> errorPageSecurityFilter(ApplicationContext context) {
FilterRegistrationBean<ErrorPageSecurityFilter> registration = new FilterRegistrationBean<>(
new ErrorPageSecurityFilter(context));
registration.setDispatcherTypes(DispatcherType.ERROR);
return registration;
}
}

View File

@ -40,8 +40,7 @@ import org.springframework.security.authentication.DefaultAuthenticationEventPub
@AutoConfiguration
@ConditionalOnClass(DefaultAuthenticationEventPublisher.class)
@EnableConfigurationProperties(SecurityProperties.class)
@Import({ SpringBootWebSecurityConfiguration.class, ErrorPageSecurityFilterConfiguration.class,
SecurityDataConfiguration.class })
@Import({ SpringBootWebSecurityConfiguration.class, SecurityDataConfiguration.class })
public class SecurityAutoConfiguration {
@Bean

View File

@ -17,7 +17,6 @@
package org.springframework.boot.autoconfigure.security.servlet;
import java.security.interfaces.RSAPublicKey;
import java.util.EnumSet;
import jakarta.servlet.DispatcherType;
import org.assertj.core.api.InstanceOfAssertFactories;
@ -36,14 +35,11 @@ import org.springframework.boot.convert.ApplicationConversionService;
import org.springframework.boot.test.context.FilteredClassLoader;
import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
import org.springframework.boot.web.servlet.DelegatingFilterProxyRegistrationBean;
import org.springframework.boot.web.servlet.FilterRegistrationBean;
import org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter;
import org.springframework.boot.web.servlet.filter.OrderedFilter;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.convert.converter.Converter;
import org.springframework.mock.web.MockServletContext;
import org.springframework.orm.jpa.JpaTransactionManager;
import org.springframework.security.authentication.AuthenticationEventPublisher;
import org.springframework.security.authentication.DefaultAuthenticationEventPublisher;
@ -55,7 +51,6 @@ import org.springframework.security.core.AuthenticationException;
import org.springframework.security.data.repository.query.SecurityEvaluationContextExtension;
import org.springframework.security.web.FilterChainProxy;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.test.util.ReflectionTestUtils;
import static org.assertj.core.api.Assertions.assertThat;
@ -230,31 +225,6 @@ class SecurityAutoConfigurationTests {
.run((context) -> assertThat(context.getBean(JwtProperties.class).getPublicKey()).isNotNull());
}
@Test
@SuppressWarnings("unchecked")
void filterRegistrationBeanForErrorPageSecurityInterceptor() {
this.contextRunner.withInitializer((context) -> context.setServletContext(new MockServletContext()))
.run(((context) -> {
FilterRegistrationBean<?> bean = context.getBean(FilterRegistrationBean.class);
assertThat(bean.getFilter()).isInstanceOf(ErrorPageSecurityFilter.class);
EnumSet<DispatcherType> dispatcherTypes = (EnumSet<DispatcherType>) ReflectionTestUtils
.getField(bean, "dispatcherTypes");
assertThat(dispatcherTypes).containsExactly(DispatcherType.ERROR);
}));
}
@Test
void filterForErrorPageSecurityInterceptorWhenWebInvocationPrivilegeEvaluatorNotPresent() {
this.contextRunner.withClassLoader(new FilteredClassLoader("org.springframework.security.config"))
.run((context) -> assertThat(context).doesNotHaveBean("errorPageSecurityFilter"));
}
@Test
void filterForErrorPageSecurityInterceptorConditionalOnClass() {
this.contextRunner.withClassLoader(new FilteredClassLoader("org.springframework.security.web"))
.run((context) -> assertThat(context).doesNotHaveBean("errorPageSecurityFilter"));
}
@Configuration(proxyBeanMethods = false)
@TestAutoConfigurationPackage(City.class)
static class EntityConfiguration {

View File

@ -33,7 +33,6 @@ import org.springframework.boot.test.util.TestPropertyValues;
import org.springframework.boot.testsupport.web.servlet.MockServletWebServer.RegisteredFilter;
import org.springframework.boot.web.server.WebServerFactoryCustomizerBeanPostProcessor;
import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext;
import org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter;
import org.springframework.boot.web.servlet.filter.OrderedCharacterEncodingFilter;
import org.springframework.boot.web.servlet.filter.OrderedRequestContextFilter;
import org.springframework.context.annotation.Bean;
@ -81,7 +80,6 @@ class FilterOrderingIntegrationTests {
assertThat(iterator.next()).isInstanceOf(Filter.class);
assertThat(iterator.next()).isInstanceOf(Filter.class);
assertThat(iterator.next()).isInstanceOf(OrderedRequestContextFilter.class);
assertThat(iterator.next()).isInstanceOf(ErrorPageSecurityFilter.class);
assertThat(iterator.next()).isInstanceOf(FilterChainProxy.class);
}

View File

@ -1,133 +0,0 @@
/*
* 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.web.servlet.filter;
import java.io.IOException;
import jakarta.servlet.DispatcherType;
import jakarta.servlet.Filter;
import jakarta.servlet.FilterChain;
import jakarta.servlet.RequestDispatcher;
import jakarta.servlet.ServletException;
import jakarta.servlet.ServletRequest;
import jakarta.servlet.ServletResponse;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.context.ApplicationContext;
import org.springframework.security.authentication.AnonymousAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
import org.springframework.web.util.UrlPathHelper;
/**
* {@link Filter} that intercepts error dispatches to ensure authorized access to the
* error page.
*
* @author Madhura Bhave
* @author Andy Wilkinson
* @since 2.6.0
*/
public class ErrorPageSecurityFilter implements Filter {
private static final WebInvocationPrivilegeEvaluator ALWAYS = new AlwaysAllowWebInvocationPrivilegeEvaluator();
private final UrlPathHelper urlPathHelper = new UrlPathHelper();
private final ApplicationContext context;
private volatile WebInvocationPrivilegeEvaluator privilegeEvaluator;
public ErrorPageSecurityFilter(ApplicationContext context) {
this.context = context;
this.urlPathHelper.setAlwaysUseFullPath(true);
}
@Override
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
throws IOException, ServletException {
doFilter((HttpServletRequest) request, (HttpServletResponse) response, chain);
}
private void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
throws IOException, ServletException {
Integer errorCode = (Integer) request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
if (DispatcherType.ERROR.equals(request.getDispatcherType()) && !isAllowed(request, errorCode)) {
response.sendError((errorCode != null) ? errorCode : 401);
return;
}
chain.doFilter(request, response);
}
private boolean isAllowed(HttpServletRequest request, Integer errorCode) {
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
if (isUnauthenticated(authentication) && isNotAuthenticationError(errorCode)) {
return true;
}
return getPrivilegeEvaluator().isAllowed(this.urlPathHelper.getPathWithinApplication(request), authentication);
}
private boolean isUnauthenticated(Authentication authentication) {
return (authentication == null || authentication instanceof AnonymousAuthenticationToken);
}
private boolean isNotAuthenticationError(Integer errorCode) {
return (errorCode == null || (errorCode != 401 && errorCode != 403));
}
private WebInvocationPrivilegeEvaluator getPrivilegeEvaluator() {
WebInvocationPrivilegeEvaluator privilegeEvaluator = this.privilegeEvaluator;
if (privilegeEvaluator == null) {
privilegeEvaluator = getPrivilegeEvaluatorBean();
this.privilegeEvaluator = privilegeEvaluator;
}
return privilegeEvaluator;
}
private WebInvocationPrivilegeEvaluator getPrivilegeEvaluatorBean() {
try {
return this.context.getBean(WebInvocationPrivilegeEvaluator.class);
}
catch (NoSuchBeanDefinitionException ex) {
return ALWAYS;
}
}
@Override
public void destroy() {
}
/**
* {@link WebInvocationPrivilegeEvaluator} that always allows access.
*/
private static class AlwaysAllowWebInvocationPrivilegeEvaluator implements WebInvocationPrivilegeEvaluator {
@Override
public boolean isAllowed(String uri, Authentication authentication) {
return true;
}
@Override
public boolean isAllowed(String contextPath, String uri, String method, Authentication authentication) {
return true;
}
}
}

View File

@ -1,146 +0,0 @@
/*
* 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.web.servlet.filter;
import jakarta.servlet.DispatcherType;
import jakarta.servlet.FilterChain;
import jakarta.servlet.RequestDispatcher;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.context.ApplicationContext;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.then;
import static org.mockito.BDDMockito.willThrow;
import static org.mockito.Mockito.mock;
/**
* Tests for {@link ErrorPageSecurityFilter}.
*
* @author Madhura Bhave
*/
class ErrorPageSecurityFilterTests {
private final WebInvocationPrivilegeEvaluator privilegeEvaluator = mock(WebInvocationPrivilegeEvaluator.class);
private final ApplicationContext context = mock(ApplicationContext.class);
private final MockHttpServletRequest request = new MockHttpServletRequest();
private final MockHttpServletResponse response = new MockHttpServletResponse();
private final FilterChain filterChain = mock(FilterChain.class);
private ErrorPageSecurityFilter securityFilter;
@BeforeEach
void setup() {
this.request.setDispatcherType(DispatcherType.ERROR);
given(this.context.getBean(WebInvocationPrivilegeEvaluator.class)).willReturn(this.privilegeEvaluator);
this.securityFilter = new ErrorPageSecurityFilter(this.context);
}
@AfterEach
void tearDown() {
SecurityContextHolder.clearContext();
}
@Test
void whenAccessIsAllowedShouldContinueDownFilterChain() throws Exception {
given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(true);
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
then(this.filterChain).should().doFilter(this.request, this.response);
}
@Test
void whenAccessIsDeniedShouldCallSendError() throws Exception {
given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(false);
this.request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, 403);
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
then(this.filterChain).shouldHaveNoInteractions();
assertThat(this.response.getStatus()).isEqualTo(403);
}
@Test
void whenAccessIsDeniedAndNoErrorCodeAttributeOnRequest() throws Exception {
given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(false);
SecurityContext securityContext = mock(SecurityContext.class);
SecurityContextHolder.setContext(securityContext);
given(securityContext.getAuthentication()).willReturn(mock(Authentication.class));
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
then(this.filterChain).shouldHaveNoInteractions();
assertThat(this.response.getStatus()).isEqualTo(401);
}
@Test
void whenPrivilegeEvaluatorIsNotPresentAccessIsAllowed() throws Exception {
ApplicationContext context = mock(ApplicationContext.class);
willThrow(NoSuchBeanDefinitionException.class).given(context).getBean(WebInvocationPrivilegeEvaluator.class);
ErrorPageSecurityFilter securityFilter = new ErrorPageSecurityFilter(context);
securityFilter.doFilter(this.request, this.response, this.filterChain);
then(this.filterChain).should().doFilter(this.request, this.response);
}
@Test
void ignorePrivilegeEvaluationForNonErrorDispatchType() throws Exception {
this.request.setDispatcherType(DispatcherType.REQUEST);
given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(false);
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
then(this.privilegeEvaluator).shouldHaveNoInteractions();
then(this.filterChain).should().doFilter(this.request, this.response);
}
@Test
void whenThereIsAContextPathAndServletIsMappedToSlashContextPathIsNotPassedToEvaluator() throws Exception {
SecurityContext securityContext = mock(SecurityContext.class);
SecurityContextHolder.setContext(securityContext);
given(securityContext.getAuthentication()).willReturn(mock(Authentication.class));
this.request.setRequestURI("/example/error");
this.request.setContextPath("/example");
// Servlet mapped to /
this.request.setServletPath("/error");
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
then(this.privilegeEvaluator).should().isAllowed(eq("/error"), any());
}
@Test
void whenThereIsAContextPathAndServletIsMappedToWildcardPathCorrectPathIsPassedToEvaluator() throws Exception {
SecurityContext securityContext = mock(SecurityContext.class);
SecurityContextHolder.setContext(securityContext);
given(securityContext.getAuthentication()).willReturn(mock(Authentication.class));
this.request.setRequestURI("/example/dispatcher/path/error");
this.request.setContextPath("/example");
// Servlet mapped to /dispatcher/path/*
this.request.setServletPath("/dispatcher/path");
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
then(this.privilegeEvaluator).should().isAllowed(eq("/dispatcher/path/error"), any());
}
}

View File

@ -65,15 +65,6 @@ abstract class AbstractErrorPageTests {
assertThat(jsonResponse).isNull();
}
@Test
void testPublicNotFoundPage() {
final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange(this.pathPrefix + "/public/notfound",
HttpMethod.GET, null, JsonNode.class);
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
JsonNode jsonResponse = response.getBody();
assertThat(jsonResponse.get("error").asText()).isEqualTo("Not Found");
}
@Test
void testPublicNotFoundPageWithCorrectCredentials() {
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "password")

View File

@ -47,7 +47,6 @@ class CustomServletPathErrorPageTests extends AbstractErrorPageTests {
http.authorizeHttpRequests((requests) -> {
requests.antMatchers("/custom/servlet/path/public/**").permitAll();
requests.anyRequest().fullyAuthenticated();
requests.shouldFilterAllDispatcherTypes(false);
});
http.httpBasic();
http.formLogin((form) -> form.loginPage("/custom/servlet/path/login").permitAll());

View File

@ -46,7 +46,6 @@ class ErrorPageTests extends AbstractErrorPageTests {
http.authorizeHttpRequests((requests) -> {
requests.antMatchers("/public/**").permitAll();
requests.anyRequest().fullyAuthenticated();
requests.shouldFilterAllDispatcherTypes(false);
});
http.httpBasic();
http.formLogin((form) -> form.loginPage("/login").permitAll());

View File

@ -48,7 +48,6 @@ class NoSessionErrorPageTests extends AbstractErrorPageTests {
.authorizeHttpRequests((requests) -> {
requests.antMatchers("/public/**").permitAll();
requests.anyRequest().authenticated();
requests.shouldFilterAllDispatcherTypes(false);
});
http.httpBasic();
return http.build();