Index: src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java (revision 1631731) +++ src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java (revision ) @@ -17,7 +17,6 @@ package org.apache.jackrabbit.oak.jcr.security.authorization; import java.security.Principal; -import java.util.Collections; import java.util.HashSet; import java.util.Set; import javax.jcr.Node; @@ -26,11 +25,9 @@ import javax.jcr.RepositoryException; import javax.jcr.Session; import javax.jcr.security.AccessControlEntry; -import javax.jcr.security.AccessControlList; import javax.jcr.security.Privilege; import javax.jcr.util.TraversingItemVisitor; -import com.google.common.collect.Sets; import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager; import org.apache.jackrabbit.api.security.user.Group; import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; @@ -144,37 +141,12 @@ @Test public void testDenyRoot() throws Exception { - Set acesBefore = getACEs("/"); try { deny("/", readPrivileges); testSession.getRootNode(); fail("root should not be accessible"); } catch (Exception e) { // expected exception - } finally { - restoreAces("/", acesBefore); - } - } - - private Set getACEs(String path) throws RepositoryException { - AccessControlList acl = AccessControlUtils.getAccessControlList(superuser, path); - Set acesBefore = Sets.newHashSet(); - if (acl != null) { - Collections.addAll(acesBefore, acl.getAccessControlEntries()); - } - return acesBefore; - } - - private void restoreAces(String path, Set acesToKeep) throws RepositoryException { - AccessControlList acl = AccessControlUtils.getAccessControlList(superuser, path); - if (acl != null) { - for (AccessControlEntry ace : acl.getAccessControlEntries()) { - if (!acesToKeep.contains(ace)) { - acl.removeAccessControlEntry(ace); - } - } - acMgr.setPolicy("/", acl); - superuser.save(); } } Index: src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/UserManagementTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/UserManagementTest.java (revision 1631731) +++ src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/UserManagementTest.java (revision ) @@ -24,7 +24,6 @@ import javax.jcr.Node; import javax.jcr.NodeIterator; import javax.jcr.query.Query; -import javax.jcr.security.AccessControlEntry; import javax.jcr.security.Privilege; import com.google.common.collect.Lists; @@ -35,6 +34,7 @@ import org.apache.jackrabbit.api.security.user.UserManager; import org.apache.jackrabbit.commons.JcrUtils; import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl; import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants; import org.apache.jackrabbit.oak.spi.security.user.UserConstants; @@ -57,6 +57,16 @@ private List authorizablesToRemove = Lists.newArrayList(userId, groupId); @Override + protected void setUp() throws Exception { + super.setUp(); + + // setup default permissions + String authPath = "/rep:security/rep:authorizables"; + AccessControlUtils.addAccessControlEntry(superuser, authPath, EveryonePrincipal.getInstance(), privilegesFromName(Privilege.JCR_READ), true); + superuser.save(); + } + + @Override @Before public void tearDown() throws Exception { try { @@ -68,20 +78,6 @@ Authorizable a = userMgr.getAuthorizable(id); if (a != null) { a.remove(); - } - } - - JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, "/"); - if (acl != null) { - boolean modified = false; - for (AccessControlEntry entry : acl.getAccessControlEntries()) { - if (testUser.getPrincipal().equals(entry.getPrincipal())) { - acl.removeAccessControlEntry(entry); - modified = true; - } - } - if (modified) { - acMgr.setPolicy("/", acl); } } \ No newline at end of file Index: src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java (revision 1631731) +++ src/test/java/org/apache/jackrabbit/oak/jcr/TestContentLoader.java (revision ) @@ -69,6 +69,7 @@ } AccessControlUtils.addAccessControlEntry(session, "/", EveryonePrincipal.getInstance(), new String[]{Privilege.JCR_READ}, true); + AccessControlUtils.addAccessControlEntry(session, "/jcr:system", EveryonePrincipal.getInstance(), new String[]{Privilege.JCR_READ}, false); session.save(); } Index: src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/AbstractEvaluationTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/AbstractEvaluationTest.java (revision 1631731) +++ src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/AbstractEvaluationTest.java (revision ) @@ -18,7 +18,7 @@ import java.security.Principal; import java.util.Collections; -import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; @@ -32,8 +32,9 @@ import javax.jcr.Session; import javax.jcr.SimpleCredentials; import javax.jcr.Value; +import javax.jcr.security.AccessControlEntry; +import javax.jcr.security.AccessControlList; import javax.jcr.security.AccessControlManager; -import javax.jcr.security.AccessControlPolicy; import javax.jcr.security.Privilege; import org.apache.jackrabbit.api.JackrabbitSession; @@ -42,12 +43,14 @@ import org.apache.jackrabbit.api.security.user.User; import org.apache.jackrabbit.api.security.user.UserManager; import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; -import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.jackrabbit.test.NotExecutableException; import org.apache.jackrabbit.test.api.security.AbstractAccessControlTest; import org.junit.After; import org.junit.Before; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; + import static org.junit.Assert.assertArrayEquals; /** @@ -78,7 +81,7 @@ protected Session testSession; protected AccessControlManager testAcMgr; - private Set toClear = new HashSet(); + private List toRestore = Lists.newArrayList(); @Override @Before @@ -116,19 +119,17 @@ childchildPPath = ccp1.getPath(); siblingPath = n2.getPath(); - // setup default permissions - AccessControlUtils.addAccessControlEntry(superuser, "/", EveryonePrincipal.getInstance(), privilegesFromName(Privilege.JCR_READ), true); superuser.save(); testSession = createTestSession(); testAcMgr = getAccessControlManager(testSession); - /* - precondition: - testuser must have READ-only permission on test-node and below - */ + // preconditions: + // testuser must have READ-only permission on test-node and below assertReadOnly(path); assertReadOnly(childNPath); + // but must not see version storage node + assertFalse(testSession.itemExists("/jcr:system/jcr:versionStorage")); } @Override @@ -139,14 +140,13 @@ testSession.logout(); } superuser.refresh(false); - for (String path : toClear) { - if (path != null && superuser.nodeExists(path)) { - AccessControlPolicy[] policies = acMgr.getPolicies(path); - for (AccessControlPolicy policy : policies) { - acMgr.removePolicy(path, policy); + // restore in reverse order + for (ACL acl : Lists.reverse(toRestore)) { + if (acl.path == null || superuser.nodeExists(acl.path)) { + restoreAces(acl); - } - } + } + } - } + toRestore.clear(); if (testGroup != null) { testGroup.remove(); } @@ -225,6 +225,9 @@ } protected JackrabbitAccessControlList modify(String path, Principal principal, Privilege[] privileges, boolean isAllow, Map restrictions) throws Exception { + // remember for restore during tearDown + toRestore.add(getACL(path)); + JackrabbitAccessControlList tmpl = AccessControlUtils.getAccessControlList(acMgr, path); tmpl.addEntry(principal, privileges, isAllow, restrictions); @@ -232,8 +235,6 @@ superuser.save(); testSession.refresh(false); - // remember for clean up during tearDown - toClear.add(tmpl.getPath()); return tmpl; } @@ -269,5 +270,37 @@ protected JackrabbitAccessControlList deny(String nPath, Principal principal, Privilege[] privileges) throws Exception { return modify(nPath, principal, privileges, false, EMPTY_RESTRICTIONS); + } + + private ACL getACL(String path) throws RepositoryException { + return new ACL(path, AccessControlUtils.getAccessControlList(superuser, path)); + } + + private void restoreAces(ACL restore) throws RepositoryException { + AccessControlList acl = AccessControlUtils.getAccessControlList(superuser, path); + if (acl != null) { + for (AccessControlEntry ace : acl.getAccessControlEntries()) { + acl.removeAccessControlEntry(ace); + } + for (AccessControlEntry ace : restore.entries) { + acl.addAccessControlEntry(ace.getPrincipal(), ace.getPrivileges()); + } + acMgr.setPolicy(path, acl); + superuser.save(); + } + } + + private static final class ACL { + + private final String path; + private final Set entries = Sets.newHashSet(); + + ACL(String path, AccessControlList list) + throws RepositoryException { + this.path = path; + if (list != null) { + Collections.addAll(entries, list.getAccessControlEntries()); + } + } } } \ No newline at end of file Index: src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/VersionManagementTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/VersionManagementTest.java (revision 1631731) +++ src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/VersionManagementTest.java (revision ) @@ -48,7 +48,9 @@ super.setUp(); versionPrivileges = privilegesFromName(Privilege.JCR_VERSION_MANAGEMENT); - assertFalse(testAcMgr.hasPrivileges(VERSIONSTORE, versionPrivileges)); + // must not see version storage or must not have version privilege + assertTrue(!testSession.nodeExists(VERSIONSTORE) + || !testAcMgr.hasPrivileges(VERSIONSTORE, versionPrivileges)); } private Node createVersionableNode(Node parent) throws Exception {