Fix: [AEA-0000] - Use the APIM required header, rather than the one notify want us to use. Also, parameterise notify enable flags (#1884)

## Summary

- ⚠️ Potential issues that might be caused by this change

### Details

In order to test our callback endpoint, we need to be able to actually
use it. Since there's a mismatch between notify's spec, and APIM's
capabilities, I'm making it align with APIM for now just so we can see
how things behave.

Also, moves the notification enabling flags to deployment parameters,
and disables them both by default for prod/int

Author: wildjames

.github/workflows/ci.yml

@@ -115,6 +115,8 @@ jobs:
       ENABLE_ALERTS: true
       RUN_REGRESSION_TEST: true
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: true
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_DEPLOY_ROLE }}
       DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE }}
@@ -144,6 +146,8 @@ jobs:
       DEPLOY_CHECK_PRESCRIPTION_STATUS_UPDATE: true
       RUN_REGRESSION_TEST: false
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       REGRESSION_TESTS_PEM: ${{ secrets.REGRESSION_TESTS_PEM }}
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_DEPLOY_ROLE }}
@@ -170,6 +174,8 @@ jobs:
       ENABLE_ALERTS: true
       RUN_REGRESSION_TEST: true
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       REGRESSION_TESTS_PEM: ${{ secrets.REGRESSION_TESTS_PEM }}
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.QA_CLOUD_FORMATION_DEPLOY_ROLE }}

.github/workflows/pull_request.yml

@@ -78,6 +78,8 @@ jobs:
       ENABLE_ALERTS: false
       RUN_REGRESSION_TEST: true
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: true
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_DEPLOY_ROLE }}
       PROXYGEN_ROLE: ${{ secrets.PROXYGEN_PTL_ROLE }}
@@ -103,6 +105,8 @@ jobs:
       DEPLOY_CHECK_PRESCRIPTION_STATUS_UPDATE: true
       RUN_REGRESSION_TEST: false
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_DEPLOY_ROLE }}
       PROXYGEN_ROLE: ${{ secrets.PROXYGEN_PTL_ROLE }}

.github/workflows/release.yml

@@ -134,6 +134,8 @@ jobs:
       ENABLE_ALERTS: true
       RUN_REGRESSION_TEST: true
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_DEPLOY_ROLE }}
       DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE }}
@@ -163,6 +165,8 @@ jobs:
       DEPLOY_CHECK_PRESCRIPTION_STATUS_UPDATE: true
       RUN_REGRESSION_TEST: false
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_DEPLOY_ROLE }}
       PROXYGEN_ROLE: ${{ secrets.PROXYGEN_PTL_ROLE }}
@@ -197,6 +201,8 @@ jobs:
       ENABLE_ALERTS: true
       RUN_REGRESSION_TEST: false
       STATE_MACHINE_LOG_LEVEL: ERROR
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.REF_CLOUD_FORMATION_DEPLOY_ROLE }}
       PROXYGEN_ROLE: ${{ secrets.PROXYGEN_PTL_ROLE }}
@@ -231,6 +237,8 @@ jobs:
       ENABLE_ALERTS: true
       RUN_REGRESSION_TEST: true
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.QA_CLOUD_FORMATION_DEPLOY_ROLE }}
       PROXYGEN_ROLE: ${{ secrets.PROXYGEN_PTL_ROLE }}
@@ -260,6 +268,8 @@ jobs:
       ENABLE_ALERTS: true
       RUN_REGRESSION_TEST: true
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.INT_CLOUD_FORMATION_DEPLOY_ROLE }}
       DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE }}
@@ -289,6 +299,8 @@ jobs:
       DEPLOY_CHECK_PRESCRIPTION_STATUS_UPDATE: true
       RUN_REGRESSION_TEST: false
       STATE_MACHINE_LOG_LEVEL: ALL
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.INT_CLOUD_FORMATION_DEPLOY_ROLE }}
       PROXYGEN_ROLE: ${{ secrets.PROXYGEN_PROD_ROLE }}
@@ -325,6 +337,8 @@ jobs:
       ENABLE_ALERTS: true
       RUN_REGRESSION_TEST: false
       STATE_MACHINE_LOG_LEVEL: ERROR
+      ENABLE_NOTIFICATIONS_INTERNAL: false
+      ENABLE_NOTIFICATIONS_EXTERNAL: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE: ${{ secrets.PROD_CLOUD_FORMATION_DEPLOY_ROLE }}
       DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE: ${{ secrets.DEV_CLOUD_FORMATION_CHECK_VERSION_ROLE }}

.github/workflows/run_release_code_and_api.yml

@@ -70,6 +70,14 @@ on:
       STATE_MACHINE_LOG_LEVEL:
         required: true
         type: string
