Skip to content

Commit e41442a

Browse files
alexnuttallalexnuttall
committed
validate all ids in server actions
1 parent a51dd20 commit e41442a

File tree

8 files changed

+150
-43
lines changed

8 files changed

+150
-43
lines changed

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

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const createTemplateMock = jest.mocked(createTemplate);
1414
const redirectMock = jest.mocked(redirect);
1515

1616
const initialState: EmailTemplate = {
17-
id: 'template-id',
17+
id: '89cc73bb-6d61-4b8b-a728-b5e40f0751fd',
1818
templateType: 'EMAIL',
1919
templateStatus: 'NOT_YET_SUBMITTED',
2020
name: 'name',
@@ -109,25 +109,52 @@ describe('CreateEmailTemplate server actions', () => {
109109
})
110110
);
111111

112-
expect(saveTemplateMock).toHaveBeenCalledWith({
112+
expect(saveTemplateMock).toHaveBeenCalledWith(initialState.id, {
113113
...initialState,
114114
name: 'template-name',
115115
subject: 'template-subject-line',
116116
message: 'template-message',
117117
});
118118

119119
expect(redirectMock).toHaveBeenCalledWith(
120-
'/preview-email-template/template-id?from=edit',
120+
`/preview-email-template/${initialState.id}?from=edit`,
121121
'push'
122122
);
123123
});
124124

