Skip to content

Commit 1959c25

Browse files
marcusdacoregiomarcusdacoregio
committed
Fix mvcMatchers overriding previous paths
Closes gh-10956
1 parent 67830f4 commit 1959c25

File tree

4 files changed

+297
-7
lines changed

4 files changed

+297
-7
lines changed

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3290,20 +3290,26 @@ private <C extends SecurityConfigurerAdapter<DefaultSecurityFilterChain, HttpSec
32903290
*/
32913291
public final class MvcMatchersRequestMatcherConfigurer extends RequestMatcherConfigurer {
32923292

3293+
private final List<MvcRequestMatcher> mvcMatchers;
3294+
32933295
/**
32943296
* Creates a new instance
32953297
* @param context the {@link ApplicationContext} to use
3296-
* @param matchers the {@link MvcRequestMatcher} instances to set the servlet path
3297-
* on if {@link #servletPath(String)} is set.
3298+
* @param mvcMatchers the {@link MvcRequestMatcher} instances to set the servlet
3299+
* path on if {@link #servletPath(String)} is set.
3300+
* @param allMatchers the {@link RequestMatcher} instances to continue the
3301+
* configuration
32983302
*/
3299-
private MvcMatchersRequestMatcherConfigurer(ApplicationContext context, List<MvcRequestMatcher> matchers) {
3303+
private MvcMatchersRequestMatcherConfigurer(ApplicationContext context, List<MvcRequestMatcher> mvcMatchers,
3304+
List<RequestMatcher> allMatchers) {
33003305
super(context);
3301-
this.matchers = new ArrayList<>(matchers);
3306+
this.mvcMatchers = new ArrayList<>(mvcMatchers);
3307+
this.matchers = allMatchers;
33023308
}
33033309

33043310
public RequestMatcherConfigurer servletPath(String servletPath) {
3305-
for (RequestMatcher matcher : this.matchers) {
3306-
((MvcRequestMatcher) matcher).setServletPath(servletPath);
3311+
for (MvcRequestMatcher matcher : this.mvcMatchers) {
3312+
matcher.setServletPath(servletPath);
33073313
}
33083314
return this;
33093315
}
@@ -3328,7 +3334,7 @@ public class RequestMatcherConfigurer extends AbstractRequestMatcherRegistry<Req
33283334
public MvcMatchersRequestMatcherConfigurer mvcMatchers(HttpMethod method, String... mvcPatterns) {
33293335
List<MvcRequestMatcher> mvcMatchers = createMvcMatchers(method, mvcPatterns);
33303336
setMatchers(mvcMatchers);
3331-
return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers);
3337+
return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers, this.matchers);
33323338
}
33333339

33343340
@Override

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626

2727
import org.springframework.beans.factory.annotation.Autowired;
2828
import org.springframework.context.annotation.Bean;
29+
import org.springframework.core.Ordered;
30+
import org.springframework.core.annotation.Order;
2931
import org.springframework.security.config.annotation.ObjectPostProcessor;
3032
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
3133
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
@@ -34,11 +36,13 @@
3436
import org.springframework.security.config.test.SpringTestContextExtension;
3537
import org.springframework.security.web.PortMapperImpl;
3638
import org.springframework.security.web.RedirectStrategy;
39+
import org.springframework.security.web.SecurityFilterChain;
3740
import org.springframework.security.web.access.channel.ChannelDecisionManagerImpl;
3841
import org.springframework.security.web.access.channel.ChannelProcessingFilter;
3942
import org.springframework.security.web.access.channel.InsecureChannelProcessor;
4043
import org.springframework.security.web.access.channel.SecureChannelProcessor;
4144
import org.springframework.test.web.servlet.MockMvc;
45+
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
4246

4347
import static org.mockito.ArgumentMatchers.any;
4448
import static org.mockito.Mockito.spy;
@@ -107,6 +111,24 @@ public void requestWhenRequiresChannelConfiguredWithUrlRedirectThenRedirectsToUr
107111
this.mvc.perform(get("/")).andExpect(redirectedUrl("https://localhost/test"));
108112
}
109113

114+
// gh-10956
115+
@Test
116+
public void requestWhenRequiresChannelWithMultiMvcMatchersThenRedirectsToHttps() throws Exception {
117+
this.spring.register(RequiresChannelMultiMvcMatchersConfig.class).autowire();
118+
this.mvc.perform(get("/test-1")).andExpect(redirectedUrl("https://localhost/test-1"));
119+
this.mvc.perform(get("/test-2")).andExpect(redirectedUrl("https://localhost/test-2"));
120+
this.mvc.perform(get("/test-3")).andExpect(redirectedUrl("https://localhost/test-3"));
121+
}
122+
123+
// gh-10956
124+
@Test
125+
public void requestWhenRequiresChannelWithMultiMvcMatchersInLambdaThenRedirectsToHttps() throws Exception {
126+
this.spring.register(RequiresChannelMultiMvcMatchersInLambdaConfig.class).autowire();
127+
this.mvc.perform(get("/test-1")).andExpect(redirectedUrl("https://localhost/test-1"));
128+
this.mvc.perform(get("/test-2")).andExpect(redirectedUrl("https://localhost/test-2"));
129+
this.mvc.perform(get("/test-3")).andExpect(redirectedUrl("https://localhost/test-3"));
130+
}
131+
110132
@EnableWebSecurity
111133
static class ObjectPostProcessorConfig extends WebSecurityConfigurerAdapter {
112134

@@ -200,4 +222,59 @@ public void sendRedirect(HttpServletRequest request, HttpServletResponse respons
200222

201223
}
202224

225+
@EnableWebSecurity
226+
@EnableWebMvc
227+
static class RequiresChannelMultiMvcMatchersConfig {
228+
229+
@Bean
230+
@Order(Ordered.HIGHEST_PRECEDENCE)
231+
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
232+
// @formatter:off
233+
http
234+
.portMapper()
235+
.portMapper(new PortMapperImpl())
236+
.and()
237+
.requiresChannel()
238+
.mvcMatchers("/test-1")
239+
.requiresSecure()
240+
.mvcMatchers("/test-2")
241+
.requiresSecure()
242+
.mvcMatchers("/test-3")
243+
.requiresSecure()
244+
.anyRequest()
245+
.requiresInsecure();
246+
// @formatter:on
247+
return http.build();
248+
}
249+
250+
}
251+
252+
@EnableWebSecurity
253+
@EnableWebMvc
254+
static class RequiresChannelMultiMvcMatchersInLambdaConfig {
255+
256+
@Bean
257+
@Order(Ordered.HIGHEST_PRECEDENCE)
258+
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
259+
// @formatter:off
260+
http
261+
.portMapper((port) -> port
262+
.portMapper(new PortMapperImpl())
263+
)
264+
.requiresChannel((channel) -> channel
265+
.mvcMatchers("/test-1")
266+
.requiresSecure()
267+
.mvcMatchers("/test-2")
268+
.requiresSecure()
269+
.mvcMatchers("/test-3")
270+
.requiresSecure()
271+
.anyRequest()
272+
.requiresInsecure()
273+
);
274+
// @formatter:on
275+
return http.build();
276+
}
277+
278+
}
279+
203280
}

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@
2323
import org.junit.jupiter.api.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)