Possible bug in AbstractRequestMatcherRegistry#requireOnlyPathMappedDispatcherServlet? (DispatcherServlet not found when resolving request matcher)

[mauromol]

Describe the bug
Once I added a DispatcherServlet to my EAR application deployed on JBoss 7.4, I started to get the following exception:

Exception handling request to /myapp/rest/foo/hello: java.lang.IllegalArgumentException: 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).

This is because there is more than one mappable servlet in your servlet context:
[indeed, I have a lot of mapped servlets and the DispatcherServlet is not the first one]

For each MvcRequestMatcher, call MvcRequestMatcher#setServletPath to indicate the servlet path.

To Reproduce
Adding a security filter chain with some request matchers; something like this:

  @Bean
  public SecurityFilterChain mySecurityFilterChain(final HttpSecurity http) throws Exception {
    http.csrf()
        .disable()
        .authorizeHttpRequests(new MyRules())
        ... // and so on
  }

public class MyRules implements Customizer<AuthorizeHttpRequestsConfigurer<HttpSecurity>.AuthorizationManagerRequestMatcherRegistry> {
  @Override
  public void customize(AuthorizeHttpRequestsConfigurer<HttpSecurity>.AuthorizationManagerRequestMatcherRegistry rules) {

    // static resources
    rules.requestMatchers("/index.html").permitAll();
    rules.requestMatchers("/static/**").permitAll();
    rules.requestMatchers("/**").denyAll();
  }
}

Expected behavior
I would expect Spring Security to find my DispatcherServlet mapping and so to use a MvcRequestMatcher.

Please have a look at this line:

spring-security/config/src/main/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistry.java

Line 289 in e90a6b6

return null;

Shouldn't it be continue instead of return null, just like it is for requireOneRootDispatcherServlet?
Otherwise this loop will always end on the first mapping if it's not a DispatcherServlet...

Please note I'm on Spring Security 5.8.13 and that line is number 408 instead.

[mauromol]

Perhaps I'm starting to understand the logic behind this implementation, which is probably as intended. So, Spring Security uses MVC matchers when using AbstractRequestMatcherRegistry#requestMatchers(java.lang.String...) if either there is a "root" DispatcherServlet (that is, mapped to "/"), or if there are only DispatcherServlets (and no other servlet) registered in the ServletContext. Indeed, it uses the detected DispatcherServlet to extract the servlet path prefix to correctly match incoming request URIs. In the latter case, for some reason, it makes the last DispatcherServlet found win, which sounds a bit odd to me, also because I found no trace of the fact that servlet registrations are/can be sorted in any way.

TBH, it was not easy to grasp this and I think the documentation is not so clear. Looking at the documentation at:

• https://docs.spring.io/spring-security/reference/5.8/migration/servlet/config.html#use-new-security-matchers
• https://docs.spring.io/spring-security/reference/5.8/servlet/authorization/authorize-http-requests.html#_request_matchers

it seems to suggest that just having Spring Web MVC on the classpath makes usage of AbstractRequestMatcherRegistry#requestMatchers(java.lang.String...) method automagically work for basic usages and patterns, whichever is the servlet that will handle the target resource. Also, the code in AbstractRequestMatcherRegistry is not well documented. However, what seems like to emerge is that the whole thing works only if you have either no Spring Dispatcher Servlet usage or if you are only registering Spring Dispatcher Servlets (so in a fully-Spring Web MVC managed webapp). But in a mixed application, using different kind of servlets to handle different URIs, that method will never work and you always have to use explicit Ant or MVC request matchers.

Is my understanding right?
Why do you assume that, if you have some DispatcherServlets registered in the servlet context, then the default behaviour expected by the user when defining URL matching patterns to secure requests with AbstractRequestMatcherRegistry#requestMatchers(java.lang.String...) is to use paths relative to the mapping of one (the last one?) of those DispatcherServlets and not to the "root" servlet path returned by the ServletContext? I'm not getting this.

[jzheaux]

