Merge pull request #211 from alphagov/connect_api_keys_permission

Connect api keys permission

app/__init__.py

@@ -17,6 +17,7 @@ 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
 from app.notify_client.status_api_client import StatusApiClient
+from app.notify_client.permission_api_client import PermissionApiClient
 from app.notify_client.invite_api_client import InviteApiClient
 from app.its_dangerous_session import ItsdangerousSessionInterface
 from app.asset_fingerprinter import AssetFingerprinter
@@ -35,6 +36,7 @@ job_api_client = JobApiClient()
 status_api_client = StatusApiClient()
 invite_api_client = InviteApiClient()
 asset_fingerprinter = AssetFingerprinter()
+permission_api_client = PermissionApiClient()
 
 
 def create_app(config_name, config_overrides=None):
@@ -51,6 +53,7 @@ def create_app(config_name, config_overrides=None):
     api_key_api_client.init_app(application)
     job_api_client.init_app(application)
     status_api_client.init_app(application)
+    permission_api_client.init_app(application)
     invite_api_client.init_app(application)
 
     login_manager.init_app(application)

app/main/views/api_keys.py

@@ -3,6 +3,7 @@ from flask_login import login_required
 from app.main import main
 from app.main.forms import CreateKeyForm
 from app import api_key_api_client
+from app.utils import user_has_permissions
 
 
 @main.route("/services/<service_id>/documentation")
@@ -13,6 +14,7 @@ def documentation(service_id):
 
 @main.route("/services/<service_id>/api-keys")
 @login_required
+@user_has_permissions('manage_api_keys')
 def api_keys(service_id):
     return render_template(
         'views/api-keys.html',
@@ -23,6 +25,7 @@ def api_keys(service_id):
 
 @main.route("/services/<service_id>/api-keys/create", methods=['GET', 'POST'])
 @login_required
+@user_has_permissions('manage_api_keys')
 def create_api_key(service_id):
     key_names = [
         key['name'] for key in api_key_api_client.get_api_keys(service_id=service_id)['apiKeys']
@@ -41,6 +44,7 @@ def create_api_key(service_id):
 
 @main.route("/services/<service_id>/api-keys/revoke/<int:key_id>", methods=['GET', 'POST'])
 @login_required
+@user_has_permissions('manage_api_keys')
 def revoke_api_key(service_id, key_id):
     key_name = api_key_api_client.get_api_keys(service_id=service_id, key_id=key_id)['apiKeys'][0]['name']
     if request.method == 'GET':

app/main/views/dashboard.py

@@ -23,6 +23,7 @@ def service_dashboard(service_id):
     try:
         service = get_service_by_id(service_id)
         session['service_name'] = service['data']['name']
+        session['service_id'] = service['data']['id']
     except HTTPError as e:
         if e.status_code == 404:
             abort(404)

app/notify_client/permission_api_client.py

@@ -0,0 +1,25 @@
+import uuid
+
+from notifications_python_client.base import BaseAPIClient
+
+
+class PermissionApiClient(BaseAPIClient):
+    def __init__(self, base_url=None, client_id=None, secret=None):
+        super(self.__class__, self).__init__(base_url=base_url or 'base_url',
+                                             client_id=client_id or 'client_id',
+                                             secret=secret or 'secret')
+
+    def init_app(self, app):
+        self.base_url = app.config['API_HOST_NAME']
+        self.client_id = app.config['ADMIN_CLIENT_USER_NAME']
+        self.secret = app.config['ADMIN_CLIENT_SECRET']
+
+    def delete_permission(self, permission_id):
+        return self.delete(url='/permission/{}'.format(permission_id))['data']
+
+    def create_permission(self, permission, user_id, service_id):
+        return self.post(
+            url='/permission',
+            data={'permission': permission,
+                  'user': user_id,
+                  'service': service_id})['data']

app/notify_client/user_api_client.py

@@ -94,7 +94,7 @@ class User(UserMixin):
         self._email_address = fields.get('email_address')
         self._mobile_number = fields.get('mobile_number')
         self._password_changed_at = fields.get('password_changed_at')
-        self._permissions = set(fields.get('permissions')) if fields.get('permission') is not None else set()
+        self._permissions = fields.get('permissions')
         self._failed_login_count = 0
         self._state = fields.get('state')
         self.max_failed_login_count = max_failed_login_count
@@ -165,18 +165,12 @@ class User(UserMixin):
 
     @permissions.setter
     def permissions(self, permissions):
-        if permissions is None:
+        raise AttributeError("Read only property")
-            permissions = set()
-        self._permissions = set(permissions)
 
-    def add_permissions(self, permissions):
+    def has_permissions(self, service_id, permissions):
-        self._permissions.update(permissions)
+        if service_id in self._permissions:
-
+            return set(self._permissions[service_id]) > set(permissions)
-    def remove_permissions(self, permissions):
+        return False
-        self._permissions -= permissions
-
-    def has_permissions(self, permissions):
-        return self._permissions > set(permissions)
 
     @property
     def failed_login_count(self):

app/templates/main_nav.html

@@ -2,17 +2,23 @@
   <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(session.get('service_id', ''), ['send_messages']) %}
   <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>
     <li><a href="{{ url_for('.letters_stub', service_id=service_id) }}">Send letters</a></li>
   </ul>
+  {% endif %}
+  {% if current_user.has_permissions(session.get('service_id', ''), ['manage_service']) %}
   <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(session.get('service_id', ''), ['manage_api_keys']) %}
   <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 %}
 </nav>

