From 5c604b95fb620e1da99f8612e42935d6110cd60c Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 22 Aug 2024 11:44:01 -0600 Subject: [PATCH] Correct PostFilterAuthorizationMethodInterceptor Target Type Previously, `postFilterAuthorizationMethodInterceptor` mistakenly was published as an `Advisor`. Because `MethodSecurityAdvisorRegistrar` re-publishes each pre/post annotation interceptor also as an `Advisor`, this resulted in a duplicate advisor for `@PostFilter`. Closes gh-15651 --- .../PrePostMethodSecurityConfiguration.java | 3 +-- ...ePostMethodSecurityConfigurationTests.java | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java index a0561b58f67..d076cf8c956 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfiguration.java @@ -18,7 +18,6 @@ import org.aopalliance.intercept.MethodInterceptor; -import org.springframework.aop.Advisor; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.ApplicationContext; @@ -100,7 +99,7 @@ MethodInterceptor postAuthorizeAuthorizationMethodInterceptor() { @Bean @Role(BeanDefinition.ROLE_INFRASTRUCTURE) - Advisor postFilterAuthorizationMethodInterceptor() { + MethodInterceptor postFilterAuthorizationMethodInterceptor() { return this.postFilterAuthorizationMethodInterceptor; } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java index 282c01cffeb..ed86697f714 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/PrePostMethodSecurityConfigurationTests.java @@ -73,6 +73,8 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; /** @@ -432,6 +434,18 @@ public void configureWhenBeanOverridingDisallowedThenWorks() { .autowire(); } + // gh-15651 + @Test + @WithMockUser(roles = "ADMIN") + public void adviseWhenPrePostEnabledThenEachInterceptorRunsExactlyOnce() { + this.spring.register(MethodSecurityServiceConfig.class, CustomMethodSecurityExpressionHandlerConfig.class) + .autowire(); + MethodSecurityExpressionHandler expressionHandler = this.spring.getContext() + .getBean(MethodSecurityExpressionHandler.class); + this.methodSecurityService.manyAnnotations(new ArrayList<>(Arrays.asList("harold", "jonathan", "tim", "bo"))); + verify(expressionHandler, times(4)).createEvaluationContext(any(Supplier.class), any()); + } + private static Consumer disallowBeanOverriding() { return (context) -> ((AnnotationConfigWebApplicationContext) context).setAllowBeanDefinitionOverriding(false); } @@ -491,6 +505,19 @@ MethodSecurityService methodSecurityService() { } + @EnableMethodSecurity + static class CustomMethodSecurityExpressionHandlerConfig { + + private final MethodSecurityExpressionHandler expressionHandler = spy( + new DefaultMethodSecurityExpressionHandler()); + + @Bean + MethodSecurityExpressionHandler methodSecurityExpressionHandler() { + return this.expressionHandler; + } + + } + @EnableMethodSecurity static class CustomPermissionEvaluatorConfig {