You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2022/07/29 12:54:50 UTC
[jackrabbit-oak] branch trunk updated: OAK-9871 : AutoMembershipPrincipals.getAutoMembership must resolved inherited groups
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 088764cf8e OAK-9871 : AutoMembershipPrincipals.getAutoMembership must resolved inherited groups
088764cf8e is described below
commit 088764cf8ec12f473a97d5350055a17204d10850
Author: angela <an...@adobe.com>
AuthorDate: Fri Jul 29 14:54:39 2022 +0200
OAK-9871 : AutoMembershipPrincipals.getAutoMembership must resolved inherited groups
---
.../impl/principal/AutoMembershipPrincipals.java | 112 +++++++++++-----
.../impl/principal/AutoMembershipProvider.java | 15 +--
.../principal/ExternalGroupPrincipalProvider.java | 3 +-
.../principal/AutoMembershipPrincipalsTest.java | 148 +++++++++++++++++++--
.../impl/principal/AutoMembershipProviderTest.java | 19 ++-
.../ExternalGroupPrincipalProviderTest.java | 3 +-
.../PrincipalProviderAutoMembershipTest.java | 51 ++++++-
7 files changed, 285 insertions(+), 66 deletions(-)
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipals.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipals.java
index 4d5fbed7db..11a46fba01 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipals.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipals.java
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.prin
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterators;
+import com.google.common.collect.Maps;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.UserManager;
@@ -31,7 +32,6 @@ import org.slf4j.LoggerFactory;
import javax.jcr.RepositoryException;
import java.security.Principal;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -148,64 +148,110 @@ final class AutoMembershipPrincipals {
*
* @param idpName The name of an IDP
* @param authorizable The target user/group
+ * @param includeInherited Flag indicating if inherited groups should be resolved for global automemberbship groups.
* @return A collection of principals the given authorizable is an automatic member of.
*/
@NotNull
- Collection<Principal> getAutoMembership(@NotNull String idpName, @NotNull Authorizable authorizable) {
- ImmutableSet.Builder<Principal> builder = ImmutableSet.builder();
-
+ Map<Principal,Group> getAutoMembership(@NotNull String idpName, @NotNull Authorizable authorizable, boolean includeInherited) {
// global auto-membership
- builder.addAll(collectGlobalAutoMembershipPrincipals(idpName));
-
+ Map<Principal,Group> map = collectGlobalAutoMembershipPrincipals(idpName);
+ if (includeInherited) {
+ for (Group gr : map.values().toArray(new Group[0])) {
+ collectInheritedPrincipals(gr, map);
+ }
+ }
// conditional auto-membership
AutoMembershipConfig config = autoMembershipConfigMap.get(idpName);
if (config != null) {
- config.getAutoMembership(authorizable).forEach(groupId -> addVerifiedPrincipal(groupId, builder));
+ config.getAutoMembership(authorizable).forEach(groupId -> addVerifiedPrincipal(groupId, map, includeInherited));
}
- return builder.build();
+ return map;
}
-
- @NotNull
- private Set<Principal> collectGlobalAutoMembershipPrincipals(@NotNull String idpName) {
+
+ private Map<Principal, Group> collectGlobalAutoMembershipPrincipals(@NotNull String idpName) {
+ Map<Principal, Group> map = Maps.newHashMap();
if (!principalMap.containsKey(idpName)) {
- ImmutableSet.Builder<Principal> builder = ImmutableSet.builder();
String[] vs = autoMembershipMapping.get(idpName);
if (vs != null) {
for (String groupId : vs) {
- addVerifiedPrincipal(groupId, builder);
+ addVerifiedPrincipal(groupId, map, false);
}
}
- Set<Principal> principals = builder.build();
- principalMap.put(idpName, principals);
- return principals;
+ // only cache the principal instance but not the group (tree might become disconnected)
+ principalMap.put(idpName, ImmutableSet.copyOf(map.keySet()));
} else {
- return principalMap.get(idpName);
+ // resolve Group objects from cached principals
+ principalMap.get(idpName).forEach(groupPrincipal -> {
+ Group gr = retrieveGroup(groupPrincipal);
+ if (gr != null) {
+ map.put(groupPrincipal, gr);
+ }
+
+ });
}
+ return map;
}
-
- private void addVerifiedPrincipal(@NotNull String groupId, @NotNull ImmutableSet.Builder<Principal> builder) {
- Principal principal = getVerifiedPrincipal(groupId);
- if (principal != null) {
- builder.add(principal);
+
+ private static void collectInheritedPrincipals(@NotNull Group group,
+ @NotNull Map<Principal, Group> map) {
+ try {
+ Iterator<Group> groups = group.memberOf();
+ while (groups.hasNext()) {
+ Group gr = groups.next();
+ Principal p = getVerifiedPrincipal(gr);
+ if (p != null) {
+ map.put(p, gr);
+ }
+ }
+ } catch (RepositoryException e) {
+ log.warn("Error while resolving inherited auto-membership", e);
}
}
-
+
+ private void addVerifiedPrincipal(@NotNull String groupId, @NotNull Map<Principal,Group> builder,
+ boolean includeInherited) {
+ try {
+ Authorizable a = userManager.getAuthorizable(groupId);
+ if (a == null || !a.isGroup()) {
+ log.warn("Configured auto-membership group {} does not exist -> Ignoring", groupId);
+ return;
+ }
+
+ Group group = (Group) a;
+ Principal principal = getVerifiedPrincipal(group);
+ if (principal != null) {
+ builder.put(principal, group);
+ if (includeInherited) {
+ collectInheritedPrincipals(group, builder);
+ }
+ }
+ } catch (RepositoryException e) {
+ log.debug("Failed to retrieved 'auto-membership' group with id {}", groupId, e);
+ }
+ }
+
@Nullable
- private Principal getVerifiedPrincipal(@NotNull String groupId) {
+ private static Principal getVerifiedPrincipal(@NotNull Group group) throws RepositoryException {
+ Principal grPrincipal = group.getPrincipal();
+ if (GroupPrincipals.isGroup(grPrincipal)) {
+ return grPrincipal;
+ } else {
+ log.warn("Principal of group {} is not of group type -> Ignoring", group.getID());
+ return null;
+ }
+ }
+
+ @Nullable
+ private Group retrieveGroup(@NotNull Principal principal) {
try {
- Authorizable gr = userManager.getAuthorizable(groupId);
+ Authorizable gr = userManager.getAuthorizable(principal);
if (gr != null && gr.isGroup()) {
- Principal grPrincipal = gr.getPrincipal();
- if (GroupPrincipals.isGroup(grPrincipal)) {
- return grPrincipal;
- } else {
- log.warn("Principal of group {} is not of group type -> Ignoring", groupId);
- }
+ return (Group) gr;
} else {
- log.warn("Configured auto-membership group {} does not exist -> Ignoring", groupId);
+ log.warn("Cannot retrieve group from principal {} -> Ignoring", principal);
}
} catch (RepositoryException e) {
- log.debug("Failed to retrieved 'auto-membership' group with id {}", groupId, e);
+ log.debug("Failed to retrieved 'auto-membership' group for principal {}", principal.getName(), e);
}
return null;
}
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipProvider.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipProvider.java
index 48c33db4d5..b624679e8d 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipProvider.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipProvider.java
@@ -53,7 +53,6 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Function;
-import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants.REP_EXTERNAL_ID;
@@ -130,19 +129,7 @@ class AutoMembershipProvider implements DynamicMembershipProvider {
// not an external user (NOTE: with dynamic membership enabled external groups will not be synced into the repository)
return RangeIteratorAdapter.EMPTY;
}
- Collection<Principal> groupPrincipals = autoMembershipPrincipals.getAutoMembership(idpName, authorizable);
- Set<Group> groups = groupPrincipals.stream().map(principal -> {
- try {
- Authorizable a = userManager.getAuthorizable(principal);
- if (a != null && a.isGroup()) {
- return (Group) a;
- } else {
- return null;
- }
- } catch (RepositoryException e) {
- return null;
- }
- }).filter(Objects::nonNull).collect(Collectors.toSet());
+ Collection<Group> groups = autoMembershipPrincipals.getAutoMembership(idpName, authorizable, false).values();
Iterator<Group> groupIt = new RangeIteratorAdapter(groups);
if (!includeInherited) {
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
index f325556de3..f92f0d3c95 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProvider.java
@@ -237,7 +237,8 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
// add existing group principals as defined with the _autoMembership_ option.
String idpName = getIdpName(userTree);
if (idpName != null) {
- groupPrincipals.addAll(autoMembershipPrincipals.getAutoMembership(idpName, authorizable));
+ // resolve automembership including inherited group membership
+ groupPrincipals.addAll(autoMembershipPrincipals.getAutoMembership(idpName, authorizable, true).keySet());
}
return groupPrincipals;
}
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipalsTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipalsTest.java
index 33ad5cd6c1..9db2c568f7 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipalsTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipPrincipalsTest.java
@@ -19,10 +19,13 @@ package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.prin
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterators;
import com.google.common.collect.Sets;
+import org.apache.jackrabbit.api.security.principal.GroupPrincipal;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.api.security.user.UserManager;
import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.AutoMembershipConfig;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
+import org.jetbrains.annotations.NotNull;
import org.junit.Before;
import org.junit.Test;
@@ -41,6 +44,7 @@ import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -70,15 +74,20 @@ public class AutoMembershipPrincipalsTest extends AbstractAutoMembershipTest {
return Collections.singletonMap(IDP_VALID_AM, amConfig);
}
+ @NotNull
+ private Set<Principal> getAutoMembership(@NotNull String idpName, @NotNull Authorizable authorizable, boolean includeInherited) {
+ return amp.getAutoMembership(idpName, authorizable, includeInherited).keySet();
+ }
+
@Test
public void testGetPrincipalsUnknownIdp() {
- assertTrue(amp.getAutoMembership("unknown",authorizable).isEmpty());
+ assertTrue(amp.getAutoMembership("unknown",authorizable, false).isEmpty());
verifyNoInteractions(authorizable, amConfig);
}
@Test
public void testGetPrincipalsUnknownGroup() {
- Collection<Principal> principals = amp.getAutoMembership(IDP_INVALID_AM, authorizable);
+ Collection<Principal> principals = getAutoMembership(IDP_INVALID_AM, authorizable, false);
assertTrue(principals.isEmpty());
verifyNoInteractions(authorizable, amConfig);
}
@@ -87,7 +96,7 @@ public class AutoMembershipPrincipalsTest extends AbstractAutoMembershipTest {
public void testGetPrincipalsMultipleGroups() throws Exception {
when(amConfig.getAutoMembership(authorizable)).thenReturn(Collections.emptySet());
- Collection<Principal> principals = amp.getAutoMembership(IDP_VALID_AM, authorizable);
+ Collection<Principal> principals = getAutoMembership(IDP_VALID_AM, authorizable, false);
assertFalse(principals.isEmpty());
Set<Principal> expected = Sets.newHashSet(automembershipGroup1.getPrincipal(), automembershipGroup2.getPrincipal());
assertEquals(expected, principals);
@@ -95,7 +104,7 @@ public class AutoMembershipPrincipalsTest extends AbstractAutoMembershipTest {
// change behavior of automembership-config
when(amConfig.getAutoMembership(authorizable)).thenReturn(ImmutableSet.of(automembershipGroup3.getID()));
- principals = amp.getAutoMembership(IDP_VALID_AM, authorizable);
+ principals = getAutoMembership(IDP_VALID_AM, authorizable, false);
assertFalse(principals.isEmpty());
expected.add(automembershipGroup3.getPrincipal());
assertEquals(expected, principals);
@@ -106,11 +115,25 @@ public class AutoMembershipPrincipalsTest extends AbstractAutoMembershipTest {
@Test
public void testGetPrincipalsMixed() throws Exception {
- Collection<Principal> principals = amp.getAutoMembership(IDP_MIXED_AM, authorizable);
+ Collection<Principal> principals = getAutoMembership(IDP_MIXED_AM, authorizable, false);
assertFalse(principals.isEmpty());
assertEquals(ImmutableSet.of(automembershipGroup1.getPrincipal()), ImmutableSet.copyOf(principals));
verifyNoInteractions(authorizable, amConfig);
}
+
+ @Test
+ public void testGetPrincipalsInherited() throws Exception {
+ when(amConfig.getAutoMembership(authorizable)).thenReturn(Collections.emptySet());
+
+ Group testGroup = getTestGroup(automembershipGroup1, automembershipGroup2);
+
+ Collection<Principal> principals = getAutoMembership(IDP_VALID_AM, authorizable, true);
+ Set<Principal> expected = Sets.newHashSet(testGroup.getPrincipal(), automembershipGroup1.getPrincipal(), automembershipGroup2.getPrincipal());
+ assertEquals(expected, principals);
+
+ verifyNoInteractions(authorizable);
+ verify(amConfig).getAutoMembership(authorizable);
+ }
@Test
public void testGroupLookupFails() throws Exception {
@@ -118,11 +141,120 @@ public class AutoMembershipPrincipalsTest extends AbstractAutoMembershipTest {
when(um.getAuthorizable(anyString())).thenThrow(new RepositoryException());
AutoMembershipPrincipals amprincipals = new AutoMembershipPrincipals(um, MAPPING, getAutoMembershipConfigMapping());
- assertTrue(amprincipals.getAutoMembership(IDP_VALID_AM, authorizable).isEmpty());
+ assertTrue(amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, false).isEmpty());
verifyNoInteractions(authorizable);
verify(amConfig).getAutoMembership(authorizable);
verifyNoMoreInteractions(amConfig);
}
+
+ @Test
+ public void testGroupLookupByPrincipalFails() throws Exception {
+ UserManager um = spy(userManager);
+
+ AutoMembershipPrincipals amprincipals = new AutoMembershipPrincipals(um, MAPPING, Collections.emptyMap());
+ assertEquals(2, amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, false).size());
+
+ when(um.getAuthorizable(automembershipGroup1.getPrincipal())).thenThrow(new RepositoryException());
+ assertEquals(1, amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, false).size());
+ }
+
+ @Test
+ public void testGroupLookupByPrincipalReturnsUser() throws Exception {
+ UserManager um = spy(userManager);
+
+ AutoMembershipPrincipals amprincipals = new AutoMembershipPrincipals(um, MAPPING, Collections.emptyMap());
+ assertEquals(2, amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, false).size());
+
+ Authorizable a = mock(Authorizable.class);
+ when(a.isGroup()).thenReturn(false);
+ when(um.getAuthorizable(automembershipGroup1.getPrincipal())).thenReturn(a);
+ assertEquals(1, amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, false).size());
+
+ verify(a).isGroup();
+ }
+
+ @Test
+ public void testInheritedGroupLookupFails() throws Exception {
+ UserManager um = spy(userManager);
+
+ Group testGroup = getTestGroup(automembershipGroup1);
+
+ AutoMembershipPrincipals amprincipals = new AutoMembershipPrincipals(um, MAPPING, Collections.emptyMap());
+ Map<Principal, Group> map = amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, true);
+ assertEquals(3, map.size());
+
+ // make sure membership lookup for automembershipGroup1 fails
+ Group gr = mock(Group.class);
+ when(gr.isGroup()).thenReturn(true);
+ when(gr.memberOf()).thenThrow(new RepositoryException());
+
+ when(um.getAuthorizable(automembershipGroup1.getPrincipal())).thenReturn(gr);
+
+ // principals from cache: looking up group by principal returns
+ // 'gr' which fails upon memberof call -> should be skipped.
+ map = amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, true);
+ assertEquals(2, map.size());
+
+ verify(gr).isGroup();
+ verify(gr).memberOf();
+ verifyNoMoreInteractions(gr);
+ reset(gr, um);
+ }
+
+ @Test
+ public void testInheritedFromAMConfig() throws Exception {
+ Group testGroup = getTestGroup(automembershipGroup3);
+
+ AutoMembershipPrincipals amprincipals = new AutoMembershipPrincipals(userManager, Collections.emptyMap(),
+ getAutoMembershipConfigMapping());
+
+ Map<Principal, Group> map = amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, true);
+ assertEquals(2, map.size());
+
+ Set<Principal> principals = map.keySet();
+ assertTrue(principals.contains(testGroup.getPrincipal()));
+ assertTrue(principals.contains(automembershipGroup3.getPrincipal()));
+ }
+
+ @Test
+ public void testInheritedGroupPrincipalInvalid() throws Exception {
+ UserManager um = spy(userManager);
+
+ Group testGroup = getTestGroup(automembershipGroup1);
+ GroupPrincipal p = (GroupPrincipal) testGroup.getPrincipal();
+
+ AutoMembershipPrincipals amprincipals = new AutoMembershipPrincipals(um, MAPPING, Collections.emptyMap());
+ Map<Principal, Group> map = amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, true);
+ assertEquals(3, map.size());
+
+ // make sure membership lookup for automembershipGroup1 fails
+ Principal invalid = new PrincipalImpl("invalid");
+ Group inherited = mock(Group.class);
+ when(inherited.isGroup()).thenReturn(true);
+ when(inherited.getPrincipal()).thenReturn(invalid);
+
+ Group gr = mock(Group.class);
+ when(gr.isGroup()).thenReturn(true);
+ when(gr.memberOf()).thenReturn(Iterators.singletonIterator(inherited));
+ when(um.getAuthorizable(automembershipGroup1.getPrincipal())).thenReturn(gr);
+
+ // retrieve from cache
+ map = amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, true);
+ assertEquals(2, map.size());
+
+ Set<Principal> result = map.keySet();
+ assertFalse(result.contains(p));
+ assertFalse(result.contains(invalid));
+ assertFalse(map.containsValue(testGroup));
+
+ verify(gr).isGroup();
+ verify(gr).memberOf();
+
+ verify(inherited).getPrincipal();
+ verify(inherited).getID();
+ verifyNoMoreInteractions(gr, inherited);
+ reset(gr, inherited, um);
+ }
@Test
public void testEmptyMapping() {
@@ -132,7 +264,7 @@ public class AutoMembershipPrincipalsTest extends AbstractAutoMembershipTest {
AutoMembershipPrincipals amp = new AutoMembershipPrincipals(um, m, m2);
// verify 'getAutoMembership'
- assertTrue(amp.getAutoMembership(IDP_VALID_AM, authorizable).isEmpty());
+ assertTrue(amp.getAutoMembership(IDP_VALID_AM, authorizable, false).isEmpty());
verify(m, times(1)).size();
verify(m, times(1)).get(IDP_VALID_AM);
@@ -174,7 +306,7 @@ public class AutoMembershipPrincipalsTest extends AbstractAutoMembershipTest {
verifyNoMoreInteractions(m);
verifyNoInteractions(m2);
- assertTrue(amprincipals.getAutoMembership(IDP_VALID_AM, authorizable).isEmpty());
+ assertTrue(amprincipals.getAutoMembership(IDP_VALID_AM, authorizable, false).isEmpty());
verify(m, times(1)).get(anyString());
verify(m2, times(1)).get(anyString());
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipProviderTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipProviderTest.java
index 3e6402163f..4db4c62778 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipProviderTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/AutoMembershipProviderTest.java
@@ -27,12 +27,10 @@ import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
import org.jetbrains.annotations.NotNull;
-import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import javax.jcr.RepositoryException;
-import java.security.Principal;
import java.text.ParseException;
import java.util.Iterator;
import java.util.Map;
@@ -44,10 +42,14 @@ 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.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
public class AutoMembershipProviderTest extends AbstractAutoMembershipTest {
@@ -364,13 +366,16 @@ public class AutoMembershipProviderTest extends AbstractAutoMembershipTest {
User user = mock(User.class);
when(user.isGroup()).thenReturn(false);
- when(um.getAuthorizable(automembershipGroup1.getPrincipal())).thenReturn(user);
- when(um.getAuthorizable(automembershipGroup2.getPrincipal())).thenReturn(user);
+ doReturn(user).when(um).getAuthorizable(automembershipGroup1.getID());
+ doReturn(user).when(um).getAuthorizable(automembershipGroup2.getID());
setExternalId(getTestUser().getID(), IDP_VALID_AM);
AutoMembershipProvider amp = createAutoMembershipProvider(root, um);
assertFalse(amp.getMembership(getTestUser(), false).hasNext());
+
+ verify(um, times(2)).getAuthorizable(anyString());
+ verifyNoMoreInteractions(um);
}
@Test
@@ -379,12 +384,15 @@ public class AutoMembershipProviderTest extends AbstractAutoMembershipTest {
User user = mock(User.class);
when(user.isGroup()).thenReturn(false);
- when(um.getAuthorizable(any(Principal.class))).thenThrow(new RepositoryException());
+ doThrow(new RepositoryException()).when(um).getAuthorizable(any(String.class));
setExternalId(getTestUser().getID(), IDP_VALID_AM);
AutoMembershipProvider amp = createAutoMembershipProvider(root, um);
assertFalse(amp.getMembership(getTestUser(), false).hasNext());
+
+ verify(um, times(2)).getAuthorizable(anyString());
+ verifyNoMoreInteractions(um);
}
@Test
@@ -414,6 +422,7 @@ public class AutoMembershipProviderTest extends AbstractAutoMembershipTest {
assertEquals(1, Iterators.size(provider.getMembership(getTestUser(), false)));
assertEquals(1, Iterators.size(provider.getMembership(getTestUser(), true)));
+ // remove second group : but read principal from cache
automembershipGroup2.remove();
assertFalse(provider.getMembership(getTestUser(), false).hasNext());
assertFalse(provider.getMembership(getTestUser(), true).hasNext());
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 57f5cf76e8..23fec75d21 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
@@ -248,7 +248,8 @@ public class ExternalGroupPrincipalProviderTest extends AbstractPrincipalTest {
Set<Principal> expected = getExpectedGroupPrincipals(USER_ID);
// same as in test before even if the principal is not a tree-based-principal
- Set<? extends Principal> principals = principalProvider.getMembershipPrincipals(new PrincipalImpl(user.getPrincipal().getName()));
+ Principal notTreeBased = new PrincipalImpl(user.getPrincipal().getName());
+ Set<? extends Principal> principals = principalProvider.getMembershipPrincipals(notTreeBased);
assertEquals(expected, principals);
}
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/PrincipalProviderAutoMembershipTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/PrincipalProviderAutoMembershipTest.java
index 60b2061892..32e5005dac 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/PrincipalProviderAutoMembershipTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/PrincipalProviderAutoMembershipTest.java
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.prin
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterators;
+import com.google.common.collect.Lists;
import org.apache.jackrabbit.api.security.principal.PrincipalManager;
import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
@@ -30,8 +31,11 @@ import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
import org.jetbrains.annotations.NotNull;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
import java.security.Principal;
+import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
@@ -56,8 +60,16 @@ import static org.mockito.Mockito.when;
* Extension of the {@link ExternalGroupPrincipalProviderTest} with 'automembership'
* configured in the {@link DefaultSyncConfig}.
*/
+@RunWith(Parameterized.class)
public class PrincipalProviderAutoMembershipTest extends ExternalGroupPrincipalProviderTest {
+ @Parameterized.Parameters(name = "name={1}")
+ public static Collection<Object[]> parameters() {
+ return Lists.newArrayList(
+ new Object[] { true, "Nested automembership = true" },
+ new Object[] { false, "Nested automembership = false" });
+ }
+
private static final String USER_AUTO_MEMBERSHIP_GROUP_ID = "testGroup-" + UUID.randomUUID();
private static final String USER_AUTO_MEMBERSHIP_GROUP_PRINCIPAL_NAME = "p-" + USER_AUTO_MEMBERSHIP_GROUP_ID;
@@ -73,17 +85,34 @@ public class PrincipalProviderAutoMembershipTest extends ExternalGroupPrincipalP
private final AutoMembershipConfig amc = when(mock(AutoMembershipConfig.class).getAutoMembership(any(Authorizable.class)))
.thenReturn(Collections.singleton(CONFIG_AUTO_MEMBERSHIP_GROUP_ID)).getMock();
+ private final boolean nestedAutomembership;
+
private Group userAutoMembershipGroup;
private Group groupAutoMembershipGroup;
private Group configAutoMembershipGroup;
+ private Group baseGroup;
+ private Group baseGroup2;
+
+ public PrincipalProviderAutoMembershipTest(boolean nestedAutomembership, @NotNull String name) {
+ this.nestedAutomembership = nestedAutomembership;
+ }
@Override
public void before() throws Exception {
super.before();
- userAutoMembershipGroup = getUserManager(root).createGroup(USER_AUTO_MEMBERSHIP_GROUP_ID, new PrincipalImpl(USER_AUTO_MEMBERSHIP_GROUP_PRINCIPAL_NAME), null);
- groupAutoMembershipGroup = getUserManager(root).createGroup(GROUP_AUTO_MEMBERSHIP_GROUP_ID, new PrincipalImpl(GROUP_AUTO_MEMBERSHIP_GROUP_PRINCIPAL_NAME), null);
- configAutoMembershipGroup = getUserManager(root).createGroup(CONFIG_AUTO_MEMBERSHIP_GROUP_ID, new PrincipalImpl(CONFIG_AUTO_MEMBERSHIP_GROUP_PRINCIPAL_NAME), null);
+ UserManager uMgr = getUserManager(root);
+ userAutoMembershipGroup = uMgr.createGroup(USER_AUTO_MEMBERSHIP_GROUP_ID, new PrincipalImpl(USER_AUTO_MEMBERSHIP_GROUP_PRINCIPAL_NAME), null);
+ groupAutoMembershipGroup = uMgr.createGroup(GROUP_AUTO_MEMBERSHIP_GROUP_ID, new PrincipalImpl(GROUP_AUTO_MEMBERSHIP_GROUP_PRINCIPAL_NAME), null);
+ configAutoMembershipGroup = uMgr.createGroup(CONFIG_AUTO_MEMBERSHIP_GROUP_ID, new PrincipalImpl(CONFIG_AUTO_MEMBERSHIP_GROUP_PRINCIPAL_NAME), null);
+ if (nestedAutomembership) {
+ baseGroup = uMgr.createGroup("base");
+ baseGroup.addMember(userAutoMembershipGroup);
+ baseGroup.addMember(groupAutoMembershipGroup);
+
+ baseGroup2 = uMgr.createGroup("base2");
+ baseGroup2.addMember(configAutoMembershipGroup);
+ }
root.commit();
verify(amc, times(2)).getAutoMembership(any(Authorizable.class));
@@ -102,7 +131,7 @@ public class PrincipalProviderAutoMembershipTest extends ExternalGroupPrincipalP
}
@Override
- AutoMembershipConfig getAutoMembershipConfig() {
+ @NotNull AutoMembershipConfig getAutoMembershipConfig() {
return amc;
}
@@ -113,8 +142,14 @@ public class PrincipalProviderAutoMembershipTest extends ExternalGroupPrincipalP
.addAll(super.getExpectedGroupPrincipals(userId))
.add(userAutoMembershipGroup.getPrincipal())
.add(groupAutoMembershipGroup.getPrincipal());
+ if (nestedAutomembership) {
+ builder.add(baseGroup.getPrincipal());
+ }
if (USER_ID.equals(userId)) {
builder.add(configAutoMembershipGroup.getPrincipal());
+ if (nestedAutomembership) {
+ builder.add(baseGroup2.getPrincipal());
+ }
}
return builder.build();
}
@@ -151,6 +186,10 @@ public class PrincipalProviderAutoMembershipTest extends ExternalGroupPrincipalP
assertTrue(result.contains(userAutoMembershipGroup.getPrincipal()));
assertTrue(result.contains(groupAutoMembershipGroup.getPrincipal()));
assertTrue(result.contains(configAutoMembershipGroup.getPrincipal()));
+ if (nestedAutomembership) {
+ assertTrue(result.contains(baseGroup.getPrincipal()));
+ assertTrue(result.contains(baseGroup2.getPrincipal()));
+ }
assertFalse(result.contains(user.getPrincipal()));
assertEquals(expected, result);
}
@@ -188,6 +227,10 @@ public class PrincipalProviderAutoMembershipTest extends ExternalGroupPrincipalP
assertTrue(result.contains(userAutoMembershipGroup.getPrincipal()));
assertTrue(result.contains(groupAutoMembershipGroup.getPrincipal()));
assertTrue(result.contains(configAutoMembershipGroup.getPrincipal()));
+ if (nestedAutomembership) {
+ assertTrue(result.contains(baseGroup.getPrincipal()));
+ assertTrue(result.contains(baseGroup2.getPrincipal()));
+ }
assertFalse(result.contains(getUserManager(root).getAuthorizable(USER_ID).getPrincipal()));
assertEquals(expected, result);
}