Skip to content

Fail when several filter chains have the same securityMatcher #15982

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
franticticktick opened this issue Oct 24, 2024 · 7 comments · Fixed by #16186 · May be fixed by GuusArts/spring-security#11 or GuusArts/spring-security#22
Closed
Labels
in: web An issue in web modules (web, webmvc) type: enhancement A general enhancement

Comments

@franticticktick
Copy link
Contributor

Related #15220

@Bean 
@Order(0)
SecurityFilterChain app(HttpSecurity http) throws Exception {
    http
        .securityMatcher("/app/**")
        .authorizeHttpRequests(...)
        .formLogin(...)

    return http.build();
}

@Bean 
@Order(1)
SecurityFilterChain api(HttpSecurity http) throws Exception {
    http
    	.securityMatcher("/app/**")
        .authorizeHttpRequests(...)
        .httpBasic(...)

    return http.build();
}

Is it correct to allow filter chains with the same matcher to be created? As far as I understand, this is the same case.

@franticticktick franticticktick added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Oct 24, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Oct 25, 2024

Interestingly, DefaultFilterChainValidator already contains this check. I think it could be valuable to change the way WebSecurity works to use this filter chain validator. I'm not sure why it was originally excluded, so it may not work; however, I think it's worth considering.

@jzheaux jzheaux added this to the 6.5.x milestone Oct 25, 2024
@jzheaux jzheaux added in: web An issue in web modules (web, webmvc) and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 25, 2024
@franticticktick
Copy link
Contributor Author

FilterChainProxy does not validate itself after the bean is created, because the filterChainValidator is NullFilterChainValidator, not DefaultFilterChainValidator.

private FilterChainValidator filterChainValidator = new NullFilterChainValidator();

It seems that DefaultFilterChainValidator it is not used anywhere.

@jzheaux
Copy link
Contributor

jzheaux commented Oct 28, 2024

It's used by the XML support. I'm not sure why it's not used by the Java support. I think it would be reasonable to try using it to close this ticket.

@franticticktick
Copy link
Contributor Author

DefaultFilterChainValidator is implemented in the config module, and FilterChainProxy is implemented in the web module, perhaps this is the reason. I can make a private copy of the DefaultFilterChainValidator in the web module, but ideally I would want to remove the duplication.

@jzheaux
Copy link
Contributor

jzheaux commented Nov 25, 2024

I don't think that will be needed since where the validator is set is in a config class (WebSecurity, I believe). IOW, we should be able to do:

filterChainProxy.setFilterChainValidator(new DefaultFilterChainValidator())

DefaultFilterChainValidator would likely need to be updated with the error message enhancements already added in WebSecurity; otherwise I imagine it may be a drop-in replacement.

@franticticktick
Copy link
Contributor Author

The main problem here is DefaultFilterChainValidator, checkForDuplicateMatchers method is not working. The chain.getRequestMatcher().equals(((DefaultSecurityFilterChain) test).getRequestMatcher()) test will never give true, because equals operation is not defined. I see one way out - this is to override equals for all RequestMatchers.

franticticktick pushed a commit to franticticktick/spring-security that referenced this issue Nov 28, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Nov 28, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Nov 28, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Nov 28, 2024
@franticticktick
Copy link
Contributor Author

I made the necessary changes, @jzheaux could you please review PR?

jzheaux added a commit that referenced this issue Dec 14, 2024
This will make way for other adding other checks

Issue gh-15982
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 16, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 16, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 16, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 16, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 16, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 16, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 16, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 17, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 17, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 17, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 17, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 17, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 17, 2024
franticticktick added a commit to franticticktick/spring-security that referenced this issue Dec 17, 2024
jzheaux pushed a commit to franticticktick/spring-security that referenced this issue Dec 19, 2024
jzheaux pushed a commit to franticticktick/spring-security that referenced this issue Dec 19, 2024
jzheaux added a commit to franticticktick/spring-security that referenced this issue Dec 19, 2024
jzheaux pushed a commit that referenced this issue Dec 19, 2024
jzheaux pushed a commit that referenced this issue Dec 19, 2024
jzheaux added a commit that referenced this issue Dec 19, 2024
@rwinch rwinch removed this from the 6.5.x milestone Jan 18, 2025
kwondh5217 pushed a commit to kwondh5217/spring-security that referenced this issue Feb 4, 2025
kwondh5217 pushed a commit to kwondh5217/spring-security that referenced this issue Feb 4, 2025
kwondh5217 pushed a commit to kwondh5217/spring-security that referenced this issue Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment