From 042750f4cf09dfe84d815fe37998908bb8fdcf30 Mon Sep 17 00:00:00 2001 From: Katharina Irrgang Date: Wed, 25 Jan 2017 13:07:31 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20=20fix=20invite=20permissions=20?= =?UTF-8?q?for=20editor=20(#7889)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs #7724 - we already fixed the permissions for the editor - see https://github.com/TryGhost/Ghost/commit/3d3101ad0ec5b2a72e6a99c2e6d37e36104751b7 - but as we are inside of a refactoring process, we had two fixtures.json files - we fixed the fixtures.json in the wrong place - now that the permissions are used, we can see failing tests - i have added the correct permissions handling --- core/server/api/invites.js | 49 ++++++++++++----- core/server/permissions/index.js | 3 +- core/server/translations/en.json | 3 +- core/test/integration/api/api_invites_spec.js | 53 ++++++++++++------- core/test/utils/index.js | 2 +- 5 files changed, 75 insertions(+), 35 deletions(-) diff --git a/core/server/api/invites.js b/core/server/api/invites.js index 02cc110ab2..3e2f157c7e 100644 --- a/core/server/api/invites.js +++ b/core/server/api/invites.js @@ -91,15 +91,7 @@ invites = { function addInvite(options) { var data = options.data; - return dataProvider.User.findOne({id: loggedInUser}, options) - .then(function (user) { - if (!user) { - return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.users.userNotFound')})); - } - - loggedInUser = user; - return dataProvider.Invite.add(data.invites[0], _.omit(options, 'data')); - }) + return dataProvider.Invite.add(data.invites[0], _.omit(options, 'data')) .then(function (_invite) { invite = _invite; @@ -177,25 +169,56 @@ invites = { return Promise.reject(new errors.ValidationError({message: i18n.t('errors.api.invites.roleIsRequired')})); } - // @TODO move this logic to permissible + // @TODO remove when we have a new permission unit // Make sure user is allowed to add a user with this role - return dataProvider.Role.findOne({id: options.data.invites[0].role_id}).then(function (role) { - if (!role) { + // We cannot use permissible because we don't have access to the role_id!!! + // Adding a permissible function to the invite model, doesn't give us much context of the invite we would like to add + // As we are looking forward to replace the permission system completely, we do not add a hack here + return dataProvider.Role.findOne({id: options.data.invites[0].role_id}).then(function (roleToInvite) { + if (!roleToInvite) { return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.invites.roleNotFound')})); } - if (role.get('name') === 'Owner') { + if (roleToInvite.get('name') === 'Owner') { return Promise.reject(new errors.NoPermissionError({message: i18n.t('errors.api.invites.notAllowedToInviteOwner')})); } + + var loggedInUserRole = loggedInUser.related('roles').models[0].get('name'), + allowed = []; + + if (loggedInUserRole === 'Owner' || loggedInUserRole === 'Administrator') { + allowed = ['Administrator', 'Editor', 'Author']; + } else if (loggedInUserRole === 'Editor') { + allowed = ['Author']; + } + + if (allowed.indexOf(roleToInvite.get('name')) === -1) { + return Promise.reject(new errors.NoPermissionError({ + message: i18n.t('errors.api.invites.notAllowedToInvite') + })); + } }).then(function () { return options; }); } + function fetchLoggedInUser(options) { + return dataProvider.User.findOne({id: loggedInUser}, _.merge({}, options, {include: ['roles']})) + .then(function (user) { + if (!user) { + return Promise.reject(new errors.NotFoundError({message: i18n.t('errors.api.users.userNotFound')})); + } + + loggedInUser = user; + return options; + }); + } + tasks = [ utils.validate(docName, {opts: ['email']}), utils.handlePermissions(docName, 'add'), utils.convertOptions(allowedIncludes), + fetchLoggedInUser, validation, destroyOldInvite, addInvite diff --git a/core/server/permissions/index.js b/core/server/permissions/index.js index fdbdd4d738..7177e51b51 100644 --- a/core/server/permissions/index.js +++ b/core/server/permissions/index.js @@ -124,7 +124,8 @@ CanThisResult.prototype.buildObjectTypeHandlers = function (objTypes, actType, c user: Models.User, permission: Models.Permission, setting: Models.Settings, - subscriber: Models.Subscriber + subscriber: Models.Subscriber, + invite: Models.Invite }; // Iterate through the object types, i.e. ['post', 'tag', 'user'] diff --git a/core/server/translations/en.json b/core/server/translations/en.json index cae9b46bcc..d4e098b00a 100644 --- a/core/server/translations/en.json +++ b/core/server/translations/en.json @@ -399,7 +399,8 @@ "error": "Error sending email: {message}", "help": "Please check your email settings and resend the invitation." }, - "notAllowedToInviteOwner": "Not allowed to invire an owner user." + "notAllowedToInviteOwner": "Not allowed to invite an owner user.", + "notAllowedToInvite": "Not allowed to invite this role." } }, "data": { diff --git a/core/test/integration/api/api_invites_spec.js b/core/test/integration/api/api_invites_spec.js index 2db06e4d0e..d6cc577969 100644 --- a/core/test/integration/api/api_invites_spec.js +++ b/core/test/integration/api/api_invites_spec.js @@ -178,7 +178,7 @@ describe('Invites API', function () { } describe('Owner', function () { - it('CANNOT add an Owner', function (done) { + it('CANNOT invite an Owner', function (done) { InvitesAPI.add({ invites: [ { @@ -191,7 +191,7 @@ describe('Invites API', function () { }).catch(checkForErrorType('NoPermissionError', done)); }); - it('Can add an Admin', function (done) { + it('Can invite an Admin', function (done) { InvitesAPI.add({ invites: [ { @@ -206,7 +206,7 @@ describe('Invites API', function () { }).catch(done); }); - it('Can add an Editor', function (done) { + it('Can invite an Editor', function (done) { InvitesAPI.add({ invites: [ { @@ -221,7 +221,7 @@ describe('Invites API', function () { }).catch(done); }); - it('Can add an Author', function (done) { + it('Can invite an Author', function (done) { InvitesAPI.add({ invites: [ { @@ -236,7 +236,7 @@ describe('Invites API', function () { }).catch(done); }); - it('Can add with role set as string', function (done) { + it('Can invite with role set as string', function (done) { InvitesAPI.add({ invites: [ { @@ -253,7 +253,7 @@ describe('Invites API', function () { }); describe('Admin', function () { - it('CANNOT add an Owner', function (done) { + it('CANNOT invite an Owner', function (done) { InvitesAPI.add({ invites: [ { @@ -266,7 +266,7 @@ describe('Invites API', function () { }).catch(checkForErrorType('NoPermissionError', done)); }); - it('Can add an Admin', function (done) { + it('Can invite an Admin', function (done) { InvitesAPI.add({ invites: [ { @@ -281,7 +281,7 @@ describe('Invites API', function () { }).catch(done); }); - it('Can add an Editor', function (done) { + it('Can invite an Editor', function (done) { InvitesAPI.add({ invites: [ { @@ -296,7 +296,7 @@ describe('Invites API', function () { }).catch(done); }); - it('Can add an Author', function (done) { + it('Can invite an Author', function (done) { InvitesAPI.add({ invites: [ { @@ -313,7 +313,7 @@ describe('Invites API', function () { }); describe('Editor', function () { - it('CANNOT add an Owner', function (done) { + it('CANNOT invite an Owner', function (done) { InvitesAPI.add({ invites: [ { @@ -322,11 +322,11 @@ describe('Invites API', function () { } ] }, context.editor).then(function () { - done(new Error('Editor should not be able to add an owner')); + done(new Error('Editor should not be able to invite an owner')); }).catch(checkForErrorType('NoPermissionError', done)); }); - it('CANNOT add an Adminstrator', function (done) { + it('CANNOT invite an Adminstrator', function (done) { InvitesAPI.add({ invites: [ { @@ -335,11 +335,24 @@ describe('Invites API', function () { } ] }, context.editor).then(function () { - done(new Error('Editor should not be able to add an owner')); + done(new Error('Editor should not be able to invite an administrator')); }).catch(checkForErrorType('NoPermissionError', done)); }); - it('CANNOT add an Author', function (done) { + it('CANNOT invite an Editor', function (done) { + InvitesAPI.add({ + invites: [ + { + email: 'test@example.com', + role_id: testUtils.roles.ids.editor + } + ] + }, context.editor).then(function () { + done(new Error('Editor should not be able to invite an editor')); + }).catch(checkForErrorType('NoPermissionError', done)); + }); + + it('Can invite an Author', function (done) { InvitesAPI.add({ invites: [ { @@ -347,14 +360,16 @@ describe('Invites API', function () { role_id: testUtils.roles.ids.author } ] - }, context.editor).then(function () { - done(new Error('Editor should not be able to add an author')); - }).catch(checkForErrorType('NoPermissionError', done)); + }, context.editor).then(function (response) { + checkAddResponse(response); + response.invites[0].role_id.should.equal(testUtils.roles.ids.author); + done(); + }).catch(done); }); }); describe('Author', function () { - it('CANNOT add an Owner', function (done) { + it('CANNOT invite an Owner', function (done) { InvitesAPI.add({ invites: [ { @@ -367,7 +382,7 @@ describe('Invites API', function () { }).catch(checkForErrorType('NoPermissionError', done)); }); - it('CANNOT add an Author', function (done) { + it('CANNOT invite an Author', function (done) { InvitesAPI.add({ invites: [ { diff --git a/core/test/utils/index.js b/core/test/utils/index.js index bcd0bfd8cc..05ac7427ef 100644 --- a/core/test/utils/index.js +++ b/core/test/utils/index.js @@ -10,7 +10,7 @@ var Promise = require('bluebird'), ghost = require('../../server'), errors = require('../../server/errors'), db = require('../../server/data/db'), - fixtureUtils = require('../../server/data/migration/fixtures/utils'), + fixtureUtils = require('../../server/data/schema/fixtures/utils'), models = require('../../server/models'), SettingsAPI = require('../../server/api/settings'), permissions = require('../../server/permissions'),