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

Using WebSecurity.ignoring results in 401 status code when rendering error page #33341

Closed
rupert-madden-abbott opened this issue Nov 24, 2022 · 8 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@rupert-madden-abbott
Copy link
Contributor

rupert-madden-abbott commented Nov 24, 2022

  • Spring Boot Version: 2.7.5
  • Dependencies: spring-boot-starter-web, spring-boot-starter-security

If an error is thrown when handling a request to an endpoint that does not have the Spring Security filter chain applied to it (via WebSecurity.ignoring), then an additional error is encountered when rendering the built in error page and a 401 status code is returned, rather than the original status code e.g. a 500.

When using the default Tomcat server, the status code is altered but no additional error is logged. However, when using undertow, the following error is also logged:

2022-11-24 10:24:28.722 ERROR 1331713 --- [  XNIO-1 task-1] io.undertow.servlet.request              : UT015012: Failed to generate error page /error for original exception: java.lang.RuntimeException. Generating error page resulted in a 401.

I have upload a sample application that reproduces this issue here.

If you run these tests, you will note that shouldReturnInternalServerErrorWhenUnauthenticatedAccessToUnsecuredErrorEndpoint fails.

There is an interesting aside. If I use authorizeHttpRequests instead of authorizeRequests, then the error page can be successfully rendered. Here are some tests using that configuration and you will note that the equivalent test succeeds.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 24, 2022
@rupert-madden-abbott
Copy link
Contributor Author

rupert-madden-abbott commented Nov 24, 2022

Ah this was also reported a year ago. Sorry! Closing as duplicate of #28768

@rupert-madden-abbott
Copy link
Contributor Author

Actually this is not a duplicate of #28768. The linked fix in that case is when multiple security chains are involved. In my case, I have a single chain, but it is just not being applied to some endpoints.

The problem is that, for me, sometimes /error is going to come from a secure path and sometimes it is not. It appears that the default with authorizeRequests is for it to be applied to ERROR dispatchs, causing the FilterSecurityInterceptor to fire and cause a 401, in the case that we have got there from an unsecure path. I get the same behaviour from authorizeHttpRequests if I set shouldFilterAllDispatcherTypes(true).

So basically, it doesn't look like it is possible to have a "secured" and an "unsecured" error page. I can either remove security from my error page (or for ERROR dispatches) such that it doesn't apply even when I'm routing from a secured page, or I can include the Spring Security filter chain on all requests and use permitAll instead of ignoring. Neither of those seem like great options as I'm either having to compromise on security or performance.

@rupert-madden-abbott
Copy link
Contributor Author

rupert-madden-abbott commented Nov 24, 2022

Actually (again):

  • permitAll instead of ignoring does work fine with authorizeRequests i.e. the error page is rendered with a 500 status
  • permitAll instead of ignoring does NOT work fine with authorizeHttpRequests and shouldFilterAllDispatcherTypes(true) i.e. I get a 401 instead of the error page and original status code

The reason is that FilterSecurityInterceptor doesn't fire a second time on /error when using permitAll because it recognises that it has already fired, and it has some custom logic to ensure it is not fired more than once per request.

AuthorizationFilter, on the other hand, is a OncePerRequestFilter and OncePerRequestFilters still fire multiple times for ERROR dispatches (by default).

So for now I could use permitAll instead of ignoring (if I were happy to accept the overhead) but that will then stop working if I migrate from authorizeRequests to authorizeHttpRequests (if I still want security for my secured ERROR dispatches).

@rupert-madden-abbott
Copy link
Contributor Author

rupert-madden-abbott commented Nov 24, 2022

As far as I can see, the only reason (?) to want to have the security chain fire for ERROR dispatches is because we want some of the information added by Spring Security to be available to the ErrorController, either for logging or rendering purposes.

In my particular case, I don't need this information so I think the best workaround for me is to explicitly disable the security chain for all ERROR dispatches like this:

    @Bean
    public WebSecurityCustomizer webSecurityCustomizer() {
            return web -> web.ignoring()
                    .dispatcherTypeMatchers(DispatcherType.ERROR)
                    .mvcMatchers("/foo");
        }
    }

@mbhave
Copy link
Contributor

mbhave commented Feb 23, 2023

@rupert-madden-abbott Thanks for your patience. Your analysis here is spot on. Few things to note:

While ignoring the security filter chain for error dispatches works in that it shows you the error code, it allows anyone to view the details on the error page (even when the original request was unauthorized). This is something to be aware about before opting for web.ignoring or permitAll().

The error page security behavior has changed with Spring Boot 3.0. Spring Boot 2.7 had an ErrorPageSecurityFilter to support the use case you mentioned, "secure" and an "insecure" error page depending on which request had caused the error. This filter was removed in Spring Boot 3.0 in order to align with the defaults that Spring Security put in place. So by default, the security filter will always be applied to the error dispatch for both authorizeRequests and authorizeHttpRequests. This does mean that the original response code won't be available unless /error is explicitly configured to use permitAll or web.ignoring.

This issue also reports a similar problem. I think this is something that, if at all, needs to be fixed in Spring Security. I've created an issue for it here.

@philwebb
Copy link
Member

spring-projects/spring-security#12771 has been declined by the Spring Security team, but we're not sure that we want to start deviating from their recommendations. @rupert-madden-abbott I'm going to close this one here, but feel free to add additional comments to spring-projects/spring-security#12771 if you like.

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Jul 19, 2023
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jul 19, 2023
@vemahendran
Copy link

vemahendran commented Apr 24, 2024

Any one who is still stuck with this issue can please refer my answer in another post. I could make it working by just permiting the URL /error to see the error.

@wilkinsona
Copy link
Member

wilkinsona commented Apr 24, 2024

Thanks, @vemahendran. @mbhave has already noted this in her comment above:

This does mean that the original response code won't be available unless /error is explicitly configured to use permitAll or web.ignoring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

6 participants