mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-08-12 09:58:50 -04:00
Fix HTML being escaped in preview of email subject
`EmailPreviewTemplate.subject` returns a string of HTML, with any user-submitted HTML already escaped:b5a61bfb7b/notifications_utils/template.py (L672)What won’t be escaped is the HTML needed to redact the placeholders. We generate this HTML so we know its safe, and doesn’t need to be escaped. However when we pass it to Jinja, Jinja doesn’t know this, so will try to escape it. This means users will see the raw HTML. We can get around this by using Flask’s `Markup` class to tell Jinja that the string is already sanitised and doesn’t need escaping again. Text message templates don’t have this problem because they already return `Markup`:b5a61bfb7b/notifications_utils/template.py (L288)Letter templates don’t suffer from this problem (because they don’t support redaction) but without making the same change they would still double-escape ampersands, greater-than symbols, and so on.
This commit is contained in:
@@ -3,6 +3,7 @@
|
||||
from functools import partial
|
||||
|
||||
from flask import (
|
||||
Markup,
|
||||
Response,
|
||||
abort,
|
||||
flash,
|
||||
@@ -438,14 +439,14 @@ def get_preview_of_content(notification):
|
||||
))
|
||||
|
||||
if notification['template']['template_type'] == 'email':
|
||||
return EmailPreviewTemplate(
|
||||
return Markup(EmailPreviewTemplate(
|
||||
notification['template'],
|
||||
notification['personalisation'],
|
||||
redact_missing_personalisation=True,
|
||||
).subject
|
||||
).subject)
|
||||
|
||||
if notification['template']['template_type'] == 'letter':
|
||||
return LetterPreviewTemplate(
|
||||
return Markup(LetterPreviewTemplate(
|
||||
notification['template'],
|
||||
notification['personalisation'],
|
||||
).subject
|
||||
).subject)
|
||||
|
||||
@@ -648,29 +648,46 @@ def test_html_contains_links_for_failed_notifications(
|
||||
assert normalize_spaces(link_text) == 'Technical failure'
|
||||
|
||||
|
||||
@pytest.mark.parametrize('notification_type, expected_row_contents', (
|
||||
('sms', (
|
||||
'07123456789 hello & welcome hidden'
|
||||
)),
|
||||
('email', (
|
||||
'example@gov.uk hidden, hello & welcome'
|
||||
)),
|
||||
('letter', (
|
||||
# Letters don’t support redaction, but this test is still
|
||||
# worthwhile to show that the ampersand is not double-escaped
|
||||
'1 Example Street ((name)), hello & welcome'
|
||||
)),
|
||||
))
|
||||
def test_redacts_templates_that_should_be_redacted(
|
||||
client_request,
|
||||
mocker,
|
||||
mock_get_service_statistics,
|
||||
mock_get_service_data_retention,
|
||||
mock_get_no_api_keys,
|
||||
notification_type,
|
||||
expected_row_contents,
|
||||
):
|
||||
notifications = create_notifications(
|
||||
status='technical-failure',
|
||||
content='hello ((name))',
|
||||
content='hello & welcome ((name))',
|
||||
subject='((name)), hello & welcome',
|
||||
personalisation={'name': 'Jo'},
|
||||
redact_personalisation=True,
|
||||
template_type=notification_type,
|
||||
)
|
||||
mocker.patch('app.notification_api_client.get_notifications_for_service', return_value=notifications)
|
||||
|
||||
page = client_request.get(
|
||||
'main.view_notifications',
|
||||
service_id=SERVICE_ONE_ID,
|
||||
message_type='sms',
|
||||
message_type=notification_type,
|
||||
)
|
||||
|
||||
assert normalize_spaces(page.select('tbody tr th')[0].text) == (
|
||||
'07123456789 hello hidden'
|
||||
expected_row_contents
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user