diff --git a/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts b/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts index 4c3af0e69646a..4b0154e11ccc9 100644 --- a/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts +++ b/packages/cli/src/__tests__/workflow-execute-additional-data.test.ts @@ -17,12 +17,14 @@ import { ExecutionRepository } from '@/databases/repositories/execution.reposito import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; import { VariablesService } from '@/environments.ee/variables/variables.service.ee'; import { EventService } from '@/events/event.service'; +import { + CredentialsPermissionChecker, + SubworkflowPolicyChecker, +} from '@/executions/pre-execution-checks'; import { ExternalHooks } from '@/external-hooks'; import { SecretsHelper } from '@/secrets-helpers.ee'; import { WorkflowStatisticsService } from '@/services/workflow-statistics.service'; -import { SubworkflowPolicyChecker } from '@/subworkflows/subworkflow-policy-checker.service'; import { Telemetry } from '@/telemetry'; -import { PermissionChecker } from '@/user-management/permission-checker'; import { executeWorkflow, getBase, getRunData } from '@/workflow-execute-additional-data'; import { mockInstance } from '@test/mocking'; @@ -91,7 +93,7 @@ describe('WorkflowExecuteAdditionalData', () => { mockInstance(Telemetry); const workflowRepository = mockInstance(WorkflowRepository); const activeExecutions = mockInstance(ActiveExecutions); - mockInstance(PermissionChecker); + mockInstance(CredentialsPermissionChecker); mockInstance(SubworkflowPolicyChecker); mockInstance(WorkflowStatisticsService); diff --git a/packages/cli/src/__tests__/workflow-runner.test.ts b/packages/cli/src/__tests__/workflow-runner.test.ts index 0ea0646276e11..8a666114991eb 100644 --- a/packages/cli/src/__tests__/workflow-runner.test.ts +++ b/packages/cli/src/__tests__/workflow-runner.test.ts @@ -21,8 +21,8 @@ import config from '@/config'; import type { ExecutionEntity } from '@/databases/entities/execution-entity'; import type { User } from '@/databases/entities/user'; import { ExecutionNotFoundError } from '@/errors/execution-not-found-error'; +import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks'; import { Telemetry } from '@/telemetry'; -import { PermissionChecker } from '@/user-management/permission-checker'; import { WorkflowRunner } from '@/workflow-runner'; import { mockInstance } from '@test/mocking'; import { createExecution } from '@test-integration/db/executions'; @@ -131,7 +131,7 @@ describe('run', () => { const activeExecutions = Container.get(ActiveExecutions); jest.spyOn(activeExecutions, 'add').mockResolvedValue('1'); jest.spyOn(activeExecutions, 'attachWorkflowExecution').mockReturnValueOnce(); - const permissionChecker = Container.get(PermissionChecker); + const permissionChecker = Container.get(CredentialsPermissionChecker); jest.spyOn(permissionChecker, 'check').mockResolvedValueOnce(); jest.spyOn(WorkflowExecute.prototype, 'processRunExecutionData').mockReturnValueOnce( @@ -171,7 +171,7 @@ describe('run', () => { const activeExecutions = Container.get(ActiveExecutions); jest.spyOn(activeExecutions, 'add').mockResolvedValue('1'); jest.spyOn(activeExecutions, 'attachWorkflowExecution').mockReturnValueOnce(); - const permissionChecker = Container.get(PermissionChecker); + const permissionChecker = Container.get(CredentialsPermissionChecker); jest.spyOn(permissionChecker, 'check').mockResolvedValueOnce(); jest.spyOn(WorkflowExecute.prototype, 'processRunExecutionData').mockReturnValueOnce( diff --git a/packages/cli/src/executions/__tests__/execution-data.service.test.ts b/packages/cli/src/executions/__tests__/execution-data.service.test.ts new file mode 100644 index 0000000000000..d579279af0a61 --- /dev/null +++ b/packages/cli/src/executions/__tests__/execution-data.service.test.ts @@ -0,0 +1,47 @@ +import { mock } from 'jest-mock-extended'; +import { NodeOperationError } from 'n8n-workflow'; +import type { INode, WorkflowExecuteMode } from 'n8n-workflow'; + +import { ExecutionDataService } from '../execution-data.service'; + +describe('ExecutionDataService', () => { + const service = new ExecutionDataService(); + + describe('generateFailedExecutionFromError', () => { + const mode: WorkflowExecuteMode = 'manual'; + const node = mock({ name: 'Test Node' }); + const error = new NodeOperationError(node, 'Test error message'); + + it('should generate a failed execution with error details', () => { + const startTime = Date.now(); + + const result = service.generateFailedExecutionFromError(mode, error, node, startTime); + + expect(result.mode).toBe(mode); + expect(result.status).toBe('error'); + expect(result.startedAt).toBeInstanceOf(Date); + expect(result.stoppedAt).toBeInstanceOf(Date); + expect(result.data.resultData.error?.message).toBe(error.message); + + const taskData = result.data.resultData.runData[node.name][0]; + expect(taskData.error?.message).toBe(error.message); + expect(taskData.startTime).toBe(startTime); + expect(taskData.executionStatus).toBe('error'); + expect(result.data.resultData.lastNodeExecuted).toBe(node.name); + expect(result.data.executionData?.nodeExecutionStack[0].node).toEqual(node); + }); + + it('should generate a failed execution without node details if node is undefined', () => { + const result = service.generateFailedExecutionFromError(mode, error, undefined); + + expect(result.mode).toBe(mode); + expect(result.status).toBe('error'); + expect(result.startedAt).toBeInstanceOf(Date); + expect(result.stoppedAt).toBeInstanceOf(Date); + expect(result.data.resultData.error?.message).toBe(error.message); + expect(result.data.resultData.runData).toEqual({}); + expect(result.data.resultData.lastNodeExecuted).toBeUndefined(); + expect(result.data.executionData).toBeUndefined(); + }); + }); +}); diff --git a/packages/cli/src/executions/execution-data.service.ts b/packages/cli/src/executions/execution-data.service.ts new file mode 100644 index 0000000000000..9e23e39d1dc84 --- /dev/null +++ b/packages/cli/src/executions/execution-data.service.ts @@ -0,0 +1,62 @@ +import { Service } from '@n8n/di'; +import type { ExecutionError, INode, IRun, WorkflowExecuteMode } from 'n8n-workflow'; + +@Service() +export class ExecutionDataService { + generateFailedExecutionFromError( + mode: WorkflowExecuteMode, + error: ExecutionError, + node: INode | undefined, + startTime = Date.now(), + ): IRun { + const executionError = { + ...error, + message: error.message, + stack: error.stack, + }; + const returnData: IRun = { + data: { + resultData: { + error: executionError, + runData: {}, + }, + }, + finished: false, + mode, + startedAt: new Date(), + stoppedAt: new Date(), + status: 'error', + }; + + if (node) { + returnData.data.startData = { + destinationNode: node.name, + runNodeFilter: [node.name], + }; + returnData.data.resultData.lastNodeExecuted = node.name; + returnData.data.resultData.runData[node.name] = [ + { + startTime, + executionTime: 0, + executionStatus: 'error', + error: executionError, + source: [], + }, + ]; + returnData.data.executionData = { + contextData: {}, + metadata: {}, + waitingExecution: {}, + waitingExecutionSource: {}, + nodeExecutionStack: [ + { + node, + data: {}, + source: null, + }, + ], + }; + } + return returnData; + } +} diff --git a/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts b/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts new file mode 100644 index 0000000000000..b957a3f5175ff --- /dev/null +++ b/packages/cli/src/executions/pre-execution-checks/__tests__/credentials-permission-checker.test.ts @@ -0,0 +1,108 @@ +import { mock } from 'jest-mock-extended'; +import type { INode } from 'n8n-workflow'; + +import type { Project } from '@/databases/entities/project'; +import type { User } from '@/databases/entities/user'; +import type { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; +import type { OwnershipService } from '@/services/ownership.service'; +import type { ProjectService } from '@/services/project.service.ee'; + +import { CredentialsPermissionChecker } from '../credentials-permission-checker'; + +describe('CredentialsPermissionChecker', () => { + const sharedCredentialsRepository = mock(); + const ownershipService = mock(); + const projectService = mock(); + const permissionChecker = new CredentialsPermissionChecker( + sharedCredentialsRepository, + ownershipService, + projectService, + ); + + const workflowId = 'workflow123'; + const credentialId = 'cred123'; + const personalProject = mock({ + id: 'personal-project', + name: 'Personal Project', + type: 'personal', + }); + + const node = mock({ + name: 'Test Node', + credentials: { + someCredential: { + id: credentialId, + name: 'Test Credential', + }, + }, + disabled: false, + }); + + beforeEach(async () => { + jest.resetAllMocks(); + + node.credentials!.someCredential.id = credentialId; + ownershipService.getWorkflowProjectCached.mockResolvedValueOnce(personalProject); + projectService.findProjectsWorkflowIsIn.mockResolvedValueOnce([personalProject.id]); + }); + + it('should throw if a node has a credential without an id', async () => { + node.credentials!.someCredential.id = null; + + await expect(permissionChecker.check(workflowId, [node])).rejects.toThrow( + 'Node "Test Node" uses invalid credential', + ); + + expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId); + expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).not.toHaveBeenCalled(); + }); + + it('should throw if a credential is not accessible', async () => { + ownershipService.getPersonalProjectOwnerCached.mockResolvedValueOnce(null); + sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValueOnce([]); + + await expect(permissionChecker.check(workflowId, [node])).rejects.toThrow( + 'Node "Test Node" does not have access to the credential', + ); + + expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId); + expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith( + [personalProject.id], + [credentialId], + ); + }); + + it('should not throw an error if the workflow has no credentials', async () => { + await expect(permissionChecker.check(workflowId, [])).resolves.not.toThrow(); + + expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId); + expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).not.toHaveBeenCalled(); + }); + + it('should not throw an error if all credentials are accessible', async () => { + ownershipService.getPersonalProjectOwnerCached.mockResolvedValueOnce(null); + sharedCredentialsRepository.getFilteredAccessibleCredentials.mockResolvedValueOnce([ + credentialId, + ]); + + await expect(permissionChecker.check(workflowId, [node])).resolves.not.toThrow(); + + expect(projectService.findProjectsWorkflowIsIn).toHaveBeenCalledWith(workflowId); + expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).toHaveBeenCalledWith( + [personalProject.id], + [credentialId], + ); + }); + + it('should skip credential checks if the home project owner has global scope', async () => { + const projectOwner = mock({ + hasGlobalScope: (scope) => scope === 'credential:list', + }); + ownershipService.getPersonalProjectOwnerCached.mockResolvedValueOnce(projectOwner); + + await expect(permissionChecker.check(workflowId, [node])).resolves.not.toThrow(); + + expect(projectService.findProjectsWorkflowIsIn).not.toHaveBeenCalled(); + expect(sharedCredentialsRepository.getFilteredAccessibleCredentials).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/src/subworkflows/__tests__/subworkflow-policy-checker.test.ts b/packages/cli/src/executions/pre-execution-checks/__tests__/subworkflow-policy-checker.test.ts similarity index 99% rename from packages/cli/src/subworkflows/__tests__/subworkflow-policy-checker.test.ts rename to packages/cli/src/executions/pre-execution-checks/__tests__/subworkflow-policy-checker.test.ts index a47524fc1bb84..35eab2ceca071 100644 --- a/packages/cli/src/subworkflows/__tests__/subworkflow-policy-checker.test.ts +++ b/packages/cli/src/executions/pre-execution-checks/__tests__/subworkflow-policy-checker.test.ts @@ -15,7 +15,7 @@ import { OwnershipService } from '@/services/ownership.service'; import type { UrlService } from '@/services/url.service'; import { mockInstance } from '@test/mocking'; -import { SubworkflowPolicyChecker } from '../subworkflow-policy-checker.service'; +import { SubworkflowPolicyChecker } from '../subworkflow-policy-checker'; describe('SubworkflowPolicyChecker', () => { const ownershipService = mockInstance(OwnershipService); diff --git a/packages/cli/src/user-management/permission-checker.ts b/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts similarity index 68% rename from packages/cli/src/user-management/permission-checker.ts rename to packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts index 72ac867061052..3123ea19d2ef2 100644 --- a/packages/cli/src/user-management/permission-checker.ts +++ b/packages/cli/src/executions/pre-execution-checks/credentials-permission-checker.ts @@ -1,13 +1,36 @@ import { Service } from '@n8n/di'; import type { INode } from 'n8n-workflow'; -import { CredentialAccessError, NodeOperationError } from 'n8n-workflow'; +import { UserError } from 'n8n-workflow'; +import type { Project } from '@/databases/entities/project'; import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; import { OwnershipService } from '@/services/ownership.service'; import { ProjectService } from '@/services/project.service.ee'; +class InvalidCredentialError extends UserError { + override description = 'Please recreate the credential.'; + + constructor(readonly node: INode) { + super(`Node "${node.name}" uses invalid credential`); + } +} + +class InaccessibleCredentialError extends UserError { + override description = + this.project.type === 'personal' + ? 'Please recreate the credential or ask its owner to share it with you.' + : `Please make sure that the credential is shared with the project "${this.project.name}"`; + + constructor( + readonly node: INode, + private readonly project: Project, + ) { + super(`Node "${node.name}" does not have access to the credential`); + } +} + @Service() -export class PermissionChecker { +export class CredentialsPermissionChecker { constructor( private readonly sharedCredentialsRepository: SharedCredentialsRepository, private readonly ownershipService: OwnershipService, @@ -42,7 +65,7 @@ export class PermissionChecker { for (const credentialsId of workflowCredIds) { if (!accessible.includes(credentialsId)) { const nodeToFlag = credIdsToNodes[credentialsId][0]; - throw new CredentialAccessError(nodeToFlag, credentialsId, workflowId); + throw new InaccessibleCredentialError(nodeToFlag, homeProject); } } } @@ -52,12 +75,7 @@ export class PermissionChecker { if (node.disabled || !node.credentials) return map; Object.values(node.credentials).forEach((cred) => { - if (!cred.id) { - throw new NodeOperationError(node, 'Node uses invalid credential', { - description: 'Please recreate the credential.', - level: 'warning', - }); - } + if (!cred.id) throw new InvalidCredentialError(node); map[cred.id] = map[cred.id] ? [...map[cred.id], node] : [node]; }); diff --git a/packages/cli/src/executions/pre-execution-checks/index.ts b/packages/cli/src/executions/pre-execution-checks/index.ts new file mode 100644 index 0000000000000..8b1165e24e0fb --- /dev/null +++ b/packages/cli/src/executions/pre-execution-checks/index.ts @@ -0,0 +1,2 @@ +export { CredentialsPermissionChecker } from './credentials-permission-checker'; +export { SubworkflowPolicyChecker } from './subworkflow-policy-checker'; diff --git a/packages/cli/src/subworkflows/subworkflow-policy-checker.service.ts b/packages/cli/src/executions/pre-execution-checks/subworkflow-policy-checker.ts similarity index 100% rename from packages/cli/src/subworkflows/subworkflow-policy-checker.service.ts rename to packages/cli/src/executions/pre-execution-checks/subworkflow-policy-checker.ts diff --git a/packages/cli/src/workflow-execute-additional-data.ts b/packages/cli/src/workflow-execute-additional-data.ts index 43b478703eeb8..0155f985ec1bd 100644 --- a/packages/cli/src/workflow-execute-additional-data.ts +++ b/packages/cli/src/workflow-execute-additional-data.ts @@ -14,7 +14,6 @@ import type { INode, INodeExecutionData, INodeParameters, - IRun, IRunExecutionData, IWorkflowBase, IWorkflowExecuteAdditionalData, @@ -38,14 +37,17 @@ import { WorkflowRepository } from '@/databases/repositories/workflow.repository import { EventService } from '@/events/event.service'; import type { AiEventMap, AiEventPayload } from '@/events/maps/ai.event-map'; import { getLifecycleHooksForSubExecutions } from '@/execution-lifecycle/execution-lifecycle-hooks'; +import { ExecutionDataService } from '@/executions/execution-data.service'; +import { + CredentialsPermissionChecker, + SubworkflowPolicyChecker, +} from '@/executions/pre-execution-checks'; import type { UpdateExecutionPayload } from '@/interfaces'; import { NodeTypes } from '@/node-types'; import { Push } from '@/push'; import { SecretsHelper } from '@/secrets-helpers.ee'; import { UrlService } from '@/services/url.service'; -import { SubworkflowPolicyChecker } from '@/subworkflows/subworkflow-policy-checker.service'; import { TaskRequester } from '@/task-runners/task-managers/task-requester'; -import { PermissionChecker } from '@/user-management/permission-checker'; import { findSubworkflowStart } from '@/utils'; import { objectToError } from '@/utils/object-to-error'; import * as WorkflowHelpers from '@/workflow-helpers'; @@ -204,9 +206,11 @@ async function startExecution( */ await executionRepository.setRunning(executionId); + const startTime = Date.now(); + let data; try { - await Container.get(PermissionChecker).check(workflowData.id, workflowData.nodes); + await Container.get(CredentialsPermissionChecker).check(workflowData.id, workflowData.nodes); await Container.get(SubworkflowPolicyChecker).check( workflow, options.parentWorkflowId, @@ -239,7 +243,7 @@ async function startExecution( // If no timeout was given from the parent, then we use our timeout. subworkflowTimeout = Math.min( additionalData.executionTimeoutTimestamp || Number.MAX_SAFE_INTEGER, - Date.now() + workflowSettings.executionTimeout * 1000, + startTime + workflowSettings.executionTimeout * 1000, ); } @@ -257,20 +261,14 @@ async function startExecution( activeExecutions.attachWorkflowExecution(executionId, execution); data = await execution; } catch (error) { - const executionError = error ? (error as ExecutionError) : undefined; - const fullRunData: IRun = { - data: { - resultData: { - error: executionError, - runData: {}, - }, - }, - finished: false, - mode: 'integrated', - startedAt: new Date(), - stoppedAt: new Date(), - status: 'error', - }; + const executionError = error as ExecutionError; + const fullRunData = Container.get(ExecutionDataService).generateFailedExecutionFromError( + runData.executionMode, + executionError, + 'node' in executionError ? executionError.node : undefined, + startTime, + ); + // When failing, we might not have finished the execution // Therefore, database might not contain finished errors. // Force an update to db as there should be no harm doing this diff --git a/packages/cli/src/workflow-helpers.ts b/packages/cli/src/workflow-helpers.ts index 3f47b13c821c6..620664c78475f 100644 --- a/packages/cli/src/workflow-helpers.ts +++ b/packages/cli/src/workflow-helpers.ts @@ -1,14 +1,9 @@ import { Container } from '@n8n/di'; import type { IDataObject, - INode, INodeCredentialsDetails, IRun, ITaskData, - NodeApiError, - WorkflowExecuteMode, - WorkflowOperationError, - NodeOperationError, IWorkflowBase, } from 'n8n-workflow'; import { v4 as uuid } from 'uuid'; @@ -16,53 +11,6 @@ import { v4 as uuid } from 'uuid'; import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; import { VariablesService } from '@/environments.ee/variables/variables.service.ee'; -export function generateFailedExecutionFromError( - mode: WorkflowExecuteMode, - error: NodeApiError | NodeOperationError | WorkflowOperationError, - node: INode, -): IRun { - return { - data: { - startData: { - destinationNode: node.name, - runNodeFilter: [node.name], - }, - resultData: { - error, - runData: { - [node.name]: [ - { - startTime: 0, - executionTime: 0, - error, - source: [], - }, - ], - }, - lastNodeExecuted: node.name, - }, - executionData: { - contextData: {}, - metadata: {}, - nodeExecutionStack: [ - { - node, - data: {}, - source: null, - }, - ], - waitingExecution: {}, - waitingExecutionSource: {}, - }, - }, - finished: false, - mode, - startedAt: new Date(), - stoppedAt: new Date(), - status: 'error', - }; -} - /** * Returns the data of the last executed node */ diff --git a/packages/cli/src/workflow-runner.ts b/packages/cli/src/workflow-runner.ts index 1c51d6e442a5c..bf6b51d9c293d 100644 --- a/packages/cli/src/workflow-runner.ts +++ b/packages/cli/src/workflow-runner.ts @@ -21,22 +21,21 @@ import { ActiveExecutions } from '@/active-executions'; import config from '@/config'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import { ExecutionNotFoundError } from '@/errors/execution-not-found-error'; +import { MaxStalledCountError } from '@/errors/max-stalled-count.error'; import { getLifecycleHooksForRegularMain, getLifecycleHooksForScalingWorker, getLifecycleHooksForScalingMain, } from '@/execution-lifecycle/execution-lifecycle-hooks'; +import { ExecutionDataService } from '@/executions/execution-data.service'; +import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks'; import { ManualExecutionService } from '@/manual-execution.service'; import { NodeTypes } from '@/node-types'; import type { ScalingService } from '@/scaling/scaling.service'; import type { Job, JobData } from '@/scaling/scaling.types'; -import { PermissionChecker } from '@/user-management/permission-checker'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; -import { generateFailedExecutionFromError } from '@/workflow-helpers'; import { WorkflowStaticDataService } from '@/workflows/workflow-static-data.service'; -import { MaxStalledCountError } from './errors/max-stalled-count.error'; - @Service() export class WorkflowRunner { private scalingService: ScalingService; @@ -50,9 +49,10 @@ export class WorkflowRunner { private readonly executionRepository: ExecutionRepository, private readonly workflowStaticDataService: WorkflowStaticDataService, private readonly nodeTypes: NodeTypes, - private readonly permissionChecker: PermissionChecker, + private readonly credentialsPermissionChecker: CredentialsPermissionChecker, private readonly instanceSettings: InstanceSettings, private readonly manualExecutionService: ManualExecutionService, + private readonly executionDataService: ExecutionDataService, ) {} /** The process did error */ @@ -134,10 +134,14 @@ export class WorkflowRunner { const { id: workflowId, nodes } = data.workflowData; try { - await this.permissionChecker.check(workflowId, nodes); + await this.credentialsPermissionChecker.check(workflowId, nodes); } catch (error) { // Create a failed execution with the data for the node, save it and abort execution - const runData = generateFailedExecutionFromError(data.executionMode, error, error.node); + const runData = this.executionDataService.generateFailedExecutionFromError( + data.executionMode, + error, + error.node, + ); const lifecycleHooks = getLifecycleHooksForRegularMain(data, executionId); await lifecycleHooks.runHook('workflowExecuteBefore', [undefined, data.executionData]); await lifecycleHooks.runHook('workflowExecuteAfter', [runData]); diff --git a/packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts b/packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts index dfac6d7366d16..64b5f12a06158 100644 --- a/packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts +++ b/packages/cli/src/workflows/__tests__/workflow-execution.service.test.ts @@ -65,6 +65,7 @@ describe('WorkflowExecutionService', () => { workflowRunner, mock(), mock(), + mock(), ); const additionalData = mock({}); diff --git a/packages/cli/src/workflows/workflow-execution.service.ts b/packages/cli/src/workflows/workflow-execution.service.ts index 8693c4c6435cd..62cc6fb361e0c 100644 --- a/packages/cli/src/workflows/workflow-execution.service.ts +++ b/packages/cli/src/workflows/workflow-execution.service.ts @@ -21,12 +21,12 @@ import type { Project } from '@/databases/entities/project'; import type { User } from '@/databases/entities/user'; import { ExecutionRepository } from '@/databases/repositories/execution.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { ExecutionDataService } from '@/executions/execution-data.service'; +import { SubworkflowPolicyChecker } from '@/executions/pre-execution-checks'; import type { CreateExecutionPayload, IWorkflowErrorData } from '@/interfaces'; import { NodeTypes } from '@/node-types'; -import { SubworkflowPolicyChecker } from '@/subworkflows/subworkflow-policy-checker.service'; import { TestWebhooks } from '@/webhooks/test-webhooks'; import * as WorkflowExecuteAdditionalData from '@/workflow-execute-additional-data'; -import * as WorkflowHelpers from '@/workflow-helpers'; import { WorkflowRunner } from '@/workflow-runner'; import type { WorkflowRequest } from '@/workflows/workflow.request'; @@ -42,6 +42,7 @@ export class WorkflowExecutionService { private readonly workflowRunner: WorkflowRunner, private readonly globalConfig: GlobalConfig, private readonly subworkflowPolicyChecker: SubworkflowPolicyChecker, + private readonly executionDataService: ExecutionDataService, ) {} async runWorkflow( @@ -273,7 +274,7 @@ export class WorkflowExecutionService { ); // Create a fake execution and save it to DB. - const fakeExecution = WorkflowHelpers.generateFailedExecutionFromError( + const fakeExecution = this.executionDataService.generateFailedExecutionFromError( 'error', errorWorkflowPermissionError, initialNode, diff --git a/packages/cli/test/integration/permission-checker.test.ts b/packages/cli/test/integration/executions/pre-execution-checks/credentials-permission-checker.test.ts similarity index 90% rename from packages/cli/test/integration/permission-checker.test.ts rename to packages/cli/test/integration/executions/pre-execution-checks/credentials-permission-checker.test.ts index 40a31d423643d..3d050011cb173 100644 --- a/packages/cli/test/integration/permission-checker.test.ts +++ b/packages/cli/test/integration/executions/pre-execution-checks/credentials-permission-checker.test.ts @@ -9,20 +9,19 @@ import { ProjectRepository } from '@/databases/repositories/project.repository'; import { SharedCredentialsRepository } from '@/databases/repositories/shared-credentials.repository'; import { SharedWorkflowRepository } from '@/databases/repositories/shared-workflow.repository'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { CredentialsPermissionChecker } from '@/executions/pre-execution-checks'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { NodeTypes } from '@/node-types'; import { OwnershipService } from '@/services/ownership.service'; -import { PermissionChecker } from '@/user-management/permission-checker'; +import { mockInstance } from '@test/mocking'; +import { affixRoleToSaveCredential } from '@test-integration/db/credentials'; +import { getPersonalProject } from '@test-integration/db/projects'; +import { createOwner, createUser } from '@test-integration/db/users'; +import { randomCredentialPayload as randomCred } from '@test-integration/random'; +import * as testDb from '@test-integration/test-db'; +import type { SaveCredentialFunction } from '@test-integration/types'; import { mockNodeTypesData } from '@test-integration/utils/node-types-data'; -import { affixRoleToSaveCredential } from './shared/db/credentials'; -import { getPersonalProject } from './shared/db/projects'; -import { createOwner, createUser } from './shared/db/users'; -import { randomCredentialPayload as randomCred } from './shared/random'; -import * as testDb from './shared/test-db'; -import type { SaveCredentialFunction } from './shared/types'; -import { mockInstance } from '../shared/mocking'; - const ownershipService = mockInstance(OwnershipService); const createWorkflow = async (nodes: INode[], workflowOwner?: User): Promise => { @@ -62,14 +61,14 @@ mockInstance(LoadNodesAndCredentials, { loadedNodes: mockNodeTypesData(['start', 'actionNetwork']), }); -let permissionChecker: PermissionChecker; +let permissionChecker: CredentialsPermissionChecker; beforeAll(async () => { await testDb.init(); saveCredential = affixRoleToSaveCredential('credential:owner'); - permissionChecker = Container.get(PermissionChecker); + permissionChecker = Container.get(CredentialsPermissionChecker); [owner, member] = await Promise.all([createOwner(), createUser()]); ownerPersonalProject = await Container.get(ProjectRepository).getPersonalProjectForUserOrFail( diff --git a/packages/workflow/src/errors/credential-access-error.ts b/packages/workflow/src/errors/credential-access-error.ts deleted file mode 100644 index 6de0c3ec96be8..0000000000000 --- a/packages/workflow/src/errors/credential-access-error.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { ExecutionBaseError } from './abstract/execution-base.error'; -import type { INode } from '../Interfaces'; - -export class CredentialAccessError extends ExecutionBaseError { - override readonly description = - 'Please recreate the credential or ask its owner to share it with you.'; - - override readonly level = 'warning'; - - constructor( - readonly node: INode, - credentialId: string, - workflowId: string, - ) { - super('Node has no access to credential', { - tags: { - nodeType: node.type, - }, - extra: { - credentialId, - workflowId, - }, - }); - } -} diff --git a/packages/workflow/src/errors/index.ts b/packages/workflow/src/errors/index.ts index fd501dc56659c..db3245608e90c 100644 --- a/packages/workflow/src/errors/index.ts +++ b/packages/workflow/src/errors/index.ts @@ -5,7 +5,6 @@ export { UnexpectedError, type UnexpectedErrorOptions } from './base/unexpected. export { UserError, type UserErrorOptions } from './base/user.error'; export { ApplicationError } from './application.error'; export { ExpressionError } from './expression.error'; -export { CredentialAccessError } from './credential-access-error'; export { ExecutionCancelledError } from './execution-cancelled.error'; export { NodeApiError } from './node-api.error'; export { NodeOperationError } from './node-operation.error';