Make single definition of defaultRolePrefix and rolePrefix

Previous to this commit, role prefix had to be set in every class causing repetition. Now, bean `GrantedAuthorityDefaults` can be used to define the role prefix in a single point. Fixes gh-3701
spring-projects · Sep 21, 2016 · eabeaf3 · eabeaf3
1 parent 2e6656e
commit eabeaf3
Show file tree

Hide file tree

Showing 13 changed files with 362 additions and 29 deletions.
diff --git a/...nfig/annotation/authentication/configurers/ldap/LdapAuthenticationProviderConfigurer.java b/...nfig/annotation/authentication/configurers/ldap/LdapAuthenticationProviderConfigurer.java
@@ -15,11 +15,15 @@
  */
 package org.springframework.security.config.annotation.authentication.configurers.ldap;
 
+import java.io.IOException;
+import java.net.ServerSocket;
+
 import org.springframework.ldap.core.support.BaseLdapPathContextSource;
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.authentication.encoding.PasswordEncoder;
 import org.springframework.security.authentication.encoding.PlaintextPasswordEncoder;
+import org.springframework.security.config.GrantedAuthorityDefaults;
 import org.springframework.security.config.annotation.ObjectPostProcessor;
 import org.springframework.security.config.annotation.SecurityConfigurerAdapter;
 import org.springframework.security.config.annotation.authentication.ProviderManagerBuilder;
@@ -43,9 +47,6 @@
 import org.springframework.security.ldap.userdetails.UserDetailsContextMapper;
 import org.springframework.util.Assert;
 
-import java.io.IOException;
-import java.net.ServerSocket;
-
 /**
  * Configures LDAP {@link AuthenticationProvider} in the {@link ProviderManagerBuilder}.
  *
@@ -60,7 +61,7 @@ public class LdapAuthenticationProviderConfigurer<B extends ProviderManagerBuild
 	private String groupRoleAttribute = "cn";
 	private String groupSearchBase = "";
 	private String groupSearchFilter = "(uniqueMember={0})";
-	private String rolePrefix = "ROLE_";
+	private GrantedAuthorityDefaults rolePrefix = new GrantedAuthorityDefaults("ROLE_");
 	private String userSearchBase = ""; // only for search
 	private String userSearchFilter = null;// "uid={0}"; // only for search
 	private String[] userDnPatterns;
@@ -128,7 +129,7 @@ private LdapAuthoritiesPopulator getLdapAuthoritiesPopulator() {
 				contextSource, groupSearchBase);
 		defaultAuthoritiesPopulator.setGroupRoleAttribute(groupRoleAttribute);
 		defaultAuthoritiesPopulator.setGroupSearchFilter(groupSearchFilter);
-		defaultAuthoritiesPopulator.setRolePrefix(rolePrefix);
+		defaultAuthoritiesPopulator.setRolePrefix(this.rolePrefix.getRolePrefix());
 
 		this.ldapAuthoritiesPopulator = defaultAuthoritiesPopulator;
 		return defaultAuthoritiesPopulator;
@@ -161,7 +162,7 @@ protected GrantedAuthoritiesMapper getAuthoritiesMapper() throws Exception {
 		}
 
 		SimpleAuthorityMapper simpleAuthorityMapper = new SimpleAuthorityMapper();
-		simpleAuthorityMapper.setPrefix(rolePrefix);
+		simpleAuthorityMapper.setPrefix(this.rolePrefix.getRolePrefix());
 		simpleAuthorityMapper.afterPropertiesSet();
 		this.authoritiesMapper = simpleAuthorityMapper;
 		return simpleAuthorityMapper;
@@ -356,7 +357,7 @@ public LdapAuthenticationProviderConfigurer<B> groupSearchFilter(
 	 * @see SimpleAuthorityMapper#setPrefix(String)
 	 */
 	public LdapAuthenticationProviderConfigurer<B> rolePrefix(String rolePrefix) {
-		this.rolePrefix = rolePrefix;
+		this.rolePrefix = new GrantedAuthorityDefaults(rolePrefix);
 		return this;
 	}
 

diff --git a/...rk/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java b/...rk/security/config/annotation/method/configuration/GlobalMethodSecurityConfiguration.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -49,6 +49,7 @@
 import org.springframework.security.access.intercept.AfterInvocationManager;
 import org.springframework.security.access.intercept.AfterInvocationProviderManager;
 import org.springframework.security.access.intercept.RunAsManager;
+import org.springframework.security.access.intercept.RunAsManagerImpl;
 import org.springframework.security.access.intercept.aopalliance.MethodSecurityInterceptor;
 import org.springframework.security.access.intercept.aspectj.AspectJMethodSecurityInterceptor;
 import org.springframework.security.access.method.DelegatingMethodSecurityMetadataSource;
@@ -63,6 +64,8 @@
 import org.springframework.security.authentication.AuthenticationManager;
 import org.springframework.security.authentication.AuthenticationTrustResolver;
 import org.springframework.security.authentication.DefaultAuthenticationEventPublisher;
+
+import org.springframework.security.config.GrantedAuthorityDefaults;
 import org.springframework.security.config.annotation.ObjectPostProcessor;
 import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
 import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration;
@@ -74,6 +77,7 @@
  * {@link EnableGlobalMethodSecurity} annotation on the subclass.
  *
  * @author Rob Winch
+ * @author Eddú Meléndez
  * @since 3.2
  * @see EnableGlobalMethodSecurity
  */
@@ -130,6 +134,14 @@ public MethodInterceptor methodSecurityInterceptor() throws Exception {
 				.setSecurityMetadataSource(methodSecurityMetadataSource());
 		RunAsManager runAsManager = runAsManager();
 		if (runAsManager != null) {
+			if (runAsManager instanceof RunAsManagerImpl) {
+				GrantedAuthorityDefaults grantedAuthorityDefaults =
+						getSingleBeanOrNull(GrantedAuthorityDefaults.class);
+				if (grantedAuthorityDefaults != null) {
+					((RunAsManagerImpl) runAsManager).setRolePrefix(
+							grantedAuthorityDefaults.getRolePrefix());
+				}
+			}
 			methodSecurityInterceptor.setRunAsManager(runAsManager);
 		}
 
@@ -168,6 +180,13 @@ public void afterSingletonsInstantiated() {
 		if (trustResolver != null) {
 			this.defaultMethodExpressionHandler.setTrustResolver(trustResolver);
 		}
+
+		GrantedAuthorityDefaults grantedAuthorityDefaults = getSingleBeanOrNull(
+				GrantedAuthorityDefaults.class);
+		if (grantedAuthorityDefaults != null) {
+			this.defaultMethodExpressionHandler.setDefaultRolePrefix(
+					grantedAuthorityDefaults.getRolePrefix());
+		}
 	}
 
 	private <T> T getSingleBeanOrNull(Class<T> type) {
@@ -355,6 +374,12 @@ public MethodSecurityMetadataSource methodSecurityMetadataSource() {
 			sources.add(new SecuredAnnotationSecurityMetadataSource());
 		}
 		if (jsr250Enabled()) {
+			GrantedAuthorityDefaults grantedAuthorityDefaults =
+					getSingleBeanOrNull(GrantedAuthorityDefaults.class);
+			if (grantedAuthorityDefaults != null) {
+				this.jsr250MethodSecurityMetadataSource.setDefaultRolePrefix(
+						grantedAuthorityDefaults.getRolePrefix());
+			}
 			sources.add(jsr250MethodSecurityMetadataSource);
 		}
 		return new DelegatingMethodSecurityMetadataSource(sources);

diff --git a/...rity/config/annotation/method/configuration/GlobalMethodSecurityConfigurationTests.groovy b/...rity/config/annotation/method/configuration/GlobalMethodSecurityConfigurationTests.groovy
@@ -17,7 +17,8 @@ package org.springframework.security.config.annotation.method.configuration
 
 
 import org.springframework.security.access.hierarchicalroles.RoleHierarchy;
-import org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl;
+import org.springframework.security.access.hierarchicalroles.RoleHierarchyImpl
+import org.springframework.security.config.GrantedAuthorityDefaults;
 
 import java.lang.reflect.Proxy;
 
@@ -496,4 +497,27 @@ public class GlobalMethodSecurityConfigurationTests extends BaseSpringSpec {
 			return new RoleHierarchyImpl(hierarchy:"ROLE_USER > ROLE_ADMIN")
 		}
 	}
+
+	def "GrantedAuthorityDefaults autowires"() {
+		when:
+			loadConfig(CustomGrantedAuthorityConfig)
+			def preAdviceVoter = context.getBean(MethodInterceptor).accessDecisionManager.decisionVoters.find { it instanceof PreInvocationAuthorizationAdviceVoter}
+		then:
+		preAdviceVoter.preAdvice.expressionHandler.defaultRolePrefix == "ROLE:"
+	}
+
+	@EnableGlobalMethodSecurity(prePostEnabled = true)
+	static class CustomGrantedAuthorityConfig extends GlobalMethodSecurityConfiguration {
+
+		@Override
+		protected void configure(AuthenticationManagerBuilder auth) throws Exception {
+			auth
+				.inMemoryAuthentication()
+		}
+
+		@Bean
+		public GrantedAuthorityDefaults ga() {
+			return new GrantedAuthorityDefaults("ROLE:")
+		}
+	}
 }
diff --git a/...security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy b/...security/config/annotation/method/configuration/NamespaceGlobalMethodSecurityTests.groovy
@@ -15,7 +15,9 @@
  */
 package org.springframework.security.config.annotation.method.configuration
 
+import org.springframework.security.access.annotation.Jsr250MethodSecurityMetadataSource
 import org.springframework.security.access.intercept.aspectj.AspectJMethodSecurityInterceptor
+import org.springframework.security.config.GrantedAuthorityDefaults
 
 import static org.assertj.core.api.Assertions.assertThat
 import static org.junit.Assert.fail
@@ -146,6 +148,39 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec {
 	public static class Jsr250Config extends BaseMethodConfig {
 	}
 
+	def "enable jsr250 with custom role prefix"() {
+		when:
+		context = new AnnotationConfigApplicationContext(Jsr250WithCustomRolePrefixConfig)
+		MethodSecurityService service = context.getBean(MethodSecurityService)
+		then: "@Secured and @PreAuthorize are ignored"
+		service.secured() == null
+		service.preAuthorize() ==  null
+
+		when: "@DenyAll method invoked"
+		service.jsr250()
+		then: "access is denied"
+		thrown(AccessDeniedException)
+		when: "@PermitAll method invoked"
+		String jsr250PermitAll = service.jsr250PermitAll()
+		then: "access is allowed"
+		jsr250PermitAll == null
+		when:
+		Jsr250MethodSecurityMetadataSource jsr250MethodSecurity = context.getBean(Jsr250MethodSecurityMetadataSource)
+		then:
+		jsr250MethodSecurity.defaultRolePrefix == "ROLE:"
+	}
+
+	@EnableGlobalMethodSecurity(jsr250Enabled = true)
+	@Configuration
+	public static class Jsr250WithCustomRolePrefixConfig extends BaseMethodConfig {
+
+		@Bean
+		public GrantedAuthorityDefaults ga() {
+			return new GrantedAuthorityDefaults("ROLE:")
+		}
+
+	}
+
 	// --- metadata-source-ref ---
 
 	def "custom MethodSecurityMetadataSource can be used with higher priority than other sources"() {
@@ -320,7 +355,7 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec {
 			context = new AnnotationConfigApplicationContext(BaseMethodConfig,CustomRunAsManagerConfig)
 			MethodSecurityService service = context.getBean(MethodSecurityService)
 		then:
-			service.runAs().authorities.find { it.authority == "ROLE_RUN_AS_SUPER"}
+			service.runAs().authorities.find { it.authority == "ROLE:RUN_AS_SUPER"}
 	}
 
 	@EnableGlobalMethodSecurity(securedEnabled = true)
@@ -331,6 +366,11 @@ public class NamespaceGlobalMethodSecurityTests extends BaseSpringSpec {
 			runAsManager.setKey("some key")
 			return runAsManager
 		}
+
+		@Bean
+		public GrantedAuthorityDefaults ga() {
+			return new GrantedAuthorityDefaults("ROLE:")
+		}
 	}
 
 	// --- secured-annotation ---

diff --git a/core/src/main/java/org/springframework/security/config/GrantedAuthorityDefaults.java b/core/src/main/java/org/springframework/security/config/GrantedAuthorityDefaults.java
@@ -0,0 +1,38 @@
+/*
+ * Copyright 2002-2016 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.springframework.security.config;
+
+/**
+ * @author Eddú Meléndez
+ * @since 4.2.0
+ */
+public class GrantedAuthorityDefaults {
+
+	private String rolePrefix = "ROLE_";
+
+	public GrantedAuthorityDefaults(String rolePrefix) {
+		this.rolePrefix = rolePrefix;
+	}
+
+	public String getRolePrefix() {
+		return this.rolePrefix;
+	}
+
+	public void setRolePrefix(String rolePrefix) {
+		this.rolePrefix = rolePrefix;
+	}
+
+}
diff --git a/...n/java/org/springframework/security/ldap/userdetails/DefaultLdapAuthoritiesPopulator.java b/...n/java/org/springframework/security/ldap/userdetails/DefaultLdapAuthoritiesPopulator.java
@@ -16,6 +16,10 @@
 
 package org.springframework.security.ldap.userdetails;
 
+import org.springframework.beans.BeansException;
+import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContextAware;
+import org.springframework.security.config.GrantedAuthorityDefaults;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.ldap.SpringSecurityLdapTemplate;
@@ -97,7 +101,7 @@
  * @author Luke Taylor
  * @author Filip Hanik
  */
-public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator {
+public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator, ApplicationContextAware {
 	// ~ Static fields/initializers
 	// =====================================================================================
 
@@ -140,7 +144,7 @@ public class DefaultLdapAuthoritiesPopulator implements LdapAuthoritiesPopulator
 	/**
 	 * The role prefix that will be prepended to each role name
 	 */
-	private String rolePrefix = "ROLE_";
+	private GrantedAuthorityDefaults rolePrefix = new GrantedAuthorityDefaults("ROLE_");
 	/**
 	 * Should we convert the role name to uppercase
 	 */
@@ -250,7 +254,7 @@ public Set<GrantedAuthority> getGroupMembershipRoles(String userDn, String usern
 				role = role.toUpperCase();
 			}
 
-			authorities.add(new SimpleGrantedAuthority(rolePrefix + role));
+			authorities.add(new SimpleGrantedAuthority(rolePrefix.getRolePrefix() + role));
 		}
 
 		return authorities;
@@ -297,7 +301,7 @@ public void setGroupSearchFilter(String groupSearchFilter) {
 	 */
 	public void setRolePrefix(String rolePrefix) {
 		Assert.notNull(rolePrefix, "rolePrefix must not be null");
-		this.rolePrefix = rolePrefix;
+		this.rolePrefix = new GrantedAuthorityDefaults(rolePrefix);
 	}
 
 	/**
@@ -360,7 +364,7 @@ protected final String getGroupSearchFilter() {
 	 * @see #setRolePrefix(String)
 	 */
 	protected final String getRolePrefix() {
-		return rolePrefix;
+		return this.rolePrefix.getRolePrefix();
 	}
 
 	/**
@@ -391,4 +395,14 @@ private GrantedAuthority getDefaultRole() {
 	private SearchControls getSearchControls() {
 		return searchControls;
 	}
+
+	@Override
+	public void setApplicationContext(ApplicationContext context) throws
+			BeansException {
+		String[] beanNames = context.getBeanNamesForType(GrantedAuthorityDefaults.class);
+		if (beanNames.length == 1) {
+			this.rolePrefix = context.getBean(beanNames[0], GrantedAuthorityDefaults.class);
+		}
+	}
+
 }