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/09/23 08:19:58 UTC
[jackrabbit-oak] branch trunk updated: OAK-9954 : Dynamic membership/group should spot conflicts with existing groups (#717)
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 400bb948da OAK-9954 : Dynamic membership/group should spot conflicts with existing groups (#717)
400bb948da is described below
commit 400bb948da78f316dbba0e9d96823cdf1285dc58
Author: anchela <an...@apache.org>
AuthorDate: Fri Sep 23 10:19:53 2022 +0200
OAK-9954 : Dynamic membership/group should spot conflicts with existing groups (#717)
---
.../external/impl/DynamicSyncContext.java | 44 +++++++++++++++----
.../impl/principal/AutoMembershipProvider.java | 9 +---
.../external/impl/principal/DynamicGroupUtil.java | 43 +++++++++++++++++++
.../principal/ExternalGroupPrincipalProvider.java | 39 +++++++++--------
.../external/impl/DynamicSyncContextTest.java | 50 ++++++++++++++++++++++
.../impl/principal/DynamicGroupUtilTest.java | 33 ++++++++++++++
.../ExternalGroupPrincipalProviderDMTest.java | 15 +++++++
7 files changed, 200 insertions(+), 33 deletions(-)
diff --git a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java
index c5837fb0a9..1e4ce26ebc 100644
--- a/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java
+++ b/oak-auth-external/src/main/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/DynamicSyncContext.java
@@ -33,6 +33,7 @@ import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.Defa
import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncContext;
import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncResultImpl;
import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncedIdentity;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -204,27 +205,54 @@ public class DynamicSyncContext extends DefaultSyncContext {
* @param depth Configured membership nesting; the recursion will be stopped once depths is < 1.
* @throws ExternalIdentityException If an error occurs while resolving the the external group references.
*/
- private void collectPrincipalNames(@NotNull Set<String> principalNames, @NotNull Iterable<ExternalIdentityRef> declaredGroupIdRefs, long depth) throws ExternalIdentityException {
+ private void collectPrincipalNames(@NotNull Set<String> principalNames, @NotNull Iterable<ExternalIdentityRef> declaredGroupIdRefs, long depth) throws ExternalIdentityException, RepositoryException {
boolean shortcut = (depth <= 1 && idp instanceof PrincipalNameResolver);
for (ExternalIdentityRef ref : Iterables.filter(declaredGroupIdRefs, this::isSameIDP)) {
if (shortcut) {
- principalNames.add(((PrincipalNameResolver) idp).fromExternalIdentityRef(ref));
+ addPrincipalName(((PrincipalNameResolver) idp).fromExternalIdentityRef(ref), principalNames);
} else {
// get group from the IDP
- ExternalIdentity extId = idp.getIdentity(ref);
- if (extId instanceof ExternalGroup) {
- principalNames.add(extId.getPrincipalName());
+ ExternalIdentity extId = getExternalGroupFromRef(ref);
+ if (extId != null) {
+ addPrincipalName(extId.getPrincipalName(), principalNames);
// recursively apply further membership until the configured depth is reached
if (depth > 1) {
collectPrincipalNames(principalNames, extId.getDeclaredGroups(), depth - 1);
}
- } else {
- log.debug("Not an external group ({}) => ignore.", extId);
}
}
}
}
+ private void addPrincipalName(@NotNull String principalName, @NotNull Set<String> principalNames) throws RepositoryException {
+ if (existsConflicting(principalName)) {
+ // there exists a user or group with that principal name but it doesn't belong to the same IDP
+ // in consistency with DefaultSyncContext don't sync this very membership into the repository
+ // and log a warning about the collision instead.
+ log.warn("Existing authorizable with principal name '{}' is not a group from this IDP '{}'.", principalName, idp.getName());
+ return;
+ }
+ // no conflict detected
+ principalNames.add(principalName);
+ }
+
+ /**
+ * Tests if there exists an user/group in the repository that has the same principal name but doesn't belong to the same IDP.
+ *
+ * @param externalGroupPrincipalName A principal name
+ * @return {@code true} if there exists an user/group in the repository with the given principal name that doesn't
+ * belong to this IDP; {@code false} otherwise.
+ * @throws RepositoryException If an error occurs
+ */
+ private boolean existsConflicting(@NotNull String externalGroupPrincipalName) throws RepositoryException {
+ Authorizable a = userManager.getAuthorizable(new PrincipalImpl(externalGroupPrincipalName));
+ if (a == null) {
+ return false;
+ } else {
+ return !isSameIDP(a);
+ }
+ }
+
private void createDynamicGroups(@NotNull Iterable<ExternalIdentityRef> declaredGroupIdRefs,
long depth) throws RepositoryException, ExternalIdentityException {
for (ExternalIdentityRef groupRef : declaredGroupIdRefs) {
@@ -278,7 +306,7 @@ public class DynamicSyncContext extends DefaultSyncContext {
clearGroupMembership(grp, groupPrincipalNames, toRemove);
} else {
// some other membership that has not been added by the sync process
- log.debug("TODO");
+ log.warn("Ignoring unexpected membership of '{}' in group '{}' crossing IDP boundary.", authorizable.getID(), grp.getID());
}
}
}
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 ea7b5ba971..d65c9e86fe 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
@@ -30,8 +30,6 @@ import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.namepath.NamePathMapper;
import org.apache.jackrabbit.oak.plugins.memory.PropertyValues;
import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.AutoMembershipConfig;
-import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
-import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncContext;
import org.apache.jackrabbit.oak.spi.security.user.DynamicMembershipProvider;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -56,6 +54,7 @@ import java.util.function.Function;
import java.util.stream.StreamSupport;
import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants.REP_EXTERNAL_ID;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal.DynamicGroupUtil.getIdpName;
import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.NT_REP_AUTHORIZABLE;
import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.NT_REP_GROUP;
import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.NT_REP_USER;
@@ -232,12 +231,6 @@ class AutoMembershipProvider implements DynamicMembershipProvider {
return null;
}
}
-
- @Nullable
- private static String getIdpName(@NotNull Authorizable authorizable) throws RepositoryException {
- ExternalIdentityRef ref = DefaultSyncContext.getIdentityRef(authorizable);
- return (ref == null) ? null : ref.getProviderName();
- }
@NotNull
private static Map<String, ? extends PropertyValue> buildBinding(@NotNull String idpName) {
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 8612c64a67..664611435b 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
@@ -20,10 +20,14 @@ 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.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.api.ResultRow;
import org.apache.jackrabbit.oak.api.Root;
import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
import org.apache.jackrabbit.oak.plugins.tree.TreeAware;
import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncContext;
import org.apache.jackrabbit.oak.spi.security.user.AuthorizableType;
import org.apache.jackrabbit.oak.spi.security.user.UserConstants;
import org.apache.jackrabbit.oak.spi.security.user.util.UserUtil;
@@ -35,6 +39,8 @@ import org.slf4j.LoggerFactory;
import javax.jcr.RepositoryException;
import java.util.Set;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants.REP_EXTERNAL_ID;
+
class DynamicGroupUtil {
private static final Logger log = LoggerFactory.getLogger(DynamicGroupUtil.class);
@@ -83,4 +89,41 @@ class DynamicGroupUtil {
String primaryType = TreeUtil.getPrimaryTypeName(tree);
return primaryType != null && MEMBERS_TYPES.contains(primaryType);
}
+
+ @Nullable
+ static String getIdpName(@NotNull Tree userTree) {
+ PropertyState ps = userTree.getProperty(REP_EXTERNAL_ID);
+ if (ps != null) {
+ return ExternalIdentityRef.fromString(ps.getValue(Type.STRING)).getProviderName();
+ } else {
+ return null;
+ }
+ }
+
+ @Nullable
+ static String getIdpName(@NotNull ResultRow row) {
+ return getIdpName(row.getTree(null));
+ }
+
+ @Nullable
+ static String getIdpName(@NotNull Authorizable authorizable) throws RepositoryException {
+ ExternalIdentityRef ref = DefaultSyncContext.getIdentityRef(authorizable);
+ return (ref == null) ? null : ref.getProviderName();
+ }
+
+ static boolean isSameIDP(@NotNull Authorizable group, @NotNull Authorizable member) throws RepositoryException {
+ String groupIdpName = getIdpName(group);
+ if (groupIdpName == null) {
+ log.warn("Referenced dynamic group '{}' not associated with an external IDP.", group.getID());
+ return false;
+ }
+
+ String idpName = getIdpName(member);
+ if (groupIdpName.equals(idpName)) {
+ return true;
+ } else {
+ log.warn("IDP mismatch between dynamic group '{}' and member '{}'.", groupIdpName, idpName);
+ return false;
+ }
+ }
}
\ 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 f0ac9b88cb..c3e94001a8 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
@@ -73,6 +73,9 @@ import java.util.Spliterators;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal.DynamicGroupUtil.getIdpName;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal.DynamicGroupUtil.isSameIDP;
+
/**
* Implementation of the {@code PrincipalProvider} interface that exposes
* 'external' principals of type {@link org.apache.jackrabbit.api.security.principal.GroupPrincipal}. 'External'
@@ -226,7 +229,7 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
@NotNull
@Override
public Iterator<? extends Principal> findPrincipals(@Nullable String nameHint, boolean fullText, int searchType,
- long offset, long limit) {
+ long offset, long limit) {
Iterator<? extends Principal> principals = findPrincipals(nameHint, searchType);
if (!principals.hasNext()) {
return Collections.emptyIterator();
@@ -295,7 +298,7 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
try {
String groupPrincipalName = value.getString();
Authorizable gr = userManager.getAuthorizable(new PrincipalImpl(groupPrincipalName));
- return (gr != null && gr.isGroup()) ? (Group) gr : null;
+ return isValidGroup(gr, authorizable) ? (Group) gr : null;
} catch (RepositoryException e) {
return null;
}
@@ -303,6 +306,23 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
}
}
+ /**
+ * Verify that the dynamic external group belongs to the same IDP as the member for which groups are being retrieved.
+ * Note that {@code DynamicSyncContext#syncMembership} also asserts there are no collisions between external principal names.
+ *
+ * @param group The authorizable matching an entry in the {@code REP_EXTERNAL_PRINCIPAL_NAMES} property of the
+ * target member.
+ * @param member The target member
+ * @return {@code true} if the given authorizable is a valid external group that belongs to the same IDP; {@code false} otherwise.
+ * @throws RepositoryException If an error occors while retrieving the IDP name
+ */
+ private static boolean isValidGroup(@Nullable Authorizable group, @NotNull Authorizable member) throws RepositoryException {
+ if (group == null || !group.isGroup()) {
+ return false;
+ }
+ return isSameIDP(group, member);
+ }
+
/**
* Returns true if the given user/group belongs to an IDP that has dynamic-group configuration option enabled.
*/
@@ -324,20 +344,6 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
}
//------------------------------------------------------------< private >---
- @Nullable
- private static String getIdpName(@NotNull Tree userTree) {
- PropertyState ps = userTree.getProperty(REP_EXTERNAL_ID);
- if (ps != null) {
- return ExternalIdentityRef.fromString(ps.getValue(Type.STRING)).getProviderName();
- } else {
- return null;
- }
- }
-
- @Nullable
- private static String getIdpName(@NotNull ResultRow row) {
- return getIdpName(row.getTree(null));
- }
@NotNull
private Set<Principal> getGroupPrincipals(@Nullable Authorizable authorizable, boolean ignoreGroup) throws RepositoryException {
@@ -390,7 +396,6 @@ class ExternalGroupPrincipalProvider implements PrincipalProvider, ExternalIdent
}
}
-
/**
* Runs an Oak query searching for {@link #REP_EXTERNAL_PRINCIPAL_NAMES} properties
* that match the given name or name hint.
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 864c5b1f81..0f2552e830 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
@@ -43,6 +43,7 @@ import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncedIden
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.authentication.external.basic.DefaultSyncContext;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
import org.jetbrains.annotations.NotNull;
import org.junit.After;
import org.junit.Before;
@@ -60,6 +61,7 @@ import java.util.stream.Collectors;
import static org.apache.jackrabbit.JcrConstants.JCR_UUID;
import static org.apache.jackrabbit.oak.spi.security.authentication.external.TestIdentityProvider.ID_SECOND_USER;
import static org.apache.jackrabbit.oak.spi.security.authentication.external.TestIdentityProvider.ID_TEST_USER;
+import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants.REP_EXTERNAL_ID;
import static org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants.REP_EXTERNAL_PRINCIPAL_NAMES;
import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.REP_MEMBERS;
import static org.apache.jackrabbit.oak.spi.security.user.UserConstants.REP_MEMBERS_LIST;
@@ -328,6 +330,54 @@ public class DynamicSyncContextTest extends AbstractExternalAuthTest {
assertEquals(expected, pNames);
}
+ @Test
+ public void testSyncExternalUserGroupConflict() throws Exception {
+ ExternalUser externalUser = idp.getUser(USER_ID);
+
+ // create a local group that collides with the external group membership
+ // i.e. doesn't have an rep:externalId set
+ ExternalIdentity externalGroup = idp.getIdentity(externalUser.getDeclaredGroups().iterator().next());
+ userManager.createGroup(externalGroup.getId(), new PrincipalImpl(externalGroup.getPrincipalName()), null);
+ r.commit();
+
+ // sync the user with dynamic membership enabled
+ sync(externalUser, SyncResult.Status.ADD);
+
+ // retrieve rep:externalPrincipalNames
+ Tree tree = r.getTree(userManager.getAuthorizable(USER_ID).getPath());
+ PropertyState extPrincipalNames = tree.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
+ assertNotNull(extPrincipalNames);
+
+ // the resulting rep:externalPrincipalNames must NOT contain the name of the colliding principal
+ Set<String> pNames = Sets.newHashSet(extPrincipalNames.getValue(Type.STRINGS));
+ assertFalse(pNames + " must not contain " + externalGroup.getPrincipalName(), pNames.contains(externalGroup.getPrincipalName()));
+ }
+
+ @Test
+ public void testSyncExternalUserGroupConflict2() throws Exception {
+ ExternalUser externalUser = idp.getUser(USER_ID);
+
+ // create a local group that collides with the external group membership
+ // i.e. belongs to a different IDP
+ ExternalIdentityRef ref = externalUser.getDeclaredGroups().iterator().next();
+ ExternalIdentity externalGroup = idp.getIdentity(ref);
+ Group g = userManager.createGroup(externalGroup.getId(), new PrincipalImpl(externalGroup.getPrincipalName()), null);
+ g.setProperty(REP_EXTERNAL_ID, getValueFactory().createValue(new ExternalIdentityRef(ref.getId(), ref.getProviderName()+"_mod").getString()));
+ r.commit();
+
+ // sync the user with dynamic membership enabled
+ sync(externalUser, SyncResult.Status.ADD);
+
+ // retrieve rep:externalPrincipalNames
+ Tree tree = r.getTree(userManager.getAuthorizable(USER_ID).getPath());
+ PropertyState extPrincipalNames = tree.getProperty(REP_EXTERNAL_PRINCIPAL_NAMES);
+ assertNotNull(extPrincipalNames);
+
+ // the resulting rep:externalPrincipalNames must NOT contain the name of the colliding principal
+ Set<String> pNames = Sets.newHashSet(extPrincipalNames.getValue(Type.STRINGS));
+ assertFalse(pNames + " must not contain " + externalGroup.getPrincipalName(), pNames.contains(externalGroup.getPrincipalName()));
+ }
+
@Test
public void testSyncExternalUserExistingGroups() throws Exception {
// verify group membership of the previously synced user
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 f8c433a216..acb07adbc0 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,21 +16,29 @@
*/
package org.apache.jackrabbit.oak.spi.security.authentication.external.impl.principal;
+import org.apache.jackrabbit.api.security.user.Authorizable;
import org.apache.jackrabbit.api.security.user.Group;
import org.apache.jackrabbit.oak.AbstractSecurityTest;
import org.apache.jackrabbit.oak.api.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.user.UserConstants;
import org.junit.Test;
import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+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.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;
public class DynamicGroupUtilTest extends AbstractSecurityTest {
@@ -60,4 +68,29 @@ public class DynamicGroupUtilTest extends AbstractSecurityTest {
Group gr = when(mock(Group.class).getPath()).thenThrow(new RepositoryException()).getMock();
assertFalse(DynamicGroupUtil.hasStoredMemberInfo(gr, root));
}
+
+ @Test
+ public void testIsSameIDP() throws Exception {
+ Group gr = mock(Group.class);
+ Authorizable member = mock(Authorizable.class);
+
+ assertFalse(DynamicGroupUtil.isSameIDP(gr, member));
+
+ Value v = getValueFactory().createValue(new ExternalIdentityRef("id", "idp").getString());
+ Value v2 = getValueFactory().createValue(new ExternalIdentityRef("id", "otherIdp").getString());
+
+ when(gr.getProperty(REP_EXTERNAL_ID)).thenReturn(new Value[] {v});
+ assertFalse(DynamicGroupUtil.isSameIDP(gr, member));
+
+ when(member.getProperty(REP_EXTERNAL_ID)).thenReturn(new Value[] {v2});
+ assertFalse(DynamicGroupUtil.isSameIDP(gr, member));
+
+ when(member.getProperty(REP_EXTERNAL_ID)).thenReturn(new Value[] {v});
+ assertTrue(DynamicGroupUtil.isSameIDP(gr, member));
+
+ verify(gr, times(4)).getProperty(REP_EXTERNAL_ID);
+ verify(member, times(3)).getProperty(REP_EXTERNAL_ID);
+ verify(gr, times(1)).getID();
+ verifyNoMoreInteractions(gr, member);
+ }
}
\ No newline at end of file
diff --git a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProviderDMTest.java b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProviderDMTest.java
index ad19ccd313..20a508383b 100644
--- a/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProviderDMTest.java
+++ b/oak-auth-external/src/test/java/org/apache/jackrabbit/oak/spi/security/authentication/external/impl/principal/ExternalGroupPrincipalProviderDMTest.java
@@ -31,6 +31,7 @@ import org.apache.jackrabbit.oak.plugins.tree.TreeUtil;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityException;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentityRef;
import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncConfig;
+import org.apache.jackrabbit.oak.spi.security.authentication.external.basic.DefaultSyncContext;
import org.apache.jackrabbit.oak.spi.security.authentication.external.impl.DefaultSyncConfigImpl;
import org.apache.jackrabbit.oak.spi.security.authentication.external.impl.ExternalIdentityConstants;
import org.apache.jackrabbit.oak.spi.security.user.UserConfiguration;
@@ -355,6 +356,20 @@ public class ExternalGroupPrincipalProviderDMTest extends AbstractPrincipalTest
assertFalse(groups.hasNext());
}
+ @Test
+ public void testGetMembershipIdpMismatch() throws Exception {
+ User user = getUserManager(root).getAuthorizable(USER_ID, User.class);
+ assertNotNull(user);
+
+ // alter the rep:externalId property of the synced user to point to a different IDP
+ ExternalIdentityRef ref = DefaultSyncContext.getIdentityRef(user);
+ ExternalIdentityRef newRef = new ExternalIdentityRef(ref.getId(), "different");
+ user.setProperty(REP_EXTERNAL_PRINCIPAL_NAMES, getValueFactory().createValue(newRef.getString()));
+
+ Iterator<Group> groups = principalProvider.getMembership(user, true);
+ assertFalse(groups.hasNext());
+ }
+
private long getExpectedNumberOfGroups() throws Exception {
return getExpectedSyncedGroupIds(syncConfig.user().getMembershipNestingDepth(), idp, idp.getUser(USER_ID)).size();
}