Skip to content

DelegatingHandlerMapping does not implement MatchableHandlerMapping correctly #2007

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
taha-sk opened this issue May 6, 2021 · 12 comments
Closed
Assignees
Labels
type: bug A general bug

Comments

@taha-sk
Copy link

taha-sk commented May 6, 2021

Hello,

Recently, I encountered this problem and I couldn't find any open issue about it. I think this is not a security vulnerability, because it's preventing access. Also, this issue may be relevant with Spring Security or Spring Data Rest, in that case, please assign this to them.

I'm using Windows 7 Professional 64-bit OS with Service Pack 1 Update, JDK 11.0.10 and Spring Boot 2.4.5.

I prepared a sample project in https://github.com/taha-sk/spring-cors-issue.git . Please checkout that and follow my instructions.

Short Version

It looks like Spring MVC and Spring Data REST handles/registers CORS separately. If you enable Spring Security in your project and define HTTP Request Headers like "'X-Requested-With', 'XMLHttpRequest'" or "'Authorization', 'Bearer '", then Spring Security will not detect CORS settings for Spring Data REST. It will only handle CORS settings defined for Spring MVC. Unfortunately, this is making "@crossorigin" annotations found above repositories useless.


Long Version

Checkout the repo and import the projects into IDEs. "cors-spring" is a spring boot application project using Spring Boot, Data JPA, Data REST and H2 database. "cors-angular" is a simple angular project built by using node v13.14.0 and npm v6.14.11. Build both projects and run. For Angular, from the root directory of the project use npm install, then ng serve --open. This will start the Angular instance on localhost:4200 and open a web browser window.

If you set up and run projects correctly, it will run successfully. You won't see any errors on the console.

resim

As a browser, I'm using Firefox v88.

In this project, we have a ResourceController which is defined by "@RestController" annotation and binds to "/resource" and we have a WidgetRepository which is extending PagingAndSortingRepository and binds to "/widgets". I already set up correct CORS by using global CORS configurations.

AppRepositoryRestConfigurer implements RepositoryRestConfigurer and registers CORS for Spring Data REST. AppWebMvcConfigurer implements WebMvcConfigurer and registers CORS for Spring MVC.

You can't register entire CORS by using only one of them. You need them both. For instance, open AppRepositoryRestConfigurer.java and comment out cors mapping in line 13 and restart your Spring Boot instance. Refresh the browser page and you will see that CORS error is coming up for Repository definition ("/widgets").

resim

Yeah. CORS mapping defined in AppWebMvcConfigurer for "/**" does not cover Repository definition. If you comment out the CORS definition for Spring MVC in AppWebMvcConfigurer, you will get error for "/resource", but you won't get error for repository. That's why I think that Spring MVC and Spring Data REST handles/registers CORS separately.

Now, fix the AppRepositoryRestConfigurer.java file and re-enable CORS definition for the repository. Application should work successfully again. Right now, we know that CORS definition for the application is OK. Then, open pom.xml and erase comment declarations at line 41 and 51 and enable Spring Security. Save the pom.xml and exit. Also, go to AppWebSecurityConfigurerAdapter.java and remove comment declarations at line 1 and 38. Save the file and exit. This is will configure Spring Security for the application. Now, restart your server boot instance and refresh the page.

resim

Now, you will see HTTP 403 errors on the console. That's ok, because we're trying to call services without authentication. However, we don't see any CORS errors.

I want to mention an interesting behaviour here. Again, go to AppRepositoryRestConfigurer.java and comment out cors mapping in line 13 and restart your Spring Boot instance. Refresh the browser page and you will see that services are having HTTP 403 errors. No CORS errors. Everything seems fine. Now, application only works from the CORS definition in AppWebMvcConfigurer.java file. How so? If this application wasn't secure, we know that it will give out CORS errors. Is everything fine now? Is the whole application now working from the Spring MVC CORS definition?

Not really.

Again, fix the AppRepositoryRestConfigurer.java file and re-enable CORS definition for the repository. Then, restart the instance and refresh the page. You'll see nothing has changed.

