Skip to content

Commit 3460c24

Browse files
mbhavewilkinsona
mbhave
and
wilkinsona
committed
Ignore context path when calling privilege evaluator
Previously, the error page security filter passed the request's URI to the privilege evaluator. This was incorrect in applications with a custom context path as the privilege evaluator must be passed a path that does not include the context path and the request URI includes the context path. This commit updates the filter to use UrlPathHelper's pathWithinApplication instead. The path within the application does not include the context path. In addition, pathWithinAppliation also correctly handles applications configured with a servlet mapping other than the default of /. Closes gh-29299 Co-Authored-By: Andy Wilkinson <wilkinsona@vmware.com>
1 parent fb44e1c commit 3460c24

File tree

11 files changed

+363
-89
lines changed

11 files changed

+363
-89
lines changed

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2021 the original author or authors.
2+
* Copyright 2012-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -35,6 +35,7 @@
3535
import org.springframework.security.core.Authentication;
3636
import org.springframework.security.core.context.SecurityContextHolder;
3737
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
38+
import org.springframework.web.util.UrlPathHelper;
3839

3940
/**
4041
* {@link Filter} that intercepts error dispatches to ensure authorized access to the
@@ -48,12 +49,15 @@ public class ErrorPageSecurityFilter implements Filter {
4849

4950
private static final WebInvocationPrivilegeEvaluator ALWAYS = new AlwaysAllowWebInvocationPrivilegeEvaluator();
5051

52+
private final UrlPathHelper urlPathHelper = new UrlPathHelper();
53+
5154
private final ApplicationContext context;
5255

5356
private volatile WebInvocationPrivilegeEvaluator privilegeEvaluator;
5457

5558
public ErrorPageSecurityFilter(ApplicationContext context) {
5659
this.context = context;
60+
this.urlPathHelper.setAlwaysUseFullPath(true);
5761
}
5862

5963
@Override
@@ -81,7 +85,7 @@ private boolean isAllowed(HttpServletRequest request, Integer errorCode) {
8185
if (isUnauthenticated(authentication) && isNotAuthenticationError(errorCode)) {
8286
return true;
8387
}
84-
return getPrivilegeEvaluator().isAllowed(request.getRequestURI(), authentication);
88+
return getPrivilegeEvaluator().isAllowed(this.urlPathHelper.getPathWithinApplication(request), authentication);
8589
}
8690

8791
private boolean isUnauthenticated(Authentication authentication) {

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

+28-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2021 the original author or authors.
2+
* Copyright 2012-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@
3636
import static org.assertj.core.api.Assertions.assertThat;
3737
import static org.mockito.ArgumentMatchers.any;
3838
import static org.mockito.ArgumentMatchers.anyString;
39+
import static org.mockito.ArgumentMatchers.eq;
3940
import static org.mockito.BDDMockito.given;
4041
import static org.mockito.BDDMockito.willThrow;
4142
import static org.mockito.Mockito.mock;
@@ -118,4 +119,30 @@ void ignorePrivilegeEvaluationForNonErrorDispatchType() throws Exception {
118119
verify(this.filterChain).doFilter(this.request, this.response);
119120
}
120121

122+
@Test
123+
void whenThereIsAContextPathAndServletIsMappedToSlashContextPathIsNotPassedToEvaluator() throws Exception {
124+
SecurityContext securityContext = mock(SecurityContext.class);
125+
SecurityContextHolder.setContext(securityContext);
126+
given(securityContext.getAuthentication()).willReturn(mock(Authentication.class));
127+
this.request.setRequestURI("/example/error");
128+
this.request.setContextPath("/example");
129+
// Servlet mapped to /
130+
this.request.setServletPath("/error");
131+
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
132+
verify(this.privilegeEvaluator).isAllowed(eq("/error"), any());
133+
}
134+
135+
@Test
136+
void whenThereIsAContextPathAndServletIsMappedToWildcardPathCorrectPathIsPassedToEvaluator() throws Exception {
137+
SecurityContext securityContext = mock(SecurityContext.class);
138+
SecurityContextHolder.setContext(securityContext);
139+
given(securityContext.getAuthentication()).willReturn(mock(Authentication.class));
140+
this.request.setRequestURI("/example/dispatcher/path/error");
141+
this.request.setContextPath("/example");
142+
// Servlet mapped to /dispatcher/path/*
143+
this.request.setServletPath("/dispatcher/path");
144+
this.securityFilter.doFilter(this.request, this.response, this.filterChain);
145+
verify(this.privilegeEvaluator).isAllowed(eq("/dispatcher/path/error"), any());
146+
}
147+
121148
}

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

+15-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2021 the original author or authors.
2+
* Copyright 2012-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -41,6 +41,12 @@ abstract class AbstractErrorPageTests {
4141
@Autowired
4242
private TestRestTemplate testRestTemplate;
4343

44+
private final String pathPrefix;
45+
46+
protected AbstractErrorPageTests(String pathPrefix) {
47+
this.pathPrefix = pathPrefix;
48+
}
49+
4450
@Test
4551
void testBadCredentials() {
4652
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "wrongpassword")
@@ -52,17 +58,17 @@ void testBadCredentials() {
5258

5359
@Test
5460
void testNoCredentials() {
55-
final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange("/test", HttpMethod.GET, null,
56-
JsonNode.class);
61+
final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange(this.pathPrefix + "/test",
62+
HttpMethod.GET, null, JsonNode.class);
5763
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
5864
JsonNode jsonResponse = response.getBody();
5965
assertThat(jsonResponse).isNull();
6066
}
6167

6268
@Test
6369
void testPublicNotFoundPage() {
64-
final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange("/public/notfound", HttpMethod.GET,
65-
null, JsonNode.class);
70+
final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange(this.pathPrefix + "/public/notfound",
71+
HttpMethod.GET, null, JsonNode.class);
6672
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
6773
JsonNode jsonResponse = response.getBody();
6874
assertThat(jsonResponse.get("error").asText()).isEqualTo("Not Found");
@@ -71,7 +77,7 @@ void testPublicNotFoundPage() {
7177
@Test
7278
void testPublicNotFoundPageWithCorrectCredentials() {
7379
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "password")
74-
.exchange("/public/notfound", HttpMethod.GET, null, JsonNode.class);
80+
.exchange(this.pathPrefix + "/public/notfound", HttpMethod.GET, null, JsonNode.class);
7581
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
7682
JsonNode jsonResponse = response.getBody();
7783
assertThat(jsonResponse.get("error").asText()).isEqualTo("Not Found");
@@ -80,7 +86,7 @@ void testPublicNotFoundPageWithCorrectCredentials() {
8086
@Test
8187
void testPublicNotFoundPageWithBadCredentials() {
8288
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "wrong")
83-
.exchange("/public/notfound", HttpMethod.GET, null, JsonNode.class);
89+
.exchange(this.pathPrefix + "/public/notfound", HttpMethod.GET, null, JsonNode.class);
8490
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
8591
JsonNode jsonResponse = response.getBody();
8692
assertThat(jsonResponse).isNull();
@@ -89,7 +95,7 @@ void testPublicNotFoundPageWithBadCredentials() {
8995
@Test
9096
void testCorrectCredentialsWithControllerException() {
9197
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "password")
92-
.exchange("/fail", HttpMethod.GET, null, JsonNode.class);
98+
.exchange(this.pathPrefix + "/fail", HttpMethod.GET, null, JsonNode.class);
9399
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
94100
JsonNode jsonResponse = response.getBody();
95101
assertThat(jsonResponse.get("error").asText()).isEqualTo("Internal Server Error");
@@ -98,7 +104,7 @@ void testCorrectCredentialsWithControllerException() {
98104
@Test
99105
void testCorrectCredentials() {
100106
final ResponseEntity<String> response = this.testRestTemplate.withBasicAuth("username", "password")
101-
.exchange("/test", HttpMethod.GET, null, String.class);
107+
.exchange(this.pathPrefix + "/test", HttpMethod.GET, null, String.class);
102108
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK);
103109
response.getBody();
104110
assertThat(response.getBody()).isEqualTo("test");

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

+109
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Copyright 2012-2022 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 smoketest.web.secure;
18+
19+
import com.fasterxml.jackson.databind.JsonNode;
20+
import org.junit.jupiter.api.Test;
21+
22+
import org.springframework.beans.factory.annotation.Autowired;
23+
import org.springframework.boot.test.web.client.TestRestTemplate;
24+
import org.springframework.http.HttpMethod;
25+
import org.springframework.http.HttpStatus;
26+
import org.springframework.http.ResponseEntity;
27+
28+
import static org.assertj.core.api.Assertions.assertThat;
29+
30+
/**
31+
* Abstract base class for tests to ensure that the error page is accessible only to
32+
* authorized users.
33+
*
34+
* @author Madhura Bhave
35+
*/
36+
abstract class AbstractUnauthenticatedErrorPageTests {
37+
38+
@Autowired
39+
private TestRestTemplate testRestTemplate;
40+
41+
private final String pathPrefix;
42+
43+
protected AbstractUnauthenticatedErrorPageTests(String pathPrefix) {
44+
this.pathPrefix = pathPrefix;
45+
}
46+
47+
@Test
48+
void testBadCredentials() {
49+
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "wrongpassword")
50+
.exchange(this.pathPrefix + "/test", HttpMethod.GET, null, JsonNode.class);
51+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
52+
JsonNode jsonResponse = response.getBody();
53+
assertThat(jsonResponse.get("error").asText()).isEqualTo("Unauthorized");
54+
}
55+
56+
@Test
57+
void testNoCredentials() {
58+
final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange(this.pathPrefix + "/test",
59+
HttpMethod.GET, null, JsonNode.class);
60+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
61+
JsonNode jsonResponse = response.getBody();
62+
assertThat(jsonResponse.get("error").asText()).isEqualTo("Unauthorized");
63+
}
64+
65+
@Test
66+
void testPublicNotFoundPage() {
67+
final ResponseEntity<JsonNode> response = this.testRestTemplate.exchange(this.pathPrefix + "/public/notfound",
68+
HttpMethod.GET, null, JsonNode.class);
69+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
70+
JsonNode jsonResponse = response.getBody();
71+
assertThat(jsonResponse.get("error").asText()).isEqualTo("Not Found");
72+
}
73+
74+
@Test
75+
void testPublicNotFoundPageWithCorrectCredentials() {
76+
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "password")
77+
.exchange(this.pathPrefix + "/public/notfound", HttpMethod.GET, null, JsonNode.class);
78+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
79+
JsonNode jsonResponse = response.getBody();
80+
assertThat(jsonResponse.get("error").asText()).isEqualTo("Not Found");
81+
}
82+
83+
@Test
84+
void testPublicNotFoundPageWithBadCredentials() {
85+
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "wrong")
86+
.exchange(this.pathPrefix + "/public/notfound", HttpMethod.GET, null, JsonNode.class);
87+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
88+
JsonNode jsonResponse = response.getBody();
89+
assertThat(jsonResponse.get("error").asText()).isEqualTo("Unauthorized");
90+
}
91+
92+
@Test
93+
void testCorrectCredentialsWithControllerException() {
94+
final ResponseEntity<JsonNode> response = this.testRestTemplate.withBasicAuth("username", "password")
95+
.exchange(this.pathPrefix + "/fail", HttpMethod.GET, null, JsonNode.class);
96+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
97+
JsonNode jsonResponse = response.getBody();
98+
assertThat(jsonResponse.get("error").asText()).isEqualTo("Internal Server Error");
99+
}
100+
101+
@Test
102+
void testCorrectCredentials() {
103+
final ResponseEntity<String> response = this.testRestTemplate.withBasicAuth("username", "password")
104+
.exchange(this.pathPrefix + "/test", HttpMethod.GET, null, String.class);
105+
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK);
106+
assertThat(response.getBody()).isEqualTo("test");
107+
}
108+
109+
}

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

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright 2012-2022 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 smoketest.web.secure;
18+
19+
import org.springframework.boot.test.context.SpringBootTest;
20+
import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
21+
22+
/**
23+
* Tests to ensure that the error page with a custom context path is accessible only to
24+
* authorized users.
25+
*
26+
* @author Madhura Bhave
27+
*/
28+
@SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT,
29+
classes = { AbstractErrorPageTests.TestConfiguration.class, ErrorPageTests.SecurityConfiguration.class,
30+
SampleWebSecureApplication.class },
31+
properties = { "server.error.include-message=always", "spring.security.user.name=username",
32+
"spring.security.user.password=password", "server.servlet.context-path=/example" })
33+
class CustomContextPathErrorPageTests extends AbstractErrorPageTests {
34+
35+
CustomContextPathErrorPageTests() {
36+
super("");
37+
}
38+
39+
}

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

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2012-2022 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 smoketest.web.secure;
18+
19+
import org.springframework.boot.test.context.SpringBootTest;
20+
21+
/**
22+
* Tests for error page that permits access to all with a custom context path.
23+
*
24+
* @author Madhura Bhave
25+
*/
26+
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT,
27+
classes = { AbstractErrorPageTests.TestConfiguration.class,
28+
UnauthenticatedErrorPageTests.SecurityConfiguration.class, SampleWebSecureApplication.class },
29+
properties = { "server.error.include-message=always", "spring.security.user.name=username",
30+
"spring.security.user.password=password", "server.servlet.context-path=/example" })
31+
class CustomContextPathUnauthenticatedErrorPageTests extends AbstractUnauthenticatedErrorPageTests {
32+
33+
CustomContextPathUnauthenticatedErrorPageTests() {
34+
super("");
35+
}
36+
37+
}

0 commit comments

Comments
 (0)