Allow preserving original error code for error dispatch

[mbhave]

Right now it isn't possible to get the original status code for an error dispatch in certain cases without opening up the path to the error page. For example, if a public page results in an error, such as a 500, the error code will be 401.

Related Issues:
spring-projects/spring-boot#33341
spring-projects/spring-boot#33934

If we could preserve the original status code, while still preventing access to the error page, it would prevent users from having to always open up the error page.

[marcusdacoregio]

Hi @mbhave, I've talked to @rwinch about this and here is some feedback:

At first, this does not seem to be specific to Spring Security since the status code would change if there is any error that happens on the error dispatch. For example, consider the following:

@ControllerAdvice
class AttributeControllerAdvice {
	@ModelAttribute("attribute")
	String attribute(@RequestParam Boolean error) {
		return "attribute";
	}
}

If a request of http://localhost:8080/missing is made it should produce an HTTP status of 404. However, when the error is being processed AttributeControllerAdvice will produce an error because the parameter named error is not present. Since the parameter is missing, the 404 will be turned into a 400 status to send. This is just one example of what could go wrong during error processing. The point is that if any error happens during error processing, then the status will be overridden.

Rather than trying to solve this in Spring Security, something like this seems to make more sense:

public class DisableStatusCodeFilter implements Filter {
	@Override
	public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException {
		if (response instanceof HttpServletResponse httpResponse) {
			HttpStatus status = getStatus(request);
			httpResponse.setStatus(status.value());
			chain.doFilter(request, new DisableHttpStatus(httpResponse));
		}
		else {
			chain.doFilter(request, response);
		}
	}

	private HttpStatus getStatus(ServletRequest request) {
		Integer statusCode = (Integer) request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
		if (statusCode == null) {
			return HttpStatus.INTERNAL_SERVER_ERROR;
		}
		try {
			return HttpStatus.valueOf(statusCode);
		}
		catch (Exception ex) {
			return HttpStatus.INTERNAL_SERVER_ERROR;
		}
	}

	static class DisableHttpStatus extends HttpServletResponseWrapper {

		public DisableHttpStatus(HttpServletResponse response) {
			super(response);
		}

		@Override
		public void sendError(int sc) throws IOException {
			System.out.println("Disabled");
		}

		@Override
		public void sendError(int sc, String msg) throws IOException {
			System.out.println("Disabled");
		}

		@Override
		public void setStatus(int sc) {
			System.out.println("Disabled");
		}
	}
}

Then it can be registered like:

@Bean
static FilterRegistrationBean<DisableStatusCodeFilter> disableStatusCode() {
	FilterRegistrationBean<DisableStatusCodeFilter> result = new FilterRegistrationBean<>(new DisableStatusCodeFilter());
	result.setOrder(Ordered.HIGHEST_PRECEDENCE);
	result.setDispatcherTypes(DispatcherType.ERROR);
	return result;
}

If Boot only wants to preserve certain status codes, then you can add conditional logic to the DisableStatusCodeFilter based on the original status and the new status.

It seems similar to the ErrorPageSecurityFilter with some differences:

• It just sets the response status code and prevents other components from modifying it instead of using response.sendError;
• It won’t stop the filter chain, therefore, allowing other filters to do their job;
• It does not use the WebInvocationPrivilegeEvaluator API which has some limitations;

Boot can also use a similar approach to capture both errors and log them both in the event that error processing fails

[wilkinsona]

Thanks for the suggestion, @marcusdacoregio. Unfortunately, we don't think it's suitable for inclusion in Boot for a few reasons.

The option to leave the original error intact should be available to all Spring Security users, not just those who are using Spring Boot.

We'd argue that a @ControllerAdvice that accidentally changes the status code is faulty and that it should be fixed. In the case of your example, that fix would be to make the request parameter optional. We feel similarly about Spring Security changing the status code. It should at least be possible to disable this behavior, even if the current behavior remains the default. We also need to consider the possibility that users are deliberately using controller advice to change the status code and we should respect that.

