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=1860352&r1=1860351&r2=1860352&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
 Wed May 29 15:56:44 2019
@@ -16,26 +16,13 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-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.junit.Assert.fail;
-
-import java.security.Principal;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Set;
-import java.util.UUID;
-
-import javax.jcr.RepositoryException;
-import javax.jcr.UnsupportedRepositoryOperationException;
-
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.security.user.Authorizable;
+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.api.security.user.UserManager;
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
@@ -44,15 +31,49 @@ import org.apache.jackrabbit.oak.api.Con
 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.commons.UUIDUtils;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.namepath.impl.LocalNameMapper;
+import org.apache.jackrabbit.oak.namepath.impl.NamePathMapperImpl;
+import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
+import org.apache.jackrabbit.oak.plugins.value.jcr.PartialValueFactory;
+import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
+import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
 import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
+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.AuthorizableActionProvider;
+import org.apache.jackrabbit.oak.spi.security.user.action.GroupAction;
 import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil;
-import org.apache.jackrabbit.oak.util.NodeUtil;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
+import javax.jcr.RepositoryException;
+import javax.jcr.UnsupportedRepositoryOperationException;
+import java.security.Principal;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+
+import static 
org.apache.jackrabbit.oak.spi.security.user.UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER;
+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.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
 /**
  * @since OAK 1.0
  */
@@ -60,34 +81,21 @@ public class UserManagerImplTest extends
 
     private UserManagerImpl userMgr;
     private String testUserId = "testUser";
