🎨 Centralize error throwing for encryption keys and credentials (#3105)

* Centralized error throwing for encryption key * Unifying the error message used by cli and core packages * Improvements to error messages to make it more DRY * Removed unnecessary throw * Throwing error when credential does not exist to simplify node behavior (#3112) Co-authored-by: Iván Ovejero <[email protected]>
n8n-io · Apr 15, 2022 · d3fecb9 · d3fecb9
1 parent 17b0cd8
commit d3fecb9
Show file tree

Hide file tree

Showing 227 changed files with 348 additions and 848 deletions.
diff --git a/packages/cli/commands/export/credentials.ts b/packages/cli/commands/export/credentials.ts
@@ -123,9 +123,6 @@ export class ExportCredentialsCommand extends Command {
 
 			if (flags.decrypted) {
 				const encryptionKey = await UserSettings.getEncryptionKey();
-				if (encryptionKey === undefined) {
-					throw new Error('No encryption key got found to decrypt the credentials!');
-				}
 
 				for (let i = 0; i < credentials.length; i++) {
 					const { name, type, nodesAccess, data } = credentials[i];

diff --git a/packages/cli/commands/import/credentials.ts b/packages/cli/commands/import/credentials.ts
@@ -85,9 +85,6 @@ export class ImportCredentialsCommand extends Command {
 			await UserSettings.prepareUserSettings();
 
 			const encryptionKey = await UserSettings.getEncryptionKey();
-			if (encryptionKey === undefined) {
-				throw new Error('No encryption key found to encrypt the credentials!');
-			}
 
 			if (flags.separate) {
 				const files = await glob(

diff --git a/packages/cli/commands/start.ts b/packages/cli/commands/start.ts
@@ -33,7 +33,6 @@ import {
 } from '../src';
 
 import { getLogger } from '../src/Logger';
-import { RESPONSE_ERROR_MESSAGES } from '../src/constants';
 
 // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-var-requires
 const open = require('open');
@@ -173,9 +172,6 @@ export class Start extends Command {
 					// If we don't have a JWT secret set, generate
 					// one based and save to config.
 					const encryptionKey = await UserSettings.getEncryptionKey();
-					if (!encryptionKey) {
-						throw new Error('Fatal error setting up user management: no encryption key set.');
-					}
 
 					// For a key off every other letter from encryption key
 					// CAREFUL: do not change this or it breaks all existing tokens.
@@ -210,11 +206,7 @@ export class Start extends Command {
 				// Wait till the database is ready
 				await startDbInitPromise;
 
-				const encryptionKey = await UserSettings.getEncryptionKey();
-
-				if (!encryptionKey) {
-					throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY);
-				}
+				await UserSettings.getEncryptionKey();
 
 				// Load settings from database and set them to config.
 				const databaseSettings = await Db.collections.Settings.find({ loadOnStartup: true });

diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts
@@ -1703,13 +1703,11 @@ class App {
 					);
 				}
 
-				const encryptionKey = await UserSettings.getEncryptionKey();
-				if (!encryptionKey) {
-					throw new ResponseHelper.ResponseError(
-						RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
-						undefined,
-						500,
-					);
+				let encryptionKey: string;
+				try {
+					encryptionKey = await UserSettings.getEncryptionKey();
+				} catch (error) {
+					throw new ResponseHelper.ResponseError(error.message, undefined, 500);
 				}
 
 				const mode: WorkflowExecuteMode = 'internal';
@@ -1842,15 +1840,11 @@ class App {
 						return ResponseHelper.sendErrorResponse(res, errorResponse);
 					}
 
-					const encryptionKey = await UserSettings.getEncryptionKey();
-
-					if (!encryptionKey) {
-						const errorResponse = new ResponseHelper.ResponseError(
-							RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
-							undefined,
-							503,
-						);
-						return ResponseHelper.sendErrorResponse(res, errorResponse);
+					let encryptionKey: string;
+					try {
+						encryptionKey = await UserSettings.getEncryptionKey();
+					} catch (error) {
+						throw new ResponseHelper.ResponseError(error.message, undefined, 500);
 					}
 
 					const mode: WorkflowExecuteMode = 'internal';
@@ -1962,13 +1956,11 @@ class App {
 					);
 				}
 
-				const encryptionKey = await UserSettings.getEncryptionKey();
-				if (!encryptionKey) {
-					throw new ResponseHelper.ResponseError(
-						RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
-						undefined,
-						500,
-					);
+				let encryptionKey: string;
+				try {
+					encryptionKey = await UserSettings.getEncryptionKey();
+				} catch (error) {
+					throw new ResponseHelper.ResponseError(error.message, undefined, 500);
 				}
 
 				const mode: WorkflowExecuteMode = 'internal';
@@ -2103,15 +2095,11 @@ class App {
 						return ResponseHelper.sendErrorResponse(res, errorResponse);
 					}
 
-					const encryptionKey = await UserSettings.getEncryptionKey();
-
-					if (!encryptionKey) {
-						const errorResponse = new ResponseHelper.ResponseError(
-							RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
-							undefined,
-							503,
-						);
-						return ResponseHelper.sendErrorResponse(res, errorResponse);
+					let encryptionKey: string;
+					try {
+						encryptionKey = await UserSettings.getEncryptionKey();
+					} catch (error) {
+						throw new ResponseHelper.ResponseError(error.message, undefined, 500);
 					}
 
 					const mode: WorkflowExecuteMode = 'internal';

diff --git a/packages/cli/src/WorkflowExecuteAdditionalData.ts b/packages/cli/src/WorkflowExecuteAdditionalData.ts
@@ -66,6 +66,7 @@ import {
 	getWorkflowOwner,
 } from './UserManagement/UserManagementHelper';
 import { whereClause } from './WorkflowHelpers';
+import { RESPONSE_ERROR_MESSAGES } from './constants';
 
 const ERROR_TRIGGER_TYPE = config.getEnv('nodes.errorTriggerType');
 
@@ -1030,9 +1031,6 @@ export async function getBase(
 	const webhookTestBaseUrl = urlBaseWebhook + config.getEnv('endpoints.webhookTest');
 
 	const encryptionKey = await UserSettings.getEncryptionKey();
-	if (encryptionKey === undefined) {
-		throw new Error('No encryption key got found to decrypt the credentials!');
-	}
 
 	return {
 		credentialsHelper: new CredentialsHelper(encryptionKey),

diff --git a/packages/cli/src/api/credentials.api.ts b/packages/cli/src/api/credentials.api.ts
@@ -115,13 +115,15 @@ credentialsController.post(
 	ResponseHelper.send(async (req: CredentialRequest.Test): Promise<INodeCredentialTestResult> => {
 		const { credentials, nodeToTestWith } = req.body;
 
-		const encryptionKey = await UserSettings.getEncryptionKey();
-
-		if (!encryptionKey) {
-			return {
-				status: 'Error',
-				message: RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
-			};
+		let encryptionKey: string;
+		try {
+			encryptionKey = await UserSettings.getEncryptionKey();
+		} catch (error) {
+			throw new ResponseHelper.ResponseError(
+				RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
+				undefined,
+				500,
+			);
 		}
 
 		const helper = new CredentialsHelper(encryptionKey);
@@ -149,9 +151,10 @@ credentialsController.post(
 			nodeAccess.date = new Date();
 		}
 
-		const encryptionKey = await UserSettings.getEncryptionKey();
-
-		if (!encryptionKey) {
+		let encryptionKey: string;
+		try {
+			encryptionKey = await UserSettings.getEncryptionKey();
+		} catch (error) {
 			throw new ResponseHelper.ResponseError(
 				RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
 				undefined,
@@ -285,9 +288,10 @@ credentialsController.patch(
 			}
 		}
 
-		const encryptionKey = await UserSettings.getEncryptionKey();
-
-		if (!encryptionKey) {
+		let encryptionKey: string;
+		try {
+			encryptionKey = await UserSettings.getEncryptionKey();
+		} catch (error) {
 			throw new ResponseHelper.ResponseError(
 				RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
 				undefined,
@@ -393,9 +397,10 @@ credentialsController.get(
 
 		const { data, id, ...rest } = credential;
 
-		const encryptionKey = await UserSettings.getEncryptionKey();
-
-		if (!encryptionKey) {
+		let encryptionKey: string;
+		try {
+			encryptionKey = await UserSettings.getEncryptionKey();
+		} catch (error) {
 			throw new ResponseHelper.ResponseError(
 				RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
 				undefined,

diff --git a/packages/cli/src/constants.ts b/packages/cli/src/constants.ts
@@ -1,8 +1,12 @@
+/* eslint-disable @typescript-eslint/no-unsafe-assignment */
+/* eslint-disable @typescript-eslint/no-unsafe-member-access */
 /* eslint-disable @typescript-eslint/naming-convention */
 
+import { RESPONSE_ERROR_MESSAGES as CORE_RESPONSE_ERROR_MESSAGES } from 'n8n-core';
+
 export const RESPONSE_ERROR_MESSAGES = {
 	NO_CREDENTIAL: 'Credential not found',
-	NO_ENCRYPTION_KEY: 'Encryption key missing to decrypt credentials',
+	NO_ENCRYPTION_KEY: CORE_RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY,
 };
 
 export const AUTH_COOKIE_NAME = 'n8n-auth';
diff --git a/packages/cli/test/integration/credentials.api.test.ts b/packages/cli/test/integration/credentials.api.test.ts
@@ -7,6 +7,7 @@ import type { CredentialPayload, SaveCredentialFunction } from './shared/types';
 import type { Role } from '../../src/databases/entities/Role';
 import type { User } from '../../src/databases/entities/User';
 import * as testDb from './shared/testDb';
+import { RESPONSE_ERROR_MESSAGES } from '../../src/constants';
 import { CredentialsEntity } from '../../src/databases/entities/CredentialsEntity';
 
 jest.mock('../../src/telemetry');
@@ -91,7 +92,7 @@ test('POST /credentials should fail with invalid inputs', async () => {
 
 test('POST /credentials should fail with missing encryption key', async () => {
 	const mock = jest.spyOn(UserSettings, 'getEncryptionKey');
-	mock.mockResolvedValue(undefined);
+	mock.mockRejectedValue(new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY));
 
 	const ownerShell = await testDb.createUserShell(globalOwnerRole);
 	const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
@@ -354,7 +355,8 @@ test('PATCH /credentials/:id should fail if cred not found', async () => {
 
 test('PATCH /credentials/:id should fail with missing encryption key', async () => {
 	const mock = jest.spyOn(UserSettings, 'getEncryptionKey');
-	mock.mockResolvedValue(undefined);
+	mock.mockRejectedValue(new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY));
+
 
 	const ownerShell = await testDb.createUserShell(globalOwnerRole);
 	const authOwnerAgent = utils.createAgent(app, { auth: true, user: ownerShell });
@@ -504,7 +506,8 @@ test('GET /credentials/:id should fail with missing encryption key', async () =>
 	const savedCredential = await saveCredential(credentialPayload(), { user: ownerShell });
 
 	const mock = jest.spyOn(UserSettings, 'getEncryptionKey');
-	mock.mockResolvedValue(undefined);
+	mock.mockRejectedValue(new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY));
+
 
 	const response = await authOwnerAgent
 		.get(`/credentials/${savedCredential.id}`)

diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts
@@ -392,10 +392,6 @@ export const getMySqlOptions = ({ name }: { name: string }): ConnectionOptions =
 async function encryptCredentialData(credential: CredentialsEntity) {
 	const encryptionKey = await UserSettings.getEncryptionKey();
 
-	if (!encryptionKey) {
-		throw new Error(RESPONSE_ERROR_MESSAGES.NO_ENCRYPTION_KEY);
-	}
-
 	const coreCredential = new Credentials(
 		{ id: null, name: credential.name },
 		credential.type,

diff --git a/packages/core/src/Constants.ts b/packages/core/src/Constants.ts
@@ -1,3 +1,4 @@
+/* eslint-disable @typescript-eslint/naming-convention */
 export const BINARY_ENCODING = 'base64';
 export const CUSTOM_EXTENSION_ENV = 'N8N_CUSTOM_EXTENSIONS';
 export const ENCRYPTION_KEY_ENV_OVERWRITE = 'N8N_ENCRYPTION_KEY';
@@ -9,3 +10,7 @@ export const PLACEHOLDER_EMPTY_EXECUTION_ID = '__UNKOWN__';
 export const PLACEHOLDER_EMPTY_WORKFLOW_ID = '__EMPTY__';
 export const TUNNEL_SUBDOMAIN_ENV = 'N8N_TUNNEL_SUBDOMAIN';
 export const WAIT_TIME_UNLIMITED = '3000-01-01T00:00:00.000Z';
+
+export const RESPONSE_ERROR_MESSAGES = {
+	NO_ENCRYPTION_KEY: 'Encryption key is missing or was not set',
+};