fix(core): Improve error handling in credential decryption and parsing (#12868)

packages/core/src/__tests__/credentials.test.ts

@@ -2,12 +2,16 @@ import { Container } from '@n8n/di';
 import { mock } from 'jest-mock-extended';
 import type { CredentialInformation } from 'n8n-workflow';
 
+import { CREDENTIAL_ERRORS } from '@/constants';
 import { Cipher } from '@/encryption/cipher';
 import type { InstanceSettings } from '@/instance-settings';
 
 import { Credentials } from '../credentials';
 
 describe('Credentials', () => {
+	const nodeCredentials = { id: '123', name: 'Test Credential' };
+	const credentialType = 'testApi';
+
 	const cipher = new Cipher(mock<InstanceSettings>({ encryptionKey: 'password' }));
 	Container.set(Cipher, cipher);
 
@@ -24,7 +28,7 @@ describe('Credentials', () => {
 
 	describe('without nodeType set', () => {
 		test('should be able to set and read key data without initial data set', () => {
-			const credentials = new Credentials({ id: null, name: 'testName' }, 'testType');
+			const credentials = new Credentials(nodeCredentials, credentialType);
 
 			const key = 'key1';
 			const newData = 1234;
@@ -41,11 +45,7 @@ describe('Credentials', () => {
 			const initialData = 4321;
 			const initialDataEncoded = 'U2FsdGVkX1+0baznXt+Ag/ub8A2kHLyoLxn/rR9h4XQ=';
 
-			const credentials = new Credentials(
-				{ id: null, name: 'testName' },
-				'testType',
-				initialDataEncoded,
-			);
+			const credentials = new Credentials(nodeCredentials, credentialType, initialDataEncoded);
 
 			const newData = 1234;
 
@@ -57,4 +57,54 @@ describe('Credentials', () => {
 			expect(credentials.getData().key1).toEqual(initialData);
 		});
 	});
+
+	describe('getData', () => {
+		test('should throw an error when data is missing', () => {
+			const credentials = new Credentials(nodeCredentials, credentialType);
+			credentials.data = undefined;
+
+			expect(() => credentials.getData()).toThrow(CREDENTIAL_ERRORS.NO_DATA);
+		});
+
+		test('should throw an error when decryption fails', () => {
+			const credentials = new Credentials(nodeCredentials, credentialType);
+			credentials.data = '{"key": "already-decrypted-credentials-data" }';
+
+			expect(() => credentials.getData()).toThrow(CREDENTIAL_ERRORS.DECRYPTION_FAILED);
+
+			try {
+				credentials.getData();
+			} catch (error) {
+				expect(error.constructor.name).toBe('CredentialDataError');
+				expect(error.extra).toEqual({ ...nodeCredentials, type: credentialType });
+				expect((error.cause.code as string).startsWith('ERR_OSSL_')).toBe(true);
+			}
+		});
+
+		test('should throw an error when JSON parsing fails', () => {
+			const credentials = new Credentials(nodeCredentials, credentialType);
+			credentials.data = cipher.encrypt('invalid-json-string');
+
+			expect(() => credentials.getData()).toThrow(CREDENTIAL_ERRORS.INVALID_JSON);
+
+			try {
+				credentials.getData();
+			} catch (error) {
+				expect(error.constructor.name).toBe('CredentialDataError');
+				expect(error.extra).toEqual({ ...nodeCredentials, type: credentialType });
+				expect(error.cause).toEqual(
+					new SyntaxError('Unexpected token \'i\', "invalid-json-string" is not valid JSON'),
+				);
+			}
+		});
+
+		test('should successfully decrypt and parse valid JSON credentials', () => {
+			const credentials = new Credentials(nodeCredentials, credentialType);
+			credentials.setData({ username: 'testuser', password: 'testpass' });
+
+			const decryptedData = credentials.getData();
+			expect(decryptedData.username).toBe('testuser');
+			expect(decryptedData.password).toBe('testpass');
+		});
+	});
 });

packages/core/src/constants.ts

@@ -14,3 +14,10 @@ export const CONFIG_FILES = 'N8N_CONFIG_FILES';
 export const BINARY_DATA_STORAGE_PATH = 'N8N_BINARY_DATA_STORAGE_PATH';
 export const UM_EMAIL_TEMPLATES_INVITE = 'N8N_UM_EMAIL_TEMPLATES_INVITE';
 export const UM_EMAIL_TEMPLATES_PWRESET = 'N8N_UM_EMAIL_TEMPLATES_PWRESET';
+
+export const CREDENTIAL_ERRORS = {
+	NO_DATA: 'No data is set on this credentials.',
+	DECRYPTION_FAILED:
+		'Credentials could not be decrypted. The likely reason is that a different "encryptionKey" was used to encrypt the data.',
+	INVALID_JSON: 'Decrypted credentials data is not valid JSON.',
+};

packages/core/src/credentials.ts

@@ -2,8 +2,18 @@ import { Container } from '@n8n/di';
 import type { ICredentialDataDecryptedObject, ICredentialsEncrypted } from 'n8n-workflow';
 import { ApplicationError, ICredentials, jsonParse } from 'n8n-workflow';
 
+import { CREDENTIAL_ERRORS } from '@/constants';
 import { Cipher } from '@/encryption/cipher';
 
+class CredentialDataError extends ApplicationError {
+	constructor({ name, type, id }: Credentials<object>, message: string, cause?: unknown) {
+		super(message, {
+			extra: { name, type, id },
+			cause,
+		});
+	}
+}
+
 export class Credentials<
 	T extends object = ICredentialDataDecryptedObject,
 > extends ICredentials<T> {
@@ -21,17 +31,20 @@ export class Credentials<
 	 */
 	getData(): T {
 		if (this.data === undefined) {
-			throw new ApplicationError('No data is set so nothing can be returned.');
+			throw new CredentialDataError(this, CREDENTIAL_ERRORS.NO_DATA);
+		}
+
+		let decryptedData: string;
+		try {
+			decryptedData = this.cipher.decrypt(this.data);
+		} catch (cause) {
+			throw new CredentialDataError(this, CREDENTIAL_ERRORS.DECRYPTION_FAILED, cause);
 		}
 
 		try {
-			const decryptedData = this.cipher.decrypt(this.data);
-
 			return jsonParse(decryptedData);
-		} catch (e) {
-			throw new ApplicationError(
-				'Credentials could not be decrypted. The likely reason is that a different "encryptionKey" was used to encrypt the data.',
-			);
+		} catch (cause) {
+			throw new CredentialDataError(this, CREDENTIAL_ERRORS.INVALID_JSON, cause);
 		}
 	}