Merge pull request #2699 from alphagov/update-process-sanitised-letter-task

Update process sanitised letter task

app/__init__.py

@@ -11,6 +11,7 @@ from time import monotonic
 from notifications_utils.clients.zendesk.zendesk_client import ZendeskClient
 from notifications_utils.clients.statsd.statsd_client import StatsdClient
 from notifications_utils.clients.redis.redis_client import RedisClient
+from notifications_utils.clients.encryption.encryption_client import Encryption
 from notifications_utils import logging, request_helper
 from werkzeug.exceptions import HTTPException as WerkzeugHTTPException
 from werkzeug.local import LocalProxy
@@ -22,7 +23,6 @@ from app.clients.email.aws_ses import AwsSesClient
 from app.clients.sms.firetext import FiretextClient
 from app.clients.sms.mmg import MMGClient
 from app.clients.performance_platform.performance_platform_client import PerformancePlatformClient
-from app.encryption import Encryption
 
 DATETIME_FORMAT_NO_TIMEZONE = "%Y-%m-%d %H:%M:%S.%f"
 DATETIME_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ"

app/celery/letters_pdf_tasks.py

@@ -15,7 +15,7 @@ from celery.exceptions import MaxRetriesExceededError
 from notifications_utils.statsd_decorators import statsd
 from notifications_utils.s3 import s3upload
 
