Skip to content

Commit dd1d148

Browse files
mbhavembhave
committed
Deny unauthorized access to the error page
Fixes gh-26356 Co-authored-by Andy Wilkinson <[email protected]>
1 parent 8bd3d53 commit dd1d148

File tree

13 files changed

+377
-11
lines changed

13 files changed

+377
-11
lines changed

Diff for: ‎spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/ErrorPageSecurityFilterConfiguration.java

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Copyright 2012-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.autoconfigure.security.servlet;
18+
19+
import java.util.EnumSet;
20+
21+
import javax.servlet.DispatcherType;
22+
23+
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
24+
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
25+
import org.springframework.boot.web.servlet.FilterRegistrationBean;
26+
import org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter;
27+
import org.springframework.context.ApplicationContext;
28+
import org.springframework.context.annotation.Bean;
29+
import org.springframework.context.annotation.Configuration;
30+
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
31+
32+
/**
33+
* Configures the {@link ErrorPageSecurityFilter}.
34+
*
35+
* @author Madhura Bhave
36+
*/
37+
@Configuration(proxyBeanMethods = false)
38+
@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
39+
class ErrorPageSecurityFilterConfiguration {
40+
41+
@Bean
42+
@ConditionalOnBean(WebInvocationPrivilegeEvaluator.class)
43+
FilterRegistrationBean<ErrorPageSecurityFilter> errorPageSecurityInterceptor(ApplicationContext context) {
44+
FilterRegistrationBean<ErrorPageSecurityFilter> registration = new FilterRegistrationBean<>(
45+
new ErrorPageSecurityFilter(context));
46+
registration.setDispatcherTypes(EnumSet.of(DispatcherType.ERROR));
47+
return registration;
48+
}
49+
50+
}

Diff for: ‎spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfiguration.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
@ConditionalOnClass(DefaultAuthenticationEventPublisher.class)
4242
@EnableConfigurationProperties(SecurityProperties.class)
4343
@Import({ SpringBootWebSecurityConfiguration.class, WebSecurityEnablerConfiguration.class,
44-
SecurityDataConfiguration.class })
44+
SecurityDataConfiguration.class, ErrorPageSecurityFilterConfiguration.class })
4545
public class SecurityAutoConfiguration {
4646

4747
@Bean

Diff for: ‎spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/security/servlet/SecurityAutoConfigurationTests.java

+18
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.boot.autoconfigure.security.servlet;
1818

1919
import java.security.interfaces.RSAPublicKey;
20+
import java.util.EnumSet;
2021

2122
import javax.servlet.DispatcherType;
2223

@@ -36,11 +37,14 @@
3637
import org.springframework.boot.test.context.FilteredClassLoader;
3738
import org.springframework.boot.test.context.runner.WebApplicationContextRunner;
3839
import org.springframework.boot.web.servlet.DelegatingFilterProxyRegistrationBean;
40+
import org.springframework.boot.web.servlet.FilterRegistrationBean;
41+
import org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter;
3942
import org.springframework.boot.web.servlet.filter.OrderedFilter;
4043
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
4144
import org.springframework.context.annotation.Bean;
4245
import org.springframework.context.annotation.Configuration;
4346
import org.springframework.core.convert.converter.Converter;
47+
import org.springframework.mock.web.MockServletContext;
4448
import org.springframework.orm.jpa.JpaTransactionManager;
4549
import org.springframework.security.authentication.AuthenticationEventPublisher;
4650
import org.springframework.security.authentication.DefaultAuthenticationEventPublisher;
@@ -53,6 +57,7 @@
5357
import org.springframework.security.data.repository.query.SecurityEvaluationContextExtension;
5458
import org.springframework.security.web.FilterChainProxy;
5559
import org.springframework.security.web.SecurityFilterChain;
60+
import org.springframework.test.util.ReflectionTestUtils;
5661

5762
import static org.assertj.core.api.Assertions.assertThat;
5863

@@ -224,6 +229,19 @@ void whenTheBeanFactoryHasAConversionServiceAndAConfigurationPropertyBindingConv
224229
.run((context) -> assertThat(context.getBean(JwtProperties.class).getPublicKey()).isNotNull());
225230
}
226231

