Merge pull request #358 from alphagov/fix-permissions

Add view_activity permissions

app/main/views/choose_service.py

@@ -10,14 +10,15 @@ from app.notify_client.api_client import ServicesBrowsableItem
 def choose_service():
     return render_template(
         'views/choose-service.html',
-        services=[ServicesBrowsableItem(x) for x in service_api_client.get_services()['data']]
+        services=[ServicesBrowsableItem(x) for x in
+                  service_api_client.get_services({'user_id': current_user.id})['data']]
     )
 
 
 @main.route("/services-or-dashboard")
 @login_required
 def show_all_services_or_dashboard():
-    services = service_api_client.get_services()['data']
+    services = service_api_client.get_services({'user_id': current_user.id})['data']
 
     if 1 == len(services):
         return redirect(url_for('.service_dashboard', service_id=services[0]['id']))

app/main/views/dashboard.py

@@ -13,7 +13,7 @@ from app.utils import user_has_permissions
 
 @main.route("/services/<service_id>/dashboard")
 @login_required
-@user_has_permissions()
+@user_has_permissions('view_activity', admin_override=True)
 def service_dashboard(service_id):
     templates = service_api_client.get_service_templates(service_id)['data']
     jobs = job_api_client.get_job(service_id)['data']

app/main/views/jobs.py

@@ -6,10 +6,7 @@ from flask import (
     render_template,
     abort,
     jsonify,
-    flash,
+    request
-    redirect,
-    request,
-    url_for
 )
 from flask_login import login_required
 from utils.template import Template
@@ -24,7 +21,7 @@ from app.utils import (
 
 @main.route("/services/<service_id>/jobs")
 @login_required
-@user_has_permissions()
+@user_has_permissions('view_activity', admin_override=True)
 def view_jobs(service_id):
     jobs = job_api_client.get_job(service_id)['data']
     return render_template(
@@ -36,7 +33,7 @@ def view_jobs(service_id):
 
 @main.route("/services/<service_id>/jobs/<job_id>")
 @login_required
-@user_has_permissions()
+@user_has_permissions('view_activity', admin_override=True)
 def view_job(service_id, job_id):
     service = service_api_client.get_service(service_id)['data']
     job = job_api_client.get_job(service_id, job_id)['data']
@@ -65,7 +62,7 @@ def view_job(service_id, job_id):
 
 @main.route("/services/<service_id>/jobs/<job_id>.json")
 @login_required
-@user_has_permissions()
+@user_has_permissions('view_activity')
 def view_job_updates(service_id, job_id):
     service = service_api_client.get_service(service_id)['data']
     job = job_api_client.get_job(service_id, job_id)['data']
@@ -93,7 +90,7 @@ def view_job_updates(service_id, job_id):
 
 @main.route('/services/<service_id>/notifications')
 @login_required
-@user_has_permissions()
+@user_has_permissions('view_activity', admin_override=True)
 def view_notifications(service_id):
     # TODO get the api to return count of pages as well.
     page = get_page_from_request()
@@ -122,7 +119,7 @@ def view_notifications(service_id):
 
 @main.route("/services/<service_id>/jobs/<job_id>/notification/<string:notification_id>")
 @login_required
-@user_has_permissions()
+@user_has_permissions('view_activity', admin_override=True)
 def view_notification(service_id, job_id, notification_id):
 
     now = time.strftime('%H:%M')

app/main/views/manage_users.py

@@ -27,13 +27,13 @@ from app.utils import user_has_permissions
 roles = {
     'send_messages': ['send_texts', 'send_emails', 'send_letters'],
     'manage_service': ['manage_users', 'manage_templates', 'manage_settings'],
-    'manage_api_keys': ['manage_api_keys', 'access_developer_docs']
+    'manage_api_keys': ['manage_api_keys']
 }
 
 
 @main.route("/services/<service_id>/users")
 @login_required
-@user_has_permissions()
+@user_has_permissions('view_activity', admin_override=True)
 def manage_users(service_id):
     return render_template(
         'views/manage-users.html',
@@ -57,13 +57,15 @@ def invite_user(service_id):
 
     if form.validate_on_submit():
         email_address = form.email_address.data
+        # 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
+        permissions = ','.join(role for role in roles.keys() if request.form.get(role) == 'y').join('view_activity')
         invited_user = invite_api_client.create_invite(
             current_user.id,
             service_id,
             email_address,
-            ','.join(
+            permissions
-                role for role in roles.keys() if request.form.get(role) == 'y'
-            )
         )
 
         flash('Invite sent to {}'.format(invited_user.email_address), 'default_with_tick')
@@ -95,7 +97,7 @@ def edit_user_permissions(service_id, user_id):
             user_id, service_id,
             permissions=set(chain.from_iterable(
                 permissions for role, permissions in roles.items() if form[role].data
-            ))
+            )) | {'view_activity'}
         )
         return redirect(url_for('.manage_users', service_id=service_id))
 

app/main/views/send.py

@@ -64,8 +64,12 @@ def get_page_headings(template_type):
 
 @main.route("/services/<service_id>/send/<template_type>", methods=['GET'])
 @login_required
-@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', 'manage_api_keys',
+@user_has_permissions('view_activity',
-                      admin_override=True, or_=True)
+                      'send_texts',
+                      'send_emails',
+                      'manage_templates',
+                      'manage_api_keys',
+                      admin_override=True, any_=True)
 def choose_template(service_id, template_type):
 
     service = service_api_client.get_service(service_id)['data']
@@ -140,7 +144,7 @@ def send_messages(service_id, template_id):
 
 @main.route("/services/<service_id>/send/<template_id>.csv", methods=['GET'])
 @login_required
-@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', or_=True)
+@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', any_=True)
 def get_example_csv(service_id, template_id):
     template = Template(service_api_client.get_service_template(service_id, template_id)['data'])
     # Good practice to use context managers
@@ -203,7 +207,7 @@ def send_message_to_self(service_id, template_id):
 
 @main.route("/services/<service_id>/send/<template_id>/from-api", methods=['GET'])
 @login_required
-@user_has_permissions('manage_api_keys', 'access_developer_docs')
+@user_has_permissions('manage_api_keys')
 def send_from_api(service_id, template_id):
     template = Template(
         service_api_client.get_service_template(service_id, template_id)['data']

app/notify_client/models.py

@@ -82,7 +82,7 @@ class User(UserMixin):
     def permissions(self, permissions):
         raise AttributeError("Read only property")
 
-    def has_permissions(self, permissions=[], or_=False, admin_override=False):
+    def has_permissions(self, permissions=[], any_=False, admin_override=False):
         # Only available to the platform admin user
         if admin_override and self.platform_admin:
             return True
@@ -95,7 +95,7 @@ class User(UserMixin):
         # Service id is always set on the request for service specific views.
         service_id = request.view_args.get('service_id', None)
         if service_id in self._permissions:
-            if or_:
+            if any_:
                 return any([x in self._permissions[service_id] for x in permissions])
             return set(self._permissions[service_id]) >= set(permissions)
         return False

app/templates/main_nav.html

@@ -2,15 +2,17 @@
   <h2 class="navigation-service-name">
     <a href="{{ url_for('.service_dashboard', service_id=service_id) }}">{{ session.get('service_name', 'Service') }}</a>
   </h2>
+  {% if current_user.has_permissions(['view_activity'], admin_override=True) %}
   <ul>
     <li><a href="{{ url_for('.view_notifications', service_id=service_id) }}">View activity</a></li>
   </ul>
+  {% endif %}
   {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
   <ul>
     <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">Send text messages</a></li>
     <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='email') }}">Send emails</a></li>
   </ul>
-  {% elif current_user.has_permissions(['manage_templates','manage_api_keys'], admin_override=True, or_=True) %}
+  {% elif current_user.has_permissions(['view_activity', 'manage_templates','manage_api_keys'], admin_override=True, any_=True) %}
   <ul>
     <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='sms') }}">Text message templates</a></li>
     <li><a href="{{ url_for('.choose_template', service_id=service_id, template_type='email') }}">Email templates</a></li>
