Browse Source

SEC-1166: Added new interface PermissionGrantingStrategy to allow customization of ACL permission granting logic.

The DefaultPermissionGrantingStrategy contains the standard behaviour that was previously in AclImpl.
Luke Taylor 15 years ago
parent
commit
1474e73b11

+ 52 - 122
acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java

@@ -26,6 +26,7 @@ import org.springframework.security.acls.model.NotFoundException;
 import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.OwnershipAcl;
 import org.springframework.security.acls.model.OwnershipAcl;
 import org.springframework.security.acls.model.Permission;
 import org.springframework.security.acls.model.Permission;
+import org.springframework.security.acls.model.PermissionGrantingStrategy;
 import org.springframework.security.acls.model.Sid;
 import org.springframework.security.acls.model.Sid;
 import org.springframework.security.acls.model.UnloadedSidException;
 import org.springframework.security.acls.model.UnloadedSidException;
 import org.springframework.util.Assert;
 import org.springframework.util.Assert;
@@ -41,7 +42,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 
 
     private Acl parentAcl;
     private Acl parentAcl;
     private transient AclAuthorizationStrategy aclAuthorizationStrategy;
     private transient AclAuthorizationStrategy aclAuthorizationStrategy;
-    private transient AuditLogger auditLogger;
+    private transient PermissionGrantingStrategy permissionGrantingStrategy;
     private List<AccessControlEntry> aces = new ArrayList<AccessControlEntry>();
     private List<AccessControlEntry> aces = new ArrayList<AccessControlEntry>();
     private ObjectIdentity objectIdentity;
     private ObjectIdentity objectIdentity;
     private Serializable id;
     private Serializable id;
@@ -69,7 +70,17 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         this.objectIdentity = objectIdentity;
         this.objectIdentity = objectIdentity;
         this.id = id;
         this.id = id;
         this.aclAuthorizationStrategy = aclAuthorizationStrategy;
         this.aclAuthorizationStrategy = aclAuthorizationStrategy;
-        this.auditLogger = auditLogger;
+        this.permissionGrantingStrategy = new DefaultPermissionGrantingStrategy(auditLogger);
+    }
+
+    /**
+     * @deprecated Use the version which takes a  {@code PermissionGrantingStrategy} argument instead.
+     */
+    @Deprecated
+    public AclImpl(ObjectIdentity objectIdentity, Serializable id, AclAuthorizationStrategy aclAuthorizationStrategy,
+                    AuditLogger auditLogger, Acl parentAcl, List<Sid> loadedSids, boolean entriesInheriting, Sid owner) {
+        this(objectIdentity, id, aclAuthorizationStrategy, new DefaultPermissionGrantingStrategy(auditLogger),
+                parentAcl, loadedSids, entriesInheriting, owner);
     }
     }
 
 
     /**
     /**
@@ -88,20 +99,20 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
      * @param owner the owner (required)
      * @param owner the owner (required)
      */
      */
     public AclImpl(ObjectIdentity objectIdentity, Serializable id, AclAuthorizationStrategy aclAuthorizationStrategy,
     public AclImpl(ObjectIdentity objectIdentity, Serializable id, AclAuthorizationStrategy aclAuthorizationStrategy,
-                    AuditLogger auditLogger, Acl parentAcl, List<Sid> loadedSids, boolean entriesInheriting, Sid owner) {
+            PermissionGrantingStrategy grantingStrategy, Acl parentAcl, List<Sid> loadedSids, boolean entriesInheriting, Sid owner) {
         Assert.notNull(objectIdentity, "Object Identity required");
         Assert.notNull(objectIdentity, "Object Identity required");
         Assert.notNull(id, "Id required");
         Assert.notNull(id, "Id required");
         Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required");
         Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required");
         Assert.notNull(owner, "Owner required");
         Assert.notNull(owner, "Owner required");
-        Assert.notNull(auditLogger, "AuditLogger required");
+
         this.objectIdentity = objectIdentity;
         this.objectIdentity = objectIdentity;
         this.id = id;
         this.id = id;
         this.aclAuthorizationStrategy = aclAuthorizationStrategy;
         this.aclAuthorizationStrategy = aclAuthorizationStrategy;
-        this.auditLogger = auditLogger;
         this.parentAcl = parentAcl; // may be null
         this.parentAcl = parentAcl; // may be null
         this.loadedSids = loadedSids; // may be null
         this.loadedSids = loadedSids; // may be null
         this.entriesInheriting = entriesInheriting;
         this.entriesInheriting = entriesInheriting;
         this.owner = owner;
         this.owner = owner;
+        this.permissionGrantingStrategy = grantingStrategy;
     }
     }
 
 
     /**
     /**
@@ -168,35 +179,11 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
     }
     }
 
 
     /**
     /**
-     * Determines authorization.  The order of the <code>permission</code> and <code>sid</code> arguments is
-     * <em>extremely important</em>! The method will iterate through each of the <code>permission</code>s in the order
-     * specified. For each iteration, all of the <code>sid</code>s will be considered, again in the order they are
-     * presented. A search will then be performed for the first {@link AccessControlEntry} object that directly
-     * matches that <code>permission:sid</code> combination. When the <em>first full match</em> is found (ie an ACE
-     * that has the SID currently being searched for and the exact permission bit mask being search for), the grant or
-     * deny flag for that ACE will prevail. If the ACE specifies to grant access, the method will return
-     * <code>true</code>. If the ACE specifies to deny access, the loop will stop and the next <code>permission</code>
-     * iteration will be performed. If each permission indicates to deny access, the first deny ACE found will be
-     * considered the reason for the failure (as it was the first match found, and is therefore the one most logically
-     * requiring changes - although not always). If absolutely no matching ACE was found at all for any permission,
-     * the parent ACL will be tried (provided that there is a parent and {@link #isEntriesInheriting()} is
-     * <code>true</code>. The parent ACL will also scan its parent and so on. If ultimately no matching ACE is found,
-     * a <code>NotFoundException</code> will be thrown and the caller will need to decide how to handle the permission
-     * check. Similarly, if any of the SID arguments presented to the method were not loaded by the ACL,
-     * <code>UnloadedSidException</code> will be thrown.
+     * Delegates to the {@link PermissionGrantingStrategy}.
      *
      *
-     * @param permission the exact permissions to scan for (order is important)
-     * @param sids the exact SIDs to scan for (order is important)
-     * @param administrativeMode if <code>true</code> denotes the query is for administrative purposes and no auditing
-     *        will be undertaken
-     *
-     * @return <code>true</code> if one of the permissions has been granted, <code>false</code> if one of the
-     *         permissions has been specifically revoked
-     *
-     * @throws NotFoundException if an exact ACE for one of the permission bit masks and SID combination could not be
-     *         found
      * @throws UnloadedSidException if the passed SIDs are unknown to this ACL because the ACL was only loaded for a
      * @throws UnloadedSidException if the passed SIDs are unknown to this ACL because the ACL was only loaded for a
      *         subset of SIDs
      *         subset of SIDs
+     * @see DefaultPermissionGrantingStrategy
      */
      */
     public boolean isGranted(List<Permission> permission, List<Sid> sids, boolean administrativeMode)
     public boolean isGranted(List<Permission> permission, List<Sid> sids, boolean administrativeMode)
             throws NotFoundException, UnloadedSidException {
             throws NotFoundException, UnloadedSidException {
@@ -207,64 +194,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
             throw new UnloadedSidException("ACL was not loaded for one or more SID");
             throw new UnloadedSidException("ACL was not loaded for one or more SID");
         }
         }
 
 
