Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

User Permissions: Edit, Add, Destroy & Role management #3395

Merged
merged 4 commits into from
Jul 28, 2014
Merged

User Permissions: Edit, Add, Destroy & Role management #3395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4e3b21b
Permissions Improvements
ErisDS Jul 23, 2014
e7dc51d
Improving error handling
ErisDS Jul 24, 2014
7714dc6
Adding role API tests & fixing browse
ErisDS Jul 24, 2014
987e927
User edit, add & destroy perms restricted by role
ErisDS Jul 24, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
113 changes: 66 additions & 47 deletions core/server/api/users.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
},

Expand Down Expand Up @@ -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.'));
});
};

return dataProvider.User.edit(checkedUserData.users[0], options);
}).then(function (result) {
if (result) {
return { users: [result.toJSON()]};
// 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 when.reject(new errors.NotFoundError('User not found.'));
});
}, function () {
return when.reject(new errors.NoPermissionError('You do not have permission to edit this user.'));
});
},

/**
* ### 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;
});
return editOperation();

});
}, function () {
return when.reject(new errors.NoPermissionError('You do not have permission to remove the user.'));
}).catch(function (error) {
return errors.handleAPIError(error);
});
},

Expand All @@ -137,30 +132,35 @@ 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);
newUser.status = 'invited';
} 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);
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand All @@ -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));
});
});
Expand Down
4 changes: 2 additions & 2 deletions core/server/api/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ utils = {
*/
checkObject: function (object, docName) {
if (_.isEmpty(object) || _.isEmpty(object[docName]) || _.isEmpty(object[docName][0])) {
return when.reject(new errors.BadRequestError('No root key (\'' + docName + '\') provided.'));
return errors.logAndRejectError(new errors.BadRequestError('No root key (\'' + docName + '\') provided.'));
}

// convert author property to author_id to match the name in the database
Expand All @@ -27,7 +27,7 @@ utils = {
delete object.posts[0].author;
}
}
return when.resolve(object);
return when(object);
}
};

Expand Down
45 changes: 35 additions & 10 deletions core/server/errors/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ errors = {

throwError: function (err) {
if (!err) {
err = new Error("An error occurred");
err = new Error('An error occurred');
}

if (_.isString(err)) {
Expand Down Expand Up @@ -137,7 +137,7 @@ errors = {
logAndRejectError: function (err, context, help) {
this.logError(err, context, help);

this.rejectError(err, context, help);
return this.rejectError(err, context, help);
},

logErrorWithRedirect: function (msg, context, help, redirectTo, req, res) {
Expand All @@ -153,6 +153,22 @@ errors = {
};
},

handleAPIError: function (error) {
if (!error) {
return this.rejectError(new this.NoPermissionError('You do not have permission to perform this action'));
}

if (_.isString(error)) {
return this.rejectError(new this.NoPermissionError(error));
}

if (error.type) {
return this.rejectError(error);
}

return this.rejectError(new this.InternalServerError(error));
},

renderErrorPage: function (code, err, req, res, next) {
/*jshint unused:false*/
var self = this;
Expand Down Expand Up @@ -207,17 +223,18 @@ errors = {

// And then try to explain things to the user...
// Cheat and output the error using handlebars escapeExpression
return res.send(500, "<h1>Oops, seems there is an an error in the error template.</h1>"
+ "<p>Encountered the error: </p>"
+ "<pre>" + hbs.handlebars.Utils.escapeExpression(templateErr.message || templateErr) + "</pre>"
+ "<br ><p>whilst trying to render an error page for the error: </p>"
+ code + " " + "<pre>" + hbs.handlebars.Utils.escapeExpression(err.message || err) + "</pre>"
);
return res.send(500,
'<h1>Oops, seems there is an an error in the error template.</h1>' +
'<p>Encountered the error: </p>' +
'<pre>' + hbs.handlebars.Utils.escapeExpression(templateErr.message || templateErr) + '</pre>' +
'<br ><p>whilst trying to render an error page for the error: </p>' +
code + ' ' + '<pre>' + hbs.handlebars.Utils.escapeExpression(err.message || err) + '</pre>'
);
});
}

if (code >= 500) {
this.logError(err, "Rendering Error Page", "Ghost caught a processing error in the middleware layer.");
this.logError(err, 'Rendering Error Page', 'Ghost caught a processing error in the middleware layer.');
}

// Are we admin? If so, don't worry about the user template
Expand All @@ -230,7 +247,7 @@ errors = {
},

error404: function (req, res, next) {
var message = res.isAdmin && req.user ? "No Ghost Found" : "Page Not Found";
var message = res.isAdmin && req.user ? 'No Ghost Found' : 'Page Not Found';

// do not cache 404 error
res.set({'Cache-Control': 'no-cache, private, no-store, must-revalidate, max-stale=0, post-check=0, pre-check=0'});
Expand Down Expand Up @@ -271,8 +288,16 @@ errors = {
// Ensure our 'this' context for methods and preserve method arity by
// using Function#bind for expressjs
_.each([
'logWarn',
'logInfo',
'rejectError',
'throwError',
'logError',
'logAndThrowError',
'logAndRejectError',
'logErrorAndExit',
'logErrorWithRedirect',
'handleAPIError',
'renderErrorPage',
'error404',
'error500'
Expand Down
4 changes: 2 additions & 2 deletions core/server/models/post.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,7 @@ Post = ghostBookshelf.Model.extend({
});
},

permissable: function (postModelOrId, context, loadedPermissions, hasUserPermission, hasAppPermission) {
permissible: function (postModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
postModel = postModelOrId,
origArgs;
Expand All @@ -536,7 +536,7 @@ Post = ghostBookshelf.Model.extend({
// Build up the original args but substitute with actual model
var newArgs = [foundPostModel].concat(origArgs);

return self.permissable.apply(self, newArgs);
return self.permissible.apply(self, newArgs);
}, errors.logAndThrowError);
}

Expand Down
23 changes: 10 additions & 13 deletions core/server/models/role.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ Role = ghostBookshelf.Model.extend({
},


permissable: function (roleModelOrId, context, loadedPermissions, hasUserPermission, hasAppPermission) {
permissible: function (roleModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
checkAgainst = [],
origArgs;
Expand All @@ -55,23 +55,20 @@ Role = ghostBookshelf.Model.extend({
// Build up the original args but substitute with actual model
var newArgs = [foundRoleModel].concat(origArgs);

return self.permissable.apply(self, newArgs);
return self.permissible.apply(self, newArgs);
}, errors.logAndThrowError);
}

switch (loadedPermissions.user) {
case 'Owner':
case 'Administrator':
if (action === 'assign' && loadedPermissions.user) {
if (_.any(loadedPermissions.user.roles, { 'name': 'Owner' }) ||
_.any(loadedPermissions.user.roles, { 'name': 'Administrator' })) {
checkAgainst = ['Administrator', 'Editor', 'Author'];
break;
case 'Editor':
checkAgainst = ['Editor', 'Author'];
}
} else if (_.any(loadedPermissions.user.roles, { 'name': 'Editor' })) {
checkAgainst = ['Author'];
}

// If we have a role passed into here
if (roleModelOrId && !_.contains(checkAgainst, roleModelOrId.get('name'))) {
// Role not in the list of permissible roles
hasUserPermission = false;
// Role in the list of permissible roles
hasUserPermission = roleModelOrId && _.contains(checkAgainst, roleModelOrId.get('name'));
}

if (hasUserPermission && hasAppPermission) {
Expand Down
Loading