Skip to content

Commit de9b7b4

Browse files
marcusdacoregiomarcusdacoregio
committed
Fix mvcMatchers overriding previous paths
Closes gh-10956
1 parent 995b291 commit de9b7b4

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
@@ -3078,20 +3078,26 @@ private <C extends SecurityConfigurerAdapter<DefaultSecurityFilterChain, HttpSec
30783078
*/
30793079
public final class MvcMatchersRequestMatcherConfigurer extends RequestMatcherConfigurer {
30803080

3081+
private final List<MvcRequestMatcher> mvcMatchers;
3082+
30813083
/**
30823084
* Creates a new instance
30833085
* @param context the {@link ApplicationContext} to use
3084-
* @param matchers the {@link MvcRequestMatcher} instances to set the servlet path
3085-
* on if {@link #servletPath(String)} is set.
3086+
* @param mvcMatchers the {@link MvcRequestMatcher} instances to set the servlet
3087+
* path on if {@link #servletPath(String)} is set.
3088+
* @param allMatchers the {@link RequestMatcher} instances to continue the
3089+
* configuration
30863090
*/
3087-
private MvcMatchersRequestMatcherConfigurer(ApplicationContext context, List<MvcRequestMatcher> matchers) {
3091+
private MvcMatchersRequestMatcherConfigurer(ApplicationContext context, List<MvcRequestMatcher> mvcMatchers,
3092+
List<RequestMatcher> allMatchers) {
30883093
super(context);
3089-
this.matchers = new ArrayList<>(matchers);
3094+
this.mvcMatchers = new ArrayList<>(mvcMatchers);
3095+
this.matchers = allMatchers;
30903096
}
30913097

30923098
public RequestMatcherConfigurer servletPath(String servletPath) {
3093-
for (RequestMatcher matcher : this.matchers) {
3094-
((MvcRequestMatcher) matcher).setServletPath(servletPath);
3099+
for (MvcRequestMatcher matcher : this.mvcMatchers) {
3100+
matcher.setServletPath(servletPath);
30953101
}
30963102
return this;
30973103
}
@@ -3116,7 +3122,7 @@ public class RequestMatcherConfigurer extends AbstractRequestMatcherRegistry<Req
31163122
public MvcMatchersRequestMatcherConfigurer mvcMatchers(HttpMethod method, String... mvcPatterns) {
31173123
List<MvcRequestMatcher> mvcMatchers = createMvcMatchers(method, mvcPatterns);
31183124
setMatchers(mvcMatchers);
3119-
return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers);
3125+
return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers, this.matchers);
31203126
}
31213127

31223128
@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
@@ -25,6 +25,8 @@
2525

2626
import org.springframework.beans.factory.annotation.Autowired;
2727
import org.springframework.context.annotation.Bean;
28+
import org.springframework.core.Ordered;
29+
import org.springframework.core.annotation.Order;
2830
import org.springframework.security.config.annotation.ObjectPostProcessor;
2931
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
3032
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
@@ -33,11 +35,13 @@
3335
import org.springframework.security.config.test.SpringTestContextExtension;
3436
import org.springframework.security.web.PortMapperImpl;
3537
import org.springframework.security.web.RedirectStrategy;
38+
import org.springframework.security.web.SecurityFilterChain;
3639
import org.springframework.security.web.access.channel.ChannelDecisionManagerImpl;
3740
import org.springframework.security.web.access.channel.ChannelProcessingFilter;
3841
import org.springframework.security.web.access.channel.InsecureChannelProcessor;
3942
import org.springframework.security.web.access.channel.SecureChannelProcessor;
4043
import org.springframework.test.web.servlet.MockMvc;
44+
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
4145

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

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

@@ -199,4 +221,59 @@ public void sendRedirect(HttpServletRequest request, HttpServletResponse respons
199221

200222
}
201223

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

‎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
@@ -22,7 +22,10 @@
2222
import org.junit.jupiter.api.Test;
2323

2424
import org.springframework.beans.factory.annotation.Autowired;
25+
import org.springframework.context.annotation.Bean;
2526
import org.springframework.context.annotation.Configuration;
27+
import org.springframework.core.Ordered;
28+
import org.springframework.core.annotation.Order;
2629
import org.springframework.mock.web.MockFilterChain;
2730
import org.springframework.mock.web.MockHttpServletRequest;
2831
import org.springframework.mock.web.MockHttpServletResponse;
@@ -32,6 +35,7 @@
3235
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
3336
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
3437
import org.springframework.security.web.FilterChainProxy;
38+
import org.springframework.security.web.SecurityFilterChain;
3539
import org.springframework.web.bind.annotation.RequestMapping;
3640
import org.springframework.web.bind.annotation.RestController;
3741
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
@@ -166,6 +170,38 @@ public void requestMatcherWhensMvcMatcherServletPathInLambdaThenPathIsSecured()
166170
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
167171
}
168172

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

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

0 commit comments

Comments
 (0)