Merge branch 'master' into add-http-healthchecks

app/__init__.py

@@ -219,8 +219,6 @@ def init_app(app):
     def record_user_agent():
         statsd_client.incr("user-agent.{}".format(process_user_agent(request.headers.get('User-Agent', None))))
 
-    app.before_request(request_helper.check_proxy_header_before_request)
-
     @app.before_request
     def record_request_details():
         g.start = monotonic()

app/authentication/auth.py

@@ -1,6 +1,7 @@
 from flask import request, _request_ctx_stack, current_app, g
 from notifications_python_client.authentication import decode_jwt_token, get_token_issuer
 from notifications_python_client.errors import TokenDecodeError, TokenExpiredError, TokenIssuerError
+from notifications_utils import request_helper
 from sqlalchemy.exc import DataError
 from sqlalchemy.orm.exc import NoResultFound
 
@@ -48,6 +49,8 @@ def requires_no_auth():
 
 
 def requires_admin_auth():
+    request_helper.check_proxy_header_before_request()
+
     auth_token = get_auth_token(request)
     client = __get_token_issuer(auth_token)
 
@@ -59,6 +62,8 @@ def requires_admin_auth():
 
 
 def requires_auth():
+    request_helper.check_proxy_header_before_request()
+
     auth_token = get_auth_token(request)
     client = __get_token_issuer(auth_token)
 

app/celery/letters_pdf_tasks.py

@@ -21,6 +21,7 @@ from app.dao.notifications_dao import (
     dao_get_notifications_by_references,
     dao_update_notifications_by_reference,
 )
+from app.errors import VirusScanError
 from app.letters.utils import (
     get_reference_from_filename,
     move_scanned_pdf_to_test_or_live_pdf_bucket,
@@ -162,9 +163,9 @@ def letter_in_created_state(filename):
 
 @notify_celery.task(name='process-virus-scan-passed')
 def process_virus_scan_passed(filename):
-    current_app.logger.info('Virus scan passed: {}'.format(filename))
     reference = get_reference_from_filename(filename)
     notification = dao_get_notification_by_reference(reference)
+    current_app.logger.info('notification id {} Virus scan passed: {}'.format(notification.id, filename))
 
     is_test_key = notification.key_type == KEY_TYPE_TEST
     move_scanned_pdf_to_test_or_live_pdf_bucket(
@@ -179,9 +180,9 @@ def process_virus_scan_passed(filename):
 
 @notify_celery.task(name='process-virus-scan-failed')
 def process_virus_scan_failed(filename):
-    current_app.logger.exception('Virus scan failed: {}'.format(filename))
     move_failed_pdf(filename, ScanErrorType.FAILURE)
     reference = get_reference_from_filename(filename)
+    notification = dao_get_notification_by_reference(reference)
     updated_count = update_letter_pdf_status(reference, NOTIFICATION_VIRUS_SCAN_FAILED)
 
     if updated_count != 1:
@@ -191,12 +192,14 @@ def process_virus_scan_failed(filename):
             )
         )
 
+    raise VirusScanError('notification id {} Virus scan failed: {}'.format(notification.id, filename))
+
 
 @notify_celery.task(name='process-virus-scan-error')
 def process_virus_scan_error(filename):
-    current_app.logger.exception('Virus scan error: {}'.format(filename))
     move_failed_pdf(filename, ScanErrorType.ERROR)
     reference = get_reference_from_filename(filename)
+    notification = dao_get_notification_by_reference(reference)
     updated_count = update_letter_pdf_status(reference, NOTIFICATION_TECHNICAL_FAILURE)
 
     if updated_count != 1:
@@ -206,6 +209,8 @@ def process_virus_scan_error(filename):
             )
         )
 
