Skip to content

Commit 7983c69

Browse files
marcusdacoregiomarcusdacoregio
committed
Fix mvcMatchers overriding previous paths
Closes gh-10956
1 parent 15b3744 commit 7983c69

File tree

4 files changed

+302
-11
lines changed

4 files changed

+302
-11
lines changed

‎config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-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.
@@ -3008,20 +3008,26 @@ private <C extends SecurityConfigurerAdapter<DefaultSecurityFilterChain, HttpSec
30083008
*/
30093009
public final class MvcMatchersRequestMatcherConfigurer extends RequestMatcherConfigurer {
30103010

3011+
private final List<MvcRequestMatcher> mvcMatchers;
3012+
30113013
/**
30123014
* Creates a new instance
30133015
* @param context the {@link ApplicationContext} to use
3014-
* @param matchers the {@link MvcRequestMatcher} instances to set the servlet path
3015-
* on if {@link #servletPath(String)} is set.
3016+
* @param mvcMatchers the {@link MvcRequestMatcher} instances to set the servlet
3017+
* path on if {@link #servletPath(String)} is set.
3018+
* @param allMatchers the {@link RequestMatcher} instances to continue the
3019+
* configuration
30163020
*/
3017-
private MvcMatchersRequestMatcherConfigurer(ApplicationContext context, List<MvcRequestMatcher> matchers) {
3021+
private MvcMatchersRequestMatcherConfigurer(ApplicationContext context, List<MvcRequestMatcher> mvcMatchers,
3022+
List<RequestMatcher> allMatchers) {
30183023
super(context);
3019-
this.matchers = new ArrayList<>(matchers);
3024+
this.mvcMatchers = new ArrayList<>(mvcMatchers);
3025+
this.matchers = allMatchers;
30203026
}
30213027

30223028
public RequestMatcherConfigurer servletPath(String servletPath) {
3023-
for (RequestMatcher matcher : this.matchers) {
3024-
((MvcRequestMatcher) matcher).setServletPath(servletPath);
3029+
for (MvcRequestMatcher matcher : this.mvcMatchers) {
3030+
matcher.setServletPath(servletPath);
30253031
}
30263032
return this;
30273033
}
@@ -3046,7 +3052,7 @@ public class RequestMatcherConfigurer extends AbstractRequestMatcherRegistry<Req
30463052
public MvcMatchersRequestMatcherConfigurer mvcMatchers(HttpMethod method, String... mvcPatterns) {
30473053
List<MvcRequestMatcher> mvcMatchers = createMvcMatchers(method, mvcPatterns);
30483054
setMatchers(mvcMatchers);
3049-
return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers);
3055+
return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers, this.matchers);
30503056
}
30513057

30523058
@Override

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

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-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.
@@ -21,16 +21,21 @@
2121

2222
import org.springframework.beans.factory.annotation.Autowired;
2323
import org.springframework.context.annotation.Bean;
24+
import org.springframework.core.Ordered;
25+
import org.springframework.core.annotation.Order;
2426
import org.springframework.security.config.annotation.ObjectPostProcessor;
2527
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
2628
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
2729
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
2830
import org.springframework.security.config.test.SpringTestRule;
31+
import org.springframework.security.web.PortMapperImpl;
32+
import org.springframework.security.web.SecurityFilterChain;
2933
import org.springframework.security.web.access.channel.ChannelDecisionManagerImpl;
3034
import org.springframework.security.web.access.channel.ChannelProcessingFilter;
3135
import org.springframework.security.web.access.channel.InsecureChannelProcessor;
3236
import org.springframework.security.web.access.channel.SecureChannelProcessor;
3337
import org.springframework.test.web.servlet.MockMvc;
38+
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
3439

3540
import static org.mockito.ArgumentMatchers.any;
3641
import static org.mockito.Mockito.spy;
@@ -92,6 +97,24 @@ public void requestWhenRequiresChannelConfiguredInLambdaThenRedirectsToHttps() t
9297
this.mvc.perform(get("/")).andExpect(redirectedUrl("https://localhost/"));
9398
}
9499

