From 549692e36c501b6e15dea4a52ad9c5f608b7ffbf Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Tue, 18 Jan 2022 11:58:46 +0100 Subject: [PATCH 1/9] Runtime checks for credentials load and execute workflows --- packages/cli/commands/execute.ts | 2 + packages/cli/commands/executeBatch.ts | 7 ++++ packages/cli/commands/worker.ts | 4 ++ packages/cli/src/ActiveWorkflowRunner.ts | 24 +++++++---- packages/cli/src/CredentialsHelper.ts | 17 ++++---- packages/cli/src/Interfaces.ts | 4 +- packages/cli/src/Server.ts | 22 +++++----- .../UserManagement/UserManagementHelper.ts | 17 ++++++++ packages/cli/src/WaitTracker.ts | 7 ++++ packages/cli/src/WaitingWebhooks.ts | 10 ++++- packages/cli/src/WebhookHelpers.ts | 21 +++++++++- .../cli/src/WorkflowExecuteAdditionalData.ts | 41 ++++++++++++++----- packages/cli/src/WorkflowHelpers.ts | 4 ++ packages/cli/src/WorkflowRunner.ts | 5 +-- packages/cli/src/WorkflowRunnerProcess.ts | 14 ++++--- packages/core/src/NodeExecuteFunctions.ts | 3 +- packages/core/test/Helpers.ts | 7 ++++ packages/workflow/src/Interfaces.ts | 19 ++++++++- 18 files changed, 177 insertions(+), 51 deletions(-) diff --git a/packages/cli/commands/execute.ts b/packages/cli/commands/execute.ts index 51a24bea2cc97..e5f3443cf55e3 100644 --- a/packages/cli/commands/execute.ts +++ b/packages/cli/commands/execute.ts @@ -28,6 +28,7 @@ import { import { getLogger } from '../src/Logger'; import config = require('../config'); +import { getInstanceowner } from '../src/UserManagement/UserManagementHelper'; export class Execute extends Command { static description = '\nExecutes a given workflow'; @@ -173,6 +174,7 @@ export class Execute extends Command { startNodes: [startNode.name], // eslint-disable-next-line @typescript-eslint/no-non-null-assertion workflowData: workflowData!, + user: await getInstanceowner(), }; const workflowRunner = new WorkflowRunner(); diff --git a/packages/cli/commands/executeBatch.ts b/packages/cli/commands/executeBatch.ts index 3add81ac92b41..c14a9fcd8a53d 100644 --- a/packages/cli/commands/executeBatch.ts +++ b/packages/cli/commands/executeBatch.ts @@ -37,6 +37,8 @@ import { WorkflowRunner, } from '../src'; import config = require('../config'); +import { User } from '../src/databases/entities/User'; +import { getInstanceowner } from '../src/UserManagement/UserManagementHelper'; export class ExecuteBatch extends Command { static description = '\nExecutes multiple workflows once'; @@ -57,6 +59,8 @@ export class ExecuteBatch extends Command { static executionTimeout = 3 * 60 * 1000; + static instanceOwner: User; + static examples = [ `$ n8n executeBatch`, `$ n8n executeBatch --concurrency=10 --skipList=/data/skipList.txt`, @@ -279,6 +283,8 @@ export class ExecuteBatch extends Command { // Wait till the database is ready await startDbInitPromise; + ExecuteBatch.instanceOwner = await getInstanceowner(); + let allWorkflows; const query = Db.collections.Workflow!.createQueryBuilder('workflows'); @@ -666,6 +672,7 @@ export class ExecuteBatch extends Command { executionMode: 'cli', startNodes: [startNode!.name], workflowData, + user: ExecuteBatch.instanceOwner, }; const workflowRunner = new WorkflowRunner(); diff --git a/packages/cli/commands/worker.ts b/packages/cli/commands/worker.ts index b5615ca25ef42..0c4159ae7e06a 100644 --- a/packages/cli/commands/worker.ts +++ b/packages/cli/commands/worker.ts @@ -39,6 +39,7 @@ import { getLogger } from '../src/Logger'; import * as config from '../config'; import * as Queue from '../src/Queue'; +import { getWorkflowOwner } from '../src/UserManagement/UserManagementHelper'; export class Worker extends Command { static description = '\nStarts a n8n worker'; @@ -121,6 +122,8 @@ export class Worker extends Command { `Start job: ${job.id} (Workflow ID: ${currentExecutionDb.workflowData.id} | Execution: ${jobData.executionId})`, ); + const workflowOwner = await getWorkflowOwner(currentExecutionDb.workflowData.id!.toString()); + let { staticData } = currentExecutionDb.workflowData; if (jobData.loadStaticData) { const findOptions = { @@ -165,6 +168,7 @@ export class Worker extends Command { }); const additionalData = await WorkflowExecuteAdditionalData.getBase( + workflowOwner, undefined, executionTimeoutTimestamp, ); diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 7b9ba3d9d7c63..961c4fadb512f 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -70,7 +70,8 @@ export class ActiveWorkflowRunner { // Here I guess we can have a flag on the workflow table like hasTrigger // so intead of pulling all the active wehhooks just pull the actives that have a trigger const workflowsData: IWorkflowDb[] = (await Db.collections.Workflow!.find({ - active: true, + where: { active: true }, + relations: ['shared', 'shared.user', 'shared.user.globalRole'], })) as IWorkflowDb[]; if (!config.get('endpoints.skipWebhoooksDeregistrationOnShutdown')) { @@ -255,7 +256,9 @@ export class ActiveWorkflowRunner { }); } - const workflowData = await Db.collections.Workflow!.findOne(webhook.workflowId); + const workflowData = await Db.collections.Workflow!.findOne(webhook.workflowId, { + relations: ['shared', 'shared.user', 'shared.user.globalRole'], + }); if (workflowData === undefined) { throw new ResponseHelper.ResponseError( `Could not find workflow with id "${webhook.workflowId}"`, @@ -276,7 +279,7 @@ export class ActiveWorkflowRunner { settings: workflowData.settings, }); - const additionalData = await WorkflowExecuteAdditionalData.getBase(); + const additionalData = await WorkflowExecuteAdditionalData.getBase(workflowData.shared[0].user); const webhookData = NodeHelpers.getNodeWebhooks( workflow, @@ -520,7 +523,9 @@ export class ActiveWorkflowRunner { * @memberof ActiveWorkflowRunner */ async removeWorkflowWebhooks(workflowId: string): Promise { - const workflowData = await Db.collections.Workflow!.findOne(workflowId); + const workflowData = await Db.collections.Workflow!.findOne(workflowId, { + relations: ['shared', 'shared.user', 'shared.user.globalRole'], + }); if (workflowData === undefined) { throw new Error(`Could not find workflow with id "${workflowId}"`); } @@ -539,7 +544,7 @@ export class ActiveWorkflowRunner { const mode = 'internal'; - const additionalData = await WorkflowExecuteAdditionalData.getBase(); + const additionalData = await WorkflowExecuteAdditionalData.getBase(workflowData.shared[0].user); const webhooks = WebhookHelpers.getWorkflowWebhooks(workflow, additionalData, undefined, true); @@ -606,6 +611,7 @@ export class ActiveWorkflowRunner { // Start the workflow const runData: IWorkflowExecutionDataProcess = { + user: additionalData.user, executionMode: mode, executionData, workflowData, @@ -709,7 +715,9 @@ export class ActiveWorkflowRunner { let workflowInstance: Workflow; try { if (workflowData === undefined) { - workflowData = (await Db.collections.Workflow!.findOne(workflowId)) as IWorkflowDb; + workflowData = (await Db.collections.Workflow!.findOne(workflowId, { + relations: ['shared', 'shared.user', 'shared.user.globalRole'], + })) as IWorkflowDb; } if (!workflowData) { @@ -738,7 +746,9 @@ export class ActiveWorkflowRunner { } const mode = 'trigger'; - const additionalData = await WorkflowExecuteAdditionalData.getBase(); + const additionalData = await WorkflowExecuteAdditionalData.getBase( + (workflowData as WorkflowEntity).shared[0].user, + ); const getTriggerFunctions = this.getExecuteTriggerFunctions( workflowData, additionalData, diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index 379e8e331c31f..1c2b892787604 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -13,6 +13,7 @@ import { INodeType, INodeTypeData, INodeTypes, + N8nUserData, NodeHelpers, Workflow, WorkflowExecuteMode, @@ -50,7 +51,7 @@ export class CredentialsHelper extends ICredentialsHelper { async getCredentials( nodeCredentials: INodeCredentialsDetails, type: string, - userId?: string, + user: N8nUserData, ): Promise { if (!nodeCredentials.id) { throw new Error(`Credentials "${nodeCredentials.name}" for type "${type}" don't have an ID.`); @@ -59,9 +60,10 @@ export class CredentialsHelper extends ICredentialsHelper { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const qb = Db.collections.Credentials!.createQueryBuilder('c'); qb.where('c.id = :id and c.type = :type', { id: nodeCredentials.id, type }); - if (userId) { - // TODO UM: implement this. - // qb. + if (user.globalRole.name !== 'owner') { + qb.innerJoin('c.shared', 'shared'); + // @ts-ignore + qb.andWhere('shared.userId = :userId', { userId: user.id }); } const credentials = await qb.getOne(); @@ -124,11 +126,11 @@ export class CredentialsHelper extends ICredentialsHelper { nodeCredentials: INodeCredentialsDetails, type: string, mode: WorkflowExecuteMode, + user: N8nUserData, raw?: boolean, expressionResolveValues?: ICredentialsExpressionResolveValues, - userId?: string, ): Promise { - const credentials = await this.getCredentials(nodeCredentials, type, userId); + const credentials = await this.getCredentials(nodeCredentials, type, user); const decryptedDataOriginal = credentials.getData(this.encryptionKey); if (raw === true) { @@ -244,9 +246,10 @@ export class CredentialsHelper extends ICredentialsHelper { nodeCredentials: INodeCredentialsDetails, type: string, data: ICredentialDataDecryptedObject, + user: N8nUserData, ): Promise { // eslint-disable-next-line @typescript-eslint/await-thenable - const credentials = await this.getCredentials(nodeCredentials, type); + const credentials = await this.getCredentials(nodeCredentials, type, user); if (Db.collections.Credentials === null) { // The first time executeWorkflow gets called the Database has diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 34fe15ad203fe..090c768403c53 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -15,6 +15,7 @@ import { ITaskData, ITelemetrySettings, IWorkflowBase as IWorkflowBaseWorkflow, + N8nUserData, Workflow, WorkflowExecuteMode, } from 'n8n-workflow'; @@ -575,7 +576,7 @@ export interface IWorkflowExecutionDataProcess { sessionId?: string; startNodes?: string[]; workflowData: IWorkflowBase; - userId?: string; + user: N8nUserData; } export interface IWorkflowExecutionDataProcessWithExecution extends IWorkflowExecutionDataProcess { @@ -583,6 +584,7 @@ export interface IWorkflowExecutionDataProcessWithExecution extends IWorkflowExe credentialsTypeData: ICredentialsTypeData; executionId: string; nodeTypeData: ITransferNodeTypes; + user: User; } export interface IWorkflowExecuteProcess { diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 1830017de8e83..0631b4837203d 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -1093,7 +1093,7 @@ class App { startNodes.length === 0 || destinationNode === undefined ) { - const additionalData = await WorkflowExecuteAdditionalData.getBase(); + const additionalData = await WorkflowExecuteAdditionalData.getBase(req.user as User); if (this.isUserManagementEnabled) { // TODO UM: test this. // TODO: test this. @@ -1138,11 +1138,8 @@ class App { sessionId, startNodes, workflowData, + user: req.user as User, }; - if (this.isUserManagementEnabled) { - // TODO UM: test this. - data.userId = req.body.userId; - } const workflowRunner = new WorkflowRunner(); const executionId = await workflowRunner.run(data); @@ -1279,9 +1276,10 @@ class App { credentials, ); - const additionalData = await WorkflowExecuteAdditionalData.getBase(currentNodeParameters); - - additionalData.userId = req.user.id; + const additionalData = await WorkflowExecuteAdditionalData.getBase( + req.user as User, + currentNodeParameters, + ); return loadDataInstance.getOptions(methodName, additionalData); }, @@ -1997,7 +1995,6 @@ class App { } const encryptionKey = await UserSettings.getEncryptionKey(); - if (!encryptionKey) { throw new ResponseHelper.ResponseError( RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, @@ -2012,8 +2009,10 @@ class App { credential as INodeCredentialsDetails, credential.type, mode, + req.user, true, ); + const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( decryptedDataOriginal, credential.type, @@ -2134,6 +2133,7 @@ class App { credential as INodeCredentialsDetails, credential.type, mode, + req.user as User, true, ); const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( @@ -2220,7 +2220,6 @@ class App { } const encryptionKey = await UserSettings.getEncryptionKey(); - if (!encryptionKey) { throw new ResponseHelper.ResponseError( RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY, @@ -2235,6 +2234,7 @@ class App { credential as INodeCredentialsDetails, credential.type, mode, + req.user as User, true, ); const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( @@ -2367,6 +2367,7 @@ class App { credential as INodeCredentialsDetails, credential.type, mode, + req.user as User, true, ); const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( @@ -2648,6 +2649,7 @@ class App { executionData: fullExecutionData.data, retryOf: req.params.id, workflowData: fullExecutionData.workflowData, + user: req.user as User, }; const { lastNodeExecuted } = data.executionData!.resultData; diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 1e59542fe3bad..45834898af98e 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -45,6 +45,23 @@ export async function saveCredentialOwnership( })) as SharedCredentials; } +export async function getWorkflowOwner(workflowId: string) { + const workflowDb = await Db.collections.Workflow!.findOneOrFail(workflowId, { + relations: ['shared', 'shared.user', 'shared.user.globalRole'], + }); + + return workflowDb.shared[0].user; +} + +export async function getInstanceowner() { + const qb = Db.collections.User!.createQueryBuilder('u'); + qb.innerJoin('u.globalRole', 'gr'); + qb.andWhere('gr.name = :name and gr.scope = :scope', { name: 'owner', scope: 'global' }); + + const owner = await qb.getOneOrFail(); + return owner; +} + export function isEmailSetup(): boolean { const emailMode = config.get('userManagement.emails.mode') as string; return !!emailMode; diff --git a/packages/cli/src/WaitTracker.ts b/packages/cli/src/WaitTracker.ts index 791ff68e8ef8e..c50a66c0bc7d1 100644 --- a/packages/cli/src/WaitTracker.ts +++ b/packages/cli/src/WaitTracker.ts @@ -24,6 +24,7 @@ import { WorkflowCredentials, WorkflowRunner, } from '.'; +import { getWorkflowOwner } from './UserManagement/UserManagementHelper'; export class WaitTrackerClass { activeExecutionsInstance: ActiveExecutions.ActiveExecutions; @@ -157,10 +158,16 @@ export class WaitTrackerClass { throw new Error('The execution did succeed and can so not be started again.'); } + if (!fullExecutionData.workflowData.id) { + throw new Error('Only saved workflows can be resumed.'); + } + const user = await getWorkflowOwner(fullExecutionData.workflowData.id.toString()); + const data: IWorkflowExecutionDataProcess = { executionMode: fullExecutionData.mode, executionData: fullExecutionData.data, workflowData: fullExecutionData.workflowData, + user, }; // Start the execution again diff --git a/packages/cli/src/WaitingWebhooks.ts b/packages/cli/src/WaitingWebhooks.ts index dd54b37a96fef..0373e40c5eb97 100644 --- a/packages/cli/src/WaitingWebhooks.ts +++ b/packages/cli/src/WaitingWebhooks.ts @@ -26,6 +26,7 @@ import { WorkflowCredentials, WorkflowExecuteAdditionalData, } from '.'; +import { getWorkflowOwner } from './UserManagement/UserManagementHelper'; export class WaitingWebhooks { async executeWebhook( @@ -111,7 +112,14 @@ export class WaitingWebhooks { settings: workflowData.settings, }); - const additionalData = await WorkflowExecuteAdditionalData.getBase(); + let workflowOwner; + try { + workflowOwner = await getWorkflowOwner(workflowData.id!.toString()); + } catch (error) { + throw new ResponseHelper.ResponseError('Could not find workflow', 404, 404); + } + + const additionalData = await WorkflowExecuteAdditionalData.getBase(workflowOwner); const webhookData = NodeHelpers.getNodeWebhooks( workflow, diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index 93da2cf8f44b6..88e1546d8b790 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -1,3 +1,4 @@ +/* eslint-disable import/no-cycle */ /* eslint-disable @typescript-eslint/no-unsafe-call */ /* eslint-disable no-param-reassign */ /* eslint-disable @typescript-eslint/prefer-optional-chain */ @@ -33,6 +34,7 @@ import { IWorkflowDataProxyAdditionalKeys, IWorkflowExecuteAdditionalData, LoggerProxy as Logger, + N8nUserData, NodeHelpers, Workflow, WorkflowExecuteMode, @@ -53,6 +55,8 @@ import { // eslint-disable-next-line import/no-cycle import * as ActiveExecutions from './ActiveExecutions'; +import { WorkflowEntity } from './databases/entities/WorkflowEntity'; +import { getWorkflowOwner } from './UserManagement/UserManagementHelper'; const activeExecutions = ActiveExecutions.getInstance(); @@ -213,8 +217,22 @@ export async function executeWebhook( throw new ResponseHelper.ResponseError(errorMessage, 500, 500); } + let user: N8nUserData; + if ( + (workflowData as WorkflowEntity).shared?.length && + (workflowData as WorkflowEntity).shared[0].user + ) { + user = (workflowData as WorkflowEntity).shared[0].user; + } else { + try { + user = await getWorkflowOwner(workflowData.id.toString()); + } catch (error) { + throw new ResponseHelper.ResponseError('Cannot find workflow', 500, 500); + } + } + // Prepare everything that is needed to run the workflow - const additionalData = await WorkflowExecuteAdditionalData.getBase(); + const additionalData = await WorkflowExecuteAdditionalData.getBase(user); // Add the Response and Request so that this data can be accessed in the node additionalData.httpRequest = req; @@ -389,6 +407,7 @@ export async function executeWebhook( executionData: runExecutionData, sessionId, workflowData, + user, }; let responsePromise: IDeferredPromise | undefined; diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 0faada56f1fac..cbc2c39e7fa89 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -31,6 +31,7 @@ import { IWorkflowExecuteHooks, IWorkflowHooksOptionalParameters, LoggerProxy as Logger, + N8nUserData, Workflow, WorkflowExecuteMode, WorkflowHooks, @@ -698,6 +699,7 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { export async function getRunData( workflowData: IWorkflowBase, + user: N8nUserData, inputData?: INodeExecutionData[], ): Promise { const mode = 'integrated'; @@ -751,27 +753,38 @@ export async function getRunData( executionData: runExecutionData, // @ts-ignore workflowData, + user, }; return runData; } -export async function getWorkflowData(workflowInfo: IExecuteWorkflowInfo): Promise { +export async function getWorkflowData( + workflowInfo: IExecuteWorkflowInfo, + user: N8nUserData, +): Promise { if (workflowInfo.id === undefined && workflowInfo.code === undefined) { throw new Error( `No information about the workflow to execute found. Please provide either the "id" or "code"!`, ); } - if (Db.collections.Workflow === null) { - // The first time executeWorkflow gets called the Database has - // to get initialized first - await Db.init(); - } - let workflowData: IWorkflowBase | undefined; if (workflowInfo.id !== undefined) { - workflowData = await Db.collections.Workflow!.findOne(workflowInfo.id); + if (Db.collections.Workflow === null) { + // The first time executeWorkflow gets called the Database has + // to get initialized first + await Db.init(); + } + const qb = Db.collections.Workflow!.createQueryBuilder('w'); + if (user.globalRole.name !== 'owner') { + qb.innerJoin('w.shared', 'shared'); + qb.andWhere('shared.user = :userId', { userId: user.id }); + } + qb.andWhere('w.id = :id', { id: workflowInfo.id }); + + workflowData = await qb.getOne(); + if (workflowData === undefined) { throw new Error(`The workflow with the id "${workflowInfo.id}" does not exist.`); } @@ -805,7 +818,9 @@ export async function executeWorkflow( const nodeTypes = NodeTypes(); const workflowData = - loadedWorkflowData !== undefined ? loadedWorkflowData : await getWorkflowData(workflowInfo); + loadedWorkflowData !== undefined + ? loadedWorkflowData + : await getWorkflowData(workflowInfo, additionalData.user); const workflowName = workflowData ? workflowData.name : undefined; const workflow = new Workflow({ @@ -819,7 +834,9 @@ export async function executeWorkflow( }); const runData = - loadedRunData !== undefined ? loadedRunData : await getRunData(workflowData, inputData); + loadedRunData !== undefined + ? loadedRunData + : await getRunData(workflowData, additionalData.user, inputData); let executionId; @@ -836,7 +853,7 @@ export async function executeWorkflow( try { // Create new additionalData to have different workflow loaded and to call // different webooks - const additionalDataIntegrated = await getBase(); + const additionalDataIntegrated = await getBase(additionalData.user); additionalDataIntegrated.hooks = getWorkflowHooksIntegrated( runData.executionMode, executionId, @@ -969,6 +986,7 @@ export function sendMessageToUI(source: string, messages: any[]) { * @returns {Promise} */ export async function getBase( + user: N8nUserData, currentNodeParameters?: INodeParameters, executionTimeoutTimestamp?: number, ): Promise { @@ -995,6 +1013,7 @@ export async function getBase( webhookTestBaseUrl, currentNodeParameters, executionTimeoutTimestamp, + user, }; } diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index 47f3ba596e4db..832a4884edced 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -41,6 +41,7 @@ import * as config from '../config'; // eslint-disable-next-line import/no-cycle import { WorkflowEntity } from './databases/entities/WorkflowEntity'; import { User } from './databases/entities/User'; +import { getWorkflowOwner } from './UserManagement/UserManagementHelper'; const ERROR_TRIGGER_TYPE = config.get('nodes.errorTriggerType') as string; @@ -111,6 +112,8 @@ export async function executeErrorWorkflow( return; } + const user = await getWorkflowOwner(workflowId); + const executionMode = 'error'; const nodeTypes = NodeTypes(); @@ -174,6 +177,7 @@ export async function executeErrorWorkflow( executionMode, executionData: runExecutionData, workflowData, + user, }; const workflowRunner = new WorkflowRunner(); diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index 955532ae6aa05..3d95fb8622b43 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -246,13 +246,10 @@ export class WorkflowRunner { staticData: data.workflowData.staticData, }); const additionalData = await WorkflowExecuteAdditionalData.getBase( + data.user, undefined, workflowTimeout <= 0 ? undefined : Date.now() + workflowTimeout * 1000, ); - if (data.userId) { - // @ts-ignore - additionalData.userId = data.userId; - } // Register the active execution const executionId = await this.activeExecutions.add(data, undefined, restartExecutionId); diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index 69b3e8f8a6708..38a0e45d477c4 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -86,6 +86,7 @@ export class WorkflowRunnerProcess { LoggerProxy.init(logger); this.data = inputData; + const { user } = inputData; logger.verbose('Initializing n8n sub-process', { pid: process.pid, @@ -209,13 +210,10 @@ export class WorkflowRunnerProcess { settings: this.data.workflowData.settings, }); const additionalData = await WorkflowExecuteAdditionalData.getBase( + user, undefined, workflowTimeout <= 0 ? undefined : Date.now() + workflowTimeout * 1000, ); - if (this.data.userId) { - // @ts-ignore - additionalData.userId = this.data.userId; - } additionalData.hooks = this.getProcessForwardHooks(); additionalData.hooks.hookFunctions.sendResponse = [ @@ -250,8 +248,12 @@ export class WorkflowRunnerProcess { additionalData: IWorkflowExecuteAdditionalData, inputData?: INodeExecutionData[] | undefined, ): Promise | IRun> => { - const workflowData = await WorkflowExecuteAdditionalData.getWorkflowData(workflowInfo); - const runData = await WorkflowExecuteAdditionalData.getRunData(workflowData, inputData); + const workflowData = await WorkflowExecuteAdditionalData.getWorkflowData(workflowInfo, user); + const runData = await WorkflowExecuteAdditionalData.getRunData( + workflowData, + additionalData.user, + inputData, + ); await sendToParentProcess('startExecution', { runData }); const executionId: string = await new Promise((resolve) => { this.executionIdCallback = (executionId: string) => { diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 2b181fc48d69e..182485e95fe4b 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -944,6 +944,7 @@ export async function requestOAuth2( nodeCredentials, credentialsType, credentials, + additionalData.user, ); Logger.debug( @@ -1174,9 +1175,9 @@ export async function getCredentials( nodeCredentials, type, mode, + additionalData.user, false, expressionResolveValues, - additionalData.userId, ); return decryptedDataObject; diff --git a/packages/core/test/Helpers.ts b/packages/core/test/Helpers.ts index 7335eb0d5f2eb..ed6e0dc8d7abe 100644 --- a/packages/core/test/Helpers.ts +++ b/packages/core/test/Helpers.ts @@ -783,5 +783,12 @@ export function WorkflowExecuteAdditionalData( webhookBaseUrl: 'webhook', webhookWaitingBaseUrl: 'webhook-waiting', webhookTestBaseUrl: 'webhook-test', + user: { + id: '123', + globalRole: { + name: 'owner', + scope: 'global', + } + } }; } diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index bfafee78091d3..c6ebdc031ac28 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -138,21 +138,23 @@ export abstract class ICredentialsHelper { abstract getCredentials( nodeCredentials: INodeCredentialsDetails, type: string, + user: N8nUserData, ): Promise; abstract getDecrypted( nodeCredentials: INodeCredentialsDetails, type: string, mode: WorkflowExecuteMode, + user: N8nUserData, raw?: boolean, expressionResolveValues?: ICredentialsExpressionResolveValues, - userId?: string, ): Promise; abstract updateCredentials( nodeCredentials: INodeCredentialsDetails, type: string, data: ICredentialDataDecryptedObject, + user: N8nUserData, ): Promise; } @@ -1017,7 +1019,7 @@ export interface IWorkflowExecuteAdditionalData { webhookTestBaseUrl: string; currentNodeParameters?: INodeParameters; executionTimeoutTimestamp?: number; - userId?: string; // this is a UUID - used to firewall actions in user management + user: N8nUserData; } export type WorkflowExecuteMode = @@ -1121,3 +1123,16 @@ export interface ITelemetrySettings { enabled: boolean; config?: ITelemetryClientConfig; } + +export interface N8nUserData { + id: string; + email?: string; + firstName?: string; + lastName?: string; + globalRole: N8nRoleData; +} + +export interface N8nRoleData { + name: string; + scope: string; +} From d5d91e99a97c19f7bcc44cc302ea6ae0894fa995 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Mon, 31 Jan 2022 11:40:24 +0100 Subject: [PATCH 2/9] Fixed from reviewers --- packages/cli/commands/execute.ts | 4 ++-- packages/cli/commands/executeBatch.ts | 4 ++-- .../cli/src/UserManagement/UserManagementHelper.ts | 11 ++++++----- packages/cli/src/WebhookHelpers.ts | 2 +- packages/cli/src/WorkflowExecuteAdditionalData.ts | 9 ++------- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/packages/cli/commands/execute.ts b/packages/cli/commands/execute.ts index e5f3443cf55e3..cedd9a008175d 100644 --- a/packages/cli/commands/execute.ts +++ b/packages/cli/commands/execute.ts @@ -28,7 +28,7 @@ import { import { getLogger } from '../src/Logger'; import config = require('../config'); -import { getInstanceowner } from '../src/UserManagement/UserManagementHelper'; +import { getInstanceOwner } from '../src/UserManagement/UserManagementHelper'; export class Execute extends Command { static description = '\nExecutes a given workflow'; @@ -174,7 +174,7 @@ export class Execute extends Command { startNodes: [startNode.name], // eslint-disable-next-line @typescript-eslint/no-non-null-assertion workflowData: workflowData!, - user: await getInstanceowner(), + user: await getInstanceOwner(), }; const workflowRunner = new WorkflowRunner(); diff --git a/packages/cli/commands/executeBatch.ts b/packages/cli/commands/executeBatch.ts index c14a9fcd8a53d..a82dcaa687b38 100644 --- a/packages/cli/commands/executeBatch.ts +++ b/packages/cli/commands/executeBatch.ts @@ -38,7 +38,7 @@ import { } from '../src'; import config = require('../config'); import { User } from '../src/databases/entities/User'; -import { getInstanceowner } from '../src/UserManagement/UserManagementHelper'; +import { getInstanceOwner } from '../src/UserManagement/UserManagementHelper'; export class ExecuteBatch extends Command { static description = '\nExecutes multiple workflows once'; @@ -283,7 +283,7 @@ export class ExecuteBatch extends Command { // Wait till the database is ready await startDbInitPromise; - ExecuteBatch.instanceOwner = await getInstanceowner(); + ExecuteBatch.instanceOwner = await getInstanceOwner(); let allWorkflows; diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 45834898af98e..20e11d3ec797f 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -45,15 +45,16 @@ export async function saveCredentialOwnership( })) as SharedCredentials; } -export async function getWorkflowOwner(workflowId: string) { - const workflowDb = await Db.collections.Workflow!.findOneOrFail(workflowId, { - relations: ['shared', 'shared.user', 'shared.user.globalRole'], +export async function getWorkflowOwner(workflowId: string | number) { + const sharedWorkflow = await Db.collections.SharedWorkflow!.findOneOrFail({ + where: { workflow: { id: workflowId } }, + relations: ['user', 'user.globalRole'], }); - return workflowDb.shared[0].user; + return sharedWorkflow.user; } -export async function getInstanceowner() { +export async function getInstanceOwner() { const qb = Db.collections.User!.createQueryBuilder('u'); qb.innerJoin('u.globalRole', 'gr'); qb.andWhere('gr.name = :name and gr.scope = :scope', { name: 'owner', scope: 'global' }); diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index 88e1546d8b790..0b30ad318b0e2 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -227,7 +227,7 @@ export async function executeWebhook( try { user = await getWorkflowOwner(workflowData.id.toString()); } catch (error) { - throw new ResponseHelper.ResponseError('Cannot find workflow', 500, 500); + throw new ResponseHelper.ResponseError('Cannot find workflow', undefined, 404); } } diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index cbc2c39e7fa89..295d62e26889d 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -818,9 +818,7 @@ export async function executeWorkflow( const nodeTypes = NodeTypes(); const workflowData = - loadedWorkflowData !== undefined - ? loadedWorkflowData - : await getWorkflowData(workflowInfo, additionalData.user); + loadedWorkflowData ?? (await getWorkflowData(workflowInfo, additionalData.user)); const workflowName = workflowData ? workflowData.name : undefined; const workflow = new Workflow({ @@ -833,10 +831,7 @@ export async function executeWorkflow( staticData: workflowData.staticData, }); - const runData = - loadedRunData !== undefined - ? loadedRunData - : await getRunData(workflowData, additionalData.user, inputData); + const runData = loadedRunData ?? (await getRunData(workflowData, additionalData.user, inputData)); let executionId; From 1bd9f70da0bfd519323b0f59841bab09bf46163a Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Thu, 3 Feb 2022 14:53:48 +0100 Subject: [PATCH 3/9] Changed runtime validation for credentials to be on start instead of on demand --- packages/cli/commands/worker.ts | 7 +- packages/cli/src/CredentialsHelper.ts | 18 +---- packages/cli/src/Server.ts | 4 -- .../UserManagement/UserManagementHelper.ts | 65 ++++++++++++++++++- .../cli/src/WorkflowExecuteAdditionalData.ts | 4 ++ packages/cli/src/WorkflowRunner.ts | 4 ++ packages/cli/src/WorkflowRunnerProcess.ts | 4 ++ packages/core/src/NodeExecuteFunctions.ts | 2 - packages/workflow/src/Interfaces.ts | 3 - 9 files changed, 85 insertions(+), 26 deletions(-) diff --git a/packages/cli/commands/worker.ts b/packages/cli/commands/worker.ts index 0c4159ae7e06a..e8a55ad65fcba 100644 --- a/packages/cli/commands/worker.ts +++ b/packages/cli/commands/worker.ts @@ -39,7 +39,10 @@ import { getLogger } from '../src/Logger'; import * as config from '../config'; import * as Queue from '../src/Queue'; -import { getWorkflowOwner } from '../src/UserManagement/UserManagementHelper'; +import { + checkPermissionsForExecution, + getWorkflowOwner, +} from '../src/UserManagement/UserManagementHelper'; export class Worker extends Command { static description = '\nStarts a n8n worker'; @@ -167,6 +170,8 @@ export class Worker extends Command { settings: currentExecutionDb.workflowData.settings, }); + await checkPermissionsForExecution(workflow, workflowOwner); + const additionalData = await WorkflowExecuteAdditionalData.getBase( workflowOwner, undefined, diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts index 1c2b892787604..1abf772617b66 100644 --- a/packages/cli/src/CredentialsHelper.ts +++ b/packages/cli/src/CredentialsHelper.ts @@ -13,7 +13,6 @@ import { INodeType, INodeTypeData, INodeTypes, - N8nUserData, NodeHelpers, Workflow, WorkflowExecuteMode, @@ -51,21 +50,12 @@ export class CredentialsHelper extends ICredentialsHelper { async getCredentials( nodeCredentials: INodeCredentialsDetails, type: string, - user: N8nUserData, ): Promise { if (!nodeCredentials.id) { throw new Error(`Credentials "${nodeCredentials.name}" for type "${type}" don't have an ID.`); } - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - const qb = Db.collections.Credentials!.createQueryBuilder('c'); - qb.where('c.id = :id and c.type = :type', { id: nodeCredentials.id, type }); - if (user.globalRole.name !== 'owner') { - qb.innerJoin('c.shared', 'shared'); - // @ts-ignore - qb.andWhere('shared.userId = :userId', { userId: user.id }); - } - const credentials = await qb.getOne(); + const credentials = await Db.collections.Credentials!.findOne(nodeCredentials.id); if (!credentials) { throw new Error( @@ -126,11 +116,10 @@ export class CredentialsHelper extends ICredentialsHelper { nodeCredentials: INodeCredentialsDetails, type: string, mode: WorkflowExecuteMode, - user: N8nUserData, raw?: boolean, expressionResolveValues?: ICredentialsExpressionResolveValues, ): Promise { - const credentials = await this.getCredentials(nodeCredentials, type, user); + const credentials = await this.getCredentials(nodeCredentials, type); const decryptedDataOriginal = credentials.getData(this.encryptionKey); if (raw === true) { @@ -246,10 +235,9 @@ export class CredentialsHelper extends ICredentialsHelper { nodeCredentials: INodeCredentialsDetails, type: string, data: ICredentialDataDecryptedObject, - user: N8nUserData, ): Promise { // eslint-disable-next-line @typescript-eslint/await-thenable - const credentials = await this.getCredentials(nodeCredentials, type, user); + const credentials = await this.getCredentials(nodeCredentials, type); if (Db.collections.Credentials === null) { // The first time executeWorkflow gets called the Database has diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 0631b4837203d..b08b413d96c9a 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -2009,7 +2009,6 @@ class App { credential as INodeCredentialsDetails, credential.type, mode, - req.user, true, ); @@ -2133,7 +2132,6 @@ class App { credential as INodeCredentialsDetails, credential.type, mode, - req.user as User, true, ); const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( @@ -2234,7 +2232,6 @@ class App { credential as INodeCredentialsDetails, credential.type, mode, - req.user as User, true, ); const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( @@ -2367,7 +2364,6 @@ class App { credential as INodeCredentialsDetails, credential.type, mode, - req.user as User, true, ); const oauthCredentials = credentialsHelper.applyDefaultsAndOverwrites( diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 20e11d3ec797f..47fdbdb9b0c27 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -4,7 +4,8 @@ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ /* eslint-disable @typescript-eslint/no-non-null-assertion */ /* eslint-disable import/no-cycle */ -import { IsNull, Not } from 'typeorm'; +import { N8nUserData, Workflow } from 'n8n-workflow'; +import { In, IsNull, Not } from 'typeorm'; import { Db, ResponseHelper } from '..'; import config = require('../../config'); import { CredentialsEntity } from '../databases/entities/CredentialsEntity'; @@ -102,3 +103,65 @@ export function sanitizeUser(user: User): PublicUser { const { password, resetPasswordToken, createdAt, updatedAt, ...sanitizedUser } = user; return sanitizedUser; } + +async function getCredentialIdByName(name: string): Promise { + const credential = await Db.collections.Credentials!.findOne({ + where: { + name, + }, + }); + if (credential) { + return credential.id; + } + return null; +} + +export async function checkPermissionsForExecution( + workflow: Workflow, + user: N8nUserData, +): Promise { + const credentialIds = new Set(); + const nodeNames = Object.keys(workflow.nodes); + const pendingPromises = [] as Array>; + nodeNames.forEach((nodeName) => { + const node = workflow.nodes[nodeName]; + if (node.credentials) { + const credentialNames = Object.keys(node.credentials); + credentialNames.forEach((credentialName) => { + const credentialDetail = node.credentials![credentialName]; + if (credentialDetail.id) { + credentialIds.add(credentialDetail.id.toString()); + } else { + pendingPromises.push(getCredentialIdByName(credentialDetail.name)); + } + }); + } + }); + + const fullfilledPromises = await Promise.all(pendingPromises); + fullfilledPromises.forEach((credentialId) => { + if (credentialId !== null) { + credentialIds.add(credentialId.toString()); + } else { + throw new Error('One or more of the required credentials was not found in the database.'); + } + }); + // We converted all IDs to string so that the set cannot contain duplicates. + + const ids = Array.from(credentialIds); + + if (ids.length === 0) { + return; + } + + const credentialCount = await Db.collections.SharedCredentials!.count({ + where: { + user, + credentials: In(ids), + }, + }); + + if (ids.length !== credentialCount) { + throw new Error('One or more of the required credentials was not found in the database.'); + } +} diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 295d62e26889d..3ef9ed19e0d7b 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -61,6 +61,8 @@ import { WorkflowCredentials, WorkflowHelpers, } from '.'; +// eslint-disable-next-line import/no-cycle +import { checkPermissionsForExecution } from './UserManagement/UserManagementHelper'; const ERROR_TRIGGER_TYPE = config.get('nodes.errorTriggerType') as string; @@ -846,6 +848,8 @@ export async function executeWorkflow( let data; try { + await checkPermissionsForExecution(workflow, additionalData.user); + // Create new additionalData to have different workflow loaded and to call // different webooks const additionalDataIntegrated = await getBase(additionalData.user); diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index 3d95fb8622b43..c132e80686aa4 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -57,6 +57,7 @@ import { } from '.'; import * as Queue from './Queue'; import { InternalHooksManager } from './InternalHooksManager'; +import { checkPermissionsForExecution } from './UserManagement/UserManagementHelper'; export class WorkflowRunner { activeExecutions: ActiveExecutions.ActiveExecutions; @@ -266,6 +267,9 @@ export class WorkflowRunner { `Execution for workflow ${data.workflowData.name} was assigned id ${executionId}`, { executionId }, ); + + await checkPermissionsForExecution(workflow, data.user); + additionalData.hooks = WorkflowExecuteAdditionalData.getWorkflowHooksMain( data, executionId, diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index 38a0e45d477c4..ef0921263489b 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -50,6 +50,7 @@ import { getLogger } from './Logger'; import * as config from '../config'; import { InternalHooksManager } from './InternalHooksManager'; +import { checkPermissionsForExecution } from './UserManagement/UserManagementHelper'; export class WorkflowRunnerProcess { data: IWorkflowExecutionDataProcessWithExecution | undefined; @@ -209,6 +210,9 @@ export class WorkflowRunnerProcess { staticData: this.data.workflowData.staticData, settings: this.data.workflowData.settings, }); + + await checkPermissionsForExecution(this.workflow, user); + const additionalData = await WorkflowExecuteAdditionalData.getBase( user, undefined, diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 182485e95fe4b..071f61ab5a597 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -944,7 +944,6 @@ export async function requestOAuth2( nodeCredentials, credentialsType, credentials, - additionalData.user, ); Logger.debug( @@ -1175,7 +1174,6 @@ export async function getCredentials( nodeCredentials, type, mode, - additionalData.user, false, expressionResolveValues, ); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index c6ebdc031ac28..947fdec9cb729 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -138,14 +138,12 @@ export abstract class ICredentialsHelper { abstract getCredentials( nodeCredentials: INodeCredentialsDetails, type: string, - user: N8nUserData, ): Promise; abstract getDecrypted( nodeCredentials: INodeCredentialsDetails, type: string, mode: WorkflowExecuteMode, - user: N8nUserData, raw?: boolean, expressionResolveValues?: ICredentialsExpressionResolveValues, ): Promise; @@ -154,7 +152,6 @@ export abstract class ICredentialsHelper { nodeCredentials: INodeCredentialsDetails, type: string, data: ICredentialDataDecryptedObject, - user: N8nUserData, ): Promise; } From 3fcb3fb2c41e885ed870e49f697dc7fda1547f3a Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Wed, 9 Feb 2022 18:29:47 +0100 Subject: [PATCH 4/9] Refactored validations to use user id instead of whole User instance --- packages/cli/commands/execute.ts | 3 +- packages/cli/commands/executeBatch.ts | 2 +- packages/cli/commands/worker.ts | 4 +- packages/cli/src/ActiveWorkflowRunner.ts | 12 ++-- packages/cli/src/Interfaces.ts | 4 +- packages/cli/src/Server.ts | 16 ++--- .../UserManagement/UserManagementHelper.ts | 12 +++- packages/cli/src/WaitTracker.ts | 2 +- packages/cli/src/WaitingWebhooks.ts | 2 +- packages/cli/src/WebhookHelpers.ts | 4 +- .../cli/src/WorkflowExecuteAdditionalData.ts | 59 +++++++++++++------ packages/cli/src/WorkflowHelpers.ts | 10 +++- packages/cli/src/WorkflowRunner.ts | 4 +- packages/cli/src/WorkflowRunnerProcess.ts | 15 ++--- packages/core/test/Helpers.ts | 8 +-- packages/workflow/src/Interfaces.ts | 2 +- 16 files changed, 98 insertions(+), 61 deletions(-) diff --git a/packages/cli/commands/execute.ts b/packages/cli/commands/execute.ts index cedd9a008175d..3f13afac4b4c4 100644 --- a/packages/cli/commands/execute.ts +++ b/packages/cli/commands/execute.ts @@ -169,12 +169,13 @@ export class Execute extends Command { } try { + const user = await getInstanceOwner(); const runData: IWorkflowExecutionDataProcess = { executionMode: 'cli', startNodes: [startNode.name], // eslint-disable-next-line @typescript-eslint/no-non-null-assertion workflowData: workflowData!, - user: await getInstanceOwner(), + userId: user.id, }; const workflowRunner = new WorkflowRunner(); diff --git a/packages/cli/commands/executeBatch.ts b/packages/cli/commands/executeBatch.ts index a82dcaa687b38..a1d5992b380ba 100644 --- a/packages/cli/commands/executeBatch.ts +++ b/packages/cli/commands/executeBatch.ts @@ -672,7 +672,7 @@ export class ExecuteBatch extends Command { executionMode: 'cli', startNodes: [startNode!.name], workflowData, - user: ExecuteBatch.instanceOwner, + userId: ExecuteBatch.instanceOwner.id, }; const workflowRunner = new WorkflowRunner(); diff --git a/packages/cli/commands/worker.ts b/packages/cli/commands/worker.ts index e8a55ad65fcba..d14f2139bc436 100644 --- a/packages/cli/commands/worker.ts +++ b/packages/cli/commands/worker.ts @@ -170,10 +170,10 @@ export class Worker extends Command { settings: currentExecutionDb.workflowData.settings, }); - await checkPermissionsForExecution(workflow, workflowOwner); + await checkPermissionsForExecution(workflow, workflowOwner.id); const additionalData = await WorkflowExecuteAdditionalData.getBase( - workflowOwner, + workflowOwner.id, undefined, executionTimeoutTimestamp, ); diff --git a/packages/cli/src/ActiveWorkflowRunner.ts b/packages/cli/src/ActiveWorkflowRunner.ts index 961c4fadb512f..4b575b16c0202 100644 --- a/packages/cli/src/ActiveWorkflowRunner.ts +++ b/packages/cli/src/ActiveWorkflowRunner.ts @@ -279,7 +279,9 @@ export class ActiveWorkflowRunner { settings: workflowData.settings, }); - const additionalData = await WorkflowExecuteAdditionalData.getBase(workflowData.shared[0].user); + const additionalData = await WorkflowExecuteAdditionalData.getBase( + workflowData.shared[0].user.id, + ); const webhookData = NodeHelpers.getNodeWebhooks( workflow, @@ -544,7 +546,9 @@ export class ActiveWorkflowRunner { const mode = 'internal'; - const additionalData = await WorkflowExecuteAdditionalData.getBase(workflowData.shared[0].user); + const additionalData = await WorkflowExecuteAdditionalData.getBase( + workflowData.shared[0].user.id, + ); const webhooks = WebhookHelpers.getWorkflowWebhooks(workflow, additionalData, undefined, true); @@ -611,7 +615,7 @@ export class ActiveWorkflowRunner { // Start the workflow const runData: IWorkflowExecutionDataProcess = { - user: additionalData.user, + userId: additionalData.userId, executionMode: mode, executionData, workflowData, @@ -747,7 +751,7 @@ export class ActiveWorkflowRunner { const mode = 'trigger'; const additionalData = await WorkflowExecuteAdditionalData.getBase( - (workflowData as WorkflowEntity).shared[0].user, + (workflowData as WorkflowEntity).shared[0].user.id, ); const getTriggerFunctions = this.getExecuteTriggerFunctions( workflowData, diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 090c768403c53..686680e639cb4 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -576,7 +576,7 @@ export interface IWorkflowExecutionDataProcess { sessionId?: string; startNodes?: string[]; workflowData: IWorkflowBase; - user: N8nUserData; + userId: string; } export interface IWorkflowExecutionDataProcessWithExecution extends IWorkflowExecutionDataProcess { @@ -584,7 +584,7 @@ export interface IWorkflowExecutionDataProcessWithExecution extends IWorkflowExe credentialsTypeData: ICredentialsTypeData; executionId: string; nodeTypeData: ITransferNodeTypes; - user: User; + userId: string; } export interface IWorkflowExecuteProcess { diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index b08b413d96c9a..717e9d5f7ad9a 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -1093,13 +1093,9 @@ class App { startNodes.length === 0 || destinationNode === undefined ) { - const additionalData = await WorkflowExecuteAdditionalData.getBase(req.user as User); - if (this.isUserManagementEnabled) { - // TODO UM: test this. - // TODO: test this. - // @ts-ignore - additionalData.userId = req.body.userId; - } + const additionalData = await WorkflowExecuteAdditionalData.getBase( + (req.user as User).id, + ); const nodeTypes = NodeTypes(); const workflowInstance = new Workflow({ id: workflowData.id, @@ -1138,7 +1134,7 @@ class App { sessionId, startNodes, workflowData, - user: req.user as User, + userId: (req.user as User).id, }; const workflowRunner = new WorkflowRunner(); const executionId = await workflowRunner.run(data); @@ -1277,7 +1273,7 @@ class App { ); const additionalData = await WorkflowExecuteAdditionalData.getBase( - req.user as User, + (req.user as User).id, currentNodeParameters, ); @@ -2645,7 +2641,7 @@ class App { executionData: fullExecutionData.data, retryOf: req.params.id, workflowData: fullExecutionData.workflowData, - user: req.user as User, + userId: (req.user as User).id, }; const { lastNodeExecuted } = data.executionData!.resultData; diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 47fdbdb9b0c27..f8c5955c9dcbd 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -116,9 +116,17 @@ async function getCredentialIdByName(name: string): Promise { const credentialIds = new Set(); const nodeNames = Object.keys(workflow.nodes); @@ -156,7 +164,7 @@ export async function checkPermissionsForExecution( const credentialCount = await Db.collections.SharedCredentials!.count({ where: { - user, + user: { id: userId }, credentials: In(ids), }, }); diff --git a/packages/cli/src/WaitTracker.ts b/packages/cli/src/WaitTracker.ts index c50a66c0bc7d1..1534ac325bc27 100644 --- a/packages/cli/src/WaitTracker.ts +++ b/packages/cli/src/WaitTracker.ts @@ -167,7 +167,7 @@ export class WaitTrackerClass { executionMode: fullExecutionData.mode, executionData: fullExecutionData.data, workflowData: fullExecutionData.workflowData, - user, + userId: user.id, }; // Start the execution again diff --git a/packages/cli/src/WaitingWebhooks.ts b/packages/cli/src/WaitingWebhooks.ts index 0373e40c5eb97..e82466dfa8be9 100644 --- a/packages/cli/src/WaitingWebhooks.ts +++ b/packages/cli/src/WaitingWebhooks.ts @@ -119,7 +119,7 @@ export class WaitingWebhooks { throw new ResponseHelper.ResponseError('Could not find workflow', 404, 404); } - const additionalData = await WorkflowExecuteAdditionalData.getBase(workflowOwner); + const additionalData = await WorkflowExecuteAdditionalData.getBase(workflowOwner.id); const webhookData = NodeHelpers.getNodeWebhooks( workflow, diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index 0b30ad318b0e2..990cff2d55009 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -232,7 +232,7 @@ export async function executeWebhook( } // Prepare everything that is needed to run the workflow - const additionalData = await WorkflowExecuteAdditionalData.getBase(user); + const additionalData = await WorkflowExecuteAdditionalData.getBase(user.id); // Add the Response and Request so that this data can be accessed in the node additionalData.httpRequest = req; @@ -407,7 +407,7 @@ export async function executeWebhook( executionData: runExecutionData, sessionId, workflowData, - user, + userId: user.id, }; let responsePromise: IDeferredPromise | undefined; diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 3ef9ed19e0d7b..5205d5a9e11af 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -62,7 +62,11 @@ import { WorkflowHelpers, } from '.'; // eslint-disable-next-line import/no-cycle -import { checkPermissionsForExecution } from './UserManagement/UserManagementHelper'; +import { + checkPermissionsForExecution, + getUserById, + getWorkflowOwner, +} from './UserManagement/UserManagementHelper'; const ERROR_TRIGGER_TYPE = config.get('nodes.errorTriggerType') as string; @@ -123,20 +127,36 @@ function executeErrorWorkflow( workflowId: workflowData.id, }); // If a specific error workflow is set run only that one + + // First, do permission checks. + if (!workflowData.id) { + // Manual executions do not trigger error workflows + // So this if should never happen. It was added to + // make sure there are no possible security gaps + return; + } + // eslint-disable-next-line @typescript-eslint/no-floating-promises - WorkflowHelpers.executeErrorWorkflow( - workflowData.settings.errorWorkflow as string, - workflowErrorData, - ); + getWorkflowOwner(workflowData.id).then((user) => { + void WorkflowHelpers.executeErrorWorkflow( + workflowData.settings!.errorWorkflow as string, + workflowErrorData, + user, + ); + }); } else if ( mode !== 'error' && workflowData.id !== undefined && workflowData.nodes.some((node) => node.type === ERROR_TRIGGER_TYPE) ) { Logger.verbose(`Start internal error workflow`, { executionId, workflowId: workflowData.id }); - // If the workflow contains - // eslint-disable-next-line @typescript-eslint/no-floating-promises - WorkflowHelpers.executeErrorWorkflow(workflowData.id.toString(), workflowErrorData); + void getWorkflowOwner(workflowData.id).then((user) => { + void WorkflowHelpers.executeErrorWorkflow( + workflowData.id!.toString(), + workflowErrorData, + user, + ); + }); } } } @@ -701,7 +721,7 @@ function hookFunctionsSaveWorker(): IWorkflowExecuteHooks { export async function getRunData( workflowData: IWorkflowBase, - user: N8nUserData, + userId: string, inputData?: INodeExecutionData[], ): Promise { const mode = 'integrated'; @@ -755,7 +775,7 @@ export async function getRunData( executionData: runExecutionData, // @ts-ignore workflowData, - user, + userId, }; return runData; @@ -763,7 +783,7 @@ export async function getRunData( export async function getWorkflowData( workflowInfo: IExecuteWorkflowInfo, - user: N8nUserData, + userId: string, ): Promise { if (workflowInfo.id === undefined && workflowInfo.code === undefined) { throw new Error( @@ -778,6 +798,7 @@ export async function getWorkflowData( // to get initialized first await Db.init(); } + const user = await getUserById(userId); const qb = Db.collections.Workflow!.createQueryBuilder('w'); if (user.globalRole.name !== 'owner') { qb.innerJoin('w.shared', 'shared'); @@ -820,7 +841,7 @@ export async function executeWorkflow( const nodeTypes = NodeTypes(); const workflowData = - loadedWorkflowData ?? (await getWorkflowData(workflowInfo, additionalData.user)); + loadedWorkflowData ?? (await getWorkflowData(workflowInfo, additionalData.userId)); const workflowName = workflowData ? workflowData.name : undefined; const workflow = new Workflow({ @@ -833,7 +854,8 @@ export async function executeWorkflow( staticData: workflowData.staticData, }); - const runData = loadedRunData ?? (await getRunData(workflowData, additionalData.user, inputData)); + const runData = + loadedRunData ?? (await getRunData(workflowData, additionalData.userId, inputData)); let executionId; @@ -848,11 +870,11 @@ export async function executeWorkflow( let data; try { - await checkPermissionsForExecution(workflow, additionalData.user); + await checkPermissionsForExecution(workflow, additionalData.userId); // Create new additionalData to have different workflow loaded and to call // different webooks - const additionalDataIntegrated = await getBase(additionalData.user); + const additionalDataIntegrated = await getBase(additionalData.userId); additionalDataIntegrated.hooks = getWorkflowHooksIntegrated( runData.executionMode, executionId, @@ -924,6 +946,9 @@ export async function executeWorkflow( stoppedAt: fullRunData.stoppedAt, workflowData, }; + if (workflowData.id) { + fullExecutionData.workflowId = workflowData.id as string; + } const executionData = ResponseHelper.flattenExecutionData(fullExecutionData); @@ -985,7 +1010,7 @@ export function sendMessageToUI(source: string, messages: any[]) { * @returns {Promise} */ export async function getBase( - user: N8nUserData, + userId: string, currentNodeParameters?: INodeParameters, executionTimeoutTimestamp?: number, ): Promise { @@ -1012,7 +1037,7 @@ export async function getBase( webhookTestBaseUrl, currentNodeParameters, executionTimeoutTimestamp, - user, + userId, }; } diff --git a/packages/cli/src/WorkflowHelpers.ts b/packages/cli/src/WorkflowHelpers.ts index 832a4884edced..defe7e63b4a87 100644 --- a/packages/cli/src/WorkflowHelpers.ts +++ b/packages/cli/src/WorkflowHelpers.ts @@ -97,6 +97,7 @@ export function isWorkflowIdValid(id: string | null | undefined | number): boole export async function 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 { @@ -113,6 +114,13 @@ export async function executeErrorWorkflow( } const user = await getWorkflowOwner(workflowId); + if (user.id !== runningUser.id) { + // The error workflow could not be found + Logger.warn( + `An attempt to execute workflow ID ${workflowId} as error workflow was blocked due to wrong permission`, + ); + return; + } const executionMode = 'error'; const nodeTypes = NodeTypes(); @@ -177,7 +185,7 @@ export async function executeErrorWorkflow( executionMode, executionData: runExecutionData, workflowData, - user, + userId: user.id, }; const workflowRunner = new WorkflowRunner(); diff --git a/packages/cli/src/WorkflowRunner.ts b/packages/cli/src/WorkflowRunner.ts index c132e80686aa4..7b3804afeeb40 100644 --- a/packages/cli/src/WorkflowRunner.ts +++ b/packages/cli/src/WorkflowRunner.ts @@ -247,7 +247,7 @@ export class WorkflowRunner { staticData: data.workflowData.staticData, }); const additionalData = await WorkflowExecuteAdditionalData.getBase( - data.user, + data.userId, undefined, workflowTimeout <= 0 ? undefined : Date.now() + workflowTimeout * 1000, ); @@ -268,7 +268,7 @@ export class WorkflowRunner { { executionId }, ); - await checkPermissionsForExecution(workflow, data.user); + await checkPermissionsForExecution(workflow, data.userId); additionalData.hooks = WorkflowExecuteAdditionalData.getWorkflowHooksMain( data, diff --git a/packages/cli/src/WorkflowRunnerProcess.ts b/packages/cli/src/WorkflowRunnerProcess.ts index ef0921263489b..389b4efa99afd 100644 --- a/packages/cli/src/WorkflowRunnerProcess.ts +++ b/packages/cli/src/WorkflowRunnerProcess.ts @@ -87,7 +87,7 @@ export class WorkflowRunnerProcess { LoggerProxy.init(logger); this.data = inputData; - const { user } = inputData; + const { userId } = inputData; logger.verbose('Initializing n8n sub-process', { pid: process.pid, @@ -210,11 +210,9 @@ export class WorkflowRunnerProcess { staticData: this.data.workflowData.staticData, settings: this.data.workflowData.settings, }); - - await checkPermissionsForExecution(this.workflow, user); - + await checkPermissionsForExecution(this.workflow, userId); const additionalData = await WorkflowExecuteAdditionalData.getBase( - user, + userId, undefined, workflowTimeout <= 0 ? undefined : Date.now() + workflowTimeout * 1000, ); @@ -252,10 +250,13 @@ export class WorkflowRunnerProcess { additionalData: IWorkflowExecuteAdditionalData, inputData?: INodeExecutionData[] | undefined, ): Promise | IRun> => { - const workflowData = await WorkflowExecuteAdditionalData.getWorkflowData(workflowInfo, user); + const workflowData = await WorkflowExecuteAdditionalData.getWorkflowData( + workflowInfo, + userId, + ); const runData = await WorkflowExecuteAdditionalData.getRunData( workflowData, - additionalData.user, + additionalData.userId, inputData, ); await sendToParentProcess('startExecution', { runData }); diff --git a/packages/core/test/Helpers.ts b/packages/core/test/Helpers.ts index ed6e0dc8d7abe..ba8ee82aef3dd 100644 --- a/packages/core/test/Helpers.ts +++ b/packages/core/test/Helpers.ts @@ -783,12 +783,6 @@ export function WorkflowExecuteAdditionalData( webhookBaseUrl: 'webhook', webhookWaitingBaseUrl: 'webhook-waiting', webhookTestBaseUrl: 'webhook-test', - user: { - id: '123', - globalRole: { - name: 'owner', - scope: 'global', - } - } + userId: '123', }; } diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 947fdec9cb729..ab110cbdf8708 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -1016,7 +1016,7 @@ export interface IWorkflowExecuteAdditionalData { webhookTestBaseUrl: string; currentNodeParameters?: INodeParameters; executionTimeoutTimestamp?: number; - user: N8nUserData; + userId: string; } export type WorkflowExecuteMode = From 14b6a9186a038c26a68711cf95233b980682f5ea Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Thu, 17 Feb 2022 11:47:36 +0100 Subject: [PATCH 5/9] Removed user entity from workflow project because it is no longer needed --- .../cli/src/UserManagement/UserManagementHelper.ts | 2 +- packages/cli/src/WebhookHelpers.ts | 4 ++-- packages/cli/src/WorkflowExecuteAdditionalData.ts | 1 - packages/workflow/src/Interfaces.ts | 13 ------------- 4 files changed, 3 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 5e8a901318bbf..4e88714de4673 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ /* eslint-disable @typescript-eslint/no-non-null-assertion */ /* eslint-disable import/no-cycle */ -import { N8nUserData, Workflow } from 'n8n-workflow'; +import { Workflow } from 'n8n-workflow'; import { In, IsNull, Not } from 'typeorm'; import { Db, ResponseHelper } from '..'; import config = require('../../config'); diff --git a/packages/cli/src/WebhookHelpers.ts b/packages/cli/src/WebhookHelpers.ts index 990cff2d55009..98644b2de93f1 100644 --- a/packages/cli/src/WebhookHelpers.ts +++ b/packages/cli/src/WebhookHelpers.ts @@ -34,7 +34,6 @@ import { IWorkflowDataProxyAdditionalKeys, IWorkflowExecuteAdditionalData, LoggerProxy as Logger, - N8nUserData, NodeHelpers, Workflow, WorkflowExecuteMode, @@ -55,6 +54,7 @@ import { // eslint-disable-next-line import/no-cycle import * as ActiveExecutions from './ActiveExecutions'; +import { User } from './databases/entities/User'; import { WorkflowEntity } from './databases/entities/WorkflowEntity'; import { getWorkflowOwner } from './UserManagement/UserManagementHelper'; @@ -217,7 +217,7 @@ export async function executeWebhook( throw new ResponseHelper.ResponseError(errorMessage, 500, 500); } - let user: N8nUserData; + let user: User; if ( (workflowData as WorkflowEntity).shared?.length && (workflowData as WorkflowEntity).shared[0].user diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index 5205d5a9e11af..c201b43086667 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -31,7 +31,6 @@ import { IWorkflowExecuteHooks, IWorkflowHooksOptionalParameters, LoggerProxy as Logger, - N8nUserData, Workflow, WorkflowExecuteMode, WorkflowHooks, diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 5bd4b5a10b025..b402d7522c209 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -1428,16 +1428,3 @@ export interface ITelemetrySettings { enabled: boolean; config?: ITelemetryClientConfig; } - -export interface N8nUserData { - id: string; - email?: string; - firstName?: string; - lastName?: string; - globalRole: N8nRoleData; -} - -export interface N8nRoleData { - name: string; - scope: string; -} From 8ed719864052a394ac50035449acbd163fbe1810 Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Thu, 17 Feb 2022 16:10:03 +0100 Subject: [PATCH 6/9] General fixes and improvements to runtime checks --- packages/cli/src/Server.ts | 74 ++++++++++--------- .../UserManagement/UserManagementHelper.ts | 40 +++++----- packages/cli/src/WaitingWebhooks.ts | 2 +- packages/cli/src/requests.d.ts | 26 ++++++- 4 files changed, 90 insertions(+), 52 deletions(-) diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 26a23a9d444d6..b4d3c878352d1 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -165,6 +165,8 @@ import type { WorkflowRequest, NodeParameterOptionsRequest, OAuthRequest, + AuthenticatedRequest, + TagsRequest, } from './requests'; import { DEFAULT_EXECUTIONS_GET_ALL_LIMIT, validateEntity } from './GenericHelpers'; import { ExecutionEntity } from './databases/entities/ExecutionEntity'; @@ -1097,7 +1099,10 @@ class App { this.app.post( `/${this.restEndpoint}/workflows/run`, ResponseHelper.send( - async (req: express.Request, res: express.Response): Promise => { + async ( + req: WorkflowRequest.ManualRun, + res: express.Response, + ): Promise => { const { workflowData } = req.body; const { runData } = req.body; const { startNodes } = req.body; @@ -1114,15 +1119,13 @@ class App { startNodes.length === 0 || destinationNode === undefined ) { - const additionalData = await WorkflowExecuteAdditionalData.getBase( - (req.user as User).id, - ); + const additionalData = await WorkflowExecuteAdditionalData.getBase(req.user.id); const nodeTypes = NodeTypes(); const workflowInstance = new Workflow({ - id: workflowData.id, + id: workflowData.id?.toString(), name: workflowData.name, - nodes: workflowData.nodes, - connections: workflowData.connections, + nodes: workflowData.nodes!, + connections: workflowData.connections!, active: false, nodeTypes, staticData: undefined, @@ -1155,7 +1158,7 @@ class App { sessionId, startNodes, workflowData, - userId: (req.user as User).id, + userId: req.user.id, }; const workflowRunner = new WorkflowRunner(); const executionId = await workflowRunner.run(data); @@ -1249,31 +1252,33 @@ class App { // Deletes a tag this.app.delete( `/${this.restEndpoint}/tags/:id`, - ResponseHelper.send(async (req: express.Request, res: express.Response): Promise => { - if (config.get('workflowTagsDisabled')) { - throw new ResponseHelper.ResponseError('Workflow tags are disabled'); - } - if ( - config.get('userManagement.hasOwner') === true && - (req.user as User).globalRole.name !== 'owner' - ) { - throw new ResponseHelper.ResponseError( - 'You are not allowed to perform this action', - 403, - 403, - 'Only owners can remove tags', - ); - } - const id = Number(req.params.id); + ResponseHelper.send( + async (req: TagsRequest.Delete, res: express.Response): Promise => { + if (config.get('workflowTagsDisabled')) { + throw new ResponseHelper.ResponseError('Workflow tags are disabled'); + } + if ( + config.get('userManagement.hasOwner') === true && + req.user.globalRole.name !== 'owner' + ) { + throw new ResponseHelper.ResponseError( + 'You are not allowed to perform this action', + undefined, + 403, + 'Only owners can remove tags', + ); + } + const id = Number(req.params.id); - await this.externalHooks.run('tag.beforeDelete', [id]); + await this.externalHooks.run('tag.beforeDelete', [id]); - await Db.collections.Tag!.delete({ id }); + await Db.collections.Tag!.delete({ id }); - await this.externalHooks.run('tag.afterDelete', [id]); + await this.externalHooks.run('tag.afterDelete', [id]); - return true; - }), + return true; + }, + ), ); // Returns parameter values which normally get loaded from an external API or @@ -1307,7 +1312,7 @@ class App { ); const additionalData = await WorkflowExecuteAdditionalData.getBase( - (req.user as User).id, + req.user.id, currentNodeParameters, ); @@ -1663,7 +1668,10 @@ class App { this.app.post( `/${this.restEndpoint}/credentials-test`, ResponseHelper.send( - async (req: express.Request, res: express.Response): Promise => { + async ( + req: CredentialRequest.Test, + res: express.Response, + ): Promise => { const incomingData = req.body as INodeCredentialTestRequest; const encryptionKey = await UserSettings.getEncryptionKey(); @@ -1678,7 +1686,7 @@ class App { const credentialType = incomingData.credentials.type; return credentialsHelper.testCredentials( - req.user as User, + req.user, credentialType, incomingData.credentials, incomingData.nodeToTestWith, @@ -2623,7 +2631,7 @@ class App { executionData: fullExecutionData.data, retryOf: req.params.id, workflowData: fullExecutionData.workflowData, - userId: (req.user as User).id, + userId: req.user.id, }; const { lastNodeExecuted } = data.executionData!.resultData; diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 2e8ad5e017c9c..09826751cb609 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -7,6 +7,7 @@ import { PublicUser } from './Interfaces'; import { Db, GenericHelpers, ResponseHelper } from '..'; import config = require('../../config'); import { MAX_PASSWORD_LENGTH, MIN_PASSWORD_LENGTH, User } from '../databases/entities/User'; +import { Role } from '../databases/entities/Role'; export async function getWorkflowOwner(workflowId: string | number): Promise { const sharedWorkflow = await Db.collections.SharedWorkflow!.findOneOrFail({ @@ -17,12 +18,25 @@ export async function getWorkflowOwner(workflowId: string | number): Promise { + const ownerRole = await Db.collections.Role!.findOneOrFail({ + where: { + name: 'owner', + scope: 'global', + }, + }); + return ownerRole; +} + export async function getInstanceOwner(): Promise { - const qb = Db.collections.User!.createQueryBuilder('u'); - qb.innerJoin('u.globalRole', 'gr'); - qb.andWhere('gr.name = :name and gr.scope = :scope', { name: 'owner', scope: 'global' }); + const ownerRole = await getInstanceOwnerRole(); - const owner = await qb.getOneOrFail(); + const owner = await Db.collections.User!.findOneOrFail({ + relations: ['globalRole'], + where: { + globalRole: ownerRole, + }, + }); return owner; } @@ -92,30 +106,22 @@ export async function checkPermissionsForExecution( ): Promise { const credentialIds = new Set(); const nodeNames = Object.keys(workflow.nodes); - const pendingPromises = [] as Array>; nodeNames.forEach((nodeName) => { const node = workflow.nodes[nodeName]; if (node.credentials) { const credentialNames = Object.keys(node.credentials); credentialNames.forEach((credentialName) => { const credentialDetail = node.credentials![credentialName]; - if (credentialDetail.id) { - credentialIds.add(credentialDetail.id.toString()); - } else { - pendingPromises.push(getCredentialIdByName(credentialDetail.name)); + if (!credentialDetail.id) { + throw new Error( + 'Error initializing workflow: credential ID not present. Please open the workflow and save it to fix this error.', + ); } + credentialIds.add(credentialDetail.id.toString()); }); } }); - const fullfilledPromises = await Promise.all(pendingPromises); - fullfilledPromises.forEach((credentialId) => { - if (credentialId !== null) { - credentialIds.add(credentialId.toString()); - } else { - throw new Error('One or more of the required credentials was not found in the database.'); - } - }); // We converted all IDs to string so that the set cannot contain duplicates. const ids = Array.from(credentialIds); diff --git a/packages/cli/src/WaitingWebhooks.ts b/packages/cli/src/WaitingWebhooks.ts index e82466dfa8be9..71e7b88a1215d 100644 --- a/packages/cli/src/WaitingWebhooks.ts +++ b/packages/cli/src/WaitingWebhooks.ts @@ -116,7 +116,7 @@ export class WaitingWebhooks { try { workflowOwner = await getWorkflowOwner(workflowData.id!.toString()); } catch (error) { - throw new ResponseHelper.ResponseError('Could not find workflow', 404, 404); + throw new ResponseHelper.ResponseError('Could not find workflow', undefined, 404); } const additionalData = await WorkflowExecuteAdditionalData.getBase(workflowOwner.id); diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index 4297b3e6e319c..3de7ced86cf20 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -5,11 +5,13 @@ import { ICredentialDataDecryptedObject, ICredentialNodeAccess, INode, + INodeCredentialTestRequest, + IRunData, IWorkflowSettings, } from 'n8n-workflow'; import { User } from './databases/entities/User'; -import { IExecutionDeleteFilter } from '.'; +import { IExecutionDeleteFilter, IWorkflowDb } from '.'; import type { PublicUser } from './UserManagement/Interfaces'; export type AuthenticatedRequest< @@ -49,6 +51,17 @@ export declare namespace WorkflowRequest { type GetAllActive = AuthenticatedRequest; type GetAllActivationErrors = Get; + + type ManualRun = AuthenticatedRequest< + {}, + {}, + { + workflowData: IWorkflowDb; + runData: IRunData; + startNodes?: string[]; + destinationNode?: string; + } + >; } // ---------------------------------- @@ -75,6 +88,8 @@ export declare namespace CredentialRequest { type Update = AuthenticatedRequest<{ id: string }, {}, RequestBody>; type NewName = WorkflowRequest.NewName; + + type Test = AuthenticatedRequest<{}, {}, INodeCredentialTestRequest>; } // ---------------------------------- @@ -223,3 +238,12 @@ export type NodeParameterOptionsRequest = AuthenticatedRequest< credentials: string; } >; + + +// ---------------------------------- +// /tags +// ---------------------------------- + +export declare namespace TagsRequest { + type Delete = AuthenticatedRequest<{ id: string }>; +} From aa1cd5d22a1863cdffe83957e0868e42d491b05e Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Fri, 25 Feb 2022 16:08:13 +0100 Subject: [PATCH 7/9] Remove query builder and improve styling --- .../UserManagement/UserManagementHelper.ts | 35 ++++++++++--------- .../cli/src/WorkflowExecuteAdditionalData.ts | 21 +++++++---- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 09826751cb609..b2a0c5913a22b 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -80,21 +80,8 @@ export function sanitizeUser(user: User): PublicUser { return sanitizedUser; } -async function getCredentialIdByName(name: string): Promise { - const credential = await Db.collections.Credentials!.findOne({ - where: { - name, - }, - }); - if (credential) { - return credential.id; - } - return null; -} - export async function getUserById(userId: string): Promise { - const user = await Db.collections.User!.findOneOrFail({ - where: { id: userId }, + const user = await Db.collections.User!.findOneOrFail(userId, { relations: ['globalRole'], }); return user; @@ -103,15 +90,22 @@ export async function getUserById(userId: string): Promise { export async function checkPermissionsForExecution( workflow: Workflow, userId: string, -): Promise { +): Promise { const credentialIds = new Set(); const nodeNames = Object.keys(workflow.nodes); + // Iterate over all nodes nodeNames.forEach((nodeName) => { const node = workflow.nodes[nodeName]; + // 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. Nowaways it should not happen anymore. + // Migrations should handle the case where a credential does + // not have an id. if (!credentialDetail.id) { throw new Error( 'Error initializing workflow: credential ID not present. Please open the workflow and save it to fix this error.', @@ -122,14 +116,17 @@ export async function checkPermissionsForExecution( } }); - // We converted all IDs to string so that the set cannot contain duplicates. + // 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) { - return; + // If the workflow does not use any credential, then we're fine + return true; } + // Check for the user's permission to all used credentials const credentialCount = await Db.collections.SharedCredentials!.count({ where: { user: { id: userId }, @@ -137,7 +134,11 @@ export async function checkPermissionsForExecution( }, }); + // 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 !== credentialCount) { throw new Error('One or more of the required credentials was not found in the database.'); } + return true; } diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index c201b43086667..b6a8a40823087 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -66,6 +66,7 @@ import { getUserById, getWorkflowOwner, } from './UserManagement/UserManagementHelper'; +import { whereClause } from './WorkflowHelpers'; const ERROR_TRIGGER_TYPE = config.get('nodes.errorTriggerType') as string; @@ -798,14 +799,22 @@ export async function getWorkflowData( await Db.init(); } const user = await getUserById(userId); - const qb = Db.collections.Workflow!.createQueryBuilder('w'); - if (user.globalRole.name !== 'owner') { - qb.innerJoin('w.shared', 'shared'); - qb.andWhere('shared.user = :userId', { userId: user.id }); + let relations = ['workflow', 'workflow.tags']; + + if (config.get('workflowTagsDisabled')) { + relations = relations.filter((relation) => relation !== 'workflow.tags'); } - qb.andWhere('w.id = :id', { id: workflowInfo.id }); - workflowData = await qb.getOne(); + const shared = await Db.collections.SharedWorkflow!.findOne({ + relations, + where: whereClause({ + user, + entityType: 'workflow', + entityId: workflowInfo.id, + }), + }); + + workflowData = shared?.workflow; if (workflowData === undefined) { throw new Error(`The workflow with the id "${workflowInfo.id}" does not exist.`); From 157c75b8b6b09a81bbc7ab9d6bbc2dcc80631dbd Mon Sep 17 00:00:00 2001 From: Omar Ajoue Date: Fri, 25 Feb 2022 16:17:33 +0100 Subject: [PATCH 8/9] Fix lint issues --- packages/cli/src/WorkflowExecuteAdditionalData.ts | 3 +-- packages/cli/src/requests.d.ts | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts index b6a8a40823087..0b0d4ec004870 100644 --- a/packages/cli/src/WorkflowExecuteAdditionalData.ts +++ b/packages/cli/src/WorkflowExecuteAdditionalData.ts @@ -1,3 +1,4 @@ +/* eslint-disable import/no-cycle */ /* eslint-disable no-restricted-syntax */ /* eslint-disable @typescript-eslint/restrict-plus-operands */ /* eslint-disable @typescript-eslint/explicit-module-boundary-types */ @@ -39,7 +40,6 @@ import { import { LessThanOrEqual } from 'typeorm'; import { DateUtils } from 'typeorm/util/DateUtils'; import * as config from '../config'; -// eslint-disable-next-line import/no-cycle import { ActiveExecutions, CredentialsHelper, @@ -60,7 +60,6 @@ import { WorkflowCredentials, WorkflowHelpers, } from '.'; -// eslint-disable-next-line import/no-cycle import { checkPermissionsForExecution, getUserById, diff --git a/packages/cli/src/requests.d.ts b/packages/cli/src/requests.d.ts index 3de7ced86cf20..64129172e2b30 100644 --- a/packages/cli/src/requests.d.ts +++ b/packages/cli/src/requests.d.ts @@ -239,7 +239,6 @@ export type NodeParameterOptionsRequest = AuthenticatedRequest< } >; - // ---------------------------------- // /tags // ---------------------------------- From b9e4d095686ae58690c971124612693f2ad8f1d9 Mon Sep 17 00:00:00 2001 From: Ben Hesseldieck Date: Sat, 26 Feb 2022 14:45:52 +0100 Subject: [PATCH 9/9] =?UTF-8?q?=E2=9C=8F=EF=B8=8F=20update=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/cli/src/UserManagement/UserManagementHelper.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index b9cb063042b1b..16b69cf17135a 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -131,7 +131,7 @@ export async function checkPermissionsForExecution( const ids = Array.from(credentialIds); if (ids.length === 0) { - // If the workflow does not use any credential, then we're fine + // If the workflow does not use any credentials, then we're fine return true; } @@ -147,7 +147,7 @@ export async function checkPermissionsForExecution( // then both arrays (allowed credentials vs used credentials) // must be the same length if (ids.length !== credentialCount) { - throw new Error('One or more of the required credentials was not found in the database.'); + throw new Error('One or more of the used credentials are not accessable.'); } return true; }