Fix mvcMatchers overriding previous paths

Closes gh-10956
This commit is contained in:
Marcus Da Coregio 2022-03-22 12:37:51 -03:00 committed by Marcus Hert Da Coregio
parent 67830f4111
commit 1959c25a03
4 changed files with 297 additions and 7 deletions

View File

@ -3290,20 +3290,26 @@ public final class HttpSecurity extends AbstractConfiguredSecurityBuilder<Defaul
*/
public final class MvcMatchersRequestMatcherConfigurer extends RequestMatcherConfigurer {
private final List<MvcRequestMatcher> mvcMatchers;
/**
* Creates a new instance
* @param context the {@link ApplicationContext} to use
* @param matchers the {@link MvcRequestMatcher} instances to set the servlet path
* on if {@link #servletPath(String)} is set.
* @param mvcMatchers the {@link MvcRequestMatcher} instances to set the servlet
* path on if {@link #servletPath(String)} is set.
* @param allMatchers the {@link RequestMatcher} instances to continue the
* configuration
*/
private MvcMatchersRequestMatcherConfigurer(ApplicationContext context, List<MvcRequestMatcher> matchers) {
private MvcMatchersRequestMatcherConfigurer(ApplicationContext context, List<MvcRequestMatcher> mvcMatchers,
List<RequestMatcher> allMatchers) {
super(context);
this.matchers = new ArrayList<>(matchers);
this.mvcMatchers = new ArrayList<>(mvcMatchers);
this.matchers = allMatchers;
}
public RequestMatcherConfigurer servletPath(String servletPath) {
for (RequestMatcher matcher : this.matchers) {
((MvcRequestMatcher) matcher).setServletPath(servletPath);
for (MvcRequestMatcher matcher : this.mvcMatchers) {
matcher.setServletPath(servletPath);
}
return this;
}
@ -3328,7 +3334,7 @@ public final class HttpSecurity extends AbstractConfiguredSecurityBuilder<Defaul
public MvcMatchersRequestMatcherConfigurer mvcMatchers(HttpMethod method, String... mvcPatterns) {
List<MvcRequestMatcher> mvcMatchers = createMvcMatchers(method, mvcPatterns);
setMatchers(mvcMatchers);
return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers);
return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers, this.matchers);
}
@Override

View File

@ -26,6 +26,8 @@ import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.security.config.annotation.ObjectPostProcessor;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
@ -34,11 +36,13 @@ import org.springframework.security.config.test.SpringTestContext;
import org.springframework.security.config.test.SpringTestContextExtension;
import org.springframework.security.web.PortMapperImpl;
import org.springframework.security.web.RedirectStrategy;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.security.web.access.channel.ChannelDecisionManagerImpl;
import org.springframework.security.web.access.channel.ChannelProcessingFilter;
import org.springframework.security.web.access.channel.InsecureChannelProcessor;
import org.springframework.security.web.access.channel.SecureChannelProcessor;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.web.servlet.config.annotation.EnableWebMvc;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.spy;
@ -107,6 +111,24 @@ public class ChannelSecurityConfigurerTests {
this.mvc.perform(get("/")).andExpect(redirectedUrl("https://localhost/test"));
}
// gh-10956
@Test
public void requestWhenRequiresChannelWithMultiMvcMatchersThenRedirectsToHttps() throws Exception {
this.spring.register(RequiresChannelMultiMvcMatchersConfig.class).autowire();
this.mvc.perform(get("/test-1")).andExpect(redirectedUrl("https://localhost/test-1"));
this.mvc.perform(get("/test-2")).andExpect(redirectedUrl("https://localhost/test-2"));
this.mvc.perform(get("/test-3")).andExpect(redirectedUrl("https://localhost/test-3"));
}
// gh-10956
@Test
public void requestWhenRequiresChannelWithMultiMvcMatchersInLambdaThenRedirectsToHttps() throws Exception {
this.spring.register(RequiresChannelMultiMvcMatchersInLambdaConfig.class).autowire();
this.mvc.perform(get("/test-1")).andExpect(redirectedUrl("https://localhost/test-1"));
this.mvc.perform(get("/test-2")).andExpect(redirectedUrl("https://localhost/test-2"));
this.mvc.perform(get("/test-3")).andExpect(redirectedUrl("https://localhost/test-3"));
}
@EnableWebSecurity
static class ObjectPostProcessorConfig extends WebSecurityConfigurerAdapter {
@ -200,4 +222,59 @@ public class ChannelSecurityConfigurerTests {
}
@EnableWebSecurity
@EnableWebMvc
static class RequiresChannelMultiMvcMatchersConfig {
@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
// @formatter:off
http
.portMapper()
.portMapper(new PortMapperImpl())
.and()
.requiresChannel()
.mvcMatchers("/test-1")
.requiresSecure()
.mvcMatchers("/test-2")
.requiresSecure()
.mvcMatchers("/test-3")
.requiresSecure()
.anyRequest()
.requiresInsecure();
// @formatter:on
return http.build();
}
}
@EnableWebSecurity
@EnableWebMvc
static class RequiresChannelMultiMvcMatchersInLambdaConfig {
@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
// @formatter:off
http
.portMapper((port) -> port
.portMapper(new PortMapperImpl())
)
.requiresChannel((channel) -> channel
.mvcMatchers("/test-1")
.requiresSecure()
.mvcMatchers("/test-2")
.requiresSecure()
.mvcMatchers("/test-3")
.requiresSecure()
.anyRequest()
.requiresInsecure()
);
// @formatter:on
return http.build();
}
}
}