The Filter feels to us like a workaround for Spring Security's current behavior. We know from numerous Spring Boot issues that users want the status code to be preserved when the error page is secured and access is denied. We do not know anything about what behavior users want for other errors that occur during error dispatches. The filter will affect the behavior of any error that occurs during an error dispatch, not just those from Spring Security. Unfortunately, that makes it too broad.

We think it's also worth drawing a distinction between changes to the status code that happen during request dispatch (as we believe Spring MVC's controller advice will do) and changes to the status code that happen during an error dispatch. Our feeling is that the latter is worse and should generally be avoided unless someone's indicated that it's what they really want to happen. Spring Security will change the status code during the error dispatch. Ideally, we'd like its default behavior to not do that, perhaps with an option to reinstate the current behavior for those that need it.

In the absence of a change to Spring Security, we think that our best path forward is to direct Boot users to this issue. We can ask them to consider using the Filter-based workaround and see how they get on. To be spec-compliant, I think the response wrapper will have to be changed so that sendError actually sends an error. Once called, the method should result in the response being committed and making the call a no-op won't do that. Sending the captured status code rather than the value passed in sc would be one option.

[marcusdacoregio]

Thanks for detailing the reasons @wilkinsona.

We know from numerous Spring Boot issues that users want the status code to be preserved when the error page is secured and access is denied. We do not know anything about what behavior users want for other errors that occur during error dispatches.

I don't know if Spring Security can do something that is specifically designed around the error page, however, I can see something for the error dispatch and authentication/access denied exceptions in general.

In addition to the filter, there is another approach that feels less "aggressive" and it consists of configuring a specific AuthenticationEntryPoint and AccessDeniedHandler for the error dispatch, something like this:

public class ServletErrorAwareAuthenticationEntryPoint implements AuthenticationEntryPoint {

	@Override
	public void commence(HttpServletRequest request, HttpServletResponse response, AuthenticationException authException) throws IOException, ServletException {
		Integer statusCode = (Integer) request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
		if (statusCode != null) {
			response.sendError(statusCode);
		}
	}

}

public class ServletErrorAwareAccessDeniedHandler implements AccessDeniedHandler {

	@Override
	public void handle(HttpServletRequest request, HttpServletResponse response, AccessDeniedException accessDeniedException) throws IOException, ServletException {
		Integer statusCode = (Integer) request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
		if (statusCode != null) {
			response.sendError(statusCode);
		}
	}

}


// ...
http
        .exceptionHandling((exceptions) -> exceptions
		.defaultAuthenticationEntryPointFor(
				new ServletErrorAwareAuthenticationEntryPoint(),
				new DispatcherTypeRequestMatcher(DispatcherType.ERROR))
		.defaultAccessDeniedHandlerFor(
				new ServletErrorAwareAccessDeniedHandler(),
				new DispatcherTypeRequestMatcher(DispatcherType.ERROR))
	);

I can totally see Spring Security registering those handlers by default if the user does not specify any, but I am not clear what could be the impacts of that. I'll ping @rwinch for his feedback on our comments.

[wilkinsona]

I don't know if Spring Security can do something that is specifically designed around the error page

Where I said error page, you can really think of it as the error dispatch from Spring Security's perspective. In other words I could have written the following:

We know from numerous Spring Boot issues that users want the status code to be preserved when access is denied during an error dispatch.

[marcusdacoregio]

We'd argue that a @ControllerAdvice that accidentally changes the status code is faulty and that it should be fixed. In the case of your example, that fix would be to make the request parameter optional.

The same can be applied to the error page being secured, which could mean that there is a problem with the user's security configuration, and that could be fixed by allowing access to the error page.

Keeping the status code by default could also be more confusing for a user who is expecting the error page to be protected, in that case instead of a 401/403 they would see another status code.

Another problem is that the client usually expects a body to be returned alongside the error, for example, a 404 with an Entity not found message. In that case, the status would be there but the body not, breaking the client's logic.

If the user wants to change that behavior, they can always implement one of our suggestions in the previous comments.

All that said, we will decline this feature for now because we feel that it wouldn't make things more clear for our users.

[wilkinsona]

