Add before_request check for request size

The test checks various routes to ensure that request above our
threshold fail before any other processing happens

app/__init__.py

@@ -19,6 +19,7 @@ from notifications_utils.clients.encryption.encryption_client import Encryption
 from notifications_utils import logging, request_helper
 from sqlalchemy import event
 from werkzeug.exceptions import HTTPException as WerkzeugHTTPException
+from werkzeug.exceptions import RequestEntityTooLarge as WerkzeugRequestEntityTooLarge
 from werkzeug.local import LocalProxy
 
 from app.celery.celery import NotifyCelery
@@ -281,6 +282,15 @@ def init_app(app):
         g.start = monotonic()
         g.endpoint = request.endpoint
 
+    @app.before_request
+    def check_content_length():
+        if (
+            request.content_length is not None
+            and current_app.config['MAX_CONTENT_LENGTH'] is not None
+            and request.content_length > current_app.config['MAX_CONTENT_LENGTH']
+        ):
+            raise WerkzeugRequestEntityTooLarge()
+
     @app.after_request
     def after_request(response):
         CONCURRENT_REQUESTS.dec()

app/broadcast_message/rest.py

@@ -121,6 +121,7 @@ def create_broadcast_message(service_id):
         created_by_id=user.id,
         content=content,
         reference=reference,
+        stubbed=service.restricted
     )
 
     dao_save_object(broadcast_message)
@@ -214,7 +215,8 @@ def _create_broadcast_event(broadcast_message):
 
     dao_save_object(event)
 
-    send_broadcast_event.apply_async(
-        kwargs={'broadcast_event_id': str(event.id)},
-        queue=QueueNames.BROADCASTS
-    )
+    if not broadcast_message.stubbed:
+        send_broadcast_event.apply_async(
+            kwargs={'broadcast_event_id': str(event.id)},
+            queue=QueueNames.BROADCASTS
+        )

app/clients/cbc_proxy.py

@@ -174,8 +174,8 @@ class CBCProxyCanary(CBCProxyClientBase):
 
 
 class CBCProxyEE(CBCProxyClientBase):
-    lambda_name = 'bt-ee-1-proxy'
-    failover_lambda_name = 'bt-ee-2-proxy'
+    lambda_name = 'ee-1-proxy'
+    failover_lambda_name = 'ee-2-proxy'
 
     LANGUAGE_ENGLISH = 'en-GB'
     LANGUAGE_WELSH = 'cy-GB'

app/clients/email/aws_ses.py

@@ -62,7 +62,6 @@ class AwsSesClient(EmailClient):
         # before-call, after-call, after-call-error, request-created, response-received
         self._client.meta.events.register('request-created.ses.SendEmail', self.ses_request_created_hook)
         self._client.meta.events.register('response-received.ses.SendEmail', self.ses_response_received_hook)
-        self._client.meta.events.register('before-call.ses', self.ses_inject_connection_header)
 
     def ses_request_created_hook(self, **kwargs):
         # request created may be called multiple times if the request auto-retries. We want to count all these as the
@@ -78,11 +77,6 @@ class AwsSesClient(EmailClient):
     def get_name(self):
         return self.name
 