@@ -21,12 +23,13 @@
     <li><a href="{{ url_for('.manage_users', service_id=service_id) }}">Manage team</a></li>
     <li><a href="{{ url_for('.service_settings', service_id=service_id) }}">Manage settings</a></li>
   </ul>
-  {% else %}
+  {% endif %}
+  {% if current_user.has_permissions(['view_activity']) %}
   <ul>
     <li><a href="{{ url_for('.manage_users', service_id=service_id) }}">View team members</a></li>
   </ul>
   {% endif %}
-  {% if current_user.has_permissions(['manage_api_keys', 'access_developer_docs']) %}
+  {% if current_user.has_permissions(['manage_api_keys']) %}
   <ul>
     <li><a href="{{ url_for('.api_keys', service_id=service_id) }}">Manage API keys</a></li>
   </ul>

app/templates/views/choose-template.html

@@ -14,7 +14,7 @@
 
     <h1 class="heading-large">{{ page_heading }}</h1>
 
-    {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) %}
+    {% if current_user.has_permissions(permissions=['manage_templates'], any_=True) %}
       <a href="{{ url_for('.add_service_template', service_id=service_id, template_type=template_type) }}" class="button">Add a new template</a>
     {% else %}
       <p>You need to ask your service manager to add templates before you can send messages</p>
@@ -34,7 +34,7 @@
     </div>
 
     {% if not has_jobs %}