-    private Set<String> beforeAuthorizables = new HashSet<String>();
 
     @Before
     public void before() throws Exception {
         super.before();
 
         userMgr = new UserManagerImpl(root, getPartialValueFactory(), 
getSecurityProvider());
-        beforeAuthorizables.clear();
-        Iterator<Authorizable> iter = 
userMgr.findAuthorizables("jcr:primaryType", null, 
UserManager.SEARCH_TYPE_AUTHORIZABLE);
-        while (iter.hasNext()) {
-            beforeAuthorizables.add(iter.next().getID());
-        }
     }
 
     @After
     public void after() throws Exception {
-        Iterator<Authorizable> iter = 
userMgr.findAuthorizables("jcr:primaryType", null, 
UserManager.SEARCH_TYPE_AUTHORIZABLE);
-        while (iter.hasNext()) {
-            Authorizable auth = iter.next();
-            if (!beforeAuthorizables.remove(auth.getID())) {
-                try {
-                    auth.remove();
-                } catch (RepositoryException e) {
-                    // ignore
-                }
-            }
+        try {
+            root.refresh();
+        } finally {
+            super.after();
         }
-        super.after();
     }
 
     /**
@@ -131,10 +139,44 @@ public class UserManagerImplTest extends
     }
 
     @Test
+    public void testGetAuthorizableByPath() throws Exception {
+        Authorizable authorizable = getTestUser();
+        Authorizable byPath = 
userMgr.getAuthorizableByPath(authorizable.getPath());
+
+        assertEquals(authorizable.getPath(), byPath.getPath());
+    }
+
+    @Test(expected = RepositoryException.class)
+    public void testAuthorizableByUnresolvablePath() throws Exception {
+        NamePathMapper mapper = new NamePathMapperImpl(new 
LocalNameMapper(root, ImmutableMap.of("a","internal")));
+        UserManagerImpl um = new UserManagerImpl(root, new 
PartialValueFactory(mapper), getSecurityProvider());
+        um.getAuthorizableByPath(getTestUser().getPath());
+    }
+
+    @Test
+    public void testGetAuthorizableFromTree() throws Exception {
+        
assertNotNull(userMgr.getAuthorizable(root.getTree(getTestUser().getPath())));
+    }
+
+    @Test
+    public void testGetAuthorizableFromNullTree() throws Exception {
+        assertNull(userMgr.getAuthorizable((Tree) null));
+    }
+
+    @Test
+    public void testGetAuthorizableFromNonExistingTree() throws Exception {
+        Tree t = when(mock(Tree.class).exists()).thenReturn(false).getMock();
+        assertNull(userMgr.getAuthorizable(t));
+    }
+
+    @Test
+    public void testGtAuthorizableFromInvalidTree() throws Exception {
+        assertNull(userMgr.getAuthorizable(root.getTree(PathUtils.ROOT_PATH)));
+    }
+
+    @Test
     public void testSetPassword() throws Exception {
         User user = userMgr.createUser(testUserId, "pw");
-        root.commit();
-
         List<String> pwds = new ArrayList<String>();
         pwds.add("pw");
         pwds.add("");
@@ -162,31 +204,8 @@ public class UserManagerImplTest extends
     }
 
     @Test
-    public void setPasswordNull() throws Exception {
-        User user = userMgr.createUser(testUserId, null);
-        root.commit();
-
-        Tree userTree = root.getTree(user.getPath());
-        try {
-            userMgr.setPassword(userTree, testUserId, null, true);
-            fail("setting null password should fail");
-        } catch (NullPointerException e) {
-            // expected
-        }
-
-        try {
-            userMgr.setPassword(userTree, testUserId, null, false);
-            fail("setting null password should fail");
-        } catch (NullPointerException e) {
-            // expected
-        }
-    }
-
-    @Test
     public void testGetPasswordHash() throws Exception {
         User user = userMgr.createUser(testUserId, null);
-        root.commit();
-
         Tree userTree = root.getTree(user.getPath());
         assertNull(userTree.getProperty(UserConstants.REP_PASSWORD));
     }
@@ -207,14 +226,12 @@ public class UserManagerImplTest extends
     }
 
     @Test
-    public void testEnforceAuthorizableFolderHierarchy() throws 
RepositoryException, CommitFailedException {
-        User user = userMgr.createUser(testUserId, null);
-        root.commit();
+    public void testEnforceAuthorizableFolderHierarchy() throws Exception {
+        User user = getTestUser();
+        Tree userNode = root.getTree(user.getPath());
 
-        NodeUtil userNode = new NodeUtil(root.getTree(user.getPath()));
-
-        NodeUtil folder = userNode.addChild("folder", 
UserConstants.NT_REP_AUTHORIZABLE_FOLDER);
-        String path = folder.getTree().getPath();
+        Tree folder = TreeUtil.addChild(userNode, "folder", 
UserConstants.NT_REP_AUTHORIZABLE_FOLDER);
+        String path = folder.getPath();
 
         // authNode - authFolder -> create User
         try {
@@ -233,8 +250,8 @@ public class UserManagerImplTest extends
             }
         }
 
-        NodeUtil someContent = userNode.addChild("mystuff", 
JcrConstants.NT_UNSTRUCTURED);
-        path = someContent.getTree().getPath();
+        Tree someContent = TreeUtil.addChild(userNode, "mystuff", 
JcrConstants.NT_UNSTRUCTURED);
+        path = someContent.getPath();
         try {
             // authNode - anyNode -> create User
             try {
@@ -254,11 +271,11 @@ public class UserManagerImplTest extends
             }
 
             // authNode - anyNode - authFolder -> create User
-            folder = someContent.addChild("folder", 
UserConstants.NT_REP_AUTHORIZABLE_FOLDER);
+            folder = TreeUtil.addChild(someContent,"folder", 
UserConstants.NT_REP_AUTHORIZABLE_FOLDER);
             root.commit(); // this time save node structure
             try {
                 Principal p = new PrincipalImpl("test4");
-                userMgr.createUser(p.getName(), p.getName(), p, 
folder.getTree().getPath());
+                userMgr.createUser(p.getName(), p.getName(), p, 
folder.getPath());
                 root.commit();
 
                 fail("Users may not be nested.");
@@ -296,36 +313,45 @@ public class UserManagerImplTest extends
 
     @Test
     public void testConcurrentCreateUser() throws Exception {
-        final List<Exception> exceptions = new ArrayList<Exception>();
-        List<Thread> workers = new ArrayList<Thread>();
-        for (int i=0; i<10; i++) {
-            final String userId = "foo-user-" + i;
-            workers.add(new Thread(new Runnable() {
-                public void run() {
-                    try {
-                        ContentSession admin = login(getAdminCredentials());
-                        Root root = admin.getLatestRoot();
-                        UserManager userManager = new UserManagerImpl(root, 
getPartialValueFactory(), getSecurityProvider());
-                        userManager.createUser(userId, "pass");
-                        root.commit();
-                        admin.close();
-                    } catch (Exception e) {
-                        exceptions.add(e);
+        try {
+            final List<Exception> exceptions = new ArrayList<Exception>();
+            List<Thread> workers = new ArrayList<Thread>();
+            for (int i = 0; i < 10; i++) {
+                final String userId = "foo-user-" + i;
+                workers.add(new Thread(new Runnable() {
+                    public void run() {
+                        try {
+                            ContentSession admin = 
login(getAdminCredentials());
+                            Root root = admin.getLatestRoot();
+                            UserManager userManager = new 
UserManagerImpl(root, getPartialValueFactory(), getSecurityProvider());
+                            userManager.createUser(userId, "pass", new 
PrincipalImpl(userId), "relPath");
+                            root.commit();
+                            admin.close();
+                        } catch (Exception e) {
+                            exceptions.add(e);
+                        }
                     }
-                }
-            }));
-        }
-        for (Thread t : workers) {
-            t.start();
-        }
-        for (Thread t : workers) {
-            t.join();
-        }
-        for (Exception e : exceptions) {
-            e.printStackTrace();
-        }
-        if (!exceptions.isEmpty()) {
-            throw exceptions.get(0);
+                }));
+            }
+            for (Thread t : workers) {
+                t.start();
+            }
+            for (Thread t : workers) {
+                t.join();
+            }
+            for (Exception e : exceptions) {
+                e.printStackTrace();
+            }
+            if (!exceptions.isEmpty()) {
+                throw exceptions.get(0);
+            }
+        } finally {
+            root.refresh();
+            Tree t = root.getTree(UserConstants.DEFAULT_USER_PATH + 
"/relPath");
+            if (t.exists()) {
+                t.remove();
+                root.commit();
+            }
         }
     }
 
@@ -338,18 +364,88 @@ public class UserManagerImplTest extends
     @Test
     public void testNewUserHasNoPwdNode() throws Exception {
         String newUserId = "newuser" + UUID.randomUUID();
-        User user = null;
-        try {
-            user = getUserManager(root).createUser(newUserId, newUserId);
-            root.commit();
+        User user = getUserManager(root).createUser(newUserId, newUserId);
 
-            
Assert.assertFalse(root.getTree(user.getPath()).hasChild(UserConstants.REP_PWD));
-            Assert.assertFalse(user.hasProperty(UserConstants.REP_PWD + "/" + 
UserConstants.REP_PASSWORD_LAST_MODIFIED));
-        } finally {
-            if (user != null) {
-                user.remove();
-                root.commit();
+        
Assert.assertFalse(root.getTree(user.getPath()).hasChild(UserConstants.REP_PWD));
+        Assert.assertFalse(user.hasProperty(UserConstants.REP_PWD + "/" + 
UserConstants.REP_PASSWORD_LAST_MODIFIED));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateUserWithEmptyId() throws RepositoryException {
+        userMgr.createUser("", null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateUserWithNullId() throws RepositoryException {
+        userMgr.createUser(null, null, new PrincipalImpl("userPrincipalName"), 
null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateSystemUserWithEmptyId() throws RepositoryException {
+        userMgr.createSystemUser("", null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateSystemUserWithNullId() throws RepositoryException {
+        userMgr.createSystemUser(null, null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateGroupWithEmptyId() throws RepositoryException {
+        userMgr.createGroup("", new PrincipalImpl("groupPrincipalName"), null);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateGroupWithNullId() throws RepositoryException {
+        userMgr.createGroup((String) 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);
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testCreateGroupWithNullPrincipal() throws Exception {
+        userMgr.createGroup("another", null, null);
+    }
+
+    @Test(expected = AuthorizableExistsException.class)
+    public void testCreateUserWithExistingPrincipal() throws Exception {
+        User u = getTestUser();
+        userMgr.createUser("another", null, u.getPrincipal(), null);
+    }
+
+    @Test(expected = AuthorizableExistsException.class)
+    public void testCreateGroupWithExistingPrincipal() throws Exception {
+        User u = getTestUser();
+        userMgr.createGroup(u.getPrincipal());
+    }
+
+    @Test
+    public void testOnMembersAddedByContentId() throws Exception {
+        GroupAction groupAction = mock(GroupAction.class);
+        List actions = ImmutableList.of(groupAction);
+        AuthorizableActionProvider actionProvider = 
mock(AuthorizableActionProvider.class);
+        
when(actionProvider.getAuthorizableActions(any(SecurityProvider.class))).thenReturn(actions);
+        ConfigurationParameters params = 
ConfigurationParameters.of(PARAM_AUTHORIZABLE_ACTION_PROVIDER, actionProvider);
+
+        UserConfiguration uc = 
when(mock(UserConfiguration.class).getParameters()).thenReturn(params).getMock();
+        SecurityProvider sp = mock(SecurityProvider.class);
+        when(sp.getConfiguration(UserConfiguration.class)).thenReturn(uc);
+
+        UserManagerImpl um = new UserManagerImpl(root, new 
PartialValueFactory(getNamePathMapper()), sp);
+
+        Group testGroup = mock(Group.class);
+        Set<String> membersIds = ImmutableSet.of(UUIDUtils.generateUUID());
+
+        um.onGroupUpdate(testGroup, false, true, membersIds, 
Collections.emptySet());
+        verify(groupAction, times(1)).onMembersAddedContentId(testGroup, 
membersIds, Collections.emptySet(), root, getNamePathMapper());
+
     }
 }
\ No newline at end of file

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=1860352&r1=1860351&r2=1860352&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
 Wed May 29 15:56:44 2019
@@ -16,16 +16,16 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-import java.security.Principal;
-import java.util.Enumeration;
-import java.util.Set;
-import java.util.UUID;
-
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterators;
 import org.apache.jackrabbit.api.security.principal.GroupPrincipal;
+import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 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.api.security.user.UserManager;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
 import 
org.apache.jackrabbit.oak.security.principal.AbstractPrincipalProviderTest;
 import org.apache.jackrabbit.oak.spi.security.principal.AdminPrincipal;
 import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
@@ -33,6 +33,21 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.jetbrains.annotations.NotNull;
 import org.junit.Test;
 
+import java.security.Principal;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.UUID;
+
+import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
+import static 
org.apache.jackrabbit.api.security.principal.PrincipalManager.SEARCH_TYPE_GROUP;
+import static 
org.apache.jackrabbit.oak.spi.security.user.UserConstants.NT_REP_USER;
+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.assertTrue;
 
 public class UserPrincipalProviderTest extends AbstractPrincipalProviderTest {
@@ -66,48 +81,64 @@ public class UserPrincipalProviderTest e
 
     @Test
     public void testTreeBasedGroupPrincipal() throws Exception {
-        Group group = getUserManager(root).createGroup("testGroup" + 
UUID.randomUUID());
-        root.commit();
+        Principal principal = 
principalProvider.getPrincipal(testGroup.getPrincipal().getName());
+        assertTrue(principal instanceof AbstractGroupPrincipal);
+    }
 
-        try {
-            Principal principal = 
principalProvider.getPrincipal(group.getPrincipal().getName());
-            assertTrue(principal instanceof AbstractGroupPrincipal);
-        } finally {
-            group.remove();
-            root.commit();
-        }
+    @Test
+    public void testTreeBasedGroupPrincipalReflectsMemberChanges() throws 
Exception {
+        Principal principal = 
principalProvider.getPrincipal(testGroup.getPrincipal().getName());
+        assertTrue(principal instanceof AbstractGroupPrincipal);
+        AbstractGroupPrincipal agp = (AbstractGroupPrincipal) principal;
+
+        User u = getTestUser();
+        assertTrue(agp.isMember(u));
+
+        testGroup.removeMember(u);
+        assertFalse(agp.isMember(u));
     }
 
     @Test
-    public void testGetPrincipalsForUser() throws Exception {
-        Group group = getUserManager(root).createGroup("testGroup" + 
UUID.randomUUID());
-        group.addMember(getTestUser());
-        root.commit();
+    public void testTreeBasedGroupPrincipalReflectsRemoval() throws Exception {
+        Principal principal = 
principalProvider.getPrincipal(testGroup.getPrincipal().getName());
+        assertTrue(principal instanceof AbstractGroupPrincipal);
+        AbstractGroupPrincipal agp = (AbstractGroupPrincipal) principal;
 
-        try {
-            Set<? extends Principal> principals = 
principalProvider.getPrincipals(getTestUser().getID());
-            for (Principal p : principals) {
-                String name = p.getName();
-                if (name.equals(getTestUser().getPrincipal().getName())) {
-                    assertTrue(p instanceof TreeBasedPrincipal);
-                } else if (!EveryonePrincipal.NAME.equals(name)) {
-                    assertTrue(p instanceof AbstractGroupPrincipal);
-                }
+        testGroup.remove();
+        assertFalse(agp.isMember(getTestUser()));
+    }
+
+    @Test
+    public void testTreeBasedGroupPrincipalReflectsChangedType() throws 
Exception {
+        Principal principal = 
principalProvider.getPrincipal(testGroup.getPrincipal().getName());
+        assertTrue(principal instanceof AbstractGroupPrincipal);
+        AbstractGroupPrincipal agp = (AbstractGroupPrincipal) principal;
+
+        Tree t = root.getTree(testGroup.getPath());
+        t.setProperty(JCR_PRIMARYTYPE, NT_REP_USER, Type.NAME);
+        assertFalse(agp.isMember(getTestUser()));
+    }
+
+    @Test
+    public void testGetPrincipalsForUser() throws Exception {
+        Set<? extends Principal> principals = 
principalProvider.getPrincipals(getTestUser().getID());
+        for (Principal p : principals) {
+            String name = p.getName();
+            if (name.equals(getTestUser().getPrincipal().getName())) {
+                assertTrue(p instanceof TreeBasedPrincipal);
+            } else if (!EveryonePrincipal.NAME.equals(name)) {
+                assertTrue(p instanceof AbstractGroupPrincipal);
             }
-        } finally {
-            group.remove();
-            root.commit();
         }
     }
 
     @Test
     public void testGetPrincipalsForSystemUser() throws Exception {
-        User systemUser = getUserManager(root).createSystemUser("systemUser" + 
UUID.randomUUID(), null);
-        Group group = getUserManager(root).createGroup("testGroup" + 
UUID.randomUUID());
-        group.addMember(systemUser);
-        root.commit();
-
+        User systemUser = null;
         try {
+            systemUser = getUserManager(root).createSystemUser("systemUser" + 
UUID.randomUUID(), null);
+            testGroup.addMember(systemUser);
+            root.commit();
             Set<? extends Principal> principals = 
principalProvider.getPrincipals(systemUser.getID());
             for (Principal p : principals) {
                 String name = p.getName();
@@ -118,9 +149,10 @@ public class UserPrincipalProviderTest e
                 }
             }
         } finally {
-            systemUser.remove();
-            group.remove();
-            root.commit();
+            if (systemUser != null) {
+                systemUser.remove();
+                root.commit();
+            }
         }
     }
 
@@ -167,41 +199,89 @@ public class UserPrincipalProviderTest e
 
     @Test
     public void testGroupMembers() throws Exception {
-        Group group = getUserManager(root).createGroup("testGroup" + 
UUID.randomUUID());
-        group.addMember(getTestUser());
-        root.commit();
-
-        try {
-            Principal principal = 
principalProvider.getPrincipal(group.getPrincipal().getName());
-
-            assertTrue(principal instanceof GroupPrincipal);
+        Principal principal = 
principalProvider.getPrincipal(testGroup.getPrincipal().getName());
+        assertTrue(principal instanceof GroupPrincipal);
 
-            boolean found = false;
-            Enumeration<? extends Principal> members = ((GroupPrincipal) 
principal).members();
-            while (members.hasMoreElements() && !found) {
-                found = 
members.nextElement().equals(getTestUser().getPrincipal());
-            }
-            assertTrue(found);
-        } finally {
-            group.remove();
-            root.commit();
+        boolean found = false;
+        Enumeration<? extends Principal> members = ((GroupPrincipal) 
principal).members();
+        while (members.hasMoreElements() && !found) {
+            found = members.nextElement().equals(getTestUser().getPrincipal());
         }
+        assertTrue(found);
     }
 
     @Test
     public void testGroupIsMember() throws Exception {
-        Group group = getUserManager(root).createGroup("testGroup" + 
UUID.randomUUID());
-        group.addMember(getTestUser());
-        root.commit();
+        Principal principal = 
principalProvider.getPrincipal(testGroup.getPrincipal().getName());
 
+        assertTrue(principal instanceof GroupPrincipal);
+        assertTrue(((GroupPrincipal) 
principal).isMember(getTestUser().getPrincipal()));
+    }
+
+    @Test
+    public void testMissingUserPrincipalName() throws Exception {
+        User u = getTestUser();
+        Tree t = root.getTree(u.getPath());
+        t.removeProperty(REP_PRINCIPAL_NAME);
+
+        assertTrue(principalProvider.getPrincipals(u.getID()).isEmpty());
+    }
+
+    @Test
+    public void testMissingGroupPrincipalName() throws Exception {
+        Principal p = testGroup.getPrincipal();
+        Tree t = root.getTree(testGroup.getPath());
+        t.removeProperty(REP_PRINCIPAL_NAME);
+
+        
assertFalse(principalProvider.getPrincipals(getTestUser().getID()).contains(p));
+    }
+
+    @Test
+    public void testFindWithEmptyHint() throws Exception {
+        List<String> resultNames = 
getNames(principalProvider.findPrincipals("", 
PrincipalManager.SEARCH_TYPE_GROUP));
+
+        
assertFalse(resultNames.contains(getTestUser().getPrincipal().getName()));
+
+        assertTrue(resultNames.contains(EveryonePrincipal.NAME));
+        assertTrue(resultNames.contains(testGroup.getPrincipal().getName()));
+    }
+
+    @Test
+    public void testFindFullTextWithAndWithoutWildcard() {
+        Iterator<? extends Principal> i1 = 
principalProvider.findPrincipals("testGroup", true,
+                SEARCH_TYPE_GROUP, 0, -1);
+        Iterator<? extends Principal> i2 = 
principalProvider.findPrincipals("testGroup*", true,
+                SEARCH_TYPE_GROUP, 0, -1);
+        assertTrue(Iterators.elementsEqual(i1, i2));
+    }
+
+    @Test
+    public void testFindFiltersDuplicateEveryone() throws Exception {
+        Group everyoneGroup = null;
         try {
-            Principal principal = 
principalProvider.getPrincipal(group.getPrincipal().getName());
+            UserManager userMgr = getUserManager(root);
+            everyoneGroup = userMgr.createGroup(EveryonePrincipal.NAME);
+            root.commit();
 
-            assertTrue(principal instanceof GroupPrincipal);
-            assertTrue(((GroupPrincipal) 
principal).isMember(getTestUser().getPrincipal()));
+            Iterator<? extends Principal> principals = 
principalProvider.findPrincipals(null, SEARCH_TYPE_GROUP);
+            Iterator filtered = Iterators.filter(principals, 
(Predicate<Principal>) principal -> 
EveryonePrincipal.NAME.equals(principal.getName()));
+            assertEquals(1, Iterators.size(filtered));
         } finally {
-            group.remove();
-            root.commit();
+            if (everyoneGroup != null) {
+                everyoneGroup.remove();
+                root.commit();
+            }
+        }
+
+        List<String> expected = Arrays.asList(groupId, groupId2, groupId3);
+        Collections.sort(expected);
+
+        for (int limit = -1; limit < expected.size() + 2; limit++) {
+            Iterator<? extends Principal> i1 = 
principalProvider.findPrincipals("testGroup", true,
+                    SEARCH_TYPE_GROUP, 0, limit);
+            Iterator<? extends Principal> i2 = 
principalProvider.findPrincipals("testGroup*", true,
+                    SEARCH_TYPE_GROUP, 0, limit);
+            assertTrue(Iterators.elementsEqual(i1, i2));
         }
     }
 }

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=1860352&r1=1860351&r2=1860352&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
 Wed May 29 15:56:44 2019
@@ -16,17 +16,6 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-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 javax.jcr.NoSuchWorkspaceException;
-import javax.jcr.SimpleCredentials;
-import javax.security.auth.Subject;
-import javax.security.auth.login.LoginException;
-
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -43,19 +32,30 @@ import org.apache.jackrabbit.oak.api.Roo
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
+import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
+import 
org.apache.jackrabbit.oak.security.principal.AbstractPrincipalProviderTest;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationBase;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
 import org.apache.jackrabbit.oak.spi.security.authentication.SystemSubject;
-import 
org.apache.jackrabbit.oak.security.principal.AbstractPrincipalProviderTest;
 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.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
-import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.junit.Test;
 
+import javax.jcr.NoSuchWorkspaceException;
+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.assertNotNull;
@@ -289,6 +289,7 @@ public class UserPrincipalProviderWithCa
 
             assertTrue(p instanceof TreeBasedPrincipal);
             assertEquals(testGroup.getPath(), ((TreeBasedPrincipal) 
p).getPath());
+            assertEquals(testGroup.getPath(), ((TreeBasedPrincipal) 
p).getOakPath());
 
             GroupPrincipal principalGroup = (GroupPrincipal) p;
             assertTrue(principalGroup.isMember(testPrincipal));

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=1860352&r1=1860351&r2=1860352&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
 Wed May 29 15:56:44 2019
@@ -16,11 +16,6 @@
  */
 package org.apache.jackrabbit.oak.security.user;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.UUID;
-import javax.jcr.RepositoryException;
-
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.User;
@@ -31,6 +26,8 @@ import org.apache.jackrabbit.oak.api.Tre
 import org.apache.jackrabbit.oak.api.Type;
 import org.apache.jackrabbit.oak.commons.UUIDUtils;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;
+import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
+import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
 import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
 import org.apache.jackrabbit.oak.spi.commit.Validator;
 import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
@@ -39,12 +36,21 @@ import org.apache.jackrabbit.oak.spi.sta
 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;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+
+import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
+import static org.apache.jackrabbit.JcrConstants.JCR_UUID;
 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.junit.Assert.fail;
 
 /**
@@ -60,183 +66,224 @@ public class UserValidatorTest extends A
         userPath = getTestUser().getPath();
     }
 
+    @After
+    public void after() throws Exception {
+        try {
+            root.refresh();
+        } finally {
+            super.after();
+        }
+    }
+
+    @NotNull
     private UserValidatorProvider createValidatorProvider() {
         return new UserValidatorProvider(getConfig(), getRootProvider(), 
getTreeProvider());
     }
 
-    @Test
+    @NotNull
+    private UserValidator createUserValidator(@NotNull Tree before, @NotNull 
Tree after) {
+        UserValidatorProvider uvp = createValidatorProvider();
+        // force creation of membership provider
+        uvp.getRootValidator(getTreeProvider().asNodeState(before), 
getTreeProvider().asNodeState(after), new CommitInfo("sid", null));
+        return new UserValidator(before, after, uvp);
+    }
+
+    @NotNull
+    private ConfigurationParameters getConfig() {
+        return getUserConfiguration().getParameters();
+    }
+
+    @Test(expected = CommitFailedException.class)
     public void removePassword() throws Exception {
         try {
             Tree userTree = root.getTree(userPath);
             userTree.removeProperty(REP_PASSWORD);
             root.commit();
-            fail("removing password should fail");
         } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
+            assertEquals(25, e.getCode());
+            assertTrue(e.isConstraintViolation());
+            throw e;
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void removePrincipalName() throws Exception {
+        Tree userTree = root.getTree(userPath);
+        userTree.removeProperty(REP_PRINCIPAL_NAME);
+        root.commit();
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void removePrincipalName2() throws Exception {
         try {
             Tree userTree = root.getTree(userPath);
-            userTree.removeProperty(REP_PRINCIPAL_NAME);
-            root.commit();
-            fail("removing principal name should fail");
+            UserValidator validator = createUserValidator(userTree, userTree);
+            
validator.propertyDeleted(userTree.getProperty(REP_PRINCIPAL_NAME));
         } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
+            assertEquals(25, e.getCode());
+            assertTrue(e.isConstraintViolation());
+            throw e;
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void removeAuthorizableId() throws Exception {
         try {
             Tree userTree = root.getTree(userPath);
             userTree.removeProperty(REP_AUTHORIZABLE_ID);
             root.commit();
-            fail("removing authorizable id should fail");
         } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
+            assertEquals(25, e.getCode());
+            assertTrue(e.isConstraintViolation());
+            throw e;
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void createWithoutPrincipalName() throws Exception {
-        try {
-            User user = 
getUserManager(root).createUser("withoutPrincipalName", "pw");
-            Tree tree = root.getTree(user.getPath());
-            tree.removeProperty(REP_PRINCIPAL_NAME);
-            root.commit();
+        User user = getUserManager(root).createUser("withoutPrincipalName", 
"pw");
+        Tree tree = root.getTree(user.getPath());
+        tree.removeProperty(REP_PRINCIPAL_NAME);
+        root.commit();
+    }
 
-            fail("creating user with invalid jcr:uuid should fail");
+    @Test(expected = CommitFailedException.class)
+    public void createWithoutPrincipalName2() throws Exception {
+        Tree userTree = root.getTree(userPath);
+        userTree.removeProperty(REP_PRINCIPAL_NAME);
+        NodeState userState = getTreeProvider().asNodeState(userTree);
+
+        try {
+            Tree tree = root.getTree(userPath).getParent();
+            createUserValidator(tree, tree).childNodeAdded(userTree.getName(), 
userState);
         } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
+            assertEquals(26, e.getCode());
+            throw e;
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void createWithInvalidUUID() throws Exception {
+        User user = getUserManager(root).createUser("withInvalidUUID", "pw");
+        Tree tree = root.getTree(user.getPath());
+        tree.setProperty(JcrConstants.JCR_UUID, UUID.randomUUID().toString());
+        root.commit();
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void createSystemUserWithPw() throws Exception {
         try {
-            User user = getUserManager(root).createUser("withInvalidUUID", 
"pw");
+            User user = getUserManager(root).createSystemUser("withPw", null);
             Tree tree = root.getTree(user.getPath());
-            tree.setProperty(JcrConstants.JCR_UUID, 
UUID.randomUUID().toString());
+            tree.setProperty(REP_PASSWORD, "pw", Type.STRING);
             root.commit();
-
-            fail("creating user with invalid jcr:uuid should fail");
         } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
+            assertEquals(32, e.getCode());
+            throw e;
         }
     }
 
-    @Test
-    public void changeUUID() throws Exception {
+    @Test(expected = CommitFailedException.class)
+    public void createSystemUserWithPwNode() throws Exception {
         try {
-            Tree userTree = root.getTree(userPath);
-            userTree.setProperty(JcrConstants.JCR_UUID, 
UUID.randomUUID().toString());
+            User user = getUserManager(root).createSystemUser("withPwNode", 
null);
+            Tree tree = root.getTree(user.getPath());
+            TreeUtil.addChild(tree, REP_PWD, NT_REP_PASSWORD);
             root.commit();
-            fail("changing jcr:uuid should fail if it the uuid valid is 
invalid");
         } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
+            assertEquals(33, e.getCode());
+            throw e;
         }
     }
 
+    @Test(expected = CommitFailedException.class)
+    public void changeUUID() throws Exception {
+        Tree userTree = root.getTree(userPath);
+        userTree.setProperty(JcrConstants.JCR_UUID, 
UUID.randomUUID().toString());
+        root.commit();
+    }
+
     @Test
+    public void changeUUIDValid() throws Exception {
+        Tree userTree = root.getTree(userPath);
+        UserValidator validator = createUserValidator(userTree, userTree);
+        validator.propertyChanged(PropertyStates.createProperty(JCR_UUID, 
"invalidBefore"), userTree.getProperty(JCR_UUID));
+    }
+
+    @Test(expected = CommitFailedException.class)
     public void changePrincipalName() throws Exception {
-        try {
-            Tree userTree = root.getTree(userPath);
-            userTree.setProperty(REP_PRINCIPAL_NAME, "another");
-            root.commit();
-            fail("changing the principal name should fail");
-        } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
-        }
+        Tree userTree = root.getTree(userPath);
+        userTree.setProperty(REP_PRINCIPAL_NAME, "another");
+        root.commit();
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void changeAuthorizableId() throws Exception {
-        try {
-            Tree userTree = root.getTree(userPath);
-            userTree.setProperty(REP_AUTHORIZABLE_ID, "modified");
-            root.commit();
-            fail("changing the authorizable id should fail");
-        } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
-        }
+        Tree userTree = root.getTree(userPath);
+        userTree.setProperty(REP_AUTHORIZABLE_ID, "modified");
+        root.commit();
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void changePasswordToPlainText() throws Exception {
+        Tree userTree = root.getTree(userPath);
+        userTree.setProperty(REP_PASSWORD, "plaintext");
+        root.commit();
+    }
+
+    @Test(expected = CommitFailedException.class)
+    public void changePrimaryType() throws Exception {
         try {
             Tree userTree = root.getTree(userPath);
-            userTree.setProperty(REP_PASSWORD, "plaintext");
-            root.commit();
-            fail("storing a plaintext password should fail");
+            UserValidator validator = createUserValidator(userTree, userTree);
+            validator.propertyChanged(userTree.getProperty(JCR_PRIMARYTYPE), 
PropertyStates.createProperty(JCR_PRIMARYTYPE, NT_REP_GROUP));
         } catch (CommitFailedException e) {
-            // expected
-        } finally {
-            root.refresh();
+            assertEquals(28, e.getCode());
+            throw e;
         }
     }
 
     @Test
-    public void testRemoveAdminUser() throws Exception {
-        try {
-            String adminId = getConfig().getConfigValue(PARAM_ADMIN_ID, 
DEFAULT_ADMIN_ID);
-            UserManager userMgr = getUserManager(root);
-            Authorizable admin = userMgr.getAuthorizable(adminId);
-            if (admin == null) {
-                admin = userMgr.createUser(adminId, adminId);
-                root.commit();
-            }
+    public void changePrimaryTypeValid() throws Exception {
+        Tree userTree = root.getTree(userPath);
+        UserValidator validator = createUserValidator(userTree, userTree);
+        
validator.propertyChanged(PropertyStates.createProperty(JCR_PRIMARYTYPE, 
NT_REP_GROUP), userTree.getProperty(JCR_PRIMARYTYPE));
+    }
 
-            root.getTree(admin.getPath()).remove();
+    @Test(expected = CommitFailedException.class)
+    public void testRemoveAdminUser() throws Exception {
+        String adminId = getConfig().getConfigValue(PARAM_ADMIN_ID, 
DEFAULT_ADMIN_ID);
+        UserManager userMgr = getUserManager(root);
+        Authorizable admin = userMgr.getAuthorizable(adminId);
+        if (admin == null) {
+            admin = userMgr.createUser(adminId, adminId);
             root.commit();
-            fail("Admin user cannot be removed");
-        } catch (CommitFailedException e) {
-            // success
-        } finally {
-            root.refresh();
         }
+
+        root.getTree(admin.getPath()).remove();
+        root.commit();
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testRemoveAdminUserFolder() throws Exception {
-        try {
-            String adminId = getConfig().getConfigValue(PARAM_ADMIN_ID, 
DEFAULT_ADMIN_ID);
-            UserManager userMgr = getUserManager(root);
-            Authorizable admin = userMgr.getAuthorizable(adminId);
-            if (admin == null) {
-                admin = userMgr.createUser(adminId, adminId);
-                root.commit();
-            }
+        String adminId = getConfig().getConfigValue(PARAM_ADMIN_ID, 
DEFAULT_ADMIN_ID);
+        UserManager userMgr = getUserManager(root);
+        Authorizable admin = userMgr.getAuthorizable(adminId);
+        if (admin == null) {
+            admin = userMgr.createUser(adminId, adminId);
+            root.commit();
+        }
 
+        try {
             root.getTree(admin.getPath()).getParent().remove();
             root.commit();
-            fail("Admin user cannot be removed");
         } catch (CommitFailedException e) {
-            // success
-        } finally {
-            root.refresh();
+            assertEquals(27, e.getCode());
+            throw e;
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testDisableAdminUser() throws Exception {
         try {
             String adminId = getConfig().getConfigValue(PARAM_ADMIN_ID, 
DEFAULT_ADMIN_ID);
@@ -249,16 +296,26 @@ public class UserValidatorTest extends A
 
             root.getTree(admin.getPath()).setProperty(REP_DISABLED, 
"disabled");
             root.commit();
-            fail("Admin user cannot be disabled");
         } catch (CommitFailedException e) {
-            // success
-        } finally {
-            root.refresh();
+            assertEquals(20, e.getCode());
+            throw e;
         }
     }
 
     @Test
-    public void testEnforceHierarchy() throws RepositoryException, 
CommitFailedException {
+    public void testDisableAdminUserNonExistingTree() throws Exception {
+        String adminId = getConfig().getConfigValue(PARAM_ADMIN_ID, 
DEFAULT_ADMIN_ID);
+        Authorizable admin = getUserManager(root).getAuthorizable(adminId);
+
+        Tree userTree = root.getTree(admin.getPath());
+        UserValidator validator = createUserValidator(userTree, userTree);
+        userTree.remove();
+
+        validator.propertyAdded(PropertyStates.createProperty(REP_DISABLED, 
"disabled"));
+    }
+
+    @Test
+    public void testEnforceHierarchy() {
         List<String> invalid = new ArrayList<String>();
         invalid.add("/");
         invalid.add("/jcr:system");
@@ -280,13 +337,13 @@ public class UserValidatorTest extends A
                         Tree next = parent.getChild(segment);
                         if (!next.exists()) {
                             next = parent.addChild(segment);
-                            next.setProperty(JcrConstants.JCR_PRIMARYTYPE, 
NT_REP_AUTHORIZABLE_FOLDER, Type.NAME);
+                            next.setProperty(JCR_PRIMARYTYPE, 
NT_REP_AUTHORIZABLE_FOLDER, Type.NAME);
                             parent = next;
                         }
                     }
                 }
                 Tree userTree = parent.addChild("testUser");
-                userTree.setProperty(JcrConstants.JCR_PRIMARYTYPE, 
NT_REP_USER, Type.NAME);
+                userTree.setProperty(JCR_PRIMARYTYPE, NT_REP_USER, Type.NAME);
                 userTree.setProperty(JcrConstants.JCR_UUID, 
up.getContentID("testUser"));
                 userTree.setProperty(REP_PRINCIPAL_NAME, "testUser");
                 root.commit();
@@ -300,27 +357,25 @@ public class UserValidatorTest extends A
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testCreateNestedUser() 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", 
UserConstants.NT_REP_USER);
-        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", 
UserConstants.NT_REP_USER);
+        nested.setProperty(UserConstants.REP_PRINCIPAL_NAME, "nested");
+        nested.setProperty(UserConstants.REP_AUTHORIZABLE_ID, "nested");
+        nested.setProperty(JcrConstants.JCR_UUID, 
UUIDUtils.generateUUID("nested"));
         try {
             root.commit();
             fail("Creating nested users must be detected.");
         } catch (CommitFailedException e) {
             // success
             assertEquals(29, e.getCode());
-        } finally {
-            root.refresh();
+            throw e;
         }
     }
 
-    @Test
+    @Test(expected = CommitFailedException.class)
     public void testCreateNestedUser2Steps() throws Exception {
         Tree userTree = root.getTree(getTestUser().getPath());
         NodeUtil userNode = new NodeUtil(userTree);
@@ -332,14 +387,13 @@ public class UserValidatorTest extends A
         root.commit();
 
         try {
-            nested.setName(JcrConstants.JCR_PRIMARYTYPE, 
UserConstants.NT_REP_USER);
+            nested.setName(JCR_PRIMARYTYPE, UserConstants.NT_REP_USER);
             root.commit();
             fail("Creating nested users must be detected.");
         } catch (CommitFailedException e) {
             // success
             assertEquals(29, e.getCode());
-        } finally {
-            root.refresh();
+            throw e;
         }
     }
 
@@ -408,8 +462,4 @@ public class UserValidatorTest extends A
                 ":hidden", root.getChildNode("test").getChildNode(":hidden"));
         assertNull(hiddenValidator);
     }
-
-    private ConfigurationParameters getConfig() {
-        return getUserConfiguration().getParameters();
-    }
 }
\ No newline at end of file

Modified: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/ClearMembershipActionTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/ClearMembershipActionTest.java?rev=1860352&r1=1860351&r2=1860352&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/ClearMembershipActionTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/ClearMembershipActionTest.java
 Wed May 29 15:56:44 2019
@@ -32,8 +32,6 @@ import static org.junit.Assert.assertTru
 /**
  * Integration tests for {@link ClearMembershipAction} including a complete
  * security setup.
- *
- * @see {@link 
org.apache.jackrabbit.oak.spi.security.user.action.ClearMembershipActionTest}
  */
 public class ClearMembershipActionTest extends AbstractSecurityTest {
 

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=1860352&r1=1860351&r2=1860352&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
 Wed May 29 15:56:44 2019
@@ -17,15 +17,14 @@
 package org.apache.jackrabbit.oak.security.user.action;
 
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
 import org.junit.Test;
 
+import java.util.Collections;
 import java.util.Set;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 public class GroupActionBestEffortTest extends GroupActionTest {
 
@@ -34,8 +33,8 @@ public class GroupActionBestEffortTest e
         Set<String> nonExisting = ImmutableSet.of("blinder", "passagier");
 
         testGroup.addMembers(nonExisting.toArray(new 
String[nonExisting.size()]));
-        assertTrue(Iterables.elementsEqual(nonExisting, 
groupAction.memberIds));
-        assertFalse(groupAction.failedIds.iterator().hasNext());
+
+        verify(groupAction, times(1)).onMembersAdded(testGroup, nonExisting, 
Collections.emptySet(), root, getNamePathMapper());
     }
 
     @Test
@@ -43,8 +42,7 @@ public class GroupActionBestEffortTest e
         Set<String> nonExisting = ImmutableSet.of("blinder", "passagier");
 
         testGroup.removeMembers(nonExisting.toArray(new 
String[nonExisting.size()]));
-        assertFalse(groupAction.memberIds.iterator().hasNext());
-        assertEquals(nonExisting, groupAction.failedIds);
+        verify(groupAction, times(1)).onMembersRemoved(testGroup, 
Collections.EMPTY_SET, 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=1860352&r1=1860351&r2=1860352&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
 Wed May 29 15:56:44 2019
@@ -16,50 +16,37 @@
  */
 package org.apache.jackrabbit.oak.security.user.action;
 
-import java.util.List;
-import java.util.Set;
-import javax.jcr.RepositoryException;
-
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
-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.namepath.NamePathMapper;
 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.AbstractGroupAction;
-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.ProtectedItemImporter;
-import org.jetbrains.annotations.NotNull;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
+import java.util.Collections;
+import java.util.Set;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 public class GroupActionTest extends AbstractSecurityTest {
 
     private static final String TEST_GROUP_ID = "testGroup";
     private static final String TEST_USER_PREFIX = "testUser";
 
-    final TestGroupAction groupAction = new TestGroupAction();
-    private final AuthorizableActionProvider actionProvider = new 
AuthorizableActionProvider() {
-        @NotNull
-        @Override
-        public List<? extends AuthorizableAction> 
getAuthorizableActions(@NotNull SecurityProvider securityProvider) {
-            return ImmutableList.of(groupAction);
-        }
-    };
+    final GroupAction groupAction = mock(GroupAction.class);
+    private final AuthorizableActionProvider actionProvider = securityProvider 
-> ImmutableList.of(groupAction);
 
     private User testUser01;
     private User testUser02;
@@ -113,9 +100,7 @@ public class GroupActionTest extends Abs
         testUser01 = getUserManager(root).createUser(TEST_USER_PREFIX + "01", 
"");
 
         testGroup.addMember(testUser01);
-        assertTrue(groupAction.onMemberAddedCalled);
-        assertEquals(testGroup, groupAction.group);
-        assertEquals(testUser01, groupAction.member);
+        verify(groupAction, times(1)).onMemberAdded(testGroup, testUser01, 
root, getNamePathMapper());
     }
 
     @Test
@@ -125,9 +110,7 @@ public class GroupActionTest extends Abs
         root.commit();
 
         testGroup.removeMember(testUser01);
-        assertTrue(groupAction.onMemberRemovedCalled);
-        assertEquals(testGroup, groupAction.group);
-        assertEquals(testUser01, groupAction.member);
+        verify(groupAction, times(1)).onMemberRemoved(testGroup, testUser01, 
root, getNamePathMapper());
     }
 
     @Test
@@ -141,10 +124,8 @@ public class GroupActionTest extends Abs
         Iterable<String> ids = Iterables.concat(memberIds, failedIds);
 
         testGroup.addMembers(Iterables.toArray(ids, String.class));
-        assertTrue(groupAction.onMembersAddedCalled);
-        assertEquals(testGroup, groupAction.group);
-        assertEquals(memberIds, groupAction.memberIds);
-        assertEquals(failedIds, groupAction.failedIds);
+
+        verify(groupAction, times(1)).onMembersAdded(testGroup, memberIds, 
failedIds, root, getNamePathMapper());
     }
 
     @Test
@@ -152,8 +133,7 @@ public class GroupActionTest extends Abs
         Set<String> nonExisting = ImmutableSet.of("blinder", "passagier");
 
         testGroup.addMembers(nonExisting.toArray(new 
String[nonExisting.size()]));
-        assertFalse(groupAction.memberIds.iterator().hasNext());
-        assertEquals(nonExisting, groupAction.failedIds);
+        verify(groupAction, times(1)).onMembersAdded(testGroup, 
Collections.emptySet(), nonExisting, root, getNamePathMapper());
     }
 
     @Test
