diff --git a/src/accesscontrol.js b/src/accesscontrol.js index 122cfbde0..5d0a1c69a 100644 --- a/src/accesscontrol.js +++ b/src/accesscontrol.js @@ -9,7 +9,8 @@ exports = module.exports = { SCOPE_MAIL: 'mail', SCOPE_PROFILE: 'profile', SCOPE_SETTINGS: 'settings', - SCOPE_USERS: 'users', + SCOPE_USERS_READ: 'users:read', + SCOPE_USERS_MANAGE: 'users:manage', SCOPE_APPSTORE: 'appstore', VALID_SCOPES: [ 'apps', 'appstore', 'clients', 'cloudron', 'domains', 'mail', 'profile', 'settings', 'users' ], @@ -33,7 +34,7 @@ const ROLE_DEFINITIONS = { scopes: exports.VALID_SCOPES }, 'manage_apps': { - scopes: [ 'apps', 'domains:read', 'users' ] + scopes: [ 'apps', 'domains:read', 'users:read' ] }, 'manage_users': { scopes: [ 'users' ] diff --git a/src/routes/groups.js b/src/routes/groups.js index 5048b3a3b..9b99d6cfc 100644 --- a/src/routes/groups.js +++ b/src/routes/groups.js @@ -87,7 +87,7 @@ function updateMembers(req, res, next) { } function list(req, res, next) { - groups.getAllWithMembers(function (error, result) { + groups.getAll(function (error, result) { if (error) return next(new HttpError(500, error)); next(new HttpSuccess(200, { groups: result })); diff --git a/src/routes/test/groups-test.js b/src/routes/test/groups-test.js index 7b692d014..d5006b1aa 100644 --- a/src/routes/test/groups-test.js +++ b/src/routes/test/groups-test.js @@ -114,9 +114,7 @@ describe('Groups API', function () { expect(res.body.groups).to.be.an(Array); expect(res.body.groups.length).to.be(1); expect(res.body.groups[0].name).to.eql('admin'); - expect(res.body.groups[0].userIds).to.be.an(Array); - expect(res.body.groups[0].userIds.length).to.be(1); - expect(res.body.groups[0].userIds[0]).to.be(userId); + expect(res.body.groups[0].userIds).to.not.be.ok(); done(); }); }); diff --git a/src/routes/test/users-test.js b/src/routes/test/users-test.js index 61117f071..0fc618fe2 100644 --- a/src/routes/test/users-test.js +++ b/src/routes/test/users-test.js @@ -338,7 +338,7 @@ describe('Users API', function () { }); }); - it('list groupIds when listing users', function (done) { + it('does not list groupIds when listing users', function (done) { superagent.get(SERVER_URL + '/api/v1/users') .query({ access_token: token }) .end(function (error, res) { @@ -347,7 +347,7 @@ describe('Users API', function () { expect(res.body.users).to.be.an('array'); res.body.users.forEach(function (user) { - expect(user.groupIds).to.eql([ constants.ADMIN_GROUP_ID ]); + expect('groupIds' in user).to.be(false); }); done(); }); @@ -502,7 +502,7 @@ describe('Users API', function () { expect(user.email).to.be.ok(); expect(user.password).to.not.be.ok(); expect(user.salt).to.not.be.ok(); - expect(user.groupIds).to.be.an(Array); + expect(user.groupIds).to.not.be.ok(); }); done(); diff --git a/src/routes/users.js b/src/routes/users.js index ac0ee28ca..8c899312e 100644 --- a/src/routes/users.js +++ b/src/routes/users.js @@ -81,7 +81,7 @@ function list(req, res, next) { users.list(function (error, results) { if (error) return next(new HttpError(500, error)); - results = results.map(users.removePrivateFields); + results = results.map(users.removeRestrictedFields); next(new HttpSuccess(200, { users: results })); }); diff --git a/src/server.js b/src/server.js index 006d9cbdf..dd85e76be 100644 --- a/src/server.js +++ b/src/server.js @@ -91,7 +91,8 @@ function initializeExpressSync() { // scope middleware implicitly also adds bearer token verification var cloudronScope = routes.accesscontrol.scope(accesscontrol.SCOPE_CLOUDRON); var profileScope = routes.accesscontrol.scope(accesscontrol.SCOPE_PROFILE); - var usersScope = routes.accesscontrol.scope(accesscontrol.SCOPE_USERS); + var usersReadScope = routes.accesscontrol.scope(accesscontrol.SCOPE_USERS_READ); + var usersManageScope = routes.accesscontrol.scope(accesscontrol.SCOPE_USERS_MANAGE); var appsScope = routes.accesscontrol.scope(accesscontrol.SCOPE_APPS); var settingsScope = routes.accesscontrol.scope(accesscontrol.SCOPE_SETTINGS); var mailScope = routes.accesscontrol.scope(accesscontrol.SCOPE_MAIL); @@ -141,21 +142,21 @@ function initializeExpressSync() { router.post('/api/v1/profile/twofactorauthentication/disable', profileScope, routes.users.verifyPassword, routes.profile.disableTwoFactorAuthentication); // user routes - router.get ('/api/v1/users', usersScope, routes.users.list); - router.post('/api/v1/users', usersScope, routes.users.create); - router.get ('/api/v1/users/:userId', usersScope, routes.users.get); - router.del ('/api/v1/users/:userId', usersScope, routes.users.verifyPassword, routes.users.remove); - router.post('/api/v1/users/:userId', usersScope, routes.users.update); - router.put ('/api/v1/users/:userId/groups', usersScope, routes.users.setGroups); - router.post('/api/v1/users/:userId/invite', usersScope, routes.users.sendInvite); + router.get ('/api/v1/users', usersReadScope, routes.users.list); + router.post('/api/v1/users', usersManageScope, routes.users.create); + router.get ('/api/v1/users/:userId', usersManageScope, routes.users.get); + router.del ('/api/v1/users/:userId', usersManageScope, routes.users.verifyPassword, routes.users.remove); + router.post('/api/v1/users/:userId', usersManageScope, routes.users.update); + router.put ('/api/v1/users/:userId/groups', usersManageScope, routes.users.setGroups); + router.post('/api/v1/users/:userId/invite', usersManageScope, routes.users.sendInvite); // Group management - router.get ('/api/v1/groups', usersScope, routes.groups.list); - router.post('/api/v1/groups', usersScope, routes.groups.create); - router.get ('/api/v1/groups/:groupId', usersScope, routes.groups.get); - router.put ('/api/v1/groups/:groupId/members', usersScope, routes.groups.updateMembers); - router.post('/api/v1/groups/:groupId', usersScope, routes.groups.update); - router.del ('/api/v1/groups/:groupId', usersScope, routes.users.verifyPassword, routes.groups.remove); + router.get ('/api/v1/groups', usersReadScope, routes.groups.list); + router.post('/api/v1/groups', usersManageScope, routes.groups.create); + router.get ('/api/v1/groups/:groupId', usersManageScope, routes.groups.get); + router.put ('/api/v1/groups/:groupId/members', usersManageScope, routes.groups.updateMembers); + router.post('/api/v1/groups/:groupId', usersManageScope, routes.groups.update); + router.del ('/api/v1/groups/:groupId', usersManageScope, routes.users.verifyPassword, routes.groups.remove); // form based login routes used by oauth2 frame router.get ('/api/v1/session/login', csrf, routes.oauth2.loginForm); diff --git a/src/users.js b/src/users.js index db2896d62..89cf0225d 100644 --- a/src/users.js +++ b/src/users.js @@ -4,6 +4,7 @@ exports = module.exports = { UsersError: UsersError, removePrivateFields: removePrivateFields, + removeRestrictedFields: removeRestrictedFields, list: list, create: create, @@ -129,10 +130,16 @@ function validatePassword(password) { return null; } +// 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'); } +// remove all fields that Non-privileged users must not see +function removeRestrictedFields(user) { + return _.pick(user, 'id', 'username', 'email', 'displayName'); +} + function create(username, password, email, displayName, options, auditSource, callback) { assert(username === null || typeof username === 'string'); assert(password === null || typeof password === 'string');