Author: angela
Date: Wed Oct 21 08:27:02 2015
New Revision: 1709750

URL: http://svn.apache.org/viewvc?rev=1709750&view=rev
Log:
OAK-3530 : TreeTypeProvider returns wrong type for version related node type 
definitions

Added:
    
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProviderTest.java
   (with props)
Modified:
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProvider.java

Modified: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProvider.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProvider.java?rev=1709750&r1=1709749&r2=1709750&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProvider.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProvider.java
 Wed Oct 21 08:27:02 2015
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.securi
 
 import javax.annotation.Nonnull;
 
+import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.plugins.version.VersionConstants;
 import org.apache.jackrabbit.oak.spi.security.Context;
@@ -26,21 +27,21 @@ import org.apache.jackrabbit.oak.spi.sta
 
 /**
  * <h3>TreeTypeProvider</h3>
-  * For optimization purpose an Immutable tree will be associated with a
-  * {@code TreeTypeProvider} that allows for fast detection of the following 
types
-  * of Trees:
-  *
-  * <ul>
-  *     <li>{@link #TYPE_HIDDEN}: a hidden tree whose name starts with ":".
-  *     Please note that the whole subtree of a hidden node is considered 
hidden.</li>
-  *     <li>{@link #TYPE_AC}: A tree that stores access control content
-  *     and requires special access {@link 
org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions#READ_ACCESS_CONTROL
 permissions}.</li>
-  *     <li>{@link #TYPE_VERSION}: if a given tree is located within
-  *     any of the version related stores defined by JSR 283. Depending on the
-  *     permission evaluation implementation those items require special 
treatment.</li>
-  *     <li>{@link #TYPE_DEFAULT}: the default type for trees that don't
-  *     match any of the upper types.</li>
-  * </ul>
+ * Allows to distinguish different types of trees based on their name, ancestry
+ * or primary type. Currently the following types are supported:
+ *
+ * <ul>
+ *     <li>{@link #TYPE_HIDDEN}: a hidden tree whose name starts with ":".
+ *     Please note that the whole subtree of a hidden node is considered 
hidden.</li>
+ *     <li>{@link #TYPE_AC}: A tree that stores access control content
+ *     and requires special access {@link 
org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions#READ_ACCESS_CONTROL
 permissions}.</li>
+ *     <li>{@link #TYPE_VERSION}: if a given tree is located within
+ *     any of the version related stores defined by JSR 283. Depending on the
+ *     permission evaluation implementation those items require special 
treatment.</li>
+ *     <li>{@link #TYPE_INTERNAL}: repository internal content that is not 
hidden (e.g. permission store)</li>
+ *     <li>{@link #TYPE_DEFAULT}: the default type for trees that don't
+ *     match any of the upper types.</li>
+ * </ul>
  */
 public final class TreeTypeProvider {
 
@@ -55,53 +56,67 @@ public final class TreeTypeProvider {
     // hidden trees
     public static final int TYPE_HIDDEN = 16;
 
-    private final Context contextInfo;
+    private final Context authorizationContext;
 
-    public TreeTypeProvider(@Nonnull Context contextInfo) {
-        this.contextInfo = contextInfo;
+    public TreeTypeProvider(@Nonnull Context authorizationContext) {
+        this.authorizationContext = authorizationContext;
     }
 
-    public int getType(Tree tree) {
+    public int getType(@Nonnull Tree tree) {
         if (tree.isRoot()) {
             return TYPE_DEFAULT;
         } else {
-            return getType(tree, getType(tree.getParent()));
+            Tree t = tree;
+            while (!t.isRoot()) {
+                int type = getType(t.getName(), t);
+                // stop walking up the hierarchy as soon as a special type is 
found
+                if (TYPE_DEFAULT != type) {
+                    return type;
+                }
+                t = t.getParent();
+            }
+            return TYPE_DEFAULT;
         }
     }
 
-    public int getType(Tree tree, int parentType) {
-            if (tree.isRoot()) {
-                return TYPE_DEFAULT;
-            }
+    public int getType(@Nonnull Tree tree, int parentType) {
+        if (tree.isRoot()) {
+            return TYPE_DEFAULT;
+        }
 
-            int type;
-            switch (parentType) {
-                case TYPE_HIDDEN:
-                    type = TYPE_HIDDEN;
-                    break;
-                case TYPE_VERSION:
-                    type = TYPE_VERSION;
-                    break;
-                case TYPE_INTERNAL:
-                    type = TYPE_INTERNAL;
-                    break;
-                case TYPE_AC:
-                    type = TYPE_AC;
-                    break;
-                default:
-                    String name = tree.getName();
-                    if (NodeStateUtils.isHidden(name)) {
-                        type = TYPE_HIDDEN;
-                    } else if 
(VersionConstants.VERSION_STORE_ROOT_NAMES.contains(name)) {
-                        type = TYPE_VERSION;
-                    } else if 
(PermissionConstants.REP_PERMISSION_STORE.equals(name)) {
-                        type = TYPE_INTERNAL;
-                    } else if (contextInfo.definesContextRoot(tree)) {
-                        type = TYPE_AC;
-                    } else {
-                        type = TYPE_DEFAULT;
-                    }
-            }
-            return type;
+        int type;
+        switch (parentType) {
+            case TYPE_HIDDEN:
+                type = TYPE_HIDDEN;
+                break;
+            case TYPE_VERSION:
+                type = TYPE_VERSION;
+                break;
+            case TYPE_INTERNAL:
+                type = TYPE_INTERNAL;
+                break;
+            case TYPE_AC:
+                type = TYPE_AC;
+                break;
+            default:
+                type = getType(tree.getName(), tree);
+        }
+        return type;
+    }
+
+    private int getType(@Nonnull String name, @Nonnull Tree tree) {
+        int type;
+        if (NodeStateUtils.isHidden(name)) {
+            type = TYPE_HIDDEN;
+        } else if (VersionConstants.VERSION_STORE_ROOT_NAMES.contains(name)) {
+            type = 
(JcrConstants.JCR_SYSTEM.equals(tree.getParent().getName())) ?  TYPE_VERSION : 
TYPE_DEFAULT;
+        } else if (PermissionConstants.REP_PERMISSION_STORE.equals(name)) {
+            type = TYPE_INTERNAL;
+        } else if (authorizationContext.definesContextRoot(tree)) {
+            type = TYPE_AC;
+        } else {
+            type = TYPE_DEFAULT;
         }
+        return type;
+    }
 }

Added: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProviderTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProviderTest.java?rev=1709750&view=auto
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProviderTest.java
 (added)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProviderTest.java
 Wed Oct 21 08:27:02 2015
@@ -0,0 +1,151 @@
+/*
+ * 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.security.authorization.permission;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nonnull;
+
+import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants;
+import org.apache.jackrabbit.oak.plugins.version.VersionConstants;
+import 
org.apache.jackrabbit.oak.spi.security.authorization.AuthorizationConfiguration;
+import 
org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
+import 
org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionConstants;
+import org.apache.jackrabbit.oak.util.NodeUtil;
+import org.junit.Test;
+
+import static 
org.apache.jackrabbit.oak.security.authorization.permission.TreeTypeProvider.TYPE_AC;
+import static 
org.apache.jackrabbit.oak.security.authorization.permission.TreeTypeProvider.TYPE_DEFAULT;
+import static 
org.apache.jackrabbit.oak.security.authorization.permission.TreeTypeProvider.TYPE_HIDDEN;
+import static 
org.apache.jackrabbit.oak.security.authorization.permission.TreeTypeProvider.TYPE_INTERNAL;
+import static 
org.apache.jackrabbit.oak.security.authorization.permission.TreeTypeProvider.TYPE_VERSION;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+public class TreeTypeProviderTest extends AbstractSecurityTest {
+
+    private TreeTypeProvider typeProvider;
+    private List<TreeType> treeTypes;
+
+    @Override
+    public void before() throws Exception {
+        super.before();
+
+        typeProvider = new 
TreeTypeProvider(getConfig(AuthorizationConfiguration.class).getContext());
+
+        treeTypes = new ArrayList<TreeType>();
+        treeTypes.add(new TreeType("/", TYPE_DEFAULT));
+        treeTypes.add(new TreeType("/content", TYPE_DEFAULT));
+        treeTypes.add(new TreeType('/' + JcrConstants.JCR_SYSTEM, 
TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH, 
TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:system/rep:namedChildNodeDefinitions/jcr:versionStorage", TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:system/rep:namedChildNodeDefinitions/jcr:activities", TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:system/rep:namedChildNodeDefinitions/jcr:configurations", TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:AccessControllable/rep:namedChildNodeDefinitions/rep:policy", 
TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:AccessControllable/rep:namedChildNodeDefinitions/rep:policy/rep:Policy", 
TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:ACL/rep:residualChildNodeDefinitions/rep:ACE", TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:GrantACE/rep:namedChildNodeDefinitions/rep:restrictions", TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:RepoAccessControllable/rep:namedChildNodeDefinitions/rep:repoPolicy", 
TYPE_DEFAULT));
+        treeTypes.add(new TreeType(NodeTypeConstants.NODE_TYPES_PATH + 
"/rep:PermissionStore", TYPE_DEFAULT));
+
+        treeTypes.add(new TreeType("/:hidden", TYPE_HIDDEN));
+        treeTypes.add(new TreeType("/:hidden/child", TYPE_HIDDEN, 
TYPE_HIDDEN));
+
+        treeTypes.add(new TreeType("/oak:index/nodetype/:index", TYPE_HIDDEN));
+        treeTypes.add(new TreeType("/oak:index/nodetype/:index/child", 
TYPE_HIDDEN, TYPE_HIDDEN));
+
+        for (String versionPath : VersionConstants.SYSTEM_PATHS) {
+            treeTypes.add(new TreeType(versionPath, TYPE_VERSION));
+            treeTypes.add(new TreeType(versionPath + "/a/b/child", 
TYPE_VERSION, TYPE_VERSION));
+        }
+
+        treeTypes.add(new TreeType(PermissionConstants.PERMISSIONS_STORE_PATH, 
TYPE_INTERNAL));
+        treeTypes.add(new TreeType(PermissionConstants.PERMISSIONS_STORE_PATH 
+ "/a/b/child", TYPE_INTERNAL, TYPE_INTERNAL));
+
+        NodeUtil testTree = new NodeUtil(root.getTree("/")).addChild("test", 
NodeTypeConstants.NT_OAK_UNSTRUCTURED);
+        for (String name : AccessControlConstants.POLICY_NODE_NAMES) {
+            NodeUtil acl = testTree.addChild(name, 
AccessControlConstants.NT_REP_ACL);
+            treeTypes.add(new TreeType(acl.getTree().getPath(), TYPE_AC));
+
+            NodeUtil ace = acl.addChild("ace", 
AccessControlConstants.NT_REP_DENY_ACE);
+            treeTypes.add(new TreeType(ace.getTree().getPath(), TYPE_AC, 
TYPE_AC));
+
+            NodeUtil ace2 = acl.addChild("ace2", 
AccessControlConstants.NT_REP_GRANT_ACE);
+            treeTypes.add(new TreeType(ace2.getTree().getPath(), TYPE_AC, 
TYPE_AC));
+
+            NodeUtil rest = 
ace2.addChild(AccessControlConstants.REP_RESTRICTIONS, 
AccessControlConstants.NT_REP_RESTRICTIONS);
+            treeTypes.add(new TreeType(rest.getTree().getPath(), TYPE_AC, 
TYPE_AC));
+
+            NodeUtil invalid = rest.addChild("invalid", 
NodeTypeConstants.NT_OAK_UNSTRUCTURED);
+            treeTypes.add(new TreeType(invalid.getTree().getPath(), TYPE_AC, 
TYPE_AC));
+        }
+    }
+
+    @Override
+    public void after() throws Exception {
+        try {
+            root.refresh();
+        } finally {
+            super.after();
+        }
+    }
+
+    @Test
+    public void testGetType() {
+        for (TreeType treeType : treeTypes) {
+            assertEquals(treeType.path, treeType.type, 
typeProvider.getType(root.getTree(treeType.path)));
+        }
+    }
+
+    @Test
+    public void testGetTypeWithParentType() {
+        for (TreeType treeType : treeTypes) {
+            assertEquals(treeType.path, treeType.type, 
typeProvider.getType(root.getTree(treeType.path), treeType.parentType));
+        }
+    }
+
+    @Test
+    public void testGetTypeWithDefaultParentType() {
+        for (TreeType treeType : treeTypes) {
+            int typeIfParentDefault = 
typeProvider.getType(root.getTree(treeType.path), TYPE_DEFAULT);
+
+            if (TYPE_DEFAULT == treeType.parentType) {
+                assertEquals(treeType.path, treeType.type, 
typeIfParentDefault);
+            } else {
+                assertNotEquals(treeType.path, treeType.type, 
typeIfParentDefault);
+            }
+        }
+    }
+    
+    private static final class TreeType {
+
+        private final String path;
+        private final int type;
+        private final int parentType;
+
+        private TreeType(@Nonnull String path, int type) {
+            this(path, type, TreeTypeProvider.TYPE_DEFAULT);
+        }
+        private TreeType(@Nonnull String path, int type, int parentType) {
+            this.path = path;
+            this.type = type;
+            this.parentType = parentType;
+        }
+    }
+}
\ No newline at end of file

Propchange: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/permission/TreeTypeProviderTest.java
------------------------------------------------------------------------------
    svn:eol-style = native


Reply via email to