+      ENABLE_NOTIFICATIONS_INTERNAL:
+        required: false
+        type: boolean
+        default: false
+      ENABLE_NOTIFICATIONS_EXTERNAL:
+        required: false
+        type: boolean
+        default: false
     secrets:
       CLOUD_FORMATION_DEPLOY_ROLE:
         required: true
@@ -155,6 +163,8 @@ jobs:
           DEPLOY_CHECK_PRESCRIPTION_STATUS_UPDATE: ${{ inputs.DEPLOY_CHECK_PRESCRIPTION_STATUS_UPDATE }}
           ENABLE_ALERTS: ${{ inputs.ENABLE_ALERTS }}
           STATE_MACHINE_LOG_LEVEL: ${{ inputs.STATE_MACHINE_LOG_LEVEL }}
+          ENABLE_NOTIFICATIONS_INTERNAL: ${{ inputs.ENABLE_NOTIFICATIONS_INTERNAL }}
+          ENABLE_NOTIFICATIONS_EXTERNAL: ${{ inputs.ENABLE_NOTIFICATIONS_EXTERNAL }}
         run: ./release_code.sh
 
       - name: get mtls secrets

Makefile

@@ -97,7 +97,9 @@ sam-deploy-package: guard-artifact_bucket guard-artifact_bucket_prefix guard-sta
 			  Environment=$$TARGET_ENVIRONMENT \
 			  DeployCheckPrescriptionStatusUpdate=$$DEPLOY_CHECK_PRESCRIPTION_STATUS_UPDATE \
 			  EnableAlerts=$$ENABLE_ALERTS \
-			  StateMachineLogLevel=$$STATE_MACHINE_LOG_LEVEL
+			  StateMachineLogLevel=$$STATE_MACHINE_LOG_LEVEL \
+			  EnableNotificationsInternal=$$ENABLE_NOTIFICATIONS_INTERNAL \
+			  EnableNotificationsExternal=$$ENABLE_NOTIFICATIONS_EXTERNAL
 
 compile-node:
 	npx tsc --build tsconfig.build.json

SAMtemplates/functions/main.yaml

@@ -51,7 +51,7 @@ Parameters:
   NotifyAPIBaseURLParam:
     Type: AWS::SSM::Parameter::Name<String>
 
-  EnableNotificationExternalLinkParam:
+  EnableNotificationsExternalParam:
     Type: AWS::SSM::Parameter::Name<String>
 
   EnableNotificationsInternalParam:
@@ -407,7 +407,7 @@ Resources:
           KID_SECRET: secrets-PSU-Notify-Application-Name
           NHS_NOTIFY_ROUTING_ID_PARAM: !Ref NotifyRoutingPlanIDParam
           NOTIFY_API_BASE_URL_PARAM: !Ref NotifyAPIBaseURLParam
-          MAKE_REAL_NOTIFY_REQUESTS_PARAM: !Ref EnableNotificationExternalLinkParam
+          MAKE_REAL_NOTIFY_REQUESTS_PARAM: !Ref EnableNotificationsExternalParam
       Events:
         ScheduleEvent:
           Type: ScheduleV2

SAMtemplates/main_template.yaml

@@ -89,6 +89,20 @@ Parameters:
   StateMachineLogLevel:
     Type: String
 
+  EnableNotificationsInternal:
+    Type: String
+    Default: false
+    AllowedValues:
+      - true
+      - false
+  
+  EnableNotificationsExternal:
+    Type: String
+    Default: false
+    AllowedValues:
+      - true
+      - false
+
 Resources:
   Secrets: 
     Type: AWS::Serverless::Application
@@ -104,6 +118,8 @@ Resources:
       Parameters:
         StackName: !Ref AWS::StackName
         Environment: !Ref Environment
+        EnableNotificationsInternalValue: !Ref EnableNotificationsInternal
+        EnableNotificationsExternalValue: !Ref EnableNotificationsExternal
 
   Tables:
     Type: AWS::Serverless::Application
@@ -159,7 +175,7 @@ Resources:
         BlockedSiteODSCodesParam: !GetAtt Parameters.Outputs.BlockedSiteODSCodesParameterName
         NotifyRoutingPlanIDParam: !GetAtt Parameters.Outputs.NotifyRoutingPlanIDParameterName
         NotifyAPIBaseURLParam: !GetAtt Parameters.Outputs.NotifyAPIBaseURLParameterName
