Deny unauthorized access to the error page

Fixes gh-26356

Co-authored-by Andy Wilkinson <wilkinsona@vmware.com>

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/ErrorPageSecurityFilterConfiguration.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2012-2021 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 java.util.EnumSet;
+
+import javax.servlet.DispatcherType;
+
+import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
+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)
+@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
+class ErrorPageSecurityFilterConfiguration {
+
+	@Bean
+	@ConditionalOnBean(WebInvocationPrivilegeEvaluator.class)
+	FilterRegistrationBean<ErrorPageSecurityFilter> errorPageSecurityInterceptor(ApplicationContext context) {
+		FilterRegistrationBean<ErrorPageSecurityFilter> registration = new FilterRegistrationBean<>(
+				new ErrorPageSecurityFilter(context));
+		registration.setDispatcherTypes(EnumSet.of(DispatcherType.ERROR));
+		return registration;
+	}
+
+}

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfiguration.java

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

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfigurationTests.java

@@ -17,6 +17,7 @@
 package org.springframework.boot.autoconfigure.security.servlet;
 
 import java.security.interfaces.RSAPublicKey;
+import java.util.EnumSet;
 
 import javax.servlet.DispatcherType;
 
@@ -36,11 +37,14 @@ 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;
@@ -53,6 +57,7 @@ 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;
 
