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/04/25 12:20:16 UTC

svn commit: r1475688 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ oak-jcr/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/nodet...

Author: angela
Date: Thu Apr 25 10:20:15 2013
New Revision: 1475688

URL: http://svn.apache.org/r1475688
Log:
OAK-711 : PermissionValidator: Proper permission handling for jcr:nodetypeManagement privilege

Added:
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/nodetype/MixinTest.java
Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java
    jackrabbit/oak/trunk/oak-jcr/pom.xml
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java?rev=1475688&r1=1475687&r2=1475688&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionValidator.java Thu Apr 25 10:20:15 2013
@@ -47,8 +47,6 @@ class PermissionValidator extends Defaul
 
     /* TODO
      * - OAK-710: Renaming nodes or Move with same parent are reflected as remove+add -> needs special handling
-     * - OAK-711: Proper handling of jcr:nodeTypeManagement privilege.
-     * - OAK-785: compatibility mode for user-mgt permission
      */
 
     private final Tree parentBefore;
@@ -223,10 +221,21 @@ class PermissionValidator extends Defaul
         }
         String name = propertyState.getName();
         long perm;
-        if (JcrConstants.JCR_PRIMARYTYPE.equals(name) || JcrConstants.JCR_MIXINTYPES.equals(name)) {
-            // FIXME: OAK-711 (distinguish between autocreated and user-supplied modification (?))
-            // perm = Permissions.NODE_TYPE_MANAGEMENT;
-            perm = defaultPermission;
+        if (JcrConstants.JCR_PRIMARYTYPE.equals(name)) {
+            if (defaultPermission == Permissions.MODIFY_PROPERTY) {
+                perm = Permissions.NODE_TYPE_MANAGEMENT;
+            } else {
+                // can't determine if this was  a user supplied modification of
+                // the primary type -> omit permission check.
+                // Node#addNode(String, String) and related methods need to
+                // perform the permission check (as it used to be in jackrabbit 2.x).
+                perm = Permissions.NO_PERMISSION;
+            }
+        } else if (JcrConstants.JCR_MIXINTYPES.equals(name)) {
+            perm = Permissions.NODE_TYPE_MANAGEMENT;
+        } else if (JcrConstants.JCR_UUID.equals(name)) {
+            // TODO : jcr:uuid is never set using a method on JCR API -> omit permission check
+            perm = Permissions.NO_PERMISSION;
         } else if (isLockProperty(name)) {
             perm = Permissions.LOCK_MANAGEMENT;
         } else if (VersionConstants.VERSION_PROPERTY_NAMES.contains(name)) {

Modified: jackrabbit/oak/trunk/oak-jcr/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/pom.xml?rev=1475688&r1=1475687&r2=1475688&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-jcr/pom.xml Thu Apr 25 10:20:15 2013
@@ -257,6 +257,12 @@
       org.apache.jackrabbit.oak.jcr.security.authorization.ReadTest#testReadDenied                           <!-- OAK-766 -->
       org.apache.jackrabbit.oak.jcr.security.authorization.WriteTest#testWriteIfReadingParentIsDenied        <!-- OAK-766 -->
       org.apache.jackrabbit.oak.jcr.security.authorization.WriteTest#testRemoveNodeWithInvisibleNonRemovableChild   <!-- OAK-51 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.NodeTypeManagementTest#testRemoveMixin                   <!-- OAK-767 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.NodeTypeManagementTest#testRemoveMixinWithoutPermission  <!-- OAK-767 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.NodeTypeManagementTest#testCopy                          <!-- OAK-711 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.NodeTypeManagementTest#testWorkspaceMove                 <!-- OAK-711 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.NodeTypeManagementTest#testSessionMove                   <!-- OAK-711 -->
+      org.apache.jackrabbit.oak.jcr.security.authorization.NodeTypeManagementTest#testWorkspaceImportXML            <!-- OAK-773 -->
     </known.issues>
   </properties>
 

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1475688&r1=1475687&r2=1475688&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java Thu Apr 25 10:20:15 2013
@@ -70,6 +70,7 @@ import org.apache.jackrabbit.oak.api.Pro
 import org.apache.jackrabbit.oak.api.Root;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Tree.Status;
+import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.core.IdentifierManager;
 import org.apache.jackrabbit.oak.jcr.delegate.NodeDelegate;
@@ -79,6 +80,7 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.plugins.nodetype.EffectiveNodeType;
 import org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants;
 import org.apache.jackrabbit.oak.plugins.value.ValueFactoryImpl;
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
 import org.apache.jackrabbit.oak.util.TODO;
 import org.apache.jackrabbit.value.ValueHelper;
 import org.slf4j.Logger;
@@ -245,6 +247,13 @@ public class NodeImpl<T extends NodeDele
                         throw new ConstraintViolationException(
                                 "no matching child node definition found for " + relPath);
                     }
+                } else {
+                    // check for NODE_TYPE_MANAGEMENT permission here as we cannot
+                    // distinguish between user-supplied and system-generated
+                    // modification of that property in the PermissionValidator
+                    if (!hasNtMgtPermission(JcrConstants.JCR_PRIMARYTYPE, ntName)) {
+                        throw new AccessDeniedException("Access denied.");
+                    }
                 }
 
                 // TODO: figure out the right place for this check
