OAK-9868 : Introduce interface to extract tree from users (#660)
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/DynamicGroupUtil.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/DynamicGroupUtil.java
index d49479b..8612c64 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/DynamicGroupUtil.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/DynamicGroupUtil.java
@@ -22,6 +22,7 @@
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
@@ -65,7 +66,7 @@
@NotNull
static Tree getTree(@NotNull Authorizable authorizable, @NotNull Root root) throws RepositoryException {
- return root.getTree(authorizable.getPath());
+ return (authorizable instanceof TreeAware) ? ((TreeAware) authorizable).getTree() : root.getTree(authorizable.getPath());
}
static boolean hasStoredMemberInfo(@NotNull Group group, @NotNull Root root) {
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProviderTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProviderTest.java
index c6e6329..4925a1b 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProviderTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProviderTest.java
@@ -30,6 +30,7 @@
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalGroup;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentity;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityException;
@@ -70,6 +71,7 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.withSettings;
public class ExternalGroupPrincipalProviderTest extends AbstractPrincipalTest {
@@ -401,8 +403,10 @@
@Test
public void testGetPrincipalsNonExistingUserTree() throws Exception {
- Authorizable a = spy(getUserManager(root).getAuthorizable(USER_ID));
+ Authorizable a = mock(Authorizable.class, withSettings().extraInterfaces(User.class));
+ when(a.getID()).thenReturn(USER_ID);
when(a.getPath()).thenReturn("/path/to/non/existing/item");
+
UserManager um = when(mock(UserManager.class).getAuthorizable(USER_ID)).thenReturn(a).getMock();
UserConfiguration uc = when(mock(UserConfiguration.class).getUserManager(root, getNamePathMapper())).thenReturn(um).getMock();
@@ -415,6 +419,9 @@
Authorizable group = getUserManager(root).createGroup("testGroup");
Authorizable a = spy(getUserManager(root).getAuthorizable(USER_ID));
when(a.getPath()).thenReturn(group.getPath());
+ if (a instanceof TreeAware && group instanceof TreeAware) {
+ when(((TreeAware) a).getTree()).thenReturn(((TreeAware)group).getTree());
+ }
UserManager um = when(mock(UserManager.class).getAuthorizable(USER_ID)).thenReturn(a).getMock();
UserConfiguration uc = when(mock(UserConfiguration.class).getUserManager(root, getNamePathMapper())).thenReturn(um).getMock();
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenProviderImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenProviderImpl.java
index 82b2e56..0ea39ff 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenProviderImpl.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authentication/token/TokenProviderImpl.java
@@ -42,7 +42,9 @@
import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.namepath.PathMapper;
import org.apache.jackrabbit.oak.plugins.identifier.IdentifierManager;
+import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.spi.namespace.NamespaceConstants;
import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
import org.apache.jackrabbit.oak.spi.security.authentication.ImpersonationCredentials;
@@ -381,7 +383,7 @@
String userPath = user.getPath();
parentPath = userPath + '/' + TOKENS_NODE_NAME;
- Tree userNode = root.getTree(userPath);
+ Tree userNode = getTree(user, userPath);
tokenParent = TreeUtil.getOrAddChild(userNode, TOKENS_NODE_NAME, TOKENS_NT_NAME);
root.commit();
@@ -403,6 +405,14 @@
return tokenParent;
}
+ public @NotNull Tree getTree(@NotNull User user, @NotNull String userPath) {
+ if (user instanceof TreeAware) {
+ return ((TreeAware) user).getTree();
+ } else {
+ return root.getTree(userPath);
+ }
+ }
+
/**
* Create a new token node below the specified {@code parent}.
*
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
index 1369abb..7e44186 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/AuthorizableImpl.java
@@ -23,6 +23,7 @@
import org.apache.jackrabbit.commons.iterator.RangeIteratorAdapter;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.security.user.monitor.UserMonitor;
import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType;
import org.apache.jackrabbit.oak.spi.security.user.DynamicMembershipProvider;
@@ -43,7 +44,7 @@
/**
* Base class for {@code User} and {@code Group} implementations.
*/
-abstract class AuthorizableImpl implements Authorizable, UserConstants {
+abstract class AuthorizableImpl implements Authorizable, UserConstants, TreeAware {
/**
* logger instance
@@ -179,10 +180,11 @@
String typeStr = (isGroup()) ? "Group '" : "User '";
return new StringBuilder().append(typeStr).append(id).append('\'').toString();
}
-
- //--------------------------------------------------------------------------
+
+ //----------------------------------------------------------< TreeAware >---
+ @Override
@NotNull
- Tree getTree() {
+ public Tree getTree() {
if (tree.exists()) {
return tree;
} else {
@@ -190,6 +192,8 @@
}
}
+ //--------------------------------------------------------------------------
+
@Nullable
String getPrincipalNameOrNull() {
if (principalName == null) {
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserAuthentication.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserAuthentication.java
index 8d0828d..09b35cf 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserAuthentication.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/UserAuthentication.java
@@ -239,12 +239,7 @@
@Nullable
private Long getPasswordLastModified(@NotNull User user) throws RepositoryException {
- Tree userTree;
- if (user instanceof UserImpl) {
- userTree = ((UserImpl) user).getTree();
- } else {
- userTree = root.getTree(user.getPath());
- }
+ Tree userTree = Utils.getTree(user, root);
PropertyState property = userTree.getChild(REP_PWD).getProperty(REP_PASSWORD_LAST_MODIFIED);
return (property != null) ? property.getValue(Type.LONG) : null;
}
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 1b36403..90a0af8 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
@@ -640,7 +640,7 @@
if (!nonExisting.isEmpty()) {
Stopwatch watch = Stopwatch.createStarted();
log.debug("ImportBehavior.BESTEFFORT: Found {} entries of rep:members pointing to non-existing authorizables. Adding to rep:members.", nonExisting.size());
- Tree groupTree = root.getTree(gr.getPath());
+ Tree groupTree = Utils.getTree(gr, root);
MembershipProvider membershipProvider = userManager.getMembershipProvider();
@@ -741,7 +741,7 @@
// 2. adjust set of impersonators
List<String> nonExisting = updateImpersonators(a, imp, toRemove, toAdd);
if (!nonExisting.isEmpty()) {
- Tree userTree = checkNotNull(root.getTree(a.getPath()));
+ Tree userTree = Utils.getTree(a, root);
// copy over all existing impersonators to the nonExisting list
PropertyState impersonators = userTree.getProperty(REP_IMPERSONATORS);
if (impersonators != null) {
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/Utils.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/Utils.java
index f0f1903..197f661 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/Utils.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/user/Utils.java
@@ -18,8 +18,10 @@
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.User;
+import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
@@ -103,4 +105,13 @@
return null;
}
}
+
+ @NotNull
+ static Tree getTree(@NotNull Authorizable authorizable, @NotNull Root root) throws RepositoryException {
+ if (authorizable instanceof TreeAware) {
+ return ((TreeAware) authorizable).getTree();
+ } else {
+ return root.getTree(authorizable.getPath());
+ }
+ }
}
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UtilsTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UtilsTest.java
index 362d782..6462817 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UtilsTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/security/user/UtilsTest.java
@@ -21,8 +21,10 @@
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.oak.AbstractSecurityTest;
+import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
import org.jetbrains.annotations.NotNull;
@@ -35,8 +37,13 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
+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 UtilsTest extends AbstractSecurityTest {
@@ -149,4 +156,34 @@
when(a.isGroup()).thenReturn(true);
assertFalse(Utils.isEveryone(a));
}
+
+ @Test
+ public void testGetTreeFromTreeAware() throws Exception {
+ Tree t = mock(Tree.class);
+ Root r = mock(Root.class);
+
+ Authorizable a = mock(Authorizable.class, withSettings().extraInterfaces(TreeAware.class));
+ when(((TreeAware) a).getTree()).thenReturn(t);
+
+ assertSame(t, Utils.getTree(a, r));
+
+ verifyNoInteractions(r);
+ verify((TreeAware) a).getTree();
+ verifyNoMoreInteractions(a);
+ }
+
+ @Test
+ public void testGetTree() throws Exception {
+ Tree t = mock(Tree.class);
+ Root r = when(mock(Root.class).getTree("/user/path")).thenReturn(t).getMock();
+
+ Authorizable a = mock(Authorizable.class);
+ when(a.getPath()).thenReturn("/user/path");
+
+ assertSame(t, Utils.getTree(a, r));
+
+ verify(r).getTree(anyString());
+ verify(a).getPath();
+ verifyNoMoreInteractions(a, r);
+ }
}
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/plugins/tree/TreeAware.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/plugins/tree/TreeAware.java
new file mode 100644
index 0000000..db24b0f
--- /dev/null
+++ b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/plugins/tree/TreeAware.java
@@ -0,0 +1,32 @@
+/*
+ * 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.plugins.tree;
+
+import org.apache.jackrabbit.oak.api.Tree;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Oak internal utility interface to avoid repeated retrieval of an underlying {@link Tree}.
+ * Note, that is is the responsibility of the caller to make user that the content session and root
+ * used to retrieve {@link TreeAware} object remains the same.
+ */
+public interface TreeAware {
+
+ @NotNull
+ Tree getTree();
+
+}
\ No newline at end of file
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/plugins/tree/package-info.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/plugins/tree/package-info.java
index 221dffa..6141ac0 100644
--- a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/plugins/tree/package-info.java
+++ b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/plugins/tree/package-info.java
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-@Version("3.2.1")
+@Version("3.3.0")
package org.apache.jackrabbit.oak.plugins.tree;
import org.osgi.annotation.versioning.Version;
diff --git a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordChangeAction.java b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordChangeAction.java
index 3efcbad..014e953 100644
--- a/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordChangeAction.java
+++ b/oak-security-spi/src/main/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordChangeAction.java
@@ -21,7 +21,9 @@
import org.apache.jackrabbit.api.security.user.User;
import org.apache.jackrabbit.oak.api.Root;
+import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
+import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.apache.jackrabbit.oak.spi.security.user.util.PasswordUtil;
import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
@@ -55,7 +57,8 @@
//------------------------------------------------------------< private >---
@Nullable
- private String getPasswordHash(@NotNull Root root, @NotNull User user) throws RepositoryException {
- return TreeUtil.getString(root.getTree(user.getPath()), UserConstants.REP_PASSWORD);
+ private static String getPasswordHash(@NotNull Root root, @NotNull User user) throws RepositoryException {
+ Tree tree = (user instanceof TreeAware) ? ((TreeAware)user).getTree() : root.getTree(user.getPath());
+ return TreeUtil.getString(tree, UserConstants.REP_PASSWORD);
}
}
diff --git a/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordChangeActionTest.java b/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordChangeActionTest.java
index 784712e..aaf0567 100644
--- a/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordChangeActionTest.java
+++ b/oak-security-spi/src/test/java/org/apache/jackrabbit/oak/spi/security/user/action/PasswordChangeActionTest.java
@@ -21,6 +21,7 @@
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
+import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.spi.security.ConfigurationParameters;
import org.apache.jackrabbit.oak.spi.security.SecurityProvider;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
@@ -28,19 +29,23 @@
import org.jetbrains.annotations.Nullable;
import org.junit.Before;
import org.junit.Test;
-import org.mockito.Mockito;
import javax.jcr.nodetype.ConstraintViolationException;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+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 PasswordChangeActionTest {
private static final String USER_PATH = "/userpath";
- private final NamePathMapper namePathMapper = Mockito.mock(NamePathMapper.class);
+ private final NamePathMapper namePathMapper = mock(NamePathMapper.class);
private PasswordChangeAction pwChangeAction;
@@ -49,19 +54,19 @@
@Before
public void before() throws Exception {
pwChangeAction = new PasswordChangeAction();
- pwChangeAction.init(Mockito.mock(SecurityProvider.class), ConfigurationParameters.EMPTY);
+ pwChangeAction.init(mock(SecurityProvider.class), ConfigurationParameters.EMPTY);
- user = Mockito.mock(User.class);
+ user = mock(User.class);
when(user.getPath()).thenReturn(USER_PATH);
}
private static Root createRoot(@Nullable String pw) throws Exception {
- Tree userTree = Mockito.mock(Tree.class);
+ Tree userTree = mock(Tree.class);
if (pw != null) {
String pwHash = PasswordUtil.buildPasswordHash(pw);
when(userTree.getProperty(UserConstants.REP_PASSWORD)).thenReturn(PropertyStates.createProperty(UserConstants.REP_PASSWORD, pwHash));
}
- Root root = Mockito.mock(Root.class);
+ Root root = mock(Root.class);
when(root.getTree(USER_PATH)).thenReturn(userTree);
return root;
}
@@ -89,4 +94,20 @@
verify(user).getPath();
verifyNoMoreInteractions(user);
}
+
+ @Test
+ public void testUserIsTreeAware() throws Exception {
+ Root r = createRoot("pw");
+ Tree t = r.getTree(USER_PATH);
+
+ User u = mock(User.class, withSettings().extraInterfaces(TreeAware.class));
+ when(((TreeAware) u).getTree()).thenReturn(t);
+
+ pwChangeAction.onPasswordChange(u, "changedPassword", r, namePathMapper);
+
+ verify(u, never()).getPath();
+ verify(((TreeAware) u)).getTree();
+ verify(r).getTree(USER_PATH);
+ verifyNoMoreInteractions(r, u);
+ }
}
