Skip to content

Commit 0c64e15

Browse files
CCM-12065: Url validation (#700)
Co-authored-by: alex.nuttall1 <[email protected]>
1 parent eeb91d4 commit 0c64e15

File tree

13 files changed

+362
-1
lines changed

13 files changed

+362
-1
lines changed

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,28 @@ describe('CreateEmailTemplate server actions', () => {
6868
});
6969
});
7070

71+
it('create-email-template - should return response when when template message contains insecure url', async () => {
72+
const response = await processFormActions(
73+
initialState,
74+
getMockFormData({
75+
'form-id': 'create-email-template',
76+
emailTemplateName: 'template-name',
77+
emailTemplateSubjectLine: 'template-subject-line',
78+
emailTemplateMessage: '**a message linking to http://www.example.com**',
79+
})
80+
);
81+
82+
expect(response).toEqual({
83+
...initialState,
84+
errorState: {
85+
formErrors: [],
86+
fieldErrors: {
87+
emailTemplateMessage: ['URLs must start with https://'],
88+
},
89+
},
90+
});
91+
});
92+
7193
test('should save the template and redirect', async () => {
7294
saveTemplateMock.mockResolvedValue({
7395
...initialState,

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,50 @@ describe('CreateNHSAppTemplate server actions', () => {
7171
});
7272
});
7373

74+
it('create-nhs-app-template - should return response when when template message contains insecure url', async () => {
75+
const response = await processFormActions(
76+
initialState,
77+
getMockFormData({
78+
'form-id': 'create-nhs-app-template',
79+
nhsAppTemplateName: 'template-name',
80+
nhsAppTemplateMessage:
81+
'**a message linking to http://www.example.com**',
82+
})
83+
);
84+
85+
expect(response).toEqual({
86+
...initialState,
87+
errorState: {
88+
formErrors: [],
89+
fieldErrors: {
90+
nhsAppTemplateMessage: ['URLs must start with https://'],
91+
},
92+
},
93+
});
94+
});
95+
96+
it('create-nhs-app-template - should return response when when template message contains link with angle brackets', async () => {
97+
const response = await processFormActions(
98+
initialState,
99+
getMockFormData({
100+
'form-id': 'create-nhs-app-template',
101+
nhsAppTemplateName: 'template-name',
102+
nhsAppTemplateMessage:
103+
'**a message linking to [example.com](https://www.example.com/withparams?param1=<>)**',
104+
})
105+
);
106+
107+
expect(response).toEqual({
108+
...initialState,
109+
errorState: {
110+
formErrors: [],
111+
fieldErrors: {
112+
nhsAppTemplateMessage: ['URLs cannot include the symbols < or >'],
113+
},
114+
},
115+
});
116+
});
117+
74118
test('should save the template and redirect', async () => {
75119
saveTemplateMock.mockResolvedValue({
76120
...savedState,
@@ -98,6 +142,36 @@ describe('CreateNHSAppTemplate server actions', () => {
98142
);
99143
});
100144

145+
test('should save a template that contains a url with angle brackets, if they are url encoded', async () => {
146+
saveTemplateMock.mockResolvedValue({
147+
...savedState,
148+
name: 'template-name',
149+
message:
150+
'**a message linking to [example.com](https://www.example.com/withparams?param1=%3C%3E)**',
151+
});
152+
153+
await processFormActions(
154+
savedState,
155+
getMockFormData({
156+
nhsAppTemplateName: 'template-name',
157+
nhsAppTemplateMessage:
158+
'**a message linking to [example.com](https://www.example.com/withparams?param1=%3C%3E)**',
159+
})
160+
);
161+
162+
expect(saveTemplateMock).toHaveBeenCalledWith({
163+
...savedState,
164+
name: 'template-name',
165+
message:
166+
'**a message linking to [example.com](https://www.example.com/withparams?param1=%3C%3E)**',
167+
});
168+
169+
expect(redirectMock).toHaveBeenCalledWith(
170+
`/preview-nhs-app-template/template-id?from=edit`,
171+
'push'
172+
);
173+
});
174+
101175
test('should create the template and redirect', async () => {
102176
createTemplateMock.mockResolvedValue(savedState);
103177

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,28 @@ describe('CreateSmsTemplate server actions', () => {
6565
});
6666
});
6767

68+
it('create-sms-template - should return response when when template message contains insecure url', async () => {
69+
const response = await processFormActions(
70+
initialState,
71+
getMockFormData({
72+
'form-id': 'create-sms-template',
73+
smsTemplateName: 'template-name',
74+
smsTemplateMessage:
75+
'a message linking to http://www.example.com with http',
76+
})
77+
);
78+
79+
expect(response).toEqual({
80+
...initialState,
81+
errorState: {
82+
formErrors: [],
83+
fieldErrors: {
84+
smsTemplateMessage: ['URLs must start with https://'],
85+
},
86+
},
87+
});
88+
});
89+
6890
test('should save the template and redirect', async () => {
6991
saveTemplateMock.mockResolvedValue({
7092
...initialState,

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ export const $EmailTemplateFormSchema = z.object({
2727
.min(1, { message: form.emailTemplateMessage.error.empty })
2828
.max(MAX_EMAIL_CHARACTER_LENGTH, {
2929
message: form.emailTemplateMessage.error.max,
30+
})
31+
.refine((templateMessage) => !templateMessage.includes('http://'), {
32+
message: form.emailTemplateMessage.error.insecureLink,
3033
}),
3134
});
3235

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import markdownit from 'markdown-it';
12
import {
23
TemplateFormState,
34
NHSAppTemplate,
@@ -14,14 +15,41 @@ const {
1415
},
1516
} = content;
1617

18+
const hasInvalidCharactersInLinks = (message: string): boolean => {
19+
const md = markdownit();
20+
21+
const parsedMessage = md.parseInline(message, {});
22+
// [] branch should be unreachable
23+
/* istanbul ignore next */
24+
const tokens = parsedMessage[0]?.children || [];
25+
26+
for (const token of tokens) {
27+
if (token.type === 'link_open') {
28+
const href = token.attrGet('href');
29+
if (href && !message.includes(href)) {
30+
// markdown-it url-encodes angle brackets during parsing, so the original url must have included them
31+
return true;
32+
}
33+
}
34+
}
35+
return false;
36+
};
37+
1738
export const $CreateNhsAppTemplateSchema = z.object({
1839
nhsAppTemplateName: z
1940
.string({ message: form.nhsAppTemplateName.error.empty })
2041
.min(1, { message: form.nhsAppTemplateName.error.empty }),
2142
nhsAppTemplateMessage: z
2243
.string({ message: form.nhsAppTemplateMessage.error.empty })
2344
.min(1, { message: form.nhsAppTemplateMessage.error.empty })
24-
.max(5000, { message: form.nhsAppTemplateMessage.error.max }),
45+
.max(5000, { message: form.nhsAppTemplateMessage.error.max })
46+
.refine((templateMessage) => !templateMessage.includes('http://'), {
47+
message: form.nhsAppTemplateMessage.error.insecureLink,
48+
})
49+
.refine(
50+
(templateMessage) => !hasInvalidCharactersInLinks(templateMessage),
51+
{ message: form.nhsAppTemplateMessage.error.invalidUrlCharacter }
52+
),
2553
});
2654

2755
export async function processFormActions(

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ export const $CreateSmsTemplateSchema = z.object({
2424
.min(1, { message: form.smsTemplateMessage.error.empty })
2525
.max(MAX_SMS_CHARACTER_LENGTH, {
2626
message: form.smsTemplateMessage.error.max,
27+
})
28+
.refine((templateMessage) => !templateMessage.includes('http://'), {
29+
message: form.smsTemplateMessage.error.insecureLink,
2730
}),
2831
});
2932

frontend/src/content/content.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ const goBackButtonText = 'Go back';
1010
const enterATemplateName = 'Enter a template name';
1111
const enterATemplateMessage = 'Enter a template message';
1212
const templateMessageTooLong = 'Template message too long';
13+
const templateMessageHasInsecureLink = 'URLs must start with https://';
1314
const selectAnOption = 'Select an option';
1415

1516
const header = {
@@ -793,6 +794,8 @@ const templateFormNhsApp = {
793794
error: {
794795
empty: enterATemplateMessage,
795796
max: templateMessageTooLong,
797+
insecureLink: templateMessageHasInsecureLink,
798+
invalidUrlCharacter: 'URLs cannot include the symbols < or >',
796799
},
797800
},
798801
},
@@ -896,6 +899,7 @@ const templateFormEmail = {
896899
error: {
897900
empty: enterATemplateMessage,
898901
max: templateMessageTooLong,
902+
insecureLink: templateMessageHasInsecureLink,
899903
},
900904
},
901905
},
@@ -939,6 +943,7 @@ const templateFormSms = {
939943
error: {
940944
empty: enterATemplateMessage,
941945
max: templateMessageTooLong,
946+
insecureLink: templateMessageHasInsecureLink,
942947
},
943948
},
944949
},

tests/test-team/template-mgmt-component-tests/email/template-mgmt-create-email-page.component.spec.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,5 +338,38 @@ test.describe('Create Email message template Page', () => {
338338

339339
await expect(createEmailTemplatePage.messageTextArea).toBeFocused();
340340
});
341+
342+
test('when user submits form with an http link, then an error is displayed', async ({
343+
page,
344+
}) => {
345+
const errorMessage = 'URLs must start with https://';
346+
347+
const createEmailTemplatePage = new TemplateMgmtCreateEmailPage(page);
348+
349+
await createEmailTemplatePage.loadPage();
350+
351+
await createEmailTemplatePage.nameInput.fill('template-name');
352+
353+
await createEmailTemplatePage.subjectLineInput.fill(
354+
'template-subject-line'
355+
);
356+
357+
await createEmailTemplatePage.messageTextArea.fill(
358+
'http://www.example.com'
359+
);
360+
361+
await createEmailTemplatePage.clickSaveAndPreviewButton();
362+
363+
const emailMessageErrorLink =
364+
createEmailTemplatePage.errorSummary.locator(
365+
'[href="#emailTemplateMessage"]'
366+
);
367+
368+
await expect(emailMessageErrorLink).toHaveText(errorMessage);
369+
370+
await emailMessageErrorLink.click();
371+
372+
await expect(createEmailTemplatePage.messageTextArea).toBeFocused();
373+
});
341374
});
342375
});

tests/test-team/template-mgmt-component-tests/email/template-mgmt-edit-email-page.component.spec.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,5 +419,38 @@ test.describe('Edit Email message template Page', () => {
419419

420420
await expect(editEmailTemplatePage.messageTextArea).toBeFocused();
421421
});
422+
423+
test('when user submits form with an http link, then an error is displayed', async ({
424+
page,
425+
}) => {
426+
const errorMessage = 'URLs must start with https://';
427+
428+
const createEmailTemplatePage = new TemplateMgmtEditEmailPage(page);
429+
430+
await createEmailTemplatePage.loadPage(templates.valid.id);
431+
432+
await createEmailTemplatePage.nameInput.fill('template-name');
433+
434+
await createEmailTemplatePage.subjectLineInput.fill(
435+
'template-subject-line'
436+
);
437+
438+
await createEmailTemplatePage.messageTextArea.fill(
439+
'http://www.example.com'
440+
);
441+
442+
await createEmailTemplatePage.clickSaveAndPreviewButton();
443+
444+
const emailMessageErrorLink =
445+
createEmailTemplatePage.errorSummary.locator(
446+
'[href="#emailTemplateMessage"]'
447+
);
448+
449+
await expect(emailMessageErrorLink).toHaveText(errorMessage);
450+
451+
await emailMessageErrorLink.click();
452+
453+
await expect(createEmailTemplatePage.messageTextArea).toBeFocused();
454+
});
422455
});
423456
});

