diff --git a/app/__init__.py b/app/__init__.py index 71ff052cb..aa106d4b3 100644 --- a/app/__init__.py +++ b/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" diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 3ee3822a0..6f31d2f91 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/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( - page_count, - message, - invalid_pages, - validation_status, - filename, - notification_id, -): +def process_sanitised_letter(sanitise_data): + letter_details = encryption.decrypt(sanitise_data) + + filename = letter_details['filename'] + notification_id = letter_details['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, - invalid_pages=invalid_pages, - page_count=page_count, + message=letter_details['message'], + invalid_pages=letter_details['invalid_pages'], + 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: diff --git a/app/encryption.py b/app/encryption.py deleted file mode 100644 index 34ba6df8e..000000000 --- a/app/encryption.py +++ /dev/null @@ -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) diff --git a/app/hashing.py b/app/hashing.py new file mode 100644 index 000000000..b8721964d --- /dev/null +++ b/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) diff --git a/app/models.py b/app/models.py index 7ab5c5e89..569cf0294 100644 --- a/app/models.py +++ b/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 ) diff --git a/migrations/versions/0025_notify_service_data.py b/migrations/versions/0025_notify_service_data.py index 763cc0301..54abecdd8 100644 --- a/migrations/versions/0025_notify_service_data.py +++ b/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' diff --git a/requirements-app.txt b/requirements-app.txt index d1368dce5..5bc6ec5e0 100644 --- a/requirements-app.txt +++ b/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 diff --git a/requirements.txt b/requirements.txt index f6d903e30..6a588ba67 100644 --- a/requirements.txt +++ b/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 -six==1.13.0 +s3transfer==0.3.2 +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 diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 4e6ade009..58568d52e 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/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( - page_count=2, - message=None, - invalid_pages=None, - validation_status='passed', - filename=filename, - notification_id=str(sample_letter_notification.id) - ) + encrypted_data = encryption.encrypt({ + 'page_count': 2, + 'message': None, + 'invalid_pages': None, + 'validation_status': 'passed', + 'filename': filename, + '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( - page_count=2, - message='content-outside-printable-area', - invalid_pages=[1], - validation_status='failed', - filename=filename, - notification_id=str(sample_letter_notification.id) - ) + encrypted_data = encryption.encrypt({ + 'page_count': 2, + 'message': 'content-outside-printable-area', + 'invalid_pages': [1], + 'validation_status': 'failed', + 'filename': filename, + '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( - 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) - ) + 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 + }) + 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( - 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) - ) + process_sanitised_letter(encrypted_data) assert sample_letter_notification.status == NOTIFICATION_TECHNICAL_FAILURE diff --git a/tests/app/test_encryption.py b/tests/app/test_encryption.py deleted file mode 100644 index a985e04ea..000000000 --- a/tests/app/test_encryption.py +++ /dev/null @@ -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"}