Merge branch 'main' into 2163-consolidate-status-and-reasons

.talismanrc

@@ -1,92 +1,8 @@
 fileignoreconfig:
-- filename: README.md
-  checksum: b2cbbb8508af49abccae8b35b317f7ce09215f3508430ed31440add78f450e5a
-- filename: app/callback/webhook_callback_strategy.py
-  checksum: 47846ab651c27512d3ac7864c08cb25d647f63bb84321953f907551fd9d2e85f
-- filename: app/celery/contact_information_tasks.py
-  checksum: 80d0acf88bafb1358583016b9e143f4523ef1160d6eacdc9754ca68859b90eae
-- filename: app/celery/nightly_tasks.py
-  checksum: b5b4999f778361603fdc7d710772968461e79a016f66badfbafd590bf70c5a11
-- filename: app/celery/service_callback_tasks.py
-  checksum: 83b61b21668c1b1a0ea33a4c3130e82c9b14edbd6542079f1dd3d7493f3e9a79
-- filename: app/commands.py
-  checksum: 79d914948e4e28dabc9c074956a11a9ceec27e7ae57b8eb8fb483f4a6217f07d
-- filename: app/constants.py
-  checksum: 44390d0a1258b184cf84dc9b6e97bd0768af84a9aa346ba963aa7735fc8bcb36
 - filename: app/dao/api_key_dao.py
-  checksum: ab93313f306c8a3f6576141e8f32d9fc99b0de7da8d44a1ddbe6ea55d167dcdb
-- filename: app/letters/utils.py
-  checksum: 5e6071b9cab380f9f3ee172f8c731061241200f53453a9863f22bb5eaa05e6af
-- filename: app/notifications/process_notifications.py
-  checksum: ae4e31c6eb56d91ec80ae09d13baf4558cf461c65f08893b93fee43f036a17a7
-- filename: app/notifications/send_notifications.py
-  checksum: 12ed8e1334772c0d65cc13be03b6d5197ebbf4d35460aee2fac1fc2a9644e9e1
-- filename: app/service/rest.py
-  checksum: 7f8a30dd84b3ceb0d08bae949b5b127fd408ee2fd8097eb7d4b291ede61f8d0f
-- filename: app/service/utils.py
-  checksum: c1e34f85b5b0fc32eb6a3afcc16c7099b74294e578487004dc6ba3bc46832010
-- filename: app/template/rest.py
-  checksum: 1e5bdac8bc694d50f8f656dec127dd036b7b1b5b6156e3282d3411956c71ba0b
-- filename: app/user/rest.py
-  checksum: e867ef2c802c8a3fd187fd0df76533e62cccb1721582c4fc4d2a920eb11c1e3b
-- filename: app/v3/notifications/rest.py
-  checksum: aea8f32b796f324e4b57e1be6703b31c0550447011550d944cdb461489203f7d
-- filename: app/va/va_profile/va_profile_client.py
-  checksum: fe634f26f7dc3874f4afcfd1ba3f03bae380b53befe973a752c7347097a88701
-- filename: cd/application-deployment/dev/dev.env
-  checksum: d0897f1124e6725347d18a9ba8de56baf848151ecad688db189e0ec2d6282bcb
-- filename: cd/application-deployment/dev/vaec-api-task-definition.json
-  checksum: f328ff821339b802eb1d82559e624d5b719857c813d427da5aaa39b240331ddd
-- filename: cd/application-deployment/perf/perf.env
-  checksum: d8f93f6bf746751e364d4cb81f26e656bd7ed0f146d98c03fd704d716acc5805
-- filename: cd/application-deployment/prod/prod.env
-  checksum: 949dc3234b36d9fc8706a927b5c342372aa1ac5d6ba8f9458f7482e0ee31a180
-- filename: cd/application-deployment/staging/staging.env
-  checksum: 15e41a76cd47e45be327188513e82763aa1a8d4d64ead457a8866f6f854e61d7
-- filename: ci/.local.env
-  checksum: 2c4024d4e94ffa39ff73e6039d65f0b1d19557067ddf6d97ef14aea86cfb58ad
-- filename: ci/docker-compose-test.yml
-  checksum: e3efec2749e8c19e60f5bfc68eafabe24eba647530a482ceccfc4e0e62cff424
-- filename: documents/openapi/openapi.yaml
-  checksum: f67b71f8506be867a206e8f4605c1bbc7a3b13c9d18ce6aa5317b4cbed4a50aa
-- filename: lambda_functions/pinpoint_callback/pinpoint_callback_lambda.py
-  checksum: 7bd4900e14b1fa789bbb2568b8a8d7a400e3c8350ba32fb44cc0b5b66a2df037
-- filename: lambda_functions/ses_callback/ses_callback_lambda.py
-  checksum: b20c36921290a9609f158784e2a3278c36190887e6054ea548004a67675fd79b
-- filename: lambda_functions/va_profile/va_profile_opt_in_out_lambda.py
-  checksum: a05165537ffbfac90000c5d04d8628251d771f6d1334c91c3aed28bf6c32368c
-- filename: poetry.lock
-  checksum: 0d77076ee13746c0b551ea8bd40cf15a6f92277102b6d2020bf87a39394af49c
-- filename: scripts/trigger_task.py
-  checksum: 0e9d244dbe285de23fc84bb643407963dacf7d25a3358373f01f6272fb217778
-- filename: tests/README.md
-  checksum: 4de2edc5ca36f266118231611036c7ccc6d9328bcceefafd8ca5ca92021f8aed
-- filename: tests/app/callback/test_webhook_callback_strategy.py
-  checksum: 288841d3209dc3ca885cd0bb08591221f7f15e5b3406fb7140505096db212554
-- filename: tests/app/celery/test_process_delivery_status_result_tasks.py
-  checksum: 62fa6216b62971d62c2e53f6b31aeeb659d7a1e404665362ee89cb3ec04793a6
-- filename: tests/app/celery/test_process_ga4_measurement_task.py
-  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
+  checksum: c44cbd8ae02fb1d551a1f0941365c11977564a6444950ee2b0282ee4b5fd1314
 - filename: tests/app/dao/test_api_key_dao.py