tests/test-team/template-mgmt-component-tests/nhs-app/template-mgmt-create-nhs-app-template-page.component.spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,48 @@ test.describe('Create NHS App Template Page', () => {
156156
expect(messageContent).toHaveLength(5000);
157157
});
158158

159+
test('Validate error messages on the create NHS App message template page with http url in message', async ({
160+
page,
161+
}) => {
162+
const createTemplatePage = new TemplateMgmtCreateNhsAppPage(page);
163+
164+
await createTemplatePage.loadPage();
165+
await expect(createTemplatePage.pageHeading).toHaveText(
166+
'Create NHS App message template'
167+
);
168+
await page.locator('[id="nhsAppTemplateName"]').fill('template-name');
169+
await page
170+
.locator('[id="nhsAppTemplateMessage"]')
171+
.fill('http://www.example.com');
172+
await createTemplatePage.clickSaveAndPreviewButton();
173+
await expect(page.locator('.nhsuk-error-summary')).toBeVisible();
174+
175+
await expect(
176+
page.locator('ul[class="nhsuk-list nhsuk-error-summary__list"] > li')
177+
).toHaveText(['URLs must start with https://']);
178+
});
179+
180+
test('Validate error messages on the create NHS App message template page with angle brackets in linked url', async ({
181+
page,
182+
}) => {
183+
const createTemplatePage = new TemplateMgmtCreateNhsAppPage(page);
184+
185+
await createTemplatePage.loadPage();
186+
await expect(createTemplatePage.pageHeading).toHaveText(
187+
'Create NHS App message template'
188+
);
189+
await page.locator('[id="nhsAppTemplateName"]').fill('template-name');
190+
await page
191+
.locator('[id="nhsAppTemplateMessage"]')
192+
.fill('[example](https://www.example.com/<>)');
193+
await createTemplatePage.clickSaveAndPreviewButton();
194+
await expect(page.locator('.nhsuk-error-summary')).toBeVisible();
195+
196+
await expect(
197+
page.locator('ul[class="nhsuk-list nhsuk-error-summary__list"] > li')
198+
).toHaveText(['URLs cannot include the symbols < or >']);
199+
});
200+
159201
const detailsSections = [
160202
'pds-personalisation-fields',
161203
'custom-personalisation-fields',

0 commit comments

Comments
 (0)