From f8ca5ce975ac46fe395f25730345c6156dcf266e Mon Sep 17 00:00:00 2001 From: Josh Cummings <3627351+jzheaux@users.noreply.github.com> Date: Thu, 16 Jan 2025 11:57:56 -0700 Subject: [PATCH] Add Servlet Path support to Java DSL Closes gh-16430 --- .../web/AbstractRequestMatcherRegistry.java | 15 +-- .../AuthorizeHttpRequestsConfigurerTests.java | 37 +++++++ docs/modules/ROOT/pages/migration-7/web.adoc | 9 ++ .../authorize-http-requests.adoc | 57 ++++++---- .../ServletRequestMatcherBuilders.java | 102 ++++++++++++++++++ .../ServletRequestMatcherBuildersTests.java | 66 ++++++++++++ 6 files changed, 259 insertions(+), 27 deletions(-) create mode 100644 web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuilders.java create mode 100644 web/src/test/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuildersTests.java diff --git a/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java b/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java index 4f849f86fb1..ec800e99a87 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. @@ -264,11 +264,14 @@ private RequestMatcher resolve(AntPathRequestMatcher ant, MvcRequestMatcher mvc, } private static String computeErrorMessage(Collection<? extends ServletRegistration> registrations) { - String template = "This method cannot decide whether these patterns are Spring MVC patterns or not. " - + "If this endpoint is a Spring MVC endpoint, please use requestMatchers(MvcRequestMatcher); " - + "otherwise, please use requestMatchers(AntPathRequestMatcher).\n\n" - + "This is because there is more than one mappable servlet in your servlet context: %s.\n\n" - + "For each MvcRequestMatcher, call MvcRequestMatcher#setServletPath to indicate the servlet path."; + String template = """ + This method cannot decide whether these patterns are Spring MVC patterns or not. \ + This is because there is more than one mappable servlet in your servlet context: %s. + + To address this, please create one ServletRequestMatcherBuilder#servletPath for each servlet that has \ + authorized endpoints and use them to construct request matchers manually. \ + If all your URIs are unambiguous, then you can simply publish one ServletRequestMatcherBuilders#servletPath as \ + a @Bean and Spring Security will use it for all URIs"""; Map<String, Collection<String>> 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..26ba96e02b2 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeHttpRequestsConfigurerTests.java @@ -64,6 +64,8 @@ import org.springframework.security.web.access.intercept.RequestAuthorizationContext; import org.springframework.security.web.access.intercept.RequestMatcherDelegatingAuthorizationManager; import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; +import org.springframework.security.web.servlet.util.matcher.ServletRequestMatcherBuilders; +import org.springframework.security.web.util.matcher.RequestMatcherBuilder; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.test.web.servlet.request.RequestPostProcessor; @@ -72,6 +74,7 @@ import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.config.annotation.EnableWebMvc; import org.springframework.web.servlet.handler.HandlerMappingIntrospector; @@ -667,6 +670,19 @@ public void getWhenExcludeAuthorizationObservationsThenUnobserved() throws Excep verifyNoInteractions(handler); } + @Test + public void requestMatchersWhenMultipleDispatcherServletsAndPathBeanThenAllows() throws Exception { + this.spring.register(MvcRequestMatcherBuilderConfig.class, BasicController.class) + .postProcessor((context) -> context.getServletContext() + .addServlet("otherDispatcherServlet", DispatcherServlet.class) + .addMapping("/mvc")) + .autowire(); + this.mvc.perform(get("/mvc/path").servletPath("/mvc").with(user("user"))).andExpect(status().isOk()); + this.mvc.perform(get("/mvc/path").servletPath("/mvc").with(user("user").roles("DENIED"))) + .andExpect(status().isForbidden()); + this.mvc.perform(get("/path").with(user("user"))).andExpect(status().isForbidden()); + } + @Configuration @EnableWebSecurity static class GrantedAuthorityDefaultHasRoleConfig { @@ -1262,6 +1278,10 @@ void rootGet() { void rootPost() { } + @GetMapping("/path") + void path() { + } + } @Configuration @@ -1317,4 +1337,21 @@ SecurityObservationSettings observabilityDefaults() { } + @Configuration + @EnableWebSecurity + @EnableWebMvc + static class MvcRequestMatcherBuilderConfig { + + @Bean + SecurityFilterChain security(HttpSecurity http) throws Exception { + RequestMatcherBuilder mvc = ServletRequestMatcherBuilders.servletPath("/mvc"); + http.authorizeHttpRequests( + (authorize) -> authorize.requestMatchers(mvc.pattern("/path/**")).hasRole("USER")) + .httpBasic(withDefaults()); + + return http.build(); + } + + } + } diff --git a/docs/modules/ROOT/pages/migration-7/web.adoc b/docs/modules/ROOT/pages/migration-7/web.adoc index 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:: </b:bean> ---- ====== + +== 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..03524839543 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc @@ -577,15 +577,11 @@ http { ====== [[match-by-mvc]] -=== Using an MvcRequestMatcher +=== Matching by Servlet Path Generally speaking, you can use `requestMatchers(String)` as demonstrated above. -However, if you map Spring MVC to a different servlet path, then you need to account for that in your security configuration. - -For example, if Spring MVC is mapped to `/spring-mvc` instead of `/` (the default), then you may have an endpoint like `/spring-mvc/my/controller` that you want to authorize. - -You need to use `MvcRequestMatcher` to split the servlet path and the controller path in your configuration like so: +However, if you have authorization rules from multiple servlets, you need to specify those: .Match by MvcRequestMatcher [tabs] @@ -594,16 +590,14 @@ Java:: + [source,java,role="primary"] ---- -@Bean -MvcRequestMatcher.Builder mvc(HandlerMappingIntrospector introspector) { - return new MvcRequestMatcher.Builder(introspector).servletPath("/spring-mvc"); -} +import static org.springframework.security.web.servlet.util.matcher.ServletRequestMatcherBuilders.servletPath; @Bean -SecurityFilterChain appEndpoints(HttpSecurity http, MvcRequestMatcher.Builder mvc) { +SecurityFilterChain appEndpoints(HttpSecurity http) { http .authorizeHttpRequests((authorize) -> authorize - .requestMatchers(mvc.pattern("/my/controller/**")).hasAuthority("controller") + .requestMatchers(servletPath("/spring-mvc").pattern("/admin/**")).hasAuthority("admin") + .requestMatchers(servletPath("/spring-mvc").pattern("/my/controller/**")).hasAuthority("controller") .anyRequest().authenticated() ); @@ -616,17 +610,15 @@ Kotlin:: [source,kotlin,role="secondary"] ---- @Bean -fun mvc(introspector: HandlerMappingIntrospector): MvcRequestMatcher.Builder = - MvcRequestMatcher.Builder(introspector).servletPath("/spring-mvc"); - -@Bean -fun appEndpoints(http: HttpSecurity, mvc: MvcRequestMatcher.Builder): SecurityFilterChain = +fun appEndpoints(http: HttpSecurity): SecurityFilterChain { http { authorizeHttpRequests { - authorize(mvc.pattern("/my/controller/**"), hasAuthority("controller")) + authorize("/spring-mvc", "/admin/**", hasAuthority("admin")) + authorize("/spring-mvc", "/my/controller/**", hasAuthority("controller")) authorize(anyRequest, authenticated) } } +} ---- Xml:: @@ -634,16 +626,39 @@ Xml:: [source,xml,role="secondary"] ---- <http> + <intercept-url servlet-path="/spring-mvc" pattern="/admin/**" access="hasAuthority('admin')"/> <intercept-url servlet-path="/spring-mvc" pattern="/my/controller/**" access="hasAuthority('controller')"/> <intercept-url pattern="/**" access="authenticated"/> </http> ---- ====== -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.ServletRequestMatcherBuilders.servletPath; + +@Bean +SecurityFilterChain appEndpoints(HttpSecurity http) { + RequestMatcherBuilder mvc = servletPath("/spring-mvc"); + http + .authorizeHttpRequests((authorize) -> authorize + .requestMatchers(mvc.pattern("/admin/**")).hasAuthority("admin") + .requestMatchers(mvc.pattern("/my/controller/**")).hasAuthority("controller") + .anyRequest().authenticated() + ); + + return http.build(); +} +---- -* 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) +[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/servlet/util/matcher/ServletRequestMatcherBuilders.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuilders.java new file mode 100644 index 00000000000..c038e2a7577 --- /dev/null +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuilders.java @@ -0,0 +1,102 @@ +/* + * Copyright 2002-2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.security.web.servlet.util.matcher; + +import jakarta.servlet.http.HttpServletRequest; + +import org.springframework.security.web.util.matcher.AndRequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcher; +import org.springframework.security.web.util.matcher.RequestMatcherBuilder; +import org.springframework.util.Assert; + +/** + * A {@link RequestMatcherBuilder} for specifying the servlet path separately from the + * rest of the URI. This is helpful when you have more than one servlet. + * + * <p> + * For example, if Spring MVC is deployed to `/mvc` and another servlet to `/other`, then + * you can do + * </p> + * + * <code> + * http + * .authorizeHttpRequests((authorize) -> authorize + * .requestMatchers(servletPath("/mvc").pattern("/my/**", "/controller/**", "/endpoints/**")).hasAuthority(... + * .requestMatchers(servletPath("/other").pattern("/my/**", "/non-mvc/**", "/endpoints/**")).hasAuthority(... + * } + * ... + * </code> + * + * @author Josh Cummings + * @since 6.5 + */ +public final class ServletRequestMatcherBuilders { + + private static final ServletPathRequestMatcher DEFAULT_SERVLET = new ServletPathRequestMatcher(""); + + private ServletRequestMatcherBuilders() { + } + + /** + * Create {@link RequestMatcher}s that will only match URIs against the default + * servlet. + * @return a {@link ServletRequestMatcherBuilders} that matches URIs mapped to the + * default servlet + */ + public static RequestMatcherBuilder defaultServlet() { + return servletPathInternal(DEFAULT_SERVLET, ""); + } + + /** + * Create {@link RequestMatcher}s that will only match URIs against the given servlet + * path + * + * <p> + * The path must be of the format {@code /path}. It should not end in `/` or `/*`, nor + * should it be a file extension. To specify the default servlet, use + * {@link #defaultServlet()}. + * </p> + * @return a {@link ServletRequestMatcherBuilders} that matches URIs mapped to the + * given servlet path + */ + public static RequestMatcherBuilder servletPath(String servletPath) { + Assert.notNull(servletPath, "servletPath cannot be null"); + Assert.isTrue(servletPath.startsWith("/"), "servletPath must start with '/'"); + Assert.isTrue(!servletPath.endsWith("/"), "servletPath must not end with a slash"); + Assert.isTrue(!servletPath.contains("*"), "servletPath must not contain a star"); + return servletPathInternal(new ServletPathRequestMatcher(servletPath), servletPath); + } + + private static RequestMatcherBuilder servletPathInternal(RequestMatcher servletPathMatcher, String servletPath) { + return (method, pattern) -> { + Assert.notNull(pattern, "pattern cannot be null"); + Assert.isTrue(pattern.startsWith("/"), "pattern must start with '/'"); + PathPatternRequestMatcher pathPattern = PathPatternRequestMatcher.pathPattern(method, + servletPath + pattern); + return new AndRequestMatcher(servletPathMatcher, pathPattern); + }; + } + + private record ServletPathRequestMatcher(String path) implements RequestMatcher { + + @Override + public boolean matches(HttpServletRequest request) { + return this.path.equals(request.getServletPath()); + } + } + +} diff --git a/web/src/test/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuildersTests.java b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuildersTests.java new file mode 100644 index 00000000000..8f9f2b1ee75 --- /dev/null +++ b/web/src/test/java/org/springframework/security/web/servlet/util/matcher/ServletRequestMatcherBuildersTests.java @@ -0,0 +1,66 @@ +/* + * 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.RequestMatcherBuilder; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +/** + * Tests for {@link ServletRequestMatcherBuilders} + */ +class ServletRequestMatcherBuildersTests { + + @Test + void patternWhenServletPathThenMatchesOnlyServletPath() { + RequestMatcherBuilder builder = ServletRequestMatcherBuilders.servletPath("/servlet/path"); + RequestMatcher requestMatcher = builder.pattern(HttpMethod.GET, "/endpoint"); + assertThat(requestMatcher.matches(request("/servlet/path/endpoint", "/servlet/path"))).isTrue(); + assertThat(requestMatcher.matches(request("/endpoint", ""))).isFalse(); + } + + @Test + void patternWhenDefaultServletThenMatchesOnlyDefaultServlet() { + RequestMatcherBuilder builder = ServletRequestMatcherBuilders.defaultServlet(); + RequestMatcher requestMatcher = builder.pattern(HttpMethod.GET, "/endpoint"); + assertThat(requestMatcher.matches(request("/servlet/path/endpoint", "/servlet/path"))).isFalse(); + assertThat(requestMatcher.matches(request("/endpoint", ""))).isTrue(); + } + + @Test + void servletPathWhenEndsWithSlashOrStarThenIllegalArgument() { + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> ServletRequestMatcherBuilders.servletPath("/path/**")); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> ServletRequestMatcherBuilders.servletPath("/path/*")); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> ServletRequestMatcherBuilders.servletPath("/path/")); + } + + MockHttpServletRequest request(String uri, String servletPath) { + MockHttpServletRequest request = new MockHttpServletRequest("GET", uri); + request.setServletPath(servletPath); + return request; + } + +}