The same can be applied to the error page being secured, which could mean that there is a problem with the user's security configuration, and that could be fixed by allowing access to the error page.

I agree, but the security configuration could also be correct yet the user cannot get the desired status code because of Spring Security's behavior. Unlike their own app, a user cannot easily fix Spring Security's behavior. The best we have for them at the moment are somewhat flawed or somewhat complicated workarounds.

Keeping the status code by default could also be more confusing for a user who is expecting the error page to be protected, in that case instead of a 401/403 they would see another status code.

That may be a reason to keep the current default. I don't think it's a reason to do nothing. A configuration option would still be useful for those that want different behavior.

Another problem is that the client usually expects a body to be returned alongside the error, for example, a 404 with an Entity not found message. In that case, the status would be there but the body not, breaking the client's logic.

IME, many clients do not care about the content of an error response, just the status. A well-written client should also check the Content-Length of the response and handle a missing body gracefully. Furthermore, I don't think that the behavior of certain, arguably poorly written clients, is a good reason not to at least offer a configuration option. We know that we have users that want to original status code to be preserved. Why not offer something out-of-the-box that meets their needs?

If the user wants to change that behavior, they can always implement one of our suggestions in the previous comments.

This argument could be applied to lots of features – there are after all many things that users can implement themselves – but I think it's a mistake here. We know that there are users (and judging by the activity on Boot's issue tracker, there are quite a lot of them) that want the original status code to be preserved. Providing built-in support for making that easy for them feels like the right thing for us to do and Spring Security is in the ideal place to do it.

[marcusdacoregio]

Unlike their own app, a user cannot easily fix Spring Security's behavior.

1. If they want to permit the error page, just allow access to it.
2. If they want to permit the error page, but deny access to the body, for example, they can implement their own ErrorController and show the information based on the user's authorities:

@Configuration
@EnableWebSecurity
class SecurityConfig {

    @Bean
    SecurityFilterChain appSecurity(HttpSecurity http) throws Exception {
        http
            .authorizeHttpRequests((authorize) -> authorize
                .requestMatchers("/error").permitAll()
                .anyRequest().authenticated()
            );
            // ...
        return http.build();
    }

}


@Controller
public class MyErrorController extends BasicErrorController {

	private final AuthorizationManager<HttpServletRequest> authorizationManager = AuthorityAuthorizationManager.hasRole("ADMIN");

	private final SecurityContextHolderStrategy strategy = SecurityContextHolder.getContextHolderStrategy();

	public MyErrorController(ErrorAttributes errorAttributes) {
		super(errorAttributes, new ErrorProperties());
	}

	@Override
	protected ErrorAttributeOptions getErrorAttributeOptions(HttpServletRequest request, MediaType mediaType) {
		ErrorAttributeOptions options = ErrorAttributeOptions.defaults();

		Authentication authentication = this.strategy.getContext().getAuthentication();
		AuthorizationDecision decision = this.authorizationManager.check(() -> authentication, request);
		if (decision == null || !decision.isGranted()) {
			return options;
		}
		options = options.including(ErrorAttributeOptions.Include.EXCEPTION);
		options = options.including(ErrorAttributeOptions.Include.STACK_TRACE);
		options = options.including(ErrorAttributeOptions.Include.MESSAGE);
		options = options.including(ErrorAttributeOptions.Include.BINDING_ERRORS);
		return options;
	}
}

In addition to that, we can make things easier for users who want to customize Spring Security's exception handling in order to make it preserve the status code. For that, I've created the following enhancements:

• Create NoOpAuthenticationEntryPoint #13107
• Create NoOpAccessDeniedHandler #13109
• Create RequestMatchingAccessDeniedHandler #13108

With those enhancements, they can do:

@Configuration
@EnableWebSecurity
public class SecurityConfig {