View File

@ -23,7 +23,10 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.mock.web.MockFilterChain;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
@ -33,6 +36,7 @@ import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.web.FilterChainProxy;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
@ -167,6 +171,38 @@ public class HttpSecurityRequestMatchersTests {
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
}
@Test
public void requestMatcherWhenMultiMvcMatcherInLambdaThenAllPathsAreDenied() throws Exception {
loadConfig(MultiMvcMatcherInLambdaConfig.class);
this.request.setRequestURI("/test-1");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
setup();
this.request.setRequestURI("/test-2");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
setup();
this.request.setRequestURI("/test-3");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
}
@Test
public void requestMatcherWhenMultiMvcMatcherThenAllPathsAreDenied() throws Exception {
loadConfig(MultiMvcMatcherConfig.class);
this.request.setRequestURI("/test-1");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
setup();
this.request.setRequestURI("/test-2");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
setup();
this.request.setRequestURI("/test-3");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED);
}
public void loadConfig(Class<?>... configs) {
this.context = new AnnotationConfigWebApplicationContext();
this.context.register(configs);
@ -175,6 +211,101 @@ public class HttpSecurityRequestMatchersTests {
this.context.getAutowireCapableBeanFactory().autowireBean(this);
}
@EnableWebSecurity
@Configuration
@EnableWebMvc
static class MultiMvcMatcherInLambdaConfig {
@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
SecurityFilterChain first(HttpSecurity http) throws Exception {
// @formatter:off
http
.requestMatchers((requests) -> requests
.mvcMatchers("/test-1")
.mvcMatchers("/test-2")
.mvcMatchers("/test-3")
)
.authorizeRequests((authorize) -> authorize.anyRequest().denyAll())
.httpBasic(withDefaults());
// @formatter:on
return http.build();
}
@Bean
SecurityFilterChain second(HttpSecurity http) throws Exception {
// @formatter:off
http
.requestMatchers((requests) -> requests
.mvcMatchers("/test-1")
)
.authorizeRequests((authorize) -> authorize
.anyRequest().permitAll()
);
// @formatter:on
return http.build();
}
@RestController
static class PathController {
@RequestMapping({ "/test-1", "/test-2", "/test-3" })
String path() {
return "path";
}
}
}
@EnableWebSecurity
@Configuration
@EnableWebMvc
static class MultiMvcMatcherConfig {
@Bean
@Order(Ordered.HIGHEST_PRECEDENCE)
SecurityFilterChain first(HttpSecurity http) throws Exception {
// @formatter:off
http
.requestMatchers()
.mvcMatchers("/test-1")
.mvcMatchers("/test-2")
.mvcMatchers("/test-3")
.and()
.authorizeRequests()
.anyRequest().denyAll()
.and()
.httpBasic(withDefaults());
// @formatter:on
return http.build();
}
@Bean
SecurityFilterChain second(HttpSecurity http) throws Exception {
// @formatter:off
http
.requestMatchers()
.mvcMatchers("/test-1")
.and()
.authorizeRequests()
.anyRequest().permitAll();
// @formatter:on
return http.build();
}
@RestController
static class PathController {
@RequestMapping({ "/test-1", "/test-2", "/test-3" })
String path() {
return "path";
}
}
}
@EnableWebSecurity
@Configuration
@EnableWebMvc

View File

@ -16,6 +16,8 @@
package org.springframework.security.config.annotation.web.configurers;
import java.util.Base64;
import javax.servlet.http.HttpServletResponse;
import org.junit.jupiter.api.AfterEach;
@ -23,16 +25,24 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.mock.web.MockFilterChain;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.mock.web.MockServletContext;
import org.springframework.security.config.Customizer;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
import org.springframework.security.web.FilterChainProxy;
import org.springframework.security.web.SecurityFilterChain;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
@ -125,6 +135,35 @@ public class UrlAuthorizationConfigurerTests {
loadConfig(AnonymousUrlAuthorizationConfig.class);
}
// gh-10956
@Test
public void multiMvcMatchersConfig() throws Exception {
loadConfig(MultiMvcMatcherConfig.class);
this.request.addHeader("Authorization",
"Basic " + new String(Base64.getEncoder().encode("user:password".getBytes())));
this.request.setRequestURI("/test-1");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
setup();
this.request.addHeader("Authorization",
"Basic " + new String(Base64.getEncoder().encode("user:password".getBytes())));
this.request.setRequestURI("/test-2");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
setup();
this.request.addHeader("Authorization",
"Basic " + new String(Base64.getEncoder().encode("user:password".getBytes())));
this.request.setRequestURI("/test-3");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
setup();
this.request.addHeader("Authorization",
"Basic " + new String(Base64.getEncoder().encode("user:password".getBytes())));
this.request.setRequestURI("/test-x");
this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain);
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
}
public void loadConfig(Class<?>... configs) {
this.context = new AnnotationConfigWebApplicationContext();
this.context.register(configs);
@ -228,4 +267,41 @@ public class UrlAuthorizationConfigurerTests {
}
@EnableWebSecurity
@EnableWebMvc
static class MultiMvcMatcherConfig {
@Bean
SecurityFilterChain security(HttpSecurity http, ApplicationContext context) throws Exception {
// @formatter:off
http
.httpBasic(Customizer.withDefaults())
.apply(new UrlAuthorizationConfigurer<>(context)).getRegistry()
.mvcMatchers("/test-1").hasRole("ADMIN")
.mvcMatchers("/test-2").hasRole("ADMIN")
.mvcMatchers("/test-3").hasRole("ADMIN")
.anyRequest().hasRole("USER");
// @formatter:on
return http.build();
}
@Bean
UserDetailsService userDetailsService() {
UserDetails user = User.withDefaultPasswordEncoder().username("user").password("password").roles("USER")
.build();
return new InMemoryUserDetailsManager(user);
}
@RestController
static class PathController {
@RequestMapping({ "/test-1", "/test-2", "/test-3", "/test-x" })
String path() {
return "path";
}
}
}
}