Skip to content

Commit c2c1085

Browse files
mikebeebymikebeeby
authored
Merge pull request #729 from govuk-one-login/OJ-1539-audit
OJ-1539: Replace audit logic with cri-audit and use cri-types
2 parents ee6468d + b589f46 commit c2c1085

37 files changed

+6271
-1908
lines changed

‎integration-tests/api-gateway/abandon/abandon-happy.test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1+
import { AuditEvent } from "@govuk-one-login/cri-audit";
12
import { clearAttemptsTable, clearItemsFromTables, getItemByKey } from "../../resources/dynamodb-helper";
2-
import { ABANDONED_EVENT_NAME, AuditEvent, baseExpectedEvent, pollForTestHarnessEvents } from "../audit";
3+
import { ABANDONED_EVENT_NAME, baseExpectedEvent, pollForTestHarnessEvents } from "../audit";
34
import {
45
abandonEndpoint,
56
authorizationEndpoint,

‎integration-tests/api-gateway/audit.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import type { AuditEvent } from "../../lambdas/common/src/types/audit";
2-
import type { UnixSecondsTimestamp } from "../../lambdas/common/src/types/brands";
1+
import type { UnixSecondsTimestamp, PersonIdentityDateOfBirth, PersonIdentityNamePart} from "@govuk-one-login/cri-types";
32
import { unmarshall } from "@aws-sdk/util-dynamodb";
43
import { signedFetch } from "../resources/fetch";
54
import { pause } from "../resources/util";
5+
import { Evidence } from "../../lambdas/common/src/types/evidence";
6+
import { AuditEvent, AuditRestricted } from "@govuk-one-login/cri-audit";
67

78
const prefix = "IPV_HMRC_RECORD_CHECK_CRI" as const;
89

@@ -13,7 +14,18 @@ export const VC_ISSUED_EVENT_NAME = `${prefix}_VC_ISSUED`;
1314
export const END_EVENT_NAME = `${prefix}_END`;
1415
export const ABANDONED_EVENT_NAME = `${prefix}_ABANDONED`;
1516

16-
export type AuditEventRecord<EventType = AuditEvent> = {
17+
export interface NinoCheckAuditRestricted extends AuditRestricted {
18+
socialSecurityRecord?: { personalNumber: string }[];
19+
};
20+
21+
export type NinoCheckAuditExtensions = {
22+
evidence?:
23+
| (Evidence & { attemptNum: number; ciReasons?: { ci: string; reason: string }[] })[]
24+
| { txn: string }
25+
| [{ context: string }];
26+
};
27+
28+
export type AuditEventRecord<EventType = AuditEvent<never, never, AuditRestricted>> = {
1729
partitionKey: `SESSION#${string}`;
1830
sortKey: `TXMA#${typeof prefix}_${string}#${string}#${string}`;
1931
event: EventType;
@@ -68,5 +80,3 @@ export function baseExpectedEvent(eventName: string, sessionId: string): AuditEv
6880
}),
6981
};
7082
}
71-
72-
export { AuditEvent };

‎integration-tests/api-gateway/check/check-lambda-happy.test.ts‎

Lines changed: 42 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ import { ninoCheckEndpoint, createSession, getJarAuthorization } from "../endpoi
22
import { clearAttemptsTable, clearItemsFromTables } from "../../resources/dynamodb-helper";
33
import { AUDIENCE, NINO } from "../env-variables";
44
import {
5-
AuditEvent,
5+
NinoCheckAuditExtensions,
66
baseExpectedEvent,
77
pollForTestHarnessEvents,
88
REQUEST_SENT_EVENT_NAME,
99
RESPONSE_RECEIVED_EVENT_NAME,
10+
NinoCheckAuditRestricted,
1011
} from "../audit";
1112
import { testUser } from "../user";
13+
import { AuditEvent } from "@govuk-one-login/cri-audit";
1214

1315
jest.setTimeout(60_000); // 1 min
1416

