From b28fbc16d7bf715f8c3c283fef84d711c2615243 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Thu, 17 Mar 2016 10:46:47 +0000
Subject: [PATCH 1/7] A platform admin user is able to see a list of all
 services. Each service on the list is linked to the dashboard page of the
 service. The platform admin user can see/edit templates, see/invite users,
 see/edit service settings. The platform admin user can not send messages,
 see/edit api keys and developer docs.

---
 app/__init__.py                            |  6 +--
 app/main/__init__.py                       |  3 +-
 app/main/dao/services_dao.py               | 26 +++++------
 app/main/dao/templates_dao.py              | 12 ++---
 app/main/views/add_service.py              |  4 +-
 app/main/views/all_services.py             | 15 ++++++
 app/main/views/jobs.py                     |  1 -
 app/main/views/manage_users.py             |  9 ++--
 app/main/views/send.py                     |  2 +-
 app/main/views/service_settings.py         | 17 ++++---
 app/main/views/templates.py                |  6 +--
 app/notify_client/api_client.py            | 26 ++---------
 app/notify_client/models.py                |  9 +++-
 app/templates/main_nav.html                |  9 +++-
 app/templates/views/all-services.html      | 16 +++++++
 app/templates/views/choose-template.html   |  8 ++--
 app/templates/views/service_dashboard.html |  2 +-
 app/utils.py                               |  6 ++-
 tests/__init__.py                          |  4 +-
 tests/app/main/views/test_dashboard.py     | 20 ++++++--
 tests/app/main/views/test_sign_out.py      |  3 +-
 tests/conftest.py                          | 54 +++++++++++++---------
 22 files changed, 154 insertions(+), 104 deletions(-)
 create mode 100644 app/main/views/all_services.py
 create mode 100644 app/templates/views/all-services.html

diff --git a/app/__init__.py b/app/__init__.py
index d0ce1d245..b7cd83520 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -12,7 +12,7 @@ from pygments.lexers import JavascriptLexer
 from pygments.formatters import HtmlFormatter
 from werkzeug.exceptions import abort
 
-from app.notify_client.api_client import NotificationsAdminAPIClient
+from app.notify_client.api_client import ServiceAPIClient
 from app.notify_client.api_key_api_client import ApiKeyApiClient
 from app.notify_client.user_api_client import UserApiClient
 from app.notify_client.job_api_client import JobApiClient
@@ -29,7 +29,7 @@ from utils import logging
 login_manager = LoginManager()
 csrf = CsrfProtect()
 
-notifications_api_client = NotificationsAdminAPIClient()
+service_api_client = ServiceAPIClient()
 user_api_client = UserApiClient()
 api_key_api_client = ApiKeyApiClient()
 job_api_client = JobApiClient()
@@ -48,7 +48,7 @@ def create_app(config_name, config_overrides=None):
     logging.init_app(application)
     init_csrf(application)
 
-    notifications_api_client.init_app(application)
+    service_api_client.init_app(application)
     user_api_client.init_app(application)
     api_key_api_client.init_app(application)
     job_api_client.init_app(application)
diff --git a/app/main/__init__.py b/app/main/__init__.py
index 39f95b71d..3a0af5a11 100644
--- a/app/main/__init__.py
+++ b/app/main/__init__.py
@@ -22,5 +22,6 @@ from app.main.views import (
     choose_service,
     api_keys,
     manage_users,
-    invites
+    invites,
+    all_services
 )
diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py
index 39417fceb..69b8946b1 100644
--- a/app/main/dao/services_dao.py
+++ b/app/main/dao/services_dao.py
@@ -1,10 +1,10 @@
 from flask import url_for, current_app
-from app import notifications_api_client
+from app import service_api_client
 from app.utils import BrowsableItem
 
 
 def update_service(service):
