Skip to content

Commit

Permalink
Updated Post and Author model permissible method (#9966)
Browse files Browse the repository at this point in the history
refs #9865

Both the Post and the Author model implement the permissible method,
however the Post model does not abide by the signature of the
permissible method and add their own parameter "result" at the end.

This makes changes to the permissible method difficult as we have to
take into account multiple signatures.

This changes the Post model permissible method to the correct signature,
but still retains the current functionality. This will make it easier to
break up future permission related PR's so they can be reviwed easier
and faster!
  • Loading branch information
allouis committed Oct 9, 2018
1 parent 05568fd commit 23e9a02
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 22 deletions.
13 changes: 4 additions & 9 deletions core/server/models/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -684,11 +684,9 @@ Post = ghostBookshelf.Model.extend({
},

// NOTE: the `authors` extension is the parent of the post model. It also has a permissible function.
permissible: function permissible(postModel, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission, result) {
permissible: function permissible(postModel, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission) {
let isContributor, isEdit, isAdd, isDestroy;

result = result || {};

function isChanging(attr) {
return unsafeAttrs[attr] && unsafeAttrs[attr] !== postModel.get(attr);
}
Expand Down Expand Up @@ -717,21 +715,18 @@ Post = ghostBookshelf.Model.extend({
hasUserPermission = isDraft();
}

const excludedAttrs = [];
if (isContributor) {
// Note: at the moment primary_tag is a computed field,
// meaning we don't add it to this list. However, if the primary_tag/primary_author
// ever becomes a db field rather than a computed field, add it to this list
// TODO: once contributors are able to edit existing tags, this can be removed
// @TODO: we need a concept for making a diff between incoming tags and existing tags
if (result.excludedAttrs) {
result.excludedAttrs.push('tags');
} else {
result.excludedAttrs = ['tags'];
}
excludedAttrs.push('tags');
}

if (hasUserPermission && hasAppPermission) {
return Promise.resolve(result);
return Promise.resolve({excludedAttrs});
}

return Promise.reject(new common.errors.NoPermissionError({
Expand Down
27 changes: 14 additions & 13 deletions core/server/models/relations/authors.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,7 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
permissible: function permissible(postModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasAppPermission) {
var self = this,
postModel = postModelOrId,
origArgs, isContributor, isAuthor, isEdit, isAdd, isDestroy,
result = {};
origArgs, isContributor, isAuthor, isEdit, isAdd, isDestroy;

// If we passed in an id instead of a model, get the model
// then check the permissions
Expand Down Expand Up @@ -333,14 +332,6 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
hasUserPermission = hasUserPermission || isCurrentOwner();
}

// @TODO: we need a concept for making a diff between incoming authors and existing authors
// @TODO: for now we simply re-use the new concept of `excludedAttrs`
// We only check the primary author of `authors`, any other change will be ignored.
// By this we can deprecate `author_id` more easily.
if (isContributor || isAuthor) {
result.excludedAttrs = ['authors'];
}

if (hasUserPermission && hasAppPermission) {
return Post.permissible.call(
this,
Expand All @@ -349,9 +340,19 @@ module.exports.extendModel = function extendModel(Post, Posts, ghostBookshelf) {
unsafeAttrs,
loadedPermissions,
hasUserPermission,
hasAppPermission,
result
);
hasAppPermission
).then(({excludedAttrs}) => {
// @TODO: we need a concept for making a diff between incoming authors and existing authors
// @TODO: for now we simply re-use the new concept of `excludedAttrs`
// We only check the primary author of `authors`, any other change will be ignored.
// By this we can deprecate `author_id` more easily.
if (isContributor || isAuthor) {
return {
excludedAttrs: ['authors'].concat(excludedAttrs)
};
}
return {excludedAttrs};
});
}

return Promise.reject(new common.errors.NoPermissionError({
Expand Down

0 comments on commit 23e9a02

Please sign in to comment.