diff --git a/packages/cli/package.json b/packages/cli/package.json index 63c7eddc1b..c5151582f6 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -86,9 +86,10 @@ "ts-essentials": "^7.0.3" }, "dependencies": { - "@azure/identity": "^4.3.0", - "@azure/keyvault-secrets": "^4.8.0", - "@google-cloud/secret-manager": "^5.6.0", + "@aws-sdk/client-secrets-manager": "3.666.0", + "@azure/identity": "4.3.0", + "@azure/keyvault-secrets": "4.8.0", + "@google-cloud/secret-manager": "5.6.0", "@n8n/api-types": "workspace:*", "@n8n/client-oauth2": "workspace:*", "@n8n/config": "workspace:*", diff --git a/packages/cli/src/external-secrets.ee/external-secrets-providers.ee.ts b/packages/cli/src/external-secrets.ee/external-secrets-providers.ee.ts index 30b01bc859..fdf48f0eb9 100644 --- a/packages/cli/src/external-secrets.ee/external-secrets-providers.ee.ts +++ b/packages/cli/src/external-secrets.ee/external-secrets-providers.ee.ts @@ -1,6 +1,6 @@ import { Service } from '@n8n/di'; -import { AwsSecretsManager } from './providers/aws-secrets/aws-secrets-manager'; +import { AwsSecretsManager } from './providers/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'; diff --git a/packages/cli/src/external-secrets.ee/providers/__tests__/aws-secrets-manager.test.ts b/packages/cli/src/external-secrets.ee/providers/__tests__/aws-secrets-manager.test.ts new file mode 100644 index 0000000000..a25e8ac2a2 --- /dev/null +++ b/packages/cli/src/external-secrets.ee/providers/__tests__/aws-secrets-manager.test.ts @@ -0,0 +1,168 @@ +import { SecretsManager } from '@aws-sdk/client-secrets-manager'; +import { mock } from 'jest-mock-extended'; + +import { AwsSecretsManager, type AwsSecretsManagerContext } from '../aws-secrets-manager'; + +jest.mock('@aws-sdk/client-secrets-manager'); + +describe('AwsSecretsManager', () => { + const region = 'eu-central-1'; + const accessKeyId = 'FAKE-ACCESS-KEY-ID'; + const secretAccessKey = 'FAKE-SECRET'; + + const context = mock(); + const listSecretsSpy = jest.spyOn(SecretsManager.prototype, 'listSecrets'); + const batchGetSpy = jest.spyOn(SecretsManager.prototype, 'batchGetSecretValue'); + + let awsSecretsManager: AwsSecretsManager; + + beforeEach(() => { + jest.resetAllMocks(); + + awsSecretsManager = new AwsSecretsManager(); + }); + + describe('IAM User authentication', () => { + it('should fail to connect with invalid credentials', async () => { + context.settings = { + region, + authMethod: 'iamUser', + accessKeyId: 'invalid', + secretAccessKey: 'invalid', + }; + + await awsSecretsManager.init(context); + + listSecretsSpy.mockImplementation(() => { + throw new Error('Invalid credentials'); + }); + + await awsSecretsManager.connect(); + + expect(awsSecretsManager.state).toBe('error'); + }); + }); + + it('should update cached secrets', async () => { + context.settings = { + region, + authMethod: 'iamUser', + accessKeyId, + secretAccessKey, + }; + + await awsSecretsManager.init(context); + + listSecretsSpy.mockImplementation(async () => { + return { + SecretList: [{ Name: 'secret1' }, { Name: 'secret2' }], + }; + }); + + batchGetSpy.mockImplementation(async () => { + return { + SecretValues: [ + { Name: 'secret1', SecretString: 'value1' }, + { Name: 'secret2', SecretString: 'value2' }, + ], + }; + }); + + await awsSecretsManager.update(); + + expect(listSecretsSpy).toHaveBeenCalledTimes(1); + expect(batchGetSpy).toHaveBeenCalledWith({ + SecretIdList: expect.arrayContaining(['secret1', 'secret2']), + }); + + expect(awsSecretsManager.getSecret('secret1')).toBe('value1'); + expect(awsSecretsManager.getSecret('secret2')).toBe('value2'); + }); + + it('should properly batch secret requests', async () => { + context.settings = { + region, + authMethod: 'iamUser', + accessKeyId, + secretAccessKey, + }; + await awsSecretsManager.init(context); + + // Generate 25 secrets to test batching (default batch size is 20) + const secretsList = Array(25) + .fill(0) + .map((_, i) => ({ Name: `secret${i}` })); + + listSecretsSpy.mockImplementation(async () => { + return { SecretList: secretsList }; + }); + + batchGetSpy.mockImplementation(async (params) => { + const secretValues = (params.SecretIdList || []).map((secretId) => ({ + Name: secretId, + SecretString: `${secretId}-value`, + })); + return { SecretValues: secretValues }; + }); + + await awsSecretsManager.update(); + + // Should have been called twice for 25 secrets with batch size 20 + expect(batchGetSpy).toHaveBeenCalledTimes(2); + + // First batch should have 20 secrets + expect(batchGetSpy.mock.calls[0][0].SecretIdList?.length).toBe(20); + + // Second batch should have 5 secrets + expect(batchGetSpy.mock.calls[1][0].SecretIdList?.length).toBe(5); + + // Check a few secrets + expect(awsSecretsManager.getSecret('secret0')).toBe('secret0-value'); + expect(awsSecretsManager.getSecret('secret24')).toBe('secret24-value'); + }); + + it('should handle pagination in listing secrets', async () => { + context.settings = { + region, + authMethod: 'iamUser', + accessKeyId, + secretAccessKey, + }; + await awsSecretsManager.init(context); + + // First call with NextToken + listSecretsSpy.mockImplementationOnce(async () => { + return { + SecretList: [{ Name: 'secret1' }, { Name: 'secret2' }], + NextToken: 'next-page-token', + }; + }); + + // Second call with no NextToken + listSecretsSpy.mockImplementationOnce(async () => { + return { + SecretList: [{ Name: 'secret3' }], + }; + }); + + batchGetSpy.mockImplementation(async (params) => { + const secretValues = []; + for (const secretId of params.SecretIdList || []) { + secretValues.push({ + Name: secretId, + SecretString: `${secretId}-value`, + }); + } + return { SecretValues: secretValues }; + }); + + await awsSecretsManager.update(); + + expect(listSecretsSpy).toHaveBeenCalledWith({ NextToken: 'next-page-token' }); + expect(listSecretsSpy).toHaveBeenCalledWith({ NextToken: undefined }); + + expect(awsSecretsManager.getSecret('secret1')).toBe('secret1-value'); + expect(awsSecretsManager.getSecret('secret2')).toBe('secret2-value'); + expect(awsSecretsManager.getSecret('secret3')).toBe('secret3-value'); + }); +}); diff --git a/packages/cli/src/external-secrets.ee/providers/aws-secrets/aws-secrets-manager.ts b/packages/cli/src/external-secrets.ee/providers/aws-secrets-manager.ts similarity index 50% rename from packages/cli/src/external-secrets.ee/providers/aws-secrets/aws-secrets-manager.ts rename to packages/cli/src/external-secrets.ee/providers/aws-secrets-manager.ts index 52a6cbf667..ba389900cf 100644 --- a/packages/cli/src/external-secrets.ee/providers/aws-secrets/aws-secrets-manager.ts +++ b/packages/cli/src/external-secrets.ee/providers/aws-secrets-manager.ts @@ -1,12 +1,29 @@ +import { SecretsManager, type SecretsManagerClientConfig } from '@aws-sdk/client-secrets-manager'; import { Container } from '@n8n/di'; import { Logger } from 'n8n-core'; import type { INodeProperties } from 'n8n-workflow'; -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'; +import { DOCS_HELP_NOTICE, EXTERNAL_SECRETS_NAME_REGEX } from '../constants'; +import { UnknownAuthTypeError } from '../errors/unknown-auth-type.error'; +import type { SecretsProvider, SecretsProviderSettings, SecretsProviderState } from '../types'; + +type Secret = { + secretName: string; + secretValue: string; +}; + +export type AwsSecretsManagerContext = SecretsProviderSettings< + { + region: string; + } & ( + | { + authMethod: 'iamUser'; + accessKeyId: string; + secretAccessKey: string; + } + | { authMethod: 'autoDetect' } + ) +>; export class AwsSecretsManager implements SecretsProvider { name = 'awsSecretsManager'; @@ -37,6 +54,12 @@ export class AwsSecretsManager implements SecretsProvider { description: 'Credentials for IAM user having secretsmanager:ListSecrets and secretsmanager:BatchGetSecretValue permissions. Learn more', }, + { + name: 'Auto Detect', + value: 'autoDetect', + description: + 'Use automatic credential detection to authenticate AWS calls for external secretsLearn more.', + }, ], default: 'iamUser', required: true, @@ -75,7 +98,7 @@ export class AwsSecretsManager implements SecretsProvider { private cachedSecrets: Record = {}; - private client: AwsSecretsClient; + private client: SecretsManager; constructor(private readonly logger = Container.get(Logger)) { this.logger = this.logger.scoped('external-secrets'); @@ -84,13 +107,27 @@ export class AwsSecretsManager implements SecretsProvider { async init(context: AwsSecretsManagerContext) { this.assertAuthType(context); - this.client = new AwsSecretsClient(context.settings); + const { region, authMethod } = context.settings; + const clientConfig: SecretsManagerClientConfig = { region }; + + if (authMethod === 'iamUser') { + const { accessKeyId, secretAccessKey } = context.settings; + clientConfig.credentials = { accessKeyId, secretAccessKey }; + } + + this.client = new SecretsManager(clientConfig); this.logger.debug('AWS Secrets Manager provider initialized'); } - async test() { - return await this.client.checkConnection(); + async test(): Promise<[boolean] | [boolean, string]> { + try { + await this.client.listSecrets({ MaxResults: 1 }); + return [true]; + } catch (e) { + const error = e instanceof Error ? e : new Error(`${e}`); + return [false, error.message]; + } } async connect() { @@ -110,7 +147,7 @@ export class AwsSecretsManager implements SecretsProvider { } async update() { - const secrets = await this.client.fetchAllSecrets(); + const secrets = await this.fetchAllSecrets(); const supportedSecrets = secrets.filter((s) => EXTERNAL_SECRETS_NAME_REGEX.test(s.secretName)); @@ -134,8 +171,60 @@ export class AwsSecretsManager implements SecretsProvider { } private assertAuthType(context: AwsSecretsManagerContext) { - if (context.settings.authMethod === 'iamUser') return; + const { authMethod } = context.settings; + if (authMethod === 'iamUser' || authMethod === 'autoDetect') return; + throw new UnknownAuthTypeError(authMethod); + } - throw new UnknownAuthTypeError(context.settings.authMethod); + private async fetchAllSecretsNames() { + const names: string[] = []; + let nextToken: string | undefined; + + do { + const response = await this.client.listSecrets({ + NextToken: nextToken, + }); + + if (response.SecretList) { + names.push(...response.SecretList.filter((s) => s.Name).map((s) => s.Name!)); + } + + nextToken = response.NextToken; + } while (nextToken); + + return names; + } + + private async fetchAllSecrets() { + const secrets: Secret[] = []; + const secretNames = await this.fetchAllSecretsNames(); + + // Batch the requests to avoid hitting limits + const batches = this.batch(secretNames); + + for (const batch of batches) { + const response = await this.client.batchGetSecretValue({ + SecretIdList: batch, + }); + + if (response.SecretValues) { + for (const secret of response.SecretValues) { + if (secret.Name && secret.SecretString) { + secrets.push({ + secretName: secret.Name, + secretValue: secret.SecretString, + }); + } + } + } + } + + return secrets; + } + + private batch(arr: T[], size = 20): T[][] { + return Array.from({ length: Math.ceil(arr.length / size) }, (_, index) => + arr.slice(index * size, (index + 1) * size), + ); } } diff --git a/packages/cli/src/external-secrets.ee/providers/aws-secrets/aws-secrets-client.ts b/packages/cli/src/external-secrets.ee/providers/aws-secrets/aws-secrets-client.ts deleted file mode 100644 index e8b4c3cce2..0000000000 --- a/packages/cli/src/external-secrets.ee/providers/aws-secrets/aws-secrets-client.ts +++ /dev/null @@ -1,152 +0,0 @@ -import * as aws4 from 'aws4'; -import type { Request as Aws4Options } from 'aws4'; -import axios from 'axios'; -import type { AxiosRequestConfig } from 'axios'; - -import type { - AwsSecretsManagerContext, - ConnectionTestResult, - Secret, - SecretsNamesPage, - SecretsPage, - AwsSecretsClientSettings, -} from './types'; - -export class AwsSecretsClient { - private settings: AwsSecretsClientSettings = { - region: '', - host: '', - url: '', - accessKeyId: '', - secretAccessKey: '', - }; - - constructor(settings: AwsSecretsManagerContext['settings']) { - const { region, accessKeyId, secretAccessKey } = settings; - - this.settings = { - region, - host: `secretsmanager.${region}.amazonaws.com`, - url: `https://secretsmanager.${region}.amazonaws.com`, - accessKeyId, - secretAccessKey, - }; - } - - /** - * Check whether the client can connect to AWS Secrets Manager. - */ - async checkConnection(): ConnectionTestResult { - try { - await this.fetchSecretsNamesPage(); - return [true]; - } catch (e) { - const error = e instanceof Error ? e : new Error(`${e}`); - return [false, error.message]; - } - } - - /** - * Fetch all secrets from AWS Secrets Manager. - */ - async fetchAllSecrets() { - const secrets: Secret[] = []; - - const allSecretsNames = await this.fetchAllSecretsNames(); - - const batches = this.batch(allSecretsNames); - - for (const batch of batches) { - const page = await this.fetchSecretsPage(batch); - - secrets.push( - ...page.SecretValues.map((s) => ({ secretName: s.Name, secretValue: s.SecretString })), - ); - } - - return secrets; - } - - private batch(arr: T[], size = 20): T[][] { - return Array.from({ length: Math.ceil(arr.length / size) }, (_, index) => - arr.slice(index * size, (index + 1) * size), - ); - } - - private toRequestOptions( - action: 'ListSecrets' | 'BatchGetSecretValue', - body: string, - ): Aws4Options { - return { - method: 'POST', - service: 'secretsmanager', - region: this.settings.region, - host: this.settings.host, - headers: { - 'X-Amz-Target': `secretsmanager.${action}`, - 'Content-Type': 'application/x-amz-json-1.1', - }, - body, - }; - } - - /** - * @doc https://docs.aws.amazon.com/secretsmanager/latest/apireference/API_BatchGetSecretValue.html - */ - private async fetchSecretsPage(secretsNames: string[], nextToken?: string) { - const body = JSON.stringify( - nextToken - ? { SecretIdList: secretsNames, NextToken: nextToken } - : { SecretIdList: secretsNames }, - ); - - const options = this.toRequestOptions('BatchGetSecretValue', body); - const { headers } = aws4.sign(options, this.settings); - - const config: AxiosRequestConfig = { - method: 'POST', - url: this.settings.url, - headers, - data: body, - }; - - const response = await axios.request(config); - - return response.data; - } - - private async fetchAllSecretsNames() { - const names: string[] = []; - - let nextToken: string | undefined; - - do { - const page = await this.fetchSecretsNamesPage(nextToken); - names.push(...page.SecretList.map((s) => s.Name)); - nextToken = page.NextToken; - } while (nextToken); - - return names; - } - - /** - * @doc https://docs.aws.amazon.com/secretsmanager/latest/apireference/API_ListSecrets.html - */ - private async fetchSecretsNamesPage(nextToken?: string) { - const body = JSON.stringify(nextToken ? { NextToken: nextToken } : {}); - - const options = this.toRequestOptions('ListSecrets', body); - const { headers } = aws4.sign(options, this.settings); - - const config: AxiosRequestConfig = { - method: 'POST', - url: this.settings.url, - headers, - data: body, - }; - - const response = await axios.request(config); - - return response.data; - } -} diff --git a/packages/cli/src/external-secrets.ee/providers/aws-secrets/types.ts b/packages/cli/src/external-secrets.ee/providers/aws-secrets/types.ts deleted file mode 100644 index d6057154b5..0000000000 --- a/packages/cli/src/external-secrets.ee/providers/aws-secrets/types.ts +++ /dev/null @@ -1,50 +0,0 @@ -import type { SecretsProviderSettings } from '../../types'; - -export type SecretsNamesPage = { - NextToken?: string; - SecretList: SecretName[]; -}; - -export type SecretsPage = { - NextToken?: string; - SecretValues: SecretValue[]; -}; - -type SecretName = { - ARN: string; - CreatedDate: number; - LastAccessedDate: number; - LastChangedDate: number; - Name: string; - Tags: string[]; -}; - -type SecretValue = { - ARN: string; - CreatedDate: number; - Name: string; - SecretString: string; - VersionId: string; -}; - -export type Secret = { - secretName: string; - secretValue: string; -}; - -export type ConnectionTestResult = Promise<[boolean] | [boolean, string]>; - -export type AwsSecretsManagerContext = SecretsProviderSettings<{ - region: string; - authMethod: 'iamUser'; - accessKeyId: string; - secretAccessKey: string; -}>; - -export type AwsSecretsClientSettings = { - region: string; - host: string; - url: string; - accessKeyId: string; - secretAccessKey: string; -}; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1a0c48719c..8b6e968175 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -939,14 +939,17 @@ importers: packages/cli: dependencies: + '@aws-sdk/client-secrets-manager': + specifier: 3.666.0 + version: 3.666.0 '@azure/identity': - specifier: ^4.3.0 + specifier: 4.3.0 version: 4.3.0 '@azure/keyvault-secrets': - specifier: ^4.8.0 + specifier: 4.8.0 version: 4.8.0 '@google-cloud/secret-manager': - specifier: ^5.6.0 + specifier: 5.6.0 version: 5.6.0(encoding@0.1.13) '@n8n/api-types': specifier: workspace:* @@ -2525,6 +2528,10 @@ packages: resolution: {integrity: sha512-m7liz0sVMv08HIM66thhlqOfiIWn233izfDgXyJCA+A77NU9wUcIBHwxp6DdhSVLY/n0mE69Mq1XgyRFr/4udg==} engines: {node: '>=16.0.0'} + '@aws-sdk/client-secrets-manager@3.666.0': + resolution: {integrity: sha512-OWF0afndIgA+ujKtC7ZALT9shl4gDKlpVnKLdOWtL+p2y0Z7pN5PRVGcI75G63PmsanRZCT4rQ+rUqq38+rkYw==} + engines: {node: '>=16.0.0'} + '@aws-sdk/client-sso-oidc@3.666.0': resolution: {integrity: sha512-mW//v5EvHMU2SulW1FqmjJJPDNhzySRb/YUU+jq9AFDIYUdjF6j6wM+iavCW/4gLqOct0RT7B62z8jqyHkUCEQ==} engines: {node: '>=16.0.0'} @@ -14427,6 +14434,53 @@ snapshots: transitivePeerDependencies: - aws-crt + '@aws-sdk/client-secrets-manager@3.666.0': + dependencies: + '@aws-crypto/sha256-browser': 5.2.0 + '@aws-crypto/sha256-js': 5.2.0 + '@aws-sdk/client-sso-oidc': 3.666.0(@aws-sdk/client-sts@3.666.0) + '@aws-sdk/client-sts': 3.666.0 + '@aws-sdk/core': 3.666.0 + '@aws-sdk/credential-provider-node': 3.666.0(@aws-sdk/client-sso-oidc@3.666.0(@aws-sdk/client-sts@3.666.0))(@aws-sdk/client-sts@3.666.0) + '@aws-sdk/middleware-host-header': 3.664.0 + '@aws-sdk/middleware-logger': 3.664.0 + '@aws-sdk/middleware-recursion-detection': 3.664.0 + '@aws-sdk/middleware-user-agent': 3.666.0 + '@aws-sdk/region-config-resolver': 3.664.0 + '@aws-sdk/types': 3.664.0 + '@aws-sdk/util-endpoints': 3.664.0 + '@aws-sdk/util-user-agent-browser': 3.664.0 + '@aws-sdk/util-user-agent-node': 3.666.0 + '@smithy/config-resolver': 3.0.9 + '@smithy/core': 2.4.8 + '@smithy/fetch-http-handler': 3.2.9 + '@smithy/hash-node': 3.0.7 + '@smithy/invalid-dependency': 3.0.7 + '@smithy/middleware-content-length': 3.0.9 + '@smithy/middleware-endpoint': 3.1.4 + '@smithy/middleware-retry': 3.0.23 + '@smithy/middleware-serde': 3.0.7 + '@smithy/middleware-stack': 3.0.7 + '@smithy/node-config-provider': 3.1.8 + '@smithy/node-http-handler': 3.2.4 + '@smithy/protocol-http': 4.1.4 + '@smithy/smithy-client': 3.4.0 + '@smithy/types': 3.5.0 + '@smithy/url-parser': 3.0.7 + '@smithy/util-base64': 3.0.0 + '@smithy/util-body-length-browser': 3.0.0 + '@smithy/util-body-length-node': 3.0.0 + '@smithy/util-defaults-mode-browser': 3.0.23 + '@smithy/util-defaults-mode-node': 3.0.23 + '@smithy/util-endpoints': 2.1.3 + '@smithy/util-middleware': 3.0.7 + '@smithy/util-retry': 3.0.7 + '@smithy/util-utf8': 3.0.0 + tslib: 2.6.2 + uuid: 9.0.1 + transitivePeerDependencies: + - aws-crt + '@aws-sdk/client-sso-oidc@3.666.0(@aws-sdk/client-sts@3.666.0)': dependencies: '@aws-crypto/sha256-browser': 5.2.0