+    raise VirusScanError('notification id {} Virus scan error: {}'.format(notification.id, filename))
+
 
 def update_letter_pdf_status(reference, status):
     return dao_update_notifications_by_reference(

app/celery/reporting_tasks.py

@@ -84,15 +84,18 @@ def create_nightly_billing(day_start=None):
 
         for data in transit_data:
             update_count = FactBilling.query.filter(
-                FactBilling.bst_date == process_day,
+                FactBilling.bst_date == datetime.date(process_day),
                 FactBilling.template_id == data.template_id,
+                FactBilling.service_id == data.service_id,
                 FactBilling.provider == data.sent_by,  # This could be zero - this is a bug that needs to be fixed.
                 FactBilling.rate_multiplier == data.rate_multiplier,
                 FactBilling.notification_type == data.notification_type,
+                FactBilling.international == data.international
             ).update(
                 {"notifications_sent": data.notifications_sent,
                  "billable_units": data.billable_units},
                 synchronize_session=False)
+
             if update_count == 0:
                 billing_record = FactBilling(
                     bst_date=process_day,

app/errors.py

@@ -10,6 +10,12 @@ from jsonschema import ValidationError as JsonSchemaValidationError
 from app.authentication.auth import AuthError
 
 
+class VirusScanError(Exception):
+    def __init__(self, message):
+
+        super().__init__(message)
+
+
 class InvalidRequest(Exception):
     code = None
     fields = []

app/template/rest.py

@@ -200,7 +200,6 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file
     template = dao_get_template_by_id(notification.template_id)
 
     if template.is_precompiled_letter:
-
         try:
 
             pdf_file = get_letter_pdf(notification)
@@ -215,9 +214,9 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file
         content = base64.b64encode(pdf_file).decode('utf-8')
 
         if file_type == 'png':
-
             try:
-                page_number = page if page else "0"
+                page_number = page if page else "1"
+
                 pdf_page = extract_page_from_pdf(BytesIO(pdf_file), int(page_number) - 1)
                 content = base64.b64encode(pdf_page).decode('utf-8')
             except PdfReadError as e:
@@ -227,12 +226,12 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file
                     status_code=500
                 )
 
-            url = '{}/precompiled-preview.png'.format(
+            url = '{}/precompiled-preview.png{}'.format(
-                current_app.config['TEMPLATE_PREVIEW_API_HOST']
+                current_app.config['TEMPLATE_PREVIEW_API_HOST'],
+                '?hide_notify=true' if page_number == '1' else ''
             )
 
             content = _get_png_preview(url, content, notification.id, json=False)
-
     else:
 
         template_for_letter_print = {
@@ -256,7 +255,6 @@ def preview_letter_template_by_notification_id(service_id, notification_id, file
             file_type,
             '?page={}'.format(page) if page else ''
         )
-
         content = _get_png_preview(url, data, notification.id, json=True)
 
     return jsonify({"content": content})

gunicorn_config.py

@@ -2,7 +2,7 @@ import os
 import sys
 import traceback
 
-workers = 5
+workers = 4
 worker_class = "eventlet"
 worker_connections = 256
 errorlog = "/home/vcap/logs/gunicorn_error.log"

manifest-api-base.yml

@@ -9,7 +9,7 @@ env:
   CW_APP_NAME: api
   # required by cf run-task
   FLASK_APP: application.py
-  SQLALCHEMY_POOL_SIZE: 10
+  SQLALCHEMY_POOL_SIZE: 15
 
   # Credentials variables
   ADMIN_BASE_URL: null

requirements.txt

@@ -23,6 +23,6 @@ notifications-python-client==4.8.1
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@25.2.2#egg=notifications-utils==25.2.2
+git+https://github.com/alphagov/notifications-utils.git@25.2.3#egg=notifications-utils==25.2.3
 
 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3

requirements_for_test.txt

@@ -1,7 +1,7 @@
 -r requirements.txt
 flake8==3.5.0
 moto==1.3.1
-pytest==3.4.2
+pytest==3.5.0
 pytest-env==0.6.2
 pytest-mock==1.7.1
 pytest-cov==2.5.1

tests/app/authentication/test_authentication.py

@@ -325,11 +325,11 @@ def __create_token(service_id):
 
 @pytest.mark.parametrize('check_proxy_header,header_value,expected_status', [
     (True, 'key_1', 200),
-    (True, 'wrong_key', 403),
+    (True, 'wrong_key', 200),
     (False, 'key_1', 200),
     (False, 'wrong_key', 200),
 ])
-def test_route_correct_secret_key(notify_api, check_proxy_header, header_value, expected_status):
+def test_proxy_key_non_auth_endpoint(notify_api, check_proxy_header, header_value, expected_status):
     with set_config_values(notify_api, {
         'ROUTE_SECRET_KEY_1': 'key_1',
         'ROUTE_SECRET_KEY_2': '',
@@ -344,3 +344,29 @@ def test_route_correct_secret_key(notify_api, check_proxy_header, header_value,
                 ]
             )
         assert response.status_code == expected_status
+
+
+@pytest.mark.parametrize('check_proxy_header,header_value,expected_status', [
+    (True, 'key_1', 200),
+    (True, 'wrong_key', 403),
+    (False, 'key_1', 200),
+    (False, 'wrong_key', 200),
+])
+def test_proxy_key_on_admin_auth_endpoint(notify_api, check_proxy_header, header_value, expected_status):
+    token = create_jwt_token(current_app.config['ADMIN_CLIENT_SECRET'], current_app.config['ADMIN_CLIENT_USER_NAME'])
+
+    with set_config_values(notify_api, {
+        'ROUTE_SECRET_KEY_1': 'key_1',
+        'ROUTE_SECRET_KEY_2': '',
+        'CHECK_PROXY_HEADER': check_proxy_header,
+    }):
+
+        with notify_api.test_client() as client:
+            response = client.get(
+                path='/service',
+                headers=[
+                    ('X-Custom-Forwarder', header_value),
+                    ('Authorization', 'Bearer {}'.format(token))
+                ]
+            )
+        assert response.status_code == expected_status

tests/app/celery/test_letters_pdf_tasks.py

@@ -10,6 +10,7 @@ from celery.exceptions import MaxRetriesExceededError
 from requests import RequestException
 from sqlalchemy.orm.exc import NoResultFound
 
+from app.errors import VirusScanError
 from app.variables import Retention
 from app.celery.letters_pdf_tasks import (
     create_letters_pdf,
@@ -343,8 +344,10 @@ def test_process_letter_task_check_virus_scan_failed(sample_letter_notification,
     sample_letter_notification.status = 'pending-virus-check'
     mock_move_failed_pdf = mocker.patch('app.celery.letters_pdf_tasks.move_failed_pdf')
 
-    process_virus_scan_failed(filename)
+    with pytest.raises(VirusScanError) as e:
+        process_virus_scan_failed(filename)
 
+    assert "Virus scan failed:" in str(e)
     mock_move_failed_pdf.assert_called_once_with(filename, ScanErrorType.FAILURE)
     assert sample_letter_notification.status == NOTIFICATION_VIRUS_SCAN_FAILED
 
@@ -354,7 +357,9 @@ def test_process_letter_task_check_virus_scan_error(sample_letter_notification,
     sample_letter_notification.status = 'pending-virus-check'
     mock_move_failed_pdf = mocker.patch('app.celery.letters_pdf_tasks.move_failed_pdf')
 
-    process_virus_scan_error(filename)
+    with pytest.raises(VirusScanError) as e:
+        process_virus_scan_error(filename)
 
+    assert "Virus scan error:" in str(e)
     mock_move_failed_pdf.assert_called_once_with(filename, ScanErrorType.ERROR)
     assert sample_letter_notification.status == NOTIFICATION_TECHNICAL_FAILURE

tests/app/celery/test_reporting_tasks.py

@@ -404,3 +404,40 @@ def test_create_nightly_billing_use_BST(
     assert len(records) == 2
     assert records[0].bst_date == date(2018, 3, 25)
     assert records[-1].bst_date == date(2018, 3, 26)
+
+
+@freeze_time('2018-01-15T03:30:00')
+def test_create_nightly_billing_update_when_record_exists(
+        notify_db,
+        notify_db_session,
+        sample_service,
+        sample_template,
+        mocker):
+
+    mocker.patch('app.celery.reporting_tasks.get_rate', side_effect=mocker_get_rate)
+
+    sample_notification(
+        notify_db,
+        notify_db_session,
+        created_at=datetime.now() - timedelta(days=1),
+        service=sample_service,
+        template=sample_template,
+        status='delivered',
+        sent_by=None,
+        international=False,
+        rate_multiplier=1.0,
+        billable_units=1,
+    )
+
+    records = FactBilling.query.all()
+    assert len(records) == 0
+
+    create_nightly_billing()
+    records = FactBilling.query.order_by(FactBilling.bst_date).all()
+
+    assert len(records) == 1
+    assert records[0].bst_date == date(2018, 1, 14)
+
+    # run again, make sure create_nightly_billing() updates with no error
+    create_nightly_billing()
+    assert len(records) == 1

tests/app/letters/test_letter_utils.py

@@ -169,37 +169,33 @@ def test_move_scanned_letter_pdf_to_processing_bucket(
 @freeze_time(FROZEN_DATE_TIME)
 def test_move_failed_pdf_error(notify_api):
     filename = 'test.pdf'
-    source_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
+    bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
-    target_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
 
     conn = boto3.resource('s3', region_name='eu-west-1')
-    source_bucket = conn.create_bucket(Bucket=source_bucket_name)
+    bucket = conn.create_bucket(Bucket=bucket_name)
-    target_bucket = conn.create_bucket(Bucket=target_bucket_name)
 
     s3 = boto3.client('s3', region_name='eu-west-1')
-    s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'pdf_content')
+    s3.put_object(Bucket=bucket_name, Key=filename, Body=b'pdf_content')
 
     move_failed_pdf(filename, ScanErrorType.ERROR)
 
-    assert 'ERROR/' + filename in [o.key for o in target_bucket.objects.all()]
+    assert 'ERROR/' + filename in [o.key for o in bucket.objects.all()]
-    assert filename not in [o.key for o in source_bucket.objects.all()]
+    assert filename not in [o.key for o in bucket.objects.all()]
 
 
 @mock_s3
 @freeze_time(FROZEN_DATE_TIME)
 def test_move_failed_pdf_scan_failed(notify_api):
     filename = 'test.pdf'
-    source_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
+    bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
-    target_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
 
     conn = boto3.resource('s3', region_name='eu-west-1')
-    source_bucket = conn.create_bucket(Bucket=source_bucket_name)
+    bucket = conn.create_bucket(Bucket=bucket_name)
-    target_bucket = conn.create_bucket(Bucket=target_bucket_name)
 
     s3 = boto3.client('s3', region_name='eu-west-1')
-    s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'pdf_content')
+    s3.put_object(Bucket=bucket_name, Key=filename, Body=b'pdf_content')
 
     move_failed_pdf(filename, ScanErrorType.FAILURE)
 
-    assert 'FAILURE/' + filename in [o.key for o in target_bucket.objects.all()]
+    assert 'FAILURE/' + filename in [o.key for o in bucket.objects.all()]
-    assert filename not in [o.key for o in source_bucket.objects.all()]
+    assert filename not in [o.key for o in bucket.objects.all()]

tests/app/template/test_rest.py

@@ -1008,6 +1008,54 @@ def test_preview_letter_template_precompiled_png_file_type(
             assert base64.b64decode(resp['content']) == png_content
 
 
+@pytest.mark.parametrize('page_number,expect_preview_url', [
+    ('', 'http://localhost/notifications-template-preview/precompiled-preview.png?hide_notify=true'),
+    ('1', 'http://localhost/notifications-template-preview/precompiled-preview.png?hide_notify=true'),
+    ('2', 'http://localhost/notifications-template-preview/precompiled-preview.png')
+])
+def test_preview_letter_template_precompiled_png_file_type_hide_notify_tag_only_on_first_page(
+        notify_api,
+        client,
+        admin_request,
+        sample_service,
+        mocker,
+        page_number,
+        expect_preview_url
+):
+
+    template = create_template(sample_service,
+                               template_type='letter',
+                               template_name='Pre-compiled PDF',
+                               subject='Pre-compiled PDF',
+                               hidden=True)
+
+    notification = create_notification(template)
+
+    with set_config_values(notify_api, {
+        'TEMPLATE_PREVIEW_API_HOST': 'http://localhost/notifications-template-preview',
+        'TEMPLATE_PREVIEW_API_KEY': 'test-key'
+    }):
+        pdf_content = b'\x00\x01'
+        png_content = b'\x00\x02'
+        encoded = base64.b64encode(png_content).decode('utf-8')
+
+        mocker.patch('app.template.rest.get_letter_pdf', return_value=pdf_content)
+        mocker.patch('app.template.rest.extract_page_from_pdf', return_value=png_content)
+        mock_get_png_preview = mocker.patch('app.template.rest._get_png_preview', return_value=encoded)
+
+        admin_request.get(
+            'template.preview_letter_template_by_notification_id',
+            service_id=notification.service_id,
+            notification_id=notification.id,
+            file_type='png',
+            page=page_number
+        )
+
+        mock_get_png_preview.assert_called_once_with(
+            expect_preview_url, encoded, notification.id, json=False
+        )
+
+
 def test_preview_letter_template_precompiled_png_template_preview_500_error(
         notify_api,
         client,

tests/conftest.py

@@ -141,8 +141,10 @@ def pytest_generate_tests(metafunc):
 def set_config(app, name, value):
     old_val = app.config.get(name)
     app.config[name] = value
-    yield
+    try:
-    app.config[name] = old_val
+        yield
+    finally:
+        app.config[name] = old_val
 
 
 @contextmanager
@@ -153,7 +155,8 @@ def set_config_values(app, dict):
         old_values[key] = app.config.get(key)
         app.config[key] = dict[key]
 
-    yield
+    try:
-
+        yield
-    for key in dict:
+    finally:
-        app.config[key] = old_values[key]
+        for key in dict:
+            app.config[key] = old_values[key]