From 76d7a85bc03c4c17897c2c383a9c6d8540813156 Mon Sep 17 00:00:00 2001 From: Marcus Da Coregio Date: Tue, 4 Oct 2022 14:39:56 -0300 Subject: [PATCH] Use modified classpath test support for tests that depend on the classpath Issue gh-11347 --- ...tractRequestMatcherRegistryNoMvcTests.java | 89 ++++++++++++ .../AbstractRequestMatcherRegistryTests.java | 60 +------- ...ttpSecuritySecurityMatchersNoMvcTests.java | 134 ++++++++++++++++++ .../HttpSecuritySecurityMatchersTests.java | 73 +--------- 4 files changed, 234 insertions(+), 122 deletions(-) create mode 100644 config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryNoMvcTests.java create mode 100644 config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersNoMvcTests.java diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryNoMvcTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryNoMvcTests.java new file mode 100644 index 0000000000..04355a5982 --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryNoMvcTests.java @@ -0,0 +1,89 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.config.annotation.web; + +import java.util.List; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import org.springframework.http.HttpMethod; +import org.springframework.security.test.support.ClassPathExclusions; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link AbstractRequestMatcherRegistry} with no Spring MVC in the classpath + * + * @author Marcus Da Coregio + */ +@ClassPathExclusions("spring-webmvc-*.jar") +public class AbstractRequestMatcherRegistryNoMvcTests { + + private TestRequestMatcherRegistry matcherRegistry; + + @BeforeEach + public void setUp() { + this.matcherRegistry = new TestRequestMatcherRegistry(); + } + + @Test + public void requestMatchersWhenPatternAndMvcNotPresentThenReturnAntPathRequestMatcherType() { + List requestMatchers = this.matcherRegistry.requestMatchers("/path"); + assertThat(requestMatchers).isNotEmpty(); + assertThat(requestMatchers.size()).isEqualTo(1); + assertThat(requestMatchers.get(0)).isExactlyInstanceOf(AntPathRequestMatcher.class); + } + + @Test + public void requestMatchersWhenHttpMethodAndPatternAndMvcNotPresentThenReturnAntPathRequestMatcherType() { + List requestMatchers = this.matcherRegistry.requestMatchers(HttpMethod.GET, "/path"); + assertThat(requestMatchers).isNotEmpty(); + assertThat(requestMatchers.size()).isEqualTo(1); + assertThat(requestMatchers.get(0)).isExactlyInstanceOf(AntPathRequestMatcher.class); + } + + @Test + public void requestMatchersWhenHttpMethodAndMvcNotPresentThenReturnAntPathMatcherType() { + List requestMatchers = this.matcherRegistry.requestMatchers(HttpMethod.GET); + assertThat(requestMatchers).isNotEmpty(); + assertThat(requestMatchers.size()).isEqualTo(1); + assertThat(requestMatchers.get(0)).isExactlyInstanceOf(AntPathRequestMatcher.class); + } + + private static class TestRequestMatcherRegistry extends AbstractRequestMatcherRegistry> { + + @Override + public List mvcMatchers(String... mvcPatterns) { + return null; + } + + @Override + public List mvcMatchers(HttpMethod method, String... mvcPatterns) { + return null; + } + + @Override + protected List chainRequestMatchers(List requestMatchers) { + return requestMatchers; + } + + } + +} diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java index d5ebd3106a..ef39ae2d95 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java @@ -16,8 +16,6 @@ package org.springframework.security.config.annotation.web; -import java.lang.reflect.Field; -import java.lang.reflect.Modifier; import java.util.List; import jakarta.servlet.DispatcherType; @@ -43,6 +41,7 @@ import static org.mockito.Mockito.mock; * Tests for {@link AbstractRequestMatcherRegistry}. * * @author Joe Grandja + * @author Marcus Da Coregio */ public class AbstractRequestMatcherRegistryTests { @@ -61,6 +60,7 @@ public class AbstractRequestMatcherRegistryTests { ApplicationContext context = mock(ApplicationContext.class); given(context.getBean(ObjectPostProcessor.class)).willReturn(NO_OP_OBJECT_POST_PROCESSOR); this.matcherRegistry.setApplicationContext(context); + mockMvcIntrospector(true); } @Test @@ -113,9 +113,7 @@ public class AbstractRequestMatcherRegistryTests { } @Test - public void requestMatchersWhenPatternAndMvcPresentThenReturnMvcRequestMatcherType() throws Exception { - mockMvcPresentClasspath(true); - mockMvcIntrospector(true); + public void requestMatchersWhenPatternAndMvcPresentThenReturnMvcRequestMatcherType() { List requestMatchers = this.matcherRegistry.requestMatchers("/path"); assertThat(requestMatchers).isNotEmpty(); assertThat(requestMatchers.size()).isEqualTo(1); @@ -123,9 +121,7 @@ public class AbstractRequestMatcherRegistryTests { } @Test - public void requestMatchersWhenHttpMethodAndPatternAndMvcPresentThenReturnMvcRequestMatcherType() throws Exception { - mockMvcPresentClasspath(true); - mockMvcIntrospector(true); + public void requestMatchersWhenHttpMethodAndPatternAndMvcPresentThenReturnMvcRequestMatcherType() { List requestMatchers = this.matcherRegistry.requestMatchers(HttpMethod.GET, "/path"); assertThat(requestMatchers).isNotEmpty(); assertThat(requestMatchers.size()).isEqualTo(1); @@ -133,9 +129,7 @@ public class AbstractRequestMatcherRegistryTests { } @Test - public void requestMatchersWhenHttpMethodAndMvcPresentThenReturnMvcRequestMatcherType() throws Exception { - mockMvcPresentClasspath(true); - mockMvcIntrospector(true); + public void requestMatchersWhenHttpMethodAndMvcPresentThenReturnMvcRequestMatcherType() { List requestMatchers = this.matcherRegistry.requestMatchers(HttpMethod.GET); assertThat(requestMatchers).isNotEmpty(); assertThat(requestMatchers.size()).isEqualTo(1); @@ -143,40 +137,7 @@ public class AbstractRequestMatcherRegistryTests { } @Test - public void requestMatchersWhenPatternAndMvcNotPresentThenReturnAntPathRequestMatcherType() throws Exception { - mockMvcPresentClasspath(false); - mockMvcIntrospector(false); - List requestMatchers = this.matcherRegistry.requestMatchers("/path"); - assertThat(requestMatchers).isNotEmpty(); - assertThat(requestMatchers.size()).isEqualTo(1); - assertThat(requestMatchers.get(0)).isExactlyInstanceOf(AntPathRequestMatcher.class); - } - - @Test - public void requestMatchersWhenHttpMethodAndPatternAndMvcNotPresentThenReturnAntPathRequestMatcherType() - throws Exception { - mockMvcPresentClasspath(false); - mockMvcIntrospector(false); - List requestMatchers = this.matcherRegistry.requestMatchers(HttpMethod.GET, "/path"); - assertThat(requestMatchers).isNotEmpty(); - assertThat(requestMatchers.size()).isEqualTo(1); - assertThat(requestMatchers.get(0)).isExactlyInstanceOf(AntPathRequestMatcher.class); - } - - @Test - public void requestMatchersWhenHttpMethodAndMvcNotPresentThenReturnAntPathMatcherType() throws Exception { - mockMvcPresentClasspath(false); - mockMvcIntrospector(false); - List requestMatchers = this.matcherRegistry.requestMatchers(HttpMethod.GET); - assertThat(requestMatchers).isNotEmpty(); - assertThat(requestMatchers.size()).isEqualTo(1); - assertThat(requestMatchers.get(0)).isExactlyInstanceOf(AntPathRequestMatcher.class); - } - - @Test - public void requestMatchersWhenMvcPresentInClassPathAndMvcIntrospectorBeanNotAvailableThenException() - throws Exception { - mockMvcPresentClasspath(true); + public void requestMatchersWhenMvcPresentInClassPathAndMvcIntrospectorBeanNotAvailableThenException() { mockMvcIntrospector(false); assertThatExceptionOfType(NoSuchBeanDefinitionException.class) .isThrownBy(() -> this.matcherRegistry.requestMatchers("/path")).withMessageContaining( @@ -188,15 +149,6 @@ public class AbstractRequestMatcherRegistryTests { given(context.containsBean("mvcHandlerMappingIntrospector")).willReturn(isPresent); } - private void mockMvcPresentClasspath(Object newValue) throws Exception { - Field mvcPresentField = AbstractRequestMatcherRegistry.class.getDeclaredField("mvcPresent"); - mvcPresentField.setAccessible(true); - Field modifiersField = Field.class.getDeclaredField("modifiers"); - modifiersField.setAccessible(true); - modifiersField.setInt(mvcPresentField, mvcPresentField.getModifiers() & ~Modifier.FINAL); - mvcPresentField.set(null, newValue); - } - private static class TestRequestMatcherRegistry extends AbstractRequestMatcherRegistry> { @Override diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersNoMvcTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersNoMvcTests.java new file mode 100644 index 0000000000..5e1e0cf96b --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersNoMvcTests.java @@ -0,0 +1,134 @@ +/* + * Copyright 2002-2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.config.annotation.web.configurers; + +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.AfterEach; +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.context.annotation.Import; +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.annotation.web.builders.HttpSecurity; +import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; +import org.springframework.security.test.support.ClassPathExclusions; +import org.springframework.security.web.DefaultSecurityFilterChain; +import org.springframework.security.web.FilterChainProxy; +import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.context.support.AnnotationConfigWebApplicationContext; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Marcus Da Coregio + * + */ +@ClassPathExclusions("spring-webmvc-*.jar") +public class HttpSecuritySecurityMatchersNoMvcTests { + + AnnotationConfigWebApplicationContext context; + + MockHttpServletRequest request; + + MockHttpServletResponse response; + + MockFilterChain chain; + + @Autowired + FilterChainProxy springSecurityFilterChain; + + @BeforeEach + public void setup() throws Exception { + this.request = new MockHttpServletRequest("GET", ""); + this.request.setMethod("GET"); + this.response = new MockHttpServletResponse(); + this.chain = new MockFilterChain(); + } + + @AfterEach + public void cleanup() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + public void securityMatcherWhenNoMvcThenAntMatcher() throws Exception { + loadConfig(SecurityMatcherNoMvcConfig.class); + this.request.setServletPath("/path"); + this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); + assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED); + setup(); + this.request.setServletPath("/path.html"); + this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); + assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK); + setup(); + this.request.setServletPath("/path/"); + this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); + assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK); + assertThat(this.springSecurityFilterChain.getFilterChains()) + .extracting((c) -> ((DefaultSecurityFilterChain) c).getRequestMatcher()) + .hasOnlyElementsOfType(AntPathRequestMatcher.class); + } + + public void loadConfig(Class... configs) { + this.context = new AnnotationConfigWebApplicationContext(); + this.context.register(configs); + this.context.setServletContext(new MockServletContext()); + this.context.refresh(); + this.context.getAutowireCapableBeanFactory().autowireBean(this); + } + + @EnableWebSecurity + @Configuration + @Import(HttpSecuritySecurityMatchersTests.UsersConfig.class) + static class SecurityMatcherNoMvcConfig { + + @Bean + SecurityFilterChain appSecurity(HttpSecurity http) throws Exception { + // @formatter:off + http + .securityMatcher("/path") + .httpBasic().and() + .authorizeHttpRequests() + .anyRequest().denyAll(); + // @formatter:on + return http.build(); + } + + @RestController + static class PathController { + + @RequestMapping("/path") + String path() { + return "path"; + } + + } + + } + +} diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersTests.java index d4dd5b6093..18fda34732 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/HttpSecuritySecurityMatchersTests.java @@ -16,9 +16,6 @@ package org.springframework.security.config.annotation.web.configurers; -import java.lang.reflect.Field; -import java.lang.reflect.Modifier; - import jakarta.servlet.http.HttpServletResponse; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -34,13 +31,13 @@ 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.annotation.web.AbstractRequestMatcherRegistry; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; 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.DefaultSecurityFilterChain; import org.springframework.security.web.FilterChainProxy; import org.springframework.security.web.SecurityFilterChain; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; @@ -78,7 +75,6 @@ public class HttpSecuritySecurityMatchersTests { this.request.setMethod("GET"); this.response = new MockHttpServletResponse(); this.chain = new MockFilterChain(); - mockMvcPresentClasspath(true); } @AfterEach @@ -104,23 +100,6 @@ public class HttpSecuritySecurityMatchersTests { assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED); } - @Test - public void securityMatcherWhenNoMvcThenAntMatcher() throws Exception { - mockMvcPresentClasspath(false); - loadConfig(SecurityMatcherNoMvcConfig.class, LegacyMvcMatchingConfig.class); - this.request.setServletPath("/path"); - this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); - assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED); - setup(); - this.request.setServletPath("/path.html"); - this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); - assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK); - setup(); - this.request.setServletPath("/path/"); - this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); - assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK); - } - @Test public void securityMatcherWhenMvcMatcherAndGetFiltersNoUnsupportedMethodExceptionFromDummyRequest() { loadConfig(SecurityMatcherMvcConfig.class); @@ -141,6 +120,9 @@ public class HttpSecuritySecurityMatchersTests { this.request.setServletPath("/path/"); this.springSecurityFilterChain.doFilter(this.request, this.response, this.chain); assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_UNAUTHORIZED); + assertThat(this.springSecurityFilterChain.getFilterChains()) + .extracting((c) -> ((DefaultSecurityFilterChain) c).getRequestMatcher()) + .hasOnlyElementsOfType(MvcRequestMatcher.class); } @Test @@ -237,20 +219,6 @@ public class HttpSecuritySecurityMatchersTests { this.context.getAutowireCapableBeanFactory().autowireBean(this); } - private void mockMvcPresentClasspath(Object newValue) throws Exception { - mockMvcPresentClasspath(HttpSecurity.class, newValue); - mockMvcPresentClasspath(AbstractRequestMatcherRegistry.class, newValue); - } - - private void mockMvcPresentClasspath(Class clazz, Object newValue) throws Exception { - Field mvcPresentField = clazz.getDeclaredField("mvcPresent"); - mvcPresentField.setAccessible(true); - Field modifiersField = Field.class.getDeclaredField("modifiers"); - modifiersField.setAccessible(true); - modifiersField.setInt(mvcPresentField, mvcPresentField.getModifiers() & ~Modifier.FINAL); - mvcPresentField.set(null, newValue); - } - @EnableWebSecurity @Configuration @EnableWebMvc @@ -376,35 +344,6 @@ public class HttpSecuritySecurityMatchersTests { } - @EnableWebSecurity - @Configuration - @Import(UsersConfig.class) - static class SecurityMatcherNoMvcConfig { - - @Bean - SecurityFilterChain appSecurity(HttpSecurity http) throws Exception { - // @formatter:off - http - .securityMatcher("/path") - .httpBasic().and() - .authorizeHttpRequests() - .anyRequest().denyAll(); - // @formatter:on - return http.build(); - } - - @RestController - static class PathController { - - @RequestMapping("/path") - String path() { - return "path"; - } - - } - - } - @EnableWebSecurity @Configuration @EnableWebMvc @@ -415,9 +354,7 @@ public class HttpSecuritySecurityMatchersTests { SecurityFilterChain appSecurity(HttpSecurity http) throws Exception { // @formatter:off http - .securityMatchers() - .requestMatchers("/path") - .and() + .securityMatcher("/path") .httpBasic().and() .authorizeHttpRequests() .anyRequest().denyAll();