Skip to content

Commit d9be1f8

Browse files
alexnuttallalexnuttall
committed
remove user ownership from letter validation pipeline
1 parent 7382bc8 commit d9be1f8

22 files changed

+110
-216
lines changed

‎infrastructure/terraform/modules/backend-api/spec.tmpl.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,6 @@
208208
"files": {
209209
"$ref": "#/components/schemas/LetterFiles"
210210
},
211-
"owner": {
212-
"type": "string"
213-
},
214211
"personalisationParameters": {
215212
"items": {
216213
"type": "string"

‎lambdas/backend-api/src/__tests__/templates/api/process-proof.test.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,7 @@ test.each([
4141
'calls dependencies as expected for a %s virus scan',
4242
async (scanResultStatus, virusScanStatus, s3Expectation) => {
4343
const templateRepository = mockDeep<TemplateRepository>({
44-
getClientId: jest
45-
.fn()
46-
.mockReturnValue({ clientId: 'template-owner', clientOwned: true }),
44+
getClientId: jest.fn().mockReturnValue('template-owner'),
4745
});
4846
const letterFileRepository = mockDeep<LetterFileRepository>();
4947

@@ -73,7 +71,6 @@ test.each([
7371
{
7472
clientId: 'template-owner',
7573
templateId: 'template-id',
76-
clientOwned: true,
7774
},
7875
'proof.pdf',
7976
virusScanStatus,

‎lambdas/backend-api/src/__tests__/templates/api/set-letter-upload-virus-scan-status.test.ts‎

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,6 @@ it('sets the virus scan status on pdf uploads identified by file metadata', asyn
2929
},
3030
});
3131

32-
mocks.templateRepository.getClientId.mockResolvedValueOnce({
33-
clientId: 'template-owner',
34-
clientOwned: true,
35-
});
36-
3732
await handler(event);
3833

3934
expect(
@@ -42,7 +37,6 @@ it('sets the virus scan status on pdf uploads identified by file metadata', asyn
4237
{
4338
templateId: 'template-id',
4439
clientId: 'template-owner',
45-
clientOwned: true,
4640
},
4741
'pdf-template',
4842
'template-version',
@@ -64,11 +58,6 @@ it('sets the virus scan status on csv files identified by file metadata', async
6458
},
6559
});
6660

67-
mocks.templateRepository.getClientId.mockResolvedValueOnce({
68-
clientId: 'template-owner',
69-
clientOwned: true,
70-
});
71-
7261
await handler(event);
7362

7463
expect(
@@ -77,7 +66,6 @@ it('sets the virus scan status on csv files identified by file metadata', async
7766
{
7867
templateId: 'template-id',
7968
clientId: 'template-owner',
80-
clientOwned: true,
8169
},
8270
'test-data',
8371
'template-version',
@@ -102,11 +90,6 @@ it.each($GuardDutyMalwareScanStatusFailed.options)(
10290
},
10391
});
10492

105-
mocks.templateRepository.getClientId.mockResolvedValueOnce({
106-
clientId: 'template-owner',
107-
clientOwned: true,
108-
});
109-
11093
await handler(event);
11194

11295
expect(
@@ -115,7 +98,6 @@ it.each($GuardDutyMalwareScanStatusFailed.options)(
11598
{
11699
templateId: 'template-id',
117100
clientId: 'template-owner',
118-
clientOwned: true,
119101
},
120102
'pdf-template',
121103
'template-version',
@@ -199,11 +181,6 @@ it('errors if status update fails', async () => {
199181
},
200182
});
201183

202-
mocks.templateRepository.getClientId.mockResolvedValueOnce({
203-
clientId: 'template-owner',
204-
clientOwned: true,
205-
});
206-
207184
const error = new Error('Status Update error');
208185

209186
mocks.templateRepository.setLetterFileVirusScanStatusForUpload.mockRejectedValueOnce(
@@ -212,27 +189,3 @@ it('errors if status update fails', async () => {
212189

213190
await expect(handler(event)).rejects.toThrow(error);
214191
});
215-
216-
it('errors if owner from database does not match s3 path', async () => {
217-
const { handler, mocks } = setup();
218-
const event = makeGuardDutyMalwareScanResultNotificationEvent({
219-
detail: {
220-
s3ObjectDetails: {
221-
bucketName: 'quarantine-bucket',
222-
objectKey:
223-
'pdf-template/template-owner/template-id/template-version.pdf',
224-
versionId: 'pdf-s3-version-id',
225-
},
226-
scanResultDetails: { scanResultStatus: 'NO_THREATS_FOUND' },
227-
},
228-
});
229-
230-
mocks.templateRepository.getClientId.mockResolvedValueOnce({
231-
clientId: 'someone-else',
232-
clientOwned: true,
233-
});
234-
235-
await expect(handler(event)).rejects.toThrow(
236-
'Database owner and s3 owner mismatch'
237-
);
238-
});

‎lambdas/backend-api/src/__tests__/templates/api/validate-letter-template-files.test.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ describe('guard duty handler', () => {
124124
);
125125

126126
expect(mocks.TemplatePdf).toHaveBeenCalledWith(
127-
{ templateId, clientId: userId, clientOwned: false },
127+
{ templateId, clientId: userId },
128128
pdfData
129129
);
130130
expect(mocks.TestDataCsv).toHaveBeenCalledWith(csvData);
@@ -137,7 +137,7 @@ describe('guard duty handler', () => {
137137
expect(
138138
mocks.templateRepository.setLetterValidationResult
139139
).toHaveBeenCalledWith(
140-
{ templateId, clientId: userId, clientOwned: false },
140+
{ templateId, clientId: userId },
141141
versionId,
142142
true,
143143
pdf.personalisationParameters,
@@ -206,7 +206,7 @@ describe('guard duty handler', () => {
206206
);
207207

208208
expect(mocks.TemplatePdf).toHaveBeenCalledWith(
209-
{ templateId, clientId, clientOwned: true },
209+
{ templateId, clientId },
210210
pdfData
211211
);
212212

@@ -220,7 +220,7 @@ describe('guard duty handler', () => {
220220
expect(
221221
mocks.templateRepository.setLetterValidationResult
222222
).toHaveBeenCalledWith(
223-
{ templateId, clientId, clientOwned: true },
223+
{ templateId, clientId },
224224
versionId,
225225
true,
226226
pdf.personalisationParameters,
@@ -295,7 +295,7 @@ describe('guard duty handler', () => {
295295

296296
// assert
297297
expect(mocks.TemplatePdf).toHaveBeenCalledWith(
298-
{ templateId, clientId: userId, clientOwned: false },
298+
{ templateId, clientId: userId },
299299
pdfData
300300
);
301301
expect(mocks.TestDataCsv).toHaveBeenCalledWith(csvData);
@@ -305,7 +305,7 @@ describe('guard duty handler', () => {
305305
expect(
306306
mocks.templateRepository.setLetterValidationResult
307307
).toHaveBeenCalledWith(
308-
{ templateId, clientId: userId, clientOwned: false },
308+
{ templateId, clientId: userId },
309309
versionId,
310310
true,
311311
['firstName', 'parameter_1', 'unknown_parameter'],
@@ -385,7 +385,7 @@ describe('guard duty handler', () => {
385385
expect(
386386
mocks.templateRepository.setLetterValidationResult
387387
).toHaveBeenCalledWith(
388-
{ templateId, clientId: userId, clientOwned: false },
388+
{ templateId, clientId: userId },
389389
versionId,
390390
false,
391391
['firstName', 'parameter_1', 'unknown_parameter'],
@@ -453,7 +453,7 @@ describe('guard duty handler', () => {
453453
);
454454

455455
expect(mocks.TemplatePdf).toHaveBeenCalledWith(
456-
{ templateId, clientId: userId, clientOwned: false },
456+
{ templateId, clientId: userId },
457457
pdfData
458458
);
459459
expect(mocks.TestDataCsv).not.toHaveBeenCalled();
@@ -468,7 +468,7 @@ describe('guard duty handler', () => {
468468
expect(
469469
mocks.templateRepository.setLetterValidationResult
470470
).toHaveBeenCalledWith(
471-
{ templateId, clientId: userId, clientOwned: false },
471+
{ templateId, clientId: userId },
472472
versionId,
473473
true,
474474
pdf.personalisationParameters,
@@ -1157,7 +1157,7 @@ describe('guard duty handler', () => {
11571157
expect(
11581158
mocks.templateRepository.setLetterValidationResult
11591159
).toHaveBeenCalledWith(
1160-
{ templateId, clientId: userId, clientOwned: false },
1160+
{ templateId, clientId: userId },
11611161
versionId,
11621162
false,
11631163
[],
@@ -1221,7 +1221,7 @@ describe('guard duty handler', () => {
12211221
expect(
12221222
mocks.templateRepository.setLetterValidationResult
12231223
).toHaveBeenCalledWith(
1224-
{ templateId, clientId: userId, clientOwned: false },
1224+
{ templateId, clientId: userId },
12251225
versionId,
12261226
false,
12271227
[],
@@ -1285,7 +1285,7 @@ describe('guard duty handler', () => {
12851285
expect(
12861286
mocks.templateRepository.setLetterValidationResult
12871287
).toHaveBeenCalledWith(
1288-
{ templateId, clientId: userId, clientOwned: false },
1288+
{ templateId, clientId: userId },
12891289
versionId,
12901290
false,
12911291
pdf.personalisationParameters,

‎lambdas/backend-api/src/__tests__/templates/app/template-client.test.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ describe('templateClient', () => {
363363
updatedAt: updateTime,
364364
};
365365

366-
const { version: _, ...expectedDto } = finalTemplate;
366+
const { version: _1, owner: _2, ...expectedDto } = finalTemplate;
367367

368368
mocks.templateRepository.create.mockResolvedValueOnce({
369369
data: initialCreatedTemplate,
@@ -508,7 +508,7 @@ describe('templateClient', () => {
508508
updatedAt: updateTime,
509509
};
510510

511-
const { version: _, ...expectedDto } = finalTemplate;
511+
const { version: _1, owner: _2, ...expectedDto } = finalTemplate;
512512

513513
mocks.templateRepository.create.mockResolvedValueOnce({
514514
data: initialCreatedTemplate,
@@ -1911,7 +1911,6 @@ describe('templateClient', () => {
19111911
},
19121912
},
19131913
id: templateId,
1914-
owner: user.userId,
19151914
language: 'en',
19161915
letterType: 'x1',
19171916
name: templateName,

‎lambdas/backend-api/src/__tests__/templates/domain/template-pdf.test.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ test('has the given key attributes', () => {
99
{
1010
templateId: 'template-id',
1111
clientId: 'template-owner',
12-
clientOwned: true,
1312
},
1413
Buffer.from('')
1514
);
@@ -26,7 +25,6 @@ test('parse with no custom personalisation', async () => {
2625
{
2726
templateId: 'template-id',
2827
clientId: 'template-owner',
29-
clientOwned: true,
3028
},
3129
file
3230
);
@@ -77,7 +75,6 @@ test('parse with custom personalisation', async () => {
7775
{
7876
templateId: 'template-id',
7977
clientId: 'template-owner',
80-
clientOwned: true,
8178
},
8279
file
8380
);
@@ -139,7 +136,6 @@ test('errors if parse is not called before reading personalisation', () => {
139136
{
140137
templateId: 'template-id',
141138
clientId: 'template-owner',
142-
clientOwned: true,
143139
},
144140
file
145141
);
@@ -167,7 +163,6 @@ test('errors if file cannot be parsed', async () => {
167163
{
168164
templateId: 'template-id',
169165
clientId: 'template-owner',
170-
clientOwned: true,
171166
},
172167
file
173168
);
@@ -184,7 +179,6 @@ test('errors if file cannot be opened', async () => {
184179
{
185180
templateId: 'template-id',
186181
clientId: 'template-owner',
187-
clientOwned: true,
188182
},
189183
file
190184
);

0 commit comments

Comments
 (0)