diff --git a/app/notify_client/models.py b/app/notify_client/models.py index cd4bd607b..9c8093ff4 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -85,14 +85,9 @@ class User(UserMixin): def has_permissions(self, permissions, service_id=None, or_=False): if service_id is None: service_id = session.get('service_id', '') - #print(permissions) - #print(service_id) - #print(self._permissions) - if service_id in self._permissions: if or_: 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/views/service_dashboard.html b/app/templates/views/service_dashboard.html index b860e7f8b..90b4ecc0d 100644 --- a/app/templates/views/service_dashboard.html +++ b/app/templates/views/service_dashboard.html @@ -1,4 +1,5 @@ {% extends "withnav_template.html" %} +{% from "components/banner.html" import banner_wrapper %} {% from "components/table.html" import list_table, field, right_aligned_field_heading %} {% from "components/big-number.html" import big_number %} @@ -24,33 +25,26 @@ {% if not template_count and not jobs %} - {{ banner( - """ -
    + {% call banner_wrapper(subhead='Get started', type="tip") %} +
      + {% if current_user.has_permissions(['manage_templates']) %}
    1. - Add a template + Add a template
    2. + {% endif %} + {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %}
    3. - Send yourself a text message + Send yourself a text message
    4. -
    - """.format( - url_for(".add_service_template", service_id=service_id, template_type="sms"), - url_for(".choose_template", service_id=service_id, template_type="sms") - )|safe, - subhead='Get started', - type="tip" - )}} + {% endif %} +
+ {% endcall %} {% elif not jobs %} - {{ banner( - """ - Send yourself a text message - """.format( - url_for(".choose_template", service_id=service_id, template_type="sms") - )|safe, - subhead='Next step', - type="tip" - )}} + {% call banner_wrapper(subhead='Next step', type="tip") %} + {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} + Send yourself a text message + {% endif %} + {% endcall %} {% else %} {% call(item) list_table( jobs, @@ -69,9 +63,11 @@ {% endcall %} {% endcall %} {% if more_jobs_to_show %} - + {% if current_user.has_permissions(['send_texts', 'send_emails', 'send_letters']) %} + + {% endif %} {% endif %} {% endif %} diff --git a/tests/__init__.py b/tests/__init__.py index e5b211bd7..a316c4342 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -161,3 +161,4 @@ def validate_route_permission(mocker, pytest.fail("Invalid method call {}".format(method)) if resp.status_code != response_code: pytest.fail("Invalid permissions set for endpoint {}".format(route)) + return resp diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index 11e681a21..62c5b5058 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -6,65 +6,73 @@ from app.main.views.index import index from werkzeug.exceptions import Forbidden +def _test_permissions(app_, usr, permissions, will_succeed, or_=False): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(usr) + decorator = user_has_permissions(*permissions, or_=or_) + decorated_index = decorator(index) + if will_succeed: + response = decorated_index() + else: + try: + response = decorated_index() + pytest.fail("Failed to throw a forbidden exception") + except Forbidden: + pass + + def test_user_has_permissions_on_endpoint_fail(app_, api_user_active, mock_login, mock_get_user_with_permissions): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - decorator = user_has_permissions('something') - decorated_index = decorator(index) - try: - response = decorated_index() - pytest.fail("Failed to throw a forbidden exception") - except Forbidden: - pass + _test_permissions( + app_, + api_user_active, + ['something'], + False) def test_user_has_permissions_success(app_, api_user_active, mock_login, mock_get_user_with_permissions): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - decorator = user_has_permissions('manage_users') - decorated_index = decorator(index) - response = decorated_index() + _test_permissions( + app_, + api_user_active, + ['manage_users'], + True) def test_user_has_permissions_or(app_, api_user_active, mock_login, mock_get_user_with_permissions): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - decorator = user_has_permissions('something', 'manage_users', or_=True) - decorated_index = decorator(index) - response = decorated_index() + _test_permissions( + app_, + api_user_active, + ['something', 'manage_users'], + True, + or_=True) def test_user_has_permissions_multiple(app_, api_user_active, mock_login, mock_get_user_with_permissions): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - decorator = user_has_permissions('manage_templates', 'manage_users') - decorated_index = decorator(index) - response = decorated_index() + _test_permissions( + app_, + api_user_active, + ['manage_templates', 'manage_users'], + True) def test_exact_permissions(app_, api_user_active, mock_login, mock_get_user_with_permissions): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - decorator = user_has_permissions('manage_users', 'manage_templates', 'manage_settings') - decorated_index = decorator(index) - response = decorated_index() + _test_permissions( + app_, + api_user_active, + ['manage_users', 'manage_templates', 'manage_settings'], + True) diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 18debf7f3..6ad20e1df 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -64,7 +64,6 @@ def test_menu_send_messages(mocker, app_, api_user_active, service_one): assert url_for('main.documentation', service_id=service_one['id']) not in page - def test_menu_manage_service(mocker, app_, api_user_active, service_one): with app_.test_request_context(): resp = _test_dashboard_menu( @@ -91,7 +90,6 @@ def test_menu_manage_service(mocker, app_, api_user_active, service_one): assert url_for('main.documentation', service_id=service_one['id']) not in page - def test_menu_manage_api_keys(mocker, app_, api_user_active, service_one): with app_.test_request_context(): resp = _test_dashboard_menu( diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index b08784a81..350638465 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -8,32 +8,6 @@ from tests import validate_route_permission template_types = ['email', 'sms'] -@pytest.mark.parametrize("template_type", template_types) -def test_choose_template( - template_type, - app_, - api_user_active, - mock_login, - mock_get_user, - mock_get_service, - mock_check_verify_code, - mock_get_service_templates, - mock_get_jobs, - mock_has_permissions -): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - response = client.get(url_for('main.choose_template', template_type=template_type, service_id=12345)) - - assert response.status_code == 200 - content = response.get_data(as_text=True) - assert '{}_template_one'.format(template_type) in content - assert '{} template one content'.format(template_type) in content - assert '{}_template_two'.format(template_type) in content - assert '{} template two content'.format(template_type) in content - - def test_upload_csvfile_with_errors_shows_check_page_with_errors( app_, api_user_active, @@ -323,3 +297,81 @@ def test_route_invalid_permissions(mocker, ['blah'], api_user_active, service_one) + + +def test_route_choose_template_manage_service_permissions(mocker, + app_, + api_user_active, + service_one, + mock_login, + mock_get_user, + mock_get_service, + mock_check_verify_code, + mock_get_service_templates, + mock_get_jobs): + with app_.test_request_context(): + template_id = mock_get_service_templates(service_one['id'])['data'][0]['id'] + resp = validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for( + 'main.choose_template', + service_id=service_one['id'], + template_type='sms'), + ['manage_users', 'manage_templates', 'manage_settings'], + api_user_active, + service_one) + page = resp.get_data(as_text=True) + assert url_for( + "main.send_messages", + service_id=service_one['id'], + template_id=template_id) not in page + assert url_for( + "main.send_message_to_self", + service_id=service_one['id'], + template_id=template_id) not in page + assert url_for( + "main.edit_service_template", + service_id=service_one['id'], + template_id=template_id) in page + + +def test_route_choose_template_send_messages_permissions(mocker, + app_, + api_user_active, + service_one, + mock_login, + mock_get_user, + mock_get_service, + mock_check_verify_code, + mock_get_service_templates, + mock_get_jobs): + with app_.test_request_context(): + template_id = mock_get_service_templates(service_one['id'])['data'][0]['id'] + resp = validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for( + 'main.choose_template', + service_id=service_one['id'], + template_type='sms'), + ['send_texts', 'send_emails', 'send_letters'], + api_user_active, + service_one) + page = resp.get_data(as_text=True) + assert url_for( + "main.send_messages", + service_id=service_one['id'], + template_id=template_id) in page + assert url_for( + "main.send_message_to_self", + service_id=service_one['id'], + template_id=template_id) in page + assert url_for( + "main.edit_service_template", + service_id=service_one['id'], + template_id=template_id) not in page