Skip to content

Consider XML and Java support to simplify migration to filter-based default deny #11967

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
Tracked by #10945
jzheaux opened this issue Oct 6, 2022 · 3 comments
Closed
Tracked by #10945
Assignees
Labels
for: team-attention This ticket should be discussed as a team before proceeding in: config An issue in spring-security-config

Comments

@jzheaux
Copy link
Contributor

jzheaux commented Oct 6, 2022

Given #11958 and #11963, an application can change from default deny in Spring Security 6 by modifying their URL registry, post-processing the RequestMatcherDelegatingAuthorizationManager, or publishing one of their own.

However, these approaches may be too cumbersome for folks, especially in the midst of upgrading from 5 to 6. Consider the following program:

<http>
    <intercept-url pattern="/url/one/**" access="hasRole('ONE')"/>
    <intercept-url pattern="/url/two/**" access="hasAuthority('AUTHORITY_TWO')"/>
    <intercept-url pattern="/url/three/**" access="authenticated"/>
</http>

In Spring Security 5, the default for unmatched endpoints is to abstain (which has the effect of permitting the request), and the default in Spring Security 6 is to deny.

For an application to revert to the Spring Security 5 behavior, they can first do:

<http use-authorization-manager="false">
    <intercept-url pattern="/url/one/**" access="hasRole('ONE')"/>
    <intercept-url pattern="/url/two/**" access="hasAuthority('AUTHORITY_TWO')"/>
    <intercept-url pattern="/url/three/**" access="authenticated"/>
</http>

Or, second, they can permit all at the end of their definition:

<http>
    <intercept-url pattern="/url/one/**" access="hasRole('ONE')"/>
    <intercept-url pattern="/url/two/**" access="hasAuthority('AUTHORITY_TWO')"/>
    <intercept-url pattern="/url/three/**" access="authenticated"/>
    <intercept-url pattern="/**" access="permitAll"/>
</http>

