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 2020/09/29 15:59:11 UTC

svn commit: r1882131 [2/3] - in /jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security: principal/ user/ user/action/ user/whiteboard/

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserConfigurationImplTest.java Tue Sep 29 15:59:10 2020
@@ -16,42 +16,27 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
-import org.apache.jackrabbit.api.security.user.UserManager;
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
-import org.apache.jackrabbit.oak.api.blob.BlobAccessProvider;
-import org.apache.jackrabbit.oak.plugins.value.jcr.PartialValueFactory;
 import org.apache.jackrabbit.oak.spi.commit.MoveTracker;
 import org.apache.jackrabbit.oak.spi.commit.ThreeWayConflictHandler;
 import org.apache.jackrabbit.oak.spi.commit.ValidatorProvider;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
-import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
 import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil;
-import org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardAware;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
 import org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter;
-import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
-import org.junit.Rule;
 import org.junit.Test;
-import org.osgi.framework.ServiceRegistration;
 
-import java.lang.reflect.Field;
 import java.security.Principal;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.Hashtable;
 import java.util.List;
 
 import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.PARAM_DEFAULT_DEPTH;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-import static org.mockito.Mockito.withSettings;
 
 public class UserConfigurationImplTest extends AbstractSecurityTest {
 
@@ -68,24 +53,12 @@ public class UserConfigurationImplTest e
     private static final Integer PASSWORD_HISTORY_SIZE = 12;
     private static final boolean ENABLE_RFC7613_USERCASE_MAPPED_PROFILE = true;
 
-    @Rule
-    public final OsgiContext context = new OsgiContext();
-
     @Override
     protected ConfigurationParameters getSecurityConfigParameters() {
         return ConfigurationParameters.of(UserConfiguration.NAME, getParams());
     }
 
     @Test
-    public void testActivate() {
-        UserConfiguration userConfiguration = new UserConfigurationImpl(getSecurityProvider());
-        context.registerInjectActivateService(userConfiguration, ImmutableMap.of(PARAM_DEFAULT_DEPTH, "8"));
-
-        ConfigurationParameters params = userConfiguration.getParameters();
-        assertEquals(8, params.getConfigValue(PARAM_DEFAULT_DEPTH, UserConstants.DEFAULT_DEPTH).intValue());
-    }
-
-    @Test
     public void testValidators() {
         UserConfigurationImpl configuration = new UserConfigurationImpl(getSecurityProvider());
         configuration.setRootProvider(getRootProvider());
@@ -124,49 +97,6 @@ public class UserConfigurationImplTest e
     }
 
     @Test
-    public void testBlobAccessProviderFromNullWhiteboard() throws Exception {
-        SecurityProvider sp = mock(SecurityProvider.class, withSettings().extraInterfaces(WhiteboardAware.class));
-
-        UserConfigurationImpl uc = new UserConfigurationImpl(sp);
-        uc.setParameters(ConfigurationParameters.EMPTY);
-        uc.setRootProvider(getRootProvider());
-        uc.setTreeProvider(getTreeProvider());
-
-        when(sp.getConfiguration(UserConfiguration.class)).thenReturn(uc);
-
-        UserManager um = uc.getUserManager(root, getNamePathMapper());
-        assertTrue(um instanceof UserManagerImpl);
-
-        PartialValueFactory vf = ((UserManagerImpl) um).getPartialValueFactory();
-        Field f = PartialValueFactory.class.getDeclaredField("blobAccessProvider");
-        f.setAccessible(true);
-        assertSame(PartialValueFactory.DEFAULT_BLOB_ACCESS_PROVIDER, f.get(vf));
-    }
-    
-    @Test
-    public void testBindBlobAccessProvider() throws Exception {
-        UserConfigurationImpl uc = new UserConfigurationImpl(getSecurityProvider());
-        context.registerInjectActivateService(uc, ImmutableMap.of(PARAM_DEFAULT_DEPTH, "8"));
-
-        BlobAccessProvider bap = mock(BlobAccessProvider.class);
-        uc.getUserManager(root, getNamePathMapper());
-        Field f = UserConfigurationImpl.class.getDeclaredField("blobAccessProvider");
-        f.setAccessible(true);
-        //Validate default service
-        assertSame(PartialValueFactory.DEFAULT_BLOB_ACCESS_PROVIDER, f.get(uc));
-
-        ServiceRegistration reg = context.bundleContext().registerService(
-                BlobAccessProvider.class.getName(),
-                bap, new Hashtable<String, Object>());
-        //Validate newly registered service
-        assertSame(bap, f.get(uc));
-
-        reg.unregister();
-        //Validate default service after unregistering newly registered service
-        assertSame(PartialValueFactory.DEFAULT_BLOB_ACCESS_PROVIDER, f.get(uc));
-    }
-
-    @Test
     public void testUserConfigurationWithConstructor() {
         UserConfigurationImpl userConfiguration = new UserConfigurationImpl(getSecurityProvider());
         testConfigurationParameters(userConfiguration.getParameters());
@@ -180,18 +110,18 @@ public class UserConfigurationImplTest e
     }
     
     private void testConfigurationParameters(ConfigurationParameters parameters) {
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_USER_PATH, UserConstants.DEFAULT_USER_PATH), USER_PATH);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_GROUP_PATH, UserConstants.DEFAULT_GROUP_PATH), GROUP_PATH);
-        assertEquals(parameters.getConfigValue(PARAM_DEFAULT_DEPTH, UserConstants.DEFAULT_DEPTH), DEFAULT_DEPTH);
-        assertEquals(parameters.getConfigValue(ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, ImportBehavior.NAME_IGNORE), IMPORT_BEHAVIOR);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_PASSWORD_HASH_ALGORITHM, PasswordUtil.DEFAULT_ALGORITHM), HASH_ALGORITHM);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_PASSWORD_HASH_ITERATIONS, PasswordUtil.DEFAULT_ITERATIONS), HASH_ITERATIONS);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_PASSWORD_SALT_SIZE, PasswordUtil.DEFAULT_SALT_SIZE), SALT_SIZE);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_SUPPORT_AUTOSAVE, false), SUPPORT_AUTOSAVE);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_PASSWORD_MAX_AGE, UserConstants.DEFAULT_PASSWORD_MAX_AGE), MAX_AGE);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_PASSWORD_INITIAL_CHANGE, UserConstants.DEFAULT_PASSWORD_INITIAL_CHANGE), INITIAL_PASSWORD_CHANGE);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_PASSWORD_HISTORY_SIZE, UserConstants.PASSWORD_HISTORY_DISABLED_SIZE), PASSWORD_HISTORY_SIZE);
-        assertEquals(parameters.getConfigValue(UserConstants.PARAM_ENABLE_RFC7613_USERCASE_MAPPED_PROFILE, UserConstants.DEFAULT_ENABLE_RFC7613_USERCASE_MAPPED_PROFILE), ENABLE_RFC7613_USERCASE_MAPPED_PROFILE);
+        assertEquals(USER_PATH, parameters.getConfigValue(UserConstants.PARAM_USER_PATH, UserConstants.DEFAULT_USER_PATH));
+        assertEquals(GROUP_PATH, parameters.getConfigValue(UserConstants.PARAM_GROUP_PATH, UserConstants.DEFAULT_GROUP_PATH));
+        assertEquals(DEFAULT_DEPTH, parameters.getConfigValue(PARAM_DEFAULT_DEPTH, UserConstants.DEFAULT_DEPTH));
+        assertEquals(IMPORT_BEHAVIOR, parameters.getConfigValue(ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, ImportBehavior.NAME_IGNORE));
+        assertEquals(HASH_ALGORITHM, parameters.getConfigValue(UserConstants.PARAM_PASSWORD_HASH_ALGORITHM, PasswordUtil.DEFAULT_ALGORITHM));
+        assertEquals(HASH_ITERATIONS, parameters.getConfigValue(UserConstants.PARAM_PASSWORD_HASH_ITERATIONS, PasswordUtil.DEFAULT_ITERATIONS));
+        assertEquals(SALT_SIZE, parameters.getConfigValue(UserConstants.PARAM_PASSWORD_SALT_SIZE, PasswordUtil.DEFAULT_SALT_SIZE));
+        assertEquals(SUPPORT_AUTOSAVE, parameters.getConfigValue(UserConstants.PARAM_SUPPORT_AUTOSAVE, false));
+        assertEquals(MAX_AGE, parameters.getConfigValue(UserConstants.PARAM_PASSWORD_MAX_AGE, UserConstants.DEFAULT_PASSWORD_MAX_AGE));
+        assertEquals(INITIAL_PASSWORD_CHANGE, parameters.getConfigValue(UserConstants.PARAM_PASSWORD_INITIAL_CHANGE, UserConstants.DEFAULT_PASSWORD_INITIAL_CHANGE));
+        assertEquals(PASSWORD_HISTORY_SIZE, parameters.getConfigValue(UserConstants.PARAM_PASSWORD_HISTORY_SIZE, UserConstants.PASSWORD_HISTORY_DISABLED_SIZE));
+        assertEquals(ENABLE_RFC7613_USERCASE_MAPPED_PROFILE, parameters.getConfigValue(UserConstants.PARAM_ENABLE_RFC7613_USERCASE_MAPPED_PROFILE, UserConstants.DEFAULT_ENABLE_RFC7613_USERCASE_MAPPED_PROFILE));
     }
 
     private ConfigurationParameters getParams() {

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterBaseTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterBaseTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterBaseTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterBaseTest.java Tue Sep 29 15:59:10 2020
@@ -16,61 +16,55 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-import java.util.ArrayList;
-import java.util.List;
-import javax.jcr.ImportUUIDBehavior;
-import javax.jcr.PropertyType;
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
-import javax.jcr.Value;
-import javax.jcr.nodetype.PropertyDefinition;
-
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.JackrabbitSession;
-import org.apache.jackrabbit.api.security.user.Authorizable;
-import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
 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.api.Type;
 import org.apache.jackrabbit.oak.commons.PathUtils;
-import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
+import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
 import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableAction;
 import org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableActionProvider;
-import org.apache.jackrabbit.oak.spi.security.user.action.GroupAction;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
 import org.apache.jackrabbit.oak.spi.xml.NodeInfo;
 import org.apache.jackrabbit.oak.spi.xml.PropInfo;
 import org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter;
 import org.apache.jackrabbit.oak.spi.xml.ReferenceChangeTracker;
 import org.apache.jackrabbit.oak.spi.xml.TextValue;
-import org.apache.jackrabbit.oak.util.NodeUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
-import org.mockito.Mockito;
 
-import static org.junit.Assert.assertEquals;
+import javax.jcr.ImportUUIDBehavior;
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.Value;
+import javax.jcr.nodetype.PropertyDefinition;
+import java.util.List;
+
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.withSettings;
 
 public abstract class UserImporterBaseTest extends AbstractSecurityTest implements UserConstants {
 
     static final String TEST_USER_ID = "uid";
-    static final String TEST_GROUP_ID = "gid";
+    private static final String TEST_GROUP_ID = "gid";
 
-    TestAction testAction;
+    AuthorizableAction testAction;
     AuthorizableActionProvider actionProvider = new AuthorizableActionProvider() {
         @NotNull
         @Override
         public List<? extends AuthorizableAction> getAuthorizableActions(@NotNull SecurityProvider securityProvider) {
-            return (testAction == null) ? ImmutableList.<AuthorizableAction>of() : ImmutableList.of(testAction);
+            return (testAction == null) ? ImmutableList.of() : ImmutableList.of(testAction);
         }
     };
 
@@ -98,10 +92,12 @@ public abstract class UserImporterBaseTe
         }
     }
 
