You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by an...@apache.org on 2011/10/06 10:50:25 UTC

svn commit: r1179536 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/security/authorization/ main/java/org/apache/jackrabbit/core/security/authorization/acl/ test/java/org/apache/jackrabbit/core/security/authorization/ ...

Author: angela
Date: Thu Oct  6 08:50:25 2011
New Revision: 1179536

URL: http://svn.apache.org/viewvc?rev=1179536&view=rev
Log:
JCR-3095 : Move operation may turn AC caches stale

Added:
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/MoveTest.java   (with props)
Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/AccessControlObserver.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/CachingEntryCollector.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/AccessControlObserver.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/AccessControlObserver.java?rev=1179536&r1=1179535&r2=1179536&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/AccessControlObserver.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/AccessControlObserver.java Thu Oct  6 08:50:25 2011
@@ -25,10 +25,11 @@ import java.util.Set;
  * <code>AccessControlObserver</code>...
  */
 public abstract class AccessControlObserver implements SynchronousEventListener {
-    
+
     public static final int POLICY_ADDED = 1;
     public static final int POLICY_REMOVED = 2;
     public static final int POLICY_MODIFIED = 4;
+    public static final int MOVE = 8;
 
     private final Set<AccessControlListener> listeners = new HashSet<AccessControlListener>();
     private final Object listenerMonitor = new Object();

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/CachingEntryCollector.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/CachingEntryCollector.java?rev=1179536&r1=1179535&r2=1179536&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/CachingEntryCollector.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/CachingEntryCollector.java Thu Oct  6 08:50:25 2011
@@ -222,6 +222,10 @@ class CachingEntryCollector extends Entr
                 } else if ((type & POLICY_MODIFIED) == POLICY_MODIFIED) {
                     // simply clear the cache entry -> reload upon next access.
                     cache.remove(nodeId);
+                } else if ((type & MOVE) == MOVE) {
+                    // some sort of move operation that may affect the cache
+                    cache.clear();
+                    break; // no need for further processing.
                 }
             }
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java?rev=1179536&r1=1179535&r2=1179536&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/EntryCollector.java Thu Oct  6 08:50:25 2011
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.core.secur
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.SessionImpl;
 import org.apache.jackrabbit.core.id.NodeId;
+import org.apache.jackrabbit.core.observation.SynchronousEventListener;
 import org.apache.jackrabbit.core.security.authorization.AccessControlConstants;
 import org.apache.jackrabbit.core.security.authorization.AccessControlModifications;
 import org.apache.jackrabbit.core.security.authorization.AccessControlObserver;
@@ -33,6 +34,7 @@ import javax.jcr.RepositoryException;
 import javax.jcr.Session;
 import javax.jcr.observation.Event;
 import javax.jcr.observation.EventIterator;
+import javax.jcr.observation.EventListener;
 import javax.jcr.observation.ObservationManager;
 import javax.jcr.security.AccessControlEntry;
 import java.util.ArrayList;
@@ -65,6 +67,8 @@ public class EntryCollector extends Acce
      */
     protected final NodeId rootID;
 
