jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject svn commit: r1097363 - in /jackrabbit/trunk/jackrabbit-core/src: main/java/org/apache/jackrabbit/core/ItemManager.java main/java/org/apache/jackrabbit/core/NodeImpl.java test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
Date Thu, 28 Apr 2011 08:40:43 GMT
Author: angela
Date: Thu Apr 28 08:40:43 2011
New Revision: 1097363

URL: http://svn.apache.org/viewvc?rev=1097363&view=rev
Log:
JCR-2951 - Item.remove fails if a child-item is not visible to the editing session

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java?rev=1097363&r1=1097362&r2=1097363&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
Thu Apr 28 08:40:43 2011
@@ -304,15 +304,15 @@ public class ItemManager implements Item
      * @param path Path of the item to retrieve or <code>null</code>. In
      * the latter case the test for access permission is executed using the
      * itemId.
+     * @param permissionCheck
      * @return The item identified by the given <code>itemId</code>.
      * @throws ItemNotFoundException
      * @throws AccessDeniedException
      * @throws RepositoryException
      */
-    private ItemImpl getItem(ItemId itemId, Path path) throws ItemNotFoundException, AccessDeniedException,
RepositoryException {
+    private ItemImpl getItem(ItemId itemId, Path path, boolean permissionCheck) throws ItemNotFoundException,
AccessDeniedException, RepositoryException {
         sanityCheck();
 
-        boolean permissionCheck = true;
         ItemData data = getItemData(itemId, path, permissionCheck);
         return createItemInstance(data);
     }
@@ -540,7 +540,7 @@ public class ItemManager implements Item
             throw new PathNotFoundException(safeGetJCRPath(path));
         }
         try {
-            ItemImpl item = getItem(id, path);
+            ItemImpl item = getItem(id, path, true);
             // Test, if this item is a shareable node.
             if (item.isNode() && ((NodeImpl) item).isShareable()) {
                 return getNode(path);
@@ -570,7 +570,7 @@ public class ItemManager implements Item
         }
         try {
             if (parentId == null) {
-                return (NodeImpl) getItem(id, path);
+                return (NodeImpl) getItem(id, path, true);
             }
             // if the node is shareable, it now returns the node with the right
             // parent
@@ -594,12 +594,12 @@ public class ItemManager implements Item
             throw new PathNotFoundException(safeGetJCRPath(path));
         }
         try {
-            return (PropertyImpl) getItem(id, path);
+            return (PropertyImpl) getItem(id, path, true);
         } catch (ItemNotFoundException infe) {
             throw new PathNotFoundException(safeGetJCRPath(path));
         }
     }
-
+    
     /**
      * @param id
      * @return
@@ -607,7 +607,17 @@ public class ItemManager implements Item
      */
     public synchronized ItemImpl getItem(ItemId id)
             throws ItemNotFoundException, AccessDeniedException, RepositoryException {
-        return getItem(id, null);
+        return getItem(id, null, true);
+    }
+
+    /**
+     * @param id
+     * @return
+     * @throws RepositoryException
+     */
+    synchronized ItemImpl getItem(ItemId id, boolean permissionCheck)
+            throws ItemNotFoundException, AccessDeniedException, RepositoryException {
+        return getItem(id, null, permissionCheck);
     }
 
     /**
@@ -622,12 +632,29 @@ public class ItemManager implements Item
      */
     public synchronized NodeImpl getNode(NodeId id, NodeId parentId)
             throws ItemNotFoundException, AccessDeniedException, RepositoryException {
+        return getNode(id, parentId, true);
+    }
+
+    /**
+     * Returns a node with a given id and parent id. If the indicated node is
+     * shareable, there might be multiple nodes associated with the same id,
+     * but there'is only one node with the given parent id.
+     *
+     * @param id node id
+     * @param parentId parent node id
+     * @param permissionCheck Flag indicating if read permission must be check
+     * upon retrieving the node.
+     * @return node
+     * @throws RepositoryException if an error occurs
+     */
+    synchronized NodeImpl getNode(NodeId id, NodeId parentId, boolean permissionCheck)
+            throws ItemNotFoundException, AccessDeniedException, RepositoryException {
         if (parentId == null) {
             return (NodeImpl) getItem(id);
         }
         AbstractNodeData data = retrieveItem(id, parentId);
         if (data == null) {
-            data = (AbstractNodeData) getItemData(id);
+            data = (AbstractNodeData) getItemData(id, null, permissionCheck);
         }
         if (!data.getParentId().equals(parentId)) {
             // verify that parent actually appears in the shared set

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java?rev=1097363&r1=1097362&r2=1097363&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
Thu Apr 28 08:40:43 2011
@@ -682,7 +682,14 @@ public class NodeImpl extends ItemImpl i
                 NodeId childId = entry.getId();
                 //NodeImpl childNode = (NodeImpl) itemMgr.getItem(childId);
                 try {
-                    NodeImpl childNode = itemMgr.getNode(childId, getNodeId());
+                    /* omit the read-permission check upon retrieving the
+                       child item as this is an internal call to remove the
+                       subtree which may contain (protected) child items which
+                       are not visible to the caller of the removal. the actual
+                       validation of the remove permission however is only
+                       executed during Item.save(). 
+                     */
+                    NodeImpl childNode = itemMgr.getNode(childId, getNodeId(), false);
                     childNode.onRemove(thisState.getNodeId());
                     // remove the child node entry
                 } catch (ItemNotFoundException e) {
@@ -715,7 +722,14 @@ public class NodeImpl extends ItemImpl i
             thisState.removePropertyName(propName);
             // remove property
             PropertyId propId = new PropertyId(thisState.getNodeId(), propName);
-            itemMgr.getItem(propId).setRemoved();
+            /* omit the read-permission check upon retrieving the
+               child item as this is an internal call to remove the
+               subtree which may contain (protected) child items which
+               are not visible to the caller of the removal. the actual
+               validation of the remove permission however is only
+               executed during Item.save().
+             */
+            itemMgr.getItem(propId, false).setRemoved();
         }
 
         // finally remove this node

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java?rev=1097363&r1=1097362&r2=1097363&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
Thu Apr 28 08:40:43 2011
@@ -422,4 +422,86 @@ public class WriteTest extends AbstractW
         n.addNode("someChild");
         n.save();
     }
+
+    public void testRemoveNodeWithPolicy() throws Exception {
+        Privilege[] privileges = privilegesFromNames(new String[] {Privilege.JCR_READ, Privilege.JCR_WRITE});
+
+        /* allow READ/WRITE privilege for testUser at 'path' */
+        givePrivileges(path, testUser.getPrincipal(), privileges, getRestrictions(superuser,
path));
+        /* allow READ/WRITE privilege for testUser at 'childPath' */
+        givePrivileges(childNPath, testUser.getPrincipal(), privileges, getRestrictions(superuser,
path));
+
+        Session testSession = getTestSession();
+
+        assertTrue(testSession.nodeExists(childNPath));
+        assertTrue(testSession.hasPermission(childNPath, Session.ACTION_REMOVE));
+
+        Node n = testSession.getNode(childNPath);
+
+        // removing the child node must succeed as both remove-node and
+        // remove-child-nodes are granted to testsession.
+        // the policy node underneath childNPath should silently be removed
+        // as the editing session has no knowledge about it's existence.
+        n.remove();
+        testSession.save();
+    }
+
+    public void testRemoveNodeWithInvisibleChild() throws Exception {
+        Privilege[] privileges = privilegesFromNames(new String[] {Privilege.JCR_READ, Privilege.JCR_WRITE});
+
+        Node invisible = superuser.getNode(childNPath).addNode(nodeName3);
+        superuser.save();
+
+        /* allow READ/WRITE privilege for testUser at 'path' */
+        givePrivileges(path, testUser.getPrincipal(), privileges, getRestrictions(superuser,
path));
+        /* deny READ privilege at invisible node. (removal is still granted) */
+        withdrawPrivileges(invisible.getPath(), testUser.getPrincipal(),
+                privilegesFromNames(new String[] {Privilege.JCR_READ}),
+                getRestrictions(superuser, path));
+
+        Session testSession = getTestSession();
+
+        assertTrue(testSession.nodeExists(childNPath));
+        assertTrue(testSession.hasPermission(childNPath, Session.ACTION_REMOVE));
+
+        Node n = testSession.getNode(childNPath);
+
+        // removing the child node must succeed as both remove-node and
+        // remove-child-nodes are granted to testsession.
+        // the policy node underneath childNPath should silently be removed
+        // as the editing session has no knowledge about it's existence.
+        n.remove();
+        testSession.save();
+    }
+
+    public void testRemoveNodeWithInvisibleNonRemovableChild() throws Exception {
+        Privilege[] privileges = privilegesFromNames(new String[] {Privilege.JCR_READ, Privilege.JCR_WRITE});
+
+        Node invisible = superuser.getNode(childNPath).addNode(nodeName3);
+        superuser.save();
+
+        /* allow READ/WRITE privilege for testUser at 'path' */
+        givePrivileges(path, testUser.getPrincipal(), privileges, getRestrictions(superuser,
path));
+        /* deny READ privilege at invisible node. (removal is still granted) */
+        withdrawPrivileges(invisible.getPath(), testUser.getPrincipal(),
+                privileges,
+                getRestrictions(superuser, path));
+
+        Session testSession = getTestSession();
+
+        assertTrue(testSession.nodeExists(childNPath));
+        assertTrue(testSession.hasPermission(childNPath, Session.ACTION_REMOVE));
+
+        Node n = testSession.getNode(childNPath);
+
+        // removing the child node must fail as a hidden child node cannot
+        // be removed.
+        try {
+            n.remove();
+            testSession.save();
+            fail();
+        } catch (AccessDeniedException e) {
+            // success
+        }
+    }
 }
\ No newline at end of file



Mime
View raw message