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 edf4a4493c OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries (#825) edf4a4493c is described below commit edf4a4493c18784c83d3e50d26739b458c374a11 Author: anchela <ang...@apache.org> AuthorDate: Tue Jan 17 16:11:26 2023 +0100 OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries (#825) * OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries * OAK-10067 : ExternalGroupPrincipalProvider does not resolve inherited groups that cross IDP boundaries (missing license header) --- .../impl/principal/AutoMembershipProvider.java | 62 +------------ .../external/impl/principal/DynamicGroupUtil.java | 31 +++++++ .../principal/ExternalGroupPrincipalProvider.java | 49 +++++++++- .../principal/InheritedMembershipIterator.java | 103 +++++++++++++++++++++ .../external/impl/DynamicGroupsTest.java | 79 ++++++++++++++++ .../external/impl/DynamicSyncContextTest.java | 4 +- .../impl/principal/AutoMembershipProviderTest.java | 4 +- .../impl/principal/DynamicGroupUtilTest.java | 44 +++++++++ 8 files changed, 307 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..8b19f07532 --- /dev/null +++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/InheritedMembershipIterator.java @@ -0,0 +1,103 @@ +/* + * 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.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