From d91b153cad23cf4b3551c3338a315f3ce97d52b6 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Fri, 8 May 2020 11:30:40 -0500 Subject: [PATCH] Explicitly set useSuffixPatternMatch for Tests Spring MVC changed their default behavior in https://github.com/spring-projects/spring-framework/issues/23915 This causes failures in some of Spring Security's tests. This explicitly sets useSuffixPatternMatch=true to ensure that Spring Security still works if users have modified their defaults. Closes gh-8493 --- Jenkinsfile | 2 +- .../web/builders/WebSecurityTests.java | 14 ++++++++++++-- .../configurers/AuthorizeRequestsTests.java | 18 ++++++++++++++---- .../HttpSecurityRequestMatchersTests.java | 16 +++++++++++++--- .../UrlAuthorizationConfigurerTests.java | 14 ++++++++++++-- .../web/servlet/AuthorizeRequestsDslTests.kt | 12 +++++++++++- .../InterceptUrlConfigTests-MvcMatchers.xml | 4 +++- ...ptUrlConfigTests-MvcMatchersServletPath.xml | 4 +++- 8 files changed, 69 insertions(+), 15 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 6836a67cf0..2778a8d88a 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -89,7 +89,7 @@ try { "GRADLE_ENTERPRISE_CACHE_USERNAME=${GRADLE_ENTERPRISE_CACHE_USERNAME}", "GRADLE_ENTERPRISE_CACHE_PASSWORD=${GRADLE_ENTERPRISE_CACHE_PASSWORD}", "GRADLE_ENTERPRISE_ACCESS_KEY=${GRADLE_ENTERPRISE_ACCESS_KEY}"]) { - sh "./gradlew test -PforceMavenRepositories=snapshot -PspringVersion='5.2.+' -PreactorVersion=Dysprosium-BUILD-SNAPSHOT -PspringDataVersion=Lovelace-BUILD-SNAPSHOT -PlocksDisabled --stacktrace" + sh "./gradlew test -PforceMavenRepositories=snapshot -PspringVersion='5.+' -PreactorVersion=Dysprosium-BUILD-SNAPSHOT -PspringDataVersion=Lovelace-BUILD-SNAPSHOT -PlocksDisabled --stacktrace" } } } catch(Exception e) { diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityTests.java index 60552a0e45..a096e39ec6 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/builders/WebSecurityTests.java @@ -36,6 +36,8 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.servlet.config.annotation.EnableWebMvc; +import org.springframework.web.servlet.config.annotation.PathMatchConfigurer; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import static org.assertj.core.api.Assertions.assertThat; @@ -69,7 +71,7 @@ public class WebSecurityTests { @Test public void ignoringMvcMatcher() throws Exception { - loadConfig(MvcMatcherConfig.class); + loadConfig(MvcMatcherConfig.class, LegacyMvcMatchingConfig.class); this.request.setRequestURI("/path"); this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); @@ -141,7 +143,7 @@ public class WebSecurityTests { @Test public void ignoringMvcMatcherServletPath() throws Exception { - loadConfig(MvcMatcherServletPathConfig.class); + loadConfig(MvcMatcherServletPathConfig.class, LegacyMvcMatchingConfig.class); this.request.setServletPath("/spring"); this.request.setRequestURI("/spring/path"); @@ -216,6 +218,14 @@ public class WebSecurityTests { } } + @Configuration + static class LegacyMvcMatchingConfig implements WebMvcConfigurer { + @Override + public void configurePathMatch(PathMatchConfigurer configurer) { + configurer.setUseSuffixPatternMatch(true); + } + } + public void loadConfig(Class... configs) { this.context = new AnnotationConfigWebApplicationContext(); this.context.register(configs); diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java index b9fb86db21..d10ea89dce 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java @@ -46,6 +46,8 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.servlet.config.annotation.EnableWebMvc; +import org.springframework.web.servlet.config.annotation.PathMatchConfigurer; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.spy; @@ -292,7 +294,7 @@ public class AuthorizeRequestsTests { @Test public void mvcMatcher() throws Exception { - loadConfig(MvcMatcherConfig.class); + loadConfig(MvcMatcherConfig.class, LegacyMvcMatchingConfig.class); this.request.setRequestURI("/path"); this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); @@ -350,7 +352,7 @@ public class AuthorizeRequestsTests { @Test public void requestWhenMvcMatcherDenyAllThenRespondsWithUnauthorized() throws Exception { - loadConfig(MvcMatcherInLambdaConfig.class); + loadConfig(MvcMatcherInLambdaConfig.class, LegacyMvcMatchingConfig.class); this.request.setRequestURI("/path"); this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); @@ -410,7 +412,7 @@ public class AuthorizeRequestsTests { @Test public void mvcMatcherServletPath() throws Exception { - loadConfig(MvcMatcherServletPathConfig.class); + loadConfig(MvcMatcherServletPathConfig.class, LegacyMvcMatchingConfig.class); this.request.setServletPath("/spring"); this.request.setRequestURI("/spring/path"); @@ -487,7 +489,7 @@ public class AuthorizeRequestsTests { @Test public void requestWhenMvcMatcherServletPathDenyAllThenMatchesOnServletPath() throws Exception { - loadConfig(MvcMatcherServletPathInLambdaConfig.class); + loadConfig(MvcMatcherServletPathInLambdaConfig.class, LegacyMvcMatchingConfig.class); this.request.setServletPath("/spring"); this.request.setRequestURI("/spring/path"); @@ -697,6 +699,14 @@ public class AuthorizeRequestsTests { } } + @Configuration + static class LegacyMvcMatchingConfig implements WebMvcConfigurer { + @Override + public void configurePathMatch(PathMatchConfigurer configurer) { + configurer.setUseSuffixPatternMatch(true); + } + } + public void loadConfig(Class... configs) { this.context = new AnnotationConfigWebApplicationContext(); this.context.register(configs); diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecurityRequestMatchersTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecurityRequestMatchersTests.java index ef4d4a2e42..fec73febcc 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecurityRequestMatchersTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecurityRequestMatchersTests.java @@ -36,6 +36,8 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.servlet.config.annotation.EnableWebMvc; +import org.springframework.web.servlet.config.annotation.PathMatchConfigurer; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.security.config.Customizer.withDefaults; @@ -71,7 +73,7 @@ public class HttpSecurityRequestMatchersTests { @Test public void mvcMatcher() throws Exception { - loadConfig(MvcMatcherConfig.class); + loadConfig(MvcMatcherConfig.class, LegacyMvcMatchingConfig.class); this.request.setServletPath("/path"); this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); @@ -137,7 +139,7 @@ public class HttpSecurityRequestMatchersTests { @Test public void requestMatchersMvcMatcher() throws Exception { - loadConfig(RequestMatchersMvcMatcherConfig.class); + loadConfig(RequestMatchersMvcMatcherConfig.class, LegacyMvcMatchingConfig.class); this.request.setServletPath("/path"); this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); @@ -198,7 +200,7 @@ public class HttpSecurityRequestMatchersTests { @Test public void requestMatchersWhenMvcMatcherInLambdaThenPathIsSecured() throws Exception { - loadConfig(RequestMatchersMvcMatcherInLambdaConfig.class); + loadConfig(RequestMatchersMvcMatcherInLambdaConfig.class, LegacyMvcMatchingConfig.class); this.request.setServletPath("/path"); this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); @@ -377,6 +379,14 @@ public class HttpSecurityRequestMatchersTests { } } + @Configuration + static class LegacyMvcMatchingConfig implements WebMvcConfigurer { + @Override + public void configurePathMatch(PathMatchConfigurer configurer) { + configurer.setUseSuffixPatternMatch(true); + } + } + public void loadConfig(Class... configs) { this.context = new AnnotationConfigWebApplicationContext(); this.context.register(configs); diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/UrlAuthorizationConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/UrlAuthorizationConfigurerTests.java index c9d0fb9db8..9b86829ebc 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/UrlAuthorizationConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/UrlAuthorizationConfigurerTests.java @@ -36,6 +36,8 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; import org.springframework.web.servlet.config.annotation.EnableWebMvc; +import org.springframework.web.servlet.config.annotation.PathMatchConfigurer; +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer; import static org.assertj.core.api.Assertions.assertThat; @@ -71,7 +73,7 @@ public class UrlAuthorizationConfigurerTests { @Test public void mvcMatcher() throws Exception { - loadConfig(MvcMatcherConfig.class); + loadConfig(MvcMatcherConfig.class, LegacyMvcMatchingConfig.class); this.request.setRequestURI("/path"); this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); @@ -129,7 +131,7 @@ public class UrlAuthorizationConfigurerTests { @Test public void mvcMatcherServletPath() throws Exception { - loadConfig(MvcMatcherServletPathConfig.class); + loadConfig(MvcMatcherServletPathConfig.class, LegacyMvcMatchingConfig.class); this.request.setServletPath("/spring"); this.request.setRequestURI("/spring/path"); @@ -222,6 +224,14 @@ public class UrlAuthorizationConfigurerTests { } } + @Configuration + static class LegacyMvcMatchingConfig implements WebMvcConfigurer { + @Override + public void configurePathMatch(PathMatchConfigurer configurer) { + configurer.setUseSuffixPatternMatch(true); + } + } + public void loadConfig(Class... configs) { this.context = new AnnotationConfigWebApplicationContext(); this.context.register(configs); diff --git a/config/src/test/kotlin/org/springframework/security/config/web/servlet/AuthorizeRequestsDslTests.kt b/config/src/test/kotlin/org/springframework/security/config/web/servlet/AuthorizeRequestsDslTests.kt index d96b57af24..72c5ff57be 100644 --- a/config/src/test/kotlin/org/springframework/security/config/web/servlet/AuthorizeRequestsDslTests.kt +++ b/config/src/test/kotlin/org/springframework/security/config/web/servlet/AuthorizeRequestsDslTests.kt @@ -20,6 +20,7 @@ import org.junit.Rule import org.junit.Test import org.springframework.beans.factory.annotation.Autowired import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration import org.springframework.http.HttpMethod import org.springframework.security.config.annotation.web.builders.HttpSecurity import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity @@ -42,6 +43,8 @@ import org.springframework.web.bind.annotation.PathVariable import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RestController import org.springframework.web.servlet.config.annotation.EnableWebMvc +import org.springframework.web.servlet.config.annotation.PathMatchConfigurer +import org.springframework.web.servlet.config.annotation.WebMvcConfigurer /** * Tests for [AuthorizeRequestsDsl] @@ -128,7 +131,7 @@ class AuthorizeRequestsDslTests { @Test fun `request when allowed by mvc then responds with OK`() { - this.spring.register(AuthorizeRequestsByMvcConfig::class.java).autowire() + this.spring.register(AuthorizeRequestsByMvcConfig::class.java, LegacyMvcMatchingConfig::class.java).autowire() this.mockMvc.get("/path") .andExpect { @@ -166,6 +169,13 @@ class AuthorizeRequestsDslTests { } } + @Configuration + open class LegacyMvcMatchingConfig : WebMvcConfigurer { + override fun configurePathMatch(configurer: PathMatchConfigurer) { + configurer.setUseSuffixPatternMatch(true) + } + } + @Test fun `request when secured by mvc path variables then responds based on path variable value`() { this.spring.register(MvcMatcherPathVariablesConfig::class.java).autowire() diff --git a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MvcMatchers.xml b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MvcMatchers.xml index 6e8e79b504..9edc7c8efa 100644 --- a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MvcMatchers.xml +++ b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MvcMatchers.xml @@ -32,7 +32,9 @@ - + + + diff --git a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MvcMatchersServletPath.xml b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MvcMatchersServletPath.xml index ad9f5d9ad3..c8a4cc42e9 100644 --- a/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MvcMatchersServletPath.xml +++ b/config/src/test/resources/org/springframework/security/config/http/InterceptUrlConfigTests-MvcMatchersServletPath.xml @@ -32,7 +32,9 @@ - + + +