Skip to content
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

Multiple .requestMatchers().mvcMatchers() override previous one #10956

Closed
vova-yatsyk-theraven opened this issue Mar 10, 2022 · 1 comment
Closed
Assignees
Labels
in: config An issue in spring-security-config status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@vova-yatsyk-theraven
Copy link

vova-yatsyk-theraven commented Mar 10, 2022

Describe the bug
Defining multiple .requestMatchers().mvcMatchers() are overriding previous one.

http
        .requestMatchers()
            .mvcMatchers("/api-1")
            .mvcMatchers("/api-2")
            .mvcMatchers("/api-3")
        .and()

In the example above matcher for "/api-3" will override the one for "/api-1", and result matcher list will contain only two latest matchers: "/api-2" and "/api-3".

Expected behavior
All matches should be used together, joined by OrRequestMatcher.

Possible issue
MvcMatchersRequestMatcherConfigurer that returned after .mvcMatchers() contains only the last pattern, but it should collect all pattern combined together.
I think the line https://github.com/spring-projects/spring-security/blob/main/config/src/main/java/org/springframework/security/config/annotation/web/builders/HttpSecurity.java#L3119 from the following code:

@Override
public MvcMatchersRequestMatcherConfigurer mvcMatchers(HttpMethod method, String... mvcPatterns) {
    List<MvcRequestMatcher> mvcMatchers = createMvcMatchers(method, mvcPatterns);
    setMatchers(mvcMatchers);
    return new MvcMatchersRequestMatcherConfigurer(getContext(), mvcMatchers);
}

should be changed to return all matches: this.matchers, like below:

@Override
public MvcMatchersRequestMatcherConfigurer mvcMatchers(HttpMethod method, String... mvcPatterns) {
    List<MvcRequestMatcher> mvcMatchers = createMvcMatchers(method, mvcPatterns);
    setMatchers(mvcMatchers);
    return new MvcMatchersRequestMatcherConfigurer(getContext(), this.matchers);
}

Version
Reproduced on v5.3.4.
But main and the latest v5.6.2 contains the same code.

@vova-yatsyk-theraven vova-yatsyk-theraven added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels Mar 10, 2022
@marcusdacoregio marcusdacoregio added in: web An issue in web modules (web, webmvc) status: waiting-for-triage An issue we've not yet triaged and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 11, 2022
@marcusdacoregio marcusdacoregio self-assigned this Mar 11, 2022
@marcusdacoregio marcusdacoregio removed the status: waiting-for-triage An issue we've not yet triaged label Mar 11, 2022
@marcusdacoregio marcusdacoregio added this to the 5.7.0-M3 milestone Mar 11, 2022
@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Mar 11, 2022

Thanks for the report @vova-yatsyk-theraven, I was able to reproduce the bug here. This is now scheduled for the next patch release and should be backported to 5.5.x and 5.6.x.

As a workaround, you can specify it like this:

http
        .requestMatchers()
            .mvcMatchers("/api-1", "/api-2", "/api-3")

@marcusdacoregio marcusdacoregio added in: config An issue in spring-security-config and removed in: web An issue in web modules (web, webmvc) labels Mar 18, 2022
@marcusdacoregio marcusdacoregio modified the milestones: 5.7.0-M3, 5.7.x, 5.7.0-RC1 Mar 18, 2022
marcusdacoregio added a commit to marcusdacoregio/spring-security that referenced this issue Mar 24, 2022
@rwinch rwinch modified the milestones: 5.7.0-RC1, 5.8.x Apr 18, 2022
@marcusdacoregio marcusdacoregio modified the milestones: 5.8.x, 5.7.0 May 6, 2022
marcusdacoregio added a commit that referenced this issue May 6, 2022
marcusdacoregio added a commit that referenced this issue May 6, 2022
@marcusdacoregio marcusdacoregio modified the milestones: 5.7.0, 5.8.0-M1 May 6, 2022
marcusdacoregio added a commit that referenced this issue May 6, 2022
marcusdacoregio added a commit that referenced this issue May 6, 2022
@marcusdacoregio marcusdacoregio added the status: backported An issue that has been backported to maintenance branches label May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: config An issue in spring-security-config status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants