Index: jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
===================================================================
--- jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java (revision 1084477)
+++ jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java Wed Apr 27 16:34:25 CEST 2011
@@ -304,15 +304,15 @@
* @param path Path of the item to retrieve or null. In
* the latter case the test for access permission is executed using the
* itemId.
+ * @param permissionCheck
* @return The item identified by the given itemId.
* @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 @@
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 @@
}
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 @@
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,10 +607,20 @@
*/
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);
+ }
+
+ /**
* 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.
@@ -622,12 +632,29 @@
*/
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
Index: jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
===================================================================
--- jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java (revision 1072087)
+++ jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java Wed Apr 27 16:34:47 CEST 2011
@@ -682,7 +682,14 @@
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 @@
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
Index: jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java
===================================================================
--- jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java (revision 954964)
+++ jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/WriteTest.java Wed Apr 27 17:53:22 CEST 2011
@@ -422,4 +422,86 @@
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();
-}
\ No newline at end of file
+ }
+
+ 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