+    private final EventListener moveListener;
+
     /**
      *
      * @param systemSession
@@ -91,7 +95,14 @@ public class EntryCollector extends Acce
                 systemSession.getJCRName(NT_REP_ACL),
                 systemSession.getJCRName(NT_REP_ACE)
         };
-        observationMgr.addEventListener(this, events, systemSession.getRootNode().getPath(), true, null, ntNames, true);        
+        String rootPath = systemSession.getRootNode().getPath();
+        observationMgr.addEventListener(this, events, rootPath, true, null, ntNames, true);
+        /*
+         In addition both the collector and all subscribed listeners should be
+         informed about any kind of move events.
+         */
+        moveListener = new MoveListener();
+        observationMgr.addEventListener(moveListener, Event.NODE_MOVED, rootPath, true, null, null, true);
     }
 
     /**
@@ -102,7 +113,9 @@ public class EntryCollector extends Acce
     protected void close() {
         super.close();
         try {
-            systemSession.getWorkspace().getObservationManager().removeEventListener(this);
+            ObservationManager observationMgr = systemSession.getWorkspace().getObservationManager();
+            observationMgr.removeEventListener(this);
+            observationMgr.removeEventListener(moveListener);
         } catch (RepositoryException e) {
             log.error("Unexpected error while closing CachingEntryCollector", e);
         }
@@ -391,4 +404,28 @@ public class EntryCollector extends Acce
             modMap.put(accessControllNodeId, modType);
         }
     }
+
+    /**
+     * Listening to any kind of move events in the hierarchy. Since ac content
+     * is associated with individual nodes the caches need to be informed about
+     * any kind of move as well even if the target node is not access control
+     * content s.str.
+     */
+    private class MoveListener implements SynchronousEventListener {
+
+        public void onEvent(EventIterator events) {
+            // NOTE: simplified event handling as all listeners just clear
+            // the cache in case of any move event. therefore there is currently
+            // no need to process all events and using the rootID as marker.
+            while (events.hasNext()) {
+                Event event = events.nextEvent();
+                if (event.getType() == Event.NODE_MOVED) {
+                    Map<NodeId, Integer> m = Collections.singletonMap(rootID, AccessControlObserver.MOVE);
+                    AccessControlModifications<NodeId> mods = new AccessControlModifications<NodeId>(m);
+                    notifyListeners(mods);
+                    break;
+                } //else: illegal event-type: should never occur. ignore
+            }
+        }
+    }
 }
\ No newline at end of file

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java?rev=1179536&r1=1179535&r2=1179536&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/AbstractWriteTest.java Thu Oct  6 08:50:25 2011
@@ -62,9 +62,6 @@ public abstract class AbstractWriteTest 
     protected String childchildPPath;
     protected String siblingPath;
 