	@Bean
	public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
		http
			// ...
			.exceptionHandling((exceptions) -> exceptions
					.authenticationEntryPoint(createDelegatingAuthenticationEntryPoint())
					.accessDeniedHandler(createDelegatingAccessDeniedHandler())
			);
		return http.build();
	}

	/**
	 * An {@link AccessDeniedHandler} that does nothing if the request dispatcher type is {@link DispatcherType#ERROR}.
	 * Otherwise, it delegates to the default {@link AccessDeniedHandlerImpl}.
	 */
	private AccessDeniedHandler createDelegatingAccessDeniedHandler() {
		DispatcherTypeRequestMatcher errorDispatcher = new DispatcherTypeRequestMatcher(DispatcherType.ERROR);
		LinkedHashMap<RequestMatcher, AccessDeniedHandler> handlers = new LinkedHashMap<>();
		handlers.put(errorDispatcher, new NoOpAccessDeniedHandler());
		return new RequestMatchingAccessDeniedHandler(handlers, new AccessDeniedHandlerImpl());
	}

	/**
	 * An {@link AuthenticationEntryPoint} that does nothing if the request dispatcher type is {@link DispatcherType#ERROR}.
	 * Otherwise, it delegates to the default {@link Http403ForbiddenEntryPoint}.
	 */
	private AuthenticationEntryPoint createDelegatingAuthenticationEntryPoint() {
		DispatcherTypeRequestMatcher errorDispatcher = new DispatcherTypeRequestMatcher(DispatcherType.ERROR);
		LinkedHashMap<RequestMatcher, AuthenticationEntryPoint> entryPoints = new LinkedHashMap<>();
		entryPoints.put(errorDispatcher, new NoOpAuthenticationEntryPoint());
		DelegatingAuthenticationEntryPoint authenticationEntryPoint = new DelegatingAuthenticationEntryPoint(entryPoints);
		authenticationEntryPoint.setDefaultEntryPoint(new Http403ForbiddenEntryPoint());
		return authenticationEntryPoint;
	}

[filiphr]

I have another input for this particular problem. I've created an issue in the Spring Boot project (spring-projects/spring-boot#36948 (comment)), but @philwebb rightfully so directed me to this issue here.

I've created an example repo where the problems I am facing are shown. Everything works as expected with Spring Boot 2.7 (Spring Security 5). However, in Spring Boot 3.1 and Spring Security 6.1 things are a bit inconsistent.

The security configuration looks like:

@EnableWebSecurity
@Configuration
public class SecurityConfiguration {

    @Bean
    @Order(1)
    public SecurityFilterChain firstSecurityFilterChain(HttpSecurity http) throws Exception {
        return http.sessionManagement(sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
                .csrf(AbstractHttpConfigurer::disable)
                // Comment out line below for Spring Boot 2.7
                //.antMatcher("/ignored-api/*")
                // Comment line below for Spring Boot 2.7
                .securityMatcher(AntPathRequestMatcher.antMatcher("/ignored-api/*"))
                .authorizeHttpRequests(configurer -> configurer.anyRequest().denyAll())
                .httpBasic(Customizer.withDefaults())
                .build();
    }

    @Bean
    @Order(2)
    public SecurityFilterChain secondSecurityFilterChain(HttpSecurity http) throws Exception {
        return http.sessionManagement(sessionManagement -> sessionManagement.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
                .csrf(AbstractHttpConfigurer::disable)
                .exceptionHandling(exceptionHandling -> exceptionHandling.defaultAuthenticationEntryPointFor(new HttpStatusEntryPoint(HttpStatus.UNAUTHORIZED),
                        AnyRequestMatcher.INSTANCE))
                .securityContext(securityContext -> securityContext.securityContextRepository(new NullSecurityContextRepository()))
                .authorizeHttpRequests(configurer -> configurer.anyRequest().authenticated())
                .httpBasic(Customizer.withDefaults())
                .build();
    }

}

My rest controller looks like:

@RestController
public class TestRestController {

    @GetMapping("/ignored-api/forbidden")
    public ResponseEntity<?> ignoredForbidden() {
        return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null);
    }

    @GetMapping("/ignored-api/ok")
    public ResponseEntity<?> ignoredOk() {
        return ResponseEntity.status(HttpStatus.OK).body(null);
    }

    @GetMapping("/allowed-api/forbidden")
    public ResponseEntity<?> allowedForbidden() {
        return ResponseEntity.status(HttpStatus.FORBIDDEN).body(null);
    }

    @GetMapping("/allowed-api/ok")
    public ResponseEntity<?> allowedOk() {
        return ResponseEntity.status(HttpStatus.OK).body(null);
    }
}

and the tests are:

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
public class TestRestControllerTest {

