Skip to content

Commit a51dd20

Browse files
alexnuttallalexnuttall
committed
add some templateId validation in form server actions to see if it resolves security alerts
1 parent a26d3f4 commit a51dd20

File tree

4 files changed

+63
-45
lines changed

4 files changed

+63
-45
lines changed

‎frontend/src/__tests__/components/forms/RequestProof/server-action.test.ts‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -15,29 +15,30 @@ const redirectMock = jest.mocked(redirect);
1515
const getTemplateMock = jest.mocked(getTemplate);
1616
const requestTemplateProofMock = jest.mocked(requestTemplateProof);
1717

18-
const mockLetterTemplate = {
19-
templateType: 'LETTER',
20-
templateStatus: 'NOT_YET_SUBMITTED',
21-
name: 'name',
22-
id: '1',
23-
createdAt: '2025-01-13T10:19:25.579Z',
24-
updatedAt: '2025-01-13T10:19:25.579Z',
25-
letterType: 'x0',
26-
language: 'en',
27-
files: {
28-
pdfTemplate: {
29-
currentVersion: 'a',
30-
virusScanStatus: 'PASSED',
31-
fileName: 'a.pdf',
18+
const mockLetterTemplate = (id: string) =>
19+
({
20+
templateType: 'LETTER',
21+
templateStatus: 'NOT_YET_SUBMITTED',
22+
name: 'name',
23+
id,
24+
createdAt: '2025-01-13T10:19:25.579Z',
25+
updatedAt: '2025-01-13T10:19:25.579Z',
26+
letterType: 'x0',
27+
language: 'en',
28+
files: {
29+
pdfTemplate: {
30+
currentVersion: 'a',
31+
virusScanStatus: 'PASSED',
32+
fileName: 'a.pdf',
33+
},
3234
},
33-
},
34-
} satisfies TemplateDto;
35+
}) satisfies TemplateDto;
3536

3637
describe('requestProof', () => {
3738
beforeEach(jest.resetAllMocks);
3839

3940
it('should redirect when templateId from form is invalid', async () => {
40-
const formData = getMockFormData({});
41+
const formData = getMockFormData({ templateId: 'not-uuid' });
4142

4243
await requestProof('LETTER', formData);
4344

@@ -49,7 +50,9 @@ describe('requestProof', () => {
4950
it('should redirect when template is not found in the DB', async () => {
5051
getTemplateMock.mockResolvedValueOnce(undefined);
5152

52-
const formData = getMockFormData({ templateId: '1' });
53+
const formData = getMockFormData({
54+
templateId: '2abc25f0-7e59-4d53-b20c-7547ef983789',
55+
});
5356

5457
await requestProof('LETTER', formData);
5558

@@ -61,22 +64,26 @@ describe('requestProof', () => {
6164
id: 'template-id',
6265
} as unknown as TemplateDto);
6366

64-
const formData = getMockFormData({ templateId: '1' });
67+
const formData = getMockFormData({
68+
templateId: '992fe769-f8b3-43a9-84f1-6e10d0480bb6',
69+
});
6570

6671
await requestProof('LETTER', formData);
6772

6873
expect(redirectMock).toHaveBeenCalledWith('/invalid-template', 'replace');
6974
});
7075

7176
it('should handle error when failing to save template', async () => {
72-
getTemplateMock.mockResolvedValueOnce(mockLetterTemplate);
77+
const templateId = '14216f4b-d01b-401c-8351-1356809174d9';
78+
79+
getTemplateMock.mockResolvedValueOnce(mockLetterTemplate(templateId));
7380

7481
requestTemplateProofMock.mockImplementationOnce(() => {
7582
throw new Error('failed to save template');
7683
});
7784

7885
const formData = getMockFormData({
79-
templateId: '1',
86+
templateId: '14216f4b-d01b-401c-8351-1356809174d9',
8087
});
8188

8289
await expect(requestProof('LETTER', formData)).rejects.toThrow(
@@ -85,18 +92,20 @@ describe('requestProof', () => {
8592
});
8693

8794
it('should redirect when successfully submitted', async () => {
88-
getTemplateMock.mockResolvedValueOnce(mockLetterTemplate);
95+
const templateId = '465eecc3-2ab8-4291-a898-ee6edcb03d33';
96+
97+
getTemplateMock.mockResolvedValueOnce(mockLetterTemplate(templateId));
8998

9099
const formData = getMockFormData({
91-
templateId: '1',
100+
templateId,
92101
});
93102

94103
await requestProof('LETTER', formData);
95104

96-
expect(requestTemplateProofMock).toHaveBeenCalledWith('1');
105+
expect(requestTemplateProofMock).toHaveBeenCalledWith(templateId);
97106

98107
expect(redirectMock).toHaveBeenCalledWith(
99-
'/preview-letter-template/1',
108+
`/preview-letter-template/${templateId}`,
100109
'push'
101110
);
102111
});

‎frontend/src/__tests__/components/forms/SubmitTemplate/server-action.test.ts‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,24 @@ const redirectMock = jest.mocked(redirect);
1515
const getTemplateMock = jest.mocked(getTemplate);
1616
const setTemplateToSubmittedMock = jest.mocked(setTemplateToSubmitted);
1717

18-
const mockNhsAppTemplate = {
19-
templateType: 'NHS_APP',
20-
templateStatus: 'NOT_YET_SUBMITTED',
21-
name: 'name',
22-
message: 'body',
23-
id: '1',
24-
createdAt: '2025-01-13T10:19:25.579Z',
25-
updatedAt: '2025-01-13T10:19:25.579Z',
26-
} satisfies TemplateDto;
18+
const mockNhsAppTemplate = (id: string) =>
19+
({
20+
templateType: 'NHS_APP',
21+
templateStatus: 'NOT_YET_SUBMITTED',
22+
name: 'name',
23+
message: 'body',
24+
id,
25+
createdAt: '2025-01-13T10:19:25.579Z',
26+
updatedAt: '2025-01-13T10:19:25.579Z',
27+
}) satisfies TemplateDto;
2728

2829
describe('submitTemplate', () => {
2930
beforeEach(() => {
3031
jest.resetAllMocks();
3132
});
3233

3334
it('should redirect when templateId from form is invalid', async () => {
34-
const formData = getMockFormData({});
35+
const formData = getMockFormData({ templateId: 'non-uuid' });
3536

3637
await submitTemplate('NHS_APP', formData);
3738

@@ -43,7 +44,9 @@ describe('submitTemplate', () => {
4344
it('should redirect when template is not found in the DB', async () => {
4445
getTemplateMock.mockResolvedValueOnce(undefined);
4546

46-
const formData = getMockFormData({ templateId: '1' });
47+
const formData = getMockFormData({
48+
templateId: '7bc9fac0-ad5e-4559-b614-ad10a59295aa',
49+
});
4750

4851
await submitTemplate('EMAIL', formData);
4952

@@ -55,22 +58,26 @@ describe('submitTemplate', () => {
5558
id: 'template-id',
5659
} as unknown as TemplateDto);
5760

58-
const formData = getMockFormData({ templateId: '1' });
61+
const formData = getMockFormData({
62+
templateId: 'ff32550d-6832-4837-ada0-b6dd5c09e7b8',
63+
});
5964

6065
await submitTemplate('EMAIL', formData);
6166

6267
expect(redirectMock).toHaveBeenCalledWith('/invalid-template', 'replace');
6368
});
6469

6570
it('should handle error when failing to save template', async () => {
66-
getTemplateMock.mockResolvedValueOnce(mockNhsAppTemplate);
71+
const templateId = '32b5005c-bfbb-4435-ae59-b4d54b225eb4';
72+
73+
getTemplateMock.mockResolvedValueOnce(mockNhsAppTemplate(templateId));
6774

6875
setTemplateToSubmittedMock.mockImplementationOnce(() => {
6976
throw new Error('failed to save template');
7077
});
7178

7279
const formData = getMockFormData({
73-
templateId: '1',
80+
templateId,
7481
});
7582

7683
await expect(submitTemplate('SMS', formData)).rejects.toThrow(
@@ -79,18 +86,20 @@ describe('submitTemplate', () => {
7986
});
8087

8188
it('should redirect when successfully submitted', async () => {
82-
getTemplateMock.mockResolvedValueOnce(mockNhsAppTemplate);
89+
const templateId = '7bc9fac0-ad5e-4559-b614-ad10a59295aa';
90+
91+
getTemplateMock.mockResolvedValueOnce(mockNhsAppTemplate(templateId));
8392

8493
const formData = getMockFormData({
85-
templateId: '1',
94+
templateId,
8695
});
8796

8897
await submitTemplate('SMS', formData);
8998

90-
expect(setTemplateToSubmittedMock).toHaveBeenCalledWith('1');
99+
expect(setTemplateToSubmittedMock).toHaveBeenCalledWith(templateId);
91100

92101
expect(redirectMock).toHaveBeenCalledWith(
93-
'/text-message-template-submitted/1',
102+
`/text-message-template-submitted/${templateId}`,
94103
'push'
95104
);
96105
});

‎frontend/src/components/forms/RequestProof/server-action.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { TemplateType } from 'nhs-notify-backend-client';
1212

1313
export async function requestProof(channel: TemplateType, formData: FormData) {
1414
const { success, data: templateId } = z
15-
.string()
15+
.uuid()
1616
.safeParse(formData.get('templateId'));
1717

1818
if (!success) {

‎frontend/src/components/forms/SubmitTemplate/server-action.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export async function submitTemplate(
1515
formData: FormData
1616
) {
1717
const { success, data: templateId } = z
18-
.string()
18+
.uuid()
1919
.safeParse(formData.get('templateId'));
2020

2121
if (!success) {

0 commit comments

Comments
 (0)