From b79901fe281389ad366412c8218994d2bffa087b Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 29 Mar 2016 13:23:36 +0100 Subject: [PATCH 01/10] With the addition of has_permissions on the dashboard, jobs, and manage_users pages a platform admin user or a users with no permissions on the service could no longer see the page. A new permission has been added, view_activity, to resolve this issue. Another pull request in notifications-admin will be required to update all users with a default permission of view_activity. --- app/main/views/dashboard.py | 2 +- app/main/views/jobs.py | 18 +++++++----------- app/main/views/send.py | 6 +++++- app/templates/main_nav.html | 7 +++++-- app/templates/views/manage-users.html | 4 +++- tests/app/main/views/test_dashboard.py | 8 ++++---- tests/app/main/views/test_templates.py | 23 ++++++++++++++++++++++- 7 files changed, 47 insertions(+), 21 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 93b11cdc9..7c0514aab 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -15,7 +15,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 = templates_dao.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 4e5082d31..124b12172 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -6,24 +6,20 @@ from flask import ( render_template, abort, jsonify, - flash, - redirect, - request, - url_for + request ) from flask_login import login_required from utils.template import Template from app import job_api_client, notification_api_client from app.main import main -from app.main.dao import templates_dao -from app.main.dao import services_dao +from app.main.dao import (services_dao, templates_dao) from app.utils import (get_page_from_request, generate_previous_next_dict, user_has_permissions) @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( @@ -35,7 +31,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 = services_dao.get_service_by_id_or_404(service_id) job = job_api_client.get_job(service_id, job_id)['data'] @@ -64,7 +60,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 = services_dao.get_service_by_id_or_404(service_id) job = job_api_client.get_job(service_id, job_id)['data'] @@ -92,7 +88,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() @@ -121,7 +117,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/send.py b/app/main/views/send.py index 9f252292d..6da51784a 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -66,7 +66,11 @@ 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', +@user_has_permissions('view_activity', + 'send_texts', + 'send_emails', + 'manage_templates', + 'manage_api_keys', admin_override=True, or_=True) def choose_template(service_id, template_type): diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 0a2d26ed3..09c2229fd 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -2,15 +2,17 @@

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

+ {% 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, or_=True) %} - {% else %} + {% endif %} + {% if current_user.has_permissions(['view_activity'], admin_override=True) %} diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 0a7a5ada9..aa626e5f5 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -63,7 +63,9 @@ Manage users – GOV.UK Notify {{ boolean_field(item.has_permissions(permissions=['manage_api_keys', 'access_developer_docs'])) }} {% if item.status == 'pending' %} {% call field(align='right') %} - Cancel invitation + {% if current_user.has_permissions(['manage_users']) %} + Cancel invitation + {% endif %} {% endcall %} {% else %} {% call field() %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 34fcf71e2..6ef4127aa 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -48,7 +48,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', @@ -74,7 +74,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', @@ -99,7 +99,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', 'access_developer_docs']) page = resp.get_data(as_text=True) assert url_for( 'main.choose_template', @@ -159,6 +159,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_templates.py b/tests/app/main/views/test_templates.py index d8096f675..af85ffce4 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_tempalte(mocker, + app_, + api_user_active, + service_one, + mock_get_service_template): + 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', + template_id=123), + ['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) From 01959b8576dc09f9c938d44f4718526ad7ad0cbc Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 29 Mar 2016 15:20:43 +0100 Subject: [PATCH 02/10] Update invited user to have the view_activity permission. Update dashboard.html for view_activity --- app/main/views/manage_users.py | 2 +- app/templates/views/dashboard/dashboard.html | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 6006f0553..709ec1e8d 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -66,7 +66,7 @@ def invite_user(service_id): email_address, ','.join( role for role in roles.keys() if request.form.get(role) == 'y' - ) + ).join('view_activity') ) flash('Invite sent to {}'.format(invited_user.email_address), 'default_with_tick') 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 %}
Date: Tue, 29 Mar 2016 15:40:33 +0100 Subject: [PATCH 03/10] Test the right url for choose_template --- tests/app/main/views/test_templates.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index af85ffce4..2ece66d2a 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -154,7 +154,7 @@ def test_route_permissions(mocker, service_one) -def test_route_permissions_for_choose_tempalte(mocker, +def test_route_permissions_for_choose_template(mocker, app_, api_user_active, service_one, @@ -168,8 +168,7 @@ def test_route_permissions_for_choose_tempalte(mocker, url_for( 'main.choose_template', service_id=service_one['id'], - template_type='sms', - template_id=123), + template_type='sms'), ['view_activity'], api_user_active, service_one) From 99bd94799e062e654eb82a4067a068284f0c9eb7 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 29 Mar 2016 17:33:12 +0100 Subject: [PATCH 04/10] Remove access_developer_docs permission, it doesn't make sense to need it. Add view_activity permission when the permissions are being editted. --- app/main/views/manage_users.py | 4 ++-- app/main/views/send.py | 2 +- app/templates/main_nav.html | 2 +- app/templates/views/choose-template.html | 4 ++-- app/templates/views/manage-users.html | 4 ++-- tests/app/main/views/test_dashboard.py | 2 +- tests/app/main/views/test_manage_users.py | 5 +++-- tests/app/main/views/test_send.py | 2 +- tests/conftest.py | 3 +-- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 709ec1e8d..548b511ad 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -30,7 +30,7 @@ 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'] } @@ -98,7 +98,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 6da51784a..a48d6c6f3 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -198,7 +198,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( templates_dao.get_service_template_or_404(service_id, template_id)['data'] diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 09c2229fd..a25182536 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -29,7 +29,7 @@
  • View team members
    • {% 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..022fbfe74 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'], admin_override=True, or_=True) %} Add a new template {% else %}

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

    @@ -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/manage-users.html b/app/templates/views/manage-users.html index aa626e5f5..5f0cc021f 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -41,7 +41,7 @@ Manage users – GOV.UK Notify {% endcall %} {{ 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'])) }} + {{ boolean_field(item.has_permissions(permissions=['manage_api_keys'])) }} {% call field(align='right') %} {% if current_user.has_permissions(['manage_users']) %} {% if current_user.id != item.id %} @@ -60,7 +60,7 @@ Manage users – GOV.UK Notify {% endcall %} {{ 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'])) }} + {{ boolean_field(item.has_permissions(permissions=['manage_api_keys'])) }} {% if item.status == 'pending' %} {% call field(align='right') %} {% if current_user.has_permissions(['manage_users']) %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 6ef4127aa..5765985cf 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -99,7 +99,7 @@ def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one, mock_g app_, api_user_active, service_one, - ['view_activity', 'manage_api_keys', 'access_developer_docs']) + ['view_activity', 'manage_api_keys']) page = resp.get_data(as_text=True) assert url_for( 'main.choose_template', diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index e51e7e602..7ea994730 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -80,7 +80,7 @@ def test_edit_user_permissions( 'manage_templates', 'manage_settings', 'manage_api_keys', - 'access_developer_docs' + 'view_activity' } ) @@ -119,7 +119,8 @@ def test_edit_some_user_permissions( permissions={ 'send_texts', 'send_emails', - 'send_letters' + 'send_letters', + 'view_activity' } ) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index f45ed0c17..d7b96c4aa 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -445,7 +445,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/conftest.py b/tests/conftest.py index 011a2ef4c..2d421cc5c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -302,8 +302,7 @@ def active_user_with_permissions(): 'manage_users', 'manage_templates', 'manage_settings', - 'manage_api_keys', - 'access_developer_docs']}, + 'manage_api_keys']}, 'platform_admin': False } user = User(user_data) From a4629a290a013c2583e70c94ed3654046ec33001 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 29 Mar 2016 17:46:53 +0100 Subject: [PATCH 05/10] Add view_activity to decorator. --- app/main/views/manage_users.py | 2 +- tests/conftest.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 548b511ad..fd2a8d648 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -36,7 +36,7 @@ roles = { @main.route("/services//users") @login_required -@user_has_permissions() +@user_has_permissions('view_activity') def manage_users(service_id): return render_template( 'views/manage-users.html', diff --git a/tests/conftest.py b/tests/conftest.py index 2d421cc5c..95391c390 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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 @@ -302,7 +303,8 @@ def active_user_with_permissions(): 'manage_users', 'manage_templates', 'manage_settings', - 'manage_api_keys']}, + 'manage_api_keys', + 'view_activity']}, 'platform_admin': False } user = User(user_data) From 017baca5e528251c5adb5d9f4d541ac0e3e7c319 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 30 Mar 2016 08:41:07 +0100 Subject: [PATCH 06/10] Add proper mocks for the test. Updated the localhost in the environment_test file so that if the tests fail if a mock is missing. --- environment_test.sh | 2 +- tests/app/main/views/test_templates.py | 3 ++- tests/conftest.py | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) 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/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 2ece66d2a..bd57906e6 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -158,7 +158,8 @@ def test_route_permissions_for_choose_template(mocker, app_, api_user_active, service_one, - mock_get_service_template): + mock_get_service_templates): + mocker.patch('app.job_api_client.get_job') with app_.test_request_context(): validate_route_permission( mocker, diff --git a/tests/conftest.py b/tests/conftest.py index 95391c390..f2a548bea 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') From eb72c6e5bff9ef93d6064f07d74caf642ff96272 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 30 Mar 2016 09:31:53 +0100 Subject: [PATCH 07/10] Refactor unit test --- tests/app/main/views/test_api_keys.py | 2 +- tests/app/main/views/test_choose_services.py | 2 - tests/app/main/views/test_dashboard.py | 19 ++-- tests/app/main/views/test_manage_users.py | 105 +++++++------------ 4 files changed, 46 insertions(+), 82 deletions(-) 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 02b0845a5..0bf475fcf 100644 --- a/tests/app/main/views/test_choose_services.py +++ b/tests/app/main/views/test_choose_services.py @@ -1,6 +1,4 @@ -from tests import create_test_user 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 5765985cf..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}) @@ -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 @@ -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 @@ -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 diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 7ea994730..498fddbb9 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) + _login_user(active_user_with_permissions, client, 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=service) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - client.login(active_user_with_permissions) + _login_user(active_user_with_permissions, client, 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=service) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - client.login(active_user_with_permissions) + _login_user(active_user_with_permissions, client, 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, @@ -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) + _login_user(active_user_with_permissions, client, 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, @@ -125,12 +115,6 @@ def test_edit_some_user_permissions( ) -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, @@ -139,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) + _login_user(active_user_with_permissions, client, 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) @@ -160,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) + _login_user(active_user_with_permissions, client, 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)) @@ -191,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) + _login_user(active_user_with_permissions, client, mocker, service) response = client.get(url_for('main.cancel_invited_user', service_id=service['id'], invited_user_id=invited_user_id)) @@ -208,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) + _login_user(active_user_with_permissions, client, 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]) @@ -228,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 @@ -239,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=service) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - client.login(active_user_with_permissions) + _login_user(active_user_with_permissions, client, 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) @@ -258,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=service) - mocker.patch('app.service_api_client.get_services', return_value={'data': [service]}) - client.login(active_user_with_permissions) + _login_user(active_user_with_permissions, client, mocker, service) response = client.post( url_for('main.invite_user', service_id=service['id']), data={'email_address': active_user_with_permissions.email_address, @@ -285,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) + _login_user(api_user_active, client, 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 @@ -306,45 +274,46 @@ 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) + _login_user(active_user_with_permissions, client, 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) + _login_user(active_user_with_permissions, client, 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)) + + +def _login_user(user, client, mocker, service): + mocker.patch('app.user_api_client.get_user', return_value=user) + 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(user) From 542d0f939e1ece501fb7f1131f193814bbd617ae Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 30 Mar 2016 10:53:15 +0100 Subject: [PATCH 08/10] Add comment when adding the view_activity permission as a default. Refactor client.login method so that we can start to migrate to one way to login for unit tests. --- app/main/views/manage_users.py | 6 ++--- tests/__init__.py | 6 +++-- tests/app/main/views/test_manage_users.py | 33 +++++++++-------------- 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 76249c44d..4e60918f1 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -57,13 +57,13 @@ 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 + 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' - ).join('view_activity') + permissions ) flash('Invite sent to {}'.format(invited_user.email_address), 'default_with_tick') 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/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 17faa52bc..0e8100882 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -14,7 +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: - _login_user(active_user_with_permissions, client, mocker, service) + 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'])) @@ -31,7 +31,7 @@ def test_should_show_page_for_one_user( service = service_1(active_user_with_permissions) with app_.test_request_context(): with app_.test_client() as client: - _login_user(active_user_with_permissions, client, mocker, service) + 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 @@ -48,7 +48,7 @@ def test_edit_user_permissions( with app_.test_request_context(): with app_.test_client() as client: - _login_user(active_user_with_permissions, client, mocker, service) + 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, @@ -88,7 +88,7 @@ def test_edit_some_user_permissions( data = [InvitedUser(**sample_invite)] with app_.test_request_context(): with app_.test_client() as client: - _login_user(active_user_with_permissions, client, mocker, service) + 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) @@ -123,7 +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: - _login_user(active_user_with_permissions, client, mocker, service) + 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) @@ -143,7 +143,7 @@ def test_invite_user( data = [InvitedUser(**sample_invite)] with app_.test_request_context(): with app_.test_client() as client: - _login_user(active_user_with_permissions, client, mocker, service) + 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)) @@ -173,7 +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) - _login_user(active_user_with_permissions, client, mocker, service) + 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)) @@ -189,7 +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: - _login_user(active_user_with_permissions, client, mocker, service) + 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]) @@ -218,7 +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: - _login_user(active_user_with_permissions, client, mocker, service) + 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) @@ -241,7 +241,7 @@ def test_user_cant_invite_themselves( service = service_1(active_user_with_permissions) with app_.test_request_context(): with app_.test_client() as client: - _login_user(active_user_with_permissions, client, mocker, service) + 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, @@ -265,7 +265,7 @@ def test_no_permission_manage_users_page(app_, mocker): with app_.test_request_context(): with app_.test_client() as client: - _login_user(api_user_active, client, mocker, service_one) + 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 @@ -279,7 +279,7 @@ def test_get_remove_user_from_service(app_, mocker): with app_.test_request_context(): with app_.test_client() as client: - _login_user(active_user_with_permissions, client, mocker, service_one) + client.login(active_user_with_permissions, mocker, service_one) response = client.get( url_for( 'main.remove_user_from_service', @@ -299,7 +299,7 @@ def test_remove_user_from_service(app_, mock_remove_user_from_service): with app_.test_request_context(): with app_.test_client() as client: - _login_user(active_user_with_permissions, client, mocker, service_one) + client.login(active_user_with_permissions, mocker, service_one) response = client.post( url_for( 'main.remove_user_from_service', @@ -310,10 +310,3 @@ def test_remove_user_from_service(app_, '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)) - - -def _login_user(user, client, mocker, service): - mocker.patch('app.user_api_client.get_user', return_value=user) - 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(user) From 953f2a22452599a3c457b412ff00b3617f2d1ef3 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 30 Mar 2016 10:57:03 +0100 Subject: [PATCH 09/10] Add comment to describe view_activity --- app/main/views/manage_users.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 4e60918f1..4af565d2f 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -57,7 +57,9 @@ 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 + # 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, From e67bb5f7163457737b973ce26062b721dfaa71dd Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 30 Mar 2016 11:30:18 +0100 Subject: [PATCH 10/10] Fix main_nav for platform admin Renamed or_ to any_ --- app/main/views/choose_service.py | 5 +++-- app/main/views/manage_users.py | 2 +- app/main/views/send.py | 4 ++-- app/notify_client/models.py | 4 ++-- app/templates/main_nav.html | 4 ++-- app/templates/views/choose-template.html | 4 ++-- app/templates/views/service_dashboard.html | 2 +- app/utils.py | 4 ++-- tests/app/main/test_permissions.py | 6 +++--- tests/conftest.py | 2 +- 10 files changed, 19 insertions(+), 18 deletions(-) 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/manage_users.py b/app/main/views/manage_users.py index 4af565d2f..4f552731c 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -33,7 +33,7 @@ roles = { @main.route("/services//users") @login_required -@user_has_permissions('view_activity') +@user_has_permissions('view_activity', admin_override=True) def manage_users(service_id): return render_template( 'views/manage-users.html', diff --git a/app/main/views/send.py b/app/main/views/send.py index f1271f23c..8c163ff09 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -70,7 +70,7 @@ def get_page_headings(template_type): 'send_emails', 'manage_templates', 'manage_api_keys', - admin_override=True, or_=True) + admin_override=True, any_=True) def choose_template(service_id, template_type): service = service_api_client.get_service(service_id)['data'] @@ -145,7 +145,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(templates_dao.get_service_template_or_404(service_id, template_id)['data']) output = io.StringIO() 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 a25182536..6f94fb518 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -12,7 +12,7 @@
  • Send text messages
  • Send emails
    • - {% elif current_user.has_permissions(['view_activity', '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) %} {% endif %} - {% if current_user.has_permissions(['view_activity'], admin_override=True) %} + {% if current_user.has_permissions(['view_activity']) %} diff --git a/app/templates/views/choose-template.html b/app/templates/views/choose-template.html index 022fbfe74..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, or_=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 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/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/conftest.py b/tests/conftest.py index 21362d05e..762846a9a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -660,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',