diff --git a/api-tests/core/admin/ee/review-workflows-stage-permissions.test.api.js b/api-tests/core/admin/ee/review-workflows-stage-permissions.test.api.js new file mode 100644 index 0000000000..c6e12729ac --- /dev/null +++ b/api-tests/core/admin/ee/review-workflows-stage-permissions.test.api.js @@ -0,0 +1,255 @@ +'use strict'; + +const { mapAsync } = require('@strapi/utils'); + +const { createStrapiInstance } = require('api-tests/strapi'); +const { createAuthRequest, createRequest } = require('api-tests/request'); +const { createTestBuilder } = require('api-tests/builder'); +const { describeOnCondition } = require('api-tests/utils'); + +const { + STAGE_MODEL_UID, + WORKFLOW_MODEL_UID, + ENTITY_STAGE_ATTRIBUTE, +} = require('../../../../packages/core/admin/ee/server/constants/workflows'); +const { create } = require('lodash'); + +const edition = process.env.STRAPI_DISABLE_EE === 'true' ? 'CE' : 'EE'; + +const productUID = 'api::product.product'; +const model = { + draftAndPublish: true, + pluginOptions: {}, + singularName: 'product', + pluralName: 'products', + displayName: 'Product', + kind: 'collectionType', + attributes: { + name: { + type: 'string', + }, + }, + options: { + reviewWorkflows: true, + }, +}; + +const baseWorkflow = { + contentTypes: [productUID], + stages: [{ name: 'Stage 1' }, { name: 'Stage 2' }], +}; + +const getStageTransitionPermissions = (roleIds) => { + return roleIds.map((roleId) => ({ + action: 'admin::review-workflows.stage.transition', + role: roleId, + })); +}; + +describeOnCondition(edition === 'EE')('Review workflows', () => { + const builder = createTestBuilder(); + + let strapi; + let workflow; + let rq; + let roles; + + const createWorkflow = async (data) => { + const name = `workflow-${Math.random().toString(36)}`; + const req = await rq.post('/admin/review-workflows/workflows?populate=*', { + body: { data: { name, ...data } }, + }); + + const status = req.statusCode; + const error = req.body.error; + const workflow = req.body.data; + + return { workflow, status, error }; + }; + + const updateWorkflow = async (id, data) => { + const req = await rq.put(`/admin/review-workflows/workflows/${id}?populate=stages`, { + body: { data }, + }); + + const status = req.statusCode; + const error = req.body.error; + const workflow = req.body.data; + + return { workflow, status, error }; + }; + + const deleteWorkflow = async (id) => { + const req = await rq.delete(`/admin/review-workflows/workflows/${id}`); + + const status = req.statusCode; + const error = req.body.error; + const workflow = req.body.data; + + return { workflow, status, error }; + }; + + beforeAll(async () => { + await builder.addContentTypes([model]).build(); + + strapi = await createStrapiInstance(); + rq = await createAuthRequest({ strapi }); + + workflow = await createWorkflow(baseWorkflow); + + // Get default roles + const { body } = await rq.get('/admin/roles'); + roles = body.data; + }); + + afterAll(async () => { + await strapi.destroy(); + await builder.cleanup(); + }); + + describe('Assign workflow permissions', () => { + // Create stage with permissions + test('Can assign new stage permissions', async () => { + const { workflow } = await createWorkflow({ + ...baseWorkflow, + stages: [ + { + ...baseWorkflow.stages[0], + permissions: getStageTransitionPermissions([roles[0].id, roles[1].id]), + }, + baseWorkflow.stages[1], + ], + }); + + expect(workflow.stages[0].permissions).toHaveLength(2); + }); + + // Can unassign a role + test('Can remove stage permissions', async () => { + // Create workflow with permissions to transition to role 0 and 1 + const { workflow } = await createWorkflow({ + ...baseWorkflow, + stages: [ + { + ...baseWorkflow.stages[0], + permissions: getStageTransitionPermissions([roles[0].id, roles[1].id]), + }, + baseWorkflow.stages[1], + ], + }); + + // Update workflow to remove role 1 + const { workflow: updatedWorkflow } = await updateWorkflow(workflow.id, { + stages: [ + { + ...workflow.stages[0], + permissions: getStageTransitionPermissions([roles[0].id]), + }, + workflow.stages[1], + ], + }); + + // Validate that permissions have been removed from database + const deletedPermission = await strapi.query('admin::permission').findOne({ + where: { + id: workflow.stages[0].permissions[1].id, + }, + }); + + expect(updatedWorkflow.stages[0].permissions).toHaveLength(1); + expect(deletedPermission).toBeNull(); + }); + + test('Deleting stage removes permissions', async () => { + const { workflow } = await createWorkflow({ + ...baseWorkflow, + stages: [ + { + ...baseWorkflow.stages[0], + permissions: getStageTransitionPermissions([roles[0].id, roles[1].id]), + }, + baseWorkflow.stages[1], + ], + }); + + const { workflow: updatedWorkflow } = await updateWorkflow(workflow.id, { + ...workflow, + stages: [workflow.stages[1]], + }); + + // Deleted stage permissions should be removed from database + const permissions = await strapi.query('admin::permission').findMany({ + where: { + id: { $in: workflow.stages[0].permissions.map((p) => p.id) }, + }, + }); + + expect(permissions).toHaveLength(0); + }); + + test('Deleting workflow removes permissions', async () => { + const { workflow } = await createWorkflow({ + ...baseWorkflow, + stages: [ + { + ...baseWorkflow.stages[0], + permissions: getStageTransitionPermissions([roles[0].id, roles[1].id]), + }, + ], + }); + + await deleteWorkflow(workflow.id); + + // Deleted workflow permissions should be removed from database + const permissions = await strapi.query('admin::permission').findMany({ + where: { + id: { $in: workflow.stages[0].permissions.map((p) => p.id) }, + }, + }); + + expect(permissions).toHaveLength(0); + }); + + test('Fails when using invalid action', async () => { + const { status, error } = await createWorkflow({ + ...baseWorkflow, + stages: [ + { + ...baseWorkflow.stages[0], + permissions: [{ action: 'invalid-action', role: roles[0].id }], + }, + ], + }); + + expect(status).toBe(400); + expect(error.name).toBe('ValidationError'); + }); + + // TODO + test.skip('Can send permissions as undefined to apply partial update', async () => { + // Creates workflow with permissions + const { workflow } = await createWorkflow({ + ...baseWorkflow, + stages: [ + { + ...baseWorkflow.stages[0], + permissions: [{ action: 'invalid-action', role: roles[0].id }], + }, + ], + }); + + const { workflow: updatedWorkflow } = await updateWorkflow(workflow.id, { + ...workflow, + stages: [ + { + ...workflow.stages[0], + permissions: undefined, + }, + ], + }); + + // Permissions should be kept + expect(updatedWorkflow.stages[0].permissions).toHaveLength(1); + }); + }); +}); diff --git a/api-tests/core/admin/ee/review-workflows.test.api.js b/api-tests/core/admin/ee/review-workflows.test.api.js index ca2bfcdc0d..af54f5b5ce 100644 --- a/api-tests/core/admin/ee/review-workflows.test.api.js +++ b/api-tests/core/admin/ee/review-workflows.test.api.js @@ -1,5 +1,6 @@ 'use strict'; +const { omit } = require('lodash/fp'); const { mapAsync } = require('@strapi/utils'); const { createStrapiInstance } = require('api-tests/strapi'); @@ -439,11 +440,15 @@ describeOnCondition(edition === 'EE')('Review workflows', () => { expect(workflowRes.status).toBe(200); expect(workflowRes.body.data).toBeInstanceOf(Object); expect(workflowRes.body.data.stages).toBeInstanceOf(Array); - expect(workflowRes.body.data.stages[0]).toMatchObject(stagesUpdateData[0]); - expect(workflowRes.body.data.stages[1]).toMatchObject(stagesUpdateData[1]); + expect(workflowRes.body.data.stages[0]).toMatchObject( + omit(['updatedAt'], stagesUpdateData[0]) + ); + expect(workflowRes.body.data.stages[1]).toMatchObject( + omit(['updatedAt'], stagesUpdateData[1]) + ); expect(workflowRes.body.data.stages[2]).toMatchObject({ id: expect.any(Number), - ...stagesUpdateData[2], + ...omit(['updatedAt'], stagesUpdateData[2]), }); } else { expect(workflowRes.status).toBe(404); diff --git a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js index 026f5d6528..a35eae4c0f 100644 --- a/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js +++ b/packages/core/admin/ee/admin/pages/SettingsPage/pages/ReviewWorkflows/reducer/tests/index.test.js @@ -133,6 +133,9 @@ describe('Admin | Settings | Review Workflows | reducer', () => { expect(reducer(state, action)).toStrictEqual( expect.objectContaining({ + serverState: expect.objectContaining({ + workflow: WORKFLOW_FIXTURE, + }), clientState: expect.objectContaining({ currentWorkflow: expect.objectContaining({ data: expect.objectContaining({ diff --git a/packages/core/admin/ee/server/config/admin-actions.js b/packages/core/admin/ee/server/config/admin-actions.js index b89ea29cb6..982d6ed28e 100644 --- a/packages/core/admin/ee/server/config/admin-actions.js +++ b/packages/core/admin/ee/server/config/admin-actions.js @@ -62,5 +62,11 @@ module.exports = { category: 'review workflows', subCategory: 'options', }, + { + uid: 'review-workflows.stage.transition', + displayName: 'Change stage', + pluginName: 'admin', + section: 'internal', + }, ], }; diff --git a/packages/core/admin/ee/server/constants/workflows.js b/packages/core/admin/ee/server/constants/workflows.js index 07a79c0090..02fafb9aab 100644 --- a/packages/core/admin/ee/server/constants/workflows.js +++ b/packages/core/admin/ee/server/constants/workflows.js @@ -4,6 +4,7 @@ module.exports = { WORKFLOW_MODEL_UID: 'admin::workflow', STAGE_MODEL_UID: 'admin::workflow-stage', + STAGE_TRANSITION_UID: 'admin::review-workflows.stage.transition', STAGE_DEFAULT_COLOR: '#4945FF', ENTITY_STAGE_ATTRIBUTE: 'strapi_stage', MAX_WORKFLOWS: 200, @@ -16,4 +17,16 @@ module.exports = { 'You’ve reached the limit of stages for this workflow in your plan. Try deleting some stages or contact Sales to enable more stages.', DUPLICATED_STAGE_NAME: 'Stage names must be unique.', }, + WORKFLOW_POPULATE: { + stages: { + populate: { + permissions: { + fields: ['action', 'actionParameters'], + populate: { + role: { fields: ['id', 'name'] }, + }, + }, + }, + }, + }, }; diff --git a/packages/core/admin/ee/server/content-types/workflow-stage/index.js b/packages/core/admin/ee/server/content-types/workflow-stage/index.js index 017103c65a..05076bd75b 100644 --- a/packages/core/admin/ee/server/content-types/workflow-stage/index.js +++ b/packages/core/admin/ee/server/content-types/workflow-stage/index.js @@ -40,6 +40,12 @@ module.exports = { inversedBy: 'stages', configurable: false, }, + permissions: { + type: 'relation', + target: 'admin::permission', + relation: 'manyToMany', + configurable: false, + } }, }, }; diff --git a/packages/core/admin/ee/server/controllers/workflows/index.js b/packages/core/admin/ee/server/controllers/workflows/index.js index 9fd8468c28..9df9f61ac9 100644 --- a/packages/core/admin/ee/server/controllers/workflows/index.js +++ b/packages/core/admin/ee/server/controllers/workflows/index.js @@ -1,5 +1,6 @@ 'use strict'; +const { update, map, property } = require('lodash/fp'); const { mapAsync } = require('@strapi/utils'); const { getService } = require('../../utils'); @@ -7,7 +8,7 @@ const { validateWorkflowCreate, validateWorkflowUpdate, } = require('../../validation/review-workflows'); -const { WORKFLOW_MODEL_UID } = require('../../constants/workflows'); +const { WORKFLOW_MODEL_UID, WORKFLOW_POPULATE } = require('../../constants/workflows'); /** * @@ -22,6 +23,21 @@ function getWorkflowsPermissionChecker({ strapi }, userAbility) { .create({ userAbility, model: WORKFLOW_MODEL_UID }); } +/** + * Transforms workflow to an admin UI format. + * Some attributes (like permissions) are presented in a different format in the admin UI. + * @param {Workflow} workflow + */ +function formatWorkflowToAdmin(workflow) { + if (!workflow) return; + if (!workflow.stages) return workflow; + + // Transform permissions roles to be the id string instead of an object + const transformPermissions = map(update('role', property('id'))); + const transformStages = map(update('permissions', transformPermissions)); + return update('stages', transformStages, workflow); +} + module.exports = { /** * Create a new workflow @@ -38,10 +54,12 @@ module.exports = { const workflowBody = await validateWorkflowCreate(body.data); const workflowService = getService('workflows'); - const createdWorkflow = await workflowService.create({ - data: await sanitizeCreateInput(workflowBody), - populate, - }); + const createdWorkflow = await workflowService + .create({ + data: await sanitizeCreateInput(workflowBody), + populate, + }) + .then(formatWorkflowToAdmin); ctx.body = { data: await sanitizeOutput(createdWorkflow), @@ -61,22 +79,27 @@ module.exports = { ctx.state.userAbility ); const { populate } = await sanitizedQuery.update(query); - const workflowBody = await validateWorkflowUpdate(body.data); - const workflow = await workflowService.findById(id, { populate: ['stages'] }); + // Find if workflow exists + const workflow = await workflowService.findById(id, { populate: WORKFLOW_POPULATE }); if (!workflow) { return ctx.notFound(); } - const getPermittedFieldToUpdate = sanitizeUpdateInput(workflow); + // Sanitize input data + const getPermittedFieldToUpdate = sanitizeUpdateInput(workflow); const dataToUpdate = await getPermittedFieldToUpdate(workflowBody); - const updatedWorkflow = await workflowService.update(workflow, { - data: dataToUpdate, - populate, - }); + // Update workflow + const updatedWorkflow = await workflowService + .update(workflow, { + data: dataToUpdate, + populate, + }) + .then(formatWorkflowToAdmin); + // Send sanitized response ctx.body = { data: await sanitizeOutput(updatedWorkflow), }; @@ -96,12 +119,14 @@ module.exports = { ); const { populate } = await sanitizedQuery.delete(query); - const workflow = await workflowService.findById(id, { populate: ['stages'] }); + const workflow = await workflowService.findById(id, { populate: WORKFLOW_POPULATE }); if (!workflow) { return ctx.notFound("Workflow doesn't exist"); } - const deletedWorkflow = await workflowService.delete(workflow, { populate }); + const deletedWorkflow = await workflowService + .delete(workflow, { populate }) + .then(formatWorkflowToAdmin); ctx.body = { data: await sanitizeOutput(deletedWorkflow), @@ -122,7 +147,7 @@ module.exports = { const { populate, filters, sort } = await sanitizedQuery.read(query); const [workflows, workflowCount] = await Promise.all([ - workflowService.find({ populate, filters, sort }), + workflowService.find({ populate, filters, sort }).then(map(formatWorkflowToAdmin)), workflowService.count(), ]); @@ -151,7 +176,7 @@ module.exports = { const workflowService = getService('workflows'); const [workflow, workflowCount] = await Promise.all([ - workflowService.findById(id, { populate }), + workflowService.findById(id, { populate }).then(formatWorkflowToAdmin), workflowService.count(), ]); diff --git a/packages/core/admin/ee/server/controllers/workflows/stages/index.js b/packages/core/admin/ee/server/controllers/workflows/stages/index.js index 983fc1b166..56660f6f85 100644 --- a/packages/core/admin/ee/server/controllers/workflows/stages/index.js +++ b/packages/core/admin/ee/server/controllers/workflows/stages/index.js @@ -3,7 +3,11 @@ const { mapAsync } = require('@strapi/utils'); const { getService } = require('../../../utils'); const { validateUpdateStageOnEntity } = require('../../../validation/review-workflows'); -const { STAGE_MODEL_UID } = require('../../../constants/workflows'); +const { + STAGE_MODEL_UID, + ENTITY_STAGE_ATTRIBUTE, + STAGE_TRANSITION_UID, +} = require('../../../constants/workflows'); /** * @@ -75,18 +79,36 @@ module.exports = { */ async updateEntity(ctx) { const stagesService = getService('stages'); + const stagePermissions = getService('stage-permissions'); const workflowService = getService('workflows'); - const { model_uid: modelUID, id: entityIdString } = ctx.params; + const { model_uid: modelUID, id } = ctx.params; const { body } = ctx.request; - const entityId = Number(entityIdString); - const { sanitizeOutput } = strapi .plugin('content-manager') .service('permission-checker') .create({ userAbility: ctx.state.userAbility, model: modelUID }); + // Load entity + const entity = await strapi.entityService.findOne(modelUID, Number(id), { + populate: [ENTITY_STAGE_ATTRIBUTE], + }); + + if (!entity) { + ctx.throw(404, 'Entity not found'); + } + + // Validate if entity stage can be updated + const canTransition = stagePermissions.can( + STAGE_TRANSITION_UID, + entity[ENTITY_STAGE_ATTRIBUTE]?.id + ); + + if (!canTransition) { + ctx.throw(403, 'Forbidden stage transition'); + } + const { id: stageId } = await validateUpdateStageOnEntity( { id: Number(body?.data?.id) }, 'You should pass an id to the body of the put request.' @@ -95,8 +117,8 @@ module.exports = { const workflow = await workflowService.assertContentTypeBelongsToWorkflow(modelUID); workflowService.assertStageBelongsToWorkflow(stageId, workflow); - const entity = await stagesService.updateEntity({ id: entityId, modelUID }, stageId); + const updatedEntity = await stagesService.updateEntity({ id: entity.id, modelUID }, stageId); - ctx.body = { data: await sanitizeOutput(entity) }; + ctx.body = { data: await sanitizeOutput(updatedEntity) }; }, }; diff --git a/packages/core/admin/ee/server/services/__tests__/stages.test.js b/packages/core/admin/ee/server/services/__tests__/stages.test.js index 0d88e4b4f6..132f105d72 100644 --- a/packages/core/admin/ee/server/services/__tests__/stages.test.js +++ b/packages/core/admin/ee/server/services/__tests__/stages.test.js @@ -78,6 +78,12 @@ const servicesMock = { validateWorkflowCount: jest.fn().mockResolvedValue(true), validateWorkflowStages: jest.fn(), }, + 'admin::stage-permissions': { + register: jest.fn(), + registerMany: jest.fn(), + unregister: jest.fn(), + can: jest.fn(() => true), + }, }; const queryUpdateMock = jest.fn(() => Promise.resolve()); diff --git a/packages/core/admin/ee/server/services/__tests__/workflows.test.js b/packages/core/admin/ee/server/services/__tests__/workflows.test.js index 38bd334e64..26f4f6cc0e 100644 --- a/packages/core/admin/ee/server/services/__tests__/workflows.test.js +++ b/packages/core/admin/ee/server/services/__tests__/workflows.test.js @@ -24,7 +24,7 @@ jest.mock('../review-workflows/workflows/content-types', () => { }); const workflowsServiceFactory = require('../review-workflows/workflows'); -const { WORKFLOW_MODEL_UID } = require('../../constants/workflows'); +const { WORKFLOW_MODEL_UID, WORKFLOW_POPULATE } = require('../../constants/workflows'); const workflowMock = { id: 1, @@ -71,6 +71,12 @@ const servicesMock = { replaceStages: jest.fn(async () => stagesMock), createMany: jest.fn(async () => stagesMock), }, + 'admin::stage-permissions': { + register: jest.fn(), + registerMany: jest.fn(), + unregister: jest.fn(), + can: jest.fn(() => true), + }, }; const strapiMock = { @@ -145,7 +151,7 @@ describe('Review workflows - Workflows service', () => { return stage; }), }, - populate: undefined, + populate: WORKFLOW_POPULATE, }; test('Should call entityService with the right model UID', async () => { @@ -158,7 +164,7 @@ describe('Review workflows - Workflows service', () => { name: 'Default', stages: workflow.stages.map((stage) => stage.id), }, - populate: undefined, + populate: WORKFLOW_POPULATE, }); expect(servicesMock['admin::review-workflows-metrics'].sendDidEditWorkflow).toBeCalled(); }); @@ -177,7 +183,7 @@ describe('Review workflows - Workflows service', () => { }, ], }, - populate: undefined, + populate: WORKFLOW_POPULATE, }; test('Should call entityService with the right model UID', async () => { @@ -189,7 +195,7 @@ describe('Review workflows - Workflows service', () => { name: 'Workflow', stages: stagesMock.map((stage) => stage.id), }, - populate: { stages: true }, + populate: WORKFLOW_POPULATE, }); expect(servicesMock['admin::review-workflows-metrics'].sendDidCreateWorkflow).toBeCalled(); }); diff --git a/packages/core/admin/ee/server/services/index.js b/packages/core/admin/ee/server/services/index.js index d9f71e1b1b..71936b7c9e 100644 --- a/packages/core/admin/ee/server/services/index.js +++ b/packages/core/admin/ee/server/services/index.js @@ -8,6 +8,7 @@ module.exports = { 'seat-enforcement': require('./seat-enforcement'), workflows: require('./review-workflows/workflows'), stages: require('./review-workflows/stages'), + 'stage-permissions': require('./review-workflows/stage-permissions'), 'review-workflows': require('./review-workflows/review-workflows'), 'review-workflows-validation': require('./review-workflows/validation'), 'review-workflows-decorator': require('./review-workflows/entity-service-decorator'), diff --git a/packages/core/admin/ee/server/services/review-workflows/stage-permissions.js b/packages/core/admin/ee/server/services/review-workflows/stage-permissions.js new file mode 100644 index 0000000000..15035c74eb --- /dev/null +++ b/packages/core/admin/ee/server/services/review-workflows/stage-permissions.js @@ -0,0 +1,60 @@ +'use strict'; + +const { prop } = require('lodash/fp'); +const { + mapAsync, + errors: { ApplicationError }, +} = require('@strapi/utils'); +const { getService } = require('../../utils'); +const { STAGE_TRANSITION_UID } = require('../../constants/workflows'); + +const validActions = [STAGE_TRANSITION_UID]; + +module.exports = ({ strapi }) => { + const roleService = getService('role'); + const permissionService = getService('permission'); + + return { + async register(roleId, action, fromStage) { + if (!validActions.includes(action)) { + throw new ApplicationError(`Invalid action ${action}`); + } + const permissions = await roleService.addPermissions(roleId, [ + { + action, + actionParameters: { + from: fromStage, + }, + }, + ]); + + // TODO: Filter response + return permissions; + }, + async registerMany(permissions) { + return mapAsync(permissions, this.register); + }, + async unregister(permissions) { + const permissionIds = permissions.map(prop('id')); + await permissionService.deleteByIds(permissionIds); + }, + can(action, fromStage) { + const requestState = strapi.requestContext.get()?.state; + + if (!requestState) { + return false; + } + + // Override permissions for super admin + const userRoles = requestState.user?.roles; + if (userRoles?.some((role) => role.code === 'strapi-super-admin')) { + return true; + } + + return requestState.userAbility.can({ + name: action, + params: { from: fromStage }, + }); + }, + }; +}; diff --git a/packages/core/admin/ee/server/services/review-workflows/stages.js b/packages/core/admin/ee/server/services/review-workflows/stages.js index 2004a16407..e72a9a67d1 100644 --- a/packages/core/admin/ee/server/services/review-workflows/stages.js +++ b/packages/core/admin/ee/server/services/review-workflows/stages.js @@ -2,15 +2,20 @@ const { mapAsync, + reduceAsync, errors: { ApplicationError, ValidationError }, } = require('@strapi/utils'); -const { map } = require('lodash/fp'); +const { map, pick, isEqual } = require('lodash/fp'); const { STAGE_MODEL_UID, ENTITY_STAGE_ATTRIBUTE, ERRORS } = require('../../constants/workflows'); const { getService } = require('../../utils'); +const sanitizedStageFields = ['id', 'name', 'workflow']; +const sanitizeStageFields = pick(sanitizedStageFields); + module.exports = ({ strapi }) => { const metrics = getService('review-workflows-metrics', { strapi }); + const stagePermissionsService = getService('stage-permissions', { strapi }); const workflowsValidationService = getService('review-workflows-validation', { strapi }); return { @@ -32,20 +37,64 @@ module.exports = ({ strapi }) => { async createMany(stagesList, { fields } = {}) { const params = { select: fields ?? '*' }; + // TODO: pick the fields from the stage const stages = await Promise.all( stagesList.map((stage) => - strapi.entityService.create(STAGE_MODEL_UID, { data: stage, ...params }) + strapi.entityService.create(STAGE_MODEL_UID, { + data: sanitizeStageFields(stage), + ...params, + }) ) ); + // Create stage permissions + await reduceAsync(stagesList)(async (_, stage, idx) => { + // Ignore stages without permissions + if (!stage.permissions || stage.permissions.length === 0) { + return; + } + + const stagePermissions = stage.permissions; + const stageId = stages[idx].id; + + const permissions = await mapAsync( + stagePermissions, + // Register each stage permission + (permission) => + stagePermissionsService.register(permission.role, permission.action, stageId) + ); + + // Update stage with the new permissions + await strapi.entityService.update(STAGE_MODEL_UID, stageId, { + data: { + permissions: permissions.flat().map((p) => p.id), + }, + }); + }, []); + metrics.sendDidCreateStage(); return stages; }, - async update(stageId, stageData) { + async update(srcStage, destStage) { + let stagePermissions = []; + const stageId = destStage.id; + + await this.deleteStagePermissions([srcStage]); + + if (destStage.permissions) { + const permissions = await mapAsync(destStage.permissions, (permission) => + stagePermissionsService.register(permission.role, permission.action, stageId) + ); + stagePermissions = permissions.flat().map((p) => p.id); + } + const stage = await strapi.entityService.update(STAGE_MODEL_UID, stageId, { - data: stageData, + data: { + ...destStage, + permissions: stagePermissions, + }, }); metrics.sendDidEditStage(); @@ -53,20 +102,31 @@ module.exports = ({ strapi }) => { return stage; }, - async delete(stageId) { - const stage = await strapi.entityService.delete(STAGE_MODEL_UID, stageId); + async delete(stage) { + // Unregister all permissions related to this stage id + await this.deleteStagePermissions([stage]); + + const deletedStage = await strapi.entityService.delete(STAGE_MODEL_UID, stage.id); metrics.sendDidDeleteStage(); - return stage; + return deletedStage; }, - async deleteMany(stagesId) { + async deleteMany(stages) { + await this.deleteStagePermissions(stages); + return strapi.entityService.deleteMany(STAGE_MODEL_UID, { - filters: { id: { $in: stagesId } }, + filters: { id: { $in: stages.map((s) => s.id) } }, }); }, + async deleteStagePermissions(stages) { + // TODO: Find another way to do this for when we use the "to" parameter. + const permissions = stages.map((s) => s.permissions || []).flat(); + await stagePermissionsService.unregister(permissions || []); + }, + count({ workflowId } = {}) { const opts = {}; @@ -91,7 +151,10 @@ module.exports = ({ strapi }) => { const createdStagesIds = map('id', createdStages); // Update the workflow stages - await mapAsync(updated, (stage) => this.update(stage.id, stage)); + await mapAsync(updated, (destStage) => { + const srcStage = srcStages.find((s) => s.id === destStage.id); + return this.update(srcStage, destStage); + }); // Delete the stages that are not in the new stages list await mapAsync(deleted, async (stage) => { @@ -114,7 +177,7 @@ module.exports = ({ strapi }) => { }); }); - return this.delete(stage.id); + return this.delete(stage); }); return destStages.map((stage) => ({ ...stage, id: stage.id ?? createdStagesIds.shift() })); @@ -241,12 +304,19 @@ module.exports = ({ strapi }) => { */ function getDiffBetweenStages(sourceStages, comparisonStages) { const result = comparisonStages.reduce( + // ... + (acc, stageToCompare) => { const srcStage = sourceStages.find((stage) => stage.id === stageToCompare.id); if (!srcStage) { acc.created.push(stageToCompare); - } else if (srcStage.name !== stageToCompare.name || srcStage.color !== stageToCompare.color) { + } else if ( + !isEqual( + pick(['name', 'color', 'permissions'], srcStage), + pick(['name', 'color', 'permissions'], stageToCompare) + ) + ) { acc.updated.push(stageToCompare); } return acc; diff --git a/packages/core/admin/ee/server/services/review-workflows/workflows/index.js b/packages/core/admin/ee/server/services/review-workflows/workflows/index.js index c3c321612b..bfd6f42d3f 100644 --- a/packages/core/admin/ee/server/services/review-workflows/workflows/index.js +++ b/packages/core/admin/ee/server/services/review-workflows/workflows/index.js @@ -2,7 +2,7 @@ const { set, isString, map, get } = require('lodash/fp'); const { ApplicationError } = require('@strapi/utils').errors; -const { WORKFLOW_MODEL_UID } = require('../../../constants/workflows'); +const { WORKFLOW_MODEL_UID, WORKFLOW_POPULATE } = require('../../../constants/workflows'); const { getService } = require('../../../utils'); const { getWorkflowContentTypeFilter } = require('../../../utils/review-workflows'); const workflowsContentTypesFactory = require('./content-types'); @@ -17,6 +17,16 @@ const processFilters = ({ strapi }, filters = {}) => { return processedFilters; }; +// TODO: How can we improve this? Maybe using traversePopulate? +const processPopulate = (populate) => { + // If it does not exist or it's not an object (like an array) return the default populate + if (!populate) { + return populate; + } + + return WORKFLOW_POPULATE; +}; + module.exports = ({ strapi }) => { const workflowsContentTypes = workflowsContentTypesFactory({ strapi }); const workflowsValidationService = getService('review-workflows-validation', { strapi }); @@ -31,7 +41,9 @@ module.exports = ({ strapi }) => { */ async find(opts = {}) { const filters = processFilters({ strapi }, opts.filters); - return strapi.entityService.findMany(WORKFLOW_MODEL_UID, { ...opts, filters }); + const populate = processPopulate(opts.populate); + + return strapi.entityService.findMany(WORKFLOW_MODEL_UID, { ...opts, filters, populate }); }, /** @@ -41,7 +53,8 @@ module.exports = ({ strapi }) => { * @returns {Promise} - Workflow object matching the requested ID. */ findById(id, opts) { - return strapi.entityService.findOne(WORKFLOW_MODEL_UID, id, opts); + const populate = processPopulate(opts.populate); + return strapi.entityService.findOne(WORKFLOW_MODEL_UID, id, { ...opts, populate }); }, /** @@ -51,7 +64,7 @@ module.exports = ({ strapi }) => { * @throws {ValidationError} - If the workflow has no stages. */ async create(opts) { - let createOpts = { ...opts, populate: { stages: true } }; + let createOpts = { ...opts, populate: WORKFLOW_POPULATE }; workflowsValidationService.validateWorkflowStages(opts.data.stages); await workflowsValidationService.validateWorkflowCount(1); @@ -87,7 +100,7 @@ module.exports = ({ strapi }) => { */ async update(workflow, opts) { const stageService = getService('stages', { strapi }); - let updateOpts = { ...opts, populate: { stages: true } }; + let updateOpts = { ...opts, populate: { ...WORKFLOW_POPULATE } }; let updatedStageIds; await workflowsValidationService.validateWorkflowCount(); @@ -104,7 +117,7 @@ module.exports = ({ strapi }) => { .replaceStages(workflow.stages, opts.data.stages, workflow.contentTypes) .then((stages) => stages.map((stage) => stage.id)); - updateOpts = set('data.stages', updatedStageIds, opts); + updateOpts = set('data.stages', updatedStageIds, updateOpts); } // Update (un)assigned Content Types @@ -141,7 +154,7 @@ module.exports = ({ strapi }) => { return strapi.db.transaction(async () => { // Delete stages - await stageService.deleteMany(workflow.stages.map((stage) => stage.id)); + await stageService.deleteMany(workflow.stages); // Unassign all content types, this will migrate the content types to null await workflowsContentTypes.migrate({ diff --git a/packages/core/admin/ee/server/validation/review-workflows.js b/packages/core/admin/ee/server/validation/review-workflows.js index a7246df507..7a779d6309 100644 --- a/packages/core/admin/ee/server/validation/review-workflows.js +++ b/packages/core/admin/ee/server/validation/review-workflows.js @@ -4,11 +4,22 @@ const { yup, validateYupSchema } = require('@strapi/utils'); const { hasStageAttribute } = require('../utils/review-workflows'); +const { STAGE_TRANSITION_UID } = require('../constants/workflows'); const stageObject = yup.object().shape({ id: yup.number().integer().min(1), name: yup.string().max(255).required(), color: yup.string().matches(/^#(?:[0-9a-fA-F]{3}){1,2}$/i), // hex color + permissions: yup.array().of( + yup.object().shape({ + role: yup.number().integer().min(1).required(), + action: yup.string().oneOf([STAGE_TRANSITION_UID]).required(), + actionParameters: yup.object().shape({ + from: yup.number().integer().min(1).required(), + to: yup.number().integer().min(1), + }), + }) + ), }); const validateUpdateStageOnEntity = yup diff --git a/packages/core/admin/server/content-types/Permission.js b/packages/core/admin/server/content-types/Permission.js index aaebd94378..78dd6b9d77 100644 --- a/packages/core/admin/server/content-types/Permission.js +++ b/packages/core/admin/server/content-types/Permission.js @@ -29,6 +29,12 @@ module.exports = { configurable: false, required: true, }, + actionParameters: { + type: 'json', + configurable: false, + required: false, + default: {}, + }, subject: { type: 'string', minLength: 1, diff --git a/packages/core/admin/server/domain/permission/index.js b/packages/core/admin/server/domain/permission/index.js index 942b62b097..d6da7007ee 100644 --- a/packages/core/admin/server/domain/permission/index.js +++ b/packages/core/admin/server/domain/permission/index.js @@ -26,8 +26,16 @@ const { * @property {string} subject - The subject on which the permission should applies */ -const permissionFields = ['id', 'action', 'subject', 'properties', 'conditions', 'role']; -const sanitizedPermissionFields = ['id', 'action', 'subject', 'properties', 'conditions']; +const permissionFields = [ + 'id', + 'action', + 'actionParameters', + 'subject', + 'properties', + 'conditions', + 'role', +]; +const sanitizedPermissionFields = ['id', 'action', 'actionParameters', 'subject', 'properties', 'conditions']; const sanitizePermissionFields = pick(sanitizedPermissionFields); @@ -36,6 +44,7 @@ const sanitizePermissionFields = pick(sanitizedPermissionFields); * @return {Permission} */ const getDefaultPermission = () => ({ + actionParameters: {}, conditions: [], properties: {}, subject: null, diff --git a/packages/core/admin/server/services/__tests__/content-type.test.js b/packages/core/admin/server/services/__tests__/content-type.test.js index 405b033e53..631b431f80 100644 --- a/packages/core/admin/server/services/__tests__/content-type.test.js +++ b/packages/core/admin/server/services/__tests__/content-type.test.js @@ -232,6 +232,7 @@ describe('Content-Type', () => { expect(resultLevel1).toEqual([ { action: 'action-1', + actionParameters: {}, subject: 'country', properties: { fields: ['name', 'code'] }, conditions: [], @@ -253,12 +254,14 @@ describe('Content-Type', () => { expect(resultLevel1).toEqual([ { action: 'action-1', + actionParameters: {}, subject: 'country', properties: { fields: ['name', 'code'] }, conditions: [], }, { action: 'action-1', + actionParameters: {}, subject: 'user', properties: { fields: ['firstname', 'restaurant', 'car'] }, conditions: [], @@ -280,12 +283,14 @@ describe('Content-Type', () => { expect(resultLevel1).toEqual([ { action: 'action-1', + actionParameters: {}, subject: 'country', properties: { fields: ['name', 'code'] }, conditions: [], }, { action: 'action-1', + actionParameters: {}, subject: 'user', properties: { fields: [ @@ -312,12 +317,14 @@ describe('Content-Type', () => { expect(resultLevel1).toEqual([ { action: 'action-1', + actionParameters: {}, subject: 'country', properties: { fields: ['name', 'code'] }, conditions: [], }, { action: 'action-1', + actionParameters: {}, subject: 'user', properties: { fields: [ @@ -387,6 +394,7 @@ describe('Content-Type', () => { expect(res).toEqual([ { action: 'foo', + actionParameters: {}, subject: 'user', properties: { fields: expectedFields }, conditions: [], diff --git a/packages/core/admin/server/services/__tests__/permission.test.js b/packages/core/admin/server/services/__tests__/permission.test.js index d771256fcd..c0649a1200 100644 --- a/packages/core/admin/server/services/__tests__/permission.test.js +++ b/packages/core/admin/server/services/__tests__/permission.test.js @@ -85,34 +85,40 @@ describe('Permission Service', () => { { id: 1, action: 'action-1', + actionParameters: {}, properties: { fields: ['name'] }, }, { id: 2, action: 'action-2', + actionParameters: {}, properties: { fields: ['name'] }, }, { id: 3, action: 'action-3', + actionParameters: {}, subject: 'country', properties: { fields: ['name'] }, }, { id: 4, action: 'action-3', + actionParameters: {}, subject: 'planet', properties: { fields: ['name'] }, }, { id: 5, action: 'action-1', + actionParameters: {}, subject: 'planet', properties: { fields: ['name', 'description'] }, }, { id: 6, action: 'action-1', + actionParameters: {}, subject: 'country', properties: { fields: null }, }, diff --git a/packages/core/admin/server/services/__tests__/role.test.js b/packages/core/admin/server/services/__tests__/role.test.js index 94965deb4b..a216266362 100644 --- a/packages/core/admin/server/services/__tests__/role.test.js +++ b/packages/core/admin/server/services/__tests__/role.test.js @@ -404,6 +404,7 @@ describe('Role', () => { const permissions = [ { action: 'action-1', + actionParameters: {}, subject: 'country', properties: { fields: ['name'] }, conditions: [], @@ -413,36 +414,42 @@ describe('Role', () => { const defaultPermissions = [ { action: 'plugin::upload.read', + actionParameters: {}, conditions: ['admin::is-creator'], properties: {}, subject: null, }, { action: 'plugin::upload.configure-view', + actionParameters: {}, conditions: [], properties: {}, subject: null, }, { action: 'plugin::upload.assets.create', + actionParameters: {}, conditions: [], properties: {}, subject: null, }, { action: 'plugin::upload.assets.update', + actionParameters: {}, conditions: ['admin::is-creator'], properties: {}, subject: null, }, { action: 'plugin::upload.assets.download', + actionParameters: {}, conditions: [], properties: {}, subject: null, }, { action: 'plugin::upload.assets.copy-link', + actionParameters: {}, conditions: [], properties: {}, subject: null, @@ -625,24 +632,28 @@ describe('Role', () => { const permissions = [ { action: 'action-1', + actionParameters: {}, subject: 'country', properties: { fields: ['name'] }, conditions: [], }, { action: 'action-test2', + actionParameters: {}, subject: 'test-subject1', properties: {}, conditions: [], }, { action: 'action-test2', + actionParameters: {}, subject: 'test-subject2', properties: {}, conditions: [], }, { action: 'action-test3', + actionParameters: {}, subject: null, properties: {}, conditions: [], @@ -759,11 +770,46 @@ describe('Role', () => { expect(createMany).toHaveBeenCalledTimes(1); expect(createMany).toHaveBeenCalledWith([ - { action: 'action-0', conditions: [], properties: {}, role: 1, subject: null }, - { action: 'action-1', conditions: [], properties: {}, role: 1, subject: null }, - { action: 'action-2', conditions: [], properties: {}, role: 1, subject: null }, - { action: 'action-3', conditions: [], properties: {}, role: 1, subject: null }, - { action: 'action-4', conditions: ['cond'], properties: {}, role: 1, subject: null }, + { + action: 'action-0', + actionParameters: {}, + conditions: [], + properties: {}, + role: 1, + subject: null, + }, + { + action: 'action-1', + actionParameters: {}, + conditions: [], + properties: {}, + role: 1, + subject: null, + }, + { + action: 'action-2', + actionParameters: {}, + conditions: [], + properties: {}, + role: 1, + subject: null, + }, + { + action: 'action-3', + actionParameters: {}, + conditions: [], + properties: {}, + role: 1, + subject: null, + }, + { + action: 'action-4', + actionParameters: {}, + conditions: ['cond'], + properties: {}, + role: 1, + subject: null, + }, ]); }); }); @@ -775,6 +821,7 @@ describe('Role', () => { const permissions = [ { action: 'someAction', + actionParameters: {}, conditions: [], properties: { fields: null }, subject: null, diff --git a/packages/core/admin/server/services/role.js b/packages/core/admin/server/services/role.js index 2deb9c66fb..d9cbd715f6 100644 --- a/packages/core/admin/server/services/role.js +++ b/packages/core/admin/server/services/role.js @@ -24,7 +24,7 @@ const ACTIONS = { const sanitizeRole = omit(['users', 'permissions']); -const COMPARABLE_FIELDS = ['conditions', 'properties', 'subject', 'action']; +const COMPARABLE_FIELDS = ['conditions', 'properties', 'subject', 'action', 'actionParameters']; const pickComparableFields = pick(COMPARABLE_FIELDS); const jsonClean = (data) => JSON.parse(JSON.stringify(data)); @@ -368,17 +368,20 @@ const assignPermissions = async (roleId, permissions = []) => { return permissionsToReturn; }; + const addPermissions = async (roleId, permissions) => { const { conditionProvider, createMany } = getService('permission'); const { sanitizeConditions } = permissionDomain; const permissionsWithRole = permissions .map(set('role', roleId)) - .map(sanitizeConditions(conditionProvider)); + .map(sanitizeConditions(conditionProvider)) + .map(permissionDomain.create); return createMany(permissionsWithRole); }; + const isContentTypeAction = (action) => action.section === CONTENT_TYPE_SECTION; /** diff --git a/packages/core/admin/server/validation/action-provider.js b/packages/core/admin/server/validation/action-provider.js index 1bcf390bde..b1bb9bed78 100644 --- a/packages/core/admin/server/validation/action-provider.js +++ b/packages/core/admin/server/validation/action-provider.js @@ -17,7 +17,7 @@ const registerProviderActionSchema = yup (v) => `${v.path}: The id can only contain lowercase letters, dots and hyphens.` ) .required(), - section: yup.string().oneOf(['contentTypes', 'plugins', 'settings']).required(), + section: yup.string().oneOf(['contentTypes', 'plugins', 'settings', 'internal']).required(), pluginName: yup.mixed().when('section', { is: 'plugins', then: validators.isAPluginName.required(), diff --git a/packages/core/permissions/package.json b/packages/core/permissions/package.json index 1f93fbdd43..6687cb4f10 100644 --- a/packages/core/permissions/package.json +++ b/packages/core/permissions/package.json @@ -38,6 +38,7 @@ "@casl/ability": "5.4.4", "@strapi/utils": "4.12.1", "lodash": "4.17.21", + "qs": "6.11.1", "sift": "16.0.1" }, "devDependencies": { diff --git a/packages/core/permissions/src/domain/permission/index.ts b/packages/core/permissions/src/domain/permission/index.ts index 3e95d249eb..7d60c27cf0 100644 --- a/packages/core/permissions/src/domain/permission/index.ts +++ b/packages/core/permissions/src/domain/permission/index.ts @@ -4,8 +4,14 @@ const PERMISSION_FIELDS = ['action', 'subject', 'properties', 'conditions'] as c const sanitizePermissionFields = _.pick(PERMISSION_FIELDS); +export interface ParametrizedAction { + name: string; + params: Record; +} + export interface Permission { action: string; + actionParameters?: Record; subject?: string | object | null; properties?: object; conditions?: string[]; diff --git a/packages/core/permissions/src/engine/abilities/casl-ability.ts b/packages/core/permissions/src/engine/abilities/casl-ability.ts index f4fa9a3d7e..62e16e127d 100644 --- a/packages/core/permissions/src/engine/abilities/casl-ability.ts +++ b/packages/core/permissions/src/engine/abilities/casl-ability.ts @@ -1,9 +1,16 @@ import * as sift from 'sift'; +import qs from 'qs'; import { AbilityBuilder, Ability, Subject } from '@casl/ability'; import { pick, isNil, isObject } from 'lodash/fp'; + +export interface ParametrizedAction { + name: string; + params: Record; +} + export interface PermissionRule { - action: string; + action: string | ParametrizedAction; subject?: Subject | null; properties?: { fields?: string[]; @@ -13,6 +20,7 @@ export interface PermissionRule { export interface CustomAbilityBuilder { can(permission: PermissionRule): ReturnType['can']>; + buildParametrizedAction: (parametrizedAction: ParametrizedAction) => string; build(): Ability; } @@ -37,6 +45,10 @@ const conditionsMatcher = (conditions: unknown) => { return sift.createQueryTester(conditions, { operations }); }; +const buildParametrizedAction = ({ name, params }: ParametrizedAction) => { + return `${name}?${qs.stringify(params)}`; +}; + /** * Casl Ability Builder. */ @@ -48,16 +60,36 @@ export const caslAbilityBuilder = (): CustomAbilityBuilder => { const { action, subject, properties = {}, condition } = permission; const { fields } = properties; + const caslAction = (typeof action === "string") ? action : buildParametrizedAction(action); + return can( - action, + caslAction, isNil(subject) ? 'all' : subject, fields, isObject(condition) ? condition : undefined ); }, + buildParametrizedAction({ name, params }: ParametrizedAction) { + return `${name}?${qs.stringify(params)}`; + }, + build() { - return build({ conditionsMatcher }); + const ability = build({ conditionsMatcher }); + + function decorateCan(originalCan: Ability['can']) { + return function (...args: Parameters) { + const [action, ...rest] = args; + const caslAction = (typeof action === "string") ? action : buildParametrizedAction(action); + + // Call the original `can` method + return originalCan.apply(ability, [caslAction, ...rest]); + }; + } + + ability.can = decorateCan(ability.can); + return ability; + }, ...rest, diff --git a/packages/core/permissions/src/engine/index.ts b/packages/core/permissions/src/engine/index.ts index 34e034cd73..31a70fa465 100644 --- a/packages/core/permissions/src/engine/index.ts +++ b/packages/core/permissions/src/engine/index.ts @@ -1,4 +1,5 @@ import _ from 'lodash/fp'; +import qs from 'qs'; import { Ability } from '@casl/ability'; import { providerFactory } from '@strapi/utils'; @@ -93,7 +94,19 @@ const newEngine = (params: EngineParams): Engine => { await state.hooks['before-evaluate.permission'].call(createBeforeEvaluateContext(permission)); - const { action, subject, properties, conditions = [] } = permission; + const { + action: actionName, + subject, + properties, + conditions = [], + actionParameters = {}, + } = permission; + + let action = actionName; + + if (actionParameters && Object.keys(actionParameters).length > 0) { + action = `${actionName}?${qs.stringify(actionParameters)}`; + } if (conditions.length === 0) { return register({ action, subject, properties }); diff --git a/packages/plugins/users-permissions/admin/src/pages/Roles/ListPage/components/TableBody.js b/packages/plugins/users-permissions/admin/src/pages/Roles/ListPage/components/TableBody.js index f7ce8fd5fd..949e7eb155 100644 --- a/packages/plugins/users-permissions/admin/src/pages/Roles/ListPage/components/TableBody.js +++ b/packages/plugins/users-permissions/admin/src/pages/Roles/ListPage/components/TableBody.js @@ -39,7 +39,10 @@ const TableBody = ({ sortedRoles, canDelete, permissions, setRoleToDelete, onDel {formatMessage( - { id: 'Roles.RoleRow.user-count', defaultMessage: '{number, plural, =0 {# user} one {# user} other {# users}}' }, + { + id: 'Roles.RoleRow.user-count', + defaultMessage: '{number, plural, =0 {# user} one {# user} other {# users}}', + }, { number: role.nb_users } )} diff --git a/yarn.lock b/yarn.lock index 99b170e1fa..8cd82a22ba 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7903,6 +7903,7 @@ __metadata: "@strapi/utils": 4.12.1 eslint-config-custom: 4.12.1 lodash: 4.17.21 + qs: 6.11.1 sift: 16.0.1 tsconfig: 4.12.1 languageName: unknown