-        AccessControlEntry firstRejection = null;
-
-        for (Permission p : permission) {
-            for (Sid sid: sids) {
-                // Attempt to find exact match for this permission mask and SID
-                boolean scanNextSid = true;
-
-                for (AccessControlEntry ace : aces ) {
-
-                    if ((ace.getPermission().getMask() == p.getMask()) && ace.getSid().equals(sid)) {
-                        // Found a matching ACE, so its authorization decision will prevail
-                        if (ace.isGranting()) {
-                            // Success
-                            if (!administrativeMode) {
-                                auditLogger.logIfNeeded(true, ace);
-                            }
-
-                            return true;
-                        }
-
-                        // Failure for this permission, so stop search
-                        // We will see if they have a different permission
-                        // (this permission is 100% rejected for this SID)
-                        if (firstRejection == null) {
-                            // Store first rejection for auditing reasons
-                            firstRejection = ace;
-                        }
-
-                        scanNextSid = false; // helps break the loop
-
-                        break; // exit aces loop
-                    }
-                }
-
-                if (!scanNextSid) {
-                    break; // exit SID for loop (now try next permission)
-                }
-            }
-        }
-
-        if (firstRejection != null) {
-            // We found an ACE to reject the request at this point, as no
-            // other ACEs were found that granted a different permission
-            if (!administrativeMode) {
-                auditLogger.logIfNeeded(false, firstRejection);
-            }
-
-            return false;
-        }
-
-        // No matches have been found so far
-        if (isEntriesInheriting() && (parentAcl != null)) {
-            // We have a parent, so let them try to find a matching ACE
-            return parentAcl.isGranted(permission, sids, false);
-        } else {
-            // We either have no parent, or we're the uppermost parent
-            throw new NotFoundException("Unable to locate a matching ACE for passed permissions and SIDs");
-        }
+        return permissionGrantingStrategy.isGranted(this, permission, sids, administrativeMode);
     }
     }
 
 
     public boolean isSidLoaded(List<Sid> sids) {
     public boolean isSidLoaded(List<Sid> sids) {
@@ -320,39 +250,6 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         return parentAcl;
         return parentAcl;
     }
     }
 
 
-    public String toString() {
-        StringBuilder sb = new StringBuilder();
-        sb.append("AclImpl[");
-        sb.append("id: ").append(this.id).append("; ");
-        sb.append("objectIdentity: ").append(this.objectIdentity).append("; ");
-        sb.append("owner: ").append(this.owner).append("; ");
-
-        int count = 0;
-
-        for (AccessControlEntry ace : aces) {
-            count++;
-
-            if (count == 1) {
-                sb.append("\r\n");
-            }
-
-            sb.append(ace).append("\r\n");
-        }
-
-        if (count == 0) {
-            sb.append("no ACEs; ");
-        }
-
-        sb.append("inheriting: ").append(this.entriesInheriting).append("; ");
-        sb.append("parent: ").append((this.parentAcl == null) ? "Null" : this.parentAcl.getObjectIdentity().toString());
-        sb.append("; ");
-        sb.append("aclAuthorizationStrategy: ").append(this.aclAuthorizationStrategy).append("; ");
-        sb.append("auditLogger: ").append(this.auditLogger);
-        sb.append("]");
-
-        return sb.toString();
-    }
-
     public void updateAce(int aceIndex, Permission permission)
     public void updateAce(int aceIndex, Permission permission)
         throws NotFoundException {
         throws NotFoundException {
         aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
         aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
@@ -405,4 +302,37 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         return false;
         return false;
     }
     }
 
 
+    public String toString() {
+        StringBuilder sb = new StringBuilder();
+        sb.append("AclImpl[");
+        sb.append("id: ").append(this.id).append("; ");
+        sb.append("objectIdentity: ").append(this.objectIdentity).append("; ");
+        sb.append("owner: ").append(this.owner).append("; ");
+
+        int count = 0;
+
+        for (AccessControlEntry ace : aces) {
+            count++;
+
+            if (count == 1) {
+                sb.append("\n");
+            }
+
+            sb.append(ace).append("\n");
+        }
+
+        if (count == 0) {
+            sb.append("no ACEs; ");
+        }
+
+        sb.append("inheriting: ").append(this.entriesInheriting).append("; ");
+        sb.append("parent: ").append((this.parentAcl == null) ? "Null" : this.parentAcl.getObjectIdentity().toString());
+        sb.append("; ");
+        sb.append("aclAuthorizationStrategy: ").append(this.aclAuthorizationStrategy).append("; ");
+        sb.append("permissionGrantingStrategy: ").append(this.permissionGrantingStrategy);
+        sb.append("]");
+
+        return sb.toString();
+    }
+
 }
 }

+ 119 - 0
acl/src/main/java/org/springframework/security/acls/domain/DefaultPermissionGrantingStrategy.java

@@ -0,0 +1,119 @@
+package org.springframework.security.acls.domain;
+
+import java.util.List;
+
+import org.springframework.security.acls.model.AccessControlEntry;
+import org.springframework.security.acls.model.Acl;
+import org.springframework.security.acls.model.NotFoundException;
+import org.springframework.security.acls.model.Permission;
+import org.springframework.security.acls.model.PermissionGrantingStrategy;
+import org.springframework.security.acls.model.Sid;
+import org.springframework.util.Assert;
+
+public class DefaultPermissionGrantingStrategy implements PermissionGrantingStrategy {
+
+    private transient AuditLogger auditLogger;
+
+    /**
+     * Creates an instance with the logger which will be used to record granting and denial of requested permissions.
+     */
+    public DefaultPermissionGrantingStrategy(AuditLogger auditLogger) {
+        Assert.notNull(auditLogger, "auditLogger cannot be null");
+        this.auditLogger = auditLogger;
+    }
+
+    /**
+     * Determines authorization. The order of the <code>permission</code> and <code>sid</code> arguments is
+     * <em>extremely important</em>! The method will iterate through each of the <code>permission</code>s in the order
+     * specified. For each iteration, all of the <code>sid</code>s will be considered, again in the order they are
+     * presented. A search will then be performed for the first {@link AccessControlEntry} object that directly
+     * matches that <code>permission:sid</code> combination. When the <em>first full match</em> is found (ie an ACE
+     * that has the SID currently being searched for and the exact permission bit mask being search for), the grant or
+     * deny flag for that ACE will prevail. If the ACE specifies to grant access, the method will return
+     * <code>true</code>. If the ACE specifies to deny access, the loop will stop and the next <code>permission</code>
+     * iteration will be performed. If each permission indicates to deny access, the first deny ACE found will be
+     * considered the reason for the failure (as it was the first match found, and is therefore the one most logically
+     * requiring changes - although not always). If absolutely no matching ACE was found at all for any permission,
+     * the parent ACL will be tried (provided that there is a parent and {@link #isEntriesInheriting()} is
+     * <code>true</code>. The parent ACL will also scan its parent and so on. If ultimately no matching ACE is found,
+     * a <code>NotFoundException</code> will be thrown and the caller will need to decide how to handle the permission
+     * check. Similarly, if any of the SID arguments presented to the method were not loaded by the ACL,
+     * <code>UnloadedSidException</code> will be thrown.
+     *
+     * @param permission the exact permissions to scan for (order is important)
+     * @param sids the exact SIDs to scan for (order is important)
+     * @param administrativeMode if <code>true</code> denotes the query is for administrative purposes and no auditing
+     *        will be undertaken
+     *
+     * @return <code>true</code> if one of the permissions has been granted, <code>false</code> if one of the
+     *         permissions has been specifically revoked
+     *
+     * @throws NotFoundException if an exact ACE for one of the permission bit masks and SID combination could not be
+     *         found
+     */
+    public boolean isGranted(Acl acl, List<Permission> permission, List<Sid> sids, boolean administrativeMode)
+            throws NotFoundException {
+
+        final List<AccessControlEntry> aces = acl.getEntries();
+
+        AccessControlEntry firstRejection = null;
+
+        for (Permission p : permission) {
+            for (Sid sid: sids) {
+                // Attempt to find exact match for this permission mask and SID
+                boolean scanNextSid = true;
+
+                for (AccessControlEntry ace : aces ) {
+
+                    if ((ace.getPermission().getMask() == p.getMask()) && ace.getSid().equals(sid)) {
+                        // Found a matching ACE, so its authorization decision will prevail
+                        if (ace.isGranting()) {
+                            // Success
+                            if (!administrativeMode) {
+                                auditLogger.logIfNeeded(true, ace);
+                            }
+
+                            return true;
+                        }
+
+                        // Failure for this permission, so stop search
+                        // We will see if they have a different permission
+                        // (this permission is 100% rejected for this SID)
+                        if (firstRejection == null) {
+                            // Store first rejection for auditing reasons
+                            firstRejection = ace;
+                        }
+
+                        scanNextSid = false; // helps break the loop
+
+                        break; // exit aces loop
+                    }
+                }
+
+                if (!scanNextSid) {
+                    break; // exit SID for loop (now try next permission)
+                }
+            }
+        }
+
+        if (firstRejection != null) {
+            // We found an ACE to reject the request at this point, as no
+            // other ACEs were found that granted a different permission
+            if (!administrativeMode) {
+                auditLogger.logIfNeeded(false, firstRejection);
+            }
+
+            return false;
+        }
+
+        // No matches have been found so far
+        if (acl.isEntriesInheriting() && (acl.getParentAcl() != null)) {
+            // We have a parent, so let them try to find a matching ACE
+            return acl.getParentAcl().isGranted(permission, sids, false);
+        } else {
+            // We either have no parent, or we're the uppermost parent
+            throw new NotFoundException("Unable to locate a matching ACE for passed permissions and SIDs");
+        }
+    }
+
+}