@@ -914,8 +923,10 @@ public class NodeImpl<T extends NodeDele
         perform(new ItemWriteOperation<Void>() {
             @Override
             public Void perform() throws RepositoryException {
-                if (!isNodeType(mixinName)) {
-                    throw new NoSuchNodeTypeException();
+                PropertyDelegate propDlg = dlg.getPropertyOrNull(JcrConstants.JCR_MIXINTYPES);
+                String oakName = getOakName(mixinName);
+                if (propDlg == null || !ImmutableSet.copyOf(propDlg.getPropertyState().getValue(Type.NAMES)).contains(oakName)) {
+                    throw new NoSuchNodeTypeException("Mixin " + mixinName +" not contained in " + this);
                 }
 
                 // TODO: implement #removeMixin (OAK-767)
@@ -929,10 +940,18 @@ public class NodeImpl<T extends NodeDele
         return perform(new ItemReadOperation<Boolean>() {
             @Override
             public Boolean perform() throws RepositoryException {
+
                 // TODO: figure out the right place for this check
-                getNodeTypeManager().getNodeType(mixinName); // throws on not found
+                NodeType nt = getNodeTypeManager().getNodeType(mixinName); // throws on not found
+                if (!nt.isMixin()) {
+                    return false;
+                }
                 // TODO: END
 
+                if (!hasNtMgtPermission(JcrConstants.JCR_MIXINTYPES, mixinName)) {
+                    return false;
+                }
+
                 return getEffectiveNodeType().supportsMixin(mixinName);
             }
         });
@@ -1498,4 +1517,14 @@ public class NodeImpl<T extends NodeDele
         });
     }
 
+    private boolean hasNtMgtPermission(String propertyName, String ntName) throws RepositoryException {
+        PropertyState property;
+        if (JcrConstants.JCR_MIXINTYPES.equals(propertyName)) {
+            property = PropertyStates.createProperty(propertyName, Collections.singleton(getOakName(ntName)), Type.NAMES);
+        } else {
+            property = PropertyStates.createProperty(propertyName, getOakName(ntName), Type.NAME);
+        }
+        return sessionContext.getPermissionProvider().isGranted(dlg.getTree(), property, Permissions.NODE_TYPE_MANAGEMENT);
+    }
+
 }

Added: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/nodetype/MixinTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/nodetype/MixinTest.java?rev=1475688&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/nodetype/MixinTest.java (added)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/nodetype/MixinTest.java Thu Apr 25 10:20:15 2013
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.jcr.nodetype;
+
+import javax.jcr.Node;
+import javax.jcr.nodetype.NoSuchNodeTypeException;
+
+import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.api.JackrabbitSession;
+import org.apache.jackrabbit.api.security.user.Authorizable;
+import org.apache.jackrabbit.test.AbstractJCRTest;
+import org.apache.jackrabbit.test.NotExecutableException;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class MixinTest extends AbstractJCRTest {
+
+    @Override
+    @Before()
+    public void setUp() throws Exception {
+        super.setUp();
+    }
+
+    @Override
+    @After()
+    public void tearDown() throws Exception {
+        super.tearDown();
+    }
+
+    @Test
+    public void testRemoveMixinWithoutMixinProperty() throws Exception {
+        try {
+            assertFalse(testRootNode.hasProperty(JcrConstants.JCR_MIXINTYPES));
+            testRootNode.removeMixin(JcrConstants.MIX_REFERENCEABLE);
+            fail();
+        } catch (NoSuchNodeTypeException e) {
+            // success
+        }
+
+    }
+
+    @Test
+    public void testRemoveInheritedMixin() throws Exception {
+        testRootNode.addMixin(JcrConstants.MIX_VERSIONABLE);
+        superuser.save();
+
+        try {
+            testRootNode.removeMixin(JcrConstants.MIX_REFERENCEABLE);
+            fail();
+        } catch (NoSuchNodeTypeException e) {
+            // success
+        }
+    }
+
+    @Test
+    public void testRemoveInheritedMixin2() throws Exception {
+        try {
+            Authorizable user = ((JackrabbitSession) superuser).getUserManager().getAuthorizable("admin");
+            if (user == null) {
+                throw new NotExecutableException();
+            }
+
+            Node node = superuser.getNode(user.getPath());
+            assertTrue(node.isNodeType(JcrConstants.MIX_REFERENCEABLE));
+            node.removeMixin(JcrConstants.MIX_REFERENCEABLE);
+        }  catch (NoSuchNodeTypeException e) {
+            // success
+        } finally {
+            superuser.refresh(false);
+        }
+    }
+
+}
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java?rev=1475688&r1=1475687&r2=1475688&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/NodeTypeManagementTest.java Thu Apr 25 10:20:15 2013
@@ -32,7 +32,6 @@ import org.junit.Test;
 /**
  * Permission evaluation tests related to {@link Privilege#JCR_NODE_TYPE_MANAGEMENT} privilege.
  */