-    return notifications_api_client.update_service(
+    return service_api_client.update_service(
         service['id'],
         service['name'],
         service['active'],
@@ -14,24 +14,24 @@ def update_service(service):
 
 
 def get_service_by_id(id_):
-    return notifications_api_client.get_service(id_)
+    return service_api_client.get_service(id_)
 
 
 def get_service_by_id_or_404(id_):
-    return notifications_api_client.get_service(id_)['data']
+    return service_api_client.get_service(id_)['data']
 
 
 def get_services(user_id=None):
     if user_id:
-        return notifications_api_client.get_services({'user_id': str(user_id)})
+        return service_api_client.get_services({'user_id': str(user_id)})
     else:
-        return notifications_api_client.get_services()
+        return service_api_client.get_services()
 
 
 def unrestrict_service(service_id):
-    resp = notifications_api_client.get_service(service_id)
+    resp = service_api_client.get_service(service_id)
     if resp['data']['restricted']:
-        resp = notifications_api_client.update_service(
+        resp = service_api_client.update_service(
             service_id,
             resp['data']['name'],
             resp['data']['active'],
@@ -41,9 +41,9 @@ def unrestrict_service(service_id):
 
 
 def activate_service(service_id):
-    resp = notifications_api_client.get_service(service_id)
+    resp = service_api_client.get_service(service_id)
     if not resp['data']['active']:
-        resp = notifications_api_client.update_service(
+        resp = service_api_client.update_service(
             service_id,
             resp['data']['name'],
             True,
@@ -54,7 +54,7 @@ def activate_service(service_id):
 
 # TODO Fix when functionality is added to the api.
 def find_service_by_service_name(service_name, user_id=None):
-    resp = notifications_api_client.get_services(user_id)
+    resp = service_api_client.get_services(user_id)
     retval = None
     for srv_json in resp['data']:
         if srv_json['name'] == service_name:
@@ -64,11 +64,11 @@ def find_service_by_service_name(service_name, user_id=None):
 
 
 def delete_service(id_):
-    return notifications_api_client.delete_service(id_)
+    return service_api_client.delete_service(id_)
 
 
 def find_all_service_names(user_id=None):
-    resp = notifications_api_client.get_services(user_id)
+    resp = service_api_client.get_services(user_id)
     return [x['name'] for x in resp['data']]
 
 
diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py
index c6a22ccf2..63e5466bf 100644
--- a/app/main/dao/templates_dao.py
+++ b/app/main/dao/templates_dao.py
@@ -1,28 +1,28 @@
 from flask import url_for
-from app import notifications_api_client
+from app import service_api_client
 from app.utils import BrowsableItem
 
 
 def insert_service_template(name, type_, content, service_id, subject=None):
-    return notifications_api_client.create_service_template(
+    return service_api_client.create_service_template(
         name, type_, content, service_id, subject)
 
 
 def update_service_template(id_, name, type_, content, service_id, subject=None):
-    return notifications_api_client.update_service_template(
+    return service_api_client.update_service_template(
         id_, name, type_, content, service_id)
 
 
 def get_service_templates(service_id):
-    return notifications_api_client.get_service_templates(service_id)
+    return service_api_client.get_service_templates(service_id)
 
 
 def get_service_template_or_404(service_id, template_id):
-    return notifications_api_client.get_service_template(service_id, template_id)
+    return service_api_client.get_service_template(service_id, template_id)
 
 
 def delete_service_template(service_id, template_id):
-    return notifications_api_client.delete_service_template(
+    return service_api_client.delete_service_template(
         service_id, template_id)
 
 
diff --git a/app/main/views/add_service.py b/app/main/views/add_service.py
index 415ef4c30..4ab30ed55 100644
--- a/app/main/views/add_service.py
+++ b/app/main/views/add_service.py
@@ -15,7 +15,7 @@ from app.notify_client.models import InvitedUser
 from app import (
     invite_api_client,
     user_api_client,
-    notifications_api_client)
+    service_api_client)
 
 
 @main.route("/add-service", methods=['GET', 'POST'])
@@ -36,7 +36,7 @@ def add_service():
     heading = 'Which service do you want to set up notifications for?'
     if form.validate_on_submit():
         session['service_name'] = form.name.data
-        service_id = notifications_api_client.create_service(
+        service_id = service_api_client.create_service(
             session['service_name'], False, current_app.config['DEFAULT_SERVICE_LIMIT'], True, session['user_id'])
 
         return redirect(url_for('main.service_dashboard', service_id=service_id))
diff --git a/app/main/views/all_services.py b/app/main/views/all_services.py
new file mode 100644
index 000000000..0efd1171d
--- /dev/null
+++ b/app/main/views/all_services.py
@@ -0,0 +1,15 @@
+from flask import render_template
+from flask_login import login_required
+
+from app import service_api_client
+from app.main import main
+from app.main.dao import services_dao
+from app.utils import user_has_permissions
+
+
+@main.route("/all-services")
+@login_required
+@user_has_permissions(None, admin_override=True)
+def show_all_services():
+    services = [services_dao.ServicesBrowsableItem(x) for x in service_api_client.get_services()['data']]
+    return render_template('views/all-services.html', services=services)
diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py
index 9847ef2cc..7bfe5247a 100644
--- a/app/main/views/jobs.py
+++ b/app/main/views/jobs.py
@@ -4,7 +4,6 @@ import time
 
 from flask import (
     render_template,
-    abort,
     jsonify
 )
 from flask_login import login_required
diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py
index f40b81637..4ad38587c 100644
--- a/app/main/views/manage_users.py
+++ b/app/main/views/manage_users.py
@@ -2,7 +2,6 @@ from flask import (
     request,
     render_template,
     redirect,
-    abort,
     url_for,
     flash)
 
@@ -24,7 +23,7 @@ from app.utils import user_has_permissions
 
 @main.route("/services/<service_id>/users")
 @login_required
-@user_has_permissions('manage_users', 'manage_templates', 'manage_settings')
+@user_has_permissions('manage_users', 'manage_templates', 'manage_settings', admin_override=True)
 def manage_users(service_id):
     users = user_api_client.get_users_for_service(service_id=service_id)
     invited_users = invite_api_client.get_invites_for_service(service_id=service_id)
@@ -41,7 +40,7 @@ def manage_users(service_id):
 
 @main.route("/services/<service_id>/users/invite", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_users', 'manage_templates', 'manage_settings')
+@user_has_permissions('manage_users', 'manage_templates', 'manage_settings', admin_override=True)
 def invite_user(service_id):
 
     service = get_service_by_id(service_id)
@@ -64,7 +63,7 @@ def invite_user(service_id):
 
 @main.route("/services/<service_id>/users/<user_id>", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_users', 'manage_templates', 'manage_settings')
+@user_has_permissions('manage_users', 'manage_templates', 'manage_settings', admin_override=True)
 def edit_user_permissions(service_id, user_id):
     # TODO we should probably using the service id here in the get user
     # call as well. eg. /user/<user_id>?&service_id=service_id
@@ -101,7 +100,7 @@ def edit_user_permissions(service_id, user_id):
 
 
 @main.route("/services/<service_id>/cancel-invited-user/<invited_user_id>", methods=['GET'])
-@user_has_permissions('manage_users', 'manage_templates', 'manage_settings')
+@user_has_permissions('manage_users', 'manage_templates', 'manage_settings', admin_override=True)
 def cancel_invited_user(service_id, invited_user_id):
     invite_api_client.cancel_invited_user(service_id=service_id, invited_user_id=invited_user_id)
 
diff --git a/app/main/views/send.py b/app/main/views/send.py
index aab52a6cf..ed5f6e02c 100644
--- a/app/main/views/send.py
+++ b/app/main/views/send.py
@@ -66,7 +66,7 @@ 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', or_=True)
+@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', admin_override=True, or_=True)
 def choose_template(service_id, template_type):
 
     service = services_dao.get_service_by_id_or_404(service_id)
diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 6c7232e77..f596ee1dd 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -3,7 +3,6 @@ from flask import (
     redirect,
     request,
     url_for,
-    abort,
     session,
     flash
 )
@@ -29,7 +28,7 @@ from app.main.forms import ConfirmPasswordForm, ServiceNameForm
 
 @main.route("/services/<service_id>/service-settings")
 @login_required
-@user_has_permissions('manage_settings')
+@user_has_permissions('manage_settings', admin_override=True)
 def service_settings(service_id):
     service = get_service_by_id(service_id)['data']
 
@@ -42,7 +41,7 @@ def service_settings(service_id):
 
 @main.route("/services/<service_id>/service-settings/name", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_settings')
+@user_has_permissions('manage_settings', admin_override=True)
 def service_name_change(service_id):
     service = get_service_by_id(service_id)['data']
 
@@ -61,7 +60,7 @@ def service_name_change(service_id):
 
 @main.route("/services/<service_id>/service-settings/name/confirm", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_settings')
+@user_has_permissions('manage_settings', admin_override=True)
 def service_name_change_confirm(service_id):
     service = get_service_by_id(service_id)['data']
 
@@ -95,7 +94,7 @@ def service_name_change_confirm(service_id):
 
 @main.route("/services/<service_id>/service-settings/request-to-go-live", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_settings')
+@user_has_permissions('manage_settings', admin_override=True)
 def service_request_to_go_live(service_id):
     service = get_service_by_id(service_id)['data']
     if request.method == 'GET':
@@ -112,7 +111,7 @@ def service_request_to_go_live(service_id):
 
 @main.route("/services/<service_id>/service-settings/status", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_settings')
+@user_has_permissions('manage_settings', admin_override=True)
 def service_status_change(service_id):
     service = get_service_by_id(service_id)['data']
 
@@ -128,7 +127,7 @@ def service_status_change(service_id):
 
 @main.route("/services/<service_id>/service-settings/status/confirm", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_settings')
+@user_has_permissions('manage_settings', admin_override=True)
 def service_status_change_confirm(service_id):
     service = get_service_by_id(service_id)['data']
 
@@ -151,7 +150,7 @@ def service_status_change_confirm(service_id):
 
 @main.route("/services/<service_id>/service-settings/delete", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_settings')
+@user_has_permissions('manage_settings', admin_override=True)
 def service_delete(service_id):
     service = get_service_by_id(service_id)['data']
 
@@ -167,7 +166,7 @@ def service_delete(service_id):
 
 @main.route("/services/<service_id>/service-settings/delete/confirm", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_settings')
+@user_has_permissions('manage_settings', admin_override=True)
 def service_delete_confirm(service_id):
     service = get_service_by_id(service_id)['data']
 
diff --git a/app/main/views/templates.py b/app/main/views/templates.py
index f390ff9af..7e34cc79f 100644
--- a/app/main/views/templates.py
+++ b/app/main/views/templates.py
@@ -21,7 +21,7 @@ page_headings = {
 
 @main.route("/services/<service_id>/templates/add-<template_type>", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_templates')
+@user_has_permissions('manage_templates', admin_override=True)
 def add_service_template(service_id, template_type):
 
     service = sdao.get_service_by_id_or_404(service_id)
@@ -54,7 +54,7 @@ def add_service_template(service_id, template_type):
 
 @main.route("/services/<service_id>/templates/<int:template_id>", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_templates')
+@user_has_permissions('manage_templates', admin_override=True)
 def edit_service_template(service_id, template_id):
     template = tdao.get_service_template_or_404(service_id, template_id)['data']
     template['template_content'] = template['content']
@@ -83,7 +83,7 @@ def edit_service_template(service_id, template_id):
 
 @main.route("/services/<service_id>/templates/<int:template_id>/delete", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_templates')
+@user_has_permissions('manage_templates', admin_override=True)
 def delete_service_template(service_id, template_id):
     template = tdao.get_service_template_or_404(service_id, template_id)['data']
 
diff --git a/app/notify_client/api_client.py b/app/notify_client/api_client.py
index dbe66d2d4..6f703e622 100644
--- a/app/notify_client/api_client.py
+++ b/app/notify_client/api_client.py
@@ -2,14 +2,14 @@ from __future__ import unicode_literals
 from notifications_python_client.notifications import NotificationsAPIClient
 
 
-class NotificationsAdminAPIClient(NotificationsAPIClient):
+class ServiceAPIClient(NotificationsAPIClient):
 
     # Fudge assert in the super __init__ so
     # we can set those variables later.
     def __init__(self):
-        super(NotificationsAdminAPIClient, self).__init__("api_url",
-                                                          "client",
-                                                          "secret")
+        super(ServiceAPIClient, self).__init__("api_url",
+                                               "client",
+                                               "secret")
 
     def init_app(self, application):
         self.base_url = application.config['API_HOST_NAME']
@@ -128,21 +128,3 @@ class NotificationsAdminAPIClient(NotificationsAPIClient):
         """
         endpoint = "/service/{0}/template/{1}".format(service_id, template_id)
         return self.delete(endpoint)
-
-    # The implementation of these will change after the notifications-api
-    # functionality updates to include the ability to send notifications.
-    def send_sms(self,
-                 mobile_number,
-                 message,
-                 job_id=None,
-                 description=None):
-        self.send_sms_notification(mobile_number, message)
-
-    def send_email(self,
-                   email_address,
-                   message,
-                   from_address,
-                   subject,
-                   job_id=None,
-                   description=None):
-        self.send_email_notification(email_address, message, from_address, subject)
diff --git a/app/notify_client/models.py b/app/notify_client/models.py
index 3c27c09e4..3a4c9e92b 100644
--- a/app/notify_client/models.py
+++ b/app/notify_client/models.py
@@ -13,6 +13,7 @@ class User(UserMixin):
         self._failed_login_count = fields.get('failed_login_count')
         self._state = fields.get('state')
         self.max_failed_login_count = max_failed_login_count
+        self.platform_admin = fields.get('platform_admin')
 
     def get_id(self):
         return self.id
@@ -82,7 +83,9 @@ class User(UserMixin):
     def permissions(self, permissions):
         raise AttributeError("Read only property")
 
-    def has_permissions(self, permissions, service_id=None, or_=False):
+    def has_permissions(self, permissions, service_id=None, or_=False, admin_override=False):
+        if admin_override and self.platform_admin:
+            return True
         if service_id is None:
             service_id = session.get('service_id', '')
         if service_id in self._permissions:
@@ -91,6 +94,10 @@ class User(UserMixin):
             return set(self._permissions[service_id]) >= set(permissions)
         return False
 
+    def has_platform_admin_permissions(self):
+        print('platform_permissions {}'.format(self.platform_admin))
+        self.platform_admin
+
     @property
     def failed_login_count(self):
         return self._failed_login_count
diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html
index 96c1c372b..da78a1110 100644
--- a/app/templates/main_nav.html
+++ b/app/templates/main_nav.html
@@ -7,13 +7,13 @@
     <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']) %}
+  {% elif current_user.has_permissions(['manage_templates'], admin_override=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>
   </ul>
   {% endif %}
-  {% if current_user.has_permissions(['manage_users', 'manage_settings']) %}
+  {% if current_user.has_permissions(['manage_users', 'manage_settings'], admin_override=True) %}
   <ul>
     <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>
@@ -25,4 +25,9 @@
     <li><a href="{{ url_for('.documentation', service_id=service_id) }}">Developer documentation</a></li>
   </ul>
   {% endif %}
+  {% if current_user.has_permissions([], admin_override=True) %}
+    <ul>
+      <li><a href="{{ url_for('.show_all_services') }}"> List all services </a></li>
+    </ul>
+  {% endif %}
 </nav>
diff --git a/app/templates/views/all-services.html b/app/templates/views/all-services.html
new file mode 100644
index 000000000..c278c8c5b
--- /dev/null
+++ b/app/templates/views/all-services.html
@@ -0,0 +1,16 @@
+{% extends "withoutnav_template.html" %}
+{% from "components/browse-list.html" import browse_list %}
+
+{% block page_title %}
+  All service – GOV.UK Notify
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-large">
+    All services
+  </h1>
+
+  {{ browse_list(services) }}
+
+{% endblock %}
\ No newline at end of file
diff --git a/app/templates/views/choose-template.html b/app/templates/views/choose-template.html
index a5d28fd88..5d061c3df 100644
--- a/app/templates/views/choose-template.html
+++ b/app/templates/views/choose-template.html
@@ -14,7 +14,7 @@
 
     <h1 class="heading-large">{{ page_heading }}</h1>
 
-    {% if current_user.has_permissions(['manage_templates']) %}
+    {% if current_user.has_permissions(['manage_templates'], admin_override=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>
@@ -26,7 +26,7 @@
       <div class="column-two-thirds">
         <h1 class="heading-large">{{ page_heading }}</h1>
       </div>
-      {% if current_user.has_permissions(['manage_templates']) %}
+      {% if current_user.has_permissions(['manage_templates'], admin_override=True) %}
         <div class="column-one-third">
           <a href="{{ url_for('.add_service_template', service_id=service_id, template_type=template_type) }}" class="button align-with-heading">Add new template</a>
         </div>
@@ -54,7 +54,7 @@
               name=template.name,
               edit_link=(
                 url_for(".edit_service_template", service_id=service_id, template_id=template.id)
-                if current_user.has_permissions(['manage_templates']) else
+                if current_user.has_permissions(['manage_templates'], admin_override=True) else
                 None
               )
             ) }}
@@ -64,7 +64,7 @@
                 name=template.name,
                 edit_link=(
                   url_for(".edit_service_template", service_id=service_id, template_id=template.id)
-                  if current_user.has_permissions(['manage_templates']) else
+                  if current_user.has_permissions(['manage_templates'], admin_override=True) else
                   None
                 )
               ) }}
diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html
index f7a6dc7fa..0a276915d 100644
--- a/app/templates/views/service_dashboard.html
+++ b/app/templates/views/service_dashboard.html
@@ -27,7 +27,7 @@
     {% if not template_count and not jobs %}
       {% call banner_wrapper(subhead='Get started', type="tip") %}
         <ol>
-          {% if current_user.has_permissions(['manage_templates']) %}
+          {% if current_user.has_permissions(['manage_templates'], admin_override=True) %}
             <li>
               <a href='{{ url_for(".add_service_template", service_id=service_id, template_type="sms") }}'>Add a template</a>
             </li>
diff --git a/app/utils.py b/app/utils.py
index c6688a5bc..f83c6ad97 100644
--- a/app/utils.py
+++ b/app/utils.py
@@ -30,12 +30,14 @@ class BrowsableItem(object):
         pass
 
 
-def user_has_permissions(*permissions, or_=False):
+def user_has_permissions(*permissions, admin_override=False, or_=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, or_=or_):
+            if current_user and admin_override and current_user.platform_admin:
+                return func(*args, **kwargs)
+            if current_user and current_user.has_permissions(permissions, admin_override=admin_override, or_=or_):
                 return func(*args, **kwargs)
             else:
                 abort(403)
diff --git a/tests/__init__.py b/tests/__init__.py
index a316c4342..557ff8108 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -141,11 +141,11 @@ def validate_route_permission(mocker,
         'app.user_api_client.check_verify_code',
         return_value=(True, ''))
     mocker.patch(
-        'app.notifications_api_client.get_services',
+        '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.notifications_api_client.get_service', return_value={'data': service})
+    mocker.patch('app.service_api_client.get_service', return_value={'data': service})
 
     with app_.test_request_context():
         with app_.test_client() as client:
diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py
index 0902f6293..1362c08d5 100644
--- a/tests/app/main/views/test_dashboard.py
+++ b/tests/app/main/views/test_dashboard.py
@@ -1,5 +1,4 @@
-from flask import url_for, session
-from bs4 import BeautifulSoup
+from flask import url_for
 
 
 def test_should_show_recent_jobs_on_dashboard(app_,
@@ -29,11 +28,11 @@ def _test_dashboard_menu(mocker, app_, usr, service, permissions):
                 'app.user_api_client.check_verify_code',
                 return_value=(True, ''))
             mocker.patch(
-                'app.notifications_api_client.get_services',
+                '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.notifications_api_client.get_service', return_value={'data': service})
+            mocker.patch('app.service_api_client.get_service', return_value={'data': service})
             client.login(usr)
             return client.get(url_for('main.service_dashboard', service_id=service['id']))
 
@@ -111,3 +110,16 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g
 
         assert url_for('main.api_keys', service_id=service_one['id']) in page
         assert url_for('main.documentation', service_id=service_one['id']) in page
+
+
+def test_menu_all_services_for_platform_admin_user(mocker, app_, platform_admin_user, service_one,
+                                                   mock_get_service_templates, mock_get_jobs):
+    with app_.test_request_context():
+        resp = _test_dashboard_menu(
+            mocker,
+            app_,
+            platform_admin_user,
+            service_one,
+            [])
+        page = resp.get_data(as_text=True)
+        assert url_for('main.show_all_services') in page
diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py
index a6b8e5fcb..8717684cf 100644
--- a/tests/app/main/views/test_sign_out.py
+++ b/tests/app/main/views/test_sign_out.py
@@ -17,7 +17,8 @@ def test_sign_out_user(app_,
                        mock_get_user_by_email,
                        mock_get_service_templates,
                        mock_login,
-                       mock_get_jobs):
+                       mock_get_jobs,
+                       mock_has_permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)
diff --git a/tests/conftest.py b/tests/conftest.py
index 9d5c88f39..38bb03903 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -45,12 +45,7 @@ def service_one(request, api_user_active):
 
 @pytest.fixture(scope='function')
 def mock_send_sms(request, mocker):
-    return mocker.patch("app.notifications_api_client.send_sms")
-
-
-@pytest.fixture(scope='function')
-def mock_send_email(request, mocker):
-    return mocker.patch("app.notifications_api_client.send_email", autospec=True)
+    return mocker.patch("app.service_api_client.send_sms")
 
 
 @pytest.fixture(scope='function')
@@ -61,7 +56,7 @@ def mock_get_service(mocker, api_user_active):
             active=False, restricted=True)
         return {'data': service}
 
-    return mocker.patch('app.notifications_api_client.get_service', side_effect=_get)
+    return mocker.patch('app.service_api_client.get_service', side_effect=_get)
 
 
 @pytest.fixture(scope='function')
@@ -73,7 +68,7 @@ def mock_create_service(mocker):
         return service['id']
 
     return mocker.patch(
-        'app.notifications_api_client.create_service', side_effect=_create)
+        'app.service_api_client.create_service', side_effect=_create)
 
 
 @pytest.fixture(scope='function')
@@ -90,7 +85,7 @@ def mock_update_service(mocker):
         return {'data': service}
 
     return mocker.patch(
-        'app.notifications_api_client.update_service', side_effect=_update)
+        'app.service_api_client.update_service', side_effect=_update)
 
 
 @pytest.fixture(scope='function')
@@ -108,7 +103,7 @@ def mock_update_service_raise_httperror_duplicate_name(mocker):
         raise http_error
 
     return mocker.patch(
-        'app.notifications_api_client.update_service', side_effect=_update)
+        'app.service_api_client.update_service', side_effect=_update)
 
 
 SERVICE_ONE_ID = "596364a0-858e-42c8-9062-a8fe822260eb"
@@ -128,7 +123,7 @@ def mock_get_services(mocker, user=None):
         return {'data': [service_one, service_two]}
 
     return mocker.patch(
-        'app.notifications_api_client.get_services', side_effect=_create)
+        'app.service_api_client.get_services', side_effect=_create)
 
 
 @pytest.fixture(scope='function')
@@ -142,7 +137,7 @@ def mock_get_services_with_one_service(mocker, user=None):
         )]}
 
     return mocker.patch(
-        'app.notifications_api_client.get_services', side_effect=_create)
+        'app.service_api_client.get_services', side_effect=_create)
 
 
 @pytest.fixture(scope='function')
@@ -151,7 +146,7 @@ def mock_delete_service(mocker, mock_get_service):
         return mock_get_service.side_effect(service_id)
 
     return mocker.patch(
-        'app.notifications_api_client.delete_service', side_effect=_delete)
+        'app.service_api_client.delete_service', side_effect=_delete)
 
 
 @pytest.fixture(scope='function')
@@ -162,7 +157,7 @@ def mock_get_service_template(mocker):
         return {'data': template}
 
     return mocker.patch(
-        'app.notifications_api_client.get_service_template', side_effect=_create)
+        'app.service_api_client.get_service_template', side_effect=_create)
 
 
 @pytest.fixture(scope='function')
@@ -173,7 +168,7 @@ def mock_get_service_email_template(mocker):
         return {'data': template}
 
     return mocker.patch(
-        'app.notifications_api_client.get_service_template', side_effect=_create)
+        'app.service_api_client.get_service_template', side_effect=_create)
 
 
 @pytest.fixture(scope='function')
@@ -184,7 +179,7 @@ def mock_create_service_template(mocker):
         return {'data': template}
 
     return mocker.patch(
-        'app.notifications_api_client.create_service_template',
+        'app.service_api_client.create_service_template',
         side_effect=_create)
 
 
@@ -196,7 +191,7 @@ def mock_update_service_template(mocker):
         return {'data': template}
 
     return mocker.patch(
-        'app.notifications_api_client.update_service_template',
+        'app.service_api_client.update_service_template',
         side_effect=_update)
 
 
@@ -219,7 +214,7 @@ def mock_get_service_templates(mocker):
         ]}
 
     return mocker.patch(
-        'app.notifications_api_client.get_service_templates',
+        'app.service_api_client.get_service_templates',
         side_effect=_create)
 
 
@@ -232,7 +227,7 @@ def mock_delete_service_template(mocker):
         return {'data': template}
 
     return mocker.patch(
-        'app.notifications_api_client.delete_service_template', side_effect=_delete)
+        'app.service_api_client.delete_service_template', side_effect=_delete)
 
 
 @pytest.fixture(scope='function')
@@ -251,6 +246,23 @@ def api_user_pending():
     return user
 
 
+@pytest.fixture(scope='function')
+def platform_admin_user():
+    from app.notify_client.user_api_client import User
+    user_data = {'id': 222,
+                 'name': 'Platform admin user',
+                 'password': 'somepassword',
+                 'email_address': 'platform@admin.gov.uk',
+                 'mobile_number': '+4472341234',
+                 'state': 'active',
+                 'failed_login_count': 0,
+                 'permissions': {},
+                 'platform_admin': True
+                 }
+    user = User(user_data)
+    return user
+
+
 @pytest.fixture(scope='function')
 def api_user_active():
     from app.notify_client.user_api_client import User
@@ -498,7 +510,7 @@ def mock_login(mocker, mock_get_user, mock_update_user):
             side_effect=_verify_code
         ),
         mocker.patch(
-            'app.notifications_api_client.get_services',
+            'app.service_api_client.get_services',
             side_effect=_no_services
         )
     )
@@ -590,7 +602,7 @@ def mock_get_notifications(mocker):
 
 @pytest.fixture(scope='function')
 def mock_has_permissions(mocker):
-    def _has_permission(permissions, service_id=None, or_=False):
+    def _has_permission(permissions, service_id=None, or_=False, admin_override=False):
         return True
     return mocker.patch(
         'app.notify_client.user_api_client.User.has_permissions',

From 7eb4bcc5920e7771ff141b4c72730537f78d2636 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Thu, 17 Mar 2016 14:25:28 +0000
Subject: [PATCH 2/7] Remove unused method Added more tests

---
 app/notify_client/models.py                   |  4 --
 tests/__init__.py                             |  4 ++
 tests/app/main/test_permissions.py            | 28 ++++++++++++--
 tests/app/main/views/test_manage_users.py     | 38 ++++++++++++++++++-
 tests/app/main/views/test_service_settings.py | 23 +++++++++++
 tests/conftest.py                             |  8 ++++
 6 files changed, 96 insertions(+), 9 deletions(-)

diff --git a/app/notify_client/models.py b/app/notify_client/models.py
index 3a4c9e92b..a40db7cd3 100644
--- a/app/notify_client/models.py
+++ b/app/notify_client/models.py
@@ -94,10 +94,6 @@ class User(UserMixin):
             return set(self._permissions[service_id]) >= set(permissions)
         return False
 
-    def has_platform_admin_permissions(self):
-        print('platform_permissions {}'.format(self.platform_admin))
-        self.platform_admin
-
     @property
     def failed_login_count(self):
         return self._failed_login_count
diff --git a/tests/__init__.py b/tests/__init__.py
index 557ff8108..20d046665 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -146,6 +146,10 @@ def validate_route_permission(mocker,
     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})
+    mocker.patch('app.user_api_client.get_users_for_service', return_value=[usr])
+    mocker.patch('app.invite_api_client.get_invites_for_service', return_value=[])
+    mocker.patch('app.invite_api_client.cancel_invited_user')
+
 
     with app_.test_request_context():
         with app_.test_client() as client:
diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py
index 62c5b5058..586f0238f 100644
--- a/tests/app/main/test_permissions.py
+++ b/tests/app/main/test_permissions.py
@@ -1,16 +1,15 @@
 import pytest
-from flask import url_for
 
 from app.utils import user_has_permissions
 from app.main.views.index import index
 from werkzeug.exceptions import Forbidden
 
 
-def _test_permissions(app_, usr, permissions, will_succeed, or_=False):
+def _test_permissions(app_, usr, permissions, will_succeed, or_=False, admin_override=False):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(usr)
-            decorator = user_has_permissions(*permissions, or_=or_)
+            decorator = user_has_permissions(*permissions, or_=or_, admin_override=admin_override)
             decorated_index = decorator(index)
             if will_succeed:
                 response = decorated_index()
@@ -76,3 +75,26 @@ def test_exact_permissions(app_,
         api_user_active,
         ['manage_users', 'manage_templates', 'manage_settings'],
         True)
+
+
+def test_platform_admin_user_can_access_page(app_,
+                                             platform_admin_user,
+                                             mock_login,
+                                             mock_get_platform_admin_user_with_permissions):
+    _test_permissions(
+        app_,
+        platform_admin_user,
+        [],
+        True,
+        admin_override=True)
+
+
+def test_platform_admin_user_can_not_access_page(app_,
+                                                 platform_admin_user,
+                                                 mock_login,
+                                                 mock_get_platform_admin_user_with_permissions):
+    _test_permissions(
+        app_,
+        platform_admin_user,
+        [],
+        will_succeed=False)
diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py
index d0ed7e571..474445f23 100644
--- a/tests/app/main/views/test_manage_users.py
+++ b/tests/app/main/views/test_manage_users.py
@@ -3,6 +3,7 @@ from flask import url_for
 from bs4 import BeautifulSoup
 
 from app.notify_client.models import InvitedUser
+from tests import validate_route_permission
 
 
 def test_should_show_overview_page(
@@ -258,10 +259,8 @@ def test_user_cant_invite_themselves(
     mock_get_invites_for_service,
     mock_has_permissions
 ):
-    from_user = api_user_active.id
     service_id = service_one['id']
     email_address = api_user_active.email_address
-    permissions = 'send_messages,manage_service,manage_api_keys'
 
     with app_.test_request_context():
         with app_.test_client() as client:
@@ -280,3 +279,38 @@ 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"
+
+
+def test_platform_admin_user_can_manage_user(mocker, app_, platform_admin_user, service_one, api_user_active):
+    routes = [
+        'main.manage_users',
+        'main.invite_user'
+        ]
+    with app_.test_request_context():
+        # for route in routes:
+        #     validate_route_permission(mocker,
+        #                               app_,
+        #                               "GET",
+        #                               200,
+        #                               url_for(route, service_id=service_one['id']),
+        #                               [],
+        #                               platform_admin_user,
+        #                               service_one)
+        #
+        # validate_route_permission(mocker,
+        #                           app_,
+        #                           "GET",
+        #                           200,
+        #                           url_for('main.edit_user_permissions', service_id=service_one['id'], user_id=platform_admin_user.id),
+        #                           [],
+        #                           platform_admin_user,
+        #                           service_one)
+        validate_route_permission(mocker,
+                                  app_,
+                                  "GET",
+                                  200,
+                                  url_for('main.cancel_invited_user', service_id=service_one['id'],
+                                          invited_user_id=api_user_active.id),
+                                  [],
+                                  platform_admin_user,
+                                  service_one)
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 65f7030e7..59af4e3d6 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -422,3 +422,26 @@ def test_route_invalid_permissions(mocker, app_, api_user_active, service_one):
                 ['blah'],
                 api_user_active,
                 service_one)
+
+
+def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one):
+    routes = [
+        'main.service_settings',
+        'main.service_name_change',
+        'main.service_name_change_confirm',
+        'main.service_request_to_go_live',
+        'main.service_status_change',
+        'main.service_status_change_confirm',
+        'main.service_delete',
+        'main.service_delete_confirm'
+        ]
+    with app_.test_request_context():
+        for route in routes:
+            validate_route_permission(mocker,
+                                      app_,
+                                      "GET",
+                                      200,
+                                      url_for(route, service_id=service_one['id']),
+                                      [],
+                                      platform_admin_user,
+                                      service_one)
diff --git a/tests/conftest.py b/tests/conftest.py
index 38bb03903..626161968 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -380,6 +380,14 @@ def mock_get_user_with_permissions(mocker, api_user_active):
         'app.user_api_client.get_user', side_effect=_get_user)
 
 
+@pytest.fixture(scope='function')
+def mock_get_platform_admin_user_with_permissions(mocker, platform_admin_user):
+    def _get_user(id):
+        return platform_admin_user
+    return mocker.patch(
+        'app.user_api_client.get_user', side_effect=_get_user)
+
+
 @pytest.fixture(scope='function')
 def mock_dont_get_user_by_email(mocker):
 

From d003dc4aa928a904cbb839cc1dcf5a72ab6a7f7a Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Fri, 18 Mar 2016 10:49:22 +0000
Subject: [PATCH 3/7] [WIP]: fixing unit tests

---
 app/main/views/manage_users.py            |  10 +-
 app/main/views/two_factor.py              |   5 +-
 app/notify_client/models.py               |   3 +-
 app/templates/main_nav.html               |   8 +-
 app/utils.py                              |   3 +-
 tests/__init__.py                         |   6 +-
 tests/app/main/views/test_manage_users.py | 199 ++++++++++------------
 tests/conftest.py                         |  40 ++++-
 8 files changed, 140 insertions(+), 134 deletions(-)

diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py
index 4ad38587c..7d2611fdb 100644
--- a/app/main/views/manage_users.py
+++ b/app/main/views/manage_users.py
@@ -23,7 +23,7 @@ from app.utils import user_has_permissions
 
 @main.route("/services/<service_id>/users")
 @login_required
-@user_has_permissions('manage_users', 'manage_templates', 'manage_settings', admin_override=True)
+@user_has_permissions('manage_users', admin_override=True)
 def manage_users(service_id):
     users = user_api_client.get_users_for_service(service_id=service_id)
     invited_users = invite_api_client.get_invites_for_service(service_id=service_id)
@@ -40,9 +40,8 @@ def manage_users(service_id):
 
 @main.route("/services/<service_id>/users/invite", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_users', 'manage_templates', 'manage_settings', admin_override=True)
+@user_has_permissions('manage_users', admin_override=True)
 def invite_user(service_id):
-
     service = get_service_by_id(service_id)
 
     form = InviteUserForm(current_user.email_address)
@@ -50,6 +49,7 @@ def invite_user(service_id):
         email_address = form.email_address.data
         permissions = _get_permissions(request.form)
         invited_user = invite_api_client.create_invite(current_user.id, service_id, email_address, permissions)
+
         flash('Invite sent to {}'.format(invited_user.email_address), 'default_with_tick')
         return redirect(url_for('.manage_users', service_id=service_id))
 
@@ -63,7 +63,7 @@ def invite_user(service_id):
 
 @main.route("/services/<service_id>/users/<user_id>", methods=['GET', 'POST'])
 @login_required
-@user_has_permissions('manage_users', 'manage_templates', 'manage_settings', admin_override=True)
+@user_has_permissions('manage_users', admin_override=True)
 def edit_user_permissions(service_id, user_id):
     # TODO we should probably using the service id here in the get user
     # call as well. eg. /user/<user_id>?&service_id=service_id
@@ -100,7 +100,7 @@ def edit_user_permissions(service_id, user_id):
 
 
 @main.route("/services/<service_id>/cancel-invited-user/<invited_user_id>", methods=['GET'])
-@user_has_permissions('manage_users', 'manage_templates', 'manage_settings', admin_override=True)
+@user_has_permissions('manage_users', admin_override=True)
 def cancel_invited_user(service_id, invited_user_id):
     invite_api_client.cancel_invited_user(service_id=service_id, invited_user_id=invited_user_id)
 
diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py
index bc6a2e945..063819b2d 100644
--- a/app/main/views/two_factor.py
+++ b/app/main/views/two_factor.py
@@ -7,8 +7,7 @@ from flask import (
     request
 )
 
-from flask_login import login_user
-
+from flask_login import login_user, current_user
 from app.main import main
 from app.main.dao import users_dao, services_dao
 from app.main.forms import TwoFactorForm
@@ -43,6 +42,8 @@ def two_factor():
         if next_url and _is_safe_redirect_url(next_url):
             return redirect(next_url)
 
+        if current_user.platform_admin:
+            return redirect(url_for('main.show_all_services'))
         if len(services) == 1:
             return redirect(url_for('main.service_dashboard', service_id=services[0]['id']))
         else:
diff --git a/app/notify_client/models.py b/app/notify_client/models.py
index a40db7cd3..2081ab6af 100644
--- a/app/notify_client/models.py
+++ b/app/notify_client/models.py
@@ -86,8 +86,6 @@ class User(UserMixin):
     def has_permissions(self, permissions, service_id=None, or_=False, admin_override=False):
         if admin_override and self.platform_admin:
             return True
-        if service_id is None:
-            service_id = session.get('service_id', '')
         if service_id in self._permissions:
             if or_:
                 return any([x in self._permissions[service_id] for x in permissions])
@@ -140,6 +138,7 @@ class InvitedUser(object):
         self.created_at = created_at
 
     def has_permissions(self, permissions):
+        print('here')
         return set(self.permissions) > set(permissions)
 
     def __eq__(self, other):
diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html
index da78a1110..ffb1364dc 100644
--- a/app/templates/main_nav.html
+++ b/app/templates/main_nav.html
@@ -2,24 +2,24 @@
   <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(['send_texts', 'send_emails', 'send_letters']) %}
+  {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters'], service_id=service_id) %}
   <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'], admin_override=True) %}
+  {% elif current_user.has_permissions(['manage_templates'], service_id=service_id, admin_override=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>
   </ul>
   {% endif %}
-  {% if current_user.has_permissions(['manage_users', 'manage_settings'], admin_override=True) %}
+  {% if current_user.has_permissions(['manage_users', 'manage_settings'], service_id=service_id, admin_override=True) %}
   <ul>
     <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>
   {% endif %}
-  {% if current_user.has_permissions(['manage_api_keys', 'access_developer_docs']) %}
+  {% if current_user.has_permissions(['manage_api_keys', 'access_developer_docs'], service_id=service_id) %}
   <ul>
     <li><a href="{{ url_for('.api_keys', service_id=service_id) }}">API keys</a></li>
     <li><a href="{{ url_for('.documentation', service_id=service_id) }}">Developer documentation</a></li>
diff --git a/app/utils.py b/app/utils.py
index f83c6ad97..13a634c8a 100644
--- a/app/utils.py
+++ b/app/utils.py
@@ -37,7 +37,8 @@ def user_has_permissions(*permissions, admin_override=False, or_=False):
             from flask_login import current_user
             if current_user and admin_override and current_user.platform_admin:
                 return func(*args, **kwargs)
-            if current_user and current_user.has_permissions(permissions, admin_override=admin_override, or_=or_):
+            from flask import request
+            if current_user and current_user.has_permissions(permissions, service_id=request.view_args.get('service_id', None), admin_override=admin_override, or_=or_):
                 return func(*args, **kwargs)
             else:
                 abort(403)
diff --git a/tests/__init__.py b/tests/__init__.py
index 20d046665..7aeef051b 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -1,6 +1,7 @@
 import pytest
 from flask.testing import FlaskClient
 from flask import url_for
+from flask_login import login_user
 
 
 class TestClient(FlaskClient):
@@ -13,6 +14,7 @@ class TestClient(FlaskClient):
         # Include mock_login fixture in test for this to work.
         # TODO would be better for it to be mocked in this
         # function
+
         response = self.post(
             url_for('main.two_factor'), data={'sms_code': '12345'})
         assert response.status_code == 302
@@ -147,10 +149,6 @@ def validate_route_permission(mocker,
     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})
     mocker.patch('app.user_api_client.get_users_for_service', return_value=[usr])
-    mocker.patch('app.invite_api_client.get_invites_for_service', return_value=[])
-    mocker.patch('app.invite_api_client.cancel_invited_user')
-
-
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(usr)
diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py
index 474445f23..0039dcce2 100644
--- a/tests/app/main/views/test_manage_users.py
+++ b/tests/app/main/views/test_manage_users.py
@@ -1,23 +1,25 @@
+import uuid
+
 from flask import url_for
 
 from bs4 import BeautifulSoup
 
 from app.notify_client.models import InvitedUser
 from tests import validate_route_permission
+from tests.conftest import service_one as service_1
 
 
 def test_should_show_overview_page(
     app_,
-    api_user_active,
+    active_user_with_permissions,
     mock_login,
     mock_get_service,
     mock_get_users_by_service,
-    mock_get_invites_for_service,
-    mock_has_permissions
+    mock_get_invites_for_service
 ):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions)
             response = client.get(url_for('main.manage_users', service_id=55555))
 
         assert 'Manage team' in response.get_data(as_text=True)
@@ -27,14 +29,13 @@ def test_should_show_overview_page(
 
 def test_should_show_page_for_one_user(
     app_,
-    api_user_active,
+    active_user_with_permissions,
     mock_login,
-    mock_get_service,
-    mock_has_permissions
+    mock_get_service
 ):
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            client.login(active_user_with_permissions)
             response = client.get(url_for('main.edit_user_permissions', service_id=55555, user_id=0))
 
         assert response.status_code == 200
@@ -42,21 +43,20 @@ def test_should_show_page_for_one_user(
 
 def test_edit_user_permissions(
     app_,
-    api_user_active,
+    active_user_with_permissions,
     mock_login,
     mock_get_service,
     mock_get_users_by_service,
     mock_get_invites_for_service,
-    mock_has_permissions,
     mock_set_user_permissions
 ):
     with app_.test_request_context():
         with app_.test_client() as client:
             service_id = '55555'
-            client.login(api_user_active)
+            client.login(active_user_with_permissions)
             response = client.post(url_for(
-                'main.edit_user_permissions', service_id=service_id, user_id=api_user_active.id
-            ), data={'email_address': api_user_active.email_address,
+                'main.edit_user_permissions', service_id=service_id, user_id=active_user_with_permissions.id
+            ), data={'email_address': active_user_with_permissions.email_address,
                      'send_messages': 'yes',
                      'manage_service': 'yes',
                      'manage_api_keys': 'yes'})
@@ -66,7 +66,7 @@ def test_edit_user_permissions(
             'main.manage_users', service_id=service_id, _external=True
         )
         mock_set_user_permissions.assert_called_with(
-            str(api_user_active.id),
+            str(active_user_with_permissions.id),
             service_id,
             ['send_texts',
              'send_emails',
@@ -80,21 +80,21 @@ def test_edit_user_permissions(
 
 def test_edit_some_user_permissions(
     app_,
-    api_user_active,
+    active_user_with_permissions,
     mock_login,
     mock_get_service,
     mock_get_users_by_service,
     mock_get_invites_for_service,
-    mock_has_permissions,
     mock_set_user_permissions
 ):
+    service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            service_id = '55555'
-            client.login(api_user_active)
+            service_id = service['id']
+            client.login(active_user_with_permissions)
             response = client.post(url_for(
-                'main.edit_user_permissions', service_id=service_id, user_id=api_user_active.id
-            ), data={'email_address': api_user_active.email_address,
+                'main.edit_user_permissions', service_id=service_id, user_id=active_user_with_permissions.id
+            ), data={'email_address': active_user_with_permissions.email_address,
                      'send_messages': 'yes',
                      'manage_service': 'no',
                      'manage_api_keys': 'no'})
@@ -104,7 +104,7 @@ def test_edit_some_user_permissions(
             'main.manage_users', service_id=service_id, _external=True
         )
         mock_set_user_permissions.assert_called_with(
-            str(api_user_active.id),
+            str(active_user_with_permissions.id),
             service_id,
             ['send_texts',
              'send_emails',
@@ -113,16 +113,18 @@ def test_edit_some_user_permissions(
 
 def test_should_show_page_for_inviting_user(
     app_,
-    api_user_active,
+    active_user_with_permissions,
     mock_login,
-    mock_get_user,
-    mock_get_service,
-    mock_has_permissions
+    mocker
 ):
+    service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
-            response = client.get(url_for('main.invite_user', service_id=55555))
+            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            mocker.patch('app.service_api_client.get_service', return_value=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.invite_user', service_id=service['id']))
 
         assert 'Invite a team member' in response.get_data(as_text=True)
         assert response.status_code == 200
@@ -130,26 +132,32 @@ def test_should_show_page_for_inviting_user(
 
 def test_invite_user(
     app_,
-    service_one,
-    api_user_active,
+    active_user_with_permissions,
     mock_login,
-    mock_get_user,
-    mock_get_service,
-    mock_get_users_by_service,
-    mock_create_invite,
-    mock_get_invites_for_service,
-    mock_has_permissions
+    mocker,
+    sample_invite
 ):
-    from_user = api_user_active.id
-    service_id = service_one['id']
+    from_user = active_user_with_permissions.id
+    service = service_1(active_user_with_permissions)
     email_address = 'test@example.gov.uk'
     permissions = 'send_messages,manage_service,manage_api_keys'
+    sample_invite['id'] = str(uuid.uuid4())
+    sample_invite['service'] = service['id']
+    sample_invite['from_user'] = active_user_with_permissions.id
+    sample_invite['email_address'] = 'test@example.gov.uk'
 
+    data = [InvitedUser(**sample_invite)]
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            mocker.patch('app.service_api_client.get_service', return_value=service)
+            mocker.patch('app.service_api_client.get_services', return_value={'data': [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))
             response = client.post(
-                url_for('main.invite_user', service_id=service_id),
+                url_for('main.invite_user', service_id=service['id']),
                 data={'email_address': email_address,
                       'send_messages': 'yes',
                       'manage_service': 'yes',
@@ -158,8 +166,8 @@ def test_invite_user(
             )
 
         assert response.status_code == 200
-        mock_create_invite.assert_called_with(from_user, service_id, email_address, permissions)
-        mock_get_invites_for_service.assert_called_with(service_id=service_id)
+        # mock_create_invite.assert_called_with(from_user, service['id'], email_address, permissions)
+        # mock_get_invites_for_service.assert_called_with(service_id=service['id'])
         page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
         assert page.h1.string.strip() == 'Manage team'
         flash_banner = page.find('div', class_='banner-default-with-tick').string.strip()
@@ -167,45 +175,52 @@ def test_invite_user(
 
 
 def test_cancel_invited_user_cancels_user_invitations(app_,
-                                                      api_user_active,
+                                                      active_user_with_permissions,
                                                       mock_login,
-                                                      mocker,
-                                                      mock_has_permissions):
+                                                      mocker
+                                                      ):
     with app_.test_request_context():
         with app_.test_client() as client:
             mocker.patch('app.invite_api_client.cancel_invited_user')
             import uuid
             invited_user_id = uuid.uuid4()
-            client.login(api_user_active)
-            service_id = uuid.uuid4()
-            response = client.get(url_for('main.cancel_invited_user', service_id=service_id,
+            service = service_1(active_user_with_permissions)
+            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            mocker.patch('app.service_api_client.get_service', return_value=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.cancel_invited_user', service_id=service['id'],
                                           invited_user_id=invited_user_id))
 
             assert response.status_code == 302
-            assert response.location == url_for('main.manage_users', service_id=service_id, _external=True)
+            assert response.location == url_for('main.manage_users', service_id=service['id'], _external=True)
 
 
 def test_manage_users_shows_invited_user(app_,
                                          mocker,
-                                         api_user_active,
-                                         mock_get_service,
+                                         active_user_with_permissions,
                                          mock_login,
-                                         mock_has_permissions,
-                                         mock_get_users_by_service,
                                          sample_invite):
 
     import uuid
     invited_user_id = uuid.uuid4()
+    service = service_1(active_user_with_permissions)
     sample_invite['id'] = invited_user_id
-    data = [InvitedUser(**sample_invite)]
+    sample_invite['service'] = service['id']
+    sample_invite['from_user'] = active_user_with_permissions.id
 
+    data = [InvitedUser(**sample_invite)]
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            mocker.patch('app.service_api_client.get_service', return_value=service)
+            mocker.patch('app.service_api_client.get_services', return_value={'data': [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])
 
-            response = client.get(url_for('main.manage_users', service_id=55555))
+            response = client.get(url_for('main.manage_users', service_id=service['id']))
 
             assert response.status_code == 200
             page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
@@ -218,11 +233,8 @@ def test_manage_users_shows_invited_user(app_,
 
 def test_manage_users_does_not_show_accepted_invite(app_,
                                                     mocker,
-                                                    api_user_active,
-                                                    mock_get_service,
+                                                    active_user_with_permissions,
                                                     mock_login,
-                                                    mock_has_permissions,
-                                                    mock_get_users_by_service,
                                                     sample_invite):
 
     import uuid
@@ -230,14 +242,17 @@ def test_manage_users_does_not_show_accepted_invite(app_,
     sample_invite['id'] = invited_user_id
     sample_invite['status'] = 'accepted'
     data = [InvitedUser(**sample_invite)]
-
+    service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
-
+            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            mocker.patch('app.service_api_client.get_service', return_value=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)
 
-            response = client.get(url_for('main.manage_users', service_id=55555))
+            response = client.get(url_for('main.manage_users', service_id=service['id']))
 
             assert response.status_code == 200
             page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
@@ -249,25 +264,22 @@ def test_manage_users_does_not_show_accepted_invite(app_,
 
 def test_user_cant_invite_themselves(
     app_,
-    service_one,
-    api_user_active,
     mock_login,
-    mock_get_user,
-    mock_get_service,
-    mock_get_users_by_service,
+    mocker,
+    active_user_with_permissions,
     mock_create_invite,
-    mock_get_invites_for_service,
-    mock_has_permissions
+    mock_get_invites_for_service
 ):
-    service_id = service_one['id']
-    email_address = api_user_active.email_address
-
+    service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            client.login(api_user_active)
+            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            mocker.patch('app.service_api_client.get_service', return_value=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': email_address,
+                url_for('main.invite_user', service_id=service['id']),
+                data={'email_address': active_user_with_permissions.email_address,
                       'send_messages': 'yes',
                       'manage_service': 'yes',
                       'manage_api_keys': 'yes'},
@@ -279,38 +291,3 @@ 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"
-
-
-def test_platform_admin_user_can_manage_user(mocker, app_, platform_admin_user, service_one, api_user_active):
-    routes = [
-        'main.manage_users',
-        'main.invite_user'
-        ]
-    with app_.test_request_context():
-        # for route in routes:
-        #     validate_route_permission(mocker,
-        #                               app_,
-        #                               "GET",
-        #                               200,
-        #                               url_for(route, service_id=service_one['id']),
-        #                               [],
-        #                               platform_admin_user,
-        #                               service_one)
-        #
-        # validate_route_permission(mocker,
-        #                           app_,
-        #                           "GET",
-        #                           200,
-        #                           url_for('main.edit_user_permissions', service_id=service_one['id'], user_id=platform_admin_user.id),
-        #                           [],
-        #                           platform_admin_user,
-        #                           service_one)
-        validate_route_permission(mocker,
-                                  app_,
-                                  "GET",
-                                  200,
-                                  url_for('main.cancel_invited_user', service_id=service_one['id'],
-                                          invited_user_id=api_user_active.id),
-                                  [],
-                                  platform_admin_user,
-                                  service_one)
diff --git a/tests/conftest.py b/tests/conftest.py
index 626161968..35da4a647 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -38,9 +38,8 @@ def app_(request):
 
 
 @pytest.fixture(scope='function')
-def service_one(request, api_user_active):
-    import uuid
-    return service_json(str(uuid.uuid4()), 'service one', [api_user_active.id])
+def service_one(api_user_active):
+    return service_json(SERVICE_ONE_ID, 'service one', [api_user_active.id])
 
 
 @pytest.fixture(scope='function')
@@ -113,7 +112,7 @@ SERVICE_TWO_ID = "147ad62a-2951-4fa1-9ca0-093cd1a52c52"
 @pytest.fixture(scope='function')
 def mock_get_services(mocker, user=None):
     if user is None:
-        user = api_user_active()
+        user = active_user_with_permissions(service_one)
 
     def _create(user_id=None):
         service_one = service_json(
@@ -279,6 +278,30 @@ def api_user_active():
     return user
 
 
+@pytest.fixture(scope='function')
+def active_user_with_permissions(service_one):
+    from app.notify_client.user_api_client import User
+
+    user_data = {'id': 222,
+                 'name': 'Test User',
+                 'password': 'somepassword',
+                 'email_address': 'test@user.gov.uk',
+                 'mobile_number': '+4412341234',
+                 'state': 'active',
+                 'failed_login_count': 0,
+                 'permissions': {SERVICE_ONE_ID: ['send_texts',
+                                                          'send_emails',
+                                                          'send_letters',
+                                                          'manage_users',
+                                                          'manage_templates',
+                                                          'manage_settings',
+                                                          'manage_api_keys',
+                                                          'access_developer_docs']}
+                 }
+    user = User(user_data)
+    return user
+
+
 @pytest.fixture(scope='function')
 def api_user_locked():
     from app.notify_client.user_api_client import User
@@ -623,7 +646,14 @@ def mock_get_users_by_service(mocker):
         data = [{'id': 1,
                  'logged_in_at': None,
                  'mobile_number': '+447700900986',
-                 'permissions': [],
+                 'permissions': {SERVICE_ONE_ID: ['send_texts',
+                                                  'send_emails',
+                                                  'send_letters',
+                                                  'manage_users',
+                                                  'manage_templates',
+                                                  'manage_settings',
+                                                  'manage_api_keys',
+                                                  'access_developer_docs']},
                  'state': 'active',
                  'password_changed_at': None,
                  'name': 'Test User',

From 13d9acf7dd25671d39a31e14ac0047eb04ad94c2 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Fri, 18 Mar 2016 16:20:37 +0000
Subject: [PATCH 4/7] Completion of the platform admin user story.

---
 app/__init__.py                            |  3 +
 app/main/views/manage_users.py             |  6 +-
 app/main/views/send.py                     |  8 +-
 app/notify_client/models.py                | 13 ++-
 app/templates/main_nav.html                | 10 +--
 app/templates/views/choose-template.html   | 14 ++--
 app/templates/views/manage-users.html      | 12 +--
 app/templates/views/service_dashboard.html |  8 +-
 app/utils.py                               |  6 +-
 tests/__init__.py                          | 17 ++--
 tests/app/main/test_permissions.py         | 85 ++++++++++++--------
 tests/app/main/views/test_manage_users.py  | 93 ++++++++++------------
 tests/app/main/views/test_send.py          |  6 +-
 tests/conftest.py                          | 58 +++++++-------
 14 files changed, 177 insertions(+), 162 deletions(-)

diff --git a/app/__init__.py b/app/__init__.py
index b7cd83520..48ba1ed19 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -203,4 +203,7 @@ def register_errorhandlers(application):
 
     @application.errorhandler(Exception)
     def handle_bad_request(error):
+        from flask import current_app
+        if current_app.config.get('DEBUG'):
+            print(error)
         return _error_response(500)
diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py
index 7d2611fdb..19c908d2e 100644
--- a/app/main/views/manage_users.py
+++ b/app/main/views/manage_users.py
@@ -73,11 +73,11 @@ def edit_user_permissions(service_id, user_id):
     # Do it through the template or the form class?
     form = PermisisonsForm(**{
         'send_messages': 'yes' if user.has_permissions(
-            ['send_texts', 'send_emails', 'send_letters']) else 'no',
+            permissions=['send_texts', 'send_emails', 'send_letters']) else 'no',
         'manage_service': 'yes' if user.has_permissions(
-            ['manage_users', 'manage_templates', 'manage_settings']) else 'no',
+            permissions=['manage_users', 'manage_templates', 'manage_settings']) else 'no',
         'manage_api_keys': 'yes' if user.has_permissions(
-            ['manage_api_keys', 'access_developer_docs']) else 'no'
+            permissions=['manage_api_keys', 'access_developer_docs']) else 'no'
         })
 
     if form.validate_on_submit():
diff --git a/app/main/views/send.py b/app/main/views/send.py
index ed5f6e02c..f37333553 100644
--- a/app/main/views/send.py
+++ b/app/main/views/send.py
@@ -56,9 +56,9 @@ def get_send_button_text(template_type, number_of_messages):
         }[template_type].format(number_of_messages)
 
 
-def get_page_headings(template_type):
+def get_page_headings(template_type, service_id):
     # User has manage_service role
-    if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']):
+    if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters']):
         return send_messages_page_headings[template_type]
     else:
         return manage_templates_page_headings[template_type]
@@ -85,7 +85,7 @@ def choose_template(service_id, template_type):
             if template['template_type'] == template_type
         ],
         template_type=template_type,
-        page_heading=get_page_headings(template_type),
+        page_heading=get_page_headings(template_type, service_id),
         service=service,
         has_jobs=len(jobs),
         service_id=service_id
@@ -253,7 +253,7 @@ def check_messages(service_id, upload_id):
         'views/check.html',
         recipients=recipients,
         template=template,
-        page_heading=get_page_headings(template.template_type),
+        page_heading=get_page_headings(template.template_type, service_id),
         errors=get_errors_for_csv(recipients, template.template_type),
         rows_have_errors=any(recipients.rows_with_errors),
         count_of_recipients=session['upload_data']['notification_count'],
diff --git a/app/notify_client/models.py b/app/notify_client/models.py
index 2081ab6af..12664fb1f 100644
--- a/app/notify_client/models.py
+++ b/app/notify_client/models.py
@@ -1,5 +1,4 @@
 from flask.ext.login import (UserMixin, login_fresh)
-from flask import session
 
 
 class User(UserMixin):
@@ -83,9 +82,18 @@ class User(UserMixin):
     def permissions(self, permissions):
         raise AttributeError("Read only property")
 
-    def has_permissions(self, permissions, service_id=None, or_=False, admin_override=False):
+    def has_permissions(self, permissions=[], or_=False, admin_override=False):
+        # Only available to the platform admin user
         if admin_override and self.platform_admin:
             return True
+        # Not available to the non platform admin users.
+        # For example the list all-services page is only available to platform admin users and is not service specific
+        if admin_override and not permissions:
+            return False
+
+        from flask import request
+        # 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_:
                 return any([x in self._permissions[service_id] for x in permissions])
@@ -138,7 +146,6 @@ class InvitedUser(object):
         self.created_at = created_at
 
     def has_permissions(self, permissions):
-        print('here')
         return set(self.permissions) > set(permissions)
 
     def __eq__(self, other):
diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html
index ffb1364dc..e5be21e9e 100644
--- a/app/templates/main_nav.html
+++ b/app/templates/main_nav.html
@@ -2,30 +2,30 @@
   <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(['send_texts', 'send_emails', 'send_letters'], service_id=service_id) %}
+  {% if current_user.has_permissions(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'], service_id=service_id, admin_override=True) %}
+  {% elif current_user.has_permissions(permissions=['manage_templates'], admin_override=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>
   </ul>
   {% endif %}
-  {% if current_user.has_permissions(['manage_users', 'manage_settings'], service_id=service_id, admin_override=True) %}
+  {% if current_user.has_permissions(permissions=['manage_users', 'manage_settings'], admin_override=True) %}
   <ul>
     <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>
   {% endif %}
-  {% if current_user.has_permissions(['manage_api_keys', 'access_developer_docs'], service_id=service_id) %}
+  {% if current_user.has_permissions(permissions=['manage_api_keys', 'access_developer_docs']) %}
   <ul>
     <li><a href="{{ url_for('.api_keys', service_id=service_id) }}">API keys</a></li>
     <li><a href="{{ url_for('.documentation', service_id=service_id) }}">Developer documentation</a></li>
   </ul>
   {% endif %}
-  {% if current_user.has_permissions([], admin_override=True) %}
+  {% if current_user.has_permissions(admin_override=True) %}
     <ul>
       <li><a href="{{ url_for('.show_all_services') }}"> List all services </a></li>
     </ul>
diff --git a/app/templates/views/choose-template.html b/app/templates/views/choose-template.html
index 5d061c3df..4a1f3b946 100644
--- a/app/templates/views/choose-template.html
+++ b/app/templates/views/choose-template.html
@@ -14,7 +14,7 @@
 
     <h1 class="heading-large">{{ page_heading }}</h1>
 
-    {% if current_user.has_permissions(['manage_templates'], admin_override=True) %}
+    {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=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>
@@ -26,7 +26,7 @@
       <div class="column-two-thirds">
         <h1 class="heading-large">{{ page_heading }}</h1>
       </div>
-      {% if current_user.has_permissions(['manage_templates'], admin_override=True) %}
+      {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) %}
         <div class="column-one-third">
           <a href="{{ url_for('.add_service_template', service_id=service_id, template_type=template_type) }}" class="button align-with-heading">Add new template</a>
         </div>
@@ -34,7 +34,7 @@
     </div>
 
     {% if not has_jobs %}
-      {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters'], or_=True) %}
+      {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters'], or_=True) %}
       {{ banner(
         """
           Send yourself a test message
@@ -54,7 +54,7 @@
               name=template.name,
               edit_link=(
                 url_for(".edit_service_template", service_id=service_id, template_id=template.id)
-                if current_user.has_permissions(['manage_templates'], admin_override=True) else
+                if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) else
                 None
               )
             ) }}
@@ -64,7 +64,7 @@
                 name=template.name,
                 edit_link=(
                   url_for(".edit_service_template", service_id=service_id, template_id=template.id)
-                  if current_user.has_permissions(['manage_templates'], admin_override=True) else
+                  if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) else
                   None
                 )
               ) }}
@@ -72,11 +72,11 @@
         </div>
         <div class="column-one-third">
           <div class="sms-message-use-links">
-            {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
+            {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters']) %}
               <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(['manage_api_keys', 'access_developer_docs']) %}
+            {% if current_user.has_permissions(permissions=['manage_api_keys', 'access_developer_docs']) %}
               <a href="{{ url_for(".send_from_api", service_id=service_id, template_id=template.id) }}">API integration</a>
             {% endif %}
           </div>
diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html
index e0f7fe1a5..5334b1b43 100644
--- a/app/templates/views/manage-users.html
+++ b/app/templates/views/manage-users.html
@@ -33,9 +33,9 @@ Manage users – GOV.UK Notify
     {% call field() %}
       {{ item.name }}
     {% endcall %}
-    {{ boolean_field(item.has_permissions(['send_texts', 'send_emails', 'send_letters'], service_id=service_id)) }}
-    {{ boolean_field(item.has_permissions(['manage_users', 'manage_templates', 'manage_settings'], service_id=service_id)) }}
-    {{ boolean_field(item.has_permissions(['manage_api_keys', 'access_developer_docs'], service_id=service_id)) }}
+    {{ 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'])) }}
     {% call field(align='right') %}
       {% if current_user.id != item.id %}
         <a href="{{ url_for('.edit_user_permissions', service_id=service_id, user_id=item.id)}}">Edit permission</a>
@@ -50,9 +50,9 @@ Manage users – GOV.UK Notify
       {% call field() %}
         {{ item.email_address }}
       {% endcall %}
-      {{ boolean_field(item.has_permissions(['send_texts', 'send_emails', 'send_letters'])) }}
-      {{ boolean_field(item.has_permissions(['manage_users', 'manage_templates', 'manage_settings'])) }}
-      {{ boolean_field(item.has_permissions(['manage_api_keys', 'access_developer_docs'])) }}
+      {{ 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'])) }}
       {% 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>
diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html
index 0a276915d..440aaa21a 100644
--- a/app/templates/views/service_dashboard.html
+++ b/app/templates/views/service_dashboard.html
@@ -27,12 +27,12 @@
     {% if not template_count and not jobs %}
       {% call banner_wrapper(subhead='Get started', type="tip") %}
         <ol>
-          {% if current_user.has_permissions(['manage_templates'], admin_override=True) %}
+          {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) %}
             <li>
               <a href='{{ url_for(".add_service_template", service_id=service_id, template_type="sms") }}'>Add a template</a>
             </li>
           {% endif %}
-          {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
+          {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters']) %}
             <li>
               <a href='{{ url_for(".choose_template", service_id=service_id, template_type="sms") }}'>Send yourself a text message</a>
             </li>
@@ -41,7 +41,7 @@
       {% endcall %}
     {% elif not jobs %}
       {% call banner_wrapper(subhead='Next step', type="tip") %}
-        {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
+        {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters']) %}
           <a href='{{ url_for(".choose_template", service_id=service_id, template_type="sms") }}'>Send yourself a text message</a>
         {% endif %}
       {% endcall %}
@@ -63,7 +63,7 @@
         {% endcall %}
       {% endcall %}
       {% if more_jobs_to_show %}
-        {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
+        {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters']) %}
           <p class="table-show-more-link">
             <a href="{{ url_for('.view_jobs', service_id=service_id) }}">See all sent text messages</a>
           </p>
diff --git a/app/utils.py b/app/utils.py
index 13a634c8a..2a7bb0b3e 100644
--- a/app/utils.py
+++ b/app/utils.py
@@ -35,10 +35,8 @@ def user_has_permissions(*permissions, admin_override=False, or_=False):
         @wraps(func)
         def wrap_func(*args, **kwargs):
             from flask_login import current_user
-            if current_user and admin_override and current_user.platform_admin:
-                return func(*args, **kwargs)
-            from flask import request
-            if current_user and current_user.has_permissions(permissions, service_id=request.view_args.get('service_id', None), admin_override=admin_override, or_=or_):
+            if current_user and current_user.has_permissions(permissions=permissions,
+                                                             admin_override=admin_override, or_=or_):
                 return func(*args, **kwargs)
             else:
                 abort(403)
diff --git a/tests/__init__.py b/tests/__init__.py
index 7aeef051b..68d2afc62 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -8,16 +8,13 @@ class TestClient(FlaskClient):
     def login(self, user):
         # Skipping authentication here and just log them in
         with self.session_transaction() as session:
-            session['user_details'] = {
-                "email": user.email_address,
-                "id": user.id}
-        # Include mock_login fixture in test for this to work.
-        # TODO would be better for it to be mocked in this
-        # function
+            session['user_id'] = user.id
+            session['_fresh'] = True
 
-        response = self.post(
-            url_for('main.two_factor'), data={'sms_code': '12345'})
-        assert response.status_code == 302
+        login_user(user, remember=True)
+
+    def login_fresh(self):
+        return True
 
     def logout(self, user):
         self.get(url_for("main.logout"))
@@ -153,8 +150,6 @@ def validate_route_permission(mocker,
         with app_.test_client() as client:
             client.login(usr)
             resp = None
-            with client.session_transaction() as session:
-                session['service_id'] = str(service['id'])
             if method == 'GET':
                 resp = client.get(route)
             elif method == 'POST':
diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py
index 586f0238f..6314e6999 100644
--- a/tests/app/main/test_permissions.py
+++ b/tests/app/main/test_permissions.py
@@ -1,12 +1,13 @@
 import pytest
-
 from app.utils import user_has_permissions
 from app.main.views.index import index
 from werkzeug.exceptions import Forbidden
+from flask import request
 
 
-def _test_permissions(app_, usr, permissions, will_succeed, or_=False, admin_override=False):
-    with app_.test_request_context():
+def _test_permissions(app_, usr, permissions, service_id, will_succeed, or_=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)
@@ -21,80 +22,102 @@ def _test_permissions(app_, usr, permissions, will_succeed, or_=False, admin_ove
                     pass
 
 
-def test_user_has_permissions_on_endpoint_fail(app_,
-                                               api_user_active,
-                                               mock_login,
-                                               mock_get_user_with_permissions):
+def test_user_has_permissions_on_endpoint_fail(app_, mocker):
+    user = _user_with_permissions()
+    mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
         app_,
-        api_user_active,
+        user,
         ['something'],
+        '',
         False)
 
 
 def test_user_has_permissions_success(app_,
-                                      api_user_active,
-                                      mock_login,
-                                      mock_get_user_with_permissions):
+                                      mocker):
+    user = _user_with_permissions()
+    mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
         app_,
-        api_user_active,
+        user,
         ['manage_users'],
+        '',
         True)
 
 
-def test_user_has_permissions_or(app_,
-                                 api_user_active,
-                                 mock_login,
-                                 mock_get_user_with_permissions):
+def test_user_has_permissions_or(app_, mocker):
+    user = _user_with_permissions()
+    mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
         app_,
-        api_user_active,
+        user,
         ['something', 'manage_users'],
+        '',
         True,
         or_=True)
 
 
 def test_user_has_permissions_multiple(app_,
-                                       api_user_active,
-                                       mock_login,
-                                       mock_get_user_with_permissions):
+                                       mocker):
+    user = _user_with_permissions()
+    mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
         app_,
-        api_user_active,
+        user,
         ['manage_templates', 'manage_users'],
-        True)
+        '',
+        will_succeed=True)
 
 
 def test_exact_permissions(app_,
-                           api_user_active,
-                           mock_login,
-                           mock_get_user_with_permissions):
+                           mocker):
+    user = _user_with_permissions()
+    mocker.patch('app.user_api_client.get_user', return_value=user)
     _test_permissions(
         app_,
-        api_user_active,
+        user,
         ['manage_users', 'manage_templates', 'manage_settings'],
+        '',
         True)
 
 
 def test_platform_admin_user_can_access_page(app_,
                                              platform_admin_user,
-                                             mock_login,
-                                             mock_get_platform_admin_user_with_permissions):
+                                             mocker):
+    mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
     _test_permissions(
         app_,
         platform_admin_user,
         [],
-        True,
+        '',
+        will_succeed=True,
         admin_override=True)
 
 
 def test_platform_admin_user_can_not_access_page(app_,
                                                  platform_admin_user,
-                                                 mock_login,
-                                                 mock_get_platform_admin_user_with_permissions):
+                                                 mocker):
+    mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
     _test_permissions(
         app_,
         platform_admin_user,
         [],
+        '',
         will_succeed=False)
+
+
+def _user_with_permissions():
+    from app.notify_client.user_api_client import User
+
+    user_data = {'id': 999,
+                 'name': 'Test User',
+                 'password': 'somepassword',
+                 'email_address': 'test@user.gov.uk',
+                 'mobile_number': '+4412341234',
+                 'state': 'active',
+                 'failed_login_count': 0,
+                 'permissions': {'': ['manage_users', 'manage_templates', 'manage_settings']},
+                 'platform_admin': False
+                 }
+    user = User(user_data)
+    return user
diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py
index 0039dcce2..d5922433a 100644
--- a/tests/app/main/views/test_manage_users.py
+++ b/tests/app/main/views/test_manage_users.py
@@ -1,42 +1,43 @@
-import uuid
-
 from flask import url_for
-
 from bs4 import BeautifulSoup
-
+import app
 from app.notify_client.models import InvitedUser
-from tests import validate_route_permission
 from tests.conftest import service_one as service_1
 
 
 def test_should_show_overview_page(
     app_,
     active_user_with_permissions,
-    mock_login,
-    mock_get_service,
-    mock_get_users_by_service,
+    mocker,
     mock_get_invites_for_service
 ):
+    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)
-            response = client.get(url_for('main.manage_users', service_id=55555))
+            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']))
 
         assert 'Manage team' in response.get_data(as_text=True)
         assert response.status_code == 200
-        mock_get_users_by_service.assert_called_once_with(service_id='55555')
+        app.user_api_client.get_users_for_service.assert_called_once_with(service_id=service['id'])
 
 
 def test_should_show_page_for_one_user(
     app_,
     active_user_with_permissions,
-    mock_login,
-    mock_get_service
+    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)
+            mocker.patch('app.service_api_client.get_service', return_value=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=55555, user_id=0))
+            response = client.get(url_for('main.edit_user_permissions', service_id=service['id'], user_id=0))
 
         assert response.status_code == 200
 
@@ -45,17 +46,20 @@ def test_edit_user_permissions(
     app_,
     active_user_with_permissions,
     mock_login,
-    mock_get_service,
-    mock_get_users_by_service,
+    mocker,
     mock_get_invites_for_service,
     mock_set_user_permissions
 ):
+    service = service_1(active_user_with_permissions)
     with app_.test_request_context():
         with app_.test_client() as client:
-            service_id = '55555'
+
+            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
+            mocker.patch('app.service_api_client.get_service', return_value=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
+                'main.edit_user_permissions', service_id=service['id'], user_id=active_user_with_permissions.id
             ), data={'email_address': active_user_with_permissions.email_address,
                      'send_messages': 'yes',
                      'manage_service': 'yes',
@@ -63,11 +67,11 @@ def test_edit_user_permissions(
 
         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['id'], _external=True
         )
         mock_set_user_permissions.assert_called_with(
             str(active_user_with_permissions.id),
-            service_id,
+            service['id'],
             ['send_texts',
              'send_emails',
              'send_letters',
@@ -80,18 +84,21 @@ def test_edit_user_permissions(
 
 def test_edit_some_user_permissions(
     app_,
+    mocker,
     active_user_with_permissions,
-    mock_login,
-    mock_get_service,
-    mock_get_users_by_service,
+    sample_invite,
     mock_get_invites_for_service,
     mock_set_user_permissions
 ):
     service = service_1(active_user_with_permissions)
+    data = [InvitedUser(**sample_invite)]
     with app_.test_request_context():
         with app_.test_client() as client:
-            service_id = service['id']
             client.login(active_user_with_permissions)
+            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,
@@ -111,18 +118,21 @@ def test_edit_some_user_permissions(
              'send_letters'])
 
 
+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=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,
-    mock_login,
     mocker
 ):
     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)
-            mocker.patch('app.service_api_client.get_service', return_value=service)
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
+            _mocks_for_test_manage_users(mocker, active_user_with_permissions, service)
             client.login(active_user_with_permissions)
             response = client.get(url_for('main.invite_user', service_id=service['id']))
 
@@ -133,25 +143,17 @@ def test_should_show_page_for_inviting_user(
 def test_invite_user(
     app_,
     active_user_with_permissions,
-    mock_login,
     mocker,
     sample_invite
 ):
-    from_user = active_user_with_permissions.id
     service = service_1(active_user_with_permissions)
     email_address = 'test@example.gov.uk'
-    permissions = 'send_messages,manage_service,manage_api_keys'
-    sample_invite['id'] = str(uuid.uuid4())
-    sample_invite['service'] = service['id']
-    sample_invite['from_user'] = active_user_with_permissions.id
     sample_invite['email_address'] = 'test@example.gov.uk'
 
     data = [InvitedUser(**sample_invite)]
     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)
-            mocker.patch('app.service_api_client.get_service', return_value=service)
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
+            _mocks_for_test_manage_users(mocker, active_user_with_permissions, 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])
@@ -166,8 +168,6 @@ def test_invite_user(
             )
 
         assert response.status_code == 200
-        # mock_create_invite.assert_called_with(from_user, service['id'], email_address, permissions)
-        # mock_get_invites_for_service.assert_called_with(service_id=service['id'])
         page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
         assert page.h1.string.strip() == 'Manage team'
         flash_banner = page.find('div', class_='banner-default-with-tick').string.strip()
@@ -176,7 +176,6 @@ def test_invite_user(
 
 def test_cancel_invited_user_cancels_user_invitations(app_,
                                                       active_user_with_permissions,
-                                                      mock_login,
                                                       mocker
                                                       ):
     with app_.test_request_context():
@@ -185,9 +184,7 @@ def test_cancel_invited_user_cancels_user_invitations(app_,
             import uuid
             invited_user_id = uuid.uuid4()
             service = service_1(active_user_with_permissions)
-            mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions)
-            mocker.patch('app.service_api_client.get_service', return_value=service)
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
+            _mocks_for_test_manage_users(mocker, active_user_with_permissions, 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))
@@ -199,22 +196,12 @@ def test_cancel_invited_user_cancels_user_invitations(app_,
 def test_manage_users_shows_invited_user(app_,
                                          mocker,
                                          active_user_with_permissions,
-                                         mock_login,
                                          sample_invite):
-
-    import uuid
-    invited_user_id = uuid.uuid4()
     service = service_1(active_user_with_permissions)
-    sample_invite['id'] = invited_user_id
-    sample_invite['service'] = service['id']
-    sample_invite['from_user'] = active_user_with_permissions.id
-
     data = [InvitedUser(**sample_invite)]
     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)
-            mocker.patch('app.service_api_client.get_service', return_value=service)
-            mocker.patch('app.service_api_client.get_services', return_value={'data': [service]})
+            _mocks_for_test_manage_users(mocker, active_user_with_permissions, service)
             client.login(active_user_with_permissions)
 
             mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data)
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index 8cd8a9ca5..a58079c7e 100644
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -390,10 +390,8 @@ def test_route_choose_template_manage_service_permissions(mocker,
 
 def test_route_choose_template_send_messages_permissions(mocker,
                                                          app_,
-                                                         api_user_active,
+                                                         active_user_with_permissions,
                                                          service_one,
-                                                         mock_login,
-                                                         mock_get_user,
                                                          mock_get_service,
                                                          mock_check_verify_code,
                                                          mock_get_service_templates,
@@ -410,7 +408,7 @@ def test_route_choose_template_send_messages_permissions(mocker,
                 service_id=service_one['id'],
                 template_type='sms'),
             ['send_texts', 'send_emails', 'send_letters'],
-            api_user_active,
+            active_user_with_permissions,
             service_one)
         page = resp.get_data(as_text=True)
         assert url_for(
diff --git a/tests/conftest.py b/tests/conftest.py
index 35da4a647..d5391c2ff 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -2,9 +2,7 @@ import uuid
 from datetime import date, datetime, timedelta
 from unittest.mock import Mock
 import pytest
-
 from app import create_app
-
 from . import (
     service_json,
     TestClient,
@@ -18,7 +16,6 @@ from app.notify_client.models import (
     User,
     InvitedUser
 )
-
 from notifications_python_client.errors import HTTPError
 
 
@@ -89,7 +86,6 @@ def mock_update_service(mocker):
 
 @pytest.fixture(scope='function')
 def mock_update_service_raise_httperror_duplicate_name(mocker):
-
     def _update(service_id,
                 service_name,
                 active,
@@ -112,7 +108,7 @@ SERVICE_TWO_ID = "147ad62a-2951-4fa1-9ca0-093cd1a52c52"
 @pytest.fixture(scope='function')
 def mock_get_services(mocker, user=None):
     if user is None:
-        user = active_user_with_permissions(service_one)
+        user = active_user_with_permissions()
 
     def _create(user_id=None):
         service_one = service_json(
@@ -279,7 +275,7 @@ def api_user_active():
 
 
 @pytest.fixture(scope='function')
-def active_user_with_permissions(service_one):
+def active_user_with_permissions():
     from app.notify_client.user_api_client import User
 
     user_data = {'id': 222,
@@ -290,13 +286,14 @@ def active_user_with_permissions(service_one):
                  'state': 'active',
                  'failed_login_count': 0,
                  'permissions': {SERVICE_ONE_ID: ['send_texts',
-                                                          'send_emails',
-                                                          'send_letters',
-                                                          'manage_users',
-                                                          'manage_templates',
-                                                          'manage_settings',
-                                                          'manage_api_keys',
-                                                          'access_developer_docs']}
+                                                  'send_emails',
+                                                  'send_letters',
+                                                  'manage_users',
+                                                  'manage_templates',
+                                                  'manage_settings',
+                                                  'manage_api_keys',
+                                                  'access_developer_docs']},
+                 'platform_admin': False
                  }
     user = User(user_data)
     return user
@@ -369,6 +366,7 @@ def mock_get_user(mocker, api_user_active):
     def _get_user(id):
         api_user_active.id = id
         return api_user_active
+
     return mocker.patch(
         'app.user_api_client.get_user', side_effect=_get_user)
 
@@ -387,10 +385,10 @@ def mock_get_user_pending(mocker, api_user_pending):
 
 @pytest.fixture(scope='function')
 def mock_get_user_by_email(mocker, api_user_active):
-
     def _get_user(email_address):
         api_user_active._email_address = email_address
         return api_user_active
+
     return mocker.patch('app.user_api_client.get_user_by_email', side_effect=_get_user)
 
 
@@ -399,23 +397,16 @@ def mock_get_user_with_permissions(mocker, api_user_active):
     def _get_user(id):
         api_user_active._permissions[''] = ['manage_users', 'manage_templates', 'manage_settings']
         return api_user_active
-    return mocker.patch(
-        'app.user_api_client.get_user', side_effect=_get_user)
 
-
-@pytest.fixture(scope='function')
-def mock_get_platform_admin_user_with_permissions(mocker, platform_admin_user):
-    def _get_user(id):
-        return platform_admin_user
     return mocker.patch(
         'app.user_api_client.get_user', side_effect=_get_user)
 
 
 @pytest.fixture(scope='function')
 def mock_dont_get_user_by_email(mocker):
-
     def _get_user(email_address):
         return None
+
     return mocker.patch(
         'app.user_api_client.get_user_by_email',
         side_effect=_get_user,
@@ -464,6 +455,7 @@ def mock_get_user_by_email_not_found(mocker):
 def mock_verify_password(mocker):
     def _verify_password(user, password):
         return True
+
     return mocker.patch(
         'app.user_api_client.verify_password',
         side_effect=_verify_password)
@@ -471,9 +463,9 @@ def mock_verify_password(mocker):
 
 @pytest.fixture(scope='function')
 def mock_update_user(mocker):
-
     def _update(user):
         return user
+
     return mocker.patch('app.user_api_client.update_user', side_effect=_update)
 
 
@@ -489,7 +481,6 @@ def mock_get_all_users_from_api(mocker):
 
 @pytest.fixture(scope='function')
 def mock_create_api_key(mocker):
-
     def _create(service_id, key_name):
         import uuid
         return {'data': str(uuid.uuid4())}
@@ -528,7 +519,6 @@ def mock_get_no_api_keys(mocker):
 
 @pytest.fixture(scope='function')
 def mock_login(mocker, mock_get_user, mock_update_user):
-
     def _verify_code(user_id, code, code_type):
         return True, ''
 
@@ -556,6 +546,7 @@ def mock_send_verify_code(mocker):
 def mock_check_verify_code(mocker):
     def _verify(user_id, code, code_type):
         return True, ''
+
     return mocker.patch(
         'app.user_api_client.check_verify_code',
         side_effect=_verify)
@@ -565,6 +556,7 @@ def mock_check_verify_code(mocker):
 def mock_check_verify_code_code_not_found(mocker):
     def _verify(user_id, code, code_type):
         return False, 'Code not found'
+
     return mocker.patch(
         'app.user_api_client.check_verify_code',
         side_effect=_verify)
@@ -574,6 +566,7 @@ def mock_check_verify_code_code_not_found(mocker):
 def mock_check_verify_code_code_expired(mocker):
     def _verify(user_id, code, code_type):
         return False, 'Code has expired'
+
     return mocker.patch(
         'app.user_api_client.check_verify_code',
         side_effect=_verify)
@@ -595,6 +588,7 @@ def mock_create_job(mocker, job_data):
         job_data['file_name'] = '{}.csv'.format(job_id)
         job_data['notification_count'] = notification_count
         return job_data
+
     return mocker.patch('app.job_api_client.create_job', side_effect=_create)
 
 
@@ -604,6 +598,7 @@ def mock_get_job(mocker, job_data):
         job_data['id'] = job_id
         job_data['service'] = service_id
         return {"data": job_data}
+
     return mocker.patch('app.job_api_client.get_job', side_effect=_get_job)
 
 
@@ -618,6 +613,7 @@ def mock_get_jobs(mocker):
             job_data['service'] = service_id
             data.append(job_data)
         return {"data": data}
+
     return mocker.patch('app.job_api_client.get_job', side_effect=_get_jobs)
 
 
@@ -625,6 +621,7 @@ def mock_get_jobs(mocker):
 def mock_get_notifications(mocker):
     def _get_notifications(service_id, job_id):
         return notification_json()
+
     return mocker.patch(
         'app.notification_api_client.get_notifications_for_service',
         side_effect=_get_notifications
@@ -633,8 +630,9 @@ def mock_get_notifications(mocker):
 
 @pytest.fixture(scope='function')
 def mock_has_permissions(mocker):
-    def _has_permission(permissions, service_id=None, or_=False, admin_override=False):
+    def _has_permission(permissions=None, or_=False, admin_override=False):
         return True
+
     return mocker.patch(
         'app.notify_client.user_api_client.User.has_permissions',
         side_effect=_has_permission)
@@ -660,6 +658,7 @@ def mock_get_users_by_service(mocker):
                  'email_address': 'notify@digital.cabinet-office.gov.uk',
                  'failed_login_count': 0}]
         return [User(data[0])]
+
     return mocker.patch('app.user_api_client.get_users_for_service', side_effect=_get_users_for_service, autospec=True)
 
 
@@ -667,6 +666,7 @@ def mock_get_users_by_service(mocker):
 def mock_s3_upload(mocker):
     def _upload(upload_id, service_id, filedata, region):
         pass
+
     return mocker.patch('app.main.views.send.s3upload', side_effect=_upload)
 
 
@@ -689,7 +689,6 @@ def sample_invited_user(mocker, sample_invite):
 
 @pytest.fixture(scope='function')
 def mock_create_invite(mocker, sample_invite):
-
     def _create_invite(from_user, service_id, email_address, permissions):
         sample_invite['from_user'] = from_user
         sample_invite['service'] = service_id
@@ -697,6 +696,7 @@ def mock_create_invite(mocker, sample_invite):
         sample_invite['status'] = 'pending'
         sample_invite['permissions'] = permissions
         return InvitedUser(**sample_invite)
+
     return mocker.patch('app.invite_api_client.create_invite', side_effect=_create_invite)
 
 
@@ -711,6 +711,7 @@ def mock_get_invites_for_service(mocker, service_one, sample_invite):
             invite['email_address'] = 'user_{}@testnotify.gov.uk'.format(i)
             data.append(InvitedUser(**invite))
         return data
+
     return mocker.patch('app.invite_api_client.get_invites_for_service', side_effect=_get_invites)
 
 
@@ -718,6 +719,7 @@ def mock_get_invites_for_service(mocker, service_one, sample_invite):
 def mock_check_invite_token(mocker, sample_invite):
     def _check_token(token):
         return InvitedUser(**sample_invite)
+
     return mocker.patch('app.invite_api_client.check_token', side_effect=_check_token)
 
 
@@ -725,6 +727,7 @@ def mock_check_invite_token(mocker, sample_invite):
 def mock_accept_invite(mocker, sample_invite):
     def _accept(service_id, invite_id):
         return InvitedUser(**sample_invite)
+
     return mocker.patch('app.invite_api_client.accept_invite', side_effect=_accept)
 
 
@@ -732,6 +735,7 @@ def mock_accept_invite(mocker, sample_invite):
 def mock_add_user_to_service(mocker, service_one, api_user_active):
     def _add_user(service_id, user_id, permissions):
         return api_user_active
+
     return mocker.patch('app.user_api_client.add_user_to_service', side_effect=_add_user)
 
 

From a66f634d9b025dbc08e49f2ffd14072ebdbf7909 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Mon, 21 Mar 2016 13:50:56 +0000
Subject: [PATCH 5/7] Added a test for the all_services view

---
 tests/app/main/views/test_all_services.py | 34 +++++++++++++++++++++++
 1 file changed, 34 insertions(+)
 create mode 100644 tests/app/main/views/test_all_services.py

diff --git a/tests/app/main/views/test_all_services.py b/tests/app/main/views/test_all_services.py
new file mode 100644
index 000000000..0706d73e1
--- /dev/null
+++ b/tests/app/main/views/test_all_services.py
@@ -0,0 +1,34 @@
+from bs4 import BeautifulSoup
+from flask import url_for
+
+
+def test_all_services_should_render_all_services_template(app_,
+                                                          platform_admin_user,
+                                                          service_one,
+                                                          mocker):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            _login_user(client, mocker, platform_admin_user, service_one)
+            mocker.patch('app.service_api_client.get_services', return_value={'data': [service_one]})
+            response = client.get(url_for('main.show_all_services'))
+            assert response.status_code == 200
+            page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+            assert page.h1.string.strip() == 'All services'
+
+
+def _login_user(client, mocker, platform_admin_user, service_one):
+    mocker.patch('app.user_api_client.get_user_by_email', return_value=platform_admin_user)
+    mocker.patch('app.service_api_client.get_service', return_value={'data': service_one})
+    mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
+    client.login(platform_admin_user)
+
+
+def test_all_service_returns_403_when_not_a_platform_admin(app_,
+                                                           active_user_with_permissions,
+                                                           service_one,
+                                                           mocker):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            _login_user(client, mocker, active_user_with_permissions, service_one)
+            response = client.get(url_for('main.show_all_services'))
+            assert response.status_code == 403

From 300cdf00bd17462eabffea292a3b21ecd57ee657 Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Mon, 21 Mar 2016 15:36:47 +0000
Subject: [PATCH 6/7] Added more test for the side nav

---
 app/templates/main_nav.html               |  4 ++--
 tests/app/main/test_permissions.py        |  3 ++-
 tests/app/main/views/test_all_services.py | 15 ++++++++-------
 tests/app/main/views/test_dashboard.py    | 10 ++++++++++
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html
index 5f670c24d..f7aa3aed9 100644
--- a/app/templates/main_nav.html
+++ b/app/templates/main_nav.html
@@ -7,13 +7,13 @@
     <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'], or_=True) %}
+  {% elif current_user.has_permissions(['manage_templates','manage_api_keys'], admin_override=True, or_=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>
   </ul>
   {% endif %}
-  {% if current_user.has_permissions(['manage_users', 'manage_settings']) %}
+  {% if current_user.has_permissions(['manage_users', 'manage_settings'], admin_override=True) %}
   <ul>
     <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>
diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py
index 6314e6999..30a00e336 100644
--- a/tests/app/main/test_permissions.py
+++ b/tests/app/main/test_permissions.py
@@ -103,7 +103,8 @@ def test_platform_admin_user_can_not_access_page(app_,
         platform_admin_user,
         [],
         '',
-        will_succeed=False)
+        will_succeed=False,
+        admin_override=False)
 
 
 def _user_with_permissions():
diff --git a/tests/app/main/views/test_all_services.py b/tests/app/main/views/test_all_services.py
index 0706d73e1..28756451d 100644
--- a/tests/app/main/views/test_all_services.py
+++ b/tests/app/main/views/test_all_services.py
@@ -1,6 +1,8 @@
 from bs4 import BeautifulSoup
 from flask import url_for
 
+import app
+
 
 def test_all_services_should_render_all_services_template(app_,
                                                           platform_admin_user,
@@ -14,13 +16,7 @@ def test_all_services_should_render_all_services_template(app_,
             assert response.status_code == 200
             page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
             assert page.h1.string.strip() == 'All services'
-
-
-def _login_user(client, mocker, platform_admin_user, service_one):
-    mocker.patch('app.user_api_client.get_user_by_email', return_value=platform_admin_user)
-    mocker.patch('app.service_api_client.get_service', return_value={'data': service_one})
-    mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
-    client.login(platform_admin_user)
+            assert app.service_api_client.get_services.call_count == 1
 
 
 def test_all_service_returns_403_when_not_a_platform_admin(app_,
@@ -32,3 +28,8 @@ def test_all_service_returns_403_when_not_a_platform_admin(app_,
             _login_user(client, mocker, active_user_with_permissions, service_one)
             response = client.get(url_for('main.show_all_services'))
             assert response.status_code == 403
+
+
+def _login_user(client, mocker, platform_admin_user, service_one):
+    mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
+    client.login(platform_admin_user)
diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py
index 50c4f63d3..7345a19bd 100644
--- a/tests/app/main/views/test_dashboard.py
+++ b/tests/app/main/views/test_dashboard.py
@@ -62,6 +62,7 @@ def test_menu_send_messages(mocker, app_, api_user_active, service_one, mock_get
 
         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
 
 
 def test_menu_manage_service(mocker, app_, api_user_active, service_one, mock_get_service_templates, mock_get_jobs):
@@ -86,6 +87,7 @@ def test_menu_manage_service(mocker, app_, api_user_active, service_one, mock_ge
         assert url_for('main.service_settings', service_id=service_one['id']) 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
 
 
 def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_get_service_templates, mock_get_jobs):
@@ -108,6 +110,7 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g
 
         assert url_for('main.manage_users', service_id=service_one['id']) not 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
 
         assert url_for('main.api_keys', service_id=service_one['id']) in page
 
@@ -123,3 +126,10 @@ def test_menu_all_services_for_platform_admin_user(mocker, app_, platform_admin_
             [])
         page = resp.get_data(as_text=True)
         assert url_for('main.show_all_services') in page
+        assert url_for('main.choose_template', service_id=service_one['id'], template_type='sms') in page
+        assert url_for('main.choose_template', service_id=service_one['id'], template_type='email') 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.view_notifications', service_id=service_one['id']) in page
+        assert url_for('main.view_jobs', service_id=service_one['id']) in page
+        assert url_for('main.api_keys', service_id=service_one['id']) not in page

From abef0ae793900420e1c872671e3de4f8b21da94c Mon Sep 17 00:00:00 2001
From: Rebecca Law <rebecca.law@digital.cabinet-office.gov.uk>
Date: Mon, 21 Mar 2016 16:06:48 +0000
Subject: [PATCH 7/7] Add all-services link for platform admin user on the
 choose-services page

---
 app/templates/views/choose-service.html      |  9 +++++++++
 tests/app/main/views/test_choose_services.py | 21 ++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/app/templates/views/choose-service.html b/app/templates/views/choose-service.html
index 38fe8efc2..d99e888e1 100644
--- a/app/templates/views/choose-service.html
+++ b/app/templates/views/choose-service.html
@@ -10,6 +10,14 @@
   <h1 class="heading-large">
     Choose service
   </h1>
+  {% if current_user.has_permissions(admin_override=True) %}
+    {{ browse_list([
+      {
+          'title': 'List all services',
+          'link': url_for('.show_all_services')
+      }
+      ]) }}
+  {% endif %}
 
   {{ browse_list(services) }}
   {{ browse_list([
@@ -19,4 +27,5 @@
     },
   ]) }}
 
+
 {% endblock %}
diff --git a/tests/app/main/views/test_choose_services.py b/tests/app/main/views/test_choose_services.py
index d373da631..02b0845a5 100644
--- a/tests/app/main/views/test_choose_services.py
+++ b/tests/app/main/views/test_choose_services.py
@@ -20,3 +20,24 @@ def test_should_show_choose_services_page(app_,
         assert mock_get_services.called
         assert services['data'][0]['name'] in resp_data
         assert services['data'][1]['name'] in resp_data
+        assert 'List all services' not in resp_data
+
+
+def test_should_show_all_services_for_platform_admin_user(app_,
+                                                          platform_admin_user,
+                                                          mock_get_services,
+                                                          mocker):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            mocker.patch('app.user_api_client.get_user', return_value=platform_admin_user)
+            client.login(platform_admin_user)
+        response = client.get(url_for('main.choose_service'))
+
+        assert response.status_code == 200
+        resp_data = response.get_data(as_text=True)
+        assert 'Choose service' in resp_data
+        services = mock_get_services.side_effect()
+        assert mock_get_services.called
+        assert services['data'][0]['name'] in resp_data
+        assert services['data'][1]['name'] in resp_data
+        assert 'List all services' in resp_data