User edit, add & destroy perms restricted by role

closes #3096, closes #3378, refs #3100

- user.permissible updated to reflect proper permissions
- small amount of API refactoring to handle extra cases
- extensive integration testing
This commit is contained in:
Hannah Wolfe 2014-07-24 10:46:05 +01:00
parent 7714dc6ab1
commit 987e9277dc
5 changed files with 1118 additions and 312 deletions

View File

@ -10,7 +10,6 @@ var when = require('when'),
globalUtils = require('../utils'),
config = require('../config'),
mail = require('./mail'),
rolesAPI = require('./roles'),
docName = 'users',
ONE_DAY = 60 * 60 * 24 * 1000,
@ -44,8 +43,8 @@ users = {
options.include = prepareInclude(options.include);
}
return dataProvider.User.findPage(options);
}, function () {
return when.reject(new errors.NoPermissionError('You do not have permission to browse users.'));
}).catch(function (error) {
return errors.handleAPIError(error);
});
},
@ -84,48 +83,44 @@ users = {
* @returns {Promise(User)}
*/
edit: function edit(object, options) {
var editOperation;
if (options.id === 'me' && options.context && options.context.user) {
options.id = options.context.user;
}
return canThis(options.context).edit.user(options.id).then(function () {
// TODO: add permission check for roles
// if (data.roles) {
// return canThis(options.context).assign.role(<role-id>)
// }
// }.then(function (){
return utils.checkObject(object, docName).then(function (checkedUserData) {
if (options.include) {
options.include = prepareInclude(options.include);
}
if (options.include) {
options.include = prepareInclude(options.include);
return utils.checkObject(object, docName).then(function (data) {
// Edit operation
editOperation = function () {
return dataProvider.User.edit(data.users[0], options)
.then(function (result) {
if (result) {
return { users: [result.toJSON()]};
}
return when.reject(new errors.NotFoundError('User not found.'));
});
};
// Check permissions
return canThis(options.context).edit.user(options.id).then(function () {
if (data.users[0].roles) {
if (options.id === options.context.user) {
return when.reject(new errors.NoPermissionError('You cannot change your own role.'));
}
return canThis(options.context).assign.role(data.users[0].roles[0]).then(function () {
return editOperation();
});
}
return dataProvider.User.edit(checkedUserData.users[0], options);
}).then(function (result) {
if (result) {
return { users: [result.toJSON()]};
}
return when.reject(new errors.NotFoundError('User not found.'));
});
}, function () {
return when.reject(new errors.NoPermissionError('You do not have permission to edit this user.'));
});
},
return editOperation();
/**
* ### Destroy
* @param {{id, context}} options
* @returns {Promise(User)}
*/
destroy: function destroy(options) {
return canThis(options.context).destroy.user(options.id).then(function () {
return users.read(options).then(function (result) {
return dataProvider.User.destroy(options).then(function () {
return result;
});
});
}, function () {
return when.reject(new errors.NoPermissionError('You do not have permission to remove the user.'));
}).catch(function (error) {
return errors.handleAPIError(error);
});
},
@ -137,23 +132,26 @@ users = {
*/
add: function add(object, options) {
var newUser,
user;
user,
roleId;
return canThis(options.context).add.user().then(function () {
return canThis(options.context).add.user(object).then(function () {
return utils.checkObject(object, docName).then(function (checkedUserData) {
if (options.include) {
options.include = prepareInclude(options.include);
}
newUser = checkedUserData.users[0];
newUser.role = parseInt(newUser.roles[0].id || newUser.roles[0], 10);
roleId = parseInt(newUser.roles[0].id || newUser.roles[0], 10);
return rolesAPI.browse({ context: options.context, permissions: 'assign' }).then(function (results) {
// Make sure user is allowed to add a user with this role
if (!_.any(results.roles, { id: newUser.role })) {
return when.reject(new errors.NoPermissionError('Not allowed to create user with that role.'));
// Make sure user is allowed to add a user with this role
return dataProvider.Role.findOne({id: roleId}).then(function (role) {
if (role.get('name') === 'Owner') {
return when.reject(new errors.NoPermissionError('Not allowed to create an owner user.'));
}
return canThis(options.context).assign.role(role);
}).then(function () {
if (newUser.email) {
newUser.name = object.users[0].email.substring(0, newUser.email.indexOf('@'));
newUser.password = globalUtils.uid(50);
@ -161,6 +159,8 @@ users = {
} else {
return when.reject(new errors.BadRequestError('No email provided.'));
}
}).catch(function () {
return when.reject(new errors.NoPermissionError('Not allowed to create user with that role.'));
});
}).then(function () {
return dataProvider.User.getByEmail(newUser.email);
@ -208,7 +208,7 @@ users = {
});
}).then(function () {
return when.resolve({users: [user]});
}).otherwise(function (error) {
}).catch(function (error) {
if (error && error.type === 'EmailError') {
error.message = 'Error sending email: ' + error.message + ' Please check your email settings and resend the invitation.';
errors.logWarn(error.message);
@ -222,11 +222,30 @@ users = {
}
return when.reject(error);
});
}, function () {
return when.reject(new errors.NoPermissionError('You do not have permission to add a user.'));
}).catch(function (error) {
return errors.handleAPIError(error);
});
},
/**
* ### Destroy
* @param {{id, context}} options
* @returns {Promise(User)}
*/
destroy: function destroy(options) {
return canThis(options.context).destroy.user(options.id).then(function () {
return users.read(options).then(function (result) {
return dataProvider.User.destroy(options).then(function () {
return result;
});
});
}).catch(function (error) {
return errors.handleAPIError(error);
});
},
/**
* ### Change Password
* @param {password} object
@ -244,7 +263,7 @@ users = {
return dataProvider.User.changePassword(oldPassword, newPassword, ne2Password, options).then(function () {
return when.resolve({password: [{message: 'Password changed successfully.'}]});
}).otherwise(function (error) {
}).catch(function (error) {
return when.reject(new errors.ValidationError(error.message));
});
});

View File

@ -93,6 +93,14 @@ User = ghostBookshelf.Model.extend({
permissions: function () {
return this.belongsToMany('Permission');
},
hasRole: function (roleName) {
var roles = this.related('roles');
return roles.some(function (role) {
return role.get('name') === roleName;
});
}
}, {
@ -183,7 +191,9 @@ User = ghostBookshelf.Model.extend({
if (options.status && options.status !== 'all') {
// make sure that status is valid
//TODO: need a better way of getting a list of statuses other than hard-coding them...
options.status = _.indexOf(['active', 'warn-1', 'warn-2', 'warn-3', 'locked', 'invited'], options.status) !== -1 ? options.status : 'active';
options.status = _.indexOf(
['active', 'warn-1', 'warn-2', 'warn-3', 'locked', 'invited'],
options.status) !== -1 ? options.status : 'active';
options.where.status = options.status;
}
@ -300,8 +310,6 @@ User = ghostBookshelf.Model.extend({
*/
edit: function (data, options) {
var self = this,
adminRole,
ownerRole,
roleId;
options = options || {};
@ -312,11 +320,10 @@ User = ghostBookshelf.Model.extend({
if (data.roles) {
roleId = parseInt(data.roles[0].id || data.roles[0], 10);
if (user.id === options.context.user) {
return when.reject(new errors.ValidationError('You are not allowed to assign a new role to yourself'));
}
if (data.roles.length > 1) {
return when.reject(new errors.ValidationError('Only one role per user is supported at the moment.'));
return when.reject(
new errors.ValidationError('Only one role per user is supported at the moment.')
);
}
return user.roles().fetch().then(function (roles) {
@ -325,27 +332,9 @@ User = ghostBookshelf.Model.extend({
return user;
}
return Role.findOne({id: roleId});
}).then(function (role) {
if (role && role.get('name') === 'Owner') {
// Get admin and owner role
return Role.findOne({name: 'Administrator'}).then(function (result) {
adminRole = result;
return Role.findOne({name: 'Owner'});
}).then(function (result) {
ownerRole = result;
return User.findOne({id: options.context.user});
}).then(function (contextUser) {
// check if user has the owner role
var currentRoles = contextUser.toJSON().roles;
if (!_.contains(currentRoles, ownerRole.id)) {
return when.reject(new errors.ValidationError('Only owners are able to transfer the owner role.'));
}
// convert owner to admin
return contextUser.roles().updatePivot({role_id: adminRole.id});
}).then(function () {
// assign owner role to a new user
return user.roles().updatePivot({role_id: ownerRole.id});
});
}).then(function (roleToAssign) {
if (roleToAssign && roleToAssign.get('name') === 'Owner') {
return self.transferOwnership(user, roleToAssign, options.context);
} else {
// assign all other roles
return user.roles().updatePivot({role_id: roleId});
@ -371,7 +360,18 @@ User = ghostBookshelf.Model.extend({
add: function (data, options) {
var self = this,
// Clone the _user so we don't expose the hashed password unnecessarily
userData = this.filterData(data);
userData = this.filterData(data),
// Get the role we're going to assign to this user, or the author role if there isn't one
// TODO: don't reference Author role by ID!
roles = data.roles || [1];
// remove roles from the object
delete data.roles;
// check for too many roles
if (roles.length > 1) {
return when.reject(new errors.ValidationError('Only one role per user is supported at the moment.'));
}
options = this.filterOptions(options, 'add');
options.withRelated = _.union([ 'roles' ], options.include);
@ -392,13 +392,9 @@ User = ghostBookshelf.Model.extend({
}).then(function (addedUser) {
// Assign the userData to our created user so we can pass it back
userData = addedUser;
if (!data.role) {
// TODO: needs change when owner role is introduced and setup is changed
data.role = 1;
}
return userData.roles().attach(data.role);
}).then(function (addedUserRole) {
/*jshint unused:false*/
return userData.roles().attach(roles);
}).then(function () {
// find and return the added user
return self.findOne({id: userData.id}, options);
});
@ -435,9 +431,9 @@ User = ghostBookshelf.Model.extend({
userModel = userModelOrId,
origArgs;
// If we passed in an id instead of a model, get the model
// then check the permissions
// If we passed in an id instead of a model, get the model then check the permissions
if (_.isNumber(userModelOrId) || _.isString(userModelOrId)) {
// Grab the original args without the first one
origArgs = _.toArray(arguments).slice(1);
// Get the actual post model
@ -449,9 +445,37 @@ User = ghostBookshelf.Model.extend({
}, errors.logAndThrowError);
}
if (userModel) {
// If this is the same user that requests the operation allow it.
hasUserPermission = hasUserPermission || context.user === userModel.get('id');
if (action === 'edit') {
// Users with the role 'Editor' and 'Author' have complex permissions when the action === 'edit'
// We now have all the info we need to construct the permissions
if (_.any(loadedPermissions.user.roles, { 'name': 'Author' })) {
// If this is the same user that requests the operation allow it.
hasUserPermission = hasUserPermission || context.user === userModel.get('id');
}
if (_.any(loadedPermissions.user.roles, { 'name': 'Editor' })) {
// If this is the same user that requests the operation allow it.
hasUserPermission = context.user === userModel.get('id');
// Alternatively, if the user we are trying to edit is an Author, allow it
hasUserPermission = hasUserPermission || userModel.hasRole('Author');
}
}
if (action === 'destroy') {
// Owner cannot be deleted EVER
if (userModel.hasRole('Owner')) {
return when.reject();
}
// Users with the role 'Editor' have complex permissions when the action === 'destroy'
if (_.any(loadedPermissions.user.roles, { 'name': 'Editor' })) {
// If this is the same user that requests the operation allow it.
hasUserPermission = context.user === userModel.get('id');
// Alternatively, if the user we are trying to edit is an Author, allow it
hasUserPermission = hasUserPermission || userModel.hasRole('Author');
}
}
if (hasUserPermission && hasAppPermission) {
@ -490,8 +514,9 @@ User = ghostBookshelf.Model.extend({
if (!user) {
return when.reject(new errors.NotFoundError('There is no user with that email address.'));
}
if (user.get('status') === 'invited' || user.get('status') === 'invited-pending'
|| user.get('status') === 'inactive') {
if (user.get('status') === 'invited' || user.get('status') === 'invited-pending' ||
user.get('status') === 'inactive'
) {
return when.reject(new Error('The user with that email address is inactive.'));
}
if (user.get('status') !== 'locked') {
@ -588,24 +613,25 @@ User = ghostBookshelf.Model.extend({
// Check if invalid structure
if (!parts || parts.length !== 3) {
return when.reject(new Error("Invalid token structure"));
return when.reject(new Error('Invalid token structure'));
}
expires = parseInt(parts[0], 10);
email = parts[1];
if (isNaN(expires)) {
return when.reject(new Error("Invalid token expiration"));
return when.reject(new Error('Invalid token expiration'));
}
// Check if token is expired to prevent replay attacks
if (expires < Date.now()) {
return when.reject(new Error("Expired token"));
return when.reject(new Error('Expired token'));
}
// to prevent brute force attempts to reset the password the combination of email+expires is only allowed for 10 attempts
// to prevent brute force attempts to reset the password the combination of email+expires is only allowed for
// 10 attempts
if (tokenSecurity[email + '+' + expires] && tokenSecurity[email + '+' + expires].count >= 10) {
return when.reject(new Error("Token locked"));
return when.reject(new Error('Token locked'));
}
return this.generateResetToken(email, expires, dbHash).then(function (generatedToken) {
@ -627,8 +653,10 @@ User = ghostBookshelf.Model.extend({
}
// increase the count for email+expires for each failed attempt
tokenSecurity[email + '+' + expires] = {count: tokenSecurity[email + '+' + expires] ? tokenSecurity[email + '+' + expires].count + 1 : 1};
return when.reject(new Error("Invalid token"));
tokenSecurity[email + '+' + expires] = {
count: tokenSecurity[email + '+' + expires] ? tokenSecurity[email + '+' + expires].count + 1 : 1
};
return when.reject(new Error('Invalid token'));
});
},
@ -636,7 +664,7 @@ User = ghostBookshelf.Model.extend({
var self = this;
if (newPassword !== ne2Password) {
return when.reject(new Error("Your new passwords do not match"));
return when.reject(new Error('Your new passwords do not match'));
}
return validatePasswordLength(newPassword).then(function () {
@ -657,10 +685,30 @@ User = ghostBookshelf.Model.extend({
});
},
transferOwnership: function (user, ownerRole, context) {
var adminRole;
// Get admin role
return Role.findOne({name: 'Administrator'}).then(function (result) {
adminRole = result;
return User.findOne({id: context.user});
}).then(function (contextUser) {
// check if user has the owner role
var currentRoles = contextUser.toJSON().roles;
if (!_.contains(currentRoles, ownerRole.id)) {
return when.reject(new errors.NoPermissionError('Only owners are able to transfer the owner role.'));
}
// convert owner to admin
return contextUser.roles().updatePivot({role_id: adminRole.id});
}).then(function () {
// assign owner role to a new user
return user.roles().updatePivot({role_id: ownerRole.id});
});
},
gravatarLookup: function (userData) {
var gravatarUrl = '//www.gravatar.com/avatar/' +
crypto.createHash('md5').update(userData.email.toLowerCase().trim()).digest('hex') +
"?d=404&s=250",
'?d=404&s=250',
checkPromise = when.defer();
http.get('http:' + gravatarUrl, function (res) {
@ -675,7 +723,6 @@ User = ghostBookshelf.Model.extend({
return checkPromise.promise;
},
// Get the user by email address, enforces case insensitivity rejects if the user is not found
// When multi-user support is added, email addresses must be deduplicated with case insensitivity, so that
// joe@bloggs.com and JOE@BLOGGS.COM cannot be created as two separate users.

File diff suppressed because it is too large Load Diff

View File

@ -158,20 +158,20 @@ DataGenerator.Content = {
roles: [
{
"name": "Administrator",
"description": "Administrators"
name: 'Administrator',
description: 'Administrators'
},
{
"name": "Editor",
"description": "Editors"
name: 'Editor',
description: 'Editors'
},
{
"name": "Author",
"description": "Authors"
name: 'Author',
description: 'Authors'
},
{
"name": "Owner",
"description": "Blog Owner"
name: 'Owner',
description: 'Blog Owner'
}
],

View File

@ -141,6 +141,27 @@ fixtures = {
});
},
createExtraUsers: function createExtraUsers() {
var knex = config.database.knex,
// grab 3 more users
extraUsers = DataGenerator.Content.users.slice(2, 5);
extraUsers = _.map(extraUsers, function (user) {
return DataGenerator.forKnex.createUser(_.extend({}, user, {
email: 'a' + user.email,
slug: 'a' + user.slug
}));
});
return knex('users').insert(extraUsers).then(function () {
return knex('roles_users').insert([
{ user_id: 5, role_id: 1},
{ user_id: 6, role_id: 2},
{ user_id: 7, role_id: 3}
]);
});
},
insertOne: function insertOne(obj, fn) {
var knex = config.database.knex;
return knex(obj)
@ -164,7 +185,7 @@ fixtures = {
try {
data = JSON.parse(fileContents);
} catch (e) {
return when.reject(new Error("Failed to parse the file"));
return when.reject(new Error('Failed to parse the file'));
}
return data;
@ -244,6 +265,7 @@ toDoList = {
return settings.populateDefaults().then(function () { return SettingsAPI.updateSettingsCache(); });
},
'users:roles': function createUsersWithRoles() { return fixtures.createUsersWithRoles(); },
'users': function createExtraUsers() { return fixtures.createExtraUsers(); },
'owner': function insertOwnerUser() { return fixtures.insertOwnerUser(); },
'owner:pre': function initOwnerUser() { return fixtures.initOwnerUser(); },
'owner:post': function overrideOwnerUser() { return fixtures.overrideOwnerUser(); },
@ -376,11 +398,31 @@ module.exports = {
fork: fork,
// Helpers to make it easier to write tests which are easy to read
context: {
internal: {context: {internal: true}},
owner: {context: {user: 1}},
admin: {context: {user: 2}},
editor: {context: {user: 3}},
author: {context: {user: 4}}
},
users: {
ids: {
owner: 1,
admin: 2,
editor: 3,
author: 4,
admin2: 5,
editor2: 6,
author2: 7
}
},
roles: {
ids: {
owner: 4,
admin: 1,
editor: 2,
author: 3
}
}
};