From 5c5d5286dec87967247157b3e64866cb50d48797 Mon Sep 17 00:00:00 2001 From: Michael Wellman Date: Fri, 13 Dec 2024 16:11:32 -0500 Subject: [PATCH] #1357 Redact Personalization in Serialized Notifications (#2180) --- .talismanrc | 6 +- app/models.py | 9 ++- tests/app/test_model.py | 2 +- .../notifications/test_get_notifications.py | 55 ++++++++++++++++++- 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/.talismanrc b/.talismanrc index 565dfae0c0..0832a6f9ad 100644 --- a/.talismanrc +++ b/.talismanrc @@ -69,6 +69,8 @@ fileignoreconfig: checksum: 6ffb8742a19c5b834c608826fd459cc1b6ea35ebfffd2d929a3a0f269c74183d - filename: tests/app/celery/test_service_callback_tasks.py checksum: 70575434f7a4fedd43d4c9164bc899a606768526d432c364db372524eec26542 +- filename: tests/app/clients/test_twilio.py + checksum: cad49e634cc5ba56157358aa3dfba2dafe7b9dbd3a0c580ec5cda3072f6a76e5 - filename: tests/app/conftest.py checksum: a80aa727586db82ed1b50bdb81ddfe1379e649a9dfc1ece2c36047486b41b83d - filename: tests/app/dao/test_api_key_dao.py @@ -77,6 +79,8 @@ fileignoreconfig: checksum: 4e15e63d349635131173ffdd7aebcd547621db08de877ef926d3a41fde72d065 - filename: tests/app/notifications/test_send_notifications.py checksum: 69304e1edd6993acd9a842a87dba8ee16854befc643863e1589b15c303a71464 +- filename: tests/app/v2/notifications/test_get_notifications.py + checksum: f2460d8b22ff7ff2e89778dbbf3e0740cbb41dead60c49c32648d367aa05fe0e - filename: tests/app/v2/notifications/test_post_notifications.py checksum: 3181930a13e3679bb2f17eaa3f383512eb9caf4ed5d5e14496ca4193c6083965 - filename: tests/app/v3/notifications/test_rest.py @@ -85,6 +89,4 @@ fileignoreconfig: checksum: 4f0f4d7a4113762219e45a51f7b26a7c0cb83f1d8f10c5598533f6cdcf0e0ada - filename: tests/lambda_functions/vetext_incoming_forwarder_lambda/test_vetext_incoming_forwarder_lambda.py checksum: 7494eb4321fd2fbc3ff3915d8753d8fec7a936a69dc6ab78f0b532a701f032eb -- filename: tests/app/clients/test_twilio.py - checksum: cad49e634cc5ba56157358aa3dfba2dafe7b9dbd3a0c580ec5cda3072f6a76e5 version: "1.0" diff --git a/app/models.py b/app/models.py index 0ee53de0ec..658845759f 100644 --- a/app/models.py +++ b/app/models.py @@ -2,6 +2,7 @@ from typing import Any, Dict, Optional import datetime +import html import itertools import uuid @@ -1331,7 +1332,7 @@ def _substitute_status_seq(_statuses): def content(self): from app.utils import get_template_instance - template_object = get_template_instance(self.template.__dict__, self.personalisation) + template_object = get_template_instance(self.template.__dict__, {k: '' for k in self.personalisation}) return str(template_object) @property @@ -1339,8 +1340,10 @@ def subject(self): from app.utils import get_template_instance if self.notification_type != SMS_TYPE: - template_object = get_template_instance(self.template.__dict__, self.personalisation) - return template_object.subject + template_object = get_template_instance( + self.template.__dict__, {k: '' for k in self.personalisation} + ) + return html.unescape(str(template_object.subject)) @property def formatted_status(self): diff --git a/tests/app/test_model.py b/tests/app/test_model.py index d259268c52..6bc05b3611 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -133,7 +133,7 @@ def test_notification_subject_fills_in_placeholders( ): template = sample_template(template_type=EMAIL_TYPE, subject='((name))') notification = sample_notification(template=template, personalisation={'name': 'hello'}) - assert notification.subject == 'hello' + assert notification.subject == '' def test_notification_serializes_created_by_name_with_no_created_by_id(client, sample_notification): diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 64ff0dd892..3a6d43839f 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -155,7 +155,7 @@ def test_get_notification_by_id_with_placeholders_and_recipient_identifiers_retu 'template': expected_template_response, 'created_at': notification.created_at.strftime(DATETIME_FORMAT), 'created_by_name': None, - 'body': 'Hello Bob\nThis is an email from va.gov', + 'body': 'Hello \nThis is an email from va.gov', 'subject': 'Subject', 'sent_at': notification.sent_at, 'sent_by': None, @@ -881,3 +881,56 @@ def test_get_notifications_renames_letter_statuses(client, sample_letter_templat json_response = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 assert json_response['status'] == expected_status + + +@pytest.mark.parametrize('template_type', [SMS_TYPE, EMAIL_TYPE]) +def test_get_notifications_removes_personalisation_from_content( + client, + sample_api_key, + sample_notification, + sample_template, + template_type, +): + template = sample_template( + content='Hello ((name))\nThis is an email from VA Notify about some ((thing))', + template_type=template_type, + ) + notification = sample_notification( + template=template, + personalisation={'name': 'Bob', 'thing': 'important stuff'}, + ) + auth_header = create_authorization_header(sample_api_key(service=template.service)) + response = client.get( + path=url_for('v2_notifications.get_notification_by_id', notification_id=notification.id), + headers=[('Content-Type', 'application/json'), auth_header], + ) + + json_response = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 + assert json_response['body'] == 'Hello \nThis is an email from VA Notify about some ' + + +def test_get_notifications_removes_personalisation_from_subject( + client, + sample_api_key, + sample_notification, + sample_template, +): + template = sample_template( + subject='Hello ((name))', + content='This is an email', + template_type=EMAIL_TYPE, + ) + notification = sample_notification( + template=template, + personalisation={'name': 'Bob'}, + ) + auth_header = create_authorization_header(sample_api_key(service=template.service)) + response = client.get( + path=url_for('v2_notifications.get_notification_by_id', notification_id=notification.id), + headers=[('Content-Type', 'application/json'), auth_header], + ) + + json_response = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 + assert json_response['subject'] == 'Hello '