DefaultMethodSecurityExpressionHandler is not eligible for getting processed by all BeanPostProcessors

[dodgex]

Describe the bug
This is kind of a follow up to #8407. I just found that in my project there are two beans related to Method Security logged that they are not eligible for post processing.

 trationDelegate$BeanPostProcessorChecker : Bean 'org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler@6d2dc9d2' of type [org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)
 trationDelegate$BeanPostProcessorChecker : Bean 'methodSecurityMetadataSource' of type [org.springframework.security.access.method.DelegatingMethodSecurityMetadataSource] is not eligible for getting processed by all BeanPostProcessors (for example: not eligible for auto-proxying)

To Reproduce
As this also happens in a fresh project this is kind of easy to reproduce:

• go to start.spring.io
• create a spring boot 2.5.0 project (i'm sure other options are not relevant but i had maven+java11+jar)
• add following dependencies
  • spring-data-jpa
  • spring-security
  • spring-web
  • h2 database (to have a db for data-jpa)
• add @EnableGlobalMethodSecurity(prePostEnabled = true) to the application class
• build & start the application

The issue only seems to occur with spring-web and spring-data-jpa (maybe other combinations too)

Expected behavior
Ideally the message should not be logged.

Additional Info
Adding a custom config extending GlobalMethodSecurityConfiguration and overriding methodSecurityMetadataSource to add the @Role annotation allowed me to get rid of the related log message but I found no way to do this for the message related to DefaultMethodSecurityExpressionHandler.

The custom config:

import org.springframework.beans.factory.config.BeanDefinition;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Role;
import org.springframework.security.access.method.MethodSecurityMetadataSource;
import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
import org.springframework.security.config.annotation.method.configuration.GlobalMethodSecurityConfiguration;

@Configuration
@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
@EnableGlobalMethodSecurity(prePostEnabled = true)
public class MethodSecurityConfig extends GlobalMethodSecurityConfiguration {
    @Bean
    @Override
    @Role(BeanDefinition.ROLE_INFRASTRUCTURE)
    public MethodSecurityMetadataSource methodSecurityMetadataSource() {
        return super.methodSecurityMetadataSource();
    }
}

I'm not sure to what extend this is an actual issue as the security seems to work as expected. but as in the original issue #8407 and the #8429 there where attempts to work against these log messages and it is suggested that those INFO logs maybe should be warnings I thought I should mention it.

[jzheaux]

Thanks, @dodgex, for the report. I agree that this is a continuation of #8407, specifically this comment. Would you be able to submit a PR to add @Role(BeanDefinition.ROLE_INFRASTRUCTURE) to the two beans you listed?

[dodgex]

Hey @jzheaux I openend #9860 that addes the @Role annotation for the methodSecurityMetadataSource bean.

Unfortunately I have not found a way to handle the DefaultMethodSecurityExpressionHandler. I found that the log message appears when the objectPostProcessor is injected into the setObjectPostProcessor() method in GlobalMethodSecurityConfiguration (see here)

I'll try to spend some more time to see if I find a way but I'm not sure if i can.

[dodgex]

Even having a custom MethodSecurityExpressionHandler bean will cause this issue.

The issue seems to be that the PostProcessorRegistrationDelegate can't check if the expression handler is an infrastructure bean as it is not even a registread bean and so beanFactory.containsBeanDefinition() returns false. I don't think registring the defaultMethodExpressionHandler as a bean makes sense as it might conflict with custom beans and we don't have spring boot's @ConditionalOnMissingBean available here.

I think a possible solution, although possibly a bit ugly, could be merging setObjectPostProcessor and setMethodSecurityExpressionHandler. The merged method could either set the expressionHandler and not try to process the defaultMethodExpressionHandler one, or if there are no custom expression handler beans manually register the defaultMethodExpressionHandler bean with the correct role (if that is possible programatically).

[jzheaux]

I think this might be addressable by moving the post processing of DefaultMethodSecurityExpressionHandler to be after all bean post processors are instantiated.

The default handler is already being further configured in the afterSingletonsInstantiated method, so it seems reasonable to place it at the end of that method instead.

Also, with this change, applications will be able to register a BeanPostProcessor of their own to override the handler's configuration. Spring Security does the same with many other objects it constructs, so it would be nice for this to align better with that.

It appears the post processing was added for #2526, so we should ensure that such a change does not regress that ticket.

Can you confirm that moving the post processing to the end of afterSingletonsInitialized addresses the issue?

[dodgex]

I just tested it and the log message does no longer appear. As a result I added a second commit to the PR, it is not yet rebased in case there is more changes necessary. If you want me to squash the commits I'll do that.

I would check if that causes a regression for #2526 but I'm not how to that in a way to be sure it is not regressing that without knowing what he had set up in his project what caused the problem.

[jzheaux]

Thanks, @dodgex.

I'm not how to that in a way to be sure it is not regressing

No worries. I took a look, and I believe GlobalMethodSecurityConfigurationTests#defaultWebSecurityExpressionHandlerHasBeenResolverSet sufficiently tests #2526.