From 3afc1936248c737b0b0f68b189d098d6397eed53 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 1 Mar 2018 10:37:55 +0000 Subject: [PATCH] remove any_ from has_permissions we branch on any_ to either say "require ALL these permissions" or "require ANY of these permissions". But we only ever call the decorator with one permission, or with any_=True, so it's unnecessary --- app/main/views/send.py | 2 +- app/main/views/service_settings.py | 8 ++++---- app/notify_client/models.py | 9 ++------- app/templates/components/table.html | 2 +- app/templates/main_nav.html | 2 +- app/templates/views/dashboard/dashboard.html | 2 +- tests/app/main/test_permissions.py | 3 +-- tests/conftest.py | 2 +- 8 files changed, 12 insertions(+), 18 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 1020400cf..9520eb74d 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -159,7 +159,7 @@ def send_messages(service_id, template_id): @main.route("/services//send/.csv", methods=['GET']) @login_required -@user_has_permissions('send_messages', 'manage_templates', any_=True) +@user_has_permissions('send_messages', 'manage_templates') def get_example_csv(service_id, template_id): template = get_template( service_api_client.get_service_template(service_id, template_id)['data'], current_service diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 8f75c4e59..fd2479714 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -53,7 +53,7 @@ from app.utils import ( @main.route("/services//service-settings") @login_required -@user_has_permissions('manage_service', 'manage_api_keys', any_=True) +@user_has_permissions('manage_service', 'manage_api_keys') def service_settings(service_id): letter_branding_organisations = email_branding_client.get_letter_email_branding() organisation = organisations_client.get_service_organisation(service_id).get('name', None) @@ -360,7 +360,7 @@ def service_set_reply_to_email(service_id): @main.route("/services//service-settings/email-reply-to", methods=['GET']) @login_required -@user_has_permissions('manage_service', 'manage_api_keys', any_=True) +@user_has_permissions('manage_service', 'manage_api_keys') def service_email_reply_to(service_id): reply_to_email_addresses = service_api_client.get_reply_to_email_addresses(service_id) return render_template( @@ -540,7 +540,7 @@ def service_set_auth_type(service_id): @main.route("/services//service-settings/letter-contacts", methods=['GET']) @login_required -@user_has_permissions('manage_service', 'manage_api_keys', any_=True) +@user_has_permissions('manage_service', 'manage_api_keys') def service_letter_contact_details(service_id): letter_contact_details = service_api_client.get_letter_contacts(service_id) return render_template( @@ -596,7 +596,7 @@ def service_edit_letter_contact(service_id, letter_contact_id): @main.route("/services//service-settings/sms-sender", methods=['GET']) @login_required -@user_has_permissions('manage_service', 'manage_api_keys', any_=True) +@user_has_permissions('manage_service', 'manage_api_keys') def service_sms_senders(service_id): def attach_hint(sender): diff --git a/app/notify_client/models.py b/app/notify_client/models.py index 290d8a3d7..e1ae9e024 100644 --- a/app/notify_client/models.py +++ b/app/notify_client/models.py @@ -159,7 +159,7 @@ class User(UserMixin): def permissions(self, permissions): raise AttributeError("Read only property") - def has_permissions(self, *permissions, any_=False, restrict_admin_usage=False): + def has_permissions(self, *permissions, restrict_admin_usage=False): unknown_permissions = set(permissions) - all_permissions if unknown_permissions: @@ -172,12 +172,7 @@ class User(UserMixin): # Service id is always set on the request for service specific views. service_id = _get_service_id_from_view_args() if service_id in self._permissions: - if any_: - has_permissions = any(x in self._permissions[service_id] for x in permissions) - else: - has_permissions = set(self._permissions[service_id]) >= set(permissions) - - return has_permissions + return any(x in self._permissions[service_id] for x in permissions) return False def has_permission_for_service(self, service_id, permission): diff --git a/app/templates/components/table.html b/app/templates/components/table.html index 21ed11de9..7e30a35e9 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -107,7 +107,7 @@ {% macro edit_field(text, link, permissions=[]) -%} {% call field(align='right') %} - {% if current_user.has_permissions(*permissions, **{'any_': True}) or not permissions %} + {% if current_user.has_permissions(*permissions) or not permissions %} {{ text }} {% endif %} {% endcall %} diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 82d17e044..ef5039379 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -51,7 +51,7 @@ {% if current_user.has_permissions('manage_service') %}
  • Usage
    • {% endif %} - {% if current_user.has_permissions('manage_api_keys', 'manage_service', any_=True) %} + {% if current_user.has_permissions('manage_api_keys', 'manage_service') %}
  • Settings
    • {% endif %} {% if current_user.has_permissions('manage_api_keys') %} diff --git a/app/templates/views/dashboard/dashboard.html b/app/templates/views/dashboard/dashboard.html index 57600860a..c347e86e0 100644 --- a/app/templates/views/dashboard/dashboard.html +++ b/app/templates/views/dashboard/dashboard.html @@ -19,7 +19,7 @@ {% if not templates %} {% include 'views/dashboard/write-first-messages.html' %} {% endif %} - {% elif not current_user.has_permissions('send_messages', 'manage_api_keys', any_=True) %} + {% elif not current_user.has_permissions('send_messages', 'manage_api_keys') %} {% include 'views/dashboard/no-permissions-banner.html' %} {% endif %} diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index 120750fe3..c10f76d3b 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -70,8 +70,7 @@ def test_user_has_permissions_or( client, user, ['send_messages', 'manage_service'], - True, - kwargs={'any_': True}) + True) def test_user_has_permissions_multiple( diff --git a/tests/conftest.py b/tests/conftest.py index 062bf26a0..88cdd919d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1931,7 +1931,7 @@ def mock_no_inbound_number_for_service(mocker): @pytest.fixture(scope='function') def mock_has_permissions(mocker): - def _has_permission(*permissions, any_=False, restrict_admin_usage=False): + def _has_permission(*permissions, restrict_admin_usage=False): return True return mocker.patch(