From 68cf74468c57fe8719a4682e781f3f9171181edb Mon Sep 17 00:00:00 2001 From: Josh Cummings Date: Thu, 8 Apr 2021 14:33:30 -0600 Subject: [PATCH] Add check for custom advice - Because publishing an advice bean replaces Spring Security defaults, the code should error if both a custom bean and either secureEnabled or prePostEnabled are specified Issue gh-9289 --- .../MethodSecurityConfiguration.java | 23 +++++++++---- .../MethodSecurityConfigurationTests.java | 32 +++++++++++++++---- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java index 090b193baa..dda256507d 100644 --- a/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java +++ b/config/src/main/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfiguration.java @@ -36,6 +36,7 @@ import org.springframework.aop.support.AopUtils; import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.aop.support.Pointcuts; import org.springframework.aop.support.StaticMethodMatcher; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.Bean; @@ -45,26 +46,27 @@ import org.springframework.context.annotation.Role; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.type.AnnotationMetadata; -import org.springframework.security.authorization.method.Jsr250AuthorizationManager; import org.springframework.security.access.annotation.Secured; -import org.springframework.security.authorization.method.SecuredAuthorizationManager; import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler; import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler; -import org.springframework.security.authorization.method.AuthorizationMethodInterceptor; +import org.springframework.security.access.prepost.PostAuthorize; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.authorization.method.AuthorizationManagerMethodAfterAdvice; import org.springframework.security.authorization.method.AuthorizationManagerMethodBeforeAdvice; import org.springframework.security.authorization.method.AuthorizationMethodAfterAdvice; import org.springframework.security.authorization.method.AuthorizationMethodBeforeAdvice; +import org.springframework.security.authorization.method.AuthorizationMethodInterceptor; import org.springframework.security.authorization.method.DelegatingAuthorizationMethodAfterAdvice; import org.springframework.security.authorization.method.DelegatingAuthorizationMethodBeforeAdvice; +import org.springframework.security.authorization.method.Jsr250AuthorizationManager; import org.springframework.security.authorization.method.MethodAuthorizationContext; -import org.springframework.security.access.prepost.PostAuthorize; -import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.authorization.method.PostAuthorizeAuthorizationManager; import org.springframework.security.authorization.method.PostFilterAuthorizationMethodAfterAdvice; import org.springframework.security.authorization.method.PreAuthorizeAuthorizationManager; import org.springframework.security.authorization.method.PreFilterAuthorizationMethodBeforeAdvice; +import org.springframework.security.authorization.method.SecuredAuthorizationManager; import org.springframework.security.config.core.GrantedAuthorityDefaults; +import org.springframework.util.Assert; /** * Base {@link Configuration} for enabling Spring Security Method Security. @@ -75,7 +77,7 @@ import org.springframework.security.config.core.GrantedAuthorityDefaults; */ @Configuration(proxyBeanMethods = false) @Role(BeanDefinition.ROLE_INFRASTRUCTURE) -final class MethodSecurityConfiguration implements ImportAware { +final class MethodSecurityConfiguration implements ImportAware, InitializingBean { private MethodSecurityExpressionHandler methodSecurityExpressionHandler; @@ -214,6 +216,15 @@ final class MethodSecurityConfiguration implements ImportAware { this.enableMethodSecurity = AnnotationAttributes.fromMap(attributes); } + @Override + public void afterPropertiesSet() throws Exception { + if (!securedEnabled() && !jsr250Enabled()) { + return; + } + Assert.isNull(this.authorizationMethodBeforeAdvice, + "You have specified your own advice, meaning that the annotation attributes securedEnabled and jsr250Enabled will be ignored. Please choose one or the other."); + } + private boolean securedEnabled() { return this.enableMethodSecurity.getBoolean("securedEnabled"); } diff --git a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfigurationTests.java b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfigurationTests.java index d50fc09652..69baeaffb0 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfigurationTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/method/configuration/MethodSecurityConfigurationTests.java @@ -27,6 +27,7 @@ import org.junit.runner.RunWith; import org.springframework.aop.MethodMatcher; import org.springframework.aop.support.JdkRegexpMethodPointcut; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.security.access.AccessDeniedException; @@ -35,12 +36,12 @@ import org.springframework.security.access.annotation.BusinessService; import org.springframework.security.access.annotation.ExpressionProtectedBusinessServiceImpl; import org.springframework.security.access.expression.method.DefaultMethodSecurityExpressionHandler; import org.springframework.security.access.expression.method.MethodSecurityExpressionHandler; +import org.springframework.security.authorization.AuthorizationDecision; +import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.authorization.method.AuthorizationManagerMethodBeforeAdvice; import org.springframework.security.authorization.method.AuthorizationMethodAfterAdvice; import org.springframework.security.authorization.method.AuthorizationMethodBeforeAdvice; import org.springframework.security.authorization.method.MethodAuthorizationContext; -import org.springframework.security.authorization.AuthorizationDecision; -import org.springframework.security.authorization.AuthorizationManager; import org.springframework.security.config.test.SpringTestRule; import org.springframework.security.core.Authentication; import org.springframework.security.test.context.annotation.SecurityTestExecutionListeners; @@ -103,7 +104,7 @@ public class MethodSecurityConfigurationTests { @WithMockUser @Test public void securedWhenRoleUserThenAccessDeniedException() { - this.spring.register(MethodSecurityServiceConfig.class).autowire(); + this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(this.methodSecurityService::secured) .withMessage("Access Denied"); } @@ -119,7 +120,7 @@ public class MethodSecurityConfigurationTests { @WithMockUser(roles = "ADMIN") @Test public void securedUserWhenRoleAdminThenAccessDeniedException() { - this.spring.register(MethodSecurityServiceConfig.class).autowire(); + this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(this.methodSecurityService::securedUser) .withMessage("Access Denied"); } @@ -244,7 +245,7 @@ public class MethodSecurityConfigurationTests { @WithMockUser(roles = "ADMIN") @Test public void jsr250WhenRoleAdminThenAccessDeniedException() { - this.spring.register(MethodSecurityServiceConfig.class).autowire(); + this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); assertThatExceptionOfType(AccessDeniedException.class).isThrownBy(this.methodSecurityService::jsr250) .withMessage("Access Denied"); } @@ -252,7 +253,7 @@ public class MethodSecurityConfigurationTests { @WithAnonymousUser @Test public void jsr250PermitAllWhenRoleAnonymousThenPasses() { - this.spring.register(MethodSecurityServiceConfig.class).autowire(); + this.spring.register(MethodSecurityServiceEnabledConfig.class).autowire(); String result = this.methodSecurityService.jsr250PermitAll(); assertThat(result).isNull(); } @@ -272,7 +273,14 @@ public class MethodSecurityConfigurationTests { this.businessService.rolesAllowedUser(); } - @EnableMethodSecurity(securedEnabled = true, jsr250Enabled = true) + @Test + public void configureWhenCustomAdviceAndSecureEnabledThenException() { + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(() -> this.spring + .register(CustomAuthorizationManagerBeforeAdviceConfig.class, MethodSecurityServiceEnabledConfig.class) + .autowire()); + } + + @EnableMethodSecurity static class MethodSecurityServiceConfig { @Bean @@ -292,6 +300,16 @@ public class MethodSecurityConfigurationTests { } + @EnableMethodSecurity(securedEnabled = true, jsr250Enabled = true) + static class MethodSecurityServiceEnabledConfig { + + @Bean + MethodSecurityService methodSecurityService() { + return new MethodSecurityServiceImpl(); + } + + } + @EnableMethodSecurity static class CustomPermissionEvaluatorConfig {