@@ -58,23 +60,26 @@ describe("Given the session and NINO is valid", () => {
5860

5961
const reqSentEvents = await pollForTestHarnessEvents(REQUEST_SENT_EVENT_NAME, sessionId);
6062
expect(reqSentEvents).toHaveLength(1);
61-
expect(reqSentEvents[0].event).toStrictEqual<AuditEvent>({
63+
64+
const expectedRequestSentAuditEvent: AuditEvent<never, never, NinoCheckAuditRestricted> = {
6265
...baseExpectedEvent(REQUEST_SENT_EVENT_NAME, sessionId),
6366
restricted: {
6467
birthDate: [{ value: testUser.dob }],
6568
name: testUser.formattedName,
6669
socialSecurityRecord: [{ personalNumber: NINO }],
67-
},
68-
});
70+
}
71+
};
72+
expect(reqSentEvents[0].event).toStrictEqual(expectedRequestSentAuditEvent);
6973

7074
const resReceivedEvents = await pollForTestHarnessEvents(RESPONSE_RECEIVED_EVENT_NAME, sessionId);
7175
expect(resReceivedEvents).toHaveLength(1);
72-
expect(resReceivedEvents[0].event).toStrictEqual<AuditEvent>({
76+
const expectedResponseReceivedAuditEvent: AuditEvent<NinoCheckAuditExtensions, never, NinoCheckAuditRestricted> = {
7377
...baseExpectedEvent(RESPONSE_RECEIVED_EVENT_NAME, sessionId),
7478
extensions: {
7579
evidence: { txn: expect.any(String) },
76-
},
77-
});
80+
}
81+
}
82+
expect(resReceivedEvents[0].event).toStrictEqual(expectedResponseReceivedAuditEvent);
7883
});
7984

