Acl.
@@ -42,10 +40,10 @@ import java.util.Vector;
*/
public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
//~ Instance fields ================================================================================================
-
+
private Acl parentAcl;
- private AclAuthorizationStrategy aclAuthorizationStrategy;
- private AuditLogger auditLogger;
+ private transient AclAuthorizationStrategy aclAuthorizationStrategy;
+ private transient AuditLogger auditLogger;
private List aces = new Vector();
private ObjectIdentity objectIdentity;
private Serializable id;
@@ -368,6 +366,8 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
sb.append("inheriting: ").append(this.entriesInheriting).append("; ");
sb.append("parent: ").append((this.parentAcl == null) ? "Null" : this.parentAcl.getObjectIdentity().toString());
+ sb.append("aclAuthorizationStrategy: ").append(this.aclAuthorizationStrategy).append("; ");
+ sb.append("auditLogger: ").append(this.auditLogger);
sb.append("]");
return sb.toString();
@@ -404,4 +404,36 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
ace.setAuditFailure(auditFailure);
}
}
+
+ public boolean equals(Object obj) {
+ if (obj instanceof AclImpl) {
+
+ AclImpl rhs = (AclImpl) obj;
+ if (this.aces.equals(rhs.aces)) {
+ if ((this.parentAcl == null && rhs.parentAcl == null) || (this.parentAcl.equals(rhs.parentAcl))) {
+ if ((this.objectIdentity == null && rhs.objectIdentity == null) || (this.objectIdentity.equals(rhs.objectIdentity))) {
+ if ((this.id == null && rhs.id == null) || (this.id.equals(rhs.id))) {
+ if ((this.owner == null && rhs.owner == null) || this.owner.equals(rhs.owner)) {
+ if (this.entriesInheriting == rhs.entriesInheriting) {
+ 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])) {
+ return false;
+ }
+ }
+ return true;
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ return false;
+ }
+
}
diff --git a/acl/src/main/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCache.java b/acl/src/main/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCache.java
index 4a217410a8..aa2508b65a 100644
--- a/acl/src/main/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCache.java
+++ b/acl/src/main/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCache.java
@@ -14,20 +14,28 @@
*/
package org.springframework.security.acls.jdbc;
+import java.io.Serializable;
+
import net.sf.ehcache.CacheException;
-import net.sf.ehcache.Element;
import net.sf.ehcache.Ehcache;
+import net.sf.ehcache.Element;
import org.springframework.security.acls.MutableAcl;
+import org.springframework.security.acls.domain.AclAuthorizationStrategy;
+import org.springframework.security.acls.domain.AclImpl;
+import org.springframework.security.acls.domain.AuditLogger;
import org.springframework.security.acls.objectidentity.ObjectIdentity;
-
+import org.springframework.security.util.FieldUtils;
import org.springframework.util.Assert;
-import java.io.Serializable;
-
/**
* Simple implementation of {@link AclCache} that delegates to EH-CACHE.
+ *
+ * + * Designed to handle the transient fields in {@link AclImpl}. Note that this implementation assumes all + * {@link AclImpl} instances share the same {@link AuditLogger} and {@link AclAuthorizationStrategy} instance. + *
* * @author Ben Alex * @version $Id$ @@ -36,6 +44,8 @@ public class EhCacheBasedAclCache implements AclCache { //~ Instance fields ================================================================================================ private Ehcache cache; + private AuditLogger auditLogger; + private AclAuthorizationStrategy aclAuthorizationStrategy; //~ Constructors =================================================================================================== @@ -81,10 +91,10 @@ public class EhCacheBasedAclCache implements AclCache { return null; } - return (MutableAcl) element.getValue(); + return initializeTransientFields((MutableAcl)element.getValue()); } - public MutableAcl getFromCache(Serializable pk) { + public MutableAcl getFromCache(Serializable pk) { Assert.notNull(pk, "Primary key (identifier) required"); Element element = null; @@ -97,7 +107,7 @@ public class EhCacheBasedAclCache implements AclCache { return null; } - return (MutableAcl) element.getValue(); + return initializeTransientFields((MutableAcl) element.getValue()); } public void putInCache(MutableAcl acl) { @@ -105,6 +115,13 @@ public class EhCacheBasedAclCache implements AclCache { Assert.notNull(acl.getObjectIdentity(), "ObjectIdentity required"); Assert.notNull(acl.getId(), "ID required"); + if (this.aclAuthorizationStrategy == null) { + if (acl instanceof AclImpl) { + this.aclAuthorizationStrategy = (AclAuthorizationStrategy) FieldUtils.getProtectedFieldValue("aclAuthorizationStrategy", acl); + this.auditLogger = (AuditLogger) FieldUtils.getProtectedFieldValue("auditLogger", acl); + } + } + if ((acl.getParentAcl() != null) && (acl.getParentAcl() instanceof MutableAcl)) { putInCache((MutableAcl) acl.getParentAcl()); } @@ -112,4 +129,12 @@ public class EhCacheBasedAclCache implements AclCache { cache.put(new Element(acl.getObjectIdentity(), acl)); cache.put(new Element(acl.getId(), acl)); } + + private MutableAcl initializeTransientFields(MutableAcl value) { + if (value instanceof AclImpl) { + FieldUtils.setProtectedFieldValue("aclAuthorizationStrategy", value, this.aclAuthorizationStrategy); + FieldUtils.setProtectedFieldValue("auditLogger", value, this.auditLogger); + } + return value; + } } diff --git a/acl/src/main/java/org/springframework/security/acls/sid/Sid.java b/acl/src/main/java/org/springframework/security/acls/sid/Sid.java index 725551a182..ecb06167eb 100644 --- a/acl/src/main/java/org/springframework/security/acls/sid/Sid.java +++ b/acl/src/main/java/org/springframework/security/acls/sid/Sid.java @@ -14,6 +14,8 @@ */ package org.springframework.security.acls.sid; +import java.io.Serializable; + /** * A security identity recognised by the ACL system. * @@ -29,7 +31,7 @@ package org.springframework.security.acls.sid; * @author Ben Alex * @version $Id$ */ -public interface Sid { +public interface Sid extends Serializable { //~ Methods ======================================================================================================== /** diff --git a/acl/src/test/java/org/springframework/security/acls/domain/AccessControlEntryTests.java b/acl/src/test/java/org/springframework/security/acls/domain/AccessControlEntryTests.java index 7c0700c704..8dd8ad3797 100644 --- a/acl/src/test/java/org/springframework/security/acls/domain/AccessControlEntryTests.java +++ b/acl/src/test/java/org/springframework/security/acls/domain/AccessControlEntryTests.java @@ -86,8 +86,6 @@ public class AccessControlEntryTests extends TestCase { BasePermission.ADMINISTRATION, true, true, true))); Assert.assertFalse(ace.equals(new AccessControlEntryImpl(new Long(2), mockAcl, sid, BasePermission.ADMINISTRATION, true, true, true))); - Assert.assertFalse(ace.equals(new AccessControlEntryImpl(new Long(1), new MockAcl(), sid, - BasePermission.ADMINISTRATION, true, true, true))); Assert.assertFalse(ace.equals(new AccessControlEntryImpl(new Long(1), mockAcl, new PrincipalSid("scott"), BasePermission.ADMINISTRATION, true, true, true))); Assert.assertFalse(ace.equals(new AccessControlEntryImpl(new Long(1), mockAcl, sid, BasePermission.WRITE, true, diff --git a/acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java b/acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java index 3740239423..dbc25ccf60 100644 --- a/acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java +++ b/acl/src/test/java/org/springframework/security/acls/domain/AclImplementationSecurityCheckTests.java @@ -216,7 +216,7 @@ public class AclImplementationSecurityCheckTests extends TestCase { // access MutableAcl parentAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); parentAcl.insertAce(null, BasePermission.ADMINISTRATION, new PrincipalSid(auth), true); - MutableAcl childAcl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); + MutableAcl childAcl = new AclImpl(identity, new Long(2), aclAuthorizationStrategy, new ConsoleAuditLogger()); // Check against the 'child' acl, which doesn't offer any authorization // rights on CHANGE_OWNERSHIP diff --git a/acl/src/test/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCacheTests.java b/acl/src/test/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCacheTests.java index e518c01ce4..4d227b2774 100644 --- a/acl/src/test/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCacheTests.java +++ b/acl/src/test/java/org/springframework/security/acls/jdbc/EhCacheBasedAclCacheTests.java @@ -1,11 +1,25 @@ package org.springframework.security.acls.jdbc; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import java.io.File; +import java.io.FileInputStream; +import java.io.FileOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.io.Serializable; import net.sf.ehcache.Cache; -import net.sf.ehcache.Ehcache; import net.sf.ehcache.CacheManager; +import net.sf.ehcache.Ehcache; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; import org.springframework.security.Authentication; import org.springframework.security.GrantedAuthority; import org.springframework.security.GrantedAuthorityImpl; @@ -18,12 +32,7 @@ import org.springframework.security.acls.objectidentity.ObjectIdentity; import org.springframework.security.acls.objectidentity.ObjectIdentityImpl; import org.springframework.security.context.SecurityContextHolder; import org.springframework.security.providers.TestingAuthenticationToken; - -import org.junit.BeforeClass; -import org.junit.AfterClass; -import org.junit.After; -import org.junit.Test; -import static org.junit.Assert.*; +import org.springframework.security.util.FieldUtils; /** * Tests {@link EhCacheBasedAclCache} @@ -38,7 +47,8 @@ public class EhCacheBasedAclCacheTests { @BeforeClass public static void initCacheManaer() { cacheManager = new CacheManager(); - cacheManager.addCache(new Cache("ehcachebasedacltests", 500, false, false, 30, 30)); + // Use disk caching immediately (to test for serialization issue reported in SEC-527) + cacheManager.addCache(new Cache("ehcachebasedacltests", 0, true, false, 30, 30)); } @AfterClass @@ -115,6 +125,36 @@ public class EhCacheBasedAclCacheTests { assertTrue(true); } } + + // SEC-527 + @Test + public void testDiskSerializationOfMutableAclObjectInstance() throws Exception { + ObjectIdentity identity = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100)); + AclAuthorizationStrategy aclAuthorizationStrategy = new AclAuthorizationStrategyImpl(new GrantedAuthority[] { + new GrantedAuthorityImpl("ROLE_OWNERSHIP"), new GrantedAuthorityImpl("ROLE_AUDITING"), + new GrantedAuthorityImpl("ROLE_GENERAL") }); + MutableAcl acl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); + + // Serialization test + File file = File.createTempFile("SEC_TEST", ".object"); + FileOutputStream fos = new FileOutputStream(file); + ObjectOutputStream oos = new ObjectOutputStream(fos); + oos.writeObject(acl); + oos.close(); + + FileInputStream fis = new FileInputStream(file); + ObjectInputStream ois = new ObjectInputStream(fis); + MutableAcl retrieved = (MutableAcl) ois.readObject(); + ois.close(); + + assertEquals(acl, retrieved); + + Object retrieved1 = FieldUtils.getProtectedFieldValue("aclAuthorizationStrategy", retrieved); + assertEquals(null, retrieved1); + + Object retrieved2 = FieldUtils.getProtectedFieldValue("auditLogger", retrieved); + assertEquals(null, retrieved2); + } @Test public void cacheOperationsAclWithoutParent() throws Exception { @@ -127,9 +167,13 @@ public class EhCacheBasedAclCacheTests { new GrantedAuthorityImpl("ROLE_GENERAL") }); MutableAcl acl = new AclImpl(identity, new Long(1), aclAuthorizationStrategy, new ConsoleAuditLogger()); + assertEquals(0, cache.getDiskStoreSize()); myCache.putInCache(acl); assertEquals(cache.getSize(), 2); - + assertEquals(2, cache.getDiskStoreSize()); + assertTrue(cache.isElementOnDisk(acl.getObjectIdentity())); + assertFalse(cache.isElementInMemory(acl.getObjectIdentity())); + // Check we can get from cache the same objects we put in assertEquals(myCache.getFromCache(new Long(1)), acl); assertEquals(myCache.getFromCache(identity), acl); @@ -140,14 +184,17 @@ public class EhCacheBasedAclCacheTests { myCache.putInCache(acl2); assertEquals(cache.getSize(), 4); + assertEquals(4, cache.getDiskStoreSize()); // Try to evict an entry that doesn't exist myCache.evictFromCache(new Long(3)); myCache.evictFromCache(new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(102))); assertEquals(cache.getSize(), 4); + assertEquals(4, cache.getDiskStoreSize()); myCache.evictFromCache(new Long(1)); assertEquals(cache.getSize(), 2); + assertEquals(2, cache.getDiskStoreSize()); // Check the second object inserted assertEquals(myCache.getFromCache(new Long(2)), acl2); @@ -177,8 +224,12 @@ public class EhCacheBasedAclCacheTests { acl.setParent(parentAcl); + assertEquals(0, cache.getDiskStoreSize()); myCache.putInCache(acl); assertEquals(cache.getSize(), 4); + assertEquals(4, cache.getDiskStoreSize()); + assertTrue(cache.isElementOnDisk(acl.getObjectIdentity())); + assertFalse(cache.isElementInMemory(acl.getObjectIdentity())); // Check we can get from cache the same objects we put in assertEquals(myCache.getFromCache(new Long(1)), acl);