From 313c94dde2a4a3b7d9f041eef0d253c065960a44 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Tue, 26 Nov 2024 17:12:23 -0700 Subject: [PATCH] A RequestMatcherBuilder API Closes gh-13562 --- .../web/AbstractRequestMatcherRegistry.java | 24 +- .../AbstractRequestMatcherRegistryTests.java | 9 + .../AuthorizeHttpRequestsConfigurerTests.java | 39 +++ .../authorize-http-requests.adoc | 110 +++++++++ .../util/matcher/MvcRequestMatcher.java | 12 +- .../matcher/MvcRequestMatcherBuilder.java | 230 ++++++++++++++++++ .../util/matcher/RequestMatcherBuilder.java | 40 +++ .../web/servlet/MockServletContext.java | 149 ++++++++++++ .../MvcRequestMatcherBuilderTests.java | 141 +++++++++++ 9 files changed, 746 insertions(+), 8 deletions(-) create mode 100644 web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherBuilder.java create mode 100644 web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcherBuilder.java create mode 100644 web/src/test/java/org/springframework/security/web/servlet/MockServletContext.java create mode 100644 web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherBuilderTests.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java b/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java index d94e9d9083e..e351f5f4b04 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java @@ -48,6 +48,7 @@ import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcherBuilder; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.web.context.WebApplicationContext; @@ -73,6 +74,8 @@ public abstract class AbstractRequestMatcherRegistry { private static final RequestMatcher ANY_REQUEST = AnyRequestMatcher.INSTANCE; + private final RequestMatcherBuilder requestMatcherBuilder = new DefaultRequestMatcherBuilder(); + private ApplicationContext context; private boolean anyRequestConfigured = false; @@ -216,13 +219,9 @@ public C requestMatchers(HttpMethod method, String... patterns) { if (servletContext == null) { return requestMatchers(RequestMatchers.antMatchersAsArray(method, patterns)); } - List matchers = new ArrayList<>(); - for (String pattern : patterns) { - AntPathRequestMatcher ant = new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null); - MvcRequestMatcher mvc = createMvcMatchers(method, pattern).get(0); - matchers.add(new DeferredRequestMatcher((c) -> resolve(ant, mvc, c), mvc, ant)); - } - return requestMatchers(matchers.toArray(new RequestMatcher[0])); + RequestMatcherBuilder builder = context.getBeanProvider(RequestMatcherBuilder.class) + .getIfUnique(() -> this.requestMatcherBuilder); + return requestMatchers(builder.requestMatchers(method, patterns)); } private boolean anyPathsDontStartWithLeadingSlash(String... patterns) { @@ -473,6 +472,17 @@ static List regexMatchers(String... regexPatterns) { } + class DefaultRequestMatcherBuilder implements RequestMatcherBuilder { + + @Override + public RequestMatcher requestMatcher(HttpMethod method, String pattern) { + AntPathRequestMatcher ant = new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null); + MvcRequestMatcher mvc = createMvcMatchers(method, pattern).get(0); + return new DeferredRequestMatcher((c) -> resolve(ant, mvc, c), mvc, ant); + } + + } + static class DeferredRequestMatcher implements RequestMatcher { final Function requestMatcherFactory; 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 8561390515e..7d40ba7d06b 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 @@ -24,6 +24,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.beans.BeansException; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.ObjectProvider; import org.springframework.context.ApplicationContext; @@ -42,6 +43,7 @@ import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcherBuilder; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; import org.springframework.web.context.WebApplicationContext; @@ -87,6 +89,13 @@ public void setUp() { given(given).willReturn(postProcessors); given(postProcessors.getObject()).willReturn(NO_OP_OBJECT_POST_PROCESSOR); given(this.context.getServletContext()).willReturn(MockServletContext.mvc()); + ObjectProvider requestMatcherBuilders = new ObjectProvider<>() { + @Override + public RequestMatcherBuilder getObject() throws BeansException { + return AbstractRequestMatcherRegistryTests.this.matcherRegistry.new DefaultRequestMatcherBuilder(); + } + }; + given(this.context.getBeanProvider(RequestMatcherBuilder.class)).willReturn(requestMatcherBuilders); this.matcherRegistry.setApplicationContext(this.context); mockMvcIntrospector(true); } diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java index 41850d67561..448c3b9c24f 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java @@ -64,6 +64,8 @@ import org.springframework.security.web.access.intercept.RequestAuthorizationContext; import org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; +import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcherBuilder; +import org.springframework.security.web.util.matcher.RequestMatcherBuilder; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.test.web.servlet.request.RequestPostProcessor; @@ -72,6 +74,7 @@ import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.config.annotation.EnableWebMvc; import org.springframework.web.servlet.handler.HandlerMappingIntrospector; @@ -667,6 +670,19 @@ public void getWhenExcludeAuthorizationObservationsThenUnobserved() throws Excep verifyNoInteractions(handler); } + @Test + public void requestMatchersWhenMultipleDispatcherServletsAndPathBeanThenAllows() throws Exception { + this.spring.register(MvcRequestMatcherBuilderConfig.class, BasicController.class) + .postProcessor((context) -> context.getServletContext() + .addServlet("otherDispatcherServlet", DispatcherServlet.class) + .addMapping("/mvc")) + .autowire(); + this.mvc.perform(get("/mvc/path").servletPath("/mvc").with(user("user"))).andExpect(status().isOk()); + this.mvc.perform(get("/mvc/path").servletPath("/mvc").with(user("user").roles("DENIED"))) + .andExpect(status().isForbidden()); + this.mvc.perform(get("/path").with(user("user"))).andExpect(status().isForbidden()); + } + @Configuration @EnableWebSecurity static class GrantedAuthorityDefaultHasRoleConfig { @@ -1262,6 +1278,10 @@ void rootGet() { void rootPost() { } + @GetMapping("/path") + void path() { + } + } @Configuration @@ -1317,4 +1337,23 @@ SecurityObservationSettings observabilityDefaults() { } + @Configuration + @EnableWebSecurity + @EnableWebMvc + static class MvcRequestMatcherBuilderConfig { + + @Bean + RequestMatcherBuilder servletPath(HandlerMappingIntrospector introspector) { + return new MvcRequestMatcherBuilder(introspector, "/mvc"); + } + + @Bean + SecurityFilterChain security(HttpSecurity http) throws Exception { + http.authorizeHttpRequests((authorize) -> authorize.requestMatchers("/path").hasRole("USER")) + .httpBasic(withDefaults()); + return http.build(); + } + + } + } diff --git a/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc b/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc index 4eaf5f3d5ee..c5d6cbb54dd 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc @@ -640,11 +640,121 @@ Xml:: ---- ====== +[[conditions-for-servlet-path-matching]] This need can arise in at least two different ways: * If you use the `spring.mvc.servlet.path` Boot property to change the default path (`/`) to something else * If you register more than one Spring MVC `DispatcherServlet` (thus requiring that one of them not be the default path) +=== Using a `RequestMatcherBuilder` + +You can reduce the boilerplate of constructing several `MvcRequestMatcher` instances by providing a single instance of `RequestMatcherBuilder`. + +For example, if all of your requests in `requestMatcher(String)` are MVC requests, then you can do: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +RequestMatcherBuilder allRequestsAreMvc(HandlerMappingIntrospector introspector) { + MvcRequestMatcher.Builder mvc = new MvcRequestMatcher.Builder(introspector).servletPath("/my-servlet-path"); + return mvc::pattern; +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean fun allRequestsAreMvc(introspector: HandlerMappingIntrospector?): RequestMatcherBuilder { + var mvc = MvcRequestMatcher.Builder(introspector).servletPath("/my-servlet-path") + return mvc::pattern +} +---- +====== + +Spring Security will use this builder for all request matchers specified as a `String`. + +[TIP] +==== +Often the only non-MVC requests that there are in a Spring Boot application are those to static resources like `/css", '/js', and 'favicon.ico`. +==== + +You can permit these by using Spring Boot's `RequestMatchers` static factory like so: + +[tabs] +====== +Java:: ++ +[source,java] +---- +@Bean +SecurityFilterChain security(HttpSecurity http) throws Exception { + http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(PathRequest.toStaticResources().atCommonLocations()).permitAll() + .requestMatchers("/my", "/mvc", "/requests").hasAuthority("app") + ) +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +val printview: RequestMatcher = { (request) -> request.getParameter("print") != null } +http { + authorizeHttpRequests { + authorize(PathRequest.toStaticResources().atCommonLocations(), permitAll) + authorize("/my", hasAuthority("app")) + authorize("/mvc", hasAuthority("app")) + authorize("/requests", hasAuthority("app")) + } +} +---- +====== + +Since `atCommonLocations` returns instances of `RequestMatcher`, this technique allows you to publish an MVC-based `RequestMatcherBuilder` for the rest. + +In the event that <>, you can publish an `MvcDelegatingRequestMatcherBuilder` instance instead: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +RequestMatcherBuilder allRequestsAreMvc(HandlerMappingIntrospector introspector) { + return MvcDelegatingRequestMatcherBuilder(introspector, "/my-mvc-servlet-path"); +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun allRequestsAreMvc(introspector: HandlerMappingIntrospector?): RequestMatcherBuilder { + return MvcDelegatingRequestMatcherBuilder(introspector, "/my-mvc-servlet-path"); +} +---- +====== + +This produces matchers that check first if the request is an MVC request; if it is, use an `MvcRequestMatcher` and otherwise use an `AntPathRequestMatcher`. + +[NOTE] +==== +The reason this `RequestMatcherBuilder` is not used by default is because of potential ambiguities in the meaning of given `String` patterns. +For example, consider a servlet mapped to `/example` and a Spring MVC endpoint mapped to `/mvc-servlet-path/example` where `/mvc-servlet-path` is the servlet path for MVC endpoints. +In this case, it's unclear whether by `requestMatchers("/example")` you mean to secure `/example`` or `/mvc-servlet-path/example`. + +Publishing any `RequestMatcherBuilder` indicates that you will handle these ambiguous situations, should they arise. +==== + [[match-by-custom]] === Using a Custom Matcher diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java index 9576f0533e6..3fa2559aec6 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java @@ -49,7 +49,7 @@ */ public class MvcRequestMatcher implements RequestMatcher, RequestVariablesExtractor { - private final DefaultMatcher defaultMatcher = new DefaultMatcher(); + private RequestMatcher defaultMatcher = new DefaultMatcher(); private final HandlerMappingIntrospector introspector; @@ -130,6 +130,16 @@ protected final String getServletPath() { return this.servletPath; } + /** + * The matcher that this should fall back on in the event that the request isn't + * recognized by Spring MVC + * @param defaultMatcher the default matcher to use + * @since 6.4 + */ + public void setDefaultMatcher(RequestMatcher defaultMatcher) { + this.defaultMatcher = defaultMatcher; + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherBuilder.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherBuilder.java new file mode 100644 index 00000000000..c52f7e598b6 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherBuilder.java @@ -0,0 +1,230 @@ +/* + * Copyright 2002-2024 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.web.servlet.util.matcher; + +import jakarta.servlet.ServletRegistration; +import jakarta.servlet.http.HttpServletMapping; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.MappingMatch; + +import org.springframework.http.HttpMethod; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.OrRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcherBuilder; +import org.springframework.util.Assert; +import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.handler.HandlerMappingIntrospector; + +/** + * A {@link RequestMatcherBuilder} that builder {@link RequestMatcher} instances that + * first check to see if the request is a Spring MVC request. If so, it matches using + * {@link HandlerMappingIntrospector}. If it's not an MVC request, it falls back to ant + * path request matching. + * + *

+ * Note that this implementation is stricter than {@link MvcRequestMatcher} in that it + * requires {@link MvcRequestMatcher#setServletPath} be configured if Spring MVC has a + * custom servlet path. + *

+ * + * @author Josh Cummings + * @since 6.4 + */ +public final class MvcRequestMatcherBuilder implements RequestMatcherBuilder { + + private final HandlerMappingIntrospector introspector; + + private final RequestMatcher isMvcRequest; + + private String servletPath; + + public MvcRequestMatcherBuilder(HandlerMappingIntrospector introspector) { + this(introspector, null); + } + + public MvcRequestMatcherBuilder(HandlerMappingIntrospector introspector, String servletPath) { + this.introspector = introspector; + if (servletPath != null) { + Assert.isTrue(servletPath.startsWith("/") && !servletPath.endsWith("/*"), + "Please sure the each servlet path is of the format /path"); + } + this.servletPath = servletPath; + this.isMvcRequest = new OrRequestMatcher(new MockMvcRequestMatcher(), + new DispatcherServletRequestMatcher(this.introspector)); + } + + /** + * @inheritDoc + */ + @Override + public RequestMatcher requestMatcher(HttpMethod method, String pattern) { + Assert.isTrue(pattern.startsWith("/"), "Please ensure that all patterns start with a /"); + AntPathRequestMatcher ant = new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null); + MvcRequestMatcher mvc = new StrictMvcRequestMatcher(this.introspector, pattern); + mvc.setMethod(method); + mvc.setServletPath(this.servletPath); + mvc.setDefaultMatcher((request) -> false); + return new MvcDelegatingRequestMatcher(ant, mvc, this.isMvcRequest); + } + + /** + * MockMvc does not populate the entire servlet registration. However, it's reasonable + * to assume that if a request is using MockMvc, then it is targeting MVC endpoints. + */ + static class MockMvcRequestMatcher implements RequestMatcher { + + @Override + public boolean matches(HttpServletRequest request) { + return request.getAttribute("org.springframework.test.web.servlet.MockMvc.MVC_RESULT_ATTRIBUTE") != null; + } + + } + + static class DispatcherServletRequestMatcher implements RequestMatcher { + + private final HandlerMappingIntrospector introspector; + + DispatcherServletRequestMatcher(HandlerMappingIntrospector introspector) { + this.introspector = introspector; + } + + @Override + public boolean matches(HttpServletRequest request) { + String name = request.getHttpServletMapping().getServletName(); + ServletRegistration registration = request.getServletContext().getServletRegistration(name); + if (registration != null) { + return isDispatcherServlet(registration); + } + // in some testing scenarios, the servlet context is not configured, so fall + // back to introspection + return foundMapping(request); + } + + private boolean isDispatcherServlet(ServletRegistration registration) { + try { + Class clazz = Class.forName(registration.getClassName()); + return DispatcherServlet.class.isAssignableFrom(clazz); + } + catch (ClassNotFoundException ex) { + throw new IllegalStateException(ex); + } + } + + private boolean foundMapping(HttpServletRequest request) { + try { + return this.introspector.getMatchableHandlerMapping(request) != null; + } + catch (Exception ex) { + throw new IllegalStateException(ex); + } + } + + } + + static class MvcDelegatingRequestMatcher implements RequestMatcher { + + private final RequestMatcher ant; + + private final RequestMatcher mvc; + + private final RequestMatcher isMvcRequest; + + MvcDelegatingRequestMatcher(RequestMatcher ant, RequestMatcher mvc, RequestMatcher isMvcRequest) { + this.ant = ant; + this.mvc = mvc; + this.isMvcRequest = isMvcRequest; + } + + RequestMatcher requestMatcher(HttpServletRequest request) { + return (this.isMvcRequest.matches(request)) ? this.mvc : this.ant; + } + + @Override + public boolean matches(HttpServletRequest request) { + return requestMatcher(request).matches(request); + } + + @Override + public MatchResult matcher(HttpServletRequest request) { + return requestMatcher(request).matcher(request); + } + + @Override + public String toString() { + return "MvcDelegating [ant = " + this.ant + ", mvc = " + this.mvc + "]"; + } + + } + + /** + * A matcher implementation that errors if {@link DispatcherServlet} is mapped to a + * path and this matcher does not have a servlet path specified. + */ + static final class StrictMvcRequestMatcher extends MvcRequestMatcher { + + StrictMvcRequestMatcher(HandlerMappingIntrospector introspector, String pattern) { + super(introspector, pattern); + } + + private void validateConfiguration(HttpServletRequest request) { + String requestServletPath = getRequestServletPath(request); + String configuredServletPath = getServletPath(); + Assert.state(requestServletPath == null || configuredServletPath != null, + String.format( + "It appears the Spring MVC servlet path is not root. " + + "Please provide the servlet path %s when constructing MvcRequestMatcherBuilder", + requestServletPath)); + } + + @Override + public boolean matches(HttpServletRequest request) { + validateConfiguration(request); + return super.matches(request); + } + + @Override + public MatchResult matcher(HttpServletRequest request) { + validateConfiguration(request); + return super.matcher(request); + } + + private String getRequestServletPath(HttpServletRequest request) { + HttpServletMapping mapping = request.getHttpServletMapping(); + if (mapping == null) { + // some testing scenarios do not configure a servlet mapping, so we cannot + // validate + return null; + } + if (mapping.getMappingMatch() != MappingMatch.PATH) { + return null; + } + String servletMapping = mapping.getPattern(); + if (servletMapping.length() <= 2) { + // this is either an EXACT or a CONTEXT_ROOT match so we'll ignore + return null; + } + if (!servletMapping.endsWith("/*")) { + // this is an EXACT match so we'll ignore + return null; + } + return servletMapping.substring(0, servletMapping.length() - 2); + } + + } + +} diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcherBuilder.java b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcherBuilder.java new file mode 100644 index 00000000000..86c2cf07ade --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatcherBuilder.java @@ -0,0 +1,40 @@ +/* + * Copyright 2002-2024 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.web.util.matcher; + +import java.util.ArrayList; +import java.util.List; + +import org.springframework.http.HttpMethod; + +public interface RequestMatcherBuilder { + + default RequestMatcher[] requestMatchers(HttpMethod method, String... patterns) { + List requestMatchers = new ArrayList<>(); + for (String pattern : patterns) { + requestMatchers.add(requestMatcher(method, pattern)); + } + return requestMatchers.toArray(RequestMatcher[]::new); + } + + default RequestMatcher[] requestMatchers(String... patterns) { + return requestMatchers(null, patterns); + } + + RequestMatcher requestMatcher(HttpMethod method, String pattern); + +} diff --git a/web/src/test/java/org/springframework/security/web/servlet/MockServletContext.java b/web/src/test/java/org/springframework/security/web/servlet/MockServletContext.java new file mode 100644 index 00000000000..fff01a5f3b0 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/servlet/MockServletContext.java @@ -0,0 +1,149 @@ +/* + * 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.web.servlet; + +import java.util.Arrays; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; + +import jakarta.servlet.MultipartConfigElement; +import jakarta.servlet.Servlet; +import jakarta.servlet.ServletRegistration; +import jakarta.servlet.ServletSecurityElement; + +import org.springframework.lang.NonNull; +import org.springframework.web.servlet.DispatcherServlet; + +public class MockServletContext extends org.springframework.mock.web.MockServletContext { + + private final Map registrations = new LinkedHashMap<>(); + + public static MockServletContext mvc() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/"); + return servletContext; + } + + @NonNull + @Override + public ServletRegistration.Dynamic addServlet(@NonNull String servletName, Class clazz) { + ServletRegistration.Dynamic dynamic = new MockServletRegistration(servletName, clazz); + this.registrations.put(servletName, dynamic); + return dynamic; + } + + @NonNull + @Override + public Map getServletRegistrations() { + return this.registrations; + } + + @Override + public ServletRegistration getServletRegistration(String servletName) { + return this.registrations.get(servletName); + } + + private static class MockServletRegistration implements ServletRegistration.Dynamic { + + private final String name; + + private final Class clazz; + + private final Set mappings = new LinkedHashSet<>(); + + MockServletRegistration(String name, Class clazz) { + this.name = name; + this.clazz = clazz; + } + + @Override + public void setLoadOnStartup(int loadOnStartup) { + + } + + @Override + public Set setServletSecurity(ServletSecurityElement constraint) { + return null; + } + + @Override + public void setMultipartConfig(MultipartConfigElement multipartConfig) { + + } + + @Override + public void setRunAsRole(String roleName) { + + } + + @Override + public void setAsyncSupported(boolean isAsyncSupported) { + + } + + @Override + public Set addMapping(String... urlPatterns) { + this.mappings.addAll(Arrays.asList(urlPatterns)); + return this.mappings; + } + + @Override + public Collection getMappings() { + return this.mappings; + } + + @Override + public String getRunAsRole() { + return null; + } + + @Override + public String getName() { + return this.name; + } + + @Override + public String getClassName() { + return this.clazz.getName(); + } + + @Override + public boolean setInitParameter(String name, String value) { + return false; + } + + @Override + public String getInitParameter(String name) { + return null; + } + + @Override + public Set setInitParameters(Map initParameters) { + return null; + } + + @Override + public Map getInitParameters() { + return null; + } + + } + +} diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherBuilderTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherBuilderTests.java new file mode 100644 index 00000000000..01145680a20 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcherBuilderTests.java @@ -0,0 +1,141 @@ +/* + * Copyright 2012-2024 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.web.servlet.util.matcher; + +import jakarta.servlet.ServletContext; +import jakarta.servlet.http.MappingMatch; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import org.springframework.http.HttpMethod; +import org.springframework.mock.web.MockHttpServletMapping; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.web.servlet.MockServletContext; +import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcherBuilder.MvcDelegatingRequestMatcher; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.web.context.WebApplicationContext; +import org.springframework.web.context.support.GenericWebApplicationContext; +import org.springframework.web.servlet.DispatcherServlet; +import org.springframework.web.servlet.handler.HandlerMappingIntrospector; +import org.springframework.web.servlet.handler.MatchableHandlerMapping; +import org.springframework.web.servlet.handler.RequestMatchResult; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; + +@ExtendWith(MockitoExtension.class) +class MvcRequestMatcherBuilderTests { + + @Mock + HandlerMappingIntrospector introspector; + + @Mock + MatchableHandlerMapping handlerMapping; + + MvcRequestMatcherBuilder builder; + + @BeforeEach + void mocks() { + this.builder = new MvcRequestMatcherBuilder(this.introspector, "/servlet/path"); + } + + @Test + void requestWhenNotDispatcherServletThenUsesAntPath() { + ServletContext servletContext = MockServletContext.mvc(); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/servlet/path"); + MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", "/endpoint"); + MvcDelegatingRequestMatcher matcher = (MvcDelegatingRequestMatcher) this.builder.requestMatcher(HttpMethod.GET, + "/endpoint"); + RequestMatcher delegateMatcher = matcher.requestMatcher(request); + assertThat(delegateMatcher).isInstanceOf(AntPathRequestMatcher.class); + } + + @Test + void requestWhenDispatcherServletThenUsesMvc() { + MockHttpServletRequest request = mvcRequest(); + MvcDelegatingRequestMatcher matcher = (MvcDelegatingRequestMatcher) this.builder.requestMatcher(HttpMethod.GET, + "/endpoint"); + RequestMatcher delegateMatcher = matcher.requestMatcher(request); + assertThat(delegateMatcher).isInstanceOf(MvcRequestMatcher.class); + } + + @Test + void mvcRequestWhenNoMvcMappingThenDoesNotMatch() { + MockHttpServletRequest request = mvcRequest(); + MvcDelegatingRequestMatcher matcher = (MvcDelegatingRequestMatcher) this.builder.requestMatcher(HttpMethod.GET, + "/endpoint"); + RequestMatcher delegateMatcher = matcher.requestMatcher(request); + assertThat(delegateMatcher.matches(request)).isFalse(); + } + + @Test + void mvcRequestWhenMvcMappingThenMatches() throws Exception { + given(this.introspector.getMatchableHandlerMapping(any())).willReturn(this.handlerMapping); + given(this.handlerMapping.match(any(), any())).willReturn(mock(RequestMatchResult.class)); + MockHttpServletRequest request = mvcRequest(); + MvcDelegatingRequestMatcher matcher = (MvcDelegatingRequestMatcher) this.builder.requestMatcher(HttpMethod.GET, + "/endpoint"); + RequestMatcher delegateMatcher = matcher.requestMatcher(request); + assertThat(delegateMatcher.matcher(request).isMatch()).isTrue(); + } + + @Test + void mvcRequestWhenDispatcherServletPathThenRequiresServletPath() { + MvcRequestMatcherBuilder builder = new MvcRequestMatcherBuilder(this.introspector); + MockHttpServletRequest request = mvcRequest(); + MvcDelegatingRequestMatcher matcher = (MvcDelegatingRequestMatcher) builder.requestMatcher(HttpMethod.GET, + "/endpoint"); + assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> matcher.matcher(request)); + } + + @Test + void mvcReqwestWhenMockMvcThenUsesMvc() throws Exception { + WebApplicationContext wac = new GenericWebApplicationContext(); + MockMvc mvc = MockMvcBuilders.standaloneSetup(wac).build(); + MockHttpServletRequest request = mvc.perform(get("/endpoint")).andReturn().getRequest(); + MvcDelegatingRequestMatcher matcher = (MvcDelegatingRequestMatcher) this.builder.requestMatcher(HttpMethod.GET, + "/endpoint"); + RequestMatcher delegateMatcher = matcher.requestMatcher(request); + assertThat(delegateMatcher).isInstanceOf(MvcRequestMatcher.class); + } + + private MockServletContext mvcWithServletPath() { + MockServletContext servletContext = MockServletContext.mvc(); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/servlet/path"); + return servletContext; + } + + private MockHttpServletRequest mvcRequest() { + ServletContext servletContext = mvcWithServletPath(); + MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", "/servlet/path/endpoint"); + request.setServletPath("/servlet/path"); + request.setHttpServletMapping( + new MockHttpServletMapping("/servlet/path", "/servlet/path/*", "dispatcherServlet", MappingMatch.PATH)); + return request; + } + +}