Skip to content

Commit ce404ea

Browse files
authored
Fix: [AEA-0000] - Fix Security on the notify callback (#1827)
## Summary - 🤖 Operational or Infrastructure Change ### Details Adds a new access bloc that should fix the apigee access
1 parent 3d29714 commit ce404ea

5 files changed

Lines changed: 24 additions & 30 deletions

File tree

packages/nhsNotifyLambda/src/nhsNotifyLambda.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ async function processBatch(
5858
}
5959

6060
if (processed.length) {
61-
await addPrescriptionMessagesToNotificationStateStore(logger, processed)
62-
await removeSQSMessages(logger, processed)
61+
await Promise.all([
62+
addPrescriptionMessagesToNotificationStateStore(logger, processed),
63+
removeSQSMessages(logger, processed)
64+
])
6365
}
6466
}
6567

@@ -115,9 +117,9 @@ export const lambdaHandler = async (
115117
throw new Error("No Routing Plan ID found")
116118
}
117119

118-
logger.info("NHS Notify lambda triggered by scheduler", {event})
119-
logger.info("Routing Plan ID:", {routingId})
120+
logger.info("NHS Notify lambda triggered by scheduler", {event, routingId})
120121

122+
// Done sequentially so that the queue report is accurate.
121123
await reportQueueStatus(logger)
122124
await drainAndProcess(routingId)
123125
}

packages/nhsNotifyLambda/src/utils/dynamo.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,19 @@ export async function checkCooldownForUpdate(
8585
TableName: dynamoTable,
8686
Key: {
8787
NHSNumber: update.PatientNHSNumber,
88-
ODSCode: update.PharmacyODSCode
88+
ODSCode: update.PharmacyODSCode,
89+
requestID: update.RequestID
8990
}
9091
})
9192
const {Item} = await docClient.send(getCmd)
9293

9394
// If no previous record, we're okay to send a notification
9495
if (!Item?.LastNotificationRequestTimestamp) {
95-
logger.info("No previous notification state found. Notification allowed.")
96+
logger.debug("No previous notification state found. Notification allowed.", {
97+
NHSNumber: update.PatientNHSNumber,
98+
ODSCode: update.PharmacyODSCode,
99+
requestID: update.RequestID
100+
})
96101
return true
97102
}
98103

@@ -102,19 +107,21 @@ export async function checkCooldownForUpdate(
102107
const secondsSince = Math.floor((nowTs - lastTs) / 1000)
103108

104109
if (secondsSince > cooldownPeriod) {
105-
logger.info("Cooldown period has passed. Notification allowed.", {
110+
logger.debug("Cooldown period has passed. Notification allowed.", {
106111
NHSNumber: update.PatientNHSNumber,
107112
ODSCode: update.PharmacyODSCode,
108113
cooldownPeriod,
109-
secondsSince
114+
secondsSince,
115+
requestID: update.RequestID
110116
})
111117
return true
112118
} else {
113-
logger.info("Within cooldown period. Notification suppressed.", {
119+
logger.debug("Within cooldown period. Notification suppressed.", {
114120
NHSNumber: update.PatientNHSNumber,
115121
ODSCode: update.PharmacyODSCode,
116122
cooldownPeriod,
117-
secondsSince
123+
secondsSince,
124+
requestID: update.RequestID
118125
})
119126
return false
120127
}

packages/nhsNotifyLambda/tests/testUtils.test.ts

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -380,9 +380,6 @@ describe("NHS notify lambda helper functions", () => {
380380
const result = await checkCooldownForUpdate(logger, update, 900)
381381

382382
expect(sendSpy).toHaveBeenCalledWith(expect.any(GetCommand))
383-
expect(infoSpy).toHaveBeenCalledWith(
384-
"No previous notification state found. Notification allowed."
385-
)
386383
expect(result).toBe(true)
387384
})
388385

@@ -395,10 +392,6 @@ describe("NHS notify lambda helper functions", () => {
395392
const update = constructPSUDataItemMessage().PSUDataItem
396393
const result = await checkCooldownForUpdate(logger, update, 900)
397394

398-
expect(infoSpy).toHaveBeenCalledWith(
399-
"Cooldown period has passed. Notification allowed.",
400-
expect.objectContaining({secondsSince: expect.any(Number)})
401-
)
402395
expect(result).toBe(true)
403396
})
404397

@@ -411,10 +404,6 @@ describe("NHS notify lambda helper functions", () => {
411404
const update = constructPSUDataItemMessage().PSUDataItem
412405
const result = await checkCooldownForUpdate(logger, update, 900)
413406

414-
expect(infoSpy).toHaveBeenCalledWith(
415-
"Within cooldown period. Notification suppressed.",
416-
expect.objectContaining({secondsSince: expect.any(Number)})
417-
)
418407
expect(result).toBe(false)
419408
})
420409

@@ -428,10 +417,6 @@ describe("NHS notify lambda helper functions", () => {
428417
const update = constructPSUDataItemMessage().PSUDataItem
429418
const result = await checkCooldownForUpdate(logger, update, 60)
430419

431-
expect(infoSpy).toHaveBeenCalledWith(
432-
"Within cooldown period. Notification suppressed.",
433-
expect.objectContaining({secondsSince: expect.any(Number)})
434-
)
435420
expect(result).toBe(false)
436421
})
437422

@@ -441,10 +426,6 @@ describe("NHS notify lambda helper functions", () => {
441426

442427
const update = constructPSUDataItemMessage().PSUDataItem
443428
await expect(checkCooldownForUpdate(logger, update)).rejects.toThrow("DDB failure")
444-
expect(errorSpy).toHaveBeenCalledWith(
445-
"Error checking cooldown state",
446-
expect.objectContaining({error: awsErr})
447-
)
448429
})
449430

450431
it("does nothing when passed an empty array", async () => {

packages/specification/eps-prescription-status-update-api.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,13 +435,17 @@ components:
435435
$ref: schemas/resources/CheckPrescriptionStatusUpdates.yaml
436436
security:
437437
- app-level3: []
438+
- app-level0: []
438439
x-nhsd-apim:
439440
temporary: false
440441
monitoring: true
441442
access:
442443
- title: Application Restricted
443444
grants:
444445
app-level3: []
446+
- title: API Key Restricted
447+
grants:
448+
app-level0: []
445449
target:
446450
type: external
447451
healthcheck: /_status

packages/updatePrescriptionStatus/src/updatePrescriptionStatus.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ const lambdaHandler = async (event: APIGatewayProxyEvent): Promise<APIGatewayPro
165165

166166
if (hasTimedOut(persistResponse)) {
167167
responseEntries = [timeoutResponse()]
168-
logger.info("DynamoDB operation timed out.")
168+
logger.error("DynamoDB operation timed out.")
169169
// It's okay to just call the function here, since if the enableNotifications
170170
// boolean is False, this function does nothing
171171
await removeSqsMessages(logger, created_messageIds)

0 commit comments

Comments
 (0)