Merge pull request #2938 from alphagov/more-letter-validation

Check for invalid characters in letter addresses

app/v2/notifications/post_notifications.py

@@ -409,6 +409,10 @@ def validate_address(service, letter_data):
         raise ValidationError(
             message='Must be a real UK postcode'
         )
+    if address.has_invalid_characters:
+        raise ValidationError(
+            message='Address lines must not start with any of the following characters: @ ( ) = [ ] ” \\ / ,'
+        )
 
 
 def process_precompiled_letter_notifications(*, letter_data, api_key, service, template, reply_to_text):

requirements-app.txt

@@ -29,7 +29,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@40.2.1#egg=notifications-utils==40.2.1
+git+https://github.com/alphagov/notifications-utils.git@40.6.0#egg=notifications-utils==40.6.0
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1

requirements.txt

@@ -31,7 +31,7 @@ notifications-python-client==5.5.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@40.2.1#egg=notifications-utils==40.2.1
+git+https://github.com/alphagov/notifications-utils.git@40.6.0#egg=notifications-utils==40.6.0
 
 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains
 prometheus-client==0.7.1
@@ -42,29 +42,30 @@ alembic==1.4.2
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.18.97
+awscli==1.18.106
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.4
 blinker==1.4
 boto==2.49.0
 boto3==1.10.38
-botocore==1.17.20
+botocore==1.17.29
 certifi==2020.6.20
 chardet==3.0.4
 click==7.1.2
 colorama==0.4.3
-dnspython==1.16.0
+dnspython==2.0.0
 docutils==0.15.2
 flask-redis==0.4.0
 future==0.18.2
+geojson==2.5.0
 govuk-bank-holidays==0.6
 greenlet==0.4.16
 idna==2.10
-importlib-metadata==1.7.0
 Jinja2==2.11.2
 jmespath==0.10.0
 kombu==3.0.37
+lxml==4.5.1
 Mako==1.1.3
 MarkupSafe==1.1.1
 mistune==0.8.4
@@ -87,7 +88,6 @@ s3transfer==0.3.3
 six==1.15.0
 smartypants==2.0.1
 statsd==3.3.0
-urllib3==1.25.9
+urllib3==1.25.10
 webencodings==0.5.1
 Werkzeug==1.0.1
-zipp==3.1.0

tests/app/v2/notifications/test_post_letter_notifications.py

@@ -176,31 +176,49 @@ def test_post_letter_notification_stores_country(
     )
 
 
-@pytest.mark.parametrize('permissions, expected_error', (
+@pytest.mark.parametrize('permissions, personalisation, expected_error', (
     (
         [LETTER_TYPE],
-        'Must be a real UK postcode',
+        {
-    ),
-    (
-        [LETTER_TYPE, INTERNATIONAL_LETTERS],
-        'Last line of address must be a real UK postcode or another country',
-    ),
-))
-def test_post_letter_notification_throws_error_for_bad_postcode(
-    client, notify_db_session, mocker, permissions, expected_error
-):
-    service = create_service(service_permissions=[LETTER_TYPE])
-    template = create_template(service, template_type="letter", postage="first")
-    mocker.patch('app.celery.tasks.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async')
-    data = {
-        'template_id': str(template.id),
-        'personalisation': {
             'address_line_1': 'Her Royal Highness Queen Elizabeth II',
             'address_line_2': 'Buckingham Palace',
             'address_line_3': 'London',
             'postcode': 'not a real postcode',
             'name': 'Lizzie'
-        }
+        },
+        'Must be a real UK postcode',
+    ),
+    (
+        [LETTER_TYPE],
+        {
+            'address_line_1': 'Her Royal Highness Queen Elizabeth II',
+            'address_line_2': ']Buckingham Palace',
+            'postcode': 'SW1A 1AA',
+            'name': 'Lizzie'
+        },
+        'Address lines must not start with any of the following characters: @ ( ) = [ ] ” \\ / ,',
+    ),
+    (
+        [LETTER_TYPE, INTERNATIONAL_LETTERS],
+        {
+            'address_line_1': 'Her Royal Highness Queen Elizabeth II',
+            'address_line_2': 'Buckingham Palace',
+            'address_line_3': 'London',
+            'postcode': 'not a real postcode',
+            'name': 'Lizzie'
+        },
+        'Last line of address must be a real UK postcode or another country',
+    ),
+))
+def test_post_letter_notification_throws_error_for_bad_address(
+    client, notify_db_session, mocker, permissions, personalisation, expected_error
+):
+    service = create_service(service_permissions=permissions)
+    template = create_template(service, template_type="letter", postage="first")
+    mocker.patch('app.celery.tasks.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async')
+    data = {
+        'template_id': str(template.id),
+        'personalisation': personalisation
     }
 
     error_json = letter_request(client, data, service_id=service.id, _expected_status=400)
@@ -208,7 +226,7 @@ def test_post_letter_notification_throws_error_for_bad_postcode(
     assert error_json['status_code'] == 400
     assert error_json['errors'] == [{
         'error': 'ValidationError',
-        'message': 'Must be a real UK postcode'
+        'message': expected_error
     }]