ソースを参照

SEC-999: Introduced custom SecurityExpressionEvaluationContext which is responsible for lazy initialization of parameter values in the context. Also some further conversion of code using GrantedAuthority arrays.

Luke Taylor 17 年 前
コミット
514bca669f
26 ファイル変更607 行追加449 行削除
  1. 8 7
      acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java
  2. 212 247
      acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java
  3. 3 3
      core/src/main/java/org/springframework/security/expression/ExpressionUtils.java
  4. 48 0
      core/src/main/java/org/springframework/security/expression/SecurityEvaluationContext.java
  5. 8 3
      core/src/main/java/org/springframework/security/expression/annotation/PreFilter.java
  6. 10 0
      core/src/main/java/org/springframework/security/expression/support/AbstractExpressionBasedMethodConfigAttribute.java
  7. 2 2
      core/src/main/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSource.java
  8. 5 5
      core/src/main/java/org/springframework/security/expression/support/MethodExpressionAfterInvocationProvider.java
  9. 29 40
      core/src/main/java/org/springframework/security/expression/support/MethodExpressionVoter.java
  10. 2 2
      core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionAttribute.java
  11. 2 2
      core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionAttribute.java
  12. 27 0
      core/src/main/java/org/springframework/security/expression/support/WebExpressionConfigAttribute.java
  13. 26 0
      core/src/main/java/org/springframework/security/expression/support/WebExpressionVoter.java
  14. 3 3
      core/src/main/java/org/springframework/security/util/SimpleMethodInvocation.java
  15. 2 12
      core/src/test/java/org/springframework/security/context/rmi/ContextPropagatingRemoteInvocationTests.java
  16. 30 0
      core/src/test/java/org/springframework/security/expression/SecurityExpressionTests.java
  17. 14 14
      core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java
  18. 81 19
      core/src/test/java/org/springframework/security/expression/support/MethodExpressionVoterTests.java
  19. 2 1
      core/src/test/java/org/springframework/security/intercept/method/aspectj/AspectJSecurityInterceptorTests.java
  20. 2 1
      core/src/test/java/org/springframework/security/providers/anonymous/AnonymousAuthenticationTokenTests.java
  21. 2 1
      core/src/test/java/org/springframework/security/providers/jaas/JaasAuthenticationProviderTests.java
  22. 66 62
      core/src/test/java/org/springframework/security/providers/preauth/PreAuthenticatedAuthenticationProviderTests.java
  23. 3 3
      core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestWrapperTests.java
  24. 16 18
      taglibs/src/test/java/org/springframework/security/taglibs/authz/AclTagTests.java
  25. 2 2
      taglibs/src/test/java/org/springframework/security/taglibs/authz/AuthenticationTagTests.java
  26. 2 2
      taglibs/src/test/java/org/springframework/security/taglibs/velocity/AuthzImplTests.java

+ 8 - 7
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}.<p>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.</p>
+ * Default implementation of {@link AclAuthorizationStrategy}.
+ * <p>
+ * 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];

+ 212 - 247
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");
+        }
+    }
 }

+ 3 - 3
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) {

+ 48 - 0
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]);
+        }
+    }
+
+}

+ 8 - 3
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 <tt>filterTarget</tt> 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.
+ * <p>
+ * For methods which have a single argument which is a collection type, this argument will be used as the filter
+ * target.
  * <p>
  * 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 "";
 }

+ 10 - 0
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;
     }

+ 2 - 2
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);

+ 5 - 5
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<ConfigAttribute> 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<ConfigAttribute> config) {
+    private PostInvocationExpressionAttribute findMethodAccessControlExpression(List<ConfigAttribute> 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) {

+ 29 - 40
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<ConfigAttribute> 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<ConfigAttribute> config) {
+    private PreInvocationExpressionAttribute findMethodAccessControlExpression(List<ConfigAttribute> config) {
         // Find the MethodAccessControlExpression attribute
         for (ConfigAttribute attribute : config) {
-            if (attribute instanceof PreInvocationExpressionConfigAttribute) {
-                return (PreInvocationExpressionConfigAttribute)attribute;
+            if (attribute instanceof PreInvocationExpressionAttribute) {
+                return (PreInvocationExpressionAttribute)attribute;
             }
         }
 

+ 2 - 2
core/src/main/java/org/springframework/security/expression/support/PostInvocationExpressionConfigAttribute.java → 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);
     }

+ 2 - 2
core/src/main/java/org/springframework/security/expression/support/PreInvocationExpressionConfigAttribute.java → 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);
 

+ 27 - 0
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;
+    }
+
+}

+ 26 - 0
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<ConfigAttribute> attributes) {
+        // TODO Auto-generated method stub
+        return 0;
+    }
+
+}

+ 3 - 3
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() {}

+ 2 - 12
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();
 

+ 30 - 0
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 {
+
+    }
+}

+ 14 - 14
core/src/test/java/org/springframework/security/expression/support/ExpressionAnnotationMethodDefinitionSourceTests.java

@@ -39,8 +39,8 @@ public class ExpressionAnnotationMethodDefinitionSourceTests {
         List<ConfigAttribute> 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<ConfigAttribute> 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<ConfigAttribute> 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<ConfigAttribute> 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<ConfigAttribute> 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<ConfigAttribute> 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());

+ 81 - 19
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<ConfigAttribute> cad = new ArrayList<ConfigAttribute>(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<ConfigAttribute> 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;}
+    }
+
+
 }

+ 2 - 1
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);

+ 2 - 1
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);

+ 2 - 1
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 ==================================================================================================

+ 66 - 62
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");
             }
-		};
-	}
+        };
+    }
 
 }

+ 3 - 3
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();

+ 16 - 18
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 {
-    }    
+    }
 }

+ 2 - 2
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());

+ 2 - 2
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());