From f97645647b3bb49884ea9213eb661005ea8bfa14 Mon Sep 17 00:00:00 2001 From: will Farrell Date: Fri, 9 Jun 2023 14:40:24 -0600 Subject: [PATCH 1/4] feat: add in support for secret rotation --- packages/secrets-manager/__tests__/index.js | 41 +++++++++++++++++ packages/secrets-manager/index.js | 49 ++++++++++++++++----- packages/util/__tests__/index.js | 46 +++++++++++++++++-- packages/util/index.js | 10 +++-- website/docs/middlewares/secrets-manager.md | 1 + 5 files changed, 129 insertions(+), 18 deletions(-) diff --git a/packages/secrets-manager/__tests__/index.js b/packages/secrets-manager/__tests__/index.js index 3c19630de..795944bef 100644 --- a/packages/secrets-manager/__tests__/index.js +++ b/packages/secrets-manager/__tests__/index.js @@ -1,3 +1,4 @@ +import { setTimeout } from 'node:timers/promises' import test from 'ava' import sinon from 'sinon' import { mockClient } from 'aws-sdk-client-mock' @@ -5,6 +6,7 @@ import middy from '../../core/index.js' import { getInternal, clearCache } from '../../util/index.js' import { SecretsManagerClient, + DescribeSecretCommand, GetSecretValueCommand } from '@aws-sdk/client-secrets-manager' import secretsManager from '../index.js' @@ -242,6 +244,45 @@ test.serial( } ) +test.serial( + 'It should call aws-sdk if cache enabled but cached param has expired using rotation', + async (t) => { + const mockService = mockClient(SecretsManagerClient) + .on(DescribeSecretCommand, { SecretId: 'api_key' }) + .resolves({ NextRotationDate: Date.now() + 50 }) + .on(GetSecretValueCommand, { SecretId: 'api_key' }) + .resolves({ SecretString: 'token' }) + const sendStub = mockService.send + const handler = middy(() => {}) + + const middleware = async (request) => { + const values = await getInternal(true, request) + t.is(values.token, 'token') + } + + handler + .use( + secretsManager({ + AwsClient: SecretsManagerClient, + cacheExpiry: 0, + fetchData: { + token: 'api_key' + }, + fetchRotationDate: true, + disablePrefetch: true + }) + ) + .before(middleware) + + await handler(event, context) // fetch x 2 + await handler(event, context) + await setTimeout(100) + await handler(event, context) // fetch x 2 + + t.is(sendStub.callCount, 2 * 2) + } +) + test.serial('It should catch if an error is returned from fetch', async (t) => { const mockService = mockClient(SecretsManagerClient) .on(GetSecretValueCommand, { SecretId: 'api_key' }) diff --git a/packages/secrets-manager/index.js b/packages/secrets-manager/index.js index c4c96be5f..1c55a3a65 100644 --- a/packages/secrets-manager/index.js +++ b/packages/secrets-manager/index.js @@ -10,6 +10,7 @@ import { } from '@middy/util' import { SecretsManagerClient, + DescribeSecretCommand, GetSecretValueCommand } from '@aws-sdk/client-secrets-manager' @@ -18,30 +19,56 @@ const defaults = { awsClientOptions: {}, awsClientAssumeRole: undefined, awsClientCapture: undefined, - fetchData: {}, // If more than 2, consider writing own using ListSecrets + fetchData: {}, + fetchRotationDate: false, // true: apply to all or {key: true} for individual disablePrefetch: false, cacheKey: 'secrets-manager', - cacheExpiry: -1, + cacheExpiry: -1, // ignored when fetchRotationRules is true/object setToContext: false } +const future = Date.now() * 2 const secretsManagerMiddleware = (opts = {}) => { const options = { ...defaults, ...opts } const fetch = (request, cachedValues = {}) => { const values = {} - // Multiple secrets can be requested in a single requests, - // however this is likely uncommon IRL, increases complexity to handle, - // and will require recursive promise resolution impacting performance. - // See https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/SecretsManager.html#listSecrets-property + if (options.fetchRotationRules) { + options.cacheExpiry = 0 + } + for (const internalKey of Object.keys(options.fetchData)) { if (cachedValues[internalKey]) continue - values[internalKey] = client - .send( - new GetSecretValueCommand({ - SecretId: options.fetchData[internalKey] - }) + + values[internalKey] = Promise.resolve() + .then(() => { + if ( + options.fetchRotationDate === true || + options.fetchRotationDate?.[internalKey] + ) { + return client + .send( + new DescribeSecretCommand({ + SecretId: options.fetchData[internalKey] + }) + ) + .then((resp) => { + if (resp.NextRotationDate) { + options.cacheExpiry = Math.min( + options.cacheExpiry || future, + resp.NextRotationDate * 1000 + ) + } + }) + } + }) + .then(() => + client.send( + new GetSecretValueCommand({ + SecretId: options.fetchData[internalKey] + }) + ) ) .then((resp) => jsonSafeParse(resp.SecretString)) .catch((e) => { diff --git a/packages/util/__tests__/index.js b/packages/util/__tests__/index.js index 64f4962cd..37a73eac5 100644 --- a/packages/util/__tests__/index.js +++ b/packages/util/__tests__/index.js @@ -268,6 +268,25 @@ test.serial('processCache should cache when not expired', async (t) => { t.is(fetch.callCount, 1) clearCache() }) +test.serial( + 'processCache should cache when not expired w/ unix timestamp', + async (t) => { + const fetch = sinon.stub().resolves('value') + const options = { + cacheKey: 'key', + cacheExpiry: Date.now() + 100 + } + processCache(options, fetch, cacheRequest) + await setTimeout(50) + const cacheValue = getCache('key').value + t.is(await cacheValue, 'value') + const { value, cache } = processCache(options, fetch, cacheRequest) + t.is(await value, 'value') + t.is(cache, true) + t.is(fetch.callCount, 1) + clearCache() + } +) test.serial( 'processCache should clear and re-fetch modified cache', @@ -331,20 +350,41 @@ test.serial( test.serial('processCache should cache and expire', async (t) => { const fetch = sinon.stub().resolves('value') const options = { - cacheKey: 'key', + cacheKey: 'key-cache-expire', cacheExpiry: 150 } processCache(options, fetch, cacheRequest) await setTimeout(100) - let cache = getCache('key') + let cache = getCache('key-cache-expire') t.not(cache, undefined) await setTimeout(250) // expire twice - cache = getCache('key') + cache = getCache('key-cache-expire') t.true(cache.expiry > Date.now()) t.is(fetch.callCount, 3) clearCache() }) +test.serial( + 'processCache should cache and expire w/ unix timestamp', + async (t) => { + const fetch = sinon.stub().resolves('value') + const options = { + cacheKey: 'key-cache-unix-expire', + cacheExpiry: Date.now() + 155 + } + processCache(options, fetch, cacheRequest) + await setTimeout(100) + let cache = getCache('key-cache-unix-expire') + t.not(cache, undefined) + await setTimeout(250) // expire once, then doesn't cache + cache = getCache('key-cache-unix-expire') + + t.true(cache.expiry < Date.now()) + t.is(fetch.callCount, 3) + clearCache() + } +) + test.serial('processCache should clear single key cache', async (t) => { const fetch = sinon.stub().resolves('value') processCache( diff --git a/packages/util/index.js b/packages/util/index.js index 926b871c7..5be282b07 100644 --- a/packages/util/index.js +++ b/packages/util/index.js @@ -118,12 +118,14 @@ export const processCache = (options, fetch = () => undefined, request) => { } } const value = fetch(request) - const expiry = Date.now() + cacheExpiry - + const now = Date.now() + // secrets-manager overrides to unix timestamp + const expiry = cacheExpiry > 86400000 ? cacheExpiry : now + cacheExpiry + const duration = cacheExpiry > 86400000 ? cacheExpiry - now : cacheExpiry if (cacheExpiry) { const refresh = - cacheExpiry > 0 - ? setInterval(() => processCache(options, fetch, request), cacheExpiry) + duration > 0 + ? setInterval(() => processCache(options, fetch, request), duration) : undefined cache[cacheKey] = { value, expiry, refresh } } diff --git a/website/docs/middlewares/secrets-manager.md b/website/docs/middlewares/secrets-manager.md index ac6bff8ae..8cbba42ee 100644 --- a/website/docs/middlewares/secrets-manager.md +++ b/website/docs/middlewares/secrets-manager.md @@ -28,6 +28,7 @@ npm install --save-dev @aws-sdk/client-secrets-manager - `awsClientAssumeRole` (string) (optional): Internal key where secrets are stored. See [@middy/sts](/docs/middlewares/sts) on to set this. - `awsClientCapture` (function) (optional): Enable XRay by passing `captureAWSv3Client` from `aws-xray-sdk` in. - `fetchData` (object) (required): Mapping of internal key name to API request parameter `SecretId`. +- `fetchRotationDate` (boolean|object) (default `false`): Boolean to apply to all or mapping of internal key name to boolean indicating what secrets should fetch NextRotationDate before the secret. `cacheExpiry` is ignored when this is not `false`. - `disablePrefetch` (boolean) (default `false`): On cold start requests will trigger early if they can. Setting `awsClientAssumeRole` disables prefetch. - `cacheKey` (string) (default `secrets-manager`): Cache key for the fetched data responses. Must be unique across all middleware. - `cacheExpiry` (number) (default `-1`): How long fetch data responses should be cached for. `-1`: cache forever, `0`: never cache, `n`: cache for n ms. From b6fe88c954cd4d1f4c5883f72f79865a15118fd5 Mon Sep 17 00:00:00 2001 From: will Farrell Date: Fri, 9 Jun 2023 14:46:18 -0600 Subject: [PATCH 2/4] docs: add in iam --- website/docs/middlewares/secrets-manager.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/middlewares/secrets-manager.md b/website/docs/middlewares/secrets-manager.md index 8cbba42ee..2e7792f83 100644 --- a/website/docs/middlewares/secrets-manager.md +++ b/website/docs/middlewares/secrets-manager.md @@ -36,7 +36,7 @@ npm install --save-dev @aws-sdk/client-secrets-manager NOTES: -- Lambda is required to have IAM permission for `secretsmanager:GetSecretValue` +- Lambda is required to have IAM permission for `secretsmanager:GetSecretValue`. If using `fetchRotationDate` add `secretsmanager:DescribeSecret` in as well. ## Sample usage From c3db9d10e13cbae2562e55216a5496658d9f1b70 Mon Sep 17 00:00:00 2001 From: will Farrell Date: Mon, 7 Aug 2023 09:45:17 -0600 Subject: [PATCH 3/4] feat: add in cacheKeyExpiry --- packages/appconfig/index.js | 1 + packages/dynamodb/index.js | 1 + packages/rds-signer/index.js | 1 + packages/s3/index.js | 1 + packages/service-discovery/index.js | 1 + packages/ssm/index.js | 1 + packages/sts/index.js | 1 + packages/util/__tests__/index.js | 40 +++++++++++++++++++++++++++++ packages/util/index.js | 3 ++- 9 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/appconfig/index.js b/packages/appconfig/index.js index 72e41d0e5..7c0649ca5 100644 --- a/packages/appconfig/index.js +++ b/packages/appconfig/index.js @@ -20,6 +20,7 @@ const defaults = { fetchData: {}, disablePrefetch: false, cacheKey: 'appconfig', + cacheKeyExpiry: {}, cacheExpiry: -1, setToContext: false } diff --git a/packages/dynamodb/index.js b/packages/dynamodb/index.js index 898d430eb..d2bb6c404 100644 --- a/packages/dynamodb/index.js +++ b/packages/dynamodb/index.js @@ -18,6 +18,7 @@ const defaults = { fetchData: {}, disablePrefetch: false, cacheKey: 'dynamodb', + cacheKeyExpiry: {}, cacheExpiry: -1, setToContext: false } diff --git a/packages/rds-signer/index.js b/packages/rds-signer/index.js index 57230ca6f..690ff1494 100644 --- a/packages/rds-signer/index.js +++ b/packages/rds-signer/index.js @@ -13,6 +13,7 @@ const defaults = { fetchData: {}, disablePrefetch: false, cacheKey: 'rds-signer', + cacheKeyExpiry: {}, cacheExpiry: -1, setToContext: false } diff --git a/packages/s3/index.js b/packages/s3/index.js index d794ea5e8..7e1801cae 100644 --- a/packages/s3/index.js +++ b/packages/s3/index.js @@ -17,6 +17,7 @@ const defaults = { fetchData: {}, disablePrefetch: false, cacheKey: 's3', + cacheKeyExpiry: {}, cacheExpiry: -1, setToContext: false } diff --git a/packages/service-discovery/index.js b/packages/service-discovery/index.js index 4ea5bfb2b..aac21bd3a 100644 --- a/packages/service-discovery/index.js +++ b/packages/service-discovery/index.js @@ -20,6 +20,7 @@ const defaults = { fetchData: {}, // { contextKey: {NamespaceName, ServiceName, HealthStatus} } disablePrefetch: false, cacheKey: 'cloud-map', + cacheKeyExpiry: {}, cacheExpiry: -1, setToContext: false } diff --git a/packages/ssm/index.js b/packages/ssm/index.js index 2566d6338..13eebafc7 100644 --- a/packages/ssm/index.js +++ b/packages/ssm/index.js @@ -24,6 +24,7 @@ const defaults = { fetchData: {}, // { contextKey: fetchKey, contextPrefix: fetchPath/ } disablePrefetch: false, cacheKey: 'ssm', + cacheKeyExpiry: {}, cacheExpiry: -1, setToContext: false } diff --git a/packages/sts/index.js b/packages/sts/index.js index cd6407023..81ad4f4f3 100644 --- a/packages/sts/index.js +++ b/packages/sts/index.js @@ -17,6 +17,7 @@ const defaults = { fetchData: {}, // { contextKey: {RoleArn, RoleSessionName} } disablePrefetch: false, cacheKey: 'sts', + cacheKeyExpiry: {}, cacheExpiry: -1, setToContext: false } diff --git a/packages/util/__tests__/index.js b/packages/util/__tests__/index.js index 37a73eac5..939b8ae87 100644 --- a/packages/util/__tests__/index.js +++ b/packages/util/__tests__/index.js @@ -287,6 +287,46 @@ test.serial( clearCache() } ) +test.serial( + 'processCache should cache when not expired using cacheKeyExpire', + async (t) => { + const fetch = sinon.stub().resolves('value') + const options = { + cacheKey: 'key', + cacheExpiry: 0, + cacheKeyExpiry: { key: Date.now() + 100 } + } + processCache(options, fetch, cacheRequest) + await setTimeout(50) + const cacheValue = getCache('key').value + t.is(await cacheValue, 'value') + const { value, cache } = processCache(options, fetch, cacheRequest) + t.is(await value, 'value') + t.is(cache, true) + t.is(fetch.callCount, 1) + clearCache() + } +) +test.serial( + 'processCache should cache when not expired using cacheKeyExpire w/ unix timestamp', + async (t) => { + const fetch = sinon.stub().resolves('value') + const options = { + cacheKey: 'key', + cacheExpiry: Date.now() + 0, + cacheKeyExpiry: { key: Date.now() + 100 } + } + processCache(options, fetch, cacheRequest) + await setTimeout(50) + const cacheValue = getCache('key').value + t.is(await cacheValue, 'value') + const { value, cache } = processCache(options, fetch, cacheRequest) + t.is(await value, 'value') + t.is(cache, true) + t.is(fetch.callCount, 1) + clearCache() + } +) test.serial( 'processCache should clear and re-fetch modified cache', diff --git a/packages/util/index.js b/packages/util/index.js index 5be282b07..59dd1a909 100644 --- a/packages/util/index.js +++ b/packages/util/index.js @@ -99,7 +99,8 @@ export const sanitizeKey = (key) => { // fetch Cache const cache = {} // key: { value:{fetchKey:Promise}, expiry } export const processCache = (options, fetch = () => undefined, request) => { - const { cacheExpiry, cacheKey } = options + let { cacheKey, cacheKeyExpiry, cacheExpiry } = options + cacheExpiry = cacheKeyExpiry?.[cacheKey] ?? cacheExpiry if (cacheExpiry) { const cached = getCache(cacheKey) const unexpired = From 07a824cdb7ef71f1c5a010e12988e567c8fc1723 Mon Sep 17 00:00:00 2001 From: will Farrell Date: Mon, 7 Aug 2023 09:48:07 -0600 Subject: [PATCH 4/4] feat: add in support for manual rotation --- packages/secrets-manager/__tests__/index.js | 91 ++++++++++++++++++++- packages/secrets-manager/index.js | 17 ++-- website/docs/middlewares/secrets-manager.md | 2 +- 3 files changed, 98 insertions(+), 12 deletions(-) diff --git a/packages/secrets-manager/__tests__/index.js b/packages/secrets-manager/__tests__/index.js index 795944bef..876f80f22 100644 --- a/packages/secrets-manager/__tests__/index.js +++ b/packages/secrets-manager/__tests__/index.js @@ -245,11 +245,14 @@ test.serial( ) test.serial( - 'It should call aws-sdk if cache enabled but cached param has expired using rotation', + 'It should call aws-sdk if cache enabled but cached param has expired using LastRotationDate', async (t) => { const mockService = mockClient(SecretsManagerClient) .on(DescribeSecretCommand, { SecretId: 'api_key' }) - .resolves({ NextRotationDate: Date.now() + 50 }) + .resolves({ + LastRotationDate: Date.now() / 1000 - 50, + LastChangedDate: Date.now() / 1000 - 50 + }) .on(GetSecretValueCommand, { SecretId: 'api_key' }) .resolves({ SecretString: 'token' }) const sendStub = mockService.send @@ -264,7 +267,89 @@ test.serial( .use( secretsManager({ AwsClient: SecretsManagerClient, - cacheExpiry: 0, + cacheExpiry: 100, + fetchData: { + token: 'api_key' + }, + fetchRotationDate: true, + disablePrefetch: true + }) + ) + .before(middleware) + + await handler(event, context) // fetch x 2 + await handler(event, context) + await setTimeout(100) + await handler(event, context) // fetch x 2 + + t.is(sendStub.callCount, 2 * 2) + } +) + +test.serial( + 'It should call aws-sdk if cache enabled but cached param has expired using LastRotationDate, fallback to NextRotationDate', + async (t) => { + const mockService = mockClient(SecretsManagerClient) + .on(DescribeSecretCommand, { SecretId: 'api_key' }) + .resolves({ + LastRotationDate: Date.now() / 1000 - 25, + LastChangedDate: Date.now() / 1000 - 25, + NextRotationDate: Date.now() / 1000 + 50 + }) + .on(GetSecretValueCommand, { SecretId: 'api_key' }) + .resolves({ SecretString: 'token' }) + const sendStub = mockService.send + const handler = middy(() => {}) + + const middleware = async (request) => { + const values = await getInternal(true, request) + t.is(values.token, 'token') + } + + handler + .use( + secretsManager({ + AwsClient: SecretsManagerClient, + cacheExpiry: 100, + fetchData: { + token: 'api_key' + }, + fetchRotationDate: true, + disablePrefetch: true + }) + ) + .before(middleware) + + await handler(event, context) // fetch x 2 + await handler(event, context) + await setTimeout(100) + await handler(event, context) // fetch x 2 + + t.is(sendStub.callCount, 2 * 2) + } +) + +test.serial( + 'It should call aws-sdk if cache enabled but cached param has expired using NextRotationDate', + async (t) => { + const mockService = mockClient(SecretsManagerClient) + .on(DescribeSecretCommand, { SecretId: 'api_key' }) + .resolves({ NextRotationDate: Date.now() / 1000 + 50 }) + .on(GetSecretValueCommand, { SecretId: 'api_key' }) + .resolves({ SecretString: 'token' }) + const sendStub = mockService.send + const handler = middy(() => {}) + + const middleware = async (request) => { + const values = await getInternal(true, request) + t.is(values.token, 'token') + } + + handler + .use( + secretsManager({ + AwsClient: SecretsManagerClient, + cacheExpiry: -1, fetchData: { token: 'api_key' }, diff --git a/packages/secrets-manager/index.js b/packages/secrets-manager/index.js index 1c55a3a65..5d604df48 100644 --- a/packages/secrets-manager/index.js +++ b/packages/secrets-manager/index.js @@ -23,21 +23,17 @@ const defaults = { fetchRotationDate: false, // true: apply to all or {key: true} for individual disablePrefetch: false, cacheKey: 'secrets-manager', + cacheKeyExpiry: {}, cacheExpiry: -1, // ignored when fetchRotationRules is true/object setToContext: false } -const future = Date.now() * 2 const secretsManagerMiddleware = (opts = {}) => { const options = { ...defaults, ...opts } const fetch = (request, cachedValues = {}) => { const values = {} - if (options.fetchRotationRules) { - options.cacheExpiry = 0 - } - for (const internalKey of Object.keys(options.fetchData)) { if (cachedValues[internalKey]) continue @@ -54,9 +50,14 @@ const secretsManagerMiddleware = (opts = {}) => { }) ) .then((resp) => { - if (resp.NextRotationDate) { - options.cacheExpiry = Math.min( - options.cacheExpiry || future, + if (options.cacheExpiry < 0) { + options.cacheKeyExpiry[internalKey] = + resp.NextRotationDate * 1000 + } else { + options.cacheKeyExpiry[internalKey] = Math.min( + Math.max(resp.LastRotationDate, resp.LastChangedDate) * + 1000 + + options.cacheExpiry, resp.NextRotationDate * 1000 ) } diff --git a/website/docs/middlewares/secrets-manager.md b/website/docs/middlewares/secrets-manager.md index 2e7792f83..6adce4a77 100644 --- a/website/docs/middlewares/secrets-manager.md +++ b/website/docs/middlewares/secrets-manager.md @@ -28,7 +28,7 @@ npm install --save-dev @aws-sdk/client-secrets-manager - `awsClientAssumeRole` (string) (optional): Internal key where secrets are stored. See [@middy/sts](/docs/middlewares/sts) on to set this. - `awsClientCapture` (function) (optional): Enable XRay by passing `captureAWSv3Client` from `aws-xray-sdk` in. - `fetchData` (object) (required): Mapping of internal key name to API request parameter `SecretId`. -- `fetchRotationDate` (boolean|object) (default `false`): Boolean to apply to all or mapping of internal key name to boolean indicating what secrets should fetch NextRotationDate before the secret. `cacheExpiry` is ignored when this is not `false`. +- `fetchRotationDate` (boolean|object) (default `false`): Boolean to apply to all or mapping of internal key name to boolean. This indicates what secrets should fetch and cached based on `NextRotationDate`/`LastRotationDate`/`LastChangedDate`. `cacheExpiry` of `-1` will use `NextRotationDate`, while any other value will be added to the `LastRotationDate` or `LastChangedDate`, whichever is more recent. If secrets have different rotation schedules, use multiple instances of this middleware. - `disablePrefetch` (boolean) (default `false`): On cold start requests will trigger early if they can. Setting `awsClientAssumeRole` disables prefetch. - `cacheKey` (string) (default `secrets-manager`): Cache key for the fetched data responses. Must be unique across all middleware. - `cacheExpiry` (number) (default `-1`): How long fetch data responses should be cached for. `-1`: cache forever, `0`: never cache, `n`: cache for n ms.