232+
@Test
233+
@SuppressWarnings("unchecked")
234+
void filterRegistrationBeanForErrorPageSecurityInterceptor() {
235+
this.contextRunner.withInitializer((context) -> context.setServletContext(new MockServletContext()))
236+
.run(((context) -> {
237+
FilterRegistrationBean<?> bean = context.getBean(FilterRegistrationBean.class);
238+
assertThat(bean.getFilter()).isInstanceOf(ErrorPageSecurityFilter.class);
239+
EnumSet<DispatcherType> dispatcherTypes = (EnumSet<DispatcherType>) ReflectionTestUtils
240+
.getField(bean, "dispatcherTypes");
241+
assertThat(dispatcherTypes).containsExactly(DispatcherType.ERROR);
242+
}));
243+
}
244+
227245
@Configuration(proxyBeanMethods = false)
228246
@TestAutoConfigurationPackage(City.class)
229247
static class EntityConfiguration {

Diff for: ‎spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/FilterOrderingIntegrationTests.java

+2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.springframework.boot.testsupport.web.servlet.MockServletWebServer.RegisteredFilter;
3535
import org.springframework.boot.web.server.WebServerFactoryCustomizerBeanPostProcessor;
3636
import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext;
37+
import org.springframework.boot.web.servlet.filter.ErrorPageSecurityFilter;
3738
import org.springframework.boot.web.servlet.filter.OrderedCharacterEncodingFilter;
3839
import org.springframework.boot.web.servlet.filter.OrderedRequestContextFilter;
3940
import org.springframework.context.annotation.Bean;
@@ -81,6 +82,7 @@ void testFilterOrdering() {
8182
assertThat(iterator.next()).isInstanceOf(Filter.class);
8283
assertThat(iterator.next()).isInstanceOf(Filter.class);
8384
assertThat(iterator.next()).isInstanceOf(OrderedRequestContextFilter.class);
85+
assertThat(iterator.next()).isInstanceOf(ErrorPageSecurityFilter.class);
8486
assertThat(iterator.next()).isInstanceOf(FilterChainProxy.class);
8587
}
8688

Diff for: ‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/filter/ErrorPageSecurityFilter.java

+105
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright 2012-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.web.servlet.filter;
18+
19+
import java.io.IOException;
20+
21+
import javax.servlet.FilterChain;
22+
import javax.servlet.RequestDispatcher;
23+
import javax.servlet.ServletException;
24+
import javax.servlet.http.HttpFilter;
25+
import javax.servlet.http.HttpServletRequest;
26+
import javax.servlet.http.HttpServletResponse;
27+
28+
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
29+
import org.springframework.context.ApplicationContext;
30+
import org.springframework.security.core.Authentication;
31+
import org.springframework.security.core.context.SecurityContextHolder;
32+
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
33+
34+
/**
35+
* {@link HttpFilter} that intercepts error dispatches to ensure authorized access to the
36+
* error page.
37+
*
38+
* @author Madhura Bhave
39+
* @author Andy Wilkinson
40+
* @since 2.6.0
41+
*/
42+
public class ErrorPageSecurityFilter extends HttpFilter {
43+
44+
private static final WebInvocationPrivilegeEvaluator ALWAYS = new AlwaysAllowWebInvocationPrivilegeEvaluator();
45+
46+
private final ApplicationContext context;
47+
48+
private volatile WebInvocationPrivilegeEvaluator privilegeEvaluator;
49+
50+
public ErrorPageSecurityFilter(ApplicationContext context) {
51+
this.context = context;
52+
}
53+
54+
@Override
55+
public void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
56+
throws IOException, ServletException {
57+
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
58+
if (!getPrivilegeEvaluator().isAllowed(request.getRequestURI(), authentication)) {
59+
sendError(request, response);
60+
return;
61+
}
62+
chain.doFilter(request, response);
63+
}
64+
65+
private WebInvocationPrivilegeEvaluator getPrivilegeEvaluator() {
66+
WebInvocationPrivilegeEvaluator privilegeEvaluator = this.privilegeEvaluator;
67+
if (privilegeEvaluator == null) {
68+
privilegeEvaluator = getPrivilegeEvaluatorBean();
69+
this.privilegeEvaluator = privilegeEvaluator;
70+
}
71+
return privilegeEvaluator;
72+
}
73+
74+
private WebInvocationPrivilegeEvaluator getPrivilegeEvaluatorBean() {
75+
try {
76+
return this.context.getBean(WebInvocationPrivilegeEvaluator.class);
77+
}
78+
catch (NoSuchBeanDefinitionException ex) {
79+
return ALWAYS;
80+
}
81+
}
82+
83+
private void sendError(HttpServletRequest request, HttpServletResponse response) throws IOException {
84+
Integer errorCode = (Integer) request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
85+
response.sendError((errorCode != null) ? errorCode : 401);
86+
}
87+
88+
/**
89+
* {@link WebInvocationPrivilegeEvaluator} that always allows access.
90+
*/
91+
private static class AlwaysAllowWebInvocationPrivilegeEvaluator implements WebInvocationPrivilegeEvaluator {
92+
93+
@Override
94+
public boolean isAllowed(String uri, Authentication authentication) {
95+
return true;
96+
}
97+
98+
@Override
99+
public boolean isAllowed(String contextPath, String uri, String method, Authentication authentication) {
100+
return true;
101+
}
102+
103+
}
104+
105+
}

Diff for: ‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/filter/ErrorPageSecurityFilterTests.java

+98
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/*
2+
* Copyright 2012-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.web.servlet.filter;
18+
19+
import javax.servlet.FilterChain;
20+
import javax.servlet.RequestDispatcher;
21+
22+
import org.junit.jupiter.api.BeforeEach;
23+
import org.junit.jupiter.api.Test;
24+
25+
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
26+
import org.springframework.context.ApplicationContext;
27+
import org.springframework.mock.web.MockHttpServletRequest;
28+
import org.springframework.mock.web.MockHttpServletResponse;
29+
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
30+
31+
import static org.assertj.core.api.Assertions.assertThat;
32+
import static org.mockito.ArgumentMatchers.any;
33+
import static org.mockito.ArgumentMatchers.anyString;
34+
import static org.mockito.BDDMockito.given;
35+
import static org.mockito.BDDMockito.willThrow;
36+
import static org.mockito.Mockito.mock;
37+
import static org.mockito.Mockito.verify;
38+
import static org.mockito.Mockito.verifyNoInteractions;
39+
40+
/**
41+
* Tests for {@link ErrorPageSecurityFilter}.
42+
*
43+
* @author Madhura Bhave
44+
*/
45+
class ErrorPageSecurityFilterTests {
46+
47+
private final WebInvocationPrivilegeEvaluator privilegeEvaluator = mock(WebInvocationPrivilegeEvaluator.class);
48+
49+
private final ApplicationContext context = mock(ApplicationContext.class);
50+
51+
private final MockHttpServletRequest request = new MockHttpServletRequest();
52+
53+
private final MockHttpServletResponse response = new MockHttpServletResponse();
54+
55+
private final FilterChain filterChain = mock(FilterChain.class);
56+
57+
private ErrorPageSecurityFilter securityFilter;
58+
59+
@BeforeEach
60+
void setup() {
61+
given(this.context.getBean(WebInvocationPrivilegeEvaluator.class)).willReturn(this.privilegeEvaluator);
62+
this.securityFilter = new ErrorPageSecurityFilter(this.context);
63+
}
64+
65+
@Test
66+
void whenAccessIsAllowedShouldContinueDownFilterChain() throws Exception {
67+
given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(true);
68+
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
69+
verify(this.filterChain).doFilter(this.request, this.response);
70+
}
71+
72+
@Test
73+
void whenAccessIsDeniedShouldCallSendError() throws Exception {
74+
given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(false);
75+
this.request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, 403);
76+
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
77+
verifyNoInteractions(this.filterChain);
78+
assertThat(this.response.getStatus()).isEqualTo(403);
79+
}
80+
81+
@Test
82+
void whenAccessIsDeniedAndNoErrorCodeAttributeOnRequest() throws Exception {
83+
given(this.privilegeEvaluator.isAllowed(anyString(), any())).willReturn(false);
84+
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
85+
verifyNoInteractions(this.filterChain);
86+
assertThat(this.response.getStatus()).isEqualTo(401);
87+
}
88+
89+
@Test
90+
void whenPrivilegeEvaluatorIsNotPresentAccessIsAllowed() throws Exception {
91+
ApplicationContext context = mock(ApplicationContext.class);
92+
willThrow(NoSuchBeanDefinitionException.class).given(context).getBean(WebInvocationPrivilegeEvaluator.class);
93+
ErrorPageSecurityFilter securityFilter = new ErrorPageSecurityFilter(context);
94+
securityFilter.doFilter(this.request, this.response, this.filterChain);
95+
verify(this.filterChain).doFilter(this.request, this.response);
96+
}
97+
98+
}

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