+ 6 - 6
acl/src/main/java/org/springframework/security/acls/domain/EhCacheBasedAclCache.java

@@ -23,17 +23,17 @@ import net.sf.ehcache.Element;
 import org.springframework.security.acls.model.AclCache;
 import org.springframework.security.acls.model.AclCache;
 import org.springframework.security.acls.model.MutableAcl;
 import org.springframework.security.acls.model.MutableAcl;
 import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.ObjectIdentity;
+import org.springframework.security.acls.model.PermissionGrantingStrategy;
 import org.springframework.security.util.FieldUtils;
 import org.springframework.security.util.FieldUtils;
 import org.springframework.util.Assert;
 import org.springframework.util.Assert;
 
 
 
 
 /**
 /**
  * Simple implementation of {@link AclCache} that delegates to EH-CACHE.
  * Simple implementation of {@link AclCache} that delegates to EH-CACHE.
- *
  * <p>
  * <p>
  * Designed to handle the transient fields in {@link AclImpl}. Note that this implementation assumes all
  * Designed to handle the transient fields in {@link AclImpl}. Note that this implementation assumes all
- * {@link AclImpl} instances share the same {@link AuditLogger} and {@link AclAuthorizationStrategy} instance.
- * </p>
+ * {@link AclImpl} instances share the same {@link PermissionGrantingStrategy} and {@link AclAuthorizationStrategy}
+ * instances.
  *
  *
  * @author Ben Alex
  * @author Ben Alex
  */
  */