Now, open app.module.ts file found under cors-angular\src\app directory. At line 19, you will see that I commented out the HttpInterceptor of the Angular. That interceptor sets 'X-Requested-With', 'XMLHttpRequest' to the Http Request Headers. Erase comment syntax and enable line 19. Normally, ng serve should hot deploy and shows changes instantly. If it does not, you can restart Angular instance.

resim

Voila! We set up a header and now CORS is broken for the repository. However, "/resource" is still working fine. If you check the CORS definitions, you will see that they both have allowedHeaders("*"). This should work for both. Why is it broken only for the repository, but not for the ResourceController? You can put any definitions or you can declare any annotations, it won't work for the repository CORS definition.

Comment out, line 19 in app.module.ts file and you will see that CORS error for the repository will disappear. Re-enable it and you'll see the CORS error again.

It looks like Spring Data REST CORS definitions are not working after enabling the Spring Security, but only MVC definitions work.


Solution

This is not a blocker issue. I found the solution by registering my own CORS filter. You can find a commented out corsFilter() method in AppWebSecurityConfigurerAdapter.java file. Enable it, and you will see that CORS error will disappear even with the additional Http Request Headers. However, it looks like this is the only solution.

Probably, default CORS filter registered by Spring Security is only taking care of MVC CORS definitions. It's not taking care of the Spring Data REST CORS definitions. I believe that's because they both register CORS definitions separately. In my opinion, "@crossorigin" annotations should work in the entire application, for MVC and the REST repositories.

Thanks.

Best regards,
Taha.

@wilkinsona
Copy link
Member

Thanks for the detailed problem report. Spring Boot itself isn't really involved with Spring Data REST's handling of CORS so I suspect that this may be a Spring Data REST issue. The problem is quite similar to #1732, which could also be worked around by manually declaring a CorsFilter bean. I think we need to let the Spring Data team conclude their investigation into the issues with their CORS support so I'm going to close this issue in favour of #1732. I've subscribed to the Data REST issue and if it reveals that some changes in Spring Boot are required then we can re-open this issue and take another look.

@odrotbohm
Copy link
Member

@taha-sk – Any chance you can come up with a test case in the original server application that issues a failing request? There's quite a few moving parts here and it feels a bit hard to distill what's really going on or wrong.

@taha-sk
Copy link
Author

taha-sk commented May 7, 2021

@odrotbohm Ok, I'll see to that. I'll let you know when I commit the case.

@taha-sk
Copy link
Author

taha-sk commented May 7, 2021

@odrotbohm Hello. I set up the server and produced the error. Now you can see it from my git repo. Spring MVC and Data REST have same CORS settings, but request on repository fails for CORS. You can find the test case in CorsTests.java file. Thanks.

@odrotbohm
Copy link
Member

Lovely, thanks. I'll have a look! 🙇

@odrotbohm
Copy link
Member

The failing tests runs into Spring Security rejecting the request due to no authentication being provided with the request. It looks like you actually actively set up the security configuration to behave that way (AppWebSecurityConfigurerAdapter.configure(…)). I.e. isn't what you see expected behavior and the question actually is, why the request hitting a Spring MVC endpoint does not get rejected?

@taha-sk
Copy link
Author

taha-sk commented May 7, 2021

Test cases are the options preflight requests for CORS, for those methods. I'm not trying to authenticate. Also, in my environment I get the following output for the failing request:

MockHttpServletRequest:
HTTP Method = OPTIONS
Request URI = /widgets
Parameters = {}
Headers = [Access-Control-Request-Method:"GET", Access-Control-Request-Headers:"x-requested-with", Origin:"http://localhost:4200"]
Body = null
Session Attrs = {}

Handler:
Type = null

Async:
Async started = false
Async result = null

Resolved Exception:
Type = null

ModelAndView:
View name = null
View = null
Model = null

FlashMap:
Attributes = null

MockHttpServletResponse:
Status = 403
Error message = null
Headers = [Vary:"Origin", "Access-Control-Request-Method", "Access-Control-Request-Headers", X-Content-Type-Options:"nosniff", X-XSS-Protection:"1; mode=block", Cache-Control:"no-cache, no-store, max-age=0, must-revalidate", Pragma:"no-cache", Expires:"0", X-Frame-Options:"DENY"]
Content type = null
Body = Invalid CORS request
Forwarded URL = null
Redirected URL = null
Cookies = []