-    // TODO: test AC for moved node
-    // TODO: test AC for moved AC-controlled node
-
     @Override
     protected void setUp() throws Exception {
         super.setUp();

Added: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/MoveTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/MoveTest.java?rev=1179536&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/MoveTest.java (added)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/MoveTest.java Thu Oct  6 08:50:25 2011
@@ -0,0 +1,262 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.security.authorization.acl;
+
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
+import org.apache.jackrabbit.core.security.authorization.AbstractEvaluationTest;
+import org.apache.jackrabbit.core.security.authorization.AccessControlConstants;
+import org.apache.jackrabbit.spi.commons.name.NameConstants;
+import org.apache.jackrabbit.test.NotExecutableException;
+
+import javax.jcr.AccessDeniedException;
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.Value;
+import javax.jcr.ValueFactory;
+import javax.jcr.security.AccessControlManager;
+import javax.jcr.security.Privilege;
+import java.security.Principal;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * <code>MoveTest</code>...
+ */
+public class MoveTest extends AbstractEvaluationTest {
+
+    private String path;
+    private String childNPath;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        // create some nodes below the test root in order to apply ac-stuff
+        Node node = testRootNode.addNode(nodeName1, testNodeType);
+        Node cn1 = node.addNode(nodeName2, testNodeType);
+        superuser.save();
+
+        path = node.getPath();
+        childNPath = cn1.getPath();
+    }
+
+    @Override
+    protected boolean isExecutable() {
+        return EvaluationUtil.isExecutable(acMgr);
+    }
+
+    @Override
+    protected JackrabbitAccessControlList getPolicy(AccessControlManager acM, String path, Principal principal) throws RepositoryException, AccessDeniedException, NotExecutableException {
+        return EvaluationUtil.getPolicy(acM, path, principal);
+    }
+
+    @Override
+    protected Map<String, Value> getRestrictions(Session s, String path) {
+        return Collections.emptyMap();
+    }
+
+    public void testMoveAccessControlledNode() throws Exception {
+        Session testSession = getTestSession();
+        AccessControlManager testAcMgr = getTestACManager();
+
+        /*
+        precondition:
+        testuser must have READ-only permission on test-node and below
+        */
+        checkReadOnly(path);
+        checkReadOnly(childNPath);
+
+        Node node3 = superuser.getNode(childNPath).addNode(nodeName3);
+        superuser.save();
+
+        String node3Path = node3.getPath();
+        Privilege[] privileges = privilegesFromName(NameConstants.JCR_READ.toString());
+
+        // permissions defined @ childNode
+        // -> revoke read permission
+        withdrawPrivileges(childNPath, privileges, getRestrictions(superuser, childNPath));
+
+        assertFalse(testSession.nodeExists(childNPath));
+        assertFalse(testAcMgr.hasPrivileges(childNPath, privileges));
+        assertFalse(testSession.nodeExists(node3Path));
+        assertFalse(testAcMgr.hasPrivileges(node3Path, privileges));
+
+        // move the ancestor node
+        String movedChildNPath = path + "/movedNode";
+        String movedNode3Path = movedChildNPath + "/" + nodeName3;
+
+        superuser.move(childNPath, movedChildNPath);
+        superuser.save();
+
+        // expected behavior:
+        // the AC-content present on childNode is still enforced both on
+        // the node itself and on the subtree.
+        assertFalse(testSession.nodeExists(movedChildNPath));
+        assertFalse(testAcMgr.hasPrivileges(movedChildNPath, privileges));
+        assertFalse(testSession.nodeExists(movedNode3Path));
+        assertFalse(testAcMgr.hasPrivileges(movedNode3Path, privileges));
+    }
+
+    public void testMoveAccessControlledNodeInSubtree() throws Exception {
+        Session testSession = getTestSession();
+        AccessControlManager testAcMgr = getTestACManager();
+
+        /*
+        precondition:
+        testuser must have READ-only permission on test-node and below
+        */
+        checkReadOnly(path);
+        checkReadOnly(childNPath);
+
+        Node node3 = superuser.getNode(childNPath).addNode(nodeName3);
+        superuser.save();
+
+        String node3Path = node3.getPath();
+        Privilege[] privileges = privilegesFromName(NameConstants.JCR_READ.toString());
+
+        // permissions defined @ node3Path
+        // -> revoke read permission
+        withdrawPrivileges(node3Path, privileges, getRestrictions(superuser, node3Path));
+
+        assertFalse(testSession.nodeExists(node3Path));
+        assertFalse(testAcMgr.hasPrivileges(node3Path, privileges));
+
+        // move the ancestor node
+        String movedChildNPath = path + "/movedNode";
+        String movedNode3Path = movedChildNPath + "/" + nodeName3;
+
+        superuser.move(childNPath, movedChildNPath);
+        superuser.save();
+
+        // expected behavior:
+        // the AC-content present on node3 is still enforced
+        assertFalse(testSession.nodeExists(movedNode3Path));
+        assertFalse(testAcMgr.hasPrivileges(movedNode3Path, privileges));
+    }
+
+    public void testMoveWithDifferentEffectiveAc() throws Exception {
+        Session testSession = getTestSession();
+        AccessControlManager testAcMgr = getTestACManager();
+        ValueFactory vf = superuser.getValueFactory();
+
+        /*
+        precondition:
+        testuser must have READ-only permission on test-node and below
+        */
+        checkReadOnly(path);
+        checkReadOnly(childNPath);
+
+        Node node3 = superuser.getNode(childNPath).addNode(nodeName3);
+        superuser.save();
+
+        String node3Path = node3.getPath();
+        Privilege[] privileges = privilegesFromName(NameConstants.JCR_READ.toString());
+
+        // @path read is denied, @childNode its allowed again
+        withdrawPrivileges(path, privileges, getRestrictions(superuser, path));
+        givePrivileges(childNPath, privileges, getRestrictions(superuser, childNPath));
+
+        assertTrue(testSession.nodeExists(node3Path));
+        assertTrue(testAcMgr.hasPrivileges(node3Path, privileges));
+
+        // move the ancestor node
+        String movedPath = path + "/movedNode";
+
+        superuser.move(node3Path, movedPath);
+        superuser.save();
+
+        // expected behavior:
+        // due to move node3 should not e visible any more
+        assertFalse(testSession.nodeExists(movedPath));
+        assertFalse(testAcMgr.hasPrivileges(movedPath, privileges));
+    }
+
+    public void testMoveNodeWithGlobRestriction() throws Exception {
+        Session testSession = getTestSession();
+        AccessControlManager testAcMgr = getTestACManager();
+        ValueFactory vf = superuser.getValueFactory();
+
+        /*
+        precondition:
+        testuser must have READ-only permission on test-node and below
+        */
+        checkReadOnly(path);
+        checkReadOnly(childNPath);
+
+        Node node3 = superuser.getNode(childNPath).addNode(nodeName3);
+        superuser.save();
+
+        String node3Path = node3.getPath();
+        Privilege[] privileges = privilegesFromName(NameConstants.JCR_READ.toString());
+
+        // permissions defined @ path
+        // restriction: remove read priv to nodeName3 node
+        Map<String, Value> restrictions = new HashMap<String, Value>(getRestrictions(superuser, childNPath));
+        restrictions.put(AccessControlConstants.P_GLOB.toString(), vf.createValue("/"+nodeName3));
+        withdrawPrivileges(childNPath, privileges, restrictions);
+
+        assertFalse(testSession.nodeExists(node3Path));
+        assertFalse(testAcMgr.hasPrivileges(node3Path, privileges));
+
+        String movedChildNPath = path + "/movedNode";
+        String movedNode3Path = movedChildNPath + "/" + node3.getName();
+
+        superuser.move(childNPath, movedChildNPath);
+        superuser.save();
+
+        assertFalse(testSession.nodeExists(movedNode3Path));
+        assertFalse(testAcMgr.hasPrivileges(movedNode3Path, privileges));
+    }
+
+    public void testMoveNodeWithGlobRestriction2() throws Exception {
+        Session testSession = getTestSession();
+        AccessControlManager testAcMgr = getTestACManager();
+        ValueFactory vf = superuser.getValueFactory();
+
+        /*
+        precondition:
+        testuser must have READ-only permission on test-node and below
+        */
+        checkReadOnly(path);
+        checkReadOnly(childNPath);
+
+        Node node3 = superuser.getNode(childNPath).addNode(nodeName3);
+        superuser.save();
+
+        Privilege[] privileges = privilegesFromName(NameConstants.JCR_READ.toString());
+
+        // permissions defined @ path
+        // restriction: remove read priv to nodeName3 node
+        Map<String, Value> restrictions = new HashMap<String, Value>(getRestrictions(superuser, childNPath));
+        restrictions.put(AccessControlConstants.P_GLOB.toString(), vf.createValue("/"+nodeName3));
+        withdrawPrivileges(childNPath, privileges, restrictions);
+
+        // don't fill the per-session read-cache by calling Session.nodeExists
+        assertFalse(testAcMgr.hasPrivileges(node3.getPath(), privileges));
+
+        String movedChildNPath = path + "/movedNode";
+        String movedNode3Path = movedChildNPath + "/" + node3.getName();
+
+        superuser.move(childNPath, movedChildNPath);
+        superuser.save();
+
+        assertFalse(testSession.nodeExists(movedNode3Path));
+        assertFalse(testAcMgr.hasPrivileges(movedNode3Path, privileges));
+    }
+}
\ No newline at end of file

Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/MoveTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/MoveTest.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision Rev URL

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java?rev=1179536&r1=1179535&r2=1179536&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/TestAll.java Thu Oct  6 08:50:25 2011
@@ -50,6 +50,7 @@ public class TestAll extends TestCase {
         suite.addTestSuite(EffectivePolicyTest.class);
         suite.addTestSuite(ACLEditorTest.class);
         suite.addTestSuite(RepositoryOperationTest.class);
+        suite.addTestSuite(MoveTest.class);
 
         return suite;
     }