diff --git a/app/main/views/choose_service.py b/app/main/views/choose_service.py index 4da0740e8..23a3a80ff 100644 --- a/app/main/views/choose_service.py +++ b/app/main/views/choose_service.py @@ -10,14 +10,15 @@ from app.notify_client.api_client import ServicesBrowsableItem def choose_service(): return render_template( 'views/choose-service.html', - services=[ServicesBrowsableItem(x) for x in service_api_client.get_services()['data']] + services=[ServicesBrowsableItem(x) for x in + service_api_client.get_services({'user_id': current_user.id})['data']] ) @main.route("/services-or-dashboard") @login_required def show_all_services_or_dashboard(): - services = service_api_client.get_services()['data'] + services = service_api_client.get_services({'user_id': current_user.id})['data'] if 1 == len(services): return redirect(url_for('.service_dashboard', service_id=services[0]['id'])) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index ffed74775..71c13ff7f 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -13,7 +13,7 @@ from app.utils import user_has_permissions @main.route("/services//dashboard") @login_required -@user_has_permissions() +@user_has_permissions('view_activity', admin_override=True) def service_dashboard(service_id): templates = service_api_client.get_service_templates(service_id)['data'] jobs = job_api_client.get_job(service_id)['data'] diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index ac231d053..d4971aba4 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -6,10 +6,7 @@ from flask import ( render_template, abort, jsonify, - flash, - redirect, - request, - url_for + request ) from flask_login import login_required from utils.template import Template @@ -24,7 +21,7 @@ from app.utils import ( @main.route("/services//jobs") @login_required -@user_has_permissions() +@user_has_permissions('view_activity', admin_override=True) def view_jobs(service_id): jobs = job_api_client.get_job(service_id)['data'] return render_template( @@ -36,7 +33,7 @@ def view_jobs(service_id): @main.route("/services//jobs/") @login_required -@user_has_permissions() +@user_has_permissions('view_activity', admin_override=True) def view_job(service_id, job_id): service = service_api_client.get_service(service_id)['data'] job = job_api_client.get_job(service_id, job_id)['data'] @@ -65,7 +62,7 @@ def view_job(service_id, job_id): @main.route("/services//jobs/.json") @login_required -@user_has_permissions() +@user_has_permissions('view_activity') def view_job_updates(service_id, job_id): service = service_api_client.get_service(service_id)['data'] job = job_api_client.get_job(service_id, job_id)['data'] @@ -93,7 +90,7 @@ def view_job_updates(service_id, job_id): @main.route('/services//notifications') @login_required -@user_has_permissions() +@user_has_permissions('view_activity', admin_override=True) def view_notifications(service_id): # TODO get the api to return count of pages as well. page = get_page_from_request() @@ -122,7 +119,7 @@ def view_notifications(service_id): @main.route("/services//jobs//notification/") @login_required -@user_has_permissions() +@user_has_permissions('view_activity', admin_override=True) def view_notification(service_id, job_id, notification_id): now = time.strftime('%H:%M') diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 0c9727301..4f552731c 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -27,13 +27,13 @@ from app.utils import user_has_permissions roles = { 'send_messages': ['send_texts', 'send_emails', 'send_letters'], 'manage_service': ['manage_users', 'manage_templates', 'manage_settings'], - 'manage_api_keys': ['manage_api_keys', 'access_developer_docs'] + 'manage_api_keys': ['manage_api_keys'] } @main.route("/services//users") @login_required -@user_has_permissions() +@user_has_permissions('view_activity', admin_override=True) def manage_users(service_id): return render_template( 'views/manage-users.html', @@ -57,13 +57,15 @@ def invite_user(service_id): if form.validate_on_submit(): email_address = form.email_address.data + # view_activity is a default role to be added to all users. + # All users will have at minimum view_activity to allow users to see notifications, + # templates, team members but no update privileges + permissions = ','.join(role for role in roles.keys() if request.form.get(role) == 'y').join('view_activity') invited_user = invite_api_client.create_invite( current_user.id, service_id, email_address, - ','.join( - role for role in roles.keys() if request.form.get(role) == 'y' - ) + permissions ) flash('Invite sent to {}'.format(invited_user.email_address), 'default_with_tick') @@ -95,7 +97,7 @@ def edit_user_permissions(service_id, user_id): user_id, service_id, permissions=set(chain.from_iterable( permissions for role, permissions in roles.items() if form[role].data - )) + )) | {'view_activity'} ) return redirect(url_for('.manage_users', service_id=service_id)) diff --git a/app/main/views/send.py b/app/main/views/send.py index ec92a5860..6b1fe2618 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -64,8 +64,12 @@ def get_page_headings(template_type): @main.route("/services//send/", methods=['GET']) @login_required -@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', 'manage_api_keys', - admin_override=True, or_=True) +@user_has_permissions('view_activity', + 'send_texts', + 'send_emails', + 'manage_templates', + 'manage_api_keys', + admin_override=True, any_=True) def choose_template(service_id, template_type): service = service_api_client.get_service(service_id)['data'] @@ -140,7 +144,7 @@ def send_messages(service_id, template_id): @main.route("/services//send/.csv", methods=['GET']) @login_required -@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', or_=True) +@user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', any_=True) def get_example_csv(service_id, template_id): template = Template(service_api_client.get_service_template(service_id, template_id)['data']) # Good practice to use context managers @@ -203,7 +207,7 @@ def send_message_to_self(service_id, template_id): @main.route("/services//send//from-api", methods=['GET']) @login_required -@user_has_permissions('manage_api_keys', 'access_developer_docs') +@user_has_permissions('manage_api_keys') def send_from_api(service_id, template_id): template = Template( service_api_client.get_service_template(service_id, template_id)['data'] diff --git a/app/notify_client/models.py b/app/notify_client/models.py index 12664fb1f..4ce76fe94 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -82,7 +82,7 @@ class User(UserMixin): def permissions(self, permissions): raise AttributeError("Read only property") - def has_permissions(self, permissions=[], or_=False, admin_override=False): + def has_permissions(self, permissions=[], any_=False, admin_override=False): # Only available to the platform admin user if admin_override and self.platform_admin: return True @@ -95,7 +95,7 @@ class User(UserMixin): # Service id is always set on the request for service specific views. service_id = request.view_args.get('service_id', None) if service_id in self._permissions: - if or_: + if any_: return any([x in self._permissions[service_id] for x in permissions]) return set(self._permissions[service_id]) >= set(permissions) return False diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 0a2d26ed3..6f94fb518 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -2,15 +2,17 @@ + {% if current_user.has_permissions(['view_activity'], admin_override=True) %} + {% endif %} {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} - {% elif current_user.has_permissions(['manage_templates','manage_api_keys'], admin_override=True, or_=True) %} + {% elif current_user.has_permissions(['view_activity', 'manage_templates','manage_api_keys'], admin_override=True, any_=True) %} - {% else %} + {% endif %} + {% if current_user.has_permissions(['view_activity']) %} {% endif %} - {% if current_user.has_permissions(['manage_api_keys', 'access_developer_docs']) %} + {% if current_user.has_permissions(['manage_api_keys']) %} diff --git a/app/templates/views/choose-template.html b/app/templates/views/choose-template.html index 9cb126643..1b97ad899 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(permissions=['manage_templates'], admin_override=True) %} + {% if current_user.has_permissions(permissions=['manage_templates'], any_=True) %} Add a new template {% else %}

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

@@ -34,7 +34,7 @@ {% if not has_jobs %} - {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters'], or_=True) %} + {% if current_user.has_permissions(permissions=['send_texts', 'send_emails', 'send_letters'], any_=True) %} {{ banner( """ Send yourself a test @@ -76,7 +76,7 @@ Send a batch Send yourself a test {% endif %} - {% if current_user.has_permissions(permissions=['manage_api_keys', 'access_developer_docs']) %} + {% if current_user.has_permissions(permissions=['manage_api_keys']) %} API integration {% endif %} diff --git a/app/templates/views/dashboard/dashboard.html b/app/templates/views/dashboard/dashboard.html index 411642b04..bdf9c1838 100644 --- a/app/templates/views/dashboard/dashboard.html +++ b/app/templates/views/dashboard/dashboard.html @@ -11,7 +11,9 @@ {% endif %} {% if not jobs %} - {% include 'views/dashboard/get-started.html' %} + {% if current_user.has_permissions(['manage_templates','send_texts', 'send_emails', 'send_letters']) %} + {% include 'views/dashboard/get-started.html' %} + {% endif%} {% else %}
Cancel invitation + {% if current_user.has_permissions(['manage_users']) %} + Cancel invitation + {% endif %} {% endcall %} {% else %} {% call field() %} diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html index 9b923c66b..de2d7992a 100644 --- a/app/templates/views/service_dashboard.html +++ b/app/templates/views/service_dashboard.html @@ -25,7 +25,7 @@ {% if not template_count and not jobs %} - {% if current_user.has_permissions(['manage_templates', 'send_texts', 'send_emails', 'send_letters'], or_=True, admin_override=True) %} + {% if current_user.has_permissions(['manage_templates', 'send_texts', 'send_emails', 'send_letters'], any_=True, admin_override=True) %} {% call banner_wrapper(subhead='Get started', type="tip") %}
    {% if current_user.has_permissions(['manage_templates'], admin_override=True) %} diff --git a/app/utils.py b/app/utils.py index b8c023744..563bb07ff 100644 --- a/app/utils.py +++ b/app/utils.py @@ -30,13 +30,13 @@ class BrowsableItem(object): pass -def user_has_permissions(*permissions, admin_override=False, or_=False): +def user_has_permissions(*permissions, admin_override=False, any_=False): def wrap(func): @wraps(func) def wrap_func(*args, **kwargs): from flask_login import current_user if current_user and current_user.has_permissions(permissions=permissions, - admin_override=admin_override, or_=or_): + admin_override=admin_override, any_=any_): return func(*args, **kwargs) else: abort(403) diff --git a/environment_test.sh b/environment_test.sh index 9baca39f1..57f89ae58 100644 --- a/environment_test.sh +++ b/environment_test.sh @@ -1,6 +1,6 @@ export NOTIFY_ADMIN_ENVIRONMENT='config.Test' export ADMIN_CLIENT_SECRET='dev-notify-secret-key' export ADMIN_CLIENT_USER_NAME='dev-notify-admin' -export API_HOST_NAME='http://localhost:6011' +export API_HOST_NAME='http://localhost:6311' export DANGEROUS_SALT='dev-notify-salt' export SECRET_KEY='dev-notify-secret-key' diff --git a/tests/__init__.py b/tests/__init__.py index 707c5edb9..7064eac13 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -5,12 +5,14 @@ from flask_login import login_user class TestClient(FlaskClient): - def login(self, user): + def login(self, user, mocker=None, service=None): # Skipping authentication here and just log them in with self.session_transaction() as session: session['user_id'] = user.id session['_fresh'] = True - + if mocker and service: + mocker.patch('app.user_api_client.get_user', return_value=user) + mocker.patch('app.service_api_client.get_service', return_value={'data': service}) login_user(user, remember=True) def login_fresh(self): diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index 30a00e336..f168a3a39 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -5,12 +5,12 @@ from werkzeug.exceptions import Forbidden from flask import request -def _test_permissions(app_, usr, permissions, service_id, will_succeed, or_=False, admin_override=False): +def _test_permissions(app_, usr, permissions, service_id, will_succeed, any_=False, admin_override=False): with app_.test_request_context() as ctx: request.view_args.update({'service_id': service_id}) with app_.test_client() as client: client.login(usr) - decorator = user_has_permissions(*permissions, or_=or_, admin_override=admin_override) + decorator = user_has_permissions(*permissions, any_=any_, admin_override=admin_override) decorated_index = decorator(index) if will_succeed: response = decorated_index() @@ -54,7 +54,7 @@ def test_user_has_permissions_or(app_, mocker): ['something', 'manage_users'], '', True, - or_=True) + any_=True) def test_user_has_permissions_multiple(app_, diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index 72fd6007a..12509d391 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/tests/app/main/views/test_api_keys.py @@ -161,6 +161,6 @@ def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, m "GET", 403, url_for(route, service_id=service_one['id'], key_id=123), - ['blah'], + ['view_activity'], api_user_active, service_one) diff --git a/tests/app/main/views/test_choose_services.py b/tests/app/main/views/test_choose_services.py index ebdd32eca..ba8714fec 100644 --- a/tests/app/main/views/test_choose_services.py +++ b/tests/app/main/views/test_choose_services.py @@ -1,5 +1,4 @@ from flask import url_for -import pytest def test_should_show_choose_services_page(app_, diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 34fcf71e2..b59d474b8 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -27,12 +27,8 @@ def _test_dashboard_menu(mocker, app_, usr, service, permissions): with app_.test_request_context(): with app_.test_client() as client: usr._permissions[str(service['id'])] = permissions - mocker.patch( - 'app.user_api_client.check_verify_code', - return_value=(True, '')) - mocker.patch( - 'app.service_api_client.get_services', - return_value={'data': []}) + mocker.patch('app.user_api_client.check_verify_code', return_value=(True, '')) + mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) mocker.patch('app.user_api_client.get_user', return_value=usr) mocker.patch('app.user_api_client.get_user_by_email', return_value=usr) mocker.patch('app.service_api_client.get_service', return_value={'data': service}) @@ -48,7 +44,7 @@ def test_menu_send_messages(mocker, app_, api_user_active, service_one, mock_get app_, api_user_active, service_one, - ['send_texts', 'send_emails', 'send_letters']) + ['view_activity', 'send_texts', 'send_emails', 'send_letters']) page = resp.get_data(as_text=True) assert url_for( 'main.choose_template', @@ -58,12 +54,12 @@ def test_menu_send_messages(mocker, app_, api_user_active, service_one, mock_get 'main.choose_template', service_id=service_one['id'], template_type='sms')in page - + assert url_for('main.view_notifications', service_id=service_one['id']) in page assert url_for('main.manage_users', service_id=service_one['id']) in page - assert url_for('main.service_settings', service_id=service_one['id']) not in page + assert url_for('main.documentation') in page + assert url_for('main.service_settings', service_id=service_one['id']) not in page assert url_for('main.api_keys', service_id=service_one['id']) not in page - assert url_for('main.documentation', service_id=service_one['id']) not in page assert url_for('main.show_all_services') not in page @@ -74,7 +70,7 @@ def test_menu_manage_service(mocker, app_, api_user_active, service_one, mock_ge app_, api_user_active, service_one, - ['manage_users', 'manage_templates', 'manage_settings']) + ['view_activity', 'manage_users', 'manage_templates', 'manage_settings']) page = resp.get_data(as_text=True) assert url_for( 'main.choose_template', @@ -84,9 +80,10 @@ def test_menu_manage_service(mocker, app_, api_user_active, service_one, mock_ge 'main.choose_template', service_id=service_one['id'], template_type='sms') in page - + assert url_for('main.view_notifications', service_id=service_one['id']) in page assert url_for('main.manage_users', service_id=service_one['id']) in page assert url_for('main.service_settings', service_id=service_one['id']) in page + assert url_for('main.documentation') in page assert url_for('main.api_keys', service_id=service_one['id']) not in page assert url_for('main.show_all_services') not in page @@ -99,7 +96,7 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g app_, api_user_active, service_one, - ['manage_api_keys', 'access_developer_docs']) + ['view_activity', 'manage_api_keys']) page = resp.get_data(as_text=True) assert url_for( 'main.choose_template', @@ -109,7 +106,7 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g 'main.choose_template', service_id=service_one['id'], template_type='sms') in page - + assert url_for('main.view_notifications', service_id=service_one['id']) in page assert url_for('main.manage_users', service_id=service_one['id']) in page assert url_for('main.service_settings', service_id=service_one['id']) not in page assert url_for('main.show_all_services') not in page @@ -159,6 +156,6 @@ def test_route_for_service_permissions(mocker, url_for( route, service_id=service_one['id']), - [], + ['view_activity'], api_user_active, service_one) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 9cbe24aa8..0e8100882 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -14,8 +14,7 @@ def test_should_show_overview_page( service = service_1(active_user_with_permissions) with app_.test_request_context(): with app_.test_client() as client: - _mocks_for_test_manage_users(mocker, active_user_with_permissions, service) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) response = client.get(url_for('main.manage_users', service_id=service['id'])) @@ -27,16 +26,12 @@ def test_should_show_overview_page( def test_should_show_page_for_one_user( app_, active_user_with_permissions, - mocker, - 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={'data': service}) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) response = client.get(url_for('main.edit_user_permissions', service_id=service['id'], user_id=0)) assert response.status_code == 200 @@ -45,7 +40,6 @@ def test_should_show_page_for_one_user( def test_edit_user_permissions( app_, active_user_with_permissions, - mock_login, mocker, mock_get_invites_for_service, mock_set_user_permissions @@ -54,10 +48,7 @@ def test_edit_user_permissions( with app_.test_request_context(): with app_.test_client() as client: - mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions) - mocker.patch('app.service_api_client.get_service', return_value={'data': service}) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, 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, @@ -80,7 +71,7 @@ def test_edit_user_permissions( 'manage_templates', 'manage_settings', 'manage_api_keys', - 'access_developer_docs' + 'view_activity' } ) @@ -97,11 +88,10 @@ def test_edit_some_user_permissions( data = [InvitedUser(**sample_invite)] with app_.test_request_context(): with app_.test_client() as client: - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) service_id = service['id'] mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) - _mocks_for_test_manage_users(mocker, active_user_with_permissions, service) response = client.post(url_for( 'main.edit_user_permissions', service_id=service_id, user_id=active_user_with_permissions.id ), data={'email_address': active_user_with_permissions.email_address, @@ -119,17 +109,12 @@ def test_edit_some_user_permissions( permissions={ 'send_texts', 'send_emails', - 'send_letters' + 'send_letters', + 'view_activity' } ) -def _mocks_for_test_manage_users(mocker, active_user_with_permissions, service): - mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions) - mocker.patch('app.service_api_client.get_service', return_value={'data': service}) - mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) - - def test_should_show_page_for_inviting_user( app_, active_user_with_permissions, @@ -138,8 +123,7 @@ def test_should_show_page_for_inviting_user( service = service_1(active_user_with_permissions) with app_.test_request_context(): with app_.test_client() as client: - _mocks_for_test_manage_users(mocker, active_user_with_permissions, service) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) response = client.get(url_for('main.invite_user', service_id=service['id'])) assert 'Invite a team member' in response.get_data(as_text=True) @@ -159,8 +143,7 @@ def test_invite_user( data = [InvitedUser(**sample_invite)] with app_.test_request_context(): with app_.test_client() as client: - _mocks_for_test_manage_users(mocker, active_user_with_permissions, service) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) mocker.patch('app.invite_api_client.create_invite', return_value=InvitedUser(**sample_invite)) @@ -190,8 +173,7 @@ def test_cancel_invited_user_cancels_user_invitations(app_, import uuid invited_user_id = uuid.uuid4() service = service_1(active_user_with_permissions) - _mocks_for_test_manage_users(mocker, active_user_with_permissions, service) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) response = client.get(url_for('main.cancel_invited_user', service_id=service['id'], invited_user_id=invited_user_id)) @@ -207,8 +189,7 @@ def test_manage_users_shows_invited_user(app_, data = [InvitedUser(**sample_invite)] with app_.test_request_context(): with app_.test_client() as client: - _mocks_for_test_manage_users(mocker, active_user_with_permissions, service) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) @@ -227,7 +208,6 @@ def test_manage_users_shows_invited_user(app_, def test_manage_users_does_not_show_accepted_invite(app_, mocker, active_user_with_permissions, - mock_login, sample_invite): import uuid @@ -238,10 +218,7 @@ def test_manage_users_does_not_show_accepted_invite(app_, service = service_1(active_user_with_permissions) with app_.test_request_context(): with app_.test_client() as client: - mocker.patch('app.user_api_client.get_user', return_value=active_user_with_permissions) - mocker.patch('app.service_api_client.get_service', return_value={'data': service}) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) mocker.patch('app.user_api_client.get_users_for_service', return_value=[active_user_with_permissions]) mocker.patch('app.invite_api_client.get_invites_for_service', return_value=data) @@ -257,19 +234,14 @@ def test_manage_users_does_not_show_accepted_invite(app_, def test_user_cant_invite_themselves( app_, - mock_login, mocker, active_user_with_permissions, - mock_create_invite, - mock_get_invites_for_service + mock_create_invite ): 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={'data': service}) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - client.login(active_user_with_permissions) + client.login(active_user_with_permissions, mocker, service) response = client.post( url_for('main.invite_user', service_id=service['id']), data={'email_address': active_user_with_permissions.email_address, @@ -284,19 +256,16 @@ def test_user_cant_invite_themselves( assert page.h1.string.strip() == 'Invite a team member' form_error = page.find('span', class_='error-message').string.strip() assert form_error == "You can't send an invitation to yourself" + assert not mock_create_invite.called def test_no_permission_manage_users_page(app_, service_one, api_user_active, - mock_login, - mock_get_user, - mock_get_service, - mock_get_users_by_service, - mock_get_invites_for_service): + mocker): with app_.test_request_context(): with app_.test_client() as client: - client.login(api_user_active) + client.login(api_user_active, mocker, service_one) response = client.get(url_for('main.manage_users', service_id=service_one['id'])) resp_text = response.get_data(as_text=True) assert url_for('.invite_user', service_id=service_one['id']) not in resp_text @@ -305,45 +274,39 @@ def test_no_permission_manage_users_page(app_, def test_get_remove_user_from_service(app_, - api_user_active, - mock_login, - mock_get_user_by_email, - mock_get_service, - mock_get_users_by_service, - mock_get_user, - mock_has_permissions): + active_user_with_permissions, + service_one, + mocker): with app_.test_request_context(): with app_.test_client() as client: - service = mock_get_service("12345")['data'] - client.login(api_user_active) + client.login(active_user_with_permissions, mocker, service_one) response = client.get( url_for( 'main.remove_user_from_service', - service_id=service['id'], - user_id=api_user_active.id)) + service_id=service_one['id'], + user_id=active_user_with_permissions.id)) assert response.status_code == 200 assert "Are you sure you want to remove" in response.get_data(as_text=True) assert "Remove user from service" in response.get_data(as_text=True) def test_remove_user_from_service(app_, - api_user_active, - mock_login, - mock_get_user_by_email, - mock_get_service, + active_user_with_permissions, + service_one, + mocker, mock_get_users_by_service, mock_get_user, - mock_has_permissions, mock_remove_user_from_service): with app_.test_request_context(): with app_.test_client() as client: - service = mock_get_service("12345")['data'] - client.login(api_user_active) + client.login(active_user_with_permissions, mocker, service_one) response = client.post( url_for( 'main.remove_user_from_service', - service_id=service['id'], - user_id=api_user_active.id)) + service_id=service_one['id'], + user_id=active_user_with_permissions.id)) assert response.status_code == 302 assert response.location == url_for( - 'main.manage_users', service_id=service['id'], _external=True) + 'main.manage_users', service_id=service_one['id'], _external=True) + mock_remove_user_from_service.assert_called_once_with(service_one['id'], + str(active_user_with_permissions.id)) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 8646cf3be..520ccc5be 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -442,7 +442,7 @@ def test_route_choose_template_manage_api_keys_permissions(mocker, 'main.choose_template', service_id=service_one['id'], template_type='sms'), - ['manage_api_keys', 'access_developer_docs'], + ['manage_api_keys'], api_user_active, service_one) page = resp.get_data(as_text=True) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index d8096f675..bd57906e6 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -154,6 +154,27 @@ def test_route_permissions(mocker, service_one) +def test_route_permissions_for_choose_template(mocker, + app_, + api_user_active, + service_one, + mock_get_service_templates): + mocker.patch('app.job_api_client.get_job') + with app_.test_request_context(): + validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for( + 'main.choose_template', + service_id=service_one['id'], + template_type='sms'), + ['view_activity'], + api_user_active, + service_one) + + def test_route_invalid_permissions(mocker, app_, api_user_active, @@ -175,6 +196,6 @@ def test_route_invalid_permissions(mocker, service_id=service_one['id'], template_type='sms', template_id=123), - ['blah'], + ['view_activity'], api_user_active, service_one) diff --git a/tests/conftest.py b/tests/conftest.py index 9c9a9925a..762846a9a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -158,13 +158,13 @@ def mock_get_service_statistics(mocker): @pytest.fixture(scope='function') def mock_get_service_template(mocker): - def _create(service_id, template_id): + def _get(service_id, template_id): template = template_json( service_id, template_id, "Two week reminder", "sms", "Your vehicle tax is about to expire") return {'data': template} return mocker.patch( - 'app.service_api_client.get_service_template', side_effect=_create) + 'app.service_api_client.get_service_template', side_effect=_get) @pytest.fixture(scope='function') @@ -279,7 +279,8 @@ def api_user_active(): 'mobile_number': '+4412341234', 'state': 'active', 'failed_login_count': 0, - 'permissions': {} + 'permissions': {}, + 'platform_admin': False } user = User(user_data) return user @@ -303,7 +304,7 @@ def active_user_with_permissions(): 'manage_templates', 'manage_settings', 'manage_api_keys', - 'access_developer_docs']}, + 'view_activity']}, 'platform_admin': False } user = User(user_data) @@ -659,7 +660,7 @@ def mock_get_notifications_with_previous_next(mocker): @pytest.fixture(scope='function') def mock_has_permissions(mocker): - def _has_permission(permissions=None, or_=False, admin_override=False): + def _has_permission(permissions=None, any_=False, admin_override=False): return True return mocker.patch( 'app.notify_client.user_api_client.User.has_permissions',