[jira] [Comment Edited] (OAK-1115) Remove of Subtree after Move is not subjected to permission validation

2013-10-24 Thread angela (JIRA)

[ 
https://issues.apache.org/jira/browse/OAK-1115?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13804182#comment-13804182
 ] 

angela edited comment on OAK-1115 at 10/24/13 1:43 PM:
---

well, i don't see it... as the move is perfectly valid and there was no removal 
at all except for the subtree.
as soon as a node was really removed the validator checks if that node can be 
removed but doesn't check for all the child nodes which makes sense IMO.

in other words: i don't see how i can find out that the only tree that was 
really removed was actually the subtree and not the one that is being reported 
(but which was associated with the move and which hasn't been removed at all)?

if i was able to detect that the removeNode is in fact triggered by a move i 
would walk down and verify that the subtree hasn't been modified (neither add 
nor removal)


was (Author: anchela):
well, i don't see it... as the move is perfectly valid and there was no removal 
at all except for the subtree.
as soon as a node was really removed the validator checks if that node can be 
removed but doesn't check for all the child nodes which makes sense IMO.

in other words: i don't see how i can find out that the only tree that was 
really removed was actually the subtree and not the one that is being reported 
(but which was associated with the move and which has been removed at all)?

 Remove of Subtree after Move is not subjected to permission validation
 --

 Key: OAK-1115
 URL: https://issues.apache.org/jira/browse/OAK-1115
 Project: Jackrabbit Oak
  Issue Type: Bug
  Components: core
Reporter: angela
Priority: Critical

 the following test passes in Jackrabbit-Core but fails in OAK:
 {code}
 @Test
 public void testMoveRemoveSubTree() throws Exception {
 superuser.getNode(childNPath).addNode(nodeName3);
 superuser.save();
 /* allow READ/WRITE privilege for testUser at 'path' */
 givePrivileges(path, privilegesFromNames(new String[] 
 {Privilege.JCR_READ, rep:write}), Collections.String, ValueemptyMap());
 /* deny READ/REMOVE property privileges at subtree. */
 withdrawPrivileges(path, privilegesFromNames(new String[] 
 {Privilege.JCR_REMOVE_NODE}), Collections.singletonMap(rep:glob, 
 superuser.getValueFactory().createValue(*/+nodeName3)));
 Session testSession = getTestSession();
 assertTrue(testSession.nodeExists(childNPath));
 assertTrue(testSession.hasPermission(childNPath, 
 Session.ACTION_REMOVE));
 assertTrue(testSession.hasPermission(childNPath2, 
 Session.ACTION_ADD_NODE));
 testSession.move(childNPath, childNPath2 + /dest);
 Node dest = testSession.getNode(childNPath2 + /dest);
 dest.getNode(nodeName3).remove();
 try {
 testSession.save();
 fail(Removing child node must be denied.);
 } catch (AccessDeniedException e) {
 // success
 }
 }
 {code}
 this is a critical security issue as it moving around the parent is 
 sufficient in order to be able to remove a node that was otherwise not 
 removable due to limited permissions.
 Afaik this behavior is caused by a limitation in the Diff process which 
 doesn't allow to identify the move and thus makes it impossible to find out 
 if that the subtree has been removed.



--
This message was sent by Atlassian JIRA
(v6.1#6144)


[jira] [Comment Edited] (OAK-1115) Remove of Subtree after Move is not subjected to permission validation

2013-10-24 Thread angela (JIRA)

[ 
https://issues.apache.org/jira/browse/OAK-1115?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13804226#comment-13804226
 ] 

angela edited comment on OAK-1115 at 10/24/13 2:14 PM:
---

 I think it's important that the remove permissions are evaluated for the 
 entire subtree instead of just the parent.

well, the last time we discussed this, you said that it's seems better to just 
enforce the remove permission for the remove target, didn't you? apart from 
that we will not be able to enforce remove permissions on the entire subtree 
for backwards compatibility reasons. an example: if you don't have permissions 
to read/edit AC content in a given subtree but you are allowed to remove the 
root of that subtree, you could remove that tree in jr2 irrespective of any 
kind of ac content being present in the subtree... that's something that used 
to cause troubles in the beginning when we introduced access control in jr2.

example 2: a user that is not allowed to execute version operations but is a 
allowed to remove a node should IMO be able to do so, although removing the 
version related protected properties would not be allowed due to lack of the 
corresponding version-mgt permission.


was (Author: anchela):
 I think it's important that the remove permissions are evaluated for the 
 entire subtree instead of just the parent.

well, the last time we discussed this, you said that it's seems better to just 
enforce the remove permission for the remove target, didn't you? apart from 
that we will not be able to enforce remove permissions on the entire subtree 
for backwards compatibility reasons. an example: if you don't have permissions 
to read/edit AC content in a given subtree but you are allowed to remove the 
root of that subtree, you could remove that tree in jr2 irrespective of any 
kind of ac content being present in the subtree... that's something that used 
to cause troubles in the beginning when we introduced access control in jr2.

 Remove of Subtree after Move is not subjected to permission validation
 --

 Key: OAK-1115
 URL: https://issues.apache.org/jira/browse/OAK-1115
 Project: Jackrabbit Oak
  Issue Type: Bug
  Components: core
Reporter: angela
Priority: Critical

 the following test passes in Jackrabbit-Core but fails in OAK:
 {code}
 @Test
 public void testMoveRemoveSubTree() throws Exception {
 superuser.getNode(childNPath).addNode(nodeName3);
 superuser.save();
 /* allow READ/WRITE privilege for testUser at 'path' */
 givePrivileges(path, privilegesFromNames(new String[] 
 {Privilege.JCR_READ, rep:write}), Collections.String, ValueemptyMap());
 /* deny READ/REMOVE property privileges at subtree. */
 withdrawPrivileges(path, privilegesFromNames(new String[] 
 {Privilege.JCR_REMOVE_NODE}), Collections.singletonMap(rep:glob, 
 superuser.getValueFactory().createValue(*/+nodeName3)));
 Session testSession = getTestSession();
 assertTrue(testSession.nodeExists(childNPath));
 assertTrue(testSession.hasPermission(childNPath, 
 Session.ACTION_REMOVE));
 assertTrue(testSession.hasPermission(childNPath2, 
 Session.ACTION_ADD_NODE));
 testSession.move(childNPath, childNPath2 + /dest);
 Node dest = testSession.getNode(childNPath2 + /dest);
 dest.getNode(nodeName3).remove();
 try {
 testSession.save();
 fail(Removing child node must be denied.);
 } catch (AccessDeniedException e) {
 // success
 }
 }
 {code}
 this is a critical security issue as it moving around the parent is 
 sufficient in order to be able to remove a node that was otherwise not 
 removable due to limited permissions.
 Afaik this behavior is caused by a limitation in the Diff process which 
 doesn't allow to identify the move and thus makes it impossible to find out 
 if that the subtree has been removed.



--
This message was sent by Atlassian JIRA
(v6.1#6144)