Skip to content

Commit

Permalink
Support for attribute-based permissions (#9025)
Browse files Browse the repository at this point in the history 
refs #8602

- Add the wiring to pass attributes around the permission system
- Allows us to get access to the important "unsafe" attributes that are changing
- E.g. status for posts
- This can then be used to determine whether a user has permission to perform an attribute-based action
- E.g. publish a post (change status)
  • Loading branch information
@ErisDS @kirrg001
ErisDS authored and kirrg001 committed Sep 26, 2017
1 parent a80a09e commit b468d6d
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 9 deletions.
6 changes: 4 additions & 2 deletions core/server/api/utils.js
View file
Expand Up @@ -194,9 +194,10 @@ utils = {
* ## Handle Permissions
* @param {String} docName
* @param {String} method (browse || read || edit || add || destroy)
* * @param {Array} unsafeAttrNames - attribute names (e.g. post.status) that could change the outcome
* @returns {Function}
*/
handlePermissions: function handlePermissions(docName, method) {
handlePermissions: function handlePermissions(docName, method, unsafeAttrNames) {
var singular = docName.replace(/s$/, '');

/**
Expand All @@ -206,7 +207,8 @@ utils = {
* @returns {Object} options
*/
return function doHandlePermissions(options) {
var permsPromise = permissions.canThis(options.context)[method][singular](options.id);
var unsafeAttrObject = unsafeAttrNames && _.has(options, 'data.[' + docName + '][0]') ? _.pick(options.data[docName][0], unsafeAttrNames) : {},
permsPromise = permissions.canThis(options.context)[method][singular](options.id, unsafeAttrObject);

return permsPromise.then(function permissionGranted() {
return options;
Expand Down
2 changes: 1 addition & 1 deletion core/server/models/post.js
View file
Expand Up @@ -810,7 +810,7 @@ Post = ghostBookshelf.Model.extend({
});
}),

permissible: function permissible(postModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
permissible: function permissible(postModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
postModel = postModelOrId,
origArgs;
Expand Down
2 changes: 1 addition & 1 deletion core/server/models/role.js
View file
Expand Up @@ -41,7 +41,7 @@ Role = ghostBookshelf.Model.extend({
return options;
},

permissible: function permissible(roleModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
permissible: function permissible(roleModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
checkAgainst = [],
origArgs;
Expand Down
2 changes: 1 addition & 1 deletion core/server/models/subscriber.js
View file
Expand Up @@ -59,7 +59,7 @@ Subscriber = ghostBookshelf.Model.extend({
return options;
},

permissible: function permissible(postModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
permissible: function permissible(postModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission) {
// CASE: external is only allowed to add and edit subscribers
if (context.external) {
if (['add', 'edit'].indexOf(action) !== -1) {
Expand Down
2 changes: 1 addition & 1 deletion core/server/models/user.js
View file
Expand Up @@ -583,7 +583,7 @@ User = ghostBookshelf.Model.extend({
});
},

permissible: function permissible(userModelOrId, action, context, loadedPermissions, hasUserPermission, hasAppPermission) {
permissible: function permissible(userModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
userModel = userModelOrId,
origArgs;
Expand Down
5 changes: 3 additions & 2 deletions core/server/permissions/index.js
View file
Expand Up @@ -45,8 +45,9 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c

// Create the 'handler' for the object type;
// the '.post()' in canThis(user).edit.post()
objTypeHandlers[objType] = function (modelOrId) {
objTypeHandlers[objType] = function (modelOrId, unsafeAttrs) {
var modelId;
unsafeAttrs = unsafeAttrs || {};

// If it's an internal request, resolve immediately
if (context.internal) {
Expand Down Expand Up @@ -105,7 +106,7 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c
// Offer a chance for the TargetModel to override the results
if (TargetModel && _.isFunction(TargetModel.permissible)) {
return TargetModel.permissible(
modelId, actType, context, loadedPermissions, hasUserPermission, hasAppPermission
modelId, actType, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission
);
}

Expand Down
83 changes: 83 additions & 0 deletions core/test/unit/api/utils_spec.js
View file
Expand Up @@ -607,7 +607,90 @@ describe('API Utils', function () {
.then(function (res) {
permsStub.callCount.should.eql(1);
testStub.callCount.should.eql(1);
testStub.firstCall.args.length.should.eql(2);
testStub.firstCall.args[0].should.eql(5);
testStub.firstCall.args[1].should.eql({});

res.should.eql(testObj);

done();
})
.catch(done);
});

it('should ignore unsafe attrs if none are provided', function (done) {
var testStub = sandbox.stub().returns(new Promise.resolve()),
permsStub = sandbox.stub(permissions, 'canThis', function () {
return {
testing: {
test: testStub
}
};
}),
permsFunc = apiUtils.handlePermissions('tests', 'testing', ['foo']),
testObj = {data: {tests: [{}]}, id: 5};

permsFunc(testObj)
.then(function (res) {
permsStub.callCount.should.eql(1);
testStub.callCount.should.eql(1);
testStub.firstCall.args.length.should.eql(2);
testStub.firstCall.args[0].should.eql(5);
testStub.firstCall.args[1].should.eql({});

res.should.eql(testObj);

done();
})
.catch(done);
});

it('should ignore unsafe attrs if they are provided but not present', function (done) {
var testStub = sandbox.stub().returns(new Promise.resolve()),
permsStub = sandbox.stub(permissions, 'canThis', function () {
return {
testing: {
test: testStub
}
};
}),
permsFunc = apiUtils.handlePermissions('tests', 'testing', ['foo']),
testObj = {foo: 'bar', id: 5};

permsFunc(testObj)
.then(function (res) {
permsStub.callCount.should.eql(1);
testStub.callCount.should.eql(1);
testStub.firstCall.args.length.should.eql(2);
testStub.firstCall.args[0].should.eql(5);
testStub.firstCall.args[1].should.eql({});

res.should.eql(testObj);

done();
})
.catch(done);
});

it('should pass through unsafe attrs if they DO exist', function (done) {
var testStub = sandbox.stub().returns(new Promise.resolve()),
permsStub = sandbox.stub(permissions, 'canThis', function () {
return {
testing: {
test: testStub
}
};
}),
permsFunc = apiUtils.handlePermissions('tests', 'testing', ['foo']),
testObj = {data: {tests: [{foo: 'bar'}]}, id: 5};

permsFunc(testObj)
.then(function (res) {
permsStub.callCount.should.eql(1);
testStub.callCount.should.eql(1);
testStub.firstCall.args.length.should.eql(2);
testStub.firstCall.args[0].should.eql(5);
testStub.firstCall.args[1].should.eql({foo: 'bar'});

res.should.eql(testObj);

Expand Down
21 changes: 20 additions & 1 deletion core/test/unit/permissions/index_spec.js
View file
@@ -1,4 +1,4 @@
var should = require('should'),
var should = require('should'), // jshint ignore:line
sinon = require('sinon'),
testUtils = require('../../utils'),
Promise = require('bluebird'),
Expand Down Expand Up @@ -594,6 +594,16 @@ describe('Permissions', function () {
})
.catch(function (err) {
permissibleStub.callCount.should.eql(1);
permissibleStub.firstCall.args.should.have.lengthOf(7);

permissibleStub.firstCall.args[0].should.eql(1);
permissibleStub.firstCall.args[1].should.eql('edit');
permissibleStub.firstCall.args[2].should.be.an.Object();
permissibleStub.firstCall.args[3].should.be.an.Object();
permissibleStub.firstCall.args[4].should.be.an.Object();
permissibleStub.firstCall.args[5].should.be.true();
permissibleStub.firstCall.args[6].should.be.true();

effectiveUserStub.callCount.should.eql(1);
err.message.should.eql('Hello World!');
done();
Expand All @@ -618,6 +628,15 @@ describe('Permissions', function () {
.post({id: 1}) // tag id in model syntax
.then(function (res) {
permissibleStub.callCount.should.eql(1);
permissibleStub.firstCall.args.should.have.lengthOf(7);
permissibleStub.firstCall.args[0].should.eql(1);
permissibleStub.firstCall.args[1].should.eql('edit');
permissibleStub.firstCall.args[2].should.be.an.Object();
permissibleStub.firstCall.args[3].should.be.an.Object();
permissibleStub.firstCall.args[4].should.be.an.Object();
permissibleStub.firstCall.args[5].should.be.true();
permissibleStub.firstCall.args[6].should.be.true();

effectiveUserStub.callCount.should.eql(1);
should.not.exist(res);
done();
Expand Down

0 comments on commit b468d6d

Please sign in to comment.