You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2013/04/19 12:30:39 UTC
svn commit: r1469772 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/
oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/
oak-jcr/ oak-jcr/src/test/java/org/...
Author: angela
Date: Fri Apr 19 10:30:39 2013
New Revision: 1469772
URL: http://svn.apache.org/r1469772
Log:
OAK-526 : Implement AC-Postprocessing in PermissionHook (add handling for simple reordering of aces)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java
jackrabbit/oak/trunk/oak-jcr/pom.xml
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/InheritanceTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java?rev=1469772&r1=1469771&r2=1469772&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ChildOrderDiff.java Fri Apr 19 10:30:39 2013
@@ -16,14 +16,14 @@
*/
package org.apache.jackrabbit.oak.security.authorization.permission;
+import java.util.ArrayList;
import java.util.List;
import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
import com.google.common.collect.Lists;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Type;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Helper class to handle modifications to the hidden
@@ -31,11 +31,6 @@ import org.slf4j.LoggerFactory;
*/
class ChildOrderDiff {
- /**
- * logger instance
- */
- private static final Logger log = LoggerFactory.getLogger(ChildOrderDiff.class);
-
private final PropertyState before;
private final PropertyState after;
@@ -68,4 +63,27 @@ class ChildOrderDiff {
}
return null;
}
+
+ /**
+ * Returns the names of all reordered nodes present in the 'after' property.
+ *
+ * @return
+ */
+ @Nonnull
+ List<String> getReordered() {
+ List<String> beforeNames = Lists.newArrayList(before.getValue(Type.NAMES));
+ List<String> afterNames = Lists.newArrayList(after.getValue(Type.NAMES));
+ // remove elements from before that have been deleted
+ beforeNames.retainAll(afterNames);
+
+ List<String> reordered = new ArrayList<String>();
+ for (int i = 0; i < afterNames.size(); i++) {
+ String aName = afterNames.get(i);
+ int index = beforeNames.indexOf(aName);
+ if (index != -1 && index != i) {
+ reordered.add(aName);
+ }
+ }
+ return reordered;
+ }
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java?rev=1469772&r1=1469771&r2=1469772&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java Fri Apr 19 10:30:39 2013
@@ -16,6 +16,8 @@
*/
package org.apache.jackrabbit.oak.security.authorization.permission;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Set;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
@@ -46,6 +48,8 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeStateUtils;
import org.apache.jackrabbit.oak.util.TreeUtil;
import org.apache.jackrabbit.util.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import static com.google.common.base.Preconditions.checkNotNull;
import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
@@ -59,6 +63,8 @@ import static org.apache.jackrabbit.oak.
*/
public class PermissionHook implements PostValidationHook, AccessControlConstants, PermissionConstants {
+ private static final Logger log = LoggerFactory.getLogger(PermissionHook.class);
+
private final RestrictionProvider restrictionProvider;
private final String workspaceName;
@@ -135,6 +141,8 @@ public class PermissionHook implements P
private final Node parentBefore;
private final AfterNode parentAfter;
+ private final List<String> processed = new ArrayList<String>();
+
private Diff(@Nonnull Node parentBefore, @Nonnull AfterNode parentAfter) {
this.parentBefore = parentBefore;
this.parentAfter = parentAfter;
@@ -148,7 +156,15 @@ public class PermissionHook implements P
@Override
public void propertyChanged(PropertyState before, PropertyState after) {
if (isACL(parentAfter) && TreeImpl.OAK_CHILD_ORDER.equals(before.getName())) {
- // TODO: update if order has changed without child-node modifications
+ List<String> reordered = new ChildOrderDiff(before, after).getReordered();
+ for (String name : reordered) {
+ NodeState beforeNode = parentBefore.getNodeState().getChildNode(name);
+ NodeState afterNode = parentAfter.getNodeState().getChildNode(name);
+ updateEntry(name, beforeNode, afterNode);
+
+ log.debug("Processed reordered child node " + name);
+ processed.add(name);
+ }
}
}
@@ -219,6 +235,10 @@ public class PermissionHook implements P
}
private void updateEntry(String name, NodeState before, NodeState after) {
+ if (processed.contains(name)) {
+ log.debug("ACE entry already processed -> skip updateEntry.");
+ return;
+ }
removeEntry(name, before);
addEntry(name, after);
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java?rev=1469772&r1=1469771&r2=1469772&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHookTest.java Fri Apr 19 10:30:39 2013
@@ -16,19 +16,24 @@
*/
package org.apache.jackrabbit.oak.security.authorization.permission;
+import java.security.Principal;
+import java.util.ArrayList;
import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
import java.util.Set;
import javax.jcr.RepositoryException;
+import javax.jcr.security.AccessControlEntry;
import javax.jcr.security.AccessControlManager;
import com.google.common.collect.ImmutableSet;
import org.apache.jackrabbit.JcrConstants;
import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
+import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.security.authorization.AccessControlConstants;
-import org.apache.jackrabbit.oak.security.principal.PrincipalImpl;
import org.apache.jackrabbit.oak.security.privilege.PrivilegeBits;
import org.apache.jackrabbit.oak.security.privilege.PrivilegeBitsProvider;
import org.apache.jackrabbit.oak.security.privilege.PrivilegeConstants;
@@ -37,47 +42,57 @@ import org.apache.jackrabbit.oak.spi.sec
import org.apache.jackrabbit.oak.util.NodeUtil;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
/**
- * PermissionHookTest... TODO
+ * Testing the {@code PermissionHook}
*/
public class PermissionHookTest extends AbstractAccessControlTest implements AccessControlConstants, PermissionConstants {
private String testPath = "/testPath";
- private String testPrincipalName = "admin"; // TODO
-
+ private String testPrincipalName;
private PrivilegeBitsProvider bitsProvider;
+ private List<Principal> principals = new ArrayList<Principal>();
@Override
@Before
public void before() throws Exception {
super.before();
+ Principal testPrincipal = getTestPrincipal();
NodeUtil rootNode = new NodeUtil(root.getTree("/"), namePathMapper);
rootNode.addChild("testPath", JcrConstants.NT_UNSTRUCTURED);
AccessControlManager acMgr = getAccessControlManager(root);
JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, testPath);
- acl.addAccessControlEntry(new PrincipalImpl(testPrincipalName), privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES));
+ acl.addAccessControlEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES));
acl.addAccessControlEntry(EveryonePrincipal.getInstance(), privilegesFromNames(PrivilegeConstants.JCR_READ));
acMgr.setPolicy(testPath, acl);
root.commit();
+ testPrincipalName = testPrincipal.getName();
bitsProvider = new PrivilegeBitsProvider(root);
}
@After
public void after() throws Exception {
- root.refresh();
- Tree test = root.getTree(testPath);
- if (test != null) {
- test.remove();
+ try {
+ root.refresh();
+ Tree test = root.getTree(testPath);
+ if (test != null) {
+ test.remove();
+ }
+
+ for (Principal principal : principals) {
+ getUserManager().getAuthorizable(principal).remove();
+ }
root.commit();
+ } finally {
+ super.after();
}
}
@@ -95,6 +110,16 @@ public class PermissionHookTest extends
throw new RepositoryException("no such entry");
}
+ private void createPrincipals() throws Exception {
+ if (principals.isEmpty()) {
+ for (int i = 0; i < 10; i++) {
+ Group gr = getUserManager().createGroup("testGroup"+i);
+ principals.add(gr.getPrincipal());
+ }
+ root.commit();
+ }
+ }
+
@Test
public void testDuplicateAce() throws Exception {
// add duplicate policy on OAK-API
@@ -157,7 +182,6 @@ public class PermissionHookTest extends
}
- @Ignore("OAK-526 : PermissionHook#propertyChange") // TODO
@Test
public void testReorderAce() throws Exception {
Tree entry = getEntry(testPrincipalName, testPath);
@@ -172,20 +196,126 @@ public class PermissionHookTest extends
assertEquals(1, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
}
- @Ignore("OAK-526 : PermissionHook#propertyChange") // TODO
@Test
public void testReorderAndAddAce() throws Exception {
Tree entry = getEntry(testPrincipalName, testPath);
assertEquals(0, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
Tree aclTree = root.getTree(testPath + "/rep:policy");
+ // reorder
+ aclTree.getChildren().iterator().next().orderBefore(null);
+
+ // add a new entry
+ NodeUtil ace = new NodeUtil(aclTree).addChild("denyEveryoneLockMgt", NT_REP_DENY_ACE);
+ ace.setString(REP_PRINCIPAL_NAME, EveryonePrincipal.NAME);
+ ace.setStrings(REP_PRIVILEGES, PrivilegeConstants.JCR_LOCK_MANAGEMENT);
+ root.commit();
+
+ entry = getEntry(testPrincipalName, testPath);
+ assertEquals(1, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
+ }
+
+ @Test
+ public void testReorderAddAndRemoveAces() throws Exception {
+ Tree entry = getEntry(testPrincipalName, testPath);
+ assertEquals(0, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
+
+ Tree aclTree = root.getTree(testPath + "/rep:policy");
+
+ // reorder testPrincipal entry to the end
aclTree.getChildren().iterator().next().orderBefore(null);
+
+ Iterator<Tree> aceIt = aclTree.getChildren().iterator();
+ // remove the everyone entry
+ aceIt.next().remove();
+ // remember the name of the testPrincipal entry.
+ String name = aceIt.next().getName();
+
+ // add a new entry
NodeUtil ace = new NodeUtil(aclTree).addChild("denyEveryoneLockMgt", NT_REP_DENY_ACE);
ace.setString(REP_PRINCIPAL_NAME, EveryonePrincipal.NAME);
ace.setStrings(REP_PRIVILEGES, PrivilegeConstants.JCR_LOCK_MANAGEMENT);
+
+ // reorder the new entry before the remaining existing entry
+ ace.getTree().orderBefore(name);
+
root.commit();
entry = getEntry(testPrincipalName, testPath);
assertEquals(1, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
}
+
+ /**
+ * ACE : 0 1 2 3 4 5 6 7
+ * Before : tp ev p0 p1 p2 p3
+ * After : ev p2 p1 p3 p4 p5
+ */
+ @Test
+ public void testReorderAddAndRemoveAces2() throws Exception {
+ createPrincipals();
+
+ AccessControlManager acMgr = getAccessControlManager(root);
+ JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, testPath);
+ for (int i = 0; i < 4; i++) {
+ acl.addAccessControlEntry(principals.get(i), privilegesFromNames(PrivilegeConstants.JCR_READ));
+ }
+ acMgr.setPolicy(testPath, acl);
+ root.commit();
+
+ AccessControlEntry[] aces = acl.getAccessControlEntries();
+ acl.removeAccessControlEntry(aces[0]);
+ acl.removeAccessControlEntry(aces[2]);
+ acl.orderBefore(aces[4], aces[3]);
+ acl.addAccessControlEntry(principals.get(4), privilegesFromNames(PrivilegeConstants.JCR_READ));
+ acl.addAccessControlEntry(principals.get(5), privilegesFromNames(PrivilegeConstants.JCR_READ));
+ acMgr.setPolicy(testPath, acl);
+ root.commit();
+
+ Tree entry = getEntry(principals.get(2).getName(), testPath);
+ assertEquals(1, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
+
+ entry = getEntry(principals.get(1).getName(), testPath);
+ assertEquals(2, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
+ }
+
+ /**
+ * ACE : 0 1 2 3 4 5 6 7
+ * Before : tp ev p0 p1 p2 p3
+ * After : p1 ev p3 p2
+ */
+ @Test
+ public void testReorderAndRemoveAces() throws Exception {
+ createPrincipals();
+
+ AccessControlManager acMgr = getAccessControlManager(root);
+ JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, testPath);
+ for (int i = 0; i < 4; i++) {
+ acl.addAccessControlEntry(principals.get(i), privilegesFromNames(PrivilegeConstants.JCR_READ));
+ }
+ acMgr.setPolicy(testPath, acl);
+ root.commit();
+
+ AccessControlEntry[] aces = acl.getAccessControlEntries();
+ acl.removeAccessControlEntry(aces[0]);
+ acl.removeAccessControlEntry(aces[2]);
+ acl.orderBefore(aces[4], null);
+ acl.orderBefore(aces[3], aces[1]);
+ acMgr.setPolicy(testPath, acl);
+ root.commit();
+
+ Tree entry = getEntry(EveryonePrincipal.NAME, testPath);
+ assertEquals(1, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
+
+ entry = getEntry(principals.get(2).getName(), testPath);
+ assertEquals(3, entry.getProperty(REP_INDEX).getValue(Type.LONG).longValue());
+
+ for (String pName : new String[] {testPrincipalName, principals.get(0).getName()}) {
+ try {
+ getEntry(pName, testPath);
+ fail();
+ } catch (RepositoryException e) {
+ // success
+ }
+ }
+ }
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-jcr/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/pom.xml?rev=1469772&r1=1469771&r2=1469772&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-jcr/pom.xml Fri Apr 19 10:30:39 2013
@@ -256,8 +256,6 @@
org.apache.jackrabbit.oak.jcr.security.authorization.ReadTest#testReadDenied <!-- OAK-766 -->
org.apache.jackrabbit.oak.jcr.security.authorization.WriteTest#testWriteIfReadingParentIsDenied <!-- OAK-766 -->
org.apache.jackrabbit.oak.jcr.security.authorization.WriteTest#testRemoveNodeWithInvisibleNonRemovableChild <!-- OAK-51 -->
- org.apache.jackrabbit.oak.jcr.security.authorization.InheritanceTest#testReorderGroupPermissions <!-- OAK-526 -->
-
</known.issues>
</properties>
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/InheritanceTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/InheritanceTest.java?rev=1469772&r1=1469771&r2=1469772&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/InheritanceTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/InheritanceTest.java Fri Apr 19 10:30:39 2013
@@ -214,7 +214,6 @@ public class InheritanceTest extends Abs
assertTrue(testAcMgr.hasPrivileges(path, modPropPrivileges));
}
- @Ignore("OAK-526 : missing handling for reorder in PermissionHook")
@Test
public void testReorderGroupPermissions() throws Exception {
/* add privileges for the Group the test-user is member of */