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 2014/01/30 17:44:34 UTC

svn commit: r1562891 - in /jackrabbit/oak/trunk: oak-doc/src/site/markdown/ oak-jcr/ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/

Author: angela
Date: Thu Jan 30 16:44:34 2014
New Revision: 1562891

URL: http://svn.apache.org/r1562891
Log:
OAK-710: permission eval for move; OAK-942: document permission evaluation

Added:
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/MultipleMoveTest.java
      - copied, changed from r1562713, jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/SessionMoveTest.java
Modified:
    jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences_permission.md
    jackrabbit/oak/trunk/oak-jcr/pom.xml
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/SessionMoveTest.java

Modified: jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences_permission.md
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences_permission.md?rev=1562891&r1=1562890&r2=1562891&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences_permission.md (original)
+++ jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences_permission.md Thu Jan 30 16:44:34 2014
@@ -16,28 +16,131 @@
   -->
 ### Permission Evaluation : Differences wrt Jackrabbit 2.x
 
-*NOTE: Work in Progress*
+#### 1. Characteristics of the Default Implementation
 
-Refer to [OAK-942](https://issues.apache.org/jira/browse/OAK-942) for a general overview of changes
-with respect to Jackrabbit 2.
+##### General
+_TODO_
 
-* As of Oak `Node#remove()` only requires sufficient permissions to remove the target node. In
-  contrast to jackrabbit the validation will not traverse the tree and verify remove permission on
-  all child nodes/properties. There exists a configuration flag that aims to produce best effort
-  backwards compatibility but this flag is currently not enabled by default. Please let us know if
-  you suspect this causes wrong behavior in your application.
-
-* 'JackrabbitNode#rename': Due to the nature of the diff mechanism in oak it is
-  not possible to distinguish between rename and a move-call with subsequent
-  reordering. Consequently the permission evaluation will no longer apply the
-  special handling for the renaming as it was present in Jackrabbit 2.x (renaming
-  just required the ability to modify the child collection of the parent node).
-
-* By default user management operations require the specific user mgt related
-  permission that has been introduced with OAK-1.0. This behavior can be
-  turned off by setting the corresponding configuration flag.
-
-* As of OAK reading and writing items in the version store does not follow the
-  regular permission evaluation but depends on access rights present on the
-  corresponding versionable node [OAK-444](https://issues.apache.org/jira/browse/OAK-444).
+##### JCR API
+###### `Session#hasPermission` and `Session#checkPermission`
 
+Since Oak the permission related API calls not only allow to pass the action strings defined by JCR specification (see constants defined in `Session.java`) but also handles the names of the permission defined by Oak (see `Permissions#getString(long permissions)`).
+
+##### Mapping of JCR Actions to Permissions
+_TODO_
+
+##### Permissions
+The set of permissions supported by Oak are listed in [Permissions]. The following changes have been compared compared to Jackrabbit 2.x:
+
+- `READ_NODE`: permission to read a node
+- `READ_PROPERTY`: permission to read a property
+- `ADD_PROPERTY`: permission to create a new property
+- `MODIFY_PROPERTY`: permission to change an existing property
+- `REMOVE`: aggregation of `REMOVE_NODE` and `REMOVE_PROPERTY`
+- `USER_MANAGEMENT`: permission to execute user management related tasks such as e.g. creating or removing user/group, changing user password and editing group membership.
+
+The following permissions are now an aggregation of new permissions:
+
+- `READ`: aggregates `READ_NODE` and `READ_PROPERTY`
+- `SET_PROPERTY`: aggregates `ADD_PROPERTY`, `MODIFY_PROPERTY` and `REMOVE_PROPERTY`
+
+#### 2. Permission Evaluation
+
+##### Reading
+Due to the fine grained read permissions Oak read access can be separately granted/denied
+for nodes and properties. See also the section about extended restriction management
+in [OAK-792]. Granting the `jcr:read` privilege will result in a backwards compatible
+read access for nodes and their properties, while specifying `rep:readNodes` or
+`rep:readProperties` privileges allows separately granting or denying access to
+nodes and properties (see also [OAK-910] for changes in the privilege definitions).
+Together with the restrictions this new behavior now allows to individually grant/deny
+access to properties that match a given name/path/nodetype (and as a possible extension even property value).
+
+The only break in terms of backwards compatibility is the accessibility of version
+content underneath `/jcr:system/jcr:versionStore`. As of Oak the access to version
+content depends on the read permissions present with the versionable node while
+Jackrabbit 2.x doesn't apply any special rule. These changes are covered by [OAK-444]
+and address the concerns summarized in [JCR-2963].
+
+##### Property Modification
+Since Oak the former `SET_PROPERTY` permission has been split such to allow for more fined grained control on writing JCR properties. In particular Oak clearly distinguishes between creating a new property that didn't exist before, modifying or removing an existing property.
+This will allow to cover those cases where a given subject is only allowed to create content but doesn't have the ability to modify/delete it later on.
+
+##### Node Removal
+As of Oak `Node#remove()` only requires sufficient permissions to remove the target node. In contrast to Jackrabbit 2.x the validation will not traverse the tree and verify remove permission on all child nodes/properties.
+In order to obtain backwards compatible behavior with respect to tree removal the permission evaluation can be configured to traverse down the hierarchy upon removal. This config flag is a best effort approach but doesn't guarantee an identical behavior.
+
+##### Rename
+Due to the nature of the diff mechanism in Oak it is not possible to distinguish
+between `JackrabbitNode#rename` and a move with subsequent reordering. Consequently
+the permission evaluation will no longer apply the special handling for the renaming
+as it was present in Jackrabbit 2.x (renaming just required the ability to modify
+the child collection of the parent node).
+
+##### Move
+_TODO: permission evaluation with move is not yet implemented [OAK-710]_
+
+##### Copy
+_TODO: permission evaluation with copy is not yet implemented [OAK-920]_
+
+##### User Management
+By default user management operations require the specific user mgt related permission to be granted for the editing subject. This permission (including a corresponding privilege) has been introduced with Oak 1.0.
+For backwards compatibility with Jackrabbit 2.x this behavior can be turned off by setting the corresponding configuration flag.
+
+##### Version Management
+Reading and writing items in the version store does not follow the regular permission evaluation but depends on access rights present on the corresponding versionable node. In case the version information does no longer have a versionable node in this workspace that original path is used to evaluate the effective permissions that would apply to that node if the version was restored.
+Note, that as in Jackrabbit VERSION_MANAGEMENT permission instead of the regular JCR write permissions is required in order to execute version operations and thus modify the version store. These changes are covered by [OAK-444] and address the concerns summarized in [JCR-2963].
+
+#### 3. Administrative Principals
+The following principals always have full access to the whole content repository irrespective of the access control content:
+
+- `SystemPrincipal`
+- All instances of `AdminPrincipal`
+- All principals whose name matches the configured administrative principal names (see Configuration section below). This configuration only applies to the permission evaluation and is currently not reflected in other security models nor methods that deal with the administrator (i.e. `User#isAdmin`).
+
+#### 4. Node Types
+
+    [rep:PermissionStore]
+      - rep:accessControlledPath (STRING) protected IGNORE
+      - rep:numPermissions (LONG) protected IGNORE
+      - rep:modCount (LONG) protected IGNORE
+      + * (rep:PermissionStore) = rep:PermissionStore protected IGNORE
+      + * (rep:Permissions) = rep:Permissions protected IGNORE
+
+    [rep:Permissions]
+      - * (UNDEFINED) protected IGNORE
+      - * (UNDEFINED) protected multiple IGNORE
+      + * (rep:Permissions) = rep:Permissions protected IGNORE
+
+    [rep:VersionablePaths]
+      mixin
+      - * (PATH) protected ABORT
+
+#### 5. API Extensions
+
+org.apache.jackrabbit.oak.spi.security.authorization.permission
+
+- `PermissionProvider`: _TODO_
+- `Permissions`: _TODO_
+- `PermissionConstants`: _TODO_
+
+#### 6. Configuration
+
+Configuration Parameters supported by the default implementation
+
+- `PARAM_PERMISSIONS_JR2`: Enables backwards compatible behavior for the permissions listed in the parameter value. Currently the following values are allowed: `USER_MANAGEMENT` and `REMOVE_NODE`. The parameter value must contain the permission names separated by ','.
+- `PARAM_READ_PATHS`: default set of paths that are always readable to all principals irrespective of other permissions defined at that path or inherited from other nodes.
+- `PARAM_ADMINISTRATIVE_PRINCIPALS`: The names of the additional principals that have full permission and for which the permission evaluation can be skipped altogether.
+
+Differences to Jackrabbit 2.x
+The `omit-default-permission` configuration option present with the Jackrabbit's AccessControlProvider implementations is no longer supported with Oak.
+Since there are no permissions installed by default this flag has become superfluous.
+
+<!-- hidden references -->
+[Permissions]: http://svn.apache.org/repos/asf/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/permission/Permissions.java
+[OAK-444]: https://issues.apache.org/jira/browse/OAK-444
+[OAK-792]: https://issues.apache.org/jira/browse/OAK-792
+[OAK-910]: https://issues.apache.org/jira/browse/OAK-910
+[OAK-920]: https://issues.apache.org/jira/browse/OAK-920
+[OAK-710]: https://issues.apache.org/jira/browse/OAK-710
+[JCR-2963]: https://issues.apache.org/jira/browse/JCR-2963
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-jcr/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/pom.xml?rev=1562891&r1=1562890&r2=1562891&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-jcr/pom.xml Thu Jan 30 16:44:34 2014
@@ -120,6 +120,9 @@
       org.apache.jackrabbit.oak.jcr.security.authorization.VersionManagementTest#testRemoveVersion3  <!-- OAK-168 -->
 
       org.apache.jackrabbit.oak.jcr.security.authorization.CopyTest#testCopyInvisibleSubTree         <!-- OAK-920 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.MultipleMoveTest#testMoveSubTreeBack4     <!-- OAK-710 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.MultipleMoveTest#testMoveDestParent2      <!-- OAK-710 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.MultipleMoveTest#testMoveDestParent4      <!-- OAK-710 -->
 
       <!-- Query -->
       org.apache.jackrabbit.test.api.query.ElementTest#testElementTestNameTestSomeNTWithSNS          <!-- OAK-203 -->

Copied: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/MultipleMoveTest.java (from r1562713, 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/MultipleMoveTest.java?p2=jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/MultipleMoveTest.java&p1=jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/SessionMoveTest.java&r1=1562713&r2=1562891&rev=1562891&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/MultipleMoveTest.java Thu Jan 30 16:44:34 2014
@@ -18,534 +18,183 @@ package org.apache.jackrabbit.oak.jcr.se
 
 import javax.jcr.AccessDeniedException;
 import javax.jcr.Node;
-import javax.jcr.Property;
 import javax.jcr.RepositoryException;
 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.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 
 /**
- * Permission evaluation tests for move operations.
+ * Permission evaluation tests for multiple move operations.
  */
-public class SessionMoveTest extends AbstractMoveTest {
+public class MultipleMoveTest extends AbstractEvaluationTest {
 
-    protected void move(String source, String dest) throws RepositoryException {
+    protected String nodePath3;
+    protected String siblingDestPath;
+
+    @Override
+    @Before
+    protected void setUp() throws Exception {
+        super.setUp();
+
+        Node node3 = superuser.getNode(childNPath).addNode(nodeName3);
+        node3.setProperty("movedProp", "val");
+        nodePath3 = node3.getPath();
+        superuser.save();
+        testSession.refresh(false);
+
+        siblingDestPath = siblingPath + "/destination";
+    }
+
+    private void move(String source, String dest) throws RepositoryException {
         move(source, dest, testSession);
     }
 
-    @Override
-    protected void move(String source, String dest, Session session) throws RepositoryException {
+    private void move(String source, String dest, Session session) throws RepositoryException {
         session.move(source, dest);
-        session.save();
     }
 
-    private void setupMovePermissions() throws Exception {
-        allow(path, privilegesFromNames(new String[]{
+    private void setupMovePermissions(String source, String dest) throws Exception {
+        allow(source, privilegesFromNames(new String[]{
                 Privilege.JCR_REMOVE_NODE,
                 Privilege.JCR_REMOVE_CHILD_NODES
         }));
-        allow(siblingPath, privilegesFromNames(new String[] {
+        allow(dest, privilegesFromNames(new String[] {
                 Privilege.JCR_ADD_CHILD_NODES,
                 Privilege.JCR_NODE_TYPE_MANAGEMENT}));
 
     }
 
     @Test
-    public void testMoveAndRemoveSubTree() throws Exception {
-        allow(path, privilegesFromName(Privilege.JCR_REMOVE_CHILD_NODES));
-        allow(siblingPath, privilegesFromNames(new String[]{
-                Privilege.JCR_ADD_CHILD_NODES,
-                Privilege.JCR_NODE_TYPE_MANAGEMENT}));
-
-        testSession.move(childNPath, siblingDestPath);
-
-        Node moved = testSession.getNode(siblingDestPath);
-        Node child = moved.getNode(nodeName3);
+    public void testMoveSubTreeBack() throws Exception {
+        setupMovePermissions(path, siblingPath);
 
         try {
-            child.remove();
+            // first move must succeed
+            move(childNPath, siblingDestPath);
+            // moving child back must fail due to missing privileges
+            move(siblingDestPath + '/' + nodeName3, path + "/subtreeBack");
             testSession.save();
-            fail("Removing subtree after move requires 'jcr:removeNode' privilege on the target");
+            fail();
         } catch (AccessDeniedException e) {
             // success
-
         }
     }
 
     @Test
-    public void testMoveAndRemoveSubTree2() throws Exception {
-        allow(path, privilegesFromNames(new String[] {
+    public void testMoveSubTreeBack2() throws Exception {
+        allow(testRootNode.getPath(), privilegesFromNames(new String[]{
+                Privilege.JCR_REMOVE_NODE,
                 Privilege.JCR_REMOVE_CHILD_NODES,
-                Privilege.JCR_REMOVE_NODE}));
-        allow(siblingPath, privilegesFromNames(new String[]{
                 Privilege.JCR_ADD_CHILD_NODES,
-                Privilege.JCR_NODE_TYPE_MANAGEMENT}));
-        deny(testSession.getNode(nodePath3).getPath(), privilegesFromName(Privilege.JCR_REMOVE_NODE));
-
-        try {
-            testSession.move(childNPath, siblingDestPath);
-
-            Node moved = testSession.getNode(siblingDestPath);
-            Node child = moved.getNode(nodeName3);
-
-            child.remove();
-            testSession.save();
-            fail("Removing subtree after move requires 'jcr:removeNode' on the removed child.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-    }
-
-    @Test
-    public void testMoveAndRemoveSubTree3() 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
+                Privilege.JCR_NODE_TYPE_MANAGEMENT
         }));
 
-        testSession.move(childNPath, siblingDestPath);
-
-        Node moved = testSession.getNode(siblingDestPath);
-        Node child = moved.getNode(nodeName3);
-        child.remove();
-
+        // first move must succeed
+        move(childNPath, siblingDestPath);
+        // moving child back must fail due to missing privileges
+        move(siblingDestPath + '/' + nodeName3, path + "/subtreeBack");
         testSession.save();
     }
 
     @Test
-    public void testMoveRemoveSubTreeWithRestriction() throws Exception {
-        /* allow READ/WRITE privilege for testUser at 'path' */
-        allow(path, testUser.getPrincipal(), readWritePrivileges);
-        /* deny REMOVE_NODE privileges at subtree. */
-        deny(path, privilegesFromName(PrivilegeConstants.JCR_REMOVE_NODE), createGlobRestriction("*/"+nodeName3));
-
-        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();
+    public void testMoveSubTreeBack3() throws Exception {
+        setupMovePermissions(path, siblingPath);
 
         try {
+            // first move must succeed
+            move(childNPath, siblingDestPath);
+            // moving child back must fail due to missing privileges
+            move(siblingDestPath + '/' + nodeName3, childNPath);
             testSession.save();
-            fail("Removing child node must be denied.");
+            fail();
         } catch (AccessDeniedException e) {
             // success
         }
     }
 
+    @Ignore("OAK-710")
     @Test
-    public void testMoveRemoveSubTreeWithRestriction2() throws Exception {
-            /* allow READ/WRITE privilege for testUser at 'path' */
-        allow(path, testUser.getPrincipal(), readWritePrivileges);
-            /* deny REMOVE_NODE privileges at subtree. */
-        deny(path, privilegesFromName(PrivilegeConstants.JCR_REMOVE_CHILD_NODES), createGlobRestriction("*/" + Text.getName(childNPath)));
-
-        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
-        }
-    }
-
-    @Test
-    public void testMoveAndAddSubTree() 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 moved = testSession.getNode(siblingDestPath);
-        Node child = moved.getNode(nodeName3);
-        child.addNode(nodeName4);
-
-        try {
-            testSession.save();
-            fail("Adding child node at moved node must be denied: no add_child_node privilege at original location.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-    }
-
-    @Test
-    public void testMoveAndAddSubTree2() 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 moved = testSession.getNode(siblingDestPath);
-        Node child = moved.getNode(nodeName3);
-        child.addNode(nodeName4);
-
-        testSession.save();
-    }
-
-    @Test
-    public void testMoveAndAddSubTree3() throws Exception {
-        allow(path, privilegesFromName(Privilege.JCR_REMOVE_CHILD_NODES));
-        allow(childNPath, privilegesFromNames(new String[] {
-                Privilege.JCR_REMOVE_NODE, Privilege.JCR_ADD_CHILD_NODES
-        }));
-        allow(siblingPath, privilegesFromNames(new String[] {
-                PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT
-        }));
-
-        testSession.move(childNPath, siblingDestPath);
-
-        Node moved = testSession.getNode(siblingDestPath);
-        Node child = moved.getNode(nodeName3);
-        child.addNode(nodeName4);
-
-        testSession.save();
-    }
-
-    @Test
-    public void testMoveAddSubTreeWithRestriction() throws Exception {
-        /* allow READ/WRITE privilege for testUser at 'path' */
-        allow(path, testUser.getPrincipal(), readWritePrivileges);
-        /* deny ADD_CHILD_NODES privileges at subtree. */
-        deny(path, privilegesFromName(PrivilegeConstants.JCR_ADD_CHILD_NODES), createGlobRestriction("*/"+nodeName3));
-
-        testSession.move(childNPath, childNPath2 + "/dest");
-        Node dest = testSession.getNode(childNPath2 + "/dest");
-        dest.getNode(nodeName3).addNode(nodeName4);
-
-        try {
-            testSession.save();
-            fail("Adding child node must be denied.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-    }
-
-    @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
+    public void testMoveSubTreeBack4() throws Exception {
+        allow(testRootNode.getPath(), privilegesFromNames(new String[]{
+                Privilege.JCR_REMOVE_NODE,
+                Privilege.JCR_REMOVE_CHILD_NODES,
+                Privilege.JCR_ADD_CHILD_NODES,
+                Privilege.JCR_NODE_TYPE_MANAGEMENT
         }));
 
-        testSession.move(childNPath, siblingDestPath);
-
-        Node sourceParent = testSession.getNode(path);
-        sourceParent.addNode(nodeName4);
-
+        // first move must succeed
+        move(childNPath, siblingDestPath);
+        // moving child back must fail due to missing privileges
+        move(siblingDestPath + '/' + nodeName3, childNPath);
         testSession.save();
     }
 
     @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");
+    public void testMoveDestParent() throws Exception {
+        setupMovePermissions(path, siblingPath);
 
         try {
+            // first move must succeed
+            move(childNPath, siblingDestPath);
+            // moving dest parent must fail due to missing privileges
+            move(siblingPath, path + "/parentMove");
             testSession.save();
-            fail("Missing ADD_NODE and ADD_PROPERTY permission on source parent.");
+            fail();
         } catch (AccessDeniedException e) {
             // success
         }
     }
 
+    @Ignore("OAK-710")
     @Test
-    public void testMoveAndAddReplacementAtSource2() throws Exception {
-        allow(siblingPath, privilegesFromNames(new String[] {
-                PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_NODE_TYPE_MANAGEMENT
+    public void testMoveDestParent2() throws Exception {
+        allow(testRootNode.getPath(), privilegesFromNames(new String[]{
+                Privilege.JCR_REMOVE_NODE,
+                Privilege.JCR_REMOVE_CHILD_NODES,
+                Privilege.JCR_ADD_CHILD_NODES,
+                Privilege.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
-    public void testMoveAndAddProperty() throws Exception {
-        setupMovePermissions();
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node destNode = testSession.getNode(siblingDestPath);
-        Property p = destNode.setProperty("newProp", "val");
-        try {
-            testSession.save();
-            fail("Missing ADD_PROPERTY permission.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-    }
-
-    @Test
-    public void testMoveAndAddProperty2() throws Exception {
-        setupMovePermissions();
-        allow(childNPath, privilegesFromName(PrivilegeConstants.REP_ADD_PROPERTIES));
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node destNode = testSession.getNode(siblingDestPath);
-        Property p = destNode.setProperty("newProp", "val");
-        // now save must succeed
+        // first move must succeed
+        move(childNPath, siblingDestPath);
+        // moving dest parent to original source location
+        move(siblingPath, path + "/parentMove");
         testSession.save();
     }
 
     @Test
-    public void testMoveAndModifyProperty() throws Exception {
-        setupMovePermissions();
+    public void testMoveDestParent3() throws Exception {
+        setupMovePermissions(path, siblingPath);
 
-        testSession.move(nodePath3, siblingDestPath);
-        Node destNode = testSession.getNode(siblingDestPath);
-        destNode.setProperty("movedProp", "modified");
         try {
+            // first move must succeed
+            move(childNPath, siblingDestPath);
+            // moving dest parent to location of originally moved node must fail due to missing privileges
+            move(siblingPath, childNPath);
             testSession.save();
-            fail("Missing MODIFY_PROPERTY permission.");
+            fail();
         } catch (AccessDeniedException e) {
             // success
         }
     }
 
+    @Ignore("OAK-710")
     @Test
-    public void testMoveAndModifyProperty2() throws Exception {
-        setupMovePermissions();
-        allow(siblingPath, privilegesFromName(PrivilegeConstants.REP_ALTER_PROPERTIES));
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node destNode = testSession.getNode(siblingDestPath);
-        destNode.setProperty("movedProp", "modified");
-        try {
-            testSession.save();
-            fail("Missing MODIFY_PROPERTY permission.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-    }
-
-    @Test
-    public void testMoveAndModifyProperty3() throws Exception {
-        setupMovePermissions();
-        allow(childNPath, privilegesFromName(PrivilegeConstants.REP_ALTER_PROPERTIES));
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node destNode = testSession.getNode(siblingDestPath);
-        destNode.setProperty("movedProp", "modified");
-    }
-
-    @Test
-    public void testMoveAndRemoveProperty() throws Exception {
-        setupMovePermissions();
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node destNode = testSession.getNode(siblingDestPath);
-        destNode.getProperty("movedProp").remove();
-        try {
-            testSession.save();
-            fail("Missing REMOVE_PROPERTY permission.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-    }
-
-    @Test
-    public void testMoveAndRemoveProperty2() throws Exception {
-        allow(path, privilegesFromNames(new String[]{
+    public void testMoveDestParent4() throws Exception {
+        allow(testRootNode.getPath(), privilegesFromNames(new String[]{
                 Privilege.JCR_REMOVE_NODE,
                 Privilege.JCR_REMOVE_CHILD_NODES,
-                PrivilegeConstants.REP_REMOVE_PROPERTIES
-        }));
-        allow(siblingPath, privilegesFromNames(new String[] {
                 Privilege.JCR_ADD_CHILD_NODES,
-                Privilege.JCR_NODE_TYPE_MANAGEMENT}));
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node destNode = testSession.getNode(siblingDestPath);
-        destNode.getProperty("movedProp").remove();
-        testSession.save();
-    }
-
-    @Test
-    public void testMoveAndAddPropertyAtSource() throws Exception {
-        setupMovePermissions();
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node n = testSession.getNode(childNPath);
-        Property p = n.setProperty("newProp", "val");
-        try {
-            testSession.save();
-            fail("Missing ADD_PROPERTY permission.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-    }
-
-    @Test
-    public void testMoveAndAddPropertyAtSource2() throws Exception {
-        setupMovePermissions();
-        allow(childNPath, privilegesFromName(PrivilegeConstants.REP_ADD_PROPERTIES));
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node n = testSession.getNode(childNPath);
-        Property p = n.setProperty("newProp", "val");
-        testSession.save();
-    }
-
-    @Test
-    public void testMoveAndModifyPropertyAtSource() throws Exception {
-        setupMovePermissions();
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node n = testSession.getNode(childNPath);
-        assertTrue(n.hasProperty(propertyName1));
-        n.setProperty(propertyName1, "modified");
-        try {
-            testSession.save();
-            fail("Missing MODIFY_PROPERTY permission.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-
-        allow(childNPath, privilegesFromName(PrivilegeConstants.REP_ALTER_PROPERTIES));
-        testSession.save();
-    }
-
-    @Test
-    public void testMoveAndModifyPropertyAtSource2() throws Exception {
-        setupMovePermissions();
-        allow(childNPath, privilegesFromName(PrivilegeConstants.REP_ALTER_PROPERTIES));
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node n = testSession.getNode(childNPath);
-        assertTrue(n.hasProperty(propertyName1));
-        n.setProperty(propertyName1, "modified");
-        testSession.save();
-    }
-
-    @Test
-    public void testMoveAndRemovePropertyAtSource() throws Exception {
-        setupMovePermissions();
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node n = testSession.getNode(childNPath);
-        assertTrue(n.hasProperty(propertyName1));
-        n.getProperty(propertyName1).remove();
-        try {
-            testSession.save();
-            fail("Missing REMOVE_PROPERTY permission.");
-        } catch (AccessDeniedException e) {
-            // success
-        }
-    }
-
-    @Test
-    public void testMoveAndRemovePropertyAtSource2() throws Exception {
-        setupMovePermissions();
-        allow(childNPath, privilegesFromName(PrivilegeConstants.REP_REMOVE_PROPERTIES));
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node n = testSession.getNode(childNPath);
-        assertTrue(n.hasProperty(propertyName1));
-        n.getProperty(propertyName1).remove();
-        testSession.save();
-    }
+                Privilege.JCR_NODE_TYPE_MANAGEMENT
+        }));
 
-    /**
-     * Moving and removing the moved node at destination should be treated like
-     * a simple removal at the original position.
-     */
-    @Test
-    public void testMoveAndRemoveDestination() throws Exception {
-        allow(path, privilegesFromName(Privilege.JCR_REMOVE_CHILD_NODES));
-        allow(childNPath, privilegesFromName(Privilege.JCR_REMOVE_NODE));
-
-        testSession.move(nodePath3, siblingDestPath);
-        Node destNode = testSession.getNode(siblingDestPath);
-        destNode.remove();
+        // first move must succeed
+        move(childNPath, siblingDestPath);
+        // moving dest parent to original source location
+        move(siblingPath, childNPath);
         testSession.save();
     }
-
-    @Test
-    public void testMoveAndMoveSubTreeBack() throws Exception {
-        // TODO
-    }
 }

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=1562891&r1=1562890&r2=1562891&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 Thu Jan 30 16:44:34 2014
@@ -543,9 +543,4 @@ public class SessionMoveTest extends Abs
         destNode.remove();
         testSession.save();
     }
-
-    @Test
-    public void testMoveAndMoveSubTreeBack() throws Exception {
-        // TODO
-    }
 }