From 6f6f23bec46d651a555ebd31d043df440488434f Mon Sep 17 00:00:00 2001
From: Juan Ferrer Toribio <juan@verdnatura.es>
Date: Wed, 9 Dec 2020 12:28:18 +0100
Subject: [PATCH] LDAP user sync only syncs changed attributes

---
 modules/account/back/models/ldap-config.js | 143 ++++++++++++++-------
 1 file changed, 95 insertions(+), 48 deletions(-)

diff --git a/modules/account/back/models/ldap-config.js b/modules/account/back/models/ldap-config.js
index 6b9d9f4dc..819659066 100644
--- a/modules/account/back/models/ldap-config.js
+++ b/modules/account/back/models/ldap-config.js
@@ -35,14 +35,13 @@ module.exports = Self => {
                 accountConfig
             } = this;
 
-            let newEntry;
+            let dn = `uid=${userName},${this.userDn}`;
 
             if (info.hasAccount) {
                 let {user} = info;
 
                 let oldUser = await client.searchOne(this.userDn, {
                     scope: 'sub',
-                    attributes: ['userPassword', 'sambaNTPassword'],
                     filter: `&(uid=${userName})`
                 });
 
@@ -52,7 +51,7 @@ module.exports = Self => {
                     ? nameArgs.splice(1).join(' ')
                     : '-';
 
-                newEntry = {
+                let newEntry = {
                     uid: userName,
                     objectClass: [
                         'inetOrgPerson',
@@ -101,67 +100,115 @@ module.exports = Self => {
                     if (newEntry[prop] == null)
                         delete newEntry[prop];
                 }
+
+                if (oldUser) {
+                    let changes = [];
+                    let skipProps = new Set([
+                        'dn',
+                        'controls'
+                    ]);
+
+                    for (let prop in oldUser) {
+                        let deleteProp = !skipProps.has(prop)
+                            && !newEntry.hasOwnProperty(prop);
+                        if (!deleteProp) continue;
+                        changes.push(new ldap.Change({
+                            operation: 'delete',
+                            modification: {
+                                [prop]: oldUser[prop]
+                            }
+                        }));
+                    }
+                    for (let prop in newEntry) {
+                        if (this.isEqual(oldUser[prop], newEntry[prop]))
+                            continue;
+                        changes.push(new ldap.Change({
+                            operation: 'replace',
+                            modification: {
+                                [prop]: newEntry[prop]
+                            }
+                        }));
+                    }
+
+                    if (changes.length)
+                        await client.modify(dn, changes);
+                } else
+                    await client.add(dn, newEntry);
+            } else {
+                try {
+                    await client.del(dn);
+                    console.log(` -> User '${userName}' removed from LDAP`);
+                } catch (e) {
+                    if (e.name !== 'NoSuchObjectError') throw e;
+                }
             }
+        },
 
-            // Remove and recreate (if applicable) user
-
-            let dn = `uid=${userName},${this.userDn}`;
-            let operation;
-
-            try {
-                await client.del(dn);
-                operation = 'delete';
-            } catch (e) {
-                if (e.name !== 'NoSuchObjectError') throw e;
-            }
-
-            if (info.hasAccount) {
-                await client.add(dn, newEntry);
-                operation = 'add';
-            }
-
-            if (operation === 'delete')
-                console.log(` -> User '${userName}' removed from LDAP`);
+        isEqual(a, b) {
+            if (Array.isArray(a) && Array.isArray(b)) {
+                if (a.length !== b.length)
+                    return false;
+                for (let element of a) {
+                    if (b.indexOf(element) === -1)
+                        return false;
+                }
+                return true;
+            } else
+                return a == b;
         },
 
         async syncUserGroups(userName, info) {
             let {client} = this;
+            let {user} = info;
+            let groupDn = this.groupDn;
 
             let opts = {
                 scope: 'sub',
-                attributes: ['dn'],
+                attributes: ['dn', 'cn'],
                 filter: `&(memberUid=${userName})(objectClass=posixGroup)`
             };
-            let oldGroups = await client.searchAll(this.groupDn, opts);
+            let oldGroups = await client.searchAll(groupDn, opts);
 
-            let reqs = [];
-            for (let oldGroup of oldGroups) {
-                let change = new ldap.Change({
-                    operation: 'delete',
-                    modification: {memberUid: userName}
-                });
-                reqs.push(client.modify(oldGroup.dn, change));
+            let deleteGroups = [];
+            let addGroups = [];
+
+            if (info.hasAccount) {
+                let oldSet = new Set();
+                oldGroups.forEach(e => oldSet.add(e.cn));
+
+                let newSet = new Set();
+                user.roles().forEach(e => newSet.add(e.inherits().name));
+
+                for (let group of oldGroups) {
+                    if (!newSet.has(group.cn))
+                        deleteGroups.push(group.cn);
+                }
+                for (let role of user.roles()) {
+                    if (!oldSet.has(role.inherits().name))
+                        addGroups.push(role.inherits().name);
+                }
+            } else {
+                for (let group of oldGroups)
+                    deleteGroups.push(group.cn);
             }
-            await Promise.all(reqs);
 
-            if (!info.hasAccount) return;
-
-            for (let role of info.user.roles()) {
-                let roleName = role.inherits().name;
-
-                let dn = `cn=${roleName},${this.groupDn}`;
-                let change = new ldap.Change({
-                    operation: 'add',
-                    modification: {memberUid: userName}
-                });
-
-                try {
-                    await client.modify(dn, change);
-                } catch (err) {
-                    if (err.name !== 'NoSuchObjectError')
-                        throw err;
+            async function applyOperations(groups, operation) {
+                for (let group of groups) {
+                    try {
+                        let dn = `cn=${group},${groupDn}`;
+                        await client.modify(dn, new ldap.Change({
+                            operation,
+                            modification: {memberUid: userName}
+                        }));
+                    } catch (err) {
+                        if (err.name !== 'NoSuchObjectError')
+                            throw err;
+                    }
                 }
             }
+
+            await applyOperations(deleteGroups, 'delete');
+            await applyOperations(addGroups, 'add');
         },
 
         async getUsers(usersToSync) {