@@ -167,10 +147,7 @@ public class GroupActionTest extends Abs
         Iterable<String> ids = Iterables.concat(memberIds, failedIds);
 
         testGroup.removeMembers(Iterables.toArray(ids, String.class));
-        assertTrue(groupAction.onMembersRemovedCalled);
-        assertEquals(testGroup, groupAction.group);
-        assertEquals(memberIds, groupAction.memberIds);
-        assertEquals(failedIds, groupAction.failedIds);
+        verify(groupAction, times(1)).onMembersRemoved(testGroup, memberIds, 
failedIds, root, getNamePathMapper());
     }
 
     @Test
@@ -178,50 +155,6 @@ public class GroupActionTest extends Abs
         Set<String> nonExisting = ImmutableSet.of("blinder", "passagier");
 
         testGroup.removeMembers(nonExisting.toArray(new 
String[nonExisting.size()]));
-        assertFalse(groupAction.memberIds.iterator().hasNext());
-        assertEquals(nonExisting, groupAction.failedIds);
-    }
-
-    class TestGroupAction extends AbstractGroupAction {
-
-        boolean onMemberAddedCalled = false;
-        boolean onMembersAddedCalled = false;
-        boolean onMemberRemovedCalled = false;
-        boolean onMembersRemovedCalled = false;
-
-        Group group;
-        Set<String> memberIds;
-        Set<String> failedIds;
-        Authorizable member;
-
-        @Override
-        public void onMemberAdded(@NotNull Group group, @NotNull Authorizable 
member, @NotNull Root root, @NotNull NamePathMapper namePathMapper) throws 
RepositoryException {
-            this.group = group;
-            this.member = member;
-            onMemberAddedCalled = true;
-        }
-
-        @Override
-        public void onMembersAdded(@NotNull Group group, @NotNull 
Iterable<String> memberIds, @NotNull Iterable<String> failedIds, @NotNull Root 
root, @NotNull NamePathMapper namePathMapper) throws RepositoryException {
-            this.group = group;
-            this.memberIds = ImmutableSet.copyOf(memberIds);
-            this.failedIds = ImmutableSet.copyOf(failedIds);
-            onMembersAddedCalled = true;
-        }
-
-        @Override
-        public void onMemberRemoved(@NotNull Group group, @NotNull 
Authorizable member, @NotNull Root root, @NotNull NamePathMapper 
namePathMapper) throws RepositoryException {
-            this.group = group;
-            this.member = member;
-            onMemberRemovedCalled = true;
-        }
-
-        @Override
-        public void onMembersRemoved(@NotNull Group group, @NotNull 
Iterable<String> memberIds, @NotNull Iterable<String> failedIds, @NotNull Root 
root, @NotNull NamePathMapper namePathMapper) throws RepositoryException {
-            this.group = group;
-            this.memberIds = ImmutableSet.copyOf(memberIds);
-            this.failedIds = ImmutableSet.copyOf(failedIds);
-            onMembersRemovedCalled = true;
-        }
+        verify(groupAction, times(1)).onMembersRemoved(testGroup, 
Collections.EMPTY_SET, nonExisting, root, getNamePathMapper());
     }
 }