-3
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@ void homeIsSecure() {
4646
@SuppressWarnings("rawtypes")
4747
ResponseEntity<Map> entity = restTemplate().getForEntity(getPath() + "/", Map.class);
4848
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
49-
@SuppressWarnings("unchecked")
50-
Map<String, Object> body = entity.getBody();
51-
assertThat(body.get("error")).isEqualTo("Unauthorized");
5249
assertThat(entity.getHeaders()).doesNotContainKey("Set-Cookie");
5350
}
5451

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

+1-3
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,7 @@ void testInsecureApplicationPath() {
6666
@SuppressWarnings("rawtypes")
6767
ResponseEntity<Map> entity = restTemplate().getForEntity(getPath() + "/foo", Map.class);
6868
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
69-
@SuppressWarnings("unchecked")
70-
Map<String, Object> body = entity.getBody();
71-
assertThat((String) body.get("message")).contains("Expected exception in controller");
69+
assertThat(entity.getBody()).isNull();
7270
}
7371

7472
@Test

Diff for: ‎spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/ManagementPathSampleActuatorApplicationTests.java

-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ void testHealth() {
5353
void testHomeIsSecure() {
5454
ResponseEntity<Map<String, Object>> entity = asMapEntity(this.restTemplate.getForEntity("/", Map.class));
5555
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
56-
assertThat(entity.getBody().get("error")).isEqualTo("Unauthorized");
5756
assertThat(entity.getHeaders()).doesNotContainKey("Set-Cookie");
5857
}
5958

Diff for: ‎spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/SampleActuatorApplicationTests.java

-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ class SampleActuatorApplicationTests {
5757
void testHomeIsSecure() {
5858
ResponseEntity<Map<String, Object>> entity = asMapEntity(this.restTemplate.getForEntity("/", Map.class));
5959
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
60-
assertThat(entity.getBody().get("error")).isEqualTo("Unauthorized");
6160
assertThat(entity.getHeaders()).doesNotContainKey("Set-Cookie");
6261
}
6362

Diff for: ‎spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-actuator/src/test/java/smoketest/actuator/ServletPathSampleActuatorApplicationTests.java

-2
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@ void testHealth() {
6161
void testHomeIsSecure() {
6262
ResponseEntity<Map<String, Object>> entity = asMapEntity(this.restTemplate.getForEntity("/spring/", Map.class));
6363
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
64-
Map<String, Object> body = entity.getBody();
65-
assertThat(body.get("error")).isEqualTo("Unauthorized");
6664
assertThat(entity.getHeaders()).doesNotContainKey("Set-Cookie");
6765
}
6866

Diff for: ‎spring-boot-tests/spring-boot-smoke-tests/spring-boot-smoke-test-web-secure/src/main/java/smoketest/web/secure/SampleWebSecureApplication.java

+2
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,10 @@ protected static class ApplicationSecurity {
6666
SecurityFilterChain configure(HttpSecurity http) throws Exception {
6767
http.authorizeRequests((requests) -> {
6868
requests.requestMatchers(PathRequest.toStaticResources().atCommonLocations()).permitAll();
69+
requests.antMatchers("/public/**").permitAll();
6970
requests.anyRequest().fullyAuthenticated();
7071
});
72+
http.httpBasic();
7173
http.formLogin((form) -> {
7274
form.loginPage("/login");
7375
form.failureUrl("/login?error").permitAll();

0 commit comments

Comments
 (0)