diff --git a/acl/src/main/java/org/springframework/security/acls/Acl.java b/acl/src/main/java/org/springframework/security/acls/Acl.java index 476f57aeaa..9dfc1b63dd 100644 --- a/acl/src/main/java/org/springframework/security/acls/Acl.java +++ b/acl/src/main/java/org/springframework/security/acls/Acl.java @@ -18,6 +18,7 @@ import org.springframework.security.acls.objectidentity.ObjectIdentity; import org.springframework.security.acls.sid.Sid; import java.io.Serializable; +import java.util.List; /** @@ -55,7 +56,7 @@ public interface Acl extends Serializable { * part of advanced permission checking.

* *

Do NOT use this method for making authorization decisions. Instead use {@link - * #isGranted(Permission[], Sid[], boolean)}.

+ * #isGranted(List, List, boolean)}.

* *

This method must operate correctly even if the Acl only represents a subset of * Sids. The caller is responsible for correctly handling the result if only a subset of @@ -64,7 +65,7 @@ public interface Acl extends Serializable { * @return the list of entries represented by the Acl, or null if there are * no entries presently associated with this Acl. */ - AccessControlEntry[] getEntries(); + List getEntries(); /** * Obtains the domain object this Acl provides entries for. This is immutable once an @@ -146,7 +147,7 @@ public interface Acl extends Serializable { * @throws UnloadedSidException thrown if the Acl does not have details for one or more of the * Sids passed as arguments */ - boolean isGranted(Permission[] permission, Sid[] sids, boolean administrativeMode) + boolean isGranted(List permission, List sids, boolean administrativeMode) throws NotFoundException, UnloadedSidException; /** @@ -166,5 +167,5 @@ public interface Acl extends Serializable { * * @return true if every passed Sid is represented by this Acl instance */ - boolean isSidLoaded(Sid[] sids); + boolean isSidLoaded(List sids); } diff --git a/acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java b/acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java index 33b973477e..8504800228 100644 --- a/acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java +++ b/acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java @@ -1,6 +1,8 @@ package org.springframework.security.acls; import java.io.Serializable; +import java.util.Arrays; +import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -60,8 +62,8 @@ public class AclPermissionEvaluator implements PermissionEvaluator { private boolean checkPermission(Authentication authentication, ObjectIdentity oid, Object permission) { // Obtain the SIDs applicable to the principal - Sid[] sids = sidRetrievalStrategy.getSids(authentication); - Permission[] requiredPermission = resolvePermission(permission); + List sids = sidRetrievalStrategy.getSids(authentication); + List requiredPermission = resolvePermission(permission); try { // Lookup only ACLs for SIDs we're interested in @@ -90,17 +92,17 @@ public class AclPermissionEvaluator implements PermissionEvaluator { } // TODO: Add permission resolver/PermissionFactory rewrite - Permission[] resolvePermission(Object permission) { + List resolvePermission(Object permission) { if (permission instanceof Integer) { - return new Permission[] {BasePermission.buildFromMask(((Integer)permission).intValue())}; + return Arrays.asList(BasePermission.buildFromMask(((Integer)permission).intValue())); } if (permission instanceof Permission) { - return new Permission[] {(Permission)permission}; + return Arrays.asList((Permission)permission); } if (permission instanceof Permission[]) { - return (Permission[]) permission; + return Arrays.asList((Permission[])permission); } if (permission instanceof String) { @@ -114,7 +116,7 @@ public class AclPermissionEvaluator implements PermissionEvaluator { } if (p != null) { - return new Permission[] {p}; + return Arrays.asList(p); } } diff --git a/acl/src/main/java/org/springframework/security/acls/AclService.java b/acl/src/main/java/org/springframework/security/acls/AclService.java index f9450b14f2..8c8db89fee 100644 --- a/acl/src/main/java/org/springframework/security/acls/AclService.java +++ b/acl/src/main/java/org/springframework/security/acls/AclService.java @@ -17,6 +17,7 @@ package org.springframework.security.acls; import org.springframework.security.acls.objectidentity.ObjectIdentity; import org.springframework.security.acls.sid.Sid; +import java.util.List; import java.util.Map; @@ -36,10 +37,10 @@ public interface AclService { * * @return the children (or null if none were found) */ - ObjectIdentity[] findChildren(ObjectIdentity parentIdentity); + List findChildren(ObjectIdentity parentIdentity); /** - * Same as {@link #readAclsById(ObjectIdentity[])} except it returns only a single Acl. + * Same as {@link #readAclsById(Java.util.List)} except it returns only a single Acl. *

* This method should not be called as it does not leverage the underlying implementation's potential ability to * filter Acl entries based on a {@link Sid} parameter.

@@ -53,7 +54,7 @@ public interface AclService { Acl readAclById(ObjectIdentity object) throws NotFoundException; /** - * Same as {@link #readAclsById(ObjectIdentity[], Sid[])} except it returns only a single Acl. + * Same as {@link #readAclsById(List, List)} except it returns only a single Acl. * * @param object to locate an {@link Acl} for * @param sids the security identities for which {@link Acl} information is required @@ -63,8 +64,7 @@ public interface AclService { * * @throws NotFoundException if an {@link Acl} was not found for the requested {@link ObjectIdentity} */ - Acl readAclById(ObjectIdentity object, Sid[] sids) - throws NotFoundException; + Acl readAclById(ObjectIdentity object, List sids) throws NotFoundException; /** * Obtains all the Acls that apply for the passed Objects.

The returned map is @@ -77,7 +77,7 @@ public interface AclService { * * @throws NotFoundException if an {@link Acl} was not found for each requested {@link ObjectIdentity} */ - Map readAclsById(ObjectIdentity[] objects) throws NotFoundException; + Map readAclsById(List objects) throws NotFoundException; /** * Obtains all the Acls that apply for the passed Objects, but only for the @@ -97,6 +97,5 @@ public interface AclService { * * @throws NotFoundException if an {@link Acl} was not found for each requested {@link ObjectIdentity} */ - Map readAclsById(ObjectIdentity[] objects, Sid[] sids) - throws NotFoundException; + Map readAclsById(List objects, List sids) throws NotFoundException; } diff --git a/acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java b/acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java index d99b8d5795..88e90cbddf 100644 --- a/acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java +++ b/acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java @@ -26,7 +26,7 @@ import org.springframework.security.SpringSecurityException; public class AlreadyExistsException extends SpringSecurityException { //~ Constructors =================================================================================================== -/** + /** * Constructs an AlreadyExistsException with the specified message. * * @param msg the detail message @@ -35,7 +35,7 @@ public class AlreadyExistsException extends SpringSecurityException { super(msg); } -/** + /** * Constructs an AlreadyExistsException with the specified message * and root cause. * diff --git a/acl/src/main/java/org/springframework/security/acls/AuditableAcl.java b/acl/src/main/java/org/springframework/security/acls/AuditableAcl.java index d102cb45aa..d268a32b47 100644 --- a/acl/src/main/java/org/springframework/security/acls/AuditableAcl.java +++ b/acl/src/main/java/org/springframework/security/acls/AuditableAcl.java @@ -14,8 +14,6 @@ */ package org.springframework.security.acls; - - /** * A mutable ACL that provides audit capabilities. * diff --git a/acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java b/acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java index ddb7530f5b..04f043714d 100644 --- a/acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java +++ b/acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java @@ -26,7 +26,7 @@ import org.springframework.security.SpringSecurityException; public class ChildrenExistException extends SpringSecurityException { //~ Constructors =================================================================================================== -/** + /** * Constructs an ChildrenExistException with the specified * message. * @@ -36,7 +36,7 @@ public class ChildrenExistException extends SpringSecurityException { super(msg); } -/** + /** * Constructs an ChildrenExistException with the specified * message and root cause. * diff --git a/acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java b/acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java index 0277026082..e96d2af9f8 100644 --- a/acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java +++ b/acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java @@ -26,7 +26,7 @@ import org.springframework.security.SpringSecurityException; public class IdentityUnavailableException extends SpringSecurityException { //~ Constructors =================================================================================================== -/** + /** * Constructs an IdentityUnavailableException with the specified message. * * @param msg the detail message @@ -35,7 +35,7 @@ public class IdentityUnavailableException extends SpringSecurityException { super(msg); } -/** + /** * Constructs an IdentityUnavailableException with the specified message * and root cause. * diff --git a/acl/src/main/java/org/springframework/security/acls/MutableAcl.java b/acl/src/main/java/org/springframework/security/acls/MutableAcl.java index 0d51d1db43..aa8ab2beea 100644 --- a/acl/src/main/java/org/springframework/security/acls/MutableAcl.java +++ b/acl/src/main/java/org/springframework/security/acls/MutableAcl.java @@ -21,11 +21,9 @@ import org.springframework.security.acls.sid.Sid; /** * A mutable Acl. - * *

* A mutable ACL must ensure that appropriate security checks are performed * before allowing access to its methods. - *

* * @author Ben Alex * @version $Id$ @@ -47,7 +45,7 @@ public interface MutableAcl extends Acl { /** * Changes the present owner to a different owner. - * + * * @param newOwner the new owner (mandatory; cannot be null) */ void setOwner(Sid newOwner); diff --git a/acl/src/main/java/org/springframework/security/acls/MutableAclService.java b/acl/src/main/java/org/springframework/security/acls/MutableAclService.java index a805a3a566..01513c2880 100644 --- a/acl/src/main/java/org/springframework/security/acls/MutableAclService.java +++ b/acl/src/main/java/org/springframework/security/acls/MutableAclService.java @@ -55,8 +55,6 @@ public interface MutableAclService extends AclService { * * @param acl to modify * - * @return DOCUMENT ME! - * * @throws NotFoundException if the relevant record could not be found (did you remember to use {@link * #createAcl(ObjectIdentity)} to create the object, rather than creating it with the new * keyword?) diff --git a/acl/src/main/java/org/springframework/security/acls/NotFoundException.java b/acl/src/main/java/org/springframework/security/acls/NotFoundException.java index 63f73a80c6..cd7284a75b 100644 --- a/acl/src/main/java/org/springframework/security/acls/NotFoundException.java +++ b/acl/src/main/java/org/springframework/security/acls/NotFoundException.java @@ -26,7 +26,7 @@ import org.springframework.security.SpringSecurityException; public class NotFoundException extends SpringSecurityException { //~ Constructors =================================================================================================== -/** + /** * Constructs an NotFoundException with the specified message. * * @param msg the detail message @@ -35,7 +35,7 @@ public class NotFoundException extends SpringSecurityException { super(msg); } -/** + /** * Constructs an NotFoundException with the specified message * and root cause. * diff --git a/acl/src/main/java/org/springframework/security/acls/OwnershipAcl.java b/acl/src/main/java/org/springframework/security/acls/OwnershipAcl.java index 524d90290b..35c8a82b0e 100644 --- a/acl/src/main/java/org/springframework/security/acls/OwnershipAcl.java +++ b/acl/src/main/java/org/springframework/security/acls/OwnershipAcl.java @@ -23,7 +23,6 @@ import org.springframework.security.acls.sid.Sid; *

* Generally the owner of an ACL is able to call any ACL mutator method, as * well as assign a new owner. - *

* * @author Ben Alex * @version $Id$ diff --git a/acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java b/acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java index a2fa3f4e99..3f040a6cb9 100644 --- a/acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java +++ b/acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java @@ -27,7 +27,7 @@ import org.springframework.security.SpringSecurityException; public class UnloadedSidException extends SpringSecurityException { //~ Constructors =================================================================================================== -/** + /** * Constructs an NotFoundException with the specified message. * * @param msg the detail message @@ -36,7 +36,7 @@ public class UnloadedSidException extends SpringSecurityException { super(msg); } -/** + /** * Constructs an NotFoundException with the specified message * and root cause. * diff --git a/acl/src/main/java/org/springframework/security/acls/afterinvocation/AbstractAclProvider.java b/acl/src/main/java/org/springframework/security/acls/afterinvocation/AbstractAclProvider.java index d21246f644..1627ef5008 100644 --- a/acl/src/main/java/org/springframework/security/acls/afterinvocation/AbstractAclProvider.java +++ b/acl/src/main/java/org/springframework/security/acls/afterinvocation/AbstractAclProvider.java @@ -15,6 +15,9 @@ package org.springframework.security.acls.afterinvocation; +import java.util.Arrays; +import java.util.List; + import org.springframework.security.Authentication; import org.springframework.security.ConfigAttribute; @@ -48,15 +51,15 @@ public abstract class AbstractAclProvider implements AfterInvocationProvider { protected ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl(); protected SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl(); protected String processConfigAttribute; - protected Permission[] requirePermission = {BasePermission.READ}; + protected List requirePermission = Arrays.asList(BasePermission.READ); //~ Constructors =================================================================================================== - public AbstractAclProvider(AclService aclService, String processConfigAttribute, Permission[] requirePermission) { + public AbstractAclProvider(AclService aclService, String processConfigAttribute, List requirePermission) { Assert.hasText(processConfigAttribute, "A processConfigAttribute is mandatory"); Assert.notNull(aclService, "An AclService is mandatory"); - if ((requirePermission == null) || (requirePermission.length == 0)) { + if (requirePermission == null || requirePermission.isEmpty()) { throw new IllegalArgumentException("One or more requirePermission entries is mandatory"); } @@ -76,7 +79,7 @@ public abstract class AbstractAclProvider implements AfterInvocationProvider { ObjectIdentity objectIdentity = objectIdentityRetrievalStrategy.getObjectIdentity(domainObject); // Obtain the SIDs applicable to the principal - Sid[] sids = sidRetrievalStrategy.getSids(authentication); + List sids = sidRetrievalStrategy.getSids(authentication); Acl acl = null; diff --git a/acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationCollectionFilteringProvider.java b/acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationCollectionFilteringProvider.java index cf9f368a2a..4fd17d5e8b 100644 --- a/acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationCollectionFilteringProvider.java +++ b/acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationCollectionFilteringProvider.java @@ -15,7 +15,6 @@ package org.springframework.security.acls.afterinvocation; import java.util.Collection; -import java.util.Iterator; import java.util.List; import org.apache.commons.logging.Log; @@ -39,8 +38,8 @@ import org.springframework.security.acls.Permission; *

* This after invocation provider will fire if any {@link ConfigAttribute#getAttribute()} matches the {@link * #processConfigAttribute}. The provider will then lookup the ACLs from the AclService and ensure the - * principal is {@link org.springframework.security.acls.Acl#isGranted(org.springframework.security.acls.Permission[], - * org.springframework.security.acls.sid.Sid[], boolean) Acl.isGranted(Permission[], Sid[], boolean)} + * principal is {@link org.springframework.security.acls.Acl#isGranted(List, + * List, boolean) Acl.isGranted(Permission[], Sid[], boolean)} * when presenting the {@link #requirePermission} array to that method. *

* If the principal does not have permission, that element will not be included in the returned @@ -67,12 +66,13 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract //~ Constructors =================================================================================================== - public AclEntryAfterInvocationCollectionFilteringProvider(AclService aclService, Permission[] requirePermission) { + public AclEntryAfterInvocationCollectionFilteringProvider(AclService aclService, List requirePermission) { super(aclService, "AFTER_ACL_COLLECTION_READ", requirePermission); } //~ Methods ======================================================================================================== + @SuppressWarnings("unchecked") public Object decide(Authentication authentication, Object object, List config, Object returnedObject) throws AccessDeniedException { @@ -93,7 +93,7 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract Filterer filterer; if (returnedObject instanceof Collection) { - filterer = new CollectionFilterer((Collection) returnedObject); + filterer = new CollectionFilterer((Collection) returnedObject); } else if (returnedObject.getClass().isArray()) { filterer = new ArrayFilterer((Object[]) returnedObject); } else { @@ -102,10 +102,7 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract } // Locate unauthorised Collection elements - Iterator collectionIter = filterer.iterator(); - for (Object domainObject : filterer) { - // Ignore nulls or entries which aren't instances of the configured domain object class if (domainObject == null || !getProcessDomainObjectClass().isAssignableFrom(domainObject.getClass())) { continue; diff --git a/acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationProvider.java b/acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationProvider.java index 17a028d7bd..359affa64c 100644 --- a/acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationProvider.java +++ b/acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationProvider.java @@ -14,7 +14,6 @@ */ package org.springframework.security.acls.afterinvocation; -import java.util.Iterator; import java.util.List; import org.apache.commons.logging.Log; @@ -39,8 +38,8 @@ import org.springframework.security.acls.Permission; *

* This after invocation provider will fire if any {@link ConfigAttribute#getAttribute()} matches the {@link * #processConfigAttribute}. The provider will then lookup the ACLs from the AclService and ensure the - * principal is {@link org.springframework.security.acls.Acl#isGranted(org.springframework.security.acls.Permission[], - org.springframework.security.acls.sid.Sid[], boolean) Acl.isGranted(Permission[], Sid[], boolean)} + * principal is {@link org.springframework.security.acls.Acl#isGranted(List, + List, boolean) Acl.isGranted(Permission[], Sid[], boolean)} * when presenting the {@link #requirePermission} array to that method. *

* Often users will setup an AclEntryAfterInvocationProvider with a {@link @@ -65,7 +64,7 @@ public class AclEntryAfterInvocationProvider extends AbstractAclProvider impleme //~ Constructors =================================================================================================== - public AclEntryAfterInvocationProvider(AclService aclService, Permission[] requirePermission) { + public AclEntryAfterInvocationProvider(AclService aclService, List requirePermission) { super(aclService, "AFTER_ACL_READ", requirePermission); } @@ -74,8 +73,6 @@ public class AclEntryAfterInvocationProvider extends AbstractAclProvider impleme public Object decide(Authentication authentication, Object object, List config, Object returnedObject) throws AccessDeniedException { - Iterator iter = config.iterator(); - if (returnedObject == null) { // AclManager interface contract prohibits nulls // As they have permission to null/nothing, grant access @@ -94,9 +91,7 @@ public class AclEntryAfterInvocationProvider extends AbstractAclProvider impleme return returnedObject; } - while (iter.hasNext()) { - ConfigAttribute attr = (ConfigAttribute) iter.next(); - + for (ConfigAttribute attr : config) { if (!this.supports(attr)) { continue; } diff --git a/acl/src/main/java/org/springframework/security/acls/afterinvocation/ArrayFilterer.java b/acl/src/main/java/org/springframework/security/acls/afterinvocation/ArrayFilterer.java index b6427e509e..e14b679033 100644 --- a/acl/src/main/java/org/springframework/security/acls/afterinvocation/ArrayFilterer.java +++ b/acl/src/main/java/org/springframework/security/acls/afterinvocation/ArrayFilterer.java @@ -60,6 +60,7 @@ class ArrayFilterer implements Filterer { * * @see org.springframework.security.acls.afterinvocation.Filterer#getFilteredObject() */ + @SuppressWarnings("unchecked") public T[] getFilteredObject() { // Recreate an array of same type and filter the removed objects. int originalSize = list.length; @@ -87,6 +88,7 @@ class ArrayFilterer implements Filterer { * * @see org.springframework.security.acls.afterinvocation.Filterer#iterator() */ + @SuppressWarnings("unchecked") public Iterator iterator() { return new ArrayIterator(list); } diff --git a/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java b/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java index 26b78292d9..b8992c4f15 100644 --- a/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java @@ -15,21 +15,18 @@ package org.springframework.security.acls.domain; +import java.util.Arrays; import java.util.List; import org.springframework.security.AccessDeniedException; import org.springframework.security.Authentication; import org.springframework.security.GrantedAuthority; - import org.springframework.security.acls.Acl; -import org.springframework.security.acls.Permission; import org.springframework.security.acls.sid.PrincipalSid; import org.springframework.security.acls.sid.Sid; import org.springframework.security.acls.sid.SidRetrievalStrategy; import org.springframework.security.acls.sid.SidRetrievalStrategyImpl; - import org.springframework.security.context.SecurityContextHolder; - import org.springframework.util.Assert; @@ -112,14 +109,14 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy { } // Try to get permission via ACEs within the ACL - Sid[] sids = sidRetrievalStrategy.getSids(authentication); + List sids = sidRetrievalStrategy.getSids(authentication); - if (acl.isGranted(new Permission[] {BasePermission.ADMINISTRATION}, sids, false)) { + if (acl.isGranted(Arrays.asList(BasePermission.ADMINISTRATION), sids, false)) { return; } throw new AccessDeniedException( - "Principal does not have required ACL permissions to perform requested operation"); + "Principal does not have required ACL permissions to perform requested operation"); } public void setSidRetrievalStrategy(SidRetrievalStrategy sidRetrievalStrategy) { diff --git a/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java b/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java index 124609d379..8cdd2a5772 100644 --- a/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java @@ -15,9 +15,10 @@ package org.springframework.security.acls.domain; import java.io.Serializable; +import java.util.ArrayList; +import java.util.Collections; import java.util.Iterator; import java.util.List; -import java.util.Vector; import org.springframework.security.acls.AccessControlEntry; import org.springframework.security.acls.Acl; @@ -44,11 +45,11 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { private Acl parentAcl; private transient AclAuthorizationStrategy aclAuthorizationStrategy; private transient AuditLogger auditLogger; - private List aces = new Vector(); + private List aces = new ArrayList(); private ObjectIdentity objectIdentity; private Serializable id; private Sid owner; // OwnershipAcl - private Sid[] loadedSids = null; // includes all SIDs the WHERE clause covered, even if there was no ACE for a SID + private List loadedSids = null; // includes all SIDs the WHERE clause covered, even if there was no ACE for a SID private boolean entriesInheriting = true; //~ Constructors =================================================================================================== @@ -90,7 +91,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { * @param owner the owner (required) */ public AclImpl(ObjectIdentity objectIdentity, Serializable id, AclAuthorizationStrategy aclAuthorizationStrategy, - AuditLogger auditLogger, Acl parentAcl, Sid[] loadedSids, boolean entriesInheriting, Sid owner) { + AuditLogger auditLogger, Acl parentAcl, List loadedSids, boolean entriesInheriting, Sid owner) { Assert.notNull(objectIdentity, "Object Identity required"); Assert.notNull(id, "Id required"); Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required"); @@ -110,19 +111,11 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { * Private no-argument constructor for use by reflection-based persistence * tools along with field-level access. */ + @SuppressWarnings("unused") private AclImpl() {} //~ Methods ======================================================================================================== - 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"); - } - } - public void deleteAce(int aceIndex) throws NotFoundException { aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL); verifyAceIndexExists(aceIndex); @@ -132,25 +125,13 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { } } - public AccessControlEntry[] getEntries() { - // Can safely return AccessControlEntry directly, as they're immutable outside the ACL package - return (AccessControlEntry[]) aces.toArray(new AccessControlEntry[] {}); - } - - public Serializable getId() { - return this.id; - } - - public ObjectIdentity getObjectIdentity() { - return objectIdentity; - } - - public Sid getOwner() { - return this.owner; - } - - public Acl getParentAcl() { - return parentAcl; + 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"); + } } public void insertAce(int atIndexLocation, Permission permission, Sid sid, boolean granting) throws NotFoundException { @@ -171,6 +152,19 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { } } + public List getEntries() { + // Can safely return AccessControlEntry directly, as they're immutable outside the ACL package + return new ArrayList(aces); + } + + public Serializable getId() { + return this.id; + } + + public ObjectIdentity getObjectIdentity() { + return objectIdentity; + } + public boolean isEntriesInheriting() { return entriesInheriting; } @@ -206,7 +200,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { * @throws UnloadedSidException if the passed SIDs are unknown to this ACL because the ACL was only loaded for a * subset of SIDs */ - public boolean isGranted(Permission[] permission, Sid[] sids, boolean administrativeMode) + public boolean isGranted(List permission, List sids, boolean administrativeMode) throws NotFoundException, UnloadedSidException { Assert.notEmpty(permission, "Permissions required"); Assert.notEmpty(sids, "SIDs required"); @@ -217,16 +211,14 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { AccessControlEntry firstRejection = null; - for (int i = 0; i < permission.length; i++) { - for (int x = 0; x < sids.length; x++) { + for (Permission p : permission) { + for (Sid sid: sids) { // Attempt to find exact match for this permission mask and SID - Iterator acesIterator = aces.iterator(); boolean scanNextSid = true; - while (acesIterator.hasNext()) { - AccessControlEntry ace = (AccessControlEntry) acesIterator.next(); + for (AccessControlEntry ace : aces ) { - if ((ace.getPermission().getMask() == permission[i].getMask()) && ace.getSid().equals(sids[x])) { + if ((ace.getPermission().getMask() == p.getMask()) && ace.getSid().equals(sid)) { // Found a matching ACE, so its authorization decision will prevail if (ace.isGranting()) { // Success @@ -246,7 +238,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { scanNextSid = false; // helps break the loop - break; // exit "aceIterator" while loop + break; // exit aces loop } } } @@ -277,19 +269,19 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { } } - public boolean isSidLoaded(Sid[] sids) { + public boolean isSidLoaded(List sids) { // If loadedSides is null, this indicates all SIDs were loaded // Also return true if the caller didn't specify a SID to find - if ((this.loadedSids == null) || (sids == null) || (sids.length == 0)) { + if ((this.loadedSids == null) || (sids == null) || (sids.size() == 0)) { return true; } // This ACL applies to a SID subset only. Iterate to check it applies. - for (int i = 0; i < sids.length; i++) { + for (Sid sid: sids) { boolean found = false; - for (int y = 0; y < this.loadedSids.length; y++) { - if (sids[i].equals(this.loadedSids[y])) { + for (Sid loadedSid : loadedSids) { + if (sid.equals(loadedSid)) { // this SID is OK found = true; @@ -316,12 +308,20 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { this.owner = newOwner; } + public Sid getOwner() { + return this.owner; + } + public void setParent(Acl newParent) { aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL); Assert.isTrue(newParent == null || !newParent.equals(this), "Cannot be the parent of yourself"); this.parentAcl = newParent; } + public Acl getParentAcl() { + return parentAcl; + } + public String toString() { StringBuffer sb = new StringBuffer(); sb.append("AclImpl["); @@ -389,9 +389,9 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { if ((this.loadedSids == null && rhs.loadedSids == null)) { return true; } - if (this.loadedSids.length == rhs.loadedSids.length) { - for (int i = 0; i < this.loadedSids.length; i++) { - if (!this.loadedSids[i].equals(rhs.loadedSids[i])) { + if (this.loadedSids.size() == rhs.loadedSids.size()) { + for (int i = 0; i < this.loadedSids.size(); i++) { + if (!this.loadedSids.get(i).equals(rhs.loadedSids.get(i))) { return false; } } diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java b/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java index 99d8e0cdbc..ccb756b11a 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java @@ -178,15 +178,14 @@ public final class BasicLookupStrategy implements LookupStrategy { fieldAcl.setAccessible(true); // Obtain the "aces" from the input ACL - Iterator i = ((List) fieldAces.get(inputAcl)).iterator(); + List aces = (List) fieldAces.get(inputAcl); // Create a list in which to store the "aces" for the "result" AclImpl instance List acesNew = new ArrayList(); // Iterate over the "aces" input and replace each nested AccessControlEntryImpl.getAcl() with the new "result" AclImpl instance // This ensures StubAclParent instances are removed, as per SEC-951 - while(i.hasNext()) { - AccessControlEntryImpl ace = (AccessControlEntryImpl) i.next(); + for (AccessControlEntryImpl ace : aces) { fieldAcl.set(ace, result); acesNew.add(ace); } @@ -285,15 +284,132 @@ public final class BasicLookupStrategy implements LookupStrategy { } /** - * Looks up a batch of ObjectIdentitys directly from the database.

The caller is responsible - * for optimization issues, such as selecting the identities to lookup, ensuring the cache doesn't contain them - * already, and adding the returned elements to the cache etc.

+ * Locates the primary key IDs specified in "findNow", adding AclImpl instances with StubAclParents to the + * "acls" Map. + * + * @param acls the AclImpls (with StubAclParents) + * @param findNow Long-based primary keys to retrieve + * @param sids + */ + private void lookupPrimaryKeys(final Map acls, final Set findNow, final List sids) { + Assert.notNull(acls, "ACLs are required"); + Assert.notEmpty(findNow, "Items to find now required"); + + String sql = computeRepeatingSql("(acl_object_identity.id = ?)", findNow.size()); + + Set parentsToLookup = (Set) jdbcTemplate.query(sql, + new PreparedStatementSetter() { + public void setValues(PreparedStatement ps) + throws SQLException { + Iterator iter = findNow.iterator(); + int i = 0; + + while (iter.hasNext()) { + i++; + ps.setLong(i, ((Long) iter.next()).longValue()); + } + } + }, new ProcessResultSet(acls, sids)); + + // Lookup the parents, now that our JdbcTemplate has released the database connection (SEC-547) + if (parentsToLookup.size() > 0) { + lookupPrimaryKeys(acls, parentsToLookup, sids); + } + } + + /** + * The main method. + *

+ * WARNING: This implementation completely disregards the "sids" argument! Every item in the cache is expected to + * contain all SIDs. If you have serious performance needs (e.g. a very large number of + * SIDs per object identity), you'll probably want to develop a custom {@link LookupStrategy} implementation + * instead. + *

+ * The implementation works in batch sizes specified by {@link #batchSize}. + * + * @param objects the identities to lookup (required) + * @param sids the SIDs for which identities are required (ignored by this implementation) + * + * @return a Map where keys represent the {@link ObjectIdentity} of the located {@link Acl} and values + * are the located {@link Acl} (never null although some entries may be missing; this method + * should not throw {@link NotFoundException}, as a chain of {@link LookupStrategy}s may be used + * to automatically create entries if required) + */ + public Map readAclsById(List objects, List sids) { + Assert.isTrue(batchSize >= 1, "BatchSize must be >= 1"); + Assert.notEmpty(objects, "Objects to lookup required"); + + // Map + Map result = new HashMap(); // contains FULLY loaded Acl objects + + Set currentBatchToLoad = new HashSet(); // contains ObjectIdentitys + + for (int i = 0; i < objects.size(); i++) { + boolean aclFound = false; + + // Check we don't already have this ACL in the results + if (result.containsKey(objects.get(i))) { + aclFound = true; + } + + // Check cache for the present ACL entry + if (!aclFound) { + Acl acl = aclCache.getFromCache(objects.get(i)); + + // Ensure any cached element supports all the requested SIDs + // (they should always, as our base impl doesn't filter on SID) + if (acl != null) { + if (acl.isSidLoaded(sids)) { + result.put(acl.getObjectIdentity(), acl); + aclFound = true; + } else { + throw new IllegalStateException( + "Error: SID-filtered element detected when implementation does not perform SID filtering " + + "- have you added something to the cache manually?"); + } + } + } + + // Load the ACL from the database + if (!aclFound) { + currentBatchToLoad.add(objects.get(i)); + } + + // Is it time to load from JDBC the currentBatchToLoad? + if ((currentBatchToLoad.size() == this.batchSize) || ((i + 1) == objects.size())) { + if (currentBatchToLoad.size() > 0) { + Map loadedBatch = lookupObjectIdentities(currentBatchToLoad.toArray(new ObjectIdentity[] {}), sids); + + // Add loaded batch (all elements 100% initialized) to results + result.putAll(loadedBatch); + + // Add the loaded batch to the cache + Iterator loadedAclIterator = loadedBatch.values().iterator(); + + while (loadedAclIterator.hasNext()) { + aclCache.putInCache((AclImpl) loadedAclIterator.next()); + } + + currentBatchToLoad.clear(); + } + } + } + + return result; + } + + /** + * Looks up a batch of ObjectIdentitys directly from the database. + *

+ * The caller is responsible for optimization issues, such as selecting the identities to lookup, ensuring the + * cache doesn't contain them already, and adding the returned elements to the cache etc. *

* This subclass is required to return fully valid Acls, including properly-configured * parent ACLs. * */ - private Map lookupObjectIdentities(final ObjectIdentity[] objectIdentities, Sid[] sids) { + @SuppressWarnings("unchecked") + private Map lookupObjectIdentities(final ObjectIdentity[] objectIdentities, List sids) { Assert.notEmpty(objectIdentities, "Must provide identities to lookup"); final Map acls = new HashMap(); // contains Acls with StubAclParents @@ -343,120 +459,6 @@ public final class BasicLookupStrategy implements LookupStrategy { return resultMap; } - /** - * Locates the primary key IDs specified in "findNow", adding AclImpl instances with StubAclParents to the - * "acls" Map. - * - * @param acls the AclImpls (with StubAclParents) - * @param findNow Long-based primary keys to retrieve - * @param sids - */ - private void lookupPrimaryKeys(final Map acls, final Set findNow, final Sid[] sids) { - Assert.notNull(acls, "ACLs are required"); - Assert.notEmpty(findNow, "Items to find now required"); - - String sql = computeRepeatingSql("(acl_object_identity.id = ?)", findNow.size()); - - Set parentsToLookup = (Set) jdbcTemplate.query(sql, - new PreparedStatementSetter() { - public void setValues(PreparedStatement ps) - throws SQLException { - Iterator iter = findNow.iterator(); - int i = 0; - - while (iter.hasNext()) { - i++; - ps.setLong(i, ((Long) iter.next()).longValue()); - } - } - }, new ProcessResultSet(acls, sids)); - - // Lookup the parents, now that our JdbcTemplate has released the database connection (SEC-547) - if (parentsToLookup.size() > 0) { - lookupPrimaryKeys(acls, parentsToLookup, sids); - } - } - - /** - * The main method. - *

- * WARNING: This implementation completely disregards the "sids" argument! Every item in the cache is expected to - * contain all SIDs. If you have serious performance needs (e.g. a very large number of - * SIDs per object identity), you'll probably want to develop a custom {@link LookupStrategy} implementation - * instead. - *

- * The implementation works in batch sizes specified by {@link #batchSize}. - * - * @param objects the identities to lookup (required) - * @param sids the SIDs for which identities are required (ignored by this implementation) - * - * @return a Map where keys represent the {@link ObjectIdentity} of the located {@link Acl} and values - * are the located {@link Acl} (never null although some entries may be missing; this method - * should not throw {@link NotFoundException}, as a chain of {@link LookupStrategy}s may be used - * to automatically create entries if required) - */ - public Map readAclsById(ObjectIdentity[] objects, Sid[] sids) { - Assert.isTrue(batchSize >= 1, "BatchSize must be >= 1"); - Assert.notEmpty(objects, "Objects to lookup required"); - - // Map - Map result = new HashMap(); // contains FULLY loaded Acl objects - - Set currentBatchToLoad = new HashSet(); // contains ObjectIdentitys - - for (int i = 0; i < objects.length; i++) { - boolean aclFound = false; - - // Check we don't already have this ACL in the results - if (result.containsKey(objects[i])) { - aclFound = true; - } - - // Check cache for the present ACL entry - if (!aclFound) { - Acl acl = aclCache.getFromCache(objects[i]); - - // Ensure any cached element supports all the requested SIDs - // (they should always, as our base impl doesn't filter on SID) - if (acl != null) { - if (acl.isSidLoaded(sids)) { - result.put(acl.getObjectIdentity(), acl); - aclFound = true; - } else { - throw new IllegalStateException( - "Error: SID-filtered element detected when implementation does not perform SID filtering " - + "- have you added something to the cache manually?"); - } - } - } - - // Load the ACL from the database - if (!aclFound) { - currentBatchToLoad.add(objects[i]); - } - - // Is it time to load from JDBC the currentBatchToLoad? - if ((currentBatchToLoad.size() == this.batchSize) || ((i + 1) == objects.length)) { - if (currentBatchToLoad.size() > 0) { - Map loadedBatch = lookupObjectIdentities(currentBatchToLoad.toArray(new ObjectIdentity[] {}), sids); - - // Add loaded batch (all elements 100% initialized) to results - result.putAll(loadedBatch); - - // Add the loaded batch to the cache - Iterator loadedAclIterator = loadedBatch.values().iterator(); - - while (loadedAclIterator.hasNext()) { - aclCache.putInCache((AclImpl) loadedAclIterator.next()); - } - - currentBatchToLoad.clear(); - } - } - } - - return result; - } public void setBatchSize(int batchSize) { this.batchSize = batchSize; @@ -466,9 +468,9 @@ public final class BasicLookupStrategy implements LookupStrategy { private class ProcessResultSet implements ResultSetExtractor { private Map acls; - private Sid[] sids; + private List sids; - public ProcessResultSet(Map acls, Sid[] sids) { + public ProcessResultSet(Map acls, List sids) { Assert.notNull(acls, "ACLs cannot be null"); this.acls = acls; this.sids = sids; // can be null @@ -526,7 +528,7 @@ public final class BasicLookupStrategy implements LookupStrategy { this.id = id; } - public AccessControlEntry[] getEntries() { + public List getEntries() { throw new UnsupportedOperationException("Stub only"); } @@ -550,12 +552,12 @@ public final class BasicLookupStrategy implements LookupStrategy { throw new UnsupportedOperationException("Stub only"); } - public boolean isGranted(Permission[] permission, Sid[] sids, boolean administrativeMode) + public boolean isGranted(List permission, List sids, boolean administrativeMode) throws NotFoundException, UnloadedSidException { throw new UnsupportedOperationException("Stub only"); } - public boolean isSidLoaded(Sid[] sids) { + public boolean isSidLoaded(List sids) { throw new UnsupportedOperationException("Stub only"); } } diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java index 6c962cb2a9..bfe7025d06 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java @@ -33,6 +33,7 @@ import org.springframework.util.StringUtils; import java.sql.ResultSet; import java.sql.SQLException; +import java.util.Arrays; import java.util.List; import java.util.Map; @@ -42,7 +43,7 @@ import javax.sql.DataSource; /** * Simple JDBC-based implementation of AclService. *

- * Requires the "dirty" flags in {@link org.springframework.security.acls.domain.AclImpl} and + * Requires the "dirty" flags in {@link org.springframework.security.acls.domain.AclImpl} and * {@link org.springframework.security.acls.domain.AccessControlEntryImpl} to be set, so that the implementation can * detect changed parameters easily. * @@ -75,7 +76,7 @@ public class JdbcAclService implements AclService { //~ Methods ======================================================================================================== - public ObjectIdentity[] findChildren(ObjectIdentity parentIdentity) { + public List findChildren(ObjectIdentity parentIdentity) { Object[] args = {parentIdentity.getIdentifier(), parentIdentity.getJavaType().getName()}; List objects = jdbcTemplate.query(selectAclObjectWithParent, args, new RowMapper() { @@ -91,12 +92,12 @@ public class JdbcAclService implements AclService { if (objects.size() == 0) { return null; } - - return (ObjectIdentityImpl[]) objects.toArray(new ObjectIdentityImpl[objects.size()]); + + return objects; } - public Acl readAclById(ObjectIdentity object, Sid[] sids) throws NotFoundException { - Map map = readAclsById(new ObjectIdentity[] {object}, sids); + public Acl readAclById(ObjectIdentity object, List sids) throws NotFoundException { + Map map = readAclsById(Arrays.asList(object), sids); Assert.isTrue(map.containsKey(object), "There should have been an Acl entry for ObjectIdentity " + object); return (Acl) map.get(object); @@ -106,21 +107,21 @@ public class JdbcAclService implements AclService { return readAclById(object, null); } - public Map readAclsById(ObjectIdentity[] objects) throws NotFoundException { + public Map readAclsById(List objects) throws NotFoundException { return readAclsById(objects, null); } - public Map readAclsById(ObjectIdentity[] objects, Sid[] sids) throws NotFoundException { - Map result = lookupStrategy.readAclsById(objects, sids); - + public Map readAclsById(List objects, List sids) throws NotFoundException { + Map result = lookupStrategy.readAclsById(objects, sids); + // Check every requested object identity was found (throw NotFoundException if needed) - for (int i = 0; i < objects.length; i++) { - if (!result.containsKey(objects[i])) { + for (int i = 0; i < objects.size(); i++) { + if (!result.containsKey(objects.get(i))) { throw new NotFoundException("Unable to find ACL information for object identity '" - + objects[i].toString() + "'"); + + objects.get(i).toString() + "'"); } } - + return result; } } diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java index af9d1fbc6d..09f4751fe9 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java @@ -14,8 +14,15 @@ */ package org.springframework.security.acls.jdbc; -import org.springframework.security.Authentication; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.List; +import javax.sql.DataSource; + +import org.springframework.dao.DataAccessException; +import org.springframework.jdbc.core.BatchPreparedStatementSetter; +import org.springframework.security.Authentication; import org.springframework.security.acls.AccessControlEntry; import org.springframework.security.acls.Acl; import org.springframework.security.acls.AlreadyExistsException; @@ -29,26 +36,10 @@ import org.springframework.security.acls.objectidentity.ObjectIdentityImpl; import org.springframework.security.acls.sid.GrantedAuthoritySid; import org.springframework.security.acls.sid.PrincipalSid; import org.springframework.security.acls.sid.Sid; - import org.springframework.security.context.SecurityContextHolder; - -import org.springframework.dao.DataAccessException; - -import org.springframework.jdbc.core.BatchPreparedStatementSetter; - import org.springframework.transaction.support.TransactionSynchronizationManager; - import org.springframework.util.Assert; -import java.lang.reflect.Array; - -import java.sql.PreparedStatement; -import java.sql.SQLException; - -import java.util.List; - -import javax.sql.DataSource; - /** * Provides a base implementation of {@link MutableAclService}. @@ -123,14 +114,13 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS jdbcTemplate.batchUpdate(insertEntry, new BatchPreparedStatementSetter() { public int getBatchSize() { - return acl.getEntries().length; + return acl.getEntries().size(); } public void setValues(PreparedStatement stmt, int i) throws SQLException { - AccessControlEntry entry_ = (AccessControlEntry) Array.get(acl.getEntries(), i); + AccessControlEntry entry_ = acl.getEntries().get(i); Assert.isTrue(entry_ instanceof AccessControlEntryImpl, "Unknown ACE class"); - AccessControlEntryImpl entry = (AccessControlEntryImpl) entry_; stmt.setLong(1, ((Long) acl.getId()).longValue()); @@ -167,23 +157,21 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS * * @return the primary key or null if not found */ - protected Long createOrRetrieveClassPrimaryKey(Class clazz, boolean allowCreate) { - List classIds = jdbcTemplate.queryForList(selectClassPrimaryKey, new Object[] {clazz.getName()}, Long.class); - Long classId = null; + protected Long createOrRetrieveClassPrimaryKey(Class clazz, boolean allowCreate) { + List classIds = jdbcTemplate.queryForList(selectClassPrimaryKey, new Object[] {clazz.getName()}, Long.class); - if (classIds.isEmpty()) { - if (allowCreate) { - classId = null; - jdbcTemplate.update(insertClass, new Object[] {clazz.getName()}); - Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(), - "Transaction must be running"); - classId = new Long(jdbcTemplate.queryForLong(classIdentityQuery)); - } - } else { - classId = (Long) classIds.iterator().next(); + if (!classIds.isEmpty()) { + return classIds.get(0); } - return classId; + if (allowCreate) { + jdbcTemplate.update(insertClass, new Object[] {clazz.getName()}); + Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(), + "Transaction must be running"); + return new Long(jdbcTemplate.queryForLong(classIdentityQuery)); + } + + return null; } /** @@ -195,7 +183,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS * * @return the primary key or null if not found * - * @throws IllegalArgumentException DOCUMENT ME! + * @throws IllegalArgumentException if the Sid is not a recognized implementation. */ protected Long createOrRetrieveSidPrimaryKey(Sid sid, boolean allowCreate) { Assert.notNull(sid, "Sid required"); @@ -212,23 +200,21 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS throw new IllegalArgumentException("Unsupported implementation of Sid"); } - List sidIds = jdbcTemplate.queryForList(selectSidPrimaryKey, new Object[] {new Boolean(principal), sidName}, - Long.class); - Long sidId = null; + List sidIds = jdbcTemplate.queryForList(selectSidPrimaryKey, + new Object[] {new Boolean(principal), sidName}, Long.class); - if (sidIds.isEmpty()) { - if (allowCreate) { - sidId = null; - jdbcTemplate.update(insertSid, new Object[] {new Boolean(principal), sidName}); - Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(), - "Transaction must be running"); - sidId = new Long(jdbcTemplate.queryForLong(sidIdentityQuery)); - } - } else { - sidId = (Long) sidIds.iterator().next(); + if (!sidIds.isEmpty()) { + return sidIds.get(0); } - return sidId; + if (allowCreate) { + jdbcTemplate.update(insertSid, new Object[] {new Boolean(principal), sidName}); + Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(), + "Transaction must be running"); + return new Long(jdbcTemplate.queryForLong(sidIdentityQuery)); + } + + return null; } public void deleteAcl(ObjectIdentity objectIdentity, boolean deleteChildren) @@ -237,26 +223,26 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS Assert.notNull(objectIdentity.getIdentifier(), "Object Identity doesn't provide an identifier"); if (deleteChildren) { - ObjectIdentity[] children = findChildren(objectIdentity); + List children = findChildren(objectIdentity); if (children != null) { - for (int i = 0; i < children.length; i++) { - deleteAcl(children[i], true); + for (int i = 0; i < children.size(); i++) { + deleteAcl(children.get(i), true); } } } else { if (!foreignKeysInDatabase) { // We need to perform a manual verification for what a FK would normally do // We generally don't do this, in the interests of deadlock management - ObjectIdentity[] children = findChildren(objectIdentity); + List children = findChildren(objectIdentity); if (children != null) { - throw new ChildrenExistException("Cannot delete '" + objectIdentity + "' (has " + children.length + throw new ChildrenExistException("Cannot delete '" + objectIdentity + "' (has " + children.size() + " children)"); } } } Long oidPrimaryKey = retrieveObjectIdentityPrimaryKey(objectIdentity); - + // Delete this ACL's ACEs in the acl_entry table deleteEntries(oidPrimaryKey); @@ -279,11 +265,9 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS /** * Deletes a single row from acl_object_identity that is associated with the presented ObjectIdentity primary key. - * *

* We do not delete any entries from acl_class, even if no classes are using that class any longer. This is a * deadlock avoidance approach. - *

* * @param oidPrimaryKey to delete the acl_object_identity */ @@ -314,12 +298,6 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS * This implementation will simply delete all ACEs in the database and recreate them on each invocation of * this method. A more comprehensive implementation might use dirty state checking, or more likely use ORM * capabilities for create, update and delete operations of {@link MutableAcl}. - * - * @param acl DOCUMENT ME! - * - * @return DOCUMENT ME! - * - * @throws NotFoundException DOCUMENT ME! */ public MutableAcl updateAcl(MutableAcl acl) throws NotFoundException { Assert.notNull(acl.getId(), "Object Identity doesn't provide an identifier"); @@ -339,13 +317,13 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS // Retrieve the ACL via superclass (ensures cache registration, proper retrieval etc) return (MutableAcl) super.readAclById(acl.getObjectIdentity()); } - + private void clearCacheIncludingChildren(ObjectIdentity objectIdentity) { Assert.notNull(objectIdentity, "ObjectIdentity required"); - ObjectIdentity[] children = findChildren(objectIdentity); + List children = findChildren(objectIdentity); if (children != null) { - for (int i = 0; i < children.length; i++) { - clearCacheIncludingChildren(children[i]); + for (int i = 0; i < children.size(); i++) { + clearCacheIncludingChildren(children.get(i)); } } aclCache.evictFromCache(objectIdentity); @@ -357,7 +335,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS * * @param acl to modify (a row must already exist in acl_object_identity) * - * @throws NotFoundException DOCUMENT ME! + * @throws NotFoundException if the ACL could not be found to update. */ protected void updateObjectIdentity(MutableAcl acl) { Long parentId = null; diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/LookupStrategy.java b/acl/src/main/java/org/springframework/security/acls/jdbc/LookupStrategy.java index 2794d83bd8..1b87f454be 100644 --- a/acl/src/main/java/org/springframework/security/acls/jdbc/LookupStrategy.java +++ b/acl/src/main/java/org/springframework/security/acls/jdbc/LookupStrategy.java @@ -19,6 +19,7 @@ import org.springframework.security.acls.NotFoundException; import org.springframework.security.acls.objectidentity.ObjectIdentity; import org.springframework.security.acls.sid.Sid; +import java.util.List; import java.util.Map; @@ -43,5 +44,5 @@ public interface LookupStrategy { * should not throw {@link NotFoundException}, as a chain of {@link LookupStrategy}s may be used * to automatically create entries if required) */ - Map readAclsById(ObjectIdentity[] objects, Sid[] sids); + Map readAclsById(List objects, List sids); } diff --git a/acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategy.java b/acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategy.java index 85908c9dd9..e9f5745bbf 100644 --- a/acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategy.java +++ b/acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategy.java @@ -15,6 +15,8 @@ package org.springframework.security.acls.sid; +import java.util.List; + import org.springframework.security.Authentication; @@ -28,5 +30,5 @@ import org.springframework.security.Authentication; public interface SidRetrievalStrategy { //~ Methods ======================================================================================================== - Sid[] getSids(Authentication authentication); + List getSids(Authentication authentication); } diff --git a/acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategyImpl.java b/acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategyImpl.java index d517c72c56..1839173de8 100644 --- a/acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategyImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategyImpl.java @@ -15,6 +15,7 @@ package org.springframework.security.acls.sid; +import java.util.ArrayList; import java.util.List; import org.springframework.security.Authentication; @@ -32,14 +33,14 @@ import org.springframework.security.GrantedAuthority; public class SidRetrievalStrategyImpl implements SidRetrievalStrategy { //~ Methods ======================================================================================================== - public Sid[] getSids(Authentication authentication) { + public List getSids(Authentication authentication) { List authorities = authentication.getAuthorities(); - Sid[] sids = new Sid[authorities.size() + 1]; + List sids = new ArrayList(authorities.size() + 1); - sids[0] = new PrincipalSid(authentication); + sids.add(new PrincipalSid(authentication)); - for (int i = 1; i <= authorities.size(); i++) { - sids[i] = new GrantedAuthoritySid(authorities.get(i - 1)); + for (GrantedAuthority authority : authorities) { + sids.add(new GrantedAuthoritySid(authority)); } return sids; diff --git a/acl/src/main/java/org/springframework/security/acls/vote/AclEntryVoter.java b/acl/src/main/java/org/springframework/security/acls/vote/AclEntryVoter.java index 2a94f291f6..a8a4d18e6f 100644 --- a/acl/src/main/java/org/springframework/security/acls/vote/AclEntryVoter.java +++ b/acl/src/main/java/org/springframework/security/acls/vote/AclEntryVoter.java @@ -16,6 +16,7 @@ package org.springframework.security.acls.vote; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.List; import org.apache.commons.logging.Log; @@ -49,8 +50,8 @@ import org.springframework.util.StringUtils; * The voter will vote if any {@link ConfigAttribute#getAttribute()} matches the {@link #processConfigAttribute}. * The provider will then locate the first method argument of type {@link #processDomainObjectClass}. Assuming that * method argument is non-null, the provider will then lookup the ACLs from the AclManager and ensure the - * principal is {@link Acl#isGranted(org.springframework.security.acls.Permission[], - * org.springframework.security.acls.sid.Sid[], boolean)} when presenting the {@link #requirePermission} array to that + * principal is {@link Acl#isGranted(List, + * List, boolean)} when presenting the {@link #requirePermission} array to that * method. *

* If the method argument is null, the voter will abstain from voting. If the method argument @@ -91,7 +92,7 @@ public class AclEntryVoter extends AbstractAclVoter { private SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl(); private String internalMethod; private String processConfigAttribute; - private Permission[] requirePermission; + private List requirePermission; //~ Constructors =================================================================================================== @@ -105,7 +106,7 @@ public class AclEntryVoter extends AbstractAclVoter { this.aclService = aclService; this.processConfigAttribute = processConfigAttribute; - this.requirePermission = requirePermission; + this.requirePermission = Arrays.asList(requirePermission); } //~ Methods ======================================================================================================== @@ -196,7 +197,7 @@ public class AclEntryVoter extends AbstractAclVoter { ObjectIdentity objectIdentity = objectIdentityRetrievalStrategy.getObjectIdentity(domainObject); // Obtain the SIDs applicable to the principal - Sid[] sids = sidRetrievalStrategy.getSids(authentication); + List sids = sidRetrievalStrategy.getSids(authentication); Acl acl; diff --git a/acl/src/test/java/org/springframework/security/acls/AclPermissionEvaluatorTests.java b/acl/src/test/java/org/springframework/security/acls/AclPermissionEvaluatorTests.java index 9969f6140e..d58e7ada33 100644 --- a/acl/src/test/java/org/springframework/security/acls/AclPermissionEvaluatorTests.java +++ b/acl/src/test/java/org/springframework/security/acls/AclPermissionEvaluatorTests.java @@ -1,6 +1,8 @@ package org.springframework.security.acls; -import static org.junit.Assert.*; +import static org.junit.Assert.assertTrue; + +import java.util.List; import org.jmock.Expectations; import org.jmock.Mockery; @@ -10,7 +12,6 @@ import org.junit.Test; import org.springframework.security.Authentication; import org.springframework.security.acls.objectidentity.ObjectIdentity; import org.springframework.security.acls.objectidentity.ObjectIdentityRetrievalStrategy; -import org.springframework.security.acls.sid.Sid; import org.springframework.security.acls.sid.SidRetrievalStrategy; /** @@ -45,9 +46,9 @@ public class AclPermissionEvaluatorTests { ignoring(user); ignoring(oidStrategy); ignoring(sidStrategy); - oneOf(service).readAclById(with(any(ObjectIdentity.class)), with(any(Sid[].class))); + oneOf(service).readAclById(with(any(ObjectIdentity.class)), with(any(List.class))); will(returnValue(acl)); - oneOf(acl).isGranted(with(any(Permission[].class)), with(any(Sid[].class)), with(equal(false))); + oneOf(acl).isGranted(with(any(List.class)), with(any(List.class)), with(equal(false))); will(returnValue(true)); }}); diff --git a/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java b/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java index 40ea0a99be..a26502e356 100644 --- a/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java +++ b/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java @@ -3,6 +3,8 @@ package org.springframework.security.acls.domain; import static org.junit.Assert.*; import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; @@ -41,6 +43,13 @@ import org.springframework.security.util.FieldUtils; * @author Andrei Stefan */ public class AclImplTests { + private static final List READ = Arrays.asList(BasePermission.READ ); + private static final List WRITE = Arrays.asList(BasePermission.WRITE); + private static final List CREATE = Arrays.asList(BasePermission.CREATE ); + private static final List DELETE = Arrays.asList(BasePermission.DELETE ); + private static final List SCOTT = Arrays.asList((Sid)new PrincipalSid("scott")); + private static final List BEN = Arrays.asList((Sid)new PrincipalSid("ben")); + Authentication auth = new TestingAuthenticationToken("johndoe", "ignored", "ROLE_ADMINISTRATOR"); Mockery jmockCtx = new Mockery(); AclAuthorizationStrategy mockAuthzStrategy; @@ -137,31 +146,31 @@ public class AclImplTests { acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST1"), true); service.updateAcl(acl); // Check it was successfully added - assertEquals(1, acl.getEntries().length); - assertEquals(acl.getEntries()[0].getAcl(), acl); - assertEquals(acl.getEntries()[0].getPermission(), BasePermission.READ); - assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST1")); + assertEquals(1, acl.getEntries().size()); + assertEquals(acl.getEntries().get(0).getAcl(), acl); + assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.READ); + assertEquals(acl.getEntries().get(0).getSid(), new GrantedAuthoritySid("ROLE_TEST1")); // Add a second permission 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); - assertEquals(acl.getEntries()[1].getAcl(), acl); - assertEquals(acl.getEntries()[1].getPermission(), BasePermission.READ); - assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST2")); + assertEquals(2, acl.getEntries().size()); + assertEquals(acl.getEntries().get(1).getAcl(), acl); + assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.READ); + assertEquals(acl.getEntries().get(1).getSid(), new GrantedAuthoritySid("ROLE_TEST2")); // Add a third permission, after the first one acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_TEST3"), false); service.updateAcl(acl); - assertEquals(3, acl.getEntries().length); + assertEquals(3, acl.getEntries().size()); // Check the third entry was added between the two existent ones - assertEquals(acl.getEntries()[0].getPermission(), BasePermission.READ); - assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST1")); - assertEquals(acl.getEntries()[1].getPermission(), BasePermission.WRITE); - assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST3")); - assertEquals(acl.getEntries()[2].getPermission(), BasePermission.READ); - assertEquals(acl.getEntries()[2].getSid(), new GrantedAuthoritySid("ROLE_TEST2")); + assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.READ); + assertEquals(acl.getEntries().get(0).getSid(), new GrantedAuthoritySid("ROLE_TEST1")); + assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.WRITE); + assertEquals(acl.getEntries().get(1).getSid(), new GrantedAuthoritySid("ROLE_TEST3")); + assertEquals(acl.getEntries().get(2).getPermission(), BasePermission.READ); + assertEquals(acl.getEntries().get(2).getSid(), new GrantedAuthoritySid("ROLE_TEST2")); } @Test(expected=NotFoundException.class) @@ -191,22 +200,22 @@ public class AclImplTests { // Delete first permission and check the order of the remaining permissions is kept 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")); + assertEquals(2, acl.getEntries().size()); + assertEquals(acl.getEntries().get(0).getSid(), new GrantedAuthoritySid("ROLE_TEST2")); + assertEquals(acl.getEntries().get(1).getSid(), new GrantedAuthoritySid("ROLE_TEST3")); // Add one more permission and remove the permission in the middle acl.insertAce(2, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST4"), true); service.updateAcl(acl); 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")); + assertEquals(2, acl.getEntries().size()); + assertEquals(acl.getEntries().get(0).getSid(), new GrantedAuthoritySid("ROLE_TEST2")); + assertEquals(acl.getEntries().get(1).getSid(), new GrantedAuthoritySid("ROLE_TEST4")); // Remove remaining permissions acl.deleteAce(1); acl.deleteAce(0); - assertEquals(0, acl.getEntries().length); + assertEquals(0, acl.getEntries().size()); } @Test @@ -229,14 +238,15 @@ public class AclImplTests { public void testIsGrantingRejectsEmptyParameters() throws Exception { MutableAcl acl = new AclImpl(objectIdentity, new Long(1), mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid( "johndoe")); + Sid ben = new PrincipalSid("ben"); try { - acl.isGranted(new Permission[] {}, new Sid[] { new PrincipalSid("ben") }, false); + acl.isGranted(new ArrayList(0), Arrays.asList(ben) , false); fail("It should have thrown IllegalArgumentException"); } catch (IllegalArgumentException expected) { } try { - acl.isGranted(new Permission[] { BasePermission.READ }, new Sid[] {}, false); + acl.isGranted(READ, new ArrayList(0), false); fail("It should have thrown IllegalArgumentException"); } catch (IllegalArgumentException expected) { @@ -261,25 +271,22 @@ public class AclImplTests { rootAcl.insertAce(3, BasePermission.WRITE, new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), true); // Check permissions granting - Permission[] permissions = new Permission[] { BasePermission.READ, BasePermission.CREATE }; - Sid[] sids = new Sid[] { new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_GUEST") }; + List permissions = Arrays.asList(BasePermission.READ, BasePermission.CREATE); + List sids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_GUEST")); assertFalse(rootAcl.isGranted(permissions, sids, false)); try { - rootAcl.isGranted(permissions, new Sid[] { new PrincipalSid("scott") }, false); + rootAcl.isGranted(permissions, SCOTT, false); fail("It should have thrown NotFoundException"); } catch (NotFoundException expected) { } - assertTrue(rootAcl.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new PrincipalSid("scott") }, - false)); - assertFalse(rootAcl.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new PrincipalSid("rod"), - new GrantedAuthoritySid("WRITE_ACCESS_ROLE") }, false)); - assertTrue(rootAcl.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { - new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), new PrincipalSid("rod") }, false)); + assertTrue(rootAcl.isGranted(WRITE, SCOTT, false)); + assertFalse(rootAcl.isGranted(WRITE, + Arrays.asList(new PrincipalSid("rod"), new GrantedAuthoritySid("WRITE_ACCESS_ROLE")), false)); + assertTrue(rootAcl.isGranted(WRITE, Arrays.asList(new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), new PrincipalSid("rod")), false)); try { // Change the type of the Sid and check the granting process - rootAcl.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new GrantedAuthoritySid("rod"), - new PrincipalSid("WRITE_ACCESS_ROLE") }, false); + rootAcl.isGranted(WRITE, Arrays.asList(new GrantedAuthoritySid("rod"), new PrincipalSid("WRITE_ACCESS_ROLE")), false); fail("It should have thrown NotFoundException"); } catch (NotFoundException expected) { @@ -326,45 +333,33 @@ public class AclImplTests { 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") }, - false)); - assertTrue(parentAcl1.isGranted(new Permission[] { BasePermission.READ }, new Sid[] { new GrantedAuthoritySid( - "ROLE_USER_READ") }, false)); - assertTrue(parentAcl1.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new PrincipalSid("ben") }, - false)); - assertFalse(parentAcl1.isGranted(new Permission[] { BasePermission.DELETE }, new Sid[] { new PrincipalSid("ben") }, - false)); - assertFalse(parentAcl1.isGranted(new Permission[] { BasePermission.DELETE }, - new Sid[] { new PrincipalSid("scott") }, false)); + assertTrue(parentAcl1.isGranted(READ, SCOTT, false)); + assertTrue(parentAcl1.isGranted(READ, Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_USER_READ")), false)); + assertTrue(parentAcl1.isGranted(WRITE, BEN, false)); + assertFalse(parentAcl1.isGranted(DELETE, BEN, false)); + assertFalse(parentAcl1.isGranted(DELETE, SCOTT, false)); // Check granting process for parent2 - assertTrue(parentAcl2.isGranted(new Permission[] { BasePermission.CREATE }, new Sid[] { new PrincipalSid("ben") }, - false)); - assertTrue(parentAcl2.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new PrincipalSid("ben") }, - false)); - assertFalse(parentAcl2.isGranted(new Permission[] { BasePermission.DELETE }, new Sid[] { new PrincipalSid("ben") }, - false)); + assertTrue(parentAcl2.isGranted(CREATE, BEN, false)); + assertTrue(parentAcl2.isGranted(WRITE, BEN, false)); + assertFalse(parentAcl2.isGranted(DELETE, BEN, false)); // Check granting process for child1 - assertTrue(childAcl1.isGranted(new Permission[] { BasePermission.CREATE }, new Sid[] { new PrincipalSid("scott") }, - false)); - assertTrue(childAcl1.isGranted(new Permission[] { BasePermission.READ }, new Sid[] { new GrantedAuthoritySid( - "ROLE_USER_READ") }, false)); - assertFalse(childAcl1.isGranted(new Permission[] { BasePermission.DELETE }, new Sid[] { new PrincipalSid("ben") }, + assertTrue(childAcl1.isGranted(CREATE, SCOTT, false)); + assertTrue(childAcl1.isGranted(READ, Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_USER_READ")), false)); + assertFalse(childAcl1.isGranted(DELETE, BEN, false)); // Check granting process for child2 (doesn't inherit the permissions from its parent) try { - assertTrue(childAcl2.isGranted(new Permission[] { BasePermission.CREATE }, - new Sid[] { new PrincipalSid("scott") }, false)); + assertTrue(childAcl2.isGranted(CREATE, SCOTT, false)); fail("It should have thrown NotFoundException"); } catch (NotFoundException expected) { assertTrue(true); } try { - assertTrue(childAcl2.isGranted(new Permission[] { BasePermission.CREATE }, new Sid[] { new PrincipalSid( - "johndoe") }, false)); + assertTrue(childAcl2.isGranted(CREATE, Arrays.asList((Sid)new PrincipalSid("johndoe")), false)); fail("It should have thrown NotFoundException"); } catch (NotFoundException expected) { @@ -386,9 +381,9 @@ public class AclImplTests { acl.insertAce(2, BasePermission.CREATE, new PrincipalSid("ben"), true); service.updateAcl(acl); - assertEquals(acl.getEntries()[0].getPermission(), BasePermission.READ); - assertEquals(acl.getEntries()[1].getPermission(), BasePermission.WRITE); - assertEquals(acl.getEntries()[2].getPermission(), BasePermission.CREATE); + assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.READ); + assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.WRITE); + assertEquals(acl.getEntries().get(2).getPermission(), BasePermission.CREATE); // Change each permission acl.updateAce(0, BasePermission.CREATE); @@ -396,9 +391,9 @@ public class AclImplTests { acl.updateAce(2, BasePermission.READ); // Check the change was successfuly made - assertEquals(acl.getEntries()[0].getPermission(), BasePermission.CREATE); - assertEquals(acl.getEntries()[1].getPermission(), BasePermission.DELETE); - assertEquals(acl.getEntries()[2].getPermission(), BasePermission.READ); + assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.CREATE); + assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.DELETE); + assertEquals(acl.getEntries().get(2).getPermission(), BasePermission.READ); } @Test @@ -414,20 +409,20 @@ public class AclImplTests { acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true); service.updateAcl(acl); - assertFalse(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditFailure()); - assertFalse(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditFailure()); - assertFalse(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditSuccess()); - assertFalse(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditSuccess()); + assertFalse(((AuditableAccessControlEntry) acl.getEntries().get(0)).isAuditFailure()); + assertFalse(((AuditableAccessControlEntry) acl.getEntries().get(1)).isAuditFailure()); + assertFalse(((AuditableAccessControlEntry) acl.getEntries().get(0)).isAuditSuccess()); + assertFalse(((AuditableAccessControlEntry) acl.getEntries().get(1)).isAuditSuccess()); // Change each permission ((AuditableAcl) acl).updateAuditing(0, true, true); ((AuditableAcl) acl).updateAuditing(1, true, true); // Check the change was successfuly made - assertTrue(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditFailure()); - assertTrue(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditFailure()); - assertTrue(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditSuccess()); - assertTrue(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditSuccess()); + assertTrue(((AuditableAccessControlEntry) acl.getEntries().get(0)).isAuditFailure()); + assertTrue(((AuditableAccessControlEntry) acl.getEntries().get(1)).isAuditFailure()); + assertTrue(((AuditableAccessControlEntry) acl.getEntries().get(0)).isAuditSuccess()); + assertTrue(((AuditableAccessControlEntry) acl.getEntries().get(1)).isAuditSuccess()); } @Test @@ -452,7 +447,7 @@ public class AclImplTests { assertEquals(acl.getOwner(), new PrincipalSid("johndoe")); assertNull(acl.getParentAcl()); assertTrue(acl.isEntriesInheriting()); - assertEquals(2, acl.getEntries().length); + assertEquals(2, acl.getEntries().size()); acl.setParent(parentAcl); assertEquals(acl.getParentAcl(), parentAcl); @@ -466,19 +461,19 @@ public class AclImplTests { @Test public void testIsSidLoaded() throws Exception { - Sid[] loadedSids = new Sid[] { new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_IGNORED") }; + List loadedSids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_IGNORED")); MutableAcl acl = new AclImpl(objectIdentity, new Long(1), mockAuthzStrategy, mockAuditLogger, null, loadedSids, true, new PrincipalSid( "johndoe")); assertTrue(acl.isSidLoaded(loadedSids)); - assertTrue(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_IGNORED"), new PrincipalSid("ben") })); - assertTrue(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_IGNORED")})); - assertTrue(acl.isSidLoaded(new Sid[] { new PrincipalSid("ben") })); + assertTrue(acl.isSidLoaded(Arrays.asList(new GrantedAuthoritySid("ROLE_IGNORED"), new PrincipalSid("ben")))); + assertTrue(acl.isSidLoaded(Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_IGNORED")))); + assertTrue(acl.isSidLoaded(BEN)); assertTrue(acl.isSidLoaded(null)); - assertTrue(acl.isSidLoaded(new Sid[] { })); - assertTrue(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_IGNORED"), new GrantedAuthoritySid("ROLE_IGNORED") })); - assertFalse(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_GENERAL"), new GrantedAuthoritySid("ROLE_IGNORED") })); - assertFalse(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_IGNORED"), new GrantedAuthoritySid("ROLE_GENERAL") })); + assertTrue(acl.isSidLoaded(new ArrayList(0))); + assertTrue(acl.isSidLoaded(Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_IGNORED"), new GrantedAuthoritySid("ROLE_IGNORED")))); + assertFalse(acl.isSidLoaded(Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_GENERAL"), new GrantedAuthoritySid("ROLE_IGNORED")))); + assertFalse(acl.isSidLoaded(Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_IGNORED"), new GrantedAuthoritySid("ROLE_GENERAL")))); } //~ Inner Classes ================================================================================================== @@ -495,8 +490,9 @@ public class AclImplTests { * Mock implementation that populates the aces list with fully initialized AccessControlEntries * @see org.springframework.security.acls.MutableAclService#updateAcl(org.springframework.security.acls.MutableAcl) */ + @SuppressWarnings("unchecked") public MutableAcl updateAcl(MutableAcl acl) throws NotFoundException { - AccessControlEntry[] oldAces = acl.getEntries(); + List oldAces = acl.getEntries(); Field acesField = FieldUtils.getField(AclImpl.class, "aces"); acesField.setAccessible(true); List newAces; @@ -504,8 +500,8 @@ public class AclImplTests { newAces = (List) acesField.get(acl); newAces.clear(); - for (int i = 0; i < oldAces.length; i++) { - AccessControlEntry ac = oldAces[i]; + for (int i = 0; i < oldAces.size(); i++) { + AccessControlEntry ac = oldAces.get(i); // Just give an ID to all this acl's aces, rest of the fields are just copied newAces.add(new AccessControlEntryImpl(new Long(i + 1), ac.getAcl(), ac.getSid(), ac.getPermission(), ac .isGranting(), ((AuditableAccessControlEntry) ac).isAuditSuccess(), @@ -519,7 +515,7 @@ public class AclImplTests { return acl; } - public ObjectIdentity[] findChildren(ObjectIdentity parentIdentity) { + public List findChildren(ObjectIdentity parentIdentity) { return null; } @@ -527,15 +523,15 @@ public class AclImplTests { return null; } - public Acl readAclById(ObjectIdentity object, Sid[] sids) throws NotFoundException { + public Acl readAclById(ObjectIdentity object, List sids) throws NotFoundException { return null; } - public Map readAclsById(ObjectIdentity[] objects) throws NotFoundException { + public Map readAclsById(List objects) throws NotFoundException { return null; } - public Map readAclsById(ObjectIdentity[] objects, Sid[] sids) throws NotFoundException { + public Map readAclsById(List objects, List sids) throws NotFoundException { return null; } } diff --git a/acl/src/test/java/org/springframework/security/acls/jdbc/AclPermissionInheritanceTests.java b/acl/src/test/java/org/springframework/security/acls/jdbc/AclPermissionInheritanceTests.java index dcb60d5c4e..7de1282823 100644 --- a/acl/src/test/java/org/springframework/security/acls/jdbc/AclPermissionInheritanceTests.java +++ b/acl/src/test/java/org/springframework/security/acls/jdbc/AclPermissionInheritanceTests.java @@ -89,12 +89,11 @@ public class AclPermissionInheritanceTests extends TestCase { parent = (MutableAcl) child.getParentAcl(); - assertEquals("Fails because child has a stale reference to its parent", - 2, parent.getEntries().length); - assertEquals(1, parent.getEntries()[0].getPermission().getMask()); - assertEquals(new PrincipalSid("john"), parent.getEntries()[0].getSid()); - assertEquals(1, parent.getEntries()[1].getPermission().getMask()); - assertEquals(new PrincipalSid("joe"), parent.getEntries()[1].getSid()); + assertEquals("Fails because child has a stale reference to its parent", 2, parent.getEntries().size()); + assertEquals(1, parent.getEntries().get(0).getPermission().getMask()); + assertEquals(new PrincipalSid("john"), parent.getEntries().get(0).getSid()); + assertEquals(1, parent.getEntries().get(1).getPermission().getMask()); + assertEquals(new PrincipalSid("joe"), parent.getEntries().get(1).getSid()); } public void test2() throws Exception { @@ -121,11 +120,11 @@ public class AclPermissionInheritanceTests extends TestCase { parent = (MutableAcl) child.getParentAcl(); - assertEquals(2, parent.getEntries().length); - assertEquals(16, parent.getEntries()[0].getPermission().getMask()); - assertEquals(new GrantedAuthoritySid("ROLE_ADMINISTRATOR"), parent.getEntries()[0].getSid()); - assertEquals(8, parent.getEntries()[1].getPermission().getMask()); - assertEquals(new PrincipalSid("terry"), parent.getEntries()[1].getSid()); + assertEquals(2, parent.getEntries().size()); + assertEquals(16, parent.getEntries().get(0).getPermission().getMask()); + assertEquals(new GrantedAuthoritySid("ROLE_ADMINISTRATOR"), parent.getEntries().get(0).getSid()); + assertEquals(8, parent.getEntries().get(1).getPermission().getMask()); + assertEquals(new PrincipalSid("terry"), parent.getEntries().get(1).getSid()); } diff --git a/acl/src/test/java/org/springframework/security/acls/jdbc/BasicLookupStrategyTests.java b/acl/src/test/java/org/springframework/security/acls/jdbc/BasicLookupStrategyTests.java index 8fde9dc0ea..cd972f8b43 100644 --- a/acl/src/test/java/org/springframework/security/acls/jdbc/BasicLookupStrategyTests.java +++ b/acl/src/test/java/org/springframework/security/acls/jdbc/BasicLookupStrategyTests.java @@ -1,5 +1,7 @@ package org.springframework.security.acls.jdbc; +import java.util.Arrays; +import java.util.List; import java.util.Map; import junit.framework.Assert; @@ -42,11 +44,8 @@ public class BasicLookupStrategyTests { //~ Instance fields ================================================================================================ private static JdbcTemplate jdbcTemplate; - private LookupStrategy strategy; - private static TestDataSource dataSource; - private static CacheManager cacheManager; //~ Methods ======================================================================================================== @@ -123,7 +122,7 @@ public class BasicLookupStrategyTests { // Deliberately use an integer for the child, to reproduce bug report in SEC-819 ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(102)); - Map map = this.strategy.readAclsById(new ObjectIdentity[] { topParentOid, middleParentOid, childOid }, null); + Map map = this.strategy.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null); checkEntries(topParentOid, middleParentOid, childOid, map); } @@ -134,11 +133,11 @@ public class BasicLookupStrategyTests { ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(102)); // Objects were put in cache - strategy.readAclsById(new ObjectIdentity[] { topParentOid, middleParentOid, childOid }, null); + strategy.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null); // Let's empty the database to force acls retrieval from cache emptyDatabase(); - Map map = this.strategy.readAclsById(new ObjectIdentity[] { topParentOid, middleParentOid, childOid }, null); + Map map = this.strategy.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null); checkEntries(topParentOid, middleParentOid, childOid, map); } @@ -151,7 +150,7 @@ public class BasicLookupStrategyTests { // Set a batch size to allow multiple database queries in order to retrieve all acls ((BasicLookupStrategy) this.strategy).setBatchSize(1); - Map map = this.strategy.readAclsById(new ObjectIdentity[] { topParentOid, middleParentOid, childOid }, null); + Map map = this.strategy.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null); checkEntries(topParentOid, middleParentOid, childOid, map); } @@ -174,9 +173,9 @@ public class BasicLookupStrategyTests { Assert.assertEquals(middleParentOid, child.getParentAcl().getObjectIdentity()); // Check their ACEs were correctly retrieved - Assert.assertEquals(2, topParent.getEntries().length); - Assert.assertEquals(1, middleParent.getEntries().length); - Assert.assertEquals(1, child.getEntries().length); + Assert.assertEquals(2, topParent.getEntries().size()); + Assert.assertEquals(1, middleParent.getEntries().size()); + Assert.assertEquals(1, child.getEntries().size()); // Check object identities were correctly retrieved Assert.assertEquals(topParentOid, topParent.getObjectIdentity()); @@ -187,39 +186,39 @@ public class BasicLookupStrategyTests { Assert.assertTrue(topParent.isEntriesInheriting()); Assert.assertEquals(topParent.getId(), new Long(1)); Assert.assertEquals(topParent.getOwner(), new PrincipalSid("ben")); - Assert.assertEquals(topParent.getEntries()[0].getId(), new Long(1)); - Assert.assertEquals(topParent.getEntries()[0].getPermission(), BasePermission.READ); - Assert.assertEquals(topParent.getEntries()[0].getSid(), new PrincipalSid("ben")); - Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[0]).isAuditFailure()); - Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[0]).isAuditSuccess()); - Assert.assertTrue(((AuditableAccessControlEntry) topParent.getEntries()[0]).isGranting()); + Assert.assertEquals(topParent.getEntries().get(0).getId(), new Long(1)); + Assert.assertEquals(topParent.getEntries().get(0).getPermission(), BasePermission.READ); + Assert.assertEquals(topParent.getEntries().get(0).getSid(), new PrincipalSid("ben")); + Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(0)).isAuditFailure()); + Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(0)).isAuditSuccess()); + Assert.assertTrue(((AuditableAccessControlEntry) topParent.getEntries().get(0)).isGranting()); - Assert.assertEquals(topParent.getEntries()[1].getId(), new Long(2)); - Assert.assertEquals(topParent.getEntries()[1].getPermission(), BasePermission.WRITE); - Assert.assertEquals(topParent.getEntries()[1].getSid(), new PrincipalSid("ben")); - Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[1]).isAuditFailure()); - Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[1]).isAuditSuccess()); - Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[1]).isGranting()); + Assert.assertEquals(topParent.getEntries().get(1).getId(), new Long(2)); + Assert.assertEquals(topParent.getEntries().get(1).getPermission(), BasePermission.WRITE); + Assert.assertEquals(topParent.getEntries().get(1).getSid(), new PrincipalSid("ben")); + Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(1)).isAuditFailure()); + Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(1)).isAuditSuccess()); + Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(1)).isGranting()); Assert.assertTrue(middleParent.isEntriesInheriting()); Assert.assertEquals(middleParent.getId(), new Long(2)); Assert.assertEquals(middleParent.getOwner(), new PrincipalSid("ben")); - Assert.assertEquals(middleParent.getEntries()[0].getId(), new Long(3)); - Assert.assertEquals(middleParent.getEntries()[0].getPermission(), BasePermission.DELETE); - Assert.assertEquals(middleParent.getEntries()[0].getSid(), new PrincipalSid("ben")); - Assert.assertFalse(((AuditableAccessControlEntry) middleParent.getEntries()[0]).isAuditFailure()); - Assert.assertFalse(((AuditableAccessControlEntry) middleParent.getEntries()[0]).isAuditSuccess()); - Assert.assertTrue(((AuditableAccessControlEntry) middleParent.getEntries()[0]).isGranting()); + Assert.assertEquals(middleParent.getEntries().get(0).getId(), new Long(3)); + Assert.assertEquals(middleParent.getEntries().get(0).getPermission(), BasePermission.DELETE); + Assert.assertEquals(middleParent.getEntries().get(0).getSid(), new PrincipalSid("ben")); + Assert.assertFalse(((AuditableAccessControlEntry) middleParent.getEntries().get(0)).isAuditFailure()); + Assert.assertFalse(((AuditableAccessControlEntry) middleParent.getEntries().get(0)).isAuditSuccess()); + Assert.assertTrue(((AuditableAccessControlEntry) middleParent.getEntries().get(0)).isGranting()); Assert.assertTrue(child.isEntriesInheriting()); Assert.assertEquals(child.getId(), new Long(3)); Assert.assertEquals(child.getOwner(), new PrincipalSid("ben")); - Assert.assertEquals(child.getEntries()[0].getId(), new Long(4)); - Assert.assertEquals(child.getEntries()[0].getPermission(), BasePermission.DELETE); - Assert.assertEquals(child.getEntries()[0].getSid(), new PrincipalSid("ben")); - Assert.assertFalse(((AuditableAccessControlEntry) child.getEntries()[0]).isAuditFailure()); - Assert.assertFalse(((AuditableAccessControlEntry) child.getEntries()[0]).isAuditSuccess()); - Assert.assertFalse((child.getEntries()[0]).isGranting()); + Assert.assertEquals(child.getEntries().get(0).getId(), new Long(4)); + Assert.assertEquals(child.getEntries().get(0).getPermission(), BasePermission.DELETE); + Assert.assertEquals(child.getEntries().get(0).getSid(), new PrincipalSid("ben")); + Assert.assertFalse(((AuditableAccessControlEntry) child.getEntries().get(0)).isAuditFailure()); + Assert.assertFalse(((AuditableAccessControlEntry) child.getEntries().get(0)).isAuditSuccess()); + Assert.assertFalse((child.getEntries().get(0)).isGranting()); } @Test @@ -233,7 +232,7 @@ public class BasicLookupStrategyTests { ObjectIdentity middleParent2Oid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(103)); // Retrieve the child - Map map = this.strategy.readAclsById(new ObjectIdentity[] { childOid }, null); + Map map = this.strategy.readAclsById(Arrays.asList(childOid), null); // Check that the child and all its parents were retrieved Assert.assertNotNull(map.get(childOid)); @@ -265,9 +264,9 @@ public class BasicLookupStrategyTests { ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(107)); // First lookup only child, thus populating the cache with grandParent, parent1 and child - Permission[] checkPermission = new Permission[] { BasePermission.READ }; - Sid[] sids = new Sid[] { new PrincipalSid("ben") }; - ObjectIdentity[] childOids = new ObjectIdentity[] { childOid }; + List checkPermission = Arrays.asList(BasePermission.READ); + List sids = Arrays.asList((Sid)new PrincipalSid("ben")); + List childOids = Arrays.asList(childOid); ((BasicLookupStrategy) this.strategy).setBatchSize(6); Map foundAcls = strategy.readAclsById(childOids, sids); @@ -278,7 +277,7 @@ public class BasicLookupStrategyTests { // Search for object identities has to be done in the following order: last element have to be one which // is already in cache and the element before it must not be stored in cache - ObjectIdentity[] allOids = new ObjectIdentity[] { grandParentOid, parent1Oid, parent2Oid, childOid }; + List allOids = Arrays.asList(grandParentOid, parent1Oid, parent2Oid, childOid); try { foundAcls = strategy.readAclsById(allOids, sids); Assert.assertTrue(true); diff --git a/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java b/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java index d4ac82c695..070c89f8f1 100644 --- a/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java +++ b/acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java @@ -14,6 +14,8 @@ */ package org.springframework.security.acls.jdbc; +import java.util.Arrays; +import java.util.List; import java.util.Map; import org.springframework.security.Authentication; @@ -35,7 +37,6 @@ import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.providers.TestingAuthenticationToken; import org.springframework.test.AbstractTransactionalDataSourceSpringContextTests; - /** * Integration tests the ACL system using an in-memory database. * @@ -45,23 +46,23 @@ import org.springframework.test.AbstractTransactionalDataSourceSpringContextTest */ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringContextTests { //~ Constant fields ================================================================================================ - + public static final String SELECT_ALL_CLASSES = "SELECT * FROM acl_class WHERE class = ?"; - + public static final String SELECT_ALL_OBJECT_IDENTITIES = "SELECT * FROM acl_object_identity"; - + public static final String SELECT_OBJECT_IDENTITY = "SELECT * FROM acl_object_identity WHERE object_id_identity = ?"; - + public static final String SELECT_ACL_ENTRY = "SELECT * FROM acl_entry, acl_object_identity WHERE " + "acl_object_identity.id = acl_entry.acl_object_identity " + "AND acl_object_identity.object_id_identity <= ?"; //~ Instance fields ================================================================================================ - + private JdbcMutableAclService jdbcMutableAclService; - + private AclCache aclCache; - + private LookupStrategy lookupStrategy; //~ Methods ======================================================================================================== @@ -119,7 +120,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo jdbcMutableAclService.updateAcl(child); // Let's check if we can read them back correctly - Map map = jdbcMutableAclService.readAclsById(new ObjectIdentity[] {topParentOid, middleParentOid, childOid}); + Map map = jdbcMutableAclService.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid)); assertEquals(3, map.size()); // Replace our current objects with their retrieved versions @@ -138,27 +139,33 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo assertEquals(middleParentOid, child.getParentAcl().getObjectIdentity()); // Check their ACEs were correctly persisted - assertEquals(2, topParent.getEntries().length); - assertEquals(1, middleParent.getEntries().length); - assertEquals(1, child.getEntries().length); + assertEquals(2, topParent.getEntries().size()); + assertEquals(1, middleParent.getEntries().size()); + assertEquals(1, child.getEntries().size()); // 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(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false)); + List read = Arrays.asList(BasePermission.READ); + List write = Arrays.asList(BasePermission.WRITE); + List delete = Arrays.asList(BasePermission.DELETE); + List pSid = Arrays.asList((Sid)new PrincipalSid(auth)); + + + assertTrue(topParent.isGranted(read, pSid, false)); + assertFalse(topParent.isGranted(write, pSid, false)); + assertTrue(middleParent.isGranted(delete, pSid, false)); + assertFalse(child.isGranted(delete, pSid, false)); try { - child.isGranted(new Permission[] {BasePermission.ADMINISTRATION}, new Sid[] {new PrincipalSid(auth)}, false); + child.isGranted(Arrays.asList(BasePermission.ADMINISTRATION), pSid, false); fail("Should have thrown NotFoundException"); } catch (NotFoundException expected) { assertTrue(true); } // Now check the inherited rights (when not explicitly overridden) also look OK - assertTrue(child.isGranted(new Permission[] {BasePermission.READ}, new Sid[] {new PrincipalSid(auth)}, false)); - assertFalse(child.isGranted(new Permission[] {BasePermission.WRITE}, new Sid[] {new PrincipalSid(auth)}, false)); - assertFalse(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false)); + assertTrue(child.isGranted(read, pSid, false)); + assertFalse(child.isGranted(write, pSid, false)); + assertFalse(child.isGranted(delete, pSid, false)); // Next change the child so it doesn't inherit permissions from above child.setEntriesInheriting(false); @@ -167,17 +174,17 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo assertFalse(child.isEntriesInheriting()); // Check the child permissions no longer inherit - assertFalse(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, true)); + assertFalse(child.isGranted(delete, pSid, true)); try { - child.isGranted(new Permission[] {BasePermission.READ}, new Sid[] {new PrincipalSid(auth)}, true); + child.isGranted(read, pSid, true); fail("Should have thrown NotFoundException"); } catch (NotFoundException expected) { assertTrue(true); } try { - child.isGranted(new Permission[] {BasePermission.WRITE}, new Sid[] {new PrincipalSid(auth)}, true); + child.isGranted(write, pSid, true); fail("Should have thrown NotFoundException"); } catch (NotFoundException expected) { assertTrue(true); @@ -192,19 +199,19 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo // Save the changed child jdbcMutableAclService.updateAcl(child); child = (MutableAcl) jdbcMutableAclService.readAclById(childOid); - assertEquals(3, child.getEntries().length); + assertEquals(3, child.getEntries().size()); // Output permissions - for (int i = 0; i < child.getEntries().length; i++) { - System.out.println(child.getEntries()[i]); + for (int i = 0; i < child.getEntries().size(); i++) { + System.out.println(child.getEntries().get(i)); } // Check the permissions are as they should be - assertFalse(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, true)); // as earlier permission overrode - assertTrue(child.isGranted(new Permission[] {BasePermission.CREATE}, new Sid[] {new PrincipalSid(auth)}, true)); + assertFalse(child.isGranted(delete, pSid, true)); // as earlier permission overrode + assertTrue(child.isGranted(Arrays.asList(BasePermission.CREATE), pSid, true)); // Now check the first ACE (index 0) really is DELETE for our Sid and is non-granting - AccessControlEntry entry = child.getEntries()[0]; + AccessControlEntry entry = child.getEntries().get(0); assertEquals(BasePermission.DELETE.getMask(), entry.getPermission().getMask()); assertEquals(new PrincipalSid(auth), entry.getSid()); assertFalse(entry.isGranting()); @@ -215,12 +222,12 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo // Save and check it worked child = jdbcMutableAclService.updateAcl(child); - assertEquals(2, child.getEntries().length); - assertTrue(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false)); + assertEquals(2, child.getEntries().size()); + assertTrue(child.isGranted(delete, pSid, false)); SecurityContextHolder.clearContext(); } - + /** * Test method that demonstrates eviction failure from cache - SEC-676 */ @@ -232,10 +239,10 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo // Check the childOid really is a child of middleParentOid Acl childAcl = jdbcMutableAclService.readAclById(childOid); assertEquals(middleParentOid, childAcl.getParentAcl().getObjectIdentity()); - + // Delete the mid-parent and test if the child was deleted, as well jdbcMutableAclService.deleteAcl(middleParentOid, true); - + try { Acl acl = jdbcMutableAclService.readAclById(middleParentOid); fail("It should have thrown NotFoundException"); @@ -250,12 +257,12 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo catch (NotFoundException expected) { assertTrue(true); } - + Acl acl = jdbcMutableAclService.readAclById(topParentOid); assertNotNull(acl); assertEquals(((MutableAcl) acl).getObjectIdentity(), topParentOid); } - + public void testConstructorRejectsNullParameters() throws Exception { try { JdbcAclService service = new JdbcMutableAclService(null, lookupStrategy, aclCache); @@ -264,7 +271,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo catch (IllegalArgumentException expected) { assertTrue(true); } - + try { JdbcAclService service = new JdbcMutableAclService(this.getJdbcTemplate().getDataSource(), null, aclCache); fail("It should have thrown IllegalArgumentException"); @@ -272,7 +279,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo catch (IllegalArgumentException expected) { assertTrue(true); } - + try { JdbcAclService service = new JdbcMutableAclService(this.getJdbcTemplate().getDataSource(), lookupStrategy, null); fail("It should have thrown IllegalArgumentException"); @@ -281,7 +288,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo assertTrue(true); } } - + public void testCreateAclRejectsNullParameter() throws Exception { try { jdbcMutableAclService.createAcl(null); @@ -291,10 +298,10 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo assertTrue(true); } } - + public void testCreateAclForADuplicateDomainObject() throws Exception { ObjectIdentity duplicateOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); - + // Try to add the same object second time try { jdbcMutableAclService.createAcl(duplicateOid); @@ -304,7 +311,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo assertTrue(true); } } - + public void testDeleteAclRejectsNullParameters() throws Exception { try { jdbcMutableAclService.deleteAcl(null, true); @@ -314,7 +321,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo assertTrue(true); } } - + public void testDeleteAclWithChildrenThrowsException() throws Exception { try { ObjectIdentity topParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); @@ -328,7 +335,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo jdbcMutableAclService.setForeignKeysInDatabase(true); // restore to the default } } - + public void testDeleteAclRemovesRowsFromDatabase() throws Exception { Authentication auth = new TestingAuthenticationToken("ben", "ignored", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ADMINISTRATOR")}); @@ -338,19 +345,19 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo ObjectIdentity topParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); ObjectIdentity middleParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(101)); ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(102)); - + // Remove the child and check all related database rows were removed accordingly jdbcMutableAclService.deleteAcl(childOid, false); assertEquals(1, getJdbcTemplate().queryForList(SELECT_ALL_CLASSES, new Object[] {"org.springframework.security.TargetObject"} ).size()); assertEquals(0, getJdbcTemplate().queryForList(SELECT_OBJECT_IDENTITY, new Object[] {new Long(102)}).size()); assertEquals(2, getJdbcTemplate().queryForList(SELECT_ALL_OBJECT_IDENTITIES).size()); assertEquals(3, getJdbcTemplate().queryForList(SELECT_ACL_ENTRY, new Object[] {new Long(103)} ).size()); - + // Check the cache assertNull(aclCache.getFromCache(childOid)); assertNull(aclCache.getFromCache(new Long(102))); } - + /** * SEC-655 */ @@ -365,7 +372,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo MutableAcl parent = jdbcMutableAclService.createAcl(parentOid); MutableAcl child = jdbcMutableAclService.createAcl(childOid); - + child.setParent(parent); jdbcMutableAclService.updateAcl(child); @@ -380,13 +387,13 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo child = (MutableAcl) jdbcMutableAclService.readAclById(childOid); parent = (MutableAcl) child.getParentAcl(); - assertEquals("Fails because child has a stale reference to its parent", 2, parent.getEntries().length); + assertEquals("Fails because child has a stale reference to its parent", 2, parent.getEntries().size()); assertEquals(1, parent.getEntries()[0].getPermission().getMask()); assertEquals(new PrincipalSid("ben"), parent.getEntries()[0].getSid()); assertEquals(1, parent.getEntries()[1].getPermission().getMask()); assertEquals(new PrincipalSid("scott"), parent.getEntries()[1].getSid()); }*/ - + /* public void testCumulativePermissions() { setComplete(); Authentication auth = new TestingAuthenticationToken("ben", "ignored", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ADMINISTRATOR")}); @@ -400,14 +407,14 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo CumulativePermission cm = new CumulativePermission().set(BasePermission.READ).set(BasePermission.ADMINISTRATION); assertEquals(17, cm.getMask()); topParent.insertAce(null, cm, new PrincipalSid(auth), true); - assertEquals(1, topParent.getEntries().length); + assertEquals(1, topParent.getEntries().size()); // Explictly save the changed ACL topParent = jdbcMutableAclService.updateAcl(topParent); // Check the mask was retrieved correctly assertEquals(17, topParent.getEntries()[0].getPermission().getMask()); - assertTrue(topParent.isGranted(new Permission[] {cm}, new Sid[] {new PrincipalSid(auth)}, true)); + assertTrue(topParent.isGranted(new Permission[] {cm}, pSid, true)); SecurityContextHolder.clearContext(); } diff --git a/acl/src/test/java/org/springframework/security/acls/sid/SidRetrievalStrategyTests.java b/acl/src/test/java/org/springframework/security/acls/sid/SidRetrievalStrategyTests.java index 032c5f1011..3431490223 100644 --- a/acl/src/test/java/org/springframework/security/acls/sid/SidRetrievalStrategyTests.java +++ b/acl/src/test/java/org/springframework/security/acls/sid/SidRetrievalStrategyTests.java @@ -1,5 +1,7 @@ package org.springframework.security.acls.sid; +import java.util.List; + import junit.framework.Assert; import junit.framework.TestCase; @@ -18,20 +20,20 @@ public class SidRetrievalStrategyTests extends TestCase { public void testSidsRetrieval() throws Exception { Authentication authentication = new TestingAuthenticationToken("scott", "password", "ROLE_1", "ROLE_2", "ROLE_3"); SidRetrievalStrategy retrStrategy = new SidRetrievalStrategyImpl(); - Sid[] sids = retrStrategy.getSids(authentication); + List sids = retrStrategy.getSids(authentication); assertNotNull(sids); - assertEquals(4, sids.length); - assertNotNull(sids[0]); - assertTrue(sids[0] instanceof PrincipalSid); + assertEquals(4, sids.size()); + assertNotNull(sids.get(0)); + assertTrue(sids.get(0) instanceof PrincipalSid); - for (int i = 1; i < sids.length; i++) { - assertTrue(sids[i] instanceof GrantedAuthoritySid); + for (int i = 1; i < sids.size(); i++) { + assertTrue(sids.get(i) instanceof GrantedAuthoritySid); } - Assert.assertEquals("scott", ((PrincipalSid) sids[0]).getPrincipal()); - Assert.assertEquals("ROLE_1", ((GrantedAuthoritySid) sids[1]).getGrantedAuthority()); - Assert.assertEquals("ROLE_2", ((GrantedAuthoritySid) sids[2]).getGrantedAuthority()); - Assert.assertEquals("ROLE_3", ((GrantedAuthoritySid) sids[3]).getGrantedAuthority()); + Assert.assertEquals("scott", ((PrincipalSid) sids.get(0)).getPrincipal()); + Assert.assertEquals("ROLE_1", ((GrantedAuthoritySid) sids.get(1)).getGrantedAuthority()); + Assert.assertEquals("ROLE_2", ((GrantedAuthoritySid) sids.get(2)).getGrantedAuthority()); + Assert.assertEquals("ROLE_3", ((GrantedAuthoritySid) sids.get(3)).getGrantedAuthority()); } }