Add PathPatternRequestMatcher #16499

jzheaux · 2025-01-29T01:19:32Z

This PR adds PathPatternRequestMatcher, a request matcher that doesn't require HandlerMappingIntrospector.

Also, it adds a servlet-aware builder for specifying the servlet path (using this request matcher) in a way that looks similar to Kotlin and XML:

import static org.springframework.security.web.servlet.util.matcher.RequestMatchers.servlet;

@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")
			.anyRequest().authenticated()
		);

	return http.build();
}

Future tickets like #16509 are taking a look at a first-class way to represent the servlet inside the authorization DSL. This builder is still helpful since there are many other places in the Java DSL where request matchers are provided that may need the servlet path specified.

Closes spring-projectsgh-16429

This static factory simplifes the creation of RequestMatchers that specify the servlet path and other request elements Closes spring-projectsgh-16430

rwinch

Thanks for all of your work on this @jzheaux! I've provided feedback below:

rwinch · 2025-02-07T16:49:11Z

.../java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java

+	 * @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) {


I'd prefer that interfaces be passed in to the DSL rather than implementations. Implementations are quite limiting (which is why we tend to use interfaces).

Perhaps for now, users must leverage Builder.matcher() and pass this into the existing DSL since we plan on refining the DLS going forward.

I would like the DSL to avoid needing static imports in order to function. It makes discovery more difficult. Even if it is only a little more difficult, each small difficulty adds up.

rwinch · 2025-02-07T16:54:44Z

docs/modules/ROOT/pages/migration-7/web.adoc

+
+== Use Absolute Authorization URIs
+
+The Java DSL now requires that all URIs be absolute (less any context root).


I would rephrase this a bit to state that Spring Security 7 will require since I don't think that this is true now.

rwinch · 2025-02-07T16:55:32Z

docs/modules/ROOT/pages/migration-7/web.adoc

+
+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].


I'm having trouble understanding this section.

rwinch · 2025-02-07T16:57:25Z

docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc

    http {
        authorizeHttpRequests {
-            authorize(mvc.pattern("/my/controller/**"), hasAuthority("controller"))
+            authorize("/spring-mvc", "/admin/**", hasAuthority("admin"))


Does this need updated? I think that authorize was in a different revision, but does not exist anymore.

The documentation was mistaken. AFAIK, this is the correct way to specify the servlet path in the Kotlin DSL. See this test as an example.

rwinch · 2025-02-07T17:01:03Z

docs/modules/ROOT/pages/servlet/authorization/authorize-http-requests.adoc

+
+[TIP]
+=====
+There are several other components that create request matchers for you like `PathRequest#toStaticResources#atCommonLocations`


What is PathRequest#toStaticResources#atCommonLocations?

It is a Spring Boot feature. I'll add a link for additional context.

rwinch · 2025-02-07T19:18:23Z

web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java

+	 * {@code servletPath}.
+	 *
+	 * <p>
+	 * The {@code servletPath} must correlate to a configured servlet in your application.


This sounds like servletPath might be the equivalent of <servlet-mapping>/<url-pattern>, but I suspect it is corresponding to HttpServletRequest.servletPath. Perhaps, this could be clarified?

rwinch · 2025-02-07T19:20:24Z

web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java

+	 * @return a {@link Builder} that treats URIs as relative to the context path, if any
+	 * @since 6.5
+	 */
+	public static Builder request() {


Perhaps this could be emptyServletPath() or similar?

rwinch · 2025-02-07T19:22:01Z

web/src/main/java/org/springframework/security/web/util/matcher/RequestMatchers.java

+	 * @since 6.5
+	 */
+	public static Builder request() {
+		return new Builder();


I wonder if this is accurate since it matches any servletPath vs an empty servlet path?

Perhaps. Though this method is not really about declaring a request in absence of something else. I wouldn't, for example, want AntPathRequestMatcher#antPattern(String pattern) to be AntPathRequestMatcher#emptyMethodPattern(String pattern) because no method is specified.

I'm open to thinking about other names, but I don't see why we'd want to define this in terms of what is not specified.

Furthermore, I think the 95% case is that defining a servlet path isn't necessary, so I'd prefer not to define the common case in terms of a concept that commonly is abstracted away from the developer.

Could you elaborate on why it's important to you?

(as a side note, even though I think that specifying a servlet is not in the 95%, I like that having it as an entry point allows for the returned builder to be reused, and I also like that it makes it clear that such a matcher should only be described once)

rwinch · 2025-02-07T19:23:51Z

...in/java/org/springframework/security/web/servlet/util/matcher/PathPatternRequestMatcher.java

+ */
+public final class PathPatternRequestMatcher implements RequestMatcher {
+
+	private final PathPattern pattern;


While PathPattern does not match on the servlet path, is that meaningful for Spring Security applications? I am concerned that this makes it too easy to do the wrong thing?

I'm open to other possibilities. One that I considered was to enhance PathPatternRequestMatcher to be something more of an general-purpose request matcher. In this way, its pattern of construction would indicate a place for the servlet path. Perhaps the builder in RequestMatchers can move to PathPatternRequestMatcher.

What are your thoughts on this?

rwinch · 2025-02-07T19:27:28Z

docs/modules/ROOT/pages/migration-7/web.adoc

@@ -102,3 +102,12 @@ Xml::
 </b:bean>
 ----
 ======
+
+== Use Absolute Authorization URIs


Can we add some examples?

- Refined documentation - Applied method naming feedback

Closes spring-projectsgh-16573

jzheaux added this to the 6.5.0-M2 milestone Jan 29, 2025

jzheaux self-assigned this Jan 29, 2025

jzheaux mentioned this pull request Jan 29, 2025

Favor PathPatternParser Over HandlerMappingIntrospector #16408

Closed

jzheaux force-pushed the gh-16429 branch 2 times, most recently from bdd1cf1 to 12f1cf2 Compare January 29, 2025 21:29

jzheaux mentioned this pull request Feb 3, 2025

Prepare Request Matching for Spring Framework Changes #16417

Open

8 tasks

jzheaux force-pushed the gh-16429 branch from bcbbeb5 to bca1d7f Compare February 7, 2025 00:56

jzheaux added 2 commits February 6, 2025 18:01

Add PathPatternRequestMatcher

2dae803

Closes spring-projectsgh-16429

Add RequestMatchers.Builder

a22f87d

This static factory simplifes the creation of RequestMatchers that specify the servlet path and other request elements Closes spring-projectsgh-16430

jzheaux force-pushed the gh-16429 branch from bca1d7f to a22f87d Compare February 7, 2025 01:01

rstoyanchev mentioned this pull request Feb 7, 2025

Provide access to servletPath in ServletRequestPathUtils spring-projects/spring-framework#34379

Closed

jzheaux marked this pull request as ready for review February 7, 2025 18:08

jzheaux added in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement labels Feb 7, 2025

rwinch requested changes Feb 7, 2025

View reviewed changes

jzheaux added 4 commits February 7, 2025 15:53

Updates

9b77542

- Refined documentation - Applied method naming feedback

Polish PathPattern

470966b

Polish Usage Updates

8441b55

Add Ability to Opt-in to PathPattern

146fd61

Closes spring-projectsgh-16573

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PathPatternRequestMatcher #16499

Add PathPatternRequestMatcher #16499

jzheaux commented Jan 29, 2025 •

edited

Loading

rwinch left a comment

rwinch Feb 7, 2025 •

edited

Loading

rwinch Feb 7, 2025

rwinch Feb 7, 2025

rwinch Feb 7, 2025

rwinch Feb 7, 2025

jzheaux Feb 7, 2025

rwinch Feb 7, 2025

jzheaux Feb 7, 2025

rwinch Feb 7, 2025

rwinch Feb 7, 2025

rwinch Feb 7, 2025

jzheaux Feb 7, 2025

rwinch Feb 7, 2025

jzheaux Feb 7, 2025 •

edited

Loading

rwinch Feb 7, 2025


		== Use Absolute Authorization URIs

		The Java DSL now requires that all URIs be absolute (less any context root).


		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].

Add PathPatternRequestMatcher #16499

Are you sure you want to change the base?

Add PathPatternRequestMatcher #16499

Conversation

jzheaux commented Jan 29, 2025 • edited Loading

rwinch left a comment

Choose a reason for hiding this comment

rwinch Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jzheaux Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jzheaux commented Jan 29, 2025 •

edited

Loading

rwinch Feb 7, 2025 •

edited

Loading

jzheaux Feb 7, 2025 •

edited

Loading