-  checksum: ef306fcc1dc512b74abeb5dde5f20977cf95e67a2fa049df6289a7b5500339a9
-- filename: tests/app/delivery/test_send_to_providers.py
-  checksum: f36fe9a01507e7d83739619501abf93d7071830bfbe6037f955a59c034d7c45e
-- filename: tests/app/notifications/test_process_notifications_for_profile_v3.py
-  checksum: 4e15e63d349635131173ffdd7aebcd547621db08de877ef926d3a41fde72d065
-- filename: tests/app/notifications/test_send_notifications.py
-  checksum: 69304e1edd6993acd9a842a87dba8ee16854befc643863e1589b15c303a71464
-- filename: tests/app/v2/notifications/test_post_notifications.py
-  checksum: 3181930a13e3679bb2f17eaa3f383512eb9caf4ed5d5e14496ca4193c6083965
-- filename: tests/app/v3/notifications/test_rest.py
-  checksum: 25aeb31ce5974f0a414ff43a9a75f787a6cf41c783f7eacf2415ce1fedf54c1d
-- filename: tests/lambda_functions/va_profile/test_va_profile_integration.py
-  checksum: 4f0f4d7a4113762219e45a51f7b26a7c0cb83f1d8f10c5598533f6cdcf0e0ada
-- filename: tests/lambda_functions/vetext_incoming_forwarder_lambda/test_vetext_incoming_forwarder_lambda.py
-  checksum: 7494eb4321fd2fbc3ff3915d8753d8fec7a936a69dc6ab78f0b532a701f032eb
+  checksum: 5bab4eaddf8760c502111ae3e5f9f8bee59482d99f053f94598e8c77bd10b7b6
+- filename: tests/app/v2/notifications/test_get_notifications.py
+  checksum: 167aa80c8aac01566b3cf627cdaa839feac9dc77f0b13b6d0a008035aa8e15f1
 version: "1.0"

app/dao/api_key_dao.py

@@ -1,12 +1,12 @@
+import secrets
 import uuid
 from datetime import datetime, timedelta
 
-from app import db
-from app.models import ApiKey
+from sqlalchemy import func, or_, select
 
+from app import db
 from app.dao.dao_utils import transactional, version_class
-
-from sqlalchemy import or_, func, select
+from app.models import ApiKey
 
 
 @transactional
@@ -15,7 +15,7 @@ def save_model_api_key(api_key):
     if not api_key.id:
         api_key.id = uuid.uuid4()  # must be set now so version history model can use same id
     if not api_key.secret:
-        api_key.secret = uuid.uuid4()
+        api_key.secret = secrets.token_urlsafe(64)
     db.session.add(api_key)
 
 

app/models.py

@@ -2,6 +2,7 @@
 from typing import Any, Dict, Optional
 
 import datetime
+import html
 import itertools
 import uuid
 
@@ -1331,16 +1332,18 @@ 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: '<redacted>' for k in self.personalisation})
         return str(template_object)
 
     @property
     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: '<redacted>' for k in self.personalisation}
+            )
+            return html.unescape(str(template_object.subject))
 
     @property
     def formatted_status(self):

app/v2/notifications/post_notifications.py

@@ -1,4 +1,5 @@
 import functools
+import html
 
 import werkzeug
 from flask import request, jsonify, current_app, abort
@@ -128,13 +129,13 @@ def post_notification(notification_type):  # noqa: C901
                 current_app.logger.debug('Sending a notification without contact information is not implemented.')
                 return jsonify(result='error', message='Not Implemented'), 501
 
