diff --git a/app/__init__.py b/app/__init__.py index cf4529634..77cd41c86 100644 --- a/app/__init__.py +++ b/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.its_dangerous_session import ItsdangerousSessionInterface from app.asset_fingerprinter import AssetFingerprinter from app.utils import validate_phone_number, InvalidPhoneError @@ -33,6 +34,7 @@ api_key_api_client = ApiKeyApiClient() job_api_client = JobApiClient() status_api_client = StatusApiClient() asset_fingerprinter = AssetFingerprinter() +permission_api_client = PermissionApiClient() def create_app(config_name, config_overrides=None): @@ -49,6 +51,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) login_manager.init_app(application) login_manager.login_view = 'main.sign_in' diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 36959a86d..8053e2cf9 100644 --- a/app/main/views/api_keys.py +++ b/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//documentation") @@ -13,6 +14,7 @@ def documentation(service_id): @main.route("/services//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//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//api-keys/revoke/", 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': diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index cb15fce90..f11722854 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -23,6 +23,8 @@ def service_dashboard(service_id): try: service = get_service_by_id(service_id) session['service_name'] = service['data']['name'] + print(service['data']['id']) + session['service_id'] = service['data']['id'] except HTTPError as e: if e.status_code == 404: abort(404) diff --git a/app/main/views/sms.py b/app/main/views/sms.py index 42340cbaa..2e3f69091 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -44,10 +44,6 @@ def choose_sms_template(service_id): abort(404) else: raise e - print("="*80) - print(jobs) - print(len(jobs)) - print(bool(len(jobs))) return render_template( 'views/choose-sms-template.html', templates=[ diff --git a/app/notify_client/permission_api_client.py b/app/notify_client/permission_api_client.py new file mode 100644 index 000000000..a112251af --- /dev/null +++ b/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'] diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 693a321f7..dafd4f66b 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -89,7 +89,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 @@ -154,18 +154,12 @@ class User(UserMixin): @permissions.setter def permissions(self, permissions): - if permissions is None: - permissions = set() - self._permissions = set(permissions) + raise AttributeError("Read only property") - def add_permissions(self, permissions): - self._permissions.update(permissions) - - def remove_permissions(self, permissions): - self._permissions -= permissions - - def has_permissions(self, permissions): - return self._permissions > set(permissions) + def has_permissions(self, service_id, permissions): + if service_id in self._permissions: + return set(self._permissions[service_id]) > set(permissions) + return False @property def failed_login_count(self): diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 4a379f3d8..2fb1c7186 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -2,16 +2,22 @@

{{ session.get('service_name', 'Service') }}

+ {% if current_user.has_permissions(session.get('service_id', ''), ['send_messages']) %} + {% endif %} + {% if current_user.has_permissions(session.get('service_id', ''), ['manage_service']) %} + {% endif %} + {% if current_user.has_permissions(session.get('service_id', ''), ['manage_api_keys']) %} + {% endif %} diff --git a/app/utils.py b/app/utils.py index f814f4b81..ad89aea60 100644 --- a/app/utils.py +++ b/app/utils.py @@ -1,5 +1,5 @@ from functools import wraps -from flask import abort +from flask import (abort, session) class BrowsableItem(object): @@ -80,8 +80,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 diff --git a/tests/__init__.py b/tests/__init__.py index 850f1b53a..6170a7817 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -57,14 +57,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 diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index aaba5f2b0..d22855fe6 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/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) diff --git a/tests/conftest.py b/tests/conftest.py index 308ee42b7..9d6578fc0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -195,7 +195,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 @@ -210,7 +211,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 @@ -225,7 +227,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 @@ -240,7 +243,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 @@ -501,3 +505,12 @@ def mock_get_jobs(mocker): data.append(job_data) return {"data": data} 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)