diff --git a/src/externalldap.js b/src/externalldap.js index 10e6c95a4..f19356cc4 100644 --- a/src/externalldap.js +++ b/src/externalldap.js @@ -424,26 +424,26 @@ async function syncGroups(config, progressCallback) { debug('syncGroups: sync done'); } -async function syncGroupUsers(config, progressCallback) { +async function syncGroupMembers(config, progressCallback) { assert.strictEqual(typeof config, 'object'); assert.strictEqual(typeof progressCallback, 'function'); if (!config.syncGroups) { - debug('syncGroupUsers: Group users sync is disabled'); + debug('syncGroupMembers: Group users sync is disabled'); progressCallback({ percent: 99, message: 'Skipping group users sync...' }); return []; } const allGroups = await groups.list(); const ldapGroups = allGroups.filter(function (g) { return g.source === 'ldap'; }); - debug(`syncGroupUsers: Found ${ldapGroups.length} groups to sync users`); + debug(`syncGroupMembers: Found ${ldapGroups.length} groups to sync users`); for (const group of ldapGroups) { - debug(`syncGroupUsers: Sync users for group ${group.name}`); + debug(`syncGroupMembers: Sync users for group ${group.name}`); const result = await ldapGroupSearch(config, {}); if (!result || result.length === 0) { - debug(`syncGroupUsers: Unable to find group ${group.name} ignoring for now.`); + debug(`syncGroupMembers: Unable to find group ${group.name} ignoring for now.`); continue; } @@ -454,7 +454,7 @@ async function syncGroupUsers(config, progressCallback) { }); if (!found) { - debug(`syncGroupUsers: Unable to find group ${group.name} ignoring for now.`); + debug(`syncGroupMembers: Unable to find group ${group.name} ignoring for now.`); continue; } @@ -463,32 +463,34 @@ async function syncGroupUsers(config, progressCallback) { // if only one entry is in the group ldap returns a string, not an array! if (typeof ldapGroupMembers === 'string') ldapGroupMembers = [ ldapGroupMembers ]; - debug(`syncGroupUsers: Group ${group.name} has ${ldapGroupMembers.length} members.`); + debug(`syncGroupMembers: Group ${group.name} has ${ldapGroupMembers.length} members.`); + const userIds = []; for (const memberDn of ldapGroupMembers) { const [ldapError, result] = await safe(ldapGetByDN(config, memberDn)); if (ldapError) { - debug(`syncGroupUsers: Failed to get ${memberDn}: %o`, ldapError); + debug(`syncGroupMembers: Group ${group.name} failed to get ${memberDn}: %o`, ldapError); continue; } - debug(`syncGroupUsers: Found member object at ${memberDn} adding to group ${group.name}`); + debug(`syncGroupMembers: Group ${group.name} has member object ${memberDn}`); - const username = result[config.usernameField].toLowerCase(); + const username = result[config.usernameField]?.toLowerCase(); if (!username) continue; const [getError, userObject] = await safe(users.getByUsername(username)); if (getError || !userObject) { - debug(`syncGroupUsers: Failed to get user by username ${username}. %o`, getError ? getError : 'User not found'); + debug(`syncGroupMembers: Failed to get user by username ${username}. %o`, getError ? getError : 'User not found'); continue; } - const [addError] = await safe(groups.addMember(group.id, userObject.id)); - if (addError && addError.reason !== BoxError.ALREADY_EXISTS) debug('syncGroupUsers: Failed to add member. %o', addError); + userIds.push(userObject.id); } + const [setError] = await safe(groups.setMembers(group.id, userIds)); + if (setError) debug(`syncGroupMembers: Failed to set members of group ${group.name}. %o`, setError); } - debug('syncGroupUsers: done'); + debug('syncGroupMembers: done'); } async function sync(progressCallback) { @@ -501,7 +503,7 @@ async function sync(progressCallback) { await syncUsers(config, progressCallback); await syncGroups(config, progressCallback); - await syncGroupUsers(config, progressCallback); + await syncGroupMembers(config, progressCallback); progressCallback({ percent: 100, message: 'Done' }); diff --git a/src/routes/groups.js b/src/routes/groups.js index a8272326c..ec74de054 100644 --- a/src/routes/groups.js +++ b/src/routes/groups.js @@ -64,7 +64,7 @@ async function update(req, res, next) { async function setMembers(req, res, next) { assert.strictEqual(typeof req.params.groupId, 'string'); - if (req.resource.source === 'ldap') return next(new HttpError(409, 'external group members cannot be set')); + if (req.resource.source === 'ldap') return next(new HttpError(409, 'members of an external group cannot be set')); if (!req.body.userIds) return next(new HttpError(404, 'missing or invalid userIds fields')); if (!Array.isArray(req.body.userIds)) return next(new HttpError(404, 'userIds must be an array'));