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 2023/01/17 08:26:20 UTC
[jackrabbit-oak] 01/01: OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries
This is an automated email from the ASF dual-hosted git repository.
angela pushed a commit to branch OAK-10067
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 7cc0623cde8c672c362d90a138511af02f3862d4
Author: angela <an...@adobe.com>
AuthorDate: Tue Jan 17 09:25:53 2023 +0100
OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries
---
.../impl/principal/AutoMembershipProvider.java | 62 +--------------
.../external/impl/principal/DynamicGroupUtil.java | 31 ++++++++
.../principal/ExternalGroupPrincipalProvider.java | 49 ++++++++++--
.../principal/InheritedMembershipIterator.java | 87 ++++++++++++++++++++++
.../external/impl/DynamicGroupsTest.java | 79 ++++++++++++++++++++
.../external/impl/DynamicSyncContextTest.java | 4 +-
.../impl/principal/AutoMembershipProviderTest.java | 4 +-
.../impl/principal/DynamicGroupUtilTest.java | 44 +++++++++++
8 files changed, 291 insertions(+), 69 deletions(-)
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 102d6fe9b7..a4c49a70b4 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
@@ -20,7 +20,6 @@ import com.google.common.collect.Iterators;
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.commons.iterator.AbstractLazyIterator;
import org.apache.jackrabbit.commons.iterator.RangeIteratorAdapter;
import org.apache.jackrabbit.oak.api.PropertyValue;
import org.apache.jackrabbit.oak.api.QueryEngine;
@@ -33,8 +32,6 @@ import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.Auto
import org.apache.jackrabbit.oak.spi.security.user.DynamicMembershipProvider;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import javax.jcr.PropertyType;
import javax.jcr.RepositoryException;
@@ -44,7 +41,6 @@ import java.text.ParseException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -61,9 +57,7 @@ import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.NT_REP_U
import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.REP_AUTHORIZABLE_ID;
class AutoMembershipProvider implements DynamicMembershipProvider {
-
- private static final Logger log = LoggerFactory.getLogger(AutoMembershipProvider.class);
-
+
private static final String BINDING_AUTHORIZABLE_IDS = "authorizableIds";
private final Root root;
@@ -174,8 +168,7 @@ class AutoMembershipProvider implements DynamicMembershipProvider {
if (!includeInherited) {
return groupIt;
} else {
- Set<Group> processed = new HashSet<>();
- return Iterators.filter(new InheritedMembershipIterator(groupIt), processed::add);
+ return new InheritedMembershipIterator(groupIt);
}
}
@@ -238,56 +231,5 @@ class AutoMembershipProvider implements DynamicMembershipProvider {
String val = "%;" + idpName.replace("%", "\\%").replace("_", "\\_");
return Collections.singletonMap(BINDING_AUTHORIZABLE_IDS, PropertyValues.newString(val));
}
-
- private static class InheritedMembershipIterator extends AbstractLazyIterator<Group> {
- private final Iterator<Group> groupIterator;
- private final List<Iterator<Group>> inherited = new ArrayList<>();
- private Iterator<Group> inheritedIterator = null;
-
- private InheritedMembershipIterator(Iterator<Group> groupIterator) {
- this.groupIterator = groupIterator;
- }
-
- @Nullable
- @Override
- protected Group getNext() {
- if (groupIterator.hasNext()) {
- Group gr = groupIterator.next();
- try {
- // call 'memberof' to cover nested inheritance
- Iterator<Group> it = gr.memberOf();
- if (it.hasNext()) {
- inherited.add(it);
- }
- } catch (RepositoryException e) {
- log.error("Failed to retrieve membership of group {}", gr, e);
- }
- return gr;
- }
-
- if (inheritedIterator == null || !inheritedIterator.hasNext()) {
- inheritedIterator = getNextInheritedIterator();
- }
-
- if (inheritedIterator.hasNext()) {
- return inheritedIterator.next();
- } else {
- // all inherited groups have been processed
- return null;
- }
- }
-
- @NotNull
- private Iterator<Group> getNextInheritedIterator() {
- if (inherited.isEmpty()) {
- // no more inherited groups to retrieve
- return Collections.emptyIterator();
- } else {
- // no need to verify if the inherited iterator has any elements as this has been asserted before
- // adding it to the list.
- return inherited.remove(0);
- }
- }
- }
}
\ No newline at end of file
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 664611435b..c80f71839f 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
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.prin
import com.google.common.collect.ImmutableSet;
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.api.PropertyState;
import org.apache.jackrabbit.oak.api.ResultRow;
import org.apache.jackrabbit.oak.api.Root;
@@ -37,7 +38,15 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import javax.jcr.RepositoryException;
+import java.security.Principal;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Objects;
import java.util.Set;
+import java.util.Spliterator;
+import java.util.Spliterators;
+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;
@@ -126,4 +135,26 @@ class DynamicGroupUtil {
return false;
}
}
+
+ static Set<Principal> getInheritedPrincipals(@NotNull Principal dynamicGroupPrincipal, @NotNull UserManager userManager) {
+ try {
+ Authorizable gr = userManager.getAuthorizable(dynamicGroupPrincipal);
+ if (gr != null && gr.isGroup()) {
+ Iterator<Group> inherited = gr.memberOf();
+ if (inherited.hasNext()) {
+ Spliterator<Group> spliterator = Spliterators.spliteratorUnknownSize(inherited, 0);
+ return StreamSupport.stream(spliterator, false).map(group -> {
+ try {
+ return group.getPrincipal();
+ } catch (RepositoryException repositoryException) {
+ return null;
+ }
+ }).filter(Objects::nonNull).collect(Collectors.toSet());
+ }
+ }
+ } catch (RepositoryException e) {
+ log.error("Failed to retrieve inherited group principals", e);
+ }
+ return Collections.emptySet();
+ }
}
\ No newline at end of file
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 4e65313c35..37861f2cdf 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
@@ -275,11 +275,23 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
@Override
public boolean isMember(@NotNull Group group, @NotNull Authorizable authorizable, boolean includeInherited) throws RepositoryException {
- if (authorizable.isGroup() || !isDynamic(group) || !isDynamic(authorizable)) {
+ if (authorizable.isGroup() || !isDynamic(authorizable)) {
return false;
} else {
- String principalName = group.getPrincipal().getName();
- return isDynamicMember(principalName, authorizable);
+ if (isDynamic(group) && isDynamicMember(group.getPrincipal().getName(), authorizable)) {
+ return true;
+ }
+ // test for inheritance from dynamic groups through cross-IDP membership
+ if (includeInherited) {
+ Iterator<Group> dynamicGroups = getMembership(authorizable, false);
+ while (dynamicGroups.hasNext()) {
+ Group dynamicGroup = dynamicGroups.next();
+ if (group.isMember(dynamicGroup)) {
+ return true;
+ }
+ }
+ }
+ return false;
}
}
@@ -294,7 +306,7 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
}
Set<Value> valueSet = ImmutableSet.copyOf(vs);
- return Iterators.filter(Iterators.transform(valueSet.iterator(), value -> {
+ Iterator<Group> declared = Iterators.filter(Iterators.transform(valueSet.iterator(), value -> {
try {
String groupPrincipalName = value.getString();
Authorizable gr = userManager.getAuthorizable(new PrincipalImpl(groupPrincipalName));
@@ -303,6 +315,12 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
return null;
}
}), Objects::nonNull);
+ if (includeInherited) {
+ // retrieve groups inherited from dynamic groups through cross-IDP membership
+ return new InheritedMembershipIterator(declared);
+ } else {
+ return declared;
+ }
}
}
@@ -355,7 +373,7 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
}
@NotNull
- private Set<Principal> getGroupPrincipals(@NotNull Authorizable authorizable, @NotNull Tree tree) {
+ private Set<Principal> getGroupPrincipals(@NotNull Authorizable authorizable, @NotNull Tree tree) throws RepositoryException {
if (!tree.exists()) {
return Collections.emptySet();
}
@@ -373,6 +391,9 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
groupPrincipals.add(new ExternalGroupPrincipal(principalName, idpName));
}
+ // add inherited local groups (crossing IDP boundary)
+ groupPrincipals.addAll(getInheritedPrincipals(groupPrincipals, idpName));
+
// add existing group principals as defined with the _autoMembership_ option.
groupPrincipals.addAll(getAutomembershipPrincipals(idpName, authorizable));
return groupPrincipals;
@@ -381,10 +402,28 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
}
} else {
// resolve automembership for dynamic groups
+ // NOTE: no need to resolve inherited local principals as this is covered by the default principal provider
return getAutomembershipPrincipals(idpName, authorizable);
}
}
+ /**
+ * Special handling for the case where dynamic groups have been added to local groups
+ * @return set of inherited group principals
+ */
+ private Set<Principal> getInheritedPrincipals(@NotNull Set<Principal> externalGroupPrincipals, @NotNull String idpName) {
+ if (idpNamesWithDynamicGroups.contains(idpName)) {
+ Set<Principal> inherited = new HashSet<>();
+ for (Principal p : externalGroupPrincipals) {
+ inherited.addAll(DynamicGroupUtil.getInheritedPrincipals(p, userManager));
+ }
+ return inherited;
+ } else {
+ // no dynamic groups created => membership with local groups cannot be created
+ return Collections.emptySet();
+ }
+ }
+
private Set<Principal> getAutomembershipPrincipals(@NotNull String idpName, @NotNull Authorizable authorizable) {
if (authorizable.isGroup()) {
// no need to check for 'groupAutoMembershipPrincipals' being null as it is created if 'idpNamesWithDynamicGroups' is not empty
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/InheritedMembershipIterator.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/InheritedMembershipIterator.java
new file mode 100644
index 0000000000..bf8d2a211a
--- /dev/null
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/InheritedMembershipIterator.java
@@ -0,0 +1,87 @@
+package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal;
+
+import org.apache.jackrabbit.api.security.user.Group;
+import org.apache.jackrabbit.commons.iterator.AbstractLazyIterator;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.jcr.RepositoryException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+
+
+class InheritedMembershipIterator extends AbstractLazyIterator<Group> {
+
+ private static final Logger log = LoggerFactory.getLogger(InheritedMembershipIterator.class);
+
+ private final Iterator<Group> groupIterator;
+ private final List<Iterator<Group>> inherited = new ArrayList<>();
+ private final Set<String> processed = new HashSet<>();
+ private Iterator<Group> inheritedIterator = null;
+
+ InheritedMembershipIterator(@NotNull Iterator<Group> groupIterator) {
+ this.groupIterator = groupIterator;
+ }
+
+ @Nullable
+ @Override
+ protected Group getNext() {
+ if (groupIterator.hasNext()) {
+ Group gr = groupIterator.next();
+ try {
+ // call 'memberof' to cover nested inheritance
+ Iterator<Group> it = gr.memberOf();
+ if (it.hasNext()) {
+ inherited.add(it);
+ }
+ } catch (RepositoryException e) {
+ log.error("Failed to retrieve membership of group {}", gr, e);
+ }
+ return gr;
+ }
+
+ if (inheritedIterator == null) {
+ inheritedIterator = getNextInheritedIterator();
+ }
+
+ while (inheritedIterator.hasNext()) {
+ Group gr = inheritedIterator.next();
+ if (notProcessedBefore(gr)) {
+ return gr;
+ }
+ if (!inheritedIterator.hasNext()) {
+ inheritedIterator = getNextInheritedIterator();
+ }
+ }
+
+ // all inherited groups have been processed
+ return null;
+ }
+
+ private boolean notProcessedBefore(@NotNull Group group) {
+ try {
+ return processed.add(group.getID()) && !EveryonePrincipal.NAME.equals(group.getPrincipal().getName());
+ } catch (RepositoryException repositoryException) {
+ return true;
+ }
+ }
+
+ @NotNull
+ private Iterator<Group> getNextInheritedIterator() {
+ if (inherited.isEmpty()) {
+ // no more inherited groups to retrieve
+ return Collections.emptyIterator();
+ } else {
+ // no need to verify if the inherited iterator has any elements as this has been asserted before
+ // adding it to the list.
+ return inherited.remove(0);
+ }
+ }
+}
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicGroupsTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicGroupsTest.java
index 2e260bc2ca..ba6caa8b3f 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicGroupsTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicGroupsTest.java
@@ -16,13 +16,16 @@
*/
package org.apache.jackrabbit.oak.spi.security.authentication.external.impl;
+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.ItemBasedPrincipal;
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.spi.security.authentication.external.ExternalGroup;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentity;
@@ -32,6 +35,7 @@ import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalUs
import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncResult;
import org.apache.jackrabbit.oak.spi.security.authentication.external.TestIdentityProvider;
import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncConfig;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.jetbrains.annotations.NotNull;
import org.junit.Test;
@@ -42,6 +46,8 @@ import javax.jcr.RepositoryException;
import java.security.Principal;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
@@ -284,4 +290,77 @@ public class DynamicGroupsTest extends DynamicSyncContextTest {
assertEquals(ref.getId(), group.getID());
}
}
+
+ @Test
+ public void testCrossIDPMembership() throws Exception {
+ UserManager um = getUserManager(r);
+ PrincipalManager pm = getPrincipalManager(r);
+
+ List<ExternalIdentityRef> declaredGroupRefs = ImmutableList.copyOf(previouslySyncedUser.getDeclaredGroups());
+ assertTrue(declaredGroupRefs.size() > 1);
+
+ String groupId = declaredGroupRefs.get(0).getId();
+ String groupId2 = declaredGroupRefs.get(1).getId();
+ Group local = um.createGroup("localGroup");
+ local.addMembers(groupId, groupId2);
+ userManager.createGroup(EveryonePrincipal.getInstance());
+ r.commit();
+
+ Authorizable a = getUserManager(r).getAuthorizable(PREVIOUS_SYNCED_ID);
+ assertFalse(Iterators.contains(a.memberOf(), local));
+
+ // sync again to establish dynamic membership
+ syncContext.setForceUserSync(true);
+ syncContext.setForceGroupSync(true);
+ syncContext.sync(idp.getUser(PREVIOUS_SYNCED_ID));
+
+ a = um.getAuthorizable(PREVIOUS_SYNCED_ID);
+ assertTrue(r.getTree(a.getPath()).hasProperty(REP_EXTERNAL_PRINCIPAL_NAMES));
+
+ // verify membership
+ List<String> groupIds = toIds(a.memberOf());
+ if (membershipNestingDepth == 0) {
+ assertFalse(groupIds.contains("localGroup"));
+ assertFalse(local.isMember(a));
+ } else {
+ assertEquals((membershipNestingDepth > 1) ? 5 : 4, groupIds.size());
+ assertTrue(groupIds.contains("localGroup"));
+ assertTrue(local.isMember(a));
+
+ for (String id : new String[] {groupId, groupId2}) {
+ Authorizable extGroup = um.getAuthorizable(id);
+ assertTrue(toIds(extGroup.declaredMemberOf()).contains("localGroup"));
+ assertTrue(local.isMember(extGroup));
+ }
+ }
+
+ // verify effective principals of external user
+ List<String> principalNames = getPrincipalNames(pm.getGroupMembership(a.getPrincipal()));
+ assertEquals(membershipNestingDepth != 0, principalNames.contains(local.getPrincipal().getName()));
+ for (ExternalIdentityRef ref : declaredGroupRefs) {
+ ExternalIdentity eg = idp.getIdentity(ref);
+ assertEquals(membershipNestingDepth != 0, principalNames.contains(eg.getPrincipalName()));
+ }
+
+ // verify effective principals of dynamic group
+ Authorizable dynamicGroup = userManager.getAuthorizable(groupId);
+ if (dynamicGroup != null) {
+ principalNames = getPrincipalNames(pm.getGroupMembership(dynamicGroup.getPrincipal()));
+ assertTrue(principalNames.contains(local.getPrincipal().getName()));
+ }
+ }
+
+ private @NotNull List<String> toIds(@NotNull Iterator<Group> groups) {
+ return ImmutableList.copyOf(Iterators.transform(groups, group -> {
+ try {
+ return group.getID();
+ } catch (RepositoryException repositoryException) {
+ throw new RuntimeException();
+ }
+ }));
+ }
+
+ private @NotNull List<String> getPrincipalNames(@NotNull Iterator<Principal> groupPrincipals) {
+ return ImmutableList.copyOf(Iterators.transform(groupPrincipals, Principal::getName));
+ }
}
\ No newline at end of file
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
index 9eb2ce1562..9a5920af97 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContextTest.java
@@ -37,7 +37,6 @@ import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalId
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityProvider;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalUser;
-import org.apache.jackrabbit.oak.spi.security.authentication.external.PrincipalNameResolver;
import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncException;
import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncResult;
import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncedIdentity;
@@ -148,7 +147,8 @@ public class DynamicSyncContextTest extends AbstractExternalAuthTest {
tidp.addGroup(new TestIdentityProvider.TestGroup("ttt", idpName));
tidp.addGroup(new TestIdentityProvider.TestGroup("tt", idpName).withGroups("ttt"));
tidp.addGroup(new TestIdentityProvider.TestGroup("thirdGroup", idpName).withGroups("tt"));
- tidp.addUser(new TestIdentityProvider.TestUser(PREVIOUS_SYNCED_ID, idpName).withGroups("thirdGroup"));
+ tidp.addGroup(new TestIdentityProvider.TestGroup("forthGroup", idpName));
+ tidp.addUser(new TestIdentityProvider.TestUser(PREVIOUS_SYNCED_ID, idpName).withGroups("thirdGroup", "forthGroup"));
UserManager um = getUserManager(r);
DefaultSyncContext ctx = new DefaultSyncContext(priorSyncConfig, idp, um, getValueFactory(r));
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 5939d526d8..fe9ec35617 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
@@ -415,10 +415,10 @@ public class AutoMembershipProviderTest extends AbstractAutoMembershipTest {
assertTrue(groups.contains(automembershipGroup2));
groups = ImmutableSet.copyOf(provider.getMembership(getTestUser(), true));
- assertEquals(3, groups.size()); // all duplicates must be properly filtered
+ assertEquals(2, groups.size()); // all duplicates must be properly filtered and everyone must be omitted
assertTrue(groups.contains(automembershipGroup1));
assertTrue(groups.contains(automembershipGroup2));
- assertTrue(groups.contains(everyone));
+ assertFalse(groups.contains(everyone));
}
@Test
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/DynamicGroupUtilTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/DynamicGroupUtilTest.java
index acb07adbc0..dd99614c06 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/DynamicGroupUtilTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/DynamicGroupUtilTest.java
@@ -16,25 +16,32 @@
*/
package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal;
+import com.google.common.collect.Iterators;
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.AbstractSecurityTest;
import org.apache.jackrabbit.oak.api.Tree;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
import org.apache.jackrabbit.oak.spi.nodetype.NodeTypeConstants;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.junit.Test;
import javax.jcr.RepositoryException;
import javax.jcr.Value;
+import java.security.Principal;
+
import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants.REP_EXTERNAL_ID;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
@@ -93,4 +100,41 @@ public class DynamicGroupUtilTest extends AbstractSecurityTest {
verify(gr, times(1)).getID();
verifyNoMoreInteractions(gr, member);
}
+
+ @Test
+ public void testGetInheritedPrincipalsMissingGroup() throws Exception {
+ UserManager um = mock(UserManager.class);
+ when(um.getAuthorizable(any(Principal.class))).thenReturn(null);
+ assertTrue(DynamicGroupUtil.getInheritedPrincipals(new PrincipalImpl("test"), um).isEmpty());
+ }
+
+ @Test
+ public void testGetInheritedPrincipalsUserPrincipal() throws Exception {
+ UserManager um = mock(UserManager.class);
+ User user = when(mock(User.class).isGroup()).thenReturn(false).getMock();
+ when(um.getAuthorizable(any(Principal.class))).thenReturn(user);
+ assertTrue(DynamicGroupUtil.getInheritedPrincipals(new PrincipalImpl("test"), um).isEmpty());
+ }
+
+ @Test
+ public void testGetInheritedPrincipalsLookupFails() throws Exception {
+ UserManager um = mock(UserManager.class);
+ when(um.getAuthorizable(any(Principal.class))).thenThrow(new RepositoryException());
+ assertTrue(DynamicGroupUtil.getInheritedPrincipals(new PrincipalImpl("test"), um).isEmpty());
+ }
+
+ @Test
+ public void testGetInheritedPrincipalsGetPrincipalFromGroupFails() throws Exception {
+ Group member = mock(Group.class);
+ when(member.getPrincipal()).thenThrow(new RepositoryException());
+
+ Group group = mock(Group.class);
+ when(group.isGroup()).thenReturn(true);
+ when(group.memberOf()).thenReturn(Iterators.singletonIterator(member));
+
+ UserManager um = mock(UserManager.class);
+ when(um.getAuthorizable(any(Principal.class))).thenReturn(group);
+
+ assertTrue(DynamicGroupUtil.getInheritedPrincipals(new PrincipalImpl("test"), um).isEmpty());
+ }
}
\ No newline at end of file