app/utils.py

@@ -1,7 +1,7 @@
 import re
 
 from functools import wraps
-from flask import abort
+from flask import (abort, session)
 
 
 class BrowsableItem(object):
@@ -100,8 +100,10 @@ def user_has_permissions(*permissions):
         def wrap_func(*args, **kwargs):
             # We are making the assumption that the user is logged in.
             from flask_login import current_user
-            if set(permissions) > set(current_user.permissions):
+            service_id = session.get('service_id', '')
+            if current_user and current_user.has_permissions(service_id, permissions):
+                return func(*args, **kwargs)
+            else:
                 abort(403)
-            return func(*args, **kwargs)
         return wrap_func
     return wrap

tests/__init__.py

@@ -66,14 +66,15 @@ def create_test_user(state):
     return user
 
 
-def create_test_api_user(state):
+def create_test_api_user(state, permissions={}):
     from app.notify_client.user_api_client import User
     user_data = {'id': 1,
                  'name': 'Test User',
                  'password': 'somepassword',
                  'email_address': TEST_USER_EMAIL,
                  'mobile_number': '+441234123412',
-                 'state': state
+                 'state': state,
+                 'permissions': permissions
                  }
     user = User(user_data)
     return user

tests/app/main/views/test_api_keys.py

@@ -21,7 +21,8 @@ def test_should_show_empty_api_keys_page(app_,
                                          mock_get_user,
                                          mock_get_user_by_email,
                                          mock_get_no_api_keys,
-                                         mock_login):
+                                         mock_login,
+                                         mock_has_permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)
@@ -39,7 +40,8 @@ def test_should_show_api_keys_page(app_,
                                    mock_get_user,
                                    mock_get_user_by_email,
                                    mock_get_api_keys,
-                                   mock_login):
+                                   mock_login,
+                                   mock_has_permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)
@@ -58,7 +60,8 @@ def test_should_show_name_api_key_page(app_,
                                        mock_get_user,
                                        mock_get_user_by_email,
                                        mock_get_api_keys,
-                                       mock_login):
+                                       mock_login,
+                                       mock_has_permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)
@@ -74,7 +77,8 @@ def test_should_render_show_api_key(app_,
                                     mock_get_user_by_email,
                                     mock_create_api_key,
                                     mock_get_api_keys,
-                                    mock_login):
+                                    mock_login,
+                                    mock_has_permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)
@@ -92,7 +96,8 @@ def test_should_show_confirm_revoke_api_key(app_,
                                             mock_get_user,
                                             mock_get_user_by_email,
                                             mock_get_api_keys,
-                                            mock_login):
+                                            mock_login,
+                                            mock_has_permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)
@@ -110,7 +115,8 @@ def test_should_redirect_after_revoking_api_key(app_,
                                                 mock_get_user_by_email,
                                                 mock_revoke_api_key,
                                                 mock_get_api_keys,
-                                                mock_login):
+                                                mock_login,
+                                                mock_has_permissions):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)

tests/conftest.py

@@ -205,7 +205,8 @@ def api_user_pending():
                  'email_address': 'test@user.gov.uk',
                  'mobile_number': '+4412341234',
                  'state': 'pending',
-                 'failed_login_count': 0
+                 'failed_login_count': 0,
+                 'permissions': {}
                  }
     user = User(user_data)
     return user
@@ -220,7 +221,8 @@ def api_user_active():
                  'email_address': 'test@user.gov.uk',
                  'mobile_number': '+4412341234',
                  'state': 'active',
-                 'failed_login_count': 0
+                 'failed_login_count': 0,
+                 'permissions': {}
                  }
     user = User(user_data)
     return user
@@ -235,7 +237,8 @@ def api_user_locked():
                  'email_address': 'test@user.gov.uk',
                  'mobile_number': '+4412341234',
                  'state': 'active',
-                 'failed_login_count': 5
+                 'failed_login_count': 5,
+                 'permissions': {}
                  }
     user = User(user_data)
     return user
@@ -250,7 +253,8 @@ def api_user_request_password_reset():
                  'email_address': 'test@user.gov.uk',
                  'mobile_number': '+4412341234',
                  'state': 'request_password_reset',
-                 'failed_login_count': 5
+                 'failed_login_count': 5,
+                 'permissions': {}
                  }
     user = User(user_data)
     return user
@@ -513,6 +517,15 @@ def mock_get_jobs(mocker):
     return mocker.patch('app.job_api_client.get_job', side_effect=_get_jobs)
 
 
+@pytest.fixture(scope='function')
+def mock_has_permissions(mocker):
+    def _has_permission(service_id, permissions):
+        return True
+    return mocker.patch(
+        'app.notify_client.user_api_client.User.has_permissions',
+        side_effect=_has_permission)
+
+
 @pytest.fixture(scope='function')
 def mock_get_users_by_service(mocker):
     def _get_users_for_service(service_id):