-    ConfigurationParameters getImportConfig() {
+    @NotNull
+    private ConfigurationParameters getImportConfig() {
         return getSecurityConfigParameters().getConfigValue(UserConfiguration.NAME, ConfigurationParameters.EMPTY);
     }
 
+    @NotNull
     String getImportBehavior() {
         return ImportBehavior.NAME_IGNORE;
     }
@@ -115,8 +111,9 @@ public abstract class UserImporterBaseTe
         return ConfigurationParameters.of(UserConfiguration.NAME, userParams);
     }
 
+    @NotNull
     Session mockJackrabbitSession() throws Exception {
-        JackrabbitSession s = Mockito.mock(JackrabbitSession.class);
+        JackrabbitSession s = mock(JackrabbitSession.class);
         when(s.getUserManager()).thenReturn(getUserManager(root));
         return s;
     }
@@ -125,13 +122,17 @@ public abstract class UserImporterBaseTe
         return false;
     }
 
-    boolean init() throws Exception {
-        return init(false);
+    void init() throws Exception {
+        init(false);
     }
 
-    boolean init(boolean createAction) throws Exception {
+    boolean init(boolean createAction, Class<?>... extraInterfaces) throws Exception {
         if (createAction) {
-            testAction = new TestAction();
+            if (extraInterfaces.length > 0) {
+                testAction = mock(AuthorizableAction.class, withSettings().extraInterfaces(extraInterfaces));
+            } else {
+                testAction = mock(AuthorizableAction.class);
+            }
         }
         return importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_COLLISION_REMOVE_EXISTING, refTracker, getSecurityProvider());
     }
@@ -158,10 +159,10 @@ public abstract class UserImporterBaseTe
     Tree createGroupTree() throws Exception {
         String groupPath = getUserConfiguration().getParameters().getConfigValue(PARAM_GROUP_PATH, DEFAULT_GROUP_PATH);
 
-        NodeUtil node = new NodeUtil(root.getTree(PathUtils.ROOT_PATH));
-        NodeUtil groupRoot = node.getOrAddTree(PathUtils.relativize(PathUtils.ROOT_PATH, groupPath), NT_REP_AUTHORIZABLE_FOLDER);
+        Tree node = root.getTree(PathUtils.ROOT_PATH);
+        node = Utils.getOrAddTree(node, PathUtils.relativize(PathUtils.ROOT_PATH, groupPath), NT_REP_AUTHORIZABLE_FOLDER);
 
-        Tree groupTree = groupRoot.addChild("testGroup", NT_REP_GROUP).getTree();
+        Tree groupTree = TreeUtil.addChild(node,"testGroup", NT_REP_GROUP);
         groupTree.setProperty(JcrConstants.JCR_UUID, new UserProvider(root, ConfigurationParameters.EMPTY).getContentID(TEST_GROUP_ID));
         return groupTree;
     }
@@ -192,7 +193,7 @@ public abstract class UserImporterBaseTe
 
     @NotNull
     PropertyDefinition mockPropertyDefinition(@NotNull String declaringNt, boolean mv) throws Exception {
-        PropertyDefinition def = Mockito.mock(PropertyDefinition.class);
+        PropertyDefinition def = mock(PropertyDefinition.class);
         when(def.isMultiple()).thenReturn(mv);
         when(def.getDeclaringNodeType()).thenReturn(ReadOnlyNodeTypeManager.getInstance(root, getNamePathMapper()).getNodeType(declaringNt));
         return def;
@@ -200,70 +201,6 @@ public abstract class UserImporterBaseTe
 
     @NotNull
     NodeInfo createNodeInfo(@NotNull String name, @NotNull String primaryTypeName) {
-        return new NodeInfo(name, primaryTypeName, ImmutableList.<String>of(), null);
-    }
-
-    //--------------------------------------------------------------------------
-
-    final class TestAction implements AuthorizableAction, GroupAction {
-
-        private List<String> methodCalls = new ArrayList();
-
-        private void clear() {
-            methodCalls.clear();
-        }
-
-        void checkMethods(String... expected) {
-            assertEquals(ImmutableList.copyOf(expected), methodCalls);
-        }
-
-        @Override
-        public void init(SecurityProvider securityProvider, ConfigurationParameters config) {
-        }
-
-        @Override
-        public void onCreate(Group group, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onCreate-Group");
-        }
-
-        @Override
-        public void onCreate(User user, String password, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onCreate-User");
-        }
-
-        @Override
-        public void onRemove(Authorizable authorizable, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onRemove");
-        }
-
-        @Override
-        public void onPasswordChange(User user, String newPassword, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onPasswordChange");
-        }
-
-        @Override
-        public void onMemberAdded(Group group, Authorizable member, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onMemberAdded");
-        }
-
-        @Override
-        public void onMembersAdded(Group group, Iterable<String> memberIds, Iterable<String> failedIds, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onMembersAdded");
-        }
-
-        @Override
-        public void onMembersAddedContentId(Group group, Iterable<String> memberContentIds, Iterable<String> failedIds, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onMembersAddedContentId");
-        }
-
-        @Override
-        public void onMemberRemoved(Group group, Authorizable member, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onMemberRemoved");
-        }
-
-        @Override
-        public void onMembersRemoved(Group group, Iterable<String> memberIds, Iterable<String> failedIds, Root root, NamePathMapper namePathMapper) throws RepositoryException {
-            methodCalls.add("onMembersRemoved");
-        }
+        return new NodeInfo(name, primaryTypeName, ImmutableList.of(), null);
     }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationAbortTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationAbortTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationAbortTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationAbortTest.java Tue Sep 29 15:59:10 2020
@@ -19,12 +19,14 @@ package org.apache.jackrabbit.oak.securi
 import javax.jcr.nodetype.ConstraintViolationException;
 
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
+import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 
 import static org.junit.Assert.assertTrue;
 
 public class UserImporterImpersonationAbortTest extends UserImporterImpersonationIgnoreTest {
 
+    @NotNull
     @Override
     String getImportBehavior() {
         return ImportBehavior.NAME_ABORT;
@@ -38,7 +40,16 @@ public class UserImporterImpersonationAb
 
     @Test(expected = ConstraintViolationException.class)
     public void testMixedImpersonators() throws Exception {
-        assertTrue(importer.handlePropInfo(userTree, createPropInfo(REP_IMPERSONATORS, "impersonator1", testUser.getPrincipal().getName()), mockPropertyDefinition(NT_REP_USER, true)));
-        importer.processReferences();
+        super.testMixedImpersonators();
+    }
+
+    @Test(expected = ConstraintViolationException.class)
+    public void testGrantImpersonationGroupPrincipal() throws Exception {
+        super.testGrantImpersonationGroupPrincipal();
+    }
+
+    @Test(expected = ConstraintViolationException.class)
+    public void testRevokeImpersonationAlreadyRemoved() throws Exception {
+        super.testRevokeImpersonationAlreadyRemoved();
     }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationBestEffortTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationBestEffortTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationBestEffortTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationBestEffortTest.java Tue Sep 29 15:59:10 2020
@@ -20,6 +20,7 @@ import com.google.common.collect.Immutab
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
+import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -28,6 +29,7 @@ import static org.junit.Assert.assertTru
 
 public class UserImporterImpersonationBestEffortTest extends UserImporterImpersonationIgnoreTest {
 
+    @NotNull
     @Override
     String getImportBehavior() {
         return ImportBehavior.NAME_BESTEFFORT;

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationIgnoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationIgnoreTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationIgnoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterImpersonationIgnoreTest.java Tue Sep 29 15:59:10 2020
@@ -16,20 +16,34 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-import javax.jcr.RepositoryException;
-
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.oak.api.PropertyState;
+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.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
+import org.apache.jackrabbit.oak.spi.security.user.action.UserAction;
 import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import javax.jcr.RepositoryException;
+
+import java.security.Principal;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.when;
 
 public class UserImporterImpersonationIgnoreTest extends UserImporterBaseTest {
 
@@ -39,7 +53,7 @@ public class UserImporterImpersonationIg
     public void before() throws Exception {
         super.before();
 
-        init();
+        init(true, UserAction.class);
         userTree = createUserTree();
     }
 
@@ -55,6 +69,7 @@ public class UserImporterImpersonationIg
 
     @Test
     public void testKnownImpersonators() throws Exception {
+        userTree.setProperty(UserConstants.REP_PRINCIPAL_NAME, TEST_USER_ID);
         assertTrue(importer.handlePropInfo(userTree, createPropInfo(REP_IMPERSONATORS, testUser.getPrincipal().getName()), mockPropertyDefinition(NT_REP_USER, true)));
         importer.processReferences();
 
@@ -100,6 +115,18 @@ public class UserImporterImpersonationIg
     }
 
     @Test
+    public void testReplaceExistingProperty2() throws Exception {
+        userTree.setProperty(REP_IMPERSONATORS, ImmutableList.of("impersonator1"), Type.STRINGS);
+
+        assertTrue(importer.handlePropInfo(userTree, createPropInfo(REP_IMPERSONATORS, "impersonator1", testUser.getPrincipal().getName()), mockPropertyDefinition(NT_REP_USER, true)));
+        importer.processReferences();
+
+        PropertyState impersonators = userTree.getProperty(REP_IMPERSONATORS);
+        assertNotNull(impersonators);
+        assertEquals(ImmutableSet.of("impersonator1", testUser.getPrincipal().getName()), ImmutableSet.copyOf(impersonators.getValue(Type.STRINGS)));
+    }
+
+    @Test
     public void testNewImpersonator() throws Exception {
         Tree folder = root.getTree(getUserConfiguration().getParameters().getConfigValue(PARAM_USER_PATH, DEFAULT_USER_PATH));
         Tree impersonatorTree = folder.addChild("impersonatorTree");
@@ -133,4 +160,30 @@ public class UserImporterImpersonationIg
         assertNotNull(impersonators);
         assertEquals(ImmutableList.of("impersonator1"), impersonators.getValue(Type.STRINGS));
     }
+
+    @Test
+    public void testGrantImpersonationGroupPrincipal() throws Exception {
+        assertTrue(importer.handlePropInfo(userTree, createPropInfo(REP_IMPERSONATORS, EveryonePrincipal.NAME), mockPropertyDefinition(NT_REP_USER, true)));
+        importer.processReferences();
+
+        PropertyState impersonators = userTree.getProperty(REP_IMPERSONATORS);
+        assertNull(impersonators);
+    }
+
+    @Test
+    public void testRevokeImpersonationAlreadyRemoved() throws Exception {
+        UserAction ua = (UserAction) testAction;
+        doAnswer(invocationOnMock -> {
+            userTree.removeProperty(REP_IMPERSONATORS);
+            return null;
+        }).when(ua).onRevokeImpersonation(any(User.class), any(Principal.class), any(Root.class), any(NamePathMapper.class));
+
+        userTree.setProperty(REP_IMPERSONATORS, ImmutableList.of("impersonator1"), Type.STRINGS);
+
+        assertTrue(importer.handlePropInfo(userTree, createPropInfo(REP_IMPERSONATORS), mockPropertyDefinition(NT_REP_USER, true)));
+        importer.processReferences();
+
+        PropertyState impersonators = userTree.getProperty(REP_IMPERSONATORS);
+        assertNull(impersonators);
+    }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipAbortTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipAbortTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipAbortTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipAbortTest.java Tue Sep 29 15:59:10 2020
@@ -23,12 +23,14 @@ import org.apache.jackrabbit.oak.api.Tre
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
+import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 
 import static org.junit.Assert.assertTrue;
 
 public class UserImporterMembershipAbortTest extends UserImporterMembershipIgnoreTest {
 
+    @NotNull
     @Override
     String getImportBehavior() {
         return ImportBehavior.NAME_ABORT;

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipBesteffortTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipBesteffortTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipBesteffortTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipBesteffortTest.java Tue Sep 29 15:59:10 2020
@@ -18,12 +18,14 @@ package org.apache.jackrabbit.oak.securi
 
 import com.google.common.collect.ImmutableList;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
+import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 
 import static org.junit.Assert.assertTrue;
 
 public class UserImporterMembershipBesteffortTest extends UserImporterMembershipIgnoreTest {
 
+    @NotNull
     @Override
     String getImportBehavior() {
         return ImportBehavior.NAME_BESTEFFORT;

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipIgnoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipIgnoreTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipIgnoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterMembershipIgnoreTest.java Tue Sep 29 15:59:10 2020
@@ -23,6 +23,7 @@ import javax.jcr.RepositoryException;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
@@ -30,6 +31,7 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
 import org.junit.Test;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -39,7 +41,7 @@ import static org.junit.Assert.assertTru
 public class UserImporterMembershipIgnoreTest extends UserImporterBaseTest {
 
     Tree groupTree;
-    Tree memberRefList;
+    private Tree memberRefList;
 
     UserProvider userProvider;
 
@@ -88,7 +90,7 @@ public class UserImporterMembershipIgnor
 
     @Test
     public void testKnownMemberThresholdReached() throws Exception {
-        List<String> memberIds = new ArrayList();
+        List<String> memberIds = new ArrayList<>();
         for (int i = 0; i <= MembershipWriter.DEFAULT_MEMBERSHIP_THRESHOLD; i++) {
             memberIds.add(userProvider.getContentID("m"+i));
         }
@@ -208,4 +210,22 @@ public class UserImporterMembershipIgnor
         assertNotNull(members);
         assertEquals(ImmutableList.of(knownMemberContentId), ImmutableList.copyOf(members.getValue(Type.STRINGS)));
     }
+
+    @Test
+    public void testAddExistingMembers() throws Exception {
+        Tree userTree = createUserTree();
+        String contentId = userProvider.getContentID(userTree);
+
+        // member to be imported has already been added before
+        Group gr = (Group) ((UserManagerImpl) getUserManager(root)).getAuthorizable(groupTree);
+        checkNotNull(gr).addMembers(TEST_USER_ID);
+
+        assertTrue(importer.handlePropInfo(userTree, createPropInfo(REP_AUTHORIZABLE_ID, TEST_USER_ID), mockPropertyDefinition(NT_REP_AUTHORIZABLE, false)));
+        assertTrue(importer.handlePropInfo(groupTree, createPropInfo(REP_MEMBERS, contentId), mockPropertyDefinition(NT_REP_MEMBER_REFERENCES, true)));
+        importer.processReferences();
+
+        PropertyState members = groupTree.getProperty(REP_MEMBERS);
+        assertNotNull(members);
+        assertEquals(ImmutableList.of(contentId), ImmutableList.copyOf(members.getValue(Type.STRINGS)));
+    }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterPasswordTreeTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterPasswordTreeTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterPasswordTreeTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterPasswordTreeTest.java Tue Sep 29 15:59:10 2020
@@ -40,7 +40,6 @@ import static org.mockito.Mockito.when;
 
 public class UserImporterPasswordTreeTest extends UserImporterBaseTest {
 
-    private Tree userTree;
     private Tree pwTree;
 
     @Override
@@ -48,7 +47,7 @@ public class UserImporterPasswordTreeTes
         super.before();
 
         init();
-        userTree = createUserTree();
+        Tree userTree = createUserTree();
         pwTree = TreeUtil.addChild(userTree, REP_PWD, NT_REP_PASSWORD);
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterSessionAutosaveTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterSessionAutosaveTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterSessionAutosaveTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterSessionAutosaveTest.java Tue Sep 29 15:59:10 2020
@@ -64,9 +64,9 @@ public class UserImporterSessionAutosave
     }
 
     @Override
-    boolean init(boolean createAction) throws Exception {
+    boolean init(boolean createAction, Class<?>... extraInterfaces) throws Exception {
         getUserManager(root).autoSave(false);
-        boolean b = super.init(createAction);
+        boolean b = super.init(createAction, extraInterfaces);
         getUserManager(root).autoSave(true);
         return b;
     }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java Tue Sep 29 15:59:10 2020
@@ -16,31 +16,34 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-import javax.jcr.ImportUUIDBehavior;
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
-import javax.jcr.nodetype.ConstraintViolationException;
-import javax.jcr.nodetype.PropertyDefinition;
-
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterators;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.JackrabbitSession;
 import org.apache.jackrabbit.api.security.user.AuthorizableExistsException;
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.oak.api.PropertyState;
+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.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.user.UserConstants;
+import org.apache.jackrabbit.oak.spi.security.user.action.AuthorizableAction;
 import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil;
 import org.apache.jackrabbit.oak.spi.xml.PropInfo;
 import org.apache.jackrabbit.oak.spi.xml.ReferenceChangeTracker;
 import org.junit.Test;
-import org.mockito.Mockito;
 
+import javax.jcr.ImportUUIDBehavior;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.nodetype.ConstraintViolationException;
+import javax.jcr.nodetype.PropertyDefinition;
 import java.lang.reflect.Field;
 import java.util.Collections;
 import java.util.Map;
@@ -51,12 +54,14 @@ import static org.junit.Assert.assertFal
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.nullable;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.when;
 
 public class UserImporterTest extends UserImporterBaseTest implements UserConstants {
@@ -100,13 +105,12 @@ public class UserImporterTest extends Us
         assertFalse(importer.init(mockJackrabbitSession(), root, getNamePathMapper(), isWorkspaceImport(), ImportUUIDBehavior.IMPORT_UUID_CREATE_NEW, new ReferenceChangeTracker(), getSecurityProvider()));
     }
 
+    //-----------------------------------------------------< handlePropInfo >---
     @Test(expected = IllegalStateException.class)
     public void testHandlePropInfoNotInitialized() throws Exception {
         importer.handlePropInfo(createUserTree(), mock(PropInfo.class), mock(PropertyDefinition.class));
     }
 
-    //-----------------------------------------------------< handlePropInfo >---
-
     @Test
     public void testHandlePropInfoParentNotAuthorizable() throws Exception {
         init();
@@ -238,6 +242,36 @@ public class UserImporterTest extends Us
     }
 
     @Test
+    public void testHandlePwNodePropertyInvalidDef() throws Exception {
+        init();
+        Tree userTree = createUserTree();
+        Tree pwHistory = TreeUtil.addChild(userTree, REP_PWD, NT_REP_PASSWORD);
+        PropertyDefinition pd = mockPropertyDefinition(NT_REP_PASSWORD, false);
+        when(pd.getName()).thenReturn(NodeTypeConstants.RESIDUAL_NAME);
+        assertFalse(importer.handlePropInfo(pwHistory, createPropInfo(null, PasswordUtil.buildPasswordHash("pw")), pd));
+    }
+
+    @Test
+    public void testHandlePwNodePropertyInvalidDef2() throws Exception {
+        init();
+        Tree userTree = createUserTree();
+        Tree pwHistory = TreeUtil.addChild(userTree, REP_PWD, NT_REP_PASSWORD);
+        PropertyDefinition pd = mockPropertyDefinition(NT_REP_PASSWORD, false);
+        when(pd.getName()).thenReturn(null);
+        assertFalse(importer.handlePropInfo(pwHistory, createPropInfo(null, PasswordUtil.buildPasswordHash("pw")), pd));
+    }
+
+    @Test
+    public void testHandlePwNodePropertyValidDef() throws Exception {
+        init();
+        Tree userTree = createUserTree();
+        Tree pwHistory = TreeUtil.addChild(userTree, REP_PWD, NT_REP_PASSWORD);
+        PropertyDefinition pd = mockPropertyDefinition(NT_REP_PASSWORD, true);
+        when(pd.getName()).thenReturn(REP_PWD_HISTORY);
+        assertTrue(importer.handlePropInfo(pwHistory, createPropInfo(null, PasswordUtil.buildPasswordHash("pw")), pd));
+    }
+
+    @Test
     public void testHandleImpersonators() throws Exception {
         init();
         Tree userTree = createUserTree();
@@ -421,7 +455,7 @@ public class UserImporterTest extends Us
     @Test
     public void testPropertiesCompletedIdPresent() throws Exception {
         init();
-        testAction = new TestAction();
+        testAction = mock(AuthorizableAction.class);
 
         Tree userTree = createUserTree();
         userTree.setProperty(REP_AUTHORIZABLE_ID, "userId");
@@ -436,7 +470,7 @@ public class UserImporterTest extends Us
     public void testPropertiesCompletedNewUser() throws Exception {
         init(true);
         importer.propertiesCompleted(createUserTree());
-        testAction.checkMethods("onCreate-User");
+        verify(testAction, times(1)).onCreate(any(User.class), nullable(String.class), any(Root.class), any(NamePathMapper.class));
     }
 
     @Test
@@ -444,7 +478,7 @@ public class UserImporterTest extends Us
         init(true);
         importer.propertiesCompleted(createSystemUserTree());
         // create-actions must not be called for system users
-        testAction.checkMethods();
+        verifyNoInteractions(testAction);
     }
 
     @Test
@@ -453,14 +487,14 @@ public class UserImporterTest extends Us
 
         init(true);
         importer.propertiesCompleted(groupTree);
-        testAction.checkMethods("onCreate-Group");
+        verify(testAction, times(1)).onCreate(any(Group.class), any(Root.class), any(NamePathMapper.class));
     }
 
     @Test
     public void testPropertiesCompletedExistingUser() throws Exception {
         init(true);
         importer.propertiesCompleted(root.getTree(testUser.getPath()));
-        testAction.checkMethods();
+        verifyNoInteractions(testAction);
     }
 
     //--------------------------------------------------------------< start >---
@@ -562,7 +596,7 @@ public class UserImporterTest extends Us
     //-----------------------------------------------------< startChildInfo >---
 
     @Test(expected = IllegalStateException.class)
-    public void testStartChildInfoIllegalState() throws Exception {
+    public void testStartChildInfoIllegalState() {
         importer.startChildInfo(createNodeInfo("memberRef", NT_REP_MEMBER_REFERENCES), ImmutableList.of(createPropInfo(REP_MEMBERS, "member1")));
     }
 
@@ -584,7 +618,7 @@ public class UserImporterTest extends Us
         memberRefList.setProperty(JcrConstants.JCR_PRIMARYTYPE, NT_REP_MEMBER_REFERENCES_LIST);
 
         importer.start(memberRefList);
-        importer.startChildInfo(createNodeInfo("memberRef", NT_REP_MEMBER_REFERENCES), ImmutableList.<PropInfo>of());
+        importer.startChildInfo(createNodeInfo("memberRef", NT_REP_MEMBER_REFERENCES), ImmutableList.of());
     }
 
     @Test
@@ -622,7 +656,7 @@ public class UserImporterTest extends Us
         Tree repMembers = groupTree.addChild("memberTree");
         repMembers.setProperty(JcrConstants.JCR_PRIMARYTYPE, NT_REP_MEMBERS);
         importer.start(repMembers);
-        importer.startChildInfo(createNodeInfo("memberTree", NT_REP_MEMBERS), ImmutableList.<PropInfo>of(createPropInfo("anyProp", "memberValue")));
+        importer.startChildInfo(createNodeInfo("memberTree", NT_REP_MEMBERS), ImmutableList.of(createPropInfo("anyProp", "memberValue")));
     }
 
     @Test
@@ -658,5 +692,4 @@ public class UserImporterTest extends Us
         importer.end(memberRefList);
         assertNull(f.get(importer));
     }
-
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserInitializerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserInitializerTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserInitializerTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserInitializerTest.java Tue Sep 29 15:59:10 2020
@@ -160,7 +160,7 @@ public class UserInitializerTest extends
      */
     @Test
     public void testAdminConfiguration() throws Exception {
-        Map<String,Object> userParams = new HashMap();
+        Map<String,Object> userParams = new HashMap<>();
         userParams.put(UserConstants.PARAM_ADMIN_ID, "admin");
         userParams.put(UserConstants.PARAM_OMIT_ADMIN_PW, true);
 
@@ -173,13 +173,7 @@ public class UserInitializerTest extends
                 .with(sp)
                 .createContentRepository();
 
-        ContentSession cs = Subject.doAs(SystemSubject.INSTANCE, new PrivilegedExceptionAction<ContentSession>() {
-            @Override
-            public ContentSession run() throws Exception {
-                return repo.login(null, null);
-            }
-        });
-        try {
+        try (ContentSession cs = Subject.doAs(SystemSubject.INSTANCE, (PrivilegedExceptionAction<ContentSession>) () -> repo.login(null, null))) {
             Root root = cs.getLatestRoot();
             UserConfiguration uc = sp.getConfiguration(UserConfiguration.class);
             UserManager umgr = uc.getUserManager(root, NamePathMapper.DEFAULT);
@@ -189,21 +183,13 @@ public class UserInitializerTest extends
             Tree adminTree = root.getTree(adminUser.getPath());
             assertTrue(adminTree.exists());
             assertNull(adminTree.getProperty(UserConstants.REP_PASSWORD));
-        } finally {
-            cs.close();
         }
 
         // login as admin should fail
-        ContentSession adminSession = null;
-        try {
-            adminSession = repo.login(new SimpleCredentials("admin", new char[0]), null);
+        try (ContentSession adminSession = repo.login(new SimpleCredentials("admin", new char[0]), null)) {
             fail();
         } catch (LoginException e) {
             //success
-        } finally {
-            if (adminSession != null) {
-                adminSession.close();
-            }
         }
     }
 
@@ -212,7 +198,7 @@ public class UserInitializerTest extends
      */
     @Test
     public void testAnonymousConfiguration() throws Exception {
-        Map<String,Object> userParams = new HashMap();
+        Map<String,Object> userParams = new HashMap<>();
         userParams.put(UserConstants.PARAM_ANONYMOUS_ID, "");
 
         ConfigurationParameters params = ConfigurationParameters.of(UserConfiguration.NAME, ConfigurationParameters.of(userParams));
@@ -224,33 +210,19 @@ public class UserInitializerTest extends
                 .with(sp)
                 .createContentRepository();
 
-        ContentSession cs = Subject.doAs(SystemSubject.INSTANCE, new PrivilegedExceptionAction<ContentSession>() {
-            @Override
-            public ContentSession run() throws Exception {
-                return repo.login(null, null);
-            }
-        });
-        try {
+        try (ContentSession cs = Subject.doAs(SystemSubject.INSTANCE, (PrivilegedExceptionAction<ContentSession>) () -> repo.login(null, null))) {
             Root root = cs.getLatestRoot();
             UserConfiguration uc = sp.getConfiguration(UserConfiguration.class);
             UserManager umgr = uc.getUserManager(root, NamePathMapper.DEFAULT);
             Authorizable anonymous = umgr.getAuthorizable(UserConstants.DEFAULT_ANONYMOUS_ID);
             assertNull(anonymous);
-        } finally {
-            cs.close();
         }
 
         // login as admin should fail
-        ContentSession anonymousSession = null;
-        try {
-            anonymousSession = repo.login(new GuestCredentials(), null);
+        try (ContentSession anonymousSession = repo.login(new GuestCredentials(), null)) {
             fail();
         } catch (LoginException e) {
             //success
-        } finally {
-            if (anonymousSession != null) {
-                anonymousSession.close();
-            }
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java Tue Sep 29 15:59:10 2020
@@ -211,18 +211,13 @@ public class UserManagerImplTest extends
     }
 
     @Test
-    public void testIsAutoSave() throws Exception {
+    public void testIsAutoSave() {
         assertFalse(userMgr.isAutoSave());
     }
 
-    @Test
+    @Test(expected = UnsupportedRepositoryOperationException.class)
     public void testAutoSave() throws Exception {
-        try {
-            userMgr.autoSave(true);
-            fail("should fail");
-        } catch (UnsupportedRepositoryOperationException e) {
-            // success
-        }
+        userMgr.autoSave(true);
     }
 
     @Test
@@ -397,17 +392,12 @@ public class UserManagerImplTest extends
 
     @Test(expected = IllegalArgumentException.class)
     public void testCreateGroupWithNullId() throws RepositoryException {
-        userMgr.createGroup((String) null, new PrincipalImpl("groupPrincipalName"), null);
+        userMgr.createGroup(null, new PrincipalImpl("groupPrincipalName"), null);
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void testCreateUserWithEmptyPrincipalName() throws Exception {
-        userMgr.createUser("another", null, new Principal() {
-            @Override
-            public String getName() {
-                return "";
-            }
-        }, null);
+        userMgr.createUser("another", null, () -> "", null);
     }
 
     @Test(expected = IllegalArgumentException.class)

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderTest.java Tue Sep 29 15:59:10 2020
@@ -24,21 +24,28 @@ import org.apache.jackrabbit.api.securit
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.api.security.user.UserManager;
+import org.apache.jackrabbit.oak.api.QueryEngine;
+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.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.security.principal.AbstractPrincipalProviderTest;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.apache.jackrabbit.oak.spi.security.principal.AdminPrincipal;
 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.principal.PrincipalProvider;
 import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 
 import java.security.Principal;
+import java.text.ParseException;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 
@@ -48,7 +55,14 @@ import static org.apache.jackrabbit.oak.
 import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.REP_PRINCIPAL_NAME;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class UserPrincipalProviderTest extends AbstractPrincipalProviderTest {
 
@@ -284,4 +298,16 @@ public class UserPrincipalProviderTest e
             assertTrue(Iterators.elementsEqual(i1, i2));
         }
     }
+
+    @Test
+    public void testFindPrincipalsQueryFails() throws ParseException {
+        QueryEngine qe = mock(QueryEngine.class);
+        when(qe.executeQuery(anyString(), anyString(), anyLong(), anyLong(), any(Map.class), any(Map.class))).thenThrow(new ParseException("err",0));
+
+        Root r = when(mock(Root.class).getQueryEngine()).thenReturn(qe).getMock();
+        UserPrincipalProvider upp = new UserPrincipalProvider(r, getUserConfiguration(), NamePathMapper.DEFAULT);
+        Iterator<? extends Principal> it = upp.findPrincipals("a", false, PrincipalManager.SEARCH_TYPE_ALL, -1, -1);
+        assertNotNull(it);
+        assertFalse(it.hasNext());
+    }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderWithCacheTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderWithCacheTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderWithCacheTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserPrincipalProviderWithCacheTest.java Tue Sep 29 15:59:10 2020
@@ -16,11 +16,8 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import org.apache.jackrabbit.JcrConstants;
-import org.apache.jackrabbit.api.security.principal.GroupPrincipal;
 import org.apache.jackrabbit.api.security.principal.PrincipalIterator;
 import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.api.security.user.Group;
@@ -42,26 +39,23 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.apache.jackrabbit.oak.spi.security.principal.PrincipalProvider;
 import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 import org.junit.Test;
 
 import javax.jcr.NoSuchWorkspaceException;
-import javax.jcr.RepositoryException;
 import javax.jcr.SimpleCredentials;
 import javax.security.auth.Subject;
 import javax.security.auth.login.LoginException;
 import java.security.Principal;
 import java.security.PrivilegedExceptionAction;
 import java.util.ArrayList;
-import java.util.Enumeration;
 import java.util.List;
 import java.util.Set;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -116,28 +110,21 @@ public class UserPrincipalProviderWithCa
 
     private ContentSession getSystemSession() throws Exception {
         if (systemSession == null) {
-            systemSession = Subject.doAs(SystemSubject.INSTANCE, new PrivilegedExceptionAction<ContentSession>() {
-                @Override
-                public ContentSession run() throws LoginException, NoSuchWorkspaceException {
-                    return login(null);
-
-                }
-            });
+            systemSession = Subject.doAs(SystemSubject.INSTANCE, (PrivilegedExceptionAction<ContentSession>) () -> login(null));
         }
         return systemSession;
     }
 
-    private UserConfiguration changeUserConfiguration(ConfigurationParameters params) {
+    private void changeUserConfiguration(ConfigurationParameters params) {
         UserConfiguration userConfig = getUserConfiguration();
         ((ConfigurationBase) userConfig).setParameters(params);
-        return userConfig;
     }
 
     private Tree getCacheTree(Root root) throws Exception {
         return getCacheTree(root, getTestUser().getPath());
     }
 
-    private Tree getCacheTree(Root root, String authorizablePath) throws Exception {
+    private Tree getCacheTree(Root root, String authorizablePath) {
         return root.getTree(authorizablePath + '/' + CacheConstants.REP_CACHE);
     }
 
@@ -269,81 +256,6 @@ public class UserPrincipalProviderWithCa
     }
 
     @Test
-    public void testGroupPrincipals() throws Exception {
-        // a) force the cache to be created
-        PrincipalProvider pp = createPrincipalProvider(systemRoot);
-        Iterable<? extends Principal> principals = Iterables.filter(pp.getPrincipals(userId), new GroupPredicate());
-
-        for (Principal p : principals) {
-            String className = p.getClass().getName();
-            assertEquals("org.apache.jackrabbit.oak.security.user.UserPrincipalProvider$GroupPrincipalImpl", className);
-        }
-
-        Principal testPrincipal = getTestUser().getPrincipal();
-
-        // b) retrieve principals again (this time from the cache)
-        // -> verify that they are a different implementation
-        Iterable<? extends Principal> principalsAgain = Iterables.filter(pp.getPrincipals(userId), new GroupPredicate());
-        for (Principal p : principalsAgain) {
-            String className = p.getClass().getName();
-            assertEquals("org.apache.jackrabbit.oak.security.user.UserPrincipalProvider$CachedGroupPrincipal", className);
-
-            assertTrue(p instanceof TreeBasedPrincipal);
-            assertEquals(testGroup.getPath(), ((TreeBasedPrincipal) p).getPath());
-            assertEquals(testGroup.getPath(), ((TreeBasedPrincipal) p).getOakPath());
-
-            GroupPrincipal principalGroup = (GroupPrincipal) p;
-            assertTrue(principalGroup.isMember(testPrincipal));
-
-            Enumeration<? extends Principal> members = principalGroup.members();
-            assertTrue(members.hasMoreElements());
-            assertEquals(testPrincipal, members.nextElement());
-            assertEquals(testGroup2.getPrincipal(), members.nextElement());
-            assertFalse(members.hasMoreElements());
-        }
-    }
-
-    @Test
-    public void testCachedPrincipalsGroupRemoved() throws Exception {
-        // a) force the cache to be created
-        PrincipalProvider pp = createPrincipalProvider(systemRoot);
-        Iterable<? extends Principal> principals = Iterables.filter(pp.getPrincipals(userId), new GroupPredicate());
-
-        for (Principal p : principals) {
-            String className = p.getClass().getName();
-            assertEquals("org.apache.jackrabbit.oak.security.user.UserPrincipalProvider$GroupPrincipalImpl", className);
-        }
-
-        testGroup.remove();
-        root.commit();
-
-        systemRoot.refresh();
-
-        // b) retrieve principals again (this time from the cache)
-        //    principal for 'testGroup' is no longer backed by an user mgt group
-        //    verify that this doesn't lead to runtime exceptions
-        Iterable<? extends Principal> principalsAgain = Iterables.filter(pp.getPrincipals(userId), new GroupPredicate());
-        for (Principal p : principalsAgain) {
-            String className = p.getClass().getName();
-            assertEquals("org.apache.jackrabbit.oak.security.user.UserPrincipalProvider$CachedGroupPrincipal", className);
-
-            assertTrue(p instanceof TreeBasedPrincipal);
-            try {
-                ((TreeBasedPrincipal) p).getPath();
-                fail("RepositoryException expected");
-            } catch (RepositoryException e) {
-                // success
-            }
-
-            GroupPrincipal principalGroup = (GroupPrincipal) p;
-            assertFalse(principalGroup.isMember(getTestUser().getPrincipal()));
-
-            Enumeration<? extends Principal> members = principalGroup.members();
-            assertFalse(members.hasMoreElements());
-        }
-    }
-
-    @Test
     public void testGroupPrincipalNameEscape() throws Exception {
         String gId = null;
         try {
@@ -417,7 +329,7 @@ public class UserPrincipalProviderWithCa
         // retrieve principals again to have cache updated
         pp = createPrincipalProvider(systemRoot);
         Set<? extends Principal> principalsAgain = pp.getPrincipals(userId);
-        assertFalse(principals.equals(principalsAgain));
+        assertNotEquals(principals, principalsAgain);
         assertPrincipals(principalsAgain, EveryonePrincipal.getInstance(), getTestUser().getPrincipal());
 
         // verify that the cache has really been updated
@@ -427,7 +339,7 @@ public class UserPrincipalProviderWithCa
 
         // check that an cached empty membership set doesn't break the retrieval (OAK-8306)
         principalsAgain = pp.getPrincipals(userId);
-        assertFalse(principals.equals(principalsAgain));
+        assertNotEquals(principals, principalsAgain);
         assertPrincipals(principalsAgain, EveryonePrincipal.getInstance(), getTestUser().getPrincipal());
     }
 
@@ -450,7 +362,7 @@ public class UserPrincipalProviderWithCa
         // not causing NPE although the property is missing
         pp = createPrincipalProvider(systemRoot);
         Set<? extends Principal> principalsAgain = pp.getPrincipals(userId);
-        assertTrue(principals.equals(principalsAgain));
+        assertEquals(principals, principalsAgain);
 
         // verify that the cache has really been updated
         cache = getCacheTree(systemRoot);
@@ -540,31 +452,19 @@ public class UserPrincipalProviderWithCa
         props.add(PropertyStates.createProperty(JcrConstants.JCR_PRIMARYTYPE, JcrConstants.NT_UNSTRUCTURED));
         props.add(PropertyStates.createProperty("residualProp", "anyvalue"));
 
-        // changing cache with (normally) sufficiently privileged session must not succeed
-        for (PropertyState ps : props) {
-            try {
-                Tree cache = getCacheTree(root);
-                cache.setProperty(ps);
-                root.commit();
-                fail("Attempt to modify the cache tree must fail.");
-            } catch (CommitFailedException e) {
-                // success
-            } finally {
-                root.refresh();
-            }
-        }
-
-        // changing cache with system session must not succeed either
-        for (PropertyState ps : props) {
-            try {
-                Tree cache = getCacheTree(systemRoot);
-                cache.setProperty(ps);
-                systemRoot.commit();
-                fail("Attempt to modify the cache tree must fail.");
-            } catch (CommitFailedException e) {
-                // success
-            } finally {
-                systemRoot.refresh();
+        // changing cache with (normally) sufficiently privileged session and with system-session must not succeed
+        for (Root r : new Root[] {root, systemRoot}) {
+            for (PropertyState ps : props) {
+                try {
+                    Tree cache = getCacheTree(r);
+                    cache.setProperty(ps);
+                    r.commit();
+                    fail("Attempt to modify the cache tree must fail.");
+                } catch (CommitFailedException e) {
+                    // success
+                } finally {
+                    r.refresh();
+                }
             }
         }
     }
@@ -585,16 +485,14 @@ public class UserPrincipalProviderWithCa
     public void testConcurrentLoginWithCacheRemoval() throws Exception {
         changeUserConfiguration(ConfigurationParameters.of(UserPrincipalProvider.PARAM_CACHE_EXPIRATION, 1));
 
-        final List<Exception> exceptions = new ArrayList<Exception>();
-        List<Thread> threads = new ArrayList<Thread>();
+        final List<Exception> exceptions = new ArrayList<>();
+        List<Thread> threads = new ArrayList<>();
         for (int i = 0; i < 100; i++) {
-            threads.add(new Thread(new Runnable() {
-                public void run() {
-                    try {
-                        login(new SimpleCredentials(userId, userId.toCharArray())).close();
-                    } catch (Exception e) {
-                        exceptions.add(e);
-                    }
+            threads.add(new Thread(() -> {
+                try {
+                    login(new SimpleCredentials(userId, userId.toCharArray())).close();
+                } catch (Exception e) {
+                    exceptions.add(e);
                 }
             }));
         }
@@ -611,13 +509,4 @@ public class UserPrincipalProviderWithCa
             fail();
         }
     }
-
-    //--------------------------------------------------------------------------
-
-    private static final class GroupPredicate implements Predicate<Principal> {
-        @Override
-        public boolean apply(@Nullable Principal input) {
-            return (input instanceof GroupPrincipal) && !EveryonePrincipal.getInstance().equals(input);
-        }
-    }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserProviderTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserProviderTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserProviderTest.java Tue Sep 29 15:59:10 2020
@@ -16,8 +16,10 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
+import java.text.ParseException;
 import java.util.HashMap;
 import java.util.Map;
+import javax.jcr.AccessDeniedException;
 import javax.jcr.RepositoryException;
 import javax.jcr.nodetype.NodeDefinition;
 import javax.jcr.nodetype.NodeType;
@@ -25,16 +27,20 @@ import javax.jcr.nodetype.PropertyDefini
 
 import org.apache.jackrabbit.oak.Oak;
 import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.api.QueryEngine;
 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.namepath.NamePathMapper;
 import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
+import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
 import org.apache.jackrabbit.oak.plugins.nodetype.ReadOnlyNodeTypeManager;
 import org.apache.jackrabbit.oak.InitialContent;
 import org.apache.jackrabbit.oak.InitialContentHelper;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.oak.spi.security.user.AuthorizableNodeName;
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 import org.apache.jackrabbit.oak.spi.security.user.util.UserUtil;
@@ -44,10 +50,19 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
+import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.NT_REP_AUTHORIZABLE_FOLDER;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 /**
  * @since OAK 1.0
@@ -76,7 +91,7 @@ public class UserProviderTest {
         defaultUserPath = defaultConfig.getConfigValue(UserConstants.PARAM_USER_PATH, UserConstants.DEFAULT_USER_PATH);
         defaultGroupPath = defaultConfig.getConfigValue(UserConstants.PARAM_GROUP_PATH, UserConstants.DEFAULT_GROUP_PATH);
 
-        customOptions = new HashMap<String, Object>();
+        customOptions = new HashMap<>();
         customOptions.put(UserConstants.PARAM_GROUP_PATH, customGroupPath);
         customOptions.put(UserConstants.PARAM_USER_PATH, customUserPath);
     }
@@ -92,13 +107,13 @@ public class UserProviderTest {
     }
 
     private UserProvider createUserProvider(int defaultDepth) {
-        Map<String, Object> options = new HashMap<String, Object>(customOptions);
+        Map<String, Object> options = new HashMap<>(customOptions);
         options.put(UserConstants.PARAM_DEFAULT_DEPTH, defaultDepth);
         return new UserProvider(root, ConfigurationParameters.of(options));
     }
 
     private UserProvider createUserProviderRFC7612() {
-        Map<String, Object> options = new HashMap<String, Object>(customOptions);
+        Map<String, Object> options = new HashMap<>(customOptions);
         options.put(UserConstants.PARAM_ENABLE_RFC7613_USERCASE_MAPPED_PROFILE, true);
         return new UserProvider(root, ConfigurationParameters.of(options));
     }
@@ -119,7 +134,7 @@ public class UserProviderTest {
         userTree = up.createUser("b", null);
         assertEquals(defaultUserPath + "/b/bb/b", userTree.getPath());
 
-        Map<String, String> m = new HashMap<String,String>();
+        Map<String, String> m = new HashMap<>();
         m.put("bb",     "/b/bb/bb");
         m.put("bbb",    "/b/bb/bbb");
         m.put("bbbb",   "/b/bb/bbbb");
@@ -146,6 +161,18 @@ public class UserProviderTest {
         assertEquals(userPath, userTree.getPath());
     }
 
+    @Test(expected = AccessDeniedException.class)
+    public void testCreateUserMissingAccessOnFolders() throws RepositoryException {
+        Tree t = mock(Tree.class);
+        when(t.getParent()).thenReturn(t);
+        when(t.exists()).thenReturn(false);
+        when(t.isRoot()).thenReturn(false, false, true);
+        Root r = when(mock(Root.class).getTree(anyString())).thenReturn(t).getMock();
+
+        UserProvider up = new UserProvider(r, ConfigurationParameters.EMPTY);
+        up.createUser("uid", null);
+    }
+
     @Test
     public void testCreateGroup() throws RepositoryException {
         UserProvider up = createUserProvider();
@@ -178,7 +205,7 @@ public class UserProviderTest {
         Tree userTree = userProvider.createUser("b", null);
         assertEquals(customUserPath + "/b/bb/bbb/b", userTree.getPath());
 
-        Map<String, String> m = new HashMap<String,String>();
+        Map<String, String> m = new HashMap<>();
         m.put("bb",     "/b/bb/bbb/bb");
         m.put("bbb",    "/b/bb/bbb/bbb");
         m.put("bbbb",   "/b/bb/bbb/bbbb");
@@ -197,17 +224,17 @@ public class UserProviderTest {
     public void testCreateWithCollision() throws Exception {
         UserProvider userProvider = createUserProvider();
 
-        Tree userTree = userProvider.createUser("AmaLia", null);
+        userProvider.createUser("AmaLia", null);
 
-        Map<String, String> colliding = new HashMap<String, String>();
+        Map<String, String> colliding = new HashMap<>();
         colliding.put("AmaLia", null);
-        colliding.put("AmaLia", "s/ome/path");
+        colliding.put("AmalIa", "s/ome/path");
         colliding.put("amalia", null);
         colliding.put("Amalia", "a/b/c");
 
         for (String uid : colliding.keySet()) {
             try {
-                Tree c = userProvider.createUser(uid, colliding.get(uid));
+                userProvider.createUser(uid, colliding.get(uid));
                 root.commit();
                 fail("userID collision must be detected");
             } catch (CommitFailedException e) {
@@ -217,7 +244,7 @@ public class UserProviderTest {
 
         for (String uid : colliding.keySet()) {
             try {
-                Tree c = userProvider.createGroup(uid, colliding.get(uid));
+                userProvider.createGroup(uid, colliding.get(uid));
                 root.commit();
                 fail("userID collision must be detected");
             } catch (CommitFailedException e) {
@@ -227,6 +254,28 @@ public class UserProviderTest {
     }
 
     @Test
+    public void testCreateWithCollidingFolder() throws Exception {
+        Tree ut = mock(Tree.class);
+
+        Tree t = mock(Tree.class);
+        when(t.getParent()).thenReturn(t);
+        when(t.exists()).thenReturn(true);
+        when(t.isRoot()).thenReturn(true);
+        when(t.hasChild("uid")).thenReturn(true, false);
+        when(t.addChild("uid")).thenReturn(ut);
+        when(t.getChild(anyString())).thenReturn(t);
+        when(t.getPath()).thenReturn(UserConstants.DEFAULT_USER_PATH);
+        when(t.getProperty(JCR_PRIMARYTYPE)).thenReturn(PropertyStates.createProperty(JCR_PRIMARYTYPE, NT_REP_AUTHORIZABLE_FOLDER, Type.NAME));
+
+        Root r = when(mock(Root.class).getTree(anyString())).thenReturn(t).getMock();
+        when(r.getContentSession()).thenReturn(root.getContentSession());
+
+        UserProvider up = new UserProvider(r, ConfigurationParameters.EMPTY);
+        Tree tree = up.createUser("uid", null);
+        assertSame(ut, tree);
+    }
+
+    @Test
     public void testCreateUserRFC7613Disabled() throws Exception {
         String userHalfWidth = "Amalia";
         String userFullWidth = "\uff21\uff4d\uff41\uff4c\uff49\uff41";
@@ -264,7 +313,7 @@ public class UserProviderTest {
     public void testIllegalChars() throws Exception {
         UserProvider userProvider = createUserProvider();
 
-        Map<String, String> m = new HashMap<String, String>();
+        Map<String, String> m = new HashMap<>();
         m.put("z[x]", "/z/" + Text.escapeIllegalJcrChars("z[") + '/' + Text.escapeIllegalJcrChars("z[x]"));
         m.put("z*x", "/z/" + Text.escapeIllegalJcrChars("z*") + '/' + Text.escapeIllegalJcrChars("z*x"));
         m.put("z/x", "/z/" + Text.escapeIllegalJcrChars("z/") + '/' + Text.escapeIllegalJcrChars("z/x"));
@@ -318,6 +367,43 @@ public class UserProviderTest {
     }
 
     @Test
+    public void testGetAuthorizableByPrincipal() throws Exception {
+        UserProvider up = createUserProvider();
+        up.createUser("uid", null);
+        TreeBasedPrincipal tbp = new TreeBasedPrincipal("uid", "/path", NamePathMapper.DEFAULT) {
+            @Override
+            @NotNull String getOakPath() {
+                return "/path";
+            }
+        };
+        // changes not yet persisted -> query returns no result.
+        assertNotNull(up.getAuthorizableByPrincipal(tbp));
+    }
+
+    @Test
+    public void testGetAuthorizableByPrincipal2() throws Exception {
+        UserProvider up = createUserProvider();
+        up.createUser("uid", null);
+        TreeBasedPrincipal tbp = new TreeBasedPrincipal("uid", "/path", NamePathMapper.DEFAULT) {
+            @Override
+            @NotNull String getOakPath() throws RepositoryException {
+                throw new RepositoryException();
+            }
+        };
+        // changes not yet persisted -> query returns no result.
+        assertNull(up.getAuthorizableByPrincipal(tbp));
+    }
+
+    @Test
+    public void testGetAuthorizableByPrincipalQueryFails() throws Exception {
+        QueryEngine qe = mock(QueryEngine.class);
+        when(qe.executeQuery(anyString(), anyString(), anyLong(), anyLong(), any(Map.class), any(Map.class))).thenThrow(new ParseException("err",0));
+        Root r = when(mock(Root.class).getQueryEngine()).thenReturn(qe).getMock();
+        UserProvider up = new UserProvider(r, ConfigurationParameters.EMPTY);
+        assertNull(up.getAuthorizableByPrincipal(new PrincipalImpl("name")));
+    }
+
+    @Test
     public void testGetAuthorizableId() throws Exception {
         UserProvider up = createUserProvider();
 
@@ -350,13 +436,7 @@ public class UserProviderTest {
 
     @Test
     public void testCollisions() throws Exception {
-        ConfigurationParameters config = ConfigurationParameters.of(UserConstants.PARAM_AUTHORIZABLE_NODE_NAME, new AuthorizableNodeName() {
-            @NotNull
-            @Override
-            public String generateNodeName(@NotNull String authorizableId) {
-                return "aaa";
-            }
-        });
+        ConfigurationParameters config = ConfigurationParameters.of(UserConstants.PARAM_AUTHORIZABLE_NODE_NAME, (AuthorizableNodeName) authorizableId -> "aaa");
         UserProvider up = new UserProvider(root, config);
 
         try {

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserValidatorTest.java Tue Sep 29 15:59:10 2020
@@ -34,7 +34,6 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
 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.apache.jackrabbit.util.Text;
 import org.jetbrains.annotations.NotNull;
 import org.junit.After;
@@ -162,6 +161,16 @@ public class UserValidatorTest extends A
         }
     }
 
+    @Test
+    public void createWithoutAuthorizableId() throws Exception {
+        User user = getUserManager(root).createUser("withoutId", "pw");
+        Tree tree = root.getTree(user.getPath());
+        tree.removeProperty(REP_AUTHORIZABLE_ID);
+        root.commit();
+
+        assertNotNull(getUserManager(root).getAuthorizable("withoutId"));
+    }
+
     @Test(expected = CommitFailedException.class)
     public void createWithInvalidUUID() throws Exception {
         User user = getUserManager(root).createUser("withInvalidUUID", "pw");
@@ -316,7 +325,7 @@ public class UserValidatorTest extends A
 
     @Test
     public void testEnforceHierarchy() {
-        List<String> invalid = new ArrayList<String>();
+        List<String> invalid = new ArrayList<>();
         invalid.add("/");
         invalid.add("/jcr:system");
         String groupRoot = getConfig().getConfigValue(PARAM_GROUP_PATH, DEFAULT_GROUP_PATH);
@@ -378,16 +387,15 @@ public class UserValidatorTest extends A
     @Test(expected = CommitFailedException.class)
     public void testCreateNestedUser2Steps() throws Exception {
         Tree userTree = root.getTree(getTestUser().getPath());
-        NodeUtil userNode = new NodeUtil(userTree);
-        NodeUtil profile = userNode.addChild("profile", JcrConstants.NT_UNSTRUCTURED);
-        NodeUtil nested = profile.addChild("nested", JcrConstants.NT_UNSTRUCTURED);
-        nested.setString(UserConstants.REP_PRINCIPAL_NAME, "nested");
-        nested.setString(UserConstants.REP_AUTHORIZABLE_ID, "nested");
-        nested.setString(JcrConstants.JCR_UUID, UUIDUtils.generateUUID("nested"));
+        Tree profile = TreeUtil.addChild(userTree, "profile", JcrConstants.NT_UNSTRUCTURED);
+        Tree nested = TreeUtil.addChild(profile, "nested", JcrConstants.NT_UNSTRUCTURED);
+        nested.setProperty(UserConstants.REP_PRINCIPAL_NAME, "nested");
+        nested.setProperty(UserConstants.REP_AUTHORIZABLE_ID, "nested");
+        nested.setProperty(JcrConstants.JCR_UUID, UUIDUtils.generateUUID("nested"));
         root.commit();
 
         try {
-            nested.setName(JCR_PRIMARYTYPE, UserConstants.NT_REP_USER);
+            nested.setProperty(JCR_PRIMARYTYPE, UserConstants.NT_REP_USER, Type.NAME);
             root.commit();
             fail("Creating nested users must be detected.");
         } catch (CommitFailedException e) {

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/GroupActionBestEffortTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/GroupActionBestEffortTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/GroupActionBestEffortTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/GroupActionBestEffortTest.java Tue Sep 29 15:59:10 2020
@@ -32,7 +32,7 @@ public class GroupActionBestEffortTest e
     public void testMembersAddedNonExisting() throws Exception {
         Set<String> nonExisting = ImmutableSet.of("blinder", "passagier");
 
-        testGroup.addMembers(nonExisting.toArray(new String[nonExisting.size()]));
+        testGroup.addMembers(nonExisting.toArray(new String[0]));
 
         verify(groupAction, times(1)).onMembersAdded(testGroup, nonExisting, Collections.emptySet(), root, getNamePathMapper());
     }
@@ -41,8 +41,8 @@ public class GroupActionBestEffortTest e
     public void testMembersRemovedNonExisting() throws Exception {
         Set<String> nonExisting = ImmutableSet.of("blinder", "passagier");
 
-        testGroup.removeMembers(nonExisting.toArray(new String[nonExisting.size()]));
-        verify(groupAction, times(1)).onMembersRemoved(testGroup, Collections.EMPTY_SET, nonExisting, root, getNamePathMapper());
+        testGroup.removeMembers(nonExisting.toArray(new String[0]));
+        verify(groupAction, times(1)).onMembersRemoved(testGroup, Collections.emptySet(), nonExisting, root, getNamePathMapper());
     }
 
     @Override

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/GroupActionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/GroupActionTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/GroupActionTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/GroupActionTest.java Tue Sep 29 15:59:10 2020
@@ -132,7 +132,7 @@ public class GroupActionTest extends Abs
     public void testMembersAddedNonExisting() throws Exception {
         Set<String> nonExisting = ImmutableSet.of("blinder", "passagier");
 
-        testGroup.addMembers(nonExisting.toArray(new String[nonExisting.size()]));
+        testGroup.addMembers(nonExisting.toArray(new String[0]));
         verify(groupAction, times(1)).onMembersAdded(testGroup, Collections.emptySet(), nonExisting, root, getNamePathMapper());
     }
 
@@ -154,7 +154,7 @@ public class GroupActionTest extends Abs
     public void testMembersRemovedNonExisting() throws Exception {
         Set<String> nonExisting = ImmutableSet.of("blinder", "passagier");
 
-        testGroup.removeMembers(nonExisting.toArray(new String[nonExisting.size()]));
-        verify(groupAction, times(1)).onMembersRemoved(testGroup, Collections.EMPTY_SET, nonExisting, root, getNamePathMapper());
+        testGroup.removeMembers(nonExisting.toArray(new String[0]));
+        verify(groupAction, times(1)).onMembersRemoved(testGroup, Collections.emptySet(), nonExisting, root, getNamePathMapper());
     }
 }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/UserActionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/UserActionTest.java?rev=1882131&r1=1882130&r2=1882131&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/UserActionTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/UserActionTest.java Tue Sep 29 15:59:10 2020
@@ -136,12 +136,12 @@ public class UserActionTest extends Abst
         }
 
         @Override
-        public void onGrantImpersonation(@NotNull User user, @NotNull Principal principal, @NotNull Root root, @NotNull NamePathMapper namePathMapper) throws RepositoryException {
+        public void onGrantImpersonation(@NotNull User user, @NotNull Principal principal, @NotNull Root root, @NotNull NamePathMapper namePathMapper) {
             // nothing to do
         }
 
         @Override
-        public void onRevokeImpersonation(@NotNull User user, @NotNull Principal principal, @NotNull Root root, @NotNull NamePathMapper namePathMapper) throws RepositoryException {
+        public void onRevokeImpersonation(@NotNull User user, @NotNull Principal principal, @NotNull Root root, @NotNull NamePathMapper namePathMapper) {
             // nothing to do
         }
     }