diff --git a/packages/strapi-admin/controllers/__tests__/role.test.js b/packages/strapi-admin/controllers/__tests__/role.test.js index d3158437f9..b0e41fceef 100644 --- a/packages/strapi-admin/controllers/__tests__/role.test.js +++ b/packages/strapi-admin/controllers/__tests__/role.test.js @@ -177,6 +177,7 @@ describe('Role controller', () => { getAll: jest.fn(() => [{ id: 'admin::is-creator' }]), }, actionProvider: { + getAll: jest.fn(() => [{ actionId: 'test', subjects: ['model1'] }]), getAllByMap: jest.fn(), getByActionId: jest.fn(() => ({ options: { fieldsRestriction: true } })), }, diff --git a/packages/strapi-admin/controllers/role.js b/packages/strapi-admin/controllers/role.js index 9075465575..bdd5996e0a 100644 --- a/packages/strapi-admin/controllers/role.js +++ b/packages/strapi-admin/controllers/role.js @@ -107,7 +107,7 @@ module.exports = { const err = new yup.ValidationError("Super admin permissions can't be edited."); throw formatYupErrors(err); } - await validatedUpdatePermissionsInput(input); + await validatedUpdatePermissionsInput(input, role); } catch (err) { return ctx.badRequest('ValidationError', err); } diff --git a/packages/strapi-admin/validation/permission.js b/packages/strapi-admin/validation/permission.js index 8f9a163ef4..adeb761038 100644 --- a/packages/strapi-admin/validation/permission.js +++ b/packages/strapi-admin/validation/permission.js @@ -1,25 +1,37 @@ 'use strict'; const _ = require('lodash'); -const { yup, formatYupErrors } = require('strapi-utils'); +const { + yup, + formatYupErrors, + contentTypes: { hasDraftAndPublish }, +} = require('strapi-utils'); const validators = require('./common-validators'); +const { AUTHOR_CODE } = require('../services/constants'); const handleReject = error => Promise.reject(formatYupErrors(error)); // validatedUpdatePermissionsInput -const BOUND_ACTIONS = [ - 'plugins::content-manager.explorer.read', - 'plugins::content-manager.explorer.create', - 'plugins::content-manager.explorer.update', - 'plugins::content-manager.explorer.delete', -]; +const READ_ACTION = 'plugins::content-manager.explorer.read'; +const CREATE_ACTION = 'plugins::content-manager.explorer.create'; +const UPDATE_ACTION = 'plugins::content-manager.explorer.update'; +const DELETE_ACTION = 'plugins::content-manager.explorer.delete'; +const PUBLISH_ACTION = 'plugins::content-manager.explorer.publish'; -const BOUND_ACTIONS_FOR_FIELDS = [ - 'plugins::content-manager.explorer.read', - 'plugins::content-manager.explorer.create', - 'plugins::content-manager.explorer.update', -]; +const BOUND_ACTIONS = [READ_ACTION, CREATE_ACTION, UPDATE_ACTION, DELETE_ACTION, PUBLISH_ACTION]; + +const BOUND_ACTIONS_FOR_FIELDS = [READ_ACTION, CREATE_ACTION, UPDATE_ACTION]; + +const getBoundActionsBySubject = (role, subject) => { + const model = strapi.getModel(subject); + + if (role.code === AUTHOR_CODE || !hasDraftAndPublish(model)) { + return [READ_ACTION, UPDATE_ACTION, CREATE_ACTION, DELETE_ACTION]; + } + + return BOUND_ACTIONS; +}; const actionFieldsAreEqual = (a, b) => { const aFields = a.fields || []; @@ -31,37 +43,57 @@ const actionFieldsAreEqual = (a, b) => { const haveSameFieldsAsOtherActions = (a, i, allActions) => allActions.slice(i + 1).every(b => actionFieldsAreEqual(a, b)); -const checkPermissionsAreBound = function(permissions) { - const permsBySubject = _.groupBy( - permissions.filter(perm => BOUND_ACTIONS.includes(perm.action)), - 'subject' - ); +const checkPermissionsAreBound = role => + function(permissions) { + const permsBySubject = _.groupBy( + permissions.filter(perm => BOUND_ACTIONS.includes(perm.action)), + 'subject' + ); - for (const perms of Object.values(permsBySubject)) { - const missingActions = - _.xor( - perms.map(p => p.action), - BOUND_ACTIONS - ).length !== 0; - if (missingActions) return false; + for (const [subject, perms] of Object.entries(permsBySubject)) { + const boundActions = getBoundActionsBySubject(role, subject); + const missingActions = + _.xor( + perms.map(p => p.action), + boundActions + ).length !== 0; + if (missingActions) return false; - const permsBoundByFields = perms.filter(p => BOUND_ACTIONS_FOR_FIELDS.includes(p.action)); - const everyActionsHaveSameFields = _.every(permsBoundByFields, haveSameFieldsAsOtherActions); - if (!everyActionsHaveSameFields) return false; - } + const permsBoundByFields = perms.filter(p => BOUND_ACTIONS_FOR_FIELDS.includes(p.action)); + const everyActionsHaveSameFields = _.every(permsBoundByFields, haveSameFieldsAsOtherActions); + if (!everyActionsHaveSameFields) return false; + } - return true; -}; + return true; + }; -const updatePermissionsSchemas = [ +const noPublishPermissionForAuthorRole = role => + function(permissions) { + const isAuthor = role.code === AUTHOR_CODE; + const hasPublishPermission = permissions.some(perm => perm.action === PUBLISH_ACTION); + + return !(isAuthor && hasPublishPermission); + }; + +const getUpdatePermissionsSchemas = role => [ validators.updatePermissions, + yup.object().shape({ permissions: actionsExistSchema.clone() }), + yup.object().shape({ + permissions: yup + .array() + .test( + 'author-no-publish', + 'The author role cannot have the publish permission.', + noPublishPermissionForAuthorRole(role) + ), + }), yup.object().shape({ permissions: yup .array() .test( 'are-bond', - 'Read, Create, Update and Delete have to be defined all together for a subject field or not at all', - checkPermissionsAreBound + 'Permissions have to be defined all together for a subject field or not at all', + checkPermissionsAreBound(role) ), }), ]; @@ -85,10 +117,11 @@ const validateCheckPermissionsInput = data => { .catch(handleReject); }; -const validatedUpdatePermissionsInput = async data => { +const validatedUpdatePermissionsInput = async (permissions, role) => { try { - for (const schema of updatePermissionsSchemas) { - await schema.validate(data, { strict: true, abortEarly: false }); + const schemas = getUpdatePermissionsSchemas(role); + for (const schema of schemas) { + await schema.validate(permissions, { strict: true, abortEarly: false }); } } catch (e) { return handleReject(e);