jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject svn commit: r1497394 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
Date Thu, 27 Jun 2013 15:24:48 GMT
Author: angela
Date: Thu Jun 27 15:24:48 2013
New Revision: 1497394

URL: http://svn.apache.org/r1497394
Log:
OAK-878 : IllegalArgumentException while adding/removing read permission to user/group

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionHook.java
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java

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=1497394&r1=1497393&r2=1497394&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
Thu Jun 27 15:24:48 2013
@@ -16,8 +16,6 @@
  */
 package org.apache.jackrabbit.oak.security.authorization.permission;
 
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Set;
 import javax.annotation.Nonnull;
 
@@ -40,6 +38,7 @@ import org.apache.jackrabbit.oak.spi.com
 import org.apache.jackrabbit.oak.spi.security.authorization.AccessControlConstants;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restriction;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
+import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
 import org.apache.jackrabbit.oak.spi.state.DefaultNodeStateDiff;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
@@ -103,38 +102,17 @@ 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;
         }
 
         @Override
-        public boolean propertyChanged(PropertyState before, PropertyState after) {
-            if (isACL(parentAfter) && TreeImpl.OAK_CHILD_ORDER.equals(before.getName()))
{
-                List<String> reordered = new ChildOrderDiff(before, after).getReordered();
-                for (String name : reordered) {
-                    NodeState beforeNode = parentBefore.getNodeState().getChildNode(name);
-                    removeEntry(name, beforeNode);
-                }
-                for (String name : reordered) {
-                    NodeState afterNode = parentAfter.getNodeState().getChildNode(name);
-                    addEntry(name, afterNode);
-
-                    log.debug("Processed reordered child node " + name);
-                    processed.add(name);
-                }
-            }
-            return true;
-        }
-
-        @Override
         public boolean childNodeAdded(String name, NodeState after) {
             if (NodeStateUtils.isHidden(name)) {
                 // ignore hidden nodes
-            } else if (isACE(name, after)) {
-                addEntry(name, after);
+            } else if (isACL(name, after)) {
+                addEntries(name, after);
             } else {
                 Node before = new BeforeNode(parentBefore.getPath(), name, EMPTY_NODE);
                 AfterNode node = new AfterNode(parentAfter, name);
@@ -147,8 +125,9 @@ public class PermissionHook implements P
         public boolean childNodeChanged(String name, final NodeState before, NodeState after)
{
             if (NodeStateUtils.isHidden(name)) {
                 // ignore hidden nodes
-            } else if (isACE(name, before) || isACE(name, after)) {
-                updateEntry(name, before, after);
+            } else if (isACL(name, before) || isACL(name, after)) {
+                removeEntries(name, before);
+                addEntries(name, after);
             } else {
                 BeforeNode nodeBefore = new BeforeNode(parentBefore.getPath(), name, before);
                 AfterNode nodeAfter = new AfterNode(parentAfter, name);
@@ -161,8 +140,8 @@ public class PermissionHook implements P
         public boolean childNodeDeleted(String name, NodeState before) {
             if (NodeStateUtils.isHidden(name)) {
                 // ignore hidden nodes
-            } else if (isACE(name, before)) {
-                removeEntry(name, before);
+            } else if (isACL(name, before)) {
+                removeEntries(name, before);
             } else {
                 BeforeNode nodeBefore = new BeforeNode(parentBefore.getPath(), name, before);
                 AfterNode after = new AfterNode(parentAfter.getPath(), name, EMPTY_NODE);
@@ -172,34 +151,34 @@ public class PermissionHook implements P
         }
 
         //--------------------------------------------------------< private >---
-        private boolean isACL(Node parent) {
-            return ntMgr.isNodeType(getTree(parent.getName(), parent.getNodeState()), NT_REP_POLICY);
+
+        private boolean isACL(String name, NodeState nodeState) {
+            return ntMgr.isNodeType(getTree(name, nodeState), NT_REP_ACL);
         }
 
         private boolean isACE(String name, NodeState nodeState) {
             return ntMgr.isNodeType(getTree(name, nodeState), NT_REP_ACE);
         }
 
-        private void addEntry(String name, NodeState ace) {
-            PermissionEntry entry = createPermissionEntry(name, ace, parentAfter);
-            entry.add();
-        }
-
-        private void removeEntry(String name, NodeState ace) {
-            PermissionEntry entry = createPermissionEntry(name, ace, parentBefore);
-            entry.remove();
-        }
-
-        private void updateEntry(String name, NodeState before, NodeState after) {
-            if (processed.contains(name)) {
-                log.debug("ACE entry already processed -> skip updateEntry.");
-                return;
-            }
-            PermissionEntry beforeEntry = createPermissionEntry(name, before, parentBefore);
-            PermissionEntry afterEntry = createPermissionEntry(name, after, parentAfter);
-            if (!beforeEntry.equals(afterEntry)) {
-                beforeEntry.remove();
-                afterEntry.add();
+        private void addEntries(String aclName, NodeState acl) {
+            for (ChildNodeEntry cne : acl.getChildNodeEntries()) {
+                NodeState child = cne.getNodeState();
+                if (isACE(cne.getName(), child)) {
+                    PermissionEntry entry = createPermissionEntry(cne.getName(),
+                            child, new AfterNode(parentAfter, aclName));
+                    entry.add();
+                }
+            }
+        }
+
+        private void removeEntries(String aclName, NodeState acl) {
+            for (ChildNodeEntry cne : acl.getChildNodeEntries()) {
+                NodeState child = cne.getNodeState();
+                if (isACE(cne.getName(), child)) {
+                    PermissionEntry entry = createPermissionEntry(cne.getName(),
+                            child, new BeforeNode(parentBefore, aclName, acl));
+                    entry.remove();
+                }
             }
         }
 
@@ -244,12 +223,16 @@ public class PermissionHook implements P
             this.nodeState = root;
         }
 
-
         BeforeNode(String parentPath, String name, NodeState nodeState) {
             super(parentPath, name);
             this.nodeState = nodeState;
         }
 
+        BeforeNode(Node parent, String name, NodeState nodeState) {
+            super(parent.getPath(), name);
+            this.nodeState = nodeState;
+        }
+
         @Override
         NodeState getNodeState() {
             return nodeState;

Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java?rev=1497394&r1=1497393&r2=1497394&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadTest.java
Thu Jun 27 15:24:48 2013
@@ -21,11 +21,15 @@ import java.util.HashSet;
 import java.util.Set;
 import javax.jcr.Node;
 import javax.jcr.PathNotFoundException;
+import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.security.AccessControlEntry;
 import javax.jcr.security.Privilege;
+import javax.jcr.util.TraversingItemVisitor;
 
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
 import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.oak.security.authorization.AccessControlUtils;
 import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
 import org.junit.Test;
 
@@ -411,4 +415,43 @@ public class ReadTest extends AbstractEv
         Node n2 = test.getNode("a");
         assertTrue(n.isSame(n2));
     }
+
+    /**
+     * @see <a href="https://issues.apache.org/jira/browse/OAK-878">OAK-878 :
+     * IllegalArgumentException while adding/removing permission to user/group</a>
+     */
+    @Test
+    public void testImplicitReorder() throws Exception{
+        allow(path, testUser.getPrincipal(), readPrivileges);
+        assertEntry(0, true);
+
+        allow(path, getTestGroup().getPrincipal(), readPrivileges);
+        assertEntry(0, true);
+
+        deny(path, testUser.getPrincipal(), readPrivileges);
+        assertEntry(1, false);
+
+        deny(path, getTestGroup().getPrincipal(), readPrivileges);
+        assertEntry(0, false);
+
+        allow(path, testUser.getPrincipal(), readPrivileges);
+    }
+
+    private void assertEntry(final int index, final boolean isAllow) throws RepositoryException
{
+        AccessControlEntry first = AccessControlUtils.getAccessControlList(superuser, path).getAccessControlEntries()[index];
+
+        assertEquals(testUser.getPrincipal(), first.getPrincipal());
+
+        Node n = superuser.getNode("/jcr:system/rep:permissionStore/default/" + testUser.getPrincipal().getName());
+        TraversingItemVisitor v = new TraversingItemVisitor.Default(true, -1) {
+            @Override
+            protected void entering(Node node, int level) throws RepositoryException {
+                if (node.isNodeType("rep:Permissions") && path.equals(node.getProperty("rep:accessControlledPath").getString()))
{
+                    assertEquals(index, node.getProperty("rep:index").getLong());
+                    assertEquals(isAllow, node.getProperty("rep:isAllow").getBoolean());
+                }
+            }
+        };
+        v.visit(n);
+    }
 }



Mime
View raw message