From 86d642c8a339d4a72e93d4d4eef7f53e70f2ca93 Mon Sep 17 00:00:00 2001 From: Johannes Zellner Date: Thu, 9 Dec 2021 17:23:14 +0100 Subject: [PATCH] Fixup ldap group tests --- src/ldap.js | 18 +++++---- src/test/ldap-test.js | 92 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 84 insertions(+), 26 deletions(-) diff --git a/src/ldap.js b/src/ldap.js index 4610bff59..19b1b4a9a 100644 --- a/src/ldap.js +++ b/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) { - console.log('----', group) + if (req.app.accessRestriction && req.app.accessRestriction.groups) { + 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 } }; diff --git a/src/test/ldap-test.js b/src/test/ldap-test.js index 75eab7cee..aa606096c 100644 --- a/src/test/ldap-test.js +++ b/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[1]).to.equal(user.id); + expect(entries[0].memberuid).to.contain(admin.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[1]).to.equal(user.id); + expect(entries[0].memberuid).to.contain(admin.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[1]).to.equal(user.id); + expect(entries[0].memberuid).to.contain(admin.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[1]).to.equal(user.id); + expect(entries[0].memberuid).to.contain(admin.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); }); });