diff --git a/packages/cli/src/UserManagement/PermissionChecker.ts b/packages/cli/src/UserManagement/PermissionChecker.ts new file mode 100644 index 0000000000..9d911343a9 --- /dev/null +++ b/packages/cli/src/UserManagement/PermissionChecker.ts @@ -0,0 +1,75 @@ +import { INode, NodeOperationError, Workflow } from 'n8n-workflow'; +import { In } from 'typeorm'; +import * as Db from '@/Db'; + +export class PermissionChecker { + /** + * Check if a user is permitted to execute a workflow. + */ + static async check(workflow: Workflow, userId: string) { + // allow if no nodes in this workflow use creds + + const credIdsToNodes = PermissionChecker.mapCredIdsToNodes(workflow); + + const workflowCredIds = Object.keys(credIdsToNodes); + + if (workflowCredIds.length === 0) return; + + // allow if requesting user is instance owner + + const user = await Db.collections.User.findOneOrFail(userId, { + relations: ['globalRole'], + }); + + if (user.globalRole.name === 'owner') return; + + // allow if all creds used in this workflow are a subset of + // all creds accessible to users who have access to this workflow + + const workflowSharings = await Db.collections.SharedWorkflow.find({ + relations: ['workflow'], + where: { workflow: { id: Number(workflow.id) } }, + }); + + const workflowUserIds = workflowSharings.map((s) => s.userId); + + const credentialSharings = await Db.collections.SharedCredentials.find({ + where: { user: In(workflowUserIds) }, + }); + + const accessibleCredIds = credentialSharings.map((s) => s.credentialId.toString()); + + const inaccessibleCredIds = workflowCredIds.filter((id) => !accessibleCredIds.includes(id)); + + if (inaccessibleCredIds.length === 0) return; + + // if disallowed, flag only first node using first inaccessible cred + + const nodeToFlag = credIdsToNodes[inaccessibleCredIds[0]][0]; + + throw new NodeOperationError(nodeToFlag, 'Node has no access to credential', { + description: 'Please recreate the credential or ask its owner to share it with you.', + }); + } + + private static mapCredIdsToNodes(workflow: Workflow) { + return Object.values(workflow.nodes).reduce<{ [credentialId: string]: INode[] }>( + (map, node) => { + 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.', + }); + } + + map[cred.id] = map[cred.id] ? [...map[cred.id], node] : [node]; + }); + + return map; + }, + {}, + ); + } +} diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index e1aaeed9af..98cec9ce53 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -145,97 +145,6 @@ export async function getUserById(userId: string): Promise { return user; } -export async function checkPermissionsForExecution( - workflow: Workflow, - userId: string, -): Promise { - const credentialIds = new Set(); - const nodeNames = Object.keys(workflow.nodes); - const credentialUsedBy = new Map(); - // Iterate over all nodes - nodeNames.forEach((nodeName) => { - const node = workflow.nodes[nodeName]; - if (node.disabled === true) { - // If a node is disabled there is no need to check its credentials - return; - } - // And check if any of the nodes uses credentials. - if (node.credentials) { - const credentialNames = Object.keys(node.credentials); - // For every credential this node uses - credentialNames.forEach((credentialName) => { - const credentialDetail = node.credentials![credentialName]; - // If it does not contain an id, it means it is a very old - // workflow. Nowadays it should not happen anymore. - // Migrations should handle the case where a credential does - // not have an id. - if (credentialDetail.id === null) { - throw new NodeOperationError( - node, - `The credential on node '${node.name}' is not valid. Please open the workflow and set it to a valid value.`, - ); - } - if (!credentialDetail.id) { - throw new NodeOperationError( - node, - `Error initializing workflow: credential ID not present. Please open the workflow and save it to fix this error. [Node: '${node.name}']`, - ); - } - credentialIds.add(credentialDetail.id.toString()); - if (!credentialUsedBy.has(credentialDetail.id)) { - credentialUsedBy.set(credentialDetail.id, node); - } - }); - } - }); - - // Now that we obtained all credential IDs used by this workflow, we can - // now check if the owner of this workflow has access to all of them. - - const ids = Array.from(credentialIds); - - if (ids.length === 0) { - // If the workflow does not use any credentials, then we're fine - return true; - } - // If this check happens on top, we may get - // uninitialized db errors. - // Db is certainly initialized if workflow uses credentials. - const user = await getUserById(userId); - if (user.globalRole.name === 'owner') { - return true; - } - - // Check for the user's permission to all used credentials - const credentialsWithAccess = await Db.collections.SharedCredentials.find({ - where: { - user: { id: userId }, - credentials: In(ids), - }, - }); - - // Considering the user needs to have access to all credentials - // then both arrays (allowed credentials vs used credentials) - // must be the same length - if (ids.length !== credentialsWithAccess.length) { - credentialsWithAccess.forEach((credential) => { - credentialUsedBy.delete(credential.credentialId.toString()); - }); - - // Find the first missing node from the Set - this is arbitrarily fetched - const firstMissingCredentialNode = credentialUsedBy.values().next().value as INode; - throw new NodeOperationError( - firstMissingCredentialNode, - 'This node does not have access to the required credential', - { - description: - 'Maybe the credential was removed or you have lost access to it. Try contacting the owner if this credential does not belong to you', - }, - ); - } - return true; -} - /** * Check if a URL contains an auth-excluded endpoint. */ diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 6a9caef48d..3e72eb5ed1 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -62,12 +62,9 @@ import * as Push from '@/Push'; import * as ResponseHelper from '@/ResponseHelper'; import * as WebhookHelpers from '@/WebhookHelpers'; import * as WorkflowHelpers from '@/WorkflowHelpers'; -import { - checkPermissionsForExecution, - getUserById, - getWorkflowOwner, -} from '@/UserManagement/UserManagementHelper'; +import { getUserById, getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { findSubworkflowStart } from '@/utils'; +import { PermissionChecker } from './UserManagement/PermissionChecker'; const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType'); @@ -942,7 +939,7 @@ export async function executeWorkflow( let data; try { - await checkPermissionsForExecution(workflow, additionalData.userId); + await PermissionChecker.check(workflow, additionalData.userId); // Create new additionalData to have different workflow loaded and to call // different webhooks diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index 7bbee72c25..5dc546bf25 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -53,9 +53,9 @@ import * as WebhookHelpers from '@/WebhookHelpers'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; import { InternalHooksManager } from '@/InternalHooksManager'; -import { checkPermissionsForExecution } from '@/UserManagement/UserManagementHelper'; import { generateFailedExecutionFromError } from '@/WorkflowHelpers'; import { initErrorHandling } from '@/ErrorReporting'; +import { PermissionChecker } from '@/UserManagement/PermissionChecker'; export class WorkflowRunner { activeExecutions: ActiveExecutions.ActiveExecutions; @@ -267,7 +267,7 @@ export class WorkflowRunner { ); try { - await checkPermissionsForExecution(workflow, data.userId); + await PermissionChecker.check(workflow, data.userId); } catch (error) { ErrorReporter.error(error); // Create a failed execution with the data for the node diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index 88e8be7150..8271bee426 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -46,10 +46,10 @@ import { getLogger } from '@/Logger'; import config from '@/config'; import { InternalHooksManager } from '@/InternalHooksManager'; -import { checkPermissionsForExecution } from '@/UserManagement/UserManagementHelper'; import { loadClassInIsolation } from '@/CommunityNodes/helpers'; import { generateFailedExecutionFromError } from '@/WorkflowHelpers'; import { initErrorHandling } from '@/ErrorReporting'; +import { PermissionChecker } from '@/UserManagement/PermissionChecker'; export class WorkflowRunnerProcess { data: IWorkflowExecutionDataProcessWithExecution | undefined; @@ -225,7 +225,7 @@ export class WorkflowRunnerProcess { pinData: this.data.pinData, }); try { - await checkPermissionsForExecution(this.workflow, userId); + await PermissionChecker.check(this.workflow, userId); } catch (error) { const caughtError = error as NodeOperationError; const failedExecutionData = generateFailedExecutionFromError( diff --git a/packages/cli/src/commands/worker.ts b/packages/cli/src/commands/worker.ts index b851a6b0a2..d58420c0c1 100644 --- a/packages/cli/src/commands/worker.ts +++ b/packages/cli/src/commands/worker.ts @@ -38,13 +38,11 @@ import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData' import { InternalHooksManager } from '@/InternalHooksManager'; import { LoadNodesAndCredentials } from '@/LoadNodesAndCredentials'; import { getLogger } from '@/Logger'; +import { PermissionChecker } from '@/UserManagement/PermissionChecker'; import config from '@/config'; import * as Queue from '@/Queue'; -import { - checkPermissionsForExecution, - getWorkflowOwner, -} from '@/UserManagement/UserManagementHelper'; +import { getWorkflowOwner } from '@/UserManagement/UserManagementHelper'; import { generateFailedExecutionFromError } from '@/WorkflowHelpers'; export class Worker extends Command { @@ -199,7 +197,7 @@ export class Worker extends Command { ); try { - await checkPermissionsForExecution(workflow, workflowOwner.id); + await PermissionChecker.check(workflow, workflowOwner.id); } catch (error) { const failedExecution = generateFailedExecutionFromError( currentExecutionDb.mode, diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index cbc2398a03..569a30709c 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -39,7 +39,7 @@ export type AuthenticatedRequest< // ---------------------------------- export declare namespace WorkflowRequest { - type RequestBody = Partial<{ + type CreateUpdatePayload = Partial<{ id: string; // delete if sent name: string; nodes: INode[]; @@ -50,13 +50,26 @@ export declare namespace WorkflowRequest { hash: string; }>; - type Create = AuthenticatedRequest<{}, {}, RequestBody>; + type ManualRunPayload = { + workflowData: IWorkflowDb; + runData: IRunData; + pinData: IPinData; + startNodes?: string[]; + destinationNode?: string; + }; + + type Create = AuthenticatedRequest<{}, {}, CreateUpdatePayload>; type Get = AuthenticatedRequest<{ id: string }>; type Delete = Get; - type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody, { forceSave?: string }>; + type Update = AuthenticatedRequest< + { id: string }, + {}, + CreateUpdatePayload, + { forceSave?: string } + >; type NewName = AuthenticatedRequest<{}, {}, {}, { name?: string }>; @@ -66,17 +79,7 @@ export declare namespace WorkflowRequest { type GetAllActivationErrors = Get; - type ManualRun = AuthenticatedRequest< - {}, - {}, - { - workflowData: IWorkflowDb; - runData: IRunData; - pinData: IPinData; - startNodes?: string[]; - destinationNode?: string; - } - >; + type ManualRun = AuthenticatedRequest<{}, {}, ManualRunPayload>; type Share = AuthenticatedRequest<{ workflowId: string }, {}, { shareWithIds: string[] }>; } diff --git a/packages/cli/src/workflows/workflows.controller.ee.ts b/packages/cli/src/workflows/workflows.controller.ee.ts index 45ea5cc9ff..4735e389fe 100644 --- a/packages/cli/src/workflows/workflows.controller.ee.ts +++ b/packages/cli/src/workflows/workflows.controller.ee.ts @@ -15,6 +15,8 @@ import { LoggerProxy } from 'n8n-workflow'; import * as TagHelpers from '@/TagHelpers'; import { EECredentialsService as EECredentials } from '../credentials/credentials.service.ee'; import { WorkflowsService } from './workflows.services'; +import { IExecutionPushResponse } from '@/Interfaces'; +import * as GenericHelpers from '@/GenericHelpers'; // eslint-disable-next-line @typescript-eslint/naming-convention export const EEWorkflowController = express.Router(); @@ -214,9 +216,11 @@ EEWorkflowController.patch( const { tags, ...rest } = req.body; Object.assign(updateData, rest); - const updatedWorkflow = await EEWorkflows.updateWorkflow( + const safeWorkflow = await EEWorkflows.preventTampering(updateData, workflowId, req.user); + + const updatedWorkflow = await WorkflowsService.update( req.user, - updateData, + safeWorkflow, workflowId, tags, forceSave, @@ -230,3 +234,24 @@ EEWorkflowController.patch( }; }), ); + +/** + * (EE) POST /workflows/run + */ +EEWorkflowController.post( + '/run', + ResponseHelper.send(async (req: WorkflowRequest.ManualRun): Promise => { + const workflow = new WorkflowEntity(); + Object.assign(workflow, req.body.workflowData); + + const safeWorkflow = await EEWorkflows.preventTampering( + workflow, + workflow.id.toString(), + req.user, + ); + + req.body.workflowData.nodes = safeWorkflow.nodes; + + return WorkflowsService.runManually(req.body, req.user, GenericHelpers.getSessionId(req)); + }), +); diff --git a/packages/cli/src/workflows/workflows.controller.ts b/packages/cli/src/workflows/workflows.controller.ts index a324dea302..663f71b609 100644 --- a/packages/cli/src/workflows/workflows.controller.ts +++ b/packages/cli/src/workflows/workflows.controller.ts @@ -1,7 +1,7 @@ /* eslint-disable no-param-reassign */ import express from 'express'; -import { INode, LoggerProxy, Workflow } from 'n8n-workflow'; +import { LoggerProxy } from 'n8n-workflow'; import axios from 'axios'; import * as ActiveWorkflowRunner from '@/ActiveWorkflowRunner'; @@ -10,15 +10,7 @@ import * as GenericHelpers from '@/GenericHelpers'; import * as ResponseHelper from '@/ResponseHelper'; import * as WorkflowHelpers from '@/WorkflowHelpers'; import { whereClause } from '@/CredentialsHelper'; -import { NodeTypes } from '@/NodeTypes'; -import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; -import * as TestWebhooks from '@/TestWebhooks'; -import { WorkflowRunner } from '@/WorkflowRunner'; -import { - IWorkflowResponse, - IExecutionPushResponse, - IWorkflowExecutionDataProcess, -} from '@/Interfaces'; +import { IWorkflowResponse, IExecutionPushResponse } from '@/Interfaces'; import config from '@/config'; import * as TagHelpers from '@/TagHelpers'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; @@ -260,12 +252,7 @@ workflowsController.patch( const { tags, ...rest } = req.body; Object.assign(updateData, rest); - const updatedWorkflow = await WorkflowsService.updateWorkflow( - req.user, - updateData, - workflowId, - tags, - ); + const updatedWorkflow = await WorkflowsService.update(req.user, updateData, workflowId, tags); const { id, ...remainder } = updatedWorkflow; @@ -326,82 +313,8 @@ workflowsController.delete( * POST /workflows/run */ workflowsController.post( - `/run`, + '/run', ResponseHelper.send(async (req: WorkflowRequest.ManualRun): Promise => { - const { workflowData } = req.body; - const { runData } = req.body; - const { pinData } = req.body; - const { startNodes } = req.body; - const { destinationNode } = req.body; - const executionMode = 'manual'; - const activationMode = 'manual'; - - const sessionId = GenericHelpers.getSessionId(req); - - const pinnedTrigger = WorkflowsService.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(req.user.id); - const nodeTypes = NodeTypes(); - const workflowInstance = new Workflow({ - id: workflowData.id?.toString(), - name: workflowData.name, - nodes: workflowData.nodes, - connections: workflowData.connections, - active: false, - nodeTypes, - staticData: undefined, - settings: workflowData.settings, - }); - const needsWebhook = await TestWebhooks.getInstance().needsWebhookData( - workflowData, - workflowInstance, - additionalData, - executionMode, - activationMode, - 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, - runData, - pinData, - sessionId, - startNodes, - workflowData, - userId: req.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, - }; + return WorkflowsService.runManually(req.body, req.user, GenericHelpers.getSessionId(req)); }), ); diff --git a/packages/cli/src/workflows/workflows.services.ee.ts b/packages/cli/src/workflows/workflows.services.ee.ts index 046f457423..0ff086c867 100644 --- a/packages/cli/src/workflows/workflows.services.ee.ts +++ b/packages/cli/src/workflows/workflows.services.ee.ts @@ -158,21 +158,17 @@ export class EEWorkflowsService extends WorkflowsService { }); } - static async updateWorkflow( - user: User, - workflow: WorkflowEntity, - workflowId: string, - tags?: string[], - forceSave?: boolean, - ): Promise { + static async preventTampering(workflow: WorkflowEntity, workflowId: string, user: User) { const previousVersion = await EEWorkflowsService.get({ id: parseInt(workflowId, 10) }); + if (!previousVersion) { throw new ResponseHelper.ResponseError('Workflow not found', undefined, 404); } + const allCredentials = await EECredentials.getAll(user); + try { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call - workflow = WorkflowHelpers.validateWorkflowCredentialUsage( + return WorkflowHelpers.validateWorkflowCredentialUsage( workflow, previousVersion, allCredentials, @@ -184,7 +180,5 @@ export class EEWorkflowsService extends WorkflowsService { 400, ); } - - return super.updateWorkflow(user, workflow, workflowId, tags, forceSave); } } diff --git a/packages/cli/src/workflows/workflows.services.ts b/packages/cli/src/workflows/workflows.services.ts index 0148804263..98de76c726 100644 --- a/packages/cli/src/workflows/workflows.services.ts +++ b/packages/cli/src/workflows/workflows.services.ts @@ -1,6 +1,6 @@ -import { IPinData, JsonObject, jsonParse, LoggerProxy } from 'n8n-workflow'; -import { FindManyOptions, FindOneOptions, In, ObjectLiteral } from 'typeorm'; import { validate as jsonSchemaValidate } from 'jsonschema'; +import { INode, IPinData, JsonObject, jsonParse, LoggerProxy, Workflow } from 'n8n-workflow'; +import { FindManyOptions, FindOneOptions, In, ObjectLiteral } from 'typeorm'; import * as ActiveWorkflowRunner from '@/ActiveWorkflowRunner'; import * as Db from '@/Db'; import { InternalHooksManager } from '@/InternalHooksManager'; @@ -14,8 +14,13 @@ import { WorkflowEntity } from '@db/entities/WorkflowEntity'; import { validateEntity } from '@/GenericHelpers'; import { externalHooks } from '@/Server'; import * as TagHelpers from '@/TagHelpers'; +import { WorkflowRequest } from '@/requests'; +import { IWorkflowDb, IWorkflowExecutionDataProcess } from '@/Interfaces'; +import { NodeTypes } from '@/NodeTypes'; +import { WorkflowRunner } from '@/WorkflowRunner'; +import * as WorkflowExecuteAdditionalData from '@/WorkflowExecuteAdditionalData'; +import * as TestWebhooks from '@/TestWebhooks'; import { getSharedWorkflowIds } from '@/WorkflowHelpers'; -import { IWorkflowDb } from '..'; export interface IGetWorkflowsQueryFilter { id?: number | string; @@ -162,7 +167,7 @@ export class WorkflowsService { }); } - static async updateWorkflow( + static async update( user: User, workflow: WorkflowEntity, workflowId: string, @@ -308,4 +313,86 @@ export class WorkflowsService { return updatedWorkflow; } + + static async runManually( + { + workflowData, + runData, + pinData, + startNodes, + destinationNode, + }: WorkflowRequest.ManualRunPayload, + user: User, + sessionId?: string, + ) { + const EXECUTION_MODE = 'manual'; + const ACTIVATION_MODE = 'manual'; + + const pinnedTrigger = WorkflowsService.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 workflow = new Workflow({ + id: workflowData.id?.toString(), + name: workflowData.name, + nodes: workflowData.nodes, + connections: workflowData.connections, + active: false, + nodeTypes: NodeTypes(), + staticData: undefined, + settings: workflowData.settings, + }); + + const additionalData = await WorkflowExecuteAdditionalData.getBase(user.id); + + const needsWebhook = await TestWebhooks.getInstance().needsWebhookData( + workflowData, + workflow, + additionalData, + EXECUTION_MODE, + ACTIVATION_MODE, + 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: EXECUTION_MODE, + 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, + }; + } } diff --git a/packages/cli/test/integration/shared/random.ts b/packages/cli/test/integration/shared/random.ts index 1eac084060..1855e50735 100644 --- a/packages/cli/test/integration/shared/random.ts +++ b/packages/cli/test/integration/shared/random.ts @@ -17,7 +17,13 @@ export function randomApiKey() { const chooseRandomly = (array: T[]) => array[Math.floor(Math.random() * array.length)]; -const randomDigit = () => Math.floor(Math.random() * 10); +export const randomDigit = () => Math.floor(Math.random() * 10); + +export const randomPositiveDigit = (): number => { + const digit = randomDigit(); + + return digit === 0 ? randomPositiveDigit() : digit; +}; const randomUppercaseLetter = () => chooseRandomly('ABCDEFGHIJKLMNOPQRSTUVWXYZ'.split('')); diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index 5c3029f2c6..3360096fcf 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -322,6 +322,10 @@ export async function createUser(attributes: Partial = {}): Promise return Db.collections.User.save(user); } +export async function createOwner() { + return createUser({ globalRole: await getGlobalOwnerRole() }); +} + export function createUserShell(globalRole: Role): Promise { if (globalRole.scope !== 'global') { throw new Error(`Invalid role received: ${JSON.stringify(globalRole)}`); diff --git a/packages/cli/test/integration/workflows.controller.ee.test.ts b/packages/cli/test/integration/workflows.controller.ee.test.ts index 1ba75e0f2f..5a24349eb1 100644 --- a/packages/cli/test/integration/workflows.controller.ee.test.ts +++ b/packages/cli/test/integration/workflows.controller.ee.test.ts @@ -17,12 +17,12 @@ jest.mock('@/telemetry'); let app: express.Application; let testDbName = ''; - let globalOwnerRole: Role; let globalMemberRole: Role; let credentialOwnerRole: Role; let authAgent: AuthAgent; let saveCredential: SaveCredentialFunction; +let isSharingEnabled: jest.SpyInstance; let workflowRunner: ActiveWorkflowRunner; let sharingSpy: jest.SpyInstance; @@ -45,7 +45,9 @@ beforeAll(async () => { utils.initTestLogger(); utils.initTestTelemetry(); - config.set('enterprise.workflowSharingEnabled', true); + isSharingEnabled = jest.spyOn(UserManagementHelpers, 'isSharingEnabled').mockReturnValue(true); + + config.set('enterprise.workflowSharingEnabled', true); // @TODO: Remove once temp flag is removed await utils.initNodeTypes(); workflowRunner = await utils.initActiveWorkflowRunner(); @@ -62,6 +64,32 @@ afterAll(async () => { await testDb.terminate(testDbName); }); +test('Router should switch dynamically', async () => { + const owner = await testDb.createUser({ globalRole: globalOwnerRole }); + const member = await testDb.createUser({ globalRole: globalMemberRole }); + + const createWorkflowResponse = await authAgent(owner).post('/workflows').send(makeWorkflow()); + const { id } = createWorkflowResponse.body.data; + + // free router + + isSharingEnabled.mockReturnValueOnce(false); + + const freeShareResponse = await authAgent(owner) + .put(`/workflows/${id}/share`) + .send({ shareWithIds: [member.id] }); + + expect(freeShareResponse.status).toBe(404); + + // EE router + + const paidShareResponse = await authAgent(owner) + .put(`/workflows/${id}/share`) + .send({ shareWithIds: [member.id] }); + + expect(paidShareResponse.status).toBe(200); +}); + describe('PUT /workflows/:id', () => { test('PUT /workflows/:id/share should save sharing with new users', async () => { const owner = await testDb.createUser({ globalRole: globalOwnerRole }); diff --git a/packages/cli/test/unit/Helpers.ts b/packages/cli/test/unit/Helpers.ts index 6b0bbbf45b..64ca1c2679 100644 --- a/packages/cli/test/unit/Helpers.ts +++ b/packages/cli/test/unit/Helpers.ts @@ -36,7 +36,9 @@ class NodeTypesClass implements INodeTypes { }, }; - async init(nodeTypes: INodeTypeData): Promise {} + async init(nodeTypes: INodeTypeData): Promise { + this.nodeTypes = nodeTypes; + } getAll(): INodeType[] { return Object.values(this.nodeTypes).map((data) => NodeHelpers.getVersionedNodeType(data.type)); diff --git a/packages/cli/test/unit/PermissionChecker.test.ts b/packages/cli/test/unit/PermissionChecker.test.ts new file mode 100644 index 0000000000..2c029e6bdc --- /dev/null +++ b/packages/cli/test/unit/PermissionChecker.test.ts @@ -0,0 +1,223 @@ +import { v4 as uuid } from 'uuid'; +import { INodeTypeData, INodeTypes, Workflow } from 'n8n-workflow'; + +import { Db } from '../../src'; +import * as testDb from '../integration/shared/testDb'; +import { NodeTypes as MockNodeTypes } from './Helpers'; +import { PermissionChecker } from '../../src/UserManagement/PermissionChecker'; +import { + randomCredentialPayload as randomCred, + randomPositiveDigit, +} from '../integration/shared/random'; + +import type { Role } from '../../src/databases/entities/Role'; +import type { SaveCredentialFunction } from '../integration/shared/types'; + +let testDbName = ''; +let mockNodeTypes: INodeTypes; +let credentialOwnerRole: Role; +let workflowOwnerRole: Role; +let saveCredential: SaveCredentialFunction; + +beforeAll(async () => { + const initResult = await testDb.init(); + testDbName = initResult.testDbName; + + mockNodeTypes = MockNodeTypes(); + await mockNodeTypes.init(MOCK_NODE_TYPES_DATA); + + credentialOwnerRole = await testDb.getCredentialOwnerRole(); + workflowOwnerRole = await testDb.getWorkflowOwnerRole(); + + saveCredential = testDb.affixRoleToSaveCredential(credentialOwnerRole); +}); + +beforeEach(async () => { + await testDb.truncate(['SharedWorkflow', 'SharedCredentials'], testDbName); + await testDb.truncate(['User', 'Workflow', 'Credentials'], testDbName); +}); + +afterAll(async () => { + await testDb.terminate(testDbName); +}); + +describe('PermissionChecker.check()', () => { + test('should allow if workflow has no creds', async () => { + const userId = uuid(); + + const workflow = new Workflow({ + id: randomPositiveDigit().toString(), + name: 'test', + active: false, + connections: {}, + nodeTypes: mockNodeTypes, + nodes: [ + { + id: uuid(), + name: 'Start', + type: 'n8n-nodes-base.start', + typeVersion: 1, + parameters: {}, + position: [0, 0], + }, + ], + }); + + expect(() => PermissionChecker.check(workflow, userId)).not.toThrow(); + }); + + test('should allow if requesting user is instance owner', async () => { + const owner = await testDb.createOwner(); + + const workflow = new Workflow({ + id: randomPositiveDigit().toString(), + name: 'test', + active: false, + connections: {}, + nodeTypes: mockNodeTypes, + nodes: [ + { + id: uuid(), + name: 'Action Network', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + actionNetworkApi: { + id: randomPositiveDigit().toString(), + name: 'Action Network Account', + }, + }, + }, + ], + }); + + expect(async () => await PermissionChecker.check(workflow, owner.id)).not.toThrow(); + }); + + test('should allow if workflow creds are valid subset', async () => { + const [owner, member] = await Promise.all([testDb.createOwner(), testDb.createUser()]); + + const ownerCred = await saveCredential(randomCred(), { user: owner }); + const memberCred = await saveCredential(randomCred(), { user: member }); + + const workflow = new Workflow({ + id: randomPositiveDigit().toString(), + name: 'test', + active: false, + connections: {}, + nodeTypes: mockNodeTypes, + nodes: [ + { + id: uuid(), + name: 'Action Network', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + actionNetworkApi: { + id: ownerCred.id.toString(), + name: ownerCred.name, + }, + }, + }, + { + id: uuid(), + name: 'Action Network 2', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0], + credentials: { + actionNetworkApi: { + id: memberCred.id.toString(), + name: memberCred.name, + }, + }, + }, + ], + }); + + expect(async () => await PermissionChecker.check(workflow, owner.id)).not.toThrow(); + }); + + test('should deny if workflow creds are not valid subset', async () => { + const member = await testDb.createUser(); + + const memberCred = await saveCredential(randomCred(), { user: member }); + + const workflowDetails = { + id: randomPositiveDigit(), + name: 'test', + active: false, + connections: {}, + nodeTypes: mockNodeTypes, + nodes: [ + { + id: uuid(), + name: 'Action Network', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0] as [number, number], + credentials: { + actionNetworkApi: { + id: memberCred.id.toString(), + name: memberCred.name, + }, + }, + }, + { + id: uuid(), + name: 'Action Network 2', + type: 'n8n-nodes-base.actionNetwork', + parameters: {}, + typeVersion: 1, + position: [0, 0] as [number, number], + credentials: { + actionNetworkApi: { + id: 'non-existing-credential-id', + name: 'Non-existing credential name', + }, + }, + }, + ], + }; + + const workflowEntity = await Db.collections.Workflow.save(workflowDetails); + + await Db.collections.SharedWorkflow.save({ + workflow: workflowEntity, + user: member, + role: workflowOwnerRole, + }); + + const workflow = new Workflow({ ...workflowDetails, id: workflowDetails.id.toString() }); + + expect(PermissionChecker.check(workflow, member.id)).rejects.toThrow(); + }); +}); + +const MOCK_NODE_TYPES_DATA = ['start', 'actionNetwork'].reduce((acc, nodeName) => { + return ( + (acc[`n8n-nodes-base.${nodeName}`] = { + sourcePath: '', + type: { + description: { + displayName: nodeName, + name: nodeName, + group: [], + description: '', + version: 1, + defaults: {}, + inputs: [], + outputs: [], + properties: [], + }, + }, + }), + acc + ); +}, {});