diff --git a/app/config.py b/app/config.py index ce022374f..aa4f0eaeb 100644 --- a/app/config.py +++ b/app/config.py @@ -72,6 +72,7 @@ class Config(object): NOTIFY_ENVIRONMENT = 'development' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-local' MOU_BUCKET_NAME = 'local-mou' + TRANSIENT_UPLOADED_LETTERS = 'local-transient-uploaded-letters' ROUTE_SECRET_KEY_1 = os.environ.get('ROUTE_SECRET_KEY_1', '') ROUTE_SECRET_KEY_2 = os.environ.get('ROUTE_SECRET_KEY_2', '') CHECK_PROXY_HEADER = False @@ -94,6 +95,7 @@ class Development(Config): CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-tools' MOU_BUCKET_NAME = 'notify.tools-mou' + TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters' ADMIN_CLIENT_SECRET = 'dev-notify-secret-key' API_HOST_NAME = 'http://localhost:6011' @@ -115,6 +117,7 @@ class Test(Development): CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-test' MOU_BUCKET_NAME = 'test-mou' + TRANSIENT_UPLOADED_LETTERS = 'test-transient-uploaded-letters' NOTIFY_ENVIRONMENT = 'test' API_HOST_NAME = 'http://you-forgot-to-mock-an-api-call-to' TEMPLATE_PREVIEW_API_HOST = 'http://localhost:9999' @@ -132,6 +135,7 @@ class Preview(Config): CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-preview' MOU_BUCKET_NAME = 'notify.works-mou' + TRANSIENT_UPLOADED_LETTERS = 'preview-transient-uploaded-letters' NOTIFY_ENVIRONMENT = 'preview' CHECK_PROXY_HEADER = False ASSET_DOMAIN = 'static.notify.works' @@ -146,6 +150,7 @@ class Staging(Config): CSV_UPLOAD_BUCKET_NAME = 'staging-notifications-csv-upload' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-staging' MOU_BUCKET_NAME = 'staging-notify.works-mou' + TRANSIENT_UPLOADED_LETTERS = 'staging-transient-uploaded-letters' NOTIFY_ENVIRONMENT = 'staging' CHECK_PROXY_HEADER = False ASSET_DOMAIN = 'static.staging-notify.works' @@ -160,6 +165,7 @@ class Live(Config): CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-production' MOU_BUCKET_NAME = 'notifications.service.gov.uk-mou' + TRANSIENT_UPLOADED_LETTERS = 'production-transient-uploaded-letters' NOTIFY_ENVIRONMENT = 'live' CHECK_PROXY_HEADER = False ASSET_DOMAIN = 'static.notifications.service.gov.uk' diff --git a/app/main/forms.py b/app/main/forms.py index 18ea30058..27146b068 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1109,9 +1109,9 @@ class ServiceLetterBrandingDetails(StripWhitespaceForm): class PDFUploadForm(StripWhitespaceForm): file = FileField_wtf( - 'Upload a letter in PDF format to check if it fits in the printable area', + 'Upload a letter in PDF format', validators=[ - FileAllowed(['pdf'], 'PDF documents only!'), + FileAllowed(['pdf'], 'Letters must be saved as a PDF'), DataRequired(message="You need to upload a file to submit") ] ) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 4fe8efc36..82b0373fc 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -1,10 +1,161 @@ -from flask import render_template +import base64 +import uuid +from io import BytesIO +from flask import ( + abort, + current_app, + flash, + redirect, + render_template, + request, + url_for, +) +from notifications_utils.pdf import pdf_page_count +from PyPDF2.utils import PdfReadError +from requests import RequestException + +from app import current_service, notification_api_client, service_api_client +from app.extensions import antivirus_client from app.main import main -from app.utils import user_has_permissions +from app.main.forms import PDFUploadForm +from app.s3_client.s3_letter_upload_client import ( + get_letter_pdf_and_metadata, + get_transient_letter_file_location, + upload_letter_to_s3, +) +from app.template_previews import TemplatePreview, sanitise_letter +from app.utils import get_template, user_has_permissions + +MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB @main.route("/services//uploads") @user_has_permissions('send_messages') def uploads(service_id): return render_template('views/uploads/index.html') + + +@main.route("/services//upload-letter", methods=['GET', 'POST']) +@user_has_permissions('send_messages') +def upload_letter(service_id): + form = PDFUploadForm() + + if form.validate_on_submit(): + pdf_file_bytes = form.file.data.read() + + virus_free = antivirus_client.scan(BytesIO(pdf_file_bytes)) + if not virus_free: + return invalid_upload_error('Your file has failed the virus check') + + if len(pdf_file_bytes) > MAX_FILE_UPLOAD_SIZE: + return invalid_upload_error('Your file must be smaller than 2MB') + + try: + # TODO: get page count from the sanitise response once template preview handles malformed files nicely + page_count = pdf_page_count(BytesIO(pdf_file_bytes)) + except PdfReadError: + current_app.logger.info('Invalid PDF uploaded for service_id: {}'.format(service_id)) + return invalid_upload_error('Your file must be a valid PDF') + + upload_id = uuid.uuid4() + file_location = get_transient_letter_file_location(service_id, upload_id) + + try: + response = sanitise_letter(BytesIO(pdf_file_bytes)) + response.raise_for_status() + except RequestException as ex: + if ex.response is not None and ex.response.status_code == 400: + status = 'invalid' + upload_letter_to_s3(pdf_file_bytes, file_location, status) + else: + raise ex + else: + status = 'valid' + file_contents = base64.b64decode(response.json()['file'].encode()) + upload_letter_to_s3(file_contents, file_location, status) + + return redirect( + url_for( + 'main.uploaded_letter_preview', + service_id=current_service.id, + file_id=upload_id, + original_filename=form.file.data.filename, + page_count=page_count, + status=status, + ) + ) + + return render_template('views/uploads/choose-file.html', form=form) + + +def invalid_upload_error(message): + flash(message, 'dangerous') + return render_template('views/uploads/choose-file.html', form=PDFUploadForm()), 400 + + +@main.route("/services//preview-letter/") +@user_has_permissions('send_messages') +def uploaded_letter_preview(service_id, file_id): + original_filename = request.args.get('original_filename') + page_count = request.args.get('page_count') + status = request.args.get('status') + + template_dict = service_api_client.get_precompiled_template(service_id) + + template = get_template( + template_dict, + service_id, + letter_preview_url=url_for( + '.view_letter_upload_as_preview', + service_id=service_id, + file_id=file_id + ), + page_count=page_count + ) + + return render_template( + 'views/uploads/preview.html', + original_filename=original_filename, + template=template, + status=status, + file_id=file_id, + ) + + +@main.route("/services//preview-letter-image/") +@user_has_permissions('send_messages') +def view_letter_upload_as_preview(service_id, file_id): + file_location = get_transient_letter_file_location(service_id, file_id) + pdf_file, metadata = get_letter_pdf_and_metadata(file_location) + + page = request.args.get('page') + + if metadata['status'] == 'invalid': + return TemplatePreview.from_invalid_pdf_file(pdf_file, page) + else: + return TemplatePreview.from_valid_pdf_file(pdf_file, page) + + +@main.route("/services//upload-letter/send", methods=['POST']) +@user_has_permissions('send_messages', restrict_admin_usage=True) +def send_uploaded_letter(service_id): + filename = request.form['filename'] + file_id = request.form['file_id'] + + if not (current_service.has_permission('letter') and current_service.has_permission('upload_letters')): + abort(403) + + file_location = get_transient_letter_file_location(service_id, file_id) + _, metadata = get_letter_pdf_and_metadata(file_location) + + if metadata.get('status') != 'valid': + abort(403) + + notification_api_client.send_precompiled_letter(service_id, filename, file_id) + + return redirect(url_for( + '.view_notification', + service_id=service_id, + notification_id=file_id, + )) diff --git a/app/navigation.py b/app/navigation.py index fe05bcd40..5cc6b833d 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -244,6 +244,7 @@ class HeaderNavigation(Navigation): 'send_test', 'send_test_preview', 'send_test_step', + 'send_uploaded_letter', 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', @@ -304,6 +305,8 @@ class HeaderNavigation(Navigation): 'template_history', 'template_usage', 'trial_mode', + 'upload_letter', + 'uploaded_letter_preview', 'uploads', 'usage', 'view_job', @@ -312,6 +315,7 @@ class HeaderNavigation(Navigation): 'view_jobs', 'view_letter_notification_as_preview', 'view_letter_template_preview', + 'view_letter_upload_as_preview', 'view_notification', 'view_notification_updates', 'view_notifications', @@ -365,6 +369,8 @@ class MainNavigation(Navigation): 'view_template_versions', }, 'uploads': { + 'upload_letter', + 'uploaded_letter_preview', 'uploads', }, 'team-members': { @@ -551,6 +557,7 @@ class MainNavigation(Navigation): 'robots', 'security', 'send_notification', + 'send_uploaded_letter', 'service_dashboard_updates', 'service_delete_email_reply_to', 'service_delete_letter_contact', @@ -603,6 +610,7 @@ class MainNavigation(Navigation): 'view_job_updates', 'view_letter_notification_as_preview', 'view_letter_template_preview', + 'view_letter_upload_as_preview', 'view_notification_updates', 'view_notifications_csv', 'view_provider', @@ -786,6 +794,7 @@ class CaseworkNavigation(Navigation): 'send_messages', 'send_notification', 'send_test_preview', + 'send_uploaded_letter', 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', @@ -859,6 +868,8 @@ class CaseworkNavigation(Navigation): 'two_factor_email_sent', 'update_email_branding', 'update_letter_branding', + 'upload_letter', + 'uploaded_letter_preview', 'uploads', 'usage', 'usage_for_all_services', @@ -881,6 +892,7 @@ class CaseworkNavigation(Navigation): 'view_job_updates', 'view_letter_notification_as_preview', 'view_letter_template_preview', + 'view_letter_upload_as_preview', 'view_notification_updates', 'view_notifications_csv', 'view_provider', @@ -1065,6 +1077,7 @@ class OrgNavigation(Navigation): 'send_test', 'send_test_preview', 'send_test_step', + 'send_uploaded_letter', 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', @@ -1138,6 +1151,8 @@ class OrgNavigation(Navigation): 'two_factor_email_sent', 'update_email_branding', 'update_letter_branding', + 'upload_letter', + 'uploaded_letter_preview', 'uploads', 'usage', 'usage_for_all_services', @@ -1162,6 +1177,7 @@ class OrgNavigation(Navigation): 'view_jobs', 'view_letter_notification_as_preview', 'view_letter_template_preview', + 'view_letter_upload_as_preview', 'view_notification', 'view_notification_updates', 'view_notifications', diff --git a/app/notify_client/notification_api_client.py b/app/notify_client/notification_api_client.py index 66b16448a..d474214bc 100644 --- a/app/notify_client/notification_api_client.py +++ b/app/notify_client/notification_api_client.py @@ -59,6 +59,14 @@ class NotificationApiClient(NotifyAdminAPIClient): data = _attach_current_user(data) return self.post(url='/service/{}/send-notification'.format(service_id), data=data) + def send_precompiled_letter(self, service_id, filename, file_id): + data = { + 'filename': filename, + 'file_id': file_id, + } + data = _attach_current_user(data) + return self.post(url='/service/{}/send-pdf-letter'.format(service_id), data=data) + def get_notification(self, service_id, notification_id): return self.get(url='/service/{}/notifications/{}'.format(service_id, notification_id)) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 04448e67e..f247597be 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -261,6 +261,12 @@ class ServiceAPIClient(NotifyAdminAPIClient): ) return self.get(endpoint) + def get_precompiled_template(self, service_id): + """ + Returns the precompiled template for a service, creating it if it doesn't already exist + """ + return self.get('/service/{}/template/precompiled'.format(service_id)) + @cache.set('service-{service_id}-templates') def get_service_templates(self, service_id): """ diff --git a/app/s3_client/s3_letter_upload_client.py b/app/s3_client/s3_letter_upload_client.py new file mode 100644 index 000000000..27848f105 --- /dev/null +++ b/app/s3_client/s3_letter_upload_client.py @@ -0,0 +1,27 @@ +from boto3 import resource +from flask import current_app +from notifications_utils.s3 import s3upload as utils_s3upload + + +def get_transient_letter_file_location(service_id, upload_id): + return 'service-{}/{}.pdf'.format(service_id, upload_id) + + +def upload_letter_to_s3(data, file_location, status): + utils_s3upload( + filedata=data, + region=current_app.config['AWS_REGION'], + bucket_name=current_app.config['TRANSIENT_UPLOADED_LETTERS'], + file_location=file_location, + metadata={'status': status} + ) + + +def get_letter_pdf_and_metadata(file_location): + s3 = resource('s3') + s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get() + + pdf = s3_object['Body'].read() + metadata = s3_object['Metadata'] + + return pdf, metadata diff --git a/app/template_previews.py b/app/template_previews.py index 2dbcba122..03bb15f77 100644 --- a/app/template_previews.py +++ b/app/template_previews.py @@ -1,5 +1,9 @@ +import base64 +from io import BytesIO + import requests from flask import current_app, json +from notifications_utils.pdf import extract_page_from_pdf from app import current_service @@ -24,6 +28,36 @@ class TemplatePreview: ) return (resp.content, resp.status_code, resp.headers.items()) + @classmethod + def from_valid_pdf_file(cls, pdf_file, page): + pdf_page = extract_page_from_pdf(BytesIO(pdf_file), int(page) - 1) + + response = requests.post( + '{}/precompiled-preview.png{}'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'], + '?hide_notify=true' if page == '1' else '' + ), + data=base64.b64encode(pdf_page).decode('utf-8'), + headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} + ) + + return (response.content, response.status_code, response.headers.items()) + + @classmethod + def from_invalid_pdf_file(cls, pdf_file, page): + pdf_page = extract_page_from_pdf(BytesIO(pdf_file), int(page) - 1) + + response = requests.post( + '{}/precompiled/overlay.png{}'.format( + current_app.config['TEMPLATE_PREVIEW_API_HOST'], + '?page_number={}'.format(page) + ), + data=pdf_page, + headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} + ) + + return (response.content, response.status_code, response.headers.items()) + @classmethod def from_example_template(cls, template, filename): data = { @@ -66,3 +100,11 @@ def validate_letter(pdf_file): data=pdf_file, headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} ) + + +def sanitise_letter(pdf_file): + return requests.post( + '{}/precompiled/sanitise'.format(current_app.config['TEMPLATE_PREVIEW_API_HOST']), + data=pdf_file, + headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} + ) diff --git a/app/templates/views/uploads/choose-file.html b/app/templates/views/uploads/choose-file.html new file mode 100644 index 000000000..435081ac9 --- /dev/null +++ b/app/templates/views/uploads/choose-file.html @@ -0,0 +1,28 @@ +{% extends "withnav_template.html" %} +{% from "components/file-upload.html" import file_upload %} +{% from "components/page-header.html" import page_header %} + +{% block service_page_title %} + Upload a letter +{% endblock %} + +{% block maincolumn_content %} +
+
+ {{ page_header( + 'Upload a letter', + back_link=url_for('main.uploads', service_id=current_service.id) + ) }} + +

