From 0bb671df456f2a6a37ffdd97eb024690d7bc112d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 12 Feb 2021 17:36:25 +0000 Subject: [PATCH] Validate content length on broadcast API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The maximum content count of a broadcast varies depending on its encoding, so we can’t simply validate it against a schema. This commit moves to using the validation from `notifications-utils`, and raising a custom error response. --- app/v2/broadcast/broadcast_schemas.py | 1 - app/v2/broadcast/post_broadcast.py | 19 +++++++++- requirements-app.txt | 2 +- requirements.txt | 8 ++-- .../v2/broadcast/sample_cap_xml_documents.py | 37 +++++++++++++++++++ tests/app/v2/broadcast/test_post_broadcast.py | 31 ++++++++++++++++ 6 files changed, 91 insertions(+), 7 deletions(-) diff --git a/app/v2/broadcast/broadcast_schemas.py b/app/v2/broadcast/broadcast_schemas.py index 83fc65607..67886507e 100644 --- a/app/v2/broadcast/broadcast_schemas.py +++ b/app/v2/broadcast/broadcast_schemas.py @@ -40,7 +40,6 @@ post_broadcast_schema = { "content": { "type": "string", "minLength": 1, - "maxLength": 1395, }, "web": { "type": "string", diff --git a/app/v2/broadcast/post_broadcast.py b/app/v2/broadcast/post_broadcast.py index 6833babde..fd8e00b2c 100644 --- a/app/v2/broadcast/post_broadcast.py +++ b/app/v2/broadcast/post_broadcast.py @@ -1,6 +1,7 @@ from itertools import chain from flask import current_app, jsonify, request from notifications_utils.polygons import Polygons +from notifications_utils.template import BroadcastMessageTemplate from app import authenticated_service, api_user from app.broadcast_message.translators import cap_xml_to_dict from app.dao.dao_utils import dao_save_object @@ -9,7 +10,7 @@ from app.models import BROADCAST_TYPE, BroadcastMessage, BroadcastStatusType from app.schema_validation import validate from app.v2.broadcast import v2_broadcast_blueprint from app.v2.broadcast.broadcast_schemas import post_broadcast_schema -from app.v2.errors import BadRequestError +from app.v2.errors import BadRequestError, ValidationError from app.xml_schemas import validate_xml @@ -43,6 +44,22 @@ def create_broadcast(): area['polygons'] for area in broadcast_json['areas'] )))) + template = BroadcastMessageTemplate.from_content( + broadcast_json['content'] + ) + + if template.content_too_long: + raise ValidationError( + message=( + f'description must be {template.max_content_count:,.0f} ' + f'characters or fewer' + ) + ( + ' (because it could not be GSM7 encoded)' + if template.non_gsm_characters else '' + ), + status_code=400, + ) + broadcast_message = BroadcastMessage( service_id=authenticated_service.id, content=broadcast_json['content'], diff --git a/requirements-app.txt b/requirements-app.txt index b358e3c3c..8d99c0d8f 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -33,7 +33,7 @@ notifications-python-client==5.7.1 # PaaS awscli-cwlogs==1.4.6 -git+https://github.com/alphagov/notifications-utils.git@43.8.3#egg=notifications-utils==43.8.3 +git+https://github.com/alphagov/notifications-utils.git@43.9.0#egg=notifications-utils==43.9.0 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains prometheus-client==0.9.0 diff --git a/requirements.txt b/requirements.txt index 9ec6beb50..41242987a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -35,7 +35,7 @@ notifications-python-client==5.7.1 # PaaS awscli-cwlogs==1.4.6 -git+https://github.com/alphagov/notifications-utils.git@43.8.3#egg=notifications-utils==43.8.3 +git+https://github.com/alphagov/notifications-utils.git@43.9.0#egg=notifications-utils==43.9.0 # gds-metrics requires prometheseus 0.2.0, override that requirement as 0.7.1 brings significant performance gains prometheus-client==0.9.0 @@ -46,14 +46,14 @@ alembic==1.5.4 amqp==1.4.9 anyjson==0.3.3 attrs==20.3.0 -awscli==1.19.5 +awscli==1.19.8 bcrypt==3.2.0 billiard==3.3.0.23 bleach==3.3.0 blinker==1.4 boto==2.49.0 -boto3==1.17.5 -botocore==1.20.5 +boto3==1.17.8 +botocore==1.20.8 certifi==2020.12.5 chardet==4.0.0 click==7.1.2 diff --git a/tests/app/v2/broadcast/sample_cap_xml_documents.py b/tests/app/v2/broadcast/sample_cap_xml_documents.py index 51244e379..a8a0b8cab 100644 --- a/tests/app/v2/broadcast/sample_cap_xml_documents.py +++ b/tests/app/v2/broadcast/sample_cap_xml_documents.py @@ -198,3 +198,40 @@ CANCEL = """ """ + +WITH_PLACEHOLDER_FOR_CONTENT = """ + + 50385fcb0ab7aa447bbd46d848ce8466E + www.gov.uk/environment-agency + 2020-02-16T23:01:13-00:00 + Actual + Alert + Flood warning service + Public + www.gov.uk/environment-agency,4f6d28b10ab7aa447bbd46d85f1e9effE,2020-02-16T19:20:03+00:00 + + en-GB + Met + 053/055 Issue Severe Flood Warning EA + Immediate + Severe + Likely + 2020-02-26T23:01:14-00:00 + Environment Agency + {} + https://flood-warning-information.service.gov.uk + 0345 988 1188 + + River Steeping in Wainfleet All Saints + 53.10569,0.24453 53.10593,0.24430 53.10601,0.24375 53.10615,0.24349 53.10629,0.24356 53.10656,0.24336 53.10697,0.24354 53.10684,0.24298 53.10694,0.24264 53.10721,0.24302 53.10752,0.24310 53.10777,0.24308 53.10805,0.24320 53.10803,0.24187 53.10776,0.24085 53.10774,0.24062 53.10702,0.24056 53.10679,0.24088 53.10658,0.24071 53.10651,0.24049 53.10656,0.24022 53.10642,0.24022 53.10632,0.24052 53.10629,0.24082 53.10612,0.24093 53.10583,0.24133 53.10564,0.24178 53.10541,0.24282 53.10569,0.24453 + + TargetAreaCode + 053FWFSTEEP4 + + + + +""" + +LONG_GSM7 = WITH_PLACEHOLDER_FOR_CONTENT.format('a' * 1396) +LONG_UCS2 = WITH_PLACEHOLDER_FOR_CONTENT.format('ŵ' * 616) diff --git a/tests/app/v2/broadcast/test_post_broadcast.py b/tests/app/v2/broadcast/test_post_broadcast.py index 8cd0256d5..0b9de61a4 100644 --- a/tests/app/v2/broadcast/test_post_broadcast.py +++ b/tests/app/v2/broadcast/test_post_broadcast.py @@ -191,3 +191,34 @@ def test_unsupported_message_types_400( } in ( json.loads(response.get_data(as_text=True))['errors'] ) + + +@pytest.mark.parametrize('xml_document, expected_error', ( + (sample_cap_xml_documents.LONG_UCS2, ( + 'description must be 615 characters or fewer (because it ' + 'could not be GSM7 encoded)' + )), + (sample_cap_xml_documents.LONG_GSM7, ( + 'description must be 1,395 characters or fewer' + )), +)) +def test_content_too_long_returns_400( + client, + sample_broadcast_service, + xml_document, + expected_error, +): + auth_header = create_authorization_header(service_id=sample_broadcast_service.id) + response = client.post( + path='/v2/broadcast', + data=xml_document, + headers=[('Content-Type', 'application/cap+xml'), auth_header], + ) + + assert json.loads(response.get_data(as_text=True)) == { + 'errors': [{ + 'error': 'ValidationError', + 'message': expected_error, + }], + 'status_code': 400, + }