-        template_with_content.values = notification.personalisation
+        template_with_content.values = {k: '<redacted>' for k in notification.personalisation}
 
     if notification_type == SMS_TYPE:
         create_resp_partial = functools.partial(create_post_sms_response_from_notification, from_number=reply_to)
     elif notification_type == EMAIL_TYPE:
         create_resp_partial = functools.partial(
-            create_post_email_response_from_notification, subject=template_with_content.subject
+            create_post_email_response_from_notification, subject=html.unescape(template_with_content.subject)
         )
     elif notification_type == LETTER_TYPE:
         create_resp_partial = functools.partial(

tests/app/dao/test_api_key_dao.py

@@ -187,3 +187,14 @@ def test_should_not_return_revoked_api_keys_older_than_7_days(
     all_api_keys = get_model_api_keys(service_id=service.id)
 
     assert len(all_api_keys) == expected_length
+
+
+def test_save_api_key_should_generate_secret_with_expected_format(sample_service):
+    service = sample_service()
+    api_key = ApiKey(
+        **{'service': service, 'name': service.name, 'created_by': service.created_by, 'key_type': KEY_TYPE_NORMAL}
+    )
+    save_model_api_key(api_key)
+
+    assert api_key.secret is not None
+    assert len(api_key.secret) >= 86

tests/app/test_model.py

@@ -126,7 +126,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 == '<redacted>'
 
 
 def test_notification_serializes_created_by_name_with_no_created_by_id(client, sample_notification):

tests/app/v2/notifications/test_get_notifications.py

@@ -161,7 +161,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 <redacted>\nThis is an email from va.gov',
         'subject': 'Subject',
         'sent_at': notification.sent_at,
         'sent_by': None,
@@ -883,3 +883,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 <redacted>\nThis is an email from VA Notify about some <redacted>'
+
+
+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 <redacted>'

tests/app/v2/notifications/test_post_notifications.py

@@ -98,7 +98,7 @@ def test_post_sms_notification_returns_201(
     # endpoint checks
     assert resp_json['id'] == str(notifications[0].id)
     assert resp_json['reference'] == reference
-    assert resp_json['content']['body'] == template.content.replace('(( Name))', 'Jo')
+    assert resp_json['content']['body'] == template.content.replace('(( Name))', '<redacted>')
     assert resp_json['content']['from_number'] == current_app.config['FROM_NUMBER']
     assert f'v2/notifications/{notifications[0].id}' in resp_json['uri']
     assert resp_json['template']['id'] == str(template.id)
@@ -423,7 +423,7 @@ def test_post_sms_notification_without_callback_url(
     assert notification.callback_url is None
 
     assert resp_json['id'] == str(notification.id)
-    assert resp_json['content']['body'] == template.content.replace('(( Name))', 'Jo')
+    assert resp_json['content']['body'] == template.content.replace('(( Name))', '<redacted>')
     assert resp_json['callback_url'] is None
 
 
@@ -504,8 +504,8 @@ def test_post_email_notification_returns_201(
     assert resp_json['reference'] == reference
     assert notification.reference is None
     assert notification.reply_to_text is None
-    assert resp_json['content']['body'] == template.content.replace('((name))', 'Bob')
-    assert resp_json['content']['subject'] == template.subject.replace('((name))', 'Bob')
+    assert resp_json['content']['body'] == template.content.replace('((name))', '<redacted>')
+    assert resp_json['content']['subject'] == template.subject.replace('((name))', '<redacted>')
     assert 'v2/notifications/{}'.format(notification.id) in resp_json['uri']
     assert resp_json['template']['id'] == str(template.id)
     assert resp_json['template']['version'] == template.version
@@ -560,8 +560,8 @@ def test_post_email_notification_with_reply_to_returns_201(
     assert resp_json['id'] == str(notification.id)
     assert resp_json['billing_code'] == 'TESTCODE'
     assert resp_json['reference'] == reference
-    assert resp_json['content']['body'] == template.content.replace('((name))', 'Bob')
-    assert resp_json['content']['subject'] == template.subject.replace('((name))', 'Bob')
+    assert resp_json['content']['body'] == template.content.replace('((name))', '<redacted>')
+    assert resp_json['content']['subject'] == template.subject.replace('((name))', '<redacted>')
     assert 'v2/notifications/{}'.format(notification.id) in resp_json['uri']
     assert resp_json['template']['id'] == str(template.id)
     assert resp_json['template']['version'] == template.version
@@ -1337,7 +1337,7 @@ def test_simulated(
         resp_json = response.get_json()
         assert validate(resp_json, post_email_response) == resp_json
 
-        assert resp_json['content']['body'] == 'Document: simulated-attachment-url'
+        assert resp_json['content']['body'] == 'Document: <redacted>'
 
     def test_without_document_upload_permission(
         self,