From bb4c86008a00549b323142117a6d437664eee773 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Thu, 8 Jul 2021 15:20:24 +0100 Subject: [PATCH] Add audit event for suspending a service This is particularly important for broadcast services, where a rogue service or platform admin could launch a DoS attack by suspending a service at a critical moment when it needs to send alerts. --- app/event_handlers.py | 11 +++++++++++ app/main/views/service_settings.py | 6 +++++- tests/app/main/views/test_service_settings.py | 7 +++++-- tests/app/test_event_handlers.py | 19 +++++++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/app/event_handlers.py b/app/event_handlers.py index 526b57481..716428535 100644 --- a/app/event_handlers.py +++ b/app/event_handlers.py @@ -67,6 +67,17 @@ def create_broadcast_account_type_change_event( ) +def create_suspend_service_event( + service_id, + suspended_by_id, +): + _send_event( + 'suspend_service', + service_id=service_id, + suspended_by_id=suspended_by_id, + ) + + def _send_event(event_type, **kwargs): event_data = _construct_event_data(request) event_data.update(kwargs) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index cfd5568c7..f748d82ee 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -28,7 +28,10 @@ from app import ( service_api_client, user_api_client, ) -from app.event_handlers import create_broadcast_account_type_change_event +from app.event_handlers import ( + create_broadcast_account_type_change_event, + create_suspend_service_event, +) from app.extensions import zendesk_client from app.formatters import email_safe from app.main import main @@ -443,6 +446,7 @@ def archive_service(service_id): def suspend_service(service_id): if request.method == 'POST': service_api_client.suspend_service(service_id) + create_suspend_service_event(service_id, suspended_by_id=current_user.id) return redirect(url_for('.service_settings', service_id=service_id)) else: flash("This will suspend the service and revoke all api keys. Are you sure you want to suspend this service?", diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 2480e252a..5cc2eb498 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -4203,17 +4203,20 @@ def test_cant_archive_inactive_service( def test_suspend_service_after_confirm( platform_admin_client, + api_user_active, service_one, mocker, mock_get_inbound_number_for_service, ): - mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) + mock_api = mocker.patch('app.service_api_client.post', return_value=service_one) + mock_event = mocker.patch('app.main.views.service_settings.create_suspend_service_event') response = platform_admin_client.post(url_for('main.suspend_service', service_id=service_one['id'])) assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - assert mocked_fn.call_args == call('/service/{}/suspend'.format(service_one['id']), data=None) + mock_api.assert_called_once_with('/service/{}/suspend'.format(service_one['id']), data=None) + mock_event.assert_called_once_with(service_one['id'], suspended_by_id=api_user_active['id']) def test_suspend_service_prompts_user( diff --git a/tests/app/test_event_handlers.py b/tests/app/test_event_handlers.py index 8aa30d433..7c63cef8a 100644 --- a/tests/app/test_event_handlers.py +++ b/tests/app/test_event_handlers.py @@ -8,6 +8,7 @@ from app.event_handlers import ( create_email_change_event, create_mobile_number_change_event, create_remove_user_from_service_event, + create_suspend_service_event, on_user_logged_in, ) from app.models.user import User @@ -133,3 +134,21 @@ def test_create_broadcast_account_type_change_event(notify_admin, mock_events): 'service_mode': 'training', 'broadcast_channel': 'severe', 'provider_restriction': None}) + + +def test_suspend_service(client, mock_events): + service_id = str(uuid.uuid4()) + suspended_by_id = str(uuid.uuid4()) + + create_suspend_service_event( + service_id, + suspended_by_id, + ) + + mock_events.assert_called_with( + 'suspend_service', + {'browser_fingerprint': {'browser': ANY, 'version': ANY, 'platform': ANY, 'user_agent_string': ''}, + 'ip_address': ANY, + 'service_id': service_id, + 'suspended_by_id': suspended_by_id}, + )