Spring Security 7 and @PreAuthorize and .oauth2ResourceServer: AuthorizationDeniedException results in 500 response

[ah1508]

Describe the bug

When a @PreAuthorize annotated method is called with insufficient permissions (anonymous call, missing authorities, missing roles), a AuthorizationDeniedException is thrown (like with Spring 6) but it is translated into HTTP 500 response when .oauth2ResourceServer is enabled.

To Reproduce
Execute a request on a path handled by a method decorated with @PreAuthorize with oauth2ResourceServer enabled.

@SpringBootApplication
@EnableMethodSecurity
public class DemoApplication {

    @Bean
    SecurityFilterChain securityFilterChain(HttpSecurity httpSecurity) throws Exception {
        return httpSecurity
                .oauth2ResourceServer(x -> x.jwt(Customizer.withDefaults()))
                //.httpBasic(Customizer.withDefaults())
                .build();
    }


    public static void main(String[] args) {

        var ctx = SpringApplication.run(DemoApplication.class, "--spring.security.oauth2.resourceserver.jwt.issuer-uri=http://whatever");

        RestClient restClient = RestClient.create();
        var resp = restClient.get()
                .uri("http://localhost:8080/foo")
                .retrieve()
                .toBodilessEntity();
    }

}

@RestController
class FooEndpoint {

    @GetMapping("foo")
    @PreAuthorize("isAuthenticated()")
    public String foo() {
        return "foo";
    }
}

It looks like a jakarta.servlet.error.exception attribute is added to the request. When Tomcat sees that it returns a 500 response with HTML code in the body

Code from org.apache.catalina.valves.ErrorReportValve (ERROR_EXCEPTION is jakarta.servlet.error.exception).

        Throwable throwable = (Throwable) request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);

        // If an async request is in progress and is not going to end once this
        // container thread finishes, do not process any error page here.
        if (request.isAsync() && !request.isAsyncCompleting()) {
            return;
        }

        if (throwable != null && !response.isError()) {
            // Make sure that the necessary methods have been called on the
            // response. (It is possible a component may just have set the
            // Throwable. Tomcat won't do that but other components might.)
            // These are safe to call at this point as we know that the response
            // has not been committed.
            response.reset();
            response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
        }

Expected behavior
Status 401 or 403, but not 500.

Sample

demo.zip

[therepanic]

Hi, @ah1508. My idea is that the thrown AuthorizationDeniedException is not handled in ExceptionTranslationFilter, which causes a pure exception to be thrown to the tomcat container. I have attached a PR with a test that will now fail.

Note: Actually, this is not the case. AuthorizedDeniedException inherits from AccessDeniedException, and my assumption turned out to be incorrect. The test that fails is not entirely correct.

[ah1508]

@therepanic : adding a break point in the ExceptionTranslationFilter shows that the exception is properly handled by the filter.

The last line of the doFilter method is called:

handleSpringSecurityException(request, response, chain, securityException);

The securityException is an instance of org.springframework.security.authorization.AuthorizationDeniedException

Status 401 is then properly set.

With Spring Security 6 the request attribute jakarta.servlet.error.exception is not added so Tomcat does not change the status and does not add the HTML body.

[jzheaux]

Thanks, @ah1508, I'm able to reproduce the error. I believe it is due to #16058 which aims to propagate Security exceptions thrown while the return value is processed.

I will look into this and report back.

cc @evgeniycheban

[jzheaux]

@ah1508, I've pushed a fix, the updated snapshot should be available shortly. Can you check that the snapshot version works for you?