From ae29d07ee26065b71a7ef1a2b1a6e10fc77010e8 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 16 Jan 2025 11:57:56 -0700 Subject: [PATCH] Add Servlet Path support to Java DSL Closes gh-16430 --- .../web/AbstractRequestMatcherRegistry.java | 37 ++- .../AbstractRequestMatcherRegistryTests.java | 9 + .../AuthorizeHttpRequestsConfigurerTests.java | 40 +++ docs/modules/ROOT/pages/migration-7/web.adoc | 20 ++ .../authorize-http-requests.adoc | 109 ++++++-- .../matcher/ServletRegistrationsSupport.java | 77 ++++++ .../ServletRequestMatcherBuilders.java | 249 ++++++++++++++++++ .../ServletRequestMatcherBuildersTests.java | 171 ++++++++++++ 8 files changed, 679 insertions(+), 33 deletions(-) create mode 100644 web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRegistrationsSupport.java create mode 100644 web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuilders.java create mode 100644 web/src/test/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuildersTests.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 4f849f86fb..2181da01ae 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 @@ -49,6 +49,7 @@ import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatche 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; @@ -74,6 +75,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; @@ -217,13 +220,9 @@ public abstract class AbstractRequestMatcherRegistry { 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.pattern(method, patterns)); } private boolean anyPathsDontStartWithLeadingSlash(String... patterns) { @@ -264,11 +263,14 @@ public abstract class AbstractRequestMatcherRegistry { } private static String computeErrorMessage(Collection registrations) { - String template = "This method cannot decide whether these patterns are Spring MVC patterns or not. " - + "If this endpoint is a Spring MVC endpoint, please use requestMatchers(MvcRequestMatcher); " - + "otherwise, please use requestMatchers(AntPathRequestMatcher).\n\n" - + "This is because there is more than one mappable servlet in your servlet context: %s.\n\n" - + "For each MvcRequestMatcher, call MvcRequestMatcher#setServletPath to indicate the servlet path."; + String template = """ + This method cannot decide whether these patterns are Spring MVC patterns or not. \ + This is because there is more than one mappable servlet in your servlet context: %s. + + To address this, please create one ServletRequestMatcherBuilder#servletPath for each servlet that has \ + authorized endpoints and use them to construct request matchers manually. \ + If all your URIs are unambiguous, then you can simply publish one ServletRequestMatcherBuilders#servletPath as \ + a @Bean and Spring Security will use it for all URIs"""; Map> mappings = new LinkedHashMap<>(); for (ServletRegistration registration : registrations) { mappings.put(registration.getClassName(), registration.getMappings()); @@ -402,6 +404,17 @@ public abstract class AbstractRequestMatcherRegistry { } + class DefaultRequestMatcherBuilder implements RequestMatcherBuilder { + + @Override + public RequestMatcher pattern(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 70f383c203..4a7ae59d07 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 jakarta.servlet.Servlet; 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.AntPathRequestMatcher; 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 class AbstractRequestMatcherRegistryTests { given(given).willReturn(postProcessors); given(postProcessors.getObject()).willReturn(NO_OP_OBJECT_POST_PROCESSOR); given(this.context.getServletContext()).willReturn(MockServletContext.mvc()); + ObjectProvider requestMatcherFactories = new ObjectProvider<>() { + @Override + public RequestMatcherBuilder getObject() throws BeansException { + return AbstractRequestMatcherRegistryTests.this.matcherRegistry.new DefaultRequestMatcherBuilder(); + } + }; + given(this.context.getBeanProvider(RequestMatcherBuilder.class)).willReturn(requestMatcherFactories); 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 41850d6756..1d36d3f638 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.AuthorizationFilter; 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.PathPatternRequestMatcher; +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.PathVariable; 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 class AuthorizeHttpRequestsConfigurerTests { 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 @@ public class AuthorizeHttpRequestsConfigurerTests { void rootPost() { } + @GetMapping("/path") + void path() { + } + } @Configuration @@ -1317,4 +1337,24 @@ public class AuthorizeHttpRequestsConfigurerTests { } + @Configuration + @EnableWebSecurity + @EnableWebMvc + static class MvcRequestMatcherBuilderConfig { + + @Bean + RequestMatcherBuilder servletPath() { + return PathPatternRequestMatcher.builder().servletPath("/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/migration-7/web.adoc b/docs/modules/ROOT/pages/migration-7/web.adoc index 024d560449..1c5ac51566 100644 --- a/docs/modules/ROOT/pages/migration-7/web.adoc +++ b/docs/modules/ROOT/pages/migration-7/web.adoc @@ -102,3 +102,23 @@ Xml:: ---- ====== + +== Favor PathPatternRequestMatcher + +`MvcRequestMatcher` is deprecated in 6.5. +XML, Kotlin, and Java will all favor `PathPatternRequestMatcher` by default in 7.0. + +If you aren't already publishing a `RequestMatcherBuilder` bean, you can prepare for this change in defaults by publishing the following bean: + +[source,java] +---- +@Bean +RequestMatcherBuilder favorPathPattern() { + return ServletRequestMatcherBuilders.deducePath(); +} +---- + +This static factory aligns with the Spring Security defaults for request matchers except that it uses `PathPatternRequestMatcher` instead. +It reflects what the default will be in Spring Security 7. + +If this creates problems for you and you cannot use this bean at the moment, then change each of your `String` URI authorization rules to xref:servlet/authorization/authorize-http-requests.adoc#security-matchers[use a `RequestMatcher`]. 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 4eaf5f3d5e..9e0518f8e0 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc @@ -577,15 +577,11 @@ http { ====== [[match-by-mvc]] -=== Using an MvcRequestMatcher +=== Matching by Servlet Path Generally speaking, you can use `requestMatchers(String)` as demonstrated above. -However, if you map Spring MVC to a different servlet path, then you need to account for that in your security configuration. - -For example, if Spring MVC is mapped to `/spring-mvc` instead of `/` (the default), then you may have an endpoint like `/spring-mvc/my/controller` that you want to authorize. - -You need to use `MvcRequestMatcher` to split the servlet path and the controller path in your configuration like so: +However, if you have authorization rules from multiple servlets, you need to specify those: .Match by MvcRequestMatcher [tabs] @@ -594,16 +590,14 @@ Java:: + [source,java,role="primary"] ---- -@Bean -MvcRequestMatcher.Builder mvc(HandlerMappingIntrospector introspector) { - return new MvcRequestMatcher.Builder(introspector).servletPath("/spring-mvc"); -} +import static org.springframework.security.web.servlet.util.matcher.ServletRequestMatcherBuilders.servletPath; @Bean -SecurityFilterChain appEndpoints(HttpSecurity http, MvcRequestMatcher.Builder mvc) { +SecurityFilterChain appEndpoints(HttpSecurity http) { http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(mvc.pattern("/my/controller/**")).hasAuthority("controller") + .requestMatchers(servletPath("/spring-mvc").pattern("/admin/**")).hasAuthority("admin") + .requestMatchers(servletPath("/spring-mvc").pattern("/my/controller/**")).hasAuthority("controller") .anyRequest().authenticated() ); @@ -616,17 +610,15 @@ Kotlin:: [source,kotlin,role="secondary"] ---- @Bean -fun mvc(introspector: HandlerMappingIntrospector): MvcRequestMatcher.Builder = - MvcRequestMatcher.Builder(introspector).servletPath("/spring-mvc"); - -@Bean -fun appEndpoints(http: HttpSecurity, mvc: MvcRequestMatcher.Builder): SecurityFilterChain = +fun appEndpoints(http: HttpSecurity): SecurityFilterChain { http { authorizeHttpRequests { - authorize(mvc.pattern("/my/controller/**"), hasAuthority("controller")) + authorize("/spring-mvc", "/admin/**", hasAuthority("admin")) + authorize("/spring-mvc", "/my/controller/**", hasAuthority("controller")) authorize(anyRequest, authenticated) } } +} ---- Xml:: @@ -634,16 +626,91 @@ Xml:: [source,xml,role="secondary"] ---- + ---- ====== -This need can arise in at least two different ways: +The primary reason for this is that Spring MVC URIs are relative to the servlet. +In other words, an authorization rule usually doesn't include the servlet path. -* 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) +Other URIs may include the servlet path. +Because of that, the best practice is to always supply the servlet path when your application has more than one servlet. + +==== But I do only have one servlet, why is Spring Security complaining? + +Sometimes, application containers include additional servlets. +This can cause some confusion when you know as the developer that the only authorization rules you are writing are for your one servlet (Spring MVC, for example) + +In this case, in the Java DSL you can publish a `ServletRequestMatcherBuilders#servletPath` as a `@Bean` and Spring Security will use it for all URIs. + +For example, the above Java sample can be rewritten as: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +RequestMatcherBuilder mvc() { + return ServeltRequestMatcherBuilders.servletPath("/spring-mvc"); +} + +@Bean +SecurityFilterChain security(HttpSecurity http) throws Exception { + http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers("/admin/**").hasAuthority("admin") + .requestMatchers("/my/controller/**").hasAuthority("controller") + .anyRequest().authenticated() + ); + return http.build(); +} +---- +====== + +[TIP] +==== +If you are a Spring Boot application, you may be able to publish the above bean like so: + +[source,java] +---- +@Bean +RequestMatcherBuilder mvc(WebMvcProperties properties) { + return ServletRequestMatcherBuilders.servletPath(proeprties.getServlet().getPath()); +} +---- +==== + +This same strategy is useful when it comes to static resources. +You can permit these by using Spring Boot's `RequestMatchers` static factory like so: + +[tabs] +====== +Java:: ++ +[source,java] +---- +@Bean +RequestMatcherBuilder mvc() { + return ServletRequestMatcherBuilders.servletPath("/mvc"); +} + +@Bean +SecurityFilterChain security(HttpSecurity http) throws Exception { + http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(PathRequest.toStaticResources().atCommonLocations()).permitAll() + .requestMatchers("/my/**", "/app/**", "/requests/**").hasAuthority("app") + ) +} +---- +====== + +Since `atCommonLocations` returns instances of `RequestMatcher`, this technique allows you to have all your `String`-based authorizations relative to the globally-configured `ServletRequestMatcherBuilders#servletPath`. [[match-by-custom]] === Using a Custom Matcher diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRegistrationsSupport.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRegistrationsSupport.java new file mode 100644 index 0000000000..eb22aea69d --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRegistrationsSupport.java @@ -0,0 +1,77 @@ +/* + * Copyright 2002-2025 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 java.util.ArrayList; +import java.util.Collection; +import java.util.Map; + +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletRegistration; + +import org.springframework.util.ClassUtils; + +class ServletRegistrationsSupport { + + private final Collection registrations; + + ServletRegistrationsSupport(ServletContext servletContext) { + Map registrations = servletContext.getServletRegistrations(); + Collection mappings = new ArrayList<>(); + for (Map.Entry entry : registrations.entrySet()) { + if (!entry.getValue().getMappings().isEmpty()) { + for (String mapping : entry.getValue().getMappings()) { + mappings.add(new RegistrationMapping(entry.getValue(), mapping)); + } + } + } + this.registrations = mappings; + } + + Collection dispatcherServletMappings() { + Collection mappings = new ArrayList<>(); + for (RegistrationMapping registration : this.registrations) { + if (registration.isDispatcherServlet()) { + mappings.add(registration); + } + } + return mappings; + } + + Collection mappings() { + return this.registrations; + } + + record RegistrationMapping(ServletRegistration registration, String mapping) { + boolean isDispatcherServlet() { + Class dispatcherServlet = ClassUtils + .resolveClassName("org.springframework.web.servlet.DispatcherServlet", null); + try { + Class clazz = Class.forName(this.registration.getClassName()); + return dispatcherServlet.isAssignableFrom(clazz); + } + catch (ClassNotFoundException ex) { + return false; + } + } + + boolean isDefault() { + return "/".equals(this.mapping); + } + } + +} diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuilders.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuilders.java new file mode 100644 index 0000000000..43e0fe929d --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuilders.java @@ -0,0 +1,249 @@ +/* + * Copyright 2002-2025 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 java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletRegistration; +import jakarta.servlet.http.HttpServletRequest; + +import org.springframework.http.HttpMethod; +import org.springframework.security.web.servlet.util.matcher.ServletRegistrationsSupport.RegistrationMapping; +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; + +/** + * A {@link RequestMatcherBuilder} for specifying the servlet path separately from the + * rest of the URI. This is helpful when you have more than one servlet. + * + *

+ * For example, if Spring MVC is deployed to `/mvc` and another servlet to `/other`, then + * you can do + *

+ * + * + * http + * .authorizeHttpRequests((authorize) -> authorize + * .requestMatchers(servletPath("/mvc").pattern("/my/**", "/controller/**", "/endpoints/**")).hasAuthority(... + * .requestMatchers(servletPath("/other").pattern("/my/**", "/non-mvc/**", "/endpoints/**")).hasAuthority(... + * } + * ... + * + * + * @author Josh Cummings + * @since 6.5 + */ +public final class ServletRequestMatcherBuilders { + + private ServletRequestMatcherBuilders() { + } + + /** + * Create {@link RequestMatcher}s that will only match URIs against the default + * servlet. + * @return a {@link ServletRequestMatcherBuilders} that matches URIs mapped to the + * default servlet + */ + public static RequestMatcherBuilder defaultServlet() { + return servletPathInternal(""); + } + + /** + * Create {@link RequestMatcher}s that will only match URIs against the given servlet + * path + * + *

+ * The path must be of the format {@code /path}. It should not end in `/` or `/*`, nor + * should it be a file extension. To specify the default servlet, use + * {@link #defaultServlet()}. + *

+ * @return a {@link ServletRequestMatcherBuilders} that matches URIs mapped to the + * given servlet path + */ + public static RequestMatcherBuilder servletPath(String servletPath) { + Assert.notNull(servletPath, "servletPath cannot be null"); + Assert.isTrue(servletPath.startsWith("/"), "servletPath must start with '/'"); + Assert.isTrue(!servletPath.endsWith("/"), "servletPath must not end with '/'"); + Assert.isTrue(!servletPath.endsWith("/*"), "servletPath must not end with '/*'"); + return servletPathInternal(servletPath); + } + + private static RequestMatcherBuilder servletPathInternal(String servletPath) { + return (method, pattern) -> { + Assert.notNull(pattern, "pattern cannot be null"); + Assert.isTrue(pattern.startsWith("/"), "pattern must start with '/'"); + return PathPatternRequestMatcher.builder().servletPath(servletPath).pattern(method, pattern); + }; + } + + /** + * Create {@link RequestMatcher}s that will deduce the servlet path by testing the + * given patterns as relative and absolute. If the target servlet is + * {@link DispatcherServlet}, then it tests the pattern as relative to the servlet + * path; otherwise, it tests the pattern as absolute + * @return a {@link ServletRequestMatcherBuilders} that deduces the servlet path at + * request time + */ + public static RequestMatcherBuilder servletPathDeducing() { + return (method, pattern) -> { + Assert.notNull(pattern, "pattern cannot be null"); + Assert.isTrue(pattern.startsWith("/"), "pattern must start with '/'"); + return new PathDeducingRequestMatcher(method, pattern); + }; + } + + static final class PathDeducingRequestMatcher implements RequestMatcher { + + private static final RequestMatcher isMockMvc = (request) -> request + .getAttribute("org.springframework.test.web.servlet.MockMvc.MVC_RESULT_ATTRIBUTE") != null; + + private static final RequestMatcher isDispatcherServlet = (request) -> { + String name = request.getHttpServletMapping().getServletName(); + ServletContext servletContext = request.getServletContext(); + ServletRegistration registration = servletContext.getServletRegistration(name); + Assert.notNull(registration, () -> computeErrorMessage(servletContext.getServletRegistrations().values())); + String mapping = request.getHttpServletMapping().getPattern(); + return new RegistrationMapping(registration, mapping).isDispatcherServlet(); + }; + + private final Map delegates = new ConcurrentHashMap<>(); + + private HttpMethod method; + + private String pattern; + + PathDeducingRequestMatcher(HttpMethod method, String pattern) { + this.method = method; + this.pattern = pattern; + } + + RequestMatcher requestMatcher(HttpServletRequest request) { + return this.delegates.computeIfAbsent(request.getServletContext(), (servletContext) -> { + PathPatternRequestMatcher absolute = PathPatternRequestMatcher.builder() + .pattern(this.method, this.pattern); + PathPatternRequestMatcher relative = PathPatternRequestMatcher.builder() + .pattern(this.method, this.pattern); + ServletRegistrationsSupport registrations = new ServletRegistrationsSupport(servletContext); + Collection mappings = registrations.mappings(); + if (mappings.isEmpty()) { + relative.setServletPath(PathPatternRequestMatcher.ANY_SERVLET); + return new EitherRequestMatcher(relative, absolute, isMockMvc); + } + Collection dispatcherServletMappings = registrations.dispatcherServletMappings(); + if (dispatcherServletMappings.isEmpty()) { + relative.setServletPath(PathPatternRequestMatcher.ANY_SERVLET); + return new EitherRequestMatcher(relative, absolute, isMockMvc); + } + if (dispatcherServletMappings.size() > 1) { + String errorMessage = computeErrorMessage(servletContext.getServletRegistrations().values()); + throw new IllegalArgumentException(errorMessage); + } + RegistrationMapping dispatcherServlet = dispatcherServletMappings.iterator().next(); + if (mappings.size() > 1 && !dispatcherServlet.isDefault()) { + String errorMessage = computeErrorMessage(servletContext.getServletRegistrations().values()); + throw new IllegalArgumentException(errorMessage); + } + if (dispatcherServlet.isDefault()) { + relative.setServletPath(""); + if (mappings.size() == 1) { + return relative; + } + return new EitherRequestMatcher(relative, absolute, + new OrRequestMatcher(isMockMvc, isDispatcherServlet)); + } + String mapping = dispatcherServlet.mapping(); + relative.setServletPath(mapping.substring(0, mapping.length() - 2)); + return relative; + }); + } + + @Override + public boolean matches(HttpServletRequest request) { + return matcher(request).isMatch(); + } + + @Override + public MatchResult matcher(HttpServletRequest request) { + return requestMatcher(request).matcher(request); + } + + private static String computeErrorMessage(Collection registrations) { + String template = """ + This method cannot decide whether these patterns are Spring MVC patterns or not. \ + This is because there is more than one mappable servlet in your servlet context: %s. + + To address this, please create one ServletRequestMatcherBuilder#servletPath for each servlet that has \ + authorized endpoints and use them to construct request matchers manually. \ + If all your URIs are unambiguous, then you can simply publish one ServletRequestMatcherBuilders#servletPath as \ + a @Bean and Spring Security will use it for all URIs"""; + Map> mappings = new LinkedHashMap<>(); + for (ServletRegistration registration : registrations) { + mappings.put(registration.getClassName(), registration.getMappings()); + } + return String.format(template, mappings); + } + + @Override + public String toString() { + return "PathDeducingRequestMatcher [delegates = " + this.delegates + "]"; + } + + } + + static class EitherRequestMatcher implements RequestMatcher { + + final RequestMatcher right; + + final RequestMatcher left; + + final RequestMatcher test; + + EitherRequestMatcher(RequestMatcher right, RequestMatcher left, RequestMatcher test) { + this.left = left; + this.right = right; + this.test = test; + } + + RequestMatcher requestMatcher(HttpServletRequest request) { + return this.test.matches(request) ? this.right : this.left; + } + + @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 "Either [" + "left = " + this.left + ", right = " + this.right + "]"; + } + + } + +} diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuildersTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuildersTests.java new file mode 100644 index 0000000000..a37294e78a --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuildersTests.java @@ -0,0 +1,171 @@ +/* + * Copyright 2002-2025 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.Servlet; +import org.junit.jupiter.api.Test; + +import org.springframework.http.HttpMethod; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.web.servlet.MockServletContext; +import org.springframework.security.web.servlet.TestMockHttpServletMappings; +import org.springframework.security.web.servlet.util.matcher.ServletRequestMatcherBuilders.EitherRequestMatcher; +import org.springframework.security.web.servlet.util.matcher.ServletRequestMatcherBuilders.PathDeducingRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcherBuilder; +import org.springframework.web.servlet.DispatcherServlet; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +class ServletRequestMatcherBuildersTests { + + @Test + void patternWhenServletPathThenUsesPathPattern() { + RequestMatcherBuilder builder = ServletRequestMatcherBuilders.servletPath("/servlet/path"); + RequestMatcher requestMatcher = builder.pattern(HttpMethod.GET, "/endpoint"); + assertThat(requestMatcher).isInstanceOf(PathPatternRequestMatcher.class); + } + + @Test + void patternWhenDefaultServletThenUsesPathPattern() { + RequestMatcherBuilder builder = ServletRequestMatcherBuilders.defaultServlet(); + RequestMatcher requestMatcher = builder.pattern(HttpMethod.GET, "/endpoint"); + assertThat(requestMatcher).isInstanceOf(PathPatternRequestMatcher.class); + } + + @Test + void patternWhenServletPathDeducingThenUsesComposite() { + RequestMatcherBuilder builder = ServletRequestMatcherBuilders.servletPathDeducing(); + RequestMatcher requestMatcher = builder.pattern(HttpMethod.GET, "/endpoint"); + assertThat(requestMatcher).isInstanceOf(PathDeducingRequestMatcher.class); + } + + @Test + void requestMatchersWhenAmbiguousServletsThenException() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/"); + servletContext.addServlet("servletTwo", DispatcherServlet.class).addMapping("/servlet/*"); + RequestMatcher requestMatcher = ServletRequestMatcherBuilders.servletPathDeducing().pattern("/**"); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> requestMatcher.matches(new MockHttpServletRequest(servletContext))); + } + + @Test + void requestMatchersWhenMultipleDispatcherServletMappingsThenException() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/", "/mvc/*"); + RequestMatcher requestMatcher = ServletRequestMatcherBuilders.servletPathDeducing().pattern("/**"); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> requestMatcher.matcher(new MockHttpServletRequest(servletContext))); + } + + @Test + void requestMatchersWhenPathDispatcherServletAndOtherServletsThenException() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/mvc/*"); + servletContext.addServlet("default", Servlet.class).addMapping("/"); + RequestMatcher requestMatcher = ServletRequestMatcherBuilders.servletPathDeducing().pattern("/**"); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> requestMatcher.matcher(new MockHttpServletRequest(servletContext))); + } + + @Test + void requestMatchersWhenUnmappableServletsThenSkips() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/"); + servletContext.addServlet("servletTwo", Servlet.class); + PathDeducingRequestMatcher requestMatcher = (PathDeducingRequestMatcher) ServletRequestMatcherBuilders + .servletPathDeducing() + .pattern("/**"); + RequestMatcher deduced = requestMatcher.requestMatcher(new MockHttpServletRequest(servletContext)); + assertThat(deduced).isInstanceOf(PathPatternRequestMatcher.class); + } + + @Test + void requestMatchersWhenOnlyDispatcherServletThenAllows() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/mvc/*"); + PathDeducingRequestMatcher requestMatcher = (PathDeducingRequestMatcher) ServletRequestMatcherBuilders + .servletPathDeducing() + .pattern("/**"); + RequestMatcher deduced = requestMatcher.requestMatcher(new MockHttpServletRequest(servletContext)); + assertThat(deduced).isInstanceOf(PathPatternRequestMatcher.class); + } + + @Test + void requestMatchersWhenImplicitServletsThenAllows() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("defaultServlet", Servlet.class); + servletContext.addServlet("jspServlet", Servlet.class).addMapping("*.jsp", "*.jspx"); + servletContext.addServlet("dispatcherServlet", DispatcherServlet.class).addMapping("/"); + PathDeducingRequestMatcher requestMatcher = (PathDeducingRequestMatcher) ServletRequestMatcherBuilders + .servletPathDeducing() + .pattern("/**"); + RequestMatcher deduced = requestMatcher.requestMatcher(new MockHttpServletRequest(servletContext)); + assertThat(deduced).isInstanceOf(EitherRequestMatcher.class); + } + + @Test + void requestMatchersWhenPathBasedNonDispatcherServletThenAllows() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("path", Servlet.class).addMapping("/services/*"); + servletContext.addServlet("default", DispatcherServlet.class).addMapping("/"); + PathDeducingRequestMatcher requestMatcher = (PathDeducingRequestMatcher) ServletRequestMatcherBuilders + .servletPathDeducing() + .pattern("/services/*"); + RequestMatcher deduced = requestMatcher.requestMatcher(new MockHttpServletRequest(servletContext)); + assertThat(deduced).isInstanceOf(EitherRequestMatcher.class); + MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", "/services/endpoint"); + request.setHttpServletMapping(TestMockHttpServletMappings.defaultMapping()); + request.setServletPath(""); + assertThat(deduced.matcher(request).isMatch()).isTrue(); + request.setHttpServletMapping(TestMockHttpServletMappings.path(request, "/services")); + request.setServletPath("/services"); + request.setPathInfo("/endpoint"); + assertThat(deduced.matcher(request).isMatch()).isTrue(); + } + + @Test + void matchesWhenDispatcherServletThenMvc() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("default", DispatcherServlet.class).addMapping("/"); + servletContext.addServlet("path", Servlet.class).addMapping("/services/*"); + PathDeducingRequestMatcher requestMatcher = (PathDeducingRequestMatcher) ServletRequestMatcherBuilders + .servletPathDeducing() + .pattern("/services/*"); + MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", "/services/endpoint"); + request.setHttpServletMapping(TestMockHttpServletMappings.defaultMapping()); + EitherRequestMatcher either = (EitherRequestMatcher) requestMatcher.requestMatcher(request); + RequestMatcher deduced = either.requestMatcher(request); + assertThat(deduced).isEqualTo(either.right); + request.setHttpServletMapping(TestMockHttpServletMappings.path(request, "/services")); + deduced = either.requestMatcher(request); + assertThat(deduced).isEqualTo(either.left); + } + + @Test + void matchesWhenNoMappingThenException() { + MockServletContext servletContext = new MockServletContext(); + servletContext.addServlet("default", DispatcherServlet.class).addMapping("/"); + servletContext.addServlet("path", Servlet.class).addMapping("/services/*"); + MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", "/services/endpoint"); + RequestMatcher requestMatcher = ServletRequestMatcherBuilders.servletPathDeducing().pattern("/services/*"); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> requestMatcher.matcher(request)); + } + +}