@PreAuthorize should not apply method security to inherited concrete methods

[MartinHaeusler]

Describe the bug
I don't know the exact spring security patch version where this behavior changed, but in 6.2.4 I had a setup like this:

@RestController
@PreAuthorize("hasAdminRole()") // expression doesn't matter
public class MyRestController {

    @ResponseBody
    @GetMapping("/status")
    public String someEndpoint(){
        return "foo";
    }

   @ExceptionHandler
   public ResponseEntity<String> handleAccessDeniedException(ServletRequets request, AccessDeniedException e){
        // .... build some nice string... 
   }

}

In Spring Security 6.2.4, if an request comes in that does not match the @PreAuthorize condition, then Spring Security creates an AccessDeniedException, and hands it over to the handleAccessDeniedException(...) method. Everything was fine.

In Spring Security 6.3.1 (and maybe earlier as well, not sure, this is the version I'm currently upgrading to), something else happens:

• The REST call comes in
• The @PreAuthorize expression is evaluated and fails
• An AccessDeniedException is created
• Spring Security attempts to call handleAccessDeniedException...
• ... but gets intercepted by the method security again
• ... which again throws an AccessDeniedException
• ... which causes the original handler invocation to fail, logging the second AccessDeniedException as a warning with "Failure in @ExceptionHandler handleAccessDeniedException"

Overall, this results in a HTTP 403 for the client, no matter what the handleAccessDeniedException method would have done, because it never gets called.

The workaround is to move all @ExceptionHandler methods to an external @ControllerAdvice class, as those are not subject to the method security imposed by the class-level @PreAuthorize annotation. In Spring Security 6.2.4 this worked out of the box.

I think this is a weird breaking change and a potential pitfall for developers. Nobody would think that spring would intercept itself, applying method security on an exception handler method.

[marcusdacoregio]

Hi @MartinHaeusler, thanks for the report.

Are you sure that it used to work in Spring Security 6.2.x? I'm trying a sample with Spring Boot 3.2.4, therefore Spring Security 6.2.4 and I see the handler method not being invoked:

Failure in @ExceptionHandler com.example.springsecurityexamples.MyRestController#handleAccessDeniedException(AccessDeniedException)

Are you able to provide a minimal, reproducible sample where I can just change versions to simulate the problem?

[marcusdacoregio]

I just checked against all the supported branches and that doesn't work in any of them.

[MartinHaeusler]

@marcusdacoregio I apologize for the delay, I've been on vacation for the last two weeks. There is one detail we had in our codebase which I didn't include in the example (because I thought it doesn't matter):

We had our @ExceptionHandler methods defined in an abstract class AbstractRestController from which all our REST controllers derived. This used to be our way to share the handler logic. Our AbstractRestController had no spring security annotations at all. With the upgrade from 6.2.4 to 6.3.1, those handlers stopped working for the reasons listed in the opening post. This may or may not have been an intentional change which is why I thought I should report it.

Also, the popular tutorial site Baeldung recommends the abstract base class approach.

We've moved all our @ExceptionHandlers into a @ControllerAdvice class and eliminated our AbstractRestController, it works fine that way.

In general, I would question if it is a good idea to apply any method security rules to a method that is annotated as @ExceptionHandler. I could imagine that this trips up developers and the resulting exceptions are quite hard to understand.

[kse-music]

I just checked against all the supported branches and that doesn't work in any of them.

This bug maybe exist in all branches.

When an exception occurs, the ExceptionHandler defined in Controller is used first, so when the handleAccessDeniedException method is executed, it is also intercepted by AuthorizationManagerBeforeMethodInterceptor

Here is the test code

@RequestMapping("test")
@RestController
@PreAuthorize("hasRole('admin')")
@EnableMethodSecurity
@SpringBootApplication
public class TestApplication {

    public static void main(String[] args) {
        SpringApplication.run(TestApplication.class, args);
    }

    @Bean
    static SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
        return http
                .authorizeHttpRequests(c -> c.anyRequest().permitAll())
                .build();
    }

    @GetMapping("list")
    public String list(String query) {
        return query;
    }

    @ExceptionHandler
    public ResponseEntity<String> handleAccessDeniedException(AccessDeniedException e){
        return ResponseEntity.ok(e.getMessage());
    }

}