Annotated class with @Secured and @PreAuthorize does not secure super methods

Question

Situation
I have created an CrudController which contains some basic crud methods. This works fine. When i want to secure this controller i can override the method from the super class and that works fine. However i don't want to override every method just for security. I want to use @Secured or @PreAuthorize on the class for that.

I'm using Spring Boot 2.2.2 (latest at time of writing).

Example
Crud base class

public class CrudController<T, ID> {
    @GetMapping
    public ResponseEntity<List<T>> findAll() {
        // code for returning the list
    }
}

Implementing class

@RestController
@RequestMapping("/helloworld")
@Secured("ROLE_world.view") 
public class HelloWorld extends CrudController<World, Long> {

   @GetMapping("test")
   public ResponseEntity test() {
      // This method is secured by the annotation on the class
   }

   // the findAll from the super class is not secured

}

Expected behaviour
When a class is annotated with either @PreAuthorize or @Secured i expect that all public methods are secured. So also all public methods from the extended class should be secured. In my example findAll is insecure and that is an high security risk.

Tried
I already have tried setting the folowing annotations for configuration:

On the class itself:

@Scope( proxyMode = ScopedProxyMode.TARGET_CLASS )

On the configuration class:

@Configuration
@EnableAspectJAutoProxy
@EnableWebSecurity
@EnableGlobalMethodSecurity(
        prePostEnabled = true,
        securedEnabled = true,
        jsr250Enabled = true,
        mode = AdviceMode.PROXY,
        proxyTargetClass = true
)

In the application.properties:

spring.aop.proxy-target-class=true
spring.aop.auto=true

Answer 1

As a solution i have created an class which extends the HandlerInterceptorAdapter. This will only work with the @Secured as this is easier for checking the roles.

@Component
public class SecuredInterceptor extends HandlerInterceptorAdapter {
    private static final Logger LOGGER = getLogger(SecuredInterceptor.class);


    @Override
    public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception {
        LOGGER.debug("preHandle {}", handler.getClass());

        if (!(handler instanceof HandlerMethod)) {
            return super.preHandle(request, response, handler);
        }

        HandlerMethod handlerMethod = (HandlerMethod) handler;
        String[] allowedRoles = getAllowedRoles(handlerMethod);

        // No allowed roles, so allow everything
        if (allowedRoles.length == 0) {
            return super.preHandle(request, response, handler);
        }

        // Check if we have the needed role
        for (String allowedRole : allowedRoles) {
            // SecurityUtil checks if an loggedIn keycloak user has the right role 
            if (SecurityUtil.hasRole(allowedRole)) {
                return super.preHandle(request, response, handler);
            }
        }

        // UnauthorizedException is a custom exception which is handle in an @AdviceController
        throw new UnauthorizedException("Forbidden");
    }

    private String[] getAllowedRoles(HandlerMethod method) {
        Class<?> targetClass = method.getBean().getClass();
        Secured[] annotationsByType = targetClass.getAnnotationsByType(Secured.class);
        Secured methodAnnotation = method.getMethodAnnotation(Secured.class);

        // Methods are overriding class annotations
        if (methodAnnotation != null) {
            return methodAnnotation.value();
        }

        // Get from the class
        if (annotationsByType.length > 0) {
            return annotationsByType[0].value();
        }

        // No annotations
        return new String[0];
    }
}

I configure this interceptor in the WebConfig:

@Configuration
public class WebConfig implements WebMvcConfigurer {
    @Override
    public void addInterceptors(InterceptorRegistry registry) {
        registry.addInterceptor(securedInterceptor);
    }
}

Please up vote if you think i should accept my own answer.
Comments or other solutions are appreciated also.

Answer 2

Secured annotation is handled in the class AbstractFallbackMethodSecurityMetadataSource, in the following method:

public Collection<ConfigAttribute> getAttributes(Method method, Class<?> targetClass) {
        // The method may be on an interface, but we need attributes from the target
        // class.
        // If the target class is null, the method will be unchanged.
        Method specificMethod = AopUtils.getMostSpecificMethod(method, targetClass);
        // First try is the method in the target class.
        Collection<ConfigAttribute> attr = findAttributes(specificMethod, targetClass);
        if (attr != null) {
            return attr;
        }

        // Second try is the config attribute on the target class.
        attr = findAttributes(specificMethod.getDeclaringClass());
        if (attr != null) {
            return attr;
        }

        if (specificMethod != method || targetClass == null) {
            // Fallback is to look at the original method.
            attr = findAttributes(method, method.getDeclaringClass());
            if (attr != null) {
                return attr;
            }
            // Last fallback is the class of the original method.
            return findAttributes(method.getDeclaringClass());
        }
        return Collections.emptyList();
}

So if the method does not have @Secured annotation, the annotation is searched in the declaring class findAttributes(method.getDeclaringClass()), for your case the declaring class for findAll is CrudController that does not have any Secured annotation.

To solve this issue you should provide your own implementation of MethodSecurityMetadataSource.

Here after a simple way to do it:

1. Define a custom implementation of MethodSecurityMetadataSource

   public class CustomMethodSecurityMetadaSource extends
       SecuredAnnotationSecurityMetadataSource {
   
       public CustomMethodSecurityMetadaSource(){
           super();
       }
       @Override
       public Collection<ConfigAttribute> getAttributes(Method method,
       Class<?> targetClass) {
           Collection<ConfigAttribute> attr =  super.getAttributes(method, targetClass);
           if (CollectionUtils.isEmpty(attr)) {
               attr = findAttributes(targetClass);
               if (attr != null) return attr;
           }
           return Collections.emptyList();
       }
   }
   

2. Inject the custom implemetation by extending GlobalMethodSecurityConfiguration:

   @EnableGlobalMethodSecurity(prePostEnabled = true, securedEnabled = true)
   @Configuration
   public class SecuredConfig extends GlobalMethodSecurityConfiguration {
       @Override
       protected MethodSecurityMetadataSource customMethodSecurityMetadataSource() {
           return new CustomMethodSecurityMetadaSource();
       }
   }