Update the code to fetch secrets at runtime

Author: wildjames

SAMtemplates/functions/main.yaml

@@ -444,8 +444,8 @@ Resources:
         Variables:
           LOG_LEVEL: !Ref LogLevel
           TABLE_NAME: !Ref PrescriptionNotificationStatesTableName
-          APP_NAME: !ImportValue secrets:PSUNotifyCallbackAppName
-          API_KEY:  !ImportValue secrets:PSUNotifyCallbackApiKey
+          APP_NAME_SECRET_ARN: !ImportValue secrets:PSUNotifyCallbackAppName
+          API_KEY_SECRET_ARN:  !ImportValue secrets:PSUNotifyCallbackApiKey
     Metadata:
       BuildMethod: esbuild
       guard:

package-lock.json

@@ -1,4 +1,4 @@
 /* eslint-disable no-undef */
 process.env.TABLE_NAME = "dummy_table";
-process.env.APP_NAME = "app name";
-process.env.API_KEY = "api key";
+process.env.APP_NAME_SECRET_ARN = "app name";
+process.env.API_KEY_SECRET_ARN = "api key";

packages/nhsNotifyUpdateCallback/.jest/setEnvVars.js

@@ -17,6 +17,7 @@
     "@aws-lambda-powertools/commons": "^2.17.0",
     "@aws-lambda-powertools/logger": "^2.18.0",
     "@aws-lambda-powertools/parameters": "^2.18.0",
+    "@aws-sdk/client-secrets-manager": "^3.812.0",
     "@middy/core": "^6.2.2",
     "@middy/input-output-logger": "^6.2.2",
     "@nhs/fhir-middy-error-handler": "^2.1.29",

packages/nhsNotifyUpdateCallback/package.json

@@ -4,13 +4,11 @@ import {Logger} from "@aws-lambda-powertools/logger"
 import {DynamoDBClient} from "@aws-sdk/client-dynamodb"
 import {DynamoDBDocumentClient, UpdateCommand, QueryCommand} from "@aws-sdk/lib-dynamodb"
 
+import {SecretsManagerClient, GetSecretValueCommand} from "@aws-sdk/client-secrets-manager"
 import {createHmac, timingSafeEqual} from "crypto"
 
 import {MessageStatusResponse} from "./types"
 
-const APP_NAME = process.env.APP_NAME
-const API_KEY = process.env.API_KEY
-
 // TTL is one week in seconds
 const TTL_DELTA = 60 * 60 * 24 * 7
 
@@ -19,25 +17,66 @@ const dynamoTable = process.env.TABLE_NAME
 const dynamo = new DynamoDBClient({region: process.env.AWS_REGION})
 const docClient = DynamoDBDocumentClient.from(dynamo)
 
+// Do a bit of secret caching to help reduce the number of fetches.
+let cachedAppName: string | undefined
+let cachedApiKey: string | undefined
+
+const secretsClient = new SecretsManagerClient({
+  region: process.env.AWS_REGION
+})
+
 export function response(statusCode: number, body: unknown = {}) {
   return {
     statusCode,
     body: JSON.stringify(body)
   }
 }
 
+async function getSecretValue(secretArn: string): Promise<string> {
+  const cmd = new GetSecretValueCommand({SecretId: secretArn})
+  const resp = await secretsClient.send(cmd)
+
+  if (resp.SecretString) {
+    return resp.SecretString
+  }
+
+  throw new Error(`Secret ${secretArn} has no usable SecretString`)
+}
+
+/**
+ * Loads both APP_NAME and API_KEY from Secrets Manager, if not already cached.
+ * I'm loading these at runtime so that we can update the secret and have that change
+ * reflected without the need for a full redeployment.
+ */
+async function loadSecrets() {
+  if (cachedAppName && cachedApiKey) return
+
+  const appNameArn = process.env.APP_NAME_SECRET_ARN
+  const apiKeyArn = process.env.API_KEY_SECRET_ARN
+
+  if (!appNameArn) {
+    throw new Error("APP_NAME_SECRET_ARN environment variable is not set.")
+  }
+  if (!apiKeyArn) {
+    throw new Error("API_KEY_SECRET_ARN environment variable is not set.")
+  }
+
+  const [nameValue, keyValue] = await Promise.all([
+    getSecretValue(appNameArn),
+    getSecretValue(apiKeyArn)
+  ])
+
+  cachedAppName = nameValue.trim()
+  cachedApiKey = keyValue.trim()
+}
+
 /**
  * Checks the incoming NHS Notify request signature.
  * If it's okay, returns undefined.
  * If it's not okay, it returns the error response object.
  */
-export function checkSignature(logger: Logger, event: APIGatewayProxyEvent) {
-  if (!APP_NAME) {
-    throw new Error("APP_NAME environment variable is not set.")
-  }
-  if (!API_KEY) {
-    throw new Error("API_KEY environment variable is not set.")
-  }
+export async function checkSignature(logger: Logger, event: APIGatewayProxyEvent) {
+  await loadSecrets()
 
   const signature = event.headers["x-hmac-sha256-signature"]
   if (!signature) {
@@ -52,10 +91,10 @@ export function checkSignature(logger: Logger, event: APIGatewayProxyEvent) {
   }
 
   // FIXME: Delete this line before PR
-  logger.info("Secret data", {APP_NAME, API_KEY})
+  logger.info("Secret data", {cachedAppName, cachedApiKey})
 
   // Compute the HMAC-SHA256 hash of the combination of the request body and the secret value
-  const secretValue = `${APP_NAME}.${API_KEY}`
+  const secretValue = `${cachedAppName!}.${cachedApiKey!}`
   const payload = event.body ?? ""
 
   // compare hashes as Buffers, rather than hex

packages/nhsNotifyUpdateCallback/src/helpers.ts

@@ -26,7 +26,7 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPro
   if (!event.headers["x-request-id"]) return response(400, {message: "No x-request-id given"})
 
   // Check the request signature
-  const isErr = checkSignature(logger, event)
+  const isErr = await checkSignature(logger, event)
   if (isErr) return isErr
   logger.info("Signature OK!")
 

packages/nhsNotifyUpdateCallback/src/lambdaHandler.ts

@@ -8,6 +8,7 @@ import {
 } from "@jest/globals"
 import {createHmac} from "crypto"
 import {DynamoDBDocumentClient, QueryCommand, UpdateCommand} from "@aws-sdk/lib-dynamodb"
+import {SecretsManagerClient} from "@aws-sdk/client-secrets-manager"
 
 import {response, checkSignature, updateNotificationsTable} from "../src/helpers"
 import {Logger} from "@aws-lambda-powertools/logger"
@@ -44,8 +45,18 @@ describe("helpers.ts", () => {
 
   describe("checkSignature()", () => {
     let logger: Logger
-    let validHeaders: { "x-request-id": string; "x-api-key": string; "x-hmac-sha256-signature": string }
+    let validHeaders: Record<string, string>
+    let smSendSpy: jest.SpiedFunction<typeof SecretsManagerClient.prototype.send>
+
     beforeEach(() => {
+      // Stub SecretsManagerClient.send so we never call AWS in tests
+      smSendSpy = jest
+        .spyOn(SecretsManagerClient.prototype, "send")
+        // first call: APP_NAME
+        .mockImplementationOnce(() => Promise.resolve({SecretString: process.env.APP_NAME_SECRET_ARN!}))
+        // second call: API_KEY
+        .mockImplementationOnce(() => Promise.resolve({SecretString: process.env.API_KEY_SECRET_ARN!}))
+
       logger = new Logger({serviceName: "nhsNotifyUpdateCallback"})
       validHeaders = {
         "x-request-id": "requestid",
@@ -54,40 +65,48 @@ describe("helpers.ts", () => {
       }
     })
 
-    it("401 when missing signature header", () => {
-      const ev = generateMockEvent("{}", {"x-api-key": "foobar", "x-request-id": "rid"})
-      const resp = checkSignature(logger, ev)
+    afterEach(() => {
+      smSendSpy.mockRestore()
+    })
+
+    it("401 when missing signature header", async () => {
+      const ev = generateMockEvent("{}", {
+        "x-api-key": "foobar",
+        "x-request-id": "rid"
+      })
+      const resp = await checkSignature(logger, ev)
       expect(resp).toEqual({
         statusCode: 401,
         body: JSON.stringify({message: "No x-hmac-sha256-signature given"})
       })
     })
 
-    it("401 when missing API key header", () => {
-      const ev = generateMockEvent("{}", {"x-hmac-sha256-signature": "foobar", "x-request-id": "rid"})
-      const resp = checkSignature(logger, ev)
-
+    it("401 when missing API key header", async () => {
+      const ev = generateMockEvent("{}", {
+        "x-hmac-sha256-signature": "foobar",
+        "x-request-id": "rid"
+      })
+      const resp = await checkSignature(logger, ev)
       expect(resp).toEqual({
         statusCode: 401,
         body: JSON.stringify({message: "No x-api-key header given"})
       })
     })
 
-    it("403 when signature hex is malformed", () => {
+    it("403 when signature hex is malformed", async () => {
       const headers = {
         ...validHeaders,
         "x-hmac-sha256-signature": "not a hex string!@!#zzz"
       }
       const ev = generateMockEvent(JSON.stringify({message: "blah blah blah"}), headers)
-      const resp = checkSignature(logger, ev)
-
+      const resp = await checkSignature(logger, ev)
       expect(resp).toEqual({
         statusCode: 403,
         body: JSON.stringify({message: "Incorrect signature"})
       })
     })
 
-    it("403 when signature does not match HMAC", () => {
+    it("403 when signature does not match HMAC", async () => {
       const payload = "payload"
       const wrongSig = createHmac(
         "sha256",
@@ -100,17 +119,16 @@ describe("helpers.ts", () => {
         ...validHeaders,
         "x-hmac-sha256-signature": wrongSig
       })
-      const resp = checkSignature(logger, ev)
-
+      const resp = await checkSignature(logger, ev)
       expect(resp).toEqual({
         statusCode: 403,
         body: JSON.stringify({message: "Incorrect signature"})
       })
     })
 
-    it("returns undefined when signature is valid", () => {
+    it("returns undefined when signature is valid", async () => {
       const payload = "hi there"
-      const secret = `${process.env.APP_NAME}.${process.env.API_KEY}`
+      const secret = `${process.env.APP_NAME_SECRET_ARN}.${process.env.API_KEY_SECRET_ARN}`
       const goodSig = createHmac("sha256", secret)
         .update(payload, "utf8")
         .digest("hex")
@@ -119,13 +137,14 @@ describe("helpers.ts", () => {
         ...validHeaders,
         "x-hmac-sha256-signature": goodSig
       })
-      const resp = checkSignature(logger, ev)
+      const resp = await checkSignature(logger, ev)
       expect(resp).toBeUndefined()
     })
   })
 
   describe("updateNotificationsTable()", () => {
     let logger: Logger
+
     beforeEach(() => {
       logger = new Logger({serviceName: "nhsNotifyUpdateCallback"})
       jest.spyOn(logger, "error")