-@Ignore("OAK-711 : permission validator doesn't detect changes to mixin/primary type")
 public class NodeTypeManagementTest extends AbstractEvaluationTest {
 
     private Node childNode;
@@ -68,37 +67,49 @@ public class NodeTypeManagementTest exte
     }
 
     @Test
-    public void testAddMixin() throws Exception {
+    public void testAddMixinWithoutPermission() throws Exception {
         try {
             childNode.addMixin(mixinName);
-            superuser.save();
+            testSession.save();
             fail("TestSession does not have sufficient privileges to add a mixin type.");
         } catch (AccessDeniedException e) {
             // success
         }
+    }
 
+    @Test
+    public void testAddMixin() throws Exception {
         modify(childNode.getPath(), Privilege.JCR_NODE_TYPE_MANAGEMENT, true);
         childNode.addMixin(mixinName);
-        superuser.save();
+        testSession.save();
     }
 
     @Ignore("OAK-767 : Implement Node#removeMixin")
     @Test
-    public void testRemoveMixin() throws Exception {
+    public void testRemoveMixinWithoutPermission() throws Exception {
         ((Node) superuser.getItem(childNode.getPath())).addMixin(mixinName);
         superuser.save();
+        testSession.refresh(false);
 
         try {
             childNode.removeMixin(mixinName);
-            superuser.save();
+            testSession.save();
             fail("TestSession does not have sufficient privileges to remove a mixin type.");
         } catch (AccessDeniedException e) {
             // success
         }
+    }
+
+    @Ignore("OAK-767 : Implement Node#removeMixin")
+    @Test
+    public void testRemoveMixin() throws Exception {
+        ((Node) superuser.getItem(childNode.getPath())).addMixin(mixinName);
+        superuser.save();
+        testSession.refresh(false);
 
         modify(childNode.getPath(), Privilege.JCR_NODE_TYPE_MANAGEMENT, true);
         childNode.removeMixin(mixinName);
-        superuser.save();
+        testSession.save();
     }
 
     @Test
@@ -108,7 +119,7 @@ public class NodeTypeManagementTest exte
 
         try {
             childNode.setPrimaryType("nt:folder");
-            superuser.save();
+            testSession.save();
             fail("TestSession does not have sufficient privileges to change the primary type.");
         } catch (AccessDeniedException e) {
             // success
@@ -128,11 +139,11 @@ public class NodeTypeManagementTest exte
 
         String changedNtName = "nt:folder";
         child.setPrimaryType(changedNtName);
-        superuser.save();
+        testSession.save();
 
         modify(childNode.getPath(), Privilege.JCR_NODE_TYPE_MANAGEMENT, true);
         childNode.setPrimaryType(ntName);
-        superuser.save();
+        testSession.save();
     }
 
     /**
@@ -172,6 +183,7 @@ public class NodeTypeManagementTest exte
         }
     }
 
+    @Ignore("OAK-711") // FIXME
     @Test
     public void testCopy() throws Exception {
         Workspace wsp = testSession.getWorkspace();
@@ -200,6 +212,7 @@ public class NodeTypeManagementTest exte
         wsp.copy(srcPath, destPath);
     }
 
+    @Ignore("OAK-711") // FIXME
     @Test
     public void testWorkspaceMove() throws Exception {
         Workspace wsp = testSession.getWorkspace();
@@ -228,6 +241,7 @@ public class NodeTypeManagementTest exte
         wsp.move(srcPath, destPath);
     }
 
+    @Ignore("OAK-711") // FIXME
     @Test
     public void testSessionMove() throws Exception {
         String parentPath = childNode.getParent().getPath();