From 3da9e84ecea5c9466974f791d068e214bf6f5d1b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 1 Jul 2019 13:45:21 +0100 Subject: [PATCH] Enforce order of permissions decorators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the moment we mostly have `user_has_permissions` execute first. It shouldn’t matter, but it feels right for us to check that a user is logged in before we check their permissions to a service. Otherwise a malicious user could (maybe) check if a service ID belongs to a real service, and go on to do something malicious with that information. This commit adds some extra test code to enforce that the order is always the same. N.B. decorators in Python execute from closest to furthest (from the line on which the function is defined). --- app/main/views/agreement.py | 8 +-- app/main/views/api_keys.py | 18 +++--- app/main/views/conversation.py | 8 +-- app/main/views/dashboard.py | 22 +++---- app/main/views/jobs.py | 10 ++-- app/main/views/manage_users.py | 16 ++--- app/main/views/notifications.py | 8 +-- app/main/views/organisations.py | 42 ++++++------- app/main/views/platform_admin.py | 2 +- app/main/views/send.py | 26 ++++---- app/main/views/service_settings.py | 96 +++++++++++++++--------------- app/main/views/templates.py | 38 ++++++------ tests/app/main/test_permissions.py | 44 ++++++++++---- 13 files changed, 179 insertions(+), 159 deletions(-) diff --git a/app/main/views/agreement.py b/app/main/views/agreement.py index 2a1989d31..b934a34dc 100644 --- a/app/main/views/agreement.py +++ b/app/main/views/agreement.py @@ -22,8 +22,8 @@ def agreement(): @main.route('/services//agreement') -@login_required @user_has_permissions('manage_service') +@login_required def service_agreement(service_id): return render_template( 'views/agreement/service-{}.html'.format(current_service.organisation.as_jinja_template), @@ -32,8 +32,8 @@ def service_agreement(service_id): @main.route('/services//agreement.pdf') -@login_required @user_has_permissions('manage_service') +@login_required def service_download_agreement(service_id): return send_file(**get_mou( current_service.organisation.crown_status_or_404 @@ -41,8 +41,8 @@ def service_download_agreement(service_id): @main.route('/services//agreement/accept', methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_accept_agreement(service_id): if not current_service.organisation: @@ -65,8 +65,8 @@ def service_accept_agreement(service_id): @main.route('/services//agreement/confirm', methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_confirm_agreement(service_id): if ( diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index f6f75549c..c07202d61 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -33,8 +33,8 @@ dummy_bearer_token = 'bearer_token_set' @main.route("/services//api") -@login_required @user_has_permissions('manage_api_keys') +@login_required def api_integration(service_id): callbacks_link = ( '.api_callbacks' if current_service.has_permission('inbound_sms') @@ -48,15 +48,15 @@ def api_integration(service_id): @main.route("/services//api/documentation") -@login_required @user_has_permissions('manage_api_keys') +@login_required def api_documentation(service_id): return redirect(url_for('.documentation'), code=301) @main.route("/services//api/whitelist", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_api_keys') +@login_required def whitelist(service_id): form = Whitelist() if form.validate_on_submit(): @@ -75,8 +75,8 @@ def whitelist(service_id): @main.route("/services//api/keys") -@login_required @user_has_permissions('manage_api_keys') +@login_required def api_keys(service_id): return render_template( 'views/api/keys.html', @@ -84,8 +84,8 @@ def api_keys(service_id): @main.route("/services//api/keys/create", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_api_keys', restrict_admin_usage=True) +@login_required def create_api_key(service_id): form = CreateKeyForm(current_service.api_keys) form.key_type.choices = [ @@ -125,8 +125,8 @@ def create_api_key(service_id): @main.route("/services//api/keys/revoke/", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_api_keys') +@login_required def revoke_api_key(service_id, key_id): key_name = current_service.get_api_key(key_id)['name'] if request.method == 'GET': @@ -168,8 +168,8 @@ def check_token_against_dummy_bearer(token): @main.route("/services//api/callbacks", methods=['GET']) -@login_required @user_has_permissions('manage_api_keys') +@login_required def api_callbacks(service_id): if not current_service.has_permission('inbound_sms'): return redirect(url_for('.delivery_status_callback', service_id=service_id)) @@ -193,8 +193,8 @@ def get_delivery_status_callback_details(): @main.route("/services//api/callbacks/delivery-status-callback", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_api_keys') +@login_required def delivery_status_callback(service_id): delivery_status_callback = get_delivery_status_callback_details() back_link = ( @@ -256,8 +256,8 @@ def get_received_text_messages_callback(): @main.route("/services//api/callbacks/received-text-messages-callback", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_api_keys') +@login_required def received_text_messages_callback(service_id): if not current_service.has_permission('inbound_sms'): return redirect(url_for('.api_integration', service_id=service_id)) diff --git a/app/main/views/conversation.py b/app/main/views/conversation.py index e3ef576df..e30f72396 100644 --- a/app/main/views/conversation.py +++ b/app/main/views/conversation.py @@ -12,8 +12,8 @@ from app.utils import user_has_permissions @main.route("/services//conversation/") -@login_required @user_has_permissions('view_activity') +@login_required def conversation(service_id, notification_id): user_number = get_user_number(service_id, notification_id) @@ -28,8 +28,8 @@ def conversation(service_id, notification_id): @main.route("/services//conversation/.json") -@login_required @user_has_permissions('view_activity') +@login_required def conversation_updates(service_id, notification_id): return jsonify(get_conversation_partials( @@ -40,8 +40,8 @@ def conversation_updates(service_id, notification_id): @main.route("/services//conversation//reply-with") @main.route("/services//conversation//reply-with/from-folder/") -@login_required @user_has_permissions('send_messages') +@login_required def conversation_reply( service_id, notification_id, @@ -63,8 +63,8 @@ def conversation_reply( @main.route("/services//conversation//reply-with/") -@login_required @user_has_permissions('send_messages') +@login_required def conversation_reply_with_template( service_id, notification_id, diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 923a4fa6f..8b9abbaeb 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -42,8 +42,8 @@ from app.utils import ( # when product team makes decision about how/what/when # to view history @main.route("/services//history") -@login_required @user_has_permissions() +@login_required def temp_service_history(service_id): data = service_api_client.get_service_history(service_id)['data'] return render_template('views/temp-history.html', @@ -53,15 +53,15 @@ def temp_service_history(service_id): @main.route("/services//dashboard") -@login_required @user_has_permissions('view_activity', 'send_messages') +@login_required def old_service_dashboard(service_id): return redirect(url_for('.service_dashboard', service_id=service_id)) @main.route("/services/") -@login_required @user_has_permissions() +@login_required def service_dashboard(service_id): if session.get('invited_user'): @@ -79,23 +79,23 @@ def service_dashboard(service_id): @main.route("/services//dashboard.json") -@login_required @user_has_permissions('view_activity') +@login_required def service_dashboard_updates(service_id): return jsonify(**get_dashboard_partials(service_id)) @main.route("/services//template-activity") -@login_required @user_has_permissions('view_activity') +@login_required def template_history(service_id): return redirect(url_for('main.template_usage', service_id=service_id), code=301) @main.route("/services//template-usage") -@login_required @user_has_permissions('view_activity') +@login_required def template_usage(service_id): year, current_financial_year = requested_and_current_financial_year(request) @@ -144,8 +144,8 @@ def template_usage(service_id): @main.route("/services//usage") -@login_required @user_has_permissions('manage_service', allow_org_user=True) +@login_required def usage(service_id): year, current_financial_year = requested_and_current_financial_year(request) @@ -175,8 +175,8 @@ def usage(service_id): @main.route("/services//monthly") -@login_required @user_has_permissions('view_activity') +@login_required def monthly(service_id): year, current_financial_year = requested_and_current_financial_year(request) return render_template( @@ -194,8 +194,8 @@ def monthly(service_id): @main.route("/services//inbox") -@login_required @user_has_permissions('view_activity') +@login_required def inbox(service_id): return render_template( @@ -206,16 +206,16 @@ def inbox(service_id): @main.route("/services//inbox.json") -@login_required @user_has_permissions('view_activity') +@login_required def inbox_updates(service_id): return jsonify(get_inbox_partials(service_id)) @main.route("/services//inbox.csv") -@login_required @user_has_permissions('view_activity') +@login_required def inbox_download(service_id): return Response( Spreadsheet.from_rows( diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 46a6dfddf..8822acf3c 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -38,8 +38,8 @@ from app.utils import ( @main.route("/services//jobs") -@login_required @user_has_permissions() +@login_required def view_jobs(service_id): page = int(request.args.get('page', 1)) jobs_response = job_api_client.get_page_of_jobs(service_id, page=page) @@ -73,8 +73,8 @@ def view_jobs(service_id): @main.route("/services//jobs/") -@login_required @user_has_permissions() +@login_required def view_job(service_id, job_id): job = job_api_client.get_job(service_id, job_id)['data'] if job['job_status'] == 'cancelled': @@ -119,8 +119,8 @@ def view_job(service_id, job_id): @main.route("/services//jobs/.csv") -@login_required @user_has_permissions('view_activity') +@login_required def view_job_csv(service_id, job_id): job = job_api_client.get_job(service_id, job_id)['data'] template = service_api_client.get_service_template( @@ -154,8 +154,8 @@ def view_job_csv(service_id, job_id): @main.route("/services//jobs/", methods=['POST']) -@login_required @user_has_permissions('send_messages') +@login_required def cancel_job(service_id, job_id): job_api_client.cancel_job(service_id, job_id) return redirect(url_for('main.service_dashboard', service_id=service_id)) @@ -180,8 +180,8 @@ def view_job_updates(service_id, job_id): @main.route('/services//notifications', methods=['GET', 'POST']) @main.route('/services//notifications/', methods=['GET', 'POST']) -@login_required @user_has_permissions() +@login_required def view_notifications(service_id, message_type=None): return render_template( 'views/notifications.html', diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index ff462b82f..3dfa8d7b8 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -30,8 +30,8 @@ from app.utils import is_gov_user, redact_mobile_number, user_has_permissions @main.route("/services//users") -@login_required @user_has_permissions(allow_org_user=True) +@login_required def manage_users(service_id): return render_template( 'views/manage-users.html', @@ -44,8 +44,8 @@ def manage_users(service_id): @main.route("/services//users/invite", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def invite_user(service_id): form = InviteUserForm( @@ -81,8 +81,8 @@ def invite_user(service_id): @main.route("/services//users/", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def edit_user_permissions(service_id, user_id): service_has_email_auth = current_service.has_permission('email_auth') user = current_service.get_team_member(user_id) @@ -122,8 +122,8 @@ def edit_user_permissions(service_id, user_id): @main.route("/services//users//delete", methods=['POST']) -@login_required @user_has_permissions('manage_service') +@login_required def remove_user_from_service(service_id, user_id): try: service_api_client.remove_user_from_service(service_id, user_id) @@ -144,8 +144,8 @@ def remove_user_from_service(service_id, user_id): @main.route("/services//users//edit-email", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def edit_user_email(service_id, user_id): user = current_service.get_team_member(user_id) user_email = user.email_address @@ -172,8 +172,8 @@ def edit_user_email(service_id, user_id): @main.route("/services//users//edit-email/confirm", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def confirm_edit_user_email(service_id, user_id): user = current_service.get_team_member(user_id) if 'team_member_email_change' in session: @@ -207,8 +207,8 @@ def confirm_edit_user_email(service_id, user_id): @main.route("/services//users//edit-mobile-number", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def edit_user_mobile_number(service_id, user_id): user = current_service.get_team_member(user_id) user_mobile_number = redact_mobile_number(user.mobile_number) @@ -232,8 +232,8 @@ def edit_user_mobile_number(service_id, user_id): @main.route("/services//users//edit-mobile-number/confirm", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def confirm_edit_user_mobile_number(service_id, user_id): user = current_service.get_team_member(user_id) if 'team_member_mobile_change' in session: diff --git a/app/main/views/notifications.py b/app/main/views/notifications.py index 8dc2db67d..6a2c52dce 100644 --- a/app/main/views/notifications.py +++ b/app/main/views/notifications.py @@ -50,8 +50,8 @@ from app.utils import ( @main.route("/services//notification/") -@login_required @user_has_permissions('view_activity', 'send_messages') +@login_required def view_notification(service_id, notification_id): notification = notification_api_client.get_notification(service_id, str(notification_id)) notification['template'].update({'reply_to_text': notification['reply_to_text']}) @@ -157,8 +157,8 @@ def view_notification(service_id, notification_id): @main.route("/services//notification//cancel", methods=['GET', 'POST']) -@login_required @user_has_permissions('view_activity', 'send_messages') +@login_required def cancel_letter(service_id, notification_id): if request.method == 'POST': @@ -188,8 +188,8 @@ def get_preview_error_image(): @main.route("/services//notification/.") -@login_required @user_has_permissions('view_activity') +@login_required def view_letter_notification_as_preview(service_id, notification_id, filetype): if filetype not in ('pdf', 'png'): @@ -254,8 +254,8 @@ def get_all_personalisation_from_notification(notification): @main.route("/services//download-notifications.csv") -@login_required @user_has_permissions('view_activity') +@login_required def download_notifications_csv(service_id): filter_args = parse_filter_args(request.args) filter_args['status'] = set_status_filters(filter_args) diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index 78ebfec2d..fc3db32b7 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -37,8 +37,8 @@ from app.utils import user_has_permissions, user_is_platform_admin @main.route("/organisations", methods=['GET']) -@login_required @user_is_platform_admin +@login_required def organisations(): return render_template( 'views/organisations/index.html', @@ -48,8 +48,8 @@ def organisations(): @main.route("/organisations/add", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def add_organisation(): form = CreateOrUpdateOrganisation() @@ -67,8 +67,8 @@ def add_organisation(): @main.route("/organisations/", methods=['GET']) -@login_required @user_has_permissions() +@login_required def organisation_dashboard(org_id): return render_template( 'views/organisations/organisation/index.html', @@ -76,8 +76,8 @@ def organisation_dashboard(org_id): @main.route("/organisations//trial-services", methods=['GET']) -@login_required @user_is_platform_admin +@login_required def organisation_trial_mode_services(org_id): return render_template( 'views/organisations/organisation/trial-mode-services.html', @@ -86,8 +86,8 @@ def organisation_trial_mode_services(org_id): @main.route("/organisations//users", methods=['GET']) -@login_required @user_has_permissions() +@login_required def manage_org_users(org_id): return render_template( 'views/organisations/organisation/users/index.html', @@ -98,8 +98,8 @@ def manage_org_users(org_id): @main.route("/organisations//users/invite", methods=['GET', 'POST']) -@login_required @user_has_permissions() +@login_required def invite_org_user(org_id): form = InviteOrgUserForm( invalid_email_address=current_user.email_address @@ -122,8 +122,8 @@ def invite_org_user(org_id): @main.route("/organisations//users/", methods=['GET', 'POST']) -@login_required @user_has_permissions() +@login_required def edit_user_org_permissions(org_id, user_id): return render_template( 'views/organisations/organisation/users/user/index.html', @@ -132,8 +132,8 @@ def edit_user_org_permissions(org_id, user_id): @main.route("/organisations//users//delete", methods=['GET', 'POST']) -@login_required @user_has_permissions() +@login_required def remove_user_from_organisation(org_id, user_id): user = User.from_id(user_id) if request.method == 'POST': @@ -162,8 +162,8 @@ def remove_user_from_organisation(org_id, user_id): @main.route("/organisations//cancel-invited-user/", methods=['GET']) -@login_required @user_has_permissions() +@login_required def cancel_invited_org_user(org_id, invited_user_id): org_invite_api_client.cancel_invited_user(org_id=org_id, invited_user_id=invited_user_id) @@ -171,8 +171,8 @@ def cancel_invited_org_user(org_id, invited_user_id): @main.route("/organisations//settings/", methods=['GET']) -@login_required @user_is_platform_admin +@login_required def organisation_settings(org_id): email_branding = 'GOV.UK' @@ -197,8 +197,8 @@ def organisation_settings(org_id): @main.route("/organisations//settings/edit-name", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_organisation_name(org_id): form = RenameOrganisationForm() @@ -220,8 +220,8 @@ def edit_organisation_name(org_id): @main.route("/organisations//settings/edit-type", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_organisation_type(org_id): form = OrganisationOrganisationTypeForm( @@ -242,8 +242,8 @@ def edit_organisation_type(org_id): @main.route("/organisations//settings/edit-crown-status", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_organisation_crown_status(org_id): form = OrganisationCrownStatusForm( @@ -272,8 +272,8 @@ def edit_organisation_crown_status(org_id): @main.route("/organisations//settings/edit-agreement", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_organisation_agreement(org_id): form = OrganisationAgreementSignedForm( @@ -302,8 +302,8 @@ def edit_organisation_agreement(org_id): @main.route("/organisations//settings/set-email-branding", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_organisation_email_branding(org_id): email_branding = email_branding_client.get_all_email_branding() @@ -328,8 +328,8 @@ def edit_organisation_email_branding(org_id): @main.route("/organisations//settings/preview-email-branding", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def organisation_preview_email_branding(org_id): branding_style = request.args.get('branding_style', None) @@ -351,8 +351,8 @@ def organisation_preview_email_branding(org_id): @main.route("/organisations//settings/set-letter-branding", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_organisation_letter_branding(org_id): letter_branding = letter_branding_client.get_all_letter_branding() @@ -376,8 +376,8 @@ def edit_organisation_letter_branding(org_id): @main.route("/organisations//settings/preview-letter-branding", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def organisation_preview_letter_branding(org_id): branding_style = request.args.get('branding_style') @@ -398,8 +398,8 @@ def organisation_preview_letter_branding(org_id): @main.route("/organisations//settings/edit-organisation-domains", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_organisation_domains(org_id): form = OrganisationDomainsForm() @@ -423,8 +423,8 @@ def edit_organisation_domains(org_id): @main.route("/organisations//settings/edit-name/confirm", methods=['GET', 'POST']) -@login_required @user_has_permissions() +@login_required def confirm_edit_organisation_name(org_id): # Validate password for form def _check_password(pwd): @@ -456,8 +456,8 @@ def confirm_edit_organisation_name(org_id): @main.route("/organisations//settings/edit-go-live-notes", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_organisation_go_live_notes(org_id): form = GoLiveNotesForm() diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index d5809b3ab..f0d12569d 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -339,8 +339,8 @@ def platform_admin_letter_validation_preview(): @main.route("/services//letter-validation-preview", methods=["GET", "POST"]) -@login_required @user_has_permissions() +@login_required def service_letter_validation_preview(service_id): return letter_validation_preview(from_platform_admin=False) diff --git a/app/main/views/send.py b/app/main/views/send.py index 7d8696219..d4c82c59f 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -100,8 +100,8 @@ def get_example_letter_address(key): @main.route("/services//send//csv", methods=['GET', 'POST']) -@login_required @user_has_permissions('send_messages', restrict_admin_usage=True) +@login_required def send_messages(service_id, template_id): # if there's lots of data in the session, lets log it for debugging purposes # TODO: Remove this once we're confident we have session size under control @@ -185,8 +185,8 @@ def send_messages(service_id, template_id): @main.route("/services//send/.csv", methods=['GET']) -@login_required @user_has_permissions('send_messages', 'manage_templates') +@login_required def get_example_csv(service_id, template_id): template = get_template( service_api_client.get_service_template(service_id, template_id)['data'], current_service @@ -201,8 +201,8 @@ def get_example_csv(service_id, template_id): @main.route("/services//send//set-sender", methods=['GET', 'POST']) -@login_required @user_has_permissions('send_messages', restrict_admin_usage=True) +@login_required def set_sender(service_id, template_id): session['sender_id'] = None redirect_to_one_off = redirect( @@ -290,8 +290,8 @@ def get_sender_details(service_id, template_type): @main.route("/services//send//test", endpoint='send_test') @main.route("/services//send//one-off", endpoint='send_one_off') -@login_required @user_has_permissions('send_messages', restrict_admin_usage=True) +@login_required def send_test(service_id, template_id): session['recipient'] = None session['placeholders'] = {} @@ -341,8 +341,8 @@ def get_notification_check_endpoint(service_id, template): methods=['GET', 'POST'], endpoint='send_one_off_step', ) -@login_required @user_has_permissions('send_messages', restrict_admin_usage=True) +@login_required def send_test_step(service_id, template_id, step_index): if {'recipient', 'placeholders'} - set(session.keys()): return redirect(url_for( @@ -472,8 +472,8 @@ def send_test_step(service_id, template_id, step_index): @main.route("/services//send//test.", methods=['GET']) -@login_required @user_has_permissions('send_messages') +@login_required def send_test_preview(service_id, template_id, filetype): if filetype not in ('pdf', 'png'): @@ -604,8 +604,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ @main.route("/services///check/", methods=['GET']) @main.route("/services///check//row-", methods=['GET']) -@login_required @user_has_permissions('send_messages', restrict_admin_usage=True) +@login_required def check_messages(service_id, template_id, upload_id, row_index=2): data = _check_messages(service_id, template_id, upload_id, row_index) @@ -657,8 +657,8 @@ def check_messages(service_id, template_id, upload_id, row_index=2): "/services///check//row-.", methods=['GET'], ) -@login_required @user_has_permissions('send_messages') +@login_required def check_messages_preview(service_id, template_id, upload_id, filetype, row_index=2): if filetype == 'pdf': page = None @@ -677,8 +677,8 @@ def check_messages_preview(service_id, template_id, upload_id, filetype, row_ind "/services///check.", methods=['GET'], ) -@login_required @user_has_permissions('send_messages') +@login_required def check_notification_preview(service_id, template_id, filetype): if filetype == 'pdf': page = None @@ -694,8 +694,8 @@ def check_notification_preview(service_id, template_id, filetype): @main.route("/services//start-job/", methods=['POST']) -@login_required @user_has_permissions('send_messages', restrict_admin_usage=True) +@login_required def start_job(service_id, upload_id): job_api_client.create_job( @@ -718,8 +718,8 @@ def start_job(service_id, upload_id): @main.route("/services//end-tour/") -@login_required @user_has_permissions('manage_templates') +@login_required def go_to_dashboard_after_tour(service_id, example_template_id): service_api_client.delete_service_template(service_id, example_template_id) @@ -848,8 +848,8 @@ def get_back_link(service_id, template, step_index): @main.route("/services//template//notification/check", methods=['GET']) -@login_required @user_has_permissions('send_messages', restrict_admin_usage=True) +@login_required def check_notification(service_id, template_id): return render_template( 'views/notifications/check.html', @@ -923,8 +923,8 @@ def get_template_error_dict(exception): @main.route("/services//template//notification/check", methods=['POST']) -@login_required @user_has_permissions('send_messages', restrict_admin_usage=True) +@login_required def send_notification(service_id, template_id): if {'recipient', 'placeholders'} - set(session.keys()): return redirect(url_for( diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 874488587..f9e6f841a 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -72,8 +72,8 @@ PLATFORM_ADMIN_SERVICE_PERMISSIONS = OrderedDict([ @main.route("/services//service-settings") -@login_required @user_has_permissions('manage_service', 'manage_api_keys') +@login_required def service_settings(service_id): return render_template( 'views/service-settings.html', @@ -82,8 +82,8 @@ def service_settings(service_id): @main.route("/services//service-settings/name", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_name_change(service_id): form = RenameServiceForm() @@ -111,8 +111,8 @@ def service_name_change(service_id): @main.route("/services//service-settings/name/confirm", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_name_change_confirm(service_id): # Validate password for form def _check_password(pwd): @@ -144,8 +144,8 @@ def service_name_change_confirm(service_id): @main.route("/services//service-settings/request-to-go-live/estimate-usage", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def estimate_usage(service_id): form = EstimateUsageForm( @@ -177,8 +177,8 @@ def estimate_usage(service_id): @main.route("/services//service-settings/request-to-go-live", methods=['GET']) -@login_required @user_has_permissions('manage_service') +@login_required def request_to_go_live(service_id): agreement_signed = current_service.organisation.agreement_signed @@ -191,8 +191,8 @@ def request_to_go_live(service_id): @main.route("/services//service-settings/request-to-go-live", methods=['POST']) -@login_required @user_has_permissions('manage_service') +@login_required @user_is_gov_user def submit_request_to_go_live(service_id): @@ -238,8 +238,8 @@ def submit_request_to_go_live(service_id): @main.route("/services//service-settings/switch-live", methods=["GET", "POST"]) -@login_required @user_is_platform_admin +@login_required def service_switch_live(service_id): form = ServiceOnOffSettingForm( name="Make service live", @@ -258,8 +258,8 @@ def service_switch_live(service_id): @main.route("/services//service-settings/switch-count-as-live", methods=["GET", "POST"]) -@login_required @user_is_platform_admin +@login_required def service_switch_count_as_live(service_id): form = ServiceOnOffSettingForm( @@ -281,8 +281,8 @@ def service_switch_count_as_live(service_id): @main.route("/services//service-settings/permissions/", methods=["GET", "POST"]) -@login_required @user_is_platform_admin +@login_required def service_set_permission(service_id, permission): if permission not in PLATFORM_ADMIN_SERVICE_PERMISSIONS: abort(404) @@ -306,8 +306,8 @@ def service_set_permission(service_id, permission): @main.route("/services//service-settings/can-upload-document", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def service_switch_can_upload_document(service_id): if current_service.contact_link: return redirect(url_for('.service_set_permission', service_id=service_id, permission='upload_document')) @@ -327,8 +327,8 @@ def service_switch_can_upload_document(service_id): @main.route("/services//service-settings/archive", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def archive_service(service_id): if not current_service.active and ( current_service.trial_mode or current_user.platform_admin @@ -350,8 +350,8 @@ def archive_service(service_id): @main.route("/services//service-settings/suspend", methods=["GET", "POST"]) -@login_required @user_has_permissions('manage_service') +@login_required def suspend_service(service_id): if request.method == 'POST': service_api_client.suspend_service(service_id) @@ -363,8 +363,8 @@ def suspend_service(service_id): @main.route("/services//service-settings/resume", methods=["GET", "POST"]) -@login_required @user_has_permissions('manage_service') +@login_required def resume_service(service_id): if request.method == 'POST': service_api_client.resume_service(service_id) @@ -375,8 +375,8 @@ def resume_service(service_id): @main.route("/services//service-settings/contact-link", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_contact_link(service_id): form = ServiceContactDetailsForm() @@ -400,22 +400,22 @@ def service_set_contact_link(service_id): @main.route("/services//service-settings/set-reply-to-email", methods=['GET']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_reply_to_email(service_id): return redirect(url_for('.service_email_reply_to', service_id=service_id)) @main.route("/services//service-settings/email-reply-to", methods=['GET']) -@login_required @user_has_permissions('manage_service', 'manage_api_keys') +@login_required def service_email_reply_to(service_id): return render_template('views/service-settings/email_reply_to.html') @main.route("/services//service-settings/email-reply-to/add", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_add_email_reply_to(service_id): form = ServiceReplyToEmailForm() first_email_address = current_service.count_email_reply_to_addresses == 0 @@ -446,8 +446,8 @@ def service_add_email_reply_to(service_id): @main.route("/services//service-settings/email-reply-to//verify", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_verify_reply_to_address(service_id, notification_id): replace = request.args.get('replace', False) is_default = request.args.get('is_default', False) @@ -463,8 +463,8 @@ def service_verify_reply_to_address(service_id, notification_id): @main.route("/services//service-settings/email-reply-to//verify.json") -@login_required @user_has_permissions('manage_service') +@login_required def service_verify_reply_to_address_updates(service_id, notification_id): return jsonify(**get_service_verify_reply_to_address_partials(service_id, notification_id)) @@ -529,8 +529,8 @@ def get_service_verify_reply_to_address_partials(service_id, notification_id): methods=['GET'], endpoint="service_confirm_delete_email_reply_to" ) -@login_required @user_has_permissions('manage_service') +@login_required def service_edit_email_reply_to(service_id, reply_to_email_id): form = ServiceReplyToEmailForm() reply_to_email_address = current_service.get_email_reply_to_address(reply_to_email_id) @@ -575,8 +575,8 @@ def service_edit_email_reply_to(service_id, reply_to_email_id): @main.route("/services//service-settings/email-reply-to//delete", methods=['POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_delete_email_reply_to(service_id, reply_to_email_id): service_api_client.delete_reply_to_email_address( service_id=current_service.id, @@ -586,8 +586,8 @@ def service_delete_email_reply_to(service_id, reply_to_email_id): @main.route("/services//service-settings/set-inbound-number", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_inbound_number(service_id): available_inbound_numbers = inbound_number_client.get_available_inbound_sms_numbers() inbound_numbers_value_and_label = [ @@ -616,8 +616,8 @@ def service_set_inbound_number(service_id): @main.route("/services//service-settings/sms-prefix", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_sms_prefix(service_id): form = SMSPrefixForm(enabled=( @@ -639,8 +639,8 @@ def service_set_sms_prefix(service_id): @main.route("/services//service-settings/set-international-sms", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_international_sms(service_id): form = InternationalSMSForm( enabled='on' if current_service.has_permission('international_sms') else 'off' @@ -660,8 +660,8 @@ def service_set_international_sms(service_id): @main.route("/services//service-settings/set-inbound-sms", methods=['GET']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_inbound_sms(service_id): return render_template( 'views/service-settings/set-inbound-sms.html', @@ -669,8 +669,8 @@ def service_set_inbound_sms(service_id): @main.route("/services//service-settings/set-letters", methods=['GET']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_letters(service_id): return redirect( url_for( @@ -683,8 +683,8 @@ def service_set_letters(service_id): @main.route("/services//service-settings/set-", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_channel(service_id, channel): if channel not in {'email', 'sms', 'letter'}: @@ -711,8 +711,8 @@ def service_set_channel(service_id, channel): @main.route("/services//service-settings/set-auth-type", methods=['GET']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_auth_type(service_id): return render_template( 'views/service-settings/set-auth-type.html', @@ -720,8 +720,8 @@ 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') +@login_required def service_letter_contact_details(service_id): letter_contact_details = service_api_client.get_letter_contacts(service_id) return render_template( @@ -730,8 +730,8 @@ def service_letter_contact_details(service_id): @main.route("/services//service-settings/letter-contact/add", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_add_letter_contact(service_id): form = ServiceLetterContactBlockForm() first_contact_block = current_service.count_letter_contact_details == 0 @@ -754,8 +754,8 @@ def service_add_letter_contact(service_id): @main.route("/services//service-settings/letter-contact//edit", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_edit_letter_contact(service_id, letter_contact_id): letter_contact_block = current_service.get_letter_contact_block(letter_contact_id) form = ServiceLetterContactBlockForm( @@ -778,8 +778,8 @@ 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') +@login_required def service_sms_senders(service_id): return render_template( 'views/service-settings/sms-senders.html', @@ -787,8 +787,8 @@ def service_sms_senders(service_id): @main.route("/services//service-settings/sms-sender/add", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_add_sms_sender(service_id): form = ServiceSmsSenderForm() first_sms_sender = current_service.count_sms_senders == 0 @@ -815,8 +815,8 @@ def service_add_sms_sender(service_id): methods=['GET'], endpoint="service_confirm_delete_sms_sender" ) -@login_required @user_has_permissions('manage_service') +@login_required def service_edit_sms_sender(service_id, sms_sender_id): sms_sender = current_service.get_sms_sender(sms_sender_id) is_inbound_number = sms_sender['inbound_number_id'] @@ -850,8 +850,8 @@ def service_edit_sms_sender(service_id, sms_sender_id): "/services//service-settings/sms-sender//delete", methods=['POST'], ) -@login_required @user_has_permissions('manage_service') +@login_required def service_delete_sms_sender(service_id, sms_sender_id): service_api_client.delete_sms_sender( service_id=current_service.id, @@ -861,8 +861,8 @@ def service_delete_sms_sender(service_id, sms_sender_id): @main.route("/services//service-settings/set-letter-contact-block", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def service_set_letter_contact_block(service_id): if not current_service.has_permission('letter'): @@ -885,8 +885,8 @@ def service_set_letter_contact_block(service_id): @main.route("/services//service-settings/set-organisation-type", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def set_organisation_type(service_id): form = OrganisationTypeForm(organisation_type=current_service.organisation_type) @@ -909,8 +909,8 @@ def set_organisation_type(service_id): @main.route("/services//service-settings/set-free-sms-allowance", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def set_free_sms_allowance(service_id): form = FreeSMSAllowance(free_sms_allowance=current_service.free_sms_fragment_limit) @@ -927,8 +927,8 @@ def set_free_sms_allowance(service_id): @main.route("/services//service-settings/set-email-branding", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def service_set_email_branding(service_id): email_branding = email_branding_client.get_all_email_branding() @@ -952,8 +952,8 @@ def service_set_email_branding(service_id): @main.route("/services//service-settings/preview-email-branding", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def service_preview_email_branding(service_id): branding_style = request.args.get('branding_style', None) @@ -974,8 +974,8 @@ def service_preview_email_branding(service_id): @main.route("/services//service-settings/set-letter-branding", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def service_set_letter_branding(service_id): letter_branding = letter_branding_client.get_all_letter_branding() @@ -999,8 +999,8 @@ def service_set_letter_branding(service_id): @main.route("/services//service-settings/preview-letter-branding", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def service_preview_letter_branding(service_id): branding_style = request.args.get('branding_style') @@ -1021,8 +1021,8 @@ def service_preview_letter_branding(service_id): @main.route("/services//service-settings/request-letter-branding", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service', 'manage_templates') +@login_required def request_letter_branding(service_id): return render_template( 'views/service-settings/request-letter-branding.html', @@ -1031,8 +1031,8 @@ def request_letter_branding(service_id): @main.route("/services//service-settings/link-service-to-organisation", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def link_service_to_organisation(service_id): all_organisations = organisations_client.get_organisations() @@ -1059,8 +1059,8 @@ def link_service_to_organisation(service_id): @main.route("/services//branding-request/email", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_service') +@login_required def branding_request(service_id): branding_type = 'govuk' @@ -1108,8 +1108,8 @@ def branding_request(service_id): @main.route("/services//data-retention", methods=['GET']) -@login_required @user_is_platform_admin +@login_required def data_retention(service_id): return render_template( 'views/service-settings/data-retention.html', @@ -1117,8 +1117,8 @@ def data_retention(service_id): @main.route("/services//data-retention/add", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def add_data_retention(service_id): form = ServiceDataRetentionForm() if form.validate_on_submit(): @@ -1133,8 +1133,8 @@ def add_data_retention(service_id): @main.route("/services//data-retention//edit", methods=['GET', 'POST']) -@login_required @user_is_platform_admin +@login_required def edit_data_retention(service_id, data_retention_id): data_retention_item = current_service.get_data_retention_item(data_retention_id) form = ServiceDataRetentionEditForm(days_of_retention=data_retention_item['days_of_retention']) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 27ac5d1c1..755096cd5 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -46,8 +46,8 @@ form_objects = { @main.route("/services//templates/") -@login_required @user_has_permissions() +@login_required def view_template(service_id, template_id): template = current_service.get_template(template_id) template_folder = current_service.get_template_folder(template['folder']) @@ -87,8 +87,8 @@ def view_template(service_id, template_id): @main.route("/services//start-tour/") -@login_required @user_has_permissions('view_activity') +@login_required def start_tour(service_id, template_id): template = current_service.get_template(template_id) @@ -111,8 +111,8 @@ def start_tour(service_id, template_id): @main.route("/services//templates/folders/", methods=['GET', 'POST']) @main.route("/services//templates/", methods=['GET', 'POST']) @main.route("/services//templates//folders/", methods=['GET', 'POST']) -@login_required @user_has_permissions() +@login_required def choose_template(service_id, template_type='all', template_folder_id=None): template_folder = current_service.get_template_folder(template_folder_id) @@ -220,8 +220,8 @@ def get_template_nav_items(template_folder_id): @main.route("/services//templates/.") -@login_required @user_has_permissions() +@login_required def view_letter_template_preview(service_id, template_id, filetype): if filetype not in ('pdf', 'png'): abort(404) @@ -275,8 +275,8 @@ def _view_template_version(service_id, template_id, version, letters_as_pdf=Fals @main.route("/services//templates//version/") -@login_required @user_has_permissions() +@login_required def view_template_version(service_id, template_id, version): return render_template( 'views/templates/template_history.html', @@ -285,8 +285,8 @@ def view_template_version(service_id, template_id, version): @main.route("/services//templates//version/.") -@login_required @user_has_permissions() +@login_required def view_template_version_preview(service_id, template_id, version, filetype): db_template = current_service.get_template(template_id, version=version) return TemplatePreview.from_database_object(db_template, filetype) @@ -337,8 +337,8 @@ def _add_template_by_type(template_type, template_folder_id): @main.route("/services//templates/copy/from-folder/") @main.route("/services//templates/copy/from-service/") @main.route("/services//templates/copy/from-service//from-folder/") -@login_required @user_has_permissions('manage_templates') +@login_required def choose_template_to_copy( service_id, from_service=None, @@ -373,8 +373,8 @@ def choose_template_to_copy( @main.route("/services//templates/copy/", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def copy_template(service_id, template_id): from_service = request.args.get('from_service') @@ -417,8 +417,8 @@ def _get_template_copy_name(template, existing_templates): @main.route("/services//templates/action-blocked///") -@login_required @user_has_permissions('manage_templates') +@login_required def action_blocked(service_id, notification_type, return_to, template_id): if notification_type == 'sms': notification_type = 'text messages' @@ -435,8 +435,8 @@ def action_blocked(service_id, notification_type, return_to, template_id): @main.route("/services//templates/folders//manage", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def manage_template_folder(service_id, template_folder_id): template_folder = current_service.get_template_folder_with_user_permission_or_403(template_folder_id, current_user) form = TemplateFolderForm( @@ -470,8 +470,8 @@ def manage_template_folder(service_id, template_folder_id): @main.route("/services//templates/folders//delete", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def delete_template_folder(service_id, template_folder_id): template_folder = current_service.get_template_folder_with_user_permission_or_403(template_folder_id, current_user) @@ -515,8 +515,8 @@ def delete_template_folder(service_id, template_folder_id): @main.route("/services//templates/add-", methods=['GET', 'POST']) @main.route("/services//templates/folders//add-", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def add_service_template(service_id, template_type, template_folder_id=None): if template_type not in ['sms', 'email', 'letter']: @@ -577,8 +577,8 @@ def abort_403_if_not_admin_user(): @main.route("/services//templates//edit", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def edit_service_template(service_id, template_id): template = current_service.get_template_with_user_permission_or_403(template_id, current_user) template['template_content'] = template['content'] @@ -660,8 +660,8 @@ def edit_service_template(service_id, template_id): @main.route("/services//templates//delete", methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def delete_service_template(service_id, template_id): template = current_service.get_template_with_user_permission_or_403(template_id, current_user) @@ -710,8 +710,8 @@ def delete_service_template(service_id, template_id): @main.route("/services//templates//redact", methods=['GET']) -@login_required @user_has_permissions('manage_templates') +@login_required def confirm_redact_template(service_id, template_id): template = current_service.get_template_with_user_permission_or_403(template_id, current_user) @@ -735,8 +735,8 @@ def confirm_redact_template(service_id, template_id): @main.route("/services//templates//redact", methods=['POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def redact_template(service_id, template_id): service_api_client.redact_service_template(service_id, template_id) @@ -754,8 +754,8 @@ def redact_template(service_id, template_id): @main.route('/services//templates//versions') -@login_required @user_has_permissions('view_activity') +@login_required def view_template_versions(service_id, template_id): return render_template( 'views/templates/choose_history.html', @@ -778,8 +778,8 @@ def view_template_versions(service_id, template_id): @main.route('/services//templates//set-template-sender', methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def set_template_sender(service_id, template_id): template = current_service.get_template_with_user_permission_or_403(template_id, current_user) sender_details = get_template_sender_form_dict(service_id, template) @@ -809,8 +809,8 @@ def set_template_sender(service_id, template_id): @main.route('/services//templates//edit-postage', methods=['GET', 'POST']) -@login_required @user_has_permissions('manage_templates') +@login_required def edit_template_postage(service_id, template_id): template = current_service.get_template_with_user_permission_or_403(template_id, current_user) if template["template_type"] != "letter": diff --git a/tests/app/main/test_permissions.py b/tests/app/main/test_permissions.py index aa320e5e6..bf6847940 100644 --- a/tests/app/main/test_permissions.py +++ b/tests/app/main/test_permissions.py @@ -3,6 +3,7 @@ import inspect import pytest from flask import request +from orderedset import OrderedSet from werkzeug.exceptions import Forbidden, Unauthorized from app.main.views.index import index @@ -375,16 +376,11 @@ def test_service_navigation_for_org_user( def get_name_of_decorator_from_ast_node(node): - if isinstance(node, ast.Attribute): - return '{}.{}'.format( - get_name_of_decorator_from_ast_node(node.value), - node.attr, - ) if isinstance(node, ast.Name): return str(node.id) if isinstance(node, ast.Call) and isinstance(node.func, ast.Name): return get_name_of_decorator_from_ast_node(node.func) - return node.func.attr + return '{}.{}'.format(node.func.value.id, node.func.attr) def get_decorators_for_function(function): @@ -407,20 +403,27 @@ def get_routes_and_decorators_with_argument(argument_name): argument_name in inspect.signature(function).parameters.keys() ): decorators = list(get_decorators_for_function(function)) - if 'route' in decorators: + if 'main.route' in decorators: yield '{}.{}'.format(module_name, function_name), decorators +def format_decorators(decorators, indent=8): + return '\n'.join( + '{}@{}'.format(' ' * indent, decorator) + for decorator in decorators + ) + + def test_code_to_extract_decorators_works_with_known_examples(): assert ( 'templates.choose_template', - ['route', 'route', 'route', 'route', 'login_required', 'user_has_permissions'], + ['main.route', 'main.route', 'main.route', 'main.route', 'user_has_permissions', 'login_required'], ) in list( get_routes_and_decorators_with_argument(SERVICE_ID_ARGUMENT) ) assert ( 'organisations.organisation_dashboard', - ['route', 'login_required', 'user_has_permissions'], + ['main.route', 'user_has_permissions', 'login_required'], ) in list( get_routes_and_decorators_with_argument(ORGANISATION_ID_ARGUMENT) ) @@ -432,12 +435,29 @@ def test_service_routes_have_decorator(): list(get_routes_and_decorators_with_argument(SERVICE_ID_ARGUMENT)) + list(get_routes_and_decorators_with_argument(ORGANISATION_ID_ARGUMENT)) ): + file, function = endpoint.split('.') if 'user_is_platform_admin' in decorators: - required_decorators = {'login_required'} + required_decorators = ('main.route', 'user_is_platform_admin', 'login_required') else: - required_decorators = {'login_required', 'user_has_permissions'} + required_decorators = ('main.route', 'user_has_permissions', 'login_required') for required_decorator in required_decorators: assert required_decorator in decorators, ( 'Missing {} decorator on app/main/views/{}.py::{}' - ).format(required_decorator, *endpoint.split('.')) + ).format(required_decorator, file, function) + + present_required_decorators = tuple(OrderedSet( + decorator for decorator in decorators + if decorator in required_decorators + )) + assert present_required_decorators == required_decorators, ( + 'Wrong order of permissions decorators on app/main/views/{}.py::{}\n' + ' Expected:\n' + '{}\n' + ' Actual:\n' + '{}\n' + ).format( + file, function, + format_decorators(required_decorators), + format_decorators(present_required_decorators), + )