✨ Runtime checks for credentials load and execute workflows (#2697)

* Runtime checks for credentials load and execute workflows * Fixed from reviewers * Changed runtime validation for credentials to be on start instead of on demand * Refactored validations to use user id instead of whole User instance * Removed user entity from workflow project because it is no longer needed * General fixes and improvements to runtime checks * Remove query builder and improve styling * Fix lint issues
n8n-io · Feb 26, 2022 · 3748fa1 · 3748fa1
1 parent 9705165
commit 3748fa1
Show file tree

Hide file tree

Showing 21 changed files with 347 additions and 101 deletions.
diff --git 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';
@@ -169,11 +170,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!,
+				userId: user.id,
 			};
 
 			const workflowRunner = new WorkflowRunner();

diff --git 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,
+					userId: ExecuteBatch.instanceOwner.id,
 				};
 
 				const workflowRunner = new WorkflowRunner();

diff --git a/packages/cli/commands/worker.ts b/packages/cli/commands/worker.ts
@@ -41,6 +41,10 @@ import { getLogger } from '../src/Logger';
 
 import * as config from '../config';
 import * as Queue from '../src/Queue';
+import {
+	checkPermissionsForExecution,
+	getWorkflowOwner,
+} from '../src/UserManagement/UserManagementHelper';
 
 export class Worker extends Command {
 	static description = '\nStarts a n8n worker';
@@ -123,6 +127,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 = {
@@ -166,7 +172,10 @@ export class Worker extends Command {
 			settings: currentExecutionDb.workflowData.settings,
 		});
 
+		await checkPermissionsForExecution(workflow, workflowOwner.id);
+
 		const additionalData = await WorkflowExecuteAdditionalData.getBase(
+			workflowOwner.id,
 			undefined,
 			executionTimeoutTimestamp,
 		);

diff --git 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,9 @@ export class ActiveWorkflowRunner {
 			settings: workflowData.settings,
 		});
 
-		const additionalData = await WorkflowExecuteAdditionalData.getBase();
+		const additionalData = await WorkflowExecuteAdditionalData.getBase(
+			workflowData.shared[0].user.id,
+		);
 
 		const webhookData = NodeHelpers.getNodeWebhooks(
 			workflow,
@@ -512,7 +517,9 @@ export class ActiveWorkflowRunner {
 	 * @memberof ActiveWorkflowRunner
 	 */
 	async removeWorkflowWebhooks(workflowId: string): Promise<void> {
-		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}"`);
 		}
@@ -531,7 +538,9 @@ export class ActiveWorkflowRunner {
 
 		const mode = 'internal';
 
-		const additionalData = await WorkflowExecuteAdditionalData.getBase();
+		const additionalData = await WorkflowExecuteAdditionalData.getBase(
+			workflowData.shared[0].user.id,
+		);
 
 		const webhooks = WebhookHelpers.getWorkflowWebhooks(workflow, additionalData, undefined, true);
 
@@ -598,6 +607,7 @@ export class ActiveWorkflowRunner {
 
 		// Start the workflow
 		const runData: IWorkflowExecutionDataProcess = {
+			userId: additionalData.userId,
 			executionMode: mode,
 			executionData,
 			workflowData,
@@ -701,7 +711,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) {
@@ -730,7 +742,9 @@ export class ActiveWorkflowRunner {
 			}
 
 			const mode = 'trigger';
-			const additionalData = await WorkflowExecuteAdditionalData.getBase();
+			const additionalData = await WorkflowExecuteAdditionalData.getBase(
+				(workflowData as WorkflowEntity).shared[0].user.id,
+			);
 			const getTriggerFunctions = this.getExecuteTriggerFunctions(
 				workflowData,
 				additionalData,

diff --git a/packages/cli/src/CredentialsHelper.ts b/packages/cli/src/CredentialsHelper.ts
@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/no-non-null-assertion */
 /* eslint-disable no-restricted-syntax */
 /* eslint-disable @typescript-eslint/no-unsafe-member-access */
 /* eslint-disable @typescript-eslint/no-unsafe-assignment */
@@ -226,20 +227,12 @@ export class CredentialsHelper extends ICredentialsHelper {
 	async getCredentials(
 		nodeCredentials: INodeCredentialsDetails,
 		type: string,
-		userId?: string,
 	): Promise<Credentials> {
 		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 (userId) {
-			// TODO UM: implement this.
-			// qb.
-		}
-		const credentials = await qb.getOne();
+		const credentials = await Db.collections.Credentials!.findOne(nodeCredentials.id);
 
 		if (!credentials) {
 			throw new Error(
@@ -300,9 +293,8 @@ export class CredentialsHelper extends ICredentialsHelper {
 		mode: WorkflowExecuteMode,
 		raw?: boolean,
 		expressionResolveValues?: ICredentialsExpressionResolveValues,
-		userId?: string,
 	): Promise<ICredentialDataDecryptedObject> {
-		const credentials = await this.getCredentials(nodeCredentials, type, userId);
+		const credentials = await this.getCredentials(nodeCredentials, type);
 		const decryptedDataOriginal = credentials.getData(this.encryptionKey);
 
 		if (raw === true) {
@@ -506,6 +498,7 @@ export class CredentialsHelper extends ICredentialsHelper {
 	}
 
 	async testCredentials(
+		user: User,
 		credentialType: string,
 		credentialsDecrypted: ICredentialsDecrypted,
 		nodeToTestWith?: string,
@@ -604,7 +597,7 @@ export class CredentialsHelper extends ICredentialsHelper {
 			},
 		};
 
-		const additionalData = await WorkflowExecuteAdditionalData.getBase(node.parameters);
+		const additionalData = await WorkflowExecuteAdditionalData.getBase(user.id, node.parameters);
 
 		const routingNode = new RoutingNode(
 			workflow,

diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts
@@ -574,14 +574,15 @@ export interface IWorkflowExecutionDataProcess {
 	sessionId?: string;
 	startNodes?: string[];
 	workflowData: IWorkflowBase;
-	userId?: string;
+	userId: string;
 }
 
 export interface IWorkflowExecutionDataProcessWithExecution extends IWorkflowExecutionDataProcess {
 	credentialsOverwrite: ICredentialsOverwrite;
 	credentialsTypeData: ICredentialsTypeData;
 	executionId: string;
 	nodeTypeData: ITransferNodeTypes;
+	userId: string;
 }
 
 export interface IWorkflowExecuteProcess {