100+
// gh-10956
101+
@Test
102+
public void requestWhenRequiresChannelWithMultiMvcMatchersThenRedirectsToHttps() throws Exception {
103+
this.spring.register(RequiresChannelMultiMvcMatchersConfig.class).autowire();
104+
this.mvc.perform(get("/test-1")).andExpect(redirectedUrl("https://localhost/test-1"));
105+
this.mvc.perform(get("/test-2")).andExpect(redirectedUrl("https://localhost/test-2"));
106+
this.mvc.perform(get("/test-3")).andExpect(redirectedUrl("https://localhost/test-3"));
107+
}
108+
109+
// gh-10956
110+
@Test
111+
public void requestWhenRequiresChannelWithMultiMvcMatchersInLambdaThenRedirectsToHttps() throws Exception {
112+
this.spring.register(RequiresChannelMultiMvcMatchersInLambdaConfig.class).autowire();
113+
this.mvc.perform(get("/test-1")).andExpect(redirectedUrl("https://localhost/test-1"));
114+
this.mvc.perform(get("/test-2")).andExpect(redirectedUrl("https://localhost/test-2"));
115+
this.mvc.perform(get("/test-3")).andExpect(redirectedUrl("https://localhost/test-3"));
116+
}
117+
95118
@EnableWebSecurity
96119
static class ObjectPostProcessorConfig extends WebSecurityConfigurerAdapter {
97120

@@ -154,4 +177,59 @@ protected void configure(HttpSecurity http) throws Exception {
154177

155178
}
156179

180+
@EnableWebSecurity
181+
@EnableWebMvc
182+
static class RequiresChannelMultiMvcMatchersConfig {
183+
184+
@Bean
185+
@Order(Ordered.HIGHEST_PRECEDENCE)
186+
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
187+
// @formatter:off
188+
http
189+
.portMapper()
190+
.portMapper(new PortMapperImpl())
191+
.and()
192+
.requiresChannel()
193+
.mvcMatchers("/test-1")
194+
.requiresSecure()
195+
.mvcMatchers("/test-2")
196+
.requiresSecure()
197+
.mvcMatchers("/test-3")
198+
.requiresSecure()
199+
.anyRequest()
200+
.requiresInsecure();
201+
// @formatter:on
202+
return http.build();
203+
}
204+
205+
}
206+
207+
@EnableWebSecurity
208+
@EnableWebMvc
209+
static class RequiresChannelMultiMvcMatchersInLambdaConfig {
210+
211+
@Bean
212+
@Order(Ordered.HIGHEST_PRECEDENCE)
213+
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
214+
// @formatter:off
215+
http
216+
.portMapper((port) -> port
217+
.portMapper(new PortMapperImpl())
218+
)
219+
.requiresChannel((channel) -> channel
220+
.mvcMatchers("/test-1")
221+
.requiresSecure()
222+
.mvcMatchers("/test-2")
223+
.requiresSecure()
224+
.mvcMatchers("/test-3")
225+
.requiresSecure()
226+
.anyRequest()
227+
.requiresInsecure()
228+
);
229+
// @formatter:on
230+
return http.build();
231+
}
232+
233+
}
234+
157235
}

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

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-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.
@@ -23,7 +23,10 @@
2323
import org.junit.Test;
2424

2525
import org.springframework.beans.factory.annotation.Autowired;
26+
import org.springframework.context.annotation.Bean;
2627
import org.springframework.context.annotation.Configuration;
28+
import org.springframework.core.Ordered;
29+
import org.springframework.core.annotation.Order;
2730
import org.springframework.mock.web.MockFilterChain;
2831
import org.springframework.mock.web.MockHttpServletRequest;
2932
import org.springframework.mock.web.MockHttpServletResponse;
@@ -33,6 +36,7 @@
3336
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
3437
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
3538
import org.springframework.security.web.FilterChainProxy;
39+
import org.springframework.security.web.SecurityFilterChain;
3640
import org.springframework.web.bind.annotation.RequestMapping;
3741
import org.springframework.web.bind.annotation.RestController;
3842
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
@@ -167,6 +171,38 @@ public void requestMatcherWhensMvcMatcherServletPathInLambdaThenPathIsSecured()
167171
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
168172
}
169173

