From f04512a31d552aae6af9b730e3a6a5db0d8baebe Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 6 Sep 2019 10:22:36 +0100 Subject: [PATCH 1/9] Bump utils --- requirements-app.txt | 2 +- requirements.txt | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) 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 From c57741686653866930b209bf7a5696dcc5d870a1 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 6 Sep 2019 10:39:23 +0100 Subject: [PATCH 2/9] Add letter upload form which redirects to blank preview page Added a form to upload a single letter. Currently this only uses the form to validate that a file is submitted and that the file is a PDF. If either of these validations fail, the form will display an error. Otherwise, we redirect to a new preview page which just has the filename as the heading for now. --- app/main/forms.py | 4 +- app/main/views/uploads.py | 34 +++++++++- app/navigation.py | 8 +++ app/templates/views/uploads/choose-file.html | 28 ++++++++ app/templates/views/uploads/index.html | 4 +- app/templates/views/uploads/preview.html | 14 ++++ tests/app/main/views/test_platform_admin.py | 2 +- tests/app/main/views/test_uploads.py | 67 +++++++++++++++++++- 8 files changed, 155 insertions(+), 6 deletions(-) create mode 100644 app/templates/views/uploads/choose-file.html create mode 100644 app/templates/views/uploads/preview.html 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..f35ae95f9 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -1,6 +1,10 @@ -from flask import render_template +import uuid +from flask import redirect, render_template, request, url_for + +from app import current_service from app.main import main +from app.main.forms import PDFUploadForm from app.utils import user_has_permissions @@ -8,3 +12,31 @@ from app.utils import user_has_permissions @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(): + upload_id = uuid.uuid4() + + return redirect( + url_for( + 'main.uploaded_letter_preview', + service_id=current_service.id, + file_id=upload_id, + original_filename=form.file.data.filename, + ) + ) + + return render_template('views/uploads/choose-file.html', form=form) + + +@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') + + return render_template('views/uploads/preview.html', original_filename=original_filename) diff --git a/app/navigation.py b/app/navigation.py index fe05bcd40..39b937dd4 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -304,6 +304,8 @@ class HeaderNavigation(Navigation): 'template_history', 'template_usage', 'trial_mode', + 'upload_letter', + 'uploaded_letter_preview', 'uploads', 'usage', 'view_job', @@ -365,6 +367,8 @@ class MainNavigation(Navigation): 'view_template_versions', }, 'uploads': { + 'upload_letter', + 'uploaded_letter_preview', 'uploads', }, 'team-members': { @@ -859,6 +863,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', @@ -1138,6 +1144,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', 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..fe14c5bb6 --- /dev/null +++ b/app/templates/views/uploads/preview.html @@ -0,0 +1,14 @@ +{% 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) + ) }} + +{% endblock %} 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..d02b65a9e 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -1,5 +1,70 @@ +from flask import url_for + 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') + + 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}, + _expected_redirect=url_for( + 'main.uploaded_letter_preview', + service_id=SERVICE_ONE_ID, + file_id='fake-uuid', + original_filename='tests/test_pdf_files/one_page_pdf.pdf', + _external=True + ) + ) + + +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_uploaded_letter_preview(client_request): + page = client_request.get( + 'main.uploaded_letter_preview', + service_id=SERVICE_ONE_ID, + file_id='fake-uuid', + original_filename='my_letter.pdf', + ) + + assert page.find('h1').text == 'my_letter.pdf' From be6b5b922af0a19058d92feefe0ea411b70fccd1 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 6 Sep 2019 11:04:35 +0100 Subject: [PATCH 3/9] Add virus scan stage when uploading a letter --- app/main/views/uploads.py | 11 ++++++++++- tests/app/main/views/test_uploads.py | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index f35ae95f9..a93fc7427 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -1,8 +1,10 @@ import uuid +from io import BytesIO -from flask import redirect, render_template, request, url_for +from flask import flash, redirect, render_template, request, url_for from app import current_service +from app.extensions import antivirus_client from app.main import main from app.main.forms import PDFUploadForm from app.utils import user_has_permissions @@ -20,6 +22,13 @@ 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: + flash('Your file has failed the virus check', 'dangerous') + return render_template('views/uploads/choose-file.html', form=form), 400 + upload_id = uuid.uuid4() return redirect( diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index d02b65a9e..67e5999b0 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -1,5 +1,6 @@ from flask import url_for +from app.utils import normalize_spaces from tests.conftest import SERVICE_ONE_ID @@ -22,6 +23,7 @@ def test_get_upload_letter(client_request): 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) with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: client_request.post( @@ -36,6 +38,7 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): _external=True ) ) + assert antivirus_mock.called def test_post_upload_letter_shows_error_when_file_is_not_a_pdf(client_request): @@ -59,6 +62,20 @@ def test_post_upload_letter_shows_error_when_no_file_uploaded(client_request): 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_uploaded_letter_preview(client_request): page = client_request.get( 'main.uploaded_letter_preview', From a103dbf801a96399d39a44f4a0f5c8007bb33cb8 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 6 Sep 2019 11:13:05 +0100 Subject: [PATCH 4/9] Add max file size check when uploading a letter --- app/main/views/uploads.py | 6 ++++++ tests/app/main/views/test_uploads.py | 14 ++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index a93fc7427..052329766 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -9,6 +9,8 @@ from app.main import main from app.main.forms import PDFUploadForm from app.utils import user_has_permissions +MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB + @main.route("/services//uploads") @user_has_permissions('send_messages') @@ -29,6 +31,10 @@ def upload_letter(service_id): flash('Your file has failed the virus check', 'dangerous') return render_template('views/uploads/choose-file.html', form=form), 400 + if len(pdf_file_bytes) > MAX_FILE_UPLOAD_SIZE: + flash('Your file must be smaller than 2MB', 'dangerous') + return render_template('views/uploads/choose-file.html', form=form), 400 + upload_id = uuid.uuid4() return redirect( diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 67e5999b0..530130e74 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -76,6 +76,20 @@ def test_post_upload_letter_shows_error_when_file_contains_virus(mocker, client_ 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_uploaded_letter_preview(client_request): page = client_request.get( 'main.uploaded_letter_preview', From 5fa9e071c71d22c6c415d9ee195833451682d860 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 6 Sep 2019 11:35:51 +0100 Subject: [PATCH 5/9] Add check that PDF file can be opened and is not malformed This checks that the PDF file is not malformed in some way (e.g. by missing the EOF marker). We check this by trying to get the page count of the letter which will be needed to display the preview of the letter. --- app/main/views/uploads.py | 28 ++++++++++++++++++++----- tests/app/main/views/test_uploads.py | 14 +++++++++++++ tests/test_pdf_files/no_eof_marker.pdf | Bin 0 -> 14046 bytes 3 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 tests/test_pdf_files/no_eof_marker.pdf diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 052329766..6a2315a90 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -1,7 +1,16 @@ import uuid from io import BytesIO -from flask import flash, redirect, render_template, request, url_for +from flask import ( + current_app, + flash, + redirect, + render_template, + request, + url_for, +) +from notifications_utils.pdf import pdf_page_count +from PyPDF2.utils import PdfReadError from app import current_service from app.extensions import antivirus_client @@ -28,12 +37,16 @@ def upload_letter(service_id): virus_free = antivirus_client.scan(BytesIO(pdf_file_bytes)) if not virus_free: - flash('Your file has failed the virus check', 'dangerous') - return render_template('views/uploads/choose-file.html', form=form), 400 + return invalid_upload_error('Your file has failed the virus check') if len(pdf_file_bytes) > MAX_FILE_UPLOAD_SIZE: - flash('Your file must be smaller than 2MB', 'dangerous') - return render_template('views/uploads/choose-file.html', form=form), 400 + return invalid_upload_error('Your file must be smaller than 2MB') + + try: + 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() @@ -49,6 +62,11 @@ def upload_letter(service_id): 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): diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 530130e74..d8daf1398 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -90,6 +90,20 @@ def test_post_choose_upload_file_when_file_is_too_big(mocker, client_request): 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_uploaded_letter_preview(client_request): page = client_request.get( 'main.uploaded_letter_preview', 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 0000000000000000000000000000000000000000..857ae1fbfbbfb6b0bd0addbcf8401cec6dbdadc8 GIT binary patch literal 14046 zcmch8byQqWwr&Unf`{O4!JP&gYY1+^9fH%i1b0Y6fDqi>-Q6{~yEg9LxZ5N1n|o*G z&Ry%>Ki+-a>r|axr%u)RzWr6Lb-MSaQV=S=%^}agebn*n^GDjT~+5U!cEWBV$p2VSg;$y#Sh=2TE0WZ3UU`Gmr*fLF4N&t(wp{W;+2TRHI`V~80a;0#gDz*gDx8y+Hk= zE-_a}31!C@`Tfn30Q^zbACde~m-x#Re`^-!cu~oJHY?cM8YzPvUnXLCS-LNa4eaXp zqCV>v`h@>pi~PNodYS*vNXm}(PDYM@J&420KrDZ;`-ky=qV^Ax|8T~COYPrDW#i=h z7gkf`g^)T}-!&gIY%_8!$tGsQoRXQ1O1C5AGhxwdJfqt{L{2T>JB*#3xft}tsN8Ep zI=wQ1*xh;Q-#PmwPUF)Y7uIq(F+m7PnL2mgdJsn1dS{J(euX6-lr$)rQGBy8{*Pd3=FDQ4ZU08W5Ace5Lh3AEvn+Rp!Mv5TFV6Ofv5iZZ|H6EHL}u z#LN9ZL-?|!xq-lcsECb?ixu!MkdM3&9aY4xu1nmfm!u@g4rPSTBE#Q)`0&nIRFcd9 z-xxifOqw@^+PqJ^QKVh*ZPr_Vv0guz#aBSM7-};3&+R&jU8unDZt$YGP%oH+{PD4H z`C#OA*12BmdCGZ8_G61j(Z1S}cAd>q;q1E);-c31zc}`fX=~%K4^un=iaY2(c}raw zuXYIsmUTaIWD-WyJU>)^UmuVmc?!&zCy(gXR;YG-if>Jw6xDogrxo*)^LScY6I^-y zM5ixFPS1-~w*vDdk*;n~VcNfD4dtNV3eKO5%~T3VaeaT6C;RdC$?!h1auq0SE5dgX zp=-$EEHH>9{(g9X`qx_q2A5c`^W9AOpGspS_Nm!N4<$Yiw>_g=!XV2tZifI-Z>FkWKj(SevS1o*+AKg+08+x2uI%LX#6Hkxx{~ zryz#_BD)v%#`%JThd2c99`6(d!^f%|OWve+)r|+XClXuYprK8aW~%bq$*}d^0wT zl0McwxZtYyM_J=#xC67k$1r@w=|^Aj(t$od{Lkesu*{IQef-#LSHbkr4roJ&S4mtt zZ##RWWMNX|VeLHYj~&KVscUPxE}2*r2_;q2k-xpw=M@z0UbiLG$E9+)2{ zlx%A#$p>FzXqi!nhtgdK1#8Y#E(q2xDzlqBsB~Wo<6j?o@W|sU!~ANYY0WsDfUiNo z(DtUkCbo*bH-JqOD-r@yhL**#F&v#M@8RNG4$#bDQWMV`mp)FDz)b&KAJS-BmVa!0 zJ0{#nYP79&9h%G1%gz!eC*nX7rWqu;jVIl8DV6uEKF>F{0 zX1pyB)YVo>I3JGx1z%(M++wf}z!ZcPBoBoe`G;n(3l^Qj@DS9G|5RVG{dVISL-I;z zBWx7MuCZLq)4w)&Fjggnsqgs73yII>Y*cSsB4NWgG}=vW8^6s{29oW=x$=!W*6rr) zFx$0LaiHMu9{Hh`F#h1n){dR3&~uRS(!oNh(}{Im?)T?cft62| zY++Cc{h`DZtV53cl-4_^sGaZpm^FN>rfxVTSqC*6^(=j@umvr=EaH>c<&UbW0RUAc zP1A<&wVG`iDs0FNSMRn4Ya-9Xulj1x3)9QfNBS^(&Az(gv8T|%*=t9mPwbLo<5nIE zF9xk5C;3|aoTm$q*sxahn#lLv3%ieDQSn(6Gxn6y)Bj{?nwSB_Tp4SL)pUGwHKwMG zZCx|ck$UiHfvpF27bpYy?hTr9^Fq)Ge5@g8Fz2*KE4WG9D4KLO6p#s@g(*Q2fJiMd zs-CO~b6%quY|g3LKb!H<3ibTFMB6lQORM)S6r%uZ;ZE>Ql~j?*3m+3qEq#4lAoaMW zz6j$w=MuV>&5!ev7;Fk|rtEt(l*tO6p?5+j1N%1TeQdkJww`_xEyC#C6LM)sXDA>Y zLH3^udJT`Kumz@`Ay9Byg9o!|aqHymBTOuK2F$?QF^rBq8op`hvXY*LS8y3my$h@> zSNF-`t8wO#C(7j$V=9ROk4Ly$>JIs^Kw~SPXl0`=AFQK+6GUw9AJvru<0WjYfriIA zNEqi8GrOjmVcdIuPlEcNF%Oj!vxa+?8!;ZrS|zfRCn{!L{C>oLd{k+pR;axj?=TTS z^uEWO!Mc)eO=v0S)X!V#37X5>rt1|Gq@#p>Xa?NKy*h=rjvtvFm(xdn{@G&@Wlpyc z3g>_uVw48!lHeLmP1{Hq9}4hhZNibn@!dnze{=I;%=D{{IKE5WZ$~1Hz>2gtOOz|f zA8zmnDCqW~FkG+8ZWj`T_Zv05lr<>w-EQp)BlQLEMr5j^1JVLMuDr9ue?TO|v5hEa z&f}@=)@YMaAoXL@j?SP_<(ET#>Pr=T%_sdBgN~ZbnUduL`q=b`+E4!~~34 zfM=;#EBCUSp6Hg>1ngI-Q78!fYC)0IoE8k8k;-pDK55yEd|Qn!{sfL4Lc}*Wu&!5+t3Eo6?P`yZrW2UPvcoX9_sG_xYWB zOJfLcVOL0;-lV%=PCvr65?%J9+|>U}XWDM5`N*CQG)bO?s-Ur#tbnHVq^TEaXG3hg1cS<)5U)Gk3Zu)GkXTVKZfe+~UYpv((N!c^$IQ0sCb-F+4$;bsF z)N48SSfK4XbR$n=)IaxKmFJFK9!ku$nOj`UwW*U2M}fPK^0me;)&6GQ8I~BT7 zwvf6u(?F``8AuNzwlAspQ-$#Rchht@<_}4tC+}{FiysK0{m0pBlb42Uz}g9MDf$Ab zoqF44XJMSV=33=Ac}$aqH8l{mBUB9f++Pibsr9lCvWrYmOpmUpb&=B;aNCyK;A|{u zNyf}u%W9_|PX10AHFJ%9jYIBPPFa`U2c0uIGqWYV#ckpx!FQ^*c`h?q7aAZT`I-9z z*h`E%Nw4VCjJW;5%lpfK3kgF;He#d3uCAUBT_1XlyFU{$q-Spk)o=Tniu1oabcB^-4%R4cW**__AJow+B`WVj`NXGBa@ z%z9kpGSppNCt&BU!=d2r%cKMHYQz0%1Mx6F5U@QEul+jJ5`EMuEQB2tN0?N`pZ82S zDvRt2jlE$y<$ImVhcfE*YBc1n>qtWRs!T3_??@mo{ODx~A^d30bVv!II2!Iio<{m+R8?y9RzOUDdF`MJ{XDM*YX$L%*IATLbhD&5lF4}MQd_~fKi*o>YIk8Q zF9SGdxksS&NlZmDZZiKa>_C31P4ijnaqq6~AWXXdPWF=UfV5!c*QoX7;DK%avrJ2o zH`#ceI);~h$Bo+ah4gADKs~i}?V`-IHmy{25Al`TrQ_w^!H7z-))$MtJ=iqr6kQ*S z`vO!NVDh4~J?eNr0(eDPN1mBCaLBXrxxYFJR{n~Y4Jx`F7(wHohG%cM8u&zWm;4ii zVV60H=={*Mu|o8GcC*tf7|{~i=J>FBY8omsc#0b)_{DSfbZEmUG>`PyiT}!V=ro@s z!2MxnGX&DEZDnMWr|;qqgE?5C320e?1^j1TkhJp$3uIuGYD*2Qf;yvEkuIjRDSMf_n&e5w+u#7f3ClVM-)CyEp@RotxQld#difx>>V(Z@q-h_n3I4 zZ6}OWKCX=G?diBDEejQUi>-{wR)76m9sUcWg{s<5+8iTBk*A^HJO9r;>AT2Svc|ZH zv_0OgTg>X1*h%)nOzaEJN8hU|)I-}~KnS8q25+G$;XHM1!jjZZ!8gr>kT(%wOWjbD z>vE5XpxR1(oDN92LT$*Bgy1C0q4(7*k8n^|ZB@WLT5S}lg9IE#Zt6B~qtpaaL6VY*zDLO`N3-X37oP&&nY-pL z^45Pg7OYI0XF1e6RKFsWF?K3WO)*7fMBKr%>*1NLU&XQ6;v(ttb$ffzyKtgF(z9^p z%%5DhL+mQC97uIO(u^!)@aU$#9OAMAa)k@q1i4{_ZS(348o4qOwkx?25%zhUx~8_X zthKn2F2~WFO+Qe})aQT%i%P~yXVolCWa889W=B|*t>w&gAL~Vf z6KG!{TwgKcT5^*&u*0`EJhuFD(H6+Wr%o2|ApV{TPMk9Hua<10vQR- z9Ft(CR>$=ojhN2~I#Hv2O@a{t%F4WqpR3nBMU1ER zM((i7vePx_+Gf7-sP&%I^(5m6{dlebbhc~M)ZAowb?D`}f?0Eq&VM^KIZDKTap*AG zcCs$R(ss7LHmfUm)-w*=E2&#?yPoqn_BmOT=#}^MJX1MXs>pm$>pmcUXLh@ByCb7d$Sccl+PrNR}Ni!e4mP<-aNx;CsjGAQWAMD{> zKWoMU_AD?f299au^jH!f)X;G@%8dBW>^4QAsg{+X<~1*NjXl(R-Y;5}nT$s8LTH3- z{p$`m)0`|B4NuUFs?<5HL&gdLO%0Bk#ihDA9(A8aZ6E}cX*@i5uwHHt^^5y<3N1Kt z$sEc7T&?Uh3xjIHIn1UMrr@Db{5U#Gd)(dS)p|BNziDLq3Jy?V3x|>UC)qF(^Lwwn z0*PGRlo(>%4Ps)9IHS>@i%Wrpri@FIAlc*P<{Z5V=*gA8kxQMmDSL*2EWT;Ftex~{ z`zRAQ{=sZ9J4}psx=R_Ik%!zv=JB4~0g5$7cwHw^onwPZVc2$VzP;OO0b${nm2I={ z!}ts@<@=5r>{gz#+(MR$D|!N+MI&SE-oNFnzIU@V%&fE3!>6dN&`_S^mX1NVmeKk= zxUoLa;qj1|UngYR0XYI};p5ne@me3eaee7LRax@zE7& z_M4I7m;rcfdn@L(;D9_N83k%^TZ>=I8I1$n#rU793e!i*YTl63tD;W!x^+6x;Eg7S z$HzaHE0-K6ALd9J0Vh8l|Loe{l1Yk5O2E}B`K=$(P6Hk;C`B`$@mBy~$#Ec3q*hiM zT~z2M`laB9JzIDM_)Beyx=yZERw%tXHC0LIHu;W0o#so5pYj;lLq4vm9u_p&tGc7M zVCkjmX6Nqu`O^D|$fvZC5im>2UDFC{NzuG9v)!)lXC>Np!EKS}lvDK;eTj*3j;OwIPpH}e`YORV{-bXryHBXK*ZYGM7(FCt&LRGYEj zK>xY7B3A8WS-ft+LM&43W&t--^im=nO6WN*zo#E%zbfOW2Dg1Y!pky)WI9o^v~w0| zM&&xqO)`*QF8d~ueMaE$w=}Ol4)3w(faK){d?6K-A*N0A>22fGx~cPk)XKL2ALSn~ zA#aG_IGzv%OmFbDM&ykUsyiJxzOa0G$BmTyruczFAn$~yUtW0MNE${Q_WY~IH{W*6 z_8!3=Kyq66y%dN_kYM!zE_LmJJJ*S5)cJw(3+ZW?K7@}`Bp?r}{>kuHzi4mLr`fkr zdt+2Jv28l%mT9|MPkSUFHg_3!pC@KF7&P#);faeBLM}|hM|}M^*Ch4V`AGF|=3cwP zFx)i<2pngO8mv04g7(p~VJ5$lJ(-|RO`TCn@L>zC9aGnlRb%`^*_Kt3a>S_LXg@YA49$^th<5q}7 zJBoA(#+P3z!1rw==S55=`a~_6wD!A{SXLai+(C(spM&jdc4gkSujOUt$`QqGB{h5) zxNjxv4VeCvCU0I!S}hbgf%j*)U!%f)@SL~zOLR;zesP^T^GMxwlDZfxKu$fjT2jr4 zvq;)lHKLgQmugN$A&`V!@HV5eh$nZ+YnNTEU+?itTyQ=II zT6>yA?@k5E=p@}o{coufc%U$$)ZSM|Cq$@2uxK!*Fr@l^j*KX6nxKWXgxA6F+c1OQ zs=lfwIN+?RSR~fV;Kv#gfK+2|&TqQbF*}VW-2El|Y5e8KY(kBXGUujf^{_~#bvCuV z1FnRQPHStA+Nw_)2G=4`tPE(H@>zE2I7J05xxSA4D5>#zJe z2!UAKMwR4Kgis_}rerH7M-8<-S7S|)l;kggcF2J4eWp*y1LIv$pBCEHU>{A^%B+3&GKCdbcS(kv$@ZhEuSL2#%IMs!*uZq z2$ID+r){DqAY!AE@t7c(4Y268SY89$cI)nVgt|l+M>hPRzS#5lc-2tw?KMrUkBPuC7=Mnab|DOly!eMy;F>@%Ij22)rM>AKaTan!cj%&@+pZOD&6_B}V6Y_@v^i z4+?+zsm1OJhVz&OF%o-9i--NH7=Bve`r!)BFZkj!?7=DBocC(JJsgB+R* zB2O=e5;j|8m%-N?kGG#qdc2#ZeK(DdXT>W>*-iW0tyRS3MzXcIwEH7QS_6nEwmqnD zvBVmqDM7RY;R{xM-C2U0?ozKCR`q|W+n@gaEiK1NSGTal05EbX)0NIGlCz=OfEEH{ zc!(am-%wQA*HHUbV04-G7XfWrxj4(5=Y1iP!?WY9()Dgorv ztSpR?hyzs>EL6%E zQJ|F{nJ9A?F7$?Eh|2pleQyLevIjpe-U!9y3G*(xQa3&?p+neDmt)ti-pQU_rSSCg z5nL}WaS>WLY(+OyJ~&xJjMp2@Rj9MdLhEOnI|Fsbo#E7ow67g~Ar>EX4J!#15nHSvx)s~(}{S4f=vwL*It|G7GXobUv zw)q8l52aJLh-0UsXQni1}3~pq2l&;t#7Bx|F>l&R`y^fXlMUB!C3SGl67D zD-;HKRvud&rcxr!D7LHOmh}Uw{m&T)#APHpWxUL|wWRm(-4m9L?G80{Ef8z6wI zZWDJvSQEL43p6FX-Stjsjd~?bRthkOlWo|)QJa2?H zUU{DNzwxo{+NHtZymzG)EL@^-{>5uK>Oa|9s}TX$7zNEW)xF+cSiPuT-vM_72)*G^ea|hpnGe zKO;P=9u#(41VZ$!vtzSjIto=GOgH?cDHxzG#WmkFWhO$#Ypq&sXRXvIpH!23xX`L3yr1njEr1^vSSpd9)k7qc7`cQ4clH*m>CaP3LD&(PT{pCl42IZZsq{f z?TR;67}~Gpm)p6j5TcL5D)LryuIDx9hHK>W|9y<3L_oDZjMZLW=!lKnbu@Na0gK^JA{puxNUX9C# zYMM=M7YPiRYM0GQYP`c_hG-W!791B;Ym=<+t&ZWh25c4g?A$Z=NS(?jAxKQh9?3K3 z-!l1@HI_f}k7jvVvrA;39a(X6sx#tuB?+lJ*}fsa zy4s4hd&mdBY+z;Eq-P19BIlQWbsL(%JA$-cGop}6eyQnouSBUVFH5i9cS|i120%p; zXkMB@?21q)+|fH!tN0F=`yO*}{4rf3)T0k~SU7)7T~v9G&b{4)yoqUCMPcffT6HFN ziJ2IK*7$!tefg_mOK+-bRfJ&PY)E%RpbBDfn_C@fjFD!JWr&%^wohVQF# zBuRtaNe1QFZSyulxxs0H6nYi3CKYg7%TaeU`c2PHd)Ws5_PIqx zrs-HVHL66UvEMR!f_!>Y3=<#O&pF=gOs#K0c3Iqw7)e&(C$BRK@K%4|aAI&%q`qZI zn|j3`-q9|>lNR-`a(=V3nui!I$x~YzYA^aU2rukwouRXjRrb_9Q)-Q68*`~ciDj#N z8AmH1c`AZUYE>HXo6yquq=x{%xk4QN1q$9jEZgFLx%dxLN+7%6b0NPWSl!bs>aNxT zs!l8Lc_hd3+dAL(3Yho4iMvSFN~HT~GB`{*?Di9$l~J%Nc>&)T=~W(#>2uz<8%ghR;N;k! zoS93s$Vlzx(+NKtf59(Yei9_2qtZ{11B@$Wd$aV0+hy5MZssofT$Jifvjdd1?78Qodg$T7@&PGJSVOH6hZyk34X5DlRys4h^MUuEA> z%I%58;fYqY$&sy;077V-$BS1+tUmOYYFu2l&E?_SN~7F2+8 zC5UM8Q_bmie#gsgVdLhr)piyaD5S<(PUBJKSm+GINCc_)m=^?x8dk;W`HEcXSy?zG z0Aj5DfrKrrq>=Ug_Qr)QCSzSJw}5Mzf1)cuDYssCZKp1J>(=T-@OjZ7@O$=WQ_y?UW@ z)0t>Pfe4hmGl*n*#kRudf*o6=n?B&qTBen(EYz&lf_()$|0b@8@4HFT-9v-g_JD!w z{uCi)KPRpF{qH#muf$+Ze7hwL_fOq!4GqUZ)4Ry9mJch#w>^zm@d;z*+jTws;9y?l zoo{lJDn2Riqg(r3<@}(x^!AS=Q5Ab?%gh`SGP!i&gB^QpXm~;RQr`3y;AcBm-O_!4 zA^=mq0imZeDBP|Jb3#LHh{u}nGoGm~ou@)+W|o4~ z{m#bd!;|ZPa)}JN9>&<-+f9W|7wke?Bd9}SlZZ)vkEit}Vq8Mfk_nVnA&VQfLH&-a zcZ#~I&k$g`^F|INlDFf+zY$h4@Dax`k#7Nij&F-z-fgIvZ6)|V=ytGeA*VN4R@{6D z2X}uExFOSHpE}U)Kkc?X5FJmcXXVGNSY-jFqdeEc!^X9^+EGYgI@%XE3YejoT(AMC zKk_a0(_hIXZl34h7W+*$4|6Q>^w1fo12aEjlJ0or{t?FCBjny$W+#<0V+OXvXK8 zipPZ-J^z~5zHxu@>4%83=;deJwq!Pl$(`38{zlv*6-s5_%RiDhvmz4rHKQF7As&UH zz3ja2AxjtRcNx*-=#M@uds5pa^F;BcMTIjiBj><7rhadKA?!p$=i7?qNk2`anQ}Dj z(kudQ^>LAtWEMFZ8Tj}aFE4#?sCFV3pI(H}bQkM9Rn=m7b?b>kZo+=TI+T=gF*vw`!;;M z8Oxk{qb`j)Z>Xf)-5UmVv-!xtE*TZ|)r$qVYFG<0pKZdRwAFD7#3YViJU*kdB;}Jj2@g)rA-5Sw|-Q^eiFN#FxF|vcC4sc4Bc}ZNO6{DOEgB1c$2#uJFu8@ zZjj4v;4P+zp#f%?!i^ISEc(#aaipm1z?r@=EM(8>MgXQWgy4HTY z?_6TuV_a&0i=uPJVVY(G0&yB7LlqC{m7tgLb{kxmJY$rj1?u%FxBkGCAo}bIY_vTe zFK@csU<@|8uwGRqvOHaASEI+GlzqbDmxN!GSV2c`t}H)O;X(H7KH$rVTzkef75BBO z6o$9RB*#G6J@7yIN(y|ss9&_#r_)u_)%sj0E_2np|JKL?ko*Q)?CK1oYFu92+TyHS zrRCZY($;9$QUU0fbs z9$qZW=(jRyuesSCPh@hWSoE$Ofv>DOeNk7fUS%)hH_Kymor<287P$wn@sEziUy+{J zUOy4xqJLR-{XiZ^vGWjWn54@BYK{uk3G)f(amd?%E-p9#;_gLC{^^)ON6|&fP)FBy zgy(Db;!T^CaX8#DKaqky3Nq4A2$XgBW*C^zIU7VF>a1T8`mLhZQ;cbf=fB`oF&Gt5 zfh_$j%x#If)TE*xiMuoX#*+}_%VaRS1eOc1Y#p>)8**~35EaWmv>Y9E>E{&Rj4tYm z`A{?%W2-7|W}p&LG)F}PM*qvPoW;Nu7Y_ven>z(vbu*5^34bb2~&ZBSXSS?K&m*tN^ zR%2rVcVjS}cbBA7Z9ZR4ily=o1@+t0gNipsYn*I&tZ;q-B35@1e!?jmd{+qgys*e9a5Kw9IVMOX@a;6rAq=v7BZ^+%pmoT zIR2fUFavr8MPJ*zavK>37$TahH!YKDmLQW5A^kSzj~?0m8qQ8^Cvl_s6)D4xJV_zB zJ(!AyU*kXY1Pz`Lp_w8Yb~-%UC(6V?VUhTM(-X;i^-Ts|1DIT=YJNe zO~%gtuaVh0;j$K8tSElh0)DZR-TLy4sNdLg!o_^dAh6uEkpu+uv@iNEJM^bh(f55= zrb*2ydo+?K_x0Y`g&$&la~5p5I7c4Lji3|<@A>6+cmD_{DH_LhZZ=p(URlK&IekS; z2XOqw{lu<|FGp{l5+QNz!i_kav^KicPG~@1W%gUI6g6n!!Vniur^4Y_4Bbr-`F!VQ z4pUh9e$bWx7gXBMYb*mOoRM@I6OeL3>VKX@w@6-$+uCo%tz!!*fNr#W+48AVurZ3| zcS7Wp41D+2)NyD*vbqmeDozp=#jC)eOVV)p+E>X$dPPwHz2fZGb3s^eQf53&06qrtcJdEOFcB1kWLQ6P8GNzcbYV%K%B!pECym7Qwj8lS zo*ezHab$=b1$r(oPx$qGmu{i@7UeD9>bqoZ<605*t40#sTVS>4yls;vILHMqTz_#8 zp}WH0FCKdBB_dg=La6J%Brblp4r5uH(kdrTNoIa7v4g9o_3t7_<2q?RUB038dRtyr z8jOYO4{$W1UuZKjRh0kuyGnFlg>HcBsKjYH>=jexhq&Yiw@G5w2;mQY3ZBx@H3jbZ zfNM{v55}}Huc}n6U5Zhpbc|W48&W?wHCqModmb-Fv*dX`4K=flHPx8|3)>e{v@2hu zB7KXxG)`4(#Ln8^B&{M5I>WbJq#|cTN18B>pAwiS2J*|9_eI^tXJbm%On5 qt(2u#uc^f+gHT?*wuV9Z_oSTvk~#H~>ttZ>=xPr(L1Jg)Li#_e-o9r5 literal 0 HcmV?d00001 From 8a322b844b358470b8459a20666a56a996c0c4de Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Fri, 6 Sep 2019 17:10:48 +0100 Subject: [PATCH 6/9] Sanitise uploaded letters and store in S3 This sanitises uploaded letters and stores the sanitised result in S3 with if it passes validation or the original PDF in S3 if validation fails. A metadata value of 'status' is set to either 'valid' or 'invalid'. --- app/config.py | 6 ++ app/main/views/uploads.py | 24 +++++- app/s3_client/s3_letter_upload_client.py | 16 ++++ app/template_previews.py | 8 ++ app/templates/views/uploads/preview.html | 5 ++ tests/app/main/views/test_uploads.py | 78 ++++++++++++++++--- .../s3_client/test_s3_letter_upload_client.py | 17 ++++ tests/app/test_template_previews.py | 18 ++++- 8 files changed, 161 insertions(+), 11 deletions(-) create mode 100644 app/s3_client/s3_letter_upload_client.py create mode 100644 tests/app/s3_client/test_s3_letter_upload_client.py 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/views/uploads.py b/app/main/views/uploads.py index 6a2315a90..ef47b61da 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -11,11 +11,17 @@ from flask import ( ) from notifications_utils.pdf import pdf_page_count from PyPDF2.utils import PdfReadError +from requests import RequestException from app import current_service from app.extensions import antivirus_client from app.main import main from app.main.forms import PDFUploadForm +from app.s3_client.s3_letter_upload_client import ( + get_transient_letter_file_location, + upload_letter_to_s3, +) +from app.template_previews import sanitise_letter from app.utils import user_has_permissions MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB @@ -49,6 +55,20 @@ def upload_letter(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' + upload_letter_to_s3(response.content, file_location, status) return redirect( url_for( @@ -56,6 +76,7 @@ def upload_letter(service_id): service_id=current_service.id, file_id=upload_id, original_filename=form.file.data.filename, + status=status, ) ) @@ -71,5 +92,6 @@ def invalid_upload_error(message): @user_has_permissions('send_messages') def uploaded_letter_preview(service_id, file_id): original_filename = request.args.get('original_filename') + status = request.args.get('status') - return render_template('views/uploads/preview.html', original_filename=original_filename) + return render_template('views/uploads/preview.html', original_filename=original_filename, status=status) 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..e73266c79 --- /dev/null +++ b/app/s3_client/s3_letter_upload_client.py @@ -0,0 +1,16 @@ +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} + ) diff --git a/app/template_previews.py b/app/template_previews.py index 2dbcba122..14a7ece98 100644 --- a/app/template_previews.py +++ b/app/template_previews.py @@ -66,3 +66,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/preview.html b/app/templates/views/uploads/preview.html index fe14c5bb6..2597b1c0e 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -11,4 +11,9 @@ back_link=url_for('main.upload_letter', service_id=current_service.id) ) }} + {% if status == 'invalid' %} +

+ Validation failed +

+ {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index d8daf1398..ee98153a2 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -1,4 +1,8 @@ +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 @@ -24,21 +28,26 @@ def test_get_upload_letter(client_request): 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')) + mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3') with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: - client_request.post( + page = client_request.post( 'main.upload_letter', service_id=SERVICE_ONE_ID, _data={'file': file}, - _expected_redirect=url_for( - 'main.uploaded_letter_preview', - service_id=SERVICE_ONE_ID, - file_id='fake-uuid', - original_filename='tests/test_pdf_files/one_page_pdf.pdf', - _external=True - ) + _follow_redirects=True, ) - assert antivirus_mock.called + assert antivirus_mock.called + + mock_s3.assert_called_once_with( + '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') def test_post_upload_letter_shows_error_when_file_is_not_a_pdf(client_request): @@ -104,6 +113,57 @@ def test_post_choose_upload_file_when_file_is_malformed(mocker, client_request): 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) + + 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' + + +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(client_request): page = client_request.get( 'main.uploaded_letter_preview', 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..f37464e8d 100644 --- a/tests/app/test_template_previews.py +++ b/tests/app/test_template_previews.py @@ -4,7 +4,11 @@ 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', [ @@ -119,3 +123,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' + ) From 7368245c9a3bbfa6e7d7ef75a3e2c9ef8b31f4fd Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 9 Sep 2019 10:59:32 +0100 Subject: [PATCH 7/9] Show letter preview once file is uploaded This shows the sanitised letter preview if the file had no validation errors or the preview with the overlay if it failed validation. --- app/main/views/uploads.py | 45 ++++++++-- app/navigation.py | 4 + app/notify_client/service_api_client.py | 6 ++ app/s3_client/s3_letter_upload_client.py | 11 +++ app/template_previews.py | 34 ++++++++ app/templates/views/uploads/preview.html | 4 + tests/app/main/views/test_uploads.py | 83 ++++++++++++++++++- .../notify_client/test_service_api_client.py | 8 ++ tests/app/test_template_previews.py | 39 +++++++++ 9 files changed, 228 insertions(+), 6 deletions(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index ef47b61da..28e40b8f6 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -13,16 +13,17 @@ from notifications_utils.pdf import pdf_page_count from PyPDF2.utils import PdfReadError from requests import RequestException -from app import current_service +from app import current_service, service_api_client from app.extensions import antivirus_client from app.main import main 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 sanitise_letter -from app.utils import user_has_permissions +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 @@ -49,7 +50,7 @@ def upload_letter(service_id): return invalid_upload_error('Your file must be smaller than 2MB') try: - pdf_page_count(BytesIO(pdf_file_bytes)) + 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') @@ -76,6 +77,7 @@ def upload_letter(service_id): service_id=current_service.id, file_id=upload_id, original_filename=form.file.data.filename, + page_count=page_count, status=status, ) ) @@ -92,6 +94,39 @@ def invalid_upload_error(message): @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') - return render_template('views/uploads/preview.html', original_filename=original_filename, status=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, + ) + + +@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) diff --git a/app/navigation.py b/app/navigation.py index 39b937dd4..cbd05f76d 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -314,6 +314,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', @@ -607,6 +608,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', @@ -887,6 +889,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', @@ -1170,6 +1173,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/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 index e73266c79..27848f105 100644 --- a/app/s3_client/s3_letter_upload_client.py +++ b/app/s3_client/s3_letter_upload_client.py @@ -1,3 +1,4 @@ +from boto3 import resource from flask import current_app from notifications_utils.s3 import s3upload as utils_s3upload @@ -14,3 +15,13 @@ def upload_letter_to_s3(data, file_location, status): 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 14a7ece98..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 = { diff --git a/app/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html index 2597b1c0e..9001411b0 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -16,4 +16,8 @@ Validation failed

{% endif %} + +
+ {{ template|string }} +
{% endblock %} diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index ee98153a2..609ff116b 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -30,6 +30,7 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): 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')) 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( @@ -50,6 +51,43 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): assert not page.find(id='validation-error-message') +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')) + 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( @@ -121,6 +159,7 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): 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() @@ -145,6 +184,43 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): ) == 'Validation failed' +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) @@ -164,12 +240,17 @@ def test_post_upload_letter_does_not_upload_to_s3_if_template_preview_raises_unk assert not mock_s3.called -def test_uploaded_letter_preview(client_request): +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') 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/test_template_previews.py b/tests/app/test_template_previews.py index f37464e8d..4fdd73bbf 100644 --- a/tests/app/test_template_previews.py +++ b/tests/app/test_template_previews.py @@ -1,3 +1,4 @@ +import base64 from functools import partial from unittest.mock import Mock @@ -79,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' ]) From 79053dec93c2c3921b4dc4e653fe0f5bc4caaa37 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 9 Sep 2019 12:22:52 +0100 Subject: [PATCH 8/9] Allow uploaded letters to be sent if valid Added a send button which only appears on the page if the query string indicates that the PDF is valid. Before actually sending, we check that the service has the right permissions and that the metadata for the letter confirms the letter is valid (because the query string can be changed). --- app/main/views/uploads.py | 28 +++++++- app/navigation.py | 4 ++ app/notify_client/notification_api_client.py | 8 +++ app/templates/views/uploads/preview.html | 14 ++++ tests/app/main/views/test_uploads.py | 68 +++++++++++++++++++ .../notify_client/test_notification_client.py | 17 +++++ 6 files changed, 138 insertions(+), 1 deletion(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 28e40b8f6..5acb8a898 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -2,6 +2,7 @@ import uuid from io import BytesIO from flask import ( + abort, current_app, flash, redirect, @@ -13,7 +14,7 @@ from notifications_utils.pdf import pdf_page_count from PyPDF2.utils import PdfReadError from requests import RequestException -from app import current_service, service_api_client +from app import current_service, notification_api_client, service_api_client from app.extensions import antivirus_client from app.main import main from app.main.forms import PDFUploadForm @@ -115,6 +116,7 @@ def uploaded_letter_preview(service_id, file_id): original_filename=original_filename, template=template, status=status, + file_id=file_id, ) @@ -130,3 +132,27 @@ def view_letter_upload_as_preview(service_id, file_id): 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 cbd05f76d..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', @@ -556,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', @@ -792,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', @@ -1074,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', 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/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html index 9001411b0..c8d954887 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -20,4 +20,18 @@
{{ template|string }}
+ + {% if status == 'valid' %} +
+
+ + + + +
+
+ {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 609ff116b..6638ef3ef 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -50,6 +50,10 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): 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', @@ -182,6 +186,7 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): 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): @@ -254,3 +259,66 @@ def test_uploaded_letter_preview(mocker, client_request): 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') From 81da8e762dae3555a8a402b2c86a20eabad1a30b Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Mon, 9 Sep 2019 16:05:00 +0100 Subject: [PATCH 9/9] Update for new template preview sanitise response --- app/main/views/uploads.py | 5 ++++- tests/app/main/views/test_uploads.py | 12 +++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 5acb8a898..82b0373fc 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -1,3 +1,4 @@ +import base64 import uuid from io import BytesIO @@ -51,6 +52,7 @@ def upload_letter(service_id): 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)) @@ -70,7 +72,8 @@ def upload_letter(service_id): raise ex else: status = 'valid' - upload_letter_to_s3(response.content, file_location, status) + file_contents = base64.b64decode(response.json()['file'].encode()) + upload_letter_to_s3(file_contents, file_location, status) return redirect( url_for( diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 6638ef3ef..b9b532f84 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -28,7 +28,10 @@ def test_get_upload_letter(client_request): 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')) + 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') @@ -42,7 +45,7 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, client_request): assert antivirus_mock.called mock_s3.assert_called_once_with( - 'The sanitised content', + b'The sanitised content', 'service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID), 'valid', ) @@ -64,7 +67,10 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file(mocker, client_r 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')) + 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)