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 2019/06/06 17:20:27 UTC

svn commit: r1860730 [3/3] - in /jackrabbit/oak/trunk/oak-core: ./ src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/ src/test/java/org/apache/jackrabbit/oak/ src/test/java/org/apache/jackrabbit/oak/security/authorization/acc...

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerLimitedPermissionsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerLimitedPermissionsTest.java?rev=1860730&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerLimitedPermissionsTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlManagerLimitedPermissionsTest.java Thu Jun  6 17:20:27 2019
@@ -0,0 +1,531 @@
+/*
+ * 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.accesscontrol;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
+import org.apache.jackrabbit.api.security.principal.PrincipalManager;
+import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.namepath.NameMapper;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.namepath.impl.GlobalNameMapper;
+import org.apache.jackrabbit.oak.namepath.impl.NamePathMapperImpl;
+import org.apache.jackrabbit.oak.plugins.name.ReadWriteNamespaceRegistry;
+import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
+import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.jcr.AccessDeniedException;
+import javax.jcr.NamespaceRegistry;
+import javax.jcr.PathNotFoundException;
+import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.security.AccessControlManager;
+import javax.jcr.security.AccessControlPolicy;
+import javax.jcr.security.AccessControlPolicyIterator;
+import javax.jcr.security.Privilege;
+import java.security.Principal;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.REP_POLICY;
+import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.REP_RESTRICTIONS;
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class AccessControlManagerLimitedPermissionsTest extends AbstractSecurityTest {
+
+    private static final String TEST_PREFIX = "jr";
+    private static final String TEST_URI = "http://jackrabbit.apache.org";
+
+    private static final String TEST_NAME = TEST_PREFIX + ":testRoot";
+    private String testPath;
+    private String childPath;
+
+    private NamePathMapper npMapper;
+
+    private Root testRoot;
+    private AccessControlManagerImpl testAcMgr;
+
+    private Privilege[] testPrivileges;
+    private Principal testPrincipal;
+
+    @Before
+    public void before() throws Exception {
+        super.before();
+
+        NamespaceRegistry nsRegistry = new ReadWriteNamespaceRegistry(root) {
+            @Override
+            protected Root getWriteRoot() {
+                return root;
+            }
+        };
+        nsRegistry.registerNamespace(TEST_PREFIX, TEST_URI);
+
+        NameMapper nameMapper = new GlobalNameMapper(root);
+        npMapper = new NamePathMapperImpl(nameMapper);
+
+        Tree testTree = TreeUtil.addChild(root.getTree(PathUtils.ROOT_PATH), TEST_NAME, JcrConstants.NT_UNSTRUCTURED);
+        testPath = testTree.getPath();
+        Tree child = TreeUtil.addChild(testTree, "child", JcrConstants.NT_UNSTRUCTURED);
+        childPath = child.getPath();
+        root.commit();
+
+        testRoot = createTestSession().getLatestRoot();
+        testAcMgr = new AccessControlManagerImpl(testRoot, getNamePathMapper(), getSecurityProvider());
+        testPrivileges = privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES, PrivilegeConstants.JCR_READ);
+        testPrincipal = getTestUser().getPrincipal();    }
+
+    @After
+    public void after() throws Exception {
+        try {
+            testRoot.getContentSession().close();
+        } finally {
+            super.after();
+        }
+    }
+
+    @Override
+    protected NamePathMapper getNamePathMapper() {
+        return npMapper;
+    }
+
+    private static void assertPolicies(@Nullable AccessControlPolicy[] policies, long expectedSize) {
+        assertNotNull(policies);
+        assertEquals(expectedSize, policies.length);
+    }
+
+    @NotNull
+    private ACL setupPolicy(@Nullable String path, @Nullable Privilege... privileges) throws RepositoryException {
+        Privilege[] privs = (privileges == null || privileges.length == 0) ? testPrivileges : privileges;
+        return TestUtility.setupPolicy(getAccessControlManager(root), path, testPrincipal, privs, true, TestUtility.getGlobRestriction("*", getValueFactory(root)), null);
+    }
+
+    @NotNull
+    private List<String> getAcContentPaths() throws RepositoryException {
+        AccessControlManager acMgr = getAccessControlManager(root);
+        ACL policy = TestUtility.getApplicablePolicy(acMgr, testPath);
+        policy.addEntry(testPrincipal, testPrivileges, true, TestUtility.getGlobRestriction("*", getValueFactory(root)));
+        acMgr.setPolicy(testPath, policy);
+
+        String aclPath = testPath + '/' + REP_POLICY;
+        Tree acl = root.getTree(aclPath);
+        assertTrue(acl.exists());
+        Iterator<Tree> aces = acl.getChildren().iterator();
+        assertTrue(aces.hasNext());
+        Tree ace = aces.next();
+        assertNotNull(ace);
+
+        List<String> acContentPath = new ArrayList<>();
+        acContentPath.add(aclPath);
+        acContentPath.add(ace.getPath());
+
+        Tree rest = ace.getChild(REP_RESTRICTIONS);
+        if (rest.exists()) {
+            acContentPath.add(rest.getPath());
+        }
+        return acContentPath;
+    }
+
+    /**
+     * @since OAK 1.0 As of OAK AccessControlManager#hasPrivilege will throw
+     * PathNotFoundException in case the node associated with a given path is
+     * not readable to the editing session (compatibility with the specification
+     * which was missing in jackrabbit).
+     */
+    @Test
+    public void testHasPrivilegesNotAccessiblePath() throws Exception {
+        List<String> notAccessible = new ArrayList<>();
+        notAccessible.add(PathUtils.ROOT_PATH);
+        notAccessible.addAll(getAcContentPaths());
+
+        Privilege[] privs = privilegesFromNames(PrivilegeConstants.JCR_ALL);
+        for (String path : notAccessible) {
+            try {
+                testAcMgr.hasPrivileges(path, privs);
+                fail("AccessControlManager#hasPrivileges for node that is not accessible should fail.");
+            } catch (PathNotFoundException e) {
+                // success
+            }
+        }
+        for (String path : notAccessible) {
+            try {
+                testAcMgr.hasPrivileges(path, getPrincipals(root.getContentSession()), privs);
+                fail("AccessControlManager#hasPrivileges for node that is not accessible should fail.");
+            } catch (PathNotFoundException e) {
+                // success
+            }
+        }
+        for (String path : notAccessible) {
+            try {
+                testAcMgr.hasPrivileges(path, getPrincipals(testRoot.getContentSession()), privs);
+                fail("AccessControlManager#hasPrivileges for node that is not accessible should fail.");
+            } catch (PathNotFoundException e) {
+                // success
+            }
+        }
+        for (String path : notAccessible) {
+            try {
+                testAcMgr.hasPrivileges(path, ImmutableSet.of(), privs);
+                fail("AccessControlManager#hasPrivileges for node that is not accessible should fail.");
+            } catch (PathNotFoundException e) {
+                // success
+            }
+        }
+    }
+
+    @Test
+    public void testTestSessionHasRepoPrivileges() throws Exception {
+        assertFalse(testAcMgr.hasPrivileges(null, testPrivileges));
+        assertFalse(testAcMgr.hasPrivileges(null, getPrincipals(testRoot.getContentSession()), testPrivileges));
+    }
+
+    @Test
+    public void testHasRepoPrivilegesNoAccessToPrincipals() throws Exception {
+        // the test-session doesn't have sufficient permissions to read privilege set for admin session.
+        try {
+            testAcMgr.getPrivileges(null, getPrincipals(adminSession));
+            fail("testSession doesn't have sufficient permission to read access control information");
+        } catch (AccessDeniedException e) {
+            // success
+        }
+    }
+
+    @Test(expected = AccessDeniedException.class)
+    public void testHasRepoPrivilegesForEmptyPrincipalSet() throws Exception {
+        // the test-session doesn't have sufficient permissions to read privilege set.
+        testAcMgr.getPrivileges(null, Collections.emptySet());
+    }
+
+    @Test
+    public void testHasPrivileges() throws Exception {
+        setupPolicy(testPath);
+        root.commit();
+
+        testRoot.refresh();
+
+        // granted privileges
+        List<Privilege[]> granted = new ArrayList<>();
+        granted.add(privilegesFromNames(PrivilegeConstants.JCR_READ));
+        granted.add(privilegesFromNames(PrivilegeConstants.REP_READ_NODES));
+        granted.add(privilegesFromNames(PrivilegeConstants.REP_READ_PROPERTIES));
+        granted.add(privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES));
+        granted.add(testPrivileges);
+
+        for (Privilege[] privileges : granted) {
+            assertTrue(testAcMgr.hasPrivileges(testPath, privileges));
+            assertTrue(testAcMgr.hasPrivileges(testPath, getPrincipals(testRoot.getContentSession()), privileges));
+        }
+
+        // denied privileges
+        List<Privilege[]> denied = new ArrayList<>();
+        denied.add(privilegesFromNames(PrivilegeConstants.JCR_ALL));
+        denied.add(privilegesFromNames(PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
+        denied.add(privilegesFromNames(PrivilegeConstants.JCR_WRITE));
+        denied.add(privilegesFromNames(PrivilegeConstants.JCR_LOCK_MANAGEMENT));
+
+        for (Privilege[] privileges : denied) {
+            assertFalse(testAcMgr.hasPrivileges(testPath, privileges));
+            assertFalse(testAcMgr.hasPrivileges(testPath, getPrincipals(testRoot.getContentSession()), privileges));
+        }
+    }
+
+    @Test(expected = AccessDeniedException.class)
+    public void testHasPrivilegesForPrincipals() throws Exception {
+        setupPolicy(testPath);
+        root.commit();
+
+        testRoot.refresh();
+
+        // but for 'admin' the test-session doesn't have sufficient privileges
+        testAcMgr.getPrivileges(testPath, getPrincipals(adminSession));
+    }
+
+    /**
+     * @since OAK 1.0 As of OAK AccessControlManager#hasPrivilege will throw
+     * PathNotFoundException in case the node associated with a given path is
+     * not readable to the editing session.
+     */
+    @Test
+    public void testGetPrivilegesNotAccessiblePath() throws Exception {
+        List<String> notAccessible = new ArrayList<>();
+        notAccessible.add("/");
+        notAccessible.addAll(getAcContentPaths());
+
+        for (String path : notAccessible) {
+            try {
+                testAcMgr.getPrivileges(path);
+                fail("AccessControlManager#getPrivileges for node that is not accessible should fail.");
+            } catch (PathNotFoundException e) {
+                // success
+            }
+        }
+
+        for (String path : notAccessible) {
+            try {
+                testAcMgr.getPrivileges(path, getPrincipals(adminSession));
+                fail("AccessControlManager#getPrivileges for node that is not accessible should fail.");
+            } catch (PathNotFoundException e) {
+                // success
+            }
+        }
+
+        for (String path : notAccessible) {
+            try {
+                testAcMgr.getPrivileges(path, Collections.singleton(testPrincipal));
+                fail("AccessControlManager#getPrivileges for node that is not accessible should fail.");
+            } catch (PathNotFoundException e) {
+                // success
+            }
+        }
+
+    }
+
+    @Test
+    public void testGetPrivileges() throws Exception {
+        setupPolicy(testPath);
+        root.commit();
+
+        testRoot.refresh();
+
+        Set<Principal> testPrincipals = getPrincipals(testRoot.getContentSession());
+
+        assertArrayEquals(new Privilege[0], testAcMgr.getPrivileges(null));
+        assertArrayEquals(new Privilege[0], testAcMgr.getPrivileges(null, testPrincipals));
+
+        Privilege[] privs = testAcMgr.getPrivileges(testPath);
+        assertEquals(ImmutableSet.copyOf(testPrivileges), ImmutableSet.copyOf(privs));
+
+        privs = testAcMgr.getPrivileges(testPath, testPrincipals);
+        assertEquals(ImmutableSet.copyOf(testPrivileges), ImmutableSet.copyOf(privs));
+
+        // but for 'admin' the test-session doesn't have sufficient privileges
+        try {
+            testAcMgr.getPrivileges(testPath, getPrincipals(adminSession));
+            fail("testSession doesn't have sufficient permission to read access control information at testPath");
+        } catch (AccessDeniedException e) {
+            // success
+        }
+    }
+
+    @Test
+    public void testGetApplicablePolicies() throws Exception {
+        setupPolicy(testPath);
+        root.commit();
+
+        testRoot.refresh();
+        List<Principal> principals = ImmutableList.of(testPrincipal, EveryonePrincipal.getInstance());
+        for (Principal principal : principals) {
+            // testRoot can't read access control content -> doesn't see
+            // the existing policies and creates a new applicable policy.
+            AccessControlPolicy[] applicable = testAcMgr.getApplicablePolicies(principal);
+            assertPolicies(applicable, 1);
+            assertTrue(applicable[0] instanceof ACL);
+        }
+    }
+
+
+
+    @Test
+    public void testGetPolicies() throws Exception {
+        setupPolicy(testPath);
+        root.commit();
+
+        testRoot.refresh();
+        PrincipalManager testPrincipalMgr = getPrincipalManager(testRoot);
+
+        List<Principal> principals = ImmutableList.of(testPrincipal, EveryonePrincipal.getInstance());
+        for (Principal principal : principals) {
+            if (testPrincipalMgr.hasPrincipal(principal.getName())) {
+                // testRoot can't read access control content -> doesn't see
+                // the existing policies and creates a new applicable policy.
+                AccessControlPolicy[] policies = testAcMgr.getPolicies(principal);
+                assertPolicies(policies, 0);
+            } else {
+                // testRoot can't read principal -> no policies for that principal
+                assertPolicies(testAcMgr.getPolicies(principal), 0);
+            }
+        }
+    }
+
+    /**
+     * @since OAK 1.0
+     */
+    @Test
+    public void testGetEffectivePolicies() throws Exception {
+        // grant 'testUser' READ + READ_AC privileges at 'path'
+        Privilege[] privileges = privilegesFromNames(PrivilegeConstants.JCR_READ, PrivilegeConstants.JCR_READ_ACCESS_CONTROL);
+        setupPolicy(testPath, privileges);
+        root.commit();
+
+        testRoot.refresh();
+        assertTrue(testAcMgr.hasPrivileges(testPath, privileges));
+
+        // diff to jr core: getEffectivePolicies will just return the policies
+        // accessible for the editing session but not throw an exception.
+        AccessControlPolicy[] effective = testAcMgr.getEffectivePolicies(testPath);
+        assertPolicies(effective, 1);
+    }
+
+    /**
+     * @since OAK 1.0
+     */
+    @Test
+    public void testGetEffectivePolicies2() throws Exception {
+        setupPolicy(testPath, privilegesFromNames(PrivilegeConstants.JCR_READ));
+        setupPolicy(childPath, privilegesFromNames(PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
+        root.commit();
+
+        testRoot.refresh();
+        assertTrue(testAcMgr.hasPrivileges(childPath, privilegesFromNames(PrivilegeConstants.JCR_READ, PrivilegeConstants.JCR_READ_ACCESS_CONTROL)));
+
+        // diff to jr core: getEffectivePolicies will just return the policies
+        // accessible for the editing session but not throw an exception.
+        AccessControlPolicy[] effective = testAcMgr.getEffectivePolicies(childPath);
+        assertPolicies(effective, 1);
+    }
+
+    @Test
+    public void testGetEffectivePoliciesWithoutPrivilege() throws Exception {
+        // grant 'testUser' READ + READ_AC privileges at 'path'
+        Privilege[] privileges = privilegesFromNames(PrivilegeConstants.JCR_READ);
+        setupPolicy(testPath, privileges);
+        root.commit();
+
+        testRoot.refresh();
+
+        List<String> paths = ImmutableList.of(testPath, NodeTypeConstants.NODE_TYPES_PATH);
+        for (String path : paths) {
+            assertFalse(testAcMgr.hasPrivileges(path, privilegesFromNames(PrivilegeConstants.JCR_READ_ACCESS_CONTROL)));
+            try {
+                testAcMgr.getEffectivePolicies(path);
+                fail("READ_ACCESS_CONTROL is not granted at " + path);
+            } catch (AccessDeniedException e) {
+                // success
+            }
+        }
+    }
+
+    @Test
+    public void testGetEffectivePoliciesByPrincipal() throws Exception {
+        Privilege[] privs = privilegesFromNames(PrivilegeConstants.JCR_READ, PrivilegeConstants.JCR_READ_ACCESS_CONTROL);
+        setupPolicy(testPath, privs);
+        setupPolicy(childPath, privs);
+        root.commit();
+
+        testRoot.refresh();
+        AccessControlPolicy[] effective = testAcMgr.getEffectivePolicies(Collections.singleton(testPrincipal));
+        assertPolicies(effective, 2);
+    }
+
+    /**
+     * @since OAK 1.0 Policy at testPath not accessible -> getEffectivePolicies
+     * only returns the readable policy but doesn't fail.
+     */
+    @Test
+    public void testGetEffectivePoliciesByPrincipal2() throws Exception {
+        // policy at testPath: ac content was visible but the policy can't be
+        // retrieved from AcMgr as the accesscontrolled node is not visible.
+        setupPolicy(testPath, privilegesFromNames(PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
+        // policy at childPath: will be found by the getEffectivePolicies
+        setupPolicy(childPath, privilegesFromNames(PrivilegeConstants.JCR_READ, PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
+        root.commit();
+
+        testRoot.refresh();
+
+        AccessControlPolicy[] effective = testAcMgr.getEffectivePolicies(Collections.singleton(testPrincipal));
+        assertPolicies(effective, 1);
+    }
+
+    /**
+     * @since OAK 1.0 Policy at testPath not accessible -> getEffectivePolicies
+     * only returns the readable policy but doesn't fail.
+     */
+    @Test
+    public void testGetEffectivePoliciesByPrincipal3() throws Exception {
+        setupPolicy(testPath, privilegesFromNames(PrivilegeConstants.JCR_READ));
+        setupPolicy(childPath, privilegesFromNames(PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
+        root.commit();
+
+        testRoot.refresh();
+
+        AccessControlPolicy[] effective = testAcMgr.getEffectivePolicies(Collections.singleton(testPrincipal));
+        assertPolicies(effective, 1);
+    }
+
+    @Test
+    public void testGetEffectivePoliciesByPrincipals() throws Exception {
+        Privilege[] privs = privilegesFromNames(PrivilegeConstants.JCR_READ, PrivilegeConstants.JCR_READ_ACCESS_CONTROL);
+        setupPolicy(testPath, privilegesFromNames(PrivilegeConstants.JCR_READ, PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
+
+        AccessControlManager acMgr = getAccessControlManager(root);
+        JackrabbitAccessControlList acl = TestUtility.getApplicablePolicy(acMgr, childPath);
+        acl.addEntry(EveryonePrincipal.getInstance(), privs, true);
+        acMgr.setPolicy(childPath, acl);
+
+        root.commit();
+
+        testRoot.refresh();
+        Set<Principal> principals = ImmutableSet.of(testPrincipal, EveryonePrincipal.getInstance());
+        AccessControlPolicy[] policies = testAcMgr.getEffectivePolicies(principals);
+        assertPolicies(policies, 2);
+    }
+
+    /**
+     * @since OAK 1.0 : only accessible policies are returned but not exception
+     * is raised.
+     */
+    @Test
+    public void testGetEffectivePoliciesByPrincipals2() throws Exception {
+        Privilege[] privs = privilegesFromNames(PrivilegeConstants.JCR_READ, PrivilegeConstants.JCR_READ_ACCESS_CONTROL);
+
+        // create policy on testPath -> but deny access to test session
+        AccessControlManager acMgr = getAccessControlManager(root);
+        JackrabbitAccessControlList acl = TestUtility.getApplicablePolicy(acMgr, testPath);
+        acl.addEntry(testPrincipal, privs, false);
+        acMgr.setPolicy(testPath, acl);
+
+        // grant access at childpath
+        setupPolicy(childPath, privilegesFromNames(PrivilegeConstants.JCR_READ, PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
+        root.commit();
+
+        testRoot.refresh();
+
+        Set<Principal> principals = ImmutableSet.of(testPrincipal, EveryonePrincipal.getInstance());
+        AccessControlPolicy[] policies = testAcMgr.getEffectivePolicies(principals);
+        assertPolicies(policies, 1);
+    }
+}

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

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java?rev=1860730&r1=1860729&r2=1860730&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java Thu Jun  6 17:20:27 2019
@@ -25,12 +25,16 @@ import org.apache.jackrabbit.api.securit
 import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.api.Root;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
+import org.apache.jackrabbit.oak.plugins.tree.TreeProvider;
 import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.apache.jackrabbit.oak.security.authorization.AuthorizationConfigurationImpl;
+import org.apache.jackrabbit.oak.security.authorization.ProviderCtx;
 import org.apache.jackrabbit.oak.security.authorization.composite.CompositeAuthorizationConfiguration;
 import org.apache.jackrabbit.oak.security.authorization.restriction.RestrictionProviderImpl;
 import org.apache.jackrabbit.oak.security.internal.SecurityProviderHelper;
@@ -39,14 +43,15 @@ import org.apache.jackrabbit.oak.spi.com
 import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.apache.jackrabbit.oak.spi.security.Context;
+import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
 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.restriction.Restriction;
 import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionProvider;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConfiguration;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
-import org.apache.jackrabbit.oak.util.NodeUtil;
 import org.jetbrains.annotations.NotNull;
 import org.junit.After;
 import org.junit.Before;
@@ -54,6 +59,7 @@ import org.junit.Test;
 
 import javax.jcr.AccessDeniedException;
 import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
 import javax.jcr.Value;
 import javax.jcr.ValueFactory;
 import javax.jcr.security.AccessControlManager;
@@ -61,14 +67,16 @@ import java.security.Principal;
 
 import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES;
 import static org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants.JCR_READ;
-import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.when;
 
 public class AccessControlValidatorTest extends AbstractSecurityTest implements AccessControlConstants {
@@ -83,9 +91,7 @@ public class AccessControlValidatorTest
     public void before() throws Exception {
         super.before();
 
-        NodeUtil rootNode = new NodeUtil(root.getTree("/"), getNamePathMapper());
-        rootNode.addChild(testName, JcrConstants.NT_UNSTRUCTURED);
-
+        TreeUtil.addChild(root.getTree(PathUtils.ROOT_PATH), testName, JcrConstants.NT_UNSTRUCTURED);
         root.commit();
 
         testPrincipal = getTestUser().getPrincipal();
@@ -105,16 +111,34 @@ public class AccessControlValidatorTest
         }
     }
 
-    private NodeUtil getTestRoot() {
-        return new NodeUtil(root.getTree(testPath));
+    @NotNull
+    private Tree getTestTree() {
+        return root.getTree(testPath);
     }
 
+    @NotNull
     private AccessControlValidatorProvider createValidatorProvider() {
         CompositeAuthorizationConfiguration cac = (CompositeAuthorizationConfiguration) getConfig(AuthorizationConfiguration.class);
         return new AccessControlValidatorProvider((AuthorizationConfigurationImpl) cac.getDefaultConfig());
     }
 
     @NotNull
+    private AccessControlValidatorProvider createValidatorProvider(@NotNull RestrictionProvider restrictionProvider, @NotNull PrivilegeConfiguration privilegeConfiguration) {
+        ProviderCtx ctx = mock(ProviderCtx.class);
+        when(ctx.getRootProvider()).thenReturn(getRootProvider());
+        when(ctx.getTreeProvider()).thenReturn(getTreeProvider());
+
+        SecurityProvider sp = mock(SecurityProvider.class);
+        when(sp.getConfiguration(PrivilegeConfiguration.class)).thenReturn(privilegeConfiguration);
+        AuthorizationConfiguration ac = mock(AuthorizationConfiguration.class);
+        when(ac.getRestrictionProvider()).thenReturn(restrictionProvider);
+        when(ac.getContext()).thenReturn(getConfig(AuthorizationConfiguration.class).getContext());
+        when(sp.getConfiguration(AuthorizationConfiguration.class)).thenReturn(ac);
+        when(ctx.getSecurityProvider()).thenReturn(sp);
+        return new AccessControlValidatorProvider(ctx);
+    }
+
+    @NotNull
     private Validator createRootValidator(@NotNull Tree rootTree) {
         NodeState ns = getTreeProvider().asNodeState(rootTree);
         return createValidatorProvider().getRootValidator(ns, ns, new CommitInfo("sid", null));
@@ -133,23 +157,6 @@ public class AccessControlValidatorTest
         return acl;
     }
 
-    private NodeUtil createAcl() throws AccessDeniedException {
-        NodeUtil testRoot = getTestRoot();
-        testRoot.setNames(JcrConstants.JCR_MIXINTYPES, MIX_REP_ACCESS_CONTROLLABLE);
-
-        NodeUtil acl = testRoot.addChild(REP_POLICY, NT_REP_ACL);
-        NodeUtil ace = createACE(acl, aceName, NT_REP_GRANT_ACE, testPrincipal.getName(), PrivilegeConstants.JCR_READ);
-        ace.addChild(REP_RESTRICTIONS, NT_REP_RESTRICTIONS);
-        return acl;
-    }
-
-    private static NodeUtil createACE(NodeUtil acl, String aceName, String ntName, String principalName, String... privilegeNames) throws AccessDeniedException {
-        NodeUtil ace = acl.addChild(aceName, ntName);
-        ace.setString(REP_PRINCIPAL_NAME, principalName);
-        ace.setNames(REP_PRIVILEGES, privilegeNames);
-        return ace;
-    }
-
     @NotNull
     private static Tree createACE(@NotNull Tree acl, @NotNull String aceName, @NotNull String ntName, @NotNull String principalName, @NotNull String... privilegeNames) throws AccessDeniedException {
         Tree ace = TreeUtil.addChild(acl, aceName, ntName);
@@ -164,262 +171,222 @@ public class AccessControlValidatorTest
         return e;
     }
 
-    @Test
-    public void testPolicyWithOutChildOrder() throws AccessDeniedException {
-        NodeUtil testRoot = getTestRoot();
-        testRoot.setNames(JcrConstants.JCR_MIXINTYPES, MIX_REP_ACCESS_CONTROLLABLE);
-        testRoot.addChild(REP_POLICY, NT_REP_ACL);
+    @Test(expected = CommitFailedException.class)
+    public void testPolicyWithOutChildOrder() throws Exception {
+        Tree testRoot = getTestTree();
+        testRoot.setProperty(JCR_MIXINTYPES, ImmutableList.of(MIX_REP_ACCESS_CONTROLLABLE), Type.NAMES);
+        TreeUtil.addChild(testRoot, REP_POLICY, NT_REP_ACL);
 
         try {
             root.commit();
-            fail("Policy node with child node ordering");
         } catch (CommitFailedException e) {
-            // success
-            assertTrue(e.isAccessControlViolation());
-            assertThat(e.getMessage(), containsString("OakAccessControl0004")); // Order of children is not stable
-            assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 4);
         }
     }
 
-    @Test
-    public void testOnlyRootIsRepoAccessControllable() {
-        NodeUtil testRoot = getTestRoot();
-        testRoot.setNames(JcrConstants.JCR_MIXINTYPES, MIX_REP_REPO_ACCESS_CONTROLLABLE);
+    @Test(expected = CommitFailedException.class)
+    public void testOnlyRootIsRepoAccessControllable() throws Exception {
+        Tree testRoot = getTestTree();
+        testRoot.setProperty(JCR_MIXINTYPES, ImmutableList.of(MIX_REP_REPO_ACCESS_CONTROLLABLE), Type.NAMES);
 
         try {
             root.commit();
-            fail("Only the root node can be made RepoAccessControllable.");
         } catch (CommitFailedException e) {
-            // success
-            assertTrue(e.isAccessControlViolation());
-            assertThat(e.getMessage(), containsString("/testRoot"));
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 12);
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testAddInvalidRepoPolicy() throws Exception {
-        NodeUtil testRoot = getTestRoot();
-        testRoot.setNames(JcrConstants.JCR_MIXINTYPES, MIX_REP_ACCESS_CONTROLLABLE);
-        NodeUtil policy = getTestRoot().addChild(REP_REPO_POLICY, NT_REP_ACL);
+        Tree testRoot = getTestTree();
+        testRoot.setProperty(JCR_MIXINTYPES, ImmutableList.of(MIX_REP_ACCESS_CONTROLLABLE), Type.NAMES);
+        TreeUtil.addChild(testRoot, REP_REPO_POLICY, NT_REP_ACL);
         try {
             root.commit();
-            fail("Attempt to add repo-policy with rep:AccessControllable node.");
         } catch (CommitFailedException e) {
-            // success
-            assertTrue(e.isAccessControlViolation());
-            assertThat(e.getMessage(), containsString("/testRoot"));
-        } finally {
-            policy.getTree().remove();
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 6);
         }
     }
 
-    @Test
-    public void testAddPolicyWithAcContent() throws Exception {
-        NodeUtil acl = createAcl();
-        NodeUtil ace = acl.getChild(aceName);
-
-        NodeUtil[] acContent = new NodeUtil[]{acl, ace, ace.getChild(REP_RESTRICTIONS)};
-        for (NodeUtil node : acContent) {
-            NodeUtil policy = node.addChild(REP_POLICY, NT_REP_ACL);
-            try {
-                root.commit();
-                fail("Adding an ACL below access control content should fail");
-            } catch (CommitFailedException e) {
-                // success
-                assertTrue(e.isConstraintViolation());
-                assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
-            } finally {
-                policy.getTree().remove();
-            }
-        }
+    @Test(expected = CommitFailedException.class)
+    public void testAddPolicyWithAcl() throws Exception {
+        addPolicyWithAcContent();
     }
 
-    @Test
-    public void testAddRepoPolicyWithAcContent() throws Exception {
-        NodeUtil acl = createAcl();
-        NodeUtil ace = acl.getChild(aceName);
-
-        NodeUtil[] acContent = new NodeUtil[]{acl, ace, ace.getChild(REP_RESTRICTIONS)};
-        for (NodeUtil node : acContent) {
-            NodeUtil policy = node.addChild(REP_REPO_POLICY, NT_REP_ACL);
-            try {
-                root.commit();
-                fail("Adding an ACL below access control content should fail");
-            } catch (CommitFailedException e) {
-                // success
-                assertTrue(e.isConstraintViolation());
-                assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
-            } finally {
-                policy.getTree().remove();
-            }
-        }
+    @Test(expected = CommitFailedException.class)
+    public void testAddPolicyWithAce() throws Exception {
+        addPolicyWithAcContent(aceName);
     }
 
-    @Test
-    public void testAddAceWithAcContent() throws Exception {
-        NodeUtil acl = createAcl();
-        NodeUtil ace = acl.getChild(aceName);
-
-        NodeUtil[] acContent = new NodeUtil[]{ace, ace.getChild(REP_RESTRICTIONS)};
-        for (NodeUtil node : acContent) {
-            NodeUtil entry = node.addChild("invalidACE", NT_REP_DENY_ACE);
-            try {
-                root.commit();
-                fail("Adding an ACE below an ACE or restriction should fail");
-            } catch (CommitFailedException e) {
-                // success
-                assertTrue(e.isConstraintViolation());
-                assertThat(e.getMessage(), containsString("/testRoot/rep:policy/validAce"));
-            } finally {
-                entry.getTree().remove();
-            }
-        }
+    @Test(expected = CommitFailedException.class)
+    public void testAddPolicyWithRestriction() throws Exception {
+        addPolicyWithAcContent(aceName, REP_RESTRICTIONS);
     }
 
-    @Test
-    public void testAddRestrictionWithAcContent() throws Exception {
-        NodeUtil acl = createAcl();
-        NodeUtil ace = acl.getChild(aceName);
-
-        NodeUtil[] acContent = new NodeUtil[]{acl, ace.getChild(REP_RESTRICTIONS)};
-        for (NodeUtil node : acContent) {
-            NodeUtil entry = node.addChild("invalidRestriction", NT_REP_RESTRICTIONS);
-            try {
-                root.commit();
-                fail("Adding an ACE below an ACE or restriction should fail");
-            } catch (CommitFailedException e) {
-                // success
-                assertTrue(e.isConstraintViolation());
-                assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
-            } finally {
-                entry.getTree().remove();
+    private void addPolicyWithAcContent(@NotNull String... childNames) throws Exception {
+        Tree rootTree = root.getTree(PathUtils.ROOT_PATH);
+        Tree policy = createPolicy(rootTree, true);
+        Tree acContent = policy;
+        for (String name : childNames) {
+            acContent = acContent.getChild(name);
+        }
+        TreeUtil.addChild(acContent, REP_POLICY, NT_REP_ACL);
+
+        TreeProvider tp = getTreeProvider();
+        Validator v = createRootValidator(rootTree);
+        try {
+            v = v.childNodeAdded(policy.getName(), tp.asNodeState(policy));
+            Tree t = policy;
+            for (String name : childNames) {
+                t = t.getChild(name);
+                v = v.childNodeAdded(name, tp.asNodeState(t));
             }
+            v.childNodeAdded(REP_POLICY, tp.asNodeState(t.getChild(REP_POLICY)));
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 5);
         }
     }
 
-    @Test
-    public void testAddIsolatedPolicy() throws Exception {
-        String[] policyNames = new String[]{"isolatedACL", REP_POLICY, REP_REPO_POLICY};
-        NodeUtil node = getTestRoot();
-
-        for (String policyName : policyNames) {
-            NodeUtil policy = node.addChild(policyName, NT_REP_ACL);
-            try {
-                root.commit();
-                fail("Writing an isolated ACL without the parent being rep:AccessControllable should fail.");
-            } catch (CommitFailedException e) {
-                // success
-                assertTrue(e.isAccessControlViolation());
-                assertThat(e.getMessage(), containsString("/testRoot"));
-            } finally {
-                // revert pending changes that cannot be saved.
-                policy.getTree().remove();
-            }
+    @Test(expected = CommitFailedException.class)
+    public void tesAddIsolatedRepPolicy() throws Exception {
+        addIsolatedPolicy(REP_POLICY);
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void tesAddIsolatedRepRepoPolicy() throws Exception {
+        addIsolatedPolicy(REP_REPO_POLICY);
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void tesAddIsolatedUnknownPolicy() throws Exception {
+        addIsolatedPolicy("isolatedPolicy");
+    }
+
+    private void addIsolatedPolicy(@NotNull String name) throws Exception {
+        TreeUtil.addChild(getTestTree(), name, NT_REP_ACL);
+        try {
+            root.commit();
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 6);
         }
+    }
 
+    @Test(expected = CommitFailedException.class)
+    public void testAddIsolatedGrantAce() throws Exception {
+        createACE(getTestTree(), "isolatedACE", NT_REP_GRANT_ACE, testPrincipal.getName(), JCR_READ);
+        try {
+            root.commit();
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 7);
+        }
     }
 
-    @Test
-    public void testAddIsolatedAce() throws Exception {
-        String[] ntNames = new String[]{NT_REP_DENY_ACE, NT_REP_GRANT_ACE};
-        NodeUtil node = getTestRoot();
-
-        for (String aceNtName : ntNames) {
-            NodeUtil ace = createACE(node, "isolatedACE", aceNtName, testPrincipal.getName(), PrivilegeConstants.JCR_READ);
-            try {
-                root.commit();
-                fail("Writing an isolated ACE should fail.");
-            } catch (CommitFailedException e) {
-                // success
-                assertTrue(e.isAccessControlViolation());
-                assertThat(e.getMessage(), containsString("/testRoot/isolatedACE"));
-            } finally {
-                // revert pending changes that cannot be saved.
-                ace.getTree().remove();
-            }
+    @Test(expected = CommitFailedException.class)
+    public void testAddIsolatedDenyAce() throws Exception {
+        createACE(getTestTree(), "isolatedACE", NT_REP_DENY_ACE, testPrincipal.getName(), JCR_READ);
+        try {
+            root.commit();
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 7);
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testAddIsolatedRestriction() throws Exception {
-        NodeUtil node = getTestRoot();
-        NodeUtil restriction = node.addChild("isolatedRestriction", NT_REP_RESTRICTIONS);
+        TreeUtil.addChild(getTestTree(), "isolatedRestriction", NT_REP_RESTRICTIONS);
         try {
             root.commit();
-            fail("Writing an isolated Restriction should fail.");
         } catch (CommitFailedException e) {
-            // success
-            assertTrue(e.isAccessControlViolation());
-            assertEquals(2, e.getCode());
-            assertThat(e.getMessage(), containsString("/testRoot"));
-        } finally {
-            // revert pending changes that cannot be saved.
-            restriction.getTree().remove();
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 2);
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testInvalidPrivilege() throws Exception {
-        NodeUtil acl = createAcl();
-
-        String privName = "invalidPrivilegeName";
-        createACE(acl, "invalid", NT_REP_GRANT_ACE, testPrincipal.getName(), privName);
+        createACE(createPolicy(getTestTree(), false), "aceWithInvalidPrivilege", NT_REP_GRANT_ACE, testPrincipal.getName(), "invalidPrivilegeName");
         try {
             root.commit();
-            fail("Creating an ACE with invalid privilege should fail.");
         } catch (CommitFailedException e) {
-            // success
-            assertTrue(e.isAccessControlViolation());
-            assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 10);
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testAbstractPrivilege() throws Exception {
         PrivilegeManager pMgr = getPrivilegeManager(root);
         pMgr.registerPrivilege("abstractPrivilege", true, new String[0]);
 
-        NodeUtil acl = createAcl();
-        createACE(acl, "invalid", NT_REP_GRANT_ACE, testPrincipal.getName(), "abstractPrivilege");
+        createACE(createPolicy(getTestTree(), false), "invalid", NT_REP_GRANT_ACE, testPrincipal.getName(), "abstractPrivilege");
         try {
             root.commit();
-            fail("Creating an ACE with an abstract privilege should fail.");
         } catch (CommitFailedException e) {
-            // success
-            assertTrue(e.isAccessControlViolation());
-            assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 11);
         }
     }
 
-    @Test
+    @Test(expected = IllegalStateException.class)
+    public void testValidatingPrivilegesCausesInternalError() throws Exception {
+        PrivilegeManager privMgr = when(mock(PrivilegeManager.class).getPrivilege(anyString())).thenThrow(new RepositoryException()).getMock();
+        PrivilegeConfiguration pc = when( mock(PrivilegeConfiguration.class).getPrivilegeManager(any(Root.class), any(NamePathMapper.class))).thenReturn(privMgr).getMock();
+
+        Tree rootTree = root.getTree(PathUtils.ROOT_PATH);
+        Tree policy = createPolicy(rootTree, false);
+
+        TreeProvider tp = getTreeProvider();
+        AccessControlValidatorProvider provider = createValidatorProvider(getConfig(AuthorizationConfiguration.class).getRestrictionProvider(), pc);
+        NodeState ns = tp.asNodeState(rootTree);
+        Validator v = provider.getRootValidator(ns, ns, new CommitInfo("sid", null));
+
+        v.childNodeAdded(policy.getName(), tp.asNodeState(policy)).childNodeAdded(aceName, tp.asNodeState(policy.getChild(aceName)));
+    }
+
+    @Test(expected = CommitFailedException.class)
     public void testInvalidRestriction() throws Exception {
-        NodeUtil restriction = createAcl().getChild(aceName).getChild(REP_RESTRICTIONS);
-        restriction.setString("invalid", "value");
+        Tree restriction = createPolicy(getTestTree(), true).getChild(aceName).getChild(REP_RESTRICTIONS);
+        restriction.setProperty("invalid", "value");
         try {
             root.commit();
-            fail("Creating an unsupported restriction should fail.");
         } catch (CommitFailedException e) {
-            // success
-            assertTrue(e.isAccessControlViolation());
-            assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 1);
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testRestrictionWithInvalidType() throws Exception {
-        NodeUtil restriction = createAcl().getChild(aceName).getChild(REP_RESTRICTIONS);
-        restriction.setName(REP_GLOB, "rep:glob");
+        Tree restriction = createPolicy(getTestTree(), true).getChild(aceName).getChild(REP_RESTRICTIONS);
+        restriction.setProperty(REP_GLOB, "rep:glob", Type.NAME);
         try {
             root.commit();
-            fail("Creating restriction with invalid type should fail.");
         } catch (CommitFailedException e) {
-            // success
-            assertTrue(e.isAccessControlViolation());
-            assertThat(e.getMessage(), containsString("/testRoot/rep:policy"));
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 1);
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
+    public void testValidatingRestrictionsCausesInternalError() throws Exception {
+        RestrictionProvider rp = spy(getConfig(AuthorizationConfiguration.class).getRestrictionProvider());
+        doAnswer(invocationOnMock -> {
+            throw new RepositoryException();
+        }).when(rp).validateRestrictions(anyString(), any(Tree.class));
+
+        Tree rootTree = root.getTree(PathUtils.ROOT_PATH);
+        Tree policy = createPolicy(rootTree, false);
+        Tree entry = policy.getChild(aceName);
+        entry.setProperty(REP_GLOB, "any");
+
+        TreeProvider tp = getTreeProvider();
+        AccessControlValidatorProvider provider = createValidatorProvider(rp, getConfig(PrivilegeConfiguration.class));
+        NodeState ns = tp.asNodeState(rootTree);
+        Validator v = provider.getRootValidator(ns, ns, new CommitInfo("sid", null));
+
+        try {
+            v.childNodeAdded(policy.getName(), tp.asNodeState(policy)).childNodeAdded(entry.getName(), tp.asNodeState(entry));
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.OAK, 13);
+        }
+    }
+
+    @Test(expected = CommitFailedException.class)
     public void testDuplicateAce() throws Exception {
         AccessControlManager acMgr = getAccessControlManager(root);
         JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, testPath);
@@ -427,24 +394,21 @@ public class AccessControlValidatorTest
         acMgr.setPolicy(testPath, acl);
 
         // add duplicate ac-entry on OAK-API
-        NodeUtil policy = new NodeUtil(root.getTree(testPath + "/rep:policy"));
-        NodeUtil ace = policy.addChild("duplicateAce", NT_REP_GRANT_ACE);
-        ace.setString(REP_PRINCIPAL_NAME, testPrincipal.getName());
-        ace.setNames(AccessControlConstants.REP_PRIVILEGES, PrivilegeConstants.JCR_ADD_CHILD_NODES);
+        Tree policy = root.getTree(testPath + "/rep:policy");
+        Tree ace = TreeUtil.addChild(policy, "duplicateAce", NT_REP_GRANT_ACE);
+        ace.setProperty(REP_PRINCIPAL_NAME, testPrincipal.getName());
+        ace.setProperty(AccessControlConstants.REP_PRIVILEGES, ImmutableList.of(PrivilegeConstants.JCR_ADD_CHILD_NODES), Type.NAMES);
 
         try {
             root.commit();
-            fail("Creating duplicate ACE must be detected");
         } catch (CommitFailedException e) {
-            assertTrue(e.isAccessControlViolation());
-            assertThat(e.getMessage(), containsString("/testRoot/rep:policy/duplicateAce"));
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 13);
         }
     }
 
     @Test
     public void testAceDifferentByAllowStatus() throws Exception {
-        Tree policy = createAcl().getTree();
-        policy.setOrderableChildren(true);
+        Tree policy = createPolicy(root.getTree(PathUtils.ROOT_PATH), false);
         Tree entry = policy.getChild(aceName);
         Tree entry2 = TreeUtil.addChild(policy, "second", NT_REP_DENY_ACE);
         entry2.setProperty(entry.getProperty(REP_PRINCIPAL_NAME));
@@ -460,12 +424,12 @@ public class AccessControlValidatorTest
         AccessControlManager acMgr = getAccessControlManager(root);
         JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, testPath);
         acl.addEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES), true,
-                ImmutableMap.<String, Value>of(),
+                ImmutableMap.of(),
                 ImmutableMap.of(AccessControlConstants.REP_NT_NAMES, new Value[] {vf.createValue(NodeTypeConstants.NT_OAK_UNSTRUCTURED, PropertyType.NAME)}));
 
         // add ac-entry that only differs by the value of the restriction
         acl.addEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES), true,
-                ImmutableMap.<String, Value>of(),
+                ImmutableMap.of(),
                 ImmutableMap.of(AccessControlConstants.REP_NT_NAMES, new Value[] {vf.createValue(NodeTypeConstants.NT_UNSTRUCTURED, PropertyType.NAME)}));
         assertEquals(2, acl.getAccessControlEntries().length);
 
@@ -566,7 +530,7 @@ public class AccessControlValidatorTest
      * Test case illustrating OAK-8081, where a given 'aceTree' is not covered by the authorization-context and thus
      * the AccessControlValidator will still fail.
      */
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testRestrictionsUsedByOtherModule2() throws Exception {
         AuthorizationConfiguration sc = mock(AuthorizationConfiguration.class);
         // new acNode is not covered by Context.definesTree
@@ -582,14 +546,23 @@ public class AccessControlValidatorTest
         try {
             root.commit();
         } catch (CommitFailedException e) {
-            assertTrue(e.isAccessControlViolation());
-            assertEquals(2, e.getCode());
-        } finally {
-            root.refresh();
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 2);
         }
     }
 
+    @Test(expected = CommitFailedException.class)
+    public void testAddPolicyTreeWithInvalidName() throws Exception {
+        Tree rootTree = root.getTree(PathUtils.ROOT_PATH);
+        rootTree.setProperty(JCR_MIXINTYPES, ImmutableList.of(MIX_REP_ACCESS_CONTROLLABLE), Type.NAMES);
+        TreeUtil.addChild(rootTree, "invalidName", NT_REP_ACL);
 
+        Validator v = createRootValidator(rootTree);
+        try {
+            v.childNodeAdded("invalidName", mock(NodeState.class));
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 3);
+        }
+    }
 
     @Test(expected = CommitFailedException.class)
     public void testAddEntyWithEmptyPrivileges() throws Exception {
@@ -650,4 +623,83 @@ public class AccessControlValidatorTest
             throw assertCommitFailedException(e, CommitFailedException.ACCESS_CONTROL, 8);
         }
     }
+
+    //---< additional tests verifying the that type-editor spots invalid sequences of access controlled nodes >---------
+
+    @Test(expected = CommitFailedException.class)
+    public void testAddRepoPolicyWithAcl() throws Exception {
+        addRepoPolicyWithAcContent(createPolicy(getTestTree(), false));
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void testAddRepoPolicyWithAce() throws Exception {
+        addRepoPolicyWithAcContent(createPolicy(getTestTree(), false).addChild(aceName));
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void testAddRepoPolicyWithRestriction() throws Exception {
+        Tree ace = createPolicy(getTestTree(), true).getChild(aceName);
+        addRepoPolicyWithAcContent(ace.getChild(REP_RESTRICTIONS));
+    }
+
+    private void addRepoPolicyWithAcContent(@NotNull Tree acContent) throws Exception {
+        TreeUtil.addChild(acContent, REP_REPO_POLICY, NT_REP_ACL);
+        try {
+            root.commit();
+            fail("Adding an ACL below access control content should fail");
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.CONSTRAINT, 25);
+        }
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void testAddAceWithAce() throws Exception {
+        addAceWithAcContent(createPolicy(getTestTree(), false).getChild(aceName));
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void testAddAceWithRestriction() throws Exception {
+        Tree ace = createPolicy(getTestTree(), true).getChild(aceName);
+        addAceWithAcContent(ace.getChild(REP_RESTRICTIONS));
+    }
+
+    private void addAceWithAcContent(@NotNull Tree acContent) throws Exception {
+        TreeUtil.addChild(acContent, "invalidACE", NT_REP_DENY_ACE);
+        try {
+            root.commit();
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.CONSTRAINT, 25);
+        }
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void testAddRestrictionWithAcl() throws Exception {
+        addRestrictionWithAcContent(createPolicy(getTestTree(), false));
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void testAddRestrictionWithRestriction() throws Exception {
+        Tree ace = createPolicy(getTestTree(), true).getChild(aceName);
+        addRestrictionWithAcContent(ace.getChild(REP_RESTRICTIONS));
+    }
+
+    private void addRestrictionWithAcContent(@NotNull Tree acContent) throws Exception {
+        TreeUtil.addChild(acContent, REP_RESTRICTIONS, NT_REP_RESTRICTIONS);
+        try {
+            root.commit();
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.CONSTRAINT, 25);
+        }
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void testInvalidRestrictionWithACE() throws Exception {
+        Tree ace = createPolicy(getTestTree(), false).getChild(aceName);
+        TreeUtil.addChild(ace, "invalidRestriction", NT_REP_RESTRICTIONS);
+        try {
+            root.commit();
+        } catch (CommitFailedException e) {
+            throw assertCommitFailedException(e, CommitFailedException.CONSTRAINT, 25);
+        }
+    }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AdminPrincipalsBaseTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AdminPrincipalsBaseTest.java?rev=1860730&r1=1860729&r2=1860730&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AdminPrincipalsBaseTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AdminPrincipalsBaseTest.java Thu Jun  6 17:20:27 2019
@@ -27,12 +27,13 @@ import javax.jcr.security.AccessControlP
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
-import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.apache.jackrabbit.oak.spi.security.principal.AdminPrincipal;
 import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.oak.spi.security.principal.SystemPrincipal;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
-import org.apache.jackrabbit.oak.util.NodeUtil;
 import org.junit.Test;
 
 public abstract class AdminPrincipalsBaseTest extends AbstractSecurityTest {
@@ -46,8 +47,8 @@ public abstract class AdminPrincipalsBas
     public void before() throws Exception {
         super.before();
 
-        NodeUtil rootNode = new NodeUtil(root.getTree("/"), NamePathMapper.DEFAULT);
-        rootNode.addChild("testNode", JcrConstants.NT_UNSTRUCTURED);
+        Tree rootNode = root.getTree(PathUtils.ROOT_PATH);
+        TreeUtil.addChild(rootNode, "testNode", JcrConstants.NT_UNSTRUCTURED);
 
         administrativePrincipal = getUserManager(root).createGroup(new PrincipalImpl(ADMINISTRATORS_PRINCIPAL_NAME)).getPrincipal();
         root.commit();

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/EntryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/EntryTest.java?rev=1860730&r1=1860729&r2=1860730&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/EntryTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/EntryTest.java Thu Jun  6 17:20:27 2019
@@ -128,7 +128,7 @@ public class EntryTest extends AbstractA
             Privilege[] privs = new Privilege[]{
                     acMgr.privilegeFromName(PrivilegeConstants.JCR_ALL)
             };
-            createEntry(null, privs, true);
+            createEntry(null, privs, false);
             fail("Principal must not be null");
         } catch (AccessControlException e) {
             // success

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/NodeACLTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/NodeACLTest.java?rev=1860730&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/NodeACLTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/NodeACLTest.java Thu Jun  6 17:20:27 2019
@@ -0,0 +1,104 @@
+/*
+ * 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.accesscontrol;
+
+import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
+import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.security.AccessControlList;
+import javax.jcr.security.AccessControlPolicy;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+public class NodeACLTest extends AbstractAccessControlTest {
+
+    private ACL nodeAcl;
+
+    @Override
+    @Before
+    public void before() throws Exception {
+        super.before();
+
+        JackrabbitAccessControlManager acMgr = getAccessControlManager(root);
+        AccessControlList policy = AccessControlUtils.getAccessControlList(acMgr, TEST_PATH);
+        policy.addAccessControlEntry(testPrincipal, testPrivileges);
+        policy.addAccessControlEntry(EveryonePrincipal.getInstance(), testPrivileges);
+        acMgr.setPolicy(TEST_PATH, policy);
+
+        nodeAcl = getNodeAcl(acMgr, TEST_PATH);
+    }
+
+    @After
+    public void after() throws Exception {
+        try {
+            root.refresh();
+        } finally {
+            super.after();
+        }
+    }
+
+    @NotNull
+    private static ACL getNodeAcl(@NotNull JackrabbitAccessControlManager acMgr, @Nullable String path) throws RepositoryException {
+        for (AccessControlPolicy acp : acMgr.getPolicies(path)) {
+            if (acp instanceof ACL) {
+                return (ACL) acp;
+            }
+        }
+        throw new RuntimeException("no node acl found");
+    }
+
+    @Test
+    public void testEquals() throws Exception {
+        assertEquals(nodeAcl, nodeAcl);
+        assertEquals(nodeAcl, getNodeAcl(getAccessControlManager(root), TEST_PATH));
+    }
+
+    @Test
+    public void testEqualsDifferentPath() throws Exception {
+        getAccessControlManager(root);
+        AccessControlList acl = AccessControlUtils.getAccessControlList(getAccessControlManager(root), null);
+        acl.addAccessControlEntry(testPrincipal, testPrivileges);
+        acl.addAccessControlEntry(EveryonePrincipal.getInstance(), testPrivileges);
+        assertNotEquals(nodeAcl, acl);
+    }
+
+    @Test
+    public void testEqualsDifferentEntries() throws Exception {
+        ACL acl = getNodeAcl(getAccessControlManager(root), TEST_PATH);
+        acl.removeAccessControlEntry(acl.getAccessControlEntries()[0]);
+        assertNotEquals(nodeAcl, acl);
+    }
+
+    @Test
+    public void testEqualsDifferentAcessControlList() {
+        assertNotEquals(nodeAcl, createACL(TEST_PATH, nodeAcl.getEntries(), getNamePathMapper(), getRestrictionProvider()));
+    }
+
+    @Test
+    public void testHashCode() {
+        assertEquals(0, nodeAcl.hashCode());
+    }
+
+}
\ No newline at end of file

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

Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/TestUtility.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/TestUtility.java?rev=1860730&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/TestUtility.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/TestUtility.java Thu Jun  6 17:20:27 2019
@@ -0,0 +1,74 @@
+/*
+ * 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.accesscontrol;
+
+import com.google.common.collect.ImmutableMap;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.ValueFactory;
+import javax.jcr.security.AccessControlManager;
+import javax.jcr.security.AccessControlPolicyIterator;
+import javax.jcr.security.Privilege;
+import java.security.Principal;
+import java.util.Map;
+
+import static org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants.REP_GLOB;
+
+final class TestUtility {
+
+    private static final Logger log = LoggerFactory.getLogger(TestUtility.class);
+
+    private TestUtility() {}
+
+    @NotNull
+    static ACL getApplicablePolicy( @NotNull AccessControlManager acMgr, @Nullable String path) throws RepositoryException {
+        AccessControlPolicyIterator itr = acMgr.getApplicablePolicies(path);
+        if (itr.hasNext()) {
+            return (ACL) itr.nextAccessControlPolicy();
+        } else {
+            throw new RepositoryException("No applicable policy found.");
+        }
+    }
+
+    @NotNull
+    static ACL setupPolicy(@NotNull AccessControlManager acMgr,
+                           @Nullable String path,
+                           @NotNull Principal testPrincipal,
+                           @NotNull Privilege[] privileges,
+                           boolean isAllow,
+                           @Nullable Map<String, Value> restrictions,
+                           @Nullable Map<String, Value[]> mvRestrictions) throws RepositoryException {
+        ACL policy = getApplicablePolicy(acMgr, path);
+        if (path == null) {
+            policy.addAccessControlEntry(testPrincipal, privileges);
+        } else {
+            policy.addEntry(testPrincipal, privileges, isAllow, restrictions, mvRestrictions);
+        }
+        acMgr.setPolicy(path, policy);
+        return policy;
+    }
+
+    @NotNull
+    static Map<String, Value> getGlobRestriction(@NotNull String value, @NotNull ValueFactory valueFactory) {
+        return ImmutableMap.of(REP_GLOB, valueFactory.createValue(value));
+    }
+}
\ No newline at end of file

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

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/UtilTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/UtilTest.java?rev=1860730&r1=1860729&r2=1860730&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/UtilTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/UtilTest.java Thu Jun  6 17:20:27 2019
@@ -21,19 +21,33 @@ import javax.jcr.security.AccessControlE
 import javax.jcr.security.Privilege;
 
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
+import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.authorization.AuthorizationConfiguration;
 import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.ACE;
+import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;
 import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
 import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBitsProvider;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
+import org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class UtilTest extends AbstractSecurityTest {
 
@@ -66,6 +80,42 @@ public class UtilTest extends AbstractSe
     }
 
     @Test
+    public void testIsAceNonExistingTree() {
+        Tree t = when(mock(Tree.class).exists()).thenReturn(false).getMock();
+        assertFalse(Util.isACE(t, ReadOnlyNodeTypeManager.getInstance(root, getNamePathMapper())));
+    }
+
+    @Test
+    public void testIsAceOtherTree() {
+        assertFalse(Util.isACE(root.getTree(PathUtils.ROOT_PATH), ReadOnlyNodeTypeManager.getInstance(root, getNamePathMapper())));
+    }
+
+    @Test
+    public void testIsAce() {
+        Tree t = when(mock(Tree.class).exists()).thenReturn(true).getMock();
+        when(t.getProperty(JCR_PRIMARYTYPE)).thenReturn(PropertyStates.createProperty(JCR_PRIMARYTYPE, AccessControlConstants.NT_REP_DENY_ACE, Type.NAME));
+        assertTrue(Util.isACE(t, ReadOnlyNodeTypeManager.getInstance(root, getNamePathMapper())));
+    }
+
+    @Test
+    public void testGetImportBehaviorDefault() {
+        AuthorizationConfiguration config = when(mock(AuthorizationConfiguration.class).getParameters()).thenReturn(ConfigurationParameters.EMPTY).getMock();
+        assertSame(ImportBehavior.ABORT, Util.getImportBehavior(config));
+    }
+
+    @Test
+    public void testGetImportBehaviorInvalid() {
+        AuthorizationConfiguration config = when(mock(AuthorizationConfiguration.class).getParameters()).thenReturn(ConfigurationParameters.of(ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, "invalid")).getMock();
+        assertSame(ImportBehavior.ABORT, Util.getImportBehavior(config));
+    }
+
+    @Test
+    public void testGetImportBehavior() {
+        AuthorizationConfiguration config = when(mock(AuthorizationConfiguration.class).getParameters()).thenReturn(ConfigurationParameters.of(ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, ImportBehavior.NAME_BESTEFFORT)).getMock();
+        assertSame(ImportBehavior.BESTEFFORT, Util.getImportBehavior(config));
+    }
+
+    @Test
     public void testGenerateName() throws AccessControlException {
         ACE ace = new TestAce(true);
         String name = Util.generateAceName(ace, 0);