    @Autowired
    protected TestRestTemplate restTemplate;

    @Test
    void ignoredForbiddenNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/ignored-api/forbidden", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void ignoredForbiddenAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/ignored-api/forbidden", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.FORBIDDEN);
    }

    @Test
    void ignoredOkNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/ignored-api/ok", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void ignoredOkAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/ignored-api/ok", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.FORBIDDEN);
    }

    @Test
    void ignoredUnknownNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/ignored-api/dummy", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void ignoredUnknownAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/ignored-api/dummy", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.FORBIDDEN);
    }

    @Test
    void allowedForbiddenNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/allowed-api/forbidden", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void allowedForbiddenAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/allowed-api/forbidden", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.FORBIDDEN);
    }

    @Test
    void allowedOkNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/allowed-api/ok", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void allowedOkAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/allowed-api/ok", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.OK);
    }

    @Test
    void allowedUnknownNotAuthenticated() {
        ResponseEntity<String> response = restTemplate.getForEntity("/allowed-api/dummy", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.UNAUTHORIZED);
    }

    @Test
    void allowedUnknownAuthenticated() {
        ResponseEntity<String> response = restTemplate
                .withBasicAuth("user", "test")
                .getForEntity("/allowed-api/dummy", String.class);
        assertThat(response.getStatusCode()).as(response.toString()).isEqualTo(HttpStatus.NOT_FOUND);
    }
}

In Spring Boot 2.7 all the tests are green. With Spring Boot 3.1 the following ones are failing:

• ignoredUnknownAuthenticated - in 2.7 the status code is HTTP 403, with 3.1 it is HTTP 401
• ignoredForbiddenAuthenticated - in 2.7 the status code is HTTP 403, with 3.1 it is HTTP 401
• ignoredOkAuthenticated - in 2.7 the status code is HTTP 403, with 3.1 it is HTTP 401
• allowedUnknownAuthenticated - in 2.7 the status code is HTTP 404, with 3.1 it is HTTP 401

As you can see even though I am authenticated I keep getting HTTP 401, which in my honest opinion is not correct. In a way if Spring Security does this we will keep going in a loop to try to authenticate, even though we have valid credentials.

Why is Spring Security using the exception handling of the last configured SecurityFilterChain?

The proposed workarounds are not the most user friendly and what the user expects. This also affects a lot of people which are migrating from Spring Boot 2 to Spring Boot 3 and had some more complex security configuration.

Is it possible to reopen this issue and try to find a solution that would be different that the needs to adapt the last security filter chain with some workarounds?

[vemahendran]

As per my understanding, there is nothing to change the behaviors of Spring Security. Need to really appreciate them for improving the component more better in their latest versions.

After a day long research, allowing /error mapping makes sense to me.

By default, Spring Boot 3 provides /error mapping to handle all the exceptions. We need to ask spring-security component to permit /error mapping. Then Spring Security will allow the app to get the actual errors with appropriate HTTP status codes. In other words, all the perserved errors will be dispatched smoothly.

@Bean
public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
    return http
               .cors().and()
               .csrf().disable()
               .authorizeHttpRequests(auth -> auth
                    .requestMatchers("/error").permitAll()
                    .requestMatchers("/auth**").permitAll()
                    .anyRequest().authenticated())
               .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS))
               .addFilterBefore(jwtAuthFilter, UsernamePasswordAuthenticationFilter.class)
               .userDetailsService(jpaUserDetailsService)
               .build();
}

This particular line (.requestMatchers("/error").permitAll()) does the magic. I didn't do any changes in my CustomExceptionHandler classes. Now, I can see the orginal errors and the appropriate HTTP status codes in my API responses.

For more reference, please see the stackoverflow answer.