From 9f74c129856f76ee04ef30c76adc38ca82eb812a Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 16 Jan 2025 11:15:38 -0700 Subject: [PATCH 01/10] Add PathPatternRequestMatcher Closes gh-16429 --- .../matcher/PathPatternRequestMatcher.java | 127 ++++++++++++++++++ .../PathPatternRequestMatcherTests.java | 96 +++++++++++++ 2 files changed, 223 insertions(+) create mode 100644 web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java create mode 100644 web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java new file mode 100644 index 00000000000..402ca5fafd3 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java @@ -0,0 +1,127 @@ +/* + * 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.Objects; + +import jakarta.servlet.http.HttpServletRequest; + +import org.springframework.http.server.PathContainer; +import org.springframework.http.server.RequestPath; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatchers; +import org.springframework.web.util.ServletRequestPathUtils; +import org.springframework.web.util.pattern.PathPattern; +import org.springframework.web.util.pattern.PathPatternParser; + +/** + * A {@link RequestMatcher} that uses {@link PathPattern}s to match against each + * {@link HttpServletRequest}. The provided path should be relative to the servlet (that + * is, it should exclude any context or servlet path). + * + *

+ * To also match the servlet, please see {@link RequestMatchers#servlet} + * + *

+ * Note that the {@link org.springframework.web.servlet.HandlerMapping} that contains the + * related URI patterns must be using the same + * {@link org.springframework.web.util.pattern.PathPatternParser} configured in this + * class. + *

+ * + * @author Josh Cummings + * @since 6.5 + */ +public final class PathPatternRequestMatcher implements RequestMatcher { + + private final PathPattern pattern; + + /** + * Creates a {@link PathPatternRequestMatcher} that uses the provided {@code pattern}. + *

+ * The {@code pattern} should be relative to the servlet path + *

+ * @param pattern the pattern used to match + */ + public PathPatternRequestMatcher(PathPattern pattern) { + this.pattern = pattern; + } + + /** + * Creates a {@link PathPatternRequestMatcher} that uses the provided {@code pattern}, + * parsing it with {@link PathPatternParser#defaultInstance}. + *

+ * The {@code pattern} should be relative to the servlet path + *

+ * @param pattern the pattern used to match + */ + public static PathPatternRequestMatcher pathPattern(String pattern) { + PathPatternParser parser = PathPatternParser.defaultInstance; + PathPattern pathPattern = parser.parse(pattern); + return new PathPatternRequestMatcher(pathPattern); + } + + /** + * {@inheritDoc} + */ + @Override + public boolean matches(HttpServletRequest request) { + return matcher(request).isMatch(); + } + + /** + * {@inheritDoc} + */ + @Override + public MatchResult matcher(HttpServletRequest request) { + PathContainer path = getRequestPath(request).pathWithinApplication(); + PathPattern.PathMatchInfo info = this.pattern.matchAndExtract(path); + return (info != null) ? MatchResult.match(info.getUriVariables()) : MatchResult.notMatch(); + } + + private RequestPath getRequestPath(HttpServletRequest request) { + return ServletRequestPathUtils.parseAndCache(request); + } + + /** + * {@inheritDoc} + */ + @Override + public boolean equals(Object o) { + if (!(o instanceof PathPatternRequestMatcher that)) { + return false; + } + return Objects.equals(this.pattern, that.pattern); + } + + /** + * {@inheritDoc} + */ + @Override + public int hashCode() { + return Objects.hash(this.pattern); + } + + /** + * {@inheritDoc} + */ + @Override + public String toString() { + return "PathPattern [" + this.pattern + "]"; + } + +} diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java new file mode 100644 index 00000000000..bd8c3fde66d --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java @@ -0,0 +1,96 @@ +/* + * 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 org.junit.jupiter.api.Test; + +import org.springframework.http.HttpMethod; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatchers; +import org.springframework.web.util.ServletRequestPathUtils; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@link PathPatternRequestMatcher} + */ +public class PathPatternRequestMatcherTests { + + @Test + void matcherWhenPatternMatchesRequestThenMatchResult() { + RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/uri"); + assertThat(matcher.matches(request("/uri"))).isTrue(); + } + + @Test + void matcherWhenPatternContainsPlaceholdersThenMatchResult() { + RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/uri/{username}"); + assertThat(matcher.matcher(request("/uri/bob")).getVariables()).containsEntry("username", "bob"); + } + + @Test + void matcherWhenOnlyPathInfoMatchesThenMatches() { + RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/uri"); + assertThat(matcher.matches(request("GET", "/mvc/uri", "/mvc"))).isTrue(); + } + + @Test + void matcherWhenUriContainsServletPathThenNoMatch() { + RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/mvc/uri"); + assertThat(matcher.matches(request("GET", "/mvc/uri", "/mvc"))).isFalse(); + } + + @Test + void matcherWhenSameMethodThenMatchResult() { + RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).uris("/uri").matcher(); + assertThat(matcher.matches(request("/uri"))).isTrue(); + } + + @Test + void matcherWhenDifferentPathThenNoMatch() { + RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).uris("/uri").matcher(); + assertThat(matcher.matches(request("GET", "/urj", ""))).isFalse(); + } + + @Test + void matcherWhenDifferentMethodThenNoMatch() { + RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).uris("/uri").matcher(); + assertThat(matcher.matches(request("POST", "/mvc/uri", "/mvc"))).isFalse(); + } + + @Test + void matcherWhenNoMethodThenMatches() { + RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/uri"); + assertThat(matcher.matches(request("POST", "/uri", ""))).isTrue(); + assertThat(matcher.matches(request("GET", "/uri", ""))).isTrue(); + } + + MockHttpServletRequest request(String uri) { + MockHttpServletRequest request = new MockHttpServletRequest("GET", uri); + ServletRequestPathUtils.parseAndCache(request); + return request; + } + + MockHttpServletRequest request(String method, String uri, String servletPath) { + MockHttpServletRequest request = new MockHttpServletRequest(method, uri); + request.setServletPath(servletPath); + ServletRequestPathUtils.parseAndCache(request); + return request; + } + +} From 9b0b5066bef431bddc49ce4d480d5b819c4fbd7e 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 02/10] Add RequestMatchers.Builder This static factory simplifes the creation of RequestMatchers that specify the servlet path and other request elements Closes gh-16430 --- .../web/AbstractRequestMatcherRegistry.java | 27 +- .../AuthorizeHttpRequestsConfigurerTests.java | 40 +++ docs/modules/ROOT/pages/migration-7/web.adoc | 9 + .../authorize-http-requests.adoc | 57 ++-- .../web/util/matcher/RequestMatchers.java | 252 ++++++++++++++++++ .../util/matcher/RequestMatchersTests.java | 51 ++++ 6 files changed, 409 insertions(+), 27 deletions(-) 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 4f849f86fb1..fb69ac7b3de 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * 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. @@ -179,6 +179,19 @@ public C requestMatchers(RequestMatcher... requestMatchers) { return chainRequestMatchers(Arrays.asList(requestMatchers)); } + /** + * Register the {@link RequestMatcher} represented by this builder + * @param builder the + * {@link org.springframework.security.web.util.matcher.RequestMatchers.Builder} to + * use + * @return the object that is chained after creating the {@link RequestMatcher} + * @since 6.5 + */ + public C requestMatchers(org.springframework.security.web.util.matcher.RequestMatchers.Builder builder) { + Assert.state(!this.anyRequestConfigured, "Can't configure requestMatchers after anyRequest"); + return chainRequestMatchers(List.of(builder.matcher())); + } + /** *

* If the {@link HandlerMappingIntrospector} is available in the classpath, maps to an @@ -264,11 +277,13 @@ private RequestMatcher resolve(AntPathRequestMatcher ant, MvcRequestMatcher mvc, } 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 RequestMatchers#servlet for each servlet that has \ + authorized endpoints and use them to construct request matchers manually. + """; Map> mappings = new LinkedHashMap<>(); for (ServletRegistration registration : registrations) { mappings.put(registration.getClassName(), registration.getMappings()); 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..87096ac59e2 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,7 @@ 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.util.matcher.RequestMatchers; 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 +73,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 +669,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 +1277,10 @@ void rootGet() { void rootPost() { } + @GetMapping("/path") + void path() { + } + } @Configuration @@ -1317,4 +1336,25 @@ SecurityObservationSettings observabilityDefaults() { } + @Configuration + @EnableWebSecurity + @EnableWebMvc + static class MvcRequestMatcherBuilderConfig { + + @Bean + SecurityFilterChain security(HttpSecurity http) throws Exception { + RequestMatchers.Builder mvc = RequestMatchers.servlet("/mvc"); + // @formatter:off + http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(mvc.uris("/path/**")).hasRole("USER") + ) + .httpBasic(withDefaults()); + // @formatter:on + + return http.build(); + } + + } + } diff --git a/docs/modules/ROOT/pages/migration-7/web.adoc b/docs/modules/ROOT/pages/migration-7/web.adoc index 024d5604494..b928b057017 100644 --- a/docs/modules/ROOT/pages/migration-7/web.adoc +++ b/docs/modules/ROOT/pages/migration-7/web.adoc @@ -102,3 +102,12 @@ Xml:: ---- ====== + +== Use Absolute Authorization URIs + +The Java DSL now requires that all URIs be absolute (less any context root). + +This means any endpoints that are not part of the default servlet, xref:servlet/authorization/authorize-http-requests.adoc#match-by-mvc[the servlet path needs to be specified]. +For URIs that match an extension, like `.jsp`, use `regexMatcher("\\.jsp$")`. + +Alternatively, you can 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 4eaf5f3d5ee..3443f501c8b 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.RequestMatchers.servlet; @Bean -SecurityFilterChain appEndpoints(HttpSecurity http, MvcRequestMatcher.Builder mvc) { +SecurityFilterChain appEndpoints(HttpSecurity http) { http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(mvc.pattern("/my/controller/**")).hasAuthority("controller") + .requestMatchers(servlet("/spring-mvc").uris("/admin/**")).hasAuthority("admin") + .requestMatchers(servlet("/spring-mvc").uris("/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,39 @@ Xml:: [source,xml,role="secondary"] ---- + ---- ====== -This need can arise in at least two different ways: +This is because Spring Security requires all URIs to be absolute (minus the context path). + +With Java, note that the `ServletRequestMatcherBuilders` return value can be reused, reducing repeated boilerplate: + +[source,java,role="primary"] +---- +import static org.springframework.security.web.servlet.util.matcher.RequestMatchers.servlet; + +@Bean +SecurityFilterChain appEndpoints(HttpSecurity http) { + RequestMatchers.Builder mvc = servlet("/spring-mvc"); + http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(mvc.uris("/admin/**")).hasAuthority("admin") + .requestMatchers(mvc.uris("/my/controller/**")).hasAuthority("controller") + .anyRequest().authenticated() + ); -* 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) + return http.build(); +} +---- + +[TIP] +===== +There are several other components that create request matchers for you like `PathRequest#toStaticResources#atCommonLocations` +===== [[match-by-custom]] === Using a Custom Matcher diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java index a4da49deeed..411b7c2a7ed 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java @@ -16,7 +16,30 @@ package org.springframework.security.web.util.matcher; +import java.nio.charset.StandardCharsets; +import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; + +import jakarta.servlet.DispatcherType; +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.ServletContext; +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.lang.Nullable; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; +import org.springframework.web.util.UriUtils; +import org.springframework.web.util.WebUtils; +import org.springframework.web.util.pattern.PathPattern; +import org.springframework.web.util.pattern.PathPatternParser; /** * A factory class to create {@link RequestMatcher} instances. @@ -60,7 +83,236 @@ public static RequestMatcher not(RequestMatcher matcher) { return (request) -> !matcher.matches(request); } + /** + * Create {@link RequestMatcher}s whose URIs do not have a servlet path prefix + *

+ * When there is no context path, then these URIs are effectively absolute. + * @return a {@link Builder} that treats URIs as relative to the context path, if any + * @since 6.5 + */ + public static Builder request() { + return new Builder(); + } + + /** + * Create {@link RequestMatcher}s whose URIs are relative to the given + * {@code servletPath}. + * + *

+ * The {@code servletPath} must correlate to a configured servlet in your application. + * The path must be of the format {@code /path}. + * @return a {@link Builder} that treats URIs as relative to the given + * {@code servletPath} + * @since 6.5 + */ + public static Builder servlet(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 a slash"); + Assert.isTrue(!servletPath.contains("*"), "servletPath must not contain a star"); + return new Builder(servletPath); + } + private RequestMatchers() { } + /** + * A builder for specifying various elements of a request for the purpose of creating + * a {@link RequestMatcher}. + * + *

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

+ * + * + * http + * .authorizeHttpRequests((authorize) -> authorize + * .requestMatchers(servlet("/mvc").uris("/user/**")).hasAuthority("user") + * .requestMatchers(servlet("/other").uris("/admin/**")).hasAuthority("admin") + * ) + * ... + * + * + * @author Josh Cummings + * @since 6.5 + */ + public static final class Builder { + + private final RequestMatcher servletPath; + + private final RequestMatcher methods; + + private final RequestMatcher uris; + + private final RequestMatcher dispatcherTypes; + + private final RequestMatcher matchers; + + private Builder() { + this(AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, + AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE); + } + + private Builder(String servletPath) { + this(new ServletPathRequestMatcher(servletPath), AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, + AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE); + } + + private Builder(RequestMatcher servletPath, RequestMatcher methods, RequestMatcher uris, + RequestMatcher dispatcherTypes, RequestMatcher matchers) { + this.servletPath = servletPath; + this.methods = methods; + this.uris = uris; + this.dispatcherTypes = dispatcherTypes; + this.matchers = matchers; + } + + /** + * Match requests with any of these methods + * @param methods the {@link HttpMethod} to match + * @return the {@link Builder} for more configuration + */ + public Builder methods(HttpMethod... methods) { + RequestMatcher[] matchers = new RequestMatcher[methods.length]; + for (int i = 0; i < methods.length; i++) { + matchers[i] = new HttpMethodRequestMatcher(methods[i]); + } + return new Builder(this.servletPath, anyOf(matchers), this.uris, this.dispatcherTypes, this.matchers); + } + + /** + * Match requests with any of these URIs + * + *

+ * URIs can be Ant patterns like {@code /path/**}. + * @param uris the URIs to match + * @return the {@link Builder} for more configuration + */ + public Builder uris(String... uris) { + RequestMatcher[] matchers = new RequestMatcher[uris.length]; + for (int i = 0; i < uris.length; i++) { + Assert.isTrue(uris[i].startsWith("/"), "pattern must start with '/'"); + PathPatternParser parser = PathPatternParser.defaultInstance; + matchers[i] = new PathPatternRequestMatcher(parser.parse(uris[i])); + } + return new Builder(this.servletPath, this.methods, anyOf(matchers), this.dispatcherTypes, this.matchers); + } + + /** + * Match requests with any of these {@link PathPattern}s + * + *

+ * Use this when you have a non-default {@link PathPatternParser} + * @param uris the URIs to match + * @return the {@link Builder} for more configuration + */ + public Builder uris(PathPattern... uris) { + RequestMatcher[] matchers = new RequestMatcher[uris.length]; + for (int i = 0; i < uris.length; i++) { + matchers[i] = new PathPatternRequestMatcher(uris[i]); + } + return new Builder(this.servletPath, this.methods, anyOf(matchers), this.dispatcherTypes, this.matchers); + } + + /** + * Match requests with any of these dispatcherTypes + * + *

+ * URIs can be Ant patterns like {@code /path/**}. + * @param dispatcherTypes the {@link DispatcherType}s to match + * @return the {@link Builder} for more configuration + */ + public Builder dispatcherTypes(DispatcherType... dispatcherTypes) { + RequestMatcher[] matchers = new RequestMatcher[dispatcherTypes.length]; + for (int i = 0; i < dispatcherTypes.length; i++) { + matchers[i] = new DispatcherTypeRequestMatcher(dispatcherTypes[i]); + } + return new Builder(this.servletPath, this.methods, this.uris, anyOf(matchers), this.matchers); + } + + /** + * Match requests with any of these {@link RequestMatcher}s + * @param requestMatchers the {@link RequestMatchers}s to match + * @return the {@link Builder} for more configuration + */ + public Builder matching(RequestMatcher... requestMatchers) { + return new Builder(this.servletPath, this.methods, this.uris, this.dispatcherTypes, anyOf(requestMatchers)); + } + + /** + * Create the {@link RequestMatcher} + * @return the composite {@link RequestMatcher} + */ + public RequestMatcher matcher() { + return allOf(this.servletPath, this.methods, this.uris, this.dispatcherTypes, this.matchers); + } + + } + + private record HttpMethodRequestMatcher(HttpMethod method) implements RequestMatcher { + + @Override + public boolean matches(HttpServletRequest request) { + return this.method.name().equals(request.getMethod()); + } + + @Override + public String toString() { + return "HttpMethod [" + this.method + "]"; + } + + } + + private record ServletPathRequestMatcher(String path) implements RequestMatcher { + + @Override + public boolean matches(HttpServletRequest request) { + Assert.isTrue(servletExists(request), () -> this.path + "/* does not exist in your servlet registration " + + registrationMappings(request)); + return Objects.equals(this.path, getServletPathPrefix(request)); + } + + private boolean servletExists(HttpServletRequest request) { + if (request.getAttribute("org.springframework.test.web.servlet.MockMvc.MVC_RESULT_ATTRIBUTE") != null) { + return true; + } + ServletContext servletContext = request.getServletContext(); + for (ServletRegistration registration : servletContext.getServletRegistrations().values()) { + if (registration.getMappings().contains(this.path + "/*")) { + return true; + } + } + return false; + } + + private Map> registrationMappings(HttpServletRequest request) { + Map> map = new LinkedHashMap<>(); + ServletContext servletContext = request.getServletContext(); + for (ServletRegistration registration : servletContext.getServletRegistrations().values()) { + map.put(registration.getName(), registration.getMappings()); + } + return map; + } + + @Nullable + private static String getServletPathPrefix(HttpServletRequest request) { + HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING); + mapping = (mapping != null) ? mapping : request.getHttpServletMapping(); + if (ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) { + String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE); + servletPath = (servletPath != null) ? servletPath : request.getServletPath(); + servletPath = servletPath.endsWith("/") ? servletPath.substring(0, servletPath.length() - 1) + : servletPath; + return UriUtils.encodePath(servletPath, StandardCharsets.UTF_8); + } + return null; + } + + @Override + public String toString() { + return "ServletPath [" + this.path + "]"; + } + } + } diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java index abff0c345c6..8508a171ce0 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java +++ b/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java @@ -16,9 +16,17 @@ package org.springframework.security.web.util.matcher; +import jakarta.servlet.Servlet; +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletRegistration; import org.junit.jupiter.api.Test; +import org.springframework.http.HttpMethod; +import org.springframework.security.web.servlet.MockServletContext; + import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; /** * Tests for {@link RequestMatchers}. @@ -83,4 +91,47 @@ void checkNotWhenNotMatchThenMatch() { assertThat(match).isTrue(); } + @Test + void matcherWhenServletPathThenMatchesOnlyServletPath() { + RequestMatchers.Builder servlet = RequestMatchers.servlet("/servlet/path"); + RequestMatcher matcher = servlet.methods(HttpMethod.GET).uris("/endpoint").matcher(); + ServletContext servletContext = servletContext("/servlet/path"); + assertThat(matcher + .matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(servletContext))).isTrue(); + assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(servletContext))).isFalse(); + } + + @Test + void matcherWhenRequestPathThenIgnoresServletPath() { + RequestMatchers.Builder request = RequestMatchers.request(); + RequestMatcher matcher = request.methods(HttpMethod.GET).uris("/endpoint").matcher(); + assertThat(matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))) + .isTrue(); + assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(null))).isTrue(); + } + + @Test + void matcherWhenServletPathThenRequiresServletPathToExist() { + RequestMatchers.Builder servlet = RequestMatchers.servlet("/servlet/path"); + RequestMatcher matcher = servlet.methods(HttpMethod.GET).uris("/endpoint").matcher(); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy( + () -> matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))); + } + + @Test + void servletPathWhenEndsWithSlashOrStarThenIllegalArgument() { + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> RequestMatchers.servlet("/path/**")); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> RequestMatchers.servlet("/path/*")); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> RequestMatchers.servlet("/path/")); + } + + MockServletContext servletContext(String... servletPath) { + MockServletContext servletContext = new MockServletContext(); + ServletRegistration.Dynamic registration = servletContext.addServlet("servlet", Servlet.class); + for (String s : servletPath) { + registration.addMapping(s + "/*"); + } + return servletContext; + } + } From 109396af2a03587400c7cad445ea1a12697c8395 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Fri, 7 Feb 2025 15:46:50 -0700 Subject: [PATCH 03/10] Updates - Refined documentation - Applied method naming feedback --- .../web/AbstractRequestMatcherRegistry.java | 13 -- .../AuthorizeHttpRequestsConfigurerTests.java | 4 +- docs/modules/ROOT/pages/migration-7/web.adoc | 41 +++++- .../authorize-http-requests.adoc | 2 +- .../matcher/PathPatternRequestMatcher.java | 8 +- .../web/util/matcher/RequestMatchers.java | 125 +++++++++++------- .../PathPatternRequestMatcherTests.java | 6 +- .../util/matcher/RequestMatchersTests.java | 19 +-- 8 files changed, 133 insertions(+), 85 deletions(-) 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 fb69ac7b3de..66f4daf1a71 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 @@ -179,19 +179,6 @@ public C requestMatchers(RequestMatcher... requestMatchers) { return chainRequestMatchers(Arrays.asList(requestMatchers)); } - /** - * Register the {@link RequestMatcher} represented by this builder - * @param builder the - * {@link org.springframework.security.web.util.matcher.RequestMatchers.Builder} to - * use - * @return the object that is chained after creating the {@link RequestMatcher} - * @since 6.5 - */ - public C requestMatchers(org.springframework.security.web.util.matcher.RequestMatchers.Builder builder) { - Assert.state(!this.anyRequestConfigured, "Can't configure requestMatchers after anyRequest"); - return chainRequestMatchers(List.of(builder.matcher())); - } - /** *

* If the {@link HandlerMappingIntrospector} is available in the classpath, maps to an 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 87096ac59e2..cd7e9873430 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 @@ -1343,11 +1343,11 @@ static class MvcRequestMatcherBuilderConfig { @Bean SecurityFilterChain security(HttpSecurity http) throws Exception { - RequestMatchers.Builder mvc = RequestMatchers.servlet("/mvc"); + RequestMatchers.Builder mvc = RequestMatchers.servletPath("/mvc"); // @formatter:off http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(mvc.uris("/path/**")).hasRole("USER") + .requestMatchers(mvc.pathPatterns("/path/**").matcher()).hasRole("USER") ) .httpBasic(withDefaults()); // @formatter:on diff --git a/docs/modules/ROOT/pages/migration-7/web.adoc b/docs/modules/ROOT/pages/migration-7/web.adoc index b928b057017..4bd1eff7d7d 100644 --- a/docs/modules/ROOT/pages/migration-7/web.adoc +++ b/docs/modules/ROOT/pages/migration-7/web.adoc @@ -103,11 +103,42 @@ Xml:: ---- ====== -== Use Absolute Authorization URIs +== Include the Servlet Path Prefix in Authorization Rules -The Java DSL now requires that all URIs be absolute (less any context root). +As of Spring Security 7, `AntPathRequestMatcher` and `MvcRequestMatcher` are no longer supported and the Java DSL requires that all URIs be absolute (less any context root). -This means any endpoints that are not part of the default servlet, xref:servlet/authorization/authorize-http-requests.adoc#match-by-mvc[the servlet path needs to be specified]. -For URIs that match an extension, like `.jsp`, use `regexMatcher("\\.jsp$")`. +For many applications this will make no difference since most commonly all URIs listed are matched by the default servlet. -Alternatively, you can change each of your `String` URI authorization rules to xref:servlet/authorization/authorize-http-requests.adoc#security-matchers[use a `RequestMatcher`]. +However, if you have other servlets with servlet path prefixes, xref:servlet/authorization/authorize-http-requests.adoc[then these paths need to be supplied separately]. + +For example, if I have a Spring MVC controller with `@RequestMapping("/orders")` and my MVC application is deployed to `/mvc` (instead of the default servlet), then the URI for this endpoint is `/mvc/orders`. +Historically, the Java DSL hasn't had a simple way to specify the servlet path prefix and Spring Security attempted to infer it. + +Over time, we learned that these inference would surprise developers. +Instead of taking this responsibility away from developers, now it is simpler to specify the servlet path prefix like so: + +[method,java] +---- +RequestMatchers.Builder servlet = RequestMatchers.servlet("/mvc"); +http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(servlet.uris("/orders/**").matcher()).authenticated() + ) +---- + + +For paths that belong to the default servlet, use `RequestMatchers.request()` instead: + +[method,java] +---- +RequestMatchers.Builder request = RequestMatchers.request(); +http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(request.uris("/js/**").matcher()).authenticated() + ) +---- + +Note that this doesn't address every kind of servlet since not all servlets have a path prefix. +For example, expressions that match the JSP Servlet might use an ant pattern `/**/*.jsp`. + +There is not yet a general-purpose replacement for these, and so you are encouraged to use `RegexRequestMatcher`, like so: `regexMatcher("\\.jsp$")`. 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 3443f501c8b..82b922e8e8d 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc @@ -657,7 +657,7 @@ SecurityFilterChain appEndpoints(HttpSecurity http) { [TIP] ===== -There are several other components that create request matchers for you like `PathRequest#toStaticResources#atCommonLocations` +There are several other components that create request matchers for you like {spring-boot-api-url}org/springframework/boot/autoconfigure/security/servlet/PathRequest.html[`PathRequest#toStaticResources#atCommonLocations`] ===== [[match-by-custom]] diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java index 402ca5fafd3..e95cd748149 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java @@ -34,13 +34,13 @@ * is, it should exclude any context or servlet path). * *