Thanks for reaching out, @mauromol. You can see a more detailed explanation in the 6.x documentation. There may be value in updating the 5.8.x documentation to include this. I will summarize a bit of it here in order to address your questions.

So, Spring Security uses MVC matchers when using AbstractRequestMatcherRegistry#requestMatchers(java.lang.String...) if either there is a "root" DispatcherServlet (that is, mapped to "/"), or if there are only DispatcherServlets (and no other servlet) registered in the ServletContext.

Close. Because Spring MVC endpoints are expressed without the servlet path, Spring Security can only safely compute them when DispatcherServlet's servlet path is / or when DispatcherServlet is the only servlet.

In the latter case, for some reason, it makes the last DispatcherServlet found win, which sounds a bit odd to me, also because I found no trace of the fact that servlet registrations are/can be sorted in any way.
...
Why do you assume that, if you have some DispatcherServlets registered in the servlet context, then the default behaviour expected by the user when defining URL matching patterns to secure requests with AbstractRequestMatcherRegistry#requestMatchers(java.lang.String...) is to use paths relative to the mapping of one (the last one?) of those DispatcherServlets and not to the "root" servlet path returned by the ServletContext?

I don't think that's the case, though I'd welcome a unit test to demonstrate what you are surmising from the code. The method in question might be better named requireExactlyOnePathMappedDispatcherServlet.

The intent is that the configuration fail with Spring MVC if either one of these is true:

• There is more than one DispatcherServlet
• There is more than one servlet and the DispatcherServlet is not mapped to root

In those cases, the application must clarify whether each is an MVC endpoint or not. You can see this tested for in requestMatchersWhenMultipleDispatcherServletMappingsThenException.

Does this help?

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[mauromol]

I don't think that's the case, though I'd welcome a unit test to demonstrate what you are surmising from the code.

Hi Josh, I admit I was wrong: even though I read that code multiple times, I didn't get that the key is this exit condition from the loop:

if (pathDispatcherServlet != null) {
  return null;
}

I wasn't seeing it, and so I assumed the loop was going on and finally taking the last dispatcher servlet instance matching the required conditions. Anyway, it's probably a bit unusual as a way to implement the algorithm you mentioned, but it's right, so please apologize.

Still, I don't fully grasp why you really need to do all of this anyway. "Because Spring MVC endpoints are expressed without the servlet path"... right, but... since we're serving a request and hence we have the HTTP request at hand, its context path and servlet path, can't some URL matching be done to determine if the incoming request is served by any of the registered DispatcherServlets, whatever is their mapping? I also did a quick look at the Spring Security 6 docs, but it didn't help (perhaps I didn't find it?).

The main issue is that: if you have a regular web application with some servlets, and you register even a single DispatcherServlet along with them, the requestMatchers(String) method, which is also the recommended one to use when upgrading from a previous version, will simply fail by default. I know this application setup is not so frequent nowadays, but probably an explicit mention of this case could help people like me, still struggling with Java EE/Jakarta EE applications.

[jzheaux]

The main issue is that: if you have a regular web application with some servlets, and you register even a single DispatcherServlet along with them, the requestMatchers(String) method, which is also the recommended one to use when upgrading from a previous version, will simply fail by default.

This isn't true unless DispatcherServlet isn't root. If you are experiencing this with a root-mapped DispatcherServlet, it may be a bug and I'd welcome a sample.

Still, I don't fully grasp why you really need to do all of this anyway

I'd recommend taking a look at the unit tests in AbstractRequestMatcherRegistryTests to dive into this further. A lot of it is about corner cases where URI patterns can overlap. For example, if Spring MVC is deployed to /mvc/, then requestMatchers("/authenticated") could either mean /mvc/authenticated or /authenticated and there is no way to know which pattern the application means for us to secure.

will simply fail by default.

I do agree that it would be nice to alert the developer sooner. Perhaps there is an evolution of the DSL that would make the configuration unambiguous. You can follow #13562 and its linked issues to learn more about this.