Skip to content

Commit 531c5ca

Browse files
jzheauxjzheaux
committed
Standardize Mocked Request Paths
Historically, Spring Security tests have set the servlet path to indicate the path of a MockHttpServletRequest. This was needed for AntPath and MvcRequestMatcher to correctly match the specified request path. This can leave MockHttpServletRequest in an inconsistent state since requestURI is null while servletPath has a value. For example, PathPatternRequestMatcher does not use the servlet path. For tests to continue working both before and after the migration from AntPath/MvcRequestMatcher to PathPatternRequestMatcher, the mock requests should have a consistent representation of path in getRequestURI and getServletPath. This commit updates classes to use TestMockHttpServletRequests, which ensures that the given path is applied to the servletPath and requestURI, while also overriding with contextPath, servletPath, and pathInfo when necessary.
1 parent ef50ff2 commit 531c5ca

File tree

65 files changed

+553
-721
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+553
-721
lines changed

‎cas/spring-security-cas.gradle

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ dependencies {
1414

1515
provided 'jakarta.servlet:jakarta.servlet-api'
1616

17+
testImplementation project(path : ':spring-security-web', configuration : 'tests')
1718
testImplementation "org.assertj:assertj-core"
1819
testImplementation "org.junit.jupiter:junit-jupiter-api"
1920
testImplementation "org.junit.jupiter:junit-jupiter-params"

‎cas/src/test/java/org/springframework/security/cas/web/CasAuthenticationFilterTests.java

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@
5555
import static org.mockito.Mockito.verify;
5656
import static org.mockito.Mockito.verifyNoInteractions;
5757
import static org.mockito.Mockito.verifyNoMoreInteractions;
58+
import static org.springframework.security.web.servlet.TestMockHttpServletRequests.get;
59+
import static org.springframework.security.web.servlet.TestMockHttpServletRequests.post;
5860

5961
/**
6062
* Tests {@link CasAuthenticationFilter}.
@@ -79,9 +81,7 @@ public void testGettersSetters() {
7981

8082
@Test
8183
public void testNormalOperation() throws Exception {
82-
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/login/cas");
83-
request.setServletPath("/login/cas");
84-
request.addParameter("ticket", "ST-0-ER94xMJmn6pha35CQRoZ");
84+
MockHttpServletRequest request = post("/login/cas").param("ticket", "ST-0-ER94xMJmn6pha35CQRoZ").build();
8585
CasAuthenticationFilter filter = new CasAuthenticationFilter();
8686
filter.setAuthenticationManager((a) -> a);
8787
assertThat(filter.requiresAuthentication(request, new MockHttpServletResponse())).isTrue();
@@ -104,24 +104,22 @@ public void testRequiresAuthenticationFilterProcessUrl() {
104104
String url = "/login/cas";
105105
CasAuthenticationFilter filter = new CasAuthenticationFilter();
106106
filter.setFilterProcessesUrl(url);
107-
MockHttpServletRequest request = new MockHttpServletRequest("POST", url);
107+
MockHttpServletRequest request = post(url).build();
108108
MockHttpServletResponse response = new MockHttpServletResponse();
109-
request.setServletPath(url);
110109
assertThat(filter.requiresAuthentication(request, response)).isTrue();
111110
}
112111

113112
@Test
114113
public void testRequiresAuthenticationProxyRequest() {
115114
CasAuthenticationFilter filter = new CasAuthenticationFilter();
116-
MockHttpServletRequest request = new MockHttpServletRequest();
115+
MockHttpServletRequest request = get("/pgtCallback").build();
117116
MockHttpServletResponse response = new MockHttpServletResponse();
118-
request.setServletPath("/pgtCallback");
119117
assertThat(filter.requiresAuthentication(request, response)).isFalse();
120118
filter.setProxyReceptorUrl(request.getServletPath());
121119
assertThat(filter.requiresAuthentication(request, response)).isFalse();
122120
filter.setProxyGrantingTicketStorage(mock(ProxyGrantingTicketStorage.class));
123121
assertThat(filter.requiresAuthentication(request, response)).isTrue();
124-
request.setServletPath("/other");
122+
request = get("/other").build();
125123
assertThat(filter.requiresAuthentication(request, response)).isFalse();
126124
}
127125

@@ -133,12 +131,10 @@ public void testRequiresAuthenticationAuthAll() {
133131
CasAuthenticationFilter filter = new CasAuthenticationFilter();
134132
filter.setFilterProcessesUrl(url);
135133
filter.setServiceProperties(properties);
136-
MockHttpServletRequest request = new MockHttpServletRequest("POST", url);
134+
MockHttpServletRequest request = post(url).build();
137135
MockHttpServletResponse response = new MockHttpServletResponse();
138-
request.setServletPath(url);
139136
assertThat(filter.requiresAuthentication(request, response)).isTrue();
140-
request = new MockHttpServletRequest("POST", "/other");
141-
request.setServletPath("/other");
137+
request = post("/other").build();
142138
assertThat(filter.requiresAuthentication(request, response)).isFalse();
143139
request.setParameter(properties.getArtifactParameter(), "value");
144140
assertThat(filter.requiresAuthentication(request, response)).isTrue();
@@ -156,9 +152,8 @@ public void testRequiresAuthenticationAuthAll() {
156152
@Test
157153
public void testAuthenticateProxyUrl() throws Exception {
158154
CasAuthenticationFilter filter = new CasAuthenticationFilter();
159-
MockHttpServletRequest request = new MockHttpServletRequest();
155+
MockHttpServletRequest request = get("/pgtCallback").build();
160156
MockHttpServletResponse response = new MockHttpServletResponse();
161-
request.setServletPath("/pgtCallback");
162157
filter.setProxyGrantingTicketStorage(mock(ProxyGrantingTicketStorage.class));
163158
filter.setProxyReceptorUrl(request.getServletPath());
164159
assertThat(filter.attemptAuthentication(request, response)).isNull();
@@ -172,9 +167,7 @@ public void testDoFilterAuthenticateAll() throws Exception {
172167
given(manager.authenticate(any(Authentication.class))).willReturn(authentication);
173168
ServiceProperties serviceProperties = new ServiceProperties();
174169
serviceProperties.setAuthenticateAllArtifacts(true);
175-
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/authenticate");
176-
request.setParameter("ticket", "ST-1-123");
177-
request.setServletPath("/authenticate");
170+
MockHttpServletRequest request = post("/authenticate").param("ticket", "ST-1-123").build();
178171
MockHttpServletResponse response = new MockHttpServletResponse();
179172
FilterChain chain = mock(FilterChain.class);
180173
CasAuthenticationFilter filter = new CasAuthenticationFilter();
@@ -200,10 +193,9 @@ public void testDoFilterAuthenticateAll() throws Exception {
200193
@Test
201194
public void testChainNotInvokedForProxyReceptor() throws Exception {
202195
CasAuthenticationFilter filter = new CasAuthenticationFilter();
203-
MockHttpServletRequest request = new MockHttpServletRequest();
196+
MockHttpServletRequest request = get("/pgtCallback").build();
204197
MockHttpServletResponse response = new MockHttpServletResponse();
205198
FilterChain chain = mock(FilterChain.class);
206-
request.setServletPath("/pgtCallback");
207199
filter.setProxyGrantingTicketStorage(mock(ProxyGrantingTicketStorage.class));
208200
filter.setProxyReceptorUrl(request.getServletPath());
209201
filter.doFilter(request, response, chain);
@@ -271,16 +263,14 @@ void successfulAuthenticationWhenSecurityContextHolderStrategySetThenUses() thro
271263
@Test
272264
public void requiresAuthenticationWhenProxyRequestMatcherThenMatches() {
273265
CasAuthenticationFilter filter = new CasAuthenticationFilter();
274-
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/pgtCallback");
266+
MockHttpServletRequest request = get("/pgtCallback").build();
275267
MockHttpServletResponse response = new MockHttpServletResponse();
276-
request.setServletPath("/pgtCallback");
277268
assertThat(filter.requiresAuthentication(request, response)).isFalse();
278269
filter.setProxyReceptorMatcher(PathPatternRequestMatcher.withDefaults().matcher(request.getServletPath()));
279270
assertThat(filter.requiresAuthentication(request, response)).isFalse();
280271
filter.setProxyGrantingTicketStorage(mock(ProxyGrantingTicketStorage.class));
281272
assertThat(filter.requiresAuthentication(request, response)).isTrue();
282-
request.setRequestURI("/other");
283-
request.setServletPath("/other");
273+
request = get("/other").build();
284274
assertThat(filter.requiresAuthentication(request, response)).isFalse();
285275
}
286276

‎config/src/test/java/org/springframework/security/config/FilterChainProxyConfigTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import static org.mockito.ArgumentMatchers.any;
4646
import static org.mockito.Mockito.mock;
4747
import static org.mockito.Mockito.verify;
48+
import static org.springframework.security.web.servlet.TestMockHttpServletRequests.get;
4849

4950
/**
5051
* Tests {@link FilterChainProxy}.
@@ -144,13 +145,12 @@ private void checkPathAndFilterOrder(FilterChainProxy filterChainProxy) {
144145
}
145146

146147
private void doNormalOperation(FilterChainProxy filterChainProxy) throws Exception {
147-
MockHttpServletRequest request = new MockHttpServletRequest("GET", "");
148-
request.setServletPath("/foo/secure/super/somefile.html");
148+
MockHttpServletRequest request = get("/foo/secure/super/somefile.html").build();
149149
MockHttpServletResponse response = new MockHttpServletResponse();
150150
FilterChain chain = mock(FilterChain.class);
151151
filterChainProxy.doFilter(request, response, chain);
152152
verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));
153-
request.setServletPath("/a/path/which/doesnt/match/any/filter.html");
153+
request = get("/a/path/which/doesnt/match/any/filter.html").build();
154154
chain = mock(FilterChain.class);
155155
filterChainProxy.doFilter(request, response, chain);
156156
verify(chain).doFilter(any(HttpServletRequest.class), any(HttpServletResponse.class));

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ public class AuthorizeRequestsTests {
7777
public void setup() {
7878
this.servletContext = spy(MockServletContext.mvc());
7979
this.request = new MockHttpServletRequest(this.servletContext, "GET", "");
80-
this.request.setMethod("GET");
8180
this.response = new MockHttpServletResponse();
8281
this.chain = new MockFilterChain();
8382
}
@@ -110,12 +109,10 @@ public void postWhenPostDenyAllInLambdaThenRespondsWithForbidden() throws Except
110109
@Test
111110
public void antMatchersPathVariables() throws Exception {
112111
loadConfig(AntPatchersPathVariables.class);
113-
this.request.setServletPath("/user/user");
114112
this.request.setRequestURI("/user/user");
115113
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
116114
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
117115
this.setup();
118-
this.request.setServletPath("/user/deny");
119116
this.request.setRequestURI("/user/deny");
120117
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
121118
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
@@ -125,12 +122,10 @@ public void antMatchersPathVariables() throws Exception {
125122
@Test
126123
public void antMatchersPathVariablesCaseInsensitive() throws Exception {
127124
loadConfig(AntPatchersPathVariables.class);
128-
this.request.setServletPath("/USER/user");
129125
this.request.setRequestURI("/USER/user");
130126
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
131127
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
132128
this.setup();
133-
this.request.setServletPath("/USER/deny");
134129
this.request.setRequestURI("/USER/deny");
135130
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
136131
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
@@ -140,12 +135,10 @@ public void antMatchersPathVariablesCaseInsensitive() throws Exception {
140135
@Test
141136
public void antMatchersPathVariablesCaseInsensitiveCamelCaseVariables() throws Exception {
142137
loadConfig(AntMatchersPathVariablesCamelCaseVariables.class);
143-
this.request.setServletPath("/USER/user");
144138
this.request.setRequestURI("/USER/user");
145139
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
146140
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
147141
this.setup();
148-
this.request.setServletPath("/USER/deny");
149142
this.request.setRequestURI("/USER/deny");
150143
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
151144
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecurityLogoutTests.java

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
4040

4141
import static org.assertj.core.api.Assertions.assertThat;
42+
import static org.springframework.security.web.servlet.TestMockHttpServletRequests.post;
4243

4344
/**
4445
* @author Rob Winch
@@ -48,8 +49,6 @@ public class HttpSecurityLogoutTests {
4849

4950
AnnotationConfigWebApplicationContext context;
5051

51-
MockHttpServletRequest request;
52-
5352
MockHttpServletResponse response;
5453

5554
MockFilterChain chain;
@@ -59,7 +58,6 @@ public class HttpSecurityLogoutTests {
5958

6059
@BeforeEach
6160
public void setup() {
62-
this.request = new MockHttpServletRequest("GET", "");
6361
this.response = new MockHttpServletResponse();
6462
this.chain = new MockFilterChain();
6563
}
@@ -77,11 +75,10 @@ public void clearAuthenticationFalse() throws Exception {
7775
loadConfig(ClearAuthenticationFalseConfig.class);
7876
SecurityContext currentContext = SecurityContextHolder.createEmptyContext();
7977
currentContext.setAuthentication(new TestingAuthenticationToken("user", "password", "ROLE_USER"));
80-
this.request.getSession()
78+
MockHttpServletRequest request = post("/logout").build();
79+
request.getSession()
8180
.setAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY, currentContext);
82-
this.request.setMethod("POST");
83-
this.request.setServletPath("/logout");
84-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
81+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
8582
assertThat(currentContext.getAuthentication()).isNotNull();
8683
}
8784

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecurityRequestMatchersTests.java

Lines changed: 25 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545

4646
import static org.assertj.core.api.Assertions.assertThat;
4747
import static org.springframework.security.config.Customizer.withDefaults;
48+
import static org.springframework.security.web.servlet.TestMockHttpServletRequests.get;
4849

4950
/**
5051
* @author Rob Winch
@@ -54,8 +55,6 @@ public class HttpSecurityRequestMatchersTests {
5455

5556
AnnotationConfigWebApplicationContext context;
5657

57-
MockHttpServletRequest request;
58-
5958
MockHttpServletResponse response;
6059

6160
MockFilterChain chain;
@@ -65,8 +64,6 @@ public class HttpSecurityRequestMatchersTests {
6564

6665
@BeforeEach
6766
public void setup() {
68-
this.request = new MockHttpServletRequest("GET", "");
69-
this.request.setMethod("GET");
7067
this.response = new MockHttpServletResponse();
7168
this.chain = new MockFilterChain();
7269
}
@@ -87,70 +84,64 @@ public void mvcMatcherGetFiltersNoUnsupportedMethodExceptionFromDummyRequest() {
8784
@Test
8885
public void requestMatchersMvcMatcherServletPath() throws Exception {
8986
loadConfig(RequestMatchersMvcMatcherServeltPathConfig.class);
90-
this.request.setServletPath("/spring");
91-
this.request.setRequestURI("/spring/path");
92-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
87+
MockHttpServletRequest request = get().requestUri(null, "/spring", "/path").build();
88+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
9389
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
9490
setup();
95-
this.request.setServletPath("");
96-
this.request.setRequestURI("/path");
97-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
91+
request = get().requestUri(null, "", "/path").build();
92+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
9893
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
9994
setup();
100-
this.request.setServletPath("/other");
101-
this.request.setRequestURI("/other/path");
102-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
95+
request = get().requestUri(null, "/other", "/path").build();
96+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
10397
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
10498
}
10599

106100
@Test
107101
public void requestMatcherWhensMvcMatcherServletPathInLambdaThenPathIsSecured() throws Exception {
108102
loadConfig(RequestMatchersMvcMatcherServletPathInLambdaConfig.class);
109-
this.request.setServletPath("/spring");
110-
this.request.setRequestURI("/spring/path");
111-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
103+
MockHttpServletRequest request = get().requestUri(null, "/spring", "/path").build();
104+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
112105
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
113106
setup();
114-
this.request.setServletPath("");
115-
this.request.setRequestURI("/path");
116-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
107+
request = get().requestUri(null, "", "/path").build();
108+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
117109
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
118110
setup();
119-
this.request.setServletPath("/other");
120-
this.request.setRequestURI("/other/path");
121-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
111+
request = get().requestUri(null, "/other", "/path").build();
112+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
122113
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
123114
}
124115

125116
@Test
126117
public void requestMatcherWhenMultiMvcMatcherInLambdaThenAllPathsAreDenied() throws Exception {
127118
loadConfig(MultiMvcMatcherInLambdaConfig.class);
128-
this.request.setRequestURI("/test-1");
129-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
119+
MockHttpServletRequest request = get("/test-1").build();
120+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
130121
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
131122
setup();
132-
this.request.setRequestURI("/test-2");
133-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
123+
request = get("/test-2").build();
124+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
134125
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
135126
setup();
136-
this.request.setRequestURI("/test-3");
137-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
127+
request = get("/test-3").build();
128+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
138129
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
139130
}
140131

141132
@Test
142133
public void requestMatcherWhenMultiMvcMatcherThenAllPathsAreDenied() throws Exception {
143134
loadConfig(MultiMvcMatcherConfig.class);
144-
this.request.setRequestURI("/test-1");
145-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
135+
MockHttpServletRequest request = get("/test-1").build();
136+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
146137
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
147138
setup();
148-
this.request.setRequestURI("/test-2");
149-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
139+
request = get("/test-2").build();
140+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
150141
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
151142
setup();
152-
this.request.setRequestURI("/test-3");
153-
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
143+
request = get("/test-3").build();
144+
this.springSecurityFilterChain.doFilter(request, this.response, this.chain);
154145
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
155146
}
156147

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersNoMvcTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public class HttpSecuritySecurityMatchersNoMvcTests {
6767

6868
@BeforeEach
6969
public void setup() throws Exception {
70-
this.request = new MockHttpServletRequest("GET", "");
70+
this.request = new MockHttpServletRequest();
7171
this.request.setMethod("GET");
7272
this.response = new MockHttpServletResponse();
7373
this.chain = new MockFilterChain();
@@ -83,15 +83,15 @@ public void cleanup() {
8383
@Test
8484
public void securityMatcherWhenNoMvcThenAntMatcher() throws Exception {
8585
loadConfig(SecurityMatcherNoMvcConfig.class);
86-
this.request.setServletPath("/path");
86+
this.request.setRequestURI("/path");
8787
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
8888
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
8989
setup();
90-
this.request.setServletPath("/path.html");
90+
this.request.setRequestURI("/path.html");
9191
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
9292
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
9393
setup();
94-
this.request.setServletPath("/path/");
94+
this.request.setRequestURI("/path/");
9595
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
9696
List<RequestMatcher> requestMatchers = this.springSecurityFilterChain.getFilterChains()
9797
.stream()

0 commit comments

Comments
 (0)