jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject 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/...
Date Fri, 19 Apr 2013 10:30:39 GMT
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 */



Mime
View raw message