From 82f08f230cba8bfbe42315b84b44cac967eec2c2 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 26 Jan 2022 15:35:00 +0000 Subject: [PATCH] Save api key id when cancelling broadcast by API call This is so that we can audit who cancelled the broadcast if there are any issues. --- app/broadcast_message/utils.py | 3 +- app/v2/broadcast/post_broadcast.py | 4 +- tests/app/broadcast_message/test_utils.py | 96 +++++++++++++++---- tests/app/v2/broadcast/test_post_broadcast.py | 8 +- 4 files changed, 89 insertions(+), 22 deletions(-) diff --git a/app/broadcast_message/utils.py b/app/broadcast_message/utils.py index 1673ab2bb..cb3a0b412 100644 --- a/app/broadcast_message/utils.py +++ b/app/broadcast_message/utils.py @@ -13,7 +13,7 @@ from app.models import ( ) -def validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user): +def validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user=None, api_key_id=None): if new_status not in BroadcastStatusType.ALLOWED_STATUS_TRANSITIONS[broadcast_message.status]: raise InvalidRequest( f'Cannot move broadcast_message {broadcast_message.id} from {broadcast_message.status} to {new_status}', @@ -39,6 +39,7 @@ def validate_and_update_broadcast_message_status(broadcast_message, new_status, if new_status == BroadcastStatusType.CANCELLED: broadcast_message.cancelled_at = datetime.utcnow() broadcast_message.cancelled_by = updating_user + broadcast_message.cancelled_by_api_key_id = api_key_id current_app.logger.info( f'broadcast_message {broadcast_message.id} moving from {broadcast_message.status} to {new_status}' diff --git a/app/v2/broadcast/post_broadcast.py b/app/v2/broadcast/post_broadcast.py index a597c2aa0..2be55cc30 100644 --- a/app/v2/broadcast/post_broadcast.py +++ b/app/v2/broadcast/post_broadcast.py @@ -81,7 +81,7 @@ def create_broadcast(): 'simple_polygons': simple_polygons.as_coordinate_pairs_lat_long, }, status=BroadcastStatusType.PENDING_APPROVAL, - api_key_id=api_user.id, + created_by_api_key_id=api_user.id, stubbed=authenticated_service.restricted # The client may pass in broadcast_json['expires'] but it’s # simpler for now to ignore it and have the rules around expiry @@ -111,7 +111,7 @@ def _cancel_or_reject_broadcast(references_to_original_broadcast, service_id): validate_and_update_broadcast_message_status( broadcast_message, new_status, - updating_user=None + api_key_id=api_user.id ) return broadcast_message diff --git a/tests/app/broadcast_message/test_utils.py b/tests/app/broadcast_message/test_utils.py index a02ac66bf..db12b0d0b 100644 --- a/tests/app/broadcast_message/test_utils.py +++ b/tests/app/broadcast_message/test_utils.py @@ -9,7 +9,12 @@ from app.models import ( BroadcastEventMessageType, BroadcastStatusType, ) -from tests.app.db import create_broadcast_message, create_template, create_user +from tests.app.db import ( + create_api_key, + create_broadcast_message, + create_template, + create_user, +) def test_validate_and_update_broadcast_message_status_stores_approved_by_and_approved_at_and_queues_task( @@ -49,11 +54,9 @@ def test_validate_and_update_broadcast_message_status_stores_approved_by_and_app assert alert_event.transmitted_content == {"body": "emergency broadcast"} -@pytest.mark.parametrize("cancel_route", ["admin_interface", "api_call"]) -def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast( +def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_from_admin_interface( sample_broadcast_service, mocker, - cancel_route ): template = create_template(sample_broadcast_service, BROADCAST_TYPE, content='emergency broadcast') broadcast_message = create_broadcast_message( @@ -64,19 +67,18 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast( "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]] } ) - if cancel_route == "admin_interface": - canceller = sample_broadcast_service.created_by - else: - canceller = None + canceller = sample_broadcast_service.created_by + mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') validate_and_update_broadcast_message_status( - broadcast_message, BroadcastStatusType.CANCELLED, canceller + broadcast_message, BroadcastStatusType.CANCELLED, updating_user=canceller, api_key_id=None ) assert broadcast_message.status == BroadcastStatusType.CANCELLED assert broadcast_message.cancelled_at is not None - assert broadcast_message.cancelled_by_id == (canceller.id if canceller else None) + assert broadcast_message.cancelled_by_id == canceller.id + assert broadcast_message.cancelled_by_api_key_id is None assert len(broadcast_message.events) == 1 alert_event = broadcast_message.events[0] @@ -87,11 +89,43 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast( assert alert_event.message_type == BroadcastEventMessageType.CANCEL -@pytest.mark.parametrize("reject_route", ["admin_interface", "api_call"]) -def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast( +def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_from_API_call( sample_broadcast_service, mocker, - reject_route +): + api_key = create_api_key(service=sample_broadcast_service) + template = create_template(sample_broadcast_service, BROADCAST_TYPE, content='emergency broadcast') + broadcast_message = create_broadcast_message( + template, + status=BroadcastStatusType.BROADCASTING, + areas={ + "ids": ["london"], + "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]] + } + ) + mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') + + validate_and_update_broadcast_message_status( + broadcast_message, BroadcastStatusType.CANCELLED, updating_user=None, api_key_id=api_key.id + ) + + assert broadcast_message.status == BroadcastStatusType.CANCELLED + assert broadcast_message.cancelled_at is not None + assert broadcast_message.cancelled_by_id is None + assert broadcast_message.cancelled_by_api_key_id == api_key.id + + assert len(broadcast_message.events) == 1 + alert_event = broadcast_message.events[0] + + mock_task.assert_called_once_with(kwargs={'broadcast_event_id': str(alert_event.id)}, queue='broadcast-tasks') + + assert alert_event.service_id == sample_broadcast_service.id + assert alert_event.message_type == BroadcastEventMessageType.CANCEL + + +def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_via_admin_interface( + sample_broadcast_service, + mocker ): template = create_template(sample_broadcast_service, BROADCAST_TYPE, content='emergency broadcast') broadcast_message = create_broadcast_message( @@ -102,14 +136,10 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast( "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]] } ) - if reject_route == "admin_interface": - canceller = sample_broadcast_service.created_by - else: - canceller = None mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') validate_and_update_broadcast_message_status( - broadcast_message, BroadcastStatusType.REJECTED, canceller + broadcast_message, BroadcastStatusType.REJECTED, updating_user=sample_broadcast_service.created_by ) assert broadcast_message.status == BroadcastStatusType.REJECTED @@ -121,6 +151,36 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast( assert len(broadcast_message.events) == 0 +def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast( + sample_broadcast_service, + mocker +): + api_key = create_api_key(service=sample_broadcast_service) + template = create_template(sample_broadcast_service, BROADCAST_TYPE, content='emergency broadcast') + broadcast_message = create_broadcast_message( + template, + status=BroadcastStatusType.PENDING_APPROVAL, + areas={ + "ids": ["london"], + "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]] + } + ) + mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') + + validate_and_update_broadcast_message_status( + broadcast_message, BroadcastStatusType.REJECTED, api_key_id=api_key.id + ) + + assert broadcast_message.status == BroadcastStatusType.REJECTED + assert broadcast_message.cancelled_at is None + assert broadcast_message.cancelled_by_id is None + assert broadcast_message.cancelled_by_api_key_id is None + assert broadcast_message.updated_at is not None + + assert not mock_task.called + assert len(broadcast_message.events) == 0 + + @pytest.mark.parametrize('current_status, new_status', [ (BroadcastStatusType.DRAFT, BroadcastStatusType.DRAFT), (BroadcastStatusType.DRAFT, BroadcastStatusType.BROADCASTING), diff --git a/tests/app/v2/broadcast/test_post_broadcast.py b/tests/app/v2/broadcast/test_post_broadcast.py index e767d33d2..c42435fcf 100644 --- a/tests/app/v2/broadcast/test_post_broadcast.py +++ b/tests/app/v2/broadcast/test_post_broadcast.py @@ -7,6 +7,7 @@ from app.dao.broadcast_message_dao import ( dao_get_broadcast_message_by_id_and_service_id, ) from tests import create_service_authorization_header +from tests.app.db import create_api_key from . import sample_cap_xml_documents @@ -130,6 +131,7 @@ def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_mess is_approved, expected_status ): + api_key = create_api_key(service=sample_broadcast_service) auth_header = create_service_authorization_header(service_id=sample_broadcast_service.id) # create a broadcast @@ -158,7 +160,11 @@ def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_mess headers=[('Content-Type', 'application/cap+xml'), auth_header], ) assert response_for_cancel.status_code == 201 - mock_update.assert_called_once_with(broadcast_message, expected_status, updating_user=None) + mock_update.assert_called_once_with( + broadcast_message, + expected_status, + api_key_id=api_key.id + ) def test_cancel_request_does_not_cancel_broadcast_if_reference_does_not_match(