diff --git a/src/mailboxdb.js b/src/mailboxdb.js index 7150583a9..66db78243 100644 --- a/src/mailboxdb.js +++ b/src/mailboxdb.js @@ -208,7 +208,7 @@ function getAlias(name, callback) { assert.strictEqual(typeof name, 'string'); assert.strictEqual(typeof callback, 'function'); - database.query('SELECT ' + MAILBOX_FIELDS + ' FROM mailboxes WHERE name = ? AND aliasTarget IS NOT null', [ name ], function (error, results) { + database.query('SELECT ' + MAILBOX_FIELDS + ' FROM mailboxes WHERE name = ? AND aliasTarget IS NOT NULL', [ name ], function (error, results) { if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); if (results.length === 0) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); diff --git a/src/mailer.js b/src/mailer.js index 2e25aba0d..cdd7d6ad8 100644 --- a/src/mailer.js +++ b/src/mailer.js @@ -232,7 +232,7 @@ function mailUserEventToAdmins(user, event) { assert.strictEqual(typeof event, 'string'); getAdminEmails(function (error, adminEmails) { - if (error) return console.log('Error getting admins', error); + if (error) return debug('Error getting admins', error); adminEmails = _.difference(adminEmails, [ user.email ]); diff --git a/src/test/database-test.js b/src/test/database-test.js index f05cf969d..02ed2b8f0 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -1340,6 +1340,13 @@ describe('database', function () { }); describe('mailboxes', function () { + before(function (done) { + async.series([ + database.initialize, + database._clear + ], done); + }); + it('add user mailbox succeeds', function (done) { mailboxdb.add('girish', 'uid-0', mailboxdb.TYPE_USER, function (error, mailbox) { expect(error).to.be(null); diff --git a/src/test/user-test.js b/src/test/user-test.js index 34da69237..37f6643d6 100644 --- a/src/test/user-test.js +++ b/src/test/user-test.js @@ -1008,7 +1008,7 @@ describe('User', function () { it('can remove valid user', function (done) { user.remove(userObject.id, { }, function (error) { - expect(error).to.be(null); + expect(!error).to.be.ok(); done(); }); }); diff --git a/src/user.js b/src/user.js index 4fbee320f..b3fac24ed 100644 --- a/src/user.js +++ b/src/user.js @@ -52,12 +52,6 @@ var CRYPTO_ITERATIONS = 10000; // iterations var CRYPTO_KEY_LENGTH = 512; // bits var CRYPTO_DIGEST = 'sha1'; // used to be the default in node 4.1.1 cannot change since it will affect existing db records -function asyncIf(cond, func, next) { - if (!cond) return next(); - - func(next); -} - // http://dustinsenos.com/articles/customErrorsInNode // http://code.google.com/p/v8/wiki/JavaScriptStackTraceApi function UserError(reason, errorOrMessage) { @@ -179,31 +173,26 @@ function createUser(username, password, email, displayName, auditSource, options displayName: displayName }; - asyncIf(!!username, mailboxdb.add.bind(null, username, user.id /* owner */, mailboxdb.TYPE_USER), function (error) { + userdb.add(user.id, user, function (error) { if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new UserError(UserError.ALREADY_EXISTS, error.message)); if (error) return callback(new UserError(UserError.INTERNAL_ERROR, error)); - userdb.add(user.id, user, function (error) { - if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new UserError(UserError.ALREADY_EXISTS, error.message)); + settings.getMailConfig(function (error, mailConfig) { if (error) return callback(new UserError(UserError.INTERNAL_ERROR, error)); - settings.getMailConfig(function (error, mailConfig) { - if (error) return callback(new UserError(UserError.INTERNAL_ERROR, error)); + if (mailConfig.enabled) { + user.alternateEmail = user.email; + user.email = user.username ? user.username + '@' + config.fqdn() : null; + } else { + user.alternateEmail = null; + } - if (mailConfig.enabled) { - user.alternateEmail = user.email; - user.email = user.username ? user.username + '@' + config.fqdn() : null; - } else { - user.alternateEmail = null; - } + callback(null, user); - callback(null, user); + eventlog.add(eventlog.ACTION_USER_ADD, auditSource, { userId: user.id, email: user.email }); - eventlog.add(eventlog.ACTION_USER_ADD, auditSource, { userId: user.id, email: user.email }); - - if (!owner) mailer.userAdded(user, sendInvite); - if (sendInvite) mailer.sendInvite(user, invitor); - }); + if (!owner) mailer.userAdded(user, sendInvite); + if (sendInvite) mailer.sendInvite(user, invitor); }); }); }); @@ -298,7 +287,7 @@ function removeUser(userId, auditSource, callback) { eventlog.add(eventlog.ACTION_USER_REMOVE, auditSource, { userId: userId }); - asyncIf(!!user.username, mailboxdb.delByOwnerId.bind(null, user.id), callback); + callback(); mailer.userRemoved(user); }); @@ -408,25 +397,14 @@ function updateUser(userId, data, auditSource, callback) { if (error) return callback(error); } - userdb.get(userId, function (error, user) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new UserError(UserError.NOT_FOUND)); + userdb.update(userId, data, function (error) { + if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new UserError(UserError.ALREADY_EXISTS, error.message)); + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new UserError(UserError.NOT_FOUND, error)); if (error) return callback(new UserError(UserError.INTERNAL_ERROR, error)); - asyncIf(data.username && user.username !== data.username, mailboxdb.add.bind(null, data.username, userId /* owner */, mailboxdb.TYPE_USER), function (error) { - if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new UserError(UserError.ALREADY_EXISTS, error.message)); - if (error) return callback(new UserError(UserError.INTERNAL_ERROR, error)); + eventlog.add(eventlog.ACTION_USER_UPDATE, auditSource, { userId: userId }); - userdb.update(userId, data, function (error) { - if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new UserError(UserError.ALREADY_EXISTS, error.message)); - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new UserError(UserError.NOT_FOUND, error)); - if (error) return callback(new UserError(UserError.INTERNAL_ERROR, error)); - - eventlog.add(eventlog.ACTION_USER_UPDATE, auditSource, { userId: userId }); - - // delete old mailbox - asyncIf(data.username && user.username && user.username !== data.username, mailboxdb.del.bind(null, user.username), callback); - }); - }); + callback(); }); } diff --git a/src/userdb.js b/src/userdb.js index 2447c449c..337b366d4 100644 --- a/src/userdb.js +++ b/src/userdb.js @@ -21,7 +21,8 @@ var assert = require('assert'), constants = require('./constants.js'), database = require('./database.js'), debug = require('debug')('box:userdb'), - DatabaseError = require('./databaseerror'); + DatabaseError = require('./databaseerror'), + mailboxdb = require('./mailboxdb.js'); var USERS_FIELDS = [ 'id', 'username', 'email', 'password', 'salt', 'createdAt', 'modifiedAt', 'resetToken', 'displayName' ].join(','); @@ -136,8 +137,19 @@ function add(userId, user, callback) { assert.strictEqual(typeof user.displayName, 'string'); assert.strictEqual(typeof callback, 'function'); - var data = [ userId, user.username, user.password, user.email, user.salt, user.createdAt, user.modifiedAt, user.resetToken, user.displayName ]; - database.query('INSERT INTO users (id, username, password, email, salt, createdAt, modifiedAt, resetToken, displayName) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)', data, function (error, result) { + var queries = []; + queries.push({ + query: 'INSERT INTO users (id, username, password, email, salt, createdAt, modifiedAt, resetToken, displayName) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)', + args: [ userId, user.username, user.password, user.email, user.salt, user.createdAt, user.modifiedAt, user.resetToken, user.displayName ] + }); + if (user.username) { + queries.push({ + query: 'INSERT INTO mailboxes (name, ownerId, ownerType) VALUES (?, ?, ?)', + args: [ user.username, userId, mailboxdb.TYPE_USER ] + }); + } + + database.transaction(queries, function (error, result) { if (error && error.code === 'ER_DUP_ENTRY') { var msg = error.message; if (error.message.indexOf('users_email') !== -1) { @@ -148,7 +160,7 @@ function add(userId, user, callback) { return callback(new DatabaseError(DatabaseError.ALREADY_EXISTS, msg)); } - if (error || result.affectedRows !== 1) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); + if (error || result[0].affectedRows !== 1) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); callback(null); }); @@ -162,6 +174,7 @@ function del(userId, callback) { var queries = []; queries.push({ query: 'DELETE FROM groupMembers WHERE userId = ?', args: [ userId ] }); queries.push({ query: 'DELETE FROM users WHERE id = ?', args: [ userId ] }); + queries.push({ query: 'DELETE FROM mailboxes WHERE ownerId=?', args: [ userId ] }); database.transaction(queries, function (error, result) { if (error && error.code === 'ER_NO_REFERENCED_ROW_2') return callback(new DatabaseError(DatabaseError.NOT_FOUND, error)); @@ -216,7 +229,13 @@ function update(userId, user, callback) { } args.push(userId); - database.query('UPDATE users SET ' + fields.join(', ') + ' WHERE id = ?', args, function (error, result) { + var queries = []; + queries.push({ query: 'UPDATE users SET ' + fields.join(', ') + ' WHERE id = ?', args: args }); + if (user.username) { + queries.push({ query: 'UPDATE mailboxes SET name = ? WHERE ownerId = ? AND aliasTarget IS NULL' , args: [ user.username, userId ] }); + } + + database.transaction(queries, function (error, result) { if (error && error.code === 'ER_DUP_ENTRY') { var msg = error.message; if (error.message.indexOf('users_email') !== -1) { @@ -228,7 +247,7 @@ function update(userId, user, callback) { return callback(new DatabaseError(DatabaseError.ALREADY_EXISTS, msg)); } if (error) return callback(new DatabaseError(DatabaseError.INTERNAL_ERROR, error)); - if (result.affectedRows !== 1) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); + if (result[0].affectedRows !== 1) return callback(new DatabaseError(DatabaseError.NOT_FOUND)); return callback(null); });