diff --git a/src/mail.js b/src/mail.js index da5e64edf..30fe35fd4 100644 --- a/src/mail.js +++ b/src/mail.js @@ -1010,34 +1010,31 @@ function getLists(domain, callback) { }); } -function getList(domain, groupId, callback) { +function getList(domain, listName, callback) { assert.strictEqual(typeof domain, 'string'); - assert.strictEqual(typeof groupId, 'string'); + assert.strictEqual(typeof listName, 'string'); assert.strictEqual(typeof callback, 'function'); - groups.get(groupId, function (error, result) { - if (error && error.reason === GroupError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'no such group')); + mailboxdb.getGroup(listName, domain, function (error, result) { + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'no such list')); if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); - mailboxdb.getGroup(result.name, domain, function (error, result) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'no such list')); - if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); - - callback(null, result); - }); + callback(null, result); }); } -function addList(domain, groupId, callback) { +function addList(domain, listName, groupId, callback) { assert.strictEqual(typeof domain, 'string'); + assert.strictEqual(typeof listName, 'string'); assert.strictEqual(typeof groupId, 'string'); assert.strictEqual(typeof callback, 'function'); - groups.get(groupId, function (error, result) { + // sanity check if the group exists + groups.get(groupId, function (error) { if (error && error.reason === GroupError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'no such group')); if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); - mailboxdb.add(result.name, domain, groupId, mailboxdb.TYPE_GROUP, function (error) { + mailboxdb.add(listName, domain, groupId, mailboxdb.TYPE_GROUP, function (error) { if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new MailError(MailError.ALREADY_EXISTS, 'list already exits')); if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); @@ -1046,20 +1043,15 @@ function addList(domain, groupId, callback) { }); } -function removeList(domain, groupId, callback) { +function removeList(domain, listName, callback) { assert.strictEqual(typeof domain, 'string'); - assert.strictEqual(typeof groupId, 'string'); + assert.strictEqual(typeof listName, 'string'); assert.strictEqual(typeof callback, 'function'); - groups.get(groupId, function (error, result) { - if (error && error.reason === GroupError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'no such group')); + mailboxdb.del(listName, domain, function (error) { + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'no such list')); if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); - mailboxdb.del(result.name, domain, function (error) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'no such list')); - if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); - - callback(); - }); + callback(); }); } diff --git a/src/routes/mail.js b/src/routes/mail.js index 89bd8c2e4..0f13412af 100644 --- a/src/routes/mail.js +++ b/src/routes/mail.js @@ -304,9 +304,9 @@ function getLists(req, res, next) { function getList(req, res, next) { assert.strictEqual(typeof req.params.domain, 'string'); - assert.strictEqual(typeof req.params.groupId, 'string'); + assert.strictEqual(typeof req.params.name, 'string'); - mail.getList(req.params.domain, req.params.groupId, function (error, result) { + mail.getList(req.params.domain, req.params.name, function (error, result) { if (error && error.reason === MailError.NOT_FOUND) return next(new HttpError(404, error.message)); if (error) return next(new HttpError(500, error)); @@ -318,9 +318,10 @@ function addList(req, res, next) { assert.strictEqual(typeof req.params.domain, 'string'); assert.strictEqual(typeof req.body, 'object'); + if (typeof req.body.name !== 'string') return next(new HttpError(400, 'name must be a string')); if (typeof req.body.groupId !== 'string') return next(new HttpError(400, 'groupId must be a string')); - mail.addList(req.params.domain, req.body.groupId, function (error) { + mail.addList(req.params.domain, req.body.name, req.body.groupId, function (error) { if (error && error.reason === MailError.NOT_FOUND) return next(new HttpError(404, error.message)); if (error && error.reason === MailError.ALREADY_EXISTS) return next(new HttpError(409, 'list already exists')); if (error) return next(new HttpError(500, error)); @@ -331,9 +332,9 @@ function addList(req, res, next) { function removeList(req, res, next) { assert.strictEqual(typeof req.params.domain, 'string'); - assert.strictEqual(typeof req.params.groupId, 'string'); + assert.strictEqual(typeof req.params.name, 'string'); - mail.removeList(req.params.domain, req.params.groupId, function (error) { + mail.removeList(req.params.domain, req.params.name, function (error) { if (error && error.reason === MailError.NOT_FOUND) return next(new HttpError(404, error.message)); if (error) return next(new HttpError(500, error)); diff --git a/src/routes/test/mail-test.js b/src/routes/test/mail-test.js index d7a6332a3..73838c924 100644 --- a/src/routes/test/mail-test.js +++ b/src/routes/test/mail-test.js @@ -27,7 +27,7 @@ const DOMAIN_0 = { tlsConfig: { provider: 'fallback' } }; var USERNAME = 'superadmin', PASSWORD = 'Foobar?1337', EMAIL ='silly@me.com'; -const GROUP_NAME = 'maillistgroup'; +const GROUP_NAME = 'maillistgroup', LIST_NAME = 'devs'; var token = null; var userId = ''; var groupObject = null; @@ -936,7 +936,7 @@ describe('Mail API', function () { it('add fails with non-existing groupId', function (done) { superagent.post(SERVER_URL + '/api/v1/mail/' + DOMAIN_0.domain + '/lists') - .send({ groupId: 'doesnotexist' }) + .send({ name: LIST_NAME, groupId: 'doesnotexist' }) .query({ access_token: token }) .end(function (err, res) { expect(res.statusCode).to.equal(404); @@ -946,7 +946,7 @@ describe('Mail API', function () { it('add succeeds', function (done) { superagent.post(SERVER_URL + '/api/v1/mail/' + DOMAIN_0.domain + '/lists') - .send({ groupId: groupObject.id }) + .send({ name: LIST_NAME, groupId: groupObject.id }) .query({ access_token: token }) .end(function (err, res) { expect(res.statusCode).to.equal(201); @@ -956,7 +956,7 @@ describe('Mail API', function () { it('add twice fails', function (done) { superagent.post(SERVER_URL + '/api/v1/mail/' + DOMAIN_0.domain + '/lists') - .send({ groupId: groupObject.id }) + .send({ name: LIST_NAME, groupId: groupObject.id }) .query({ access_token: token }) .end(function (err, res) { expect(res.statusCode).to.equal(409); @@ -974,12 +974,12 @@ describe('Mail API', function () { }); it('get succeeds', function (done) { - superagent.get(SERVER_URL + '/api/v1/mail/' + DOMAIN_0.domain + '/lists/' + groupObject.id) + superagent.get(SERVER_URL + `/api/v1/mail/${DOMAIN_0.domain}/lists/${LIST_NAME}`) .query({ access_token: token }) .end(function (err, res) { expect(res.statusCode).to.equal(200); expect(res.body.list).to.be.an('object'); - expect(res.body.list.name).to.equal(GROUP_NAME); + expect(res.body.list.name).to.equal(LIST_NAME); expect(res.body.list.ownerId).to.equal(groupObject.id); expect(res.body.list.ownerType).to.equal('group'); expect(res.body.list.aliasTarget).to.equal(null); @@ -996,7 +996,7 @@ describe('Mail API', function () { expect(res.statusCode).to.equal(200); expect(res.body.lists).to.be.an(Array); expect(res.body.lists.length).to.equal(1); - expect(res.body.lists[0].name).to.equal(GROUP_NAME); + expect(res.body.lists[0].name).to.equal(LIST_NAME); expect(res.body.lists[0].ownerId).to.equal(groupObject.id); expect(res.body.lists[0].ownerType).to.equal('group'); expect(res.body.lists[0].aliasTarget).to.equal(null); @@ -1015,12 +1015,12 @@ describe('Mail API', function () { }); it('del succeeds', function (done) { - superagent.del(SERVER_URL + '/api/v1/mail/' + DOMAIN_0.domain + '/lists/' + groupObject.id) + superagent.del(SERVER_URL + '/api/v1/mail/' + DOMAIN_0.domain + '/lists/' + LIST_NAME) .query({ access_token: token }) .end(function (err, res) { expect(res.statusCode).to.equal(204); - superagent.get(SERVER_URL + '/api/v1/mail/' + DOMAIN_0.domain + '/lists/' + groupObject.id) + superagent.get(SERVER_URL + '/api/v1/mail/' + DOMAIN_0.domain + '/lists/' + LIST_NAME) .query({ access_token: token }) .end(function (err, res) { expect(res.statusCode).to.equal(404); diff --git a/src/server.js b/src/server.js index 285127abd..4a7b0a443 100644 --- a/src/server.js +++ b/src/server.js @@ -234,8 +234,8 @@ function initializeExpressSync() { router.put ('/api/v1/mail/:domain/aliases/:userId', settingsScope, routes.user.requireAdmin, routes.mail.setUserAliases); router.get ('/api/v1/mail/:domain/lists', settingsScope, routes.user.requireAdmin, routes.mail.getLists); router.post('/api/v1/mail/:domain/lists', settingsScope, routes.user.requireAdmin, routes.mail.addList); - router.get ('/api/v1/mail/:domain/lists/:groupId', settingsScope, routes.user.requireAdmin, routes.mail.getList); - router.del ('/api/v1/mail/:domain/lists/:groupId', settingsScope, routes.user.requireAdmin, routes.mail.removeList); + router.get ('/api/v1/mail/:domain/lists/:name', settingsScope, routes.user.requireAdmin, routes.mail.getList); + router.del ('/api/v1/mail/:domain/lists/:name', settingsScope, routes.user.requireAdmin, routes.mail.removeList); // feedback router.post('/api/v1/feedback', usersScope, routes.cloudron.feedback);