diff --git a/CHANGES b/CHANGES index 0208a510e..5e71740e0 100644 --- a/CHANGES +++ b/CHANGES @@ -1784,4 +1784,5 @@ * Add UI to import backups * Display timestamps in browser timezone in the UI * Mail eventlog +* Add permission where a user can just invite other users diff --git a/migrations/20200214061201-user-add-permissionsJson.js b/migrations/20200214061201-user-add-permissionsJson.js new file mode 100644 index 000000000..97164d0ad --- /dev/null +++ b/migrations/20200214061201-user-add-permissionsJson.js @@ -0,0 +1,20 @@ +'use strict'; + +var async = require('async'); + +exports.up = function(db, callback) { + db.runSql('ALTER TABLE users ADD COLUMN permissionsJson TEXT', function (error) { + if (error) return callback(error); + + callback(); + }); +}; + +exports.down = function(db, callback) { + db.runSql('ALTER TABLE users DROP COLUMN permissionsJson', function (error) { + if (error) console.error(error); + + callback(error); + }); +}; + diff --git a/migrations/schema.sql b/migrations/schema.sql index f209c4ee1..b31a20c11 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -28,6 +28,7 @@ CREATE TABLE IF NOT EXISTS users( twoFactorAuthenticationEnabled BOOLEAN DEFAULT false, admin BOOLEAN DEFAULT false, source VARCHAR(128) DEFAULT "", + permissionsJson TEXT, PRIMARY KEY(id)); diff --git a/src/accesscontrol.js b/src/accesscontrol.js index 528647756..b77315d03 100644 --- a/src/accesscontrol.js +++ b/src/accesscontrol.js @@ -1,10 +1,12 @@ 'use strict'; exports = module.exports = { - ROLE_ADMIN: 'admin', + PERMISSION_ADMIN: 'admin', // not a real permission, but a role + PERMISSION_MANAGE_USERS: 'manange_users', verifyToken: verifyToken, - hasRole: hasRole + hasPermission: hasPermission, + validatePermissions: validatePermissions }; var assert = require('assert'), @@ -12,13 +14,26 @@ var assert = require('assert'), tokendb = require('./tokendb.js'), users = require('./users.js'); -function hasRole(user, requiredRole) { +function hasPermission(user, requiredPermission) { assert.strictEqual(typeof user, 'object'); - assert.strictEqual(typeof requiredRole, 'string'); + assert.strictEqual(typeof requiredPermission, 'string'); - if (requiredRole === exports.ROLE_ADMIN && user.admin) return null; + if (user.admin) return null; + if (user.permissions && user.permissions.includes(requiredPermission)) return null; - return new BoxError(BoxError.ACCESS_DENIED, 'Not allowed'); + return new BoxError(BoxError.ACCESS_DENIED, 'Not permitted'); +} + +function validatePermissions(permissions) { + assert(permissions === null || Array.isArray(permissions)); + + if (permissions === null || permissions.length === 0) return null; + if (permissions.length === 1 && permissions[0] === exports.PERMISSION_MANAGE_USERS) return null; + + // here for completeness + if (permissions.includes(exports.PERMISSSION_ADMIN)) return new BoxError(BoxError.BAD_FIELD, 'admin is not a permission'); + + return new BoxError(BoxError.BAD_FIELD, 'Invalid permissions'); } function verifyToken(accessToken, callback) { diff --git a/src/routes/accesscontrol.js b/src/routes/accesscontrol.js index fabfce3ae..035c0631b 100644 --- a/src/routes/accesscontrol.js +++ b/src/routes/accesscontrol.js @@ -105,13 +105,13 @@ function tokenAuth(req, res, next) { }); } -function authorize(requiredRole) { - assert.strictEqual(typeof requiredRole, 'string'); +function authorize(requiredPermission) { + assert.strictEqual(typeof requiredPermission, 'string'); return function (req, res, next) { assert.strictEqual(typeof req.user, 'object'); - var error = accesscontrol.hasRole(req.user, requiredRole); + var error = accesscontrol.hasPermission(req.user, requiredPermission); if (error) return next(new HttpError(403, error.message)); next(); diff --git a/src/routes/profile.js b/src/routes/profile.js index a60af3760..98c9666e2 100644 --- a/src/routes/profile.js +++ b/src/routes/profile.js @@ -39,6 +39,7 @@ function get(req, res, next) { twoFactorAuthenticationEnabled: req.user.twoFactorAuthenticationEnabled, admin: req.user.admin, source: req.user.source, + permissions: req.user.permissions, avatarUrl: fs.existsSync(path.join(paths.PROFILE_ICONS_DIR, req.user.id)) ? `${settings.adminOrigin()}/api/v1/profile/avatar/${req.user.id}` : `https://www.gravatar.com/avatar/${emailHash}.jpg` })); } diff --git a/src/routes/users.js b/src/routes/users.js index 43e449136..a720bd3d3 100644 --- a/src/routes/users.js +++ b/src/routes/users.js @@ -42,13 +42,21 @@ function create(req, res, next) { if ('displayName' in req.body && typeof req.body.displayName !== 'string') return next(new HttpError(400, 'displayName must be string')); if ('password' in req.body && typeof req.body.password !== 'string') return next(new HttpError(400, 'password must be string')); if ('admin' in req.body && typeof req.body.admin !== 'boolean') return next(new HttpError(400, 'admin flag must be a boolean')); + if ('permissions' in req.body) { + if (!Array.isArray(req.body.permissions)) return next(new HttpError(400, 'permissions must be an array')); + if (req.body.permissions.some((p) => typeof p !== 'string')) return next(new HttpError(400, 'permissions array must contain strings')); + } + + if (!req.user.admin) { + if ('admin' in req.body || 'permissions' in req.body) return next(new HttpError(403, 'Only admin add admins or set permissions')); + } var password = req.body.password || null; var email = req.body.email; var username = 'username' in req.body ? req.body.username : null; var displayName = req.body.displayName || ''; - users.create(username, password, email, displayName, { invitor: req.user, admin: req.body.admin }, auditSource.fromRequest(req), function (error, user) { + users.create(username, password, email, displayName, { invitor: req.user, admin: req.body.admin, permissions: req.body.permissions }, auditSource.fromRequest(req), function (error, user) { if (error) return next(BoxError.toHttpError(error)); next(new HttpSuccess(201, users.removePrivateFields(user))); @@ -71,8 +79,17 @@ function update(req, res, next) { if (req.user.id === req.resource.id && !req.body.admin) return next(new HttpError(409, 'Cannot remove admin flag on self')); } + if ('permissions' in req.body) { + if (!Array.isArray(req.body.permissions)) return next(new HttpError(400, 'permissions must be an array')); + if (req.body.permissions.some((p) => typeof p !== 'string')) return next(new HttpError(400, 'permissions array must contain strings')); + } + if ('active' in req.body && typeof req.body.active !== 'boolean') return next(new HttpError(400, 'active must be a boolean')); + if (!req.user.admin) { + if ('admin' in req.body || 'permissions' in req.body) return next(new HttpError(403, 'Only admin add admins or set permissions')); + } + users.update(req.resource, req.body, auditSource.fromRequest(req), function (error) { if (error) return next(BoxError.toHttpError(error)); @@ -109,6 +126,7 @@ function remove(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 (!req.user.admin && req.resource.admin) return next(new HttpError(403, 'Non-admin cannot remove admin user')); users.remove(req.resource, auditSource.fromRequest(req), function (error) { if (error) return next(BoxError.toHttpError(error)); @@ -134,6 +152,8 @@ function verifyPassword(req, res, next) { function createInvite(req, res, next) { assert.strictEqual(typeof req.resource, 'object'); + if (!req.user.admin && req.resource.admin) return next(new HttpError(403, 'Non-admin cannot reset admin user')); + users.createInvite(req.resource, function (error, result) { if (error) return next(BoxError.toHttpError(error)); @@ -144,6 +164,8 @@ function createInvite(req, res, next) { function sendInvite(req, res, next) { assert.strictEqual(typeof req.resource, 'object'); + if (!req.user.admin && req.resource.admin) return next(new HttpError(403, 'Non-admin cannot invite admin user')); + users.sendInvite(req.resource, { invitor: req.user }, function (error) { if (error) return next(BoxError.toHttpError(error)); @@ -156,6 +178,7 @@ function setGroups(req, res, next) { assert.strictEqual(typeof req.resource, 'object'); if (!Array.isArray(req.body.groupIds)) return next(new HttpError(400, 'API call requires a groups array.')); + if (!req.user.admin && req.resource.admin) return next(new HttpError(403, 'Non-admin cannot modify admin user')); users.setMembership(req.resource, req.body.groupIds, function (error) { if (error) return next(BoxError.toHttpError(error)); @@ -169,6 +192,7 @@ function changePassword(req, res, next) { assert.strictEqual(typeof req.resource, 'object'); if (typeof req.body.password !== 'string') return next(new HttpError(400, 'password must be a string')); + if (!req.user.admin && req.resource.admin) return next(new HttpError(403, 'Non-admin cannot modify admin user')); users.setPassword(req.resource, req.body.password, function (error) { if (error) return next(BoxError.toHttpError(error)); diff --git a/src/server.js b/src/server.js index 5d5a589c0..89c16c7e9 100644 --- a/src/server.js +++ b/src/server.js @@ -79,7 +79,8 @@ function initializeExpressSync() { // to keep routes code short const password = routes.accesscontrol.passwordAuth; const token = routes.accesscontrol.tokenAuth; - const authorizeAdmin = routes.accesscontrol.authorize(accesscontrol.ROLE_ADMIN); + const authorizeAdmin = routes.accesscontrol.authorize(accesscontrol.PERMISSION_ADMIN); + const authorizeUserManager = routes.accesscontrol.authorize(accesscontrol.PERMISSION_MANAGE_USERS); // public routes router.post('/api/v1/cloudron/setup', routes.provision.providerTokenAuth, routes.provision.setup); // only available until no-domain @@ -161,15 +162,15 @@ function initializeExpressSync() { router.del ('/api/v1/tokens/:id', token, routes.tokens.verifyOwnership, routes.tokens.del); // user routes - router.get ('/api/v1/users', token, authorizeAdmin, routes.users.list); - router.post('/api/v1/users', token, authorizeAdmin, routes.users.create); - router.get ('/api/v1/users/:userId', token, authorizeAdmin, routes.users.load, routes.users.get); // this is manage scope because it returns non-restricted fields - router.del ('/api/v1/users/:userId', token, authorizeAdmin, routes.users.load, routes.users.remove); - router.post('/api/v1/users/:userId', token, authorizeAdmin, routes.users.load, routes.users.update); - router.post('/api/v1/users/:userId/password', token, authorizeAdmin, routes.users.load, routes.users.changePassword); - router.put ('/api/v1/users/:userId/groups', token, authorizeAdmin, routes.users.load, routes.users.setGroups); - router.post('/api/v1/users/:userId/send_invite', token, authorizeAdmin, routes.users.load, routes.users.sendInvite); - router.post('/api/v1/users/:userId/create_invite', token, authorizeAdmin,routes.users.load, routes.users.createInvite); + router.get ('/api/v1/users', token, authorizeUserManager, routes.users.list); + router.post('/api/v1/users', 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.post('/api/v1/users/:userId', token, authorizeUserManager, routes.users.load, routes.users.update); + router.post('/api/v1/users/:userId/password', token, authorizeUserManager, routes.users.load, routes.users.changePassword); + router.put ('/api/v1/users/:userId/groups', token, authorizeUserManager, routes.users.load, routes.users.setGroups); + router.post('/api/v1/users/:userId/send_invite', token, authorizeUserManager, routes.users.load, routes.users.sendInvite); + router.post('/api/v1/users/:userId/create_invite', token, authorizeUserManager, routes.users.load, routes.users.createInvite); // Group management router.get ('/api/v1/groups', token, authorizeAdmin, routes.groups.list); diff --git a/src/test/apps-test.js b/src/test/apps-test.js index 9e4b1b756..2aed880cb 100644 --- a/src/test/apps-test.js +++ b/src/test/apps-test.js @@ -33,7 +33,8 @@ describe('Apps', function () { displayName: '', groupIds: [], admin: true, - source: '' + source: '', + permissions: null }; var USER_0 = { @@ -49,7 +50,8 @@ describe('Apps', function () { displayName: '', groupIds: [], admin: false, - source: '' + source: '', + permissions: null }; var USER_1 = { @@ -65,7 +67,8 @@ describe('Apps', function () { displayName: '', groupIds: [ 'somegroup' ], admin: false, - source: '' + source: '', + permissions: null }; var GROUP_0 = { diff --git a/src/test/apptask-test.js b/src/test/apptask-test.js index 99bb9858b..c344f9381 100644 --- a/src/test/apptask-test.js +++ b/src/test/apptask-test.js @@ -76,7 +76,8 @@ var ADMIN = { resetToken: '', displayName: '', admin: true, - source: '' + source: '', + permissions: null }; var APP = { diff --git a/src/test/database-test.js b/src/test/database-test.js index ad142612a..f01549952 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -40,7 +40,8 @@ var USER_0 = { twoFactorAuthenticationSecret: '', admin: false, active: true, - source: '' + source: '', + permissions: null }; var USER_1 = { @@ -58,7 +59,8 @@ var USER_1 = { twoFactorAuthenticationSecret: '', admin: false, active: true, - source: '' + source: '', + permissions: null }; var USER_2 = { @@ -76,7 +78,8 @@ var USER_2 = { twoFactorAuthenticationSecret: '', admin: false, active: true, - source: '' + source: '', + permissions: null }; const DOMAIN_0 = { diff --git a/src/test/groups-test.js b/src/test/groups-test.js index 8e20b2717..1c6f29367 100644 --- a/src/test/groups-test.js +++ b/src/test/groups-test.js @@ -39,7 +39,8 @@ var USER_0 = { modifiedAt: 'now', resetToken: hat(256), displayName: '', - source: '' + source: '', + permissions: null }; var USER_1 = { // this user has not signed up yet @@ -54,7 +55,8 @@ var USER_1 = { // this user has not signed up yet modifiedAt: 'now', resetToken: hat(256), displayName: '', - source: '' + source: '', + permissions: null }; function setup(done) { diff --git a/src/test/ldap-test.js b/src/test/ldap-test.js index 5bff266f9..a608df32e 100644 --- a/src/test/ldap-test.js +++ b/src/test/ldap-test.js @@ -35,7 +35,8 @@ var USER_0 = { username: 'userName0', password: 'Username0pass?1234', email: 'user0@' + DOMAIN_0.domain.toUpperCase(), - displayName: 'User 0' + displayName: 'User 0', + permissions: null }; var USER_0_ALIAS = 'Asterix'; @@ -45,13 +46,15 @@ var USER_1 = { username: 'Username1', password: 'Username1pass?12345', email: 'USER1@' + DOMAIN_0.domain, - displayName: 'User 1' + displayName: 'User 1', + permissions: null }; var USER_2 = { username: 'Username2', password: 'Username2pass?12345', email: 'USER2@' + DOMAIN_0.domain, - displayName: 'User 2' + displayName: 'User 2', + permissions: null }; var GROUP_ID, GROUP_NAME = 'developers'; diff --git a/src/test/notifications-test.js b/src/test/notifications-test.js index 1db796255..13f5335cf 100644 --- a/src/test/notifications-test.js +++ b/src/test/notifications-test.js @@ -21,7 +21,8 @@ var USER_0 = { password: 'Username0pass?1234', email: 'user0@email.com', fallbackEmail: 'user0fallback@email.com', - displayName: 'User 0' + displayName: 'User 0', + permissions: null }; var EVENT_0 = { diff --git a/src/test/updatechecker-test.js b/src/test/updatechecker-test.js index 35f4229b5..9321ba792 100644 --- a/src/test/updatechecker-test.js +++ b/src/test/updatechecker-test.js @@ -30,7 +30,8 @@ var USER_0 = { password: 'Username0pass?1234', email: 'user0@email.com', displayName: 'User 0', - source: '' + source: '', + permissions: null }; const DOMAIN_0 = { diff --git a/src/userdb.js b/src/userdb.js index a5d80780e..a92dec211 100644 --- a/src/userdb.js +++ b/src/userdb.js @@ -27,10 +27,11 @@ var assert = require('assert'), BoxError = require('./boxerror.js'), database = require('./database.js'), debug = require('debug')('box:userdb'), - mysql = require('mysql'); + mysql = require('mysql'), + safe = require('safetydance'); var USERS_FIELDS = [ 'id', 'username', 'email', 'fallbackEmail', 'password', 'salt', 'createdAt', 'modifiedAt', 'resetToken', 'displayName', - 'twoFactorAuthenticationEnabled', 'twoFactorAuthenticationSecret', 'admin', 'active', 'source' ].join(','); + 'twoFactorAuthenticationEnabled', 'twoFactorAuthenticationSecret', 'admin', 'active', 'source', 'permissionsJson' ].join(','); var APP_PASSWORD_FIELDS = [ 'id', 'name', 'userId', 'identifier', 'hashedPassword', 'creationTime' ].join(','); @@ -40,6 +41,8 @@ function postProcess(result) { result.twoFactorAuthenticationEnabled = !!result.twoFactorAuthenticationEnabled; result.admin = !!result.admin; result.active = !!result.active; + result.permissions = safe.JSON.parse(result.permissionsJson); + delete result.permissionsJson; return result; } @@ -183,10 +186,13 @@ function add(userId, user, callback) { assert.strictEqual(typeof user.displayName, 'string'); assert.strictEqual(typeof user.admin, 'boolean'); assert.strictEqual(typeof user.source, 'string'); + assert.strictEqual(typeof user.permissions, 'object'); assert.strictEqual(typeof callback, 'function'); - const query = 'INSERT INTO users (id, username, password, email, fallbackEmail, salt, createdAt, modifiedAt, resetToken, displayName, admin, source) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'; - const args = [ userId, user.username, user.password, user.email, user.fallbackEmail, user.salt, user.createdAt, user.modifiedAt, user.resetToken, user.displayName, user.admin, user.source ]; + const permissionsJson = user.permissions ? JSON.stringify(user.permissions) : null; + + const query = 'INSERT INTO users (id, username, password, email, fallbackEmail, salt, createdAt, modifiedAt, resetToken, displayName, admin, source, permissionsJson) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'; + const args = [ userId, user.username, user.password, user.email, user.fallbackEmail, user.salt, user.createdAt, user.modifiedAt, user.resetToken, user.displayName, user.admin, user.source, permissionsJson ]; database.query(query, args, function (error) { if (error && error.code === 'ER_DUP_ENTRY' && error.sqlMessage.indexOf('users_email') !== -1) return callback(new BoxError(BoxError.ALREADY_EXISTS, 'email already exists')); @@ -259,6 +265,9 @@ function update(userId, user, callback) { } else if (k === 'twoFactorAuthenticationEnabled' || k === 'admin' || k === 'active') { assert.strictEqual(typeof user[k], 'boolean'); args.push(user[k] ? 1 : 0); + } else if (k === 'permissions') { + fields.push(`${k}Json = ?`); + args.push(JSON.stringify(user[k])); } else { args.push(user[k]); } diff --git a/src/users.js b/src/users.js index 430bbee50..86346d0c1 100644 --- a/src/users.js +++ b/src/users.js @@ -40,7 +40,8 @@ exports = module.exports = { delAppPassword: delAppPassword }; -let assert = require('assert'), +let accesscontrol = require('./accesscontrol.js'), + assert = require('assert'), BoxError = require('./boxerror.js'), crypto = require('crypto'), constants = require('./constants.js'), @@ -115,7 +116,7 @@ function validatePassword(password) { // remove all fields that should never be sent out via REST API function removePrivateFields(user) { - return _.pick(user, 'id', 'username', 'email', 'fallbackEmail', 'displayName', 'groupIds', 'admin', 'active', 'source'); + return _.pick(user, 'id', 'username', 'email', 'fallbackEmail', 'displayName', 'groupIds', 'admin', 'active', 'source', 'permissions'); } // remove all fields that Non-privileged users must not see @@ -135,6 +136,7 @@ function create(username, password, email, displayName, options, auditSource, ca const isAdmin = !!options.admin; const source = options.source || ''; // empty is local user const invitor = options.invitor || null; + const permissions = options.permissions || null; var error; @@ -158,6 +160,9 @@ function create(username, password, email, displayName, options, auditSource, ca error = validateDisplayName(displayName); if (error) return callback(error); + error = accesscontrol.validatePermissions(permissions); + if (error) return callback(error); + crypto.randomBytes(CRYPTO_SALT_SIZE, function (error, salt) { if (error) return callback(new BoxError(BoxError.CRYPTO_ERROR, error)); @@ -177,7 +182,8 @@ function create(username, password, email, displayName, options, auditSource, ca resetToken: '', displayName: displayName, admin: isOwner || isAdmin, - source: source + source: source, + permissions: permissions }; userdb.add(user.id, user, function (error) { @@ -407,7 +413,7 @@ function updateUser(user, data, auditSource, callback) { assert.strictEqual(typeof callback, 'function'); var error; - data = _.pick(data, 'email', 'fallbackEmail', 'displayName', 'username', 'admin', 'active'); + data = _.pick(data, 'email', 'fallbackEmail', 'displayName', 'username', 'admin', 'active', 'permissions'); if (_.isEmpty(data)) return callback(); @@ -429,6 +435,11 @@ function updateUser(user, data, auditSource, callback) { if (error) return callback(error); } + if (data.permissions) { + error = accesscontrol.validatePermissions(data.permissions); + if (error) return callback(error); + } + userdb.update(user.id, data, function (error) { if (error) return callback(error);