Skip to content

Add 'securityMatcher' as an alias of 'requestMatcher' #9159

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
akuma8 opened this issue Oct 28, 2020 · 7 comments
Closed

Add 'securityMatcher' as an alias of 'requestMatcher' #9159

akuma8 opened this issue Oct 28, 2020 · 7 comments
Assignees
Labels
in: config An issue in spring-security-config type: enhancement A general enhancement
Milestone

Comments

@akuma8
Copy link
Contributor

akuma8 commented Oct 28, 2020

When configuring security rules, it will be very helpful to understand what we are configuring.
Example with this kind of configuration :

@Configuration
public class UIResourceProtection extends WebSecurityConfigurerAdapter {

    @Override
    public void configure( HttpSecurity http ) throws Exception {

        http.requestMatchers().antMatchers("/product/**") // be more explicit here 
                .and()
                .authorizeRequests()
                .antMatchers(HttpMethod.PATCH,"/product/update")
                .hasRole("user")
                .antMatchers("/product/private/**").hasRole("super_user")
                .and()
                .oauth2ResourceServer()
                .jwt().jwtAuthenticationConverter( ... ) ;
    }
}

To help beginners to understand what they are doing, it will be helpful to name explicitly what we configure:

@Configuration
public class UIResourceProtection extends WebSecurityConfigurerAdapter {

    @Override
    public void configure( HttpSecurity http ) throws Exception {

        http.filterChainMatchers().antMatchers("/product/**") // with this "filterChainMatchers()" alias we know that we are defining the filter chain selector and its more explicit.
                .and()
                .authorizeRequests()
                .antMatchers(HttpMethod.PATCH,"/product/update")
                .hasRole("user")
                .antMatchers("/product/private/**").hasRole("super_user")
                .and()
                .oauth2ResourceServer()
                .jwt().jwtAuthenticationConverter( ... ) ;
    }
}

Spring Security is not easy to understand, naming things in a clear way could only simplify the life of users, especially beginners.

@akuma8 akuma8 added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Oct 28, 2020
@rwinch
Copy link
Member

rwinch commented Oct 28, 2020

Are you speaking about making it more explicit between the matchers on HttpSecurity and the authorization rules?

@jzheaux jzheaux added in: config An issue in spring-security-config and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 28, 2020
@akuma8
Copy link
Contributor Author

akuma8 commented Nov 2, 2020

Are you speaking about making it more explicit between the matchers on HttpSecurity and the authorization rules?

Yes, in order to know exactly what we are configuring. Filter chains are the core of authorization rules and currently we do not explicitly know how one is declaratively selected by the framework. By declaratively I mean, by reading the security configuration code, the filter chain matcher should be obvious to us.

@rwinch
Copy link
Member

rwinch commented Nov 2, 2020

I think that makes sense and have thought about it a bit. I'd prefer not exposing implementation details like filterChainMatchers in the DSL. I think something like this would be what I would suggest:

http
    .httpSecurityMatcher(matcher)
    ...

Thoughts?

@akuma8
Copy link
Contributor Author

akuma8 commented Nov 13, 2020

I'd prefer not exposing implementation details like filterChainMatchers in the DSL

Is there an abstraction of filter chain? If yes, I thought you are right. In my mind filter chain was a specific Spring Security concept. I know that when chaining multiples servlet filters we can call that filters chain but I think in Spring Security it is a strong concept that is not really highlighted in the configuration.

@rwinch
Copy link
Member

rwinch commented Dec 3, 2020

I think my concern is that the DLS tries to hide the implementation details (i.e that FilterChainProxy is used under the covers). Instead, I think it makes more sense to relate the method to the configuration object HttpSecurity in this case. In any case, would you be interested in submitting a PR?

@rwinch rwinch removed their assignment Jun 9, 2022
@rwinch rwinch added this to the 6.0.x milestone Jun 9, 2022
@marcusdacoregio marcusdacoregio self-assigned this Jul 27, 2022
@marcusdacoregio
Copy link
Contributor

Related to #11347. If we deprecate/remove mvcMatchers, antMatchers, etc. and add requestMatcher variations, it can become confusing for folks that are using the Customizer.

With the deprecations/removals, instead of doing

http.antMatcher("/path/**")

We should do:

http.requestMatcher("/path/**")

This is fine, but it becomes confusing when the Customizer is used, the requestMatcher is repeated multiple times:

http.requestMatchers(customizer -> customizer
    .requestMatcher("/path1/**")
    .requestMatcher("/path2/**")
    .requestMatcher(new AntPathRequestMatcher("..."))
)

This is another point to consider when adding this feature.

@marcusdacoregio
Copy link
Contributor

marcusdacoregio commented Sep 22, 2022

Maybe the name of the method could be securityMatcher to better align with the Kotlin DSL and Reactive:

http
    .securityMatcher(matcher)
    ...

@marcusdacoregio marcusdacoregio changed the title Add 'filterChainMatcher' as an alias of 'requestMatcher' Add 'securityMatcher' as an alias of 'requestMatcher' Oct 3, 2022
marcusdacoregio added a commit to marcusdacoregio/spring-security that referenced this issue Oct 3, 2022
If Spring MVC is present in the classpath, use MvcRequestMatcher by default. This commit also adds a new securityMatcher method in HttpSecurity

Closes spring-projectsgh-11347
Closes spring-projectsgh-9159
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 type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants