From adbf18a091ba909fa2c71a9233c87036871a5310 Mon Sep 17 00:00:00 2001 From: Luke Taylor Date: Wed, 6 Feb 2008 13:14:46 +0000 Subject: [PATCH] SEC-507: Updated JSR-250 impl to include better support for PermitAll and DenyAll as suggested by Ryan Heaton. Includes JSR-250 voter which is now used by AnnotationDriverbeanDefinitionParser. --- .../Jsr250SecurityAnnotationAttributes.java | 61 ++++++++++++--- .../annotation/Jsr250SecurityConfig.java | 22 ++++++ .../security/annotation/Jsr250Voter.java | 77 +++++++++++++++++++ .../security/annotation/BusinessService.java | 3 + .../annotation/Jsr250BusinessServiceImpl.java | 5 +- ...r250SecurityAnnotationAttributesTests.java | 45 +++++++++-- ...tationDrivenBeanDefinitionParserTests.java | 9 +++ .../AnnotationDrivenBeanDefinitionParser.java | 9 ++- .../security/config/ConfigUtils.java | 20 ++++- 9 files changed, 227 insertions(+), 24 deletions(-) create mode 100644 core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityConfig.java create mode 100644 core-tiger/src/main/java/org/springframework/security/annotation/Jsr250Voter.java diff --git a/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributes.java b/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributes.java index d593b8801c..96f843846e 100644 --- a/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributes.java +++ b/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributes.java @@ -6,21 +6,22 @@ import org.springframework.core.annotation.AnnotationUtils; import javax.annotation.security.PermitAll; import javax.annotation.security.RolesAllowed; +import javax.annotation.security.DenyAll; import java.util.Collection; import java.util.HashSet; import java.util.Set; +import java.util.ArrayList; import java.lang.reflect.Method; import java.lang.reflect.Field; import java.lang.annotation.Annotation; /** - * Java 5 Annotation Attributes metadata implementation used for secure method interception. + * Java 5 Annotation {@link Attributes} metadata implementation used for secure method interception using + * the security anotations defined in JSR-250. *

* This Attributes implementation will return security configuration for classes described using the - * RolesAllowed Java JEE 5 annotation. + * Java JEE 5 annotations (DenyAll, PermitAll and RolesAllowed). *

- * The SecurityAnnotationAttributes implementation can be used to configure a - * MethodDefinitionAttributes and MethodSecurityInterceptor bean definition. * * @author Mark St.Godard * @author Usama Rashwan @@ -31,6 +32,7 @@ import java.lang.annotation.Annotation; */ public class Jsr250SecurityAnnotationAttributes implements Attributes { + //~ Methods ======================================================================================================== /** @@ -48,19 +50,56 @@ public class Jsr250SecurityAnnotationAttributes implements Attributes { } /** - * Get the RolesAllowed attributes for a given target method. + * Get the attributes for a given target method, acording to JSR-250 precedence rules. * * @param method The target method * @return Collection of SecurityConfig * @see Attributes#getAttributes */ public Collection getAttributes(Method method) { - Annotation[] annotations = AnnotationUtils.getAnnotations(method); - Collection attributes = populateSecurityConfigWithRolesAllowed(annotations); - // if there is no RolesAllowed defined on the Method then we will use the one defined on the class - // level , according to JSR 250 - if (attributes.size()==0 && !method.isAnnotationPresent(PermitAll.class)) { - attributes = populateSecurityConfigWithRolesAllowed(method.getDeclaringClass().getDeclaredAnnotations()); + ArrayList attributes = new ArrayList(); + + if (AnnotationUtils.getAnnotation(method, DenyAll.class) != null) { + attributes.add(Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE); + + return attributes; + } + + if (AnnotationUtils.getAnnotation(method, PermitAll.class) != null) { + attributes.add(Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE); + + return attributes; + } + + RolesAllowed rolesAllowed = AnnotationUtils.getAnnotation(method, RolesAllowed.class); + + if (rolesAllowed != null) { + for (String role : rolesAllowed.value()) { + attributes.add(new Jsr250SecurityConfig(role)); + } + + return attributes; + } + + // Now check the class-level attributes: + if (method.getDeclaringClass().getAnnotation(DenyAll.class) != null) { + attributes.add(Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE); + + return attributes; + } + + if (method.getDeclaringClass().getAnnotation(PermitAll.class) != null) { + attributes.add(Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE); + + return attributes; + } + + rolesAllowed = method.getDeclaringClass().getAnnotation(RolesAllowed.class); + + if (rolesAllowed != null) { + for (String role : rolesAllowed.value()) { + attributes.add(new Jsr250SecurityConfig(role)); + } } return attributes; diff --git a/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityConfig.java b/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityConfig.java new file mode 100644 index 0000000000..3e2302f47b --- /dev/null +++ b/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250SecurityConfig.java @@ -0,0 +1,22 @@ +package org.springframework.security.annotation; + +import org.springframework.security.SecurityConfig; + +import javax.annotation.security.PermitAll; +import javax.annotation.security.DenyAll; + +/** + * Security config applicable as a JSR 250 annotation attribute. + * + * @author Ryan Heaton + * @since 2.0 + */ +public class Jsr250SecurityConfig extends SecurityConfig { + public static final Jsr250SecurityConfig PERMIT_ALL_ATTRIBUTE = new Jsr250SecurityConfig(PermitAll.class.getName()); + public static final Jsr250SecurityConfig DENY_ALL_ATTRIBUTE = new Jsr250SecurityConfig(DenyAll.class.getName()); + + public Jsr250SecurityConfig(String role) { + super(role); + } + +} \ No newline at end of file diff --git a/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250Voter.java b/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250Voter.java new file mode 100644 index 0000000000..4b573ca2da --- /dev/null +++ b/core-tiger/src/main/java/org/springframework/security/annotation/Jsr250Voter.java @@ -0,0 +1,77 @@ +package org.springframework.security.annotation; + +import org.springframework.security.GrantedAuthority; +import org.springframework.security.ConfigAttribute; +import org.springframework.security.ConfigAttributeDefinition; +import org.springframework.security.Authentication; +import org.springframework.security.vote.AccessDecisionVoter; + +import java.util.Iterator; + +/** + * Voter on JSR-250 configuration attributes. + * + * @author Ryan Heaton + * @since 2.0 + */ +public class Jsr250Voter implements AccessDecisionVoter { + + /** + * The specified config attribute is supported if its an instance of a {@link Jsr250SecurityConfig}. + * + * @param configAttribute The config attribute. + * @return whether the config attribute is supported. + */ + public boolean supports(ConfigAttribute configAttribute) { + return configAttribute instanceof Jsr250SecurityConfig; + } + + /** + * All classes are supported. + * + * @param clazz the class. + * @return true + */ + public boolean supports(Class clazz) { + return true; + } + + /** + * Votes according to JSR 250. + * + * @param authentication The authentication object. + * @param object The access object. + * @param definition The configuration definition. + * @return The vote. + */ + public int vote(Authentication authentication, Object object, ConfigAttributeDefinition definition) { + int result = ACCESS_ABSTAIN; + Iterator iter = definition.getConfigAttributes().iterator(); + + while (iter.hasNext()) { + ConfigAttribute attribute = (ConfigAttribute) iter.next(); + + if (Jsr250SecurityConfig.PERMIT_ALL_ATTRIBUTE.equals(attribute)) { + return ACCESS_GRANTED; + } + + if (Jsr250SecurityConfig.DENY_ALL_ATTRIBUTE.equals(attribute)) { + return ACCESS_DENIED; + } + + if (supports(attribute)) { + result = ACCESS_DENIED; + + // Attempt to find a matching granted authority + for (GrantedAuthority authority : authentication.getAuthorities()) { + if (attribute.getAttribute().equals(authority.getAuthority())) { + return ACCESS_GRANTED; + } + } + } + } + + return result; + } +} + diff --git a/core-tiger/src/test/java/org/springframework/security/annotation/BusinessService.java b/core-tiger/src/test/java/org/springframework/security/annotation/BusinessService.java index 142b896d45..657a03dfc1 100644 --- a/core-tiger/src/test/java/org/springframework/security/annotation/BusinessService.java +++ b/core-tiger/src/test/java/org/springframework/security/annotation/BusinessService.java @@ -16,11 +16,14 @@ package org.springframework.security.annotation; import javax.annotation.security.RolesAllowed; +import javax.annotation.security.DenyAll; +import javax.annotation.security.PermitAll; /** * @version $Id$ */ @Secured({"ROLE_USER"}) +@PermitAll public interface BusinessService { //~ Methods ======================================================================================================== diff --git a/core-tiger/src/test/java/org/springframework/security/annotation/Jsr250BusinessServiceImpl.java b/core-tiger/src/test/java/org/springframework/security/annotation/Jsr250BusinessServiceImpl.java index 88493a3cd9..b757e295c8 100644 --- a/core-tiger/src/test/java/org/springframework/security/annotation/Jsr250BusinessServiceImpl.java +++ b/core-tiger/src/test/java/org/springframework/security/annotation/Jsr250BusinessServiceImpl.java @@ -1,12 +1,15 @@ package org.springframework.security.annotation; import javax.annotation.security.RolesAllowed; +import javax.annotation.security.DenyAll; +import javax.annotation.security.PermitAll; /** * * @author Luke Taylor * @version $Id$ */ +@PermitAll public class Jsr250BusinessServiceImpl implements BusinessService { @RolesAllowed({"ROLE_USER"}) @@ -25,7 +28,7 @@ public class Jsr250BusinessServiceImpl implements BusinessService { public void someAdminMethod() { } - public int someOther(int input) { + public int someOther(int input) { return input; } } \ No newline at end of file diff --git a/core-tiger/src/test/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributesTests.java b/core-tiger/src/test/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributesTests.java index 5dcd36865c..5db75e3d58 100644 --- a/core-tiger/src/test/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributesTests.java +++ b/core-tiger/src/test/java/org/springframework/security/annotation/Jsr250SecurityAnnotationAttributesTests.java @@ -10,6 +10,7 @@ import java.util.ArrayList; import javax.annotation.security.RolesAllowed; import javax.annotation.security.PermitAll; +import javax.annotation.security.DenyAll; /** * @author Luke Taylor @@ -18,7 +19,8 @@ import javax.annotation.security.PermitAll; public class Jsr250SecurityAnnotationAttributesTests { Jsr250SecurityAnnotationAttributes attributes = new Jsr250SecurityAnnotationAttributes(); A a = new A(); - B b = new B(); + UserAllowedClass userAllowed = new UserAllowedClass(); + DenyAllClass denyAll = new DenyAllClass(); @Test public void methodWithRolesAllowedHasCorrectAttribute() throws Exception { @@ -31,10 +33,27 @@ public class Jsr250SecurityAnnotationAttributesTests { } @Test - public void permitAllMethodHasNoAttributes() throws Exception { + public void permitAllMethodHasPermitAllAttribute() throws Exception { List accessAttributes = new ArrayList(attributes.getAttributes(a.getClass().getMethod("permitAllMethod"))); - assertEquals(0, accessAttributes.size()); + assertEquals(1, accessAttributes.size()); + assertEquals("javax.annotation.security.PermitAll", accessAttributes.get(0).getAttribute()); + } + + @Test + public void noRoleMethodHasDenyAllAttributeWithDenyAllClass() throws Exception { + List accessAttributes = + new ArrayList(attributes.getAttributes(denyAll.getClass().getMethod("noRoleMethod"))); + assertEquals(1, accessAttributes.size()); + assertEquals("javax.annotation.security.DenyAll", accessAttributes.get(0).getAttribute()); + } + + @Test + public void adminMethodHasAdminAttributeWithDenyAllClass() throws Exception { + List accessAttributes = + new ArrayList(attributes.getAttributes(denyAll.getClass().getMethod("adminMethod"))); + assertEquals(1, accessAttributes.size()); + assertEquals("ADMIN", accessAttributes.get(0).getAttribute()); } @Test @@ -45,9 +64,9 @@ public class Jsr250SecurityAnnotationAttributesTests { } @Test - public void classRoleIsAppliedNoRoleMethod() throws Exception { + public void classRoleIsAppliedToNoRoleMethod() throws Exception { List accessAttributes = - new ArrayList(attributes.getAttributes(b.getClass().getMethod("noRoleMethod"))); + new ArrayList(attributes.getAttributes(userAllowed.getClass().getMethod("noRoleMethod"))); assertEquals(1, accessAttributes.size()); assertEquals("USER", accessAttributes.get(0).getAttribute()); } @@ -55,7 +74,7 @@ public class Jsr250SecurityAnnotationAttributesTests { @Test public void methodRoleOverridesClassRole() throws Exception { List accessAttributes = - new ArrayList(attributes.getAttributes(b.getClass().getMethod("adminMethod"))); + new ArrayList(attributes.getAttributes(userAllowed.getClass().getMethod("adminMethod"))); assertEquals(1, accessAttributes.size()); assertEquals("ADMIN", accessAttributes.get(0).getAttribute()); } @@ -71,15 +90,25 @@ public class Jsr250SecurityAnnotationAttributesTests { @PermitAll public void permitAllMethod() {} - } @RolesAllowed("USER") - public static class B { + public static class UserAllowedClass { public void noRoleMethod() {} @RolesAllowed("ADMIN") public void adminMethod() {} } + @DenyAll + public static class DenyAllClass { + + public void noRoleMethod() {} + + @RolesAllowed("ADMIN") + public void adminMethod() {} + } + + + } diff --git a/core-tiger/src/test/java/org/springframework/security/config/Jsr250AnnotationDrivenBeanDefinitionParserTests.java b/core-tiger/src/test/java/org/springframework/security/config/Jsr250AnnotationDrivenBeanDefinitionParserTests.java index dcd432d464..df223c92a5 100644 --- a/core-tiger/src/test/java/org/springframework/security/config/Jsr250AnnotationDrivenBeanDefinitionParserTests.java +++ b/core-tiger/src/test/java/org/springframework/security/config/Jsr250AnnotationDrivenBeanDefinitionParserTests.java @@ -40,6 +40,15 @@ public class Jsr250AnnotationDrivenBeanDefinitionParserTests { target.someUserMethod1(); } + @Test + public void permitAllShouldBeDefaultAttribute() { + UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password", + new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_USER")}); + SecurityContextHolder.getContext().setAuthentication(token); + + target.someOther(0); + } + @Test public void targetShouldAllowProtectedMethodInvocationWithCorrectRole() { UsernamePasswordAuthenticationToken token = new UsernamePasswordAuthenticationToken("Test", "Password", diff --git a/core/src/main/java/org/springframework/security/config/AnnotationDrivenBeanDefinitionParser.java b/core/src/main/java/org/springframework/security/config/AnnotationDrivenBeanDefinitionParser.java index e05eb9440a..6b7fe6681d 100644 --- a/core/src/main/java/org/springframework/security/config/AnnotationDrivenBeanDefinitionParser.java +++ b/core/src/main/java/org/springframework/security/config/AnnotationDrivenBeanDefinitionParser.java @@ -25,11 +25,13 @@ import org.w3c.dom.Element; class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParser { public static final String SECURITY_ANNOTATION_ATTRIBUTES_CLASS = "org.springframework.security.annotation.SecurityAnnotationAttributes"; public static final String JSR_250_SECURITY_ANNOTATION_ATTRIBUTES_CLASS = "org.springframework.security.annotation.Jsr250SecurityAnnotationAttributes"; + public static final String JSR_250_VOTER_CLASS = "org.springframework.security.annotation.Jsr250Voter"; private static final String ATT_ACCESS_MGR = "access-decision-manager-ref"; private static final String ATT_USE_JSR250 = "jsr250"; public BeanDefinition parse(Element element, ParserContext parserContext) { - String className = "true".equals(element.getAttribute(ATT_USE_JSR250)) ? + boolean useJsr250 = "true".equals(element.getAttribute(ATT_USE_JSR250)); + String className = useJsr250 ? JSR_250_SECURITY_ANNOTATION_ATTRIBUTES_CLASS : SECURITY_ANNOTATION_ATTRIBUTES_CLASS; // Reflectively obtain the Annotation-based ObjectDefinitionSource. @@ -56,6 +58,11 @@ class AnnotationDrivenBeanDefinitionParser implements BeanDefinitionParser { if (!StringUtils.hasText(accessManagerId)) { ConfigUtils.registerDefaultAccessManagerIfNecessary(parserContext); + + if (useJsr250) { + ConfigUtils.addVoter(new RootBeanDefinition(JSR_250_VOTER_CLASS, null, null), parserContext); + } + accessManagerId = BeanIds.ACCESS_MANAGER; } diff --git a/core/src/main/java/org/springframework/security/config/ConfigUtils.java b/core/src/main/java/org/springframework/security/config/ConfigUtils.java index f614078266..041992a3f2 100644 --- a/core/src/main/java/org/springframework/security/config/ConfigUtils.java +++ b/core/src/main/java/org/springframework/security/config/ConfigUtils.java @@ -16,7 +16,6 @@ import org.springframework.security.vote.RoleVoter; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import java.util.Arrays; import java.util.Map; /** @@ -32,15 +31,30 @@ public abstract class ConfigUtils { static void registerDefaultAccessManagerIfNecessary(ParserContext parserContext) { if (!parserContext.getRegistry().containsBeanDefinition(BeanIds.ACCESS_MANAGER)) { + ManagedList defaultVoters = new ManagedList(2); + + defaultVoters.add(new RootBeanDefinition(RoleVoter.class)); + defaultVoters.add(new RootBeanDefinition(AuthenticatedVoter.class)); + BeanDefinitionBuilder accessMgrBuilder = BeanDefinitionBuilder.rootBeanDefinition(AffirmativeBased.class); - accessMgrBuilder.addPropertyValue("decisionVoters", - Arrays.asList(new Object[] {new RoleVoter(), new AuthenticatedVoter()})); + accessMgrBuilder.addPropertyValue("decisionVoters", defaultVoters); BeanDefinition accessMgr = accessMgrBuilder.getBeanDefinition(); parserContext.getRegistry().registerBeanDefinition(BeanIds.ACCESS_MANAGER, accessMgr); } } + public static void addVoter(BeanDefinition voter, ParserContext parserContext) { + registerDefaultAccessManagerIfNecessary(parserContext); + + BeanDefinition accessMgr = parserContext.getRegistry().getBeanDefinition(BeanIds.ACCESS_MANAGER); + + ManagedList voters = (ManagedList) accessMgr.getPropertyValues().getPropertyValue("decisionVoters").getValue(); + voters.add(voter); + + accessMgr.getPropertyValues().addPropertyValue("decisionVoters", voter); + } + /** * Creates and registers the bean definition for the default ProviderManager instance and returns * the BeanDefinition for it. This method will typically be called when registering authentication providers