SEC-530: Refactor ACL module so ACE manipulation is index-based as

opposed to AccessControlEntry.getId() based.

acl/src/main/java/org/springframework/security/acls/Acl.java

@@ -32,10 +32,11 @@ import java.io.Serializable;
  * </p>
  *
  * <p>
- * TODO: Clarify this paragraph
- * An implementation represents the {@link org.springframework.security.acls.Permission}
- * list applicable for some or all {@link org.springframework.security.acls.sid.Sid}
- * instances.
+ * Implementing classes may elect to return instances that represent 
+ * {@link org.springframework.security.acls.Permission} information for either
+ * some OR all {@link org.springframework.security.acls.sid.Sid}
+ * instances. Therefore, an instance may NOT necessarily contain ALL <tt>Sid</tt>s
+ * for a given domain object.
  * </p>
  *
  * @author Ben Alex
@@ -49,9 +50,9 @@ public interface Acl extends Serializable {
      * 
      * <p>This method is typically used for administrative purposes.</p>
      * 
-     * <p>The order that entries appear in the array is unspecified. However, if implementations use
-     * particular ordering logic in authorization decisions, the entries returned by this method 
-     * <em>MUST</em> be ordered in that manner.</p>
+     * <p>The order that entries appear in the array is important for methods declared in the
+     * {@link MutableAcl} interface. Furthermore, some implementations MAY use ordering as
+     * part of advanced permission checking.</p>
      * 
      * <p>Do <em>NOT</em> use this method for making authorization decisions. Instead use {@link
      * #isGranted(Permission[], Sid[], boolean)}.</p>

acl/src/main/java/org/springframework/security/acls/AuditableAcl.java

@@ -14,7 +14,6 @@
  */
 package org.springframework.security.acls;
 
-import java.io.Serializable;
 
 
 /**
@@ -27,5 +26,5 @@ import java.io.Serializable;
 public interface AuditableAcl extends MutableAcl {
     //~ Methods ========================================================================================================
 
-    void updateAuditing(Serializable aceId, boolean auditSuccess, boolean auditFailure);
+    void updateAuditing(int aceIndex, boolean auditSuccess, boolean auditFailure);
 }

acl/src/main/java/org/springframework/security/acls/MutableAcl.java

@@ -14,10 +14,10 @@
  */
 package org.springframework.security.acls;
 
-import org.springframework.security.acls.sid.Sid;
-
 import java.io.Serializable;
 
+import org.springframework.security.acls.sid.Sid;
+
 
 /**
  * A mutable <tt>Acl</tt>.
@@ -33,18 +33,7 @@ import java.io.Serializable;
 public interface MutableAcl extends Acl {
     //~ Methods ========================================================================================================
 
-    void deleteAce(Serializable aceId) throws NotFoundException;
-
-    /**
-     * Retrieves all of the non-deleted {@link AccessControlEntry} instances currently stored by the
-     * <tt>MutableAcl</tt>. The returned objects should be immutable outside the package, and therefore it is safe
-     * to return them to the caller for informational purposes. The <tt>AccessControlEntry</tt> information is
-     * needed so that invocations of update and delete methods on the <tt>MutableAcl</tt> can refer to a valid
-     * {@link AccessControlEntry#getId()}.
-     *
-     * @return DOCUMENT ME!
-     */
-    AccessControlEntry[] getEntries();
+    void deleteAce(int aceIndex) throws NotFoundException;
 
     /**
      * Obtains an identifier that represents this <tt>MutableAcl</tt>.
@@ -53,7 +42,7 @@ public interface MutableAcl extends Acl {
      */
     Serializable getId();
 
-    void insertAce(Serializable afterAceId, Permission permission, Sid sid, boolean granting)
+    void insertAce(int atIndexLocation, Permission permission, Sid sid, boolean granting)
         throws NotFoundException;
 
     /**
@@ -77,6 +66,6 @@ public interface MutableAcl extends Acl {
      */
     void setParent(Acl newParent);
 
-    void updateAce(Serializable aceId, Permission permission)
+    void updateAce(int aceIndex, Permission permission)
         throws NotFoundException;
 }

acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java

@@ -114,34 +114,22 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 
     //~ Methods ========================================================================================================
 
-    public void deleteAce(Serializable aceId) throws NotFoundException {
-        aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
-
-        synchronized (aces) {
-            int offset = findAceOffset(aceId);
-
-            if (offset == -1) {
-                throw new NotFoundException("Requested ACE ID not found");
-            }
-
-            this.aces.remove(offset);
+    private void verifyAceIndexExists(int aceIndex) {
+        if (aceIndex < 0) {
+        	throw new NotFoundException("aceIndex must be greater than or equal to zero");
+        }
+        if (aceIndex > this.aces.size()) {
+        	throw new NotFoundException("aceIndex must correctly refer to an index of the AccessControlEntry collection");
         }
     }
-
-    private int findAceOffset(Serializable aceId) {
-        Assert.notNull(aceId, "ACE ID is required");
-
+    
+    public void deleteAce(int aceIndex) throws NotFoundException {
+        aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
+        verifyAceIndexExists(aceIndex);
+        
         synchronized (aces) {
-            for (int i = 0; i < aces.size(); i++) {
-                AccessControlEntry ace = (AccessControlEntry) aces.get(i);
-
-                if (ace.getId().equals(aceId)) {
-                    return i;
-                }
-            }
+            this.aces.remove(aceIndex);
         }
-
-        return -1;
     }
 
     public AccessControlEntry[] getEntries() {
@@ -165,26 +153,22 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         return parentAcl;
     }
 
-    public void insertAce(Serializable afterAceId, Permission permission, Sid sid, boolean granting)
+    public void insertAce(int atIndexLocation, Permission permission, Sid sid, boolean granting)
         throws NotFoundException {
         aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
         Assert.notNull(permission, "Permission required");
         Assert.notNull(sid, "Sid required");
+        if (atIndexLocation < 0) {
+        	throw new NotFoundException("atIndexLocation must be greater than or equal to zero");
+        }
+        if (atIndexLocation > this.aces.size()) {
+        	throw new NotFoundException("atIndexLocation must be less than or equal to the size of the AccessControlEntry collection");
+        }
 
         AccessControlEntryImpl ace = new AccessControlEntryImpl(null, this, sid, permission, granting, false, false);
 
         synchronized (aces) {
-            if (afterAceId != null) {
-                int offset = findAceOffset(afterAceId);
-
-                if (offset == -1) {
-                    throw new NotFoundException("Requested ACE ID not found");
-                }
-
-                this.aces.add(offset + 1, ace);
-            } else {
-                this.aces.add(ace);
-            }
+            this.aces.add(atIndexLocation, ace);
         }
     }
 
@@ -372,33 +356,23 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         return sb.toString();
     }
 
-    public void updateAce(Serializable aceId, Permission permission)
+    public void updateAce(int aceIndex, Permission permission)
         throws NotFoundException {
         aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
-
+        verifyAceIndexExists(aceIndex);
+        
         synchronized (aces) {
-            int offset = findAceOffset(aceId);
-
-            if (offset == -1) {
-                throw new NotFoundException("Requested ACE ID not found");
-            }
-
-            AccessControlEntryImpl ace = (AccessControlEntryImpl) aces.get(offset);
+            AccessControlEntryImpl ace = (AccessControlEntryImpl) aces.get(aceIndex);
             ace.setPermission(permission);
         }
     }
 
-    public void updateAuditing(Serializable aceId, boolean auditSuccess, boolean auditFailure) {
+    public void updateAuditing(int aceIndex, boolean auditSuccess, boolean auditFailure) {
         aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_AUDITING);
-
+        verifyAceIndexExists(aceIndex);
+        
         synchronized (aces) {
-            int offset = findAceOffset(aceId);
-
-            if (offset == -1) {
-                throw new NotFoundException("Requested ACE ID not found");
-            }
-
-            AccessControlEntryImpl ace = (AccessControlEntryImpl) aces.get(offset);
+            AccessControlEntryImpl ace = (AccessControlEntryImpl) aces.get(aceIndex);
             ace.setAuditSuccess(auditSuccess);
             ace.setAuditFailure(auditFailure);
         }
@@ -406,7 +380,6 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
     
 	public boolean equals(Object obj) {
 		if (obj instanceof AclImpl) {
-			
 			AclImpl rhs = (AclImpl) obj;
 			if (this.aces.equals(rhs.aces)) {
 				if ((this.parentAcl == null && rhs.parentAcl == null) || (this.parentAcl.equals(rhs.parentAcl))) {

acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java

@@ -138,14 +138,14 @@ public class AclImplTests extends TestCase {
         MutableAcl acl = new AclImpl(identity, new Long(1), strategy, auditLogger, null, null, true, new PrincipalSid(
                 "johndoe"));
         try {
-            acl.insertAce(new Long(1), null, new GrantedAuthoritySid("ROLE_IGNORED"), true);
+            acl.insertAce(0, null, new GrantedAuthoritySid("ROLE_IGNORED"), true);
             fail("It should have thrown IllegalArgumentException");
         }
         catch (IllegalArgumentException expected) {
             assertTrue(true);
         }
         try {
-            acl.insertAce(new Long(1), BasePermission.READ, null, true);
+            acl.insertAce(0, BasePermission.READ, null, true);
             fail("It should have thrown IllegalArgumentException");
         }
         catch (IllegalArgumentException expected) {
@@ -168,7 +168,7 @@ public class AclImplTests extends TestCase {
         MockAclService service = new MockAclService();
 
         // Insert one permission
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST1"), true);
+        acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST1"), true);
         service.updateAcl(acl);
         // Check it was successfully added
         assertEquals(1, acl.getEntries().length);
@@ -177,7 +177,7 @@ public class AclImplTests extends TestCase {
         assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST1"));
 
         // Add a second permission
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST2"), true);
+        acl.insertAce(1, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST2"), true);
         service.updateAcl(acl);
         // Check it was added on the last position
         assertEquals(2, acl.getEntries().length);
@@ -186,7 +186,7 @@ public class AclImplTests extends TestCase {
         assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
 
         // Add a third permission, after the first one
-        acl.insertAce(acl.getEntries()[0].getId(), BasePermission.WRITE, new GrantedAuthoritySid("ROLE_TEST3"), false);
+        acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_TEST3"), false);
         service.updateAcl(acl);
         assertEquals(3, acl.getEntries().length);
         // Check the third entry was added between the two existent ones
@@ -213,11 +213,11 @@ public class AclImplTests extends TestCase {
         MockAclService service = new MockAclService();
 
         // Insert one permission
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST1"), true);
+        acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST1"), true);
         service.updateAcl(acl);
 
         try {
-            acl.insertAce(new Long(5), BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST2"), true);
+            acl.insertAce(55, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST2"), true);
             fail("It should have thrown NotFoundException");
         }
         catch (NotFoundException expected) {
@@ -240,28 +240,28 @@ public class AclImplTests extends TestCase {
         MockAclService service = new MockAclService();
 
         // Add several permissions
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST1"), true);
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST2"), true);
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST3"), true);
+        acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST1"), true);
+        acl.insertAce(1, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST2"), true);
+        acl.insertAce(2, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST3"), true);
         service.updateAcl(acl);
 
         // Delete first permission and check the order of the remaining permissions is kept
-        acl.deleteAce(new Long(1));
+        acl.deleteAce(0);
         assertEquals(2, acl.getEntries().length);
         assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
         assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST3"));
 
         // Add one more permission and remove the permission in the middle
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST4"), true);
+        acl.insertAce(2, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST4"), true);
         service.updateAcl(acl);
-        acl.deleteAce(new Long(2));
+        acl.deleteAce(1);
         assertEquals(2, acl.getEntries().length);
         assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
         assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST4"));
 
         // Remove remaining permissions
-        acl.deleteAce(new Long(1));
-        acl.deleteAce(new Long(3));
+        acl.deleteAce(1);
+        acl.deleteAce(0);
         assertEquals(0, acl.getEntries().length);
     }
 
@@ -278,7 +278,7 @@ public class AclImplTests extends TestCase {
         MutableAcl acl = new AclImpl(identity, new Long(1), strategy, auditLogger, null, null, true, new PrincipalSid(
                 "johndoe"));
         try {
-            acl.deleteAce(new Long(1));
+            acl.deleteAce(99);
             fail("It should have thrown NotFoundException");
         }
         catch (NotFoundException expected) {
@@ -327,10 +327,10 @@ public class AclImplTests extends TestCase {
                 "johndoe"));
 
         // Grant some permissions
-        rootAcl.insertAce(null, BasePermission.READ, new PrincipalSid("ben"), false);
-        rootAcl.insertAce(null, BasePermission.WRITE, new PrincipalSid("scott"), true);
-        rootAcl.insertAce(null, BasePermission.WRITE, new PrincipalSid("rod"), false);
-        rootAcl.insertAce(null, BasePermission.WRITE, new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), true);
+        rootAcl.insertAce(0, BasePermission.READ, new PrincipalSid("ben"), false);
+        rootAcl.insertAce(1, BasePermission.WRITE, new PrincipalSid("scott"), true);
+        rootAcl.insertAce(2, BasePermission.WRITE, new PrincipalSid("rod"), false);
+        rootAcl.insertAce(3, BasePermission.WRITE, new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), true);
 
         // Check permissions granting
         Permission[] permissions = new Permission[] { BasePermission.READ, BasePermission.CREATE };
@@ -394,14 +394,14 @@ public class AclImplTests extends TestCase {
         parentAcl1.setParent(grandParentAcl);
 
         // Add some permissions
-        grandParentAcl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
-        grandParentAcl.insertAce(null, BasePermission.WRITE, new PrincipalSid("ben"), true);
-        grandParentAcl.insertAce(null, BasePermission.DELETE, new PrincipalSid("ben"), false);
-        grandParentAcl.insertAce(null, BasePermission.DELETE, new PrincipalSid("scott"), true);
-        parentAcl1.insertAce(null, BasePermission.READ, new PrincipalSid("scott"), true);
-        parentAcl1.insertAce(null, BasePermission.DELETE, new PrincipalSid("scott"), false);
-        parentAcl2.insertAce(null, BasePermission.CREATE, new PrincipalSid("ben"), true);
-        childAcl1.insertAce(null, BasePermission.CREATE, new PrincipalSid("scott"), true);
+        grandParentAcl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
+        grandParentAcl.insertAce(1, BasePermission.WRITE, new PrincipalSid("ben"), true);
+        grandParentAcl.insertAce(2, BasePermission.DELETE, new PrincipalSid("ben"), false);
+        grandParentAcl.insertAce(3, BasePermission.DELETE, new PrincipalSid("scott"), true);
+        parentAcl1.insertAce(0, BasePermission.READ, new PrincipalSid("scott"), true);
+        parentAcl1.insertAce(1, BasePermission.DELETE, new PrincipalSid("scott"), false);
+        parentAcl2.insertAce(0, BasePermission.CREATE, new PrincipalSid("ben"), true);
+        childAcl1.insertAce(0, BasePermission.CREATE, new PrincipalSid("scott"), true);
 
         // Check granting process for parent1
         assertTrue(parentAcl1.isGranted(new Permission[] { BasePermission.READ }, new Sid[] { new PrincipalSid("scott") },
@@ -464,9 +464,9 @@ public class AclImplTests extends TestCase {
                 "johndoe"));
         MockAclService service = new MockAclService();
 
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
-        acl.insertAce(null, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true);
-        acl.insertAce(null, BasePermission.CREATE, new PrincipalSid("ben"), true);
+        acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
+        acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true);
+        acl.insertAce(2, BasePermission.CREATE, new PrincipalSid("ben"), true);
         service.updateAcl(acl);
 
         assertEquals(acl.getEntries()[0].getPermission(), BasePermission.READ);
@@ -474,9 +474,9 @@ public class AclImplTests extends TestCase {
         assertEquals(acl.getEntries()[2].getPermission(), BasePermission.CREATE);
 
         // Change each permission
-        acl.updateAce(new Long(1), BasePermission.CREATE);
-        acl.updateAce(new Long(2), BasePermission.DELETE);
-        acl.updateAce(new Long(3), BasePermission.READ);
+        acl.updateAce(0, BasePermission.CREATE);
+        acl.updateAce(1, BasePermission.DELETE);
+        acl.updateAce(2, BasePermission.READ);
 
         // Check the change was successfuly made
         assertEquals(acl.getEntries()[0].getPermission(), BasePermission.CREATE);
@@ -498,8 +498,8 @@ public class AclImplTests extends TestCase {
                 "johndoe"));
         MockAclService service = new MockAclService();
 
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
-        acl.insertAce(null, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true);
+        acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
+        acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true);
         service.updateAcl(acl);
 
         assertFalse(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditFailure());
@@ -508,8 +508,8 @@ public class AclImplTests extends TestCase {
         assertFalse(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditSuccess());
 
         // Change each permission
-        ((AuditableAcl) acl).updateAuditing(new Long(1), true, true);
-        ((AuditableAcl) acl).updateAuditing(new Long(2), true, true);
+        ((AuditableAcl) acl).updateAuditing(0, true, true);
+        ((AuditableAcl) acl).updateAuditing(1, true, true);
 
         // Check the change was successfuly made
         assertTrue(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditFailure());
@@ -534,8 +534,8 @@ public class AclImplTests extends TestCase {
         MutableAcl parentAcl = new AclImpl(identity2, new Long(2), strategy, auditLogger, null, null, true, new PrincipalSid(
                 "johndoe"));
         MockAclService service = new MockAclService();
-        acl.insertAce(null, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
-        acl.insertAce(null, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true);
+        acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
+        acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true);
         service.updateAcl(acl);
         
         assertEquals(acl.getId(), new Long(1));

acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java

@@ -114,7 +114,7 @@ public class AclImplementationSecurityCheckTests extends TestCase {
 		// Let's give the principal the ADMINISTRATION permission, without
 		// granting access
 		MutableAcl aclFirstDeny = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger());
-		aclFirstDeny.insertAce(null, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false);
+		aclFirstDeny.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false);
 
 		// The CHANGE_GENERAL test should pass as the principal has ROLE_GENERAL
 		try {
@@ -143,7 +143,7 @@ public class AclImplementationSecurityCheckTests extends TestCase {
 		}
 
 		// Add granting access to this principal
-		aclFirstDeny.insertAce(null, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true);
+		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 {
@@ -158,7 +158,7 @@ public class AclImplementationSecurityCheckTests extends TestCase {
 		// permission, with granting access
 		MutableAcl aclFirstAllow = new AclImpl(identity, new Long(1), aclAuthorizationStrategy,
 				new ConsoleAuditLogger());
-		aclFirstAllow.insertAce(null, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true);
+		aclFirstAllow.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true);
 
 		// The CHANGE_AUDITING test should pass as there is one ACE with
 		// granting access
@@ -171,7 +171,7 @@ public class AclImplementationSecurityCheckTests extends TestCase {
 		}
 
 		// Add a deny ACE and test again for CHANGE_AUDITING
-		aclFirstAllow.insertAce(null, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false);
+		aclFirstAllow.insertAce(1, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false);
 		try {
 			aclAuthorizationStrategy.securityCheck(aclFirstAllow, AclAuthorizationStrategy.CHANGE_AUDITING);
 			Assert.assertTrue(true);
@@ -215,7 +215,7 @@ public class AclImplementationSecurityCheckTests extends TestCase {
 		// Let's give the principal an ADMINISTRATION permission, with granting
 		// access
 		MutableAcl parentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger());
-		parentAcl.insertAce(null, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true);
+		parentAcl.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true);
 		MutableAcl childAcl = new AclImpl(identity, new Long(2), aclAuthorizationStrategy, new ConsoleAuditLogger());
 
 		// Check against the 'child' acl, which doesn't offer any authorization
@@ -244,7 +244,7 @@ public class AclImplementationSecurityCheckTests extends TestCase {
 		MutableAcl rootParentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy,
 				new ConsoleAuditLogger());
 		parentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger());
-		rootParentAcl.insertAce(null, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true);
+		rootParentAcl.insertAce(0, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true);
 		parentAcl.setEntriesInheriting(true);
 		parentAcl.setParent(rootParentAcl);
 		childAcl.setParent(parentAcl);

acl/src/test/java/org/springframework/security/acls/jdbc/AclPermissionInheritanceTests.java

@@ -75,12 +75,12 @@ public class AclPermissionInheritanceTests extends TestCase {
 		aclService.updateAcl(child);
 
 		parent = (AclImpl) aclService.readAclById(rootObject);
-		parent.insertAce(null, BasePermission.READ, 
+		parent.insertAce(0, BasePermission.READ, 
 				new PrincipalSid("john"), true);
 		aclService.updateAcl(parent);
 
 		parent = (AclImpl) aclService.readAclById(rootObject);
-		parent.insertAce(null, BasePermission.READ, 
+		parent.insertAce(1, BasePermission.READ, 
 				new PrincipalSid("joe"), true);
 		aclService.updateAcl(parent);
 
@@ -109,11 +109,11 @@ public class AclPermissionInheritanceTests extends TestCase {
 		child.setParent(parent);
 		aclService.updateAcl(child);
 
-		parent.insertAce(null, BasePermission.ADMINISTRATION, 
+		parent.insertAce(0, BasePermission.ADMINISTRATION, 
 				new GrantedAuthoritySid("ROLE_ADMINISTRATOR"), true);
 		aclService.updateAcl(parent);
 
-		parent.insertAce(null, BasePermission.DELETE, new PrincipalSid("terry"), true);
+		parent.insertAce(1, BasePermission.DELETE, new PrincipalSid("terry"), true);
 		aclService.updateAcl(parent);
 
 		child = (MutableAcl) aclService.readAclById(

acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java

@@ -108,10 +108,10 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         child.setParent(middleParent);
 
         // Now let's add a couple of permissions
-        topParent.insertAce(null, BasePermission.READ, new PrincipalSid(auth), true);
-        topParent.insertAce(null, BasePermission.WRITE, new PrincipalSid(auth), false);
-        middleParent.insertAce(null, BasePermission.DELETE, new PrincipalSid(auth), true);
-        child.insertAce(null, BasePermission.DELETE, new PrincipalSid(auth), false);
+        topParent.insertAce(0, BasePermission.READ, new PrincipalSid(auth), true);
+        topParent.insertAce(1, BasePermission.WRITE, new PrincipalSid(auth), false);
+        middleParent.insertAce(0, BasePermission.DELETE, new PrincipalSid(auth), true);
+        child.insertAce(0, BasePermission.DELETE, new PrincipalSid(auth), false);
 
         // Explictly save the changed ACL
         jdbcMutableAclService.updateAcl(topParent);
@@ -144,10 +144,8 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
 
         // Check the retrieved rights are correct
         assertTrue(topParent.isGranted(new Permission[] {BasePermission.READ}, new Sid[] {new PrincipalSid(auth)}, false));
-        assertFalse(topParent.isGranted(new Permission[] {BasePermission.WRITE}, new Sid[] {new PrincipalSid(auth)},
-                false));
-        assertTrue(middleParent.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)},
-                false));
+        assertFalse(topParent.isGranted(new Permission[] {BasePermission.WRITE}, new Sid[] {new PrincipalSid(auth)}, false));
+        assertTrue(middleParent.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false));
         assertFalse(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false));
 
         try {
@@ -186,10 +184,10 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         }
 
         // Let's add an identical permission to the child, but it'll appear AFTER the current permission, so has no impact
-        child.insertAce(null, BasePermission.DELETE, new PrincipalSid(auth), true);
+        child.insertAce(1, BasePermission.DELETE, new PrincipalSid(auth), true);
 
         // Let's also add another permission to the child
-        child.insertAce(null, BasePermission.CREATE, new PrincipalSid(auth), true);
+        child.insertAce(2, BasePermission.CREATE, new PrincipalSid(auth), true);
 
         // Save the changed child
         jdbcMutableAclService.updateAcl(child);
@@ -213,7 +211,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         assertNotNull(entry.getId());
 
         // Now delete that first ACE
-        child.deleteAce(entry.getId());
+        child.deleteAce(0);
 
         // Save and check it worked
         child = jdbcMutableAclService.updateAcl(child);

samples/contacts/src/main/java/sample/contact/ContactManagerBackend.java

@@ -66,7 +66,7 @@ public class ContactManagerBackend extends ApplicationObjectSupport implements C
             acl = mutableAclService.createAcl(oid);
         }
 
-        acl.insertAce(null, permission, recipient, true);
+        acl.insertAce(acl.getEntries().length, permission, recipient, true);
         mutableAclService.updateAcl(acl);
 
         if (logger.isDebugEnabled()) {
@@ -113,7 +113,7 @@ public class ContactManagerBackend extends ApplicationObjectSupport implements C
 
         for (int i = 0; i < entries.length; i++) {
             if (entries[i].getSid().equals(recipient) && entries[i].getPermission().equals(permission)) {
-                acl.deleteAce(entries[i].getId());
+                acl.deleteAce(i);
             }
         }
 

samples/contacts/src/main/java/sample/contact/DataSourcePopulator.java

@@ -243,7 +243,7 @@ public class DataSourcePopulator implements InitializingBean {
     private void grantPermissions(int contactNumber, String recipientUsername, Permission permission) {
         AclImpl acl = (AclImpl) mutableAclService.readAclById(new ObjectIdentityImpl(Contact.class,
                     new Long(contactNumber)));
-        acl.insertAce(null, permission, new PrincipalSid(recipientUsername), true);
+        acl.insertAce(acl.getEntries().length, permission, new PrincipalSid(recipientUsername), true);
         updateAclInTransaction(acl);
     }
 

samples/dms/src/main/java/sample/dms/secured/SecureDataSourcePopulator.java

@@ -76,9 +76,9 @@ public class SecureDataSourcePopulator extends DataSourcePopulator {
 
         // Now we have an ACL, add another ACE to it
         if (level == LEVEL_NEGATE_READ) {
-            acl.insertAce(null, permission, sid, false); // not granting
+            acl.insertAce(acl.getEntries().length, permission, sid, false); // not granting
         } else {
-            acl.insertAce(null, permission, sid, true); // granting
+            acl.insertAce(acl.getEntries().length, permission, sid, true); // granting
         }
 
         // Finally, persist the modified ACL

samples/dms/src/main/java/sample/dms/secured/SecureDocumentDaoImpl.java

@@ -54,7 +54,7 @@ public class SecureDocumentDaoImpl extends DocumentDaoImpl implements SecureDocu
             MutableAcl aclParent = (MutableAcl) mutableAclService.readAclById(parentIdentity);
             acl.setParent(aclParent);
         }
-        acl.insertAce(null, BasePermission.ADMINISTRATION, new PrincipalSid(SecurityContextHolder.getContext().getAuthentication()), true);
+        acl.insertAce(acl.getEntries().length, BasePermission.ADMINISTRATION, new PrincipalSid(SecurityContextHolder.getContext().getAuthentication()), true);
 
         mutableAclService.updateAcl(acl);
     }