From c8245b9f872fd2c85ecaaa0da426b3ef9cb03113 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Tue, 7 Feb 2023 17:40:36 +0100 Subject: [PATCH] fix: Error workflow now correctly checks for subworkflow permissions (#5390) --- packages/cli/src/WorkflowHelpers.ts | 53 ++++++++++------------------- 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index 5f6f2e0e7a..c4e19b89a5 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -29,8 +29,9 @@ import { WorkflowRunner } from '@/WorkflowRunner'; import config from '@/config'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import type { User } from '@db/entities/User'; -import { getWorkflowOwner, whereClause } from '@/UserManagement/UserManagementHelper'; +import { whereClause } from '@/UserManagement/UserManagementHelper'; import omit from 'lodash.omit'; +import { PermissionChecker } from './UserManagement/PermissionChecker'; const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType'); @@ -94,30 +95,7 @@ export async function executeErrorWorkflow( ): Promise { // Wrap everything in try/catch to make sure that no errors bubble up and all get caught here try { - let workflowData: WorkflowEntity | null = null; - if (workflowId !== workflowErrorData.workflow.id) { - // To make this code easier to understand, we split it in 2 parts: - // 1) Fetch the owner of the errored workflows and then - // 2) if now instance owner, then check if the user has access to the - // triggered workflow. - - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const user = await getWorkflowOwner(workflowErrorData.workflow.id!); - - if (user.globalRole.name === 'owner') { - workflowData = await Db.collections.Workflow.findOneBy({ id: workflowId }); - } else { - const sharedWorkflowData = await Db.collections.SharedWorkflow.findOne({ - where: { workflowId, userId: user.id }, - relations: ['workflow'], - }); - if (sharedWorkflowData) { - workflowData = sharedWorkflowData.workflow; - } - } - } else { - workflowData = await Db.collections.Workflow.findOneBy({ id: workflowId }); - } + const workflowData = await Db.collections.Workflow.findOneBy({ id: workflowId }); if (workflowData === null) { // The error workflow could not be found @@ -129,15 +107,6 @@ export async function executeErrorWorkflow( return; } - const user = await getWorkflowOwner(workflowId); - if (user.id !== runningUser.id) { - // The error workflow could not be found - Logger.warn( - `An attempt to execute workflow ID ${workflowId} as error workflow was blocked due to wrong permission`, - ); - return; - } - const executionMode = 'error'; const nodeTypes = NodeTypes(); @@ -152,6 +121,20 @@ export async function executeErrorWorkflow( settings: workflowData.settings, }); + try { + await PermissionChecker.checkSubworkflowExecutePolicy( + workflowInstance, + runningUser.id, + workflowErrorData.workflow.id, + ); + } catch (error) { + Logger.info('Error workflow execution blocked due to subworkflow settings', { + erroredWorkflowId: workflowErrorData.workflow.id, + errorWorkflowId: workflowId, + }); + return; + } + let node: INode; let workflowStartNode: INode | undefined; for (const nodeName of Object.keys(workflowInstance.nodes)) { @@ -204,7 +187,7 @@ export async function executeErrorWorkflow( executionMode, executionData: runExecutionData, workflowData, - userId: user.id, + userId: runningUser.id, }; const workflowRunner = new WorkflowRunner();