EnableGlobalMethodSecurity Misconfiguration Check

This polishes the EnableGlobalMethodSecurity misconfiguration check to not error if the user has specified a custom method security metadata source. Issue: spring-projectsgh-5341
jzheaux · Sep 18, 2018 · 8d95815 · 8d95815
1 parent 7ca97a7
commit 8d95815
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 17 deletions.
diff --git a/...rk/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java b/...rk/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java
@@ -359,19 +359,20 @@ public MethodSecurityMetadataSource methodSecurityMetadataSource() {
 			sources.add(customMethodSecurityMetadataSource);
 		}
 
+		boolean hasCustom = customMethodSecurityMetadataSource != null;
 		boolean isPrePostEnabled = prePostEnabled();
-		boolean isSecureEnabled = securedEnabled();
+		boolean isSecuredEnabled = securedEnabled();
 		boolean isJsr250Enabled = jsr250Enabled();
 
-		if (!isPrePostEnabled && !isSecureEnabled && !isJsr250Enabled) {
+		if (!isPrePostEnabled && !isSecuredEnabled && !isJsr250Enabled && !hasCustom) {
 			throw new IllegalStateException("In the composition of all global method configuration, " +
 					"no annotation support was actually activated");
 		}
 
 		if (isPrePostEnabled) {
 			sources.add(new PrePostAnnotationSecurityMetadataSource(attributeFactory));
 		}
-		if (isSecureEnabled) {
+		if (isSecuredEnabled) {
 			sources.add(new SecuredAnnotationSecurityMetadataSource());
 		}
 		if (isJsr250Enabled) {

diff --git a/...curity/config/annotation/method/configuration/GlobalMethodSecurityConfigurationTests.java b/...curity/config/annotation/method/configuration/GlobalMethodSecurityConfigurationTests.java
@@ -15,10 +15,16 @@
  */
 package org.springframework.security.config.annotation.method.configuration;
 
+import java.lang.reflect.Proxy;
+import java.util.HashMap;
+import java.util.Map;
+import javax.sql.DataSource;
+
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.junit.runner.RunWith;
+
 import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.UnsatisfiedDependencyException;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -31,6 +37,7 @@
 import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
 import org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl;
 import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor;
+import org.springframework.security.access.method.MethodSecurityMetadataSource;
 import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.AuthenticationTrustResolver;
@@ -49,11 +56,6 @@
 import org.springframework.transaction.annotation.EnableTransactionManagement;
 import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
 
-import javax.sql.DataSource;
-import java.lang.reflect.Proxy;
-import java.util.HashMap;
-import java.util.Map;
-
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
@@ -91,7 +93,7 @@ public void setMethodInterceptor(MethodSecurityInterceptor interceptor) {
 	MockEventListener<AbstractAuthenticationEvent> events;
 
 	@Test
-	public void illegalStateGlobalMethodSecurity() {
+	public void configureWhenGlobalMethodSecurityIsMissingMetadataSourceThenException() {
 		this.thrown.expect(UnsatisfiedDependencyException.class);
 		this.spring.register(IllegalStateGlobalMethodSecurityConfig.class).autowire();
 	}
@@ -101,6 +103,20 @@ public static class IllegalStateGlobalMethodSecurityConfig extends GlobalMethodS
 
 	}
 
+	@Test
+	public void configureWhenGlobalMethodSecurityHasCustomMetadataSourceThenNoEnablingAttributeIsNeeded() {
+		this.spring.register(CustomMetadataSourceConfig.class).autowire();
+	}
+
+	@EnableGlobalMethodSecurity
+	public static class CustomMetadataSourceConfig extends GlobalMethodSecurityConfiguration {
+		@Bean
+		@Override
+		protected MethodSecurityMetadataSource customMethodSecurityMetadataSource() {
+			return mock(MethodSecurityMetadataSource.class);
+		}
+	}
+
 	@Test
 	public void methodSecurityAuthenticationManagerPublishesEvent() {
 		this.spring.register(InMemoryAuthWithGlobalMethodSecurityConfig.class).autowire();

diff --git a/...k/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.java b/...k/security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.java
@@ -15,16 +15,25 @@
  */
 package org.springframework.security.config.annotation.method.configuration;
 
+import java.lang.reflect.Method;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
-import org.springframework.context.annotation.*;
+import org.springframework.context.annotation.AdviceMode;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+import org.springframework.context.annotation.ImportBeanDefinitionRegistrar;
 import org.springframework.core.Ordered;
 import org.springframework.core.type.AnnotationMetadata;
 import org.springframework.security.access.AccessDecisionManager;
@@ -46,12 +55,9 @@
 import org.springframework.security.test.context.support.WithMockUser;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 
-import java.lang.reflect.Method;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-
-import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 /**
  *
@@ -263,7 +269,7 @@ public void contextRefreshWhenUsingAspectJAndCustomGlobalMethodSecurityConfigura
 
 	}
 
-	@EnableGlobalMethodSecurity(mode = AdviceMode.ASPECTJ)
+	@EnableGlobalMethodSecurity(mode = AdviceMode.ASPECTJ, securedEnabled = true)
 	public static class AspectJModeExtendsGMSCConfig extends GlobalMethodSecurityConfiguration {
 	}