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:19 UTC
[jackrabbit-oak] branch OAK-10067 created (now 7cc0623cde)
This is an automated email from the ASF dual-hosted git repository.
angela pushed a change to branch OAK-10067
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
at 7cc0623cde OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries
This branch includes the following new commits:
new 7cc0623cde OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[jackrabbit-oak] 01/01: OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries
Posted by an...@apache.org.
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