diff --git a/SAMtemplates/functions/main.yaml b/SAMtemplates/functions/main.yaml index 6d45aa0f1e..ad84302c5f 100644 --- a/SAMtemplates/functions/main.yaml +++ b/SAMtemplates/functions/main.yaml @@ -33,6 +33,15 @@ Parameters: Type: String Default: none + EnabledSiteODSCodesParam: + Type: AWS::SSM::Parameter::Value + + EnabledSystemsParam: + Type: AWS::SSM::Parameter::Value + + BlockedSiteODSCodesParam: + Type: AWS::SSM::Parameter::Value + LogLevel: Type: String @@ -83,6 +92,9 @@ Resources: TABLE_NAME: !Ref PrescriptionStatusUpdatesTableName NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL: !Ref NHSNotifyPrescriptionsSQSQueueUrl SQS_SALT: !Sub "{{resolve:secretsmanager:${SQSSaltSecret}:SecretString:salt}}" + ENABLED_SITE_ODS_CODES: !Ref EnabledSiteODSCodesParam + ENABLED_SYSTEMS: !Ref EnabledSystemsParam + BLOCKED_SITE_ODS_CODES: !Ref BlockedSiteODSCodesParam LOG_LEVEL: !Ref LogLevel ENVIRONMENT: !Ref Environment TEST_PRESCRIPTIONS_1: "None" diff --git a/SAMtemplates/main_template.yaml b/SAMtemplates/main_template.yaml index 38923fdf5d..b17e7aa9f9 100644 --- a/SAMtemplates/main_template.yaml +++ b/SAMtemplates/main_template.yaml @@ -90,6 +90,14 @@ Parameters: Type: String Resources: + Parameters: + Type: AWS::Serverless::Application + Properties: + Location: parameters/main.yaml + Parameters: + StackName: !Ref AWS::StackName + Environment: !Ref Environment + Tables: Type: AWS::Serverless::Application Properties: @@ -136,6 +144,9 @@ Resources: PrescriptionStatusUpdatesTableName: !GetAtt Tables.Outputs.PrescriptionStatusUpdatesTableName PrescriptionNotificationStateTableName: !GetAtt Tables.Outputs.PrescriptionNotificationStateTableName NHSNotifyPrescriptionsSQSQueueUrl: !GetAtt Messaging.Outputs.NHSNotifyPrescriptionsSQSQueueUrl + EnabledSiteODSCodesParam: !GetAtt Parameters.Outputs.EnabledSiteODSCodesParameterName + EnabledSystemsParam: !GetAtt Parameters.Outputs.EnabledSystemsParameterName + BlockedSiteODSCodesParam: !GetAtt Parameters.Outputs.BlockedSiteODSCodesParameterName LogLevel: !Ref LogLevel LogRetentionInDays: !Ref LogRetentionInDays EnableSplunk: !Ref EnableSplunk diff --git a/SAMtemplates/parameters/main.yaml b/SAMtemplates/parameters/main.yaml new file mode 100644 index 0000000000..58ee61fb85 --- /dev/null +++ b/SAMtemplates/parameters/main.yaml @@ -0,0 +1,79 @@ +AWSTemplateFormatVersion: '2010-09-09' +Description: >- + SSM Parameter Store entries. Values may differ between prod and non-prod environments + +Parameters: + StackName: + Type: String + + Environment: + Type: String + +Conditions: + IsProd: !Equals [ !Ref Environment, prod ] + +Resources: + EnabledSiteODSCodesParameter: + Type: AWS::SSM::Parameter + Properties: + Name: !Sub ${StackName}-PSUNotifyEnabledSiteODSCodes + Description: "List of site ODS codes for which notifications are enabled" + Type: String + Value: !If + - IsProd + - > # Prod notification enabled + FA565 + - > # Non-prod + FA565 + + EnabledSystemsParameter: + Type: AWS::SSM::Parameter + Properties: + Name: !Sub ${StackName}-PSUNotifyEnabledSystems + Description: "List of application names for which notifications are enabled" + Type: String + Value: !If + - IsProd + - > # Prod notification enabled + Apotec Ltd - Apotec CRM - Production, + CrxPatientApp, + nhsPrescriptionApp, + Titan PSU Prod + - > # Non-prod + Internal Test System, + Apotec Ltd - Apotec CRM - Production, + CrxPatientApp, + nhsPrescriptionApp, + Titan PSU Prod + + BlockedSiteODSCodesParameter: + Type: AWS::SSM::Parameter + Properties: + Name: !Sub ${StackName}-PSUNotifyBlockedSiteODSCodes + Description: "List of site ODS codes for which notifications are blocked" + Type: String + Value: !If + - IsProd + - > # Prod notification disabled + A83008 + - > # Non-prod + A83008 + +Outputs: + EnabledSiteODSCodesParameterName: + Description: "Name of the SSM parameter holding enabled site ODS codes" + Value: !Ref EnabledSiteODSCodesParameter + Export: + Name: !Sub ${StackName}-PSUNotifyEnabledSiteODSCodesParam + + EnabledSystemsParameterName: + Description: "Name of the SSM parameter holding enabled system names" + Value: !Ref EnabledSystemsParameter + Export: + Name: !Sub ${StackName}-PSUNotifyEnabledSystemsParam + + BlockedSiteODSCodesParameterName: + Description: "Name of the SSM parameter holding blocked site ODS codes" + Value: !Ref BlockedSiteODSCodesParameter + Export: + Name: !Sub ${StackName}-PSUNotifyBlockedSiteODSCodesParam diff --git a/packages/common/commonTypes/src/index.ts b/packages/common/commonTypes/src/index.ts index b2f4cd3780..908b07fbb0 100644 --- a/packages/common/commonTypes/src/index.ts +++ b/packages/common/commonTypes/src/index.ts @@ -1,14 +1,14 @@ export interface PSUDataItem { - LastModified: string - LineItemID: string - PatientNHSNumber: string - PharmacyODSCode: string - PrescriptionID: string - RepeatNo?: number - RequestID: string - Status: string - TaskID: string - TerminalStatus: string - ApplicationName: string - ExpiryTime: number - } + LastModified: string + LineItemID: string + PatientNHSNumber: string + PharmacyODSCode: string + PrescriptionID: string + RepeatNo?: number + RequestID: string + Status: string + TaskID: string + TerminalStatus: string + ApplicationName: string + ExpiryTime: number +} diff --git a/packages/updatePrescriptionStatus/.jest/setEnvVars.js b/packages/updatePrescriptionStatus/.jest/setEnvVars.js index b9343068cd..418a51377c 100644 --- a/packages/updatePrescriptionStatus/.jest/setEnvVars.js +++ b/packages/updatePrescriptionStatus/.jest/setEnvVars.js @@ -2,3 +2,6 @@ process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL = "dummy_notify_sqs"; process.env.AWS_REGION = "eu-west-2"; process.env.SQS_SALT = "the quick brown fox something something" +process.env.ENABLED_SITE_ODS_CODES = "FA565" +process.env.ENABLED_SYSTEMS = "Internal Test System,Apotec Ltd - Apotec CRM - Production,CrxPatientApp,nhsPrescriptionApp,Titan PSU Prod" +process.env.BLOCKED_SITE_ODS_CODES = "A83008" diff --git a/packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts b/packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts index 1141625c79..79c81444d4 100644 --- a/packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts +++ b/packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts @@ -11,6 +11,8 @@ import httpHeaderNormalizer from "@middy/http-header-normalizer" import errorHandler from "@nhs/fhir-middy-error-handler" import {Bundle, BundleEntry, Task} from "fhir/r4" +import {PSUDataItem} from "@PrescriptionStatusUpdate_common/commonTypes" + import {transactionBundle, validateEntry} from "./validation/content" import {getPreviousItem, persistDataItems} from "./utils/databaseClient" import {jobWithTimeout, hasTimedOut} from "./utils/timeoutUtils" @@ -42,21 +44,6 @@ export const TEST_PRESCRIPTIONS_1 = (process.env.TEST_PRESCRIPTIONS_1 ?? "") export const TEST_PRESCRIPTIONS_2 = (process.env.TEST_PRESCRIPTIONS_2 ?? "") .split(",").map(item => item.trim()) || [] -export interface DataItem { - LastModified: string - LineItemID: string - PatientNHSNumber: string - PharmacyODSCode: string - PrescriptionID: string - RepeatNo?: number - RequestID: string - Status: string - TaskID: string - TerminalStatus: string - ApplicationName: string - ExpiryTime: number -} - const lambdaHandler = async (event: APIGatewayProxyEvent): Promise => { logger.appendKeys({ "nhsd-correlation-id": event.headers["nhsd-correlation-id"], @@ -260,8 +247,8 @@ export function buildDataItems( requestEntries: Array, xRequestID: string, applicationName: string -): Array { - const dataItems: Array = [] +): Array { + const dataItems: Array = [] for (const requestEntry of requestEntries) { const task = requestEntry.resource as Task @@ -269,7 +256,7 @@ export function buildDataItems( const repeatNo = task.input?.[0]?.valueInteger - const dataItem: DataItem = { + const dataItem: PSUDataItem = { LastModified: task.lastModified!, LineItemID: task.focus!.identifier!.value!.toUpperCase(), PatientNHSNumber: task.for!.identifier!.value!, @@ -300,7 +287,7 @@ function response(statusCode: number, responseEntries: Array) { } } -async function logTransitions(dataItems: Array): Promise { +async function logTransitions(dataItems: Array): Promise { for (const dataItem of dataItems) { try { const previousItem = await getPreviousItem(dataItem) diff --git a/packages/updatePrescriptionStatus/src/utils/databaseClient.ts b/packages/updatePrescriptionStatus/src/utils/databaseClient.ts index bf239ca724..022fe2cf22 100644 --- a/packages/updatePrescriptionStatus/src/utils/databaseClient.ts +++ b/packages/updatePrescriptionStatus/src/utils/databaseClient.ts @@ -11,15 +11,15 @@ import { } from "@aws-sdk/client-dynamodb" import {marshall, unmarshall} from "@aws-sdk/util-dynamodb" -import {DataItem} from "../updatePrescriptionStatus" +import {PSUDataItem} from "@PrescriptionStatusUpdate_common/commonTypes" import {Timeout} from "./timeoutUtils" const client = new DynamoDBClient() const tableName = process.env.TABLE_NAME ?? "PrescriptionStatusUpdates" -function createTransactionCommand(dataItems: Array, logger: Logger): TransactWriteItemsCommand { +function createTransactionCommand(dataItems: Array, logger: Logger): TransactWriteItemsCommand { logger.info("Creating transaction command to write data items.") - const transactItems: Array = dataItems.map((d: DataItem): TransactWriteItem => { + const transactItems: Array = dataItems.map((d: PSUDataItem): TransactWriteItem => { return { Put: { TableName: tableName, @@ -32,7 +32,7 @@ function createTransactionCommand(dataItems: Array, logger: Logger): T return new TransactWriteItemsCommand({TransactItems: transactItems}) } -export async function persistDataItems(dataItems: Array, logger: Logger): Promise { +export async function persistDataItems(dataItems: Array, logger: Logger): Promise { const transactionCommand = createTransactionCommand(dataItems, logger) try { logger.info("Sending TransactWriteItemsCommand to DynamoDB.", {command: transactionCommand}) @@ -72,7 +72,7 @@ export async function checkPrescriptionRecordExistence( } } -export async function getPreviousItem(currentItem: DataItem): Promise { +export async function getPreviousItem(currentItem: PSUDataItem): Promise { const query: QueryCommandInput = { TableName: tableName, KeyConditions: { @@ -90,7 +90,7 @@ export async function getPreviousItem(currentItem: DataItem): Promise = [] + let items: Array = [] do { if (lastEvaluatedKey) { query.ExclusiveStartKey = lastEvaluatedKey @@ -99,7 +99,7 @@ export async function getPreviousItem(currentItem: DataItem): Promise unmarshall(item) as DataItem) + .map((item) => unmarshall(item) as PSUDataItem) .filter((item) => item.TaskID !== currentItem.TaskID) // Can't do NE in the query so filter here ) } diff --git a/packages/updatePrescriptionStatus/src/utils/sqsClient.ts b/packages/updatePrescriptionStatus/src/utils/sqsClient.ts index a62bae396b..5f24871063 100644 --- a/packages/updatePrescriptionStatus/src/utils/sqsClient.ts +++ b/packages/updatePrescriptionStatus/src/utils/sqsClient.ts @@ -3,10 +3,13 @@ import {SQSClient, SendMessageBatchCommand} from "@aws-sdk/client-sqs" import {createHmac} from "crypto" -import {DataItem} from "../updatePrescriptionStatus" +import {PSUDataItem} from "@PrescriptionStatusUpdate_common/commonTypes" + +import {checkSiteOrSystemIsNotifyEnabled} from "../validation/notificationSiteAndSystemFilters" const sqsUrl: string | undefined = process.env.NHS_NOTIFY_PRESCRIPTIONS_SQS_QUEUE_URL -const sqsSalt: string = process.env.SQS_SALT ?? "DEVSALT" +const fallbackSalt = "DEV SALT" +const sqsSalt: string = process.env.SQS_SALT ?? fallbackSalt // The AWS_REGION is always defined in lambda environments const sqs = new SQSClient({region: process.env.AWS_REGION}) @@ -33,9 +36,9 @@ function chunkArray(arr: Array, size: number): Array> { * @param hashFunction - Which hash function to use. HMAC compatible. Defaults to SHA-256 * @returns - A hex encoded string of the hash */ -export function saltedHash(input: string, hashFunction: string = "sha256"): string { - if (sqsSalt === "DEVSALT") { - console.warn("Using the fallback salt value - please update the environment variable `SQS_SALT` to a random value.") +export function saltedHash(logger: Logger, input: string, hashFunction: string = "sha256"): string { + if (sqsSalt === fallbackSalt) { + logger.warn("Using the fallback salt value - please update the environment variable `SQS_SALT` to a random value.") } return createHmac(hashFunction, sqsSalt) .update(input, "utf8") @@ -43,16 +46,16 @@ export function saltedHash(input: string, hashFunction: string = "sha256"): stri } /** - * Pushes an array of DataItems to the notifications SQS queue + * Pushes an array of PSUDataItem to the notifications SQS queue * Uses SendMessageBatch to send up to 10 at a time * * @param requestId - The x-request-id header from the incoming event - * @param data - Array of DataItems to send to SQS + * @param data - Array of PSUDataItem to send to SQS * @param logger - Logger instance */ export async function pushPrescriptionToNotificationSQS( requestId: string, - data: Array, + data: Array, logger: Logger ) { logger.info("Checking if any items require notifications", {numItemsToBeChecked: data.length, sqsUrl}) @@ -62,8 +65,17 @@ export async function pushPrescriptionToNotificationSQS( throw new Error("Notifications SQS URL not configured") } + // Only allow through sites and systems that are allowedSitesAndSystems + const allowedSitesAndSystemsData = checkSiteOrSystemIsNotifyEnabled(data) + logger.info( + "Filtered out sites and suppliers that are not enabled, or are explicitly disabled", + { + numItemsAllowed: allowedSitesAndSystemsData.length + } + ) + // SQS batch calls are limited to 10 messages per request, so chunk the data - const batches = chunkArray(data, 10) + const batches = chunkArray(allowedSitesAndSystemsData, 10) // Only these statuses will be pushed to the SQS const updateStatuses: Array = [ @@ -80,7 +92,7 @@ export async function pushPrescriptionToNotificationSQS( MessageBody: JSON.stringify(item), // FIFO // We dedupe on both nhs number and ods code - MessageDeduplicationId: saltedHash(`${item.PatientNHSNumber}:${item.PharmacyODSCode}`), + MessageDeduplicationId: saltedHash(logger, `${item.PatientNHSNumber}:${item.PharmacyODSCode}`), MessageGroupId: requestId })) // We could do a round of deduplications here, but benefits would be minimal and AWS SQS will do it for us anyway. diff --git a/packages/updatePrescriptionStatus/src/validation/notificationSiteAndSystemFilters.ts b/packages/updatePrescriptionStatus/src/validation/notificationSiteAndSystemFilters.ts new file mode 100644 index 0000000000..b4bfbfea76 --- /dev/null +++ b/packages/updatePrescriptionStatus/src/validation/notificationSiteAndSystemFilters.ts @@ -0,0 +1,44 @@ +import {PSUDataItem} from "@PrescriptionStatusUpdate_common/commonTypes" + +function getEnvList(name: string): Set { + const raw = process.env[name] ?? "" + return new Set(raw + .split(",") + .map((s) => s.trim().toLowerCase()) + .filter(Boolean) // Remove empty entries + ) +} + +const enabledSiteODSCodes = getEnvList("ENABLED_SITE_ODS_CODES") +const enabledSystems = getEnvList("ENABLED_SYSTEMS") +const blockedSiteODSCodes = getEnvList("BLOCKED_SITE_ODS_CODES") + +/** + * Given an array of PSUDataItem, only returns those which: + * - ARE enabled at a site OR system level, + * - AND are NOT blocked at the site level. + * + * @param data - Array of PSUDataItem to be processed + * @returns - the filtered array + */ +export function checkSiteOrSystemIsNotifyEnabled( + data: Array +): Array { + return data.filter((item) => { + const appName = item.ApplicationName.toLowerCase() + const odsCode = item.PharmacyODSCode.toLowerCase() + + // Is this item either ODS enabled, or supplier enabled? + const isEnabledSystem = enabledSiteODSCodes.has(odsCode) || enabledSystems.has(appName) + if (!isEnabledSystem) { + return false + } + + // Cannot have a blocked ODS code + if (blockedSiteODSCodes.has(odsCode)) { + return false + } + + return true + }) +} diff --git a/packages/updatePrescriptionStatus/tests/testSqsClient.test.ts b/packages/updatePrescriptionStatus/tests/testSqsClient.test.ts index a6145ed547..e2bc93d18c 100644 --- a/packages/updatePrescriptionStatus/tests/testSqsClient.test.ts +++ b/packages/updatePrescriptionStatus/tests/testSqsClient.test.ts @@ -15,6 +15,7 @@ import {createMockDataItem, mockSQSClient} from "./utils/testUtils" const {mockSend} = mockSQSClient() const {pushPrescriptionToNotificationSQS, saltedHash} = await import("../src/utils/sqsClient") +const {checkSiteOrSystemIsNotifyEnabled} = await import("../src/validation/notificationSiteAndSystemFilters") const ORIGINAL_ENV = {...process.env} @@ -22,6 +23,7 @@ describe("Unit tests for pushPrescriptionToNotificationSQS", () => { let logger: Logger let infoSpy: SpiedFunction<(input: LogItemMessage, ...extraInput: LogItemExtraInput) => void> let errorSpy: SpiedFunction<(input: LogItemMessage, ...extraInput: LogItemExtraInput) => void> + let warnSpy: SpiedFunction<(input: LogItemMessage, ...extraInput: LogItemExtraInput) => void> beforeEach(() => { jest.resetModules() @@ -34,6 +36,7 @@ describe("Unit tests for pushPrescriptionToNotificationSQS", () => { logger = new Logger({serviceName: "test-service"}) infoSpy = jest.spyOn(logger, "info") errorSpy = jest.spyOn(logger, "error") + warnSpy = jest.spyOn(logger, "warn") }) it("throws if the SQS URL is not configured", async () => { @@ -72,7 +75,7 @@ describe("Unit tests for pushPrescriptionToNotificationSQS", () => { it("sends only 'ready to collect' messages and succeeds", async () => { const payload = [ - createMockDataItem({Status: "ready to collect"}), + createMockDataItem({Status: "rEaDy To CoLlEcT"}), // Test case-insensitivity createMockDataItem({Status: "ready to collect - partial"}), createMockDataItem({Status: "a status that will never be real"}) ] @@ -105,7 +108,7 @@ describe("Unit tests for pushPrescriptionToNotificationSQS", () => { // FIFO params expect(entry.MessageGroupId).toBe("req-789") expect(entry.MessageDeduplicationId).toBe( - saltedHash(`${original.PatientNHSNumber}:${original.PharmacyODSCode}`) + saltedHash(logger, `${original.PatientNHSNumber}:${original.PharmacyODSCode}`) ) }) @@ -144,11 +147,75 @@ describe("Unit tests for pushPrescriptionToNotificationSQS", () => { createMockDataItem({Status: "ready to collect"}) ) - mockSend.mockImplementationOnce(() => Promise.resolve({Successful: Array(10).fill({})})) - mockSend.mockImplementationOnce(() => Promise.resolve({Successful: Array(2).fill({})})) + mockSend + .mockImplementationOnce(() => Promise.resolve({Successful: Array(10).fill({})})) + .mockImplementationOnce(() => Promise.resolve({Successful: Array(2).fill({})})) await pushPrescriptionToNotificationSQS("req-111", payload, logger) - expect(mockSend).toHaveBeenCalledTimes(2) }) + + it("Uses the fallback salt value but logs a warning about it", async () => { + process.env.SQS_SALT = undefined + const {saltedHash: tempFunc} = await import("../src/utils/sqsClient") + + tempFunc(logger, "foobar") + + expect(warnSpy) + .toHaveBeenLastCalledWith( + "Using the fallback salt value - please update the environment variable `SQS_SALT` to a random value." + ) + }) +}) + +describe("Unit tests for checkSiteOrSystemIsNotifyEnabled", () => { + it("includes an item with an enabled ODS code", () => { + const item = createMockDataItem({ + PharmacyODSCode: "FA565", + ApplicationName: "not a real test supplier" + }) + const result = checkSiteOrSystemIsNotifyEnabled([item]) + expect(result).toEqual([item]) + }) + + it("includes an item with an enabled ApplicationName", () => { + const item = createMockDataItem({ + PharmacyODSCode: "ZZZ999", + ApplicationName: "Internal Test System" + }) + const result = checkSiteOrSystemIsNotifyEnabled([item]) + expect(result).toEqual([item]) + }) + + it("is case insensitive for both ODS code and ApplicationName", () => { + const item1 = createMockDataItem({ + PharmacyODSCode: "fa565", + ApplicationName: "not a real test supplier" + }) + const item2 = createMockDataItem({ + PharmacyODSCode: "zzz999", + ApplicationName: "internal test SYSTEM" + }) + const result = checkSiteOrSystemIsNotifyEnabled([item1, item2]) + console.log(result) + expect(result).toEqual([item1, item2]) + }) + + it("excludes an item when its ODS code is blocked, even if otherwise enabled", () => { + const item = createMockDataItem({ + PharmacyODSCode: "a83008", + ApplicationName: "Internal Test System" + }) + const result = checkSiteOrSystemIsNotifyEnabled([item]) + expect(result).toEqual([]) + }) + + it("excludes items that are neither enabled nor blocked", () => { + const item = createMockDataItem({ + PharmacyODSCode: "NOTINLIST", + ApplicationName: "Some Other System" + }) + const result = checkSiteOrSystemIsNotifyEnabled([item]) + expect(result).toEqual([]) + }) }) diff --git a/packages/updatePrescriptionStatus/tests/utils/testUtils.ts b/packages/updatePrescriptionStatus/tests/utils/testUtils.ts index 087022ced9..3de33f46ed 100644 --- a/packages/updatePrescriptionStatus/tests/utils/testUtils.ts +++ b/packages/updatePrescriptionStatus/tests/utils/testUtils.ts @@ -15,7 +15,7 @@ import { import {Task} from "fhir/r4" import valid from "../tasks/valid.json" -import {DataItem} from "../../src/updatePrescriptionStatus" +import {PSUDataItem} from "@PrescriptionStatusUpdate_common/commonTypes" export const TASK_ID_0 = "4d70678c-81e4-4ff4-8c67-17596fd0aa46" export const TASK_ID_1 = "0ae4daf3-f24b-479d-b8fa-b69e2d873b60" @@ -197,7 +197,7 @@ export function mockSQSClient() { return {mockSend} } -export function createMockDataItem(overrides: Partial): DataItem { +export function createMockDataItem(overrides: Partial): PSUDataItem { return { LastModified: "2023-01-02T00:00:00Z", LineItemID: "spamandeggs", @@ -208,7 +208,7 @@ export function createMockDataItem(overrides: Partial): DataItem { Status: "ready to collect", TaskID: "mnopqr-ghijkl-abcdef", TerminalStatus: "ready to collect", - ApplicationName: "Jim's Pills", + ApplicationName: "Internal Test System", ExpiryTime: 123, ...overrides }