125+
test('redirects to invalid-template if the ID in formState is not a uuid', async () => {
126+
const badIdState = {
127+
...initialState,
128+
id: 'non-uuid',
129+
name: 'template-name',
130+
subject: 'template-subject-line',
131+
message: 'template-message',
132+
createdAt: '2025-01-13T10:19:25.579Z',
133+
updatedAt: '2025-01-13T10:19:25.579Z',
134+
};
135+
136+
await processFormActions(
137+
badIdState,
138+
getMockFormData({
139+
emailTemplateName: 'template-name',
140+
emailTemplateSubjectLine: 'template-subject-line',
141+
emailTemplateMessage: 'template-message',
142+
})
143+
);
144+
145+
expect(saveTemplateMock).not.toHaveBeenCalled();
146+
147+
expect(redirectMock).toHaveBeenCalledWith('/invalid-template', 'replace');
148+
});
149+
125150
test('should create the template and redirect', async () => {
126151
const { id: _, ...initialDraftState } = initialState; // eslint-disable-line sonarjs/no-unused-vars
127152

153+
const generatedId = '06c6fb58-e749-4ee4-8343-57d7f7ecfe1f';
154+
128155
createTemplateMock.mockResolvedValue({
129156
...initialDraftState,
130-
id: 'new-template-id',
157+
id: generatedId,
131158
name: 'template-name',
132159
subject: 'template-subject-line',
133160
message: 'template-message',
@@ -152,7 +179,7 @@ describe('CreateEmailTemplate server actions', () => {
152179
});
153180

154181
expect(redirectMock).toHaveBeenCalledWith(
155-
'/preview-email-template/new-template-id?from=edit',
182+
`/preview-email-template/${generatedId}?from=edit`,
156183
'push'
157184
);
158185
});

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

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { getMockFormData } from '@testhelpers';
22
import { saveTemplate, createTemplate } from '@utils/form-actions';
3-
import {
3+
import type {
44
NHSAppTemplate,
55
CreateUpdateNHSAppTemplate,
66
} from 'nhs-notify-web-template-management-utils';
@@ -23,7 +23,7 @@ const initialState: CreateUpdateNHSAppTemplate = {
2323

2424
const savedState: NHSAppTemplate = {
2525
...initialState,
26-
id: 'template-id',
26+
id: 'a28c9e75-d6c9-4efe-879e-e082238938cf',
2727
templateStatus: 'NOT_YET_SUBMITTED',
2828
createdAt: '2025-01-13T10:19:25.579Z',
2929
updatedAt: '2025-01-13T10:19:25.579Z',
@@ -130,18 +130,37 @@ describe('CreateNHSAppTemplate server actions', () => {
130130
})
131131
);
132132

133-
expect(saveTemplateMock).toHaveBeenCalledWith({
133+
expect(saveTemplateMock).toHaveBeenCalledWith(savedState.id, {
134134
...savedState,
135135
name: 'template-name',
136136
message: 'template-message',
137137
});
138138

139139
expect(redirectMock).toHaveBeenCalledWith(
140-
`/preview-nhs-app-template/template-id?from=edit`,
140+
`/preview-nhs-app-template/${savedState.id}?from=edit`,
141141
'push'
142142
);
143143
});
144144

145+
test('redirects to invalid-template if the ID in formState is not a uuid', async () => {
146+
const badIdState = {
147+
...savedState,
148+
id: 'no-uuid',
149+
};
150+
151+
await processFormActions(
152+
badIdState,
153+
getMockFormData({
154+
nhsAppTemplateName: 'template-name',
155+
nhsAppTemplateMessage: 'template-message',
156+
})
157+
);
158+
159+
expect(saveTemplateMock).not.toHaveBeenCalled();
160+
161+
expect(redirectMock).toHaveBeenCalledWith('/invalid-template', 'replace');
162+
});
163+
145164
test('should save a template that contains a url with angle brackets, if they are url encoded', async () => {
146165
saveTemplateMock.mockResolvedValue({
147166
...savedState,
@@ -159,15 +178,15 @@ describe('CreateNHSAppTemplate server actions', () => {
159178
})
160179
);
161180

162-
expect(saveTemplateMock).toHaveBeenCalledWith({
181+
expect(saveTemplateMock).toHaveBeenCalledWith(savedState.id, {
163182
...savedState,
164183
name: 'template-name',
165184
message:
166185
'**a message linking to [example.com](https://www.example.com/withparams?param1=%3C%3E)**',
167186
});
168187

169188
expect(redirectMock).toHaveBeenCalledWith(
170-
`/preview-nhs-app-template/template-id?from=edit`,
189+
`/preview-nhs-app-template/${savedState.id}?from=edit`,
171190
'push'
172191
);
173192
});
@@ -190,7 +209,7 @@ describe('CreateNHSAppTemplate server actions', () => {
190209
});
191210

192211
expect(redirectMock).toHaveBeenCalledWith(
193-
'/preview-nhs-app-template/template-id?from=edit',
212+
`/preview-nhs-app-template/${savedState.id}?from=edit`,
194213
'push'
195214
);
196215
});

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

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ const createTemplateMock = jest.mocked(createTemplate);
1414
const redirectMock = jest.mocked(redirect);
1515

1616
const initialState: SMSTemplate = {
17-
id: 'template-id',
17+
id: '9e23faa1-521d-4708-8edb-be42ed4a3aae',
1818
templateType: 'SMS',
1919
templateStatus: 'NOT_YET_SUBMITTED',
2020
name: 'name',
@@ -102,24 +102,45 @@ describe('CreateSmsTemplate server actions', () => {
102102
})
103103
);
104104

105-
expect(saveTemplateMock).toHaveBeenCalledWith({
105+
expect(saveTemplateMock).toHaveBeenCalledWith(initialState.id, {
106106
...initialState,
107107
name: 'template-name',
108108
message: 'template-message',
109109
});
110110

111111
expect(redirectMock).toHaveBeenCalledWith(
112-
'/preview-text-message-template/template-id?from=edit',
112+
`/preview-text-message-template/${initialState.id}?from=edit`,
113113
'push'
114114
);
115115
});
116116

117+
test('redirects to invalid-template if the ID in formState is not a uuid', async () => {
118+
const badIdState = {
119+
...initialState,
120+
id: 'non-uuid',
121+
};
122+
123+
await processFormActions(
124+
badIdState,
125+
getMockFormData({
126+
smsTemplateName: 'template-name',
127+
smsTemplateMessage: 'template-message',
128+
})
129+
);
130+
131+
expect(saveTemplateMock).not.toHaveBeenCalled();
132+
133+
expect(redirectMock).toHaveBeenCalledWith('/invalid-template', 'replace');
134+
});
135+
117136
test('should create the template and redirect', async () => {
118137
const { id: _, ...initialDraftState } = initialState; // eslint-disable-line sonarjs/no-unused-vars
119138

139+
const generatedId = '1cc83326-2076-4917-9920-6b297c20d08a';
140+
120141
createTemplateMock.mockResolvedValue({
121142
...initialDraftState,
122-
id: 'new-template-id',
143+
id: generatedId,
123144
name: 'template-name',
124145
message: 'template-message',
125146
} as TemplateDto);
@@ -139,7 +160,7 @@ describe('CreateSmsTemplate server actions', () => {
139160
});
140161

141162
expect(redirectMock).toHaveBeenCalledWith(
142-
'/preview-text-message-template/new-template-id?from=edit',
163+
`/preview-text-message-template/${generatedId}?from=edit`,
143164
'push'
144165
);
145166
});

‎frontend/src/__tests__/utils/form-actions.test.ts‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ describe('form-actions', () => {
310310
});
311311

312312
const updateTemplateInput: NHSAppTemplate = {
313-
id: 'id',
313+
id: 'ee22daa2-9fce-455a-9e07-91679e4d7999',
314314
templateType: 'NHS_APP',
315315
templateStatus: 'NOT_YET_SUBMITTED',
316316
name: 'name',
@@ -319,7 +319,10 @@ describe('form-actions', () => {
319319
updatedAt: '2025-01-13T10:19:25.579Z',
320320
};
321321

322-
const response = await saveTemplate(updateTemplateInput);
322+
const response = await saveTemplate(
323+
updateTemplateInput.id,
324+
updateTemplateInput
325+
);
323326

324327
expect(mockedTemplateClient.updateTemplate).toHaveBeenCalledWith(
325328
updateTemplateInput.id,
@@ -341,7 +344,7 @@ describe('form-actions', () => {
341344
});
342345

343346
const updateTemplateInput: NHSAppTemplate = {
344-
id: 'id',
347+
id: 'bde7301a-e8c0-404a-8d19-c0b8ef7817f9',
345348
templateType: 'NHS_APP',
346349
templateStatus: 'NOT_YET_SUBMITTED',
347350
name: 'name',
@@ -350,9 +353,9 @@ describe('form-actions', () => {
350353
updatedAt: '2025-01-13T10:19:25.579Z',
351354
};
352355

353-
await expect(saveTemplate(updateTemplateInput)).rejects.toThrow(
354-
'Failed to save template data'
355-
);
356+
await expect(
357+
saveTemplate(updateTemplateInput.id, updateTemplateInput)
358+
).rejects.toThrow('Failed to save template data');
356359

357360
expect(mockedTemplateClient.updateTemplate).toHaveBeenCalledWith(
358361
updateTemplateInput.id,
@@ -369,7 +372,7 @@ describe('form-actions', () => {
369372
});
370373

371374
const updateTemplateInput: NHSAppTemplate = {
372-
id: 'id',
375+
id: 'bde7301a-e8c0-404a-8d19-c0b8ef7817f9',
373376
templateType: 'NHS_APP',
374377
templateStatus: 'NOT_YET_SUBMITTED',
375378
name: 'name',
@@ -378,9 +381,9 @@ describe('form-actions', () => {
378381
updatedAt: '2025-01-13T10:19:25.579Z',
379382
};
380383

381-
await expect(saveTemplate(updateTemplateInput)).rejects.toThrow(
382-
'Failed to get access token'
383-
);
384+
await expect(
385+
saveTemplate(updateTemplateInput.id, updateTemplateInput)
386+
).rejects.toThrow('Failed to get access token');
384387
});
385388

386389
test('getTemplate', async () => {

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,31 @@ export async function processFormActions(
5353
const { emailTemplateName, emailTemplateSubjectLine, emailTemplateMessage } =
5454
parsedForm.data;
5555

56-
const updatedTemplate = {
56+
const template = {
5757
...formState,
5858
name: emailTemplateName,
5959
subject: emailTemplateSubjectLine,
6060
message: emailTemplateMessage,
6161
};
6262

63-
const savedTemplate = await ('id' in updatedTemplate
64-
? saveTemplate(updatedTemplate)
65-
: createTemplate(updatedTemplate));
63+
let savedId: string;
64+
65+
if ('id' in template) {
66+
const { success, data: templateId } = z.uuid().safeParse(template.id);
67+
68+
if (!success) {
69+
return redirect('/invalid-template', RedirectType.replace);
70+
}
71+
72+
const saved = await saveTemplate(templateId, template);
73+
savedId = saved.id;
74+
} else {
75+
const saved = await createTemplate(template);
76+
savedId = saved.id;
77+
}
6678

6779
return redirect(
68-
`/preview-email-template/${savedTemplate.id}?from=edit`,
80+
`/preview-email-template/${savedId}?from=edit`,
6981
RedirectType.push
7082
);
7183
}

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,18 +71,30 @@ export async function processFormActions(
7171

7272
const { nhsAppTemplateName, nhsAppTemplateMessage } = parsedForm.data;
7373

74-
const updatedTemplate = {
74+
const template = {
7575
...formState,
7676
name: nhsAppTemplateName,
7777
message: nhsAppTemplateMessage,
7878
};
7979

80-
const savedTemplate = await ('id' in updatedTemplate
81-
? saveTemplate(updatedTemplate)
82-
: createTemplate(updatedTemplate));
80+
let savedId: string;
81+
82+
if ('id' in template) {
83+
const { success, data: templateId } = z.uuid().safeParse(template.id);
84+
85+
if (!success) {
86+
return redirect('/invalid-template', RedirectType.replace);
87+
}
88+
89+
const saved = await saveTemplate(templateId, template);
90+
savedId = saved.id;
91+
} else {
92+
const saved = await createTemplate(template);
93+
savedId = saved.id;
94+
}
8395

8496
return redirect(
85-
`/preview-nhs-app-template/${savedTemplate.id}?from=edit`,
97+
`/preview-nhs-app-template/${savedId}?from=edit`,
8698
RedirectType.push
8799
);
88100
}

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,30 @@ export async function processFormActions(
4949

5050
const { smsTemplateName, smsTemplateMessage } = parsedForm.data;
5151

52-
const updatedTemplate = {
52+
const template = {
5353
...formState,
5454
name: smsTemplateName,
5555
message: smsTemplateMessage,
5656
};
5757

58-
const savedTemplate = await ('id' in updatedTemplate
59-
? saveTemplate(updatedTemplate)
60-
: createTemplate(updatedTemplate));
58+
let savedId: string;
59+
60+
if ('id' in template) {
61+
const { success, data: templateId } = z.uuid().safeParse(template.id);
62+
63+
if (!success) {
64+
return redirect('/invalid-template', RedirectType.replace);
65+
}
66+
67+
const saved = await saveTemplate(templateId, template);
68+
savedId = saved.id;
69+
} else {
70+
const saved = await createTemplate(template);
71+
savedId = saved.id;
72+
}
6173

6274
return redirect(
63-
`/preview-text-message-template/${savedTemplate.id}?from=edit`,
75+
`/preview-text-message-template/${savedId}?from=edit`,
6476
RedirectType.push
6577
);
6678
}

0 commit comments

Comments
 (0)