Added members permission system

refs https://github.com/TryGhost/Team/issues/1664

- the new member comments API needs members to have permissions to edit and delete their own posts
- added members as a provider, and then wired up permissible logic at the model level

core/server/api/endpoints/utils/permissions.js

@@ -92,7 +92,7 @@ module.exports = {
         frame.options.context = permissions.parseContext(frame.options.context);
 
         // CASE: Content API access
-        if (frame.options.context.public) {
+        if (frame.options.context.public && frame.apiType !== 'comments') {
             debug('check content permissions');
 
             // @TODO: Remove when we drop v0.1

core/server/models/comment.js

@@ -1,4 +1,12 @@
 const ghostBookshelf = require('./base');
+const _ = require('lodash');
+const errors = require('@tryghost/errors');
+const tpl = require('@tryghost/tpl');
+
+const messages = {
+    commentNotFound: 'Comment could not be found',
+    notYourComment: 'You may only edit your own comments'
+};
 
 const Comment = ghostBookshelf.Model.extend({
     tableName: 'comments',
@@ -32,9 +40,37 @@ const Comment = ghostBookshelf.Model.extend({
         model.emitChange('added', options);
     }
 }, {
-    async permissible(model, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission, hasMemberPermission) {
+    async permissible(commentModelOrId, action, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission, hasMemberPermission) {
-        console.log('checking permissions', hasMemberPermission);
+        const self = this;
-        return true;
+
+        if (_.isString(commentModelOrId)) {
+            // Grab the original args without the first one
+            const origArgs = _.toArray(arguments).slice(1);
+
+            // Get the actual comment model
+            return this.findOne({
+                id: commentModelOrId
+            }).then(function then(foundCommentModel) {
+                if (!foundCommentModel) {
+                    throw new errors.NotFoundError({
+                        message: tpl(messages.commentNotFound)
+                    });
+                }
+
+                // Build up the original args but substitute with actual model
+                const newArgs = [foundCommentModel].concat(origArgs);
+
+                return self.permissible.apply(self, newArgs);
+            });
+        }
+
+        if ((action === 'edit' || action === 'destroy') && commentModelOrId.get('member_id') !== context.member.id) {
+            return Promise.reject(new errors.NoPermissionError({
+                message: tpl(messages.notYourComment)
+            }));
+        }
+
+        return hasMemberPermission;
     }
 });
 

core/server/services/permissions/can-this.js

@@ -27,7 +27,8 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c
         permission: models.Permission,
         setting: models.Settings,
         invite: models.Invite,
-        integration: models.Integration
+        integration: models.Integration,
+        comment: models.Comment
     };
 
     // Iterate through the object types, i.e. ['post', 'tag', 'user']
@@ -57,10 +58,12 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c
             return permissionLoad.then(function (loadedPermissions) {
                 // Iterate through the user permissions looking for an affirmation
                 const userPermissions = loadedPermissions.user ? loadedPermissions.user.permissions : null;
-
                 const apiKeyPermissions = loadedPermissions.apiKey ? loadedPermissions.apiKey.permissions : null;
+                const memberPermissions = loadedPermissions.member ? loadedPermissions.member.permissions : null;
+
                 let hasUserPermission;
                 let hasApiKeyPermission;
+                let hasMemberPermission = false;
 
                 const checkPermission = function (perm) {
                     let permObjId;
@@ -91,6 +94,10 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c
                     hasUserPermission = _.some(userPermissions, checkPermission);
                 }
 
+                if (loadedPermissions.member) {
+                    hasMemberPermission = _.some(memberPermissions, checkPermission);
+                }
+
                 // Check api key permissions if they were passed
                 hasApiKeyPermission = true;
                 if (!_.isNull(apiKeyPermissions)) {
@@ -102,7 +109,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, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission
+                        modelId, actType, context, unsafeAttrs, loadedPermissions, hasUserPermission, hasApiKeyPermission, hasMemberPermission
                     );
                 }
 
@@ -122,6 +129,7 @@ CanThisResult.prototype.beginCheck = function (context) {
     const self = this;
     let userPermissionLoad;
     let apiKeyPermissionLoad;
+    let memberPermissionLoad;
     let permissionsLoad;
 
     // Get context.user, context.api_key and context.app
@@ -147,11 +155,16 @@ CanThisResult.prototype.beginCheck = function (context) {
         apiKeyPermissionLoad = Promise.resolve(null);
     }
 
+    if (context.member) {
+        memberPermissionLoad = providers.member(context.member.id);
+    }
+
     // Wait for both user and app permissions to load
-    permissionsLoad = Promise.all([userPermissionLoad, apiKeyPermissionLoad]).then(function (result) {
+    permissionsLoad = Promise.all([userPermissionLoad, apiKeyPermissionLoad, memberPermissionLoad]).then(function (result) {
         return {
             user: result[0],
-            apiKey: result[1]
+            apiKey: result[1],
+            member: result[2]
         };
     });
 

core/server/services/permissions/parse-context.js

@@ -12,6 +12,7 @@ module.exports = function parseContext(context) {
         user: null,
         api_key: null,
         integration: null,
+        member: null,
         public: true
     };
 
@@ -37,5 +38,9 @@ module.exports = function parseContext(context) {
         parsed.public = (context.api_key.type === 'content');
     }
 
+    if (context && context.member) {
+        parsed.member = context.member;
+    }
+
     return parsed;
 };

core/server/services/permissions/providers.js

@@ -6,7 +6,8 @@ const tpl = require('@tryghost/tpl');
 
 const messages = {
     userNotFound: 'User not found',
-    apiKeyNotFound: 'API Key not found'
+    apiKeyNotFound: 'API Key not found',
+    memberNotFound: 'Member not found'
 };
 
 module.exports = {
@@ -72,5 +73,20 @@ module.exports = {
 
                 return {permissions, roles};
             });
+    },
+
+    async member(id) {
+        const foundMember = await models.Member.findOne({id});
+        if (!foundMember) {
+            throw new errors.NotFoundError({
+                message: tpl(messages.memberNotFound)
+            });
+        }
+
+        // @TODO: figure out how we want to associate members with permissions
+        // Dirty code to load all comment permissions except moderation for members
+        const permissions = await models.Permission.findAll({filter: 'object_type:comment+action_type:-moderate'});
+
+        return {permissions: permissions.models};
     }
 };

test/unit/server/services/permissions/can-this.test.js

@@ -474,7 +474,7 @@ describe('Permissions', function () {
                 })
                 .catch(function (err) {
                     permissibleStub.callCount.should.eql(1);
-                    permissibleStub.firstCall.args.should.have.lengthOf(7);
+                    permissibleStub.firstCall.args.should.have.lengthOf(8);
 
                     permissibleStub.firstCall.args[0].should.eql(1);
                     permissibleStub.firstCall.args[1].should.eql('edit');
@@ -509,7 +509,7 @@ 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.should.have.lengthOf(8);
                     permissibleStub.firstCall.args[0].should.eql(1);
                     permissibleStub.firstCall.args[1].should.eql('edit');
                     permissibleStub.firstCall.args[2].should.be.an.Object();
@@ -517,6 +517,7 @@ describe('Permissions', function () {
                     permissibleStub.firstCall.args[4].should.be.an.Object();
                     permissibleStub.firstCall.args[5].should.be.true();
                     permissibleStub.firstCall.args[6].should.be.true();
+                    permissibleStub.firstCall.args[7].should.be.false();
 
                     userProviderStub.callCount.should.eql(1);
                     should.not.exist(res);

test/unit/server/services/permissions/parse-context.test.js

@@ -9,6 +9,7 @@ describe('Permissions', function () {
                 external: false,
                 user: null,
                 api_key: null,
+                member: null,
                 public: true,
                 integration: null
             });
@@ -17,6 +18,7 @@ describe('Permissions', function () {
                 external: false,
                 user: null,
                 api_key: null,
+                member: null,
                 public: true,
                 integration: null
             });
@@ -28,6 +30,7 @@ describe('Permissions', function () {
                 external: false,
                 user: null,
                 api_key: null,
+                member: null,
                 public: true,
                 integration: null
             });
@@ -36,6 +39,7 @@ describe('Permissions', function () {
                 external: false,
                 user: null,
                 api_key: null,
+                member: null,
                 public: true,
                 integration: null
             });
@@ -47,6 +51,7 @@ describe('Permissions', function () {
                 external: false,
                 user: 1,
                 api_key: null,
+                member: null,
                 public: false,
                 integration: null
             });
@@ -64,6 +69,7 @@ describe('Permissions', function () {
                     id: 1,
                     type: 'content'
                 },
+                member: null,
                 public: true,
                 integration: {id: 2}
             });
@@ -81,6 +87,7 @@ describe('Permissions', function () {
                     id: 1,
                     type: 'admin'
                 },
+                member: null,
                 public: false,
                 integration: {id: 3}
             });
@@ -92,6 +99,7 @@ describe('Permissions', function () {
                 external: false,
                 user: null,
                 api_key: null,
+                member: null,
                 public: false,
                 integration: null
             });
@@ -101,6 +109,7 @@ describe('Permissions', function () {
                 external: false,
                 user: null,
                 api_key: null,
+                member: null,
                 public: false,
                 integration: null
             });
@@ -112,6 +121,7 @@ describe('Permissions', function () {
                 external: true,
                 user: null,
                 api_key: null,
+                member: null,
                 public: false,
                 integration: null
             });
@@ -121,6 +131,7 @@ describe('Permissions', function () {
                 external: true,
                 user: null,
                 api_key: null,
+                member: null,
                 public: false,
                 integration: null
             });