diff --git a/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java b/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java index 4d7fb1924d..26b78292d9 100644 --- a/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java @@ -34,10 +34,12 @@ import org.springframework.util.Assert; /** - * Default implementation of {@link AclAuthorizationStrategy}.

Permission will be granted provided the current - * principal is either the owner (as defined by the ACL), has {@link BasePermission#ADMINISTRATION} (as defined by the - * ACL and via a {@link Sid} retrieved for the current principal via {@link #sidRetrievalStrategy}), or if the current - * principal holds the relevant system-wide {@link GrantedAuthority} and injected into the constructor.

+ * Default implementation of {@link AclAuthorizationStrategy}. + *

+ * Permission will be granted provided the current principal is either the owner (as defined by the ACL), has + * {@link BasePermission#ADMINISTRATION} (as defined by the ACL and via a {@link Sid} retrieved for the current + * principal via {@link #sidRetrievalStrategy}), or if the current principal holds the relevant system-wide + * {@link GrantedAuthority} and injected into the constructor. * * @author Ben Alex * @version $Id$ @@ -52,7 +54,7 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy { //~ Constructors =================================================================================================== -/** + /** * Constructor. The only mandatory parameter relates to the system-wide {@link GrantedAuthority} instances that * can be held to always permit ACL changes. * @@ -62,8 +64,7 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy { * index 2 is the authority needed to change other ACL and ACE details) (required) */ public AclAuthorizationStrategyImpl(GrantedAuthority[] auths) { - Assert.notEmpty(auths, "GrantedAuthority[] with three elements required"); - Assert.isTrue(auths.length == 3, "GrantedAuthority[] with three elements required"); + Assert.isTrue(auths != null && auths.length == 3, "GrantedAuthority[] with three elements required"); this.gaTakeOwnership = auths[0]; this.gaModifyAuditing = auths[1]; this.gaGeneralChanges = auths[2]; diff --git a/acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java b/acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java index 32686b5632..f7787f0694 100644 --- a/acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java +++ b/acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java @@ -19,278 +19,243 @@ import org.springframework.security.providers.TestingAuthenticationToken; /** * Test class for {@link AclAuthorizationStrategyImpl} and {@link AclImpl} * security checks. - * + * * @author Andrei Stefan */ public class AclImplementationSecurityCheckTests extends TestCase { - - //~ Methods ======================================================================================================== - protected void setUp() throws Exception { - SecurityContextHolder.clearContext(); - } + //~ Methods ======================================================================================================== - protected void tearDown() throws Exception { - SecurityContextHolder.clearContext(); - } + protected void setUp() throws Exception { + SecurityContextHolder.clearContext(); + } - public void testSecurityCheckNoACEs() throws Exception { - Authentication auth = new TestingAuthenticationToken("user", "password", new GrantedAuthority[] { - new GrantedAuthorityImpl("ROLE_GENERAL"), new GrantedAuthorityImpl("ROLE_AUDITING"), - new GrantedAuthorityImpl("ROLE_OWNERSHIP") }); - auth.setAuthenticated(true); - SecurityContextHolder.getContext().setAuthentication(auth); + protected void tearDown() throws Exception { + SecurityContextHolder.clearContext(); + } - ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); - AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { - new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"), - new GrantedAuthorityImpl("ROLE_GENERAL") }); + public void testSecurityCheckNoACEs() throws Exception { + Authentication auth = new TestingAuthenticationToken("user", "password","ROLE_GENERAL","ROLE_AUDITING","ROLE_OWNERSHIP"); + auth.setAuthenticated(true); + SecurityContextHolder.getContext().setAuthentication(auth); - Acl acl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); - try { - aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_GENERAL); - Assert.assertTrue(true); - } - catch (AccessDeniedException notExpected) { - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } - try { - aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_AUDITING); - Assert.assertTrue(true); - } - catch (AccessDeniedException notExpected) { - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } - try { - aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); - Assert.assertTrue(true); - } - catch (AccessDeniedException notExpected) { - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } + ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); + AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { + new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"), + new GrantedAuthorityImpl("ROLE_GENERAL") }); - // Create another authorization strategy - AclAuthorizationStrategy aclAuthorizationStrategy2 = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { - new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO"), - new GrantedAuthorityImpl("ROLE_THREE") }); - Acl acl2 = new AclImpl(identity, new Long(1), aclAuthorizationStrategy2, new ConsoleAuditLogger()); - // Check access in case the principal has no authorization rights - try { - aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_GENERAL); - Assert.fail("It should have thrown NotFoundException"); - } - catch (NotFoundException expected) { - Assert.assertTrue(true); - } - try { - aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_AUDITING); - Assert.fail("It should have thrown NotFoundException"); - } - catch (NotFoundException expected) { - Assert.assertTrue(true); - } - try { - aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_OWNERSHIP); - Assert.fail("It should have thrown NotFoundException"); - } - catch (NotFoundException expected) { - Assert.assertTrue(true); - } - } + Acl acl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); - public void testSecurityCheckWithMultipleACEs() throws Exception { - // Create a simple authentication with ROLE_GENERAL - Authentication auth = new TestingAuthenticationToken("user", "password", - new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_GENERAL") }); - auth.setAuthenticated(true); - SecurityContextHolder.getContext().setAuthentication(auth); + aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_GENERAL); + aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_AUDITING); + aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); - ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); - // Authorization strategy will require a different role for each access - AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { - new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"), - new GrantedAuthorityImpl("ROLE_GENERAL") }); + // Create another authorization strategy + AclAuthorizationStrategy aclAuthorizationStrategy2 = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { + new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO"), + new GrantedAuthorityImpl("ROLE_THREE") }); + Acl acl2 = new AclImpl(identity, new Long(1), aclAuthorizationStrategy2, new ConsoleAuditLogger()); + // Check access in case the principal has no authorization rights + try { + aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_GENERAL); + Assert.fail("It should have thrown NotFoundException"); + } + catch (NotFoundException expected) { + } + try { + aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_AUDITING); + Assert.fail("It should have thrown NotFoundException"); + } + catch (NotFoundException expected) { + } + try { + aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_OWNERSHIP); + Assert.fail("It should have thrown NotFoundException"); + } + catch (NotFoundException expected) { + } + } - // Let's give the principal the ADMINISTRATION permission, without - // granting access - MutableAcl aclFirstDeny = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); - aclFirstDeny.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false); + public void testSecurityCheckWithMultipleACEs() throws Exception { + // Create a simple authentication with ROLE_GENERAL + Authentication auth = new TestingAuthenticationToken("user", "password", + new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_GENERAL") }); + auth.setAuthenticated(true); + SecurityContextHolder.getContext().setAuthentication(auth); - // The CHANGE_GENERAL test should pass as the principal has ROLE_GENERAL - try { - aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_GENERAL); - Assert.assertTrue(true); - } - catch (AccessDeniedException notExpected) { - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } - // The CHANGE_AUDITING and CHANGE_OWNERSHIP should fail since the - // principal doesn't have these authorities, - // nor granting access - try { - aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_AUDITING); - Assert.fail("It should have thrown AccessDeniedException"); - } - catch (AccessDeniedException expected) { - Assert.assertTrue(true); - } - try { - aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_OWNERSHIP); - Assert.fail("It should have thrown AccessDeniedException"); - } - catch (AccessDeniedException expected) { - Assert.assertTrue(true); - } + ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); + // Authorization strategy will require a different role for each access + AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { + new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"), + new GrantedAuthorityImpl("ROLE_GENERAL") }); - // Add granting access to this principal - aclFirstDeny.insertAce(1, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); - // and try again for CHANGE_AUDITING - the first ACE's granting flag - // (false) will deny this access - try { - aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_AUDITING); - Assert.fail("It should have thrown AccessDeniedException"); - } - catch (AccessDeniedException expected) { - Assert.assertTrue(true); - } + // Let's give the principal the ADMINISTRATION permission, without + // granting access + MutableAcl aclFirstDeny = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); + aclFirstDeny.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false); - // Create another ACL and give the principal the ADMINISTRATION - // permission, with granting access - MutableAcl aclFirstAllow = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, - new ConsoleAuditLogger()); - aclFirstAllow.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); + // The CHANGE_GENERAL test should pass as the principal has ROLE_GENERAL + aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_GENERAL); - // The CHANGE_AUDITING test should pass as there is one ACE with - // granting access - try { - aclAuthorizationStrategy.securityCheck(aclFirstAllow, AclAuthorizationStrategy.CHANGE_AUDITING); - Assert.assertTrue(true); - } - catch (AccessDeniedException notExpected) { - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } + // The CHANGE_AUDITING and CHANGE_OWNERSHIP should fail since the + // principal doesn't have these authorities, + // nor granting access + try { + aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_AUDITING); + Assert.fail("It should have thrown AccessDeniedException"); + } + catch (AccessDeniedException expected) { + } + try { + aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_OWNERSHIP); + Assert.fail("It should have thrown AccessDeniedException"); + } + catch (AccessDeniedException expected) { + } - // Add a deny ACE and test again for CHANGE_AUDITING - aclFirstAllow.insertAce(1, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false); - try { - aclAuthorizationStrategy.securityCheck(aclFirstAllow, AclAuthorizationStrategy.CHANGE_AUDITING); - Assert.assertTrue(true); - } - catch (AccessDeniedException notExpected) { - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } + // Add granting access to this principal + aclFirstDeny.insertAce(1, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); + // and try again for CHANGE_AUDITING - the first ACE's granting flag + // (false) will deny this access + try { + aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_AUDITING); + Assert.fail("It should have thrown AccessDeniedException"); + } + catch (AccessDeniedException expected) { + } - // Create an ACL with no ACE - MutableAcl aclNoACE = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); - try { - aclAuthorizationStrategy.securityCheck(aclNoACE, AclAuthorizationStrategy.CHANGE_AUDITING); - Assert.fail("It should have thrown NotFoundException"); - } - catch (NotFoundException expected) { - Assert.assertTrue(true); - } - // and still grant access for CHANGE_GENERAL - try { - aclAuthorizationStrategy.securityCheck(aclNoACE, AclAuthorizationStrategy.CHANGE_GENERAL); - Assert.assertTrue(true); - } - catch (NotFoundException expected) { - Assert.fail("It shouldn't have thrown NotFoundException"); - } - } + // Create another ACL and give the principal the ADMINISTRATION + // permission, with granting access + MutableAcl aclFirstAllow = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, + new ConsoleAuditLogger()); + aclFirstAllow.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); - public void testSecurityCheckWithInheritableACEs() throws Exception { - // Create a simple authentication with ROLE_GENERAL - Authentication auth = new TestingAuthenticationToken("user", "password", - new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_GENERAL") }); - auth.setAuthenticated(true); - SecurityContextHolder.getContext().setAuthentication(auth); + // The CHANGE_AUDITING test should pass as there is one ACE with + // granting access - ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); - // Authorization strategy will require a different role for each access - AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { - new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO"), - new GrantedAuthorityImpl("ROLE_GENERAL") }); + aclAuthorizationStrategy.securityCheck(aclFirstAllow, AclAuthorizationStrategy.CHANGE_AUDITING); - // Let's give the principal an ADMINISTRATION permission, with granting - // access - MutableAcl parentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); - parentAcl.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); - MutableAcl childAcl = new AclImpl(identity, new Long(2), aclAuthorizationStrategy, new ConsoleAuditLogger()); + // Add a deny ACE and test again for CHANGE_AUDITING + aclFirstAllow.insertAce(1, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false); + try { + aclAuthorizationStrategy.securityCheck(aclFirstAllow, AclAuthorizationStrategy.CHANGE_AUDITING); + Assert.assertTrue(true); + } + catch (AccessDeniedException notExpected) { + Assert.fail("It shouldn't have thrown AccessDeniedException"); + } - // Check against the 'child' acl, which doesn't offer any authorization - // rights on CHANGE_OWNERSHIP - try { - aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); - Assert.fail("It should have thrown NotFoundException"); - } - catch (NotFoundException expected) { - Assert.assertTrue(true); - } + // Create an ACL with no ACE + MutableAcl aclNoACE = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); + try { + aclAuthorizationStrategy.securityCheck(aclNoACE, AclAuthorizationStrategy.CHANGE_AUDITING); + Assert.fail("It should have thrown NotFoundException"); + } + catch (NotFoundException expected) { + Assert.assertTrue(true); + } + // and still grant access for CHANGE_GENERAL + try { + aclAuthorizationStrategy.securityCheck(aclNoACE, AclAuthorizationStrategy.CHANGE_GENERAL); + Assert.assertTrue(true); + } + catch (NotFoundException expected) { + Assert.fail("It shouldn't have thrown NotFoundException"); + } + } - // Link the child with its parent and test again against the - // CHANGE_OWNERSHIP right - childAcl.setParent(parentAcl); - childAcl.setEntriesInheriting(true); - try { - aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); - Assert.assertTrue(true); - } - catch (NotFoundException expected) { - Assert.fail("It shouldn't have thrown NotFoundException"); - } + public void testSecurityCheckWithInheritableACEs() throws Exception { + // Create a simple authentication with ROLE_GENERAL + Authentication auth = new TestingAuthenticationToken("user", "password", + new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_GENERAL") }); + auth.setAuthenticated(true); + SecurityContextHolder.getContext().setAuthentication(auth); - // Create a root parent and link it to the middle parent - MutableAcl rootParentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, - new ConsoleAuditLogger()); - parentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); - rootParentAcl.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); - parentAcl.setEntriesInheriting(true); - parentAcl.setParent(rootParentAcl); - childAcl.setParent(parentAcl); - try { - aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); - Assert.assertTrue(true); - } - catch (NotFoundException expected) { - Assert.fail("It shouldn't have thrown NotFoundException"); - } - } + ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); + // Authorization strategy will require a different role for each access + AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { + new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_TWO"), + new GrantedAuthorityImpl("ROLE_GENERAL") }); - public void testSecurityCheckPrincipalOwner() throws Exception { - Authentication auth = new TestingAuthenticationToken("user", "password", new GrantedAuthority[] { - new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_ONE"), - new GrantedAuthorityImpl("ROLE_ONE") }); - auth.setAuthenticated(true); - SecurityContextHolder.getContext().setAuthentication(auth); + // Let's give the principal an ADMINISTRATION permission, with granting + // access + MutableAcl parentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); + parentAcl.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); + MutableAcl childAcl = new AclImpl(identity, new Long(2), aclAuthorizationStrategy, new ConsoleAuditLogger()); - ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); - AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { - new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"), - new GrantedAuthorityImpl("ROLE_GENERAL") }); + // Check against the 'child' acl, which doesn't offer any authorization + // rights on CHANGE_OWNERSHIP + try { + aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); + Assert.fail("It should have thrown NotFoundException"); + } + catch (NotFoundException expected) { + Assert.assertTrue(true); + } - Acl acl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger(), null, null, - false, new PrincipalSid(auth)); - try { - aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_GENERAL); - Assert.assertTrue(true); - } - catch (AccessDeniedException notExpected) { - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } - try { - aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_AUDITING); - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } - catch (NotFoundException expected) { - Assert.assertTrue(true); - } - try { - aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); - Assert.assertTrue(true); - } - catch (AccessDeniedException notExpected) { - Assert.fail("It shouldn't have thrown AccessDeniedException"); - } - } + // Link the child with its parent and test again against the + // CHANGE_OWNERSHIP right + childAcl.setParent(parentAcl); + childAcl.setEntriesInheriting(true); + try { + aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); + Assert.assertTrue(true); + } + catch (NotFoundException expected) { + Assert.fail("It shouldn't have thrown NotFoundException"); + } + + // Create a root parent and link it to the middle parent + MutableAcl rootParentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, + new ConsoleAuditLogger()); + parentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); + rootParentAcl.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); + parentAcl.setEntriesInheriting(true); + parentAcl.setParent(rootParentAcl); + childAcl.setParent(parentAcl); + try { + aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); + Assert.assertTrue(true); + } + catch (NotFoundException expected) { + Assert.fail("It shouldn't have thrown NotFoundException"); + } + } + + public void testSecurityCheckPrincipalOwner() throws Exception { + Authentication auth = new TestingAuthenticationToken("user", "password", new GrantedAuthority[] { + new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_ONE"), + new GrantedAuthorityImpl("ROLE_ONE") }); + auth.setAuthenticated(true); + SecurityContextHolder.getContext().setAuthentication(auth); + + ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); + AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { + new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"), + new GrantedAuthorityImpl("ROLE_GENERAL") }); + + Acl acl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger(), null, null, + false, new PrincipalSid(auth)); + try { + aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_GENERAL); + Assert.assertTrue(true); + } + catch (AccessDeniedException notExpected) { + Assert.fail("It shouldn't have thrown AccessDeniedException"); + } + try { + aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_AUDITING); + Assert.fail("It shouldn't have thrown AccessDeniedException"); + } + catch (NotFoundException expected) { + Assert.assertTrue(true); + } + try { + aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_OWNERSHIP); + Assert.assertTrue(true); + } + catch (AccessDeniedException notExpected) { + Assert.fail("It shouldn't have thrown AccessDeniedException"); + } + } } diff --git a/core/src/main/java/org/springframework/security/expression/ExpressionUtils.java b/core/src/main/java/org/springframework/security/expression/ExpressionUtils.java index 1492382785..eee5d05f30 100644 --- a/core/src/main/java/org/springframework/security/expression/ExpressionUtils.java +++ b/core/src/main/java/org/springframework/security/expression/ExpressionUtils.java @@ -5,12 +5,12 @@ import java.util.Collection; import java.util.HashSet; import java.util.Set; +import org.springframework.expression.EvaluationContext; import org.springframework.expression.EvaluationException; import org.springframework.expression.Expression; -import org.springframework.expression.spel.standard.StandardEvaluationContext; public class ExpressionUtils { - public static Object doFilter(Object filterTarget, Expression filterExpression, StandardEvaluationContext ctx) { + public static Object doFilter(Object filterTarget, Expression filterExpression, EvaluationContext ctx) { SecurityExpressionRoot rootObject = (SecurityExpressionRoot) ctx.getRootContextObject(); Set removeList = new HashSet(); @@ -55,7 +55,7 @@ public class ExpressionUtils { throw new IllegalArgumentException("Filter target must be a collection or array type, but was " + filterTarget); } - public static boolean evaluateAsBoolean(Expression expr, StandardEvaluationContext ctx) { + public static boolean evaluateAsBoolean(Expression expr, EvaluationContext ctx) { try { return ((Boolean) expr.getValue(ctx, Boolean.class)).booleanValue(); } catch (EvaluationException e) { diff --git a/core/src/main/java/org/springframework/security/expression/SecurityEvaluationContext.java b/core/src/main/java/org/springframework/security/expression/SecurityEvaluationContext.java new file mode 100644 index 0000000000..36c5f1bc07 --- /dev/null +++ b/core/src/main/java/org/springframework/security/expression/SecurityEvaluationContext.java @@ -0,0 +1,48 @@ +package org.springframework.security.expression; + +import java.lang.reflect.Method; + +import org.aopalliance.intercept.MethodInvocation; +import org.springframework.core.LocalVariableTableParameterNameDiscoverer; +import org.springframework.core.ParameterNameDiscoverer; +import org.springframework.expression.spel.standard.StandardEvaluationContext; +import org.springframework.security.Authentication; +import org.springframework.util.ClassUtils; + +/** + * + * @author Luke Taylor + * @since 2.5 + */ +public class SecurityEvaluationContext extends StandardEvaluationContext { + + private ParameterNameDiscoverer parameterNameDiscoverer = new LocalVariableTableParameterNameDiscoverer(); + private boolean argumentsAdded; + private MethodInvocation mi; + + public SecurityEvaluationContext(Authentication user, MethodInvocation mi) { + setRootObject(new SecurityExpressionRoot(user)); + this.mi = mi; + } + + @Override + public Object lookupVariable(String name) { + if (!argumentsAdded) { + addArgumentsAsVariables(); + } + + return super.lookupVariable(name); + } + + private void addArgumentsAsVariables() { + Object[] args = mi.getArguments(); + Object targetObject = mi.getThis(); + Method method = ClassUtils.getMostSpecificMethod(mi.getMethod(), targetObject.getClass()); + String[] paramNames = parameterNameDiscoverer.getParameterNames(method); + + for(int i=0; i < args.length; i++) { + super.setVariable(paramNames[i], args[i]); + } + } + +} diff --git a/core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java b/core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java index a77d4e354e..d53a2d1126 100644 --- a/core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java +++ b/core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java @@ -11,7 +11,11 @@ import java.lang.annotation.Target; * Annotation for specifying a method filtering expression which will be evaluated before a method has been invoked. * The name of the argument to be filtered is specified using the filterTarget attribute. This must be a * Java Collection implementation which supports the {@link java.util.Collection#remove(Object) remove} method. - * Pre-filtering isn't supported on array types. + * Pre-filtering isn't supported on array types and will fail if the value of named filter target argument is null + * at runtime. + *

+ * For methods which have a single argument which is a collection type, this argument will be used as the filter + * target. *

* The annotation value contains the expression which will be evaluated for each element in the collection. If the * expression evaluates to false, the element will be removed. The reserved name "filterObject" can be used within the @@ -32,7 +36,8 @@ public @interface PreFilter { public String value(); /** - * @return the name of the parameter which should be filtered (must be an array or collection) + * @return the name of the parameter which should be filtered (must be a non-null collection instance) + * If the method contains a single collection argument, then this attribute can be omitted. */ - public String filterTarget(); + public String filterTarget() default ""; } diff --git a/core/src/main/java/org/springframework/security/expression/support/AbstractExpressionBasedMethodConfigAttribute.java b/core/src/main/java/org/springframework/security/expression/support/AbstractExpressionBasedMethodConfigAttribute.java index 658f538847..d1f2d071c1 100644 --- a/core/src/main/java/org/springframework/security/expression/support/AbstractExpressionBasedMethodConfigAttribute.java +++ b/core/src/main/java/org/springframework/security/expression/support/AbstractExpressionBasedMethodConfigAttribute.java @@ -21,6 +21,9 @@ abstract class AbstractExpressionBasedMethodConfigAttribute implements ConfigAtt private final Expression filterExpression; private final Expression authorizeExpression; + /** + * Parses the supplied expressions as Spring-EL. + */ AbstractExpressionBasedMethodConfigAttribute(String filterExpression, String authorizeExpression) throws ParseException { Assert.isTrue(filterExpression != null || authorizeExpression != null, "Filter and authorization Expressions cannot both be null"); SpelExpressionParser parser = new SpelExpressionParser(); @@ -28,6 +31,13 @@ abstract class AbstractExpressionBasedMethodConfigAttribute implements ConfigAtt this.authorizeExpression = authorizeExpression == null ? null : parser.parseExpression(authorizeExpression); } + AbstractExpressionBasedMethodConfigAttribute(Expression filterExpression, Expression authorizeExpression) throws ParseException { + Assert.isTrue(filterExpression != null || authorizeExpression != null, "Filter and authorization Expressions cannot both be null"); + SpelExpressionParser parser = new SpelExpressionParser(); + this.filterExpression = filterExpression == null ? null : filterExpression; + this.authorizeExpression = authorizeExpression == null ? null : authorizeExpression; + } + Expression getFilterExpression() { return filterExpression; } diff --git a/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java b/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java index c4c1473069..38e27cbb82 100644 --- a/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java +++ b/core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java @@ -122,9 +122,9 @@ public class ExpressionAnnotationMethodDefinitionSource extends AbstractMethodDe String postFilterExpression = postFilter == null ? null : postFilter.value(); try { - pre = new PreInvocationExpressionConfigAttribute(preFilterExpression, filterObject, preAuthorizeExpression); + pre = new PreInvocationExpressionAttribute(preFilterExpression, filterObject, preAuthorizeExpression); if (postFilterExpression != null || postAuthorizeExpression != null) { - post = new PostInvocationExpressionConfigAttribute(postFilterExpression, postAuthorizeExpression); + post = new PostInvocationExpressionAttribute(postFilterExpression, postAuthorizeExpression); } } catch (ParseException e) { throw new SecurityConfigurationException("Failed to parse expression '" + e.getExpressionString() + "'", e); diff --git a/core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java b/core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java index a62eaf0e91..ccc12855f8 100644 --- a/core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java +++ b/core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java @@ -35,7 +35,7 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP public Object decide(Authentication authentication, Object object, List config, Object returnedObject) throws AccessDeniedException { - PostInvocationExpressionConfigAttribute mca = findMethodAccessControlExpression(config); + PostInvocationExpressionAttribute mca = findMethodAccessControlExpression(config); if (mca == null) { return returnedObject; @@ -86,11 +86,11 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP } } - private PostInvocationExpressionConfigAttribute findMethodAccessControlExpression(List config) { + private PostInvocationExpressionAttribute findMethodAccessControlExpression(List config) { // Find the MethodAccessControlExpression attribute for (ConfigAttribute attribute : config) { - if (attribute instanceof PostInvocationExpressionConfigAttribute) { - return (PostInvocationExpressionConfigAttribute)attribute; + if (attribute instanceof PostInvocationExpressionAttribute) { + return (PostInvocationExpressionAttribute)attribute; } } @@ -98,7 +98,7 @@ public class MethodExpressionAfterInvocationProvider implements AfterInvocationP } public boolean supports(ConfigAttribute attribute) { - return attribute instanceof PostInvocationExpressionConfigAttribute; + return attribute instanceof PostInvocationExpressionAttribute; } public boolean supports(Class clazz) { diff --git a/core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java b/core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java index 24ed50d40f..79f6de5026 100644 --- a/core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java +++ b/core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java @@ -1,22 +1,18 @@ package org.springframework.security.expression.support; -import java.lang.reflect.Method; +import java.util.Collection; import java.util.List; import org.aopalliance.intercept.MethodInvocation; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.core.LocalVariableTableParameterNameDiscoverer; -import org.springframework.core.ParameterNameDiscoverer; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; -import org.springframework.expression.spel.standard.StandardEvaluationContext; import org.springframework.security.Authentication; import org.springframework.security.ConfigAttribute; import org.springframework.security.expression.ExpressionUtils; -import org.springframework.security.expression.SecurityExpressionRoot; +import org.springframework.security.expression.SecurityEvaluationContext; import org.springframework.security.vote.AccessDecisionVoter; -import org.springframework.util.ClassUtils; /** * Voter which performs the actions for @PreFilter and @PostAuthorize annotations. @@ -32,9 +28,6 @@ import org.springframework.util.ClassUtils; public class MethodExpressionVoter implements AccessDecisionVoter { protected final Log logger = LogFactory.getLog(getClass()); - // TODO: Share this between classes - private ParameterNameDiscoverer parameterNameDiscoverer = new LocalVariableTableParameterNameDiscoverer(); - public boolean supports(ConfigAttribute attribute) { return attribute instanceof AbstractExpressionBasedMethodConfigAttribute; } @@ -44,24 +37,21 @@ public class MethodExpressionVoter implements AccessDecisionVoter { } public int vote(Authentication authentication, Object object, List attributes) { - PreInvocationExpressionConfigAttribute mace = findMethodAccessControlExpression(attributes); + PreInvocationExpressionAttribute mace = findMethodAccessControlExpression(attributes); if (mace == null) { // No expression based metadata, so abstain return ACCESS_ABSTAIN; } - StandardEvaluationContext ctx = new StandardEvaluationContext(); - Object filterTarget = - populateContextVariablesAndFindFilterTarget(ctx, (MethodInvocation)object, mace.getFilterTarget()); - - ctx.setRootObject(new SecurityExpressionRoot(authentication)); - + MethodInvocation mi = (MethodInvocation)object; + EvaluationContext ctx = new SecurityEvaluationContext(authentication, mi); Expression preFilter = mace.getFilterExpression(); Expression preAuthorize = mace.getAuthorizeExpression(); if (preFilter != null) { - // TODO: Allow null target if only single parameter, or single collection/array? + Object filterTarget = findFilterTarget(mace.getFilterTarget(), ctx, mi); + ExpressionUtils.doFilter(filterTarget, preFilter, ctx); } @@ -72,41 +62,40 @@ public class MethodExpressionVoter implements AccessDecisionVoter { return ExpressionUtils.evaluateAsBoolean(preAuthorize, ctx) ? ACCESS_GRANTED : ACCESS_DENIED; } - private Object populateContextVariablesAndFindFilterTarget(EvaluationContext ctx, MethodInvocation mi, - String filterTargetName) { - - Object[] args = mi.getArguments(); - Object targetObject = mi.getThis(); - Method method = ClassUtils.getMostSpecificMethod(mi.getMethod(), targetObject.getClass()); + private Object findFilterTarget(String filterTargetName, EvaluationContext ctx, MethodInvocation mi) { Object filterTarget = null; - String[] paramNames = parameterNameDiscoverer.getParameterNames(method); - - for(int i=0; i < args.length; i++) { - ctx.setVariable(paramNames[i], args[i]); - if (filterTargetName != null && paramNames[i].equals(filterTargetName)) { - filterTarget = args[i]; - } - } - if (filterTargetName != null) { + if (filterTargetName.length() > 0) { + filterTarget = ctx.lookupVariable(filterTargetName); if (filterTarget == null) { - throw new IllegalArgumentException("No filter target argument with name " + filterTargetName + - " found in method: " + method.getName()); + throw new IllegalArgumentException("Filter target was null, or no argument with name " + + filterTargetName + " found in method"); + } + } else if (mi.getArguments().length == 1) { + Object arg = mi.getArguments()[0]; + if (arg.getClass().isArray() || + arg instanceof Collection) { + filterTarget = arg; } - if (filterTarget.getClass().isArray()) { - throw new IllegalArgumentException("Pre-filtering on array types is not supported. Changing '" + - filterTargetName +"' to a collection will solve this problem"); + if (filterTarget == null) { + throw new IllegalArgumentException("A PreFilter expression was set but the method argument type" + + arg.getClass() + " is not filterable"); } } + if (filterTarget.getClass().isArray()) { + throw new IllegalArgumentException("Pre-filtering on array types is not supported. " + + "Using a Collection will solve this problem"); + } + return filterTarget; } - private PreInvocationExpressionConfigAttribute findMethodAccessControlExpression(List config) { + private PreInvocationExpressionAttribute findMethodAccessControlExpression(List config) { // Find the MethodAccessControlExpression attribute for (ConfigAttribute attribute : config) { - if (attribute instanceof PreInvocationExpressionConfigAttribute) { - return (PreInvocationExpressionConfigAttribute)attribute; + if (attribute instanceof PreInvocationExpressionAttribute) { + return (PreInvocationExpressionAttribute)attribute; } } diff --git a/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java b/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionAttribute.java similarity index 51% rename from core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java rename to core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionAttribute.java index 8fd273e21a..8688753e43 100644 --- a/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java +++ b/core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionAttribute.java @@ -2,9 +2,9 @@ package org.springframework.security.expression.support; import org.springframework.expression.ParseException; -class PostInvocationExpressionConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute { +class PostInvocationExpressionAttribute extends AbstractExpressionBasedMethodConfigAttribute { - PostInvocationExpressionConfigAttribute(String filterExpression, String authorizeExpression) + PostInvocationExpressionAttribute(String filterExpression, String authorizeExpression) throws ParseException { super(filterExpression, authorizeExpression); } diff --git a/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java b/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionAttribute.java similarity index 71% rename from core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java rename to core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionAttribute.java index 8ba1140bd9..3015c333b0 100644 --- a/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java +++ b/core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionAttribute.java @@ -2,10 +2,10 @@ package org.springframework.security.expression.support; import org.springframework.expression.ParseException; -class PreInvocationExpressionConfigAttribute extends AbstractExpressionBasedMethodConfigAttribute { +class PreInvocationExpressionAttribute extends AbstractExpressionBasedMethodConfigAttribute { private final String filterTarget; - PreInvocationExpressionConfigAttribute(String filterExpression, String filterTarget, String authorizeExpression) + PreInvocationExpressionAttribute(String filterExpression, String filterTarget, String authorizeExpression) throws ParseException { super(filterExpression, authorizeExpression); diff --git a/core/src/main/java/org/springframework/security/expression/support/WebExpressionConfigAttribute.java b/core/src/main/java/org/springframework/security/expression/support/WebExpressionConfigAttribute.java new file mode 100644 index 0000000000..e85911091d --- /dev/null +++ b/core/src/main/java/org/springframework/security/expression/support/WebExpressionConfigAttribute.java @@ -0,0 +1,27 @@ +package org.springframework.security.expression.support; + +import org.springframework.expression.Expression; +import org.springframework.expression.ParseException; +import org.springframework.expression.spel.SpelExpressionParser; +import org.springframework.security.ConfigAttribute; + +public class WebExpressionConfigAttribute implements ConfigAttribute { + private final Expression authorizeExpression; + + public WebExpressionConfigAttribute(String authorizeExpression) throws ParseException { + this.authorizeExpression = new SpelExpressionParser().parseExpression(authorizeExpression); + } + + public WebExpressionConfigAttribute(Expression authorizeExpression) { + this.authorizeExpression = authorizeExpression; + } + + Expression getAuthorizeExpression() { + return authorizeExpression; + } + + public String getAttribute() { + return null; + } + +} diff --git a/core/src/main/java/org/springframework/security/expression/support/WebExpressionVoter.java b/core/src/main/java/org/springframework/security/expression/support/WebExpressionVoter.java new file mode 100644 index 0000000000..79e8966cff --- /dev/null +++ b/core/src/main/java/org/springframework/security/expression/support/WebExpressionVoter.java @@ -0,0 +1,26 @@ +package org.springframework.security.expression.support; + +import java.util.List; + +import org.springframework.security.Authentication; +import org.springframework.security.ConfigAttribute; +import org.springframework.security.intercept.web.FilterInvocation; +import org.springframework.security.vote.AccessDecisionVoter; + +public class WebExpressionVoter implements AccessDecisionVoter { + + public boolean supports(ConfigAttribute attribute) { + return false; + } + + public boolean supports(Class clazz) { + return clazz.isAssignableFrom(FilterInvocation.class); + } + + public int vote(Authentication authentication, Object object, + List attributes) { + // TODO Auto-generated method stub + return 0; + } + +} diff --git a/core/src/main/java/org/springframework/security/util/SimpleMethodInvocation.java b/core/src/main/java/org/springframework/security/util/SimpleMethodInvocation.java index 234e270317..14578f52b0 100644 --- a/core/src/main/java/org/springframework/security/util/SimpleMethodInvocation.java +++ b/core/src/main/java/org/springframework/security/util/SimpleMethodInvocation.java @@ -36,10 +36,10 @@ public class SimpleMethodInvocation implements MethodInvocation { //~ Constructors =================================================================================================== - public SimpleMethodInvocation(Object targetObject, Method method, Object[] arguments) { + public SimpleMethodInvocation(Object targetObject, Method method, Object... arguments) { this.targetObject = targetObject; - this.method = method; - this.arguments = arguments; + this.method = method; + this.arguments = arguments == null ? new Object[0] : arguments; } public SimpleMethodInvocation() {} diff --git a/core/src/test/java/org/springframework/security/context/rmi/ContextPropagatingRemoteInvocationTests.java b/core/src/test/java/org/springframework/security/context/rmi/ContextPropagatingRemoteInvocationTests.java index 47c9821575..d9b0fb5cfd 100644 --- a/core/src/test/java/org/springframework/security/context/rmi/ContextPropagatingRemoteInvocationTests.java +++ b/core/src/test/java/org/springframework/security/context/rmi/ContextPropagatingRemoteInvocationTests.java @@ -38,28 +38,18 @@ import java.lang.reflect.Method; * @version $Id$ */ public class ContextPropagatingRemoteInvocationTests extends TestCase { - //~ Constructors =================================================================================================== - - public ContextPropagatingRemoteInvocationTests() { - } - - public ContextPropagatingRemoteInvocationTests(String arg0) { - super(arg0); - } //~ Methods ======================================================================================================== - protected void tearDown() throws Exception { super.tearDown(); SecurityContextHolder.clearContext(); } - private ContextPropagatingRemoteInvocation getRemoteInvocation() - throws Exception { + private ContextPropagatingRemoteInvocation getRemoteInvocation() throws Exception { Class clazz = TargetObject.class; Method method = clazz.getMethod("makeLowerCase", new Class[] {String.class}); - MethodInvocation mi = new SimpleMethodInvocation(new TargetObject(), method, new Object[] {"SOME_STRING"}); + MethodInvocation mi = new SimpleMethodInvocation(new TargetObject(), method, "SOME_STRING"); ContextPropagatingRemoteInvocationFactory factory = new ContextPropagatingRemoteInvocationFactory(); diff --git a/core/src/test/java/org/springframework/security/expression/SecurityExpressionTests.java b/core/src/test/java/org/springframework/security/expression/SecurityExpressionTests.java new file mode 100644 index 0000000000..e221ec7ca9 --- /dev/null +++ b/core/src/test/java/org/springframework/security/expression/SecurityExpressionTests.java @@ -0,0 +1,30 @@ +package org.springframework.security.expression; + +import org.junit.Test; +import org.springframework.expression.spel.standard.StandardEvaluationContext; +import org.springframework.security.providers.UsernamePasswordAuthenticationToken; + + +/** + * + * @author Luke Taylor + * @version $Id$ + */ +public class SecurityExpressionTests { + @Test + public void someTestMethod() throws Exception { + UsernamePasswordAuthenticationToken authToken = new UsernamePasswordAuthenticationToken("joe", "password"); + + SecurityExpressionRoot root = new SecurityExpressionRoot(authToken); + StandardEvaluationContext ctx = new StandardEvaluationContext(); + + + + + } + + @Test + public void someTestMethod2() throws Exception { + + } +} diff --git a/core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java b/core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java index c20805e7cc..68e4b0a81c 100644 --- a/core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java +++ b/core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java @@ -39,8 +39,8 @@ public class ExpressionAnnotationMethodDefinitionSourceTests { List attrs = mds.getAttributes(voidImpl1); assertEquals(1, attrs.size()); - assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); - PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute) attrs.get(0); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionAttribute); + PreInvocationExpressionAttribute pre = (PreInvocationExpressionAttribute) attrs.get(0); assertNotNull(pre.getAuthorizeExpression()); assertEquals("someExpression", pre.getAuthorizeExpression().getExpressionString()); assertNull(pre.getFilterExpression()); @@ -51,8 +51,8 @@ public class ExpressionAnnotationMethodDefinitionSourceTests { List attrs = mds.getAttributes(voidImpl2); assertEquals(1, attrs.size()); - assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); - PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionAttribute); + PreInvocationExpressionAttribute pre = (PreInvocationExpressionAttribute)attrs.get(0); assertEquals("someExpression", pre.getAuthorizeExpression().getExpressionString()); assertNotNull(pre.getFilterExpression()); assertEquals("somePreFilterExpression", pre.getFilterExpression().getExpressionString()); @@ -63,8 +63,8 @@ public class ExpressionAnnotationMethodDefinitionSourceTests { List attrs = mds.getAttributes(voidImpl3); assertEquals(1, attrs.size()); - assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); - PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionAttribute); + PreInvocationExpressionAttribute pre = (PreInvocationExpressionAttribute)attrs.get(0); assertEquals("permitAll", pre.getAuthorizeExpression().getExpressionString()); assertNotNull(pre.getFilterExpression()); assertEquals("somePreFilterExpression", pre.getFilterExpression().getExpressionString()); @@ -75,10 +75,10 @@ public class ExpressionAnnotationMethodDefinitionSourceTests { List attrs = mds.getAttributes(listImpl1); assertEquals(2, attrs.size()); - assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); - assertTrue(attrs.get(1) instanceof PostInvocationExpressionConfigAttribute); - PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); - PostInvocationExpressionConfigAttribute post = (PostInvocationExpressionConfigAttribute)attrs.get(1); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionAttribute); + assertTrue(attrs.get(1) instanceof PostInvocationExpressionAttribute); + PreInvocationExpressionAttribute pre = (PreInvocationExpressionAttribute)attrs.get(0); + PostInvocationExpressionAttribute post = (PostInvocationExpressionAttribute)attrs.get(1); assertEquals("permitAll", pre.getAuthorizeExpression().getExpressionString()); assertNotNull(post.getFilterExpression()); assertEquals("somePostFilterExpression", post.getFilterExpression().getExpressionString()); @@ -89,8 +89,8 @@ public class ExpressionAnnotationMethodDefinitionSourceTests { List attrs = mds.getAttributes(notherListImpl1); assertEquals(1, attrs.size()); - assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); - PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionAttribute); + PreInvocationExpressionAttribute pre = (PreInvocationExpressionAttribute)attrs.get(0); assertNotNull(pre.getFilterExpression()); assertNotNull(pre.getAuthorizeExpression()); assertEquals("interfaceMethodAuthzExpression", pre.getAuthorizeExpression().getExpressionString()); @@ -102,8 +102,8 @@ public class ExpressionAnnotationMethodDefinitionSourceTests { List attrs = mds.getAttributes(notherListImpl2); assertEquals(1, attrs.size()); - assertTrue(attrs.get(0) instanceof PreInvocationExpressionConfigAttribute); - PreInvocationExpressionConfigAttribute pre = (PreInvocationExpressionConfigAttribute)attrs.get(0); + assertTrue(attrs.get(0) instanceof PreInvocationExpressionAttribute); + PreInvocationExpressionAttribute pre = (PreInvocationExpressionAttribute)attrs.get(0); assertNotNull(pre.getFilterExpression()); assertNotNull(pre.getAuthorizeExpression()); assertEquals("interfaceMethodAuthzExpression", pre.getAuthorizeExpression().getExpressionString()); diff --git a/core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java b/core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java index fb5964c050..8bb2fcd8f1 100644 --- a/core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java +++ b/core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertEquals; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.List; import org.aopalliance.intercept.MethodInvocation; @@ -18,7 +19,6 @@ import org.springframework.security.vote.AccessDecisionVoter; public class MethodExpressionVoterTests { private TestingAuthenticationToken joe = new TestingAuthenticationToken("joe", "joespass", "blah"); - private MethodInvocation miStringArgs; private MethodInvocation miListArg; private MethodInvocation miArrayArg; private List listArg; @@ -29,7 +29,6 @@ public class MethodExpressionVoterTests { public void setUp() throws Exception { Method m = ExpressionProtectedBusinessServiceImpl.class.getMethod("methodReturningAList", String.class, String.class); - miStringArgs = new SimpleMethodInvocation(new Object(), m, new String[] {"joe", "arg2Value"}); m = ExpressionProtectedBusinessServiceImpl.class.getMethod("methodReturningAList", List.class); listArg = new ArrayList(Arrays.asList("joe", "bob", "sam")); miListArg = new SimpleMethodInvocation(new Object(), m, new Object[] {listArg}); @@ -40,53 +39,116 @@ public class MethodExpressionVoterTests { @Test public void hasRoleExpressionAllowsUserWithRole() throws Exception { - assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, miStringArgs, createAttributes(new PreInvocationExpressionConfigAttribute(null, null, "hasRole('blah')")))); + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingAnArray()); + assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute(null, null, "hasRole('blah')")))); } @Test public void hasRoleExpressionDeniesUserWithoutRole() throws Exception { List cad = new ArrayList(1); - cad.add(new PreInvocationExpressionConfigAttribute(null, null, "hasRole('joedoesnt')")); - assertEquals(AccessDecisionVoter.ACCESS_DENIED, am.vote(joe, miStringArgs, cad)); + cad.add(new PreInvocationExpressionAttribute(null, null, "hasRole('joedoesnt')")); + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingAnArray()); + assertEquals(AccessDecisionVoter.ACCESS_DENIED, am.vote(joe, mi, cad)); } @Test public void matchingArgAgainstAuthenticationNameIsSuccessful() throws Exception { + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingAString(), "joe"); assertEquals(AccessDecisionVoter.ACCESS_GRANTED, - am.vote(joe, miStringArgs, createAttributes(new PreInvocationExpressionConfigAttribute(null, null, "(#userName == principal) and (principal == 'joe')")))); + am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute(null, null, "(#argument == principal) and (principal == 'joe')")))); } @Test public void accessIsGrantedIfNoPreAuthorizeAttributeIsUsed() throws Exception { + Collection arg = createCollectionArg("joe", "bob", "sam"); + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingACollection(), arg); assertEquals(AccessDecisionVoter.ACCESS_GRANTED, - am.vote(joe, miListArg, createAttributes(new PreInvocationExpressionConfigAttribute("(filterObject == 'jim')", "someList", null)))); + am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute("(filterObject == 'jim')", "collection", null)))); // All objects should have been removed, because the expression is always false - assertEquals(0, listArg.size()); + assertEquals(0, arg.size()); + } + + @Test + public void collectionPreFilteringIsSuccessful() throws Exception { + List arg = createCollectionArg("joe", "bob", "sam"); + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingACollection(), arg); + am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute("(filterObject == 'joe' or filterObject == 'sam')", "collection", "permitAll"))); + assertEquals("joe and sam should still be in the list", 2, arg.size()); + assertEquals("joe", arg.get(0)); + assertEquals("sam", arg.get(1)); } @Test(expected=IllegalArgumentException.class) public void arraysCannotBePrefiltered() throws Exception { - am.vote(joe, miArrayArg, - createAttributes(new PreInvocationExpressionConfigAttribute("(filterObject == 'jim')", "someArray", null))); + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingAnArray(), createArrayArg("sam","joe")); + am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute("(filterObject == 'jim')", "someArray", null))); } - @Test - public void listPreFilteringIsSuccessful() throws Exception { - am.vote(joe, miListArg, - createAttributes(new PreInvocationExpressionConfigAttribute("(filterObject == 'joe' or filterObject == 'sam')", "someList", null))); - assertEquals("joe and sam should still be in the list", 2, listArg.size()); - assertEquals("joe", listArg.get(0)); - assertEquals("sam", listArg.get(1)); + @Test(expected=IllegalArgumentException.class) + public void incorrectFilterTargetNameIsRejected() throws Exception { + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingACollection(), createCollectionArg("joe", "bob")); + am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute("(filterObject == 'joe')", "collcetion", null))); + } + + @Test(expected=IllegalArgumentException.class) + public void nullNamedFilterTargetIsRejected() throws Exception { + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingACollection(), new Object[] {null}); + am.vote(joe, mi, createAttributes(new PreInvocationExpressionAttribute("(filterObject == 'joe')", "collection", null))); } @Test public void ruleDefinedInAClassMethodIsApplied() throws Exception { - assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, miStringArgs, - createAttributes(new PreInvocationExpressionConfigAttribute(null, null, "new org.springframework.security.expression.support.SecurityRules().isJoe(#userName)")))); + MethodInvocation mi = new SimpleMethodInvocation(new TargetImpl(), methodTakingAString(), "joe"); + assertEquals(AccessDecisionVoter.ACCESS_GRANTED, am.vote(joe, mi, + createAttributes(new PreInvocationExpressionAttribute(null, null, "new org.springframework.security.expression.support.SecurityRules().isJoe(#argument)")))); } private List createAttributes(ConfigAttribute... attributes) { return Arrays.asList(attributes); } + private List createCollectionArg(Object... elts) { + ArrayList result = new ArrayList(elts.length); + result.addAll(Arrays.asList(elts)); + return result; + } + + private Object createArrayArg(Object... elts) { + ArrayList result = new ArrayList(elts.length); + result.addAll(Arrays.asList(elts)); + return result.toArray(new Object[0]); + } + + private Method methodTakingAnArray() throws Exception { + return Target.class.getMethod("methodTakingAnArray", Object[].class); + } + + private Method methodTakingAString() throws Exception { + return Target.class.getMethod("methodTakingAString", String.class); + } + + private Method methodTakingACollection() throws Exception { + return Target.class.getMethod("methodTakingACollection", Collection.class); + } + + + //~ Inner Classes ================================================================================================== + + private interface Target { + void methodTakingAnArray(Object[] args); + + void methodTakingAString(String argument); + + Collection methodTakingACollection(Collection collection); + } + + private static class TargetImpl implements Target { + public void methodTakingAnArray(Object[] args) {} + + public void methodTakingAString(String argument) {}; + + public Collection methodTakingACollection(Collection collection) {return collection;} + } + + } diff --git a/core/src/test/java/org/springframework/security/intercept/method/aspectj/AspectJSecurityInterceptorTests.java b/core/src/test/java/org/springframework/security/intercept/method/aspectj/AspectJSecurityInterceptorTests.java index 41c9318a26..a9608bcd4c 100644 --- a/core/src/test/java/org/springframework/security/intercept/method/aspectj/AspectJSecurityInterceptorTests.java +++ b/core/src/test/java/org/springframework/security/intercept/method/aspectj/AspectJSecurityInterceptorTests.java @@ -32,6 +32,7 @@ import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.intercept.method.MapBasedMethodDefinitionSource; import org.springframework.security.intercept.method.MethodDefinitionSourceEditor; import org.springframework.security.providers.TestingAuthenticationToken; +import org.springframework.security.util.AuthorityUtils; /** @@ -116,7 +117,7 @@ public class AspectJSecurityInterceptorTests extends TestCase { SecurityContextHolder.getContext() .setAuthentication(new TestingAuthenticationToken("rod", "koala", - new GrantedAuthority[] {})); + AuthorityUtils.NO_AUTHORITIES )); try { si.invoke(joinPoint, aspectJCallback); diff --git a/core/src/test/java/org/springframework/security/providers/anonymous/AnonymousAuthenticationTokenTests.java b/core/src/test/java/org/springframework/security/providers/anonymous/AnonymousAuthenticationTokenTests.java index 5c3772c4f6..22eac02fc2 100644 --- a/core/src/test/java/org/springframework/security/providers/anonymous/AnonymousAuthenticationTokenTests.java +++ b/core/src/test/java/org/springframework/security/providers/anonymous/AnonymousAuthenticationTokenTests.java @@ -20,6 +20,7 @@ import junit.framework.TestCase; import org.springframework.security.GrantedAuthority; import org.springframework.security.GrantedAuthorityImpl; import org.springframework.security.providers.UsernamePasswordAuthenticationToken; +import org.springframework.security.util.AuthorityUtils; /** @@ -63,7 +64,7 @@ public class AnonymousAuthenticationTokenTests extends TestCase { } try { - new AnonymousAuthenticationToken("key", "Test", new GrantedAuthority[] {}); + new AnonymousAuthenticationToken("key", "Test", AuthorityUtils.NO_AUTHORITIES ); fail("Should have thrown IllegalArgumentException"); } catch (IllegalArgumentException expected) { assertTrue(true); diff --git a/core/src/test/java/org/springframework/security/providers/jaas/JaasAuthenticationProviderTests.java b/core/src/test/java/org/springframework/security/providers/jaas/JaasAuthenticationProviderTests.java index d3cdf00065..eb26d901ac 100644 --- a/core/src/test/java/org/springframework/security/providers/jaas/JaasAuthenticationProviderTests.java +++ b/core/src/test/java/org/springframework/security/providers/jaas/JaasAuthenticationProviderTests.java @@ -38,6 +38,7 @@ import org.springframework.security.context.SecurityContextImpl; import org.springframework.security.providers.TestingAuthenticationToken; import org.springframework.security.providers.UsernamePasswordAuthenticationToken; import org.springframework.security.ui.session.HttpSessionDestroyedEvent; +import org.springframework.security.util.AuthorityUtils; /** @@ -225,7 +226,7 @@ public class JaasAuthenticationProviderTests extends TestCase { } public void testUnsupportedAuthenticationObjectReturnsNull() { - assertNull(jaasProvider.authenticate(new TestingAuthenticationToken("foo", "bar", new GrantedAuthority[] {}))); + assertNull(jaasProvider.authenticate(new TestingAuthenticationToken("foo", "bar", AuthorityUtils.NO_AUTHORITIES ))); } //~ Inner Classes ================================================================================================== diff --git a/core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedAuthenticationProviderTests.java b/core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedAuthenticationProviderTests.java index d199edfd09..36cae8ba9d 100755 --- a/core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedAuthenticationProviderTests.java +++ b/core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedAuthenticationProviderTests.java @@ -1,39 +1,43 @@ package org.springframework.security.providers.preauth; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; +import org.springframework.security.Authentication; import org.springframework.security.providers.UsernamePasswordAuthenticationToken; import org.springframework.security.userdetails.AuthenticationUserDetailsService; import org.springframework.security.userdetails.User; import org.springframework.security.userdetails.UserDetails; import org.springframework.security.userdetails.UsernameNotFoundException; -import org.springframework.security.Authentication; -import org.springframework.security.GrantedAuthority; - -import org.junit.Test; -import static org.junit.Assert.*; +import org.springframework.security.util.AuthorityUtils; /** - * + * * @author TSARDD * @since 18-okt-2007 */ public class PreAuthenticatedAuthenticationProviderTests { - private static final String SUPPORTED_USERNAME = "dummyUser"; + private static final String SUPPORTED_USERNAME = "dummyUser"; @Test(expected = IllegalArgumentException.class) public final void afterPropertiesSet() { - PreAuthenticatedAuthenticationProvider provider = new PreAuthenticatedAuthenticationProvider(); + PreAuthenticatedAuthenticationProvider provider = new PreAuthenticatedAuthenticationProvider(); provider.afterPropertiesSet(); - } + } @Test public final void authenticateInvalidToken() throws Exception { - UserDetails ud = new User("dummyUser", "dummyPwd", true, true, true, true, new GrantedAuthority[] {}); - PreAuthenticatedAuthenticationProvider provider = getProvider(ud); - Authentication request = new UsernamePasswordAuthenticationToken("dummyUser", "dummyPwd"); - Authentication result = provider.authenticate(request); - assertNull(result); - } + UserDetails ud = new User("dummyUser", "dummyPwd", true, true, true, true, AuthorityUtils.NO_AUTHORITIES ); + PreAuthenticatedAuthenticationProvider provider = getProvider(ud); + Authentication request = new UsernamePasswordAuthenticationToken("dummyUser", "dummyPwd"); + Authentication result = provider.authenticate(request); + assertNull(result); + } @Test public final void nullPrincipalReturnsNullAuthentication() throws Exception { @@ -45,70 +49,70 @@ public class PreAuthenticatedAuthenticationProviderTests { @Test public final void authenticateKnownUser() throws Exception { - UserDetails ud = new User("dummyUser", "dummyPwd", true, true, true, true, new GrantedAuthority[] {}); - PreAuthenticatedAuthenticationProvider provider = getProvider(ud); - Authentication request = new PreAuthenticatedAuthenticationToken("dummyUser", "dummyPwd"); - Authentication result = provider.authenticate(request); - assertNotNull(result); - assertEquals(result.getPrincipal(), ud); - // @TODO: Add more asserts? - } + UserDetails ud = new User("dummyUser", "dummyPwd", true, true, true, true, AuthorityUtils.NO_AUTHORITIES ); + PreAuthenticatedAuthenticationProvider provider = getProvider(ud); + Authentication request = new PreAuthenticatedAuthenticationToken("dummyUser", "dummyPwd"); + Authentication result = provider.authenticate(request); + assertNotNull(result); + assertEquals(result.getPrincipal(), ud); + // @TODO: Add more asserts? + } @Test public final void authenticateIgnoreCredentials() throws Exception { - UserDetails ud = new User("dummyUser1", "dummyPwd1", true, true, true, true, new GrantedAuthority[] {}); - PreAuthenticatedAuthenticationProvider provider = getProvider(ud); - Authentication request = new PreAuthenticatedAuthenticationToken("dummyUser1", "dummyPwd2"); - Authentication result = provider.authenticate(request); - assertNotNull(result); - assertEquals(result.getPrincipal(), ud); - // @TODO: Add more asserts? - } + UserDetails ud = new User("dummyUser1", "dummyPwd1", true, true, true, true, AuthorityUtils.NO_AUTHORITIES ); + PreAuthenticatedAuthenticationProvider provider = getProvider(ud); + Authentication request = new PreAuthenticatedAuthenticationToken("dummyUser1", "dummyPwd2"); + Authentication result = provider.authenticate(request); + assertNotNull(result); + assertEquals(result.getPrincipal(), ud); + // @TODO: Add more asserts? + } @Test(expected=UsernameNotFoundException.class) public final void authenticateUnknownUserThrowsException() throws Exception { - UserDetails ud = new User("dummyUser1", "dummyPwd", true, true, true, true, new GrantedAuthority[] {}); - PreAuthenticatedAuthenticationProvider provider = getProvider(ud); - Authentication request = new PreAuthenticatedAuthenticationToken("dummyUser2", "dummyPwd"); - provider.authenticate(request); - } + UserDetails ud = new User("dummyUser1", "dummyPwd", true, true, true, true, AuthorityUtils.NO_AUTHORITIES ); + PreAuthenticatedAuthenticationProvider provider = getProvider(ud); + Authentication request = new PreAuthenticatedAuthenticationToken("dummyUser2", "dummyPwd"); + provider.authenticate(request); + } @Test public final void supportsArbitraryObject() throws Exception { - PreAuthenticatedAuthenticationProvider provider = getProvider(null); - assertFalse(provider.supports(Authentication.class)); - } + PreAuthenticatedAuthenticationProvider provider = getProvider(null); + assertFalse(provider.supports(Authentication.class)); + } @Test public final void supportsPreAuthenticatedAuthenticationToken() throws Exception { - PreAuthenticatedAuthenticationProvider provider = getProvider(null); - assertTrue(provider.supports(PreAuthenticatedAuthenticationToken.class)); - } + PreAuthenticatedAuthenticationProvider provider = getProvider(null); + assertTrue(provider.supports(PreAuthenticatedAuthenticationToken.class)); + } @Test public void getSetOrder() throws Exception { - PreAuthenticatedAuthenticationProvider provider = getProvider(null); - provider.setOrder(333); - assertEquals(provider.getOrder(), 333); - } - - private PreAuthenticatedAuthenticationProvider getProvider(UserDetails aUserDetails) throws Exception { - PreAuthenticatedAuthenticationProvider result = new PreAuthenticatedAuthenticationProvider(); - result.setPreAuthenticatedUserDetailsService(getPreAuthenticatedUserDetailsService(aUserDetails)); - result.afterPropertiesSet(); - return result; - } - - private AuthenticationUserDetailsService getPreAuthenticatedUserDetailsService(final UserDetails aUserDetails) { - return new AuthenticationUserDetailsService() { - public UserDetails loadUserDetails(Authentication token) throws UsernameNotFoundException { - if (aUserDetails != null && aUserDetails.getUsername().equals(token.getName())) { - return aUserDetails; - } + PreAuthenticatedAuthenticationProvider provider = getProvider(null); + provider.setOrder(333); + assertEquals(provider.getOrder(), 333); + } + + private PreAuthenticatedAuthenticationProvider getProvider(UserDetails aUserDetails) throws Exception { + PreAuthenticatedAuthenticationProvider result = new PreAuthenticatedAuthenticationProvider(); + result.setPreAuthenticatedUserDetailsService(getPreAuthenticatedUserDetailsService(aUserDetails)); + result.afterPropertiesSet(); + return result; + } + + private AuthenticationUserDetailsService getPreAuthenticatedUserDetailsService(final UserDetails aUserDetails) { + return new AuthenticationUserDetailsService() { + public UserDetails loadUserDetails(Authentication token) throws UsernameNotFoundException { + if (aUserDetails != null && aUserDetails.getUsername().equals(token.getName())) { + return aUserDetails; + } throw new UsernameNotFoundException("notfound"); } - }; - } + }; + } } diff --git a/core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestWrapperTests.java b/core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestWrapperTests.java index 75441c6321..3727561966 100644 --- a/core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestWrapperTests.java +++ b/core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestWrapperTests.java @@ -17,13 +17,13 @@ package org.springframework.security.wrapper; import junit.framework.TestCase; +import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.Authentication; -import org.springframework.security.GrantedAuthority; import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.providers.TestingAuthenticationToken; import org.springframework.security.userdetails.User; +import org.springframework.security.util.AuthorityUtils; import org.springframework.security.util.PortResolverImpl; -import org.springframework.mock.web.MockHttpServletRequest; /** @@ -78,7 +78,7 @@ public class SecurityContextHolderAwareRequestWrapperTests extends TestCase { public void testCorrectOperationWithUserDetailsBasedPrincipal() throws Exception { Authentication auth = new TestingAuthenticationToken(new User("rodAsUserDetails", "koala", true, true, - true, true, new GrantedAuthority[] {}), "koala", "ROLE_HELLO", "ROLE_FOOBAR"); + true, true, AuthorityUtils.NO_AUTHORITIES ), "koala", "ROLE_HELLO", "ROLE_FOOBAR"); SecurityContextHolder.getContext().setAuthentication(auth); MockHttpServletRequest request = new MockHttpServletRequest(); diff --git a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AclTagTests.java b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AclTagTests.java index 44228c1037..7bf4658bdf 100644 --- a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AclTagTests.java +++ b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AclTagTests.java @@ -15,24 +15,22 @@ package org.springframework.security.taglibs.authz; +import javax.servlet.jsp.JspException; +import javax.servlet.jsp.PageContext; +import javax.servlet.jsp.tagext.Tag; + import junit.framework.TestCase; +import org.springframework.context.ApplicationContext; +import org.springframework.context.support.StaticApplicationContext; import org.springframework.security.Authentication; -import org.springframework.security.GrantedAuthority; - import org.springframework.security.acl.AclEntry; import org.springframework.security.acl.AclManager; -import org.springframework.security.acl.basic.SimpleAclEntry; import org.springframework.security.acl.basic.AclObjectIdentity; +import org.springframework.security.acl.basic.SimpleAclEntry; import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.providers.TestingAuthenticationToken; - -import org.springframework.context.ApplicationContext; -import org.springframework.context.support.StaticApplicationContext; - -import javax.servlet.jsp.JspException; -import javax.servlet.jsp.PageContext; -import javax.servlet.jsp.tagext.Tag; +import org.springframework.security.util.AuthorityUtils; /** @@ -54,7 +52,7 @@ public class AclTagTests extends TestCase { } public void testInclusionDeniedWhenAclManagerUnawareOfObject() throws JspException { - Authentication auth = new TestingAuthenticationToken("rod", "koala", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken("rod", "koala", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); aclTag.setHasPermission(new Long(SimpleAclEntry.ADMINISTRATION).toString()); @@ -63,7 +61,7 @@ public class AclTagTests extends TestCase { } public void testInclusionDeniedWhenNoListOfPermissionsGiven() throws JspException { - Authentication auth = new TestingAuthenticationToken("rod", "koala", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken("rod", "koala", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); aclTag.setHasPermission(null); @@ -72,7 +70,7 @@ public class AclTagTests extends TestCase { } public void testInclusionDeniedWhenPrincipalDoesNotHoldAnyPermissions() throws JspException { - Authentication auth = new TestingAuthenticationToken("john", "crow", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken("john", "crow", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); aclTag.setHasPermission(new Integer(SimpleAclEntry.ADMINISTRATION) + "," + new Integer(SimpleAclEntry.READ)); @@ -84,7 +82,7 @@ public class AclTagTests extends TestCase { } public void testInclusionDeniedWhenPrincipalDoesNotHoldRequiredPermissions() throws JspException { - Authentication auth = new TestingAuthenticationToken("rod", "koala", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken("rod", "koala", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); aclTag.setHasPermission(new Integer(SimpleAclEntry.DELETE).toString()); @@ -107,7 +105,7 @@ public class AclTagTests extends TestCase { } public void testJspExceptionThrownIfHasPermissionNotValidFormat() throws JspException { - Authentication auth = new TestingAuthenticationToken("john", "crow", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken("john", "crow", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); aclTag.setHasPermission("0,5, 6"); // shouldn't be any space @@ -121,7 +119,7 @@ public class AclTagTests extends TestCase { } public void testOperationWhenPrincipalHoldsPermissionOfMultipleList() throws JspException { - Authentication auth = new TestingAuthenticationToken("rod", "koala", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken("rod", "koala", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); aclTag.setHasPermission(new Integer(SimpleAclEntry.ADMINISTRATION) + "," + new Integer(SimpleAclEntry.READ)); @@ -130,7 +128,7 @@ public class AclTagTests extends TestCase { } public void testOperationWhenPrincipalHoldsPermissionOfSingleList() throws JspException { - Authentication auth = new TestingAuthenticationToken("rod", "koala", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken("rod", "koala", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); aclTag.setHasPermission(new Integer(SimpleAclEntry.READ).toString()); @@ -177,5 +175,5 @@ public class AclTagTests extends TestCase { } private static class MockAclObjectIdentity implements AclObjectIdentity { - } + } } diff --git a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthenticationTagTests.java b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthenticationTagTests.java index 81d1c95236..f030c99929 100644 --- a/taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthenticationTagTests.java +++ b/taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthenticationTagTests.java @@ -58,7 +58,7 @@ public class AuthenticationTagTests extends TestCase { public void testOperationWhenPrincipalIsAString() throws JspException { SecurityContextHolder.getContext().setAuthentication( - new TestingAuthenticationToken("rodAsString", "koala", new GrantedAuthority[] {})); + new TestingAuthenticationToken("rodAsString", "koala", AuthorityUtils.NO_AUTHORITIES )); authenticationTag.setProperty("principal"); assertEquals(Tag.SKIP_BODY, authenticationTag.doStartTag()); @@ -77,7 +77,7 @@ public class AuthenticationTagTests extends TestCase { public void testOperationWhenPrincipalIsNull() throws JspException { SecurityContextHolder.getContext().setAuthentication( - new TestingAuthenticationToken(null, "koala", new GrantedAuthority[] {})); + new TestingAuthenticationToken(null, "koala", AuthorityUtils.NO_AUTHORITIES )); authenticationTag.setProperty("principal"); assertEquals(Tag.SKIP_BODY, authenticationTag.doStartTag()); diff --git a/taglibs/src/test/java/org/springframework/security/taglibs/velocity/AuthzImplTests.java b/taglibs/src/test/java/org/springframework/security/taglibs/velocity/AuthzImplTests.java index 12a0db3ff2..a58be1a604 100644 --- a/taglibs/src/test/java/org/springframework/security/taglibs/velocity/AuthzImplTests.java +++ b/taglibs/src/test/java/org/springframework/security/taglibs/velocity/AuthzImplTests.java @@ -33,7 +33,7 @@ public class AuthzImplTests extends TestCase { //~ Methods ======================================================================================================== public void testOperationWhenPrincipalIsAString() { - Authentication auth = new TestingAuthenticationToken("rodAsString", "koala", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken("rodAsString", "koala", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); assertEquals("rodAsString", authz.getPrincipal()); @@ -48,7 +48,7 @@ public class AuthzImplTests extends TestCase { } public void testOperationWhenPrincipalIsNull() { - Authentication auth = new TestingAuthenticationToken(null, "koala", new GrantedAuthority[] {}); + Authentication auth = new TestingAuthenticationToken(null, "koala", AuthorityUtils.NO_AUTHORITIES ); SecurityContextHolder.getContext().setAuthentication(auth); assertNull(authz.getPrincipal());