- * To also match the servlet, please see {@link RequestMatchers#servlet} + * To also match the servlet, please see {@link RequestMatchers#servletPath} * *

* Note that the {@link org.springframework.web.servlet.HandlerMapping} that contains the - * related URI patterns must be using the same - * {@link org.springframework.web.util.pattern.PathPatternParser} configured in this - * class. + * related URI patterns must be using {@link PathPatternParser#defaultInstance}. If that + * is not the case, use {@link PathPatternParser} to parse your path and provide a + * {@link PathPattern} in the constructor. *

* * @author Josh Cummings diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java index 411b7c2a7ed..09dc67f510c 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; import jakarta.servlet.DispatcherType; import jakarta.servlet.RequestDispatcher; @@ -33,6 +34,7 @@ import org.springframework.http.HttpMethod; import org.springframework.lang.Nullable; +import org.springframework.security.web.access.intercept.RequestAuthorizationContext; import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -96,16 +98,23 @@ public static Builder request() { /** * Create {@link RequestMatcher}s whose URIs are relative to the given - * {@code servletPath}. + * {@code servletPath} prefix. * *

- * The {@code servletPath} must correlate to a configured servlet in your application. - * The path must be of the format {@code /path}. + * The {@code servletPath} must correlate to a value that would match the result of + * {@link HttpServletRequest#getServletPath()} and its corresponding servlet. + * + *

+ * That is, if you have a servlet mapping of {@code /path/*}, then + * {@link HttpServletRequest#getServletPath()} would return {@code /path} and so + * {@code /path} is what is specified here. + * + * Specify the path here without the trailing {@code /*}. * @return a {@link Builder} that treats URIs as relative to the given * {@code servletPath} * @since 6.5 */ - public static Builder servlet(String servletPath) { + public static Builder 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 a slash"); @@ -147,25 +156,22 @@ public static final class Builder { private final RequestMatcher dispatcherTypes; - private final RequestMatcher matchers; - private Builder() { this(AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, - AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE); + AnyRequestMatcher.INSTANCE); } private Builder(String servletPath) { this(new ServletPathRequestMatcher(servletPath), AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, - AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE); + AnyRequestMatcher.INSTANCE); } private Builder(RequestMatcher servletPath, RequestMatcher methods, RequestMatcher uris, - RequestMatcher dispatcherTypes, RequestMatcher matchers) { + RequestMatcher dispatcherTypes) { this.servletPath = servletPath; this.methods = methods; this.uris = uris; this.dispatcherTypes = dispatcherTypes; - this.matchers = matchers; } /** @@ -178,25 +184,43 @@ public Builder methods(HttpMethod... methods) { for (int i = 0; i < methods.length; i++) { matchers[i] = new HttpMethodRequestMatcher(methods[i]); } - return new Builder(this.servletPath, anyOf(matchers), this.uris, this.dispatcherTypes, this.matchers); + return new Builder(this.servletPath, anyOf(matchers), this.uris, this.dispatcherTypes); } /** - * Match requests with any of these URIs + * Match requests with any of these path patterns + * + *

+ * Path patterns always start with a slash and may contain placeholders. They can + * also be followed by {@code /**} to signify all URIs under a given path. + * + *

+ * These must be specified relative to any servlet path prefix (meaning you should + * exclude the context path and any servlet path prefix in stating your pattern). + * + *

+ * The following are valid patterns and their meaning + *

* *

- * URIs can be Ant patterns like {@code /path/**}. - * @param uris the URIs to match + * A more comprehensive list can be found at {@link PathPattern}. + * @param pathPatterns the path patterns to match * @return the {@link Builder} for more configuration */ - public Builder uris(String... uris) { - RequestMatcher[] matchers = new RequestMatcher[uris.length]; - for (int i = 0; i < uris.length; i++) { - Assert.isTrue(uris[i].startsWith("/"), "pattern must start with '/'"); + public Builder pathPatterns(String... pathPatterns) { + RequestMatcher[] matchers = new RequestMatcher[pathPatterns.length]; + for (int i = 0; i < pathPatterns.length; i++) { + Assert.isTrue(pathPatterns[i].startsWith("/"), "path patterns must start with /"); PathPatternParser parser = PathPatternParser.defaultInstance; - matchers[i] = new PathPatternRequestMatcher(parser.parse(uris[i])); + matchers[i] = new PathPatternRequestMatcher(parser.parse(pathPatterns[i])); } - return new Builder(this.servletPath, this.methods, anyOf(matchers), this.dispatcherTypes, this.matchers); + return new Builder(this.servletPath, this.methods, anyOf(matchers), this.dispatcherTypes); } /** @@ -204,22 +228,19 @@ public Builder uris(String... uris) { * *

* Use this when you have a non-default {@link PathPatternParser} - * @param uris the URIs to match + * @param pathPatterns the URIs to match * @return the {@link Builder} for more configuration */ - public Builder uris(PathPattern... uris) { - RequestMatcher[] matchers = new RequestMatcher[uris.length]; - for (int i = 0; i < uris.length; i++) { - matchers[i] = new PathPatternRequestMatcher(uris[i]); + public Builder pathPatterns(PathPattern... pathPatterns) { + RequestMatcher[] matchers = new RequestMatcher[pathPatterns.length]; + for (int i = 0; i < pathPatterns.length; i++) { + matchers[i] = new PathPatternRequestMatcher(pathPatterns[i]); } - return new Builder(this.servletPath, this.methods, anyOf(matchers), this.dispatcherTypes, this.matchers); + return new Builder(this.servletPath, this.methods, anyOf(matchers), this.dispatcherTypes); } /** * Match requests with any of these dispatcherTypes - * - *

- * URIs can be Ant patterns like {@code /path/**}. * @param dispatcherTypes the {@link DispatcherType}s to match * @return the {@link Builder} for more configuration */ @@ -228,16 +249,7 @@ public Builder dispatcherTypes(DispatcherType... dispatcherTypes) { for (int i = 0; i < dispatcherTypes.length; i++) { matchers[i] = new DispatcherTypeRequestMatcher(dispatcherTypes[i]); } - return new Builder(this.servletPath, this.methods, this.uris, anyOf(matchers), this.matchers); - } - - /** - * Match requests with any of these {@link RequestMatcher}s - * @param requestMatchers the {@link RequestMatchers}s to match - * @return the {@link Builder} for more configuration - */ - public Builder matching(RequestMatcher... requestMatchers) { - return new Builder(this.servletPath, this.methods, this.uris, this.dispatcherTypes, anyOf(requestMatchers)); + return new Builder(this.servletPath, this.methods, this.uris, anyOf(matchers)); } /** @@ -245,7 +257,7 @@ public Builder matching(RequestMatcher... requestMatchers) { * @return the composite {@link RequestMatcher} */ public RequestMatcher matcher() { - return allOf(this.servletPath, this.methods, this.uris, this.dispatcherTypes, this.matchers); + return allOf(this.servletPath, this.methods, this.uris, this.dispatcherTypes); } } @@ -264,7 +276,15 @@ public String toString() { } - private record ServletPathRequestMatcher(String path) implements RequestMatcher { + private static final class ServletPathRequestMatcher implements RequestMatcher { + + private final String path; + + private final AtomicReference servletExists = new AtomicReference(); + + ServletPathRequestMatcher(String servletPath) { + this.path = servletPath; + } @Override public boolean matches(HttpServletRequest request) { @@ -274,16 +294,22 @@ public boolean matches(HttpServletRequest request) { } private boolean servletExists(HttpServletRequest request) { - if (request.getAttribute("org.springframework.test.web.servlet.MockMvc.MVC_RESULT_ATTRIBUTE") != null) { - return true; - } - ServletContext servletContext = request.getServletContext(); - for (ServletRegistration registration : servletContext.getServletRegistrations().values()) { - if (registration.getMappings().contains(this.path + "/*")) { + return this.servletExists.updateAndGet((value) -> { + if (value != null) { + return value; + } + if (request.getAttribute("org.springframework.test.web.servlet.MockMvc.MVC_RESULT_ATTRIBUTE") != null) { return true; } - } - return false; + for (ServletRegistration registration : request.getServletContext() + .getServletRegistrations() + .values()) { + if (registration.getMappings().contains(this.path + "/*")) { + return true; + } + } + return false; + }); } private Map> registrationMappings(HttpServletRequest request) { @@ -313,6 +339,7 @@ private static String getServletPathPrefix(HttpServletRequest request) { public String toString() { return "ServletPath [" + this.path + "]"; } + } } diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java index bd8c3fde66d..b9875d12fd2 100644 --- a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java @@ -57,19 +57,19 @@ void matcherWhenUriContainsServletPathThenNoMatch() { @Test void matcherWhenSameMethodThenMatchResult() { - RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).uris("/uri").matcher(); + RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).pathPatterns("/uri").matcher(); assertThat(matcher.matches(request("/uri"))).isTrue(); } @Test void matcherWhenDifferentPathThenNoMatch() { - RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).uris("/uri").matcher(); + RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).pathPatterns("/uri").matcher(); assertThat(matcher.matches(request("GET", "/urj", ""))).isFalse(); } @Test void matcherWhenDifferentMethodThenNoMatch() { - RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).uris("/uri").matcher(); + RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).pathPatterns("/uri").matcher(); assertThat(matcher.matches(request("POST", "/mvc/uri", "/mvc"))).isFalse(); } diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java index 8508a171ce0..befb1e56a52 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java +++ b/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java @@ -93,8 +93,8 @@ void checkNotWhenNotMatchThenMatch() { @Test void matcherWhenServletPathThenMatchesOnlyServletPath() { - RequestMatchers.Builder servlet = RequestMatchers.servlet("/servlet/path"); - RequestMatcher matcher = servlet.methods(HttpMethod.GET).uris("/endpoint").matcher(); + RequestMatchers.Builder servlet = RequestMatchers.servletPath("/servlet/path"); + RequestMatcher matcher = servlet.methods(HttpMethod.GET).pathPatterns("/endpoint").matcher(); ServletContext servletContext = servletContext("/servlet/path"); assertThat(matcher .matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(servletContext))).isTrue(); @@ -104,7 +104,7 @@ void matcherWhenServletPathThenMatchesOnlyServletPath() { @Test void matcherWhenRequestPathThenIgnoresServletPath() { RequestMatchers.Builder request = RequestMatchers.request(); - RequestMatcher matcher = request.methods(HttpMethod.GET).uris("/endpoint").matcher(); + RequestMatcher matcher = request.methods(HttpMethod.GET).pathPatterns("/endpoint").matcher(); assertThat(matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))) .isTrue(); assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(null))).isTrue(); @@ -112,17 +112,20 @@ void matcherWhenRequestPathThenIgnoresServletPath() { @Test void matcherWhenServletPathThenRequiresServletPathToExist() { - RequestMatchers.Builder servlet = RequestMatchers.servlet("/servlet/path"); - RequestMatcher matcher = servlet.methods(HttpMethod.GET).uris("/endpoint").matcher(); + RequestMatchers.Builder servlet = RequestMatchers.servletPath("/servlet/path"); + RequestMatcher matcher = servlet.methods(HttpMethod.GET).pathPatterns("/endpoint").matcher(); assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy( () -> matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))); } @Test void servletPathWhenEndsWithSlashOrStarThenIllegalArgument() { - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> RequestMatchers.servlet("/path/**")); - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> RequestMatchers.servlet("/path/*")); - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> RequestMatchers.servlet("/path/")); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> RequestMatchers.servletPath("/path/**")); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> RequestMatchers.servletPath("/path/*")); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> RequestMatchers.servletPath("/path/")); } MockServletContext servletContext(String... servletPath) { From 36e6d0ba088d1d8a21889379af9623040ee950b2 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Mon, 10 Feb 2025 13:00:37 -0700 Subject: [PATCH 04/10] Polish PathPattern --- .../matcher/PathPatternRequestMatcher.java | 313 +++++++++++++++++- .../PathPatternRequestMatcherTests.java | 69 +++- 2 files changed, 360 insertions(+), 22 deletions(-) diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java index e95cd748149..55441dc085b 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java @@ -16,15 +16,32 @@ package org.springframework.security.web.servlet.util.matcher; +import java.nio.charset.StandardCharsets; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; +import jakarta.servlet.RequestDispatcher; +import jakarta.servlet.ServletContext; +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.http.server.PathContainer; import org.springframework.http.server.RequestPath; +import org.springframework.lang.Nullable; +import org.springframework.security.web.access.intercept.RequestAuthorizationContext; +import org.springframework.security.web.util.matcher.AnyRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; -import org.springframework.security.web.util.matcher.RequestMatchers; +import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; import org.springframework.web.util.ServletRequestPathUtils; +import org.springframework.web.util.UriUtils; +import org.springframework.web.util.WebUtils; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; @@ -34,7 +51,7 @@ * is, it should exclude any context or servlet path). * *

