users: asyncify and merge userdb.del

This commit is contained in:
Girish Ramakrishnan
2021-06-26 09:57:07 -07:00
parent 147c8df6e3
commit e7d9af5aed
6 changed files with 80 additions and 139 deletions
+7 -7
View File
@@ -5,7 +5,7 @@ exports = module.exports = {
update,
list,
create,
remove,
del,
changePassword,
verifyPassword,
createInvite,
@@ -18,11 +18,12 @@ exports = module.exports = {
load
};
var assert = require('assert'),
const assert = require('assert'),
auditSource = require('../auditsource.js'),
BoxError = require('../boxerror.js'),
HttpError = require('connect-lastmile').HttpError,
HttpSuccess = require('connect-lastmile').HttpSuccess,
safe = require('safetydance'),
users = require('../users.js');
function load(req, res, next) {
@@ -117,17 +118,16 @@ function get(req, res, next) {
next(new HttpSuccess(200, users.removePrivateFields(req.resource)));
}
function remove(req, res, next) {
async function del(req, res, next) {
assert.strictEqual(typeof req.resource, 'object');
if (req.user.id === req.resource.id) return next(new HttpError(409, 'Not allowed to remove yourself.'));
if (users.compareRoles(req.user.role, req.resource.role) < 0) return next(new HttpError(403, `role '${req.resource.role}' is required but user has only '${req.user.role}'`));
users.remove(req.resource, auditSource.fromRequest(req), function (error) {
if (error) return next(BoxError.toHttpError(error));
const [error] = await safe(users.remove(req.resource, auditSource.fromRequest(req)));
if (error) return next(BoxError.toHttpError(error));
next(new HttpSuccess(204));
});
next(new HttpSuccess(204));
}
function verifyPassword(req, res, next) {
+1 -1
View File
@@ -171,7 +171,7 @@ function initializeExpressSync() {
router.get ('/api/v1/users', token, authorizeUserManager, routes.users.list);
router.post('/api/v1/users', json, token, authorizeUserManager, routes.users.create);
router.get ('/api/v1/users/:userId', token, authorizeUserManager, routes.users.load, routes.users.get); // this is manage scope because it returns non-restricted fields
router.del ('/api/v1/users/:userId', token, authorizeUserManager, routes.users.load, routes.users.remove);
router.del ('/api/v1/users/:userId', token, authorizeUserManager, routes.users.load, routes.users.del);
router.post('/api/v1/users/:userId', json, token, authorizeUserManager, routes.users.load, routes.users.update);
router.post('/api/v1/users/:userId/password', json, token, authorizeUserManager, routes.users.load, routes.users.changePassword);
router.put ('/api/v1/users/:userId/groups', json, token, authorizeUserManager, routes.users.load, routes.users.setGroups);
+52 -2
View File
@@ -10,7 +10,8 @@ const appPasswords = require('../apppasswords.js'),
BoxError = require('../boxerror.js'),
common = require('./common.js'),
expect = require('expect.js'),
safe = require('safetydance');
safe = require('safetydance'),
users = require('../users.js');
describe('App passwords', function () {
const { setup, cleanup, ADMIN } = common;
@@ -18,7 +19,7 @@ describe('App passwords', function () {
before(setup);
after(cleanup);
let id;
let id, password;
it('cannot add bad app password', async function () {
const [error] = await safe(appPasswords.add(ADMIN.id, 'appid', 'x'.repeat(201)));
expect(error.reason).to.be(BoxError.BAD_FIELD);
@@ -29,6 +30,7 @@ describe('App passwords', function () {
expect(result.id).to.be.a('string');
expect(result.password).to.be.a('string');
id = result.id;
password = result.password;
});
it('can get app password', async function () {
@@ -51,10 +53,58 @@ describe('App passwords', function () {
expect(results[0].identifier).to.be('appid');
});
it('can verify app password', function (done) {
users.verify(ADMIN.id, password, 'appid', function (error, result) {
expect(error).to.not.be.ok();
expect(result).to.be.ok();
expect(result.appPassword).to.be(true);
done();
});
});
it('can verify non-app password', function (done) {
users.verify(ADMIN.id, ADMIN.password, 'appid', function (error, result) {
expect(error).to.not.be.ok();
expect(result).to.be.ok();
expect(result.appPassword).to.be(undefined);
done();
});
});
it('cannot verify bad password', function (done) {
users.verify(ADMIN.id, 'bad', 'appid', function (error, result) {
expect(error).to.be.ok();
expect(result).to.not.be.ok();
expect(error.reason).to.be(BoxError.INVALID_CREDENTIALS);
done();
});
});
it('cannot verify password for another app', function (done) {
users.verify(ADMIN.id, password, 'appid2', function (error, result) {
expect(error).to.be.ok();
expect(result).to.not.be.ok();
expect(error.reason).to.be(BoxError.INVALID_CREDENTIALS);
done();
});
});
it('can del app password', async function () {
await appPasswords.del(id);
});
it('cannot verify deleted app password', function (done) {
users.verify(ADMIN.id, password, 'appid', function (error) {
expect(error).to.be.ok();
done();
});
});
it('cannot del random app password', async function () {
const [error] = await safe(appPasswords.del('random'));
expect(error.reason).to.be(BoxError.NOT_FOUND);
+7 -99
View File
@@ -5,7 +5,7 @@
'use strict';
var async = require('async'),
const async = require('async'),
BoxError = require('../boxerror.js'),
database = require('../database.js'),
expect = require('expect.js'),
@@ -16,6 +16,7 @@ var async = require('async'),
mailer = require('../mailer.js'),
paths = require('../paths.js'),
provision = require('../provision.js'),
safe = require('safetydance'),
userdb = require('../userdb.js'),
users = require('../users.js'),
_ = require('underscore');
@@ -530,94 +531,6 @@ describe('User', function () {
});
});
describe('appPasswords', function () {
before(createOwner);
after(cleanupUsers);
let pwd;
it('can add app password', function (done) {
users.addAppPassword(userObject.id, 'appid', 'rpi', function (error, result) {
expect(error).to.be(null);
pwd = result;
done();
});
});
it('can get app passwords', function (done) {
users.getAppPasswords(userObject.id, function (error, result) {
expect(error).to.be(null);
expect(result.length).to.be(1);
expect(result[0].name).to.be('rpi');
expect(result[0].identifier).to.be('appid');
expect(result[0].hashedPassword).to.be(undefined);
done();
});
});
it('can get app password', function (done) {
users.getAppPassword(pwd.id, function (error, result) {
expect(error).to.be(null);
expect(result.name).to.be('rpi');
expect(result.identifier).to.be('appid');
expect(result.hashedPassword).to.be(undefined);
done();
});
});
it('can verify app password', function (done) {
users.verify(userObject.id, pwd.password, 'appid', function (error, result) {
expect(error).to.not.be.ok();
expect(result).to.be.ok();
expect(result.appPassword).to.be(true);
done();
});
});
it('can verify non-app password', function (done) {
users.verify(userObject.id, PASSWORD, 'appid', function (error, result) {
expect(error).to.not.be.ok();
expect(result).to.be.ok();
expect(result.appPassword).to.be(undefined);
done();
});
});
it('cannot verify bad password', function (done) {
users.verify(userObject.id, 'bad', 'appid', function (error, result) {
expect(error).to.be.ok();
expect(result).to.not.be.ok();
expect(error.reason).to.be(BoxError.INVALID_CREDENTIALS);
done();
});
});
it('cannot verify password for another app', function (done) {
users.verify(userObject.id, pwd.password, 'appid2', function (error, result) {
expect(error).to.be.ok();
expect(result).to.not.be.ok();
expect(error.reason).to.be(BoxError.INVALID_CREDENTIALS);
done();
});
});
it('can del app password', function (done) {
users.delAppPassword(pwd.id, function (error) {
if (error) return done(error);
// cannot verify anymore
users.verify(userObject.id, pwd.password, 'appid', function (error) {
expect(error).to.be.ok();
done();
});
});
});
});
describe('retrieving', function () {
before(createOwner);
after(cleanupUsers);
@@ -909,18 +822,13 @@ describe('User', function () {
before(createOwner);
after(cleanupUsers);
it('fails for unknown user', function (done) {
users.remove(_.extend({}, userObject, { id: 'unknown' }), AUDIT_SOURCE, function (error) {
expect(error.reason).to.be(BoxError.NOT_FOUND);
done();
});
it('fails for unknown user', async function () {
const [error] = await safe(users.del(_.extend({}, userObject, { id: 'unknown' }), AUDIT_SOURCE));
expect(error.reason).to.be(BoxError.NOT_FOUND);
});
it('can remove valid user', function (done) {
users.remove(userObject, AUDIT_SOURCE, function (error) {
expect(!error).to.be.ok();
done();
});
it('can remove valid user', async function () {
await users.del(userObject, AUDIT_SOURCE);
});
it('can re-create user after user was removed', createOwner);
-21
View File
@@ -10,7 +10,6 @@ exports = module.exports = {
getAllWithGroupIdsPaged,
getByRole,
add,
del,
update,
count,
@@ -182,26 +181,6 @@ function add(userId, user, callback) {
});
}
function del(userId, callback) {
assert.strictEqual(typeof userId, 'string');
assert.strictEqual(typeof callback, 'function');
// also cleanup the groupMembers table
var queries = [];
queries.push({ query: 'DELETE FROM groupMembers WHERE userId = ?', args: [ userId ] });
queries.push({ query: 'DELETE FROM tokens WHERE identifier = ?', args: [ userId ] });
queries.push({ query: 'DELETE FROM appPasswords WHERE userId = ?', args: [ userId ] });
queries.push({ query: 'DELETE FROM users WHERE id = ?', args: [ userId ] });
database.transaction(queries, function (error, result) {
if (error && error.code === 'ER_NO_REFERENCED_ROW_2') return callback(new BoxError(BoxError.NOT_FOUND, error));
if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error));
if (result[3].affectedRows !== 1) return callback(new BoxError(BoxError.NOT_FOUND, 'User not found'));
callback(error);
});
}
function getByAccessToken(accessToken, callback) {
assert.strictEqual(typeof accessToken, 'string');
assert.strictEqual(typeof callback, 'function');
+13 -9
View File
@@ -11,7 +11,7 @@ exports = module.exports = {
verify,
verifyWithUsername,
verifyWithEmail,
remove,
del,
get,
getByResetToken,
getByUsername,
@@ -314,20 +314,24 @@ function verifyWithEmail(email, password, identifier, callback) {
});
}
function remove(user, auditSource, callback) {
async function del(user, auditSource) {
assert.strictEqual(typeof user, 'object');
assert(auditSource && typeof auditSource === 'object');
assert.strictEqual(typeof callback, 'function');
if (settings.isDemo() && user.username === constants.DEMO_USERNAME) return callback(new BoxError(BoxError.BAD_FIELD, 'Not allowed in demo mode'));
if (settings.isDemo() && user.username === constants.DEMO_USERNAME) throw new BoxError(BoxError.BAD_FIELD, 'Not allowed in demo mode');
userdb.del(user.id, function (error) {
if (error) return callback(error);
const queries = [];
queries.push({ query: 'DELETE FROM groupMembers WHERE userId = ?', args: [ user.id ] });
queries.push({ query: 'DELETE FROM tokens WHERE identifier = ?', args: [ user.id ] });
queries.push({ query: 'DELETE FROM appPasswords WHERE userId = ?', args: [ user.id ] });
queries.push({ query: 'DELETE FROM users WHERE id = ?', args: [ user.id ] });
eventlog.add(eventlog.ACTION_USER_REMOVE, auditSource, { userId: user.id, user: removePrivateFields(user) });
const [error, result] = await safe(database.transaction(queries));
if (error && error.code === 'ER_NO_REFERENCED_ROW_2') throw new BoxError(BoxError.NOT_FOUND, error);
if (error) throw error;
if (result[3].affectedRows !== 1) throw new BoxError(BoxError.NOT_FOUND, 'User not found');
callback();
});
await safe(eventlog.add(eventlog.ACTION_USER_REMOVE, auditSource, { userId: user.id, user: removePrivateFields(user) }));
}
function getAll(callback) {