174+
@Test
175+
public void requestMatcherWhenMultiMvcMatcherInLambdaThenAllPathsAreDenied() throws Exception {
176+
loadConfig(MultiMvcMatcherInLambdaConfig.class);
177+
this.request.setRequestURI("/test-1");
178+
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
179+
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
180+
setup();
181+
this.request.setRequestURI("/test-2");
182+
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
183+
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
184+
setup();
185+
this.request.setRequestURI("/test-3");
186+
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
187+
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
188+
}
189+
190+
@Test
191+
public void requestMatcherWhenMultiMvcMatcherThenAllPathsAreDenied() throws Exception {
192+
loadConfig(MultiMvcMatcherConfig.class);
193+
this.request.setRequestURI("/test-1");
194+
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
195+
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
196+
setup();
197+
this.request.setRequestURI("/test-2");
198+
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
199+
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
200+
setup();
201+
this.request.setRequestURI("/test-3");
202+
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
203+
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
204+
}
205+
170206
public void loadConfig(Class<?>... configs) {
171207
this.context = new AnnotationConfigWebApplicationContext();
172208
this.context.register(configs);
@@ -175,6 +211,101 @@ public void loadConfig(Class<?>... configs) {
175211
this.context.getAutowireCapableBeanFactory().autowireBean(this);
176212
}
177213

214+
@EnableWebSecurity
215+
@Configuration
216+
@EnableWebMvc
217+
static class MultiMvcMatcherInLambdaConfig {
218+
219+
@Bean
220+
@Order(Ordered.HIGHEST_PRECEDENCE)
221+
SecurityFilterChain first(HttpSecurity http) throws Exception {
222+
// @formatter:off
223+
http
224+
.requestMatchers((requests) -> requests
225+
.mvcMatchers("/test-1")
226+
.mvcMatchers("/test-2")
227+
.mvcMatchers("/test-3")
228+
)
229+
.authorizeRequests((authorize) -> authorize.anyRequest().denyAll())
230+
.httpBasic(withDefaults());
231+
// @formatter:on
232+
return http.build();
233+
}
234+
235+
@Bean
236+
SecurityFilterChain second(HttpSecurity http) throws Exception {
237+
// @formatter:off
238+
http
239+
.requestMatchers((requests) -> requests
240+
.mvcMatchers("/test-1")
241+
)
242+
.authorizeRequests((authorize) -> authorize
243+
.anyRequest().permitAll()
244+
);
245+
// @formatter:on
246+
return http.build();
247+
}
248+
249+
@RestController
250+
static class PathController {
251+
252+
@RequestMapping({ "/test-1", "/test-2", "/test-3" })
253+
String path() {
254+
return "path";
255+
}
256+
257+
}
258+
259+
}
260+
261+
@EnableWebSecurity
262+
@Configuration
263+
@EnableWebMvc
264+
static class MultiMvcMatcherConfig {
265+
266+
@Bean
267+
@Order(Ordered.HIGHEST_PRECEDENCE)
268+
SecurityFilterChain first(HttpSecurity http) throws Exception {
269+
// @formatter:off
270+
http
271+
.requestMatchers()
272+
.mvcMatchers("/test-1")
273+
.mvcMatchers("/test-2")
274+
.mvcMatchers("/test-3")
275+
.and()
276+
.authorizeRequests()
277+
.anyRequest().denyAll()
278+
.and()
279+
.httpBasic(withDefaults());
280+
// @formatter:on
281+
return http.build();
282+
}
283+
284+
@Bean
285+
SecurityFilterChain second(HttpSecurity http) throws Exception {
286+
// @formatter:off
287+
http
288+
.requestMatchers()
289+
.mvcMatchers("/test-1")
290+
.and()
291+
.authorizeRequests()
292+
.anyRequest().permitAll();
293+
// @formatter:on
294+
return http.build();
295+
}
296+
297+
@RestController
298+
static class PathController {
299+
300+
@RequestMapping({ "/test-1", "/test-2", "/test-3" })
301+
String path() {
302+
return "path";
303+
}
304+
305+
}
306+
307+
}
308+
178309
@EnableWebSecurity
179310
@Configuration
180311
@EnableWebMvc

0 commit comments

Comments
 (0)