Stopped api key from assigning the 'Owner' role (#9971)

* Stopped api key from assigning the 'Owner' role

refs #9865

We do not want api keys to be able to assign the Owner role to any other
key or user.

* Cleaned up Role model permissible method

no-issue
This commit is contained in:
Fabien O'Carroll 2018-10-12 15:38:57 +07:00 committed by GitHub
parent caccda1aab
commit 05330482e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 37 additions and 12 deletions

View File

@ -51,33 +51,29 @@ Role = ghostBookshelf.Model.extend({
},
permissible: function permissible(roleModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
checkAgainst = [],
origArgs;
// If we passed in an id instead of a model, get the model
// then check the permissions
if (_.isNumber(roleModelOrId) || _.isString(roleModelOrId)) {
// Grab the original args without the first one
origArgs = _.toArray(arguments).slice(1);
// Get the actual role model
return this.findOne({id: roleModelOrId, status: 'all'})
.then(function then(foundRoleModel) {
.then((foundRoleModel) => {
if (!foundRoleModel) {
throw new common.errors.NotFoundError({
message: common.i18n.t('errors.models.role.roleNotFound')
});
}
// Build up the original args but substitute with actual model
var newArgs = [foundRoleModel].concat(origArgs);
// Grab the original args without the first one
const origArgs = _.toArray(arguments).slice(1);
return self.permissible.apply(self, newArgs);
return this.permissible(foundRoleModel, ...origArgs);
});
}
const roleModel = roleModelOrId;
if (action === 'assign' && loadedPermissions.user) {
let checkAgainst;
if (_.some(loadedPermissions.user.roles, {name: 'Owner'})) {
checkAgainst = ['Owner', 'Administrator', 'Editor', 'Author', 'Contributor'];
} else if (_.some(loadedPermissions.user.roles, {name: 'Administrator'})) {
@ -87,7 +83,16 @@ Role = ghostBookshelf.Model.extend({
}
// Role in the list of permissible roles
hasUserPermission = roleModelOrId && _.includes(checkAgainst, roleModelOrId.get('name'));
hasUserPermission = roleModelOrId && _.includes(checkAgainst, roleModel.get('name'));
}
if (action === 'assign' && loadedPermissions.apiKey) {
// apiKey cannot 'assign' the 'Owner' role
if (roleModel.get('name') === 'Owner') {
return Promise.reject(new common.errors.NoPermissionError({
message: common.i18n.t('errors.models.role.notEnoughPermission')
}));
}
}
if (hasUserPermission && hasAppPermission) {

View File

@ -1,4 +1,5 @@
const models = require('../../../server/models');
const {NoPermissionError} = require('../../../server/lib/common/errors');
const ghostBookshelf = require('../../../server/models/base');
const testUtils = require('../../utils');
const should = require('should');
@ -25,4 +26,23 @@ describe('Unit: models/role', function () {
.then(() => checkRolePermissionsCount(0));
});
});
describe('permissible', function () {
it('does not let api key assign the owner role', function () {
return models.Role.permissible(
models.Role.forge({name: 'Owner'}), // Owner role
'assign', // assign action
{},
{},
{apiKey: {}}, // apiKey loaded permissions
true,
true,
true
).then(() => {
throw new Error('models.Role.permissible should have thrown!');
}, (err) => {
should.equal(err instanceof NoPermissionError, true);
});
});
});
});