diff --git a/src/routes/test/user-test.js b/src/routes/test/user-test.js index 28a15e3fe..db8dab857 100644 --- a/src/routes/test/user-test.js +++ b/src/routes/test/user-test.js @@ -467,6 +467,16 @@ describe('User API', function () { }); }); + it('remove random user fails', function (done) { + superagent.del(SERVER_URL + '/api/v1/users/randomid') + .query({ access_token: token }) + .send({ password: PASSWORD }) + .end(function (err, res) { + expect(res.statusCode).to.equal(404); + done(); + }); + }); + it('user removes himself is not allowed', function (done) { superagent.del(SERVER_URL + '/api/v1/users/' + user_0.id) .query({ access_token: token }) diff --git a/src/routes/user.js b/src/routes/user.js index 9d6cbc9fe..ab6c7d79e 100644 --- a/src/routes/user.js +++ b/src/routes/user.js @@ -125,16 +125,11 @@ function remove(req, res, next) { if (req.user.id === req.params.userId) return next(new HttpError(403, 'Not allowed to remove yourself.')); - user.get(req.params.userId, function (error, userObject) { + user.remove(req.params.userId, auditSource(req), function (error) { if (error && error.reason === UserError.NOT_FOUND) return next(new HttpError(404, 'No such user')); if (error) return next(new HttpError(500, error)); - user.remove(userObject, auditSource(req), function (error) { - if (error && error.reason === UserError.NOT_FOUND) return next(new HttpError(404, 'No such user')); - if (error) return next(new HttpError(500, error)); - - next(new HttpSuccess(204)); - }); + next(new HttpSuccess(204)); }); } diff --git a/src/test/user-test.js b/src/test/user-test.js index dfae1b206..433e24cbe 100644 --- a/src/test/user-test.js +++ b/src/test/user-test.js @@ -1,4 +1,3 @@ -/* jslint node:true */ /* global it:false */ /* global describe:false */ /* global before:false */ @@ -678,4 +677,23 @@ describe('User', function () { }); }); }); + + describe('remove', function () { + before(createOwner); + after(cleanupUsers); + + it('fails for unkown user', function (done) { + user.remove('unknown', { }, function (error) { + expect(error.reason).to.be(UserError.NOT_FOUND); + done(); + }); + }); + + it('can remove valid user', function (done) { + user.remove(userObject.id, { }, function (error) { + expect(error).to.be(null); + done(); + }); + }); + }); }); diff --git a/src/user.js b/src/user.js index e36af2b03..00b34264e 100644 --- a/src/user.js +++ b/src/user.js @@ -251,21 +251,25 @@ function verifyWithEmail(email, password, callback) { }); } -function removeUser(user, auditSource, callback) { - assert.strictEqual(typeof user, 'object'); +function removeUser(userId, auditSource, callback) { + assert.strictEqual(typeof userId, 'string'); assert.strictEqual(typeof auditSource, 'object'); assert.strictEqual(typeof callback, 'function'); - userdb.del(user.id, function (error) { - if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new UserError(UserError.NOT_FOUND)); - if (error) return callback(new UserError(UserError.INTERNAL_ERROR, error)); + getUser(userId, function (error, user) { + if (error) return callback(error); - eventlog.add(eventlog.ACTION_USER_REMOVE, auditSource, { userId: user.id }); - if (user.username) mailboxes.del(user.username, NOOP_CALLBACK); + userdb.del(userId, function (error) { + if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new UserError(UserError.NOT_FOUND)); + if (error) return callback(new UserError(UserError.INTERNAL_ERROR, error)); - callback(null); + eventlog.add(eventlog.ACTION_USER_REMOVE, auditSource, { userId: userId }); + if (user.username) mailboxes.del(user.username, NOOP_CALLBACK); - mailer.userRemoved(user); + callback(null); + + mailer.userRemoved(user); + }); }); }