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 5633f057e3..55821026a2 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 @@ -20,9 +20,9 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; @@ -172,57 +172,6 @@ public final class BasicLookupStrategy implements LookupStrategy { return sqlStringBldr.toString(); } - /** - * The final phase of converting the Map of AclImpl instances which contain - * StubAclParents into proper, valid AclImpls with correct ACL parents. - * - * @param inputMap the unconverted AclImpls - * @param currentIdentity the currentAcl that we wish to convert (this may be - * - */ - private AclImpl convert(Map inputMap, Long currentIdentity) { - Assert.notEmpty(inputMap, "InputMap required"); - Assert.notNull(currentIdentity, "CurrentIdentity required"); - - // Retrieve this Acl from the InputMap - Acl uncastAcl = (Acl) inputMap.get(currentIdentity); - Assert.isInstanceOf(AclImpl.class, uncastAcl, "The inputMap contained a non-AclImpl"); - - AclImpl inputAcl = (AclImpl) uncastAcl; - - Acl parent = inputAcl.getParentAcl(); - - if ((parent != null) && parent instanceof StubAclParent) { - // Lookup the parent - StubAclParent stubAclParent = (StubAclParent) parent; - parent = convert(inputMap, stubAclParent.getId()); - } - - // Now we have the parent (if there is one), create the true AclImpl - AclImpl result = new AclImpl(inputAcl.getObjectIdentity(), (Long) inputAcl.getId(), aclAuthorizationStrategy, - auditLogger, parent, null, inputAcl.isEntriesInheriting(), inputAcl.getOwner()); - - // Copy the "aces" from the input to the destination - - // Obtain the "aces" from the input ACL - List aces = readAces(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 - for (AccessControlEntryImpl ace : aces) { - setAclOnAce(ace, result); - acesNew.add(ace); - } - - // Finally, now that the "aces" have been converted to have the "result" AclImpl instance, modify the "result" AclImpl instance - setAces(result, acesNew); - - return result; - } - @SuppressWarnings("unchecked") private List readAces(AclImpl acl) { try { @@ -305,19 +254,20 @@ public final class BasicLookupStrategy implements LookupStrategy { // Map Map result = new HashMap(); // contains FULLY loaded Acl objects - Set currentBatchToLoad = new HashSet(); // contains ObjectIdentitys + Set currentBatchToLoad = new HashSet(); for (int i = 0; i < objects.size(); i++) { + final ObjectIdentity oid = objects.get(i); boolean aclFound = false; // Check we don't already have this ACL in the results - if (result.containsKey(objects.get(i))) { + if (result.containsKey(oid)) { aclFound = true; } // Check cache for the present ACL entry if (!aclFound) { - Acl acl = aclCache.getFromCache(objects.get(i)); + Acl acl = aclCache.getFromCache(oid); // Ensure any cached element supports all the requested SIDs // (they should always, as our base impl doesn't filter on SID) @@ -335,22 +285,21 @@ public final class BasicLookupStrategy implements LookupStrategy { // Load the ACL from the database if (!aclFound) { - currentBatchToLoad.add(objects.get(i)); + currentBatchToLoad.add(oid); } // 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); + Map loadedBatch = lookupObjectIdentities(currentBatchToLoad, 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()); + for (Acl loadedAcl : loadedBatch.values()) { + aclCache.putInCache((AclImpl) loadedAcl); } currentBatchToLoad.clear(); @@ -371,31 +320,31 @@ public final class BasicLookupStrategy implements LookupStrategy { * parent ACLs. * */ - @SuppressWarnings("unchecked") - private Map lookupObjectIdentities(final ObjectIdentity[] objectIdentities, List sids) { + private Map lookupObjectIdentities(final Collection objectIdentities, List sids) { Assert.notEmpty(objectIdentities, "Must provide identities to lookup"); - final Map acls = new HashMap(); // contains Acls with StubAclParents + final Map acls = new HashMap(); // contains Acls with StubAclParents // Make the "acls" map contain all requested objectIdentities // (including markers to each parent in the hierarchy) - String sql = computeRepeatingSql(lookupObjectIdentitiesWhereClause , - objectIdentities.length); + String sql = computeRepeatingSql(lookupObjectIdentitiesWhereClause, objectIdentities.size()); - Set parentsToLookup = (Set) jdbcTemplate.query(sql, + Set parentsToLookup = jdbcTemplate.query(sql, new PreparedStatementSetter() { public void setValues(PreparedStatement ps) throws SQLException { - for (int i = 0; i < objectIdentities.length; i++) { + int i = 0; + for (ObjectIdentity oid : objectIdentities) { // Determine prepared statement values for this iteration - String javaType = objectIdentities[i].getType(); + String type = oid.getType(); // No need to check for nulls, as guaranteed non-null by ObjectIdentity.getIdentifier() interface contract - String identifier = objectIdentities[i].getIdentifier().toString(); + String identifier = oid.getIdentifier().toString(); long id = (Long.valueOf(identifier)).longValue(); // Inject values ps.setLong((2 * i) + 1, id); - ps.setString((2 * i) + 2, javaType); + ps.setString((2 * i) + 2, type); + i++; } } }, new ProcessResultSet(acls, sids)); @@ -407,10 +356,8 @@ public final class BasicLookupStrategy implements LookupStrategy { // Finally, convert our "acls" containing StubAclParents into true Acls Map resultMap = new HashMap(); - Iterator iter = acls.values().iterator(); - while (iter.hasNext()) { - Acl inputAcl = (Acl) iter.next(); + for (Acl inputAcl : acls.values()) { Assert.isInstanceOf(AclImpl.class, inputAcl, "Map should have contained an AclImpl"); Assert.isInstanceOf(Long.class, ((AclImpl) inputAcl).getId(), "Acl.getId() must be Long"); @@ -421,6 +368,57 @@ public final class BasicLookupStrategy implements LookupStrategy { return resultMap; } + /** + * The final phase of converting the Map of AclImpl instances which contain + * StubAclParents into proper, valid AclImpls with correct ACL parents. + * + * @param inputMap the unconverted AclImpls + * @param currentIdentity the currentAcl that we wish to convert (this may be + * + */ + private AclImpl convert(Map inputMap, Long currentIdentity) { + Assert.notEmpty(inputMap, "InputMap required"); + Assert.notNull(currentIdentity, "CurrentIdentity required"); + + // Retrieve this Acl from the InputMap + Acl uncastAcl = inputMap.get(currentIdentity); + Assert.isInstanceOf(AclImpl.class, uncastAcl, "The inputMap contained a non-AclImpl"); + + AclImpl inputAcl = (AclImpl) uncastAcl; + + Acl parent = inputAcl.getParentAcl(); + + if ((parent != null) && parent instanceof StubAclParent) { + // Lookup the parent + StubAclParent stubAclParent = (StubAclParent) parent; + parent = convert(inputMap, stubAclParent.getId()); + } + + // Now we have the parent (if there is one), create the true AclImpl + AclImpl result = new AclImpl(inputAcl.getObjectIdentity(), (Long) inputAcl.getId(), aclAuthorizationStrategy, + auditLogger, parent, null, inputAcl.isEntriesInheriting(), inputAcl.getOwner()); + + // Copy the "aces" from the input to the destination + + // Obtain the "aces" from the input ACL + List aces = readAces(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 + for (AccessControlEntryImpl ace : aces) { + setAclOnAce(ace, result); + acesNew.add(ace); + } + + // Finally, now that the "aces" have been converted to have the "result" AclImpl instance, modify the "result" AclImpl instance + setAces(result, acesNew); + + return result; + } + /** * Sets the {@code PermissionFactory} instance which will be used to convert loaded permission * data values to {@code Permission}s. A {@code DefaultPermissionFactory} will be used by default.