@@ -41,7 +41,7 @@ public class EhCacheBasedAclCache implements AclCache {
     //~ Instance fields ================================================================================================
     //~ Instance fields ================================================================================================
 
 
     private Ehcache cache;
     private Ehcache cache;
-    private AuditLogger auditLogger;
+    private PermissionGrantingStrategy permissionGrantingStrategy;
     private AclAuthorizationStrategy aclAuthorizationStrategy;
     private AclAuthorizationStrategy aclAuthorizationStrategy;
 
 
     //~ Constructors ===================================================================================================
     //~ Constructors ===================================================================================================
@@ -115,7 +115,7 @@ public class EhCacheBasedAclCache implements AclCache {
         if (this.aclAuthorizationStrategy == null) {
         if (this.aclAuthorizationStrategy == null) {
             if (acl instanceof AclImpl) {
             if (acl instanceof AclImpl) {
                 this.aclAuthorizationStrategy = (AclAuthorizationStrategy) FieldUtils.getProtectedFieldValue("aclAuthorizationStrategy", acl);
                 this.aclAuthorizationStrategy = (AclAuthorizationStrategy) FieldUtils.getProtectedFieldValue("aclAuthorizationStrategy", acl);
-                this.auditLogger = (AuditLogger) FieldUtils.getProtectedFieldValue("auditLogger", acl);
+                this.permissionGrantingStrategy = (PermissionGrantingStrategy) FieldUtils.getProtectedFieldValue("permissionGrantingStrategy", acl);
             }
             }
         }
         }
 
 
@@ -130,7 +130,7 @@ public class EhCacheBasedAclCache implements AclCache {
     private MutableAcl initializeTransientFields(MutableAcl value) {
     private MutableAcl initializeTransientFields(MutableAcl value) {
         if (value instanceof AclImpl) {
         if (value instanceof AclImpl) {
             FieldUtils.setProtectedFieldValue("aclAuthorizationStrategy", value, this.aclAuthorizationStrategy);
             FieldUtils.setProtectedFieldValue("aclAuthorizationStrategy", value, this.aclAuthorizationStrategy);
-            FieldUtils.setProtectedFieldValue("auditLogger", value, this.auditLogger);
+            FieldUtils.setProtectedFieldValue("permissionGrantingStrategy", value, this.permissionGrantingStrategy);
         }
         }
 
 
         if (value.getParentAcl() != null) {
         if (value.getParentAcl() != null) {

+ 17 - 6
acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java

@@ -38,6 +38,7 @@ import org.springframework.security.acls.domain.AclAuthorizationStrategy;
 import org.springframework.security.acls.domain.AclImpl;
 import org.springframework.security.acls.domain.AclImpl;
 import org.springframework.security.acls.domain.AuditLogger;
 import org.springframework.security.acls.domain.AuditLogger;
 import org.springframework.security.acls.domain.DefaultPermissionFactory;
 import org.springframework.security.acls.domain.DefaultPermissionFactory;
+import org.springframework.security.acls.domain.DefaultPermissionGrantingStrategy;
 import org.springframework.security.acls.domain.GrantedAuthoritySid;
 import org.springframework.security.acls.domain.GrantedAuthoritySid;
 import org.springframework.security.acls.domain.ObjectIdentityImpl;
 import org.springframework.security.acls.domain.ObjectIdentityImpl;
 import org.springframework.security.acls.domain.PermissionFactory;
 import org.springframework.security.acls.domain.PermissionFactory;
@@ -49,6 +50,7 @@ import org.springframework.security.acls.model.MutableAcl;
 import org.springframework.security.acls.model.NotFoundException;
 import org.springframework.security.acls.model.NotFoundException;
 import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.Permission;
 import org.springframework.security.acls.model.Permission;
+import org.springframework.security.acls.model.PermissionGrantingStrategy;
 import org.springframework.security.acls.model.Sid;
 import org.springframework.security.acls.model.Sid;
 import org.springframework.security.acls.model.UnloadedSidException;
 import org.springframework.security.acls.model.UnloadedSidException;
 import org.springframework.security.util.FieldUtils;
 import org.springframework.security.util.FieldUtils;
@@ -109,7 +111,7 @@ public final class BasicLookupStrategy implements LookupStrategy {
     private AclAuthorizationStrategy aclAuthorizationStrategy;
     private AclAuthorizationStrategy aclAuthorizationStrategy;
     private PermissionFactory permissionFactory = new DefaultPermissionFactory();
     private PermissionFactory permissionFactory = new DefaultPermissionFactory();
     private AclCache aclCache;
     private AclCache aclCache;
-    private AuditLogger auditLogger;
+    private PermissionGrantingStrategy grantingStrategy;
     private JdbcTemplate jdbcTemplate;
     private JdbcTemplate jdbcTemplate;
     private int batchSize = 50;
     private int batchSize = 50;
 
 
@@ -130,19 +132,28 @@ public final class BasicLookupStrategy implements LookupStrategy {
      * @param dataSource to access the database
      * @param dataSource to access the database
      * @param aclCache the cache where fully-loaded elements can be stored
      * @param aclCache the cache where fully-loaded elements can be stored
      * @param aclAuthorizationStrategy authorization strategy (required)
      * @param aclAuthorizationStrategy authorization strategy (required)
+     *
+     * @deprecated Use the version which takes a  {@code PermissionGrantingStrategy} argument instead.
      */
      */
+    @Deprecated
     public BasicLookupStrategy(DataSource dataSource, AclCache aclCache,
     public BasicLookupStrategy(DataSource dataSource, AclCache aclCache,
-        AclAuthorizationStrategy aclAuthorizationStrategy, AuditLogger auditLogger) {
+            AclAuthorizationStrategy aclAuthorizationStrategy, AuditLogger auditLogger) {
+        this(dataSource, aclCache, aclAuthorizationStrategy, new DefaultPermissionGrantingStrategy(auditLogger));
+    }
+
+    public BasicLookupStrategy(DataSource dataSource, AclCache aclCache,
+            AclAuthorizationStrategy aclAuthorizationStrategy, PermissionGrantingStrategy grantingStrategy) {
         Assert.notNull(dataSource, "DataSource required");
         Assert.notNull(dataSource, "DataSource required");
         Assert.notNull(aclCache, "AclCache required");
         Assert.notNull(aclCache, "AclCache required");
         Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required");
         Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required");
-        Assert.notNull(auditLogger, "AuditLogger required");
+        Assert.notNull(grantingStrategy, "grantingStrategy required");
         jdbcTemplate = new JdbcTemplate(dataSource);
         jdbcTemplate = new JdbcTemplate(dataSource);
         this.aclCache = aclCache;
         this.aclCache = aclCache;
         this.aclAuthorizationStrategy = aclAuthorizationStrategy;
         this.aclAuthorizationStrategy = aclAuthorizationStrategy;
-        this.auditLogger = auditLogger;
+        this.grantingStrategy = grantingStrategy;
         fieldAces.setAccessible(true);
         fieldAces.setAccessible(true);
         fieldAcl.setAccessible(true);
         fieldAcl.setAccessible(true);
+
     }
     }
 
 
     //~ Methods ========================================================================================================
     //~ Methods ========================================================================================================
@@ -395,7 +406,7 @@ public final class BasicLookupStrategy implements LookupStrategy {
 
 
         // Now we have the parent (if there is one), create the true AclImpl
         // Now we have the parent (if there is one), create the true AclImpl
         AclImpl result = new AclImpl(inputAcl.getObjectIdentity(), (Long) inputAcl.getId(), aclAuthorizationStrategy,
         AclImpl result = new AclImpl(inputAcl.getObjectIdentity(), (Long) inputAcl.getId(), aclAuthorizationStrategy,
-                auditLogger, parent, null, inputAcl.isEntriesInheriting(), inputAcl.getOwner());
+                grantingStrategy, parent, null, inputAcl.isEntriesInheriting(), inputAcl.getOwner());
 
 
         // Copy the "aces" from the input to the destination
         // Copy the "aces" from the input to the destination
 
 
@@ -555,7 +566,7 @@ public final class BasicLookupStrategy implements LookupStrategy {
                     owner = new GrantedAuthoritySid(rs.getString("acl_sid"));
                     owner = new GrantedAuthoritySid(rs.getString("acl_sid"));
                 }
                 }
 
 
-                acl = new AclImpl(objectIdentity, id, aclAuthorizationStrategy, auditLogger, parentAcl, null,
+                acl = new AclImpl(objectIdentity, id, aclAuthorizationStrategy, grantingStrategy, parentAcl, null,
                         entriesInheriting, owner);
                         entriesInheriting, owner);
 
 
                 acls.put(id, acl);
                 acls.put(id, acl);

+ 20 - 0
acl/src/main/java/org/springframework/security/acls/model/PermissionGrantingStrategy.java

@@ -0,0 +1,20 @@
+package org.springframework.security.acls.model;
+
+import java.util.List;
+
+/**
+ * Allow customization of the logic for determining whether a permission or permissions are granted to a particular
+ * sid or sids by an {@link Acl}.
+ *
+ * @author Luke Taylor
+ * @since 3.0.2
+ */
+public interface PermissionGrantingStrategy {
+
+    /**
+     * Returns true if the the supplied strategy decides that the supplied {@code Acl} grants access
+     * based on the supplied list of permissions and sids.
+     */
+    boolean isGranted(Acl acl, List<Permission> permission, List<Sid> sids, boolean administrativeMode);
+
+}

+ 34 - 57
acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java

@@ -25,6 +25,7 @@ import org.springframework.security.acls.model.NotFoundException;
 import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.ObjectIdentity;
 import org.springframework.security.acls.model.OwnershipAcl;
 import org.springframework.security.acls.model.OwnershipAcl;
 import org.springframework.security.acls.model.Permission;
 import org.springframework.security.acls.model.Permission;
+import org.springframework.security.acls.model.PermissionGrantingStrategy;
 import org.springframework.security.acls.model.Sid;
 import org.springframework.security.acls.model.Sid;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.authentication.TestingAuthenticationToken;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.Authentication;
@@ -50,7 +51,8 @@ public class AclImplTests {
 
 
     Authentication auth = new TestingAuthenticationToken("joe", "ignored", "ROLE_ADMINISTRATOR");
     Authentication auth = new TestingAuthenticationToken("joe", "ignored", "ROLE_ADMINISTRATOR");
     Mockery jmockCtx = new Mockery();
     Mockery jmockCtx = new Mockery();
-    AclAuthorizationStrategy mockAuthzStrategy;
+    AclAuthorizationStrategy authzStrategy;
+    PermissionGrantingStrategy pgs;
     AuditLogger mockAuditLogger;
     AuditLogger mockAuditLogger;
     ObjectIdentity objectIdentity = new ObjectIdentityImpl(TARGET_CLASS, 100);
     ObjectIdentity objectIdentity = new ObjectIdentityImpl(TARGET_CLASS, 100);
 
 
@@ -59,8 +61,9 @@ public class AclImplTests {
     @Before
     @Before
     public void setUp() throws Exception {
     public void setUp() throws Exception {
         SecurityContextHolder.getContext().setAuthentication(auth);
         SecurityContextHolder.getContext().setAuthentication(auth);
-        mockAuthzStrategy = mock(AclAuthorizationStrategy.class);
-        mockAuditLogger = mock(AuditLogger.class);;
+        authzStrategy = mock(AclAuthorizationStrategy.class);
+        mockAuditLogger = mock(AuditLogger.class);
+        pgs = new DefaultPermissionGrantingStrategy(mockAuditLogger);
         auth.setAuthenticated(true);
         auth.setAuthenticated(true);
     }
     }
 
 
@@ -72,25 +75,26 @@ public class AclImplTests {
     @Test(expected=IllegalArgumentException.class)
     @Test(expected=IllegalArgumentException.class)
     public void constructorsRejectNullObjectIdentity() throws Exception {
     public void constructorsRejectNullObjectIdentity() throws Exception {
         try {
         try {
-            new AclImpl(null, 1, mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid("joe"));
+            new AclImpl(null, 1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
             fail("Should have thrown IllegalArgumentException");
             fail("Should have thrown IllegalArgumentException");
         }
         }
         catch (IllegalArgumentException expected) {
         catch (IllegalArgumentException expected) {
         }
         }
-        new AclImpl(null, 1, mockAuthzStrategy, mockAuditLogger);
+        new AclImpl(null, 1, authzStrategy, mockAuditLogger);
     }
     }
 
 
     @Test(expected=IllegalArgumentException.class)
     @Test(expected=IllegalArgumentException.class)
     public void constructorsRejectNullId() throws Exception {
     public void constructorsRejectNullId() throws Exception {
         try {
         try {
-            new AclImpl(objectIdentity, null, mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid("joe"));
+            new AclImpl(objectIdentity, null, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
             fail("Should have thrown IllegalArgumentException");
             fail("Should have thrown IllegalArgumentException");
         }
         }
         catch (IllegalArgumentException expected) {
         catch (IllegalArgumentException expected) {
         }
         }
-        new AclImpl(objectIdentity, null, mockAuthzStrategy, mockAuditLogger);
+        new AclImpl(objectIdentity, null, authzStrategy, mockAuditLogger);
     }
     }
 
 
+    @SuppressWarnings("deprecation")
     @Test(expected=IllegalArgumentException.class)
     @Test(expected=IllegalArgumentException.class)
     public void constructorsRejectNullAclAuthzStrategy() throws Exception {
     public void constructorsRejectNullAclAuthzStrategy() throws Exception {
         try {
         try {
@@ -102,20 +106,9 @@ public class AclImplTests {
         new AclImpl(objectIdentity, 1, null, mockAuditLogger);
         new AclImpl(objectIdentity, 1, null, mockAuditLogger);
     }
     }
 
 
-    @Test(expected=IllegalArgumentException.class)
-    public void constructorsRejectNullAuditLogger() throws Exception {
-        try {
-            new AclImpl(objectIdentity, 1, mockAuthzStrategy, null, null, null, true, new PrincipalSid("joe"));
-            fail("It should have thrown IllegalArgumentException");
-        }
-        catch (IllegalArgumentException expected) {
-        }
-        new AclImpl(objectIdentity, 1, mockAuthzStrategy, null);
-    }
-
     @Test
     @Test
     public void insertAceRejectsNullParameters() throws Exception {
     public void insertAceRejectsNullParameters() throws Exception {
-        MutableAcl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid(
+        MutableAcl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, true, new PrincipalSid(
                 "joe"));
                 "joe"));
         try {
         try {
             acl.insertAce(0, null, new GrantedAuthoritySid("ROLE_IGNORED"), true);
             acl.insertAce(0, null, new GrantedAuthoritySid("ROLE_IGNORED"), true);
@@ -133,7 +126,7 @@ public class AclImplTests {
 
 
     @Test
     @Test
     public void insertAceAddsElementAtCorrectIndex() throws Exception {
     public void insertAceAddsElementAtCorrectIndex() throws Exception {
-        MutableAcl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid("joe"));
+        MutableAcl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
         MockAclService service = new MockAclService();
         MockAclService service = new MockAclService();
 
 
         // Insert one permission
         // Insert one permission
@@ -169,7 +162,7 @@ public class AclImplTests {
 
 
     @Test(expected=NotFoundException.class)
     @Test(expected=NotFoundException.class)
     public void insertAceFailsForNonExistentElement() throws Exception {
     public void insertAceFailsForNonExistentElement() throws Exception {
-        MutableAcl acl = new AclImpl(objectIdentity,1, mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid(
+        MutableAcl acl = new AclImpl(objectIdentity,1, authzStrategy, pgs, null, null, true, new PrincipalSid(
                 "joe"));
                 "joe"));
         MockAclService service = new MockAclService();
         MockAclService service = new MockAclService();
 
 
@@ -182,8 +175,7 @@ public class AclImplTests {
 
 
     @Test
     @Test
     public void deleteAceKeepsInitialOrdering() throws Exception {
     public void deleteAceKeepsInitialOrdering() throws Exception {
-        MutableAcl acl = new AclImpl(objectIdentity,1, mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid(
-                "joe"));
+        MutableAcl acl = new AclImpl(objectIdentity,1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
         MockAclService service = new MockAclService();
         MockAclService service = new MockAclService();
 
 
         // Add several permissions
         // Add several permissions
@@ -217,8 +209,7 @@ public class AclImplTests {
         AclAuthorizationStrategyImpl strategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] {
         AclAuthorizationStrategyImpl strategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] {
                 new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"),
                 new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"),
                 new GrantedAuthorityImpl("ROLE_GENERAL") });
                 new GrantedAuthorityImpl("ROLE_GENERAL") });
-        AuditLogger auditLogger = new ConsoleAuditLogger();
-        MutableAcl acl = new AclImpl(objectIdentity, (1), strategy, auditLogger, null, null, true, new PrincipalSid(
+        MutableAcl acl = new AclImpl(objectIdentity, (1), strategy, pgs, null, null, true, new PrincipalSid(
                 "joe"));
                 "joe"));
         try {
         try {
             acl.deleteAce(99);
             acl.deleteAce(99);
@@ -230,8 +221,7 @@ public class AclImplTests {
 
 
     @Test
     @Test
     public void isGrantingRejectsEmptyParameters() throws Exception {
     public void isGrantingRejectsEmptyParameters() throws Exception {
-        MutableAcl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid(
-                "joe"));
+        MutableAcl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
         Sid ben = new PrincipalSid("ben");
         Sid ben = new PrincipalSid("ben");
         try {
         try {
             acl.isGranted(new ArrayList<Permission>(0), Arrays.asList(ben) , false);
             acl.isGranted(new ArrayList<Permission>(0), Arrays.asList(ben) , false);
@@ -255,8 +245,7 @@ public class AclImplTests {
         ObjectIdentity rootOid = new ObjectIdentityImpl(TARGET_CLASS, 100);
         ObjectIdentity rootOid = new ObjectIdentityImpl(TARGET_CLASS, 100);
 
 
         // Create an ACL which owner is not the authenticated principal
         // Create an ACL which owner is not the authenticated principal
-        MutableAcl rootAcl = new AclImpl(rootOid, 1, mockAuthzStrategy, mockAuditLogger, null, null, false, new PrincipalSid(
-                "joe"));
+        MutableAcl rootAcl = new AclImpl(rootOid, 1, authzStrategy, pgs, null, null, false, new PrincipalSid("joe"));
 
 
         // Grant some permissions
         // Grant some permissions
         rootAcl.insertAce(0, BasePermission.READ, new PrincipalSid("ben"), false);
         rootAcl.insertAce(0, BasePermission.READ, new PrincipalSid("ben"), false);
@@ -299,16 +288,12 @@ public class AclImplTests {
         ObjectIdentity childOid2 = new ObjectIdentityImpl(TARGET_CLASS, 104);
         ObjectIdentity childOid2 = new ObjectIdentityImpl(TARGET_CLASS, 104);
 
 
         // Create ACLs
         // Create ACLs
-        MutableAcl grandParentAcl = new AclImpl(grandParentOid, 1, mockAuthzStrategy, mockAuditLogger, null, null, false,
-                new PrincipalSid("joe"));
-        MutableAcl parentAcl1 = new AclImpl(parentOid1, 2, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
-        MutableAcl parentAcl2 = new AclImpl(parentOid2, 3, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
-        MutableAcl childAcl1 = new AclImpl(childOid1, 4, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
-        MutableAcl childAcl2 = new AclImpl(childOid2, 4, mockAuthzStrategy, mockAuditLogger, null, null, false,
-                new PrincipalSid("joe"));
+        PrincipalSid joe = new PrincipalSid("joe");
+        MutableAcl grandParentAcl = new AclImpl(grandParentOid, 1, authzStrategy, pgs, null, null, false, joe);
+        MutableAcl parentAcl1 = new AclImpl(parentOid1, 2, authzStrategy, pgs, null, null, true, joe);
+        MutableAcl parentAcl2 = new AclImpl(parentOid2, 3, authzStrategy, pgs, null, null, true, joe);
+        MutableAcl childAcl1 = new AclImpl(childOid1, 4, authzStrategy, pgs, null, null, true, joe);
+        MutableAcl childAcl2 = new AclImpl(childOid2, 4, authzStrategy, pgs, null, null, false, joe);
 
 
         // Create hierarchies
         // Create hierarchies
         childAcl2.setParent(childAcl1);
         childAcl2.setParent(childAcl1);
@@ -366,8 +351,7 @@ public class AclImplTests {
         Authentication auth = new TestingAuthenticationToken("ben", "ignored","ROLE_GENERAL");
         Authentication auth = new TestingAuthenticationToken("ben", "ignored","ROLE_GENERAL");
         auth.setAuthenticated(true);
         auth.setAuthenticated(true);
         SecurityContextHolder.getContext().setAuthentication(auth);
         SecurityContextHolder.getContext().setAuthentication(auth);
-        MutableAcl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, false, new PrincipalSid(
-                "joe"));
+        MutableAcl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, false, new PrincipalSid("joe"));
         MockAclService service = new MockAclService();
         MockAclService service = new MockAclService();
 
 
         acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
         acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
@@ -384,7 +368,7 @@ public class AclImplTests {
         acl.updateAce(1, BasePermission.DELETE);
         acl.updateAce(1, BasePermission.DELETE);
         acl.updateAce(2, BasePermission.READ);
         acl.updateAce(2, BasePermission.READ);
 
 
-        // Check the change was successfuly made
+        // Check the change was successfully made
         assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.CREATE);
         assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.CREATE);
         assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.DELETE);
         assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.DELETE);
         assertEquals(acl.getEntries().get(2).getPermission(), BasePermission.READ);
         assertEquals(acl.getEntries().get(2).getPermission(), BasePermission.READ);
@@ -395,8 +379,7 @@ public class AclImplTests {
         Authentication auth = new TestingAuthenticationToken("ben", "ignored", "ROLE_AUDITING", "ROLE_GENERAL");
         Authentication auth = new TestingAuthenticationToken("ben", "ignored", "ROLE_AUDITING", "ROLE_GENERAL");
         auth.setAuthenticated(true);
         auth.setAuthenticated(true);
         SecurityContextHolder.getContext().setAuthentication(auth);
         SecurityContextHolder.getContext().setAuthentication(auth);
-        MutableAcl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, false, new PrincipalSid(
-                "joe"));
+        MutableAcl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, false, new PrincipalSid("joe"));
         MockAclService service = new MockAclService();
         MockAclService service = new MockAclService();
 
 
         acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
         acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_USER_READ"), true);
@@ -427,10 +410,8 @@ public class AclImplTests {
         SecurityContextHolder.getContext().setAuthentication(auth);
         SecurityContextHolder.getContext().setAuthentication(auth);
         ObjectIdentity identity = new ObjectIdentityImpl(TARGET_CLASS, (100));
         ObjectIdentity identity = new ObjectIdentityImpl(TARGET_CLASS, (100));
         ObjectIdentity identity2 = new ObjectIdentityImpl(TARGET_CLASS, (101));
         ObjectIdentity identity2 = new ObjectIdentityImpl(TARGET_CLASS, (101));
-        MutableAcl acl = new AclImpl(identity, 1, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
-        MutableAcl parentAcl = new AclImpl(identity2, 2, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
+        MutableAcl acl = new AclImpl(identity, 1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
+        MutableAcl parentAcl = new AclImpl(identity2, 2, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
         MockAclService service = new MockAclService();
         MockAclService service = new MockAclService();
         acl.insertAce(0, BasePermission.READ, 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);
         acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true);
@@ -456,7 +437,7 @@ public class AclImplTests {
     @Test
     @Test
     public void isSidLoadedBehavesAsExpected() throws Exception {
     public void isSidLoadedBehavesAsExpected() throws Exception {
         List<Sid> loadedSids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_IGNORED"));
         List<Sid> loadedSids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_IGNORED"));
-        MutableAcl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, loadedSids, true,
+        MutableAcl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, loadedSids, true,
                 new PrincipalSid("joe"));
                 new PrincipalSid("joe"));
 
 
         assertTrue(acl.isSidLoaded(loadedSids));
         assertTrue(acl.isSidLoaded(loadedSids));
@@ -472,22 +453,19 @@ public class AclImplTests {
 
 
     @Test(expected=NotFoundException.class)
     @Test(expected=NotFoundException.class)
     public void insertAceRaisesNotFoundExceptionForIndexLessThanZero() throws Exception {
     public void insertAceRaisesNotFoundExceptionForIndexLessThanZero() throws Exception {
-        AclImpl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
+        AclImpl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
         acl.insertAce(-1, mock(Permission.class), mock(Sid.class), true);
         acl.insertAce(-1, mock(Permission.class), mock(Sid.class), true);
     }
     }
 
 
     @Test(expected=NotFoundException.class)
     @Test(expected=NotFoundException.class)
     public void deleteAceRaisesNotFoundExceptionForIndexLessThanZero() throws Exception {
     public void deleteAceRaisesNotFoundExceptionForIndexLessThanZero() throws Exception {
-        AclImpl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
+        AclImpl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
         acl.deleteAce(-1);
         acl.deleteAce(-1);
     }
     }
 
 
     @Test(expected=NotFoundException.class)
     @Test(expected=NotFoundException.class)
     public void insertAceRaisesNotFoundExceptionForIndexGreaterThanSize() throws Exception {
     public void insertAceRaisesNotFoundExceptionForIndexGreaterThanSize() throws Exception {
-        AclImpl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
+        AclImpl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
         // Insert at zero, OK.
         // Insert at zero, OK.
         acl.insertAce(0, mock(Permission.class), mock(Sid.class), true);
         acl.insertAce(0, mock(Permission.class), mock(Sid.class), true);
         // Size is now 1
         // Size is now 1
@@ -497,8 +475,7 @@ public class AclImplTests {
     // SEC-1151
     // SEC-1151
     @Test(expected=NotFoundException.class)
     @Test(expected=NotFoundException.class)
     public void deleteAceRaisesNotFoundExceptionForIndexEqualToSize() throws Exception {
     public void deleteAceRaisesNotFoundExceptionForIndexEqualToSize() throws Exception {
-        AclImpl acl = new AclImpl(objectIdentity, 1, mockAuthzStrategy, mockAuditLogger, null, null, true,
-                new PrincipalSid("joe"));
+        AclImpl acl = new AclImpl(objectIdentity, 1, authzStrategy, pgs, null, null, true, new PrincipalSid("joe"));
         acl.insertAce(0, mock(Permission.class), mock(Sid.class), true);
         acl.insertAce(0, mock(Permission.class), mock(Sid.class), true);
         // Size is now 1
         // Size is now 1
         acl.deleteAce(1);
         acl.deleteAce(1);

+ 35 - 29
acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java

@@ -1,8 +1,10 @@
 package org.springframework.security.acls.domain;
 package org.springframework.security.acls.domain;
 
 
-import junit.framework.Assert;
-import junit.framework.TestCase;
+import static org.junit.Assert.*;
 
 
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.acls.model.Acl;
 import org.springframework.security.acls.model.Acl;
 import org.springframework.security.acls.model.MutableAcl;
 import org.springframework.security.acls.model.MutableAcl;
@@ -20,19 +22,22 @@ import org.springframework.security.core.context.SecurityContextHolder;
  *
  *
  * @author Andrei Stefan
  * @author Andrei Stefan
  */
  */
-public class AclImplementationSecurityCheckTests extends TestCase {
+public class AclImplementationSecurityCheckTests {
     private static final String TARGET_CLASS = "org.springframework.security.acls.TargetObject";
     private static final String TARGET_CLASS = "org.springframework.security.acls.TargetObject";
 
 
     //~ Methods ========================================================================================================
     //~ Methods ========================================================================================================
 
 
-    protected void setUp() throws Exception {
+    @Before
+    public void setUp() throws Exception {
         SecurityContextHolder.clearContext();
         SecurityContextHolder.clearContext();
     }
     }
 
 
-    protected void tearDown() throws Exception {
+    @After
+    public void tearDown() throws Exception {
         SecurityContextHolder.clearContext();
         SecurityContextHolder.clearContext();
     }
     }
 
 
+    @Test
     public void testSecurityCheckNoACEs() throws Exception {
     public void testSecurityCheckNoACEs() throws Exception {
         Authentication auth = new TestingAuthenticationToken("user", "password","ROLE_GENERAL","ROLE_AUDITING","ROLE_OWNERSHIP");
         Authentication auth = new TestingAuthenticationToken("user", "password","ROLE_GENERAL","ROLE_AUDITING","ROLE_OWNERSHIP");
         auth.setAuthenticated(true);
         auth.setAuthenticated(true);
@@ -57,24 +62,25 @@ public class AclImplementationSecurityCheckTests extends TestCase {
         // Check access in case the principal has no authorization rights
         // Check access in case the principal has no authorization rights
         try {
         try {
             aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_GENERAL);
             aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_GENERAL);
-            Assert.fail("It should have thrown NotFoundException");
+            fail("It should have thrown NotFoundException");
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
         }
         }
         try {
         try {
             aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_AUDITING);
             aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_AUDITING);
-            Assert.fail("It should have thrown NotFoundException");
+            fail("It should have thrown NotFoundException");
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
         }
         }
         try {
         try {
             aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
             aclAuthorizationStrategy2.securityCheck(acl2, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
-            Assert.fail("It should have thrown NotFoundException");
+            fail("It should have thrown NotFoundException");
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
         }
         }
     }
     }
 
 
+    @Test
     public void testSecurityCheckWithMultipleACEs() throws Exception {
     public void testSecurityCheckWithMultipleACEs() throws Exception {
         // Create a simple authentication with ROLE_GENERAL
         // Create a simple authentication with ROLE_GENERAL
         Authentication auth = new TestingAuthenticationToken("user", "password",
         Authentication auth = new TestingAuthenticationToken("user", "password",
@@ -101,13 +107,13 @@ public class AclImplementationSecurityCheckTests extends TestCase {
         // nor granting access
         // nor granting access
         try {
         try {
             aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_AUDITING);
             aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_AUDITING);
-            Assert.fail("It should have thrown AccessDeniedException");
+            fail("It should have thrown AccessDeniedException");
         }
         }
         catch (AccessDeniedException expected) {
         catch (AccessDeniedException expected) {
         }
         }
         try {
         try {
             aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
             aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
-            Assert.fail("It should have thrown AccessDeniedException");
+            fail("It should have thrown AccessDeniedException");
         }
         }
         catch (AccessDeniedException expected) {
         catch (AccessDeniedException expected) {
         }
         }
@@ -118,7 +124,7 @@ public class AclImplementationSecurityCheckTests extends TestCase {
         // (false) will deny this access
         // (false) will deny this access
         try {
         try {
             aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_AUDITING);
             aclAuthorizationStrategy.securityCheck(aclFirstDeny, AclAuthorizationStrategy.CHANGE_AUDITING);
-            Assert.fail("It should have thrown AccessDeniedException");
+            fail("It should have thrown AccessDeniedException");
         }
         }
         catch (AccessDeniedException expected) {
         catch (AccessDeniedException expected) {
         }
         }
@@ -138,31 +144,32 @@ public class AclImplementationSecurityCheckTests extends TestCase {
         aclFirstAllow.insertAce(1, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false);
         aclFirstAllow.insertAce(1, BasePermission.ADMINISTRATION, new PrincipalSid(auth), false);
         try {
         try {
             aclAuthorizationStrategy.securityCheck(aclFirstAllow, AclAuthorizationStrategy.CHANGE_AUDITING);
             aclAuthorizationStrategy.securityCheck(aclFirstAllow, AclAuthorizationStrategy.CHANGE_AUDITING);
-            Assert.assertTrue(true);
+            assertTrue(true);
         }
         }
         catch (AccessDeniedException notExpected) {
         catch (AccessDeniedException notExpected) {
-            Assert.fail("It shouldn't have thrown AccessDeniedException");
+            fail("It shouldn't have thrown AccessDeniedException");
         }
         }
 
 
         // Create an ACL with no ACE
         // Create an ACL with no ACE
         MutableAcl aclNoACE = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger());
         MutableAcl aclNoACE = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger());
         try {
         try {
             aclAuthorizationStrategy.securityCheck(aclNoACE, AclAuthorizationStrategy.CHANGE_AUDITING);
             aclAuthorizationStrategy.securityCheck(aclNoACE, AclAuthorizationStrategy.CHANGE_AUDITING);
-            Assert.fail("It should have thrown NotFoundException");
+            fail("It should have thrown NotFoundException");
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
-            Assert.assertTrue(true);
+            assertTrue(true);
         }
         }
         // and still grant access for CHANGE_GENERAL
         // and still grant access for CHANGE_GENERAL
         try {
         try {
             aclAuthorizationStrategy.securityCheck(aclNoACE, AclAuthorizationStrategy.CHANGE_GENERAL);
             aclAuthorizationStrategy.securityCheck(aclNoACE, AclAuthorizationStrategy.CHANGE_GENERAL);
-            Assert.assertTrue(true);
+            assertTrue(true);
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
-            Assert.fail("It shouldn't have thrown NotFoundException");
+            fail("It shouldn't have thrown NotFoundException");
         }
         }
     }
     }
 
 
+    @Test
     public void testSecurityCheckWithInheritableACEs() throws Exception {
     public void testSecurityCheckWithInheritableACEs() throws Exception {
         // Create a simple authentication with ROLE_GENERAL
         // Create a simple authentication with ROLE_GENERAL
         Authentication auth = new TestingAuthenticationToken("user", "password",
         Authentication auth = new TestingAuthenticationToken("user", "password",
@@ -186,10 +193,10 @@ public class AclImplementationSecurityCheckTests extends TestCase {
         // rights on CHANGE_OWNERSHIP
         // rights on CHANGE_OWNERSHIP
         try {
         try {
             aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
             aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
-            Assert.fail("It should have thrown NotFoundException");
+            fail("It should have thrown NotFoundException");
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
-            Assert.assertTrue(true);
+            assertTrue(true);
         }
         }
 
 
         // Link the child with its parent and test again against the
         // Link the child with its parent and test again against the
@@ -198,10 +205,10 @@ public class AclImplementationSecurityCheckTests extends TestCase {
         childAcl.setEntriesInheriting(true);
         childAcl.setEntriesInheriting(true);
         try {
         try {
             aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
             aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
-            Assert.assertTrue(true);
+            assertTrue(true);
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
-            Assert.fail("It shouldn't have thrown NotFoundException");
+            fail("It shouldn't have thrown NotFoundException");
         }
         }
 
 
         // Create a root parent and link it to the middle parent
         // Create a root parent and link it to the middle parent
@@ -214,13 +221,15 @@ public class AclImplementationSecurityCheckTests extends TestCase {
         childAcl.setParent(parentAcl);
         childAcl.setParent(parentAcl);
         try {
         try {
             aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
             aclAuthorizationStrategy.securityCheck(childAcl, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
-            Assert.assertTrue(true);
+            assertTrue(true);
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
-            Assert.fail("It shouldn't have thrown NotFoundException");
+            fail("It shouldn't have thrown NotFoundException");
         }
         }
     }
     }
 
 
+    @SuppressWarnings("deprecation")
+    @Test
     public void testSecurityCheckPrincipalOwner() throws Exception {
     public void testSecurityCheckPrincipalOwner() throws Exception {
         Authentication auth = new TestingAuthenticationToken("user", "password", new GrantedAuthority[] {
         Authentication auth = new TestingAuthenticationToken("user", "password", new GrantedAuthority[] {
                 new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_ONE"),
                 new GrantedAuthorityImpl("ROLE_ONE"), new GrantedAuthorityImpl("ROLE_ONE"),
@@ -237,24 +246,21 @@ public class AclImplementationSecurityCheckTests extends TestCase {
                 false, new PrincipalSid(auth));
                 false, new PrincipalSid(auth));
         try {
         try {
             aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_GENERAL);
             aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_GENERAL);
-            Assert.assertTrue(true);
         }
         }
         catch (AccessDeniedException notExpected) {
         catch (AccessDeniedException notExpected) {
-            Assert.fail("It shouldn't have thrown AccessDeniedException");
+            fail("It shouldn't have thrown AccessDeniedException");
         }
         }
         try {
         try {
             aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_AUDITING);
             aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_AUDITING);
-            Assert.fail("It shouldn't have thrown AccessDeniedException");
+            fail("It shouldn't have thrown AccessDeniedException");
         }
         }
         catch (NotFoundException expected) {
         catch (NotFoundException expected) {
-            Assert.assertTrue(true);
         }
         }
         try {
         try {
             aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
             aclAuthorizationStrategy.securityCheck(acl, AclAuthorizationStrategy.CHANGE_OWNERSHIP);
-            Assert.assertTrue(true);
         }
         }
         catch (AccessDeniedException notExpected) {
         catch (AccessDeniedException notExpected) {
-            Assert.fail("It shouldn't have thrown AccessDeniedException");
+            fail("It shouldn't have thrown AccessDeniedException");
         }
         }
     }
     }
 }
 }

+ 3 - 1
acl/src/test/java/org/springframework/security/acls/jdbc/BasicLookupStrategyTests.java

@@ -23,6 +23,7 @@ import org.springframework.security.acls.domain.AclAuthorizationStrategyImpl;
 import org.springframework.security.acls.domain.BasePermission;
 import org.springframework.security.acls.domain.BasePermission;
 import org.springframework.security.acls.domain.ConsoleAuditLogger;
 import org.springframework.security.acls.domain.ConsoleAuditLogger;
 import org.springframework.security.acls.domain.DefaultPermissionFactory;
 import org.springframework.security.acls.domain.DefaultPermissionFactory;
+import org.springframework.security.acls.domain.DefaultPermissionGrantingStrategy;
 import org.springframework.security.acls.domain.EhCacheBasedAclCache;
 import org.springframework.security.acls.domain.EhCacheBasedAclCache;
 import org.springframework.security.acls.domain.ObjectIdentityImpl;
 import org.springframework.security.acls.domain.ObjectIdentityImpl;
 import org.springframework.security.acls.domain.PrincipalSid;
 import org.springframework.security.acls.domain.PrincipalSid;
@@ -103,7 +104,8 @@ public class BasicLookupStrategyTests {
         AclAuthorizationStrategy authorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] {
         AclAuthorizationStrategy authorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] {
                 new GrantedAuthorityImpl("ROLE_ADMINISTRATOR"), new GrantedAuthorityImpl("ROLE_ADMINISTRATOR"),
                 new GrantedAuthorityImpl("ROLE_ADMINISTRATOR"), new GrantedAuthorityImpl("ROLE_ADMINISTRATOR"),
                 new GrantedAuthorityImpl("ROLE_ADMINISTRATOR") });
                 new GrantedAuthorityImpl("ROLE_ADMINISTRATOR") });
-        strategy = new BasicLookupStrategy(dataSource, cache, authorizationStrategy, new ConsoleAuditLogger());
+        strategy = new BasicLookupStrategy(dataSource, cache, authorizationStrategy,
+                new DefaultPermissionGrantingStrategy(new ConsoleAuditLogger()));
         strategy.setPermissionFactory(new DefaultPermissionFactory());
         strategy.setPermissionFactory(new DefaultPermissionFactory());
     }
     }
 
 

+ 2 - 2
acl/src/test/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCacheTests.java

@@ -150,7 +150,7 @@ public class EhCacheBasedAclCacheTests {
         Object retrieved1 = FieldUtils.getProtectedFieldValue("aclAuthorizationStrategy", retrieved);
         Object retrieved1 = FieldUtils.getProtectedFieldValue("aclAuthorizationStrategy", retrieved);
         assertEquals(null, retrieved1);
         assertEquals(null, retrieved1);
 
 
-        Object retrieved2 = FieldUtils.getProtectedFieldValue("auditLogger", retrieved);
+        Object retrieved2 = FieldUtils.getProtectedFieldValue("permissionGrantingStrategy", retrieved);
         assertEquals(null, retrieved2);
         assertEquals(null, retrieved2);
     }
     }
 
 
@@ -247,7 +247,7 @@ public class EhCacheBasedAclCacheTests {
         assertEquals(acl, aclFromCache);
         assertEquals(acl, aclFromCache);
         // SEC-951 check transient fields are set on parent
         // SEC-951 check transient fields are set on parent
         assertNotNull(FieldUtils.getFieldValue(aclFromCache.getParentAcl(), "aclAuthorizationStrategy"));
         assertNotNull(FieldUtils.getFieldValue(aclFromCache.getParentAcl(), "aclAuthorizationStrategy"));
-        assertNotNull(FieldUtils.getFieldValue(aclFromCache.getParentAcl(), "auditLogger"));
+        assertNotNull(FieldUtils.getFieldValue(aclFromCache.getParentAcl(), "permissionGrantingStrategy"));
         assertEquals(acl, myCache.getFromCache(identity));
         assertEquals(acl, myCache.getFromCache(identity));
         assertNotNull(FieldUtils.getFieldValue(aclFromCache, "aclAuthorizationStrategy"));
         assertNotNull(FieldUtils.getFieldValue(aclFromCache, "aclAuthorizationStrategy"));
         AclImpl parentAclFromCache = (AclImpl) myCache.getFromCache(new Long(2));
         AclImpl parentAclFromCache = (AclImpl) myCache.getFromCache(new Long(2));