From 23e9a02ff126208e06e4e25a5cd82bac2d94add6 Mon Sep 17 00:00:00 2001
From: Fabien O'Carroll <fabien@allou.is>
Date: Tue, 9 Oct 2018 18:03:13 +0700
Subject: [PATCH] Updated Post and Author model permissible method (#9966)

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!
---
 core/server/models/post.js              | 13 ++++--------
 core/server/models/relations/authors.js | 27 +++++++++++++------------
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/core/server/models/post.js b/core/server/models/post.js
index 34628db04e..df9683c48b 100644
--- a/core/server/models/post.js
+++ b/core/server/models/post.js
@@ -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);
         }
@@ -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({
diff --git a/core/server/models/relations/authors.js b/core/server/models/relations/authors.js
index 54568f79d6..9521267c46 100644
--- a/core/server/models/relations/authors.js
+++ b/core/server/models/relations/authors.js
@@ -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
@@ -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,
@@ -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({