From fc0b9736eb37406cf3c7fc8b79c0ca04aa5df149 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 22 Jun 2021 16:03:39 +0100 Subject: [PATCH] Remove user permissions if service becomes a broadcast service The "normal" service permissions and broadcast service permissions are going to be different with no overlap. This means that if you were viewing the team members page, there might be permissions in the database that are not visible on the frontend if a service has changed type. For example, someone could have the 'manage_api_keys' permission, which would not show up on the team members page of a broadcast service. To avoid people having permissions which aren't visible in admin, we now remove all permissions from users when their service is converted to a broadcast service. Permisions for invited users are also removed. It's not possible to convert a broadcast service to a normal service, so we don't need to cover for this scenario. --- app/dao/broadcast_service_dao.py | 10 ++++++++ app/service/rest.py | 1 + tests/app/service/test_rest.py | 40 ++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/app/dao/broadcast_service_dao.py b/app/dao/broadcast_service_dao.py index 9971bf316..fdaff24f6 100644 --- a/app/dao/broadcast_service_dao.py +++ b/app/dao/broadcast_service_dao.py @@ -7,7 +7,10 @@ from app.dao.dao_utils import autocommit, version_class from app.models import ( BROADCAST_TYPE, EMAIL_AUTH_TYPE, + INVITE_PENDING, + InvitedUser, Organisation, + Permission, Service, ServiceBroadcastSettings, ServicePermission, @@ -53,6 +56,13 @@ def set_broadcast_service_type(service, service_mode, broadcast_channel, provide service.restricted = True service.go_live_at = None + # Remove all user permissions for the service users and invited users + Permission.query.filter_by(service_id=service.id).delete() + InvitedUser.query.filter_by( + service_id=service.id, + status=INVITE_PENDING + ).update({'permissions': ''}) + # Add service to organisation organisation = Organisation.query.filter_by( id=current_app.config['BROADCAST_ORGANISATION_ID'] diff --git a/app/service/rest.py b/app/service/rest.py index 302cd5e68..86908a92b 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -1115,6 +1115,7 @@ def set_as_broadcast_service(service_id): - sets the services `count_as_live` to false - adds the service to the broadcast organisation - puts the service into training mode or live mode + - removes all permissions from current users and invited users """ data = validate(request.get_json(), service_broadcast_settings_schema) service = dao_fetch_service_by_id(service_id) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 002b0bcc0..48f311890 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -4128,3 +4128,43 @@ def test_set_as_broadcast_service_updates_services_history( new_history_records = Service.get_history_model().query.filter_by(id=sample_service.id).all() assert len(new_history_records) == len(old_history_records) + 1 + + +def test_set_as_broadcast_service_removes_user_permissions( + admin_request, + broadcast_organisation, + sample_service, + sample_service_full_permissions, + sample_invited_user, +): + service_user = sample_service.users[0] + + # make the user a member of a second service + dao_add_user_to_service( + sample_service_full_permissions, + service_user, + permissions=[ + Permission(service_id=sample_service_full_permissions.id, + user_id=service_user.id, + permission='send_emails') + ] + ) + assert len(service_user.get_permissions(service_id=sample_service.id)) == 8 + assert len(sample_invited_user.get_permissions()) == 3 + + admin_request.post( + 'service.set_as_broadcast_service', + service_id=sample_service.id, + _data={ + 'broadcast_channel': 'test', + 'service_mode': 'live', + 'provider_restriction': 'ee' + } + ) + + # The user permissions for the broadcast service get removed + assert len(service_user.get_permissions(service_id=sample_service.id)) == 0 + # Permissions for users invited to the broadcast service get removed + assert sample_invited_user.permissions == '' + # Permissions for other services remain + assert len(service_user.get_permissions(service_id=sample_service_full_permissions.id)) == 1