Skip to content

ignore OPTIONS requests by default #9711

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
wants to merge 1 commit into from

Conversation

deki
Copy link
Contributor

@deki deki commented Jul 10, 2017

If security is enabled, preflight requests are currently answered with 401. This is a problem e.g. for Angular users and you need to fix it with a custom security config. See reports in #5834, #6566 or on Stack Overflow https://stackoverflow.com/q/21696592/3156607, https://stackoverflow.com/q/28010307/3156607, https://stackoverflow.com/q/27501045/3156607

OPTIONS requests should not require authentication: https://stackoverflow.com/a/15734032/3156607

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 10, 2017
@mbhave
Copy link
Contributor

mbhave commented Jul 10, 2017

This sounds like something that Spring Security should handle. @rwinch WDYT?

@rwinch
Copy link
Member

rwinch commented Jul 10, 2017

@deki Thank you for your report.

Spring Security provides CORS integration with Spring CORS support. The ingeration allows Preflight requests to be processed by the CorsFilter which is provided by spring-web. This means that if you have registered the CORS support with Spring Security, provide a CORS Configuration, and make a Preflight Request Spring Security will not process the request (Spring's CorsFilter does).

It is important to note that Spring requires all of the following to be true to consider the request a preflight request:

  • the method must be OPTIONS
  • the presence of the Origin header
  • the presence of the Access-Control-Allow-Methods header

There has been a lot of coordination to ensure applications remain secure. For example, consider a mapping of @RequestMapping("/message"). The previous mapping does not specify which HTTP method should be matched on, so technically an OPTIONS request could return the result of the method. Spring protects you from such scenarios, but it is still NOT recommended to disable Spring Security for OPTIONS requests as it reduces your protection. For example, it would disable the X-Content-Type-Options: nosniff for the response.

You can find an example of CORS at https://github.com/rwinch/spring-security-sample/tree/boot-cors (NOTE the boot-cors branch). You can send an preflight request using:

 curl -v -H 'Access-Control-Request-Method: GET' -H 'Origin:localhost' -X OPTIONS http://localhost:8080/
*   Trying 127.0.0.1...
* Connected to localhost (127.0.0.1) port 8080 (#0)
> OPTIONS / HTTP/1.1
> Host: localhost:8080
> User-Agent: curl/7.47.0
> Accept: */*
> Access-Control-Request-Method: GET
> Origin:localhost
> 
< HTTP/1.1 200 
< 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
< Access-Control-Allow-Origin: localhost
< Vary: Origin
< Access-Control-Allow-Methods: GET
< Access-Control-Allow-Credentials: true
< Access-Control-Max-Age: 1800
< Content-Length: 0
< Date: Mon, 10 Jul 2017 19:12:58 GMT
<

I hope this helps!

@deki
Copy link
Contributor Author

deki commented Jul 10, 2017

Rob, thanks for the detailed answer and the demo application. I don't think that this needs to be addressed in Spring Security but I expect the Spring Boot AutoConfiguration to produce a configuration that works. Currently this is not the case and many people are confused.

E.g. if I simplify the testcase and drop WebSecurityConfig (which will be provided by Boot AutoConfiguration automatically) preflight requests fail (doesn't matter if @CrossOrigin is present or not): https://github.com/deki/spring-security-sample/tree/boot-cors

Of course the root cause is a spec issue but we should make it work, see discussion on the W3 list: http://lists.w3.org/Archives/Public/public-webapps/2012JulSep/0252.html

If someone is looking for a workaround, at the moment I'm adding the following code to every application:

   @Bean
   public IgnoredRequestCustomizer optionsIgnoredRequestsCustomizer() {
      return configurer -> {
         List<RequestMatcher> matchers = new ArrayList<>();
         matchers.add(new AntPathRequestMatcher("/**", "OPTIONS"));
         configurer.requestMatchers(new OrRequestMatcher(matchers));
      };
   }

@rwinch
Copy link
Member

rwinch commented Jul 10, 2017

As I mentioned above...disabling security (your workaround) is NOT recommended as it opens your application up to potential exploits. You want to ensure that the rest of Spring Security is still processing the request and in this case it is not.

@deki
Copy link
Contributor Author

deki commented Jul 10, 2017

Understood. What can we do to improve the AutoConfiguration in Spring Boot? Enable CorsFilter by default so that it takes care of it?

@rwinch
Copy link
Member

rwinch commented Jul 10, 2017

I don't think it is worth spending a lot of time on Spring Boot's auto configuration of Security. The plan going forward is to reduce what Boot does because it isn't necessarily right.

I'd have to give it some thought, but we might be able to enable it by default with Security itself. My concern here is that it may cause early initialization issues within the configuration support. This is due to limitations within Java Configuration. If you are interested in this, please create a ticket in Spring Security.

Given this may cause issues, I'm not sure this is going to be resolved so much as looked into.

@rwinch
Copy link
Member

rwinch commented Jul 10, 2017

PS: You can try to workaround this using https://docs.spring.io/spring-security/site/docs/current/reference/htmlsingle/#jc-custom-dsls and then see the applying it by default

This might cause early initialization issues but I'm not certain w/out digging in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants