Skip to content

Sync org admins #849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dankle wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Sync org admins #849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e3b2b64
adding discrepancies logic for concord org owners.
dankle Dec 13, 2023
ca58365
Merge branch 'master' of https://github.com/walmartlabs/concord into …
dankle Dec 13, 2023
07fa28f
cleanup
dankle Dec 14, 2023
0bead68
adding test
dankle Dec 14, 2023
d233a49
Refactoring
dankle Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions server/impl/src/main/java/com/walmartlabs/concord/server/cfg/LdapGroupSyncConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -71,4 +76,8 @@ public Duration getMinAgeSync() {
public Duration getDisabledAge() {
return disabledAge;
}

public String getConcordOrgOwnersGroup() {
return concordOrgOwnersGroup;
}
}
69 changes: 64 additions & 5 deletions ...src/main/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,29 @@
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;
import org.jooq.Configuration;
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.*;

/**
Expand Down Expand Up @@ -87,18 +93,26 @@ public void performTask() {
Field<OffsetDateTime> cutoff = PgUtils.nowMinus(cfg.getMinAgeSync());
Field<OffsetDateTime> disabledAge = cfg.getDisabledAge() != null ? PgUtils.nowMinus(cfg.getDisabledAge()) : null;

List<UserItem> concordOrgOwners = new ArrayList<>();

long usersCount = 0;
List<UserItem> 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<UserItem> concordOrgOwners) {
try {
Set<String> groups = ldapManager.getGroups(u.username, u.domain);
if (groups == null) {
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -149,20 +166,62 @@ public List<UserItem> list(int limit, Field<OffsetDateTime> cutoff, Field<Offset
.limit(limit)
.fetch(r -> new UserItem(r.value1(), r.value2(), r.value3(), r.value4())));
}

public List<UserItem> 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<UserItem> checkOrgOwnerDiscrepancies(List<UserItem> ldapGroupUsers, List<UserItem> ownerRoleUsers) {
List<UserItem> 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<UserItem> {

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 +
'}';
}
}
}
61 changes: 61 additions & 0 deletions ...test/java/com/walmartlabs/concord/server/security/ldap/UserLdapGroupSynchronizerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<UserLdapGroupSynchronizer.UserItem> ldapGroupUsers = new ArrayList<>();
ldapGroupUsers.add(user1);

List<UserLdapGroupSynchronizer.UserItem> ownerRoleUsers = new ArrayList<>();
ownerRoleUsers.add(user1);
ownerRoleUsers.add(user2);

List<UserLdapGroupSynchronizer.UserItem> 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<UserLdapGroupSynchronizer.UserItem> ldapGroupUsers = new ArrayList<>();
ldapGroupUsers.add(user1);
ldapGroupUsers.add(user2);

List<UserLdapGroupSynchronizer.UserItem> ownerRoleUsers = new ArrayList<>();
ownerRoleUsers.add(user1);
ownerRoleUsers.add(user2);

assertTrue(userLdapGroupSynchronizer.checkOrgOwnerDiscrepancies(ldapGroupUsers, ownerRoleUsers).isEmpty());
}
}