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

ErrorPageSecurityFilter does not honor use of multiple WebSecurityConfigurerAdapter's #28818

Closed
fast-reflexes opened this issue Nov 26, 2021 · 1 comment
Labels
for: external-project For an external project and not something we can fix status: duplicate A duplicate of another issue

Comments

@fast-reflexes
Copy link

fast-reflexes commented Nov 26, 2021

Background

ErrorPageSecurityFilter was recently added as a step to solve obscure cases where the /error path was allowed in some cases where it shouldn't be allowed. The ErrorPageSecurityFilter is installed as a filter outside Spring Security's filter chain. The purpose of the filter is to process ERROR dispatches and to determine whether the /error path is allowed given the current authentication. If it is, it should return the intended error page, otherwise, it should only return the status but no content.

This problem was raised in:

The filter was added to Spring Boot 2.6.0 in:

Previous problems

1. Inheriting from HttpFilter instead of Filter

The ErrorPageSecurityFilter inherits from HttpFilter instead of Filter which causes ClassDefNotFound exception in some cases:

2. MockMvc fails to exclude the filter for non-ERROR dispatches

This problem was raised and discussed in:

The problem is that Spring ApplicationFilterChain takes into account whether the current request is an ERROR dispatch or not, and adapts the filter chain to use accordingly, effectively excluding the new ErrorPageSecurityFilter for non-ERROR dispatches. The MockFilterChain used by MockMvc ignores dispatch type and so includes the ErrorPageSecurityFilter in all dispatches which may cause problems, especially in connection to the bug reported in this issue.

A ticket to fix this was added here:

A temporary fix in which the filter itself checks whether the current dispatch is an ERROR or not was devised for release 2.6.1 here:

Bug

Previous problem 2 is also related to the bug reported in this issue and the fact that both reporters have problems with multiple WebSecurityConfigurerAdapters is indicative of this. The problem is that the ErrorPageSecurityFilter relies on bean WebInvocationPrivilegeEvaluator, normally instantiated as a DefaultWebInvocationPrivilegeEvaluator which only takes the last processed WebSecurityConfigurerAdapter into account. This results in many times in that ErrorPageSecurityFilter makes the wrong decision about the access rights to the /error path. This happens typically with a configuration like:

@EnableWebSecurity(debug = true)
@Order(0)
class Config: WebSecurityConfigurerAdapter() {

    override fun configure(http: HttpSecurity) {
        http
            .requestMatchers()
            .antMatchers("/error/**")
            .and()
            .authorizeRequests()
            .anyRequest().permitAll()
    }
}

@EnableWebSecurity(debug = true)
@Order(1)
class CatchAllConfig: WebSecurityConfigurerAdapter() {

    override fun configure(http: HttpSecurity) {
        http
            .authorizeRequests()
            .anyRequest().denyAll()
    }

}

... where the ErrorPageSecurityFilter would always deny access to the /error path despite a security config which grants access to the /error path to anyone.

Since this is ultimately a problem in Spring Security a ticket about this has been filed there:

This ticket is only for reference and should be closed at the same time as the above ticket is closed. For more information, check that ticket.

@wilkinsona
Copy link
Member

Thanks for the detailed write-up. There's no need to create duplicate issues across two projects: the Spring Security issue is sufficient to track this.

@wilkinsona wilkinsona added for: external-project For an external project and not something we can fix status: duplicate A duplicate of another issue and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix status: duplicate A duplicate of another issue
Projects
None yet
Development

No branches or pull requests

3 participants