diff --git a/src/externalldap.js b/src/externalldap.js index b17dd88c2..545387f8f 100644 --- a/src/externalldap.js +++ b/src/externalldap.js @@ -463,27 +463,27 @@ async function syncGroupMembers(config, progressCallback) { return []; } - const allGroups = await groups.list(); + const allGroups = await groups.listWithMembers(); const ldapGroups = allGroups.filter(function (g) { return g.source === 'ldap'; }); debug(`syncGroupMembers: Found ${ldapGroups.length} groups to sync users`); - for (const group of ldapGroups) { - debug(`syncGroupMembers: Sync users for group ${group.name}`); + for (const ldapGroup of ldapGroups) { + debug(`syncGroupMembers: Sync users for group ${ldapGroup.name}`); const result = await ldapGroupSearch(config, {}); if (!result || result.length === 0) { - debug(`syncGroupMembers: Unable to find group ${group.name} ignoring for now.`); + debug(`syncGroupMembers: Unable to find group ${ldapGroup.name} ignoring for now.`); continue; } // since our group names are lowercase we cannot use potentially case matching ldap filters const found = result.find(function (r) { if (!r[config.groupnameField]) return false; - return r[config.groupnameField].toLowerCase() === group.name; + return r[config.groupnameField].toLowerCase() === ldapGroup.name; }); if (!found) { - debug(`syncGroupMembers: Unable to find group ${group.name} ignoring for now.`); + debug(`syncGroupMembers: Unable to find group ${ldapGroup.name} ignoring for now.`); continue; } @@ -492,17 +492,17 @@ async function syncGroupMembers(config, progressCallback) { // if only one entry is in the group ldap returns a string, not an array! if (typeof ldapGroupMembers === 'string') ldapGroupMembers = [ ldapGroupMembers ]; - debug(`syncGroupMembers: Group ${group.name} has ${ldapGroupMembers.length} members.`); + debug(`syncGroupMembers: Group ${ldapGroup.name} has ${ldapGroupMembers.length} members.`); const userIds = []; for (const memberDn of ldapGroupMembers) { const [ldapError, result] = await safe(ldapGetByDN(config, memberDn)); if (ldapError) { - debug(`syncGroupMembers: Group ${group.name} failed to get ${memberDn}: %o`, ldapError); + debug(`syncGroupMembers: Group ${ldapGroup.name} failed to get ${memberDn}: %o`, ldapError); continue; } - debug(`syncGroupMembers: Group ${group.name} has member object ${memberDn}`); + debug(`syncGroupMembers: Group ${ldapGroup.name} has member object ${memberDn}`); const username = result[config.usernameField]?.toLowerCase(); if (!username) continue; @@ -515,8 +515,14 @@ async function syncGroupMembers(config, progressCallback) { userIds.push(userObject.id); } - const [setError] = await safe(groups.setMembers(group, userIds, { skipSourceCheck: true }, AuditSource.EXTERNAL_LDAP)); - if (setError) debug(`syncGroupMembers: Failed to set members of group ${group.name}. %o`, setError); + const membersChanged = ldapGroup.userIds.length === userIds.length && ldapGroup.userIds.every(id => userIds.includes(id)); + if (membersChanged) { + debug(`syncGroupMembers: Group ${ldapGroup.name} changed.`); + const [setError] = await safe(groups.setMembers(ldapGroup, userIds, { skipSourceCheck: true }, AuditSource.EXTERNAL_LDAP)); + if (setError) debug(`syncGroupMembers: Failed to set members of group ${ldapGroup.name}. %o`, setError); + } else { + debug(`syncGroupMembers: Group ${ldapGroup.name} is unchanged.`); + } } debug('syncGroupMembers: done'); diff --git a/src/groups.js b/src/groups.js index f3bbd918b..f0564fc1d 100644 --- a/src/groups.js +++ b/src/groups.js @@ -168,7 +168,7 @@ async function listWithMembers() { ' FROM userGroups LEFT OUTER JOIN groupMembers ON userGroups.id = groupMembers.groupId ' + ' GROUP BY userGroups.id ORDER BY name'); - results.forEach(function (result) { result.userIds = result.userIds ? result.userIds.split(',') : [ ]; }); + results.forEach(function (result) { result.userIds = result.userIds ? result.userIds.split(',') : []; }); for (const r of results) { await postProcess(r);