refactor(core): Refactor some of the external secrets related code (no-changelog) (#14791)

packages/@n8n/config/src/index.ts

@@ -10,7 +10,6 @@ import { EndpointsConfig } from './configs/endpoints.config';
 import { EventBusConfig } from './configs/event-bus.config';
 import { ExecutionsConfig } from './configs/executions.config';
 import { ExternalHooksConfig } from './configs/external-hooks.config';
-import { ExternalSecretsConfig } from './configs/external-secrets.config';
 import { ExternalStorageConfig } from './configs/external-storage.config';
 import { GenericConfig } from './configs/generic.config';
 import { LicenseConfig } from './configs/license.config';
@@ -68,9 +67,6 @@ export class GlobalConfig {
 	@Nested
 	externalHooks: ExternalHooksConfig;
 
-	@Nested
-	externalSecrets: ExternalSecretsConfig;
-
 	@Nested
 	templates: TemplatesConfig;
 

packages/@n8n/config/test/config.test.ts

@@ -115,10 +115,6 @@ describe('GlobalConfig', () => {
 		externalHooks: {
 			files: [],
 		},
-		externalSecrets: {
-			preferGet: false,
-			updateInterval: 300,
-		},
 		nodes: {
 			communityPackages: {
 				enabled: true,

packages/cli/src/errors/external-secrets-provider-not-found.error.ts

@@ -1,7 +0,0 @@
-import { UnexpectedError } from 'n8n-workflow';
-
-export class ExternalSecretsProviderNotFoundError extends UnexpectedError {
-	constructor(public providerName: string) {
-		super(`External secrets provider not found: ${providerName}`);
-	}
-}

packages/cli/src/eventbus/message-event-bus-destination/message-event-bus-destination-webhook.ee.ts

@@ -14,7 +14,7 @@ import type {
 } from 'n8n-workflow';
 
 import { CredentialsHelper } from '@/credentials-helper';
-import * as SecretsHelpers from '@/external-secrets.ee/external-secrets-helper.ee';
+import { SecretsHelper } from '@/secrets-helpers.ee';
 
 import { MessageEventBusDestination } from './message-event-bus-destination.ee';
 import { eventMessageGenericDestinationTestEvent } from '../event-message-classes/event-message-generic';
@@ -102,7 +102,9 @@ export class MessageEventBusDestinationWebhook
 		const foundCredential = Object.entries(this.credentials).find((e) => e[0] === credentialType);
 		if (foundCredential) {
 			const credentialsDecrypted = await this.credentialsHelper?.getDecrypted(
-				{ secretsHelpers: SecretsHelpers } as unknown as IWorkflowExecuteAdditionalData,
+				{
+					secretsHelpers: Container.get(SecretsHelper),
+				} as unknown as IWorkflowExecuteAdditionalData,
 				foundCredential[1],
 				foundCredential[0],
 				'internal',

packages/cli/src/external-secrets.ee/__tests__/external-secrets-manager.ee.test.ts

@@ -1,58 +1,59 @@
-import { Container } from '@n8n/di';
 import { mock } from 'jest-mock-extended';
-import { Cipher } from 'n8n-core';
 
-import { SettingsRepository } from '@/databases/repositories/settings.repository';
+import type { SettingsRepository } from '@/databases/repositories/settings.repository';
-import { ExternalSecretsManager } from '@/external-secrets.ee/external-secrets-manager.ee';
+import type { License } from '@/license';
-import { ExternalSecretsProviders } from '@/external-secrets.ee/external-secrets-providers.ee';
-import type { ExternalSecretsSettings } from '@/interfaces';
-import { License } from '@/license';
 import {
+	AnotherDummyProvider,
 	DummyProvider,
 	ErrorProvider,
 	FailedProvider,
 	MockProviders,
 } from '@test/external-secrets/utils';
-import { mockInstance, mockLogger } from '@test/mocking';
+import { mockCipher, mockLogger } from '@test/mocking';
+
+import { ExternalSecretsManager } from '../external-secrets-manager.ee';
+import type { ExternalSecretsSettings } from '../types';
 
 describe('External Secrets Manager', () => {
+	jest.useFakeTimers();
+
 	const connectedDate = '2023-08-01T12:32:29.000Z';
-	let settings: string | null = null;
+	const providerSettings = () => ({
-
+		connected: true,
-	const mockProvidersInstance = new MockProviders();
+		connectedAt: new Date(connectedDate),
-	const license = mockInstance(License);
+		settings: {},
-	const settingsRepo = mockInstance(SettingsRepository);
-	const cipher = Container.get(Cipher);
-
-	let providersMock: ExternalSecretsProviders;
-	let manager: ExternalSecretsManager;
-
-	const createMockSettings = (settings: ExternalSecretsSettings): string => {
-		return cipher.encrypt(settings);
-	};
-
-	const decryptSettings = (settings: string) => {
-		return JSON.parse(cipher.decrypt(settings));
-	};
-
-	beforeAll(() => {
-		providersMock = mockInstance(ExternalSecretsProviders, mockProvidersInstance);
-		settings = createMockSettings({
-			dummy: { connected: true, connectedAt: new Date(connectedDate), settings: {} },
-		});
 	});
 
+	const settings: ExternalSecretsSettings = {
+		dummy: providerSettings(),
+		another_dummy: providerSettings(),
+		failed: providerSettings(),
+	};
+
+	const mockProvidersInstance = new MockProviders();
+	const license = mock<License>();
+	const settingsRepo = mock<SettingsRepository>();
+	const cipher = mockCipher();
+
+	let manager: ExternalSecretsManager;
+
 	beforeEach(() => {
+		settings.dummy.connected = true;
 		mockProvidersInstance.setProviders({
 			dummy: DummyProvider,
 		});
+
 		license.isExternalSecretsEnabled.mockReturnValue(true);
-		settingsRepo.getEncryptedSecretsProviderSettings.mockResolvedValue(settings);
+		settingsRepo.getEncryptedSecretsProviderSettings.mockImplementation(async () =>
+			JSON.stringify(settings),
+		);
+
 		manager = new ExternalSecretsManager(
 			mockLogger(),
+			mock(),
 			settingsRepo,
 			license,
-			providersMock,
+			mockProvidersInstance,
 			cipher,
 			mock(),
 			mock(),
@@ -61,107 +62,313 @@ describe('External Secrets Manager', () => {
 
 	afterEach(() => {
 		manager?.shutdown();
-		jest.useRealTimers();
 	});
 
-	test('should get secret', async () => {
+	describe('init / shutdown', () => {
-		await manager.init();
+		test('should not throw errors during init', async () => {
-
+			mockProvidersInstance.setProviders({
-		expect(manager.getSecret('dummy', 'test1')).toBe('value1');
+				dummy: ErrorProvider,
-	});
+			});
-
+			expect(async () => await manager!.init()).not.toThrow();
-	test('should not throw errors during init', async () => {
-		mockProvidersInstance.setProviders({
-			dummy: ErrorProvider,
-		});
-		expect(async () => await manager!.init()).not.toThrow();
-	});
-
-	test('should not throw errors during shutdown', async () => {
-		mockProvidersInstance.setProviders({
-			dummy: ErrorProvider,
 		});
 
-		await manager.init();
+		test('should not throw errors during shutdown', async () => {
-		expect(() => manager!.shutdown()).not.toThrow();
+			mockProvidersInstance.setProviders({
-	});
+				dummy: ErrorProvider,
+			});
 
-	test('should save provider settings', async () => {
+			await manager.init();
-		const settingsSpy = jest.spyOn(settingsRepo, 'saveEncryptedSecretsProviderSettings');
+			expect(() => manager!.shutdown()).not.toThrow();
-
-		await manager.init();
-
-		await manager.setProviderSettings('dummy', {
-			test: 'value',
 		});
 
-		expect(decryptSettings(settingsSpy.mock.calls[0][0])).toEqual({
+		test('should call provider update functions on a timer', async () => {
-			dummy: {
+			await manager.init();
-				connected: true,
+
-				connectedAt: connectedDate,
+			const updateSpy = jest.spyOn(manager.getProvider('dummy')!, 'update');
-				settings: {
+
-					test: 'value',
+			expect(updateSpy).toBeCalledTimes(0);
-				},
+
-			},
+			jest.runOnlyPendingTimers();
+
+			expect(updateSpy).toBeCalledTimes(1);
+		});
+
+		test('should not call provider update functions if the not licensed', async () => {
+			license.isExternalSecretsEnabled.mockReturnValue(false);
+
+			await manager.init();
+
+			const updateSpy = jest.spyOn(manager.getProvider('dummy')!, 'update');
+
+			expect(updateSpy).toBeCalledTimes(0);
+
+			jest.runOnlyPendingTimers();
+
+			expect(updateSpy).toBeCalledTimes(0);
+		});
+
+		test('should not call provider update functions if the provider has an error', async () => {
+			mockProvidersInstance.setProviders({
+				dummy: FailedProvider,
+			});
+
+			await manager.init();
+
+			const updateSpy = jest.spyOn(manager.getProvider('dummy')!, 'update');
+
+			expect(updateSpy).toBeCalledTimes(0);
+
+			jest.runOnlyPendingTimers();
+
+			expect(updateSpy).toBeCalledTimes(0);
+		});
+
+		test('should reinitialize a provider when save provider settings', async () => {
+			await manager.init();
+
+			const dummyInitSpy = jest.spyOn(DummyProvider.prototype, 'init');
+
+			await manager.setProviderSettings('dummy', {
+				test: 'value',
+			});
+
+			expect(dummyInitSpy).toBeCalledTimes(1);
 		});
 	});
 
-	test('should call provider update functions on a timer', async () => {
+	describe('hasProvider', () => {
-		jest.useFakeTimers();
+		test('should check if provider exists', async () => {
-		await manager.init();
+			await manager.init();
 
-		const updateSpy = jest.spyOn(manager.getProvider('dummy')!, 'update');
+			expect(manager.hasProvider('dummy')).toBe(true);
-
+			expect(manager.hasProvider('nonexistent')).toBe(false);
-		expect(updateSpy).toBeCalledTimes(0);
+		});
-
-		jest.runOnlyPendingTimers();
-
-		expect(updateSpy).toBeCalledTimes(1);
 	});
 
-	test('should not call provider update functions if the not licensed', async () => {
+	describe('getProviderNames', () => {
-		jest.useFakeTimers();
+		test('should get provider names', async () => {
+			await manager.init();
 
-		license.isExternalSecretsEnabled.mockReturnValue(false);
+			expect(manager.getProviderNames()).toEqual(['dummy']);
 
-		await manager.init();
+			// @ts-expect-error private property
-
+			manager.providers = {};
-		const updateSpy = jest.spyOn(manager.getProvider('dummy')!, 'update');
+			expect(manager.getProviderNames()).toEqual([]);
-
+		});
-		expect(updateSpy).toBeCalledTimes(0);
-
-		jest.runOnlyPendingTimers();
-
-		expect(updateSpy).toBeCalledTimes(0);
 	});
 
-	test('should not call provider update functions if the provider has an error', async () => {
+	describe('updateProvider', () => {
-		jest.useFakeTimers();
+		test('should update a specific provider and return true on success', async () => {
+			await manager.init();
 
-		mockProvidersInstance.setProviders({
+			const result = await manager.updateProvider('dummy');
-			dummy: FailedProvider,
+
+			expect(result).toBe(true);
 		});
 
-		await manager.init();
+		test('should return false if provider is not connected', async () => {
+			mockProvidersInstance.setProviders({
+				dummy: ErrorProvider,
+			});
 
-		const updateSpy = jest.spyOn(manager.getProvider('dummy')!, 'update');
+			await manager.init();
 
-		expect(updateSpy).toBeCalledTimes(0);
+			const result = await manager.updateProvider('dummy');
 
-		jest.runOnlyPendingTimers();
+			expect(result).toBe(false);
-
-		expect(updateSpy).toBeCalledTimes(0);
-	});
-
-	test('should reinitialize a provider when save provider settings', async () => {
-		await manager.init();
-
-		const dummyInitSpy = jest.spyOn(DummyProvider.prototype, 'init');
-
-		await manager.setProviderSettings('dummy', {
-			test: 'value',
 		});
 
-		expect(dummyInitSpy).toBeCalledTimes(1);
+		test('should return false if external secrets are not licensed', async () => {
+			license.isExternalSecretsEnabled.mockReturnValue(false);
+
+			await manager.init();
+
+			const result = await manager.updateProvider('dummy');
+
+			expect(result).toBe(false);
+		});
+	});
+
+	describe('reloadAllProviders', () => {
+		test('should reload all providers', async () => {
+			await manager.init();
+
+			const reloadSpy = jest.spyOn(manager, 'reloadProvider');
+
+			await manager.reloadAllProviders();
+
+			expect(reloadSpy).toHaveBeenCalledWith('dummy', undefined);
+		});
+	});
+
+	describe('getProviderWithSettings', () => {
+		test('should get provider with settings', async () => {
+			await manager.init();
+
+			const result = manager.getProviderWithSettings('dummy');
+
+			expect(result).toEqual({
+				provider: expect.any(DummyProvider),
+				settings: expect.objectContaining({
+					connected: true,
+					connectedAt: connectedDate,
+				}),
+			});
+		});
+	});
+
+	describe('getProvidersWithSettings', () => {
+		test('should return all providers with their settings', async () => {
+			mockProvidersInstance.setProviders({
+				dummy: DummyProvider,
+				another_dummy: DummyProvider,
+			});
+
+			settings.dummy.settings = { key: 'value' };
+			settings.another_dummy.settings = { key2: 'value2' };
+
+			await manager.init();
+
+			const result = manager.getProvidersWithSettings();
+
+			expect(result).toHaveLength(2);
+			expect(result[0]).toEqual({
+				provider: expect.any(DummyProvider),
+				settings: expect.objectContaining({
+					connected: true,
+					settings: { key: 'value' },
+				}),
+			});
+			expect(result[1]).toEqual({
+				provider: expect.any(DummyProvider),
+				settings: expect.objectContaining({
+					connected: true,
+					settings: { key2: 'value2' },
+				}),
+			});
+		});
+	});
+
+	describe('setProviderSettings', () => {
+		test('should save provider settings', async () => {
+			const settingsSpy = jest.spyOn(settingsRepo, 'saveEncryptedSecretsProviderSettings');
+
+			await manager.init();
+
+			await manager.setProviderSettings('dummy', {
+				test: 'value',
+			});
+
+			expect(JSON.parse(settingsSpy.mock.calls[0][0])).toEqual(
+				expect.objectContaining({
+					dummy: {
+						connected: true,
+						connectedAt: connectedDate,
+						settings: {
+							test: 'value',
+						},
+					},
+				}),
+			);
+		});
+	});
+
+	describe('testProviderSettings', () => {
+		test('should test provider settings successfully', async () => {
+			await manager.init();
+
+			const result = await manager.testProviderSettings('dummy', {});
+
+			expect(result).toEqual({
+				success: true,
+				testState: 'connected',
+			});
+		});
+
+		test('should return tested state for successful but not connected provider', async () => {
+			settings.dummy.connected = false;
+
+			await manager.init();
+
+			const result = await manager.testProviderSettings('dummy', {});
+
+			expect(result).toEqual({
+				success: true,
+				testState: 'tested',
+			});
+		});
+
+		test('should return error state if provider test fails', async () => {
+			mockProvidersInstance.setProviders({
+				error: ErrorProvider,
+			});
+
+			await manager.init();
+
+			const result = await manager.testProviderSettings('error', {});
+
+			expect(result).toEqual({
+				success: false,
+				testState: 'error',
+			});
+		});
+	});
+
+	describe('hasSecret', () => {
+		test('should return true when secret exists', async () => {
+			await manager.init();
+
+			expect(manager.hasSecret('dummy', 'test1')).toBe(true);
+		});
+
+		test('should return false when secret does not exist', async () => {
+			await manager.init();
+
+			expect(manager.hasSecret('dummy', 'nonexistent')).toBe(false);
+		});
+
+		test('should return false when provider does not exist', async () => {
+			await manager.init();
+
+			expect(manager.hasSecret('nonexistent', 'test1')).toBe(false);
+		});
+	});
+
+	describe('getSecret', () => {
+		test('should get secret', async () => {
+			await manager.init();
+
+			expect(manager.getSecret('dummy', 'test1')).toBe('value1');
+		});
+	});
+
+	describe('getSecretNames', () => {
+		test('should return list of secret names for a provider', async () => {
+			await manager.init();
+
+			expect(manager.getSecretNames('dummy')).toEqual(['test1', 'test2']);
+		});
+
+		test('should return undefined when provider does not exist', async () => {
+			await manager.init();
+
+			expect(manager.getSecretNames('nonexistent')).toBeUndefined();
+		});
+	});
+
+	describe('getAllSecretNames', () => {
+		test('should return secret names for all providers', async () => {
+			mockProvidersInstance.setProviders({
+				dummy: DummyProvider,
+				another_dummy: AnotherDummyProvider,
+			});
+
+			await manager.init();
+
+			expect(manager.getAllSecretNames()).toEqual({
+				dummy: ['test1', 'test2'],
+				another_dummy: ['test1', 'test2'],
+			});
+		});
 	});
 });

packages/cli/src/external-secrets.ee/external-secrets-helper.ee.ts

@@ -1,13 +0,0 @@
-import { GlobalConfig } from '@n8n/config';
-import { Container } from '@n8n/di';
-
-import { License } from '@/license';
-
-export const updateIntervalTime = () =>
-	Container.get(GlobalConfig).externalSecrets.updateInterval * 1000;
-export const preferGet = () => Container.get(GlobalConfig).externalSecrets.preferGet;
-
-export function isExternalSecretsEnabled() {
-	const license = Container.get(License);
-	return license.isExternalSecretsEnabled();
-}

packages/cli/src/external-secrets.ee/external-secrets-manager.ee.ts

@@ -3,18 +3,15 @@ import { Cipher, Logger } from 'n8n-core';
 import { jsonParse, type IDataObject, ensureError, UnexpectedError } from 'n8n-workflow';
 
 import { SettingsRepository } from '@/databases/repositories/settings.repository';
+import { OnShutdown } from '@/decorators/on-shutdown';
 import { EventService } from '@/events/event.service';
-import type {
-	ExternalSecretsSettings,
-	SecretsProvider,
-	SecretsProviderSettings,
-} from '@/interfaces';
 import { License } from '@/license';
 import { Publisher } from '@/scaling/pubsub/publisher.service';
 
 import { EXTERNAL_SECRETS_INITIAL_BACKOFF, EXTERNAL_SECRETS_MAX_BACKOFF } from './constants';
-import { updateIntervalTime } from './external-secrets-helper.ee';
 import { ExternalSecretsProviders } from './external-secrets-providers.ee';
+import { ExternalSecretsConfig } from './external-secrets.config';
+import type { ExternalSecretsSettings, SecretsProvider, SecretsProviderSettings } from './types';
 
 @Service()
 export class ExternalSecretsManager {
@@ -32,6 +29,7 @@ export class ExternalSecretsManager {
 
 	constructor(
 		private readonly logger: Logger,
+		private readonly config: ExternalSecretsConfig,
 		private readonly settingsRepo: SettingsRepository,
 		private readonly license: License,
 		private readonly secretsProviders: ExternalSecretsProviders,
@@ -52,16 +50,17 @@ export class ExternalSecretsManager {
 					this.initializingPromise = undefined;
 					this.updateInterval = setInterval(
 						async () => await this.updateSecrets(),
-						updateIntervalTime(),
+						this.config.updateInterval * 1000,
 					);
 				});
 			}
-			return await this.initializingPromise;
+			await this.initializingPromise;
 		}
 
 		this.logger.debug('External secrets manager initialized');
 	}
 
+	@OnShutdown()
 	shutdown() {
 		clearInterval(this.updateInterval);
 		Object.values(this.providers).forEach((p) => {
@@ -86,14 +85,19 @@ export class ExternalSecretsManager {
 		this.logger.debug('External secrets managed reloaded all providers');
 	}
 
-	broadcastReloadExternalSecretsProviders() {
+	private broadcastReloadExternalSecretsProviders() {
 		void this.publisher.publishCommand({ command: 'reload-external-secrets-providers' });
 	}
 
-	private decryptSecretsSettings(value: string): ExternalSecretsSettings {
+	private async getDecryptedSettings(): Promise<ExternalSecretsSettings | null> {
-		const decryptedData = this.cipher.decrypt(value);
+		const encryptedSettings = await this.settingsRepo.getEncryptedSecretsProviderSettings();
+		if (encryptedSettings === null) {
+			return null;
+		}
+
+		const decryptedData = this.cipher.decrypt(encryptedSettings);
 		try {
-			return jsonParse(decryptedData);
+			return jsonParse<ExternalSecretsSettings>(decryptedData);
 		} catch (e) {
 			throw new UnexpectedError(
 				'External Secrets Settings could not be decrypted. The likely reason is that a different "encryptionKey" was used to encrypt the data.',
@@ -101,18 +105,8 @@ export class ExternalSecretsManager {
 		}
 	}
 
-	private async getDecryptedSettings(
-		settingsRepo: SettingsRepository,
-	): Promise<ExternalSecretsSettings | null> {
-		const encryptedSettings = await settingsRepo.getEncryptedSecretsProviderSettings();
-		if (encryptedSettings === null) {
-			return null;
-		}
-		return this.decryptSecretsSettings(encryptedSettings);
-	}
-
 	private async internalInit() {
-		const settings = await this.getDecryptedSettings(this.settingsRepo);
+		const settings = await this.getDecryptedSettings();
 		if (!settings) {
 			return;
 		}
@@ -245,16 +239,11 @@ export class ExternalSecretsManager {
 		}));
 	}
 
-	getProviderWithSettings(provider: string):
+	getProviderWithSettings(provider: string): {
-		| {
+		provider: SecretsProvider;
-				provider: SecretsProvider;
+		settings: SecretsProviderSettings;
-				settings: SecretsProviderSettings;
+	} {
-		  }
-		| undefined {
 		const providerConstructor = this.secretsProviders.getProvider(provider);
-		if (!providerConstructor) {
-			return undefined;
-		}
 		return {
 			provider: this.getProvider(provider) ?? new providerConstructor(),
 			settings: this.cachedSettings[provider] ?? {},
@@ -276,7 +265,7 @@ export class ExternalSecretsManager {
 
 	async setProviderSettings(provider: string, data: IDataObject, userId?: string) {
 		let isNewProvider = false;
-		let settings = await this.getDecryptedSettings(this.settingsRepo);
+		let settings = await this.getDecryptedSettings();
 		if (!settings) {
 			settings = {};
 		}
@@ -289,7 +278,7 @@ export class ExternalSecretsManager {
 			settings: data,
 		};
 
-		await this.saveAndSetSettings(settings, this.settingsRepo);
+		await this.saveAndSetSettings(settings);
 		this.cachedSettings = settings;
 		await this.reloadProvider(provider);
 		this.broadcastReloadExternalSecretsProviders();
@@ -298,7 +287,7 @@ export class ExternalSecretsManager {
 	}
 
 	async setProviderConnected(provider: string, connected: boolean) {
-		let settings = await this.getDecryptedSettings(this.settingsRepo);
+		let settings = await this.getDecryptedSettings();
 		if (!settings) {
 			settings = {};
 		}
@@ -308,7 +297,7 @@ export class ExternalSecretsManager {
 			settings: settings[provider]?.settings ?? {},
 		};
 
-		await this.saveAndSetSettings(settings, this.settingsRepo);
+		await this.saveAndSetSettings(settings);
 		this.cachedSettings = settings;
 		await this.reloadProvider(provider);
 		await this.updateSecrets();
@@ -333,9 +322,9 @@ export class ExternalSecretsManager {
 		return this.cipher.encrypt(settings);
 	}
 
-	async saveAndSetSettings(settings: ExternalSecretsSettings, settingsRepo: SettingsRepository) {
+	async saveAndSetSettings(settings: ExternalSecretsSettings) {
 		const encryptedSettings = this.encryptSecretsSettings(settings);
-		await settingsRepo.saveEncryptedSecretsProviderSettings(encryptedSettings);
+		await this.settingsRepo.saveEncryptedSecretsProviderSettings(encryptedSettings);
 	}
 
 	async testProviderSettings(

packages/cli/src/external-secrets.ee/external-secrets-providers.ee.ts

@@ -1,12 +1,11 @@
 import { Service } from '@n8n/di';
 
-import type { SecretsProvider } from '@/interfaces';
-
 import { AwsSecretsManager } from './providers/aws-secrets/aws-secrets-manager';
 import { AzureKeyVault } from './providers/azure-key-vault/azure-key-vault';
 import { GcpSecretsManager } from './providers/gcp-secrets-manager/gcp-secrets-manager';
 import { InfisicalProvider } from './providers/infisical';
 import { VaultProvider } from './providers/vault';
+import type { SecretsProvider } from './types';
 
 @Service()
 export class ExternalSecretsProviders {
@@ -18,8 +17,8 @@ export class ExternalSecretsProviders {
 		gcpSecretsManager: GcpSecretsManager,
 	};
 
-	getProvider(name: string): { new (): SecretsProvider } | null {
+	getProvider(name: string): { new (): SecretsProvider } {
-		return this.providers[name] ?? null;
+		return this.providers[name];
 	}
 
 	hasProvider(name: string) {

packages/cli/src/external-secrets.ee/external-secrets.config.ts

@@ -1,4 +1,4 @@
-import { Config, Env } from '../decorators';
+import { Config, Env } from '@n8n/config';
 
 @Config
 export class ExternalSecretsConfig {

packages/cli/src/external-secrets.ee/external-secrets.controller.ee.ts

@@ -1,15 +1,29 @@
-import { Response } from 'express';
+import { Request, Response, NextFunction } from 'express';
 
-import { Get, Post, RestController, GlobalScope } from '@/decorators';
+import { Get, Post, RestController, GlobalScope, Middleware } from '@/decorators';
-import { ExternalSecretsProviderNotFoundError } from '@/errors/external-secrets-provider-not-found.error';
 import { NotFoundError } from '@/errors/response-errors/not-found.error';
-import { ExternalSecretsRequest } from '@/requests';
 
+import { ExternalSecretsProviders } from './external-secrets-providers.ee';
 import { ExternalSecretsService } from './external-secrets.service.ee';
+import { ExternalSecretsRequest } from './types';
 
 @RestController('/external-secrets')
 export class ExternalSecretsController {
-	constructor(private readonly secretsService: ExternalSecretsService) {}
+	constructor(
+		private readonly secretsService: ExternalSecretsService,
+		private readonly secretsProviders: ExternalSecretsProviders,
+	) {}
+
+	@Middleware()
+	validateProviderName(req: Request, _: Response, next: NextFunction) {
+		if ('provider' in req.params) {
+			const { provider } = req.params;
+			if (!this.secretsProviders.hasProvider(provider)) {
+				throw new NotFoundError(`Could not find provider "${provider}"`);
+			}
+		}
+		next();
+	}
 
 	@Get('/providers')
 	@GlobalScope('externalSecretsProvider:list')
@@ -21,48 +35,27 @@ export class ExternalSecretsController {
 	@GlobalScope('externalSecretsProvider:read')
 	async getProvider(req: ExternalSecretsRequest.GetProvider) {
 		const providerName = req.params.provider;
-		try {
+		return this.secretsService.getProvider(providerName);
-			return this.secretsService.getProvider(providerName);
-		} catch (e) {
-			if (e instanceof ExternalSecretsProviderNotFoundError) {
-				throw new NotFoundError(`Could not find provider "${e.providerName}"`);
-			}
-			throw e;
-		}
 	}
 
 	@Post('/providers/:provider/test')
 	@GlobalScope('externalSecretsProvider:read')
 	async testProviderSettings(req: ExternalSecretsRequest.TestProviderSettings, res: Response) {
 		const providerName = req.params.provider;
-		try {
+		const result = await this.secretsService.testProviderSettings(providerName, req.body);
-			const result = await this.secretsService.testProviderSettings(providerName, req.body);
+		if (result.success) {
-			if (result.success) {
+			res.statusCode = 200;
-				res.statusCode = 200;
+		} else {
-			} else {
+			res.statusCode = 400;
-				res.statusCode = 400;
-			}
-			return result;
-		} catch (e) {
-			if (e instanceof ExternalSecretsProviderNotFoundError) {
-				throw new NotFoundError(`Could not find provider "${e.providerName}"`);
-			}
-			throw e;
 		}
+		return result;
 	}
 
 	@Post('/providers/:provider')
 	@GlobalScope('externalSecretsProvider:create')
 	async setProviderSettings(req: ExternalSecretsRequest.SetProviderSettings) {
 		const providerName = req.params.provider;
-		try {
+		await this.secretsService.saveProviderSettings(providerName, req.body, req.user.id);
-			await this.secretsService.saveProviderSettings(providerName, req.body, req.user.id);
-		} catch (e) {
-			if (e instanceof ExternalSecretsProviderNotFoundError) {
-				throw new NotFoundError(`Could not find provider "${e.providerName}"`);
-			}
-			throw e;
-		}
 		return {};
 	}
 
@@ -70,14 +63,7 @@ export class ExternalSecretsController {
 	@GlobalScope('externalSecretsProvider:update')
 	async setProviderConnected(req: ExternalSecretsRequest.SetProviderConnected) {
 		const providerName = req.params.provider;
-		try {
+		await this.secretsService.saveProviderConnected(providerName, req.body.connected);
-			await this.secretsService.saveProviderConnected(providerName, req.body.connected);
-		} catch (e) {
-			if (e instanceof ExternalSecretsProviderNotFoundError) {
-				throw new NotFoundError(`Could not find provider "${e.providerName}"`);
-			}
-			throw e;
-		}
 		return {};
 	}
 
@@ -85,20 +71,13 @@ export class ExternalSecretsController {
 	@GlobalScope('externalSecretsProvider:sync')
 	async updateProvider(req: ExternalSecretsRequest.UpdateProvider, res: Response) {
 		const providerName = req.params.provider;
-		try {
+		const resp = await this.secretsService.updateProvider(providerName);
-			const resp = await this.secretsService.updateProvider(providerName);
+		if (resp) {
-			if (resp) {
+			res.statusCode = 200;
-				res.statusCode = 200;
+		} else {
-			} else {
+			res.statusCode = 400;
-				res.statusCode = 400;
-			}
-			return { updated: resp };
-		} catch (e) {
-			if (e instanceof ExternalSecretsProviderNotFoundError) {
-				throw new NotFoundError(`Could not find provider "${e.providerName}"`);
-			}
-			throw e;
 		}
+		return { updated: resp };
 	}
 
 	@Get('/secrets')

packages/cli/src/external-secrets.ee/external-secrets.service.ee.ts

@@ -3,20 +3,15 @@ import type { IDataObject } from 'n8n-workflow';
 import { deepCopy } from 'n8n-workflow';
 
 import { CREDENTIAL_BLANKING_VALUE } from '@/constants';
-import { ExternalSecretsProviderNotFoundError } from '@/errors/external-secrets-provider-not-found.error';
-import type { SecretsProvider } from '@/interfaces';
-import type { ExternalSecretsRequest } from '@/requests';
 
 import { ExternalSecretsManager } from './external-secrets-manager.ee';
+import type { ExternalSecretsRequest, SecretsProvider } from './types';
 
 @Service()
 export class ExternalSecretsService {
 	getProvider(providerName: string): ExternalSecretsRequest.GetProviderResponse | null {
 		const providerAndSettings =
 			Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
-		if (!providerAndSettings) {
-			throw new ExternalSecretsProviderNotFoundError(providerName);
-		}
 		const { provider, settings } = providerAndSettings;
 		return {
 			displayName: provider.displayName,
@@ -106,20 +101,12 @@ export class ExternalSecretsService {
 	async saveProviderSettings(providerName: string, data: IDataObject, userId: string) {
 		const providerAndSettings =
 			Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
-		if (!providerAndSettings) {
-			throw new ExternalSecretsProviderNotFoundError(providerName);
-		}
 		const { settings } = providerAndSettings;
 		const newData = this.unredact(data, settings.settings);
 		await Container.get(ExternalSecretsManager).setProviderSettings(providerName, newData, userId);
 	}
 
 	async saveProviderConnected(providerName: string, connected: boolean) {
-		const providerAndSettings =
-			Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
-		if (!providerAndSettings) {
-			throw new ExternalSecretsProviderNotFoundError(providerName);
-		}
 		await Container.get(ExternalSecretsManager).setProviderConnected(providerName, connected);
 		return this.getProvider(providerName);
 	}
@@ -131,20 +118,12 @@ export class ExternalSecretsService {
 	async testProviderSettings(providerName: string, data: IDataObject) {
 		const providerAndSettings =
 			Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
-		if (!providerAndSettings) {
-			throw new ExternalSecretsProviderNotFoundError(providerName);
-		}
 		const { settings } = providerAndSettings;
 		const newData = this.unredact(data, settings.settings);
 		return await Container.get(ExternalSecretsManager).testProviderSettings(providerName, newData);
 	}
 
 	async updateProvider(providerName: string) {
-		const providerAndSettings =
-			Container.get(ExternalSecretsManager).getProviderWithSettings(providerName);
-		if (!providerAndSettings) {
-			throw new ExternalSecretsProviderNotFoundError(providerName);
-		}
 		return await Container.get(ExternalSecretsManager).updateProvider(providerName);
 	}
 }

packages/cli/src/external-secrets.ee/providers/aws-secrets/aws-secrets-manager.ts

@@ -2,12 +2,11 @@ import { Container } from '@n8n/di';
 import { Logger } from 'n8n-core';
 import type { INodeProperties } from 'n8n-workflow';
 
-import { UnknownAuthTypeError } from '@/errors/unknown-auth-type.error';
-import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '@/external-secrets.ee/constants';
-import type { SecretsProvider, SecretsProviderState } from '@/interfaces';
-
 import { AwsSecretsClient } from './aws-secrets-client';
 import type { AwsSecretsManagerContext } from './types';
+import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '../../constants';
+import { UnknownAuthTypeError } from '../../errors/unknown-auth-type.error';
+import type { SecretsProvider, SecretsProviderState } from '../../types';
 
 export class AwsSecretsManager implements SecretsProvider {
 	name = 'awsSecretsManager';

packages/cli/src/external-secrets.ee/providers/aws-secrets/types.ts

@@ -1,4 +1,4 @@
-import type { SecretsProviderSettings } from '@/interfaces';
+import type { SecretsProviderSettings } from '../../types';
 
 export type SecretsNamesPage = {
 	NextToken?: string;

packages/cli/src/external-secrets.ee/providers/azure-key-vault/azure-key-vault.ts

@@ -4,10 +4,9 @@ import { Logger } from 'n8n-core';
 import { ensureError } from 'n8n-workflow';
 import type { INodeProperties } from 'n8n-workflow';
 
-import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '@/external-secrets.ee/constants';
-import type { SecretsProvider, SecretsProviderState } from '@/interfaces';
-
 import type { AzureKeyVaultContext } from './types';
+import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '../../constants';
+import type { SecretsProvider, SecretsProviderState } from '../../types';
 
 export class AzureKeyVault implements SecretsProvider {
 	name = 'azureKeyVault';

packages/cli/src/external-secrets.ee/providers/azure-key-vault/types.ts

@@ -1,4 +1,4 @@
-import type { SecretsProviderSettings } from '@/interfaces';
+import type { SecretsProviderSettings } from '../../types';
 
 export type AzureKeyVaultContext = SecretsProviderSettings<{
 	vaultName: string;

packages/cli/src/external-secrets.ee/providers/gcp-secrets-manager/gcp-secrets-manager.ts

@@ -3,14 +3,13 @@ import { Container } from '@n8n/di';
 import { Logger } from 'n8n-core';
 import { ensureError, jsonParse, type INodeProperties } from 'n8n-workflow';
 
-import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '@/external-secrets.ee/constants';
-import type { SecretsProvider, SecretsProviderState } from '@/interfaces';
-
 import type {
 	GcpSecretsManagerContext,
 	GcpSecretAccountKey,
 	RawGcpSecretAccountKey,
 } from './types';
+import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '../../constants';
+import type { SecretsProvider, SecretsProviderState } from '../../types';
 
 export class GcpSecretsManager implements SecretsProvider {
 	name = 'gcpSecretsManager';

packages/cli/src/external-secrets.ee/providers/gcp-secrets-manager/types.ts

@@ -1,4 +1,4 @@
-import type { SecretsProviderSettings } from '@/interfaces';
+import type { SecretsProviderSettings } from '../../types';
 
 type JsonString = string;
 

packages/cli/src/external-secrets.ee/providers/infisical.ts

@@ -3,9 +3,8 @@ import { getServiceTokenData } from 'infisical-node/lib/api/serviceTokenData';
 import { populateClientWorkspaceConfigsHelper } from 'infisical-node/lib/helpers/key';
 import { UnexpectedError, type IDataObject, type INodeProperties } from 'n8n-workflow';
 
-import type { SecretsProvider, SecretsProviderSettings, SecretsProviderState } from '@/interfaces';
-
 import { EXTERNAL_SECRETS_NAME_REGEX } from '../constants';
+import type { SecretsProvider, SecretsProviderSettings, SecretsProviderState } from '../types';
 
 export interface InfisicalSettings {
 	token: string;

packages/cli/src/external-secrets.ee/providers/vault.ts

@@ -4,11 +4,10 @@ import axios from 'axios';
 import { Logger } from 'n8n-core';
 import type { IDataObject, INodeProperties } from 'n8n-workflow';
 
-import type { SecretsProviderSettings, SecretsProviderState } from '@/interfaces';
-import { SecretsProvider } from '@/interfaces';
-
 import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '../constants';
-import { preferGet } from '../external-secrets-helper.ee';
+import { ExternalSecretsConfig } from '../external-secrets.config';
+import type { SecretsProviderSettings, SecretsProviderState } from '../types';
+import { SecretsProvider } from '../types';
 
 type VaultAuthMethod = 'token' | 'usernameAndPassword' | 'appRole';
 
@@ -419,7 +418,7 @@ export class VaultProvider extends SecretsProvider {
 		listPath += path;
 		let listResp: AxiosResponse<VaultResponse<VaultSecretList>>;
 		try {
-			const shouldPreferGet = preferGet();
+			const shouldPreferGet = Container.get(ExternalSecretsConfig).preferGet;
 			const url = `${listPath}${shouldPreferGet ? '?list=true' : ''}`;
 			// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
 			const method = shouldPreferGet ? 'GET' : ('LIST' as any);

packages/cli/src/external-secrets.ee/types.ts

@@ -0,0 +1,56 @@
+import type { IDataObject, INodeProperties } from 'n8n-workflow';
+
+import type { AuthenticatedRequest } from '@/requests';
+
+export interface SecretsProviderSettings<T = IDataObject> {
+	connected: boolean;
+	connectedAt: Date | null;
+	settings: T;
+}
+
+export interface ExternalSecretsSettings {
+	[key: string]: SecretsProviderSettings;
+}
+
+export type SecretsProviderState = 'initializing' | 'connected' | 'error';
+
+export abstract class SecretsProvider {
+	displayName: string;
+
+	name: string;
+
+	properties: INodeProperties[];
+
+	state: SecretsProviderState;
+
+	abstract init(settings: SecretsProviderSettings): Promise<void>;
+	abstract connect(): Promise<void>;
+	abstract disconnect(): Promise<void>;
+	abstract update(): Promise<void>;
+	abstract test(): Promise<[boolean] | [boolean, string]>;
+	abstract getSecret(name: string): unknown;
+	abstract hasSecret(name: string): boolean;
+	abstract getSecretNames(): string[];
+}
+
+export declare namespace ExternalSecretsRequest {
+	type GetProviderResponse = Pick<SecretsProvider, 'displayName' | 'name' | 'properties'> & {
+		icon: string;
+		connected: boolean;
+		connectedAt: Date | null;
+		state: SecretsProviderState;
+		data: IDataObject;
+	};
+
+	type GetProviders = AuthenticatedRequest;
+	type GetProvider = AuthenticatedRequest<{ provider: string }, GetProviderResponse>;
+	type SetProviderSettings = AuthenticatedRequest<{ provider: string }, {}, IDataObject>;
+	type TestProviderSettings = SetProviderSettings;
+	type SetProviderConnected = AuthenticatedRequest<
+		{ provider: string },
+		{},
+		{ connected: boolean }
+	>;
+
+	type UpdateProvider = AuthenticatedRequest<{ provider: string }>;
+}

packages/cli/src/interfaces.ts

@@ -5,7 +5,6 @@ import type {
 	ICredentialDataDecryptedObject,
 	ICredentialsDecrypted,
 	ICredentialsEncrypted,
-	IDataObject,
 	IDeferredPromise,
 	IExecuteResponsePromiseData,
 	IRun,
@@ -17,7 +16,6 @@ import type {
 	ExecutionStatus,
 	ExecutionSummary,
 	FeatureFlags,
-	INodeProperties,
 	IUserSettings,
 	IWorkflowExecutionDataProcess,
 	DeduplicationMode,
@@ -364,34 +362,3 @@ export interface N8nApp {
 }
 
 export type UserSettings = Pick<User, 'id' | 'settings'>;
-
-export interface SecretsProviderSettings<T = IDataObject> {
-	connected: boolean;
-	connectedAt: Date | null;
-	settings: T;
-}
-
-export interface ExternalSecretsSettings {
-	[key: string]: SecretsProviderSettings;
-}
-
-export type SecretsProviderState = 'initializing' | 'connected' | 'error';
-
-export abstract class SecretsProvider {
-	displayName: string;
-
-	name: string;
-
-	properties: INodeProperties[];
-
-	state: SecretsProviderState;
-
-	abstract init(settings: SecretsProviderSettings): Promise<void>;
-	abstract connect(): Promise<void>;
-	abstract disconnect(): Promise<void>;
-	abstract update(): Promise<void>;
-	abstract test(): Promise<[boolean] | [boolean, string]>;
-	abstract getSecret(name: string): unknown;
-	abstract hasSecret(name: string): boolean;
-	abstract getSecretNames(): string[];
-}

packages/cli/src/requests.ts

@@ -3,7 +3,6 @@ import type { AssignableRole, GlobalRole, Scope } from '@n8n/permissions';
 import type express from 'express';
 import type {
 	ICredentialDataDecryptedObject,
-	IDataObject,
 	INodeCredentialTestRequest,
 	IPersonalizationSurveyAnswersV4,
 	IUser,
@@ -15,9 +14,7 @@ import type { User } from '@/databases/entities/user';
 import type { Variables } from '@/databases/entities/variables';
 import type { WorkflowEntity } from '@/databases/entities/workflow-entity';
 import type { WorkflowHistory } from '@/databases/entities/workflow-history';
-import type { SecretsProvider, SecretsProviderState } from '@/interfaces';
+import type { ScopesField } from '@/services/role.service';
-
-import type { ScopesField } from './services/role.service';
 
 export type APIRequest<
 	RouteParams = {},
@@ -310,28 +307,6 @@ export declare namespace VariablesRequest {
 	type Delete = Get;
 }
 
-export declare namespace ExternalSecretsRequest {
-	type GetProviderResponse = Pick<SecretsProvider, 'displayName' | 'name' | 'properties'> & {
-		icon: string;
-		connected: boolean;
-		connectedAt: Date | null;
-		state: SecretsProviderState;
-		data: IDataObject;
-	};
-
-	type GetProviders = AuthenticatedRequest;
-	type GetProvider = AuthenticatedRequest<{ provider: string }, GetProviderResponse>;
-	type SetProviderSettings = AuthenticatedRequest<{ provider: string }, {}, IDataObject>;
-	type TestProviderSettings = SetProviderSettings;
-	type SetProviderConnected = AuthenticatedRequest<
-		{ provider: string },
-		{},
-		{ connected: boolean }
-	>;
-
-	type UpdateProvider = AuthenticatedRequest<{ provider: string }>;
-}
-
 // ----------------------------------
 //           /workflow-history
 // ----------------------------------

packages/cli/test/integration/external-secrets/external-secrets.api.test.ts

@@ -9,7 +9,7 @@ import { SettingsRepository } from '@/databases/repositories/settings.repository
 import type { EventService } from '@/events/event.service';
 import { ExternalSecretsManager } from '@/external-secrets.ee/external-secrets-manager.ee';
 import { ExternalSecretsProviders } from '@/external-secrets.ee/external-secrets-providers.ee';
-import type { ExternalSecretsSettings, SecretsProviderState } from '@/interfaces';
+import type { ExternalSecretsSettings, SecretsProviderState } from '@/external-secrets.ee/types';
 import { License } from '@/license';
 
 import {
@@ -60,6 +60,7 @@ const resetManager = async () => {
 		ExternalSecretsManager,
 		new ExternalSecretsManager(
 			logger,
+			mock(),
 			Container.get(SettingsRepository),
 			Container.get(License),
 			mockProvidersInstance,
@@ -114,6 +115,7 @@ beforeAll(async () => {
 		ExternalSecretsManager,
 		new ExternalSecretsManager(
 			logger,
+			mock(),
 			Container.get(SettingsRepository),
 			Container.get(License),
 			mockProvidersInstance,

packages/cli/test/integration/shared/utils/test-server.ts

@@ -1,7 +1,6 @@
 import { Container } from '@n8n/di';
 import cookieParser from 'cookie-parser';
 import express from 'express';
-import { Logger } from 'n8n-core';
 import type superagent from 'superagent';
 import request from 'supertest';
 import { URL } from 'url';
@@ -18,7 +17,7 @@ import { Push } from '@/push';
 import type { APIRequest } from '@/requests';
 import { Telemetry } from '@/telemetry';
 
-import { mockInstance } from '../../../shared/mocking';
+import { mockInstance, mockLogger } from '../../../shared/mocking';
 import { PUBLIC_API_REST_PATH_SEGMENT, REST_PATH_SEGMENT } from '../constants';
 import { LicenseMocker } from '../license';
 import * as testDb from '../test-db';
@@ -101,7 +100,7 @@ export const setupTestServer = ({
 	});
 
 	// Mock all telemetry and logging
-	mockInstance(Logger);
+	mockLogger();
 	mockInstance(PostHogClient);
 	mockInstance(Push);
 	mockInstance(Telemetry);

packages/cli/test/shared/external-secrets/utils.ts

@@ -1,7 +1,7 @@
 import type { IDataObject, INodeProperties } from 'n8n-workflow';
 
-import { SecretsProvider } from '@/interfaces';
+import { SecretsProvider } from '@/external-secrets.ee/types';
-import type { SecretsProviderSettings, SecretsProviderState } from '@/interfaces';
+import type { SecretsProviderSettings, SecretsProviderState } from '@/external-secrets.ee/types';
 
 export class MockProviders {
 	providers: Record<string, { new (): SecretsProvider }> = {
@@ -12,8 +12,8 @@ export class MockProviders {
 		this.providers = providers;
 	}
 
-	getProvider(name: string): { new (): SecretsProvider } | null {
+	getProvider(name: string): { new (): SecretsProvider } {
-		return this.providers[name] ?? null;
+		return this.providers[name];
 	}
 
 	hasProvider(name: string) {
@@ -93,6 +93,10 @@ export class DummyProvider extends SecretsProvider {
 	}
 }
 
+export class AnotherDummyProvider extends DummyProvider {
+	name = 'another_dummy';
+}
+
 export class ErrorProvider extends SecretsProvider {
 	secrets: Record<string, string> = {};
 
@@ -112,7 +116,7 @@ export class ErrorProvider extends SecretsProvider {
 	}
 
 	async disconnect(): Promise<void> {
-		throw new Error();
+		// no-op
 	}
 
 	async update(): Promise<void> {

packages/cli/test/shared/mocking.ts

@@ -1,8 +1,7 @@
 import { Container } from '@n8n/di';
 import { DataSource, EntityManager, type EntityMetadata } from '@n8n/typeorm';
 import { mock } from 'jest-mock-extended';
-import type { Class } from 'n8n-core';
+import type { Cipher, Class, Logger } from 'n8n-core';
-import type { Logger } from 'n8n-core';
 import type { DeepPartial } from 'ts-essentials';
 
 export const mockInstance = <T>(
@@ -25,3 +24,9 @@ export const mockEntityManager = (entityClass: Class) => {
 };
 
 export const mockLogger = () => mock<Logger>({ scoped: jest.fn().mockReturnValue(mock<Logger>()) });
+
+export const mockCipher = () =>
+	mock<Cipher>({
+		encrypt: (data) => (typeof data === 'string' ? data : JSON.stringify(data)),
+		decrypt: (data) => data,
+	});