From 7cdbb424e33bcc6dbc664ec5e1bff6eed4deb80f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 17 Jan 2024 10:16:13 +0100 Subject: [PATCH] refactor(core): Move methods from WorkflowHelpers into various workflow services (no-changelog) (#8348) --- packages/cli/src/Server.ts | 14 +- .../cli/src/WorkflowExecuteAdditionalData.ts | 13 +- packages/cli/src/WorkflowHelpers.ts | 361 +----------------- packages/cli/src/WorkflowRunner.ts | 19 +- .../src/executions/executions.service.ee.ts | 6 +- .../cli/src/executions/executions.service.ts | 7 +- .../src/services/userOnboarding.service.ts | 69 ++++ .../cli/src/workflows/workflow.service.ee.ts | 119 +++++- .../cli/src/workflows/workflow.service.ts | 132 +------ .../workflows/workflowExecution.service.ts | 286 ++++++++++++++ .../src/workflows/workflowSharing.service.ts | 36 ++ .../workflows/workflowStaticData.service.ts | 9 + .../cli/src/workflows/workflows.controller.ts | 42 +- .../integration/executions.controller.test.ts | 3 + .../cli/test/integration/shared/workflow.ts | 78 ++++ .../workflows/workflow.service.ee.test.ts | 180 +++++++++ .../{ => workflows}/workflow.service.test.ts | 19 +- .../workflows.controller.ee.test.ts | 50 +-- .../workflows.controller.test.ts | 29 +- .../cli/test/unit/WorkflowHelpers.test.ts | 218 +---------- .../workflowHistory.service.ee.test.ts | 6 +- 21 files changed, 895 insertions(+), 801 deletions(-) create mode 100644 packages/cli/src/services/userOnboarding.service.ts create mode 100644 packages/cli/src/workflows/workflowExecution.service.ts create mode 100644 packages/cli/src/workflows/workflowSharing.service.ts create mode 100644 packages/cli/test/integration/shared/workflow.ts create mode 100644 packages/cli/test/integration/workflows/workflow.service.ee.test.ts rename packages/cli/test/integration/{ => workflows}/workflow.service.test.ts (89%) rename packages/cli/test/integration/{ => workflows}/workflows.controller.ee.test.ts (96%) rename packages/cli/test/integration/{ => workflows}/workflows.controller.test.ts (96%) diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index cb11e29c5c04c..0ea596179f2b8 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -28,7 +28,6 @@ import history from 'connect-history-api-fallback'; import config from '@/config'; import { Queue } from '@/Queue'; -import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { WorkflowsController } from '@/workflows/workflows.controller'; import { @@ -103,6 +102,7 @@ import { RoleController } from './controllers/role.controller'; import { BadRequestError } from './errors/response-errors/bad-request.error'; import { NotFoundError } from './errors/response-errors/not-found.error'; import { MultiMainSetup } from './services/orchestration/main/MultiMainSetup.ee'; +import { WorkflowSharingService } from './workflows/workflowSharing.service'; const exec = promisify(callbackExec); @@ -436,7 +436,9 @@ export class Server extends AbstractServer { }, }; - const sharedWorkflowIds = await getSharedWorkflowIds(req.user); + const sharedWorkflowIds = await Container.get( + WorkflowSharingService, + ).getSharedWorkflowIds(req.user); if (!sharedWorkflowIds.length) return []; @@ -484,7 +486,9 @@ export class Server extends AbstractServer { const filter = req.query.filter ? jsonParse(req.query.filter) : {}; - const sharedWorkflowIds = await getSharedWorkflowIds(req.user); + const sharedWorkflowIds = await Container.get( + WorkflowSharingService, + ).getSharedWorkflowIds(req.user); for (const data of executingWorkflows) { if ( @@ -517,7 +521,9 @@ export class Server extends AbstractServer { ResponseHelper.send(async (req: ExecutionRequest.Stop): Promise => { const { id: executionId } = req.params; - const sharedWorkflowIds = await getSharedWorkflowIds(req.user); + const sharedWorkflowIds = await Container.get(WorkflowSharingService).getSharedWorkflowIds( + req.user, + ); if (!sharedWorkflowIds.length) { throw new NotFoundError('Execution not found'); diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index a170d01f85cc3..0e2d2f313d2dc 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -68,6 +68,7 @@ import { saveExecutionProgress } from './executionLifecycleHooks/saveExecutionPr import { WorkflowStaticDataService } from './workflows/workflowStaticData.service'; import { WorkflowRepository } from './databases/repositories/workflow.repository'; import { UrlService } from './services/url.service'; +import { WorkflowExecutionService } from './workflows/workflowExecution.service'; const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType'); @@ -194,7 +195,11 @@ export function executeErrorWorkflow( Container.get(OwnershipService) .getWorkflowOwnerCached(workflowId) .then((user) => { - void WorkflowHelpers.executeErrorWorkflow(errorWorkflow, workflowErrorData, user); + void Container.get(WorkflowExecutionService).executeErrorWorkflow( + errorWorkflow, + workflowErrorData, + user, + ); }) .catch((error: Error) => { ErrorReporter.error(error); @@ -218,7 +223,11 @@ export function executeErrorWorkflow( void Container.get(OwnershipService) .getWorkflowOwnerCached(workflowId) .then((user) => { - void WorkflowHelpers.executeErrorWorkflow(workflowId, workflowErrorData, user); + void Container.get(WorkflowExecutionService).executeErrorWorkflow( + workflowId, + workflowErrorData, + user, + ); }); } } diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index 9186ea5e8b821..9ea4e8335db10 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -1,52 +1,22 @@ -import type { FindOptionsWhere } from 'typeorm'; -import { In } from 'typeorm'; import { Container } from 'typedi'; - +import { v4 as uuid } from 'uuid'; import type { IDataObject, - IExecuteData, INode, INodeCredentialsDetails, IRun, - IRunExecutionData, ITaskData, NodeApiError, WorkflowExecuteMode, WorkflowOperationError, -} from 'n8n-workflow'; -import { - ErrorReporterProxy as ErrorReporter, - NodeOperationError, - SubworkflowOperationError, Workflow, + NodeOperationError, } from 'n8n-workflow'; -import { v4 as uuid } from 'uuid'; -import omit from 'lodash/omit'; -import type { - ExecutionPayload, - IWorkflowErrorData, - IWorkflowExecutionDataProcess, -} from '@/Interfaces'; -import { NodeTypes } from '@/NodeTypes'; -import { WorkflowRunner } from '@/WorkflowRunner'; -import config from '@/config'; + +import type { IWorkflowExecutionDataProcess } from '@/Interfaces'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; -import type { User } from '@db/entities/User'; -import { PermissionChecker } from './UserManagement/PermissionChecker'; -import { UserService } from './services/user.service'; -import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; -import type { SharedWorkflow } from '@db/entities/SharedWorkflow'; -import type { RoleNames } from '@db/entities/Role'; import { CredentialsRepository } from '@db/repositories/credentials.repository'; -import { ExecutionRepository } from '@db/repositories/execution.repository'; -import { RoleRepository } from '@db/repositories/role.repository'; -import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; -import { WorkflowRepository } from '@db/repositories/workflow.repository'; -import { RoleService } from './services/role.service'; -import { VariablesService } from './environments/variables/variables.service.ee'; -import { Logger } from './Logger'; - -const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType'); +import { VariablesService } from '@/environments/variables/variables.service.ee'; export function generateFailedExecutionFromError( mode: WorkflowExecuteMode, @@ -97,7 +67,6 @@ export function generateFailedExecutionFromError( /** * Returns the data of the last executed node - * */ export function getDataLastExecutedNodeData(inputData: IRun): ITaskData | undefined { const { runData, pinData = {} } = inputData.data.resultData; @@ -133,168 +102,6 @@ export function getDataLastExecutedNodeData(inputData: IRun): ITaskData | undefi return lastNodeRunData; } -/** - * Executes the error workflow - * - * @param {string} workflowId The id of the error workflow - * @param {IWorkflowErrorData} workflowErrorData The error data - */ -export async function executeErrorWorkflow( - workflowId: string, - workflowErrorData: IWorkflowErrorData, - runningUser: User, -): Promise { - const logger = Container.get(Logger); - // Wrap everything in try/catch to make sure that no errors bubble up and all get caught here - try { - const workflowData = await Container.get(WorkflowRepository).findOneBy({ id: workflowId }); - - if (workflowData === null) { - // The error workflow could not be found - logger.error( - `Calling Error Workflow for "${workflowErrorData.workflow.id}". Could not find error workflow "${workflowId}"`, - { workflowId }, - ); - return; - } - - const executionMode = 'error'; - const nodeTypes = Container.get(NodeTypes); - - const workflowInstance = new Workflow({ - id: workflowId, - name: workflowData.name, - nodeTypes, - nodes: workflowData.nodes, - connections: workflowData.connections, - active: workflowData.active, - staticData: workflowData.staticData, - settings: workflowData.settings, - }); - - try { - const failedNode = workflowErrorData.execution?.lastNodeExecuted - ? workflowInstance.getNode(workflowErrorData.execution?.lastNodeExecuted) - : undefined; - await Container.get(PermissionChecker).checkSubworkflowExecutePolicy( - workflowInstance, - workflowErrorData.workflow.id!, - failedNode ?? undefined, - ); - } catch (error) { - const initialNode = workflowInstance.getStartNode(); - if (initialNode) { - const errorWorkflowPermissionError = new SubworkflowOperationError( - `Another workflow: (ID ${workflowErrorData.workflow.id}) tried to invoke this workflow to handle errors.`, - "Unfortunately current permissions do not allow this. Please check that this workflow's settings allow it to be called by others", - ); - - // Create a fake execution and save it to DB. - const fakeExecution = generateFailedExecutionFromError( - 'error', - errorWorkflowPermissionError, - initialNode, - ); - - const fullExecutionData: ExecutionPayload = { - data: fakeExecution.data, - mode: fakeExecution.mode, - finished: false, - startedAt: new Date(), - stoppedAt: new Date(), - workflowData, - waitTill: null, - status: fakeExecution.status, - workflowId: workflowData.id, - }; - - await Container.get(ExecutionRepository).createNewExecution(fullExecutionData); - } - 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)) { - node = workflowInstance.nodes[nodeName]; - if (node.type === ERROR_TRIGGER_TYPE) { - workflowStartNode = node; - } - } - - if (workflowStartNode === undefined) { - logger.error( - `Calling Error Workflow for "${workflowErrorData.workflow.id}". Could not find "${ERROR_TRIGGER_TYPE}" in workflow "${workflowId}"`, - ); - return; - } - - // Can execute without webhook so go on - - // Initialize the data of the webhook node - const nodeExecutionStack: IExecuteData[] = []; - nodeExecutionStack.push({ - node: workflowStartNode, - data: { - main: [ - [ - { - json: workflowErrorData, - }, - ], - ], - }, - source: null, - }); - - const runExecutionData: IRunExecutionData = { - startData: {}, - resultData: { - runData: {}, - }, - executionData: { - contextData: {}, - metadata: {}, - nodeExecutionStack, - waitingExecution: {}, - waitingExecutionSource: {}, - }, - }; - - const runData: IWorkflowExecutionDataProcess = { - executionMode, - executionData: runExecutionData, - workflowData, - userId: runningUser.id, - }; - - const workflowRunner = new WorkflowRunner(); - await workflowRunner.run(runData); - } catch (error) { - ErrorReporter.error(error); - logger.error( - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - `Calling Error Workflow for "${workflowErrorData.workflow.id}": "${error.message}"`, - { workflowId: workflowErrorData.workflow.id }, - ); - } -} - -/** - * Returns the static data of workflow - */ -export async function getStaticDataById(workflowId: string) { - const workflowData = await Container.get(WorkflowRepository).findOne({ - select: ['staticData'], - where: { id: workflowId }, - }); - return workflowData?.staticData ?? {}; -} - /** * Set node ids if not already set */ @@ -416,164 +223,6 @@ export async function replaceInvalidCredentials(workflow: WorkflowEntity): Promi return workflow; } -/** - * Get the IDs of the workflows that have been shared with the user. - * Returns all IDs if user has the 'workflow:read' scope. - */ -export async function getSharedWorkflowIds(user: User, roleNames?: RoleNames[]): Promise { - const where: FindOptionsWhere = {}; - if (!user.hasGlobalScope('workflow:read')) { - where.userId = user.id; - } - if (roleNames?.length) { - const roleIds = await Container.get(RoleRepository).getIdsInScopeWorkflowByNames(roleNames); - - where.roleId = In(roleIds); - } - const sharedWorkflows = await Container.get(SharedWorkflowRepository).find({ - where, - select: ['workflowId'], - }); - return sharedWorkflows.map(({ workflowId }) => workflowId); -} -/** - * Check if user owns more than 15 workflows or more than 2 workflows with at least 2 nodes. - * If user does, set flag in its settings. - */ -export async function isBelowOnboardingThreshold(user: User): Promise { - let belowThreshold = true; - const skippedTypes = ['n8n-nodes-base.start', 'n8n-nodes-base.stickyNote']; - - const workflowOwnerRole = await Container.get(RoleService).findWorkflowOwnerRole(); - const ownedWorkflowsIds = await Container.get(SharedWorkflowRepository) - .find({ - where: { - userId: user.id, - roleId: workflowOwnerRole?.id, - }, - select: ['workflowId'], - }) - .then((ownedWorkflows) => ownedWorkflows.map(({ workflowId }) => workflowId)); - - if (ownedWorkflowsIds.length > 15) { - belowThreshold = false; - } else { - // just fetch workflows' nodes to keep memory footprint low - const workflows = await Container.get(WorkflowRepository).find({ - where: { id: In(ownedWorkflowsIds) }, - select: ['nodes'], - }); - - // valid workflow: 2+ nodes without start node - const validWorkflowCount = workflows.reduce((counter, workflow) => { - if (counter <= 2 && workflow.nodes.length > 2) { - const nodes = workflow.nodes.filter((node) => !skippedTypes.includes(node.type)); - if (nodes.length >= 2) { - return counter + 1; - } - } - return counter; - }, 0); - - // more than 2 valid workflows required - belowThreshold = validWorkflowCount <= 2; - } - - // user is above threshold --> set flag in settings - if (!belowThreshold) { - void Container.get(UserService).updateSettings(user.id, { isOnboarded: true }); - } - - return belowThreshold; -} - -/** Get all nodes in a workflow where the node credential is not accessible to the user. */ -export function getNodesWithInaccessibleCreds(workflow: WorkflowEntity, userCredIds: string[]) { - if (!workflow.nodes) { - return []; - } - return workflow.nodes.filter((node) => { - if (!node.credentials) return false; - - const allUsedCredentials = Object.values(node.credentials); - - const allUsedCredentialIds = allUsedCredentials.map((nodeCred) => nodeCred.id?.toString()); - return allUsedCredentialIds.some( - (nodeCredId) => nodeCredId && !userCredIds.includes(nodeCredId), - ); - }); -} - -export function validateWorkflowCredentialUsage( - newWorkflowVersion: WorkflowEntity, - previousWorkflowVersion: WorkflowEntity, - credentialsUserHasAccessTo: CredentialsEntity[], -) { - /** - * We only need to check nodes that use credentials the current user cannot access, - * since these can be 2 possibilities: - * - Same ID already exist: it's a read only node and therefore cannot be changed - * - It's a new node which indicates tampering and therefore must fail saving - */ - - const allowedCredentialIds = credentialsUserHasAccessTo.map((cred) => cred.id); - - const nodesWithCredentialsUserDoesNotHaveAccessTo = getNodesWithInaccessibleCreds( - newWorkflowVersion, - allowedCredentialIds, - ); - - // If there are no nodes with credentials the user does not have access to we can skip the rest - if (nodesWithCredentialsUserDoesNotHaveAccessTo.length === 0) { - return newWorkflowVersion; - } - - const previouslyExistingNodeIds = previousWorkflowVersion.nodes.map((node) => node.id); - - // If it's a new node we can't allow it to be saved - // since it uses creds the node doesn't have access - const isTamperingAttempt = (inaccessibleCredNodeId: string) => - !previouslyExistingNodeIds.includes(inaccessibleCredNodeId); - - const logger = Container.get(Logger); - nodesWithCredentialsUserDoesNotHaveAccessTo.forEach((node) => { - if (isTamperingAttempt(node.id)) { - logger.verbose('Blocked workflow update due to tampering attempt', { - nodeType: node.type, - nodeName: node.name, - nodeId: node.id, - nodeCredentials: node.credentials, - }); - // Node is new, so this is probably a tampering attempt. Throw an error - throw new NodeOperationError( - node, - `You don't have access to the credentials in the '${node.name}' node. Ask the owner to share them with you.`, - ); - } - // Replace the node with the previous version of the node - // Since it cannot be modified (read only node) - const nodeIdx = newWorkflowVersion.nodes.findIndex( - (newWorkflowNode) => newWorkflowNode.id === node.id, - ); - - logger.debug('Replacing node with previous version when saving updated workflow', { - nodeType: node.type, - nodeName: node.name, - nodeId: node.id, - }); - const previousNodeVersion = previousWorkflowVersion.nodes.find( - (previousNode) => previousNode.id === node.id, - ); - // Allow changing only name, position and disabled status for read-only nodes - Object.assign( - newWorkflowVersion.nodes[nodeIdx], - omit(previousNodeVersion, ['name', 'position', 'disabled']), - ); - }); - - return newWorkflowVersion; -} - export function getExecutionStartNode(data: IWorkflowExecutionDataProcess, workflow: Workflow) { let startNode; if ( diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index a6534af3376c0..339db979e21f7 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -1,10 +1,8 @@ /* eslint-disable @typescript-eslint/no-unsafe-argument */ - /* eslint-disable @typescript-eslint/no-unsafe-member-access */ - /* eslint-disable @typescript-eslint/no-shadow */ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ - +import { Container } from 'typedi'; import type { IProcessMessage } from 'n8n-core'; import { WorkflowExecute } from 'n8n-core'; @@ -29,6 +27,7 @@ import { fork } from 'child_process'; import { ActiveExecutions } from '@/ActiveExecutions'; import config from '@/config'; +import { ExecutionRepository } from '@db/repositories/execution.repository'; import { ExternalHooks } from '@/ExternalHooks'; import type { IExecutionResponse, @@ -38,7 +37,6 @@ import type { } from '@/Interfaces'; import { NodeTypes } from '@/NodeTypes'; import type { Job, JobData, JobResponse } from '@/Queue'; - import { Queue } from '@/Queue'; import { decodeWebhookResponse } from '@/helpers/decodeWebhookResponse'; import * as WorkflowHelpers from '@/WorkflowHelpers'; @@ -47,10 +45,9 @@ import { generateFailedExecutionFromError } from '@/WorkflowHelpers'; import { initErrorHandling } from '@/ErrorReporting'; import { PermissionChecker } from '@/UserManagement/PermissionChecker'; import { Push } from '@/push'; -import { Container } from 'typedi'; -import { InternalHooks } from './InternalHooks'; -import { ExecutionRepository } from '@db/repositories/execution.repository'; -import { Logger } from './Logger'; +import { InternalHooks } from '@/InternalHooks'; +import { Logger } from '@/Logger'; +import { WorkflowStaticDataService } from '@/workflows/workflowStaticData.service'; export class WorkflowRunner { logger: Logger; @@ -269,7 +266,8 @@ export class WorkflowRunner { ): Promise { const workflowId = data.workflowData.id; if (loadStaticData === true && workflowId) { - data.workflowData.staticData = await WorkflowHelpers.getStaticDataById(workflowId); + data.workflowData.staticData = + await Container.get(WorkflowStaticDataService).getStaticDataById(workflowId); } const nodeTypes = Container.get(NodeTypes); @@ -672,7 +670,8 @@ export class WorkflowRunner { const subprocess = fork(pathJoin(__dirname, 'WorkflowRunnerProcess.js')); if (loadStaticData === true && workflowId) { - data.workflowData.staticData = await WorkflowHelpers.getStaticDataById(workflowId); + data.workflowData.staticData = + await Container.get(WorkflowStaticDataService).getStaticDataById(workflowId); } data.restartExecutionId = restartExecutionId; diff --git a/packages/cli/src/executions/executions.service.ee.ts b/packages/cli/src/executions/executions.service.ee.ts index 17e8179bfc160..5182f147e8efb 100644 --- a/packages/cli/src/executions/executions.service.ee.ts +++ b/packages/cli/src/executions/executions.service.ee.ts @@ -1,12 +1,12 @@ +import Container from 'typedi'; import type { User } from '@db/entities/User'; -import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { ExecutionsService } from './executions.service'; import type { ExecutionRequest } from './execution.request'; import type { IExecutionResponse, IExecutionFlattedResponse } from '@/Interfaces'; import { EnterpriseWorkflowService } from '../workflows/workflow.service.ee'; import type { WorkflowWithSharingsAndCredentials } from '@/workflows/workflows.types'; -import Container from 'typedi'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { WorkflowSharingService } from '@/workflows/workflowSharing.service'; export class EnterpriseExecutionsService extends ExecutionsService { /** @@ -14,7 +14,7 @@ export class EnterpriseExecutionsService extends ExecutionsService { */ static async getWorkflowIdsForUser(user: User): Promise { // Get all workflows - return getSharedWorkflowIds(user); + return Container.get(WorkflowSharingService).getSharedWorkflowIds(user); } static async getExecution( diff --git a/packages/cli/src/executions/executions.service.ts b/packages/cli/src/executions/executions.service.ts index a0a836856a382..5ae2b26abd338 100644 --- a/packages/cli/src/executions/executions.service.ts +++ b/packages/cli/src/executions/executions.service.ts @@ -1,3 +1,4 @@ +import { Container, Service } from 'typedi'; import { validate as jsonSchemaValidate } from 'jsonschema'; import type { IWorkflowBase, @@ -8,6 +9,7 @@ import type { WorkflowExecuteMode, } from 'n8n-workflow'; import { ApplicationError, jsonParse, Workflow, WorkflowOperationError } from 'n8n-workflow'; + import { ActiveExecutions } from '@/ActiveExecutions'; import config from '@/config'; import type { User } from '@db/entities/User'; @@ -22,10 +24,8 @@ import type { import { NodeTypes } from '@/NodeTypes'; import { Queue } from '@/Queue'; import type { ExecutionRequest } from './execution.request'; -import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { WorkflowRunner } from '@/WorkflowRunner'; import * as GenericHelpers from '@/GenericHelpers'; -import { Container, Service } from 'typedi'; import { getStatusUsingPreviousExecutionStatusMethod } from './executionHelpers'; import type { IGetExecutionsQueryFilter } from '@db/repositories/execution.repository'; import { ExecutionRepository } from '@db/repositories/execution.repository'; @@ -33,6 +33,7 @@ import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { Logger } from '@/Logger'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { WorkflowSharingService } from '@/workflows/workflowSharing.service'; const schemaGetExecutionsQueryFilter = { $id: '/IGetExecutionsQueryFilter', @@ -77,7 +78,7 @@ export class ExecutionsService { */ static async getWorkflowIdsForUser(user: User): Promise { // Get all workflows using owner role - return getSharedWorkflowIds(user, ['owner']); + return Container.get(WorkflowSharingService).getSharedWorkflowIds(user, ['owner']); } static async getExecutionsList(req: ExecutionRequest.GetAll): Promise { diff --git a/packages/cli/src/services/userOnboarding.service.ts b/packages/cli/src/services/userOnboarding.service.ts new file mode 100644 index 0000000000000..f1d23cf1bc1d3 --- /dev/null +++ b/packages/cli/src/services/userOnboarding.service.ts @@ -0,0 +1,69 @@ +import { Service } from 'typedi'; +import { In } from 'typeorm'; + +import type { User } from '@db/entities/User'; +import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; +import { RoleService } from '@/services/role.service'; +import { UserService } from '@/services/user.service'; + +@Service() +export class UserOnboardingService { + constructor( + private readonly sharedWorkflowRepository: SharedWorkflowRepository, + private readonly workflowRepository: WorkflowRepository, + private readonly roleService: RoleService, + private readonly userService: UserService, + ) {} + + /** + * Check if user owns more than 15 workflows or more than 2 workflows with at least 2 nodes. + * If user does, set flag in its settings. + */ + async isBelowThreshold(user: User): Promise { + let belowThreshold = true; + const skippedTypes = ['n8n-nodes-base.start', 'n8n-nodes-base.stickyNote']; + + const workflowOwnerRole = await this.roleService.findWorkflowOwnerRole(); + const ownedWorkflowsIds = await this.sharedWorkflowRepository + .find({ + where: { + userId: user.id, + roleId: workflowOwnerRole?.id, + }, + select: ['workflowId'], + }) + .then((ownedWorkflows) => ownedWorkflows.map(({ workflowId }) => workflowId)); + + if (ownedWorkflowsIds.length > 15) { + belowThreshold = false; + } else { + // just fetch workflows' nodes to keep memory footprint low + const workflows = await this.workflowRepository.find({ + where: { id: In(ownedWorkflowsIds) }, + select: ['nodes'], + }); + + // valid workflow: 2+ nodes without start node + const validWorkflowCount = workflows.reduce((counter, workflow) => { + if (counter <= 2 && workflow.nodes.length > 2) { + const nodes = workflow.nodes.filter((node) => !skippedTypes.includes(node.type)); + if (nodes.length >= 2) { + return counter + 1; + } + } + return counter; + }, 0); + + // more than 2 valid workflows required + belowThreshold = validWorkflowCount <= 2; + } + + // user is above threshold --> set flag in settings + if (!belowThreshold) { + void this.userService.updateSettings(user.id, { isOnboarded: true }); + } + + return belowThreshold; + } +} diff --git a/packages/cli/src/workflows/workflow.service.ee.ts b/packages/cli/src/workflows/workflow.service.ee.ts index aba0422491ccf..161e580a837fc 100644 --- a/packages/cli/src/workflows/workflow.service.ee.ts +++ b/packages/cli/src/workflows/workflow.service.ee.ts @@ -1,30 +1,29 @@ -import * as WorkflowHelpers from '@/WorkflowHelpers'; +import { Service } from 'typedi'; +import omit from 'lodash/omit'; +import { ApplicationError, NodeOperationError } from 'n8n-workflow'; + +import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; import type { User } from '@db/entities/User'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; +import { CredentialsRepository } from '@db/repositories/credentials.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; +import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; +import { CredentialsService } from '@/credentials/credentials.service'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { NotFoundError } from '@/errors/response-errors/not-found.error'; +import { Logger } from '@/Logger'; import type { CredentialUsedByWorkflow, WorkflowWithSharingsAndCredentials, } from './workflows.types'; -import { CredentialsService } from '@/credentials/credentials.service'; -import { ApplicationError, NodeOperationError } from 'n8n-workflow'; -import { Service } from 'typedi'; -import type { CredentialsEntity } from '@db/entities/CredentialsEntity'; -import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { NotFoundError } from '@/errors/response-errors/not-found.error'; -import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; -import { CredentialsRepository } from '@/databases/repositories/credentials.repository'; -import { RoleService } from '@/services/role.service'; -import { UserRepository } from '@/databases/repositories/user.repository'; @Service() export class EnterpriseWorkflowService { constructor( + private readonly logger: Logger, private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly workflowRepository: WorkflowRepository, private readonly credentialsRepository: CredentialsRepository, - private readonly userRepository: UserRepository, - private readonly roleService: RoleService, ) {} async isOwned( @@ -143,11 +142,7 @@ export class EnterpriseWorkflowService { const allCredentials = await CredentialsService.getMany(user); try { - return WorkflowHelpers.validateWorkflowCredentialUsage( - workflow, - previousVersion, - allCredentials, - ); + return this.validateWorkflowCredentialUsage(workflow, previousVersion, allCredentials); } catch (error) { if (error instanceof NodeOperationError) { throw new BadRequestError(error.message); @@ -157,4 +152,90 @@ export class EnterpriseWorkflowService { ); } } + + validateWorkflowCredentialUsage( + newWorkflowVersion: WorkflowEntity, + previousWorkflowVersion: WorkflowEntity, + credentialsUserHasAccessTo: CredentialsEntity[], + ) { + /** + * We only need to check nodes that use credentials the current user cannot access, + * since these can be 2 possibilities: + * - Same ID already exist: it's a read only node and therefore cannot be changed + * - It's a new node which indicates tampering and therefore must fail saving + */ + + const allowedCredentialIds = credentialsUserHasAccessTo.map((cred) => cred.id); + + const nodesWithCredentialsUserDoesNotHaveAccessTo = this.getNodesWithInaccessibleCreds( + newWorkflowVersion, + allowedCredentialIds, + ); + + // If there are no nodes with credentials the user does not have access to we can skip the rest + if (nodesWithCredentialsUserDoesNotHaveAccessTo.length === 0) { + return newWorkflowVersion; + } + + const previouslyExistingNodeIds = previousWorkflowVersion.nodes.map((node) => node.id); + + // If it's a new node we can't allow it to be saved + // since it uses creds the node doesn't have access + const isTamperingAttempt = (inaccessibleCredNodeId: string) => + !previouslyExistingNodeIds.includes(inaccessibleCredNodeId); + + nodesWithCredentialsUserDoesNotHaveAccessTo.forEach((node) => { + if (isTamperingAttempt(node.id)) { + this.logger.verbose('Blocked workflow update due to tampering attempt', { + nodeType: node.type, + nodeName: node.name, + nodeId: node.id, + nodeCredentials: node.credentials, + }); + // Node is new, so this is probably a tampering attempt. Throw an error + throw new NodeOperationError( + node, + `You don't have access to the credentials in the '${node.name}' node. Ask the owner to share them with you.`, + ); + } + // Replace the node with the previous version of the node + // Since it cannot be modified (read only node) + const nodeIdx = newWorkflowVersion.nodes.findIndex( + (newWorkflowNode) => newWorkflowNode.id === node.id, + ); + + this.logger.debug('Replacing node with previous version when saving updated workflow', { + nodeType: node.type, + nodeName: node.name, + nodeId: node.id, + }); + const previousNodeVersion = previousWorkflowVersion.nodes.find( + (previousNode) => previousNode.id === node.id, + ); + // Allow changing only name, position and disabled status for read-only nodes + Object.assign( + newWorkflowVersion.nodes[nodeIdx], + omit(previousNodeVersion, ['name', 'position', 'disabled']), + ); + }); + + return newWorkflowVersion; + } + + /** Get all nodes in a workflow where the node credential is not accessible to the user. */ + getNodesWithInaccessibleCreds(workflow: WorkflowEntity, userCredIds: string[]) { + if (!workflow.nodes) { + return []; + } + return workflow.nodes.filter((node) => { + if (!node.credentials) return false; + + const allUsedCredentials = Object.values(node.credentials); + + const allUsedCredentialIds = allUsedCredentials.map((nodeCred) => nodeCred.id?.toString()); + return allUsedCredentialIds.some( + (nodeCredId) => nodeCredId && !userCredIds.includes(nodeCredId), + ); + }); + } } diff --git a/packages/cli/src/workflows/workflow.service.ts b/packages/cli/src/workflows/workflow.service.ts index c6546155b018d..186cb2d9c6aaf 100644 --- a/packages/cli/src/workflows/workflow.service.ts +++ b/packages/cli/src/workflows/workflow.service.ts @@ -1,34 +1,28 @@ import Container, { Service } from 'typedi'; -import type { INode, IPinData } from 'n8n-workflow'; -import { NodeApiError, Workflow } from 'n8n-workflow'; +import { NodeApiError } from 'n8n-workflow'; import pick from 'lodash/pick'; import omit from 'lodash/omit'; import { v4 as uuid } from 'uuid'; -import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; -import * as WorkflowHelpers from '@/WorkflowHelpers'; +import { BinaryDataService } from 'n8n-core'; + import config from '@/config'; import type { User } from '@db/entities/User'; import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; +import { ExecutionRepository } from '@db/repositories/execution.repository'; +import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; +import { WorkflowTagMappingRepository } from '@db/repositories/workflowTagMapping.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; +import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; +import * as WorkflowHelpers from '@/WorkflowHelpers'; import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import { hasSharing, type ListQuery } from '@/requests'; -import type { WorkflowRequest } from '@/workflows/workflow.request'; import { TagService } from '@/services/tag.service'; -import type { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; -import { NodeTypes } from '@/NodeTypes'; -import { WorkflowRunner } from '@/WorkflowRunner'; -import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; -import { TestWebhooks } from '@/TestWebhooks'; import { InternalHooks } from '@/InternalHooks'; -import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { OwnershipService } from '@/services/ownership.service'; import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee'; -import { BinaryDataService } from 'n8n-core'; import { Logger } from '@/Logger'; import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee'; -import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; -import { WorkflowTagMappingRepository } from '@db/repositories/workflowTagMapping.repository'; -import { ExecutionRepository } from '@db/repositories/execution.repository'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; @@ -45,54 +39,10 @@ export class WorkflowService { private readonly tagService: TagService, private readonly workflowHistoryService: WorkflowHistoryService, private readonly multiMainSetup: MultiMainSetup, - private readonly nodeTypes: NodeTypes, - private readonly testWebhooks: TestWebhooks, private readonly externalHooks: ExternalHooks, private readonly activeWorkflowRunner: ActiveWorkflowRunner, ) {} - /** - * Find the pinned trigger to execute the workflow from, if any. - * - * - In a full execution, select the _first_ pinned trigger. - * - In a partial execution, - * - select the _first_ pinned trigger that leads to the executed node, - * - else select the executed pinned trigger. - */ - findPinnedTrigger(workflow: IWorkflowDb, startNodes?: string[], pinData?: IPinData) { - if (!pinData || !startNodes) return null; - - const isTrigger = (nodeTypeName: string) => - ['trigger', 'webhook'].some((suffix) => nodeTypeName.toLowerCase().includes(suffix)); - - const pinnedTriggers = workflow.nodes.filter( - (node) => !node.disabled && pinData[node.name] && isTrigger(node.type), - ); - - if (pinnedTriggers.length === 0) return null; - - if (startNodes?.length === 0) return pinnedTriggers[0]; // full execution - - const [startNodeName] = startNodes; - - const parentNames = new Workflow({ - nodes: workflow.nodes, - connections: workflow.connections, - active: workflow.active, - nodeTypes: this.nodeTypes, - }).getParentNodes(startNodeName); - - let checkNodeName = ''; - - if (parentNames.length === 0) { - checkNodeName = startNodeName; - } else { - checkNodeName = parentNames.find((pn) => pn === pinnedTriggers[0].name) as string; - } - - return pinnedTriggers.find((pt) => pt.name === checkNodeName) ?? null; // partial execution - } - async getMany(sharedWorkflowIds: string[], options?: ListQuery.Options) { const { workflows, count } = await this.workflowRepository.getMany(sharedWorkflowIds, options); @@ -293,70 +243,6 @@ export class WorkflowService { return updatedWorkflow; } - async runManually( - { - workflowData, - runData, - pinData, - startNodes, - destinationNode, - }: WorkflowRequest.ManualRunPayload, - user: User, - sessionId?: string, - ) { - const pinnedTrigger = this.findPinnedTrigger(workflowData, startNodes, pinData); - - // If webhooks nodes exist and are active we have to wait for till we receive a call - if ( - pinnedTrigger === null && - (runData === undefined || - startNodes === undefined || - startNodes.length === 0 || - destinationNode === undefined) - ) { - const additionalData = await WorkflowExecuteAdditionalData.getBase(user.id); - - const needsWebhook = await this.testWebhooks.needsWebhook( - user.id, - workflowData, - additionalData, - runData, - sessionId, - destinationNode, - ); - - if (needsWebhook) return { waitingForWebhook: true }; - } - - // For manual testing always set to not active - workflowData.active = false; - - // Start the workflow - const data: IWorkflowExecutionDataProcess = { - destinationNode, - executionMode: 'manual', - runData, - pinData, - sessionId, - startNodes, - workflowData, - userId: user.id, - }; - - const hasRunData = (node: INode) => runData !== undefined && !!runData[node.name]; - - if (pinnedTrigger && !hasRunData(pinnedTrigger)) { - data.startNodes = [pinnedTrigger.name]; - } - - const workflowRunner = new WorkflowRunner(); - const executionId = await workflowRunner.run(data); - - return { - executionId, - }; - } - async delete(user: User, workflowId: string): Promise { await this.externalHooks.run('workflow.delete', [workflowId]); diff --git a/packages/cli/src/workflows/workflowExecution.service.ts b/packages/cli/src/workflows/workflowExecution.service.ts new file mode 100644 index 0000000000000..d18f02519d778 --- /dev/null +++ b/packages/cli/src/workflows/workflowExecution.service.ts @@ -0,0 +1,286 @@ +import { Service } from 'typedi'; +import type { IExecuteData, INode, IPinData, IRunExecutionData } from 'n8n-workflow'; +import { + SubworkflowOperationError, + Workflow, + ErrorReporterProxy as ErrorReporter, +} from 'n8n-workflow'; + +import config from '@/config'; +import type { User } from '@db/entities/User'; +import { ExecutionRepository } from '@db/repositories/execution.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; +import * as WorkflowHelpers from '@/WorkflowHelpers'; +import type { WorkflowRequest } from '@/workflows/workflow.request'; +import type { + ExecutionPayload, + IWorkflowDb, + IWorkflowErrorData, + IWorkflowExecutionDataProcess, +} from '@/Interfaces'; +import { NodeTypes } from '@/NodeTypes'; +import { WorkflowRunner } from '@/WorkflowRunner'; +import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; +import { TestWebhooks } from '@/TestWebhooks'; +import { Logger } from '@/Logger'; +import { PermissionChecker } from '@/UserManagement/PermissionChecker'; + +@Service() +export class WorkflowExecutionService { + constructor( + private readonly logger: Logger, + private readonly executionRepository: ExecutionRepository, + private readonly workflowRepository: WorkflowRepository, + private readonly nodeTypes: NodeTypes, + private readonly testWebhooks: TestWebhooks, + private readonly permissionChecker: PermissionChecker, + ) {} + + async executeManually( + { + workflowData, + runData, + pinData, + startNodes, + destinationNode, + }: WorkflowRequest.ManualRunPayload, + user: User, + sessionId?: string, + ) { + const pinnedTrigger = this.findPinnedTrigger(workflowData, startNodes, pinData); + + // If webhooks nodes exist and are active we have to wait for till we receive a call + if ( + pinnedTrigger === null && + (runData === undefined || + startNodes === undefined || + startNodes.length === 0 || + destinationNode === undefined) + ) { + const additionalData = await WorkflowExecuteAdditionalData.getBase(user.id); + + const needsWebhook = await this.testWebhooks.needsWebhook( + user.id, + workflowData, + additionalData, + runData, + sessionId, + destinationNode, + ); + + if (needsWebhook) return { waitingForWebhook: true }; + } + + // For manual testing always set to not active + workflowData.active = false; + + // Start the workflow + const data: IWorkflowExecutionDataProcess = { + destinationNode, + executionMode: 'manual', + runData, + pinData, + sessionId, + startNodes, + workflowData, + userId: user.id, + }; + + const hasRunData = (node: INode) => runData !== undefined && !!runData[node.name]; + + if (pinnedTrigger && !hasRunData(pinnedTrigger)) { + data.startNodes = [pinnedTrigger.name]; + } + + const workflowRunner = new WorkflowRunner(); + const executionId = await workflowRunner.run(data); + + return { + executionId, + }; + } + + /** Executes an error workflow */ + async executeErrorWorkflow( + workflowId: string, + workflowErrorData: IWorkflowErrorData, + runningUser: User, + ): Promise { + // Wrap everything in try/catch to make sure that no errors bubble up and all get caught here + try { + const workflowData = await this.workflowRepository.findOneBy({ id: workflowId }); + if (workflowData === null) { + // The error workflow could not be found + this.logger.error( + `Calling Error Workflow for "${workflowErrorData.workflow.id}". Could not find error workflow "${workflowId}"`, + { workflowId }, + ); + return; + } + + const executionMode = 'error'; + const workflowInstance = new Workflow({ + id: workflowId, + name: workflowData.name, + nodeTypes: this.nodeTypes, + nodes: workflowData.nodes, + connections: workflowData.connections, + active: workflowData.active, + staticData: workflowData.staticData, + settings: workflowData.settings, + }); + + try { + const failedNode = workflowErrorData.execution?.lastNodeExecuted + ? workflowInstance.getNode(workflowErrorData.execution?.lastNodeExecuted) + : undefined; + await this.permissionChecker.checkSubworkflowExecutePolicy( + workflowInstance, + workflowErrorData.workflow.id!, + failedNode ?? undefined, + ); + } catch (error) { + const initialNode = workflowInstance.getStartNode(); + if (initialNode) { + const errorWorkflowPermissionError = new SubworkflowOperationError( + `Another workflow: (ID ${workflowErrorData.workflow.id}) tried to invoke this workflow to handle errors.`, + "Unfortunately current permissions do not allow this. Please check that this workflow's settings allow it to be called by others", + ); + + // Create a fake execution and save it to DB. + const fakeExecution = WorkflowHelpers.generateFailedExecutionFromError( + 'error', + errorWorkflowPermissionError, + initialNode, + ); + + const fullExecutionData: ExecutionPayload = { + data: fakeExecution.data, + mode: fakeExecution.mode, + finished: false, + startedAt: new Date(), + stoppedAt: new Date(), + workflowData, + waitTill: null, + status: fakeExecution.status, + workflowId: workflowData.id, + }; + + await this.executionRepository.createNewExecution(fullExecutionData); + } + this.logger.info('Error workflow execution blocked due to subworkflow settings', { + erroredWorkflowId: workflowErrorData.workflow.id, + errorWorkflowId: workflowId, + }); + return; + } + + let node: INode; + let workflowStartNode: INode | undefined; + const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType'); + for (const nodeName of Object.keys(workflowInstance.nodes)) { + node = workflowInstance.nodes[nodeName]; + if (node.type === ERROR_TRIGGER_TYPE) { + workflowStartNode = node; + } + } + + if (workflowStartNode === undefined) { + this.logger.error( + `Calling Error Workflow for "${workflowErrorData.workflow.id}". Could not find "${ERROR_TRIGGER_TYPE}" in workflow "${workflowId}"`, + ); + return; + } + + // Can execute without webhook so go on + // Initialize the data of the webhook node + const nodeExecutionStack: IExecuteData[] = []; + nodeExecutionStack.push({ + node: workflowStartNode, + data: { + main: [ + [ + { + json: workflowErrorData, + }, + ], + ], + }, + source: null, + }); + + const runExecutionData: IRunExecutionData = { + startData: {}, + resultData: { + runData: {}, + }, + executionData: { + contextData: {}, + metadata: {}, + nodeExecutionStack, + waitingExecution: {}, + waitingExecutionSource: {}, + }, + }; + + const runData: IWorkflowExecutionDataProcess = { + executionMode, + executionData: runExecutionData, + workflowData, + userId: runningUser.id, + }; + + const workflowRunner = new WorkflowRunner(); + await workflowRunner.run(runData); + } catch (error) { + ErrorReporter.error(error); + this.logger.error( + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + `Calling Error Workflow for "${workflowErrorData.workflow.id}": "${error.message}"`, + { workflowId: workflowErrorData.workflow.id }, + ); + } + } + + /** + * Find the pinned trigger to execute the workflow from, if any. + * + * - In a full execution, select the _first_ pinned trigger. + * - In a partial execution, + * - select the _first_ pinned trigger that leads to the executed node, + * - else select the executed pinned trigger. + */ + private findPinnedTrigger(workflow: IWorkflowDb, startNodes?: string[], pinData?: IPinData) { + if (!pinData || !startNodes) return null; + + const isTrigger = (nodeTypeName: string) => + ['trigger', 'webhook'].some((suffix) => nodeTypeName.toLowerCase().includes(suffix)); + + const pinnedTriggers = workflow.nodes.filter( + (node) => !node.disabled && pinData[node.name] && isTrigger(node.type), + ); + + if (pinnedTriggers.length === 0) return null; + + if (startNodes?.length === 0) return pinnedTriggers[0]; // full execution + + const [startNodeName] = startNodes; + + const parentNames = new Workflow({ + nodes: workflow.nodes, + connections: workflow.connections, + active: workflow.active, + nodeTypes: this.nodeTypes, + }).getParentNodes(startNodeName); + + let checkNodeName = ''; + + if (parentNames.length === 0) { + checkNodeName = startNodeName; + } else { + checkNodeName = parentNames.find((pn) => pn === pinnedTriggers[0].name) as string; + } + + return pinnedTriggers.find((pt) => pt.name === checkNodeName) ?? null; // partial execution + } +} diff --git a/packages/cli/src/workflows/workflowSharing.service.ts b/packages/cli/src/workflows/workflowSharing.service.ts new file mode 100644 index 0000000000000..a322b2c327fe0 --- /dev/null +++ b/packages/cli/src/workflows/workflowSharing.service.ts @@ -0,0 +1,36 @@ +import { Service } from 'typedi'; +import { In, type FindOptionsWhere } from 'typeorm'; + +import type { RoleNames } from '@db/entities/Role'; +import type { SharedWorkflow } from '@db/entities/SharedWorkflow'; +import type { User } from '@db/entities/User'; +import { RoleRepository } from '@db/repositories/role.repository'; +import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; + +@Service() +export class WorkflowSharingService { + constructor( + private readonly roleRepository: RoleRepository, + private readonly sharedWorkflowRepository: SharedWorkflowRepository, + ) {} + + /** + * Get the IDs of the workflows that have been shared with the user. + * Returns all IDs if user has the 'workflow:read' scope. + */ + async getSharedWorkflowIds(user: User, roleNames?: RoleNames[]): Promise { + const where: FindOptionsWhere = {}; + if (!user.hasGlobalScope('workflow:read')) { + where.userId = user.id; + } + if (roleNames?.length) { + const roleIds = await this.roleRepository.getIdsInScopeWorkflowByNames(roleNames); + where.roleId = In(roleIds); + } + const sharedWorkflows = await this.sharedWorkflowRepository.find({ + where, + select: ['workflowId'], + }); + return sharedWorkflows.map(({ workflowId }) => workflowId); + } +} diff --git a/packages/cli/src/workflows/workflowStaticData.service.ts b/packages/cli/src/workflows/workflowStaticData.service.ts index b569c69a3002d..b77d1b716c3d8 100644 --- a/packages/cli/src/workflows/workflowStaticData.service.ts +++ b/packages/cli/src/workflows/workflowStaticData.service.ts @@ -11,6 +11,15 @@ export class WorkflowStaticDataService { private readonly workflowRepository: WorkflowRepository, ) {} + /** Returns the static data of workflow */ + async getStaticDataById(workflowId: string) { + const workflowData = await this.workflowRepository.findOne({ + select: ['staticData'], + where: { id: workflowId }, + }); + return workflowData?.staticData ?? {}; + } + /** Saves the static data if it changed */ async saveStaticData(workflow: Workflow): Promise { if (workflow.staticData.__dataChanged === true) { diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index c38fa32fb6769..b3fa0307ce901 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -1,22 +1,27 @@ +import { Service } from 'typedi'; import express from 'express'; import { v4 as uuid } from 'uuid'; - import axios from 'axios'; + import * as Db from '@/Db'; import * as GenericHelpers from '@/GenericHelpers'; import * as ResponseHelper from '@/ResponseHelper'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import type { IWorkflowResponse } from '@/Interfaces'; import config from '@/config'; +import { Authorized, Delete, Get, Patch, Post, Put, RestController } from '@/decorators'; +import type { RoleNames } from '@db/entities/Role'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; import { WorkflowEntity } from '@db/entities/WorkflowEntity'; +import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; +import { TagRepository } from '@db/repositories/tag.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; +import { UserRepository } from '@db/repositories/user.repository'; import { validateEntity } from '@/GenericHelpers'; import { ExternalHooks } from '@/ExternalHooks'; import { ListQuery } from '@/requests'; -import { isBelowOnboardingThreshold } from '@/WorkflowHelpers'; import { WorkflowService } from './workflow.service'; import { isSharingEnabled } from '@/UserManagement/UserManagementHelper'; -import Container, { Service } from 'typedi'; import { InternalHooks } from '@/InternalHooks'; import { RoleService } from '@/services/role.service'; import * as utils from '@/utils'; @@ -24,20 +29,17 @@ import { listQueryMiddleware } from '@/middlewares'; import { TagService } from '@/services/tag.service'; import { WorkflowHistoryService } from './workflowHistory/workflowHistory.service.ee'; import { Logger } from '@/Logger'; -import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { InternalServerError } from '@/errors/response-errors/internal-server.error'; -import { NamingService } from '@/services/naming.service'; -import { TagRepository } from '@/databases/repositories/tag.repository'; -import { EnterpriseWorkflowService } from './workflow.service.ee'; -import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; -import type { RoleNames } from '@/databases/entities/Role'; import { UnauthorizedError } from '@/errors/response-errors/unauthorized.error'; +import { NamingService } from '@/services/naming.service'; +import { UserOnboardingService } from '@/services/userOnboarding.service'; import { CredentialsService } from '../credentials/credentials.service'; -import { UserRepository } from '@/databases/repositories/user.repository'; -import { Authorized, Delete, Get, Patch, Post, Put, RestController } from '@/decorators'; import { WorkflowRequest } from './workflow.request'; +import { EnterpriseWorkflowService } from './workflow.service.ee'; +import { WorkflowExecutionService } from './workflowExecution.service'; +import { WorkflowSharingService } from './workflowSharing.service'; @Service() @Authorized() @@ -53,8 +55,11 @@ export class WorkflowsController { private readonly workflowHistoryService: WorkflowHistoryService, private readonly tagService: TagService, private readonly namingService: NamingService, + private readonly userOnboardingService: UserOnboardingService, private readonly workflowRepository: WorkflowRepository, private readonly workflowService: WorkflowService, + private readonly workflowExecutionService: WorkflowExecutionService, + private readonly workflowSharingService: WorkflowSharingService, private readonly sharedWorkflowRepository: SharedWorkflowRepository, private readonly userRepository: UserRepository, ) {} @@ -142,9 +147,12 @@ export class WorkflowsController { async getAll(req: ListQuery.Request, res: express.Response) { try { const roles: RoleNames[] = isSharingEnabled() ? [] : ['owner']; - const sharedWorkflowIds = await WorkflowHelpers.getSharedWorkflowIds(req.user, roles); + const sharedWorkflowIds = await this.workflowSharingService.getSharedWorkflowIds( + req.user, + roles, + ); - const { workflows: data, count } = await Container.get(WorkflowService).getMany( + const { workflows: data, count } = await this.workflowService.getMany( sharedWorkflowIds, req.listQueryOptions, ); @@ -166,7 +174,7 @@ export class WorkflowsController { const onboardingFlowEnabled = !config.getEnv('workflows.onboardingFlowDisabled') && !req.user.settings?.isOnboarded && - (await isBelowOnboardingThreshold(req.user)); + (await this.userOnboardingService.isBelowThreshold(req.user)); return { name, onboardingFlowEnabled }; } @@ -321,7 +329,11 @@ export class WorkflowsController { } } - return this.workflowService.runManually(req.body, req.user, GenericHelpers.getSessionId(req)); + return this.workflowExecutionService.executeManually( + req.body, + req.user, + GenericHelpers.getSessionId(req), + ); } @Put('/:workflowId/share') diff --git a/packages/cli/test/integration/executions.controller.test.ts b/packages/cli/test/integration/executions.controller.test.ts index 16f7e8d780403..5d4bfe5307448 100644 --- a/packages/cli/test/integration/executions.controller.test.ts +++ b/packages/cli/test/integration/executions.controller.test.ts @@ -1,10 +1,13 @@ import type { User } from '@db/entities/User'; +import { Push } from '@/push'; import { createSuccessfulExecution, getAllExecutions } from './shared/db/executions'; import { createOwner } from './shared/db/users'; import { createWorkflow } from './shared/db/workflows'; import * as testDb from './shared/testDb'; import { setupTestServer } from './shared/utils'; +import { mockInstance } from '../shared/mocking'; +mockInstance(Push); let testServer = setupTestServer({ endpointGroups: ['executions'] }); let owner: User; diff --git a/packages/cli/test/integration/shared/workflow.ts b/packages/cli/test/integration/shared/workflow.ts new file mode 100644 index 0000000000000..0b2b12f53ac0a --- /dev/null +++ b/packages/cli/test/integration/shared/workflow.ts @@ -0,0 +1,78 @@ +import type { INode } from 'n8n-workflow'; +import { WorkflowEntity } from '@db/entities/WorkflowEntity'; + +export const FIRST_CREDENTIAL_ID = '1'; +export const SECOND_CREDENTIAL_ID = '2'; +export const THIRD_CREDENTIAL_ID = '3'; + +const NODE_WITH_NO_CRED = '0133467b-df4a-473d-9295-fdd9d01fa45a'; +const NODE_WITH_ONE_CRED = '4673f869-f2dc-4a33-b053-ca3193bc5226'; +const NODE_WITH_TWO_CRED = '9b4208bd-8f10-4a6a-ad3b-da47a326f7da'; + +const nodeWithNoCredentials: INode = { + id: NODE_WITH_NO_CRED, + name: 'Node with no Credential', + typeVersion: 1, + type: 'n8n-nodes-base.fakeNode', + position: [0, 0], + credentials: {}, + parameters: {}, +}; + +const nodeWithOneCredential: INode = { + id: NODE_WITH_ONE_CRED, + name: 'Node with a single credential', + typeVersion: 1, + type: '', + position: [0, 0], + credentials: { + test: { + id: FIRST_CREDENTIAL_ID, + name: 'First fake credential', + }, + }, + parameters: {}, +}; + +const nodeWithTwoCredentials: INode = { + id: NODE_WITH_TWO_CRED, + name: 'Node with two credentials', + typeVersion: 1, + type: '', + position: [0, 0], + credentials: { + mcTest: { + id: SECOND_CREDENTIAL_ID, + name: 'Second fake credential', + }, + mcTest2: { + id: THIRD_CREDENTIAL_ID, + name: 'Third fake credential', + }, + }, + parameters: {}, +}; + +export function getWorkflow(options?: { + addNodeWithoutCreds?: boolean; + addNodeWithOneCred?: boolean; + addNodeWithTwoCreds?: boolean; +}) { + const workflow = new WorkflowEntity(); + + workflow.nodes = []; + + if (options?.addNodeWithoutCreds) { + workflow.nodes.push(nodeWithNoCredentials); + } + + if (options?.addNodeWithOneCred) { + workflow.nodes.push(nodeWithOneCredential); + } + + if (options?.addNodeWithTwoCreds) { + workflow.nodes.push(nodeWithTwoCredentials); + } + + return workflow; +} diff --git a/packages/cli/test/integration/workflows/workflow.service.ee.test.ts b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts new file mode 100644 index 0000000000000..4cb823b7c0663 --- /dev/null +++ b/packages/cli/test/integration/workflows/workflow.service.ee.test.ts @@ -0,0 +1,180 @@ +import Container from 'typedi'; +import { mock } from 'jest-mock-extended'; +import { CredentialsEntity } from '@db/entities/CredentialsEntity'; +import { CredentialsRepository } from '@db/repositories/credentials.repository'; +import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; +import { Telemetry } from '@/telemetry'; +import { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee'; + +import * as testDb from '../shared/testDb'; +import { mockInstance } from '../../shared/mocking'; +import { + FIRST_CREDENTIAL_ID, + SECOND_CREDENTIAL_ID, + THIRD_CREDENTIAL_ID, + getWorkflow, +} from '../shared/workflow'; + +describe('EnterpriseWorkflowService', () => { + let service: EnterpriseWorkflowService; + + beforeAll(async () => { + await testDb.init(); + mockInstance(Telemetry); + + service = new EnterpriseWorkflowService( + mock(), + Container.get(SharedWorkflowRepository), + Container.get(WorkflowRepository), + Container.get(CredentialsRepository), + ); + }); + + afterEach(async () => { + await testDb.truncate(['Workflow']); + jest.restoreAllMocks(); + }); + + afterAll(async () => { + await testDb.terminate(); + }); + + describe('validateWorkflowCredentialUsage', () => { + function generateCredentialEntity(credentialId: string) { + const credentialEntity = new CredentialsEntity(); + credentialEntity.id = credentialId; + return credentialEntity; + } + + it('Should throw error saving a workflow using credential without access', () => { + const newWorkflowVersion = getWorkflow({ addNodeWithOneCred: true }); + const previousWorkflowVersion = getWorkflow(); + expect(() => { + service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []); + }).toThrow(); + }); + + it('Should not throw error when saving a workflow using credential with access', () => { + const newWorkflowVersion = getWorkflow({ addNodeWithOneCred: true }); + const previousWorkflowVersion = getWorkflow(); + expect(() => { + service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, [ + generateCredentialEntity('1'), + ]); + }).not.toThrow(); + }); + + it('Should not throw error when saving a workflow removing node without credential access', () => { + const newWorkflowVersion = getWorkflow(); + const previousWorkflowVersion = getWorkflow({ addNodeWithOneCred: true }); + expect(() => { + service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, [ + generateCredentialEntity('1'), + ]); + }).not.toThrow(); + }); + + it('Should save fine when not making changes to workflow without access', () => { + const workflowWithOneCredential = getWorkflow({ addNodeWithOneCred: true }); + expect(() => { + service.validateWorkflowCredentialUsage( + workflowWithOneCredential, + workflowWithOneCredential, + [], + ); + }).not.toThrow(); + }); + + it('Should throw error saving a workflow adding node without credential access', () => { + const newWorkflowVersion = getWorkflow({ + addNodeWithOneCred: true, + addNodeWithTwoCreds: true, + }); + const previousWorkflowVersion = getWorkflow({ addNodeWithOneCred: true }); + expect(() => { + service.validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []); + }).toThrow(); + }); + }); + + describe('getNodesWithInaccessibleCreds', () => { + test('Should return an empty list for a workflow without nodes', () => { + const workflow = getWorkflow(); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, []); + expect(nodesWithInaccessibleCreds).toHaveLength(0); + }); + + test('Should return an empty list for a workflow with nodes without credentials', () => { + const workflow = getWorkflow({ addNodeWithoutCreds: true }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, []); + expect(nodesWithInaccessibleCreds).toHaveLength(0); + }); + + test('Should return an element for a node with a credential without access', () => { + const workflow = getWorkflow({ addNodeWithOneCred: true }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, []); + expect(nodesWithInaccessibleCreds).toHaveLength(1); + }); + + test('Should return an empty list for a node with a credential with access', () => { + const workflow = getWorkflow({ addNodeWithOneCred: true }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, [ + FIRST_CREDENTIAL_ID, + ]); + expect(nodesWithInaccessibleCreds).toHaveLength(0); + }); + + test('Should return an element for a node with two credentials and mixed access', () => { + const workflow = getWorkflow({ addNodeWithTwoCreds: true }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, [ + SECOND_CREDENTIAL_ID, + ]); + expect(nodesWithInaccessibleCreds).toHaveLength(1); + }); + + test('Should return one node for a workflow with two nodes and two credentials', () => { + const workflow = getWorkflow({ addNodeWithOneCred: true, addNodeWithTwoCreds: true }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, [ + SECOND_CREDENTIAL_ID, + THIRD_CREDENTIAL_ID, + ]); + expect(nodesWithInaccessibleCreds).toHaveLength(1); + }); + + test('Should return one element for a workflows with two nodes and one credential', () => { + const workflow = getWorkflow({ + addNodeWithoutCreds: true, + addNodeWithOneCred: true, + addNodeWithTwoCreds: true, + }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, [ + FIRST_CREDENTIAL_ID, + ]); + expect(nodesWithInaccessibleCreds).toHaveLength(1); + }); + + test('Should return one element for a workflows with two nodes and partial credential access', () => { + const workflow = getWorkflow({ addNodeWithOneCred: true, addNodeWithTwoCreds: true }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, [ + FIRST_CREDENTIAL_ID, + SECOND_CREDENTIAL_ID, + ]); + expect(nodesWithInaccessibleCreds).toHaveLength(1); + }); + + test('Should return two elements for a workflows with two nodes and partial credential access', () => { + const workflow = getWorkflow({ addNodeWithOneCred: true, addNodeWithTwoCreds: true }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, [ + SECOND_CREDENTIAL_ID, + ]); + expect(nodesWithInaccessibleCreds).toHaveLength(2); + }); + + test('Should return two elements for a workflows with two nodes and no credential access', () => { + const workflow = getWorkflow({ addNodeWithOneCred: true, addNodeWithTwoCreds: true }); + const nodesWithInaccessibleCreds = service.getNodesWithInaccessibleCreds(workflow, []); + expect(nodesWithInaccessibleCreds).toHaveLength(2); + }); + }); +}); diff --git a/packages/cli/test/integration/workflow.service.test.ts b/packages/cli/test/integration/workflows/workflow.service.test.ts similarity index 89% rename from packages/cli/test/integration/workflow.service.test.ts rename to packages/cli/test/integration/workflows/workflow.service.test.ts index b98fb8ff742e8..34f1a970edbcf 100644 --- a/packages/cli/test/integration/workflow.service.test.ts +++ b/packages/cli/test/integration/workflows/workflow.service.test.ts @@ -1,15 +1,16 @@ import Container from 'typedi'; -import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; -import * as testDb from './shared/testDb'; -import { WorkflowService } from '@/workflows/workflow.service'; -import { mockInstance } from '../shared/mocking'; -import { createOwner } from './shared/db/users'; -import { createWorkflow } from './shared/db/workflows'; -import { SharedWorkflowRepository } from '@/databases/repositories/sharedWorkflow.repository'; import { mock } from 'jest-mock-extended'; -import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; +import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; import { Telemetry } from '@/telemetry'; import { MultiMainSetup } from '@/services/orchestration/main/MultiMainSetup.ee'; +import { WorkflowService } from '@/workflows/workflow.service'; + +import * as testDb from '../shared/testDb'; +import { mockInstance } from '../../shared/mocking'; +import { createOwner } from '../shared/db/users'; +import { createWorkflow } from '../shared/db/workflows'; let workflowService: WorkflowService; let activeWorkflowRunner: ActiveWorkflowRunner; @@ -34,8 +35,6 @@ beforeAll(async () => { mock(), multiMainSetup, mock(), - mock(), - mock(), activeWorkflowRunner, ); }); diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts similarity index 96% rename from packages/cli/test/integration/workflows.controller.ee.test.ts rename to packages/cli/test/integration/workflows/workflows.controller.ee.test.ts index 8ca0955442fbf..e49267c0f642f 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.ee.test.ts @@ -3,24 +3,28 @@ import type { SuperAgentTest } from 'supertest'; import { v4 as uuid } from 'uuid'; import type { INode } from 'n8n-workflow'; -import * as UserManagementHelpers from '@/UserManagement/UserManagementHelper'; +import type { Role } from '@db/entities/Role'; import type { User } from '@db/entities/User'; -import { getSharedWorkflowIds } from '@/WorkflowHelpers'; import { WorkflowHistoryRepository } from '@db/repositories/workflowHistory.repository'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; - -import { mockInstance } from '../shared/mocking'; -import * as utils from './shared/utils/'; -import * as testDb from './shared/testDb'; -import type { SaveCredentialFunction } from './shared/types'; -import { makeWorkflow } from './shared/utils/'; -import { randomCredentialPayload } from './shared/random'; -import { affixRoleToSaveCredential, shareCredentialWithUsers } from './shared/db/credentials'; -import { getCredentialOwnerRole, getGlobalMemberRole, getGlobalOwnerRole } from './shared/db/roles'; -import { createUser } from './shared/db/users'; -import { createWorkflow, getWorkflowSharing, shareWorkflowWithUsers } from './shared/db/workflows'; -import type { Role } from '@/databases/entities/Role'; +import * as UserManagementHelpers from '@/UserManagement/UserManagementHelper'; import { Push } from '@/push'; +import { WorkflowSharingService } from '@/workflows/workflowSharing.service'; + +import { mockInstance } from '../../shared/mocking'; +import * as utils from '../shared/utils/'; +import * as testDb from '../shared/testDb'; +import type { SaveCredentialFunction } from '../shared/types'; +import { makeWorkflow } from '../shared/utils/'; +import { randomCredentialPayload } from '../shared/random'; +import { affixRoleToSaveCredential, shareCredentialWithUsers } from '../shared/db/credentials'; +import { + getCredentialOwnerRole, + getGlobalMemberRole, + getGlobalOwnerRole, +} from '../shared/db/roles'; +import { createUser } from '../shared/db/users'; +import { createWorkflow, getWorkflowSharing, shareWorkflowWithUsers } from '../shared/db/workflows'; let globalMemberRole: Role; let owner: User; @@ -31,7 +35,7 @@ let authMemberAgent: SuperAgentTest; let authAnotherMemberAgent: SuperAgentTest; let saveCredential: SaveCredentialFunction; -const activeWorkflowRunnerLike = mockInstance(ActiveWorkflowRunner); +const activeWorkflowRunner = mockInstance(ActiveWorkflowRunner); mockInstance(Push); const sharingSpy = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); @@ -60,8 +64,8 @@ beforeAll(async () => { }); beforeEach(async () => { - activeWorkflowRunnerLike.add.mockReset(); - activeWorkflowRunnerLike.remove.mockReset(); + activeWorkflowRunner.add.mockReset(); + activeWorkflowRunner.remove.mockReset(); await testDb.truncate(['Workflow', 'SharedWorkflow', 'WorkflowHistory']); }); @@ -988,7 +992,8 @@ describe('getSharedWorkflowIds', () => { owner.globalRole = await getGlobalOwnerRole(); const workflow1 = await createWorkflow({}, member); const workflow2 = await createWorkflow({}, anotherMember); - const sharedWorkflowIds = await getSharedWorkflowIds(owner); + const sharedWorkflowIds = + await Container.get(WorkflowSharingService).getSharedWorkflowIds(owner); expect(sharedWorkflowIds).toHaveLength(2); expect(sharedWorkflowIds).toContain(workflow1.id); expect(sharedWorkflowIds).toContain(workflow2.id); @@ -1001,7 +1006,8 @@ describe('getSharedWorkflowIds', () => { const workflow3 = await createWorkflow({}, anotherMember); await shareWorkflowWithUsers(workflow1, [member]); await shareWorkflowWithUsers(workflow3, [member]); - const sharedWorkflowIds = await getSharedWorkflowIds(member); + const sharedWorkflowIds = + await Container.get(WorkflowSharingService).getSharedWorkflowIds(member); expect(sharedWorkflowIds).toHaveLength(2); expect(sharedWorkflowIds).toContain(workflow1.id); expect(sharedWorkflowIds).toContain(workflow3.id); @@ -1130,7 +1136,7 @@ describe('PATCH /workflows/:id - activate workflow', () => { const response = await authOwnerAgent.patch(`/workflows/${workflow.id}`).send(payload); expect(response.statusCode).toBe(200); - expect(activeWorkflowRunnerLike.add).toBeCalled(); + expect(activeWorkflowRunner.add).toBeCalled(); const { data: { id, versionId, active }, @@ -1152,8 +1158,8 @@ describe('PATCH /workflows/:id - activate workflow', () => { const response = await authOwnerAgent.patch(`/workflows/${workflow.id}`).send(payload); expect(response.statusCode).toBe(200); - expect(activeWorkflowRunnerLike.add).not.toBeCalled(); - expect(activeWorkflowRunnerLike.remove).toBeCalled(); + expect(activeWorkflowRunner.add).not.toBeCalled(); + expect(activeWorkflowRunner.remove).toBeCalled(); const { data: { id, versionId, active }, diff --git a/packages/cli/test/integration/workflows.controller.test.ts b/packages/cli/test/integration/workflows/workflows.controller.test.ts similarity index 96% rename from packages/cli/test/integration/workflows.controller.test.ts rename to packages/cli/test/integration/workflows/workflows.controller.test.ts index a85dfee776ba5..47c8788889bca 100644 --- a/packages/cli/test/integration/workflows.controller.test.ts +++ b/packages/cli/test/integration/workflows/workflows.controller.test.ts @@ -1,27 +1,28 @@ +import Container from 'typedi'; import type { SuperAgentTest } from 'supertest'; +import { v4 as uuid } from 'uuid'; import type { INode, IPinData } from 'n8n-workflow'; + import * as UserManagementHelpers from '@/UserManagement/UserManagementHelper'; import type { User } from '@db/entities/User'; -import { v4 as uuid } from 'uuid'; +import { WorkflowRepository } from '@db/repositories/workflow.repository'; +import type { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { RoleService } from '@/services/role.service'; -import Container from 'typedi'; import type { ListQuery } from '@/requests'; import { WorkflowHistoryRepository } from '@db/repositories/workflowHistory.repository'; import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; - -import { mockInstance } from '../shared/mocking'; -import * as utils from './shared/utils/'; -import * as testDb from './shared/testDb'; -import { makeWorkflow, MOCK_PINDATA } from './shared/utils/'; -import { randomCredentialPayload } from './shared/random'; -import { saveCredential } from './shared/db/credentials'; -import { createOwner } from './shared/db/users'; -import { createWorkflow } from './shared/db/workflows'; -import { createTag } from './shared/db/tags'; import { Push } from '@/push'; import { EnterpriseWorkflowService } from '@/workflows/workflow.service.ee'; -import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; -import type { WorkflowEntity } from '@/databases/entities/WorkflowEntity'; + +import { mockInstance } from '../../shared/mocking'; +import * as utils from '../shared/utils/'; +import * as testDb from '../shared/testDb'; +import { makeWorkflow, MOCK_PINDATA } from '../shared/utils/'; +import { randomCredentialPayload } from '../shared/random'; +import { saveCredential } from '../shared/db/credentials'; +import { createOwner } from '../shared/db/users'; +import { createWorkflow } from '../shared/db/workflows'; +import { createTag } from '../shared/db/tags'; let owner: User; let authOwnerAgent: SuperAgentTest; diff --git a/packages/cli/test/unit/WorkflowHelpers.test.ts b/packages/cli/test/unit/WorkflowHelpers.test.ts index 54f8b6e912ce7..998e2b8b42809 100644 --- a/packages/cli/test/unit/WorkflowHelpers.test.ts +++ b/packages/cli/test/unit/WorkflowHelpers.test.ts @@ -1,150 +1,8 @@ -import type { INode } from 'n8n-workflow'; import { type Workflow } from 'n8n-workflow'; -import { WorkflowEntity } from '@db/entities/WorkflowEntity'; -import { CredentialsEntity } from '@db/entities/CredentialsEntity'; -import { - getExecutionStartNode, - getNodesWithInaccessibleCreds, - validateWorkflowCredentialUsage, -} from '@/WorkflowHelpers'; +import { getExecutionStartNode } from '@/WorkflowHelpers'; import type { IWorkflowExecutionDataProcess } from '@/Interfaces'; -const FIRST_CREDENTIAL_ID = '1'; -const SECOND_CREDENTIAL_ID = '2'; -const THIRD_CREDENTIAL_ID = '3'; - -const NODE_WITH_NO_CRED = '0133467b-df4a-473d-9295-fdd9d01fa45a'; -const NODE_WITH_ONE_CRED = '4673f869-f2dc-4a33-b053-ca3193bc5226'; -const NODE_WITH_TWO_CRED = '9b4208bd-8f10-4a6a-ad3b-da47a326f7da'; - describe('WorkflowHelpers', () => { - describe('getNodesWithInaccessibleCreds', () => { - test('Should return an empty list for a workflow without nodes', () => { - const workflow = getWorkflow(); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, []); - expect(nodesWithInaccessibleCreds).toHaveLength(0); - }); - - test('Should return an empty list for a workflow with nodes without credentials', () => { - const workflow = getWorkflow({ addNodeWithoutCreds: true }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, []); - expect(nodesWithInaccessibleCreds).toHaveLength(0); - }); - - test('Should return an element for a node with a credential without access', () => { - const workflow = getWorkflow({ addNodeWithOneCred: true }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, []); - expect(nodesWithInaccessibleCreds).toHaveLength(1); - }); - - test('Should return an empty list for a node with a credential with access', () => { - const workflow = getWorkflow({ addNodeWithOneCred: true }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, [ - FIRST_CREDENTIAL_ID, - ]); - expect(nodesWithInaccessibleCreds).toHaveLength(0); - }); - - test('Should return an element for a node with two credentials and mixed access', () => { - const workflow = getWorkflow({ addNodeWithTwoCreds: true }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, [ - SECOND_CREDENTIAL_ID, - ]); - expect(nodesWithInaccessibleCreds).toHaveLength(1); - }); - - test('Should return one node for a workflow with two nodes and two credentials', () => { - const workflow = getWorkflow({ addNodeWithOneCred: true, addNodeWithTwoCreds: true }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, [ - SECOND_CREDENTIAL_ID, - THIRD_CREDENTIAL_ID, - ]); - expect(nodesWithInaccessibleCreds).toHaveLength(1); - }); - - test('Should return one element for a workflows with two nodes and one credential', () => { - const workflow = getWorkflow({ - addNodeWithoutCreds: true, - addNodeWithOneCred: true, - addNodeWithTwoCreds: true, - }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, [ - FIRST_CREDENTIAL_ID, - ]); - expect(nodesWithInaccessibleCreds).toHaveLength(1); - }); - - test('Should return one element for a workflows with two nodes and partial credential access', () => { - const workflow = getWorkflow({ addNodeWithOneCred: true, addNodeWithTwoCreds: true }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, [ - FIRST_CREDENTIAL_ID, - SECOND_CREDENTIAL_ID, - ]); - expect(nodesWithInaccessibleCreds).toHaveLength(1); - }); - - test('Should return two elements for a workflows with two nodes and partial credential access', () => { - const workflow = getWorkflow({ addNodeWithOneCred: true, addNodeWithTwoCreds: true }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, [ - SECOND_CREDENTIAL_ID, - ]); - expect(nodesWithInaccessibleCreds).toHaveLength(2); - }); - - test('Should return two elements for a workflows with two nodes and no credential access', () => { - const workflow = getWorkflow({ addNodeWithOneCred: true, addNodeWithTwoCreds: true }); - const nodesWithInaccessibleCreds = getNodesWithInaccessibleCreds(workflow, []); - expect(nodesWithInaccessibleCreds).toHaveLength(2); - }); - }); - - describe('validateWorkflowCredentialUsage', () => { - it('Should throw error saving a workflow using credential without access', () => { - const newWorkflowVersion = getWorkflow({ addNodeWithOneCred: true }); - const previousWorkflowVersion = getWorkflow(); - expect(() => { - validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []); - }).toThrow(); - }); - - it('Should not throw error when saving a workflow using credential with access', () => { - const newWorkflowVersion = getWorkflow({ addNodeWithOneCred: true }); - const previousWorkflowVersion = getWorkflow(); - expect(() => { - validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, [ - generateCredentialEntity(FIRST_CREDENTIAL_ID), - ]); - }).not.toThrow(); - }); - - it('Should not throw error when saving a workflow removing node without credential access', () => { - const newWorkflowVersion = getWorkflow(); - const previousWorkflowVersion = getWorkflow({ addNodeWithOneCred: true }); - expect(() => { - validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, [ - generateCredentialEntity(FIRST_CREDENTIAL_ID), - ]); - }).not.toThrow(); - }); - - it('Should save fine when not making changes to workflow without access', () => { - const workflowWithOneCredential = getWorkflow({ addNodeWithOneCred: true }); - expect(() => { - validateWorkflowCredentialUsage(workflowWithOneCredential, workflowWithOneCredential, []); - }).not.toThrow(); - }); - - it('Should throw error saving a workflow adding node without credential access', () => { - const newWorkflowVersion = getWorkflow({ - addNodeWithOneCred: true, - addNodeWithTwoCreds: true, - }); - const previousWorkflowVersion = getWorkflow({ addNodeWithOneCred: true }); - expect(() => { - validateWorkflowCredentialUsage(newWorkflowVersion, previousWorkflowVersion, []); - }).toThrow(); - }); - }); describe('getExecutionStartNode', () => { it('Should return undefined', () => { const data = { @@ -186,77 +44,3 @@ describe('WorkflowHelpers', () => { }); }); }); - -function generateCredentialEntity(credentialId: string) { - const credentialEntity = new CredentialsEntity(); - credentialEntity.id = credentialId; - return credentialEntity; -} - -export function getWorkflow(options?: { - addNodeWithoutCreds?: boolean; - addNodeWithOneCred?: boolean; - addNodeWithTwoCreds?: boolean; -}) { - const workflow = new WorkflowEntity(); - - workflow.nodes = []; - - if (options?.addNodeWithoutCreds) { - workflow.nodes.push(nodeWithNoCredentials); - } - - if (options?.addNodeWithOneCred) { - workflow.nodes.push(nodeWithOneCredential); - } - - if (options?.addNodeWithTwoCreds) { - workflow.nodes.push(nodeWithTwoCredentials); - } - - return workflow; -} - -const nodeWithNoCredentials: INode = { - id: NODE_WITH_NO_CRED, - name: 'Node with no Credential', - typeVersion: 1, - type: 'n8n-nodes-base.fakeNode', - position: [0, 0], - credentials: {}, - parameters: {}, -}; - -const nodeWithOneCredential: INode = { - id: NODE_WITH_ONE_CRED, - name: 'Node with a single credential', - typeVersion: 1, - type: '', - position: [0, 0], - credentials: { - test: { - id: FIRST_CREDENTIAL_ID, - name: 'First fake credential', - }, - }, - parameters: {}, -}; - -const nodeWithTwoCredentials: INode = { - id: NODE_WITH_TWO_CRED, - name: 'Node with two credentials', - typeVersion: 1, - type: '', - position: [0, 0], - credentials: { - mcTest: { - id: SECOND_CREDENTIAL_ID, - name: 'Second fake credential', - }, - mcTest2: { - id: THIRD_CREDENTIAL_ID, - name: 'Third fake credential', - }, - }, - parameters: {}, -}; diff --git a/packages/cli/test/unit/services/workflowHistory.service.ee.test.ts b/packages/cli/test/unit/services/workflowHistory.service.ee.test.ts index 7a623188e6a83..05ccae70051a2 100644 --- a/packages/cli/test/unit/services/workflowHistory.service.ee.test.ts +++ b/packages/cli/test/unit/services/workflowHistory.service.ee.test.ts @@ -1,11 +1,11 @@ +import { mockClear } from 'jest-mock-extended'; import { User } from '@db/entities/User'; import { WorkflowHistoryRepository } from '@db/repositories/workflowHistory.repository'; import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository'; import { WorkflowHistoryService } from '@/workflows/workflowHistory/workflowHistory.service.ee'; -import { mockInstance } from '../../shared/mocking'; import { Logger } from '@/Logger'; -import { getWorkflow } from '../WorkflowHelpers.test'; -import { mockClear } from 'jest-mock-extended'; +import { mockInstance } from '../../shared/mocking'; +import { getWorkflow } from '../../integration/shared/workflow'; const workflowHistoryRepository = mockInstance(WorkflowHistoryRepository); const logger = mockInstance(Logger);