-      {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters'], or_=True) %}
+      {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters'], any_=True) %}
       {{ banner(
         """
           Send yourself a test
@@ -76,7 +76,7 @@
               <a href="{{ url_for(".send_messages", service_id=service_id, template_id=template.id) }}">Send a batch</a>
               <a href="{{ url_for(".send_message_to_self", service_id=service_id, template_id=template.id) }}">Send yourself a test</a>
             {% endif %}
-            {% if current_user.has_permissions(permissions=['manage_api_keys', 'access_developer_docs']) %}
+            {% if current_user.has_permissions(permissions=['manage_api_keys']) %}
               <a href="{{ url_for(".send_from_api", service_id=service_id, template_id=template.id) }}">API integration</a>
             {% endif %}
           </div>

app/templates/views/dashboard/dashboard.html

@@ -11,7 +11,9 @@
   {% endif %}
 
   {% if not jobs %}
-    {% include 'views/dashboard/get-started.html' %}
+    {% if current_user.has_permissions(['manage_templates','send_texts', 'send_emails', 'send_letters']) %}
+      {% include 'views/dashboard/get-started.html' %}
+    {% endif%}
   {% else %}
 
     <div

app/templates/views/manage-users.html

@@ -41,7 +41,7 @@ Manage users – GOV.UK Notify
     {% endcall %}
     {{ boolean_field(item.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters'])) }}
     {{ boolean_field(item.has_permissions(permissions=['manage_users', 'manage_templates', 'manage_settings'])) }}
-    {{ boolean_field(item.has_permissions(permissions=['manage_api_keys', 'access_developer_docs'])) }}
+    {{ boolean_field(item.has_permissions(permissions=['manage_api_keys'])) }}
     {% call field(align='right') %}
       {% if current_user.has_permissions(['manage_users']) %}
         {% if current_user.id != item.id %}
@@ -60,10 +60,12 @@ Manage users – GOV.UK Notify
       {% endcall %}
       {{ boolean_field(item.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters'])) }}
       {{ boolean_field(item.has_permissions(permissions=['manage_users', 'manage_templates', 'manage_settings'])) }}
-      {{ boolean_field(item.has_permissions(permissions=['manage_api_keys', 'access_developer_docs'])) }}
+      {{ boolean_field(item.has_permissions(permissions=['manage_api_keys'])) }}
       {% if item.status == 'pending' %}
         {% call field(align='right') %}
-          <a href="{{ url_for('.cancel_invited_user', service_id=service_id, invited_user_id=item.id)}}">Cancel invitation</a>
+          {% if current_user.has_permissions(['manage_users']) %}
+            <a href="{{ url_for('.cancel_invited_user', service_id=service_id, invited_user_id=item.id)}}">Cancel invitation</a>
+          {% endif %}
         {% endcall %}
       {% else %}
         {% call field() %}

app/templates/views/service_dashboard.html

@@ -25,7 +25,7 @@
     </ul>
 
     {% if not template_count and not jobs %}
-      {% if current_user.has_permissions(['manage_templates', 'send_texts', 'send_emails', 'send_letters'], or_=True, admin_override=True) %}
+      {% if current_user.has_permissions(['manage_templates', 'send_texts', 'send_emails', 'send_letters'], any_=True, admin_override=True) %}
         {% call banner_wrapper(subhead='Get started', type="tip") %}
           <ol>
             {% if current_user.has_permissions(['manage_templates'], admin_override=True) %}

app/utils.py

@@ -30,13 +30,13 @@ class BrowsableItem(object):
         pass
 
 
-def user_has_permissions(*permissions, admin_override=False, or_=False):
+def user_has_permissions(*permissions, admin_override=False, any_=False):
     def wrap(func):
         @wraps(func)
         def wrap_func(*args, **kwargs):
             from flask_login import current_user
             if current_user and current_user.has_permissions(permissions=permissions,
-                                                             admin_override=admin_override, or_=or_):
+                                                             admin_override=admin_override, any_=any_):
                 return func(*args, **kwargs)
             else:
                 abort(403)

environment_test.sh

@@ -1,6 +1,6 @@
 export NOTIFY_ADMIN_ENVIRONMENT='config.Test'
 export ADMIN_CLIENT_SECRET='dev-notify-secret-key'
 export ADMIN_CLIENT_USER_NAME='dev-notify-admin'
-export API_HOST_NAME='http://localhost:6011'
+export API_HOST_NAME='http://localhost:6311'
 export DANGEROUS_SALT='dev-notify-salt'
 export SECRET_KEY='dev-notify-secret-key'

tests/__init__.py

@@ -5,12 +5,14 @@ from flask_login import login_user
 
 
 class TestClient(FlaskClient):
-    def login(self, user):
+    def login(self, user, mocker=None, service=None):
         # Skipping authentication here and just log them in
         with self.session_transaction() as session:
             session['user_id'] = user.id
             session['_fresh'] = True
-
+        if mocker and service:
+            mocker.patch('app.user_api_client.get_user', return_value=user)
+            mocker.patch('app.service_api_client.get_service', return_value={'data': service})
         login_user(user, remember=True)
 
     def login_fresh(self):

tests/app/main/test_permissions.py

@@ -5,12 +5,12 @@ from werkzeug.exceptions import Forbidden
 from flask import request
 
 
-def _test_permissions(app_, usr, permissions, service_id, will_succeed, or_=False, admin_override=False):
+def _test_permissions(app_, usr, permissions, service_id, will_succeed, any_=False, admin_override=False):
     with app_.test_request_context() as ctx:
         request.view_args.update({'service_id': service_id})
         with app_.test_client() as client:
             client.login(usr)
-            decorator = user_has_permissions(*permissions, or_=or_, admin_override=admin_override)
+            decorator = user_has_permissions(*permissions, any_=any_, admin_override=admin_override)
             decorated_index = decorator(index)
             if will_succeed:
                 response = decorated_index()
@@ -54,7 +54,7 @@ def test_user_has_permissions_or(app_, mocker):
         ['something', 'manage_users'],
         '',
         True,
-        or_=True)
+        any_=True)
 
 
 def test_user_has_permissions_multiple(app_,

tests/app/main/views/test_api_keys.py

@@ -161,6 +161,6 @@ def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, m
                 "GET",
                 403,
                 url_for(route, service_id=service_one['id'], key_id=123),
-                ['blah'],
+                ['view_activity'],
                 api_user_active,
                 service_one)

tests/app/main/views/test_choose_services.py

@@ -1,5 +1,4 @@
 from flask import url_for
-import pytest
 
 
 def test_should_show_choose_services_page(app_,

tests/app/main/views/test_dashboard.py

@@ -27,12 +27,8 @@ def _test_dashboard_menu(mocker, app_, usr, service, permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
             usr._permissions[str(service['id'])] = permissions
-            mocker.patch(
+            mocker.patch('app.user_api_client.check_verify_code', return_value=(True, ''))
-                'app.user_api_client.check_verify_code',
+            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
-                return_value=(True, ''))
-            mocker.patch(
-                'app.service_api_client.get_services',
-                return_value={'data': []})
             mocker.patch('app.user_api_client.get_user', return_value=usr)
             mocker.patch('app.user_api_client.get_user_by_email', return_value=usr)
             mocker.patch('app.service_api_client.get_service', return_value={'data': service})
@@ -48,7 +44,7 @@ def test_menu_send_messages(mocker, app_, api_user_active, service_one, mock_get
             app_,
             api_user_active,
             service_one,
-            ['send_texts', 'send_emails', 'send_letters'])
+            ['view_activity', 'send_texts', 'send_emails', 'send_letters'])
         page = resp.get_data(as_text=True)
         assert url_for(
             'main.choose_template',
@@ -58,12 +54,12 @@ def test_menu_send_messages(mocker, app_, api_user_active, service_one, mock_get
             'main.choose_template',
             service_id=service_one['id'],
             template_type='sms')in page
-
+        assert url_for('main.view_notifications', service_id=service_one['id']) in page
         assert url_for('main.manage_users', service_id=service_one['id']) in page
-        assert url_for('main.service_settings', service_id=service_one['id']) not in page
+        assert url_for('main.documentation') in page
 
+        assert url_for('main.service_settings', service_id=service_one['id']) not in page
         assert url_for('main.api_keys', service_id=service_one['id']) not in page
-        assert url_for('main.documentation', service_id=service_one['id']) not in page
         assert url_for('main.show_all_services') not in page
 
 
@@ -74,7 +70,7 @@ def test_menu_manage_service(mocker, app_, api_user_active, service_one, mock_ge
             app_,
             api_user_active,
             service_one,
-            ['manage_users', 'manage_templates', 'manage_settings'])
+            ['view_activity', 'manage_users', 'manage_templates', 'manage_settings'])
         page = resp.get_data(as_text=True)
         assert url_for(
             'main.choose_template',
@@ -84,9 +80,10 @@ def test_menu_manage_service(mocker, app_, api_user_active, service_one, mock_ge
             'main.choose_template',
             service_id=service_one['id'],
             template_type='sms') in page
-
+        assert url_for('main.view_notifications', service_id=service_one['id']) in page
         assert url_for('main.manage_users', service_id=service_one['id']) in page
         assert url_for('main.service_settings', service_id=service_one['id']) in page
+        assert url_for('main.documentation') in page
 
         assert url_for('main.api_keys', service_id=service_one['id']) not in page
         assert url_for('main.show_all_services') not in page
@@ -99,7 +96,7 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g
             app_,
             api_user_active,
             service_one,
-            ['manage_api_keys', 'access_developer_docs'])
+            ['view_activity', 'manage_api_keys'])
         page = resp.get_data(as_text=True)
         assert url_for(
             'main.choose_template',
@@ -109,7 +106,7 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g
             'main.choose_template',
             service_id=service_one['id'],
             template_type='sms') in page
-
+        assert url_for('main.view_notifications', service_id=service_one['id']) in page
         assert url_for('main.manage_users', service_id=service_one['id']) in page
         assert url_for('main.service_settings', service_id=service_one['id']) not in page
         assert url_for('main.show_all_services') not in page
@@ -159,6 +156,6 @@ def test_route_for_service_permissions(mocker,
                 url_for(
                     route,
                     service_id=service_one['id']),
-                [],
+                ['view_activity'],
                 api_user_active,
                 service_one)

tests/app/main/views/test_manage_users.py

@@ -14,8 +14,7 @@ def test_should_show_overview_page(
     service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            _mocks_for_test_manage_users(mocker, active_user_with_permissions, service)
+            client.login(active_user_with_permissions, mocker, service)
-            client.login(active_user_with_permissions)
             mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
             response = client.get(url_for('main.manage_users', service_id=service['id']))
 
@@ -27,16 +26,12 @@ def test_should_show_overview_page(
 def test_should_show_page_for_one_user(
     app_,
     active_user_with_permissions,
-    mocker,
+    mocker
-    mock_login
 ):
     service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            client.login(active_user_with_permissions, mocker, service)
-            mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
-            client.login(active_user_with_permissions)
             response = client.get(url_for('main.edit_user_permissions', service_id=service['id'], user_id=0))
 
         assert response.status_code == 200
@@ -45,7 +40,6 @@ def test_should_show_page_for_one_user(
 def test_edit_user_permissions(
     app_,
     active_user_with_permissions,
-    mock_login,
     mocker,
     mock_get_invites_for_service,
     mock_set_user_permissions
@@ -54,10 +48,7 @@ def test_edit_user_permissions(
     with app_.test_request_context():
         with app_.test_client() as client:
 
-            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            client.login(active_user_with_permissions, mocker, service)
-            mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
-            client.login(active_user_with_permissions)
             response = client.post(url_for(
                 'main.edit_user_permissions', service_id=service['id'], user_id=active_user_with_permissions.id
             ), data={'email_address': active_user_with_permissions.email_address,
@@ -80,7 +71,7 @@ def test_edit_user_permissions(
                 'manage_templates',
                 'manage_settings',
                 'manage_api_keys',
-                'access_developer_docs'
+                'view_activity'
             }
         )
 
@@ -97,11 +88,10 @@ def test_edit_some_user_permissions(
     data = [InvitedUser(**sample_invite)]
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(active_user_with_permissions)
+            client.login(active_user_with_permissions, mocker, service)
             service_id = service['id']
 
             mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
-            _mocks_for_test_manage_users(mocker, active_user_with_permissions, service)
             response = client.post(url_for(
                 'main.edit_user_permissions', service_id=service_id, user_id=active_user_with_permissions.id
             ), data={'email_address': active_user_with_permissions.email_address,
@@ -119,17 +109,12 @@ def test_edit_some_user_permissions(
             permissions={
                 'send_texts',
                 'send_emails',
-                'send_letters'
+                'send_letters',
+                'view_activity'
             }
         )
 
 
-def _mocks_for_test_manage_users(mocker, active_user_with_permissions, service):
-    mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
-    mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-    mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
-
-
 def test_should_show_page_for_inviting_user(
     app_,
     active_user_with_permissions,
@@ -138,8 +123,7 @@ def test_should_show_page_for_inviting_user(
     service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            _mocks_for_test_manage_users(mocker, active_user_with_permissions, service)
+            client.login(active_user_with_permissions, mocker, service)
-            client.login(active_user_with_permissions)
             response = client.get(url_for('main.invite_user', service_id=service['id']))
 
         assert 'Invite a team member' in response.get_data(as_text=True)
@@ -159,8 +143,7 @@ def test_invite_user(
     data = [InvitedUser(**sample_invite)]
     with app_.test_request_context():
         with app_.test_client() as client:
-            _mocks_for_test_manage_users(mocker, active_user_with_permissions, service)
+            client.login(active_user_with_permissions, mocker, service)
-            client.login(active_user_with_permissions)
             mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
             mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
             mocker.patch('app.invite_api_client.create_invite', return_value=InvitedUser(**sample_invite))
@@ -190,8 +173,7 @@ def test_cancel_invited_user_cancels_user_invitations(app_,
             import uuid
             invited_user_id = uuid.uuid4()
             service = service_1(active_user_with_permissions)
-            _mocks_for_test_manage_users(mocker, active_user_with_permissions, service)
+            client.login(active_user_with_permissions, mocker, service)
-            client.login(active_user_with_permissions)
             response = client.get(url_for('main.cancel_invited_user', service_id=service['id'],
                                           invited_user_id=invited_user_id))
 
@@ -207,8 +189,7 @@ def test_manage_users_shows_invited_user(app_,
     data = [InvitedUser(**sample_invite)]
     with app_.test_request_context():
         with app_.test_client() as client:
-            _mocks_for_test_manage_users(mocker, active_user_with_permissions, service)
+            client.login(active_user_with_permissions, mocker, service)
-            client.login(active_user_with_permissions)
 
             mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
             mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
@@ -227,7 +208,6 @@ def test_manage_users_shows_invited_user(app_,
 def test_manage_users_does_not_show_accepted_invite(app_,
                                                     mocker,
                                                     active_user_with_permissions,
-                                                    mock_login,
                                                     sample_invite):
 
     import uuid
@@ -238,10 +218,7 @@ def test_manage_users_does_not_show_accepted_invite(app_,
     service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            client.login(active_user_with_permissions, mocker, service)
-            mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
-            client.login(active_user_with_permissions)
             mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions])
             mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
 
@@ -257,19 +234,14 @@ def test_manage_users_does_not_show_accepted_invite(app_,
 
 def test_user_cant_invite_themselves(
     app_,
-    mock_login,
     mocker,
     active_user_with_permissions,
-    mock_create_invite,
+    mock_create_invite
-    mock_get_invites_for_service
 ):
     service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            client.login(active_user_with_permissions, mocker, service)
-            mocker.patch('app.service_api_client.get_service', return_value={'data': service})
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
-            client.login(active_user_with_permissions)
             response = client.post(
                 url_for('main.invite_user', service_id=service['id']),
                 data={'email_address': active_user_with_permissions.email_address,
@@ -284,19 +256,16 @@ def test_user_cant_invite_themselves(
         assert page.h1.string.strip() == 'Invite a team member'
         form_error = page.find('span', class_='error-message').string.strip()
         assert form_error == "You can't send an invitation to yourself"
+        assert not mock_create_invite.called
 
 
 def test_no_permission_manage_users_page(app_,
                                          service_one,
                                          api_user_active,
-                                         mock_login,
+                                         mocker):
-                                         mock_get_user,
-                                         mock_get_service,
-                                         mock_get_users_by_service,
-                                         mock_get_invites_for_service):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(api_user_active, mocker, service_one)
             response = client.get(url_for('main.manage_users', service_id=service_one['id']))
             resp_text = response.get_data(as_text=True)
             assert url_for('.invite_user', service_id=service_one['id']) not in resp_text
@@ -305,45 +274,39 @@ def test_no_permission_manage_users_page(app_,
 
 
 def test_get_remove_user_from_service(app_,
-                                      api_user_active,
+                                      active_user_with_permissions,
-                                      mock_login,
+                                      service_one,
-                                      mock_get_user_by_email,
+                                      mocker):
-                                      mock_get_service,
-                                      mock_get_users_by_service,
-                                      mock_get_user,
-                                      mock_has_permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
-            service = mock_get_service("12345")['data']
+            client.login(active_user_with_permissions, mocker, service_one)
-            client.login(api_user_active)
             response = client.get(
                 url_for(
                     'main.remove_user_from_service',
-                    service_id=service['id'],
+                    service_id=service_one['id'],
-                    user_id=api_user_active.id))
+                    user_id=active_user_with_permissions.id))
             assert response.status_code == 200
             assert "Are you sure you want to remove" in response.get_data(as_text=True)
             assert "Remove user from service" in response.get_data(as_text=True)
 
 
 def test_remove_user_from_service(app_,
-                                  api_user_active,
+                                  active_user_with_permissions,
-                                  mock_login,
+                                  service_one,
-                                  mock_get_user_by_email,
+                                  mocker,
-                                  mock_get_service,
                                   mock_get_users_by_service,
                                   mock_get_user,
-                                  mock_has_permissions,
                                   mock_remove_user_from_service):
     with app_.test_request_context():
         with app_.test_client() as client:
-            service = mock_get_service("12345")['data']
+            client.login(active_user_with_permissions, mocker, service_one)
-            client.login(api_user_active)
             response = client.post(
                 url_for(
                     'main.remove_user_from_service',
-                    service_id=service['id'],
+                    service_id=service_one['id'],
-                    user_id=api_user_active.id))
+                    user_id=active_user_with_permissions.id))
             assert response.status_code == 302
             assert response.location == url_for(
-                'main.manage_users', service_id=service['id'], _external=True)
+                'main.manage_users', service_id=service_one['id'], _external=True)
+            mock_remove_user_from_service.assert_called_once_with(service_one['id'],
+                                                                  str(active_user_with_permissions.id))

tests/app/main/views/test_send.py

@@ -442,7 +442,7 @@ def test_route_choose_template_manage_api_keys_permissions(mocker,
                 'main.choose_template',
                 service_id=service_one['id'],
                 template_type='sms'),
-            ['manage_api_keys', 'access_developer_docs'],
+            ['manage_api_keys'],
             api_user_active,
             service_one)
         page = resp.get_data(as_text=True)

tests/app/main/views/test_templates.py

@@ -154,6 +154,27 @@ def test_route_permissions(mocker,
                 service_one)
 
 
+def test_route_permissions_for_choose_template(mocker,
+                                               app_,
+                                               api_user_active,
+                                               service_one,
+                                               mock_get_service_templates):
+    mocker.patch('app.job_api_client.get_job')
+    with app_.test_request_context():
+        validate_route_permission(
+            mocker,
+            app_,
+            "GET",
+            200,
+            url_for(
+                'main.choose_template',
+                service_id=service_one['id'],
+                template_type='sms'),
+            ['view_activity'],
+            api_user_active,
+            service_one)
+
+
 def test_route_invalid_permissions(mocker,
                                    app_,
                                    api_user_active,
@@ -175,6 +196,6 @@ def test_route_invalid_permissions(mocker,
                     service_id=service_one['id'],
                     template_type='sms',
                     template_id=123),
-                ['blah'],
+                ['view_activity'],
                 api_user_active,
                 service_one)

tests/conftest.py

@@ -158,13 +158,13 @@ def mock_get_service_statistics(mocker):
 
 @pytest.fixture(scope='function')
 def mock_get_service_template(mocker):
-    def _create(service_id, template_id):
+    def _get(service_id, template_id):
         template = template_json(
             service_id, template_id, "Two week reminder", "sms", "Your vehicle tax is about to expire")
         return {'data': template}
 
     return mocker.patch(
-        'app.service_api_client.get_service_template', side_effect=_create)
+        'app.service_api_client.get_service_template', side_effect=_get)
 
 
 @pytest.fixture(scope='function')
@@ -279,7 +279,8 @@ def api_user_active():
                  'mobile_number': '+4412341234',
                  'state': 'active',
                  'failed_login_count': 0,
-                 'permissions': {}
+                 'permissions': {},
+                 'platform_admin': False
                  }
     user = User(user_data)
     return user
@@ -303,7 +304,7 @@ def active_user_with_permissions():
                                                   'manage_templates',
                                                   'manage_settings',
                                                   'manage_api_keys',
-                                                  'access_developer_docs']},
+                                                  'view_activity']},
                  'platform_admin': False
                  }
     user = User(user_data)
@@ -659,7 +660,7 @@ def mock_get_notifications_with_previous_next(mocker):
 
 @pytest.fixture(scope='function')
 def mock_has_permissions(mocker):
-    def _has_permission(permissions=None, or_=False, admin_override=False):
+    def _has_permission(permissions=None, any_=False, admin_override=False):
         return True
     return mocker.patch(
         'app.notify_client.user_api_client.User.has_permissions',