diff --git a/migrations/20200214061201-user-add-permissionsJson.js b/migrations/20200214061201-user-add-permissionsJson.js deleted file mode 100644 index 97164d0ad..000000000 --- a/migrations/20200214061201-user-add-permissionsJson.js +++ /dev/null @@ -1,20 +0,0 @@ -'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/20200221211850-users-add-role.js b/migrations/20200221211850-users-add-role.js new file mode 100644 index 000000000..e7135db49 --- /dev/null +++ b/migrations/20200221211850-users-add-role.js @@ -0,0 +1,40 @@ +'use strict'; + +var async = require('async'); + +exports.up = function(db, callback) { + async.series([ + db.runSql.bind(db, 'START TRANSACTION;'), + db.runSql.bind(db, 'ALTER TABLE users ADD COLUMN role VARCHAR(32)'), + function migrateAdminFlag(done) { + db.all('SELECT * FROM users ORDER BY createdAt', function (error, results) { + if (error) return done(error); + let ownerFound = false; + + async.eachSeries(results, function (r, next) { + let role; + if (!ownerFound && r.admin) { + role = 'owner'; + ownerFound = true; + console.log(`Designating ${user.username} ${user.email} ${user.id} as the owner of this cloudron`); + } else { + role = r.admin ? 'admin' : 'user'; + } + db.runSql('UPDATE users SET role=? WHERE id=?', [ role, r.id ], next); + }, done); + }); + }, + db.runSql.bind(db, 'ALTER TABLE users DROP COLUMN admin'), + db.runSql.bind(db, 'ALTER TABLE users MODIFY role VARCHAR(32) NOT NULL'), + db.runSql.bind(db, 'COMMIT') + ], callback); +}; + +exports.down = function(db, callback) { + db.runSql('ALTER TABLE users DROP COLUMN role', function (error) { + if (error) console.error(error); + + callback(error); + }); +}; + diff --git a/migrations/schema.sql b/migrations/schema.sql index b31a20c11..41a211fc4 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -26,9 +26,8 @@ CREATE TABLE IF NOT EXISTS users( fallbackEmail VARCHAR(512) DEFAULT "", twoFactorAuthenticationSecret VARCHAR(128) DEFAULT "", twoFactorAuthenticationEnabled BOOLEAN DEFAULT false, - admin BOOLEAN DEFAULT false, source VARCHAR(128) DEFAULT "", - permissionsJson TEXT, + role VARCHAR(32), PRIMARY KEY(id)); diff --git a/scripts/cloudron-support b/scripts/cloudron-support index b73a0e961..00a4a44cf 100755 --- a/scripts/cloudron-support +++ b/scripts/cloudron-support @@ -34,7 +34,7 @@ while true; do --help) echo -e "${HELP_MESSAGE}"; exit 0;; --enable-ssh) enableSSH="true"; shift;; --admin-login) - admin_username=$(mysql -NB -uroot -ppassword -e "SELECT username FROM box.users WHERE admin=1 LIMIT 1" 2>/dev/null) + admin_username=$(mysql -NB -uroot -ppassword -e "SELECT username FROM box.users WHERE role='owner' LIMIT 1" 2>/dev/null) admin_password=$(pwgen -1s 12) printf '{"%s":"%s"}\n' "${admin_username}" "${admin_password}" > /tmp/cloudron_ghost.json echo "Login as ${admin_username} / ${admin_password} . Remove /tmp/cloudron_ghost.json when done." diff --git a/src/accesscontrol.js b/src/accesscontrol.js index 0fac85b50..6c11609fc 100644 --- a/src/accesscontrol.js +++ b/src/accesscontrol.js @@ -1,12 +1,7 @@ 'use strict'; exports = module.exports = { - PERMISSION_ADMIN: 'admin', // not a real permission, but a role - PERMISSION_MANAGE_USERS: 'manage_users', - - verifyToken: verifyToken, - hasPermission: hasPermission, - validatePermissions: validatePermissions + verifyToken: verifyToken }; var assert = require('assert'), @@ -14,28 +9,6 @@ var assert = require('assert'), tokendb = require('./tokendb.js'), users = require('./users.js'); -function hasPermission(user, requiredPermission) { - assert.strictEqual(typeof user, 'object'); - assert.strictEqual(typeof requiredPermission, 'string'); - - if (user.admin) return null; - if (user.permissions && user.permissions.includes(requiredPermission)) return null; - - 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.PERMISSION_ADMIN)) return new BoxError(BoxError.BAD_FIELD, 'admin is not a permission'); - - return new BoxError(BoxError.BAD_FIELD, 'Invalid permissions'); -} - function verifyToken(accessToken, callback) { assert.strictEqual(typeof accessToken, 'string'); assert.strictEqual(typeof callback, 'function'); diff --git a/src/apps.js b/src/apps.js index 07c242a69..b83b49ec7 100644 --- a/src/apps.js +++ b/src/apps.js @@ -132,6 +132,7 @@ var appdb = require('./appdb.js'), tasks = require('./tasks.js'), TransformStream = require('stream').Transform, updateChecker = require('./updatechecker.js'), + users = require('./users.js'), util = require('util'), uuid = require('uuid'), validator = require('validator'), @@ -455,7 +456,7 @@ function hasAccessTo(app, user, callback) { // check user access if (app.accessRestriction.users.some(function (e) { return e === user.id; })) return callback(null, true); - if (user.admin) return callback(null, true); // admins can always access any app + if (users.compareRoles(user.role, users.ROLE_ADMIN) >= 0) return callback(null, true); // admins can always access any app if (!app.accessRestriction.groups) return callback(null, false); diff --git a/src/database.js b/src/database.js index d1d1c14c1..68dc606cd 100644 --- a/src/database.js +++ b/src/database.js @@ -203,7 +203,7 @@ function exportToFile(file, callback) { // latest mysqldump enables column stats by default which is not present in MySQL 5.7 server // this option must not be set in production cloudrons which still use the old mysqldump - const disableColStats = constants.TEST ? '--column-statistics=0' : ''; + const disableColStats = (constants.TEST && process.env.DESKTOP_SESSION !== 'ubuntu') ? '--column-statistics=0' : ''; var cmd = `/usr/bin/mysqldump -h "${gDatabase.hostname}" -u root -p${gDatabase.password} ${disableColStats} --single-transaction --routines --triggers ${gDatabase.name} > "${file}"`; diff --git a/src/eventlog.js b/src/eventlog.js index 67d916e5d..db3b38da9 100644 --- a/src/eventlog.js +++ b/src/eventlog.js @@ -87,11 +87,9 @@ function add(action, source, data, callback) { api(uuid.v4(), action, source, data, function (error, id) { if (error) return callback(error); - notifications.onEvent(id, action, source, data, function (error) { - if (error) return callback(error); + callback(null, { id: id }); - callback(null, { id: id }); - }); + notifications.onEvent(id, action, source, data, NOOP_CALLBACK); }); } diff --git a/src/ldap.js b/src/ldap.js index 99a988c94..4c6d74902 100644 --- a/src/ldap.js +++ b/src/ldap.js @@ -126,16 +126,16 @@ function userSearch(req, res, next) { var results = []; // send user objects - result.forEach(function (entry) { + result.forEach(function (user) { // skip entries with empty username. Some apps like owncloud can't deal with this - if (!entry.username) return; + if (!user.username) return; - var dn = ldap.parseDN('cn=' + entry.id + ',ou=users,dc=cloudron'); + var dn = ldap.parseDN('cn=' + user.id + ',ou=users,dc=cloudron'); var groups = [ GROUP_USERS_DN ]; - if (entry.admin) groups.push(GROUP_ADMINS_DN); + if (users.compareRoles(user.role, users.ROLE_ADMIN) >= 0) groups.push(GROUP_ADMINS_DN); - var displayName = entry.displayName || entry.username || ''; // displayName can be empty and username can be null + var displayName = user.displayName || user.username || ''; // displayName can be empty and username can be null var nameParts = displayName.split(' '); var firstName = nameParts[0]; var lastName = nameParts.length > 1 ? nameParts[nameParts.length - 1] : ''; // choose last part, if it exists @@ -145,16 +145,16 @@ function userSearch(req, res, next) { attributes: { objectclass: ['user', 'inetorgperson', 'person' ], objectcategory: 'person', - cn: entry.id, - uid: entry.id, - entryuuid: entry.id, // to support OpenLDAP clients - mail: entry.email, - mailAlternateAddress: entry.fallbackEmail, + cn: user.id, + uid: user.id, + entryuuid: user.id, // to support OpenLDAP clients + mail: user.email, + mailAlternateAddress: user.fallbackEmail, displayname: displayName, givenName: firstName, - username: entry.username, - samaccountname: entry.username, // to support ActiveDirectory clients - isadmin: entry.admin, + username: user.username, + samaccountname: user.username, // to support ActiveDirectory clients + isadmin: users.compareRoles(user.role, users.ROLE_ADMIN) >= 0, memberof: groups } }; @@ -194,7 +194,7 @@ function groupSearch(req, res, next) { groups.forEach(function (group) { var dn = ldap.parseDN('cn=' + group.name + ',ou=groups,dc=cloudron'); - var members = group.admin ? result.filter(function (entry) { return entry.admin; }) : result; + var members = group.admin ? result.filter(function (user) { return users.compareRoles(user.role, users.ROLE_ADMIN) >= 0; }) : result; var obj = { dn: dn.toString(), @@ -242,8 +242,8 @@ function groupAdminsCompare(req, res, next) { // we only support memberuid here, if we add new group attributes later add them here if (req.attribute === 'memberuid') { - var found = result.find(function (u) { return u.id === req.value; }); - if (found && found.admin) return res.end(true); + var user = result.find(function (u) { return u.id === req.value; }); + if (user && users.compareRoles(user.role, users.ROLE_ADMIN) >= 0) return res.end(true); } res.end(false); diff --git a/src/mailer.js b/src/mailer.js index e850572ea..d24377195 100644 --- a/src/mailer.js +++ b/src/mailer.js @@ -3,7 +3,7 @@ exports = module.exports = { userAdded: userAdded, userRemoved: userRemoved, - adminChanged: adminChanged, + roleChanged: roleChanged, passwordReset: passwordReset, appUpdatesAvailable: appUpdatesAvailable, @@ -204,14 +204,13 @@ function userRemoved(mailTo, user) { mailUserEvent(mailTo, user, 'was removed'); } -function adminChanged(mailTo, user, isAdmin) { +function roleChanged(mailTo, user) { assert.strictEqual(typeof mailTo, 'string'); assert.strictEqual(typeof user, 'object'); - assert.strictEqual(typeof isAdmin, 'boolean'); - debug('Sending mail for adminChanged'); + debug('Sending mail for roleChanged'); - mailUserEvent(mailTo, user, isAdmin ? 'is now an admin' : 'is no more an admin'); + mailUserEvent(mailTo, user, `now has the role '${user.role}`); } function passwordReset(user) { diff --git a/src/notifications.js b/src/notifications.js index b143ee641..08d9b5200 100644 --- a/src/notifications.js +++ b/src/notifications.js @@ -99,7 +99,8 @@ function actionForAllAdmins(skippingUserIds, iterator, callback) { assert.strictEqual(typeof iterator, 'function'); assert.strictEqual(typeof callback, 'function'); - users.getAllAdmins(function (error, result) { + users.getAdmins(function (error, result) { + if (error && error.reason === BoxError.NOT_FOUND) return callback(); if (error) return callback(error); // filter out users we want to skip (like the user who did the action or the user the action was performed on) @@ -133,14 +134,14 @@ function userRemoved(performedBy, eventId, user, callback) { }, callback); } -function adminChanged(performedBy, eventId, user, callback) { +function roleChanged(performedBy, eventId, user, callback) { assert.strictEqual(typeof performedBy, 'string'); assert.strictEqual(typeof user, 'object'); assert.strictEqual(typeof callback, 'function'); actionForAllAdmins([ performedBy, user.id ], function (admin, done) { - mailer.adminChanged(admin.email, user, user.admin); - add(admin.id, eventId, `User '${user.displayName} ' ${user.admin ? 'is now an admin' : 'is no more an admin'}`, `User '${user.username || user.email || user.fallbackEmail}' ${user.admin ? 'is now an admin' : 'is no more an admin'}.`, done); + mailer.roleChanged(admin.email, user); + add(admin.id, eventId, `User '${user.displayName}'s role changed`, `User '${user.username || user.email || user.fallbackEmail}' now has the role '${user.role}'.`, done); }, callback); } @@ -324,8 +325,8 @@ function onEvent(id, action, source, data, callback) { return userRemoved(source.userId, id, data.user, callback); case eventlog.ACTION_USER_UPDATE: - if (!data.adminStatusChanged) return callback(); - return adminChanged(source.userId, id, data.user, callback); + if (!data.roleChanged) return callback(); + return roleChanged(source.userId, id, data.user, callback); case eventlog.ACTION_APP_OOM: return oomEvent(id, data.app, data.addon, data.containerId, data.event, callback); diff --git a/src/routes/accesscontrol.js b/src/routes/accesscontrol.js index 035c0631b..c6907f725 100644 --- a/src/routes/accesscontrol.js +++ b/src/routes/accesscontrol.js @@ -105,14 +105,13 @@ function tokenAuth(req, res, next) { }); } -function authorize(requiredPermission) { - assert.strictEqual(typeof requiredPermission, 'string'); +function authorize(requiredRole) { + assert.strictEqual(typeof requiredRole, 'string'); return function (req, res, next) { assert.strictEqual(typeof req.user, 'object'); - var error = accesscontrol.hasPermission(req.user, requiredPermission); - if (error) return next(new HttpError(403, error.message)); + if (users.compareRoles(req.user.role, requiredRole) < 0) return next(new HttpError(403, `role '${requiredRole}' is required but user has only '${req.user.role}'`)); next(); }; @@ -129,8 +128,7 @@ function websocketAuth(requiredRole, req, res, next) { req.user = user; - var e = accesscontrol.hasRole(req.user, requiredRole); - if (e) return next(new HttpError(403, e.message)); + if (users.compareRoles(req.user.role, requiredRole) < 0) return next(new HttpError(403, `role '${requiredRole}' is required but user has only '${user.role}'`)); next(); }); diff --git a/src/routes/profile.js b/src/routes/profile.js index 98c9666e2..13a8d5725 100644 --- a/src/routes/profile.js +++ b/src/routes/profile.js @@ -37,9 +37,8 @@ function get(req, res, next) { fallbackEmail: req.user.fallbackEmail, displayName: req.user.displayName, twoFactorAuthenticationEnabled: req.user.twoFactorAuthenticationEnabled, - admin: req.user.admin, + role: req.user.role, 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/test/eventlog-test.js b/src/routes/test/eventlog-test.js index c4dec18c4..5fd25ca70 100644 --- a/src/routes/test/eventlog-test.js +++ b/src/routes/test/eventlog-test.js @@ -58,7 +58,7 @@ function setup(done) { function (callback) { superagent.post(SERVER_URL + '/api/v1/users') .query({ access_token: token }) - .send({ username: 'nonadmin', email: 'notadmin@server.test', invite: false }) + .send({ username: 'nonadmin', email: 'notadmin@server.test' }) .end(function (err, res) { expect(res.statusCode).to.equal(201); @@ -199,7 +199,7 @@ describe('Eventlog API', function () { .query({ access_token: token, page: 1, per_page: 10, search: EMAIL }) .end(function (error, result) { expect(result.statusCode).to.equal(200); - expect(result.body.eventlogs.length).to.equal(2); + expect(result.body.eventlogs.length).to.equal(1); done(); }); diff --git a/src/routes/test/users-test.js b/src/routes/test/users-test.js index c3be261e3..da3bd6c33 100644 --- a/src/routes/test/users-test.js +++ b/src/routes/test/users-test.js @@ -168,7 +168,7 @@ describe('Users API', function () { expect(res.body.username).to.equal(USERNAME_0.toLowerCase()); expect(res.body.email).to.equal(EMAIL_0.toLowerCase()); expect(res.body.groupIds).to.eql([]); - expect(res.body.admin).to.be(true); + expect(res.body.role).to.be(users.ROLE_OWNER); done(); }); @@ -209,7 +209,7 @@ describe('Users API', function () { expect(res.body.username).to.equal(USERNAME_0.toLowerCase()); expect(res.body.email).to.equal(EMAIL_0.toLowerCase()); expect(res.body.groupIds).to.eql([]); - expect(res.body.admin).to.be(true); + expect(res.body.role).to.be(users.ROLE_OWNER); done(); }); @@ -250,7 +250,7 @@ describe('Users API', function () { expect(res.body.username).to.equal(USERNAME_0.toLowerCase()); expect(res.body.email).to.equal(EMAIL_0.toLowerCase()); expect(res.body.groupIds).to.eql([]); - expect(res.body.admin).to.be(true); + expect(res.body.role).to.be(users.ROLE_OWNER); expect(res.body.displayName).to.be.a('string'); expect(res.body.password).to.not.be.ok(); expect(res.body.salt).to.not.be.ok(); @@ -486,7 +486,7 @@ describe('Users API', function () { it('set second user as admin succeeds', function (done) { superagent.post(SERVER_URL + '/api/v1/users/' + user_1.id) .query({ access_token: token }) - .send({ admin: true }) + .send({ role: users.ROLE_ADMIN }) .end(function (err, res) { expect(res.statusCode).to.equal(204); @@ -494,17 +494,27 @@ describe('Users API', function () { .query({ access_token: token }) .end(function (err, res) { expect(res.statusCode).to.equal(200); - expect(res.body.admin).to.be(true); + expect(res.body.role).to.be(users.ROLE_ADMIN); done(); }); }); }); - it('remove self as admin fails', function (done) { + it('make self as admin fails', function (done) { superagent.post(SERVER_URL + '/api/v1/users/' + user_0.id) .query({ access_token: token }) - .send({ admin: false }) + .send({ role: users.ROLE_ADMIN }) + .end(function (err, res) { + expect(res.statusCode).to.equal(409); + done(); + }); + }); + + it('make self as normal user fails', function (done) { + superagent.post(SERVER_URL + '/api/v1/users/' + user_0.id) + .query({ access_token: token }) + .send({ role: users.ROLE_USER }) .end(function (err, res) { expect(res.statusCode).to.equal(409); done(); @@ -514,7 +524,7 @@ describe('Users API', function () { it('remove second user as admin succeeds', function (done) { superagent.post(SERVER_URL + '/api/v1/users/' + user_1.id) .query({ access_token: token }) - .send({ admin: false }) + .send({ role: users.ROLE_USER }) .end(function (err, res) { expect(res.statusCode).to.equal(204); done(); @@ -797,11 +807,11 @@ describe('Users API', function () { }); - describe('permissions', function () { + describe('role - user manager', function () { it('can make second user a usermanager', function (done) { superagent.post(SERVER_URL + '/api/v1/users/' + user_1.id) .query({ access_token: token }) - .send({ permissions: [ 'manage_users' ] }) + .send({ role: users.ROLE_USER_MANAGER }) .end(function (err, res) { expect(res.statusCode).to.equal(204); done(); @@ -850,7 +860,7 @@ describe('Users API', function () { it('cannot change admin bit of another', function (done) { superagent.post(SERVER_URL + '/api/v1/users/' + user_2.id) .query({ access_token: token_1 }) - .send({ admin: true }) + .send({ role: users.ROLE_ADMIN }) .end(function (err, result) { expect(result.statusCode).to.equal(403); done(); @@ -860,9 +870,9 @@ describe('Users API', function () { it('cannot change admin bit of self', function (done) { superagent.post(SERVER_URL + '/api/v1/users/' + user_1.id) .query({ access_token: token_1 }) - .send({ admin: true }) + .send({ role: users.ROLE_ADMIN }) .end(function (err, result) { - expect(result.statusCode).to.equal(403); + expect(result.statusCode).to.equal(409); done(); }); }); diff --git a/src/routes/users.js b/src/routes/users.js index a720bd3d3..d51b49423 100644 --- a/src/routes/users.js +++ b/src/routes/users.js @@ -41,14 +41,9 @@ function create(req, res, next) { if ('username' in req.body && typeof req.body.username !== 'string') return next(new HttpError(400, 'username must be string')); 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')); + if ('role' in req.body) { + if (typeof req.body.role !== 'string') return next(new HttpError(400, 'role must be string')); + if (users.compareRoles(req.user.role, req.body.role) < 0) return next(new HttpError(403, `role '${req.body.role}' is required but user has only '${req.user.role}'`)); } var password = req.body.password || null; @@ -56,7 +51,7 @@ function create(req, res, next) { 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, permissions: req.body.permissions }, auditSource.fromRequest(req), function (error, user) { + users.create(username, password, email, displayName, { invitor: req.user, role: req.body.role || users.ROLE_USER }, auditSource.fromRequest(req), function (error, user) { if (error) return next(BoxError.toHttpError(error)); next(new HttpSuccess(201, users.removePrivateFields(user))); @@ -73,23 +68,15 @@ function update(req, res, next) { if ('displayName' in req.body && typeof req.body.displayName !== 'string') return next(new HttpError(400, 'displayName must be string')); if ('username' in req.body && typeof req.body.username !== 'string') return next(new HttpError(400, 'username must be a string')); - if ('admin' in req.body) { - if (typeof req.body.admin !== 'boolean') return next(new HttpError(400, 'admin must be a boolean')); - // this route is only allowed for admins, so req.user has to be an admin - if (req.user.id === req.resource.id && !req.body.admin) return next(new HttpError(409, 'Cannot remove admin flag on self')); - } + if ('role' in req.body) { + if (typeof req.body.role !== 'string') return next(new HttpError(400, 'role must be a string')); + if (req.user.id === req.resource.id) return next(new HttpError(409, 'Cannot set role 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 (users.compareRoles(req.user.role, req.body.role) < 0) return next(new HttpError(403, `role '${req.body.role}' is required but user has only '${req.user.role}'`)); } 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)); @@ -126,7 +113,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')); + 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)); @@ -152,7 +139,7 @@ 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')); + 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.createInvite(req.resource, function (error, result) { if (error) return next(BoxError.toHttpError(error)); @@ -164,7 +151,7 @@ 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')); + 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.sendInvite(req.resource, { invitor: req.user }, function (error) { if (error) return next(BoxError.toHttpError(error)); @@ -178,7 +165,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')); + 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.setMembership(req.resource, req.body.groupIds, function (error) { if (error) return next(BoxError.toHttpError(error)); @@ -192,7 +179,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')); + 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.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 88b68c2af..c0b31b8e2 100644 --- a/src/server.js +++ b/src/server.js @@ -17,6 +17,7 @@ var accesscontrol = require('./accesscontrol.js'), middleware = require('./middleware'), routes = require('./routes/index.js'), settings = require('./settings.js'), + users = require('./users.js'), ws = require('ws'); var gHttpServer = null; @@ -45,7 +46,7 @@ function initializeExpressSync() { return [ 'Box', tokens.method(req, res), - tokens.url(req, res).replace(/(access_token=)[^\&]+/, '$1' + ''), + tokens.url(req, res).replace(/(access_token=)[^&]+/, '$1' + ''), tokens.status(req, res), tokens['response-time'](req, res), 'ms', '-', tokens.res(req, res, 'content-length') @@ -79,8 +80,8 @@ function initializeExpressSync() { // to keep routes code short const password = routes.accesscontrol.passwordAuth; const token = routes.accesscontrol.tokenAuth; - const authorizeAdmin = routes.accesscontrol.authorize(accesscontrol.PERMISSION_ADMIN); - const authorizeUserManager = routes.accesscontrol.authorize(accesscontrol.PERMISSION_MANAGE_USERS); + const authorizeAdmin = routes.accesscontrol.authorize(users.ROLE_ADMIN); + const authorizeUserManager = routes.accesscontrol.authorize(users.ROLE_USER_MANAGER); // public routes router.post('/api/v1/cloudron/setup', routes.provision.providerTokenAuth, routes.provision.setup); // only available until no-domain diff --git a/src/test/apps-test.js b/src/test/apps-test.js index 2aed880cb..c9407d45a 100644 --- a/src/test/apps-test.js +++ b/src/test/apps-test.js @@ -32,9 +32,8 @@ describe('Apps', function () { resetToken: hat(256), displayName: '', groupIds: [], - admin: true, - source: '', - permissions: null + role: 'owner', + source: '' }; var USER_0 = { @@ -49,9 +48,8 @@ describe('Apps', function () { resetToken: hat(256), displayName: '', groupIds: [], - admin: false, - source: '', - permissions: null + role: 'user', + source: '' }; var USER_1 = { @@ -66,9 +64,8 @@ describe('Apps', function () { resetToken: hat(256), displayName: '', groupIds: [ 'somegroup' ], - admin: false, - source: '', - permissions: null + role: 'user', + source: '' }; var GROUP_0 = { @@ -278,8 +275,8 @@ describe('Apps', function () { }); describe('hasAccessTo', function () { - const someuser = { id: 'someuser', groupIds: [], admin: false }; - const adminuser = { id: 'adminuser', groupIds: [ 'groupie' ], admin: true }; + const someuser = { id: 'someuser', groupIds: [], role: 'user' }; + const adminuser = { id: 'adminuser', groupIds: [ 'groupie' ], role: 'admin' }; it('returns true for unrestricted access', function (done) { apps.hasAccessTo({ accessRestriction: null }, someuser, function (error, access) { diff --git a/src/test/apptask-test.js b/src/test/apptask-test.js index c344f9381..6196544ba 100644 --- a/src/test/apptask-test.js +++ b/src/test/apptask-test.js @@ -75,9 +75,8 @@ var ADMIN = { modifiedAt: 'now', resetToken: '', displayName: '', - admin: true, - source: '', - permissions: null + role: 'owner', + source: '' }; var APP = { diff --git a/src/test/database-test.js b/src/test/database-test.js index f01549952..6cd103e96 100644 --- a/src/test/database-test.js +++ b/src/test/database-test.js @@ -38,10 +38,9 @@ var USER_0 = { displayName: '', twoFactorAuthenticationEnabled: false, twoFactorAuthenticationSecret: '', - admin: false, + role: 'user', active: true, - source: '', - permissions: null + source: '' }; var USER_1 = { @@ -57,10 +56,9 @@ var USER_1 = { displayName: 'Herbert 1', twoFactorAuthenticationEnabled: false, twoFactorAuthenticationSecret: '', - admin: false, + role: 'user', active: true, - source: '', - permissions: null + source: '' }; var USER_2 = { @@ -76,10 +74,9 @@ var USER_2 = { displayName: 'Herbert 2', twoFactorAuthenticationEnabled: false, twoFactorAuthenticationSecret: '', - admin: false, + role: 'user', active: true, - source: '', - permissions: null + source: '' }; const DOMAIN_0 = { @@ -606,9 +603,9 @@ describe('database', function () { }); it('can get all admins', function (done) { - userdb.getAllAdmins(function (error, all) { - expect(error).to.not.be.ok(); - expect(all.length).to.equal(0); + userdb.getByRole('owner', function (error, all) { + expect(error).to.be.ok(); + expect(error.reason).to.be(BoxError.NOT_FOUND); done(); }); }); @@ -621,6 +618,14 @@ describe('database', function () { }); }); + it('can get all users', function (done) { + userdb.getByRole('user', function (error, all) { + expect(error).to.not.be.ok(); + expect(all.length).to.equal(3); + done(); + }); + }); + it('can update the user', function (done) { userdb.update(USER_0.id, { email: 'some@thing.com', displayName: 'Heiter' }, function (error) { expect(error).to.not.be.ok(); diff --git a/src/test/groups-test.js b/src/test/groups-test.js index 1c6f29367..03846cbe7 100644 --- a/src/test/groups-test.js +++ b/src/test/groups-test.js @@ -33,7 +33,7 @@ var USER_0 = { password: 'secret', email: 'safe@me.com', fallbackEmail: 'safefallback@me.com', - admin: false, + role: 'user', salt: 'morton', createdAt: 'sometime back', modifiedAt: 'now', @@ -49,7 +49,7 @@ var USER_1 = { // this user has not signed up yet password: '', email: 'safe2@me.com', fallbackEmail: 'safe2fallback@me.com', - admin: false, + role: 'user', salt: 'morton', createdAt: 'sometime back', modifiedAt: 'now', diff --git a/src/test/ldap-test.js b/src/test/ldap-test.js index a608df32e..9a9a0c5dd 100644 --- a/src/test/ldap-test.js +++ b/src/test/ldap-test.js @@ -36,7 +36,7 @@ var USER_0 = { password: 'Username0pass?1234', email: 'user0@' + DOMAIN_0.domain.toUpperCase(), displayName: 'User 0', - permissions: null + role: 'owner' }; var USER_0_ALIAS = 'Asterix'; @@ -47,14 +47,14 @@ var USER_1 = { password: 'Username1pass?12345', email: 'USER1@' + DOMAIN_0.domain, displayName: 'User 1', - permissions: null + role: 'user' }; var USER_2 = { username: 'Username2', password: 'Username2pass?12345', email: 'USER2@' + DOMAIN_0.domain, displayName: 'User 2', - permissions: null + role: 'user' }; var GROUP_ID, GROUP_NAME = 'developers'; @@ -114,7 +114,7 @@ function setup(done) { 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) { + users.create(USER_1.username, USER_1.password, USER_1.email, USER_0.displayName, { }, AUDIT_SOURCE, function (error, result) { if (error) return callback(error); USER_1.id = result.id; @@ -123,7 +123,7 @@ function setup(done) { }); }, function (callback) { - users.create(USER_2.username, USER_2.password, USER_2.email, USER_0.displayName, { invitor: USER_0 }, AUDIT_SOURCE, function (error, result) { + users.create(USER_2.username, USER_2.password, USER_2.email, USER_0.displayName, { }, AUDIT_SOURCE, function (error, result) { if (error) return callback(error); USER_2.id = result.id; diff --git a/src/test/notifications-test.js b/src/test/notifications-test.js index 13f5335cf..d11587234 100644 --- a/src/test/notifications-test.js +++ b/src/test/notifications-test.js @@ -22,7 +22,7 @@ var USER_0 = { email: 'user0@email.com', fallbackEmail: 'user0fallback@email.com', displayName: 'User 0', - permissions: null + role: 'owner' }; var EVENT_0 = { diff --git a/src/test/users-test.js b/src/test/users-test.js index 7b411bd6e..b7055dc02 100644 --- a/src/test/users-test.js +++ b/src/test/users-test.js @@ -20,7 +20,7 @@ var async = require('async'), mailer = require('../mailer.js'), settings = require('../settings.js'), userdb = require('../userdb.js'), - users = require('../users.js'), + users = require('../users.js'), _ = require('underscore'); var USERNAME = 'noBody'; @@ -61,7 +61,7 @@ function createOwner(done) { userObject = result; - done(); + done(null, userObject); }); } @@ -730,10 +730,14 @@ describe('User', function () { }); describe('admin change mail triggers', function () { + let auditSource; + before(function (done) { - createOwner(function (error) { + createOwner(function (error, owner) { expect(error).to.not.be.ok(); + auditSource = _.extend({}, AUDIT_SOURCE, { userId: owner.id }); + groups.create(NON_ADMIN_GROUP, done); }); }); @@ -744,22 +748,20 @@ describe('User', function () { username: 'seconduser', password: 'ASDFkljsf#$^%2354', email: 'some@thi.ng', - admin: false + role: users.ROLE_ADMIN }; it('make second user admin does not send mail to action performer', function (done) { - var invitor = { username: USERNAME, email: EMAIL }; - - users.create(user1.username, user1.password, user1.email, DISPLAY_NAME, { invitor: invitor }, AUDIT_SOURCE, function (error, result) { + users.create(user1.username, user1.password, user1.email, DISPLAY_NAME, { }, auditSource, function (error, result) { expect(error).to.not.be.ok(); expect(result).to.be.ok(); user1.id = result.id; - users.update(user1, { admin: true }, AUDIT_SOURCE, function (error) { + users.update(user1, { role: users.ROLE_ADMIN }, AUDIT_SOURCE, function (error) { expect(error).to.not.be.ok(); - user1.admin = true; + user1.role = users.ROLE_ADMIN; // no emails should be sent out anymore, since the user performing the action does not get a notification anymore checkMails(0, done); }); @@ -767,29 +769,29 @@ describe('User', function () { }); it('succeeds to remove admin flag does not send mail to action performer', function (done) { - users.update(user1, { admin: false }, AUDIT_SOURCE, function (error) { + users.update(user1, { role: users.ROLE_USER }, auditSource, function (error) { expect(error).to.eql(null); - user1.admin = false; + user1.role = users.ROLE_USER; // no emails should be sent out anymore, since the user performing the action does not get a notification anymore checkMails(0, done); }); }); it('make second user admin does send mail to other admins', function (done) { - users.update(user1, { admin: true }, { ip: '1.2.3.4', userId: 'someuserid' }, function (error) { + users.update(user1, { role: users.ROLE_ADMIN }, { ip: '1.2.3.4', userId: 'someuserid' }, function (error) { expect(error).to.not.be.ok(); - user1.admin = true; + user1.role = users.ROLE_ADMIN; checkMails(1, done); }); }); it('succeeds to remove admin flag does send mail to other admins', function (done) { - users.update(user1, { admin: false }, { ip: '1.2.3.4', userId: 'someuserid' }, function (error) { + users.update(user1, { role: users.ROLE_USER }, { ip: '1.2.3.4', userId: 'someuserid' }, function (error) { expect(error).to.eql(null); - user1.admin = false; + user1.role = users.ROLE_USER; checkMails(1, done); }); }); @@ -800,7 +802,7 @@ describe('User', function () { after(cleanupUsers); it('succeeds for one admins', function (done) { - users.getAllAdmins(function (error, admins) { + users.getAdmins(function (error, admins) { expect(error).to.eql(null); expect(admins.length).to.equal(1); expect(admins[0].username).to.equal(USERNAME.toLowerCase()); @@ -813,20 +815,19 @@ describe('User', function () { username: 'seconduser', password: 'Adfasdkjf#$%43', email: 'some@thi.ng', - admin: false + role: users.ROLE_ADMIN }; - var invitor = { username: USERNAME, email: EMAIL }; - users.create(user1.username, user1.password, user1.email, DISPLAY_NAME, { invitor: invitor }, AUDIT_SOURCE, function (error, result) { - expect(error).to.eql(null); + users.create(user1.username, user1.password, user1.email, DISPLAY_NAME, { role: users.ROLE_ADMIN }, AUDIT_SOURCE, function (error, result) { + expect(error).to.not.be.ok(); expect(result).to.be.ok(); user1.id = result.id; - users.update(user1, { admin: true }, AUDIT_SOURCE, function (error) { + users.update(user1, { role: users.ROLE_ADMIN }, AUDIT_SOURCE, function (error) { expect(error).to.eql(null); - users.getAllAdmins(function (error, admins) { + users.getAdmins(function (error, admins) { expect(error).to.eql(null); expect(admins.length).to.equal(2); expect(admins[0].username).to.equal(USERNAME.toLowerCase()); diff --git a/src/updatechecker.js b/src/updatechecker.js index 2b469442b..f3616e3fc 100644 --- a/src/updatechecker.js +++ b/src/updatechecker.js @@ -122,7 +122,7 @@ function checkAppUpdates(callback) { if (notificationPending.length === 0) return callback(); - users.getAllAdmins(function (error, admins) { + users.getAdmins(function (error, admins) { if (error) { console.error(error); return callback(); diff --git a/src/userdb.js b/src/userdb.js index 7bba7e109..ff3cc3b5a 100644 --- a/src/userdb.js +++ b/src/userdb.js @@ -6,10 +6,9 @@ exports = module.exports = { getByEmail: getByEmail, getByAccessToken: getByAccessToken, getByResetToken: getByResetToken, - getOwner: getOwner, getAllWithGroupIds: getAllWithGroupIds, getAllWithGroupIdsPaged: getAllWithGroupIdsPaged, - getAllAdmins: getAllAdmins, + getByRole: getByRole, add: add, del: del, update: update, @@ -27,11 +26,10 @@ var assert = require('assert'), BoxError = require('./boxerror.js'), database = require('./database.js'), debug = require('debug')('box:userdb'), - mysql = require('mysql'), - safe = require('safetydance'); + mysql = require('mysql'); var USERS_FIELDS = [ 'id', 'username', 'email', 'fallbackEmail', 'password', 'salt', 'createdAt', 'modifiedAt', 'resetToken', 'displayName', - 'twoFactorAuthenticationEnabled', 'twoFactorAuthenticationSecret', 'admin', 'active', 'source', 'permissionsJson' ].join(','); + 'twoFactorAuthenticationEnabled', 'twoFactorAuthenticationSecret', 'active', 'source', 'role' ].join(','); var APP_PASSWORD_FIELDS = [ 'id', 'name', 'userId', 'identifier', 'hashedPassword', 'creationTime' ].join(','); @@ -39,10 +37,7 @@ function postProcess(result) { assert.strictEqual(typeof result, 'object'); result.twoFactorAuthenticationEnabled = !!result.twoFactorAuthenticationEnabled; - result.admin = !!result.admin; result.active = !!result.active; - result.permissions = safe.JSON.parse(result.permissionsJson); - delete result.permissionsJson; return result; } @@ -83,18 +78,6 @@ function getByEmail(email, callback) { }); } -function getOwner(callback) { - assert.strictEqual(typeof callback, 'function'); - - // the first created user it the 'owner' - database.query('SELECT ' + USERS_FIELDS + ' FROM users WHERE admin=1 ORDER BY createdAt LIMIT 1', function (error, result) { - if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); - if (result.length === 0) return callback(new BoxError(BoxError.NOT_FOUND, 'User not found')); - - callback(null, postProcess(result[0])); - }); -} - function getByResetToken(resetToken, callback) { assert.strictEqual(typeof resetToken, 'string'); assert.strictEqual(typeof callback, 'function'); @@ -160,12 +143,14 @@ function getAllWithGroupIdsPaged(search, page, perPage, callback) { }); } -function getAllAdmins(callback) { +function getByRole(role, callback) { + assert.strictEqual(typeof role, 'string'); assert.strictEqual(typeof callback, 'function'); // the mailer code relies on the first object being the 'owner' (thus the ORDER) - database.query('SELECT ' + USERS_FIELDS + ' FROM users WHERE admin=1 ORDER BY createdAt', function (error, results) { + database.query('SELECT ' + USERS_FIELDS + ' FROM users WHERE role=? ORDER BY createdAt', [ role ], function (error, results) { if (error) return callback(new BoxError(BoxError.DATABASE_ERROR, error)); + if (results.length === 0) return callback(new BoxError(BoxError.NOT_FOUND, 'User not found')); results.forEach(postProcess); @@ -184,15 +169,12 @@ function add(userId, user, callback) { assert.strictEqual(typeof user.modifiedAt, 'string'); assert.strictEqual(typeof user.resetToken, 'string'); 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 user.role, 'string'); assert.strictEqual(typeof callback, 'function'); - 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 ]; + const query = 'INSERT INTO users (id, username, password, email, fallbackEmail, salt, createdAt, modifiedAt, resetToken, displayName, source, role) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)'; + const args = [ userId, user.username, user.password, user.email, user.fallbackEmail, user.salt, user.createdAt, user.modifiedAt, user.resetToken, user.displayName, user.source, user.role ]; 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')); @@ -255,18 +237,15 @@ function update(userId, user, callback) { assert(!('email' in user) || (typeof user.email === 'string')); assert(!('fallbackEmail' in user) || (typeof user.fallbackEmail === 'string')); assert(!('twoFactorAuthenticationEnabled' in user) || (typeof user.twoFactorAuthenticationEnabled === 'boolean')); - assert(!('admin' in user) || (typeof user.admin === 'boolean')); + assert(!('role' in user) || (typeof user.role === 'string')); assert(!('active' in user) || (typeof user.active === 'boolean')); var args = [ ]; var fields = [ ]; for (var k in user) { - if (k === 'twoFactorAuthenticationEnabled' || k === 'admin' || k === 'active') { + if (k === 'twoFactorAuthenticationEnabled' || k === 'active') { fields.push(k + ' = ?'); args.push(user[k] ? 1 : 0); - } else if (k === 'permissions') { - fields.push(`${k}Json = ?`); - args.push(JSON.stringify(user[k])); } else { fields.push(k + ' = ?'); args.push(user[k]); diff --git a/src/users.js b/src/users.js index f2bd41a20..de2acdabc 100644 --- a/src/users.js +++ b/src/users.js @@ -15,7 +15,7 @@ exports = module.exports = { get: get, getByResetToken: getByResetToken, getByUsername: getByUsername, - getAllAdmins: getAllAdmins, + getAdmins: getAdmins, resetPasswordByIdentifier: resetPasswordByIdentifier, setPassword: setPassword, update: updateUser, @@ -34,14 +34,21 @@ exports = module.exports = { AP_SFTP: 'sftp', AP_WEBADMIN: 'webadmin', + ROLE_ADMIN: 'admin', + ROLE_USER: 'user', + ROLE_USER_MANAGER: 'usermanager', + ROLE_OWNER: 'owner', + compareRoles: compareRoles, + getAppPasswords: getAppPasswords, getAppPassword: getAppPassword, addAppPassword: addAppPassword, delAppPassword: delAppPassword }; -let accesscontrol = require('./accesscontrol.js'), - assert = require('assert'), +const ORDERED_ROLES = [ exports.ROLE_USER, exports.ROLE_USER_MANAGER, exports.ROLE_ADMIN, exports.ROLE_OWNER ]; + +let assert = require('assert'), BoxError = require('./boxerror.js'), crypto = require('crypto'), constants = require('./constants.js'), @@ -116,7 +123,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', 'permissions'); + return _.pick(user, 'id', 'username', 'email', 'fallbackEmail', 'displayName', 'groupIds', 'active', 'source', 'role'); } // remove all fields that Non-privileged users must not see @@ -132,11 +139,8 @@ function create(username, password, email, displayName, options, auditSource, ca assert(options && typeof options === 'object'); assert(auditSource && typeof auditSource === 'object'); - const isOwner = !!options.owner; - const isAdmin = !!options.admin; const source = options.source || ''; // empty is local user - const invitor = options.invitor || null; - const permissions = options.permissions || null; + const role = options.role || exports.ROLE_USER; var error; @@ -160,7 +164,7 @@ function create(username, password, email, displayName, options, auditSource, ca error = validateDisplayName(displayName); if (error) return callback(error); - error = accesscontrol.validatePermissions(permissions); + error = validateRole(role); if (error) return callback(error); crypto.randomBytes(CRYPTO_SALT_SIZE, function (error, salt) { @@ -181,21 +185,18 @@ function create(username, password, email, displayName, options, auditSource, ca modifiedAt: now, resetToken: '', displayName: displayName, - admin: isOwner || isAdmin, source: source, - permissions: permissions + role: role }; userdb.add(user.id, user, function (error) { if (error) return callback(error); // when this is used to create the owner, then we have to patch the auditSource to contain himself - if (isOwner) { - auditSource.userId = user.id; - auditSource.username = user.username; - } + if (!auditSource.userId) auditSource.userId = user.id; + if (!auditSource.username) auditSource.username= user.username; - eventlog.add(eventlog.ACTION_USER_ADD, auditSource, { userId: user.id, email: user.email, user: removePrivateFields(user), invitor: invitor }); + eventlog.add(eventlog.ACTION_USER_ADD, auditSource, { userId: user.id, email: user.email, user: removePrivateFields(user) }); callback(null, user); }); @@ -413,7 +414,7 @@ function updateUser(user, data, auditSource, callback) { assert.strictEqual(typeof callback, 'function'); var error; - data = _.pick(data, 'email', 'fallbackEmail', 'displayName', 'username', 'admin', 'active', 'permissions'); + data = _.pick(data, 'email', 'fallbackEmail', 'displayName', 'username', 'active', 'role'); if (_.isEmpty(data)) return callback(); @@ -435,8 +436,8 @@ function updateUser(user, data, auditSource, callback) { if (error) return callback(error); } - if (data.permissions) { - error = accesscontrol.validatePermissions(data.permissions); + if (data.role) { + error = validateRole(data.role); if (error) return callback(error); } @@ -448,7 +449,7 @@ function updateUser(user, data, auditSource, callback) { eventlog.add(eventlog.ACTION_USER_UPDATE, auditSource, { userId: user.id, user: removePrivateFields(newUser), - adminStatusChanged: ((newUser.admin && !user.admin) || (!newUser.admin && user.admin)), + roleChanged: newUser.role !== user.role, activeStatusChanged: ((newUser.active && !user.active) || (!newUser.active && user.active)) }); @@ -468,13 +469,18 @@ function setMembership(user, groupIds, callback) { }); } -function getAllAdmins(callback) { +function getAdmins(callback) { assert.strictEqual(typeof callback, 'function'); - userdb.getAllAdmins(function (error, admins) { + userdb.getByRole(exports.ROLE_OWNER, function (error, owners) { if (error) return callback(error); - callback(null, admins); + userdb.getByRole(exports.ROLE_ADMIN, function (error, admins) { + if (error && error.reason === BoxError.NOT_FOUND) return callback(null, owners); + if (error) return callback(error); + + callback(null, owners.concat(admins)); + }); }); } @@ -542,11 +548,11 @@ function createOwner(username, password, email, displayName, auditSource, callba // This is only not allowed for the owner if (username === '') return callback(new BoxError(BoxError.BAD_FIELD, 'Username cannot be empty')); - count(function (error, count) { + isActivated(function (error, activated) { if (error) return callback(error); - if (count !== 0) return callback(new BoxError(BoxError.ALREADY_EXISTS, 'Owner already exists')); + if (activated) return callback(new BoxError(BoxError.ALREADY_EXISTS, 'Cloudron already activated')); - create(username, password, email, displayName, { owner: true }, auditSource, function (error, user) { + create(username, password, email, displayName, { role: exports.ROLE_OWNER }, auditSource, function (error, user) { if (error) return callback(error); callback(null, user); @@ -555,10 +561,10 @@ function createOwner(username, password, email, displayName, auditSource, callba } function getOwner(callback) { - userdb.getOwner(function (error, owner) { + userdb.getByRole(exports.ROLE_OWNER, function (error, results) { if (error) return callback(error); - return callback(null, owner); + return callback(null, results[0]); }); } @@ -655,6 +661,24 @@ function disableTwoFactorAuthentication(userId, callback) { }); } +function validateRole(role) { + assert.strictEqual(typeof role, 'string'); + + if (ORDERED_ROLES.indexOf(role) !== -1) return null; + + return new BoxError(BoxError.BAD_FIELD, `Invalid role '${role}'`); +} + +function compareRoles(role1, role2) { + assert.strictEqual(typeof role1, 'string'); + assert.strictEqual(typeof role2, 'string'); + + let roleInt1 = ORDERED_ROLES.indexOf(role1); + let roleInt2 = ORDERED_ROLES.indexOf(role2); + + return roleInt1 - roleInt2; +} + function validateAppPasswordName(name) { assert.strictEqual(typeof name, 'string');