From dfa61f1b2d3c4e0170367a0b8ff196a7024a7e5f Mon Sep 17 00:00:00 2001 From: Girish Ramakrishnan Date: Thu, 6 Dec 2018 21:08:19 -0800 Subject: [PATCH] rework how app mailboxes are allocated Our current setup had a mailbox allocated for an app during app install (into the mailboxes table). This has many issues: * When set to a custom mailbox location, there was no way to access this mailbox even via IMAP. Even when using app credentials, we cannot use IMAP since the ldap logic was testing on the addon type (most of our apps only use sendmail addon and thus cannot recvmail). * The mailboxes table was being used to add hidden 'app' type entries. This made it very hard for the user to understand why a mailbox conflicts. For example, if you set an app to use custom mailbox 'blog', this is hidden from all views. The solution is to let an app send email as whatever mailbox name is allocated to it (which we now track in the apps table. the default is in the db already so that REST response contains it). When not using Cloudron email, it will just send mail as that mailbox and the auth checks the "app password" in the addons table. Any replies to that mailbox will end up in the domain's mail server (not our problem). When using cloudron email, the app can send mail like above. Any responses will not end anywhere and bounce since there is no 'mailbox'. This is the expected behavior. If user wants to access this mailbox name, he can create a concrete mailbox and set himself as owner OR set this as an alias. For apps using the recvmail addon, the workflow is to actually create a mailbox at some point. Currently, we have no UI for this 'flow'. It's fine because we have only meemo using it. Intuitive much! --- CHANGES | 2 + .../20181207042802-apps-add-mailboxName.js | 28 +++++++++ ...181207042803-mailboxes-remove-ownerType.js | 28 +++++++++ migrations/schema.sql | 7 ++- src/addons.js | 54 +++++++---------- src/appdb.js | 36 +++++++----- src/apps.js | 58 +++++-------------- src/ldap.js | 45 +++++++------- src/mail.js | 4 +- src/mailboxdb.js | 26 ++++----- src/routes/test/mail-test.js | 6 -- src/test/database-test.js | 20 ++++--- src/test/ldap-test.js | 13 +++-- 13 files changed, 170 insertions(+), 157 deletions(-) create mode 100644 migrations/20181207042802-apps-add-mailboxName.js create mode 100644 migrations/20181207042803-mailboxes-remove-ownerType.js diff --git a/CHANGES b/CHANGES index e98e38590..b62e0ae46 100644 --- a/CHANGES +++ b/CHANGES @@ -1471,3 +1471,5 @@ * Add server reboot button and warn if reboot is required for security updates * Backup and update tasks are now cancelable * Move graphite away from port 3000 (reserved by ESXi) +* Flexible mailbox management + diff --git a/migrations/20181207042802-apps-add-mailboxName.js b/migrations/20181207042802-apps-add-mailboxName.js new file mode 100644 index 000000000..af73b260d --- /dev/null +++ b/migrations/20181207042802-apps-add-mailboxName.js @@ -0,0 +1,28 @@ +'use strict'; + +var async = require('async'); + +exports.up = function(db, callback) { + async.series([ + db.runSql.bind(db, 'ALTER TABLE apps ADD COLUMN mailboxName VARCHAR(128)'), + db.runSql.bind(db, 'START TRANSACTION;'), + + function migrateMailboxNames(done) { + db.all('SELECT * FROM mailboxes', function (error, mailboxes) { + if (error) return done(error); + + async.eachSeries(mailboxes, function (mailbox, iteratorDone) { + if (mailbox.ownerType !== 'app') return iteratorDone(); + + db.runSql('UPDATE apps SET mailboxName = ? WHERE id = ?', [ mailbox.name, mailbox.ownerId ], iteratorDone); + }, done); + }); + }, + + db.runSql.bind(db, 'COMMIT') + ], callback); +}; + +exports.down = function(db, callback) { + callback(); +}; diff --git a/migrations/20181207042803-mailboxes-remove-ownerType.js b/migrations/20181207042803-mailboxes-remove-ownerType.js new file mode 100644 index 000000000..92b0aa425 --- /dev/null +++ b/migrations/20181207042803-mailboxes-remove-ownerType.js @@ -0,0 +1,28 @@ +'use strict'; + +var async = require('async'); + +exports.up = function(db, callback) { + async.series([ + db.runSql.bind(db, 'START TRANSACTION;'), + + function migrateMailboxNames(done) { + db.all('SELECT * FROM mailboxes', function (error, mailboxes) { + if (error) return done(error); + + async.eachSeries(mailboxes, function (mailbox, iteratorDone) { + if (mailbox.ownerType !== 'app') return iteratorDone(); + + db.runSql('DELETE FROM mailboxes WHERE name = ?', [ mailbox.name ], iteratorDone); + }, done); + }); + }, + + db.runSql.bind(db, 'COMMIT'), + db.runSql.bind(db, 'ALTER TABLE mailboxes DROP COLUMN ownerType') + ], callback); +}; + +exports.down = function(db, callback) { + callback(); +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index 6797212bf..d27ff3d11 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -80,6 +80,7 @@ CREATE TABLE IF NOT EXISTS apps( debugModeJson TEXT, // options for development mode robotsTxt TEXT, enableBackup BOOLEAN DEFAULT 1, // misnomer: controls automatic daily backups + mailboxName VARCHAR(128), // mailbox of this app // the following fields do not belong here, they can be removed when we use a queue for apptask restoreConfigJson VARCHAR(256), // used to pass backupId to restore from to apptask @@ -173,12 +174,14 @@ CREATE TABLE IF NOT EXISTS mail( /* Future fields: * accessRestriction - to determine who can access it. So this has foreign keys * quota - per mailbox quota + + NOTE: this table exists only real mailboxes. And has unique constraint to handle + conflict with aliases and mailbox names */ CREATE TABLE IF NOT EXISTS mailboxes( name VARCHAR(128) NOT NULL, type VARCHAR(16) NOT NULL, /* 'mailbox', 'alias', 'list' */ - ownerId VARCHAR(128) NOT NULL, /* app id or user id or group id */ - ownerType VARCHAR(16) NOT NULL, /* 'app' or 'user' or 'group' */ + ownerId VARCHAR(128) NOT NULL, /* user id */ aliasTarget VARCHAR(128), /* the target name type is an alias */ membersJson TEXT, /* members of a group */ creationTime TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, diff --git a/src/addons.js b/src/addons.js index 3d4127502..0a98e2882 100644 --- a/src/addons.js +++ b/src/addons.js @@ -879,23 +879,17 @@ function setupSendMail(app, options, callback) { var password = error ? hat(4 * 48) : existingPassword; // see box#565 for password length - mailboxdb.getByOwnerId(app.id, function (error, results) { - if (error) return callback(error); - - var mailbox = results.filter(function (r) { return !r.aliasTarget; })[0]; - - var env = [ - { name: 'MAIL_SMTP_SERVER', value: 'mail' }, - { name: 'MAIL_SMTP_PORT', value: '2525' }, - { name: 'MAIL_SMTPS_PORT', value: '2465' }, - { name: 'MAIL_SMTP_USERNAME', value: mailbox.name + '@' + app.domain }, - { name: 'MAIL_SMTP_PASSWORD', value: password }, - { name: 'MAIL_FROM', value: mailbox.name + '@' + app.domain }, - { name: 'MAIL_DOMAIN', value: app.domain } - ]; - debugApp(app, 'Setting sendmail addon config to %j', env); - appdb.setAddonConfig(app.id, 'sendmail', env, callback); - }); + var env = [ + { name: 'MAIL_SMTP_SERVER', value: 'mail' }, + { name: 'MAIL_SMTP_PORT', value: '2525' }, + { name: 'MAIL_SMTPS_PORT', value: '2465' }, + { name: 'MAIL_SMTP_USERNAME', value: app.mailboxName + '@' + app.domain }, + { name: 'MAIL_SMTP_PASSWORD', value: password }, + { name: 'MAIL_FROM', value: app.mailboxName + '@' + app.domain }, + { name: 'MAIL_DOMAIN', value: app.domain } + ]; + debugApp(app, 'Setting sendmail addon config to %j', env); + appdb.setAddonConfig(app.id, 'sendmail', env, callback); }); } @@ -921,23 +915,17 @@ function setupRecvMail(app, options, callback) { var password = error ? hat(4 * 48) : existingPassword; // see box#565 for password length - mailboxdb.getByOwnerId(app.id, function (error, results) { - if (error) return callback(error); + var env = [ + { name: 'MAIL_IMAP_SERVER', value: 'mail' }, + { name: 'MAIL_IMAP_PORT', value: '9993' }, + { name: 'MAIL_IMAP_USERNAME', value: app.mailboxName + '@' + app.domain }, + { name: 'MAIL_IMAP_PASSWORD', value: password }, + { name: 'MAIL_TO', value: app.mailboxName + '@' + app.domain }, + { name: 'MAIL_DOMAIN', value: app.domain } + ]; - var mailbox = results.filter(function (r) { return !r.aliasTarget; })[0]; - - var env = [ - { name: 'MAIL_IMAP_SERVER', value: 'mail' }, - { name: 'MAIL_IMAP_PORT', value: '9993' }, - { name: 'MAIL_IMAP_USERNAME', value: mailbox.name + '@' + app.domain }, - { name: 'MAIL_IMAP_PASSWORD', value: password }, - { name: 'MAIL_TO', value: mailbox.name + '@' + app.domain }, - { name: 'MAIL_DOMAIN', value: app.domain } - ]; - - debugApp(app, 'Setting sendmail addon config to %j', env); - appdb.setAddonConfig(app.id, 'recvmail', env, callback); - }); + debugApp(app, 'Setting sendmail addon config to %j', env); + appdb.setAddonConfig(app.id, 'recvmail', env, callback); }); } diff --git a/src/appdb.js b/src/appdb.js index 271c1c525..c626fd283 100644 --- a/src/appdb.js +++ b/src/appdb.js @@ -18,6 +18,7 @@ exports = module.exports = { getAddonConfigByName: getAddonConfigByName, unsetAddonConfig: unsetAddonConfig, unsetAddonConfigByAppId: unsetAddonConfigByAppId, + getAppIdByAddonConfigValue: getAppIdByAddonConfigValue, setHealth: setHealth, setInstallationCommand: setInstallationCommand, @@ -61,7 +62,6 @@ var assert = require('assert'), async = require('async'), database = require('./database.js'), DatabaseError = require('./databaseerror'), - mailboxdb = require('./mailboxdb.js'), safe = require('safetydance'), util = require('util'); @@ -69,7 +69,7 @@ var APPS_FIELDS_PREFIXED = [ 'apps.id', 'apps.appStoreId', 'apps.installationSta 'apps.health', 'apps.containerId', 'apps.manifestJson', 'apps.httpPort', 'subdomains.subdomain AS location', 'subdomains.domain', 'apps.accessRestrictionJson', 'apps.restoreConfigJson', 'apps.oldConfigJson', 'apps.updateConfigJson', 'apps.memoryLimit', 'apps.xFrameOptions', 'apps.sso', 'apps.debugModeJson', 'apps.robotsTxt', 'apps.enableBackup', - 'apps.creationTime', 'apps.updateTime', 'apps.ownerId', 'apps.ts' ].join(','); + 'apps.creationTime', 'apps.updateTime', 'apps.ownerId', 'apps.mailboxName', 'apps.ts' ].join(','); var PORT_BINDINGS_FIELDS = [ 'hostPort', 'type', 'environmentVariable', 'appId' ].join(','); @@ -276,13 +276,14 @@ function add(id, appStoreId, manifest, location, domain, ownerId, portBindings, var robotsTxt = 'robotsTxt' in data ? data.robotsTxt : null; var debugModeJson = data.debugMode ? JSON.stringify(data.debugMode) : null; var env = data.env || {}; + const mailboxName = data.mailboxName || null; var queries = []; queries.push({ - query: 'INSERT INTO apps (id, appStoreId, manifestJson, installationState, accessRestrictionJson, memoryLimit, xFrameOptions, restoreConfigJson, sso, debugModeJson, robotsTxt, ownerId) ' + - ' VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)', - args: [ id, appStoreId, manifestJson, installationState, accessRestrictionJson, memoryLimit, xFrameOptions, restoreConfigJson, sso, debugModeJson, robotsTxt, ownerId ] + query: 'INSERT INTO apps (id, appStoreId, manifestJson, installationState, accessRestrictionJson, memoryLimit, xFrameOptions, restoreConfigJson, sso, debugModeJson, robotsTxt, ownerId, mailboxName) ' + + ' VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)', + args: [ id, appStoreId, manifestJson, installationState, accessRestrictionJson, memoryLimit, xFrameOptions, restoreConfigJson, sso, debugModeJson, robotsTxt, ownerId, mailboxName ] }); queries.push({ @@ -304,14 +305,6 @@ function add(id, appStoreId, manifest, location, domain, ownerId, portBindings, }); }); - // only allocate a mailbox if mailboxName is set - if (data.mailboxName) { - queries.push({ - query: 'INSERT INTO mailboxes (name, type, domain, ownerId, ownerType) VALUES (?, ?, ?, ?, ?)', - args: [ data.mailboxName, mailboxdb.TYPE_MAILBOX, domain, id, mailboxdb.OWNER_TYPE_APP ] - }); - } - if (data.alternateDomains) { data.alternateDomains.forEach(function (d) { queries.push({ @@ -376,7 +369,6 @@ function del(id, callback) { var queries = [ { query: 'DELETE FROM subdomains WHERE appId = ?', args: [ id ] }, - { query: 'DELETE FROM mailboxes WHERE ownerId=?', args: [ id ] }, { query: 'DELETE FROM appPortBindings WHERE appId = ?', args: [ id ] }, { query: 'DELETE FROM appEnvVars WHERE appId = ?', args: [ id ] }, { query: 'DELETE FROM apps WHERE id = ?', args: [ id ] } @@ -384,7 +376,7 @@ function del(id, callback) { database.transaction(queries, function (error, results) { if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); - if (results[4].affectedRows !== 1) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); + if (results[3].affectedRows !== 1) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); callback(null); }); @@ -621,6 +613,20 @@ function getAddonConfigByAppId(appId, callback) { }); } +function getAppIdByAddonConfigValue(addonId, name, value, callback) { + assert.strictEqual(typeof addonId, 'string'); + assert.strictEqual(typeof name, 'string'); + assert.strictEqual(typeof value, 'string'); + assert.strictEqual(typeof callback, 'function'); + + database.query('SELECT appId FROM appAddonConfigs WHERE addonId = ? AND name = ? AND value = ?', [ addonId, name, value ], function (error, results) { + if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); + if (results.length === 0) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); + + callback(null, results[0].appId); + }); +} + function getAddonConfigByName(appId, addonId, name, callback) { assert.strictEqual(typeof appId, 'string'); assert.strictEqual(typeof addonId, 'string'); diff --git a/src/apps.js b/src/apps.js index 928f4d867..825bc871e 100644 --- a/src/apps.js +++ b/src/apps.js @@ -72,7 +72,6 @@ var appdb = require('./appdb.js'), eventlog = require('./eventlog.js'), fs = require('fs'), mail = require('./mail.js'), - mailboxdb = require('./mailboxdb.js'), manifestFormat = require('cloudron-manifestformat'), os = require('os'), path = require('path'), @@ -400,13 +399,7 @@ function get(appId, callback) { app.fqdn = domains.fqdn(app.location, domainObjectMap[app.domain]); app.alternateDomains.forEach(function (ad) { ad.fqdn = domains.fqdn(ad.subdomain, domainObjectMap[ad.domain]); }); - mailboxdb.getByOwnerId(app.id, function (error, mailboxes) { - if (error && error.reason !== DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - - if (!error) app.mailboxName = mailboxes[0].name; - - callback(null, app); - }); + callback(null, app); }); }); } @@ -440,13 +433,7 @@ function getByIpAddress(ip, callback) { app.fqdn = domains.fqdn(app.location, domainObjectMap[app.domain]); app.alternateDomains.forEach(function (ad) { ad.fqdn = domains.fqdn(ad.subdomain, domainObjectMap[ad.domain]); }); - mailboxdb.getByOwnerId(app.id, function (error, mailboxes) { - if (error && error.reason !== DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - - if (!error) app.mailboxName = mailboxes[0].name; - - callback(null, app); - }); + callback(null, app); }); }); }); @@ -472,13 +459,7 @@ function getAll(callback) { app.fqdn = domains.fqdn(app.location, domainObjectMap[app.domain]); app.alternateDomains.forEach(function (ad) { ad.fqdn = domains.fqdn(ad.subdomain, domainObjectMap[ad.domain]); }); - mailboxdb.getByOwnerId(app.id, function (error, mailboxes) { - if (error && error.reason !== DatabaseError.NOT_FOUND) return iteratorDone(new AppsError(AppsError.INTERNAL_ERROR, error)); - - if (!error) app.mailboxName = mailboxes[0].name; - - iteratorDone(null, app); - }); + iteratorDone(null, app); }, function (error) { if (error) return callback(error); @@ -683,7 +664,7 @@ function configure(appId, data, user, auditSource, callback) { get(appId, function (error, app) { if (error) return callback(error); - let domain, location, portBindings, values = { }, mailboxName; + let domain, location, portBindings, values = { }; if ('location' in data) location = values.location = data.location.toLowerCase(); else location = app.location; @@ -731,14 +712,14 @@ function configure(appId, data, user, auditSource, callback) { if ('mailboxName' in data) { if (data.mailboxName === '') { // special case to reset back to .app - mailboxName = mailboxNameForLocation(location, app.manifest); + values.mailboxName = mailboxNameForLocation(location, app.manifest); } else { error = mail.validateName(data.mailboxName); if (error) return callback(error); - mailboxName = data.mailboxName; + values.mailboxName = data.mailboxName; } } else { // keep existing name or follow the new location - mailboxName = app.mailboxName.endsWith('.app') ? mailboxNameForLocation(location, app.manifest) : app.mailboxName; + values.mailboxName = app.mailboxName.endsWith('.app') ? mailboxNameForLocation(location, app.manifest) : app.mailboxName; } if ('alternateDomains' in data) { @@ -779,29 +760,20 @@ function configure(appId, data, user, auditSource, callback) { debug('Will configure app with id:%s values:%j', appId, values); - // make the mailbox name follow the apps new location, if the user did not set it explicitly - mailboxdb.updateName(app.mailboxName /* old */, values.oldConfig.domain, mailboxName, domain, function (error) { - if (mailboxName.endsWith('.app')) error = null; // ignore internal mailbox conflict errors since we want to show location conflict errors in the UI - - if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new AppsError(AppsError.ALREADY_EXISTS, 'This mailbox is already taken')); + appdb.setInstallationCommand(appId, appdb.ISTATE_PENDING_CONFIGURE, values, function (error) { + if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(getDuplicateErrorDetails(location, portBindings, error)); if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.BAD_STATE)); if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - appdb.setInstallationCommand(appId, appdb.ISTATE_PENDING_CONFIGURE, values, function (error) { - if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(getDuplicateErrorDetails(location, portBindings, error)); - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.BAD_STATE)); + taskmanager.restartAppTask(appId); + + // fetch fresh app object for eventlog + get(appId, function (error, result) { if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - taskmanager.restartAppTask(appId); + eventlog.add(eventlog.ACTION_APP_CONFIGURE, auditSource, { appId: appId, app: result }); - // fetch fresh app object for eventlog - get(appId, function (error, result) { - if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error)); - - eventlog.add(eventlog.ACTION_APP_CONFIGURE, auditSource, { appId: appId, app: result }); - - callback(null); - }); + callback(null); }); }); }); diff --git a/src/ldap.js b/src/ldap.js index a4555395c..98c0f05cf 100644 --- a/src/ldap.js +++ b/src/ldap.js @@ -296,9 +296,6 @@ function mailboxSearch(req, res, next) { var results = []; - // only send user mailboxes - result = result.filter(function (m) { return m.ownerType === mailboxdb.OWNER_TYPE_USER; }); - // send mailbox objects result.forEach(function (mailbox) { var dn = ldap.parseDN(`cn=${mailbox.name}@${domain},domain=${domain},ou=mailboxes,dc=cloudron`); @@ -462,30 +459,30 @@ function authenticateMailbox(req, res, next) { var parts = email.split('@'); if (parts.length !== 2) return next(new ldap.NoSuchObjectError(req.dn.toString())); - mailboxdb.getMailbox(parts[0], parts[1], function (error, mailbox) { - if (error && error.reason === DatabaseError.NOT_FOUND) return next(new ldap.NoSuchObjectError(req.dn.toString())); + const addonId = req.dn.rdns[1].attrs.ou.value.toLowerCase(); // 'sendmail' or 'recvmail' + + mail.getDomain(parts[1], function (error, domain) { + if (error && error.reason === MailError.NOT_FOUND) return next(new ldap.NoSuchObjectError(req.dn.toString())); if (error) return next(new ldap.OperationsError(error.message)); - mail.getDomain(parts[1], function (error, domain) { - if (error && error.reason === MailError.NOT_FOUND) return next(new ldap.NoSuchObjectError(req.dn.toString())); - if (error) return next(new ldap.OperationsError(error.message)); + if (addonId === 'recvmail' && !domain.enabled) return next(new ldap.NoSuchObjectError(req.dn.toString())); - if (mailbox.ownerType === mailboxdb.OWNER_TYPE_APP) { - var addonId = req.dn.rdns[1].attrs.ou.value.toLowerCase(); // 'sendmail' or 'recvmail' - var name; - if (addonId === 'sendmail') name = 'MAIL_SMTP_PASSWORD'; - else if (addonId === 'recvmail') name = 'MAIL_IMAP_PASSWORD'; - else return next(new ldap.OperationsError('Invalid DN')); + let name; + if (addonId === 'sendmail') name = 'MAIL_SMTP_PASSWORD'; + else if (addonId === 'recvmail') name = 'MAIL_IMAP_PASSWORD'; + else return next(new ldap.OperationsError('Invalid DN')); - appdb.getAddonConfigByName(mailbox.ownerId, addonId, name, function (error, value) { - if (error) return next(new ldap.OperationsError(error.message)); - if (req.credentials !== value) return next(new ldap.InvalidCredentialsError(req.dn.toString())); + // note: with sendmail addon, apps can send mail without a mailbox (unlike users) + appdb.getAppIdByAddonConfigValue(addonId, name, req.credentials || '', function (error, appId) { + if (error && error.reason !== DatabaseError.NOT_FOUND) return next(new ldap.OperationsError(error.message)); + if (appId) { // matched app password + eventlog.add(eventlog.ACTION_APP_LOGIN, { authType: 'ldap', mailboxId: email }, { appId: appId, addonId: addonId }); + return res.end(); + } - eventlog.add(eventlog.ACTION_APP_LOGIN, { authType: 'ldap', mailboxId: name }, { appId: mailbox.ownerId, addonId: addonId }); - return res.end(); - }); - } else if (mailbox.ownerType === mailboxdb.OWNER_TYPE_USER) { - if (!domain.enabled) return next(new ldap.NoSuchObjectError(req.dn.toString())); + mailboxdb.getMailbox(parts[0], parts[1], function (error, mailbox) { + if (error && error.reason === DatabaseError.NOT_FOUND) return next(new ldap.NoSuchObjectError(req.dn.toString())); + if (error) return next(new ldap.OperationsError(error.message)); users.verify(mailbox.ownerId, req.credentials || '', function (error, result) { if (error && error.reason === UsersError.NOT_FOUND) return next(new ldap.NoSuchObjectError(req.dn.toString())); @@ -495,9 +492,7 @@ function authenticateMailbox(req, res, next) { eventlog.add(eventlog.ACTION_USER_LOGIN, { authType: 'ldap', mailboxId: email }, { userId: result.id, user: users.removePrivateFields(result) }); res.end(); }); - } else { - return next(new ldap.OperationsError('Unknown ownerType for mailbox')); - } + }); }); }); } diff --git a/src/mail.js b/src/mail.js index 1fd47a795..3797deee6 100644 --- a/src/mail.js +++ b/src/mail.js @@ -935,7 +935,7 @@ function addMailbox(name, domain, userId, auditSource, callback) { var error = validateName(name); if (error) return callback(error); - mailboxdb.addMailbox(name, domain, userId, mailboxdb.OWNER_TYPE_USER, function (error) { + mailboxdb.addMailbox(name, domain, userId, function (error) { if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new MailError(MailError.ALREADY_EXISTS, `mailbox ${name} already exists`)); if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); @@ -953,7 +953,7 @@ function updateMailboxOwner(name, domain, userId, callback) { name = name.toLowerCase(); - mailboxdb.updateMailboxOwner(name, domain, userId, mailboxdb.OWNER_TYPE_USER, function (error) { + mailboxdb.updateMailboxOwner(name, domain, userId, function (error) { if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new MailError(MailError.NOT_FOUND, 'no such mailbox')); if (error) return callback(new MailError(MailError.INTERNAL_ERROR, error)); diff --git a/src/mailboxdb.js b/src/mailboxdb.js index 447915254..41313f58e 100644 --- a/src/mailboxdb.js +++ b/src/mailboxdb.js @@ -29,11 +29,7 @@ exports = module.exports = { TYPE_MAILBOX: 'mailbox', TYPE_LIST: 'list', - TYPE_ALIAS: 'alias', - - OWNER_TYPE_USER: 'user', - OWNER_TYPE_APP: 'app', - OWNER_TYPE_GROUP: 'group' // obsolete + TYPE_ALIAS: 'alias' }; var assert = require('assert'), @@ -42,7 +38,7 @@ var assert = require('assert'), safe = require('safetydance'), util = require('util'); -var MAILBOX_FIELDS = [ 'name', 'type', 'ownerId', 'ownerType', 'aliasTarget', 'creationTime', 'membersJson', 'domain' ].join(','); +var MAILBOX_FIELDS = [ 'name', 'type', 'ownerId', 'aliasTarget', 'creationTime', 'membersJson', 'domain' ].join(','); function postProcess(data) { data.members = safe.JSON.parse(data.membersJson) || [ ]; @@ -51,14 +47,13 @@ function postProcess(data) { return data; } -function addMailbox(name, domain, ownerId, ownerType, callback) { +function addMailbox(name, domain, ownerId, callback) { assert.strictEqual(typeof name, 'string'); assert.strictEqual(typeof domain, 'string'); assert.strictEqual(typeof ownerId, 'string'); - assert.strictEqual(typeof ownerType, 'string'); assert.strictEqual(typeof callback, 'function'); - database.query('INSERT INTO mailboxes (name, type, domain, ownerId, ownerType) VALUES (?, ?, ?, ?, ?)', [ name, exports.TYPE_MAILBOX, domain, ownerId, ownerType ], function (error) { + database.query('INSERT INTO mailboxes (name, type, domain, ownerId) VALUES (?, ?, ?, ?)', [ name, exports.TYPE_MAILBOX, domain, ownerId ], function (error) { if (error && error.code === 'ER_DUP_ENTRY') return callback(new DatabaseError(DatabaseError.ALREADY_EXISTS, 'mailbox already exists')); if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); @@ -66,14 +61,13 @@ function addMailbox(name, domain, ownerId, ownerType, callback) { }); } -function updateMailboxOwner(name, domain, ownerId, ownerType, callback) { +function updateMailboxOwner(name, domain, ownerId, callback) { assert.strictEqual(typeof name, 'string'); assert.strictEqual(typeof domain, 'string'); assert.strictEqual(typeof ownerId, 'string'); - assert.strictEqual(typeof ownerType, 'string'); assert.strictEqual(typeof callback, 'function'); - database.query('UPDATE mailboxes SET ownerId = ?, ownerType = ? WHERE name = ? AND domain = ?', [ ownerId, ownerType, name, domain ], function (error, result) { + database.query('UPDATE mailboxes SET ownerId = ? WHERE name = ? AND domain = ?', [ ownerId, name, domain ], function (error, result) { if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); if (result.affectedRows === 0) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); @@ -87,8 +81,8 @@ function addGroup(name, domain, members, callback) { assert(Array.isArray(members)); assert.strictEqual(typeof callback, 'function'); - database.query('INSERT INTO mailboxes (name, type, domain, ownerId, ownerType, membersJson) VALUES (?, ?, ?, ?, ?, ?)', - [ name, exports.TYPE_LIST, domain, 'admin', exports.OWNER_TYPE_GROUP, JSON.stringify(members) ], function (error) { + database.query('INSERT INTO mailboxes (name, type, domain, ownerId, membersJson) VALUES (?, ?, ?, ?, ?)', + [ name, exports.TYPE_LIST, domain, 'admin', JSON.stringify(members) ], function (error) { if (error && error.code === 'ER_DUP_ENTRY') return callback(new DatabaseError(DatabaseError.ALREADY_EXISTS, 'mailbox already exists')); if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); @@ -259,8 +253,8 @@ function setAliasesForName(name, domain, aliases, callback) { // clear existing aliases queries.push({ query: 'DELETE FROM mailboxes WHERE aliasTarget = ? AND domain = ? AND type = ?', args: [ name, domain, exports.TYPE_ALIAS ] }); aliases.forEach(function (alias) { - queries.push({ query: 'INSERT INTO mailboxes (name, type, domain, aliasTarget, ownerId, ownerType) VALUES (?, ?, ?, ?, ?, ?)', - args: [ alias, exports.TYPE_ALIAS, domain, name, results[0].ownerId, results[0].ownerType ] }); + queries.push({ query: 'INSERT INTO mailboxes (name, type, domain, aliasTarget, ownerId) VALUES (?, ?, ?, ?, ?)', + args: [ alias, exports.TYPE_ALIAS, domain, name, results[0].ownerId ] }); }); database.transaction(queries, function (error) { diff --git a/src/routes/test/mail-test.js b/src/routes/test/mail-test.js index 93f63ea78..3dfefe744 100644 --- a/src/routes/test/mail-test.js +++ b/src/routes/test/mail-test.js @@ -761,7 +761,6 @@ describe('Mail API', function () { expect(res.body.mailbox).to.be.an('object'); expect(res.body.mailbox.name).to.equal(MAILBOX_NAME); expect(res.body.mailbox.ownerId).to.equal(userId); - expect(res.body.mailbox.ownerType).to.equal('user'); expect(res.body.mailbox.aliasTarget).to.equal(null); expect(res.body.mailbox.domain).to.equal(DOMAIN_0.domain); done(); @@ -777,7 +776,6 @@ describe('Mail API', function () { expect(res.body.mailboxes[0]).to.be.an('object'); expect(res.body.mailboxes[0].name).to.equal(MAILBOX_NAME); expect(res.body.mailboxes[0].ownerId).to.equal(userId); - expect(res.body.mailboxes[0].ownerType).to.equal('user'); expect(res.body.mailboxes[0].aliasTarget).to.equal(null); expect(res.body.mailboxes[0].domain).to.equal(DOMAIN_0.domain); done(); @@ -910,12 +908,10 @@ describe('Mail API', function () { expect(res.body.aliases.length).to.eql(2); expect(res.body.aliases[0].name).to.equal('hello'); expect(res.body.aliases[0].ownerId).to.equal(userId); - expect(res.body.aliases[0].ownerType).to.equal('user'); expect(res.body.aliases[0].aliasTarget).to.equal(MAILBOX_NAME); expect(res.body.aliases[0].domain).to.equal(DOMAIN_0.domain); expect(res.body.aliases[1].name).to.equal('there'); expect(res.body.aliases[1].ownerId).to.equal(userId); - expect(res.body.aliases[1].ownerType).to.equal('user'); expect(res.body.aliases[1].aliasTarget).to.equal(MAILBOX_NAME); expect(res.body.aliases[1].domain).to.equal(DOMAIN_0.domain); done(); @@ -1027,7 +1023,6 @@ describe('Mail API', function () { expect(res.body.list).to.be.an('object'); expect(res.body.list.name).to.equal(LIST_NAME); expect(res.body.list.ownerId).to.equal('admin'); - expect(res.body.list.ownerType).to.equal('group'); expect(res.body.list.aliasTarget).to.equal(null); expect(res.body.list.domain).to.equal(DOMAIN_0.domain); expect(res.body.list.members).to.eql([ 'admin2', 'superadmin' ]); @@ -1044,7 +1039,6 @@ describe('Mail API', function () { expect(res.body.lists.length).to.equal(1); expect(res.body.lists[0].name).to.equal(LIST_NAME); expect(res.body.lists[0].ownerId).to.equal('admin'); - expect(res.body.lists[0].ownerType).to.equal('group'); expect(res.body.lists[0].aliasTarget).to.equal(null); expect(res.body.lists[0].domain).to.equal(DOMAIN_0.domain); expect(res.body.lists[0].members).to.eql([ 'admin2', 'superadmin' ]); diff --git a/src/test/database-test.js b/src/test/database-test.js index 4564eb44f..7949734c0 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -232,7 +232,8 @@ describe('database', function () { robotsTxt: null, enableBackup: true, ownerId: USER_0.id, - env: {} + env: {}, + mailboxName: 'talktome' }; it('cannot delete referenced domain', function (done) { @@ -753,7 +754,8 @@ describe('database', function () { alternateDomains: [], env: { 'CUSTOM_KEY': 'CUSTOM_VALUE' - } + }, + mailboxName: 'talktome' }; var APP_1 = { @@ -781,7 +783,8 @@ describe('database', function () { enableBackup: true, ownerId: USER_0.id, alternateDomains: [], - env: {} + env: {}, + mailboxName: 'callme' }; before(function (done) { @@ -912,7 +915,7 @@ describe('database', function () { }); it('getAll succeeds', function (done) { - appdb.list(function (error, result) { + appdb.getAll(function (error, result) { expect(error).to.be(null); expect(result).to.be.an(Array); expect(result.length).to.be(2); @@ -1632,21 +1635,21 @@ describe('database', function () { }); it('add user mailbox succeeds', function (done) { - mailboxdb.addMailbox('girish', DOMAIN_0.domain, 'uid-0', mailboxdb.OWNER_TYPE_USER, function (error) { + mailboxdb.addMailbox('girish', DOMAIN_0.domain, 'uid-0', function (error) { expect(error).to.be(null); done(); }); }); it('cannot add dup entry', function (done) { - mailboxdb.addMailbox('girish', DOMAIN_0.domain, 'uid-1', mailboxdb.OWNER_TYPE_APP, function (error) { + mailboxdb.addMailbox('girish', DOMAIN_0.domain, 'uid-1', function (error) { expect(error.reason).to.be(DatabaseError.ALREADY_EXISTS); done(); }); }); it('add app mailbox succeeds', function (done) { - mailboxdb.addMailbox('support', DOMAIN_0.domain, 'osticket', mailboxdb.OWNER_TYPE_APP, function (error) { + mailboxdb.addMailbox('support', DOMAIN_0.domain, 'osticket', function (error) { expect(error).to.be(null); done(); }); @@ -1669,7 +1672,6 @@ describe('database', function () { expect(error).to.be(null); expect(mailboxes.length).to.be(2); expect(mailboxes[0].name).to.be('girish'); - expect(mailboxes[0].ownerType).to.be(mailboxdb.OWNER_TYPE_USER); expect(mailboxes[1].name).to.be('support'); done(); @@ -1815,7 +1817,7 @@ describe('database', function () { }); it('can get all domains', function (done) { - maildb.getAll(function (error, result) { + maildb.list(function (error, result) { expect(error).to.equal(null); expect(result).to.be.an(Array); expect(result[0]).to.be.an('object'); diff --git a/src/test/ldap-test.js b/src/test/ldap-test.js index 4837d9283..884680cc0 100644 --- a/src/test/ldap-test.js +++ b/src/test/ldap-test.js @@ -79,7 +79,8 @@ var APP_0 = { restoreConfig: null, oldConfig: null, memoryLimit: 4294967296, - ownerId: null + ownerId: null, + mailboxName: 'some-location-0.app' }; var dockerProxy; @@ -112,7 +113,7 @@ function setup(done) { appdb.update.bind(null, APP_0.id, { containerId: APP_0.containerId }), appdb.setAddonConfig.bind(null, APP_0.id, 'sendmail', [{ name: 'MAIL_SMTP_PASSWORD', value : 'sendmailpassword' }]), appdb.setAddonConfig.bind(null, APP_0.id, 'recvmail', [{ name: 'MAIL_IMAP_PASSWORD', value : 'recvmailpassword' }]), - mailboxdb.addMailbox.bind(null, APP_0.location + '.app', APP_0.domain, APP_0.id, mailboxdb.OWNER_TYPE_APP), + mailboxdb.addMailbox.bind(null, APP_0.location + '.app', APP_0.domain, APP_0.id), function (callback) { users.create(USER_1.username, USER_1.password, USER_1.email, USER_0.displayName, { invitor: USER_0 }, AUDIT_SOURCE, function (error, result) { @@ -812,7 +813,7 @@ describe('Ldap', function () { describe('search mailbox', function () { before(function (done) { - mailboxdb.addMailbox(USER_0.username.toLowerCase(), DOMAIN_0.domain, USER_0.id, mailboxdb.OWNER_TYPE_USER, done); + mailboxdb.addMailbox(USER_0.username.toLowerCase(), DOMAIN_0.domain, USER_0.id, done); }); it('get specific mailbox by email', function (done) { @@ -923,7 +924,7 @@ describe('Ldap', function () { var client = ldap.createClient({ url: 'ldap://127.0.0.1:' + config.get('ldapPort') }); client.bind('cn=' + USER_0.username + '@example.com,ou=sendmail,dc=cloudron', USER_0.password + 'nope', function (error) { - expect(error).to.be.a(ldap.NoSuchObjectError); + expect(error).to.be.a(ldap.InvalidCredentialsError); client.unbind(done); }); }); @@ -991,7 +992,7 @@ describe('Ldap', function () { var client = ldap.createClient({ url: 'ldap://127.0.0.1:' + config.get('ldapPort') }); client.bind('cn=' + APP_0.location + '.app@example.com,ou=sendmail,dc=cloudron', 'nope', function (error) { - expect(error).to.be.a(ldap.InvalidCredentialsError); + expect(error).to.be.a(ldap.NoSuchObjectError); client.unbind(done); }); }); @@ -1082,7 +1083,7 @@ describe('Ldap', function () { var client = ldap.createClient({ url: 'ldap://127.0.0.1:' + config.get('ldapPort') }); client.bind('cn=' + APP_0.location + '.app@example.com,ou=recvmail,dc=cloudron', 'nope', function (error) { - expect(error).to.be.a(ldap.InvalidCredentialsError); + expect(error).to.be.a(ldap.NoSuchObjectError); client.unbind(done); }); });