diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/cfg/LdapGroupSyncConfiguration.java b/server/impl/src/main/java/com/walmartlabs/concord/server/cfg/LdapGroupSyncConfiguration.java index 44aeb9c3d1..47cf324399 100644 --- a/server/impl/src/main/java/com/walmartlabs/concord/server/cfg/LdapGroupSyncConfiguration.java +++ b/server/impl/src/main/java/com/walmartlabs/concord/server/cfg/LdapGroupSyncConfiguration.java @@ -52,6 +52,11 @@ public class LdapGroupSyncConfiguration implements Serializable { @Config("ldapGroupSync.disabledAge") private Duration disabledAge; + @Inject + @Nullable + @Config("ldapGroupSync.concordOrgOwnersGroup") + private String concordOrgOwnersGroup; + public Duration getInterval() { return interval; } @@ -71,4 +76,8 @@ public Duration getMinAgeSync() { public Duration getDisabledAge() { return disabledAge; } + + public String getConcordOrgOwnersGroup() { + return concordOrgOwnersGroup; + } } diff --git a/server/impl/src/main/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizer.java b/server/impl/src/main/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizer.java index f4a23c3172..e7e8cb3a8d 100644 --- a/server/impl/src/main/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizer.java +++ b/server/impl/src/main/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizer.java @@ -26,6 +26,7 @@ import com.walmartlabs.concord.server.audit.ActionSource; import com.walmartlabs.concord.server.audit.AuditLog; import com.walmartlabs.concord.server.cfg.LdapGroupSyncConfiguration; +import com.walmartlabs.concord.server.org.team.TeamRole; import com.walmartlabs.concord.server.sdk.ScheduledTask; import com.walmartlabs.concord.server.user.UserManager; import com.walmartlabs.concord.server.user.UserType; @@ -33,16 +34,21 @@ import org.jooq.Field; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import scala.Tuple2; import javax.inject.Inject; import javax.inject.Named; import java.time.OffsetDateTime; import java.util.Collections; +import java.util.ArrayList; import java.util.List; import java.util.Set; import java.util.UUID; +import java.util.stream.Collectors; + import static com.walmartlabs.concord.server.jooq.Tables.USERS; +import static com.walmartlabs.concord.server.jooq.Tables.USER_TEAMS; import static org.jooq.impl.DSL.*; /** @@ -87,18 +93,26 @@ public void performTask() { Field cutoff = PgUtils.nowMinus(cfg.getMinAgeSync()); Field disabledAge = cfg.getDisabledAge() != null ? PgUtils.nowMinus(cfg.getDisabledAge()) : null; + List concordOrgOwners = new ArrayList<>(); + long usersCount = 0; List users; do { users = dao.list(cfg.getFetchLimit(), cutoff, disabledAge); - users.forEach(this::processUser); + users.forEach(user -> { + processUser(user, concordOrgOwners); + }); usersCount += users.size(); } while (users.size() >= cfg.getFetchLimit()); + if(cfg.getConcordOrgOwnersGroup() != null) { + checkOrgOwnerDiscrepancies(users, dao.listOrgOwners()); + } + log.info("performTask -> done, {} user(s) synchronized", usersCount); } - private void processUser(UserItem u) { + private void processUser(UserItem u, List concordOrgOwners) { try { Set groups = ldapManager.getGroups(u.username, u.domain); if (groups == null) { @@ -111,6 +125,9 @@ private void processUser(UserItem u) { } else { enableUser(u.userId); ldapGroupsDao.update(u.userId, groups); + if(cfg.getConcordOrgOwnersGroup() != null && groups.contains(cfg.getConcordOrgOwnersGroup())) { + concordOrgOwners.add(u); + } } } catch (Exception e) { log.error("processUser ['{}'] -> error", u.username, e); @@ -133,7 +150,7 @@ private void deleteUser(UUID userId) { } @Named - private static final class Dao extends AbstractDao { + static class Dao extends AbstractDao { @Inject public Dao(@MainDB Configuration cfg) { @@ -149,20 +166,62 @@ public List list(int limit, Field cutoff, Field new UserItem(r.value1(), r.value2(), r.value3(), r.value4()))); } + + public List listOrgOwners() { + // Expired users already handled at this stage + return txResult(tx -> tx.select(USERS.USER_ID, USERS.USERNAME, USERS.DOMAIN) + .from(USER_TEAMS.join(USERS).on(USER_TEAMS.USER_ID.eq(USERS.USER_ID))) + .where(USER_TEAMS.TEAM_ROLE.eq(TeamRole.OWNER.toString())) + .fetch(r -> new UserItem(r.value1(), r.value2(), r.value3(), false))); + } } - private static class UserItem { + /*** + * Check concordOrgOwners against all owners in DB. Send discrepancy report on slack or email + * + * @param ldapGroupUsers + * @param ownerRoleUsers + */ + List checkOrgOwnerDiscrepancies(List ldapGroupUsers, List ownerRoleUsers) { + List diffUsersWithOwnerRoleAndNotInLdap = ownerRoleUsers.stream() + .filter(item -> !ldapGroupUsers.contains(item)) + .toList(); + + log.info("checkOrgOwnerDiscrepancies -> done, {} user(s) only registered as owners not in ldapGroup", diffUsersWithOwnerRoleAndNotInLdap.stream().map(Object::toString).collect(Collectors.joining(", "))); + + return diffUsersWithOwnerRoleAndNotInLdap; + } + + static class UserItem implements Comparable { private final UUID userId; private final String username; private final String domain; private final boolean expired; - private UserItem(UUID userId, String username, String domain, boolean expired) { + UserItem(UUID userId, String username, String domain, boolean expired) { this.userId = userId; this.username = username; this.domain = domain; this.expired = expired; } + + @Override + public int compareTo(UserItem o) { + if (userId == o.userId) + return 0; + else + return userId.compareTo(o.userId); + } + + @Override + public String toString() { + return "UserItem{" + + "userId=" + userId + + ", username='" + username + '\'' + + ", domain='" + domain + '\'' + + ", expired=" + expired + + '}'; + } } } diff --git a/server/impl/src/test/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizerTest.java b/server/impl/src/test/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizerTest.java new file mode 100644 index 0000000000..16f051e745 --- /dev/null +++ b/server/impl/src/test/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizerTest.java @@ -0,0 +1,61 @@ +package com.walmartlabs.concord.server.security.ldap; + +import com.walmartlabs.concord.server.cfg.LdapGroupSyncConfiguration; +import com.walmartlabs.concord.server.user.UserManager; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.mock; + +class UserLdapGroupSynchronizerTest { + + private UserLdapGroupSynchronizer userLdapGroupSynchronizer; + + @BeforeEach + public void init() { + userLdapGroupSynchronizer = new UserLdapGroupSynchronizer(mock(UserLdapGroupSynchronizer.Dao.class), + mock(LdapGroupSyncConfiguration.class), + mock(LdapManager.class), + mock(LdapGroupDao.class), + mock(UserManager.class)); + } + + @Test + void checkOrgOwnerDiscrepanciesDiffOwnersNotInLdap() { + UserLdapGroupSynchronizer.UserItem user1 = new UserLdapGroupSynchronizer.UserItem(UUID.randomUUID(), "admin", "domain", false); + UserLdapGroupSynchronizer.UserItem user2 = new UserLdapGroupSynchronizer.UserItem(UUID.randomUUID(), "testUser", null, false); + + List ldapGroupUsers = new ArrayList<>(); + ldapGroupUsers.add(user1); + + List ownerRoleUsers = new ArrayList<>(); + ownerRoleUsers.add(user1); + ownerRoleUsers.add(user2); + + List ownersNotInLdapGroup = new ArrayList<>(); + ownersNotInLdapGroup.add(user2); + + assertTrue(userLdapGroupSynchronizer.checkOrgOwnerDiscrepancies(ldapGroupUsers, ownerRoleUsers).equals(ownersNotInLdapGroup)); + } + + @Test + void checkOrgOwnerDiscrepanciesNoDiff() { + UserLdapGroupSynchronizer.UserItem user1 = new UserLdapGroupSynchronizer.UserItem(UUID.randomUUID(), "admin", "domain", false); + UserLdapGroupSynchronizer.UserItem user2 = new UserLdapGroupSynchronizer.UserItem(UUID.randomUUID(), "testUser", null, false); + + List ldapGroupUsers = new ArrayList<>(); + ldapGroupUsers.add(user1); + ldapGroupUsers.add(user2); + + List ownerRoleUsers = new ArrayList<>(); + ownerRoleUsers.add(user1); + ownerRoleUsers.add(user2); + + assertTrue(userLdapGroupSynchronizer.checkOrgOwnerDiscrepancies(ldapGroupUsers, ownerRoleUsers).isEmpty()); + } +} \ No newline at end of file