In body it says it's invalid CORS.
Spring MVC is not failing because CORS setting is ok and working as expected.

I'm expecting to see a HTTP 403 for authentication only when I send GET on /widgets not on OPTIONS request.

@odrotbohm
Copy link
Member

Okay, i.e. Spring Security kicks in when it shouldn't. I can also confirm that the test case just works fine when security is removed from the project. I'll take this forward to the Spring Security team.

@taha-sk
Copy link
Author

taha-sk commented May 7, 2021

Oh yes, if you remove Spring Security, then CORS settings will work correctly. Thanks for your help.

@odrotbohm
Copy link
Member

There's been a collaborative analysis with the Security and MVC team. The problem is caused by a combination of spring-projects/spring-framework#26814 and Spring Data REST's DelegatingHandlerMapping not implementing MatchableHandlerMapping correctly. The fix is relatively straight forward. Moving this ticket to Spring Data REST.

@odrotbohm odrotbohm reopened this May 7, 2021
@odrotbohm odrotbohm transferred this issue from spring-projects/spring-boot May 7, 2021
@odrotbohm odrotbohm changed the title Enabling Spring Security with additional HTTP Request headers breaks CORS configuration of Spring Data Rest in Spring Boot DelegatingHandlerMapping does not implement MatchableHandlerMapping correctly May 7, 2021
@odrotbohm odrotbohm self-assigned this May 7, 2021
@odrotbohm odrotbohm added the type: bug A general bug label May 7, 2021
@odrotbohm odrotbohm added this to the 3.4.9 (2020.0.9) milestone May 7, 2021
odrotbohm added a commit that referenced this issue May 7, 2021
…correctly.

We now also implement MatchableHandlerMapping.getPatternParser() to expose the PathPatternParser we use. This is needed to detect that DelegatingHandlerMapping uses a PathPatternParser from the outside as the Spring MVC request processing pipeline treats those HandlerMappings in a special way. This is especially important in the context of spring-projects/spring-framework#26814 as that makes the HM implementations work properly in CORS preflight requests.

When used with Spring Security in place, CORS preflight requests are inspected through the HandlerMappingIntrospector, that looks up the CORS configuration for the handler to serve the actual request. That lookup previously failed as Spring MVC was unable to detect that Spring Data REST's DelegatingHandlerMapping works with a PathPatternParser and didn't properly set up the processing pipeline to eventually end up in an invalid state, produce an exception that caused the handler method lookup to fail and cause Spring Security to fail with a 403, claiming the request was invalid.

Fixes #2007.
@odrotbohm
Copy link
Member

You should see this resolved by adding the Spring snapshot repositories to your sample project and overriding Framework and Spring Data versions to the following:

<spring-framework.version>5.3.7-SNAPSHOT</spring-framework.version>
<spring-data-bom.version>2020.0.9-SNAPSHOT</spring-data-bom.version>

Releases scheduled for mid to end of next week.

@taha-sk
Copy link
Author

taha-sk commented May 8, 2021

Hi. This is great. I checked the snapshots on my local and everything seems fine. Even annotations are working correctly. Thank you very much. Cheers! :)

odrotbohm added a commit that referenced this issue Aug 19, 2021
…correctly.

We now also implement MatchableHandlerMapping.getPatternParser() to expose the PathPatternParser we use. This is needed to detect that DelegatingHandlerMapping uses a PathPatternParser from the outside as the Spring MVC request processing pipeline treats those HandlerMappings in a special way. This is especially important in the context of spring-projects/spring-framework#26814 as that makes the HM implementations work properly in CORS preflight requests.

When used with Spring Security in place, CORS preflight requests are inspected through the HandlerMappingIntrospector, that looks up the CORS configuration for the handler to serve the actual request. That lookup previously failed as Spring MVC was unable to detect that Spring Data REST's DelegatingHandlerMapping works with a PathPatternParser and didn't properly set up the processing pipeline to eventually end up in an invalid state, produce an exception that caused the handler method lookup to fail and cause Spring Security to fail with a 403, claiming the request was invalid.

Fixes #2007, #2054.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants