Merge pull request #3976 from alphagov/eradicate-roles-178770155

Replace the term "role" everywhere

app/main/forms.py

@@ -56,13 +56,13 @@ from app.main.validators import (
 )
 from app.models.feedback import PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE
 from app.models.organisation import Organisation
-from app.models.roles_and_permissions import (
-    broadcast_permissions,
-    permissions,
-    roles,
-)
 from app.utils import merge_jsonlike
 from app.utils.user import distinct_email_addresses
+from app.utils.user_permissions import (
+    all_ui_permissions,
+    broadcast_permission_options,
+    permission_options,
+)
 
 
 def get_time_value_and_label(future_time):
@@ -945,20 +945,20 @@ class GovukRadiosFieldWithNoneOption(FieldWithNoneOption, GovukRadiosField):
     pass
 
 
-# guard against data entries that aren't a role in permissions
+# guard against data entries that aren't a known permission
 def filter_by_permissions(valuelist):
     if valuelist is None:
         return None
     else:
-        return [entry for entry in valuelist if any(entry in role for role in permissions)]
+        return [entry for entry in valuelist if any(entry in option for option in permission_options)]
 
 
-# guard against data entries that aren't a role in broadcast_permissions
+# guard against data entries that aren't a known broadcast permission
 def filter_by_broadcast_permissions(valuelist):
     if valuelist is None:
         return None
     else:
-        return [entry for entry in valuelist if any(entry in role for role in broadcast_permissions)]
+        return [entry for entry in valuelist if any(entry in option for option in broadcast_permission_options)]
 
 
 class BasePermissionsForm(StripWhitespaceForm):
@@ -989,7 +989,7 @@ class BasePermissionsForm(StripWhitespaceForm):
         'Permissions',
         filters=[filter_by_permissions],
         choices=[
-            (value, label) for value, label in permissions
+            (value, label) for value, label in permission_options
         ],
         param_extensions={
             "hint": {"text": "All team members can see sent messages."}
@@ -1005,8 +1005,10 @@ class BasePermissionsForm(StripWhitespaceForm):
         form = cls(
             **kwargs,
             **{
-                "permissions_field": [
-                    role for role in roles.keys() if user.has_permission_for_service(service_id, role)]
+                "permissions_field": (
+                    user.permissions_for_service(service_id) & all_ui_permissions
+                )
+
             },
             login_authentication=user.auth_type
         )
@@ -1027,7 +1029,7 @@ class BroadcastPermissionsForm(BasePermissionsForm):
     permissions_field = GovukCheckboxesField(
         'Permissions',
         choices=[
-            (value, label) for value, label in broadcast_permissions
+            (value, label) for value, label in broadcast_permission_options
         ],
         filters=[filter_by_broadcast_permissions],
         param_extensions={

app/main/views/manage_users.py

@@ -28,9 +28,12 @@ from app.main.forms import (
     PermissionsForm,
     SearchUsersForm,
 )
-from app.models.roles_and_permissions import broadcast_permissions, permissions
 from app.models.user import InvitedUser, User
 from app.utils.user import is_gov_user, user_has_permissions
+from app.utils.user_permissions import (
+    broadcast_permission_options,
+    permission_options,
+)
 
 
 @main.route("/services/<uuid:service_id>/users")
@@ -43,7 +46,7 @@ def manage_users(service_id):
         show_search_box=(len(current_service.team_members) > 7),
         form=SearchUsersForm(),
         permissions=(
-            broadcast_permissions if current_service.has_permission('broadcast') else permissions
+            broadcast_permission_options if current_service.has_permission('broadcast') else permission_options
         ),
     )
 

app/models/roles_and_permissions.py

@@ -1,52 +0,0 @@
-from itertools import chain
-
-roles = {
-    'send_messages': ['send_texts', 'send_emails', 'send_letters'],
-    'manage_templates': ['manage_templates'],
-    'manage_service': ['manage_users', 'manage_settings'],
-    'manage_api_keys': ['manage_api_keys'],
-    'view_activity': ['view_activity'],
-    'create_broadcasts': ['create_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
-    'approve_broadcasts': ['approve_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
-}
-
-all_permissions = set(roles.keys())
-all_database_permissions = set(chain(*roles.values()))
-
-permissions = (
-    ('view_activity', 'See dashboard'),
-    ('send_messages', 'Send messages'),
-    ('manage_templates', 'Add and edit templates'),
-    ('manage_service', 'Manage settings, team and usage'),
-    ('manage_api_keys', 'Manage API integration'),
-)
-
-broadcast_permissions = (
-    ('manage_templates', 'Add and edit templates'),
-    ('create_broadcasts', 'Create new alerts'),
-    ('approve_broadcasts', 'Approve alerts'),
-)
-
-
-def translate_permissions_from_db_to_admin_roles(permissions):
-    """
-    Given a list of database permissions, return a set of roles
-
-    A role is returned if all of its database permissions are in the permission list that is passed in.
-    Any permissions in the list that are not database permissions are also returned.
-    """
-    unknown_database_permissions = {p for p in permissions if p not in all_database_permissions}
-
-    return {
-        admin_role for admin_role, db_role_list in roles.items()
-        if set(db_role_list) <= set(permissions)
-    } | unknown_database_permissions
-
-
-def translate_permissions_from_admin_roles_to_db(permissions):
-    """
-    Given a list of admin roles (ie: checkboxes on a permissions edit page for example), return a set of db permissions
-
-    Looks them up in the roles dict, falling back to just passing through if they're not recognised.
-    """
-    return set(chain.from_iterable(roles.get(permission, [permission]) for permission in permissions))

app/models/user.py

@@ -10,16 +10,16 @@ from app.event_handlers import (
 )
 from app.models import JSONModel, ModelList
 from app.models.organisation import Organisation
-from app.models.roles_and_permissions import (
-    all_permissions,
-    translate_permissions_from_db_to_admin_roles,
-)
 from app.models.webauthn_credential import WebAuthnCredentials
 from app.notify_client import InviteTokenError
 from app.notify_client.invite_api_client import invite_api_client
 from app.notify_client.org_invite_api_client import org_invite_api_client
 from app.notify_client.user_api_client import user_api_client
 from app.utils.user import is_gov_user
+from app.utils.user_permissions import (
+    all_ui_permissions,
+    translate_permissions_from_db_to_ui,
+)
 
 
 def _get_service_id_from_view_args():
@@ -103,7 +103,7 @@ class User(JSONModel, UserMixin):
         them out later, we'll need to rework this function.
         """
         self._permissions = {
-            service: translate_permissions_from_db_to_admin_roles(permissions)
+            service: translate_permissions_from_db_to_ui(permissions)
             for service, permissions
             in permissions_by_service.items()
         }
@@ -201,7 +201,7 @@ class User(JSONModel, UserMixin):
         return self._platform_admin and not session.get('disable_platform_admin_view', False)
 
     def has_permissions(self, *permissions, restrict_admin_usage=False, allow_org_user=False):
-        unknown_permissions = set(permissions) - all_permissions
+        unknown_permissions = set(permissions) - all_ui_permissions
         if unknown_permissions:
             raise TypeError('{} are not valid permissions'.format(list(unknown_permissions)))
 
@@ -505,7 +505,7 @@ class InvitedUser(JSONModel):
             self._permissions = permissions
         else:
             self._permissions = permissions.split(',')
-        self._permissions = translate_permissions_from_db_to_admin_roles(self.permissions)
+        self._permissions = translate_permissions_from_db_to_ui(self.permissions)
 
     @property
     def from_user(self):

app/notify_client/invite_api_client.py

@@ -1,8 +1,8 @@
-from app.models.roles_and_permissions import (
-    roles,
-    translate_permissions_from_admin_roles_to_db,
-)
 from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache
+from app.utils.user_permissions import (
+    all_ui_permissions,
+    translate_permissions_from_ui_to_db,
+)
 
 
 class InviteApiClient(NotifyAdminAPIClient):
@@ -23,7 +23,7 @@ class InviteApiClient(NotifyAdminAPIClient):
             'service': service_id,
             'email_address': email_address,
             'from_user': invite_from_id,
-            'permissions': ','.join(sorted(translate_permissions_from_admin_roles_to_db(permissions))),
+            'permissions': ','.join(sorted(translate_permissions_from_ui_to_db(permissions))),
             'auth_type': auth_type,
             'invite_link_host': self.admin_url,
             'folder_permissions': folder_permissions,
@@ -48,7 +48,7 @@ class InviteApiClient(NotifyAdminAPIClient):
         )['data']
 
     def get_count_of_invites_with_permission(self, service_id, permission):
-        if permission not in roles.keys():
+        if permission not in all_ui_permissions:
             raise TypeError('{} is not a valid permission'.format(permission))
         return len([
             invited_user for invited_user in self.get_invites_for_service(service_id)

app/notify_client/user_api_client.py

@@ -1,9 +1,7 @@
 from notifications_python_client.errors import HTTPError
 
-from app.models.roles_and_permissions import (
-    translate_permissions_from_admin_roles_to_db,
-)
 from app.notify_client import NotifyAdminAPIClient, cache
+from app.utils.user_permissions import translate_permissions_from_ui_to_db
 
 ALLOWED_ATTRIBUTES = {
     'name',
@@ -149,10 +147,10 @@ class UserApiClient(NotifyAdminAPIClient):
     @cache.delete('service-{service_id}-template-folders')
     @cache.delete('user-{user_id}')
     def add_user_to_service(self, service_id, user_id, permissions, folder_permissions):
-        # permissions passed in are the combined admin roles, not db permissions
+        # permissions passed in are the combined UI permissions, not DB permissions
         endpoint = '/service/{}/users/{}'.format(service_id, user_id)
         data = {
-            'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)],
+            'permissions': [{'permission': x} for x in translate_permissions_from_ui_to_db(permissions)],
             'folder_permissions': folder_permissions,
         }
 
@@ -166,9 +164,9 @@ class UserApiClient(NotifyAdminAPIClient):
     @cache.delete('service-{service_id}-template-folders')
     @cache.delete('user-{user_id}')
     def set_user_permissions(self, user_id, service_id, permissions, folder_permissions=None):
-        # permissions passed in are the combined admin roles, not db permissions
+        # permissions passed in are the combined UI permissions, not DB permissions
         data = {
-            'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)],
+            'permissions': [{'permission': x} for x in translate_permissions_from_ui_to_db(permissions)],
         }
 
         if folder_permissions is not None:

app/utils/user_permissions.py

@@ -0,0 +1,54 @@
+from itertools import chain
+
+permission_mappings = {
+    'send_messages': ['send_texts', 'send_emails', 'send_letters'],
+    'manage_templates': ['manage_templates'],
+    'manage_service': ['manage_users', 'manage_settings'],
+    'manage_api_keys': ['manage_api_keys'],
+    'view_activity': ['view_activity'],
+    'create_broadcasts': ['create_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
+    'approve_broadcasts': ['approve_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
+}
+
+all_ui_permissions = set(permission_mappings.keys())
+all_db_permissions = set(chain(*permission_mappings.values()))
+
+permission_options = (
+    ('view_activity', 'See dashboard'),
+    ('send_messages', 'Send messages'),
+    ('manage_templates', 'Add and edit templates'),
+    ('manage_service', 'Manage settings, team and usage'),
+    ('manage_api_keys', 'Manage API integration'),
+)
+
+broadcast_permission_options = (
+    ('manage_templates', 'Add and edit templates'),
+    ('create_broadcasts', 'Create new alerts'),
+    ('approve_broadcasts', 'Approve alerts'),
+)
+
+
+def translate_permissions_from_db_to_ui(db_permissions):
+    """
+    Given a list of database permissions, return a set of UI permissions
+
+    A UI permission is returned if all of its DB permissions are in the permission list that is passed in.
+    Any DB permissions in the list that are not known permissions are also returned.
+    """
+    unknown_database_permissions = set(db_permissions) - all_db_permissions
+
+    return {
+        ui_permission for ui_permission, db_permissions_for_ui_permission in permission_mappings.items()
+        if set(db_permissions_for_ui_permission) <= set(db_permissions)
+    } | unknown_database_permissions
+
+
+def translate_permissions_from_ui_to_db(ui_permissions):
+    """
+    Given a list of UI permissions (ie: checkboxes on a permissions edit page), return a set of DB permissions
+
+    Looks them up in the mapping, falling back to just passing through if they're not recognised.
+    """
+    return set(chain.from_iterable(
+        permission_mappings.get(ui_permission, [ui_permission]) for ui_permission in ui_permissions
+    ))

tests/app/main/test_permissions.py

@@ -5,10 +5,6 @@ import re
 import pytest
 from flask import current_app
 
-from app.models.roles_and_permissions import (
-    translate_permissions_from_admin_roles_to_db,
-    translate_permissions_from_db_to_admin_roles,
-)
 from tests import service_json
 from tests.conftest import (
     ORGANISATION_ID,
@@ -18,42 +14,6 @@ from tests.conftest import (
 )
 
 
-@pytest.mark.parametrize('db_roles,admin_roles', [
-    (
-        ['approve_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
-        {'approve_broadcasts'},
-    ),
-    (
-        ['manage_templates', 'create_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
-        {'create_broadcasts', 'manage_templates'},
-    ),
-    (
-        ['manage_templates'],
-        {'manage_templates'},
-    ),
-    (
-        ['create_broadcasts'],
-        set(),
-    ),
-    (
-        ['send_texts', 'send_emails', 'send_letters', 'manage_templates', 'some_unknown_permission'],
-        {'send_messages', 'manage_templates', 'some_unknown_permission'},
-    ),
-])
-def test_translate_permissions_from_db_to_admin_roles(
-    db_roles,
-    admin_roles,
-):
-    roles = translate_permissions_from_db_to_admin_roles(db_roles)
-    assert roles == admin_roles
-
-
-def test_translate_permissions_from_admin_roles_to_db():
-    roles = ['send_messages', 'manage_templates', 'some_unknown_permission']
-    db_perms = translate_permissions_from_admin_roles_to_db(roles)
-    assert db_perms == {'send_texts', 'send_emails', 'send_letters', 'manage_templates', 'some_unknown_permission'}
-
-
 @pytest.mark.parametrize(
     'user_services, user_organisations, expected_status, organisation_checked',
     (

tests/app/utils/test_user_permissions.py

@@ -0,0 +1,45 @@
+import pytest
+
+from app.utils.user_permissions import (
+    translate_permissions_from_db_to_ui,
+    translate_permissions_from_ui_to_db,
+)
+
+
+@pytest.mark.parametrize('db_permissions,expected_ui_permissions', [
+    (
+        ['approve_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
+        {'approve_broadcasts'},
+    ),
+    (
+        ['manage_templates', 'create_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'],
+        {'create_broadcasts', 'manage_templates'},
+    ),
+    (
+        ['manage_templates'],
+        {'manage_templates'},
+    ),
+    (
+        ['create_broadcasts'],
+        set(),
+    ),
+    (
+        ['send_texts', 'send_emails', 'send_letters', 'manage_templates', 'some_unknown_permission'],
+        {'send_messages', 'manage_templates', 'some_unknown_permission'},
+    ),
+])
+def test_translate_permissions_from_db_to_ui(
+    db_permissions,
+    expected_ui_permissions,
+):
+    ui_permissions = translate_permissions_from_db_to_ui(db_permissions)
+    assert ui_permissions == expected_ui_permissions
+
+
+def test_translate_permissions_from_ui_to_db():
+    ui_permissions = ['send_messages', 'manage_templates', 'some_unknown_permission']
+    db_permissions = translate_permissions_from_ui_to_db(ui_permissions)
+
+    assert db_permissions == {
+        'send_texts', 'send_emails', 'send_letters', 'manage_templates', 'some_unknown_permission'
+    }