diff --git a/core/server/models/post.js b/core/server/models/post.js index 6726962c0b..125da72b8e 100644 --- a/core/server/models/post.js +++ b/core/server/models/post.js @@ -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); } diff --git a/core/server/models/role.js b/core/server/models/role.js index 924f4c546c..7632dbf801 100644 --- a/core/server/models/role.js +++ b/core/server/models/role.js @@ -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); } diff --git a/core/server/models/user.js b/core/server/models/user.js index 9081b96f55..2664e28724 100644 --- a/core/server/models/user.js +++ b/core/server/models/user.js @@ -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); } diff --git a/core/server/permissions/effective.js b/core/server/permissions/effective.js index 61652faa4a..e2211f2e63 100644 --- a/core/server/permissions/effective.js +++ b/core/server/permissions/effective.js @@ -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); } }; diff --git a/core/server/permissions/index.js b/core/server/permissions/index.js index 47e0c400d6..3cf8c3fe3d 100644 --- a/core/server/permissions/index.js +++ b/core/server/permissions/index.js @@ -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) { diff --git a/core/test/unit/permissions_spec.js b/core/test/unit/permissions_spec.js index 779e6ba577..b0f9de3042 100644 --- a/core/test/unit/permissions_spec.js +++ b/core/test/unit/permissions_spec.js @@ -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)