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 @@ - {% 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']) %} - {% 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) %} {% 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) %} {% 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']) %} {% endif %} - {% if current_user.has_permissions([], admin_override=True) %} + {% if current_user.has_permissions(admin_override=True) %} 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 @@

{{ page_heading }}

- {% if current_user.has_permissions(['manage_templates'], admin_override=True) %} + {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) %} Add a new template {% else %}

You need to ask your service manager to add templates before you can send messages

@@ -26,7 +26,7 @@

{{ page_heading }}

- {% if current_user.has_permissions(['manage_templates'], admin_override=True) %} + {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) %}
Add new template
@@ -34,7 +34,7 @@ {% 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 @@
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 %} Edit permission @@ -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') %} Cancel invitation 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") %}
    - {% if current_user.has_permissions(['manage_templates'], admin_override=True) %} + {% if current_user.has_permissions(permissions=['manage_templates'], admin_override=True) %}
  1. Add a template
  2. {% endif %} - {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} + {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters']) %}
  3. Send yourself a text message
  4. @@ -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']) %} Send yourself a text message {% 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']) %} 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)