+ {{ file_upload( + form.file, + action = url_for('main.upload_letter', service_id=current_service.id), + )}} +

+

You can upload a single letter as a PDF.

+

Your file must meet our letter specification.

+ +
+
+{% endblock %} diff --git a/app/templates/views/uploads/index.html b/app/templates/views/uploads/index.html index 9e5ff2bd7..0b2ad0480 100644 --- a/app/templates/views/uploads/index.html +++ b/app/templates/views/uploads/index.html @@ -7,10 +7,12 @@ {% block maincolumn_content %}
-
+
{{ page_header('Uploads') }}

Upload a letter and Notify will print, pack and post it for you.

+ + Upload a letter
{% endblock %} diff --git a/app/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html new file mode 100644 index 000000000..c8d954887 --- /dev/null +++ b/app/templates/views/uploads/preview.html @@ -0,0 +1,37 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} + +{% block service_page_title %} + {{ original_filename }} +{% endblock %} + +{% block maincolumn_content %} + {{ page_header( + original_filename, + back_link=url_for('main.upload_letter', service_id=current_service.id) + ) }} + + {% if status == 'invalid' %} +

+ Validation failed +

+ {% endif %} + +
+ {{ template|string }} +
+ + {% if status == 'valid' %} +
+ +
+ {% endif %} +{% endblock %} diff --git a/requirements-app.txt b/requirements-app.txt index 2cf82246a..3becbe591 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -23,4 +23,4 @@ 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@34.0.1#egg=notifications-utils==34.0.1 +git+https://github.com/alphagov/notifications-utils.git@34.1.0#egg=notifications-utils==34.1.0 diff --git a/requirements.txt b/requirements.txt index f7a6b6afb..734270dca 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,13 +25,13 @@ 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@34.0.1#egg=notifications-utils==34.0.1 +git+https://github.com/alphagov/notifications-utils.git@34.1.0#egg=notifications-utils==34.1.0 ## The following requirements were added by pip freeze: -awscli==1.16.231 +awscli==1.16.233 bleach==3.1.0 boto3==1.6.16 -botocore==1.12.221 +botocore==1.12.223 certifi==2019.6.16 chardet==3.0.4 Click==7.0 @@ -72,7 +72,7 @@ statsd==3.3.0 texttable==1.6.2 urllib3==1.25.3 webencodings==0.5.1 -Werkzeug==0.15.5 +Werkzeug==0.15.6 WTForms==2.2.1 xlrd==1.2.0 xlwt==1.3.0 diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index 4f72fe549..114e297ce 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -805,7 +805,7 @@ def test_letter_validation_preview_doesnt_call_template_preview_when_file_not_pd antivirus_scan.assert_not_called() validate_letter.assert_not_called() page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.find('span', class_='error-message').text.strip() == "PDF documents only!" + assert page.find('span', class_='error-message').text.strip() == "Letters must be saved as a PDF" def test_letter_validation_preview_doesnt_call_template_preview_when_file_doesnt_pass_virus_scan( diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index e33965eea..b9b532f84 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -1,5 +1,330 @@ +from unittest.mock import Mock + +import pytest +from flask import url_for +from requests import RequestException + +from app.utils import normalize_spaces from tests.conftest import SERVICE_ONE_ID def test_get_upload_hub_page(client_request): - client_request.get('main.uploads', service_id=SERVICE_ONE_ID) + page = client_request.get('main.uploads', service_id=SERVICE_ONE_ID) + + assert page.find('h1').text == 'Uploads' + assert page.find('a', text='Upload a letter').attrs['href'] == url_for( + 'main.upload_letter', service_id=SERVICE_ONE_ID + ) + + +def test_get_upload_letter(client_request): + page = client_request.get('main.upload_letter', service_id=SERVICE_ONE_ID) + + assert page.find('h1').text == 'Upload a letter' + assert page.find('input', class_='file-upload-field') + assert page.select('button[type=submit]') + + +def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): + mocker.patch('uuid.uuid4', return_value='fake-uuid') + antivirus_mock = mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + mocker.patch( + 'app.main.views.uploads.sanitise_letter', + return_value=Mock(content='The sanitised content', json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50'}) + ) + mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template') + + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _follow_redirects=True, + ) + assert antivirus_mock.called + + mock_s3.assert_called_once_with( + b'The sanitised content', + 'service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID), + 'valid', + ) + + assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' + assert not page.find(id='validation-error-message') + + assert page.find('input', {'type': 'hidden', 'name': 'filename', 'value': 'tests/test_pdf_files/one_page_pdf.pdf'}) + assert page.find('input', {'type': 'hidden', 'name': 'file_id', 'value': 'fake-uuid'}) + assert page.find('button', {'type': 'submit'}).text == 'Send 1 letter' + + +def test_post_upload_letter_shows_letter_preview_for_valid_file(mocker, client_request): + letter_template = {'template_type': 'letter', + 'reply_to_text': '', + 'postage': 'second', + 'subject': 'hi', + 'content': 'my letter'} + + mocker.patch('uuid.uuid4', return_value='fake-uuid') + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + mocker.patch( + 'app.main.views.uploads.sanitise_letter', + return_value=Mock(content='The sanitised content', json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50'}) + ) + mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mocker.patch('app.main.views.uploads.pdf_page_count', return_value=3) + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template) + + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _follow_redirects=True, + ) + + assert len(page.select('.letter-postage')) == 1 + assert normalize_spaces(page.select_one('.letter-postage').text) == ('Postage: second class') + assert page.select_one('.letter-postage')['class'] == ['letter-postage', 'letter-postage-second'] + + letter_images = page.select('main img') + assert len(letter_images) == 3 + + for page_no, img in enumerate(letter_images, start=1): + assert img['src'] == url_for( + '.view_letter_upload_as_preview', + service_id=SERVICE_ONE_ID, + file_id='fake-uuid', + page=page_no) + + +def test_post_upload_letter_shows_error_when_file_is_not_a_pdf(client_request): + with open('tests/non_spreadsheet_files/actually_a_png.csv', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _expected_status=200 + ) + assert page.find('span', class_='error-message').text.strip() == "Letters must be saved as a PDF" + + +def test_post_upload_letter_shows_error_when_no_file_uploaded(client_request): + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': ''}, + _expected_status=200 + ) + assert page.find('span', class_='error-message').text.strip() == "You need to upload a file to submit" + + +def test_post_upload_letter_shows_error_when_file_contains_virus(mocker, client_request): + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=False) + + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _expected_status=400 + ) + assert page.find('h1').text == 'Upload a letter' + assert normalize_spaces(page.select('.banner-dangerous')[0].text) == 'Your file has failed the virus check' + + +def test_post_choose_upload_file_when_file_is_too_big(mocker, client_request): + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + + with open('tests/test_pdf_files/big.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _expected_status=400 + ) + assert page.find('h1').text == 'Upload a letter' + assert normalize_spaces(page.select('.banner-dangerous')[0].text) == 'Your file must be smaller than 2MB' + + +def test_post_choose_upload_file_when_file_is_malformed(mocker, client_request): + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + + with open('tests/test_pdf_files/no_eof_marker.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _expected_status=400 + ) + assert page.find('h1').text == 'Upload a letter' + assert normalize_spaces(page.select('.banner-dangerous')[0].text) == 'Your file must be a valid PDF' + + +def test_post_upload_letter_with_invalid_file(mocker, client_request): + mocker.patch('uuid.uuid4', return_value='fake-uuid') + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3') + + mock_sanitise_response = Mock() + mock_sanitise_response.raise_for_status.side_effect = RequestException(response=Mock(status_code=400)) + mocker.patch('app.main.views.uploads.sanitise_letter', return_value=mock_sanitise_response) + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template') + + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + file_contents = file.read() + file.seek(0) + + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _follow_redirects=True + ) + + mock_s3.assert_called_once_with( + file_contents, + 'service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID), + 'invalid', + ) + + assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' + assert normalize_spaces( + page.find(id='validation-error-message').text + ) == 'Validation failed' + assert not page.find('button', {'type': 'submit'}) + + +def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client_request): + letter_template = {'template_type': 'letter', + 'reply_to_text': '', + 'postage': 'first', + 'subject': 'hi', + 'content': 'my letter'} + + mocker.patch('uuid.uuid4', return_value='fake-uuid') + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mock_sanitise_response = Mock() + mock_sanitise_response.raise_for_status.side_effect = RequestException(response=Mock(status_code=400)) + mocker.patch('app.main.views.uploads.sanitise_letter', return_value=mock_sanitise_response) + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template) + + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _follow_redirects=True, + ) + + assert len(page.select('.letter-postage')) == 1 + assert normalize_spaces(page.select_one('.letter-postage').text) == ('Postage: first class') + assert page.select_one('.letter-postage')['class'] == ['letter-postage', 'letter-postage-first'] + + letter_images = page.select('main img') + assert len(letter_images) == 1 + assert letter_images[0]['src'] == url_for( + '.view_letter_upload_as_preview', + service_id=SERVICE_ONE_ID, + file_id='fake-uuid', + page=1 + ) + + +def test_post_upload_letter_does_not_upload_to_s3_if_template_preview_raises_unknown_error(mocker, client_request): + mocker.patch('uuid.uuid4', return_value='fake-uuid') + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3') + + mocker.patch('app.main.views.uploads.sanitise_letter', side_effect=RequestException()) + + with pytest.raises(RequestException): + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _follow_redirects=True + ) + + assert not mock_s3.called + + +def test_uploaded_letter_preview(mocker, client_request): + mocker.patch('app.main.views.uploads.service_api_client') + + page = client_request.get( + 'main.uploaded_letter_preview', + service_id=SERVICE_ONE_ID, + file_id='fake-uuid', + original_filename='my_letter.pdf', + page_count=1, + status='valid', + ) + + assert page.find('h1').text == 'my_letter.pdf' + assert page.find('div', class_='letter-sent') + + +def test_send_uploaded_letter_sends_letter_and_redirects_to_notification_page(mocker, service_one, client_request): + mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', {'status': 'valid'})) + mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter') + + service_one['permissions'] = ['letter', 'upload_letters'] + file_id = 'abcd-1234' + + client_request.post( + 'main.send_uploaded_letter', + service_id=SERVICE_ONE_ID, + _data={'filename': 'my_file.pdf', 'file_id': file_id}, + _expected_redirect=url_for( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=file_id, + _external=True + ) + ) + mock_send.assert_called_once_with(SERVICE_ONE_ID, 'my_file.pdf', file_id) + + +@pytest.mark.parametrize('permissions', [ + ['email'], + ['letter'], + ['upload_letters'], +]) +def test_send_uploaded_letter_when_service_does_not_have_correct_permissions( + mocker, + service_one, + client_request, + permissions, +): + mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', {'status': 'valid'})) + mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter') + + service_one['permissions'] = permissions + file_id = 'abcd-1234' + + client_request.post( + 'main.send_uploaded_letter', + service_id=SERVICE_ONE_ID, + _data={'filename': 'my_file.pdf', 'file_id': file_id}, + _expected_status=403 + ) + assert not mock_send.called + + +def test_send_uploaded_letter_when_metadata_states_pdf_is_invalid(mocker, service_one, client_request): + mocker.patch('app.main.views.uploads.get_letter_pdf_and_metadata', return_value=('file', {'status': 'invalid'})) + mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter') + + service_one['permissions'] = ['letter', 'upload_letters'] + file_id = 'abcd-1234' + + client_request.post( + 'main.send_uploaded_letter', + service_id=SERVICE_ONE_ID, + _data={'filename': 'my_file.pdf', 'file_id': file_id}, + _expected_status=403 + ) + assert not mock_send.called diff --git a/tests/app/notify_client/test_notification_client.py b/tests/app/notify_client/test_notification_client.py index ef48dec69..f9a90f702 100644 --- a/tests/app/notify_client/test_notification_client.py +++ b/tests/app/notify_client/test_notification_client.py @@ -59,6 +59,23 @@ def test_send_notification(mocker, logged_in_client, active_user_with_permission ) +def test_send_precompiled_letter(mocker, logged_in_client, active_user_with_permissions): + mock_post = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.post') + NotificationApiClient().send_precompiled_letter( + 'abcd-1234', + 'my_file.pdf', + 'file-ID' + ) + mock_post.assert_called_once_with( + url='/service/abcd-1234/send-pdf-letter', + data={ + 'filename': 'my_file.pdf', + 'file_id': 'file-ID', + 'created_by': active_user_with_permissions['id'] + } + ) + + def test_get_notification(mocker): mock_get = mocker.patch('app.notify_client.notification_api_client.NotificationApiClient.get') NotificationApiClient().get_notification('foo', 'bar') diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 801b7e742..1a1ee7c3a 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -93,6 +93,14 @@ def test_client_creates_service_with_correct_data( ) +def test_get_precompiled_template(mocker): + client = ServiceAPIClient() + mock_get = mocker.patch.object(client, 'get') + + client.get_precompiled_template(SERVICE_ONE_ID) + mock_get.assert_called_once_with('/service/{}/template/precompiled'.format(SERVICE_ONE_ID)) + + @pytest.mark.parametrize('template_data, extra_args, expected_count', ( ( [], diff --git a/tests/app/s3_client/test_s3_letter_upload_client.py b/tests/app/s3_client/test_s3_letter_upload_client.py new file mode 100644 index 000000000..c30ac61b5 --- /dev/null +++ b/tests/app/s3_client/test_s3_letter_upload_client.py @@ -0,0 +1,17 @@ +from flask import current_app + +from app.s3_client.s3_letter_upload_client import upload_letter_to_s3 + + +def test_upload_letter_to_s3(mocker): + s3_mock = mocker.patch('app.s3_client.s3_letter_upload_client.utils_s3upload') + + upload_letter_to_s3('pdf_data', 'service_id/upload_id.pdf', 'valid') + + s3_mock.assert_called_once_with( + bucket_name=current_app.config['TRANSIENT_UPLOADED_LETTERS'], + file_location='service_id/upload_id.pdf', + filedata='pdf_data', + metadata={'status': 'valid'}, + region=current_app.config['AWS_REGION'] + ) diff --git a/tests/app/test_template_previews.py b/tests/app/test_template_previews.py index dbfcbd159..4fdd73bbf 100644 --- a/tests/app/test_template_previews.py +++ b/tests/app/test_template_previews.py @@ -1,10 +1,15 @@ +import base64 from functools import partial from unittest.mock import Mock import pytest from notifications_utils.template import LetterPreviewTemplate -from app.template_previews import TemplatePreview, get_page_count_for_letter +from app.template_previews import ( + TemplatePreview, + get_page_count_for_letter, + sanitise_letter, +) @pytest.mark.parametrize('partial_call, expected_page_argument', [ @@ -75,6 +80,44 @@ def test_from_database_object_makes_request( request_mock.assert_called_once_with(expected_url, json=data, headers=headers) +@pytest.mark.parametrize('page_number, expected_url', [ + ('1', 'http://localhost:9999/precompiled-preview.png?hide_notify=true'), + ('2', 'http://localhost:9999/precompiled-preview.png'), +]) +def test_from_valid_pdf_file_makes_request(mocker, page_number, expected_url): + mocker.patch('app.template_previews.extract_page_from_pdf', return_value=b'pdf page') + request_mock = mocker.patch( + 'app.template_previews.requests.post', + return_value=Mock(content='a', status_code='b', headers={'c': 'd'}) + ) + + response = TemplatePreview.from_valid_pdf_file(b'pdf file', page_number) + + assert response == ('a', 'b', {'c': 'd'}.items()) + request_mock.assert_called_once_with( + expected_url, + data=base64.b64encode(b'pdf page').decode('utf-8'), + headers={'Authorization': 'Token my-secret-key'}, + ) + + +def test_from_invalid_pdf_file_makes_request(mocker): + mocker.patch('app.template_previews.extract_page_from_pdf', return_value=b'pdf page') + request_mock = mocker.patch( + 'app.template_previews.requests.post', + return_value=Mock(content='a', status_code='b', headers={'c': 'd'}) + ) + + response = TemplatePreview.from_invalid_pdf_file(b'pdf file', '1') + + assert response == ('a', 'b', {'c': 'd'}.items()) + request_mock.assert_called_once_with( + 'http://localhost:9999/precompiled/overlay.png?page_number=1', + data=b'pdf page', + headers={'Authorization': 'Token my-secret-key'}, + ) + + @pytest.mark.parametrize('template_type', [ 'email', 'sms' ]) @@ -119,3 +162,15 @@ def test_from_example_template_makes_request(mocker): 'filename': filename, 'letter_contact_block': None} ) + + +def test_sanitise_letter_calls_template_preview_sanitise_endoint_with_file(mocker): + request_mock = mocker.patch('app.template_previews.requests.post') + + sanitise_letter('pdf_data') + + request_mock.assert_called_once_with( + 'http://localhost:9999/precompiled/sanitise', + headers={'Authorization': 'Token my-secret-key'}, + data='pdf_data' + ) diff --git a/tests/test_pdf_files/no_eof_marker.pdf b/tests/test_pdf_files/no_eof_marker.pdf new file mode 100644 index 000000000..857ae1fbf Binary files /dev/null and b/tests/test_pdf_files/no_eof_marker.pdf differ