Fix author role for the publish permission action (#8153)

* Prevent Author role to have access to the publish permission in CE

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>

* Fix CE test on role's permissions update

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>

* Fix unit tests

Signed-off-by: Convly <jean-sebastien.herbaux@epitech.eu>

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 } })),
               },

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);
     }

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 = [
+const READ_ACTION = 'plugins::content-manager.explorer.read';
-  'plugins::content-manager.explorer.read',
+const CREATE_ACTION = 'plugins::content-manager.explorer.create';
-  'plugins::content-manager.explorer.create',
+const UPDATE_ACTION = 'plugins::content-manager.explorer.update';
-  'plugins::content-manager.explorer.update',
+const DELETE_ACTION = 'plugins::content-manager.explorer.delete';
-  'plugins::content-manager.explorer.delete',
+const PUBLISH_ACTION = 'plugins::content-manager.explorer.publish';
-];
 
-const BOUND_ACTIONS_FOR_FIELDS = [
+const BOUND_ACTIONS = [READ_ACTION, CREATE_ACTION, UPDATE_ACTION, DELETE_ACTION, PUBLISH_ACTION];
-  'plugins::content-manager.explorer.read',
+
-  'plugins::content-manager.explorer.create',
+const BOUND_ACTIONS_FOR_FIELDS = [READ_ACTION, CREATE_ACTION, UPDATE_ACTION];
-  'plugins::content-manager.explorer.update',
+
-];
+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 checkPermissionsAreBound = role =>
-  const permsBySubject = _.groupBy(
+  function(permissions) {
-    permissions.filter(perm => BOUND_ACTIONS.includes(perm.action)),
+    const permsBySubject = _.groupBy(
-    'subject'
+      permissions.filter(perm => BOUND_ACTIONS.includes(perm.action)),
-  );
+      'subject'
+    );
 
-  for (const perms of Object.values(permsBySubject)) {
+    for (const [subject, perms] of Object.entries(permsBySubject)) {
-    const missingActions =
+      const boundActions = getBoundActionsBySubject(role, subject);
-      _.xor(
+      const missingActions =
-        perms.map(p => p.action),
+        _.xor(
-        BOUND_ACTIONS
+          perms.map(p => p.action),
-      ).length !== 0;
+          boundActions
-    if (missingActions) return false;
+        ).length !== 0;
+      if (missingActions) return false;
 
-    const permsBoundByFields = perms.filter(p => BOUND_ACTIONS_FOR_FIELDS.includes(p.action));
+      const permsBoundByFields = perms.filter(p => BOUND_ACTIONS_FOR_FIELDS.includes(p.action));
-    const everyActionsHaveSameFields = _.every(permsBoundByFields, haveSameFieldsAsOtherActions);
+      const everyActionsHaveSameFields = _.every(permsBoundByFields, haveSameFieldsAsOtherActions);
-    if (!everyActionsHaveSameFields) return false;
+      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',
+        'Permissions have to be defined all together for a subject field or not at all',
-        checkPermissionsAreBound
+        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) {
+    const schemas = getUpdatePermissionsSchemas(role);
-      await schema.validate(data, { strict: true, abortEarly: false });
+    for (const schema of schemas) {
+      await schema.validate(permissions, { strict: true, abortEarly: false });
     }
   } catch (e) {
     return handleReject(e);