-    def ses_inject_connection_header(self, params, **kwargs):
-        # keep underlying TLS connection open, so we do not spend lots of CPU
-        # and network time renegotiating TLS
-        params['headers']['Connection'] = 'Keep-Alive'
-
     def send_email(self,
                    source,
                    to_addresses,
@@ -154,4 +148,3 @@ def punycode_encode_email(email_address):
     # only the hostname should ever be punycode encoded.
     local, hostname = email_address.split('@')
     return '{}@{}'.format(local, hostname.encode('idna').decode('utf-8'))
-

app/clients/sms/firetext.py

@@ -2,8 +2,7 @@ import json
 import logging
 
 from time import monotonic
-from requests import request, RequestException, Session
-from requests.adapters import HTTPAdapter
+from requests import request, RequestException
 
 from app.clients.sms import (SmsClient, SmsClientResponseException)
 
@@ -70,9 +69,6 @@ class FiretextClient(SmsClient):
         self.name = 'firetext'
         self.url = current_app.config.get('FIRETEXT_URL')
         self.statsd_client = statsd_client
-        # this uses urllib3 under the hood to create a connection pool
-        self.session = Session()
-        self.session.mount('https://', HTTPAdapter(pool_maxsize=32))
 
     def get_name(self):
         return self.name
@@ -107,7 +103,8 @@ class FiretextClient(SmsClient):
         response = None
         start_time = monotonic()
         try:
-            response = self.session.post(
+            response = request(
+                "POST",
                 self.url,
                 data=data,
                 timeout=60

app/clients/sms/mmg.py

@@ -1,8 +1,6 @@
 import json
 from time import monotonic
-from requests import (request, RequestException, Session)
-from requests.adapters import HTTPAdapter
-
+from requests import (request, RequestException)
 from app.clients.sms import (SmsClient, SmsClientResponseException)
 
 mmg_response_map = {
@@ -77,9 +75,6 @@ class MMGClient(SmsClient):
         self.name = 'mmg'
         self.statsd_client = statsd_client
         self.mmg_url = current_app.config.get('MMG_URL')
-        # this uses urllib3 under the hood to create a connection pool
-        self.session = Session()
-        self.session.mount('https://', HTTPAdapter(pool_maxsize=32))
 
     def record_outcome(self, success, response):
         status_code = response.status_code if response else 503
@@ -113,8 +108,8 @@ class MMGClient(SmsClient):
         response = None
         start_time = monotonic()
         try:
-
-            response = self.session.post(
+            response = request(
+                "POST",
                 self.mmg_url,
                 data=json.dumps(data),
                 headers={

app/config.py

@@ -381,6 +381,7 @@ class Config(object):
     CBC_PROXY_ENABLED = bool(CBC_PROXY_AWS_ACCESS_KEY_ID)
 
     ENABLED_CBCS = {BroadcastProvider.EE, BroadcastProvider.THREE, BroadcastProvider.O2, BroadcastProvider.VODAFONE}
+    MAX_CONTENT_LENGTH = 5 * 1024 * 1024  # 5MB
 
 
 ######################
@@ -508,6 +509,8 @@ class Staging(Config):
     API_RATE_LIMIT_ENABLED = True
     CHECK_PROXY_HEADER = True
 
+    MAX_CONTENT_LENGTH = None
+
 
 class Live(Config):
     NOTIFY_EMAIL_DOMAIN = 'notifications.service.gov.uk'
@@ -529,6 +532,8 @@ class Live(Config):
 
     CRONITOR_ENABLED = True
 
+    MAX_CONTENT_LENGTH = None
+
 
 class CloudFoundryConfig(Config):
     pass

app/models.py

@@ -488,6 +488,10 @@ class Service(db.Model, Versioned):
     organisation = db.relationship('Organisation', backref='services')
 
     notes = db.Column(db.Text, nullable=True)
+    purchase_order_number = db.Column(db.String(255), nullable=True)
+    billing_contact_names = db.Column(db.Text, nullable=True)
+    billing_contact_email_addresses = db.Column(db.Text, nullable=True)
+    billing_reference = db.Column(db.String(255), nullable=True)
 
     email_branding = db.relationship(
         'EmailBranding',
@@ -2265,6 +2269,8 @@ class BroadcastMessage(db.Model):
 
     reference = db.Column(db.String(255), nullable=True)
 
+    stubbed = db.Column(db.Boolean, nullable=True)
+
     CheckConstraint("created_by_id is not null or api_key_id is not null")
 
     @property

manifest.yml.j2

@@ -53,11 +53,6 @@
   'notify-delivery-worker-jobs': {},
   'notify-delivery-worker-research': {},
   'notify-delivery-worker-sender': {'disk_quota': '2G', 'memory': '4G'},
-  'notify-delivery-worker-sender-canary': {'disk_quota': '2G', 'memory': '4G',  'instances': {
-      'preview': 0,
-      'staging': 1,
-      'production': 0
-    },},
   'notify-delivery-worker-periodic': {},
   'notify-delivery-worker-reporting': {
     'additional_env_vars': {

migrations/versions/0339_service_billing_details.py

@@ -0,0 +1,39 @@
+"""
+
+Revision ID: 0339_service_billing_details
+Revises: 0338_add_notes_to_service
+Create Date: 2021-01-20 17:55:46.555460
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+revision = '0339_service_billing_details'
+down_revision = '0338_add_notes_to_service'
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('services', sa.Column('billing_contact_email_addresses', sa.Text(), nullable=True))
+    op.add_column('services', sa.Column('billing_contact_names', sa.Text(), nullable=True))
+    op.add_column('services', sa.Column('billing_reference', sa.String(length=255), nullable=True))
+    op.add_column('services', sa.Column('purchase_order_number', sa.String(length=255), nullable=True))
+    op.add_column('services_history', sa.Column('billing_contact_email_addresses', sa.Text(), nullable=True))
+    op.add_column('services_history', sa.Column('billing_contact_names', sa.Text(), nullable=True))
+    op.add_column('services_history', sa.Column('billing_reference', sa.String(length=255), nullable=True))
+    op.add_column('services_history', sa.Column('purchase_order_number', sa.String(length=255), nullable=True))
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column('services_history', 'purchase_order_number')
+    op.drop_column('services_history', 'billing_reference')
+    op.drop_column('services_history', 'billing_contact_names')
+    op.drop_column('services_history', 'billing_contact_email_addresses')
+    op.drop_column('services', 'purchase_order_number')
+    op.drop_column('services', 'billing_reference')
+    op.drop_column('services', 'billing_contact_names')
+    op.drop_column('services', 'billing_contact_email_addresses')
+    # ### end Alembic commands ###

migrations/versions/0340_stub_training_broadcasts.py

@@ -0,0 +1,25 @@
+"""
+
+Revision ID: 0340_stub_training_broadcasts
+Revises: 0339_service_billing_details
+Create Date: 2021-01-26 16:48:44.921065
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+revision = '0340_stub_training_broadcasts'
+down_revision = '0339_service_billing_details'
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('broadcast_message', sa.Column('stubbed', sa.Boolean(), nullable=True))
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.drop_column('broadcast_message', 'stubbed')
+    # ### end Alembic commands ###

scripts/paas_app_wrapper.sh

@@ -24,10 +24,6 @@ case $NOTIFY_APP_NAME in
     exec scripts/run_multi_worker_app_paas.sh celery multi start 3 -c 10 -A run_celery.notify_celery --loglevel=INFO \
     --logfile=/dev/null --pidfile=/tmp/celery%N.pid -Q send-sms-tasks,send-email-tasks
     ;;
-    delivery-worker-sender-canary)
-    exec scripts/run_multi_worker_app_paas.sh celery multi start 3 -c 10 -A run_celery.notify_celery --loglevel=INFO \
-    --logfile=/dev/null --pidfile=/tmp/celery%N.pid -Q send-sms-tasks,send-email-tasks
-    ;;
   delivery-worker-periodic)
     exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=2 \
     -Q periodic-tasks 2> /dev/null

tests/app/broadcast_message/test_rest.py

@@ -4,6 +4,7 @@ import uuid
 from freezegun import freeze_time
 import pytest
 
+from app.dao.broadcast_message_dao import dao_get_broadcast_message_by_id_and_service_id
 from app.models import BROADCAST_TYPE, BroadcastStatusType, BroadcastEventMessageType
 
 from tests.app.db import create_broadcast_message, create_template, create_service, create_user
@@ -117,7 +118,9 @@ def test_get_broadcast_messages_for_service(admin_request, sample_broadcast_serv
 
 
 @freeze_time('2020-01-01')
-def test_create_broadcast_message(admin_request, sample_broadcast_service):
+@pytest.mark.parametrize('training_mode_service', [True, False])
+def test_create_broadcast_message(admin_request, sample_broadcast_service, training_mode_service):
+    sample_broadcast_service.restricted = training_mode_service
     t = create_template(sample_broadcast_service, BROADCAST_TYPE)
 
     response = admin_request.post(
@@ -138,6 +141,8 @@ def test_create_broadcast_message(admin_request, sample_broadcast_service):
     assert response['personalisation'] == {}
     assert response['areas'] == []
 
+    broadcast_message = dao_get_broadcast_message_by_id_and_service_id(response["id"], sample_broadcast_service.id)
+    assert broadcast_message.stubbed == training_mode_service
 
 @pytest.mark.parametrize('data, expected_errors', [
     (
@@ -546,6 +551,47 @@ def test_update_broadcast_message_status_stores_approved_by_and_approved_at_and_
     assert alert_event.transmitted_content == {"body": "emergency broadcast"}
 
 
+def test_update_broadcast_message_status_updates_details_but_does_not_queue_task_for_stubbed_broadcast_message(
+    admin_request,
+    sample_broadcast_service,
+    mocker
+):
+    sample_broadcast_service.restricted = True
+    t = create_template(sample_broadcast_service, BROADCAST_TYPE, content='emergency broadcast')
+    bm = create_broadcast_message(
+        t,
+        status=BroadcastStatusType.PENDING_APPROVAL,
+        areas={"areas": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]},
+        stubbed=True
+    )
+    approver = create_user(email='approver@gov.uk')
+    sample_broadcast_service.users.append(approver)
+    mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
+
+    response = admin_request.post(
+        'broadcast_message.update_broadcast_message_status',
+        _data={'status': BroadcastStatusType.BROADCASTING, 'created_by': str(approver.id)},
+        service_id=t.service_id,
+        broadcast_message_id=bm.id,
+        _expected_status=200
+    )
+
+    assert response['status'] == BroadcastStatusType.BROADCASTING
+    assert response['approved_at'] is not None
+    assert response['approved_by_id'] == str(approver.id)
+
+    assert len(bm.events) == 1
+    alert_event = bm.events[0]
+
+    assert len(mock_task.mock_calls) == 0
+
+    assert alert_event.service_id == sample_broadcast_service.id
+    assert alert_event.transmitted_areas == bm.areas
+    assert alert_event.message_type == BroadcastEventMessageType.ALERT
+    assert alert_event.transmitted_finishes_at == bm.finishes_at
+    assert alert_event.transmitted_content == {"body": "emergency broadcast"}
+
+
 def test_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_template(
     admin_request,
     sample_broadcast_service,

tests/app/clients/test_cbc_proxy.py

@@ -78,7 +78,7 @@ def test_cbc_proxy_lambda_client_has_correct_keys(cbc_proxy_ee):
     ('my-description', 'en-GB'),
     ('mŷ-description', 'cy-GB'),
 ))
-@pytest.mark.parametrize('cbc', ['bt-ee', 'three', 'o2'])
+@pytest.mark.parametrize('cbc', ['ee', 'three', 'o2'])
 def test_cbc_proxy_one_2_many_create_and_send_invokes_function(
     mocker,
     cbc_proxy_client,
@@ -86,7 +86,7 @@ def test_cbc_proxy_one_2_many_create_and_send_invokes_function(
     cbc,
     expected_language,
 ):
-    cbc_proxy = cbc_proxy_client.get_proxy('ee') if cbc == 'bt-ee' else cbc_proxy_client.get_proxy(cbc)
+    cbc_proxy = cbc_proxy_client.get_proxy(cbc)
 
     identifier = 'my-identifier'
     headline = 'my-headline'
@@ -135,9 +135,9 @@ def test_cbc_proxy_one_2_many_create_and_send_invokes_function(
     assert payload['language'] == expected_language
 
 
-@pytest.mark.parametrize('cbc', ['bt-ee', 'three', 'o2'])
+@pytest.mark.parametrize('cbc', ['ee', 'three', 'o2'])
 def test_cbc_proxy_one_2_many_cancel_invokes_function(mocker, cbc_proxy_client, cbc):
-    cbc_proxy = cbc_proxy_client.get_proxy('ee') if cbc == 'bt-ee' else cbc_proxy_client.get_proxy(cbc)
+    cbc_proxy = cbc_proxy_client.get_proxy(cbc)
 
     identifier = 'my-identifier'
     MockProviderMessage = namedtuple(
@@ -310,13 +310,13 @@ def test_cbc_proxy_vodafone_cancel_invokes_function(mocker, cbc_proxy_vodafone):
     assert payload['sent'] == sent
 
 
-@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone', 'three', 'o2'])
+@pytest.mark.parametrize('cbc', ['ee', 'vodafone', 'three', 'o2'])
 def test_cbc_proxy_will_failover_to_second_lambda_if_function_error(
     mocker,
     cbc_proxy_client,
     cbc
 ):
-    cbc_proxy = cbc_proxy_client.get_proxy('ee') if cbc == 'bt-ee' else cbc_proxy_client.get_proxy(cbc)
+    cbc_proxy = cbc_proxy_client.get_proxy(cbc)
 
     ld_client_mock = mocker.patch.object(
         cbc_proxy,
@@ -362,13 +362,13 @@ def test_cbc_proxy_will_failover_to_second_lambda_if_function_error(
     ]
 
 
-@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone', 'three', 'o2'])
+@pytest.mark.parametrize('cbc', ['ee', 'vodafone', 'three', 'o2'])
 def test_cbc_proxy_will_failover_to_second_lambda_if_invoke_error(
     mocker,
     cbc_proxy_client,
     cbc
 ):
-    cbc_proxy = cbc_proxy_client.get_proxy('ee') if cbc == 'bt-ee' else cbc_proxy_client.get_proxy(cbc)
+    cbc_proxy = cbc_proxy_client.get_proxy(cbc)
 
     ld_client_mock = mocker.patch.object(
         cbc_proxy,
@@ -409,11 +409,11 @@ def test_cbc_proxy_will_failover_to_second_lambda_if_invoke_error(
     ]
 
 
-@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone', 'three', 'o2'])
+@pytest.mark.parametrize('cbc', ['ee', 'vodafone', 'three', 'o2'])
 def test_cbc_proxy_create_and_send_tries_failover_lambda_on_invoke_error_and_raises_if_both_invoke_error(
     mocker, cbc_proxy_client, cbc
 ):
-    cbc_proxy = cbc_proxy_client.get_proxy('ee') if cbc == 'bt-ee' else cbc_proxy_client.get_proxy(cbc)
+    cbc_proxy = cbc_proxy_client.get_proxy(cbc)
 
     ld_client_mock = mocker.patch.object(
         cbc_proxy,
@@ -452,11 +452,11 @@ def test_cbc_proxy_create_and_send_tries_failover_lambda_on_invoke_error_and_rai
     ]
 
 
-@pytest.mark.parametrize('cbc', ['bt-ee', 'vodafone', 'three', 'o2'])
+@pytest.mark.parametrize('cbc', ['ee', 'vodafone', 'three', 'o2'])
 def test_cbc_proxy_create_and_send_tries_failover_lambda_on_function_error_and_raises_if_both_function_error(
     mocker, cbc_proxy_client, cbc
 ):
-    cbc_proxy = cbc_proxy_client.get_proxy('ee') if cbc == 'bt-ee' else cbc_proxy_client.get_proxy(cbc)
+    cbc_proxy = cbc_proxy_client.get_proxy(cbc)
 
     ld_client_mock = mocker.patch.object(
         cbc_proxy,
@@ -532,9 +532,9 @@ def test_cbc_proxy_send_canary_invokes_function(mocker, cbc_proxy_client):
     assert payload['identifier'] == identifier
 
 
-@pytest.mark.parametrize('cbc', ['bt-ee', 'three', 'o2'])
+@pytest.mark.parametrize('cbc', ['ee', 'three', 'o2'])
 def test_cbc_proxy_one_2_many_send_link_test_invokes_function(mocker, cbc_proxy_client, cbc):
-    cbc_proxy = cbc_proxy_client.get_proxy('ee') if cbc == 'bt-ee' else cbc_proxy_client.get_proxy(cbc)
+    cbc_proxy = cbc_proxy_client.get_proxy(cbc)
 
     identifier = str(uuid.uuid4())
 

tests/app/db.py

@@ -1022,6 +1022,7 @@ def create_broadcast_message(
     starts_at=None,
     finishes_at=None,
     areas=None,
+    stubbed=False
 ):
     if template:
         service = template.service
@@ -1049,7 +1050,8 @@ def create_broadcast_message(
         finishes_at=finishes_at,
         created_by_id=created_by.id if created_by else service.created_by_id,
         areas=areas or {},
-        content=content
+        content=content,
+        stubbed=stubbed
     )
     db.session.add(broadcast_message)
     db.session.commit()

tests/app/service/test_rest.py

@@ -248,6 +248,9 @@ def test_get_service_by_id(admin_request, sample_service):
     assert set(json_resp['data'].keys()) == {
         'active',
         'allowed_broadcast_provider',
+        'billing_contact_email_addresses',
+        'billing_contact_names',
+        'billing_reference',
         'consent_to_research',
         'contact_link',
         'count_as_live',
@@ -266,6 +269,7 @@ def test_get_service_by_id(admin_request, sample_service):
         'organisation_type',
         'permissions',
         'prefix_sms',
+        'purchase_order_number',
         'rate_limit',
         'research_mode',
         'restricted',

tests/app/test_request_size.py

@@ -0,0 +1,36 @@
+import pytest
+import json
+
+@pytest.mark.parametrize('endpoint, max_content_length, expected_status_code', [
+    ("/_status", 5*1024*1024, 413),
+    ("/provider-details", 5*1024*1024, 413),
+    ("/v2/notifications/email", 5*1024*1024, 413),
+
+    ("/_status", None, 200),
+    ("/provider-details", None, 405),
+    ("/v2/notifications/email", None, 401),
+])
+def test_request_status_when_content_length_is_set(
+    notify_api,
+    sample_email_template_with_placeholders,
+    mocker,
+    endpoint,
+    max_content_length,
+    expected_status_code):
+
+    notify_api.config['MAX_CONTENT_LENGTH'] = max_content_length
+    large_name = "J" * (max_content_length or 1 + 1)
+    data = {
+        'email_address': 'ok@ok.com',
+        'template_id': str(sample_email_template_with_placeholders.id),
+        'personalisation': {
+            'name': large_name
+        }
+    }
+    with notify_api.test_client() as client:
+        response = client.post(
+            path=endpoint,
+            data=json.dumps(data),
+            headers=[('Content-Type', 'application/json')])
+
+        assert response.status_code == expected_status_code