Infrastructure/ghost
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00
Code Issues Activity

Permissions Improvements

Browse source 
refs #3083, #3096

In order to implement advanced permissions based on roles for specific
actions, we need to know
what role the current context user has and also what action we are
granting permissions for:
- Permissible gets passed the action type
- Effective permissions keeps the user role and eventually passes it to
  permissible
- Fixed spelling
- Still needs tests
This commit is contained in:
Hannah Wolfe 2014-07-23 19:17:29 +01:00
parent 6628127297
commit 4e3b21b7da
6 changed files with 32 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
core/server/models/post.js
View file

@ -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;
@ -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);
}

4
core/server/models/role.js
View file

@ -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;
@ -55,7 +55,7 @@ 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);
}

4
core/server/models/user.js
View file

@ -430,7 +430,7 @@ User = ghostBookshelf.Model.extend({
});
},
permissable: function (userModelOrId, context, loadedPermissions, hasUserPermission, hasAppPermission) {
permissible: function (userModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
userModel = userModelOrId,
origArgs;
@ -445,7 +445,7 @@ User = ghostBookshelf.Model.extend({
// Build up the original args but substitute with actual model
var newArgs = [foundUserModel].concat(origArgs);
return self.permissable.apply(self, newArgs);
return self.permissible.apply(self, newArgs);
}, errors.logAndThrowError);
}

9
core/server/permissions/effective.js
View file

@ -13,11 +13,6 @@ var effective = {
allPerms = [],
user = foundUser.toJSON();
// TODO: using 'Owner' as return value is a bit hacky.
if (_.find(user.roles, { 'name': 'Owner' })) {
return 'Owner';
}
rolePerms.push(foundUser.related('permissions').models);
_.each(rolePerms, function (rolePermGroup) {
@ -34,7 +29,7 @@ var effective = {
});
});
return allPerms;
return {permissions: allPerms, roles: user.roles};
}, errors.logAndThrowError);
},
@ -45,7 +40,7 @@ var effective = {
return [];
}
return foundApp.related('permissions').models;
return {permissions: foundApp.related('permissions').models};
}, errors.logAndThrowError);
}
};

23
core/server/permissions/index.js
View file

@ -78,8 +78,8 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (obj_types, act_type,
// Wait for the user loading to finish
return permissionLoad.then(function (loadedPermissions) {
// Iterate through the user permissions looking for an affirmation
var userPermissions = loadedPermissions.user,
appPermissions = loadedPermissions.app,
var userPermissions = loadedPermissions.user ? loadedPermissions.user.permissions : null,
appPermissions = loadedPermissions.app ? loadedPermissions.app.permissions : null,
hasUserPermission,
hasAppPermission,
checkPermission = function (perm) {
@ -105,15 +105,14 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (obj_types, act_type,
return modelId === permObjId;
};
// Check user permissions for matching action, object and id.
if (!_.isEmpty(userPermissions)) {
// TODO: using 'Owner' is a bit hacky.
if (userPermissions === 'Owner') {
hasUserPermission = true;
} else {
hasUserPermission = _.any(userPermissions, checkPermission);
}
if (_.any(loadedPermissions.user.roles, { 'name': 'Owner' })) {
hasUserPermission = true;
} else if (!_.isEmpty(userPermissions)) {
hasUserPermission = _.any(userPermissions, checkPermission);
}
// Check app permissions if they were passed
hasAppPermission = true;
if (!_.isNull(appPermissions)) {
@ -121,8 +120,10 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (obj_types, act_type,
}
// Offer a chance for the TargetModel to override the results
if (TargetModel && _.isFunction(TargetModel.permissable)) {
return TargetModel.permissable(modelId, context, loadedPermissions, hasUserPermission, hasAppPermission);
if (TargetModel && _.isFunction(TargetModel.permissible)) {
return TargetModel.permissible(
modelId, act_type, context, loadedPermissions, hasUserPermission, hasAppPermission
);
}
if (hasUserPermission && hasAppPermission) {

24
core/test/unit/permissions_spec.js
View file

@ -106,9 +106,9 @@ describe('Permissions', function () {
// }).catch(done);
// });
//
// it('can use permissable function on Model to allow something', function (done) {
// it('can use permissible function on Model to allow something', function (done) {
// var testUser,
// permissableStub = sandbox.stub(Models.Post, 'permissable', function () {
// permissibleStub = sandbox.stub(Models.Post, 'permissible', function () {
// return when.resolve();
// });
//
@ -122,22 +122,22 @@ describe('Permissions', function () {
// return permissions.canThis({user: testUser.id}).edit.post(123);
// })
// .then(function () {
// permissableStub.restore();
// permissableStub.calledWith(123, { user: testUser.id, app: null, internal: false })
// permissibleStub.restore();
// permissibleStub.calledWith(123, { user: testUser.id, app: null, internal: false })
// .should.equal(true);
//
// done();
// })
// .catch(function () {
// permissableStub.restore();
// permissibleStub.restore();
//
// done(new Error('did not allow testUser'));
// });
// });
//
// it('can use permissable function on Model to forbid something', function (done) {
// it('can use permissible function on Model to forbid something', function (done) {
// var testUser,
// permissableStub = sandbox.stub(Models.Post, 'permissable', function () {
// permissibleStub = sandbox.stub(Models.Post, 'permissible', function () {
// return when.reject();
// });
//
@ -152,13 +152,13 @@ describe('Permissions', function () {
// })
// .then(function () {
//
// permissableStub.restore();
// permissibleStub.restore();
// done(new Error('Allowed testUser to edit post'));
// })
// .catch(function () {
// permissableStub.calledWith(123, { user: testUser.id, app: null, internal: false })
// permissibleStub.calledWith(123, { user: testUser.id, app: null, internal: false })
// .should.equal(true);
// permissableStub.restore();
// permissibleStub.restore();
// done();
// });
// });
@ -257,7 +257,7 @@ describe('Permissions', function () {
// });
//
// it('allows \'internal\' to be passed for internal requests', function (done) {
// // Using tag here because post implements the custom permissable interface
// // Using tag here because post implements the custom permissible interface
// permissions.canThis('internal')
// .edit
// .tag(1)
@ -270,7 +270,7 @@ describe('Permissions', function () {
// });
//
// it('allows { internal: true } to be passed for internal requests', function (done) {
// // Using tag here because post implements the custom permissable interface
// // Using tag here because post implements the custom permissible interface
// permissions.canThis({ internal: true })
// .edit
// .tag(1)