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 bd0110328b..4f424ac000 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 @@ -49,6 +49,7 @@ import org.springframework.security.acls.sid.PrincipalSid; import org.springframework.security.acls.sid.Sid; import org.springframework.security.util.FieldUtils; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; /** @@ -293,10 +294,10 @@ public final class BasicLookupStrategy implements LookupStrategy { for (int i = 0; i < objectIdentities.length; i++) { // Determine prepared statement values for this iteration String javaType = objectIdentities[i].getJavaType().getName(); - Assert.isInstanceOf(Long.class, objectIdentities[i].getIdentifier(), - "This class requires ObjectIdentity.getIdentifier() to be a Long"); - long id = ((Long) objectIdentities[i].getIdentifier()).longValue(); + // No need to check for nulls, as guaranteed non-null by ObjectIdentity.getIdentifier() interface contract + String identifier = objectIdentities[i].getIdentifier().toString(); + long id = (Long.valueOf(identifier)).longValue(); // Inject values ps.setLong((2 * i) + 1, id); diff --git a/acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java b/acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java index b7d79e1114..cf5fb7ac29 100644 --- a/acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java @@ -15,6 +15,7 @@ package org.springframework.security.acls.objectidentity; import org.springframework.security.acls.IdentityUnavailableException; +import org.springframework.security.acls.jdbc.LookupStrategy; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -97,6 +98,12 @@ public class ObjectIdentityImpl implements ObjectIdentity { /** * Important so caching operates properly.

Considers an object of the same class equal if it has the same * classname and id properties.

+ * + *

+ * Note that this class uses string equality for the identifier field, which ensures it better supports + * differences between {@link LookupStrategy} requirements and the domain object represented by this + * ObjectIdentityImpl. + *

* * @param arg0 object to compare * @@ -113,7 +120,7 @@ public class ObjectIdentityImpl implements ObjectIdentity { ObjectIdentityImpl other = (ObjectIdentityImpl) arg0; - if (this.getIdentifier().equals(other.getIdentifier()) && this.getJavaType().equals(other.getJavaType())) { + if (this.getIdentifier().toString().equals(other.getIdentifier().toString()) && this.getJavaType().equals(other.getJavaType())) { return true; } 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 9d25edfd73..2c1d5a84c7 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 @@ -120,7 +120,8 @@ public class BasicLookupStrategyTests { public void testAclsRetrievalWithDefaultBatchSize() throws Exception { 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 Long(102)); + // 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); checkEntries(topParentOid, middleParentOid, childOid, map); @@ -128,7 +129,7 @@ public class BasicLookupStrategyTests { @Test public void testAclsRetrievalFromCacheOnly() throws Exception { - ObjectIdentity topParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); + ObjectIdentity topParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(100)); ObjectIdentity middleParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(101)); ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(102)); @@ -145,7 +146,7 @@ public class BasicLookupStrategyTests { @Test public void testAclsRetrievalWithCustomBatchSize() throws Exception { ObjectIdentity topParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); - ObjectIdentity middleParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(101)); + ObjectIdentity middleParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(101)); ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(102)); // Set a batch size to allow multiple database queries in order to retrieve all acls @@ -227,7 +228,7 @@ public class BasicLookupStrategyTests { jdbcTemplate.execute(query); ObjectIdentity topParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); - ObjectIdentity middleParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(101)); + ObjectIdentity middleParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(101)); ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(102)); ObjectIdentity middleParent2Oid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(103)); @@ -260,8 +261,8 @@ public class BasicLookupStrategyTests { ObjectIdentity grandParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(104)); ObjectIdentity parent1Oid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(105)); - ObjectIdentity parent2Oid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(106)); - ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(107)); + ObjectIdentity parent2Oid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(106)); + 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 }; @@ -290,21 +291,4 @@ public class BasicLookupStrategyTests { Assert.assertTrue(foundParent2Acl.isGranted(checkPermission, sids, false)); } - @Test - public void testAclsWithDifferentSerializableTypesAsObjectIdentities() throws Exception { - String query = "INSERT INTO acl_object_identity(ID,OBJECT_ID_CLASS,OBJECT_ID_IDENTITY,PARENT_OBJECT,OWNER_SID,ENTRIES_INHERITING) VALUES (4,2,104,null,1,1);" - + "INSERT INTO acl_entry(ID,ACL_OBJECT_IDENTITY,ACE_ORDER,SID,MASK,GRANTING,AUDIT_SUCCESS,AUDIT_FAILURE) VALUES (5,4,0,1,1,1,0,0)"; - jdbcTemplate.execute(query); - - ObjectIdentity oid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(104)); - Sid[] sids = new Sid[] { new PrincipalSid("ben") }; - ObjectIdentity[] childOids = new ObjectIdentity[] { oid }; - - try { - Map foundAcls = strategy.readAclsById(childOids, sids); - Assert.fail("It should have thrown IllegalArgumentException"); - } catch(IllegalArgumentException expected) { - Assert.assertTrue(true); - } - } } \ No newline at end of file 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 5c7cf6214c..d4ac82c695 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 @@ -97,7 +97,7 @@ 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 Long(102)); + ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(102)); MutableAcl topParent = jdbcMutableAclService.createAcl(topParentOid); MutableAcl middleParent = jdbcMutableAclService.createAcl(middleParentOid); @@ -337,7 +337,7 @@ 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 Long(102)); + 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);