Fixup ldap group tests

src/ldap.js

@@ -185,7 +185,7 @@ async function userSearch(req, res, next) {
 async function groupSearch(req, res, next) {
     debug('group search: dn %s, scope %s, filter %s (from %s)', req.dn.toString(), req.scope, req.filter.toString(), req.connection.ldap.id);
 
-    const [error, result] = await safe(getUsersWithAccessToApp(req));
+    const [error, usersWithAccess] = await safe(getUsersWithAccessToApp(req));
     if (error) return next(new ldap.OperationsError(error.toString()));
 
     const results = [];
@@ -201,14 +201,14 @@ async function groupSearch(req, res, next) {
 
     virtualGroups.forEach(function (group) {
         const dn = ldap.parseDN('cn=' + group.name + ',ou=groups,dc=cloudron');
-        const members = group.admin ? result.filter(function (user) { return users.compareRoles(user.role, users.ROLE_ADMIN) >= 0; }) : result;
+        const members = group.admin ? usersWithAccess.filter(function (user) { return users.compareRoles(user.role, users.ROLE_ADMIN) >= 0; }) : usersWithAccess;
 
         const obj = {
             dn: dn.toString(),
             attributes: {
                 objectclass: ['group'],
                 cn: group.name,
-                memberuid: members.map(function(entry) { return entry.id; })
+                memberuid: members.map(function(entry) { return entry.id; }).sort()
             }
         };
 
@@ -221,21 +221,23 @@ async function groupSearch(req, res, next) {
         }
     });
 
-    const [errorGroups, resultGroups] = await safe(groups.listWithMembers());
+    let [errorGroups, resultGroups] = await safe(groups.listWithMembers());
     if (errorGroups) return next(new ldap.OperationsError(errorGroups.toString()));
 
-    resultGroups.forEach(function (group) {
+    if (req.app.accessRestriction && req.app.accessRestriction.groups) {
-        console.log('----', group)
+        resultGroups = resultGroups.filter(function (g) { return req.app.accessRestriction.groups.indexOf(g.id) !== -1; });
+    }
 
+    resultGroups.forEach(function (group) {
         const dn = ldap.parseDN('cn=' + group.name + ',ou=groups,dc=cloudron');
-        const members = [];
+        const members = group.userIds.filter(function (uid) { return usersWithAccess.map(function (u) { return u.id; }).indexOf(uid) !== -1; });
 
         const obj = {
             dn: dn.toString(),
             attributes: {
                 objectclass: ['group'],
                 cn: group.name,
-                memberuid: members.map(function(entry) { return entry.id; })
+                memberuid: members
             }
         };
 

src/test/ldap-test.js

@@ -60,7 +60,7 @@ async function ldapSearch(dn, opts) {
 
 describe('Ldap', function () {
     const { setup, cleanup, admin, user, app, domain, auditSource } = common;
-    let group;
+    let group, group2;
     const mockApp = Object.assign({}, app);
 
     const mailboxName = 'support';
@@ -75,8 +75,12 @@ describe('Ldap', function () {
             async () => await mail.setAliases(mailboxName, domain.domain, [ { name: mailAliasName, domain: domain.domain} ], auditSource),
             ldapServer.start.bind(null),
             async () => {
-                group = await groups.add({ name: 'ldap-test' });
+                group = await groups.add({ name: 'ldap-test-1' });
                 await groups.setMembers(group.id, [ admin.id, user.id ]);
+            },
+            async () => {
+                group2 = await groups.add({ name: 'ldap-test-2' });
+                await groups.setMembers(group2.id, [ admin.id ]);
             }
         ], done);
 
@@ -203,68 +207,120 @@ describe('Ldap', function () {
 
     describe('group search', function () {
         it('succeeds with basic filter', async function () {
+            mockApp.accessRestriction = null;
+
             const entries = await ldapSearch('ou=groups,dc=cloudron', { filter: 'objectclass=group' });
-            expect(entries.length).to.equal(2);
+            expect(entries.length).to.equal(4);
 
             // ensure order for testability
-            entries.sort(function (a, b) { return a.username < b.username; });
+            entries.sort(function (a, b) { return a.cn < b.cn; });
 
             expect(entries[0].cn).to.equal('users');
             expect(entries[0].memberuid.length).to.equal(2);
-            expect(entries[0].memberuid[0]).to.equal(admin.id);
+            expect(entries[0].memberuid).to.contain(admin.id);
-            expect(entries[0].memberuid[1]).to.equal(user.id);
+            expect(entries[0].memberuid).to.contain(user.id);
+
             expect(entries[1].cn).to.equal('admins');
             // if only one entry, the array becomes a string :-/
             expect(entries[1].memberuid).to.equal(admin.id);
+
+            expect(entries[2].cn).to.equal('ldap-test-1');
+            expect(entries[2].memberuid.length).to.equal(2);
+            expect(entries[2].memberuid).to.contain(admin.id);
+            expect(entries[2].memberuid).to.contain(user.id);
+
+            expect(entries[3].cn).to.equal('ldap-test-2');
+            expect(entries[3].memberuid).to.equal(admin.id);
         });
 
         it ('succeeds with cn wildcard filter', async function () {
             const entries = await ldapSearch('ou=groups,dc=cloudron', { filter: '&(objectclass=group)(cn=*)' });
-            expect(entries.length).to.equal(2);
+            expect(entries.length).to.equal(4);
+
+            // ensure order for testability
+            entries.sort(function (a, b) { return a.cn < b.cn; });
+
             expect(entries[0].cn).to.equal('users');
             expect(entries[0].memberuid.length).to.equal(2);
-            expect(entries[0].memberuid[0]).to.equal(admin.id);
+            expect(entries[0].memberuid).to.contain(admin.id);
-            expect(entries[0].memberuid[1]).to.equal(user.id);
+            expect(entries[0].memberuid).to.contain(user.id);
+
             expect(entries[1].cn).to.equal('admins');
             // if only one entry, the array becomes a string :-/
             expect(entries[1].memberuid).to.equal(admin.id);
+
+            expect(entries[2].cn).to.equal('ldap-test-1');
+            expect(entries[2].memberuid.length).to.equal(2);
+            expect(entries[2].memberuid).to.contain(admin.id);
+            expect(entries[2].memberuid).to.contain(user.id);
+
+            expect(entries[3].cn).to.equal('ldap-test-2');
+            expect(entries[3].memberuid).to.equal(admin.id);
         });
 
         it('succeeds with memberuid filter', async function () {
             const entries = await ldapSearch('ou=groups,dc=cloudron', { filter: '&(objectclass=group)(memberuid=' + user.id + ')' });
-            expect(entries.length).to.equal(1);
+            expect(entries.length).to.equal(2);
+
+            // ensure order for testability
+            entries.sort(function (a, b) { return a.cn < b.cn; });
+
             expect(entries[0].cn).to.equal('users');
             expect(entries[0].memberuid.length).to.equal(2);
+
+            expect(entries[1].cn).to.equal('ldap-test-1');
+            expect(entries[1].memberuid.length).to.equal(2);
+            expect(entries[1].memberuid).to.contain(admin.id);
+            expect(entries[1].memberuid).to.contain(user.id);
         });
 
-        it ('does only list users who have access', async function () {
+        it ('does only list groups who have access', async function () {
             mockApp.accessRestriction = { users: [], groups: [ group.id ] };
             const entries = await ldapSearch('ou=groups,dc=cloudron', { filter: '&(objectclass=group)(cn=*)' });
-            expect(entries.length).to.equal(2);
+
+            // ensure order for testability
+            entries.sort(function (a, b) { return a.cn < b.cn; });
+
+            expect(entries.length).to.equal(3);
             expect(entries[0].cn).to.equal('users');
             expect(entries[0].memberuid.length).to.equal(2);
-            expect(entries[0].memberuid[0]).to.equal(admin.id);
+            expect(entries[0].memberuid).to.contain(admin.id);
-            expect(entries[0].memberuid[1]).to.equal(user.id);
+            expect(entries[0].memberuid).to.contain(user.id);
+
             expect(entries[1].cn).to.equal('admins');
             // if only one entry, the array becomes a string :-/
             expect(entries[1].memberuid).to.equal(admin.id);
+
+            expect(entries[2].cn).to.equal('ldap-test-1');
+            expect(entries[2].memberuid.length).to.equal(2);
+            expect(entries[2].memberuid).to.contain(admin.id);
+            expect(entries[2].memberuid).to.contain(user.id);
         });
 
         it ('succeeds with pagination', async function () {
             mockApp.accessRestriction = null;
             const entries = await ldapSearch('ou=groups,dc=cloudron', { filter: 'objectclass=group', paged: true });
-            expect(entries.length).to.equal(2);
+            expect(entries.length).to.equal(4);
 
             // ensure order for testability
-            entries.sort(function (a, b) { return a.username < b.username; });
+            entries.sort(function (a, b) { return a.cn < b.cn; });
 
             expect(entries[0].cn).to.equal('users');
             expect(entries[0].memberuid.length).to.equal(2);
-            expect(entries[0].memberuid[0]).to.equal(admin.id);
+            expect(entries[0].memberuid).to.contain(admin.id);
-            expect(entries[0].memberuid[1]).to.equal(user.id);
+            expect(entries[0].memberuid).to.contain(user.id);
+
             expect(entries[1].cn).to.equal('admins');
             // if only one entry, the array becomes a string :-/
             expect(entries[1].memberuid).to.equal(admin.id);
+
+            expect(entries[2].cn).to.equal('ldap-test-1');
+            expect(entries[2].memberuid.length).to.equal(2);
+            expect(entries[2].memberuid).to.contain(admin.id);
+            expect(entries[2].memberuid).to.contain(user.id);
+
+            expect(entries[3].cn).to.equal('ldap-test-2');
+            expect(entries[3].memberuid).to.equal(admin.id);
         });
     });