spring-projects / spring-security

Spring Security
http://spring.io/projects/spring-security
Apache License 2.0
8.87k stars 5.92k forks source link

Consider warning users if securityMatchers do not match some filter in the chain #14096

Open Haarolean opened 1 year ago

Haarolean commented 1 year ago

Describe the bug HttpSecurity configuration with securityMatcher and oauth2Login(withDefaults()) leads to 404 for some OAuth2 endpoints.

To Reproduce

  1. Clone this repo: https://github.com/Haarolean/spring-security-matchers-bug
  2. Run the app
  3. Go to http://localhost:8080/oauth2/authorization/github
  4. Observe 404

Also,

  1. Commenting line 25 in OAuthSecurityConfig fixes the issue.
  2. Endpoint like http://localhost:1337/login/oauth2/code/github?code=xxx still works for some reason.
  3. I've traced the issue down to MvcRequestMatcher, where notMatchMethodOrServletPath always results in true. Without line 25, MvcRequestMatcher is not used, rather AnyRequestMatcher is being used.
  4. I've dug through all the possible documentation on securityMatcher and didn't find anything which could explain the problem.

Expected behavior oauth, csrf, cors and other configurations are applied only for /api/web/**.

Sample

https://github.com/Haarolean/spring-security-matchers-bug

marcusdacoregio commented 1 year ago

Hi, @Haarolean.

Your configuration is doing:

https://github.com/Haarolean/spring-security-matchers-bug/blob/a57517a3e18ce97ac4be39a6f3b7c01ace00a0c3/src/main/java/com/example/demo/OAuthSecurityConfig.java#L25

Which tells Spring Security to run the SecurityFilterChain only for requests that match /api/web/**, /oauth/**, /login/**. The 3 step to reproduce is to go to http://localhost:8080/oauth2/authorization/github which is not mapped in your securityMatcher rules (there is no /oauth2/**) there. Am I missing something?

Haarolean commented 1 year ago

@marcusdacoregio hi and thanks for a swift reply!

You're absoultely right, my mistake in a missing 1 at the end of the mapping path.

This changes the subject, however, shouldn't this be handled by spring-security automatically? Like, if you're not experience with one, or, say, you use different components, that might be impossible to know all the paths spring-security uses in your app, and if you don't define them here you'll get 404, which, again, might be frustrating.

So, I believe, possibly, things like default oauth configurers, should take this into consideration and append such a path into default built-in URLs?

Currently, alternatively, you might have to specify paths for all the components manually, like csrf, cors and oauth, which would require doing this three times (and all of them might require different matcher classes!).

Let me know what you think!

marcusdacoregio commented 1 year ago

This changes the subject, however, shouldn't this be handled by spring-security automatically? Like, if you're not experience with one, or, say, you use different components, that might be impossible to know all the paths spring-security uses in your app, and if you don't define them here you'll get 404, which, again, might be frustrating.

I agree that it can be frustrating, but all the URLs used by the authentication mechanisms are described in the documentation. We could maybe warn the user about it when the securityMatchers do not match any of the configured filters, but to do that we would need to be aware of all the URLs configured and perform a verification on each of them. I am not so sure what it would look like.

If you agree, I'll repurpose this ticket to consider that and open it for contribution. If you are willing to investigate how to do that, you are welcome to take the ticket.