-from app import notify_celery
+from app import encryption, notify_celery
 from app.aws import s3
 from app.config import QueueNames, TaskNames
 from app.dao.notifications_dao import (
@@ -302,14 +302,12 @@ def sanitise_letter(self, filename):
 
 
 @notify_celery.task(name='process-sanitised-letter')
-def process_sanitised_letter(
+def process_sanitised_letter(sanitise_data):
-    page_count,
+    letter_details = encryption.decrypt(sanitise_data)
-    message,
+
-    invalid_pages,
+    filename = letter_details['filename']
-    validation_status,
+    notification_id = letter_details['notification_id']
-    filename,
+
-    notification_id,
-):
     current_app.logger.info('Processing sanitised letter with id {}'.format(notification_id))
     notification = get_notification_by_id(notification_id, _raise=True)
 
@@ -323,7 +321,7 @@ def process_sanitised_letter(
     try:
         original_pdf_object = s3.get_s3_object(current_app.config['LETTERS_SCAN_BUCKET_NAME'], filename)
 
-        if validation_status == 'failed':
+        if letter_details['validation_status'] == 'failed':
             current_app.logger.info('Processing invalid precompiled pdf with id {} (file {})'.format(
                 notification_id, filename))
 
@@ -331,16 +329,16 @@ def process_sanitised_letter(
                 notification=notification,
                 filename=filename,
                 scan_pdf_object=original_pdf_object,
-                message=message,
+                message=letter_details['message'],
-                invalid_pages=invalid_pages,
+                invalid_pages=letter_details['invalid_pages'],
-                page_count=page_count,
+                page_count=letter_details['page_count'],
             )
             return
 
         current_app.logger.info('Processing valid precompiled pdf with id {} (file {})'.format(
             notification_id, filename))
 
-        billable_units = get_billable_units_for_letter_page_count(page_count)
+        billable_units = get_billable_units_for_letter_page_count(letter_details['page_count'])
         is_test_key = notification.key_type == KEY_TYPE_TEST
 
         move_sanitised_letter_to_test_or_live_pdf_bucket(filename, is_test_key, notification.created_at)
@@ -349,7 +347,8 @@ def process_sanitised_letter(
         update_letter_pdf_status(
             reference=notification.reference,
             status=NOTIFICATION_DELIVERED if is_test_key else NOTIFICATION_CREATED,
-            billable_units=billable_units
+            billable_units=billable_units,
+            recipient_address=letter_details['address']
         )
 
     except BotoClientError:

app/encryption.py

@@ -1,24 +0,0 @@
-from flask_bcrypt import generate_password_hash, check_password_hash
-
-from itsdangerous import URLSafeSerializer
-
-
-class Encryption:
-    def init_app(self, app):
-        self.serializer = URLSafeSerializer(app.config.get('SECRET_KEY'))
-        self.salt = app.config.get('DANGEROUS_SALT')
-
-    def encrypt(self, thing_to_encrypt):
-        return self.serializer.dumps(thing_to_encrypt, salt=self.salt)
-
-    def decrypt(self, thing_to_decrypt):
-        return self.serializer.loads(thing_to_decrypt, salt=self.salt)
-
-
-def hashpw(password):
-    return generate_password_hash(password.encode('UTF-8'), 10).decode('utf-8')
-
-
-def check_hash(password, hashed_password):
-    # If salt is invalid throws a 500 should add try/catch here
-    return check_password_hash(hashed_password, password)

app/hashing.py

@@ -0,0 +1,10 @@
+from flask_bcrypt import generate_password_hash, check_password_hash
+
+
+def hashpw(password):
+    return generate_password_hash(password.encode('UTF-8'), 10).decode('utf-8')
+
+
+def check_hash(password, hashed_password):
+    # If salt is invalid throws a 500 should add try/catch here
+    return check_password_hash(hashed_password, password)

app/models.py

@@ -29,7 +29,7 @@ from notifications_utils.template import (
 )
 from notifications_utils.timezones import convert_bst_to_utc, convert_utc_to_bst
 
-from app.encryption import (
+from app.hashing import (
     hashpw,
     check_hash
 )

migrations/versions/0025_notify_service_data.py

@@ -11,7 +11,7 @@ from datetime import datetime
 
 from alembic import op
 
-from app.encryption import hashpw
+from app.hashing import hashpw
 import uuid
 revision = '0025_notify_service_data'
 down_revision = '0024_add_research_mode_defaults'

requirements-app.txt

@@ -29,6 +29,6 @@ awscli-cwlogs>=1.4,<1.5
 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default
 itsdangerous==0.24  # pyup: <1.0.0
 
-git+https://github.com/alphagov/notifications-utils.git@36.3.3#egg=notifications-utils==36.3.3
+git+https://github.com/alphagov/notifications-utils.git@36.5.0#egg=notifications-utils==36.5.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

requirements.txt

@@ -31,21 +31,21 @@ awscli-cwlogs>=1.4,<1.5
 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default
 itsdangerous==0.24  # pyup: <1.0.0
 
-git+https://github.com/alphagov/notifications-utils.git@36.3.3#egg=notifications-utils==36.3.3
+git+https://github.com/alphagov/notifications-utils.git@36.5.0#egg=notifications-utils==36.5.0
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3
 
 ## The following requirements were added by pip freeze:
-alembic==1.3.2
+alembic==1.3.3
 amqp==1.4.9
 anyjson==0.3.3
 attrs==19.3.0
-awscli==1.16.310
+awscli==1.17.8
 bcrypt==3.1.7
 billiard==3.3.0.23
 bleach==3.1.0
 boto3==1.10.38
-botocore==1.13.46
+botocore==1.14.8
 certifi==2019.11.28
 chardet==3.0.4
 Click==7.0
@@ -56,21 +56,21 @@ flask-redis==0.4.0
 future==0.18.2
 greenlet==0.4.15
 idna==2.8
-importlib-metadata==1.3.0
+importlib-metadata==1.4.0
 Jinja2==2.10.3
 jmespath==0.9.4
 kombu==3.0.37
-Mako==1.1.0
+Mako==1.1.1
 MarkupSafe==1.1.1
 mistune==0.8.4
 monotonic==1.5
-more-itertools==8.0.2
+more-itertools==8.1.0
 orderedset==2.0.1
 phonenumbers==8.11.1
 pyasn1==0.4.8
 pycparser==2.19
 PyPDF2==1.26.0
-pyrsistent==0.15.6
+pyrsistent==0.15.7
 python-dateutil==2.8.1
 python-editor==1.0.4
 python-json-logger==0.1.11
@@ -79,11 +79,11 @@ PyYAML==5.2
 redis==3.3.11
 requests==2.22.0
 rsa==3.4.2
-s3transfer==0.2.1
+s3transfer==0.3.2
-six==1.13.0
+six==1.14.0
 smartypants==2.0.1
 statsd==3.3.0
-urllib3==1.25.7
+urllib3==1.25.8
 webencodings==0.5.1
 Werkzeug==0.16.0
-zipp==0.6.0
+zipp==2.0.1

tests/app/celery/test_letters_pdf_tasks.py

@@ -13,6 +13,7 @@ from celery.exceptions import MaxRetriesExceededError, Retry
 from requests import RequestException
 from sqlalchemy.orm.exc import NoResultFound
 
+from app import encryption
 from app.errors import VirusScanError
 from app.exceptions import NotificationTechnicalFailureException
 from app.celery.letters_pdf_tasks import (
@@ -773,17 +774,20 @@ def test_process_sanitised_letter_with_valid_letter(
     sample_letter_notification.billable_units = 1
     sample_letter_notification.created_at = datetime(2018, 7, 1, 12)
 
-    process_sanitised_letter(
+    encrypted_data = encryption.encrypt({
-        page_count=2,
+        'page_count': 2,
-        message=None,
+        'message': None,
-        invalid_pages=None,
+        'invalid_pages': None,
-        validation_status='passed',
+        'validation_status': 'passed',
-        filename=filename,
+        'filename': filename,
-        notification_id=str(sample_letter_notification.id)
+        'notification_id': str(sample_letter_notification.id),
-    )
+        'address': 'A. User\nThe house on the corner'
+    })
+    process_sanitised_letter(encrypted_data)
 
     assert sample_letter_notification.status == expected_status
     assert sample_letter_notification.billable_units == 1
+    assert sample_letter_notification.to == 'A. User\nThe house on the corner'
 
     assert not [x for x in scan_bucket.objects.all()]
     assert not [x for x in template_preview_bucket.objects.all()]
@@ -815,14 +819,16 @@ def test_process_sanitised_letter_with_invalid_letter(sample_letter_notification
     sample_letter_notification.billable_units = 1
     sample_letter_notification.created_at = datetime(2018, 7, 1, 12)
 
-    process_sanitised_letter(
+    encrypted_data = encryption.encrypt({
-        page_count=2,
+        'page_count': 2,
-        message='content-outside-printable-area',
+        'message': 'content-outside-printable-area',
-        invalid_pages=[1],
+        'invalid_pages': [1],
-        validation_status='failed',
+        'validation_status': 'failed',
-        filename=filename,
+        'filename': filename,
-        notification_id=str(sample_letter_notification.id)
+        'notification_id': str(sample_letter_notification.id),
-    )
+        'address': None,
+    })
+    process_sanitised_letter(encrypted_data)
 
     assert sample_letter_notification.status == NOTIFICATION_VALIDATION_FAILED
     assert sample_letter_notification.billable_units == 0
@@ -842,14 +848,16 @@ def test_process_sanitised_letter_when_letter_status_is_not_pending_virus_scan(
     mock_s3 = mocker.patch('app.celery.letters_pdf_tasks.s3')
     sample_letter_notification.status = NOTIFICATION_CREATED
 
-    process_sanitised_letter(
+    encrypted_data = encryption.encrypt({
-        page_count=2,
+        'page_count': 2,
-        message=None,
+        'message': None,
-        invalid_pages=None,
+        'invalid_pages': None,
-        validation_status='passed',
+        'validation_status': 'passed',
-        filename='NOTIFY.{}'.format(sample_letter_notification.reference),
+        'filename': 'NOTIFY.{}'.format(sample_letter_notification.reference),
-        notification_id=str(sample_letter_notification.id)
+        'notification_id': str(sample_letter_notification.id),
-    )
+        'address': None
+    })
+    process_sanitised_letter(encrypted_data)
 
     assert not mock_s3.called
 
@@ -861,15 +869,18 @@ def test_process_sanitised_letter_puts_letter_into_tech_failure_for_boto_errors(
     mocker.patch('app.celery.letters_pdf_tasks.s3.get_s3_object', side_effect=ClientError({}, 'operation_name'))
     sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK
 
+    encrypted_data = encryption.encrypt({
+        'page_count': 2,
+        'message': None,
+        'invalid_pages': None,
+        'validation_status': 'passed',
+        'filename': 'NOTIFY.{}'.format(sample_letter_notification.reference),
+        'notification_id': str(sample_letter_notification.id),
+        'address': None
+    })
+
     with pytest.raises(NotificationTechnicalFailureException):
-        process_sanitised_letter(
+        process_sanitised_letter(encrypted_data)
-            page_count=2,
-            message=None,
-            invalid_pages=None,
-            validation_status='passed',
-            filename='NOTIFY.{}'.format(sample_letter_notification.reference),
-            notification_id=str(sample_letter_notification.id)
-        )
 
     assert sample_letter_notification.status == NOTIFICATION_TECHNICAL_FAILURE
 

tests/app/test_encryption.py

@@ -1,20 +0,0 @@
-from app.encryption import Encryption
-
-encryption = Encryption()
-
-
-def test_should_encrypt_content(notify_api):
-    encryption.init_app(notify_api)
-    assert encryption.encrypt("this") != "this"
-
-
-def test_should_decrypt_content(notify_api):
-    encryption.init_app(notify_api)
-    encrypted = encryption.encrypt("this")
-    assert encryption.decrypt(encrypted) == "this"
-
-
-def test_should_encrypt_json(notify_api):
-    encryption.init_app(notify_api)
-    encrypted = encryption.encrypt({"this": "that"})
-    assert encryption.decrypt(encrypted) == {"this": "that"}