@@ -224,6 +229,19 @@ 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);
+				}));
+	}
+
 	@Configuration(proxyBeanMethods = false)
 	@TestAutoConfigurationPackage(City.class)
 	static class EntityConfiguration {

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/FilterOrderingIntegrationTests.java

@@ -34,6 +34,7 @@ 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,6 +82,7 @@ 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);
 	}
 

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/filter/ErrorPageSecurityFilter.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2012-2021 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 javax.servlet.FilterChain;
+import javax.servlet.RequestDispatcher;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpFilter;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
+import org.springframework.context.ApplicationContext;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
+
+/**
+ * {@link HttpFilter} 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 extends HttpFilter {
+
+	private static final WebInvocationPrivilegeEvaluator ALWAYS = new AlwaysAllowWebInvocationPrivilegeEvaluator();
+
+	private final ApplicationContext context;
+
+	private volatile WebInvocationPrivilegeEvaluator privilegeEvaluator;
+
+	public ErrorPageSecurityFilter(ApplicationContext context) {
+		this.context = context;
+	}
+
+	@Override
+	public void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
+			throws IOException, ServletException {
+		Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+		if (!getPrivilegeEvaluator().isAllowed(request.getRequestURI(), authentication)) {
+			sendError(request, response);
+			return;
+		}
+		chain.doFilter(request, response);
+	}
+
+	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;
+		}
+	}
+
+	private void sendError(HttpServletRequest request, HttpServletResponse response) throws IOException {
+		Integer errorCode = (Integer) request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
+		response.sendError((errorCode != null) ? errorCode : 401);
+	}
+
+	/**
+	 * {@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;
+		}
+
+	}
+
+}

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/filter/ErrorPageSecurityFilterTests.java

@@ -0,0 +1,98 @@
+/*
+ * Copyright 2012-2021 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 javax.servlet.FilterChain;
+import javax.servlet.RequestDispatcher;
+
+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.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.BDDMockito.given;
+import static org.mockito.BDDMockito.willThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+
+/**
+ * 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() {
+		given(this.context.getBean(WebInvocationPrivilegeEvaluator.class)).willReturn(this.privilegeEvaluator);
+		this.securityFilter = new ErrorPageSecurityFilter(this.context);
+	}
+
+	@Test
+	void whenAccessIsAllowedShouldContinueDownFilterChain() throws Exception {
+		given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(true);
+		this.securityFilter.doFilter(this.request, this.response, this.filterChain);
+		verify(this.filterChain).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);
+		verifyNoInteractions(this.filterChain);
+		assertThat(this.response.getStatus()).isEqualTo(403);
+	}
+
+	@Test
+	void whenAccessIsDeniedAndNoErrorCodeAttributeOnRequest() throws Exception {
+		given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(false);
+		this.securityFilter.doFilter(this.request, this.response, this.filterChain);
+		verifyNoInteractions(this.filterChain);
+		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);
+		verify(this.filterChain).doFilter(this.request, this.response);
+	}
+
+}

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-custom-security/src/test/java/smoketest/actuator/customsecurity/AbstractSampleActuatorCustomSecurityTests.java

@@ -46,9 +46,6 @@ abstract class AbstractSampleActuatorCustomSecurityTests {
 		@SuppressWarnings("rawtypes")
 		ResponseEntity<Map> entity = restTemplate().getForEntity(getPath() + "/", Map.class);
 		assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
-		@SuppressWarnings("unchecked")
-		Map<String, Object> body = entity.getBody();
-		assertThat(body.get("error")).isEqualTo("Unauthorized");
 		assertThat(entity.getHeaders()).doesNotContainKey("Set-Cookie");
 	}
 

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator-custom-security/src/test/java/smoketest/actuator/customsecurity/SampleActuatorCustomSecurityApplicationTests.java

@@ -66,9 +66,7 @@ class SampleActuatorCustomSecurityApplicationTests extends AbstractSampleActuato
 		@SuppressWarnings("rawtypes")
 		ResponseEntity<Map> entity = restTemplate().getForEntity(getPath() + "/foo", Map.class);
 		assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
-		@SuppressWarnings("unchecked")
-		Map<String, Object> body = entity.getBody();
-		assertThat((String) body.get("message")).contains("Expected exception in controller");
+		assertThat(entity.getBody()).isNull();
 	}
 
 	@Test

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/ManagementPathSampleActuatorApplicationTests.java

@@ -53,7 +53,6 @@ class ManagementPathSampleActuatorApplicationTests {
 	void testHomeIsSecure() {
 		ResponseEntity<Map<String, Object>> entity = asMapEntity(this.restTemplate.getForEntity("/", Map.class));
 		assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
-		assertThat(entity.getBody().get("error")).isEqualTo("Unauthorized");
 		assertThat(entity.getHeaders()).doesNotContainKey("Set-Cookie");
 	}
 

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/SampleActuatorApplicationTests.java

@@ -57,7 +57,6 @@ class SampleActuatorApplicationTests {
 	void testHomeIsSecure() {
 		ResponseEntity<Map<String, Object>> entity = asMapEntity(this.restTemplate.getForEntity("/", Map.class));
 		assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
-		assertThat(entity.getBody().get("error")).isEqualTo("Unauthorized");
 		assertThat(entity.getHeaders()).doesNotContainKey("Set-Cookie");
 	}
 

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/ServletPathSampleActuatorApplicationTests.java

@@ -61,8 +61,6 @@ class ServletPathSampleActuatorApplicationTests {
 	void testHomeIsSecure() {
 		ResponseEntity<Map<String, Object>> entity = asMapEntity(this.restTemplate.getForEntity("/spring/", Map.class));
 		assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
-		Map<String, Object> body = entity.getBody();
-		assertThat(body.get("error")).isEqualTo("Unauthorized");
 		assertThat(entity.getHeaders()).doesNotContainKey("Set-Cookie");
 	}
 

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/main/java/smoketest/web/secure/SampleWebSecureApplication.java

@@ -66,8 +66,10 @@ public class SampleWebSecureApplication implements WebMvcConfigurer {
 		SecurityFilterChain configure(HttpSecurity http) throws Exception {
 			http.authorizeRequests((requests) -> {
 				requests.requestMatchers(PathRequest.toStaticResources().atCommonLocations()).permitAll();
+				requests.antMatchers("/public/**").permitAll();
 				requests.anyRequest().fullyAuthenticated();
 			});
+			http.httpBasic();
 			http.formLogin((form) -> {
 				form.loginPage("/login");
 				form.failureUrl("/login?error").permitAll();

spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/test/java/smoketest/web/secure/ErrorPageTests.java

@@ -0,0 +1,100 @@
+/*
+ * Copyright 2012-2021 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 smoketest.web.secure;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import org.junit.jupiter.api.Test;
+
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
+import org.springframework.boot.test.web.client.TestRestTemplate;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.bind.annotation.GetMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests to ensure that the error page is accessible only to authorized users.
+ *
+ * @author Madhura Bhave
+ */
+@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT,
+		classes = { ErrorPageTests.TestConfiguration.class, SampleWebSecureApplication.class },
+		properties = { "server.error.include-message=always", "spring.security.user.name=username",
+				"spring.security.user.password=password" })
+class ErrorPageTests {
+
+	@Autowired
+	private TestRestTemplate testRestTemplate;
+
+	@Test
+	void testBadCredentials() {
+		final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "wrongpassword")
+				.exchange("/test", HttpMethod.GET, null, JsonNode.class);
+		assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
+		JsonNode jsonResponse = response.getBody();
+		assertThat(jsonResponse).isNull();
+	}
+
+	@Test
+	void testNoCredentials() {
+		final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange("/test", HttpMethod.GET, null,
+				JsonNode.class);
+		assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
+		JsonNode jsonResponse = response.getBody();
+		assertThat(jsonResponse).isNull();
+	}
+
+	@Test
+	void testPublicNotFoundPage() {
+		final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange("/public/notfound", HttpMethod.GET,
+				null, JsonNode.class);
+		assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
+		JsonNode jsonResponse = response.getBody();
+		assertThat(jsonResponse).isNull();
+	}
+
+	@Test
+	void testCorrectCredentials() {
+		final ResponseEntity<String> response = this.testRestTemplate.withBasicAuth("username", "password")
+				.exchange("/test", HttpMethod.GET, null, String.class);
+		assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK);
+		response.getBody();
+		assertThat(response.getBody()).isEqualTo("test");
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	static class TestConfiguration {
+
+		@RestController
+		static class TestController {
+
+			@GetMapping("/test")
+			String test() {
+				return "test";
+			}
+
+		}
+
+	}
+
+}