-        EnableNotificationExternalLinkParam: !GetAtt Parameters.Outputs.EnableNotificationExternalLinkName
+        EnableNotificationsExternalParam: !GetAtt Parameters.Outputs.EnableNotificationsExternalName
         EnableNotificationsInternalParam: !GetAtt Parameters.Outputs.EnableNotificationsInternalName
         LogLevel: !Ref LogLevel
         LogRetentionInDays: !Ref LogRetentionInDays

SAMtemplates/parameters/main.yaml

@@ -9,6 +9,20 @@ Parameters:
   Environment:
     Type: String
 
+  EnableNotificationsInternalValue:
+    Type: String
+    Default: false
+    AllowedValues:
+      - true
+      - false
+
+  EnableNotificationsExternalValue:
+    Type: String
+    Default: false
+    AllowedValues:
+      - true
+      - false
+
 Conditions:
   IsProd: !Equals [ !Ref Environment, prod ]
 
@@ -83,21 +97,21 @@ Resources:
         # Non-prod API URL (sandbox) (INT environment: https://int.api.service.nhs.uk/comms)
         - https://sandbox.api.service.nhs.uk/comms
 
-  EnableNotificationExternalLink:
+  EnableNotificationsExternal:
     Type: AWS::SSM::Parameter
     Properties:
-      Name: !Sub ${StackName}-EnableNotificationExternalLink
+      Name: !Sub ${StackName}-EnableNotificationsExternal
       Description: "Toggle on or off if we make real requests to the NHS notify service"
       Type: String
-      Value: "false"
+      Value: !Ref EnableNotificationsExternalValue
 
   EnableNotificationsInternal:
     Type: AWS::SSM::Parameter
     Properties:
       Name: !Sub ${StackName}-EnableNotificationsInternal
       Description: "Toggle the notifications integration entirely"
       Type: String
-      Value: "true"
+      Value: !Ref EnableNotificationsInternalValue
 
   GetNotificationsParameterPolicy:
     Type: AWS::IAM::ManagedPolicy
@@ -116,7 +130,7 @@ Resources:
               - !Sub arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/${StackName}-PSUNotifyBlockedSiteODSCodes
               - !Sub arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/${StackName}-PSUNotifyRoutingPlanID
               - !Sub arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/${StackName}-PSUNotifyApiBaseUrl
-              - !Sub arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/${StackName}-EnableNotificationExternalLink
+              - !Sub arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/${StackName}-EnableNotificationsExternal
               - !Sub arn:aws:ssm:${AWS::Region}:${AWS::AccountId}:parameter/${StackName}-EnableNotificationsInternal
 
 
@@ -151,11 +165,11 @@ Outputs:
     Export:
       Name: !Sub ${StackName}-PSUNotifyApiBaseUrlParam
 
-  EnableNotificationExternalLinkName:
+  EnableNotificationsExternalName:
     Description: "Name of the SSM parameter holding the Notify API Base URL"
-    Value: !Ref EnableNotificationExternalLink
+    Value: !Ref EnableNotificationsExternal
     Export:
-      Name: !Sub ${StackName}-EnableNotificationExternalLinkName
+      Name: !Sub ${StackName}-EnableNotificationsExternalName
 
   EnableNotificationsInternalName:
     Description: "Name of the SSM parameter holding the Notify API Base URL"

packages/nhsNotifyUpdateCallback/src/helpers.ts

@@ -89,12 +89,6 @@ export async function checkSignature(logger: Logger, event: APIGatewayProxyEvent
     return response(401, {message: "No x-hmac-sha256-signature given"})
   }
 
-  const givenApiKey = event.headers["x-api-key"]
-  if (!givenApiKey) {
-    logger.error("No x-api-key header given")
-    return response(401, {message: "No x-api-key header given"})
-  }
-
   // Compute the HMAC-SHA256 hash of the combination of the request body and the secret value
   const secretValue = `${APP_NAME}.${API_KEY}`
   const payload = event.body ?? ""

packages/nhsNotifyUpdateCallback/tests/testHelpers.test.ts

@@ -66,35 +66,25 @@ 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: { "x-request-id": string; "apikey": string; "x-hmac-sha256-signature": string }
     beforeEach(() => {
       logger = new Logger({serviceName: "nhsNotifyUpdateCallback"})
       validHeaders = {
         "x-request-id": "requestid",
-        "x-api-key": "api-key",
+        "apikey": "api-key", // TODO: Should be x-api-key
         "x-hmac-sha256-signature": "deadbeef"
       }
     })
 
     it("401 when missing signature header", async () => {
-      const ev = generateMockEvent("{}", {"x-api-key": "foobar", "x-request-id": "rid"})
+      const ev = generateMockEvent("{}", {"apikey": "foobar", "x-request-id": "rid"}) // TODO: Should be x-api-key
       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", 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", async () => {
       const headers = {
         ...validHeaders,