You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2013/11/26 16:44:24 UTC
svn commit: r1545689 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/
Author: angela
Date: Tue Nov 26 15:44:24 2013
New Revision: 1545689
URL: http://svn.apache.org/r1545689
Log:
OAK-710 : PermissionValidator: Proper permission evaluation for moving/renaming nodes (WIP)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/SessionMoveTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java?rev=1545689&r1=1545688&r2=1545689&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/MoveAwarePermissionValidator.java Tue Nov 26 15:44:24 2013
@@ -31,7 +31,8 @@ import org.apache.jackrabbit.oak.spi.sec
import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
import org.apache.jackrabbit.oak.spi.security.authorization.permission.TreePermission;
import org.apache.jackrabbit.oak.spi.state.NodeState;
-import org.apache.jackrabbit.util.Text;
+
+import static org.apache.jackrabbit.oak.api.CommitFailedException.ACCESS;
/**
* MoveAwarePermissionValidator... TODO
@@ -123,36 +124,31 @@ public class MoveAwarePermissionValidato
}
private boolean processAdd(ImmutableTree child, MoveAwarePermissionValidator validator) throws CommitFailedException {
- // check permissions for adding the moved node at the target location.
- validator.checkPermissions(child, false, Permissions.ADD_NODE | Permissions.NODE_TYPE_MANAGEMENT);
-
// FIXME: respect and properly handle move-operations in the subtree
String sourcePath = moveTracker.getOriginalSourcePath(child.getPath());
if (sourcePath != null) {
ImmutableTree source = rootBefore.getTree(sourcePath);
if (source.exists()) {
- ImmutableTree rootTree = rootBefore.getTree("/");
- TreePermission tp = getPermissionProvider().getTreePermission(rootTree, TreePermission.EMPTY);
- for (String seg : Text.explode(sourcePath, '/')) {
- tp = tp.getChildPermission(seg, rootTree.getChild(seg).getNodeState());
- }
+ // check permissions for adding the moved node at the target location.
+ validator.checkPermissions(child, false, Permissions.ADD_NODE | Permissions.NODE_TYPE_MANAGEMENT);
+ checkPermissions(source, Permissions.REMOVE_NODE);
return diff(source, child, validator);
- } // FIXME: else...
+ }
}
return false;
}
private boolean processDelete(ImmutableTree child, MoveAwarePermissionValidator validator) throws CommitFailedException {
- // check permissions for removing that node.
- validator.checkPermissions(child, true, Permissions.REMOVE_NODE);
-
// FIXME: respect and properly handle move-operations in the subtree
String destPath = moveTracker.getDestPath(child.getPath());
if (destPath != null) {
ImmutableTree dest = rootAfter.getTree(destPath);
if (dest.exists()) {
+ // check permissions for removing that node.
+ validator.checkPermissions(child, true, Permissions.REMOVE_NODE);
+ checkPermissions(dest, Permissions.ADD_NODE|Permissions.NODE_TYPE_MANAGEMENT);
return diff(child, dest, validator);
- } // FIXME: else...
+ }
}
return false;
@@ -167,5 +163,11 @@ public class MoveAwarePermissionValidato
}
return true;
}
+
+ private void checkPermissions(@Nonnull Tree tree, long permissions) throws CommitFailedException {
+ if (!getPermissionProvider().isGranted(tree, null, permissions)) {
+ throw new CommitFailedException(ACCESS, 0, "Access denied");
+ }
+ }
}
}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/SessionMoveTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/SessionMoveTest.java?rev=1545689&r1=1545688&r2=1545689&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/SessionMoveTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/SessionMoveTest.java Tue Nov 26 15:44:24 2013
@@ -23,6 +23,7 @@ import javax.jcr.Session;
import javax.jcr.security.Privilege;
import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
+import org.apache.jackrabbit.util.Text;
import org.junit.Ignore;
import org.junit.Test;
@@ -207,13 +208,113 @@ public class SessionMoveTest extends Abs
}
@Test
+ public void testMoveAndAddAtSourceParent() throws Exception {
+ allow(path, privilegesFromName(Privilege.JCR_REMOVE_CHILD_NODES));
+ allow(childNPath, privilegesFromName(Privilege.JCR_REMOVE_NODE));
+ allow(siblingPath, privilegesFromNames(new String[]{
+ PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT
+ }));
+
+ testSession.move(childNPath, siblingDestPath);
+
+ Node sourceParent = testSession.getNode(path);
+ sourceParent.addNode(nodeName4);
+
+ try {
+ testSession.save();
+ fail("Adding child node at source parent be denied: missing add_child_node privilege.");
+ } catch (AccessDeniedException e) {
+ // success
+ }
+ }
+
+ @Test
+ public void testMoveAndAddAtSourceParent2() throws Exception {
+ allow(path, privilegesFromName(Privilege.JCR_REMOVE_CHILD_NODES));
+ allow(childNPath, privilegesFromName(Privilege.JCR_REMOVE_NODE));
+ allow(siblingPath, privilegesFromNames(new String[] {
+ PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT
+ }));
+ allow(nodePath3, privilegesFromName(Privilege.JCR_ADD_CHILD_NODES));
+
+ testSession.move(childNPath, siblingDestPath);
+
+ Node sourceParent = testSession.getNode(path);
+ sourceParent.addNode(nodeName4);
+
+ try {
+ testSession.save();
+ fail("Adding child node at source parent be denied: missing add_child_node privilege.");
+ } catch (AccessDeniedException e) {
+ // success
+ }
+ }
+
+ @Test
+ public void testMoveAndAddAtSourceParent3() throws Exception {
+ allow(path, privilegesFromNames(new String[]{
+ Privilege.JCR_REMOVE_CHILD_NODES, Privilege.JCR_ADD_CHILD_NODES
+ }));
+ allow(childNPath, privilegesFromName(Privilege.JCR_REMOVE_NODE));
+ allow(siblingPath, privilegesFromNames(new String[]{
+ PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT
+ }));
+
+ testSession.move(childNPath, siblingDestPath);
+
+ Node sourceParent = testSession.getNode(path);
+ sourceParent.addNode(nodeName4);
+
+ testSession.save();
+ }
+
+ @Test
public void testMoveAndRemoveProperty() throws Exception {
// TODO
}
+ // FIXME: adding replacement is not detected by node state diff.
+// @Test
+// public void testMoveAndAddReplacementAtSource() throws Exception {
+// allow(path, privilegesFromNames(new String[]{
+// Privilege.JCR_REMOVE_CHILD_NODES, Privilege.JCR_ADD_CHILD_NODES
+// }));
+// allow(siblingPath, privilegesFromNames(new String[] {
+// PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT
+// }));
+//
+// testSession.move(nodePath3, siblingDestPath);
+//
+// Node sourceParent = testSession.getNode(childNPath);
+// Node replacement = sourceParent.addNode(Text.getName(nodePath3));
+// replacement.setProperty("movedProp", "val");
+//
+// try {
+// testSession.save();
+// fail("Missing ADD_NODE and ADD_PROPERTY permission on source parent.");
+// } catch (AccessDeniedException e) {
+// // success
+// }
+// }
+
@Test
- public void testMoveAndAddReplacementAtSource() throws Exception {
- // TODO
+ public void testMoveAndAddReplacementAtSource2() throws Exception {
+ allow(siblingPath, privilegesFromNames(new String[] {
+ PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT
+ }));
+
+ testSession.move(nodePath3, siblingDestPath);
+
+ Node sourceParent = testSession.getNode(childNPath);
+ Node replacement = sourceParent.addNode(Text.getName(nodePath3));
+ replacement.setProperty("movedProp", "val");
+
+ try {
+ testSession.save();
+ fail("Missing REMOVE_NODE permission for move source.");
+ } catch (AccessDeniedException e) {
+ // success
+ }
}
@Test