diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 6618a3e16..a808e0a29 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -93,7 +93,7 @@ def get_broadcast_dashboard_partials(service_id): @main.route('/services//new-broadcast', methods=['GET', 'POST']) -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def new_broadcast(service_id): form = NewBroadcastForm() @@ -116,7 +116,7 @@ def new_broadcast(service_id): @main.route('/services//write-new-broadcast', methods=['GET', 'POST']) -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def write_new_broadcast(service_id): form = BroadcastTemplateForm() @@ -140,7 +140,7 @@ def write_new_broadcast(service_id): @main.route('/services//new-broadcast/') -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def broadcast(service_id, template_id): return redirect(url_for( @@ -154,7 +154,7 @@ def broadcast(service_id, template_id): @main.route('/services//broadcast//areas') -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def preview_broadcast_areas(service_id, broadcast_message_id): broadcast_message = BroadcastMessage.from_id( @@ -181,7 +181,7 @@ def preview_broadcast_areas(service_id, broadcast_message_id): @main.route('/services//broadcast//libraries') -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def choose_broadcast_library(service_id, broadcast_message_id): return render_template( @@ -198,7 +198,7 @@ def choose_broadcast_library(service_id, broadcast_message_id): '/services//broadcast//libraries/', methods=['GET', 'POST'], ) -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def choose_broadcast_area(service_id, broadcast_message_id, library_slug): broadcast_message = BroadcastMessage.from_id( @@ -258,7 +258,7 @@ def _get_broadcast_sub_area_back_link(service_id, broadcast_message_id, library_ '/services//broadcast//libraries//', methods=['GET', 'POST'], ) -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def choose_broadcast_sub_area(service_id, broadcast_message_id, library_slug, area_slug): broadcast_message = BroadcastMessage.from_id( @@ -310,7 +310,7 @@ def choose_broadcast_sub_area(service_id, broadcast_message_id, library_slug, ar @main.route('/services//broadcast//remove/') -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def remove_broadcast_area(service_id, broadcast_message_id, area_slug): BroadcastMessage.from_id( @@ -330,7 +330,7 @@ def remove_broadcast_area(service_id, broadcast_message_id, area_slug): '/services//broadcast//preview', methods=['GET', 'POST'], ) -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def preview_broadcast_message(service_id, broadcast_message_id): broadcast_message = BroadcastMessage.from_id( @@ -405,7 +405,7 @@ def view_broadcast(service_id, broadcast_message_id): @main.route('/services//current-alerts/', methods=['POST']) -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def approve_broadcast_message(service_id, broadcast_message_id): @@ -438,7 +438,7 @@ def approve_broadcast_message(service_id, broadcast_message_id): @main.route('/services//broadcast//reject') -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=True) @service_has_permission('broadcast') def reject_broadcast_message(service_id, broadcast_message_id): @@ -466,7 +466,7 @@ def reject_broadcast_message(service_id, broadcast_message_id): '/services//broadcast//cancel', methods=['GET', 'POST'], ) -@user_has_permissions('send_messages') +@user_has_permissions('send_messages', restrict_admin_usage=False) @service_has_permission('broadcast') def cancel_broadcast_message(service_id, broadcast_message_id): broadcast_message = BroadcastMessage.from_id( diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 4b3f4511e..4bbde132d 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -93,6 +93,7 @@ def test_broadcast_pages_403_without_permission( ) +@pytest.mark.parametrize('user_is_platform_admin', [True, False]) @pytest.mark.parametrize('endpoint, extra_args, expected_get_status, expected_post_status', ( ( '.new_broadcast', {}, @@ -127,23 +128,27 @@ def test_broadcast_pages_403_without_permission( '.preview_broadcast_message', {'broadcast_message_id': sample_uuid}, 403, 403, ), - ( - '.cancel_broadcast_message', {'broadcast_message_id': sample_uuid}, - 403, 403, - ), )) def test_broadcast_pages_403_for_user_without_permission( mocker, client_request, service_one, active_user_view_permissions, + platform_admin_user_no_service_permissions, endpoint, extra_args, expected_get_status, expected_post_status, + user_is_platform_admin ): + """ + Checks that users without permissions, including admin users, cannot create, approve or reject broadcasts. + """ service_one['permissions'] += ['broadcast'] - mocker.patch('app.user_api_client.get_user', return_value=active_user_view_permissions) + if user_is_platform_admin: + client_request.login(platform_admin_user_no_service_permissions) + else: + client_request.login(active_user_view_permissions) client_request.get( endpoint, service_id=SERVICE_ONE_ID, @@ -158,6 +163,32 @@ def test_broadcast_pages_403_for_user_without_permission( ) +def test_cancel_broadcast_page_403_for_user_without_permission( + mocker, + client_request, + service_one, + active_user_view_permissions, +): + """ + separate test for cancel_broadcast endpoint, because admin users are allowed to cancel broadcasts + """ + service_one['permissions'] += ['broadcast'] + + mocker.patch('app.user_api_client.get_user', return_value=active_user_view_permissions) + client_request.get( + '.cancel_broadcast_message', + service_id=SERVICE_ONE_ID, + _expected_status=403, + **{'broadcast_message_id': sample_uuid} + ) + client_request.post( + '.cancel_broadcast_message', + service_id=SERVICE_ONE_ID, + _expected_status=403, + **{'broadcast_message_id': sample_uuid} + ) + + @pytest.mark.parametrize('step_index, expected_link_text, expected_link_href', ( (1, 'Continue', partial(url_for, '.broadcast_tour', step_index=2)), (2, 'Continue', partial(url_for, '.broadcast_tour', step_index=3)), @@ -2051,15 +2082,25 @@ def test_no_view_page_for_draft( ) +@pytest.mark.parametrize("user_is_platform_admin", [True, False]) def test_cancel_broadcast( client_request, service_one, mock_get_live_broadcast_message, mock_get_broadcast_template, mock_update_broadcast_message_status, + platform_admin_user_no_service_permissions, fake_uuid, + user_is_platform_admin ): + """ + users with 'send_messages' permissions and platform admins should be able to cancel broadcasts. + """ service_one['permissions'] += ['broadcast'] + + if user_is_platform_admin: + client_request.login(platform_admin_user_no_service_permissions) + page = client_request.get( '.cancel_broadcast_message', service_id=SERVICE_ONE_ID, @@ -2083,15 +2124,25 @@ def test_cancel_broadcast( ) not in page +@pytest.mark.parametrize("user_is_platform_admin", [True, False]) def test_confirm_cancel_broadcast( client_request, service_one, mock_get_live_broadcast_message, mock_get_broadcast_template, mock_update_broadcast_message_status, + platform_admin_user_no_service_permissions, fake_uuid, + user_is_platform_admin ): + """ + users with 'send_messages' permissions and platform admins should be able to cancel broadcasts. + """ service_one['permissions'] += ['broadcast'] + + if user_is_platform_admin: + client_request.login(platform_admin_user_no_service_permissions) + client_request.post( '.cancel_broadcast_message', service_id=SERVICE_ONE_ID, diff --git a/tests/conftest.py b/tests/conftest.py index ba4ba6527..729623a3e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1108,6 +1108,31 @@ def platform_admin_user(fake_uuid): return user_data +@pytest.fixture(scope='function') +def platform_admin_user_no_service_permissions(): + """ + this fixture is for situations where we want to test that platform admin can access + an endpoint even though they have no explicit permissions for that service. + """ + user_data = {'id': uuid4(), + 'name': 'Platform admin user no service permissions', + 'password': 'somepassword', + 'email_address': 'platform2@admin.gov.uk', + 'mobile_number': '07700 900763', + 'state': 'active', + 'failed_login_count': 0, + 'permissions': {}, + 'platform_admin': True, + 'auth_type': 'sms_auth', + 'password_changed_at': str(datetime.utcnow()), + 'services': [], + 'organisations': [], + 'current_session_id': None, + 'logged_in_at': None, + } + return user_data + + @pytest.fixture(scope='function') def api_user_active(fake_uuid): user_data = {'id': fake_uuid,