Skip to content
Permalink
Browse files
Merge pull request #590 from mbaedke/trunk
OAK-9773: DefaultSyncContext#syncMembership() compares external ids c…
  • Loading branch information
mbaedke committed Jun 22, 2022
2 parents bb33083 + 47f57b6 commit 463402cfd4f89f8586cdde53b73149897069d2ec
Showing 3 changed files with 36 additions and 8 deletions.
@@ -518,11 +518,13 @@ protected void syncMembership(@NotNull ExternalIdentity external, @NotNull Autho

// first get the set of the existing groups that are synced ones
Map<String, Group> declaredExternalGroups = new HashMap<>();
List<String> declaredExternalGroupIds = new ArrayList<>();
Iterator<Group> grpIter = auth.declaredMemberOf();
while (grpIter.hasNext()) {
Group grp = grpIter.next();
if (isSameIDP(grp)) {
declaredExternalGroups.put(grp.getID(), grp);
declaredExternalGroupIds.add(grp.getID());
declaredExternalGroups.put(grp.getID().toLowerCase(), grp);
}
}
timer.mark("reading");
@@ -546,29 +548,34 @@ protected void syncMembership(@NotNull ExternalIdentity external, @NotNull Autho
log.debug("- idp returned '{}'", extGroup.getId());

// mark group as processed
Group grp = declaredExternalGroups.remove(extGroup.getId());
boolean idMatches = declaredExternalGroupIds.contains(extGroup.getId());
Group grp = declaredExternalGroups.remove(extGroup.getId().toLowerCase());
boolean exists = grp != null;

if (exists && !idMatches) {
log.warn("The existing authorizable {} and the external group {} have identifiers that only differ by case. Since the identifiers are compared case-insensitively, the existing authorizable will be considered to match the external group.");
}

if (!exists) {
Authorizable a = userManager.getAuthorizable(extGroup.getId());
if (a == null) {
grp = createGroup(extGroup);
log.debug("- created new group");
log.debug("- created new group '{}'", grp.getID());
} else if (a.isGroup() && isSameIDP(a)) {
grp = (Group) a;
} else {
log.warn("Existing authorizable '{}' is not a group from this IDP '{}'.", extGroup.getId(), idp.getName());
continue;
}
log.debug("- user manager returned '{}'", grp);
log.debug("- user manager returned '{}'", grp.getID());
}

syncGroup(extGroup, grp);

if (!exists) {
// ensure membership
grp.addMember(auth);
log.debug("- added '{}' as member to '{}'", auth, grp);
log.debug("- added '{}' as member to '{}'", auth, grp.getID());
}

// recursively apply further membership
@@ -90,7 +90,7 @@ public void addUser(TestIdentity user) {
externalUsers.put(user.getId().toLowerCase(), (ExternalUser) user);
}

private void addGroup(TestIdentity group) {
public void addGroup(TestIdentity group) {
externalGroups.put(group.getId().toLowerCase(), (ExternalGroup) group);
}

@@ -221,13 +221,13 @@ public Iterable<ExternalIdentityRef> getDeclaredGroups() throws ExternalIdentity
}

@NotNull
protected TestIdentity withProperty(@NotNull String name, @NotNull Object value) {
public TestIdentity withProperty(@NotNull String name, @NotNull Object value) {
props.put(name, value);
return this;
}

@NotNull
protected TestIdentity withGroups(@NotNull String ... grps) {
public TestIdentity withGroups(@NotNull String ... grps) {
for (String grp: grps) {
groups.add(new ExternalIdentityRef(grp, id.getProviderName()));
}
@@ -20,6 +20,8 @@
import com.google.common.collect.Iterators;
import com.google.common.collect.Sets;
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.Root;
import org.apache.jackrabbit.oak.spi.security.authentication.external.ExternalIdentity;
@@ -29,6 +31,7 @@
import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncHandler;
import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncResult;
import org.apache.jackrabbit.oak.spi.security.authentication.external.SyncedIdentity;
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.authentication.external.basic.DefaultSyncedIdentity;
@@ -328,6 +331,24 @@ public void testLastSynced() throws Exception {
root.commit();
}

//OAK-9773
@Test
public void testSyncMembershipCaseMismatch() throws Exception {
UserManager userManager = getUserManager(root);
User user = userManager.createUser("thirduser", "thirduser");
user.setProperty(REP_EXTERNAL_ID, getValueFactory().createValue("thirduser;test"));
Group group = userManager.createGroup("thirdgroup");
group.setProperty(REP_EXTERNAL_ID, getValueFactory().createValue("thirdgroup;test"));
assertTrue(group.addMember(user));
root.commit();
((TestIdentityProvider) idp).addGroup(new TestIdentityProvider.TestGroup("THIRDGROUP", "test"));
((TestIdentityProvider) idp).addUser(
new TestIdentityProvider.TestUser("THIRDUSER", "test").withGroups("THIRDGROUP"));

syncHandler.createContext(idp, userManager, getValueFactory()).sync(user.getID());
assertTrue(group.isMember(user));
}

@Test
public void testActivate() {
DefaultSyncHandler handler = new DefaultSyncHandler();

0 comments on commit 463402c

Please sign in to comment.