OAK-9335 : AuthorizableAction doesn't allow to monitor and respond to system user creation
git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/oak/trunk@1885915 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java
index a1ef22e..0968c9c 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserImporter.java
@@ -286,6 +286,8 @@
if (protectedParent.getStatus() == Tree.Status.NEW) {
if (a.isGroup()) {
userManager.onCreate((Group) a);
+ } else if (((User) a).isSystemUser()) {
+ userManager.onCreate((User) a);
} else {
userManager.onCreate((User) a, currentPw);
}
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java
index c0cae58..a5f1e63 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserManagerImpl.java
@@ -61,6 +61,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static com.google.common.base.Preconditions.checkArgument;
+
/**
* UserManagerImpl...
*/
@@ -194,6 +196,7 @@
setPrincipal(userTree, principal);
User user = new SystemUserImpl(userID, userTree, this);
+ onCreate(user);
log.debug("System user created: {}", userID);
return user;
@@ -274,7 +277,14 @@
action.onCreate(user, password, root, namePathMapper);
}
} else {
- log.debug("Omit onCreate action for system users.");
+ log.warn("onCreate(User,String) called for system user. Use onCreate(User) instead.");
+ }
+ }
+
+ void onCreate(@NotNull User systemUser) throws RepositoryException {
+ checkArgument(systemUser.isSystemUser());
+ for (AuthorizableAction action : actionProvider.getAuthorizableActions(securityProvider)) {
+ action.onCreate(systemUser, root, namePathMapper);
}
}
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java
index 96ac65a..4c907d6 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserImporterTest.java
@@ -478,7 +478,8 @@
init(true);
importer.propertiesCompleted(createSystemUserTree());
// create-actions must not be called for system users
- verifyNoInteractions(testAction);
+ verify(testAction).onCreate(any(User.class), any(Root.class), any(NamePathMapper.class));
+ verify(testAction, never()).onCreate(any(User.class), nullable(String.class), any(Root.class), any(NamePathMapper.class));
}
@Test
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplActionsTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplActionsTest.java
new file mode 100644
index 0000000..7358f8f
--- /dev/null
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplActionsTest.java
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.security.user;
+
+import com.google.common.collect.ImmutableSet;
+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.commons.UUIDUtils;
+import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+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.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.security.user.action.UserAction;
+import org.apache.jackrabbit.oak.spi.xml.ImportBehavior;
+import org.apache.jackrabbit.oak.spi.xml.ProtectedItemImporter;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.jcr.RepositoryException;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+
+import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.PARAM_AUTHORIZABLE_ACTION_PROVIDER;
+import static org.junit.Assert.assertNotNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.withSettings;
+
+public class UserManagerImplActionsTest extends AbstractSecurityTest {
+
+ private final AuthorizableActionProvider actionProvider = mock(AuthorizableActionProvider.class);
+ private final AuthorizableAction action = mock(AuthorizableAction.class, withSettings().extraInterfaces(GroupAction.class, UserAction.class));
+
+ private UserManagerImpl userMgr;
+
+ @Before
+ public void before() throws Exception {
+ super.before();
+ userMgr = new UserManagerImpl(root, new PartialValueFactory(getNamePathMapper()), securityProvider);
+ reset(action);
+ }
+
+ @After
+ public void after() throws Exception {
+ try {
+ reset(action, actionProvider);
+ root.refresh();
+ } finally {
+ super.after();
+ }
+ }
+
+ @Override
+ protected ConfigurationParameters getSecurityConfigParameters() {
+ List actions = Collections.singletonList(action);
+ when(actionProvider.getAuthorizableActions(any(SecurityProvider.class))).thenReturn(actions);
+ return ConfigurationParameters.of(UserConfiguration.NAME,
+ ConfigurationParameters.of(
+ PARAM_AUTHORIZABLE_ACTION_PROVIDER, actionProvider,
+ ProtectedItemImporter.PARAM_IMPORT_BEHAVIOR, ImportBehavior.NAME_BESTEFFORT));
+ }
+
+ @Test
+ public void testCreateUser() throws Exception {
+ User u1 = userMgr.createUser("uid", "pw");
+ User u2 = userMgr.createUser("uid2", null, new PrincipalImpl("name2"), "relpath");
+ verify(action, times(1)).onCreate(u1, "pw", root, getNamePathMapper());
+ verify(action, times(1)).onCreate(u2, null, root, getNamePathMapper());
+ verifyNoMoreInteractions(action);
+ }
+
+ @Test
+ public void testOnUserCreateWithSystemUser() throws RepositoryException {
+ User user = when(mock(User.class).isSystemUser()).thenReturn(true).getMock();
+ userMgr.onCreate(user, null);
+ verifyNoInteractions(action);
+ }
+
+ @Test
+ public void testCreateSystemUser() throws Exception {
+ User su = userMgr.createSystemUser("sid", "system/relpath");
+ assertNotNull(su);
+ verify(action, times(1)).onCreate(su, root, getNamePathMapper());
+ verifyNoMoreInteractions(action);
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testOnSystemUserCreateWithRegularUser() throws RepositoryException {
+ User user = when(mock(User.class).isSystemUser()).thenReturn(false).getMock();
+ userMgr.onCreate(user);
+ verifyNoInteractions(action);
+ }
+
+ @Test
+ public void testCreateGroup() throws Exception {
+ Group gr1 = userMgr.createGroup("gId");
+ Group gr2 = userMgr.createGroup(new PrincipalImpl("grName"));
+ Group gr3 = userMgr.createGroup(new PrincipalImpl("grName2"), "relPath");
+ verify(action, times(3)).onCreate(any(Group.class), any(Root.class), any(NamePathMapper.class));
+ verifyNoMoreInteractions(action);
+ }
+
+ @Test
+ public void testChangePw() throws Exception {
+ User u1 = userMgr.createUser("uid", null);
+ u1.changePassword("new");
+ u1.changePassword("changedAgain", "new");
+
+ verify(action).onCreate(u1, null, root, getNamePathMapper());
+ verify(action, times(2)).onPasswordChange(any(User.class), any(String.class), any(Root.class), any(NamePathMapper.class));
+ verifyNoMoreInteractions(action);
+ }
+
+ @Test
+ public void testDisable() throws Exception {
+ User u1 = userMgr.createUser("uid", "pw");
+ u1.disable("disable");
+ u1.disable(null);
+
+ verify(action).onCreate(u1, "pw", root, getNamePathMapper());
+ verify((UserAction) action).onDisable(u1, "disable", root, getNamePathMapper());
+ verify((UserAction) action).onDisable(u1, null, root, getNamePathMapper());
+ verifyNoMoreInteractions(action);
+ }
+
+ @Test
+ public void testAddMembers() throws Exception {
+ User testUser = getTestUser();
+ Group gr1 = userMgr.createGroup("gId");
+ gr1.addMember(testUser);
+ gr1.addMembers("memberId1", "memberId2", gr1.getID());
+
+ gr1.removeMember(testUser);
+ gr1.removeMembers("memberId1");
+
+ verify(((GroupAction) action)).onMemberAdded(gr1, testUser, root, getNamePathMapper());
+ verify(((GroupAction) action)).onMembersAdded(gr1, ImmutableSet.of("memberId1", "memberId2"), Collections.singleton(gr1.getID()), root, getNamePathMapper());
+ verify(((GroupAction) action), never()).onMembersAddedContentId(any(Group.class), any(Iterable.class), any(Iterable.class), any(Root.class), any(NamePathMapper.class));
+
+ verify(((GroupAction) action)).onMemberRemoved(gr1, testUser, root, getNamePathMapper());
+ verify(((GroupAction) action)).onMembersRemoved(gr1, Collections.singleton("memberId1"), Collections.emptySet(), root, getNamePathMapper());
+ }
+
+ @Test
+ public void testOnMembersAddedByContentId() throws Exception {
+ Group testGroup = mock(Group.class);
+ Set<String> membersIds = ImmutableSet.of(UUIDUtils.generateUUID());
+
+ userMgr.onGroupUpdate(testGroup, false, true, membersIds, Collections.emptySet());
+ verify(((GroupAction) action), times(1)).onMembersAddedContentId(testGroup, membersIds, Collections.emptySet(), root, getNamePathMapper());
+ }
+}
\ No newline at end of file
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java
index 665b0c0..196bd49 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UserManagerImplTest.java
@@ -16,13 +16,10 @@
*/
package org.apache.jackrabbit.oak.security.user;
-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;
@@ -32,19 +29,13 @@
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.junit.After;
import org.junit.Assert;
@@ -55,23 +46,17 @@
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;
/**
@@ -416,26 +401,4 @@
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
diff --git a/oak-doc/src/site/markdown/security/user/authorizableaction.md b/oak-doc/src/site/markdown/security/user/authorizableaction.md
index 0d431d6..f8a5d3b 100644
--- a/oak-doc/src/site/markdown/security/user/authorizableaction.md
+++ b/oak-doc/src/site/markdown/security/user/authorizableaction.md
@@ -91,6 +91,7 @@
- implement `AuthorizableActionProvider` interface exposing your custom action(s).
- make the provider implementation an OSGi service and make it available to the Oak repository.
+- make sure the `AuthorizableActionProvider` is listed as required service with the `SecurityProvider` (see also [Introduction](../introduction.html#configuration]))
##### Examples
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AuthorizableAction.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AuthorizableAction.java
index 18d23b6..434fb50 100644
--- a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AuthorizableAction.java
+++ b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/AuthorizableAction.java
@@ -25,6 +25,7 @@
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.jetbrains.annotations.NotNull;
/**
* The {@code AuthorizableAction} interface provide an implementation
@@ -90,6 +91,22 @@
void onCreate(User user, String password, Root root, NamePathMapper namePathMapper) throws RepositoryException;
/**
+ * Allows to add application specific modifications or validation associated
+ * with the creation of a new <strong>system</strong>system. Note, that this method is called
+ * <strong>before</strong> any {@code Root#commit()} call.
+ *
+ *
+ * @param user The new system user that has not yet been persisted;
+ * e.g. the associated tree is still 'NEW'.
+ * @param root The root associated with the user manager.
+ * @param namePathMapper The {@code NamePathMapper} present with the editing session.
+ * @throws RepositoryException If an error occurs.
+ */
+ default void onCreate(@NotNull User systemUser, @NotNull Root root, @NotNull NamePathMapper namePathMapper) throws RepositoryException {
+ // nop
+ }
+
+ /**
* Allows to add application specific behavior associated with the removal
* of an authorizable. Note, that this method is called <strong>before</strong>
* {@link org.apache.jackrabbit.api.security.user.Authorizable#remove} is executed (and persisted); thus the
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/package-info.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/package-info.java
index 88739fd..31a21e0 100644
--- a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/package-info.java
+++ b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/package-info.java
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-@Version("1.2.0")
+@Version("1.3.0")
package org.apache.jackrabbit.oak.spi.security.user.action;
import org.osgi.annotation.versioning.Version;
diff --git a/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AbstractAuthorizableActionTest.java b/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AbstractAuthorizableActionTest.java
index bf6b7d2..7168d82 100644
--- a/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AbstractAuthorizableActionTest.java
+++ b/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/AbstractAuthorizableActionTest.java
@@ -27,6 +27,7 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
public class AbstractAuthorizableActionTest {
@@ -43,12 +44,24 @@
}
@Test
- public void testOnCreate() throws Exception {
+ public void testOnCreateGroup() throws Exception {
Group gr = mock(Group.class);
action.onCreate(gr, root, namePathMapper);
+ verifyNoInteractions(gr, root, namePathMapper);
+ }
+
+ @Test
+ public void testOnCreateUser() throws Exception {
User user = mock(User.class);
action.onCreate(user, null, root, namePathMapper);
- verifyNoInteractions(user, gr, root, namePathMapper);
+ verifyNoInteractions(user, root, namePathMapper);
+ }
+
+ @Test
+ public void testOnCreateSystemUser() throws Exception {
+ User user = when(mock(User.class).isSystemUser()).thenReturn(true).getMock();
+ action.onCreate(user, root, namePathMapper);
+ verifyNoInteractions(user, root, namePathMapper);
}
@Test
