map roles and db permissions

in the db, we have several rows for single permissions - we separate
`send_messages` into `send_texts`, `send_emails` and `send_letters`,
and also `manage_service` into `manage_users` and `manage_settings`.

But on the front end we don't do anything with this distinction. It's
unhelpful for us to have to think about permissions as groups of things
when we can never split them up at all. So we should combine them. This
commit makes sure:
* when user models are read  (from JSON direct from the API), we
  should transform them from db permissions into roles.
* when permissions are persisted (editing permissions, and creating
  invites), we should send db permissions to the API.

All other interaction with permissions (should just be the endpoint
decorator and checks in html templates generally) should use admin
roles.

app/main/views/manage_users.py

@@ -1,5 +1,3 @@
-from itertools import chain
-
 from flask import abort, flash, redirect, render_template, request, url_for
 from flask_login import current_user, login_required
 from notifications_python_client.errors import HTTPError
@@ -52,7 +50,7 @@ def invite_user(service_id):
 
     if form.validate_on_submit():
         email_address = form.email_address.data
-        permissions = ','.join(sorted(get_permissions_from_form(form)))
+        permissions = get_permissions_from_form(form)
         invited_user = invite_api_client.create_invite(
             current_user.id,
             service_id,
@@ -82,7 +80,7 @@ def edit_user_permissions(service_id, user_id):
     user_has_no_mobile_number = user.mobile_number is None
 
     form = PermissionsForm(
-        **{role: user.has_permissions(*permissions) for role, permissions in roles.items()},
+        **{role: user.has_permissions(role) for role in roles.keys()},
         login_authentication=user.auth_type
     )
     if form.validate_on_submit():
@@ -111,7 +109,7 @@ def remove_user_from_service(service_id, user_id):
     # Need to make the email address read only, or a disabled field?
     # Do it through the template or the form class?
     form = PermissionsForm(**{
-        role: user.has_permissions(*permissions) for role, permissions in roles.items()
+        role: user.has_permissions(role) for role in roles.keys()
     })
 
     if request.method == 'POST':
@@ -152,11 +150,6 @@ def get_permissions_from_form(form):
     # view_activity is a default role to be added to all users.
     # All users will have at minimum view_activity to allow users to see notifications,
     # templates, team members but no update privileges
-    selected_permissions = [
-        permissions
-        for role, permissions in roles.items()
-        if form[role].data is True
-    ]
-    selected_permissions = list(chain.from_iterable(selected_permissions))
-    selected_permissions.append('view_activity')
-    return selected_permissions
+    selected_roles = {role for role in roles.keys() if form[role].data is True}
+    selected_roles.add('view_activity')
+    return selected_roles

app/notify_client/invite_api_client.py

@@ -1,6 +1,8 @@
-
 from app.notify_client import NotifyAdminAPIClient, _attach_current_user
-from app.notify_client.models import InvitedUser
+from app.notify_client.models import (
+    InvitedUser,
+    translate_permissions_from_admin_roles_to_db,
+)
 
 
 class InviteApiClient(NotifyAdminAPIClient):
@@ -17,7 +19,7 @@ class InviteApiClient(NotifyAdminAPIClient):
             'service': str(service_id),
             'email_address': email_address,
             'from_user': invite_from_id,
-            'permissions': permissions,
+            'permissions': ','.join(sorted(translate_permissions_from_admin_roles_to_db(permissions))),
             'auth_type': auth_type,
             'invite_link_host': self.admin_url,
         }

app/notify_client/models.py

@@ -10,13 +10,46 @@ roles = {
     'manage_api_keys': ['manage_api_keys']
 }
 
-all_permissions = set(chain.from_iterable(roles.values())) | {'view_activity'}
+# same dict as above, but flipped round (and with view_activity)
+roles_by_permission = {
+    'send_texts': 'send_messages',
+    'send_emails': 'send_messages',
+    'send_letters': 'send_messages',
+
+    'manage_users': 'manage_service',
+    'manage_settings': 'manage_service',
+
+    'manage_templates': 'manage_templates',
+
+    'manage_api_keys': 'manage_api_keys',
+    'view_activity': 'view_activity',
+}
+
+all_permissions = set(roles_by_permission.values())
 
 
 def _get_service_id_from_view_args():
     return request.view_args.get('service_id', None)
 
 
+def translate_permissions_from_db_to_admin_roles(permissions):
+    """
+    Given a list of database permissions, return a set of roles
+
+    look them up in roles_by_permission, falling back to just passing through from the api if they aren't in the dict
+    """
+    return {roles_by_permission.get(permission, permission) for permission in 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))
+
+
 class User(UserMixin):
     def __init__(self, fields, max_failed_login_count=3):
         self._id = fields.get('id')
@@ -24,7 +57,7 @@ class User(UserMixin):
         self._email_address = fields.get('email_address')
         self._mobile_number = fields.get('mobile_number')
         self._password_changed_at = fields.get('password_changed_at')
-        self._permissions = fields.get('permissions')
+        self._set_permissions(fields.get('permissions', {}))
         self._auth_type = fields.get('auth_type')
         self._failed_login_count = fields.get('failed_login_count')
         self._state = fields.get('state')
@@ -33,6 +66,25 @@ class User(UserMixin):
         self.current_session_id = fields.get('current_session_id')
         self._organisations = fields.get('organisations', [])
 
+    def _set_permissions(self, permissions_by_service):
+        """
+        Permissions is a dict {'service_id': ['permission a', 'permission b', 'permission c']}
+
+        The api currently returns some granular permissions that we don't set or use separately (but may want
+        to in the future):
+        * send_texts, send_letters and send_emails become send_messages
+        * manage_user and manage_settings become
+        users either have all three permissions for a service or none of them, they're not helpful to distinguish
+        between on the front end. So lets collapse them into "send_messages" and "manage_service". If we want to split
+        them out later, we'll need to rework this function.
+        """
+
+        self._permissions = {
+            service: translate_permissions_from_db_to_admin_roles(permissions)
+            for service, permissions
+            in permissions_by_service.items()
+        }
+
     def get_id(self):
         return self.id
 
@@ -112,7 +164,7 @@ class User(UserMixin):
         unknown_permissions = set(permissions) - all_permissions
 
         if unknown_permissions:
-            raise TypeError('{} are not valid permissions'.format(unknown_permissions))
+            raise TypeError('{} are not valid permissions'.format(list(unknown_permissions)))
 
         # Only available to the platform admin user
         if admin_override and self.platform_admin:

app/notify_client/user_api_client.py

@@ -1,7 +1,10 @@
 from notifications_python_client.errors import HTTPError
 
 from app.notify_client import NotifyAdminAPIClient
-from app.notify_client.models import User
+from app.notify_client.models import (
+    User,
+    translate_permissions_from_admin_roles_to_db,
+)
 
 ALLOWED_ATTRIBUTES = {
     'name',
@@ -142,8 +145,9 @@ class UserApiClient(NotifyAdminAPIClient):
         return [User(data) for data in resp['data']]
 
     def add_user_to_service(self, service_id, user_id, permissions):
+        # permissions passed in are the combined admin roles, not db permissions
         endpoint = '/service/{}/users/{}'.format(service_id, user_id)
-        data = [{'permission': x} for x in permissions]
+        data = [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)]
         resp = self.post(endpoint, data=data)
         return User(resp['data'], max_failed_login_count=self.max_failed_login_count)
 
@@ -152,7 +156,8 @@ class UserApiClient(NotifyAdminAPIClient):
         return User(resp['data'], max_failed_login_count=self.max_failed_login_count)
 
     def set_user_permissions(self, user_id, service_id, permissions):
-        data = [{'permission': x} for x in permissions]
+        # permissions passed in are the combined admin roles, not db permissions
+        data = [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)]
         endpoint = '/user/{}/service/{}/permission'.format(user_id, service_id)
         self.post(endpoint, data=data)
 

tests/__init__.py

@@ -47,6 +47,7 @@ def user_json(
     mobile_number='+447700900986',
     password_changed_at=None,
     permissions={generate_uuid(): [
+        'view_activity',
         'send_texts',
         'send_emails',
         'send_letters',

tests/app/main/test_permissions.py

@@ -3,6 +3,10 @@ from flask import request
 from werkzeug.exceptions import Forbidden, Unauthorized
 
 from app.main.views.index import index
+from app.notify_client.models import (
+    translate_permissions_from_admin_roles_to_db,
+    translate_permissions_from_db_to_admin_roles,
+)
 from app.utils import user_has_permissions
 
 
@@ -159,3 +163,15 @@ def _user_with_permissions():
                  }
     user = User(user_data)
     return user
+
+
+def test_translate_permissions_from_db_to_admin_roles():
+    db_perms = ['send_texts', 'send_emails', 'send_letters', 'manage_templates', 'some_unknown_permission']
+    roles = translate_permissions_from_db_to_admin_roles(db_perms)
+    assert roles == {'send_messages', 'manage_templates', 'some_unknown_permission'}
+
+
+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'}

tests/app/notify_client/test_user_client.py

@@ -131,3 +131,29 @@ def test_client_passes_admin_url_when_sending_email_auth(
             'email_auth_link_host': 'http://localhost:6012',
         }
     )
+
+
+def test_client_converts_admin_permissions_to_db_permissions_on_edit(app_, mocker):
+    mock_post = mocker.patch('app.notify_client.user_api_client.UserApiClient.post')
+
+    user_api_client.set_user_permissions('user_id', 'service_id', permissions={'send_messages', 'view_activity'})
+
+    assert sorted(mock_post.call_args[1]['data'], key=lambda x: x['permission']) == sorted([
+        {'permission': 'send_texts'},
+        {'permission': 'send_emails'},
+        {'permission': 'send_letters'},
+        {'permission': 'view_activity'},
+    ], key=lambda x: x['permission'])
+
+
+def test_client_converts_admin_permissions_to_db_permissions_on_add_to_service(app_, mocker):
+    mock_post = mocker.patch('app.notify_client.user_api_client.UserApiClient.post', return_value={'data': {}})
+
+    user_api_client.add_user_to_service('service_id', 'user_id', permissions={'send_messages', 'view_activity'})
+
+    assert sorted(mock_post.call_args[1]['data'], key=lambda x: x['permission']) == sorted([
+        {'permission': 'send_texts'},
+        {'permission': 'send_emails'},
+        {'permission': 'send_letters'},
+        {'permission': 'view_activity'},
+    ], key=lambda x: x['permission'])