(If they aren't using expressions, the point is moot since use-expressions="false" does not use AuthorizationManager. If that fact changes, the point still stands as an app can also do <intercept-url pattern="/**" access="IS_AUTHENTICATED_ANONYMOUSLY"/>)

If needed, the application can introduce a BeanPostProcessor to post-process the RequestMatcherDelegatingAuthorizationManager instance, or it can construct one of its own and replace the XML intercept-url elements with an authorization-manager-ref attribute in <http>.

Though the options are plentiful, it needs to be decided if they are sufficient. For example, one way that may be simpler is to introduce an XML attribute like so:

<http default-authorization-decision="abstain">
    <intercept-url pattern="/url/one/**" access="hasRole('ONE')"/>
    <intercept-url pattern="/url/two/**" access="hasAuthority('AUTHORITY_TWO')"/>
    <intercept-url pattern="/url/three/**" access="authenticated"/>
</http>

This attribute would imply use-authorization-manager="true". If use-authorization-manager="true", then default-authorization-decision would default to deny. authorization-manager-ref would supersede it.

A corollary can be made for Java. Consider the same application:

@Bean 
SecurityFilterChain app(HttpSecurity http) {
    http
        .authorizeHttpRequests((authorize) -> authorize
            .mvcMatchers("/url/one/**").hasRole("ONE")
            .mvcMatchers("/url/two/**").hasAuthority("AUTHORITY_TWO")
            .mvcMatchers("/url/three/**).authenticated()
        );

    return http.build();
}

In Spring Security 5, the default for unmatched endpoints is to abstain (which has the effect of permitting the request), and the default in Spring Security 6 is to deny.

For an application to revert to the Spring Security 5 behavior, they can first do:

A corollary can be made for Java. Consider the same application:

@Bean 
SecurityFilterChain app(HttpSecurity http) {
    http
        .authorizeRequests((authorize) -> authorize
            .mvcMatchers("/url/one/**").hasRole("ONE")
            .mvcMatchers("/url/two/**").hasAuthority("AUTHORITY_TWO")
            .mvcMatchers("/url/three/**).authenticated()
        );

    return http.build();
}

Or, second, they can permit all at the end of their definition:

@Bean 
SecurityFilterChain app(HttpSecurity http) {
    http
        .authorizeHttpRequests((authorize) -> authorize
            .mvcMatchers("/url/one/**").hasRole("ONE")
            .mvcMatchers("/url/two/**").hasAuthority("AUTHORITY_TWO")
            .mvcMatchers("/url/three/**).authenticated()
            .anyRequest().permitAll()
        );

    return http.build();
}

If needed, the application can introduce an ObjectPostProcessor to post-process the RequestMatcherDelegatingAuthorizationManager instance, or it can construct one of its own and replace the mvcMatchers call with a call to anyRequest().access(AuthorizationManager).

Though the options are plentiful, it needs to be decided if they are sufficient. For example, one way that may be simpler is to introduce a DSL property like so:

@Bean 
SecurityFilterChain app(HttpSecurity http) {
    http
        .authorizeHttpRequests((authorize) -> authorize
            .mvcMatchers("/url/one/**").hasRole("ONE")
            .mvcMatchers("/url/two/**").hasAuthority("AUTHORITY_TWO")
            .mvcMatchers("/url/three/**).authenticated()
            .defaultAuthorizationDecision(new AuthorizationDecision(true))
        );

    return http.build();
}
@jzheaux jzheaux added this to the 6.0.0-RC1 milestone Oct 6, 2022
@jzheaux
Copy link
Contributor Author

jzheaux commented Oct 6, 2022

Having described the options above, I'll throw in my two cents here.

For my taste, a catch-all <intercept-url pattern="/**" access="permitAll"/> or anyRequest().permitAll() is a good balance between simplicity for the user and low maintenance for the team. I can't say that I've thought of every scenario, but this is the way that most of the tests were changed when applying #11929. It's also nice because it aligns with our samples and demos that almost universally declare a catch-all rule.

I can see how default-authorization-decision is nice because it can be property-driven at that point.

@jzheaux jzheaux changed the title Potential RequestMatcherDelegatingAuthorizationManager simplifications Consider XML and Java support for migration to filter-based default deny Oct 6, 2022
@jzheaux jzheaux changed the title Consider XML and Java support for migration to filter-based default deny Consider XML and Java support to simplify migration to filter-based default deny Oct 6, 2022
@jgrandja
Copy link
Contributor

@jzheaux I believe this ticket is no longer valid since we decided to document the migration strategy instead. I'll leave this ticket for you to close.

For the migration guide, please document the recommended strategy:

To test Spring Security 6.0 compatibility in a Spring Security 5.8 (or earlier) application, add the authorization rule anyRequest().denyAll() or <intercept-url pattern="/**" access="denyAll"/>.
If the application performs as expected, then the application is Spring Security 6.0 compatible.
If certain requests are denied, then the user should incrementally add the specific authorization rule to resolve each of the denied requests.
After all denied requests have been resolved, remove anyRequest().denyAll() or <intercept-url pattern="/**" access="denyAll"/>.

Alternatively, if the application has upgraded to Spring Security 6.0 and certain requests are denied, and is different behaviour before the upgrade, then add the authorization rule anyRequest().permitAll() or <intercept-url pattern="/**" access="permitAll"/>. This will restore the original behaviour before the upgrade.
Now the user can incrementally add the specific authorization rule to resolve each of the denied requests.
After all denied requests have been resolved, remove anyRequest().permitAll() or <intercept-url pattern="/**" access="permitAll"/>.

@jgrandja jgrandja assigned jzheaux and unassigned jgrandja Oct 13, 2022
@jzheaux jzheaux added in: config An issue in spring-security-config for: team-attention This ticket should be discussed as a team before proceeding labels Oct 13, 2022
@jzheaux jzheaux removed this from the 6.0.0-RC1 milestone Oct 13, 2022
@jzheaux jzheaux closed this as completed Oct 13, 2022
@jzheaux
Copy link
Contributor Author

jzheaux commented Oct 13, 2022

Closed the ticket as team-attention. The purpose of the ticket was to bring together ideas that had been shared by each team member into one location, which it achieved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention This ticket should be discussed as a team before proceeding in: config An issue in spring-security-config
Projects
None yet
Development

No branches or pull requests

2 participants