Skip to content

A RequestMatcherFactory API #16208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

jzheaux
Copy link
Contributor

@jzheaux jzheaux commented Dec 2, 2024

Closes gh-13562

This allows an application to directly configure how requestMatchers(String) works, thereby delegating the responsibility of disambiguating request patterns.

For example, in an application where Spring MVC is on /servlet/path and there are other servlets, there might exist two endpoints, one at /example and one at /servlet/path/example. Because Spring MVC patterns exclude the /servlet/path prefix, a declaration like this one:

.requestMatchers("/example").permitAll()

becomes ambiguous.

With this PR, an application can alleviate this in one of three ways:

First, they can state that all their String-based request matcher endpoints are MVC endpoints, a common scenario:

@Bean
RequestMatcherFactory useMvc(HandlerMappingIntrospector introspector) {
    MvcRequestMatcher.Builder builder = new MvcRequestMatcher.Builder(introspector).servletPath("/servlet/path");
    return mvc:pattern;
}

Spring Security will consult this bean instead of using the default behavior in requestMatchers(String) , securityMatchers(String) or ignoringRequestMatchers(String) methods and interpret each one as a Spring MVC request.

Second, they can construct a MvcRequestMatcherFactory which will check the destination of the request and use MVC matching rules if the request is targeting DispatcherServlet or Ant matching rules otherwise:

@Bean
RequestMatcherFactory delegating(MvcRequestMatcherFactory.Builder builder) {
    return builder.servletPath("/servlet/path").build();
}

Or third, they can introduce logic of their own like so:

@Bean
RequestMatcherFactory custom(HandlerMappingIntrospector introspector) {
    MvcRequestMatcher.Builder mvc = new MvcRequestMatcher.Builder(introspector).servletPath("/servlet/path");
    return (method, pattern) -> ("/graphql".equals(pattern)) ? 
            new AntPathRequestMatcher("/graphql") : mvc.requestMatcher(method, pattern);
}

Though in the third case, it will likely be less work to do the first or second option and then do:

@Bean 
SecurityFilterChain security(HttpSecurity http) throws Exception {
    http
        .authorizeHttpRequests((authorize) -> authorize
            .requestMatchers(new AntPathRequestMatcher("/graphql")).hasAuthority("graphql")
            .requestMatchers("/my", "/mvc", "/endpoints').hasAuthority("app")
        )
}

@jzheaux jzheaux self-assigned this Dec 2, 2024
@jzheaux jzheaux added this to the 6.5.x milestone Dec 2, 2024
@rwinch
Copy link
Member

rwinch commented Dec 3, 2024

I think that a user that wants to change the strategy can leverage requestMatchers(RequestMatcher...) rather than us adding another API (complexity). What do you think @jzheaux ?

@jzheaux
Copy link
Contributor Author

jzheaux commented Dec 4, 2024

I think that's a good point, and there may be some value in just having the user use a builder directly for now:

@Bean 
SecurityFilterChain security(MvcRequestMatcherFactory.Builder builder) {
    MvcRequestMatcherFactory mvc = builder.servletPath("/servlet/path").build();
    http
        .authorizeHttpRequests((authorize) -> authorize
            .requestMatchers(mvc.patterns("/my/**", "/controllers/**")).permitAll()
         )
         // ...
    return http.build()
}

I like this bc it's consistent with how applications would do it if there were multiple dispatcher servlets:

@Bean 
SecurityFilterChain security(MvcRequestMatcherFactory.Builder builder) {
    MvcRequestMatcherFactory mvc = builder.servletPath("/mvc").build();
    MvcRequestMatcherFactory api = builder.servletPath("/api").build();
    http
        .authorizeHttpRequests((authorize) -> authorize
            .requestMatchers(mvc.patterns("/my/**", "/controllers/**")).permitAll()
            .requestMatchers(api.patterns("/resources/**")).hasAuthority("app")
         )
         // ...
    return http.build()
}

Does that address your concern? If so, I can leave out the support for the DSL picking up a RequestMatcherFactory bean. And I feel like that would allow removing the RequestMatcherFactory interface for the time being as well.

Or, are there more fundamental shifts that are on your mind?

@jzheaux
Copy link
Contributor Author

jzheaux commented Dec 21, 2024

Some recent developments in the Spring Framework may allow us to pursue something simpler than this solution. So I'm going to close for now and then I'll reopen a draft PR with the relevant changes.

@jzheaux jzheaux closed this Dec 21, 2024
@jzheaux jzheaux added the status: declined A suggestion or change that we don't feel we should currently apply label Dec 21, 2024
@jzheaux jzheaux removed this from the 6.5.x milestone Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify MvcRequestMatcher construction
2 participants