8085
it("Should receive a 200 response when /check endpoint is called with optional headers", async () => {
@@ -93,21 +98,24 @@ describe("Given the session and NINO is valid", () => {
9398

9499
const reqSentEvents = await pollForTestHarnessEvents(REQUEST_SENT_EVENT_NAME, sessionId);
95100
expect(reqSentEvents).toHaveLength(1);
96-
expect(reqSentEvents[0].event).toStrictEqual<AuditEvent>({
101+
102+
const expectedRequestSentAuditEvent: AuditEvent<never, never, NinoCheckAuditRestricted> = {
97103
...baseExpectedEvent(REQUEST_SENT_EVENT_NAME, sessionId),
98104
restricted: {
99105
birthDate: [{ value: testUser.dob }],
100106
name: testUser.formattedName,
101107
socialSecurityRecord: [{ personalNumber: NINO }],
102108
device_information: {
103109
encoded: "test encoded header",
104-
},
105-
},
106-
});
110+
}
111+
}
112+
}
113+
expect(reqSentEvents[0].event).toStrictEqual(expectedRequestSentAuditEvent);
107114

108115
const resReceivedEvents = await pollForTestHarnessEvents(RESPONSE_RECEIVED_EVENT_NAME, sessionId);
109116
expect(resReceivedEvents).toHaveLength(1);
110-
expect(resReceivedEvents[0].event).toStrictEqual<AuditEvent>({
117+
118+
const expectedRequestReceivedAuditEvent: AuditEvent<NinoCheckAuditExtensions, never, NinoCheckAuditRestricted> = {
111119
...baseExpectedEvent(RESPONSE_RECEIVED_EVENT_NAME, sessionId),
112120
restricted: {
113121
device_information: {
@@ -116,8 +124,9 @@ describe("Given the session and NINO is valid", () => {
116124
},
117125
extensions: {
118126
evidence: { txn: expect.any(String) },
119-
},
120-
});
127+
}
128+
}
129+
expect(resReceivedEvents[0].event).toStrictEqual(expectedRequestReceivedAuditEvent);
121130
});
122131

123132
it("Should request retry when NINo match fails", async () => {
@@ -172,7 +181,8 @@ describe("Given the session and NINO is valid", () => {
172181

173182
const reqSentEvents = await pollForTestHarnessEvents(REQUEST_SENT_EVENT_NAME, sessionId);
174183
expect(reqSentEvents).toHaveLength(1);
175-
expect(reqSentEvents[0].event).toStrictEqual<AuditEvent>({
184+
185+
const expectedRequestSentAuditEvent: AuditEvent<never, never, NinoCheckAuditRestricted> = {
176186
...baseExpectedEvent(REQUEST_SENT_EVENT_NAME, sessionId),
177187
restricted: {
178188
birthDate: deceasedPersonSession.birthDate,
@@ -182,11 +192,13 @@ describe("Given the session and NINO is valid", () => {
182192
encoded: "test encoded header",
183193
},
184194
},
185-
});
195+
}
196+
expect(reqSentEvents[0].event).toStrictEqual(expectedRequestSentAuditEvent);
186197

187198
const resReceivedEvents = await pollForTestHarnessEvents(RESPONSE_RECEIVED_EVENT_NAME, sessionId);
188199
expect(resReceivedEvents).toHaveLength(1);
189-
expect(resReceivedEvents[0].event).toStrictEqual<AuditEvent>({
200+
201+
const expectedRequestReceivedAuditEvent: AuditEvent<NinoCheckAuditExtensions, never, NinoCheckAuditRestricted> = {
190202
...baseExpectedEvent(RESPONSE_RECEIVED_EVENT_NAME, sessionId),
191203
restricted: {
192204
device_information: {
@@ -195,8 +207,9 @@ describe("Given the session and NINO is valid", () => {
195207
},
196208
extensions: {
197209
evidence: { txn: expect.any(String) },
198-
},
199-
});
210+
}
211+
}
212+
expect(resReceivedEvents[0].event).toStrictEqual(expectedRequestReceivedAuditEvent);
200213
});
201214

202215
it("Should receive a 200 response when /check endpoint is called using multiple named user", async () => {
@@ -253,22 +266,26 @@ describe("Given the session and NINO is valid", () => {
253266

254267
const reqSentEvents = await pollForTestHarnessEvents(REQUEST_SENT_EVENT_NAME, sessionId);
255268
expect(reqSentEvents).toHaveLength(1);
256-
expect(reqSentEvents[0].event).toStrictEqual<AuditEvent>({
269+
270+
const expectedRequestSentAuditEvent: AuditEvent<never, never, NinoCheckAuditRestricted> = {
257271
...baseExpectedEvent(REQUEST_SENT_EVENT_NAME, sessionId),
258272
restricted: {
259273
birthDate: [{ value: "2000-02-02" }],
260274
name: multipleNamesSession.name,
261275
socialSecurityRecord: [{ personalNumber: NINO }],
262-
},
263-
});
276+
}
277+
}
278+
expect(reqSentEvents[0].event).toStrictEqual(expectedRequestSentAuditEvent);
264279

265280
const resReceivedEvents = await pollForTestHarnessEvents(RESPONSE_RECEIVED_EVENT_NAME, sessionId);
266281
expect(resReceivedEvents).toHaveLength(1);
267-
expect(resReceivedEvents[0].event).toStrictEqual<AuditEvent>({
282+
283+
const expectedRequestReceivedAuditEvent: AuditEvent<NinoCheckAuditExtensions, never, NinoCheckAuditRestricted> = {
268284
...baseExpectedEvent(RESPONSE_RECEIVED_EVENT_NAME, sessionId),
269285
extensions: {
270286
evidence: { txn: expect.any(String) },
271-
},
272-
});
287+
}
288+
}
289+
expect(resReceivedEvents[0].event).toStrictEqual(expectedRequestReceivedAuditEvent);
273290
});
274291
});

‎integration-tests/api-gateway/e2e/e2e-happy-path.test.ts‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import { clearAttemptsTable, clearItemsFromTables } from "../../resources/dynamo
55
import { authorizationEndpoint, checkEndpoint, createSession, getJarAuthorization } from "../endpoints";
66
import { generatePrivateJwtParams } from "../crypto/private-key-jwt-helper";
77
import {
8-
AuditEvent,
8+
NinoCheckAuditExtensions,
9+
NinoCheckAuditRestricted,
910
baseExpectedEvent,
1011
END_EVENT_NAME,
1112
pollForTestHarnessEvents,
@@ -14,6 +15,7 @@ import {
1415
START_EVENT_NAME,
1516
VC_ISSUED_EVENT_NAME,
1617
} from "../audit";
18+
import { AuditEvent } from "@govuk-one-login/cri-audit";
1719

1820
let sessionData: Response;
1921
let authCode: { value: string };
@@ -142,27 +144,32 @@ describe("End to end happy path journey", () => {
142144

143145
const reqSentEvents = await pollForTestHarnessEvents(REQUEST_SENT_EVENT_NAME, sessionId);
144146
expect(reqSentEvents).toHaveLength(1);
145-
expect(reqSentEvents[0].event).toStrictEqual<AuditEvent>({
147+
148+
const expectedRequestSentAuditEvent: AuditEvent<never, never, NinoCheckAuditRestricted> = {
146149
...baseExpectedEvent(REQUEST_SENT_EVENT_NAME, sessionId),
147150
restricted: {
148151
birthDate: claimSet.shared_claims.birthDate,
149152
name: claimSet.shared_claims.name,
150153
socialSecurityRecord: [{ personalNumber: NINO }],
151-
},
152-
});
154+
}
155+
}
156+
expect(reqSentEvents[0].event).toStrictEqual(expectedRequestSentAuditEvent);
153157

154158
const resReceivedEvents = await pollForTestHarnessEvents(RESPONSE_RECEIVED_EVENT_NAME, sessionId);
155159
expect(resReceivedEvents).toHaveLength(1);
156-
expect(resReceivedEvents[0].event).toStrictEqual<AuditEvent>({
160+
161+
const expectedRequestReceivedAuditEvent: AuditEvent<NinoCheckAuditExtensions, never, NinoCheckAuditRestricted> = {
157162
...baseExpectedEvent(RESPONSE_RECEIVED_EVENT_NAME, sessionId),
158163
extensions: {
159164
evidence: { txn: expect.any(String) },
160-
},
161-
});
165+
}
166+
}
167+
expect(resReceivedEvents[0].event).toStrictEqual(expectedRequestReceivedAuditEvent);
162168

163169
const vcIssuedEvents = await pollForTestHarnessEvents(VC_ISSUED_EVENT_NAME, sessionId);
164170
expect(vcIssuedEvents).toHaveLength(1);
165-
expect(vcIssuedEvents[0].event).toEqual<AuditEvent>({
171+
172+
const expectedVCIssuedAuditEvent: AuditEvent<NinoCheckAuditExtensions, never, NinoCheckAuditRestricted> = {
166173
...baseExpectedEvent(VC_ISSUED_EVENT_NAME, sessionId),
167174
restricted: {
168175
birthDate: claimSet.shared_claims.birthDate,
@@ -180,8 +187,9 @@ describe("End to end happy path journey", () => {
180187
attemptNum: 1,
181188
},
182189
],
183-
},
184-
});
190+
}
191+
}
192+
expect(vcIssuedEvents[0].event).toEqual(expectedVCIssuedAuditEvent);
185193

186194
const endEvents = await pollForTestHarnessEvents(END_EVENT_NAME, sessionId);
187195
expect(endEvents).toHaveLength(1);

‎integration-tests/package.json‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,14 @@
1212
"compile": "tsc"
1313
},
1414
"devDependencies": {
15-
"@aws-sdk/client-cloudformation": "3.828.0",
16-
"@aws-sdk/client-kms": "3.828.0",
17-
"@aws-sdk/client-secrets-manager": "3.828.0",
18-
"@aws-sdk/client-sqs": "3.828.0",
19-
"@aws-sdk/client-ssm": "3.828.0",
20-
"@aws-sdk/lib-dynamodb": "3.828.0",
21-
"@aws-sdk/util-dynamodb": "3.828.0",
15+
"@aws-sdk/client-cloudformation": "3.934.0",
16+
"@aws-sdk/client-dynamodb": "3.934.0",
17+
"@aws-sdk/client-kms": "3.934.0",
18+
"@aws-sdk/client-secrets-manager": "3.934.0",
19+
"@aws-sdk/client-sqs": "3.934.0",
20+
"@aws-sdk/client-ssm": "3.934.0",
21+
"@aws-sdk/lib-dynamodb": "3.934.0",
22+
"@aws-sdk/util-dynamodb": "3.934.0",
2223
"@pact-foundation/pact": "16.0.0",
2324
"@types/express": "4.17.21",
2425
"@types/uuid": "9.0.8",
@@ -31,7 +32,7 @@
3132
"wait-on": "8.0.3"
3233
},
3334
"dependencies": {
34-
"@aws-sdk/credential-providers": "3.828.0",
35+
"@aws-sdk/credential-providers": "3.934.0",
3536
"aws-sigv4-fetch": "4.4.1"
3637
},
3738
"overrides": {

‎lambdas/abandon/src/abandon-handler.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@ import { AbandonHandlerConfig } from "./config/abandon-handler-config";
55
import { removeAuthCodeFromSessionRecord } from "./services/abandon-dynamo-service";
66
import { CriError } from "../../common/src/errors/cri-error";
77
import { handleErrorResponse } from "../../common/src/errors/cri-error-response";
8+
import { buildAndSendAuditEvent } from "@govuk-one-login/cri-audit";
89
import { logger } from "@govuk-one-login/cri-logger";
910
import { getSessionBySessionId } from "../../common/src/database/get-record-by-session-id";
10-
import { sendAuditEvent } from "../../common/src/util/audit";
11-
import { ABANDONED } from "../../common/src/types/audit";
11+
import { AUDIT_EVENT_TYPE } from "../../common/src/types/audit";
1212

1313
initOpenTelemetry();
1414

@@ -44,7 +44,7 @@ export class AbandonHandler implements LambdaInterface {
4444
restricted: { device_information: { encoded: txmaAuditHeader } },
4545
}
4646
: undefined;
47-
await sendAuditEvent(ABANDONED, this.config.audit, sessionItem, txmaAuditValue);
47+
await buildAndSendAuditEvent(this.config.audit.queueUrl, AUDIT_EVENT_TYPE.ABANDONED, this.config.audit.componentId, sessionItem, txmaAuditValue);
4848

4949
return {
5050
statusCode: 200,

‎lambdas/abandon/tests/abandon-handler.test.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ process.env.AUDIT_QUEUE_URL = "cool-queuez.com";
1313
process.env.AUDIT_COMPONENT_ID = "https://check-hmrc-time.account.gov.uk";
1414
import { AbandonHandler } from "../src/abandon-handler";
1515

16-
jest.mock("../../common/src/util/audit");
17-
import { sendAuditEvent } from "../../common/src/util/audit";
16+
jest.mock("@govuk-one-login/cri-audit");
17+
import { buildAndSendAuditEvent } from "@govuk-one-login/cri-audit";
1818

1919
const auditConfig = {
2020
queueUrl: "cool-queuez.com",
@@ -84,9 +84,10 @@ describe("abandon-handler", () => {
8484
TableName: "session-table",
8585
UpdateExpression: "SET authorizationCodeExpiryDate = :expiry REMOVE authorizationCode",
8686
});
87-
expect(sendAuditEvent).toHaveBeenCalledWith(
88-
"ABANDONED",
89-
auditConfig,
87+
expect(buildAndSendAuditEvent).toHaveBeenCalledWith(
88+
auditConfig.queueUrl,
89+
"IPV_HMRC_RECORD_CHECK_CRI_ABANDONED",
90+
auditConfig.componentId,
9091
{
9192
sessionId: "session-123",
9293
clientSessionId: "gov-123",
@@ -137,9 +138,10 @@ describe("abandon-handler", () => {
137138
const result = await abandonHandler.handler(event, {} as Context);
138139

139140
expect(result.statusCode).toEqual(200);
140-
expect(sendAuditEvent).toHaveBeenCalledWith(
141-
"ABANDONED",
142-
auditConfig,
141+
expect(buildAndSendAuditEvent).toHaveBeenCalledWith(
142+
auditConfig.queueUrl,
143+
"IPV_HMRC_RECORD_CHECK_CRI_ABANDONED",
144+
auditConfig.componentId,
143145
{
144146
sessionId: "session-123",
145147
clientSessionId: "gov-123",
@@ -270,7 +272,7 @@ describe("abandon-handler", () => {
270272
Count: 1,
271273
});
272274
ddbMock.on(UpdateItemCommand).resolves({});
273-
(sendAuditEvent as jest.Mock).mockRejectedValue(new Error("Audit failed"));
275+
(buildAndSendAuditEvent as jest.Mock).mockRejectedValue(new Error("Audit failed"));
274276

275277
const event = {
276278
body: JSON.stringify({}),

‎lambdas/common/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"compile": "tsc"
1212
},
1313
"dependencies": {
14-
"@aws-sdk/util-dynamodb": "3.828.0"
14+
"@aws-sdk/util-dynamodb": "3.934.0"
1515
},
1616
"devDependencies": {
1717
"@aws-lambda-powertools/logger": "2.28.1",

0 commit comments

Comments
 (0)