Modified: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/PasswordValidationActionTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/PasswordValidationActionTest.java?rev=1860352&r1=1860351&r2=1860352&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/PasswordValidationActionTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/action/PasswordValidationActionTest.java
 Wed May 29 15:56:44 2019
@@ -16,54 +16,39 @@
  */
 package org.apache.jackrabbit.oak.security.user.action;
 
-import java.util.List;
-import javax.jcr.RepositoryException;
-import javax.jcr.nodetype.ConstraintViolationException;
-
 import com.google.common.collect.ImmutableList;
 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.Type;
-import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 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.AbstractAuthorizableAction;
 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.PasswordValidationAction;
 import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
-import static org.junit.Assert.assertEquals;
+import javax.jcr.nodetype.ConstraintViolationException;
+
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 public class PasswordValidationActionTest extends AbstractSecurityTest {
 
     private final PasswordValidationAction pwAction = new 
PasswordValidationAction();
-    private final TestAction testAction = new TestAction();
-    private final AuthorizableActionProvider actionProvider = new 
AuthorizableActionProvider() {
-        @NotNull
-        @Override
-        public List<? extends AuthorizableAction> 
getAuthorizableActions(@NotNull SecurityProvider securityProvider) {
-            return ImmutableList.of(pwAction, testAction);
-        }
-    };
-
-    private User user;
+    private final AuthorizableAction testAction = 
mock(AuthorizableAction.class);
+    private final AuthorizableActionProvider actionProvider = securityProvider 
-> ImmutableList.of(pwAction, testAction);
 
     @Before
     public void before() throws Exception {
         super.before();
-        testAction.reset();
         pwAction.init(getSecurityProvider(), ConfigurationParameters.of(
                 PasswordValidationAction.CONSTRAINT, 
"^.*(?=.{8,})(?=.*[a-z])(?=.*[A-Z]).*"));
 
@@ -71,11 +56,7 @@ public class PasswordValidationActionTes
 
     @After
     public void after() throws Exception {
-        if (user != null) {
-            user.remove();
-            root.commit();
-        }
-        root = null;
+        root.refresh();
         super.after();
     }
 
@@ -89,64 +70,36 @@ public class PasswordValidationActionTes
 
     @Test
     public void testActionIsCalled() throws Exception {
-        user = getUserManager(root).createUser("testUser", "testUser12345");
-        root.commit();
-        assertEquals(1, testAction.onCreateCalled);
+        User user = getUserManager(root).createUser("testUser", 
"testUser12345");
+        verify(testAction, times(1)).onCreate(user, "testUser12345", root, 
getNamePathMapper());
 
         user.changePassword("pW12345678");
-        assertEquals(1, testAction.onPasswordChangeCalled);
+        verify(testAction, times(1)).onPasswordChange(user, "pW12345678", 
root, getNamePathMapper());
 
         user.changePassword("pW1234567890", "pW12345678");
-        assertEquals(2, testAction.onPasswordChangeCalled);
+        verify(testAction, times(1)).onPasswordChange(user, "pW12345678", 
root, getNamePathMapper());
     }
 
     @Test
     public void testPasswordValidationActionOnCreate() throws Exception {
         String hashed = PasswordUtil.buildPasswordHash("DWkej32H");
-        user = getUserManager(root).createUser("testuser", hashed);
-        root.commit();
+        User user = getUserManager(root).createUser("testuser", hashed);
 
         String pwValue = 
root.getTree(user.getPath()).getProperty(UserConstants.REP_PASSWORD).getValue(Type.STRING);
         assertFalse(PasswordUtil.isPlainTextPassword(pwValue));
         assertTrue(PasswordUtil.isSame(pwValue, hashed));
     }
 
-    @Test
+    @Test(expected = ConstraintViolationException.class)
     public void testPasswordValidationActionOnChange() throws Exception {
-        user = getUserManager(root).createUser("testuser", "testPw123456");
-        root.commit();
+        User user = getUserManager(root).createUser("testuser", 
"testPw123456");
+        String hashed = PasswordUtil.buildPasswordHash("abc");
         try {
             pwAction.init(getSecurityProvider(), 
ConfigurationParameters.of(PasswordValidationAction.CONSTRAINT, "abc"));
-
-            String hashed = PasswordUtil.buildPasswordHash("abc");
             user.changePassword(hashed);
-
-            fail("Password change must always enforce password validation.");
-
-        } catch (ConstraintViolationException e) {
-            // success
-        }
-    }
-
-    
//--------------------------------------------------------------------------
-    private class TestAction extends AbstractAuthorizableAction {
-
-        private int onCreateCalled = 0;
-        private int onPasswordChangeCalled = 0;
-
-        void reset() {
-            onCreateCalled = 0;
-            onPasswordChangeCalled = 0;
-        }
-
-        @Override
-        public void onCreate(@NotNull User user, @Nullable String password, 
@NotNull Root root, @NotNull NamePathMapper namePathMapper) throws 
RepositoryException {
-            onCreateCalled++;
-        }
-
-        @Override
-        public void onPasswordChange(@NotNull User user, @Nullable String 
newPassword, @NotNull Root root, @NotNull NamePathMapper namePathMapper) throws 
RepositoryException {
-            onPasswordChangeCalled++;
+        } finally {
+            verify(testAction, times(1)).onCreate(user, "testPw123456", root, 
getNamePathMapper());
+            verify(testAction, never()).onPasswordChange(user, hashed, 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=1860352&r1=1860351&r2=1860352&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
 Wed May 29 15:56:44 2019
@@ -16,11 +16,6 @@
  */
 package org.apache.jackrabbit.oak.security.user.action;
 
-import java.security.Principal;
-import java.util.List;
-import javax.jcr.RepositoryException;
-import javax.jcr.ValueFactory;
-
 import com.google.common.collect.ImmutableList;
 import org.apache.jackrabbit.api.security.user.User;
 import org.apache.jackrabbit.oak.AbstractSecurityTest;
@@ -28,32 +23,34 @@ import org.apache.jackrabbit.oak.api.Roo
 import org.apache.jackrabbit.oak.api.Tree;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 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.AbstractAuthorizableAction;
-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.UserAction;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.junit.Test;
 
-import static org.junit.Assert.assertEquals;
+import javax.jcr.RepositoryException;
+import javax.jcr.ValueFactory;
+import java.security.Principal;
+
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 
 public class UserActionTest extends AbstractSecurityTest {
 
-    private CountingUserAction cntAction = new CountingUserAction();
+    private UserAction userAction = mock(UserAction.class);
     private ClearProfileAction clearProfileAction = new ClearProfileAction();
 
-    private final AuthorizableActionProvider actionProvider = new 
AuthorizableActionProvider() {
-        @Override
-        public @NotNull List<? extends AuthorizableAction> 
getAuthorizableActions(@NotNull SecurityProvider securityProvider) {
-            return ImmutableList.of(cntAction, clearProfileAction);
-        }
-    };
+    private final AuthorizableActionProvider actionProvider = securityProvider 
-> ImmutableList.of(userAction, clearProfileAction);
 
     @Override
     protected ConfigurationParameters getSecurityConfigParameters() {
@@ -66,14 +63,14 @@ public class UserActionTest extends Abst
         User user = getTestUser();
         user.disable("disabled");
 
-        assertEquals(1, cntAction.onDisabledCnt);
-        assertEquals(0, cntAction.onGrantImpCnt);
-        assertEquals(0, cntAction.onRevokeImpCnt);
+        verify(userAction, times(1)).onDisable(user, "disabled", root, 
getNamePathMapper());
+        verify(userAction, never()).onGrantImpersonation(any(User.class), 
any(Principal.class), any(Root.class), any(NamePathMapper.class));
+        verify(userAction, never()).onRevokeImpersonation(any(User.class), 
any(Principal.class), any(Root.class), any(NamePathMapper.class));
 
         user.disable(null);
-        assertEquals(2, cntAction.onDisabledCnt);
-        assertEquals(0, cntAction.onGrantImpCnt);
-        assertEquals(0, cntAction.onRevokeImpCnt);
+        verify(userAction, times(1)).onDisable(user, null, root, 
getNamePathMapper());
+        verify(userAction, never()).onGrantImpersonation(any(User.class), 
any(Principal.class), any(Root.class), any(NamePathMapper.class));
+        verify(userAction, never()).onRevokeImpersonation(any(User.class), 
any(Principal.class), any(Root.class), any(NamePathMapper.class));
     }
 
     @Test
@@ -83,9 +80,9 @@ public class UserActionTest extends Abst
 
         user.getImpersonation().grantImpersonation(p2);
 
-        assertEquals(0, cntAction.onDisabledCnt);
-        assertEquals(1, cntAction.onGrantImpCnt);
-        assertEquals(0, cntAction.onRevokeImpCnt);
+        verify(userAction, never()).onDisable(any(User.class), anyString(), 
any(Root.class), any(NamePathMapper.class));
+        verify(userAction, times(1)).onGrantImpersonation(any(User.class), 
any(Principal.class), any(Root.class), any(NamePathMapper.class));
+        verify(userAction, never()).onRevokeImpersonation(any(User.class), 
any(Principal.class), any(Root.class), any(NamePathMapper.class));
     }
 
     @Test
@@ -95,9 +92,9 @@ public class UserActionTest extends Abst
 
         user.getImpersonation().revokeImpersonation(p2);
 
-        assertEquals(0, cntAction.onDisabledCnt);
-        assertEquals(0, cntAction.onGrantImpCnt);
-        assertEquals(1, cntAction.onRevokeImpCnt);
+        verify(userAction, never()).onDisable(any(User.class), anyString(), 
any(Root.class), any(NamePathMapper.class));
+        verify(userAction, never()).onGrantImpersonation(any(User.class), 
any(Principal.class), any(Root.class), any(NamePathMapper.class));
+        verify(userAction, times(1)).onRevokeImpersonation(any(User.class), 
any(Principal.class), any(Root.class), any(NamePathMapper.class));
     }
 
     @Test
@@ -126,29 +123,6 @@ public class UserActionTest extends Abst
         assertTrue(t.hasChild("profiles"));
     }
 
-
-    class CountingUserAction extends AbstractAuthorizableAction implements 
UserAction  {
-
-        int onDisabledCnt = 0;
-        int onGrantImpCnt = 0;
-        int onRevokeImpCnt = 0;
-
-        @Override
-        public void onDisable(@NotNull User user, @Nullable String 
disableReason, @NotNull Root root, @NotNull NamePathMapper namePathMapper) 
throws RepositoryException {
-            onDisabledCnt++;
-        }
-
-        @Override
-        public void onGrantImpersonation(@NotNull User user, @NotNull 
Principal principal, @NotNull Root root, @NotNull NamePathMapper 
namePathMapper) throws RepositoryException {
-            onGrantImpCnt++;
-        }
-
-        @Override
-        public void onRevokeImpersonation(@NotNull User user, @NotNull 
Principal principal, @NotNull Root root, @NotNull NamePathMapper 
namePathMapper) throws RepositoryException {
-            onRevokeImpCnt++;
-        }
-    }
-
     class ClearProfileAction extends AbstractAuthorizableAction implements 
UserAction {
 
         @Override

Modified: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/whiteboard/WhiteboardUserAuthenticationFactoryTest.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/whiteboard/WhiteboardUserAuthenticationFactoryTest.java?rev=1860352&r1=1860351&r2=1860352&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/whiteboard/WhiteboardUserAuthenticationFactoryTest.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/whiteboard/WhiteboardUserAuthenticationFactoryTest.java
 Wed May 29 15:56:44 2019
@@ -16,10 +16,7 @@
  */
 package org.apache.jackrabbit.oak.security.user.whiteboard;
 
-import java.util.ArrayList;
-import java.util.List;
 import org.apache.jackrabbit.oak.api.Root;
-import 
org.apache.jackrabbit.oak.security.user.whiteboard.WhiteboardUserAuthenticationFactory;
 import org.apache.jackrabbit.oak.spi.security.authentication.Authentication;
 import org.apache.jackrabbit.oak.spi.security.user.UserAuthenticationFactory;
 import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
@@ -28,6 +25,9 @@ import org.jetbrains.annotations.Nullabl
 import org.junit.Test;
 import org.mockito.Mockito;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 
@@ -56,6 +56,12 @@ public class WhiteboardUserAuthenticatio
     }
 
     @Test
+    public void testNoServiceNoDefault() {
+        WhiteboardUserAuthenticationFactory factory = new 
WhiteboardUserAuthenticationFactory(null);
+        assertNull(factory.getAuthentication(getUserConfiguration(), root, 
"userId"));
+    }
+
+    @Test
     public void testSingleService() throws Exception {
         WhiteboardUserAuthenticationFactory factory = createFactory(null, 
"test");
 


Reply via email to