- * To also match the servlet, please see {@link RequestMatchers#servletPath} + * To also match the servlet, please see {@link PathPatternRequestMatcher#servletPath} * *

* Note that the {@link org.springframework.web.servlet.HandlerMapping} that contains the @@ -50,6 +67,10 @@ public final class PathPatternRequestMatcher implements RequestMatcher { private final PathPattern pattern; + private RequestMatcher servletPath = AnyRequestMatcher.INSTANCE; + + private RequestMatcher method = AnyRequestMatcher.INSTANCE; + /** * Creates a {@link PathPatternRequestMatcher} that uses the provided {@code pattern}. *

@@ -57,22 +78,47 @@ public final class PathPatternRequestMatcher implements RequestMatcher { *

* @param pattern the pattern used to match */ - public PathPatternRequestMatcher(PathPattern pattern) { + private PathPatternRequestMatcher(PathPattern pattern) { this.pattern = pattern; } /** - * Creates a {@link PathPatternRequestMatcher} that uses the provided {@code pattern}, - * parsing it with {@link PathPatternParser#defaultInstance}. + * Create a {@link PathPatternRequestMatcher} whose URIs do not have a servlet path + * prefix *

- * The {@code pattern} should be relative to the servlet path - *

- * @param pattern the pattern used to match + * When there is no context path, then these URIs are effectively absolute. + * @return a {@link PathPatternRequestMatcher.Builder} that treats URIs as relative to + * the context path, if any + * @since 6.5 + */ + public static Builder path() { + return new Builder(); + } + + /** + * Create a {@link PathPatternRequestMatcher} whose URIs are relative to the given + * {@code servletPath} prefix. + * + *

+ * The {@code servletPath} must correlate to a value that would match the result of + * {@link HttpServletRequest#getServletPath()} and its corresponding servlet. + * + *

+ * That is, if you have a servlet mapping of {@code /path/*}, then + * {@link HttpServletRequest#getServletPath()} would return {@code /path} and so + * {@code /path} is what is specified here. + * + * Specify the path here without the trailing {@code /*}. + * @return a {@link PathPatternRequestMatcher.Builder} that treats URIs as relative to + * the given {@code servletPath} + * @since 6.5 */ - public static PathPatternRequestMatcher pathPattern(String pattern) { - PathPatternParser parser = PathPatternParser.defaultInstance; - PathPattern pathPattern = parser.parse(pattern); - return new PathPatternRequestMatcher(pathPattern); + public static Builder 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 a slash"); + Assert.isTrue(!servletPath.contains("*"), "servletPath must not contain a star"); + return new Builder(new ServletPathRequestMatcher(servletPath)); } /** @@ -88,11 +134,25 @@ public boolean matches(HttpServletRequest request) { */ @Override public MatchResult matcher(HttpServletRequest request) { + if (!this.servletPath.matches(request)) { + return MatchResult.notMatch(); + } + if (!this.method.matches(request)) { + return MatchResult.notMatch(); + } PathContainer path = getRequestPath(request).pathWithinApplication(); PathPattern.PathMatchInfo info = this.pattern.matchAndExtract(path); return (info != null) ? MatchResult.match(info.getUriVariables()) : MatchResult.notMatch(); } + void setMethod(RequestMatcher method) { + this.method = method; + } + + void setServletPath(RequestMatcher servletPath) { + this.servletPath = servletPath; + } + private RequestPath getRequestPath(HttpServletRequest request) { return ServletRequestPathUtils.parseAndCache(request); } @@ -121,7 +181,234 @@ public int hashCode() { */ @Override public String toString() { - return "PathPattern [" + this.pattern + "]"; + StringBuilder request = new StringBuilder(); + if (this.method instanceof HttpMethodRequestMatcher m) { + request.append(m.method.name()).append(' '); + } + if (this.servletPath instanceof ServletPathRequestMatcher s) { + request.append(s.path); + } + return "PathPattern [" + request + this.pattern + "]"; + } + + /** + * A builder for specifying various elements of a request for the purpose of creating + * a {@link PathPatternRequestMatcher}. + * + *

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

+ * + * + * http + * .authorizeHttpRequests((authorize) -> authorize + * .requestMatchers(servletPath("/mvc").pattern("/user/**").matcher()).hasAuthority("user") + * .requestMatchers(servletPath("/other").pattern("/admin/**").matcher()).hasAuthority("admin") + * ) + * ... + * + */ + public static final class Builder { + + private static final PathPattern ANY_PATH = PathPatternParser.defaultInstance.parse("/**"); + + private final RequestMatcher method; + + private final RequestMatcher servletPath; + + private final PathPattern pathPattern; + + Builder() { + this(AnyRequestMatcher.INSTANCE); + } + + Builder(RequestMatcher servletPath) { + this(AnyRequestMatcher.INSTANCE, servletPath, ANY_PATH); + } + + Builder(RequestMatcher method, RequestMatcher servletPath, PathPattern pathPattern) { + this.method = method; + this.servletPath = servletPath; + this.pathPattern = pathPattern; + } + + /** + * Match requests having this path pattern. + * + *

+ * Path patterns always start with a slash and may contain placeholders. They can + * also be followed by {@code /**} to signify all URIs under a given path. + * + *

+ * These must be specified relative to any servlet path prefix (meaning you should + * exclude the context path and any servlet path prefix in stating your pattern). + * + *

+ * The following are valid patterns and their meaning + *

+ * + *

+ * The pattern is parsed using {@link PathPatternParser#defaultInstance} A more + * comprehensive list can be found at {@link PathPattern}. + * @param pathPattern the path pattern to match + * @return the {@link Builder} for more configuration + */ + public Builder pattern(String pathPattern) { + Assert.notNull(pathPattern, "pattern cannot be null"); + Assert.isTrue(pathPattern.startsWith("/"), "pattern must start with a /"); + PathPatternParser parser = PathPatternParser.defaultInstance; + return new Builder(this.method, this.servletPath, parser.parse(pathPattern)); + } + + /** + * Match requests having this path pattern. + * + *

+ * Path patterns always start with a slash and may contain placeholders. They can + * also be followed by {@code /**} to signify all URIs under a given path. + * + *

+ * These must be specified relative to any servlet path prefix (meaning you should + * exclude the context path and any servlet path prefix in stating your pattern). + * + *

+ * The following are valid patterns and their meaning + *

+ * + *

+ * A more comprehensive list can be found at {@link PathPattern}. + * @param pathPattern the path pattern to match + * @return the {@link Builder} for more configuration + */ + public Builder pattern(PathPattern pathPattern) { + Assert.notNull(pathPattern, "pathPattern cannot be null"); + return new Builder(this.method, this.servletPath, pathPattern); + } + + /** + * Match requests having this {@link HttpMethod}. + * @param method the {@link HttpMethod} to match + * @return the {@link Builder} for more configuration + */ + public Builder method(HttpMethod method) { + Assert.notNull(method, "method cannot be null"); + return new Builder(new HttpMethodRequestMatcher(method), this.servletPath, this.pathPattern); + } + + /** + * Create the {@link PathPatternRequestMatcher}/ + * @return the {@link PathPatternRequestMatcher} + */ + public PathPatternRequestMatcher matcher() { + PathPatternRequestMatcher pathPattern = new PathPatternRequestMatcher(this.pathPattern); + if (this.method != AnyRequestMatcher.INSTANCE) { + pathPattern.setMethod(this.method); + } + if (this.servletPath != AnyRequestMatcher.INSTANCE) { + pathPattern.setServletPath(this.servletPath); + } + return pathPattern; + } + + } + + private static final class HttpMethodRequestMatcher implements RequestMatcher { + + private final HttpMethod method; + + HttpMethodRequestMatcher(HttpMethod method) { + this.method = method; + } + + @Override + public boolean matches(HttpServletRequest request) { + return this.method.name().equals(request.getMethod()); + } + + @Override + public String toString() { + return "HttpMethod [" + this.method + "]"; + } + + } + + private static final class ServletPathRequestMatcher implements RequestMatcher { + + private final String path; + + private final AtomicReference servletExists = new AtomicReference<>(); + + ServletPathRequestMatcher(String servletPath) { + this.path = servletPath; + } + + @Override + public boolean matches(HttpServletRequest request) { + Assert.isTrue(servletExists(request), () -> this.path + "/* does not exist in your servlet registration " + + registrationMappings(request)); + return Objects.equals(this.path, getServletPathPrefix(request)); + } + + private boolean servletExists(HttpServletRequest request) { + return this.servletExists.updateAndGet((value) -> { + if (value != null) { + return value; + } + if (request.getAttribute("org.springframework.test.web.servlet.MockMvc.MVC_RESULT_ATTRIBUTE") != null) { + return true; + } + for (ServletRegistration registration : request.getServletContext() + .getServletRegistrations() + .values()) { + if (registration.getMappings().contains(this.path + "/*")) { + return true; + } + } + return false; + }); + } + + private Map> registrationMappings(HttpServletRequest request) { + Map> map = new LinkedHashMap<>(); + ServletContext servletContext = request.getServletContext(); + for (ServletRegistration registration : servletContext.getServletRegistrations().values()) { + map.put(registration.getName(), registration.getMappings()); + } + return map; + } + + @Nullable + private static String getServletPathPrefix(HttpServletRequest request) { + HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING); + mapping = (mapping != null) ? mapping : request.getHttpServletMapping(); + if (ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) { + String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE); + servletPath = (servletPath != null) ? servletPath : request.getServletPath(); + servletPath = servletPath.endsWith("/") ? servletPath.substring(0, servletPath.length() - 1) + : servletPath; + return UriUtils.encodePath(servletPath, StandardCharsets.UTF_8); + } + return null; + } + + @Override + public String toString() { + return "ServletPath [" + this.path + "]"; + } + } } diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java index b9875d12fd2..04e55ab68a3 100644 --- a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java @@ -16,15 +16,20 @@ package org.springframework.security.web.servlet.util.matcher; +import jakarta.servlet.Servlet; +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletRegistration; 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.util.matcher.RequestMatcher; -import org.springframework.security.web.util.matcher.RequestMatchers; import org.springframework.web.util.ServletRequestPathUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; /** * Tests for {@link PathPatternRequestMatcher} @@ -33,53 +38,90 @@ public class PathPatternRequestMatcherTests { @Test void matcherWhenPatternMatchesRequestThenMatchResult() { - RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/uri"); + RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/uri").matcher(); assertThat(matcher.matches(request("/uri"))).isTrue(); } @Test void matcherWhenPatternContainsPlaceholdersThenMatchResult() { - RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/uri/{username}"); + RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/uri/{username}").matcher(); assertThat(matcher.matcher(request("/uri/bob")).getVariables()).containsEntry("username", "bob"); } @Test void matcherWhenOnlyPathInfoMatchesThenMatches() { - RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/uri"); + RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/uri").matcher(); assertThat(matcher.matches(request("GET", "/mvc/uri", "/mvc"))).isTrue(); } @Test void matcherWhenUriContainsServletPathThenNoMatch() { - RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/mvc/uri"); + RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/mvc/uri").matcher(); assertThat(matcher.matches(request("GET", "/mvc/uri", "/mvc"))).isFalse(); } @Test void matcherWhenSameMethodThenMatchResult() { - RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).pathPatterns("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().method(HttpMethod.GET).pattern("/uri").matcher(); assertThat(matcher.matches(request("/uri"))).isTrue(); } @Test void matcherWhenDifferentPathThenNoMatch() { - RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).pathPatterns("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().method(HttpMethod.GET).pattern("/uri").matcher(); assertThat(matcher.matches(request("GET", "/urj", ""))).isFalse(); } @Test void matcherWhenDifferentMethodThenNoMatch() { - RequestMatcher matcher = RequestMatchers.request().methods(HttpMethod.GET).pathPatterns("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().method(HttpMethod.GET).pattern("/uri").matcher(); assertThat(matcher.matches(request("POST", "/mvc/uri", "/mvc"))).isFalse(); } @Test void matcherWhenNoMethodThenMatches() { - RequestMatcher matcher = PathPatternRequestMatcher.pathPattern("/uri"); + RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/uri").matcher(); assertThat(matcher.matches(request("POST", "/uri", ""))).isTrue(); assertThat(matcher.matches(request("GET", "/uri", ""))).isTrue(); } + @Test + void matcherWhenServletPathThenMatchesOnlyServletPath() { + PathPatternRequestMatcher.Builder servlet = PathPatternRequestMatcher.servletPath("/servlet/path"); + RequestMatcher matcher = servlet.method(HttpMethod.GET).pattern("/endpoint").matcher(); + ServletContext servletContext = servletContext("/servlet/path"); + assertThat(matcher + .matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(servletContext))).isTrue(); + assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(servletContext))).isFalse(); + } + + @Test + void matcherWhenRequestPathThenIgnoresServletPath() { + PathPatternRequestMatcher.Builder request = PathPatternRequestMatcher.path(); + RequestMatcher matcher = request.method(HttpMethod.GET).pattern("/endpoint").matcher(); + assertThat(matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))) + .isTrue(); + assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(null))).isTrue(); + } + + @Test + void matcherWhenServletPathThenRequiresServletPathToExist() { + PathPatternRequestMatcher.Builder servlet = PathPatternRequestMatcher.servletPath("/servlet/path"); + RequestMatcher matcher = servlet.method(HttpMethod.GET).pattern("/endpoint").matcher(); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy( + () -> matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))); + } + + @Test + void servletPathWhenEndsWithSlashOrStarThenIllegalArgument() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> PathPatternRequestMatcher.servletPath("/path/**")); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> PathPatternRequestMatcher.servletPath("/path/*")); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> PathPatternRequestMatcher.servletPath("/path/")); + } + MockHttpServletRequest request(String uri) { MockHttpServletRequest request = new MockHttpServletRequest("GET", uri); ServletRequestPathUtils.parseAndCache(request); @@ -93,4 +135,13 @@ MockHttpServletRequest request(String method, String uri, String servletPath) { return request; } + MockServletContext servletContext(String... servletPath) { + MockServletContext servletContext = new MockServletContext(); + ServletRegistration.Dynamic registration = servletContext.addServlet("servlet", Servlet.class); + for (String s : servletPath) { + registration.addMapping(s + "/*"); + } + return servletContext; + } + } From 7d92871243dd6b16ec4135c8fd0534fa4dc02658 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Mon, 10 Feb 2025 13:02:45 -0700 Subject: [PATCH 05/10] Polish Usage Updates --- .../web/AbstractRequestMatcherRegistry.java | 2 +- .../AuthorizeHttpRequestsConfigurerTests.java | 6 +- docs/modules/ROOT/pages/migration-7/web.adoc | 10 +- .../authorize-http-requests.adoc | 14 +- .../web/util/matcher/RequestMatchers.java | 279 ------------------ .../util/matcher/RequestMatchersTests.java | 54 ---- 6 files changed, 16 insertions(+), 349 deletions(-) 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 66f4daf1a71..c76eb1bb027 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 @@ -268,7 +268,7 @@ private static String computeErrorMessage(Collection> mappings = new LinkedHashMap<>(); 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 cd7e9873430..09e1bbd8a7b 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,7 +64,7 @@ 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.util.matcher.RequestMatchers; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.test.web.servlet.request.RequestPostProcessor; @@ -1343,11 +1343,11 @@ static class MvcRequestMatcherBuilderConfig { @Bean SecurityFilterChain security(HttpSecurity http) throws Exception { - RequestMatchers.Builder mvc = RequestMatchers.servletPath("/mvc"); + PathPatternRequestMatcher.Builder mvc = PathPatternRequestMatcher.servletPath("/mvc"); // @formatter:off http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(mvc.pathPatterns("/path/**").matcher()).hasRole("USER") + .requestMatchers(mvc.pattern("/path/**").matcher()).hasRole("USER") ) .httpBasic(withDefaults()); // @formatter:on diff --git a/docs/modules/ROOT/pages/migration-7/web.adoc b/docs/modules/ROOT/pages/migration-7/web.adoc index 4bd1eff7d7d..73309411001 100644 --- a/docs/modules/ROOT/pages/migration-7/web.adoc +++ b/docs/modules/ROOT/pages/migration-7/web.adoc @@ -119,22 +119,22 @@ Instead of taking this responsibility away from developers, now it is simpler to [method,java] ---- -RequestMatchers.Builder servlet = RequestMatchers.servlet("/mvc"); +PathPatternRequestParser.Builder servlet = PathPatternRequestParser.servletPath("/mvc"); http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(servlet.uris("/orders/**").matcher()).authenticated() + .requestMatchers(servlet.pattern("/orders/**").matcher()).authenticated() ) ---- -For paths that belong to the default servlet, use `RequestMatchers.request()` instead: +For paths that belong to the default servlet, use `PathPatternRequestParser.path()` instead: [method,java] ---- -RequestMatchers.Builder request = RequestMatchers.request(); +PathPatternRequestParser.Builder request = PathPatternRequestParser.path(); http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(request.uris("/js/**").matcher()).authenticated() + .requestMatchers(request.pattern("/js/**").matcher()).authenticated() ) ---- 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 82b922e8e8d..fcef19ad009 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc @@ -590,14 +590,14 @@ Java:: + [source,java,role="primary"] ---- -import static org.springframework.security.web.servlet.util.matcher.RequestMatchers.servlet; +import static org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher.servletPath; @Bean SecurityFilterChain appEndpoints(HttpSecurity http) { http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(servlet("/spring-mvc").uris("/admin/**")).hasAuthority("admin") - .requestMatchers(servlet("/spring-mvc").uris("/my/controller/**")).hasAuthority("controller") + .requestMatchers(servletPath("/spring-mvc").pattern("/admin/**").matcher()).hasAuthority("admin") + .requestMatchers(servletPath("/spring-mvc").pattern("/my/controller/**").matcher()).hasAuthority("controller") .anyRequest().authenticated() ); @@ -639,15 +639,15 @@ With Java, note that the `ServletRequestMatcherBuilders` return value can be reu [source,java,role="primary"] ---- -import static org.springframework.security.web.servlet.util.matcher.RequestMatchers.servlet; +import static org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher.servletPath; @Bean SecurityFilterChain appEndpoints(HttpSecurity http) { - RequestMatchers.Builder mvc = servlet("/spring-mvc"); + PathPatternRequestMatcher.Builder mvc = servletPath("/spring-mvc"); http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(mvc.uris("/admin/**")).hasAuthority("admin") - .requestMatchers(mvc.uris("/my/controller/**")).hasAuthority("controller") + .requestMatchers(mvc.pattern("/admin/**").matcher()).hasAuthority("admin") + .requestMatchers(mvc.pattern("/my/controller/**").matcher()).hasAuthority("controller") .anyRequest().authenticated() ); diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java index 09dc67f510c..a4da49deeed 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java @@ -16,32 +16,7 @@ package org.springframework.security.web.util.matcher; -import java.nio.charset.StandardCharsets; -import java.util.Collection; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.concurrent.atomic.AtomicReference; - -import jakarta.servlet.DispatcherType; -import jakarta.servlet.RequestDispatcher; -import jakarta.servlet.ServletContext; -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.lang.Nullable; -import org.springframework.security.web.access.intercept.RequestAuthorizationContext; -import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; -import org.springframework.util.Assert; -import org.springframework.util.ObjectUtils; -import org.springframework.web.util.UriUtils; -import org.springframework.web.util.WebUtils; -import org.springframework.web.util.pattern.PathPattern; -import org.springframework.web.util.pattern.PathPatternParser; /** * A factory class to create {@link RequestMatcher} instances. @@ -85,261 +60,7 @@ public static RequestMatcher not(RequestMatcher matcher) { return (request) -> !matcher.matches(request); } - /** - * Create {@link RequestMatcher}s whose URIs do not have a servlet path prefix - *

- * When there is no context path, then these URIs are effectively absolute. - * @return a {@link Builder} that treats URIs as relative to the context path, if any - * @since 6.5 - */ - public static Builder request() { - return new Builder(); - } - - /** - * Create {@link RequestMatcher}s whose URIs are relative to the given - * {@code servletPath} prefix. - * - *

- * The {@code servletPath} must correlate to a value that would match the result of - * {@link HttpServletRequest#getServletPath()} and its corresponding servlet. - * - *

- * That is, if you have a servlet mapping of {@code /path/*}, then - * {@link HttpServletRequest#getServletPath()} would return {@code /path} and so - * {@code /path} is what is specified here. - * - * Specify the path here without the trailing {@code /*}. - * @return a {@link Builder} that treats URIs as relative to the given - * {@code servletPath} - * @since 6.5 - */ - public static Builder 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 a slash"); - Assert.isTrue(!servletPath.contains("*"), "servletPath must not contain a star"); - return new Builder(servletPath); - } - private RequestMatchers() { } - /** - * A builder for specifying various elements of a request for the purpose of creating - * a {@link RequestMatcher}. - * - *

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

- * - * - * http - * .authorizeHttpRequests((authorize) -> authorize - * .requestMatchers(servlet("/mvc").uris("/user/**")).hasAuthority("user") - * .requestMatchers(servlet("/other").uris("/admin/**")).hasAuthority("admin") - * ) - * ... - * - * - * @author Josh Cummings - * @since 6.5 - */ - public static final class Builder { - - private final RequestMatcher servletPath; - - private final RequestMatcher methods; - - private final RequestMatcher uris; - - private final RequestMatcher dispatcherTypes; - - private Builder() { - this(AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, - AnyRequestMatcher.INSTANCE); - } - - private Builder(String servletPath) { - this(new ServletPathRequestMatcher(servletPath), AnyRequestMatcher.INSTANCE, AnyRequestMatcher.INSTANCE, - AnyRequestMatcher.INSTANCE); - } - - private Builder(RequestMatcher servletPath, RequestMatcher methods, RequestMatcher uris, - RequestMatcher dispatcherTypes) { - this.servletPath = servletPath; - this.methods = methods; - this.uris = uris; - this.dispatcherTypes = dispatcherTypes; - } - - /** - * Match requests with any of these methods - * @param methods the {@link HttpMethod} to match - * @return the {@link Builder} for more configuration - */ - public Builder methods(HttpMethod... methods) { - RequestMatcher[] matchers = new RequestMatcher[methods.length]; - for (int i = 0; i < methods.length; i++) { - matchers[i] = new HttpMethodRequestMatcher(methods[i]); - } - return new Builder(this.servletPath, anyOf(matchers), this.uris, this.dispatcherTypes); - } - - /** - * Match requests with any of these path patterns - * - *

- * Path patterns always start with a slash and may contain placeholders. They can - * also be followed by {@code /**} to signify all URIs under a given path. - * - *

- * These must be specified relative to any servlet path prefix (meaning you should - * exclude the context path and any servlet path prefix in stating your pattern). - * - *

- * The following are valid patterns and their meaning - *

- * - *

- * A more comprehensive list can be found at {@link PathPattern}. - * @param pathPatterns the path patterns to match - * @return the {@link Builder} for more configuration - */ - public Builder pathPatterns(String... pathPatterns) { - RequestMatcher[] matchers = new RequestMatcher[pathPatterns.length]; - for (int i = 0; i < pathPatterns.length; i++) { - Assert.isTrue(pathPatterns[i].startsWith("/"), "path patterns must start with /"); - PathPatternParser parser = PathPatternParser.defaultInstance; - matchers[i] = new PathPatternRequestMatcher(parser.parse(pathPatterns[i])); - } - return new Builder(this.servletPath, this.methods, anyOf(matchers), this.dispatcherTypes); - } - - /** - * Match requests with any of these {@link PathPattern}s - * - *

- * Use this when you have a non-default {@link PathPatternParser} - * @param pathPatterns the URIs to match - * @return the {@link Builder} for more configuration - */ - public Builder pathPatterns(PathPattern... pathPatterns) { - RequestMatcher[] matchers = new RequestMatcher[pathPatterns.length]; - for (int i = 0; i < pathPatterns.length; i++) { - matchers[i] = new PathPatternRequestMatcher(pathPatterns[i]); - } - return new Builder(this.servletPath, this.methods, anyOf(matchers), this.dispatcherTypes); - } - - /** - * Match requests with any of these dispatcherTypes - * @param dispatcherTypes the {@link DispatcherType}s to match - * @return the {@link Builder} for more configuration - */ - public Builder dispatcherTypes(DispatcherType... dispatcherTypes) { - RequestMatcher[] matchers = new RequestMatcher[dispatcherTypes.length]; - for (int i = 0; i < dispatcherTypes.length; i++) { - matchers[i] = new DispatcherTypeRequestMatcher(dispatcherTypes[i]); - } - return new Builder(this.servletPath, this.methods, this.uris, anyOf(matchers)); - } - - /** - * Create the {@link RequestMatcher} - * @return the composite {@link RequestMatcher} - */ - public RequestMatcher matcher() { - return allOf(this.servletPath, this.methods, this.uris, this.dispatcherTypes); - } - - } - - private record HttpMethodRequestMatcher(HttpMethod method) implements RequestMatcher { - - @Override - public boolean matches(HttpServletRequest request) { - return this.method.name().equals(request.getMethod()); - } - - @Override - public String toString() { - return "HttpMethod [" + this.method + "]"; - } - - } - - private static final class ServletPathRequestMatcher implements RequestMatcher { - - private final String path; - - private final AtomicReference servletExists = new AtomicReference(); - - ServletPathRequestMatcher(String servletPath) { - this.path = servletPath; - } - - @Override - public boolean matches(HttpServletRequest request) { - Assert.isTrue(servletExists(request), () -> this.path + "/* does not exist in your servlet registration " - + registrationMappings(request)); - return Objects.equals(this.path, getServletPathPrefix(request)); - } - - private boolean servletExists(HttpServletRequest request) { - return this.servletExists.updateAndGet((value) -> { - if (value != null) { - return value; - } - if (request.getAttribute("org.springframework.test.web.servlet.MockMvc.MVC_RESULT_ATTRIBUTE") != null) { - return true; - } - for (ServletRegistration registration : request.getServletContext() - .getServletRegistrations() - .values()) { - if (registration.getMappings().contains(this.path + "/*")) { - return true; - } - } - return false; - }); - } - - private Map> registrationMappings(HttpServletRequest request) { - Map> map = new LinkedHashMap<>(); - ServletContext servletContext = request.getServletContext(); - for (ServletRegistration registration : servletContext.getServletRegistrations().values()) { - map.put(registration.getName(), registration.getMappings()); - } - return map; - } - - @Nullable - private static String getServletPathPrefix(HttpServletRequest request) { - HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING); - mapping = (mapping != null) ? mapping : request.getHttpServletMapping(); - if (ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) { - String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE); - servletPath = (servletPath != null) ? servletPath : request.getServletPath(); - servletPath = servletPath.endsWith("/") ? servletPath.substring(0, servletPath.length() - 1) - : servletPath; - return UriUtils.encodePath(servletPath, StandardCharsets.UTF_8); - } - return null; - } - - @Override - public String toString() { - return "ServletPath [" + this.path + "]"; - } - - } - } diff --git a/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java b/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java index befb1e56a52..abff0c345c6 100644 --- a/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java +++ b/web/src/test/java/org/springframework/security/web/util/matcher/RequestMatchersTests.java @@ -16,17 +16,9 @@ package org.springframework.security.web.util.matcher; -import jakarta.servlet.Servlet; -import jakarta.servlet.ServletContext; -import jakarta.servlet.ServletRegistration; import org.junit.jupiter.api.Test; -import org.springframework.http.HttpMethod; -import org.springframework.security.web.servlet.MockServletContext; - import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; /** * Tests for {@link RequestMatchers}. @@ -91,50 +83,4 @@ void checkNotWhenNotMatchThenMatch() { assertThat(match).isTrue(); } - @Test - void matcherWhenServletPathThenMatchesOnlyServletPath() { - RequestMatchers.Builder servlet = RequestMatchers.servletPath("/servlet/path"); - RequestMatcher matcher = servlet.methods(HttpMethod.GET).pathPatterns("/endpoint").matcher(); - ServletContext servletContext = servletContext("/servlet/path"); - assertThat(matcher - .matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(servletContext))).isTrue(); - assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(servletContext))).isFalse(); - } - - @Test - void matcherWhenRequestPathThenIgnoresServletPath() { - RequestMatchers.Builder request = RequestMatchers.request(); - RequestMatcher matcher = request.methods(HttpMethod.GET).pathPatterns("/endpoint").matcher(); - assertThat(matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))) - .isTrue(); - assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(null))).isTrue(); - } - - @Test - void matcherWhenServletPathThenRequiresServletPathToExist() { - RequestMatchers.Builder servlet = RequestMatchers.servletPath("/servlet/path"); - RequestMatcher matcher = servlet.methods(HttpMethod.GET).pathPatterns("/endpoint").matcher(); - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy( - () -> matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))); - } - - @Test - void servletPathWhenEndsWithSlashOrStarThenIllegalArgument() { - assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> RequestMatchers.servletPath("/path/**")); - assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> RequestMatchers.servletPath("/path/*")); - assertThatExceptionOfType(IllegalArgumentException.class) - .isThrownBy(() -> RequestMatchers.servletPath("/path/")); - } - - MockServletContext servletContext(String... servletPath) { - MockServletContext servletContext = new MockServletContext(); - ServletRegistration.Dynamic registration = servletContext.addServlet("servlet", Servlet.class); - for (String s : servletPath) { - registration.addMapping(s + "/*"); - } - return servletContext; - } - } From 089925303c8b2addfe82232c3f7ad3b6bc9409a0 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Tue, 11 Feb 2025 17:47:02 -0700 Subject: [PATCH 06/10] Add Ability to Opt-in to PathPattern Closes gh-16573 --- .../web/AbstractRequestMatcherRegistry.java | 22 ++++- .../annotation/web/builders/HttpSecurity.java | 32 +++---- .../web/configurers/FormLoginConfigurer.java | 11 ++- .../web/configurers/LogoutConfigurer.java | 11 ++- .../PasswordManagementConfigurer.java | 10 +- .../configurers/RequestCacheConfigurer.java | 12 ++- .../oauth2/client/OAuth2LoginConfigurer.java | 13 ++- .../ott/OneTimeTokenLoginConfigurer.java | 16 +++- .../saml2/Saml2LoginConfigurer.java | 64 +++++++++---- .../saml2/Saml2LogoutConfigurer.java | 14 ++- .../saml2/Saml2MetadataConfigurer.java | 11 ++- .../AbstractRequestMatcherRegistryTests.java | 12 +++ .../AuthorizeHttpRequestsConfigurerTests.java | 2 +- docs/modules/ROOT/pages/migration-7/web.adoc | 40 -------- docs/modules/ROOT/pages/migration/web.adoc | 92 +++++++++++++++++++ .../matcher/PathPatternRequestMatcher.java | 92 ++++--------------- .../util/matcher/AntPathRequestMatcher.java | 3 +- .../MethodPatternRequestMatcherFactory.java | 35 +++++++ .../PathPatternRequestMatcherTests.java | 22 ++--- 19 files changed, 328 insertions(+), 186 deletions(-) create mode 100644 docs/modules/ROOT/pages/migration/web.adoc create mode 100644 web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.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 c76eb1bb027..ef5aee64c35 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 @@ -46,6 +46,7 @@ import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -218,10 +219,9 @@ public C requestMatchers(HttpMethod method, String... patterns) { return requestMatchers(RequestMatchers.antMatchersAsArray(method, patterns)); } List matchers = new ArrayList<>(); + MethodPatternRequestMatcherFactory requestMatcherFactory = getRequestMatcherFactory(); 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)); + matchers.add(requestMatcherFactory.matcher(method, pattern)); } return requestMatchers(matchers.toArray(new RequestMatcher[0])); } @@ -331,6 +331,11 @@ public C requestMatchers(HttpMethod method) { */ protected abstract C chainRequestMatchers(List requestMatchers); + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return this.context.getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(DefaultMethodPatternRequestMatcherFactory::new); + } + /** * Utilities for creating {@link RequestMatcher} instances. * @@ -404,6 +409,17 @@ static List regexMatchers(String... regexPatterns) { } + class DefaultMethodPatternRequestMatcherFactory implements MethodPatternRequestMatcherFactory { + + @Override + public RequestMatcher matcher(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/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java index f96c943d557..2bef03a10e8 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java @@ -93,6 +93,7 @@ import org.springframework.security.web.session.HttpSessionEventPublisher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; @@ -3684,11 +3685,14 @@ public HttpSecurity securityMatcher(RequestMatcher requestMatcher) { * @see MvcRequestMatcher */ public HttpSecurity securityMatcher(String... patterns) { - if (mvcPresent) { - this.requestMatcher = new OrRequestMatcher(createMvcMatchers(patterns)); - return this; + List matchers = new ArrayList<>(); + MethodPatternRequestMatcherFactory factory = getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> (method, pattern) -> mvcPresent ? createMvcMatcher(pattern) : createAntMatcher(pattern)); + for (String pattern : patterns) { + matchers.add(factory.matcher(pattern)); } - this.requestMatcher = new OrRequestMatcher(createAntMatchers(patterns)); + this.requestMatcher = new OrRequestMatcher(matchers); return this; } @@ -3717,15 +3721,11 @@ public HttpSecurity webAuthn(Customizer> webAut return HttpSecurity.this; } - private List createAntMatchers(String... patterns) { - List matchers = new ArrayList<>(patterns.length); - for (String pattern : patterns) { - matchers.add(new AntPathRequestMatcher(pattern)); - } - return matchers; + private RequestMatcher createAntMatcher(String pattern) { + return new AntPathRequestMatcher(pattern); } - private List createMvcMatchers(String... mvcPatterns) { + private RequestMatcher createMvcMatcher(String mvcPattern) { ResolvableType type = ResolvableType.forClassWithGenerics(ObjectPostProcessor.class, Object.class); ObjectProvider> postProcessors = getContext().getBeanProvider(type); ObjectPostProcessor opp = postProcessors.getObject(); @@ -3736,13 +3736,9 @@ private List createMvcMatchers(String... mvcPatterns) { } HandlerMappingIntrospector introspector = getContext().getBean(HANDLER_MAPPING_INTROSPECTOR_BEAN_NAME, HandlerMappingIntrospector.class); - List matchers = new ArrayList<>(mvcPatterns.length); - for (String mvcPattern : mvcPatterns) { - MvcRequestMatcher matcher = new MvcRequestMatcher(introspector, mvcPattern); - opp.postProcess(matcher); - matchers.add(matcher); - } - return matchers; + MvcRequestMatcher matcher = new MvcRequestMatcher(introspector, mvcPattern); + opp.postProcess(matcher); + return matcher; } /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java index b28e57e4d37..c3b5ee3e80c 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java @@ -16,6 +16,8 @@ package org.springframework.security.config.annotation.web.configurers; +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; @@ -27,6 +29,7 @@ import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; /** @@ -234,7 +237,7 @@ public void init(H http) throws Exception { @Override protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingUrl) { - return new AntPathRequestMatcher(loginProcessingUrl, "POST"); + return getRequestMatcherFactory().matcher(HttpMethod.POST, loginProcessingUrl); } /** @@ -271,4 +274,10 @@ private void initDefaultLoginFilter(H http) { } } + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + } + } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java index 04a2ac35cd1..ebaa40c1916 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java @@ -22,6 +22,8 @@ import jakarta.servlet.http.HttpSession; +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; import org.springframework.security.config.annotation.SecurityConfigurer; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; @@ -38,6 +40,7 @@ import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.security.web.context.SecurityContextRepository; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; @@ -368,7 +371,13 @@ private RequestMatcher createLogoutRequestMatcher(H http) { } private RequestMatcher createLogoutRequestMatcher(String httpMethod) { - return new AntPathRequestMatcher(this.logoutUrl, httpMethod); + return getRequestMatcherFactory().matcher(HttpMethod.valueOf(httpMethod), this.logoutUrl); + } + + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index 0f9b52f6570..74037541df0 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -16,10 +16,12 @@ package org.springframework.security.config.annotation.web.configurers; +import org.springframework.context.ApplicationContext; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.web.RequestMatcherRedirectFilter; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.util.Assert; /** @@ -55,8 +57,14 @@ public PasswordManagementConfigurer changePasswordPage(String changePasswordP @Override public void configure(B http) throws Exception { RequestMatcherRedirectFilter changePasswordFilter = new RequestMatcherRedirectFilter( - new AntPathRequestMatcher(WELL_KNOWN_CHANGE_PASSWORD_PATTERN), this.changePasswordPage); + getRequestMatcherFactory().matcher(WELL_KNOWN_CHANGE_PASSWORD_PATTERN), this.changePasswordPage); http.addFilterBefore(postProcess(changePasswordFilter), UsernamePasswordAuthenticationFilter.class); } + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + } + } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java index 712c89073f8..240c9aa355f 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java @@ -21,6 +21,7 @@ import java.util.List; import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; @@ -31,6 +32,7 @@ import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.MediaTypeRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -140,13 +142,13 @@ private T getBeanOrNull(Class type) { @SuppressWarnings("unchecked") private RequestMatcher createDefaultSavedRequestMatcher(H http) { - RequestMatcher notFavIcon = new NegatedRequestMatcher(new AntPathRequestMatcher("/**/favicon.*")); + RequestMatcher notFavIcon = new NegatedRequestMatcher(getRequestMatcherFactory().matcher("/**/favicon.*")); RequestMatcher notXRequestedWith = new NegatedRequestMatcher( new RequestHeaderRequestMatcher("X-Requested-With", "XMLHttpRequest")); boolean isCsrfEnabled = http.getConfigurer(CsrfConfigurer.class) != null; List matchers = new ArrayList<>(); if (isCsrfEnabled) { - RequestMatcher getRequests = new AntPathRequestMatcher("/**", "GET"); + RequestMatcher getRequests = getRequestMatcherFactory().matcher(HttpMethod.GET, "/**"); matchers.add(0, getRequests); } matchers.add(notFavIcon); @@ -167,4 +169,10 @@ private RequestMatcher notMatchingMediaType(H http, MediaType mediaType) { return new NegatedRequestMatcher(mediaRequest); } + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + } + } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java index 4c53b3293d0..7bafbba305b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java @@ -93,6 +93,7 @@ import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; @@ -431,7 +432,7 @@ public void configure(B http) throws Exception { @Override protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingUrl) { - return new AntPathRequestMatcher(loginProcessingUrl); + return getRequestMatcherFactory().matcher(loginProcessingUrl); } private OAuth2AuthorizationRequestResolver getAuthorizationRequestResolver() { @@ -569,8 +570,8 @@ private Map getLoginLinks() { } private AuthenticationEntryPoint getLoginEntryPoint(B http, String providerLoginPage) { - RequestMatcher loginPageMatcher = new AntPathRequestMatcher(this.getLoginPage()); - RequestMatcher faviconMatcher = new AntPathRequestMatcher("/favicon.ico"); + RequestMatcher loginPageMatcher = getRequestMatcherFactory().matcher(this.getLoginPage()); + RequestMatcher faviconMatcher = getRequestMatcherFactory().matcher("/favicon.ico"); RequestMatcher defaultEntryPointMatcher = this.getAuthenticationEntryPointMatcher(http); RequestMatcher defaultLoginPageMatcher = new AndRequestMatcher( new OrRequestMatcher(loginPageMatcher, faviconMatcher), defaultEntryPointMatcher); @@ -625,6 +626,12 @@ private void registerDelegateApplicationListener(ApplicationListener delegate delegating.addListener(smartListener); } + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + } + /** * Configuration options for the Authorization Server's Authorization Endpoint. */ diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java index 6f1e02ca6ed..33e0444f0db 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java @@ -52,12 +52,12 @@ import org.springframework.security.web.authentication.ui.DefaultOneTimeTokenSubmitPageGeneratingFilter; import org.springframework.security.web.authentication.ui.DefaultResourcesFilter; import org.springframework.security.web.csrf.CsrfToken; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.util.StringUtils; -import static org.springframework.security.web.util.matcher.AntPathRequestMatcher.antMatcher; - /** * An {@link AbstractHttpConfigurer} for One-Time Token Login. * @@ -163,7 +163,7 @@ public void configure(H http) throws Exception { private void configureOttGenerateFilter(H http) { GenerateOneTimeTokenFilter generateFilter = new GenerateOneTimeTokenFilter(getOneTimeTokenService(), getOneTimeTokenGenerationSuccessHandler()); - generateFilter.setRequestMatcher(antMatcher(HttpMethod.POST, this.tokenGeneratingUrl)); + generateFilter.setRequestMatcher(getRequestMatcherFactory().matcher(HttpMethod.POST, this.tokenGeneratingUrl)); generateFilter.setRequestResolver(getGenerateRequestResolver()); http.addFilter(postProcess(generateFilter)); http.addFilter(DefaultResourcesFilter.css()); @@ -190,7 +190,7 @@ private void configureSubmitPage(H http) { } DefaultOneTimeTokenSubmitPageGeneratingFilter submitPage = new DefaultOneTimeTokenSubmitPageGeneratingFilter(); submitPage.setResolveHiddenInputs(this::hiddenInputs); - submitPage.setRequestMatcher(antMatcher(HttpMethod.GET, this.defaultSubmitPageUrl)); + submitPage.setRequestMatcher(getRequestMatcherFactory().matcher(HttpMethod.GET, this.defaultSubmitPageUrl)); submitPage.setLoginProcessingUrl(this.getLoginProcessingUrl()); http.addFilter(postProcess(submitPage)); } @@ -207,7 +207,13 @@ private AuthenticationProvider getAuthenticationProvider() { @Override protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingUrl) { - return antMatcher(HttpMethod.POST, loginProcessingUrl); + return getRequestMatcherFactory().matcher(HttpMethod.POST, loginProcessingUrl); + } + + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java index b07b034d143..88c49e2835b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java @@ -57,6 +57,7 @@ import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.ParameterRequestMatcher; @@ -127,15 +128,11 @@ public final class Saml2LoginConfigurer> private String[] authenticationRequestParams = { "registrationId={registrationId}" }; - private RequestMatcher authenticationRequestMatcher = RequestMatchers.anyOf( - new AntPathRequestMatcher(Saml2AuthenticationRequestResolver.DEFAULT_AUTHENTICATION_REQUEST_URI), - new AntPathQueryRequestMatcher(this.authenticationRequestUri, this.authenticationRequestParams)); + private RequestMatcher authenticationRequestMatcher; private Saml2AuthenticationRequestResolver authenticationRequestResolver; - private RequestMatcher loginProcessingUrl = RequestMatchers.anyOf( - new AntPathRequestMatcher(Saml2WebSsoAuthenticationFilter.DEFAULT_FILTER_PROCESSES_URI), - new AntPathRequestMatcher("/login/saml2/sso")); + private RequestMatcher loginProcessingUrl; private RelyingPartyRegistrationRepository relyingPartyRegistrationRepository; @@ -238,8 +235,8 @@ public Saml2LoginConfigurer authenticationRequestUriQuery(String authenticati this.authenticationRequestUri = parts[0]; this.authenticationRequestParams = new String[parts.length - 1]; System.arraycopy(parts, 1, this.authenticationRequestParams, 0, parts.length - 1); - this.authenticationRequestMatcher = new AntPathQueryRequestMatcher(this.authenticationRequestUri, - this.authenticationRequestParams); + this.authenticationRequestMatcher = new AntPathQueryRequestMatcher( + getRequestMatcherFactory().matcher(this.authenticationRequestUri), this.authenticationRequestParams); return this; } @@ -256,13 +253,13 @@ public Saml2LoginConfigurer authenticationRequestUriQuery(String authenticati @Override public Saml2LoginConfigurer loginProcessingUrl(String loginProcessingUrl) { Assert.hasText(loginProcessingUrl, "loginProcessingUrl cannot be empty"); - this.loginProcessingUrl = new AntPathRequestMatcher(loginProcessingUrl); + this.loginProcessingUrl = getRequestMatcherFactory().matcher(loginProcessingUrl); return this; } @Override protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingUrl) { - return new AntPathRequestMatcher(loginProcessingUrl); + return getRequestMatcherFactory().matcher(loginProcessingUrl); } /** @@ -284,7 +281,7 @@ public void init(B http) throws Exception { relyingPartyRegistrationRepository(http); this.saml2WebSsoAuthenticationFilter = new Saml2WebSsoAuthenticationFilter(getAuthenticationConverter(http)); this.saml2WebSsoAuthenticationFilter.setSecurityContextHolderStrategy(getSecurityContextHolderStrategy()); - this.saml2WebSsoAuthenticationFilter.setRequiresAuthenticationRequestMatcher(this.loginProcessingUrl); + this.saml2WebSsoAuthenticationFilter.setRequiresAuthenticationRequestMatcher(getLoginProcessingEndpoint()); setAuthenticationRequestRepository(http, this.saml2WebSsoAuthenticationFilter); setAuthenticationFilter(this.saml2WebSsoAuthenticationFilter); if (StringUtils.hasText(this.loginPage)) { @@ -340,8 +337,8 @@ RelyingPartyRegistrationRepository relyingPartyRegistrationRepository(B http) { } private AuthenticationEntryPoint getLoginEntryPoint(B http, String providerLoginPage) { - RequestMatcher loginPageMatcher = new AntPathRequestMatcher(this.getLoginPage()); - RequestMatcher faviconMatcher = new AntPathRequestMatcher("/favicon.ico"); + RequestMatcher loginPageMatcher = getRequestMatcherFactory().matcher(this.getLoginPage()); + RequestMatcher faviconMatcher = getRequestMatcherFactory().matcher("/favicon.ico"); RequestMatcher defaultEntryPointMatcher = this.getAuthenticationEntryPointMatcher(http); RequestMatcher defaultLoginPageMatcher = new AndRequestMatcher( new OrRequestMatcher(loginPageMatcher, faviconMatcher), defaultEntryPointMatcher); @@ -376,17 +373,38 @@ private Saml2AuthenticationRequestResolver getAuthenticationRequestResolver(B ht if (USE_OPENSAML_5) { OpenSaml5AuthenticationRequestResolver openSamlAuthenticationRequestResolver = new OpenSaml5AuthenticationRequestResolver( relyingPartyRegistrationRepository(http)); - openSamlAuthenticationRequestResolver.setRequestMatcher(this.authenticationRequestMatcher); + openSamlAuthenticationRequestResolver.setRequestMatcher(getAuthenticationRequestMatcher()); return openSamlAuthenticationRequestResolver; } else { OpenSaml4AuthenticationRequestResolver openSamlAuthenticationRequestResolver = new OpenSaml4AuthenticationRequestResolver( relyingPartyRegistrationRepository(http)); - openSamlAuthenticationRequestResolver.setRequestMatcher(this.authenticationRequestMatcher); + openSamlAuthenticationRequestResolver.setRequestMatcher(getAuthenticationRequestMatcher()); return openSamlAuthenticationRequestResolver; } } + private RequestMatcher getAuthenticationRequestMatcher() { + if (this.authenticationRequestMatcher == null) { + this.authenticationRequestMatcher = RequestMatchers.anyOf( + getRequestMatcherFactory() + .matcher(Saml2AuthenticationRequestResolver.DEFAULT_AUTHENTICATION_REQUEST_URI), + new AntPathQueryRequestMatcher(getRequestMatcherFactory().matcher(this.authenticationRequestUri), + this.authenticationRequestParams)); + } + return this.authenticationRequestMatcher; + } + + private RequestMatcher getLoginProcessingEndpoint() { + if (this.loginProcessingUrl == null) { + this.loginProcessingUrl = RequestMatchers.anyOf( + getRequestMatcherFactory().matcher(Saml2WebSsoAuthenticationFilter.DEFAULT_FILTER_PROCESSES_URI), + getRequestMatcherFactory().matcher("/login/saml2/sso")); + } + + return this.loginProcessingUrl; + } + private AuthenticationConverter getAuthenticationConverter(B http) { if (this.authenticationConverter != null) { return this.authenticationConverter; @@ -407,7 +425,7 @@ private AuthenticationConverter getAuthenticationConverter(B http) { OpenSaml5AuthenticationTokenConverter converter = new OpenSaml5AuthenticationTokenConverter( this.relyingPartyRegistrationRepository); converter.setAuthenticationRequestRepository(getAuthenticationRequestRepository(http)); - converter.setRequestMatcher(this.loginProcessingUrl); + converter.setRequestMatcher(getLoginProcessingEndpoint()); return converter; } authenticationConverterBean = getBeanOrNull(http, OpenSaml4AuthenticationTokenConverter.class); @@ -417,7 +435,7 @@ private AuthenticationConverter getAuthenticationConverter(B http) { OpenSaml4AuthenticationTokenConverter converter = new OpenSaml4AuthenticationTokenConverter( this.relyingPartyRegistrationRepository); converter.setAuthenticationRequestRepository(getAuthenticationRequestRepository(http)); - converter.setRequestMatcher(this.loginProcessingUrl); + converter.setRequestMatcher(getLoginProcessingEndpoint()); return converter; } @@ -441,7 +459,7 @@ private void registerDefaultCsrfOverride(B http) { if (csrf == null) { return; } - csrf.ignoringRequestMatchers(this.loginProcessingUrl); + csrf.ignoringRequestMatchers(getLoginProcessingEndpoint()); } private void initDefaultLoginFilter(B http) { @@ -487,6 +505,12 @@ private Saml2AuthenticationRequestRepository return repository; } + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + } + private C getSharedOrBean(B http, Class clazz) { C shared = http.getSharedObject(clazz); if (shared != null) { @@ -513,9 +537,9 @@ static class AntPathQueryRequestMatcher implements RequestMatcher { private final RequestMatcher matcher; - AntPathQueryRequestMatcher(String path, String... params) { + AntPathQueryRequestMatcher(RequestMatcher pathMatcher, String... params) { List matchers = new ArrayList<>(); - matchers.add(new AntPathRequestMatcher(path)); + matchers.add(pathMatcher); for (String param : params) { String[] parts = param.split("="); if (parts.length == 1) { diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java index 92c7cef819f..6acc52d307d 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java @@ -23,6 +23,7 @@ import org.opensaml.core.Version; import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; import org.springframework.security.authentication.AuthenticationManager; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; @@ -65,6 +66,7 @@ import org.springframework.security.web.csrf.CsrfTokenRepository; import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.ParameterRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -304,19 +306,19 @@ private Saml2RelyingPartyInitiatedLogoutFilter createRelyingPartyLogoutFilter( } private RequestMatcher createLogoutMatcher() { - RequestMatcher logout = new AntPathRequestMatcher(this.logoutUrl, "POST"); + RequestMatcher logout = getRequestMatcherFactory().matcher(HttpMethod.POST, this.logoutUrl); RequestMatcher saml2 = new Saml2RequestMatcher(getSecurityContextHolderStrategy()); return new AndRequestMatcher(logout, saml2); } private RequestMatcher createLogoutRequestMatcher() { - RequestMatcher logout = new AntPathRequestMatcher(this.logoutRequestConfigurer.logoutUrl); + RequestMatcher logout = getRequestMatcherFactory().matcher(this.logoutRequestConfigurer.logoutUrl); RequestMatcher samlRequest = new ParameterRequestMatcher("SAMLRequest"); return new AndRequestMatcher(logout, samlRequest); } private RequestMatcher createLogoutResponseMatcher() { - RequestMatcher logout = new AntPathRequestMatcher(this.logoutResponseConfigurer.logoutUrl); + RequestMatcher logout = getRequestMatcherFactory().matcher(this.logoutResponseConfigurer.logoutUrl); RequestMatcher samlResponse = new ParameterRequestMatcher("SAMLResponse"); return new AndRequestMatcher(logout, samlResponse); } @@ -333,6 +335,12 @@ private Saml2LogoutResponseResolver createSaml2LogoutResponseResolver( return this.logoutResponseConfigurer.logoutResponseResolver(registrations); } + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + } + private C getBeanOrNull(Class clazz) { if (this.context == null) { return null; diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java index 349e3a66066..09da6620ba6 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java @@ -33,6 +33,7 @@ import org.springframework.security.saml2.provider.service.web.metadata.RequestMatcherMetadataResponseResolver; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.util.Assert; /** @@ -111,12 +112,12 @@ public Saml2MetadataConfigurer metadataUrl(String metadataUrl) { if (USE_OPENSAML_5) { RequestMatcherMetadataResponseResolver metadata = new RequestMatcherMetadataResponseResolver( registrations, new OpenSaml5MetadataResolver()); - metadata.setRequestMatcher(new AntPathRequestMatcher(metadataUrl)); + metadata.setRequestMatcher(getRequestMatcherFactory().matcher(metadataUrl)); return metadata; } RequestMatcherMetadataResponseResolver metadata = new RequestMatcherMetadataResponseResolver(registrations, new OpenSaml4MetadataResolver()); - metadata.setRequestMatcher(new AntPathRequestMatcher(metadataUrl)); + metadata.setRequestMatcher(getRequestMatcherFactory().matcher(metadataUrl)); return metadata; }; return this; @@ -170,6 +171,12 @@ private RelyingPartyRegistrationRepository getRelyingPartyRegistrationRepository } } + private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + return getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + } + private C getBeanOrNull(Class clazz) { if (this.context == null) { return null; 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 70f383c203a..4f1772e454a 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,12 +24,14 @@ 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; import org.springframework.context.annotation.Configuration; import org.springframework.core.ResolvableType; import org.springframework.http.HttpMethod; +import org.springframework.lang.NonNull; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.config.ObjectPostProcessor; import org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry.DispatcherServletDelegatingRequestMatcher; @@ -40,6 +42,7 @@ import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.test.web.servlet.MockMvc; @@ -86,6 +89,15 @@ public void setUp() { ObjectProvider> given = this.context.getBeanProvider(type); given(given).willReturn(postProcessors); given(postProcessors.getObject()).willReturn(NO_OP_OBJECT_POST_PROCESSOR); + MethodPatternRequestMatcherFactory factory = this.matcherRegistry.new DefaultMethodPatternRequestMatcherFactory(); + ObjectProvider requestMatcherFactory = new ObjectProvider<>() { + @Override + @NonNull + public MethodPatternRequestMatcherFactory getObject() throws BeansException { + return factory; + } + }; + given(this.context.getBeanProvider(MethodPatternRequestMatcherFactory.class)).willReturn(requestMatcherFactory); given(this.context.getServletContext()).willReturn(MockServletContext.mvc()); 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 09e1bbd8a7b..80f3dc76f97 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 @@ -1347,7 +1347,7 @@ SecurityFilterChain security(HttpSecurity http) throws Exception { // @formatter:off http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(mvc.pattern("/path/**").matcher()).hasRole("USER") + .requestMatchers(mvc.matcher("/path/**")).hasRole("USER") ) .httpBasic(withDefaults()); // @formatter:on diff --git a/docs/modules/ROOT/pages/migration-7/web.adoc b/docs/modules/ROOT/pages/migration-7/web.adoc index 73309411001..024d5604494 100644 --- a/docs/modules/ROOT/pages/migration-7/web.adoc +++ b/docs/modules/ROOT/pages/migration-7/web.adoc @@ -102,43 +102,3 @@ Xml:: ---- ====== - -== Include the Servlet Path Prefix in Authorization Rules - -As of Spring Security 7, `AntPathRequestMatcher` and `MvcRequestMatcher` are no longer supported and the Java DSL requires that all URIs be absolute (less any context root). - -For many applications this will make no difference since most commonly all URIs listed are matched by the default servlet. - -However, if you have other servlets with servlet path prefixes, xref:servlet/authorization/authorize-http-requests.adoc[then these paths need to be supplied separately]. - -For example, if I have a Spring MVC controller with `@RequestMapping("/orders")` and my MVC application is deployed to `/mvc` (instead of the default servlet), then the URI for this endpoint is `/mvc/orders`. -Historically, the Java DSL hasn't had a simple way to specify the servlet path prefix and Spring Security attempted to infer it. - -Over time, we learned that these inference would surprise developers. -Instead of taking this responsibility away from developers, now it is simpler to specify the servlet path prefix like so: - -[method,java] ----- -PathPatternRequestParser.Builder servlet = PathPatternRequestParser.servletPath("/mvc"); -http - .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(servlet.pattern("/orders/**").matcher()).authenticated() - ) ----- - - -For paths that belong to the default servlet, use `PathPatternRequestParser.path()` instead: - -[method,java] ----- -PathPatternRequestParser.Builder request = PathPatternRequestParser.path(); -http - .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(request.pattern("/js/**").matcher()).authenticated() - ) ----- - -Note that this doesn't address every kind of servlet since not all servlets have a path prefix. -For example, expressions that match the JSP Servlet might use an ant pattern `/**/*.jsp`. - -There is not yet a general-purpose replacement for these, and so you are encouraged to use `RegexRequestMatcher`, like so: `regexMatcher("\\.jsp$")`. diff --git a/docs/modules/ROOT/pages/migration/web.adoc b/docs/modules/ROOT/pages/migration/web.adoc new file mode 100644 index 00000000000..d59aa65b947 --- /dev/null +++ b/docs/modules/ROOT/pages/migration/web.adoc @@ -0,0 +1,92 @@ += Web Migrations + +[[use-path-pattern]] +== Use PathPatternRequestMatcher by Default + +In Spring Security 7, `AntPathRequestMatcher` and `MvcRequestMatcher` are no longer supported and the Java DSL requires that all URIs be absolute (less any context root). +At that time, Spring Security 7 will use `PathPatternRequestMatcher` by default. + +To check how prepared you are for this change, you can publish this bean: + +[tabs] +====== +Java:: ++ +[source,java,role="primary"] +---- +@Bean +MethodPatternRequestMatcherFactory requestMatcherFactory() { + return PathPatternRequestMatcher.path(); +} +---- + +Kotlin:: ++ +[source,kotlin,role="secondary"] +---- +@Bean +fun requestMatcherFactory(): MethodPatternRequestMatcherFactory { + return PathPatternRequestMatcher.path() +} +---- +====== + +This will tell the Spring Security DSL to use `PathPatternRequestMatcher` for all request matchers that it constructs. + +In the event that you are directly constructing an object (as opposed to having the DSL contstruct it) that has a `setRequestMatcher` method. you should also proactively specify a `PathPatternRequestMatcher` there as well. + +For example, in the case of `LogoutFilter`, it constructs an `AntPathRequestMatcher` in Spring Security 6: + +[method,java] +---- +private RequestMatcher logoutUrl = new AntPathRequestMatcher("/logout"); +---- + +and will change this to a `PathPatternRequestMatcher` in 7: + +[method,java] +---- +private RequestMatcher logoutUrl = PathPatternRequestMatcher.path().matcher("/logout"); +---- + +If you are constructing your own `LogoutFilter`, consider calling `setLogoutRequestMatcher` to provide this `PathPatternRequestMatcher` in advance. + +== Include the Servlet Path Prefix in Authorization Rules + +For many applications <> will make no difference since most commonly all URIs listed are matched by the default servlet. + +However, if you have other servlets with servlet path prefixes, xref:servlet/authorization/authorize-http-requests.adoc[then these paths now need to be supplied separately]. + +For example, if I have a Spring MVC controller with `@RequestMapping("/orders")` and my MVC application is deployed to `/mvc` (instead of the default servlet), then the URI for this endpoint is `/mvc/orders`. +Historically, the Java DSL hasn't had a simple way to specify the servlet path prefix and Spring Security attempted to infer it. + +Over time, we learned that these inference would surprise developers. +Instead of taking this responsibility away from developers, now it is simpler to specify the servlet path prefix like so: + +[method,java] +---- +PathPatternRequestParser.Builder servlet = PathPatternRequestParser.servletPath("/mvc"); +http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(servlet.pattern("/orders/**").matcher()).authenticated() + ) +---- + + +For paths that belong to the default servlet, use `PathPatternRequestParser.path()` instead: + +[method,java] +---- +PathPatternRequestParser.Builder request = PathPatternRequestParser.path(); +http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(request.pattern("/js/**").matcher()).authenticated() + ) +---- + +Note that this doesn't address every kind of servlet since not all servlets have a path prefix. +For example, expressions that match the JSP Servlet might use an ant pattern `/**/*.jsp`. + +There is not yet a general-purpose replacement for these, and so you are encouraged to use `RegexRequestMatcher`, like so: `regexMatcher("\\.jsp$")`. + +For many applications this will make no difference since most commonly all URIs listed are matched by the default servlet. diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java index 55441dc085b..ad5d54c0ed3 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java @@ -36,6 +36,7 @@ import org.springframework.lang.Nullable; import org.springframework.security.web.access.intercept.RequestAuthorizationContext; import org.springframework.security.web.util.matcher.AnyRequestMatcher; +import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; @@ -203,72 +204,32 @@ public String toString() { * * http * .authorizeHttpRequests((authorize) -> authorize - * .requestMatchers(servletPath("/mvc").pattern("/user/**").matcher()).hasAuthority("user") - * .requestMatchers(servletPath("/other").pattern("/admin/**").matcher()).hasAuthority("admin") + * .requestMatchers(servletPath("/mvc").matcher("/user/**")).hasAuthority("user") + * .requestMatchers(servletPath("/other").matcher("/admin/**")).hasAuthority("admin") * ) * ... * */ - public static final class Builder { + public static final class Builder implements MethodPatternRequestMatcherFactory { - private static final PathPattern ANY_PATH = PathPatternParser.defaultInstance.parse("/**"); - - private final RequestMatcher method; + private PathPatternParser parser = PathPatternParser.defaultInstance; private final RequestMatcher servletPath; - private final PathPattern pathPattern; - Builder() { this(AnyRequestMatcher.INSTANCE); } Builder(RequestMatcher servletPath) { - this(AnyRequestMatcher.INSTANCE, servletPath, ANY_PATH); - } - - Builder(RequestMatcher method, RequestMatcher servletPath, PathPattern pathPattern) { - this.method = method; this.servletPath = servletPath; - this.pathPattern = pathPattern; } /** - * Match requests having this path pattern. + * Match requests having this {@link HttpMethod} and path pattern. * *

- * Path patterns always start with a slash and may contain placeholders. They can - * also be followed by {@code /**} to signify all URIs under a given path. - * - *

- * These must be specified relative to any servlet path prefix (meaning you should - * exclude the context path and any servlet path prefix in stating your pattern). - * - *

- * The following are valid patterns and their meaning - *

    - *
  • {@code /path} - match exactly and only `/path`
  • - *
  • {@code /path/**} - match `/path` and any of its descendents
  • - *
  • {@code /path/{value}/**} - match `/path/subdirectory` and any of its - * descendents, capturing the value of the subdirectory in - * {@link RequestAuthorizationContext#getVariables()}
  • - *
- * - *

- * The pattern is parsed using {@link PathPatternParser#defaultInstance} A more - * comprehensive list can be found at {@link PathPattern}. - * @param pathPattern the path pattern to match - * @return the {@link Builder} for more configuration - */ - public Builder pattern(String pathPattern) { - Assert.notNull(pathPattern, "pattern cannot be null"); - Assert.isTrue(pathPattern.startsWith("/"), "pattern must start with a /"); - PathPatternParser parser = PathPatternParser.defaultInstance; - return new Builder(this.method, this.servletPath, parser.parse(pathPattern)); - } - - /** - * Match requests having this path pattern. + * When the HTTP {@code method} is null, then the matcher does not consider the + * HTTP method * *

* Path patterns always start with a slash and may contain placeholders. They can @@ -290,37 +251,22 @@ public Builder pattern(String pathPattern) { * *

* A more comprehensive list can be found at {@link PathPattern}. - * @param pathPattern the path pattern to match + * @param method the {@link HttpMethod} to match, may be null + * @param pattern the path pattern to match * @return the {@link Builder} for more configuration */ - public Builder pattern(PathPattern pathPattern) { - Assert.notNull(pathPattern, "pathPattern cannot be null"); - return new Builder(this.method, this.servletPath, pathPattern); - } - - /** - * Match requests having this {@link HttpMethod}. - * @param method the {@link HttpMethod} to match - * @return the {@link Builder} for more configuration - */ - public Builder method(HttpMethod method) { - Assert.notNull(method, "method cannot be null"); - return new Builder(new HttpMethodRequestMatcher(method), this.servletPath, this.pathPattern); - } - - /** - * Create the {@link PathPatternRequestMatcher}/ - * @return the {@link PathPatternRequestMatcher} - */ - public PathPatternRequestMatcher matcher() { - PathPatternRequestMatcher pathPattern = new PathPatternRequestMatcher(this.pathPattern); - if (this.method != AnyRequestMatcher.INSTANCE) { - pathPattern.setMethod(this.method); + public PathPatternRequestMatcher matcher(@Nullable HttpMethod method, String pattern) { + Assert.notNull(pattern, "pattern cannot be null"); + Assert.isTrue(pattern.startsWith("/"), "pattern must start with a /"); + PathPattern pathPattern = this.parser.parse(pattern); + PathPatternRequestMatcher requestMatcher = new PathPatternRequestMatcher(pathPattern); + if (method != null) { + requestMatcher.setMethod(new HttpMethodRequestMatcher(method)); } if (this.servletPath != AnyRequestMatcher.INSTANCE) { - pathPattern.setServletPath(this.servletPath); + requestMatcher.setServletPath(this.servletPath); } - return pathPattern; + return requestMatcher; } } diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java index e3049b83742..9f3ad5d5672 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/AntPathRequestMatcher.java @@ -100,9 +100,8 @@ public static AntPathRequestMatcher antMatcher(HttpMethod method) { * @since 5.8 */ public static AntPathRequestMatcher antMatcher(HttpMethod method, String pattern) { - Assert.notNull(method, "method cannot be null"); Assert.hasText(pattern, "pattern cannot be empty"); - return new AntPathRequestMatcher(pattern, method.name()); + return new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null); } /** diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java b/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java new file mode 100644 index 00000000000..c7a2c19019a --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java @@ -0,0 +1,35 @@ +/* + * 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.util.matcher; + +import org.springframework.http.HttpMethod; + +/** + * A strategy for constructing request matchers that match method-path pairs + * + * @author Josh Cummings + * @since 6.5 + */ +public interface MethodPatternRequestMatcherFactory { + + default RequestMatcher matcher(String pattern) { + return matcher(null, pattern); + } + + RequestMatcher matcher(HttpMethod method, String pattern); + +} diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java index 04e55ab68a3..c969c7b726b 100644 --- a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java @@ -38,49 +38,49 @@ public class PathPatternRequestMatcherTests { @Test void matcherWhenPatternMatchesRequestThenMatchResult() { - RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().matcher("/uri"); assertThat(matcher.matches(request("/uri"))).isTrue(); } @Test void matcherWhenPatternContainsPlaceholdersThenMatchResult() { - RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/uri/{username}").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().matcher("/uri/{username}"); assertThat(matcher.matcher(request("/uri/bob")).getVariables()).containsEntry("username", "bob"); } @Test void matcherWhenOnlyPathInfoMatchesThenMatches() { - RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().matcher("/uri"); assertThat(matcher.matches(request("GET", "/mvc/uri", "/mvc"))).isTrue(); } @Test void matcherWhenUriContainsServletPathThenNoMatch() { - RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/mvc/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().matcher("/mvc/uri"); assertThat(matcher.matches(request("GET", "/mvc/uri", "/mvc"))).isFalse(); } @Test void matcherWhenSameMethodThenMatchResult() { - RequestMatcher matcher = PathPatternRequestMatcher.path().method(HttpMethod.GET).pattern("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().matcher(HttpMethod.GET, "/uri"); assertThat(matcher.matches(request("/uri"))).isTrue(); } @Test void matcherWhenDifferentPathThenNoMatch() { - RequestMatcher matcher = PathPatternRequestMatcher.path().method(HttpMethod.GET).pattern("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().matcher(HttpMethod.GET, "/uri"); assertThat(matcher.matches(request("GET", "/urj", ""))).isFalse(); } @Test void matcherWhenDifferentMethodThenNoMatch() { - RequestMatcher matcher = PathPatternRequestMatcher.path().method(HttpMethod.GET).pattern("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().matcher(HttpMethod.GET, "/uri"); assertThat(matcher.matches(request("POST", "/mvc/uri", "/mvc"))).isFalse(); } @Test void matcherWhenNoMethodThenMatches() { - RequestMatcher matcher = PathPatternRequestMatcher.path().pattern("/uri").matcher(); + RequestMatcher matcher = PathPatternRequestMatcher.path().matcher("/uri"); assertThat(matcher.matches(request("POST", "/uri", ""))).isTrue(); assertThat(matcher.matches(request("GET", "/uri", ""))).isTrue(); } @@ -88,7 +88,7 @@ void matcherWhenNoMethodThenMatches() { @Test void matcherWhenServletPathThenMatchesOnlyServletPath() { PathPatternRequestMatcher.Builder servlet = PathPatternRequestMatcher.servletPath("/servlet/path"); - RequestMatcher matcher = servlet.method(HttpMethod.GET).pattern("/endpoint").matcher(); + RequestMatcher matcher = servlet.matcher(HttpMethod.GET, "/endpoint"); ServletContext servletContext = servletContext("/servlet/path"); assertThat(matcher .matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(servletContext))).isTrue(); @@ -98,7 +98,7 @@ void matcherWhenServletPathThenMatchesOnlyServletPath() { @Test void matcherWhenRequestPathThenIgnoresServletPath() { PathPatternRequestMatcher.Builder request = PathPatternRequestMatcher.path(); - RequestMatcher matcher = request.method(HttpMethod.GET).pattern("/endpoint").matcher(); + RequestMatcher matcher = request.matcher(HttpMethod.GET, "/endpoint"); assertThat(matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))) .isTrue(); assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(null))).isTrue(); @@ -107,7 +107,7 @@ void matcherWhenRequestPathThenIgnoresServletPath() { @Test void matcherWhenServletPathThenRequiresServletPathToExist() { PathPatternRequestMatcher.Builder servlet = PathPatternRequestMatcher.servletPath("/servlet/path"); - RequestMatcher matcher = servlet.method(HttpMethod.GET).pattern("/endpoint").matcher(); + RequestMatcher matcher = servlet.matcher(HttpMethod.GET, "/endpoint"); assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy( () -> matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))); } From eb22c9b2eb5f48e27906d4721988c917a9a17a30 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Tue, 18 Feb 2025 16:09:20 -0700 Subject: [PATCH 07/10] Respond to Feedback --- docs/modules/ROOT/pages/migration/web.adoc | 8 +- .../matcher/PathPatternRequestMatcher.java | 78 +++++++++---------- .../MethodPatternRequestMatcherFactory.java | 19 ++++- 3 files changed, 61 insertions(+), 44 deletions(-) diff --git a/docs/modules/ROOT/pages/migration/web.adoc b/docs/modules/ROOT/pages/migration/web.adoc index d59aa65b947..95580d226d1 100644 --- a/docs/modules/ROOT/pages/migration/web.adoc +++ b/docs/modules/ROOT/pages/migration/web.adoc @@ -15,8 +15,8 @@ Java:: [source,java,role="primary"] ---- @Bean -MethodPatternRequestMatcherFactory requestMatcherFactory() { - return PathPatternRequestMatcher.path(); +MethodPatternRequestMatcherFactory requestMatcherFactory(@Qualifier("mvcPatternParser") PathPatternParser parser) { + return PathPatternRequestMatcher.withPathPatternParser(parser); } ---- @@ -25,8 +25,8 @@ Kotlin:: [source,kotlin,role="secondary"] ---- @Bean -fun requestMatcherFactory(): MethodPatternRequestMatcherFactory { - return PathPatternRequestMatcher.path() +fun requestMatcherFactory(@Qualifier("mvcPatternParser") parser: PathPatternParser): MethodPatternRequestMatcherFactory { + return PathPatternRequestMatcher.withPathPatternParser(parser) } ---- ====== diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java index ad5d54c0ed3..2fc8579509e 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java @@ -16,19 +16,15 @@ package org.springframework.security.web.servlet.util.matcher; -import java.nio.charset.StandardCharsets; import java.util.Collection; import java.util.LinkedHashMap; import java.util.Map; import java.util.Objects; import java.util.concurrent.atomic.AtomicReference; -import jakarta.servlet.RequestDispatcher; import jakarta.servlet.ServletContext; 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.http.server.PathContainer; @@ -39,10 +35,7 @@ import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; -import org.springframework.util.ObjectUtils; import org.springframework.web.util.ServletRequestPathUtils; -import org.springframework.web.util.UriUtils; -import org.springframework.web.util.WebUtils; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; @@ -88,9 +81,7 @@ private PathPatternRequestMatcher(PathPattern pattern) { * prefix *

* When there is no context path, then these URIs are effectively absolute. - * @return a {@link PathPatternRequestMatcher.Builder} that treats URIs as relative to - * the context path, if any - * @since 6.5 + * @return a {@link Builder} that treats URIs as relative to the context path, if any */ public static Builder path() { return new Builder(); @@ -109,17 +100,25 @@ public static Builder path() { * {@link HttpServletRequest#getServletPath()} would return {@code /path} and so * {@code /path} is what is specified here. * + *

* Specify the path here without the trailing {@code /*}. - * @return a {@link PathPatternRequestMatcher.Builder} that treats URIs as relative to - * the given {@code servletPath} - * @since 6.5 + * @return a {@link Builder} that treats URIs as relative to the given + * {@code servletPath} */ public static Builder 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 a slash"); - Assert.isTrue(!servletPath.contains("*"), "servletPath must not contain a star"); - return new Builder(new ServletPathRequestMatcher(servletPath)); + return new Builder().servletPath(servletPath); + } + + /** + * Use this {@link PathPatternParser} to parse path patterns. Uses + * {@link PathPatternParser#defaultInstance} by default. + * @param parser the {@link PathPatternParser} to use + * @return a {@link Builder} that treats URIs as relative to the given + * {@code servletPath} + */ + public static Builder withPathPatternParser(PathPatternParser parser) { + Assert.notNull(parser, "pathPatternParser cannot be null"); + return new Builder(parser); } /** @@ -212,16 +211,27 @@ public String toString() { */ public static final class Builder implements MethodPatternRequestMatcherFactory { - private PathPatternParser parser = PathPatternParser.defaultInstance; + private final PathPatternParser parser; - private final RequestMatcher servletPath; + private RequestMatcher servletPath = AnyRequestMatcher.INSTANCE; - Builder() { - this(AnyRequestMatcher.INSTANCE); + private Builder() { + this.parser = PathPatternParser.defaultInstance; } - Builder(RequestMatcher servletPath) { - this.servletPath = servletPath; + private Builder(PathPatternParser parser) { + this.parser = parser; + } + + /** + * Match requests starting with this {@code servletPath}. + * @param servletPath the servlet path prefix + * @see PathPatternRequestMatcher#servletPath + * @return the {@link Builder} for more configuration + */ + public Builder servletPath(String servletPath) { + this.servletPath = new ServletPathRequestMatcher(servletPath); + return this; } /** @@ -298,6 +308,10 @@ private static final class ServletPathRequestMatcher implements RequestMatcher { private final AtomicReference servletExists = new AtomicReference<>(); ServletPathRequestMatcher(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 a slash"); + Assert.isTrue(!servletPath.contains("*"), "servletPath must not contain a star"); this.path = servletPath; } @@ -305,7 +319,7 @@ private static final class ServletPathRequestMatcher implements RequestMatcher { public boolean matches(HttpServletRequest request) { Assert.isTrue(servletExists(request), () -> this.path + "/* does not exist in your servlet registration " + registrationMappings(request)); - return Objects.equals(this.path, getServletPathPrefix(request)); + return Objects.equals(this.path, ServletRequestPathUtils.getServletPathPrefix(request)); } private boolean servletExists(HttpServletRequest request) { @@ -336,20 +350,6 @@ private Map> registrationMappings(HttpServletRequest return map; } - @Nullable - private static String getServletPathPrefix(HttpServletRequest request) { - HttpServletMapping mapping = (HttpServletMapping) request.getAttribute(RequestDispatcher.INCLUDE_MAPPING); - mapping = (mapping != null) ? mapping : request.getHttpServletMapping(); - if (ObjectUtils.nullSafeEquals(mapping.getMappingMatch(), MappingMatch.PATH)) { - String servletPath = (String) request.getAttribute(WebUtils.INCLUDE_SERVLET_PATH_ATTRIBUTE); - servletPath = (servletPath != null) ? servletPath : request.getServletPath(); - servletPath = servletPath.endsWith("/") ? servletPath.substring(0, servletPath.length() - 1) - : servletPath; - return UriUtils.encodePath(servletPath, StandardCharsets.UTF_8); - } - return null; - } - @Override public String toString() { return "ServletPath [" + this.path + "]"; diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java b/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java index c7a2c19019a..d4559da999f 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java @@ -17,6 +17,7 @@ package org.springframework.security.web.util.matcher; import org.springframework.http.HttpMethod; +import org.springframework.lang.Nullable; /** * A strategy for constructing request matchers that match method-path pairs @@ -26,10 +27,26 @@ */ public interface MethodPatternRequestMatcherFactory { + /** + * Request a method-pattern request matcher given the following {{@code method} and + * {@code pattern}. + * This method in this case is treated as a wildcard. + * + * @param pattern the path pattern to use + * @return the {@link RequestMatcher} + */ default RequestMatcher matcher(String pattern) { return matcher(null, pattern); } - RequestMatcher matcher(HttpMethod method, String pattern); + /** + * Request a method-pattern request matcher given the following + * {@code method} and {@code pattern}. + * + * @param method the method to use, may be null + * @param pattern the path pattern to use + * @return the {@link RequestMatcher} + */ + RequestMatcher matcher(@Nullable HttpMethod method, String pattern); } From 3d5bc5460f7c905bd02ac023740923a7f0ab909c Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Tue, 18 Feb 2025 17:03:30 -0700 Subject: [PATCH 08/10] Polish Opt-in --- .../web/AbstractRequestMatcherRegistry.java | 18 +++++++-------- .../annotation/web/builders/HttpSecurity.java | 6 ++--- .../web/configurers/FormLoginConfigurer.java | 6 ++--- .../web/configurers/LogoutConfigurer.java | 6 ++--- .../PasswordManagementConfigurer.java | 6 ++--- .../configurers/RequestCacheConfigurer.java | 6 ++--- .../oauth2/client/OAuth2LoginConfigurer.java | 6 ++--- .../ott/OneTimeTokenLoginConfigurer.java | 6 ++--- .../saml2/Saml2LoginConfigurer.java | 6 ++--- .../saml2/Saml2LogoutConfigurer.java | 6 ++--- .../saml2/Saml2MetadataConfigurer.java | 6 ++--- .../AbstractRequestMatcherRegistryTests.java | 10 ++++---- .../matcher/PathPatternRequestMatcher.java | 16 ++++++------- ...a => MethodPathRequestMatcherFactory.java} | 23 ++++++++----------- 14 files changed, 62 insertions(+), 65 deletions(-) rename web/src/main/java/org/springframework/security/web/util/matcher/{MethodPatternRequestMatcherFactory.java => MethodPathRequestMatcherFactory.java} (74%) 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 ef5aee64c35..718434be133 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 @@ -46,7 +46,7 @@ import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -219,7 +219,7 @@ public C requestMatchers(HttpMethod method, String... patterns) { return requestMatchers(RequestMatchers.antMatchersAsArray(method, patterns)); } List matchers = new ArrayList<>(); - MethodPatternRequestMatcherFactory requestMatcherFactory = getRequestMatcherFactory(); + MethodPathRequestMatcherFactory requestMatcherFactory = getRequestMatcherFactory(); for (String pattern : patterns) { matchers.add(requestMatcherFactory.matcher(method, pattern)); } @@ -331,9 +331,9 @@ public C requestMatchers(HttpMethod method) { */ protected abstract C chainRequestMatchers(List requestMatchers); - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { - return this.context.getBeanProvider(MethodPatternRequestMatcherFactory.class) - .getIfUnique(DefaultMethodPatternRequestMatcherFactory::new); + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { + return this.context.getBeanProvider(MethodPathRequestMatcherFactory.class) + .getIfUnique(DefaultMethodPathRequestMatcherFactory::new); } /** @@ -409,12 +409,12 @@ static List regexMatchers(String... regexPatterns) { } - class DefaultMethodPatternRequestMatcherFactory implements MethodPatternRequestMatcherFactory { + class DefaultMethodPathRequestMatcherFactory implements MethodPathRequestMatcherFactory { @Override - public RequestMatcher matcher(HttpMethod method, String pattern) { - AntPathRequestMatcher ant = new AntPathRequestMatcher(pattern, (method != null) ? method.name() : null); - MvcRequestMatcher mvc = createMvcMatchers(method, pattern).get(0); + public RequestMatcher matcher(HttpMethod method, String path) { + AntPathRequestMatcher ant = new AntPathRequestMatcher(path, (method != null) ? method.name() : null); + MvcRequestMatcher mvc = createMvcMatchers(method, path).get(0); return new DeferredRequestMatcher((c) -> resolve(ant, mvc, c), mvc, ant); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java index 2bef03a10e8..136314b9cbc 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java @@ -93,7 +93,7 @@ import org.springframework.security.web.session.HttpSessionEventPublisher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; @@ -3686,8 +3686,8 @@ public HttpSecurity securityMatcher(RequestMatcher requestMatcher) { */ public HttpSecurity securityMatcher(String... patterns) { List matchers = new ArrayList<>(); - MethodPatternRequestMatcherFactory factory = getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + MethodPathRequestMatcherFactory factory = getSharedObject(ApplicationContext.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> (method, pattern) -> mvcPresent ? createMvcMatcher(pattern) : createAntMatcher(pattern)); for (String pattern : patterns) { matchers.add(factory.matcher(pattern)); diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java index c3b5ee3e80c..6aa3ec3dc55 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java @@ -29,7 +29,7 @@ import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; /** @@ -274,9 +274,9 @@ private void initDefaultLoginFilter(H http) { } } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java index ebaa40c1916..52fa55868d5 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java @@ -40,7 +40,7 @@ import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.security.web.context.SecurityContextRepository; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; @@ -374,9 +374,9 @@ private RequestMatcher createLogoutRequestMatcher(String httpMethod) { return getRequestMatcherFactory().matcher(HttpMethod.valueOf(httpMethod), this.logoutUrl); } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index 74037541df0..fce4afe6e92 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -21,7 +21,7 @@ import org.springframework.security.web.RequestMatcherRedirectFilter; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.util.Assert; /** @@ -61,9 +61,9 @@ public void configure(B http) throws Exception { http.addFilterBefore(postProcess(changePasswordFilter), UsernamePasswordAuthenticationFilter.class); } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java index 240c9aa355f..7e46e036f91 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java @@ -32,7 +32,7 @@ import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.MediaTypeRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -169,9 +169,9 @@ private RequestMatcher notMatchingMediaType(H http, MediaType mediaType) { return new NegatedRequestMatcher(mediaRequest); } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java index 7bafbba305b..9406520cd34 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java @@ -93,7 +93,7 @@ import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; @@ -626,9 +626,9 @@ private void registerDelegateApplicationListener(ApplicationListener delegate delegating.addListener(smartListener); } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java index 33e0444f0db..4d9223ded9f 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java @@ -53,7 +53,7 @@ import org.springframework.security.web.authentication.ui.DefaultResourcesFilter; import org.springframework.security.web.csrf.CsrfToken; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -210,9 +210,9 @@ protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingU return getRequestMatcherFactory().matcher(HttpMethod.POST, loginProcessingUrl); } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java index 88c49e2835b..90229b88a4b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java @@ -57,7 +57,7 @@ import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.ParameterRequestMatcher; @@ -505,9 +505,9 @@ private Saml2AuthenticationRequestRepository return repository; } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java index 6acc52d307d..05b56ac0eed 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java @@ -66,7 +66,7 @@ import org.springframework.security.web.csrf.CsrfTokenRepository; import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.ParameterRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -335,9 +335,9 @@ private Saml2LogoutResponseResolver createSaml2LogoutResponseResolver( return this.logoutResponseConfigurer.logoutResponseResolver(registrations); } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java index 09da6620ba6..67e14da3ece 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java @@ -33,7 +33,7 @@ import org.springframework.security.saml2.provider.service.web.metadata.RequestMatcherMetadataResponseResolver; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.util.Assert; /** @@ -171,9 +171,9 @@ private RelyingPartyRegistrationRepository getRelyingPartyRegistrationRepository } } - private MethodPatternRequestMatcherFactory getRequestMatcherFactory() { + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPatternRequestMatcherFactory.class) + .getBeanProvider(MethodPathRequestMatcherFactory.class) .getIfUnique(() -> AntPathRequestMatcher::antMatcher); } 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 4f1772e454a..8f095b07fa8 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 @@ -42,7 +42,7 @@ import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.test.web.servlet.MockMvc; @@ -89,15 +89,15 @@ public void setUp() { ObjectProvider> given = this.context.getBeanProvider(type); given(given).willReturn(postProcessors); given(postProcessors.getObject()).willReturn(NO_OP_OBJECT_POST_PROCESSOR); - MethodPatternRequestMatcherFactory factory = this.matcherRegistry.new DefaultMethodPatternRequestMatcherFactory(); - ObjectProvider requestMatcherFactory = new ObjectProvider<>() { + MethodPathRequestMatcherFactory factory = this.matcherRegistry.new DefaultMethodPathRequestMatcherFactory(); + ObjectProvider requestMatcherFactory = new ObjectProvider<>() { @Override @NonNull - public MethodPatternRequestMatcherFactory getObject() throws BeansException { + public MethodPathRequestMatcherFactory getObject() throws BeansException { return factory; } }; - given(this.context.getBeanProvider(MethodPatternRequestMatcherFactory.class)).willReturn(requestMatcherFactory); + given(this.context.getBeanProvider(MethodPathRequestMatcherFactory.class)).willReturn(requestMatcherFactory); given(this.context.getServletContext()).willReturn(MockServletContext.mvc()); this.matcherRegistry.setApplicationContext(this.context); mockMvcIntrospector(true); diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java index 2fc8579509e..8c62230e654 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java @@ -32,7 +32,7 @@ import org.springframework.lang.Nullable; import org.springframework.security.web.access.intercept.RequestAuthorizationContext; import org.springframework.security.web.util.matcher.AnyRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPatternRequestMatcherFactory; +import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.web.util.ServletRequestPathUtils; @@ -209,7 +209,7 @@ public String toString() { * ... * */ - public static final class Builder implements MethodPatternRequestMatcherFactory { + public static final class Builder implements MethodPathRequestMatcherFactory { private final PathPatternParser parser; @@ -226,8 +226,8 @@ private Builder(PathPatternParser parser) { /** * Match requests starting with this {@code servletPath}. * @param servletPath the servlet path prefix - * @see PathPatternRequestMatcher#servletPath * @return the {@link Builder} for more configuration + * @see PathPatternRequestMatcher#servletPath */ public Builder servletPath(String servletPath) { this.servletPath = new ServletPathRequestMatcher(servletPath); @@ -262,13 +262,13 @@ public Builder servletPath(String servletPath) { *

* A more comprehensive list can be found at {@link PathPattern}. * @param method the {@link HttpMethod} to match, may be null - * @param pattern the path pattern to match + * @param path the path pattern to match * @return the {@link Builder} for more configuration */ - public PathPatternRequestMatcher matcher(@Nullable HttpMethod method, String pattern) { - Assert.notNull(pattern, "pattern cannot be null"); - Assert.isTrue(pattern.startsWith("/"), "pattern must start with a /"); - PathPattern pathPattern = this.parser.parse(pattern); + public PathPatternRequestMatcher matcher(@Nullable HttpMethod method, String path) { + Assert.notNull(path, "pattern cannot be null"); + Assert.isTrue(path.startsWith("/"), "pattern must start with a /"); + PathPattern pathPattern = this.parser.parse(path); PathPatternRequestMatcher requestMatcher = new PathPatternRequestMatcher(pathPattern); if (method != null) { requestMatcher.setMethod(new HttpMethodRequestMatcher(method)); diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java b/web/src/main/java/org/springframework/security/web/util/matcher/MethodPathRequestMatcherFactory.java similarity index 74% rename from web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java rename to web/src/main/java/org/springframework/security/web/util/matcher/MethodPathRequestMatcherFactory.java index d4559da999f..b8978cde08a 100644 --- a/web/src/main/java/org/springframework/security/web/util/matcher/MethodPatternRequestMatcherFactory.java +++ b/web/src/main/java/org/springframework/security/web/util/matcher/MethodPathRequestMatcherFactory.java @@ -25,28 +25,25 @@ * @author Josh Cummings * @since 6.5 */ -public interface MethodPatternRequestMatcherFactory { +public interface MethodPathRequestMatcherFactory { /** - * Request a method-pattern request matcher given the following {{@code method} and - * {@code pattern}. - * This method in this case is treated as a wildcard. - * - * @param pattern the path pattern to use + * Request a method-pattern request matcher given the following {@code method} and + * {@code pattern}. This method in this case is treated as a wildcard. + * @param path the path pattern to use * @return the {@link RequestMatcher} */ - default RequestMatcher matcher(String pattern) { - return matcher(null, pattern); + default RequestMatcher matcher(String path) { + return matcher(null, path); } /** - * Request a method-pattern request matcher given the following - * {@code method} and {@code pattern}. - * + * Request a method-pattern request matcher given the following {@code method} and + * {@code pattern}. * @param method the method to use, may be null - * @param pattern the path pattern to use + * @param path the path pattern to use * @return the {@link RequestMatcher} */ - RequestMatcher matcher(@Nullable HttpMethod method, String pattern); + RequestMatcher matcher(@Nullable HttpMethod method, String path); } From aadb000edba20420c14ef709ca05629627053c41 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 20 Feb 2025 12:57:07 -0700 Subject: [PATCH 09/10] Polish PathPattern Support --- .../security/web/FilterChainProxy.java | 60 +++++++++++++------ .../matcher/PathPatternRequestMatcher.java | 39 +++++++++++- .../MethodPathRequestMatcherFactory.java | 49 --------------- .../security/web/FilterChainProxyTests.java | 4 ++ .../PathPatternRequestMatcherTests.java | 19 ++++-- 5 files changed, 95 insertions(+), 76 deletions(-) delete mode 100644 web/src/main/java/org/springframework/security/web/util/matcher/MethodPathRequestMatcherFactory.java diff --git a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java index 7795f2c8bac..f9ad696436b 100644 --- a/web/src/main/java/org/springframework/security/web/FilterChainProxy.java +++ b/web/src/main/java/org/springframework/security/web/FilterChainProxy.java @@ -46,6 +46,7 @@ import org.springframework.util.Assert; import org.springframework.web.filter.DelegatingFilterProxy; import org.springframework.web.filter.GenericFilterBean; +import org.springframework.web.filter.ServletRequestPathFilter; /** * Delegates {@code Filter} requests to a list of Spring-managed filter beans. As of @@ -162,6 +163,8 @@ public class FilterChainProxy extends GenericFilterBean { private FilterChainDecorator filterChainDecorator = new VirtualFilterChainDecorator(); + private Filter springWebFilter = new ServletRequestPathFilter(); + public FilterChainProxy() { } @@ -210,27 +213,29 @@ private void doFilterInternal(ServletRequest request, ServletResponse response, throws IOException, ServletException { FirewalledRequest firewallRequest = this.firewall.getFirewalledRequest((HttpServletRequest) request); HttpServletResponse firewallResponse = this.firewall.getFirewalledResponse((HttpServletResponse) response); - List filters = getFilters(firewallRequest); - if (filters == null || filters.isEmpty()) { - if (logger.isTraceEnabled()) { - logger.trace(LogMessage.of(() -> "No security for " + requestLine(firewallRequest))); + this.springWebFilter.doFilter(firewallRequest, firewallResponse, (r, s) -> { + List filters = getFilters(firewallRequest); + if (filters == null || filters.isEmpty()) { + if (logger.isTraceEnabled()) { + logger.trace(LogMessage.of(() -> "No security for " + requestLine(firewallRequest))); + } + firewallRequest.reset(); + this.filterChainDecorator.decorate(chain).doFilter(firewallRequest, firewallResponse); + return; } - firewallRequest.reset(); - this.filterChainDecorator.decorate(chain).doFilter(firewallRequest, firewallResponse); - return; - } - if (logger.isDebugEnabled()) { - logger.debug(LogMessage.of(() -> "Securing " + requestLine(firewallRequest))); - } - FilterChain reset = (req, res) -> { if (logger.isDebugEnabled()) { - logger.debug(LogMessage.of(() -> "Secured " + requestLine(firewallRequest))); + logger.debug(LogMessage.of(() -> "Securing " + requestLine(firewallRequest))); } - // Deactivate path stripping as we exit the security filter chain - firewallRequest.reset(); - chain.doFilter(req, res); - }; - this.filterChainDecorator.decorate(reset, filters).doFilter(firewallRequest, firewallResponse); + FilterChain reset = (req, res) -> { + if (logger.isDebugEnabled()) { + logger.debug(LogMessage.of(() -> "Secured " + requestLine(firewallRequest))); + } + // Deactivate path stripping as we exit the security filter chain + firewallRequest.reset(); + chain.doFilter(req, res); + }; + this.filterChainDecorator.decorate(reset, filters).doFilter(firewallRequest, firewallResponse); + }); } /** @@ -447,4 +452,23 @@ public FilterChain decorate(FilterChain original, List filters) { } + private static final class FirewallFilter implements Filter { + + private final HttpFirewall firewall; + + private FirewallFilter(HttpFirewall firewall) { + this.firewall = firewall; + } + + @Override + public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) + throws IOException, ServletException { + HttpServletRequest request = (HttpServletRequest) servletRequest; + HttpServletResponse response = (HttpServletResponse) servletResponse; + filterChain.doFilter(this.firewall.getFirewalledRequest(request), + this.firewall.getFirewalledResponse(response)); + } + + } + } diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java index 8c62230e654..40a5ce5b038 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java @@ -32,7 +32,6 @@ import org.springframework.lang.Nullable; import org.springframework.security.web.access.intercept.RequestAuthorizationContext; import org.springframework.security.web.util.matcher.AnyRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.web.util.ServletRequestPathUtils; @@ -154,7 +153,7 @@ void setServletPath(RequestMatcher servletPath) { } private RequestPath getRequestPath(HttpServletRequest request) { - return ServletRequestPathUtils.parseAndCache(request); + return ServletRequestPathUtils.getParsedRequestPath(request); } /** @@ -209,7 +208,7 @@ public String toString() { * ... * */ - public static final class Builder implements MethodPathRequestMatcherFactory { + public static final class Builder { private final PathPatternParser parser; @@ -234,6 +233,40 @@ public Builder servletPath(String servletPath) { return this; } + /** + * Match requests having this path pattern. + * + *

+ * When the HTTP {@code method} is null, then the matcher does not consider the + * HTTP method + * + *

+ * Path patterns always start with a slash and may contain placeholders. They can + * also be followed by {@code /**} to signify all URIs under a given path. + * + *

+ * These must be specified relative to any servlet path prefix (meaning you should + * exclude the context path and any servlet path prefix in stating your pattern). + * + *

+ * The following are valid patterns and their meaning + *

    + *
  • {@code /path} - match exactly and only `/path`
  • + *
  • {@code /path/**} - match `/path` and any of its descendents
  • + *
  • {@code /path/{value}/**} - match `/path/subdirectory` and any of its + * descendents, capturing the value of the subdirectory in + * {@link RequestAuthorizationContext#getVariables()}
  • + *
+ * + *

+ * A more comprehensive list can be found at {@link PathPattern}. + * @param path the path pattern to match + * @return the {@link Builder} for more configuration + */ + public PathPatternRequestMatcher matcher(String path) { + return matcher(null, path); + } + /** * Match requests having this {@link HttpMethod} and path pattern. * diff --git a/web/src/main/java/org/springframework/security/web/util/matcher/MethodPathRequestMatcherFactory.java b/web/src/main/java/org/springframework/security/web/util/matcher/MethodPathRequestMatcherFactory.java deleted file mode 100644 index b8978cde08a..00000000000 --- a/web/src/main/java/org/springframework/security/web/util/matcher/MethodPathRequestMatcherFactory.java +++ /dev/null @@ -1,49 +0,0 @@ -/* - * 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.util.matcher; - -import org.springframework.http.HttpMethod; -import org.springframework.lang.Nullable; - -/** - * A strategy for constructing request matchers that match method-path pairs - * - * @author Josh Cummings - * @since 6.5 - */ -public interface MethodPathRequestMatcherFactory { - - /** - * Request a method-pattern request matcher given the following {@code method} and - * {@code pattern}. This method in this case is treated as a wildcard. - * @param path the path pattern to use - * @return the {@link RequestMatcher} - */ - default RequestMatcher matcher(String path) { - return matcher(null, path); - } - - /** - * Request a method-pattern request matcher given the following {@code method} and - * {@code pattern}. - * @param method the method to use, may be null - * @param path the path pattern to use - * @return the {@link RequestMatcher} - */ - RequestMatcher matcher(@Nullable HttpMethod method, String path); - -} diff --git a/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java b/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java index 778efc545fc..2e8f7a552a6 100644 --- a/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java +++ b/web/src/test/java/org/springframework/security/web/FilterChainProxyTests.java @@ -48,6 +48,7 @@ import org.springframework.security.web.firewall.HttpFirewall; import org.springframework.security.web.firewall.RequestRejectedException; import org.springframework.security.web.firewall.RequestRejectedHandler; +import org.springframework.security.web.servlet.TestMockHttpServletMappings; import org.springframework.security.web.util.matcher.RequestMatcher; import static org.assertj.core.api.Assertions.assertThat; @@ -166,6 +167,7 @@ public void wrapperIsResetWhenNoMatchingFilters() throws Exception { FirewalledRequest fwr = mock(FirewalledRequest.class); given(fwr.getRequestURI()).willReturn("/"); given(fwr.getContextPath()).willReturn(""); + given(fwr.getHttpServletMapping()).willReturn(TestMockHttpServletMappings.defaultMapping()); this.fcp.setFirewall(fw); given(fw.getFirewalledRequest(this.request)).willReturn(fwr); given(this.matcher.matches(any(HttpServletRequest.class))).willReturn(false); @@ -183,9 +185,11 @@ public void bothWrappersAreResetWithNestedFcps() throws Exception { FirewalledRequest firstFwr = mock(FirewalledRequest.class, "firstFwr"); given(firstFwr.getRequestURI()).willReturn("/"); given(firstFwr.getContextPath()).willReturn(""); + given(firstFwr.getHttpServletMapping()).willReturn(TestMockHttpServletMappings.defaultMapping()); FirewalledRequest fwr = mock(FirewalledRequest.class, "fwr"); given(fwr.getRequestURI()).willReturn("/"); given(fwr.getContextPath()).willReturn(""); + given(fwr.getHttpServletMapping()).willReturn(TestMockHttpServletMappings.defaultMapping()); given(fw.getFirewalledRequest(this.request)).willReturn(firstFwr); given(fw.getFirewalledRequest(firstFwr)).willReturn(fwr); given(fwr.getRequest()).willReturn(firstFwr); diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java index c969c7b726b..8cb592fcc5f 100644 --- a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcherTests.java @@ -90,18 +90,25 @@ void matcherWhenServletPathThenMatchesOnlyServletPath() { PathPatternRequestMatcher.Builder servlet = PathPatternRequestMatcher.servletPath("/servlet/path"); RequestMatcher matcher = servlet.matcher(HttpMethod.GET, "/endpoint"); ServletContext servletContext = servletContext("/servlet/path"); - assertThat(matcher - .matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(servletContext))).isTrue(); - assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(servletContext))).isFalse(); + MockHttpServletRequest mock = get("/servlet/path/endpoint").servletPath("/servlet/path") + .buildRequest(servletContext); + ServletRequestPathUtils.parseAndCache(mock); + assertThat(matcher.matches(mock)).isTrue(); + mock = get("/endpoint").servletPath("/endpoint").buildRequest(servletContext); + ServletRequestPathUtils.parseAndCache(mock); + assertThat(matcher.matches(mock)).isFalse(); } @Test void matcherWhenRequestPathThenIgnoresServletPath() { PathPatternRequestMatcher.Builder request = PathPatternRequestMatcher.path(); RequestMatcher matcher = request.matcher(HttpMethod.GET, "/endpoint"); - assertThat(matcher.matches(get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null))) - .isTrue(); - assertThat(matcher.matches(get("/endpoint").servletPath("/endpoint").buildRequest(null))).isTrue(); + MockHttpServletRequest mock = get("/servlet/path/endpoint").servletPath("/servlet/path").buildRequest(null); + ServletRequestPathUtils.parseAndCache(mock); + assertThat(matcher.matches(mock)).isTrue(); + mock = get("/endpoint").servletPath("/endpoint").buildRequest(null); + ServletRequestPathUtils.parseAndCache(mock); + assertThat(matcher.matches(mock)).isTrue(); } @Test From e071a9de7e2ce8b5d2c1c21c45d25fbe25116a06 Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 20 Feb 2025 12:57:30 -0700 Subject: [PATCH 10/10] Polish Opt-in --- .../web/AbstractRequestMatcherRegistry.java | 8 +- .../web/MethodPathRequestMatcherFactory.java | 39 ++++++++ .../annotation/web/builders/HttpSecurity.java | 12 ++- .../MethodPathRequestMatcherFactory.java | 39 ++++++++ .../web/configurers/FormLoginConfigurer.java | 7 +- .../web/configurers/LogoutConfigurer.java | 7 +- .../MethodPathRequestMatcherFactory.java | 39 ++++++++ .../PasswordManagementConfigurer.java | 7 +- .../configurers/RequestCacheConfigurer.java | 45 ++++++++- .../MethodPathRequestMatcherFactory.java | 39 ++++++++ .../oauth2/client/OAuth2LoginConfigurer.java | 7 +- .../ott/MethodPathRequestMatcherFactory.java | 39 ++++++++ .../ott/OneTimeTokenLoginConfigurer.java | 9 +- .../MethodPathRequestMatcherFactory.java | 39 ++++++++ .../saml2/Saml2LoginConfigurer.java | 7 +- .../saml2/Saml2LogoutConfigurer.java | 7 +- .../saml2/Saml2MetadataConfigurer.java | 7 +- ...tternRequestMatcherBuilderFactoryBean.java | 92 +++++++++++++++++ .../AbstractRequestMatcherRegistryTests.java | 13 +-- .../AuthorizeHttpRequestsConfigurerTests.java | 33 +++++++ .../RequestCacheConfigurerTests.java | 28 ++++++ ...RequestMatcherBuilderFactoryBeanTests.java | 99 +++++++++++++++++++ docs/modules/ROOT/pages/migration/web.adoc | 10 +- 23 files changed, 565 insertions(+), 67 deletions(-) create mode 100644 config/src/main/java/org/springframework/security/config/annotation/web/MethodPathRequestMatcherFactory.java create mode 100644 config/src/main/java/org/springframework/security/config/annotation/web/builders/MethodPathRequestMatcherFactory.java create mode 100644 config/src/main/java/org/springframework/security/config/annotation/web/configurers/MethodPathRequestMatcherFactory.java create mode 100644 config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/MethodPathRequestMatcherFactory.java create mode 100644 config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/MethodPathRequestMatcherFactory.java create mode 100644 config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/MethodPathRequestMatcherFactory.java create mode 100644 config/src/main/java/org/springframework/security/config/web/PathPatternRequestMatcherBuilderFactoryBean.java create mode 100644 config/src/test/java/org/springframework/security/config/web/PathPatternRequestMatcherBuilderFactoryBeanTests.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 718434be133..41f60854a55 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 @@ -43,10 +43,10 @@ import org.springframework.security.config.annotation.web.ServletRegistrationsSupport.RegistrationMapping; import org.springframework.security.config.annotation.web.configurers.AbstractConfigAttributeRequestMatcherRegistry; 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.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -332,8 +332,10 @@ public C requestMatchers(HttpMethod method) { protected abstract C chainRequestMatchers(List requestMatchers); private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return this.context.getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(DefaultMethodPathRequestMatcherFactory::new); + PathPatternRequestMatcher.Builder builder = this.context + .getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + return (builder != null) ? builder::matcher : new DefaultMethodPathRequestMatcherFactory(); } /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/MethodPathRequestMatcherFactory.java b/config/src/main/java/org/springframework/security/config/annotation/web/MethodPathRequestMatcherFactory.java new file mode 100644 index 00000000000..72562502358 --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/web/MethodPathRequestMatcherFactory.java @@ -0,0 +1,39 @@ +/* + * 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.config.annotation.web; + +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; + +interface MethodPathRequestMatcherFactory { + + default RequestMatcher matcher(String path) { + return matcher(null, path); + } + + RequestMatcher matcher(HttpMethod method, String path); + + static MethodPathRequestMatcherFactory fromApplicationContext(ApplicationContext context) { + PathPatternRequestMatcher.Builder builder = context.getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + return (builder != null) ? builder::matcher : AntPathRequestMatcher::antMatcher; + } + +} diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java index 136314b9cbc..2565c89f24b 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2024 the original author or authors. + * 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. @@ -90,10 +90,10 @@ import org.springframework.security.web.context.AbstractSecurityWebApplicationInitializer; import org.springframework.security.web.context.SecurityContextRepository; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.security.web.session.HttpSessionEventPublisher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; @@ -3686,9 +3686,11 @@ public HttpSecurity securityMatcher(RequestMatcher requestMatcher) { */ public HttpSecurity securityMatcher(String... patterns) { List matchers = new ArrayList<>(); - MethodPathRequestMatcherFactory factory = getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> (method, pattern) -> mvcPresent ? createMvcMatcher(pattern) : createAntMatcher(pattern)); + PathPatternRequestMatcher.Builder builder = getSharedObject(ApplicationContext.class) + .getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + MethodPathRequestMatcherFactory factory = (builder != null) ? builder::matcher + : (method, pattern) -> mvcPresent ? createMvcMatcher(pattern) : createAntMatcher(pattern); for (String pattern : patterns) { matchers.add(factory.matcher(pattern)); } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/builders/MethodPathRequestMatcherFactory.java b/config/src/main/java/org/springframework/security/config/annotation/web/builders/MethodPathRequestMatcherFactory.java new file mode 100644 index 00000000000..2711399186c --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/web/builders/MethodPathRequestMatcherFactory.java @@ -0,0 +1,39 @@ +/* + * 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.config.annotation.web.builders; + +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; + +interface MethodPathRequestMatcherFactory { + + default RequestMatcher matcher(String path) { + return matcher(null, path); + } + + RequestMatcher matcher(HttpMethod method, String path); + + static MethodPathRequestMatcherFactory fromApplicationContext(ApplicationContext context) { + PathPatternRequestMatcher.Builder builder = context.getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + return (builder != null) ? builder::matcher : AntPathRequestMatcher::antMatcher; + } + +} diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java index 6aa3ec3dc55..01be169e829 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/FormLoginConfigurer.java @@ -28,8 +28,6 @@ import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; import org.springframework.security.web.authentication.session.SessionAuthenticationStrategy; import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; /** @@ -275,9 +273,8 @@ private void initDefaultLoginFilter(H http) { } private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + return MethodPathRequestMatcherFactory + .fromApplicationContext(getBuilder().getSharedObject(ApplicationContext.class)); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java index 52fa55868d5..1197c0dedf8 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/LogoutConfigurer.java @@ -39,8 +39,6 @@ import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; import org.springframework.security.web.context.HttpSessionSecurityContextRepository; import org.springframework.security.web.context.SecurityContextRepository; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; @@ -375,9 +373,8 @@ private RequestMatcher createLogoutRequestMatcher(String httpMethod) { } private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + return MethodPathRequestMatcherFactory + .fromApplicationContext(getBuilder().getSharedObject(ApplicationContext.class)); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/MethodPathRequestMatcherFactory.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/MethodPathRequestMatcherFactory.java new file mode 100644 index 00000000000..9fc70437381 --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/MethodPathRequestMatcherFactory.java @@ -0,0 +1,39 @@ +/* + * 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.config.annotation.web.configurers; + +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; + +interface MethodPathRequestMatcherFactory { + + default RequestMatcher matcher(String path) { + return matcher(null, path); + } + + RequestMatcher matcher(HttpMethod method, String path); + + static MethodPathRequestMatcherFactory fromApplicationContext(ApplicationContext context) { + PathPatternRequestMatcher.Builder builder = context.getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + return (builder != null) ? builder::matcher : AntPathRequestMatcher::antMatcher; + } + +} diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java index fce4afe6e92..90847f1b30f 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurer.java @@ -20,8 +20,6 @@ import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.web.RequestMatcherRedirectFilter; import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.util.Assert; /** @@ -62,9 +60,8 @@ public void configure(B http) throws Exception { } private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + return MethodPathRequestMatcherFactory + .fromApplicationContext(getBuilder().getSharedObject(ApplicationContext.class)); } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java index 7e46e036f91..adf850c14bb 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurer.java @@ -19,25 +19,31 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.regex.Pattern; + +import jakarta.servlet.http.HttpServletRequest; import org.springframework.context.ApplicationContext; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; +import org.springframework.http.server.PathContainer; +import org.springframework.http.server.RequestPath; import org.springframework.security.config.annotation.web.HttpSecurityBuilder; import org.springframework.security.config.annotation.web.builders.HttpSecurity; import org.springframework.security.web.savedrequest.HttpSessionRequestCache; import org.springframework.security.web.savedrequest.NullRequestCache; import org.springframework.security.web.savedrequest.RequestCache; import org.springframework.security.web.savedrequest.RequestCacheAwareFilter; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; import org.springframework.security.web.util.matcher.AndRequestMatcher; import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.MediaTypeRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.web.accept.ContentNegotiationStrategy; import org.springframework.web.accept.HeaderContentNegotiationStrategy; +import org.springframework.web.util.ServletRequestPathUtils; /** * Adds request cache for Spring Security. Specifically this ensures that requests that @@ -142,7 +148,7 @@ private T getBeanOrNull(Class type) { @SuppressWarnings("unchecked") private RequestMatcher createDefaultSavedRequestMatcher(H http) { - RequestMatcher notFavIcon = new NegatedRequestMatcher(getRequestMatcherFactory().matcher("/**/favicon.*")); + RequestMatcher notFavIcon = new NegatedRequestMatcher(getFaviconRequestMatcher()); RequestMatcher notXRequestedWith = new NegatedRequestMatcher( new RequestHeaderRequestMatcher("X-Requested-With", "XMLHttpRequest")); boolean isCsrfEnabled = http.getConfigurer(CsrfConfigurer.class) != null; @@ -169,10 +175,39 @@ private RequestMatcher notMatchingMediaType(H http, MediaType mediaType) { return new NegatedRequestMatcher(mediaRequest); } + private RequestMatcher getFaviconRequestMatcher() { + PathPatternRequestMatcher.Builder builder = getBuilder().getSharedObject(ApplicationContext.class) + .getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + if (builder == null) { + return new AntPathRequestMatcher("/**/favicon.*"); + } + else { + return new FilenameRequestMatcher(Pattern.compile("favicon.*")); + } + } + private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + ApplicationContext context = getBuilder().getSharedObject(ApplicationContext.class); + return MethodPathRequestMatcherFactory.fromApplicationContext(context); + } + + private static final class FilenameRequestMatcher implements RequestMatcher { + + private final Pattern pattern; + + FilenameRequestMatcher(Pattern pattern) { + this.pattern = pattern; + } + + @Override + public boolean matches(HttpServletRequest request) { + RequestPath path = ServletRequestPathUtils.getParsedRequestPath(request); + List elements = path.elements(); + String file = elements.get(elements.size() - 1).value(); + return this.pattern.matcher(file).matches(); + } + } } diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/MethodPathRequestMatcherFactory.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/MethodPathRequestMatcherFactory.java new file mode 100644 index 00000000000..130a9bcc3c2 --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/MethodPathRequestMatcherFactory.java @@ -0,0 +1,39 @@ +/* + * 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.config.annotation.web.configurers.oauth2.client; + +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; + +interface MethodPathRequestMatcherFactory { + + default RequestMatcher matcher(String path) { + return matcher(null, path); + } + + RequestMatcher matcher(HttpMethod method, String path); + + static MethodPathRequestMatcherFactory fromApplicationContext(ApplicationContext context) { + PathPatternRequestMatcher.Builder builder = context.getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + return (builder != null) ? builder::matcher : AntPathRequestMatcher::antMatcher; + } + +} diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java index 9406520cd34..54cef424ed9 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/oauth2/client/OAuth2LoginConfigurer.java @@ -91,9 +91,7 @@ import org.springframework.security.web.csrf.CsrfToken; import org.springframework.security.web.savedrequest.RequestCache; import org.springframework.security.web.util.matcher.AndRequestMatcher; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.AnyRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.RequestHeaderRequestMatcher; @@ -627,9 +625,8 @@ private void registerDelegateApplicationListener(ApplicationListener delegate } private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + return MethodPathRequestMatcherFactory + .fromApplicationContext(getBuilder().getSharedObject(ApplicationContext.class)); } /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/MethodPathRequestMatcherFactory.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/MethodPathRequestMatcherFactory.java new file mode 100644 index 00000000000..ab152435c7e --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/MethodPathRequestMatcherFactory.java @@ -0,0 +1,39 @@ +/* + * 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.config.annotation.web.configurers.ott; + +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; + +interface MethodPathRequestMatcherFactory { + + default RequestMatcher matcher(String path) { + return matcher(null, path); + } + + RequestMatcher matcher(HttpMethod method, String path); + + static MethodPathRequestMatcherFactory fromApplicationContext(ApplicationContext context) { + PathPatternRequestMatcher.Builder builder = context.getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + return (builder != null) ? builder::matcher : AntPathRequestMatcher::antMatcher; + } + +} diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java index 4d9223ded9f..2a1c5cb5426 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/ott/OneTimeTokenLoginConfigurer.java @@ -52,8 +52,6 @@ import org.springframework.security.web.authentication.ui.DefaultOneTimeTokenSubmitPageGeneratingFilter; import org.springframework.security.web.authentication.ui.DefaultResourcesFilter; import org.springframework.security.web.csrf.CsrfToken; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -123,8 +121,9 @@ public final class OneTimeTokenLoginConfigurer> private GenerateOneTimeTokenRequestResolver requestResolver; public OneTimeTokenLoginConfigurer(ApplicationContext context) { - super(new OneTimeTokenAuthenticationFilter(), OneTimeTokenAuthenticationFilter.DEFAULT_LOGIN_PROCESSING_URL); + super(new OneTimeTokenAuthenticationFilter(), null); this.context = context; + loginProcessingUrl(OneTimeTokenAuthenticationFilter.DEFAULT_LOGIN_PROCESSING_URL); } @Override @@ -211,9 +210,7 @@ protected RequestMatcher createLoginProcessingUrlMatcher(String loginProcessingU } private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + return MethodPathRequestMatcherFactory.fromApplicationContext(this.context); } /** diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/MethodPathRequestMatcherFactory.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/MethodPathRequestMatcherFactory.java new file mode 100644 index 00000000000..67a9c565ce6 --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/MethodPathRequestMatcherFactory.java @@ -0,0 +1,39 @@ +/* + * 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.config.annotation.web.configurers.saml2; + +import org.springframework.context.ApplicationContext; +import org.springframework.http.HttpMethod; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.security.web.util.matcher.AntPathRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; + +interface MethodPathRequestMatcherFactory { + + default RequestMatcher matcher(String path) { + return matcher(null, path); + } + + RequestMatcher matcher(HttpMethod method, String path); + + static MethodPathRequestMatcherFactory fromApplicationContext(ApplicationContext context) { + PathPatternRequestMatcher.Builder builder = context.getBeanProvider(PathPatternRequestMatcher.Builder.class) + .getIfUnique(); + return (builder != null) ? builder::matcher : AntPathRequestMatcher::antMatcher; + } + +} diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java index 90229b88a4b..fa1af4f0119 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LoginConfigurer.java @@ -56,8 +56,6 @@ import org.springframework.security.web.authentication.LoginUrlAuthenticationEntryPoint; import org.springframework.security.web.authentication.ui.DefaultLoginPageGeneratingFilter; import org.springframework.security.web.util.matcher.AndRequestMatcher; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.NegatedRequestMatcher; import org.springframework.security.web.util.matcher.OrRequestMatcher; import org.springframework.security.web.util.matcher.ParameterRequestMatcher; @@ -506,9 +504,8 @@ private Saml2AuthenticationRequestRepository } private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + return MethodPathRequestMatcherFactory + .fromApplicationContext(getBuilder().getSharedObject(ApplicationContext.class)); } private C getSharedOrBean(B http, Class clazz) { diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java index 05b56ac0eed..3543cc8b507 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2LogoutConfigurer.java @@ -65,8 +65,6 @@ import org.springframework.security.web.csrf.CsrfLogoutHandler; import org.springframework.security.web.csrf.CsrfTokenRepository; import org.springframework.security.web.util.matcher.AndRequestMatcher; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.ParameterRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; @@ -336,9 +334,8 @@ private Saml2LogoutResponseResolver createSaml2LogoutResponseResolver( } private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + return MethodPathRequestMatcherFactory + .fromApplicationContext(getBuilder().getSharedObject(ApplicationContext.class)); } private C getBeanOrNull(Class clazz) { diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java index 67e14da3ece..c4d2b90fc8f 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java +++ b/config/src/main/java/org/springframework/security/config/annotation/web/configurers/saml2/Saml2MetadataConfigurer.java @@ -32,8 +32,6 @@ import org.springframework.security.saml2.provider.service.web.Saml2MetadataFilter; import org.springframework.security.saml2.provider.service.web.metadata.RequestMatcherMetadataResponseResolver; import org.springframework.security.web.authentication.www.BasicAuthenticationFilter; -import org.springframework.security.web.util.matcher.AntPathRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.util.Assert; /** @@ -172,9 +170,8 @@ private RelyingPartyRegistrationRepository getRelyingPartyRegistrationRepository } private MethodPathRequestMatcherFactory getRequestMatcherFactory() { - return getBuilder().getSharedObject(ApplicationContext.class) - .getBeanProvider(MethodPathRequestMatcherFactory.class) - .getIfUnique(() -> AntPathRequestMatcher::antMatcher); + return MethodPathRequestMatcherFactory + .fromApplicationContext(getBuilder().getSharedObject(ApplicationContext.class)); } private C getBeanOrNull(Class clazz) { diff --git a/config/src/main/java/org/springframework/security/config/web/PathPatternRequestMatcherBuilderFactoryBean.java b/config/src/main/java/org/springframework/security/config/web/PathPatternRequestMatcherBuilderFactoryBean.java new file mode 100644 index 00000000000..93baaebd4aa --- /dev/null +++ b/config/src/main/java/org/springframework/security/config/web/PathPatternRequestMatcherBuilderFactoryBean.java @@ -0,0 +1,92 @@ +/* + * 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.config.web; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.FactoryBean; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.security.config.annotation.web.configurers.AuthorizeHttpRequestsConfigurer; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.web.util.pattern.PathPatternParser; + +/** + * Use this factory bean to configure the {@link PathPatternRequestMatcher.Builder} bean + * used to create request matchers in {@link AuthorizeHttpRequestsConfigurer} and other + * parts of the DSL. + * + * @author Josh Cummings + * @since 6.5 + */ +public final class PathPatternRequestMatcherBuilderFactoryBean + implements FactoryBean, ApplicationContextAware { + + static final String PATTERN_PARSER_BEAN_NAME = "mvcPatternParser"; + + private final PathPatternParser parser; + + private ApplicationContext context; + + /** + * Construct this factory bean using the default {@link PathPatternParser} + * + *

+ * If you are using Spring MVC, it will use the Spring MVC instance. + */ + public PathPatternRequestMatcherBuilderFactoryBean() { + this(null); + } + + /** + * Construct this factory bean using this {@link PathPatternParser}. + * + *

+ * If you are using Spring MVC, it is likely incorrect to call this constructor. + * Please call the default constructor instead. + * @param parser the {@link PathPatternParser} to use + */ + public PathPatternRequestMatcherBuilderFactoryBean(PathPatternParser parser) { + this.parser = parser; + } + + @Override + public PathPatternRequestMatcher.Builder getObject() throws Exception { + if (!this.context.containsBean(PATTERN_PARSER_BEAN_NAME)) { + PathPatternParser parser = (this.parser != null) ? this.parser : PathPatternParser.defaultInstance; + return PathPatternRequestMatcher.withPathPatternParser(parser); + } + PathPatternParser mvc = this.context.getBean(PATTERN_PARSER_BEAN_NAME, PathPatternParser.class); + PathPatternParser parser = (this.parser != null) ? this.parser : mvc; + if (mvc.equals(parser)) { + return PathPatternRequestMatcher.withPathPatternParser(parser); + } + throw new IllegalArgumentException("Spring Security and Spring MVC must use the same path pattern parser. " + + "To have Spring Security use Spring MVC's simply publish this bean [" + + this.getClass().getSimpleName() + "] using its default constructor"); + } + + @Override + public Class getObjectType() { + return PathPatternRequestMatcher.Builder.class; + } + + @Override + public void setApplicationContext(ApplicationContext context) throws BeansException { + this.context = context; + } + +} 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 8f095b07fa8..ae49e33f521 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 @@ -31,7 +31,6 @@ import org.springframework.context.annotation.Configuration; import org.springframework.core.ResolvableType; import org.springframework.http.HttpMethod; -import org.springframework.lang.NonNull; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.config.ObjectPostProcessor; import org.springframework.security.config.annotation.web.AbstractRequestMatcherRegistry.DispatcherServletDelegatingRequestMatcher; @@ -40,9 +39,9 @@ import org.springframework.security.web.servlet.MockServletContext; import org.springframework.security.web.servlet.TestMockHttpServletMappings; 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.AntPathRequestMatcher; import org.springframework.security.web.util.matcher.DispatcherTypeRequestMatcher; -import org.springframework.security.web.util.matcher.MethodPathRequestMatcherFactory; import org.springframework.security.web.util.matcher.RegexRequestMatcher; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.test.web.servlet.MockMvc; @@ -89,15 +88,13 @@ public void setUp() { ObjectProvider> given = this.context.getBeanProvider(type); given(given).willReturn(postProcessors); given(postProcessors.getObject()).willReturn(NO_OP_OBJECT_POST_PROCESSOR); - MethodPathRequestMatcherFactory factory = this.matcherRegistry.new DefaultMethodPathRequestMatcherFactory(); - ObjectProvider requestMatcherFactory = new ObjectProvider<>() { + ObjectProvider requestMatcherFactory = new ObjectProvider<>() { @Override - @NonNull - public MethodPathRequestMatcherFactory getObject() throws BeansException { - return factory; + public PathPatternRequestMatcher.Builder getIfUnique() throws BeansException { + return null; } }; - given(this.context.getBeanProvider(MethodPathRequestMatcherFactory.class)).willReturn(requestMatcherFactory); + given(this.context.getBeanProvider(PathPatternRequestMatcher.Builder.class)).willReturn(requestMatcherFactory); given(this.context.getServletContext()).willReturn(MockServletContext.mvc()); 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 80f3dc76f97..a299265d483 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 @@ -51,6 +51,7 @@ import org.springframework.security.config.observation.SecurityObservationSettings; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; +import org.springframework.security.config.web.PathPatternRequestMatcherBuilderFactoryBean; import org.springframework.security.core.Authentication; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.authority.SimpleGrantedAuthority; @@ -682,6 +683,13 @@ public void requestMatchersWhenMultipleDispatcherServletsAndPathBeanThenAllows() this.mvc.perform(get("/path").with(user("user"))).andExpect(status().isForbidden()); } + @Test + public void requestMatchersWhenFactoryBeanThenAuthorizes() throws Exception { + this.spring.register(PathPatternFactoryBeanConfig.class).autowire(); + this.mvc.perform(get("/path/resource")).andExpect(status().isUnauthorized()); + this.mvc.perform(get("/path/resource").with(user("user").roles("USER"))).andExpect(status().isNotFound()); + } + @Configuration @EnableWebSecurity static class GrantedAuthorityDefaultHasRoleConfig { @@ -1357,4 +1365,29 @@ SecurityFilterChain security(HttpSecurity http) throws Exception { } + @Configuration + @EnableWebSecurity + @EnableWebMvc + static class PathPatternFactoryBeanConfig { + + @Bean + SecurityFilterChain security(HttpSecurity http) throws Exception { + // @formatter:off + http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers("/path/**").hasRole("USER") + ) + .httpBasic(withDefaults()); + // @formatter:on + + return http.build(); + } + + @Bean + PathPatternRequestMatcherBuilderFactoryBean pathPatternFactoryBean() { + return new PathPatternRequestMatcherBuilderFactoryBean(); + } + + } + } diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java index f22e55043d9..e6a597e0a8a 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/RequestCacheConfigurerTests.java @@ -34,6 +34,7 @@ import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContextExtension; +import org.springframework.security.config.web.PathPatternRequestMatcherBuilderFactoryBean; import org.springframework.security.core.userdetails.User; import org.springframework.security.provisioning.InMemoryUserDetailsManager; import org.springframework.security.test.web.servlet.RequestCacheResultMatcher; @@ -291,6 +292,22 @@ public void getWhenCustomRequestCacheInLambdaThenCustomRequestCacheUsed() throws this.mvc.perform(formLogin(session)).andExpect(redirectedUrl("/")); } + @Test + public void getWhenPathPatternFactoryBeanThenFaviconIcoRedirectsToRoot() throws Exception { + this.spring + .register(RequestCacheDefaultsConfig.class, DefaultSecurityConfig.class, PathPatternFactoryBeanConfig.class) + .autowire(); + // @formatter:off + MockHttpSession session = (MockHttpSession) this.mvc.perform(get("/favicon.ico")) + .andExpect(redirectedUrl("http://localhost/login")) + .andReturn() + .getRequest() + .getSession(); + // @formatter:on + // ignores favicon.ico + this.mvc.perform(formLogin(session)).andExpect(redirectedUrl("/")); + } + private static RequestBuilder formLogin(MockHttpSession session) { // @formatter:off return post("/login") @@ -470,4 +487,15 @@ InMemoryUserDetailsManager userDetailsManager() { } + @Configuration + @EnableWebSecurity + static class PathPatternFactoryBeanConfig { + + @Bean + PathPatternRequestMatcherBuilderFactoryBean factoryBean() { + return new PathPatternRequestMatcherBuilderFactoryBean(); + } + + } + } diff --git a/config/src/test/java/org/springframework/security/config/web/PathPatternRequestMatcherBuilderFactoryBeanTests.java b/config/src/test/java/org/springframework/security/config/web/PathPatternRequestMatcherBuilderFactoryBeanTests.java new file mode 100644 index 00000000000..67d16bfa48c --- /dev/null +++ b/config/src/test/java/org/springframework/security/config/web/PathPatternRequestMatcherBuilderFactoryBeanTests.java @@ -0,0 +1,99 @@ +/* + * 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.config.web; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; + +import org.springframework.context.support.GenericApplicationContext; +import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher; +import org.springframework.web.util.pattern.PathPatternParser; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +@ExtendWith(MockitoExtension.class) +class PathPatternRequestMatcherBuilderFactoryBeanTests { + + GenericApplicationContext context; + + @BeforeEach + void setUp() { + this.context = new GenericApplicationContext(); + } + + @Test + void getObjectWhenDefaultsThenBuilder() throws Exception { + factoryBean().getObject(); + } + + @Test + void getObjectWhenMvcPatternParserThenUses() throws Exception { + PathPatternParser mvc = registerMvcPatternParser(); + PathPatternRequestMatcher.Builder builder = factoryBean().getObject(); + builder.matcher("/path/**"); + verify(mvc).parse("/path/**"); + } + + @Test + void getObjectWhenPathPatternParserThenUses() throws Exception { + PathPatternParser parser = mock(PathPatternParser.class); + PathPatternRequestMatcher.Builder builder = factoryBean(parser).getObject(); + builder.matcher("/path/**"); + verify(parser).parse("/path/**"); + } + + @Test + void getObjectWhenMvcAndPathPatternParserConflictThenIllegalArgument() { + registerMvcPatternParser(); + PathPatternParser parser = mock(PathPatternParser.class); + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> factoryBean(parser).getObject()); + } + + @Test + void getObjectWhenMvcAndPathPatternParserAgreeThenUses() throws Exception { + PathPatternParser mvc = registerMvcPatternParser(); + PathPatternRequestMatcher.Builder builder = factoryBean(mvc).getObject(); + builder.matcher("/path/**"); + verify(mvc).parse("/path/**"); + } + + PathPatternRequestMatcherBuilderFactoryBean factoryBean() { + PathPatternRequestMatcherBuilderFactoryBean factoryBean = new PathPatternRequestMatcherBuilderFactoryBean(); + factoryBean.setApplicationContext(this.context); + return factoryBean; + } + + PathPatternRequestMatcherBuilderFactoryBean factoryBean(PathPatternParser parser) { + PathPatternRequestMatcherBuilderFactoryBean factoryBean = new PathPatternRequestMatcherBuilderFactoryBean( + parser); + factoryBean.setApplicationContext(this.context); + return factoryBean; + } + + PathPatternParser registerMvcPatternParser() { + PathPatternParser mvc = mock(PathPatternParser.class); + this.context.registerBean(PathPatternRequestMatcherBuilderFactoryBean.PATTERN_PARSER_BEAN_NAME, + PathPatternParser.class, () -> mvc); + this.context.refresh(); + return mvc; + } + +} diff --git a/docs/modules/ROOT/pages/migration/web.adoc b/docs/modules/ROOT/pages/migration/web.adoc index 95580d226d1..23716dbf6c5 100644 --- a/docs/modules/ROOT/pages/migration/web.adoc +++ b/docs/modules/ROOT/pages/migration/web.adoc @@ -15,8 +15,8 @@ Java:: [source,java,role="primary"] ---- @Bean -MethodPatternRequestMatcherFactory requestMatcherFactory(@Qualifier("mvcPatternParser") PathPatternParser parser) { - return PathPatternRequestMatcher.withPathPatternParser(parser); +PathPatternRequestMatcherBuilderFactoryBean requestMatcherBuilder() { + return new PathPatternRequestMatcherBuilderFactoryBean(); } ---- @@ -25,15 +25,15 @@ Kotlin:: [source,kotlin,role="secondary"] ---- @Bean -fun requestMatcherFactory(@Qualifier("mvcPatternParser") parser: PathPatternParser): MethodPatternRequestMatcherFactory { - return PathPatternRequestMatcher.withPathPatternParser(parser) +fun requestMatcherBuilder(): PathPatternRequestMatcherBuilderFactoryBean { + return PathPatternRequestMatcherBuilderFactoryBean() } ---- ====== This will tell the Spring Security DSL to use `PathPatternRequestMatcher` for all request matchers that it constructs. -In the event that you are directly constructing an object (as opposed to having the DSL contstruct it) that has a `setRequestMatcher` method. you should also proactively specify a `PathPatternRequestMatcher` there as well. +In the event that you are directly constructing an object (as opposed to having the DSL construct it) that has a `setRequestMatcher` method. you should also proactively specify a `PathPatternRequestMatcher` there as well. For example, in the case of `LogoutFilter`, it constructs an `AntPathRequestMatcher` in Spring Security 6: