Revert "Update the code to fetch secrets at runtime"

This reverts commit e6e4abe.

Author: wildjames

SAMtemplates/functions/main.yaml

@@ -444,8 +444,8 @@ Resources:
         Variables:
           LOG_LEVEL: !Ref LogLevel
           TABLE_NAME: !Ref PrescriptionNotificationStatesTableName
-          APP_NAME_SECRET_ARN: !ImportValue secrets:PSUNotifyCallbackAppName
-          API_KEY_SECRET_ARN:  !ImportValue secrets:PSUNotifyCallbackApiKey
+          APP_NAME: !ImportValue secrets:PSUNotifyCallbackAppName
+          API_KEY:  !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_SECRET_ARN = "app name";
-process.env.API_KEY_SECRET_ARN = "api key";
+process.env.APP_NAME = "app name";
+process.env.API_KEY = "api key";

packages/nhsNotifyUpdateCallback/.jest/setEnvVars.js

@@ -17,7 +17,6 @@
     "@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,11 +4,13 @@ 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
 
@@ -17,66 +19,25 @@ 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 async function checkSignature(logger: Logger, event: APIGatewayProxyEvent) {
-  await loadSecrets()
+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.")
+  }
 
   const signature = event.headers["x-hmac-sha256-signature"]
   if (!signature) {
@@ -91,10 +52,10 @@ export async function checkSignature(logger: Logger, event: APIGatewayProxyEvent
   }
 
   // FIXME: Delete this line before PR
-  logger.info("Secret data", {cachedAppName, cachedApiKey})
+  logger.info("Secret data", {APP_NAME, API_KEY})
 
   // Compute the HMAC-SHA256 hash of the combination of the request body and the secret value
-  const secretValue = `${cachedAppName!}.${cachedApiKey!}`
+  const secretValue = `${APP_NAME}.${API_KEY}`
   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 = await checkSignature(logger, event)
+  const isErr = checkSignature(logger, event)
   if (isErr) return isErr
   logger.info("Signature OK!")
 

packages/nhsNotifyUpdateCallback/src/lambdaHandler.ts

@@ -8,7 +8,6 @@ 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"
@@ -45,18 +44,8 @@ describe("helpers.ts", () => {
 
   describe("checkSignature()", () => {
     let logger: Logger
-    let validHeaders: Record<string, string>
-    let smSendSpy: jest.SpiedFunction<typeof SecretsManagerClient.prototype.send>
-
+    let validHeaders: { "x-request-id": string; "x-api-key": string; "x-hmac-sha256-signature": string }
     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",
@@ -65,48 +54,40 @@ describe("helpers.ts", () => {
       }
     })
 
-    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)
+    it("401 when missing signature header", () => {
+      const ev = generateMockEvent("{}", {"x-api-key": "foobar", "x-request-id": "rid"})
+      const resp = 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", async () => {
-      const ev = generateMockEvent("{}", {
-        "x-hmac-sha256-signature": "foobar",
-        "x-request-id": "rid"
-      })
-      const resp = await checkSignature(logger, ev)
+    it("401 when missing API key header", () => {
+      const ev = generateMockEvent("{}", {"x-hmac-sha256-signature": "foobar", "x-request-id": "rid"})
+      const resp = 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", async () => {
+    it("403 when signature hex is malformed", () => {
       const headers = {
         ...validHeaders,
         "x-hmac-sha256-signature": "not a hex string!@!#zzz"
       }
       const ev = generateMockEvent(JSON.stringify({message: "blah blah blah"}), headers)
-      const resp = await checkSignature(logger, ev)
+      const resp = checkSignature(logger, ev)
+
       expect(resp).toEqual({
         statusCode: 403,
         body: JSON.stringify({message: "Incorrect signature"})
       })
     })
 
-    it("403 when signature does not match HMAC", async () => {
+    it("403 when signature does not match HMAC", () => {
       const payload = "payload"
       const wrongSig = createHmac(
         "sha256",
@@ -119,16 +100,17 @@ describe("helpers.ts", () => {
         ...validHeaders,
         "x-hmac-sha256-signature": wrongSig
       })
-      const resp = await checkSignature(logger, ev)
+      const resp = checkSignature(logger, ev)
+
       expect(resp).toEqual({
         statusCode: 403,
         body: JSON.stringify({message: "Incorrect signature"})
       })
     })
 
-    it("returns undefined when signature is valid", async () => {
+    it("returns undefined when signature is valid", () => {
       const payload = "hi there"
-      const secret = `${process.env.APP_NAME_SECRET_ARN}.${process.env.API_KEY_SECRET_ARN}`
+      const secret = `${process.env.APP_NAME}.${process.env.API_KEY}`
       const goodSig = createHmac("sha256", secret)
         .update(payload, "utf8")
         .digest("hex")
@@ -137,14 +119,13 @@ describe("helpers.ts", () => {
         ...validHeaders,
         "x-hmac-sha256-signature": goodSig
       })
-      const resp = await checkSignature(logger, ev)
+      const resp = checkSignature(logger, ev)
       expect(resp).toBeUndefined()
     })
   })
 
   describe("updateNotificationsTable()", () => {
     let logger: Logger
-
     beforeEach(() => {
       logger = new Logger({serviceName: "nhsNotifyUpdateCallback"})
       jest.spyOn(logger, "error")