Ensure all service route have permission decorators

We accidentally miss these sometimes. This code adds a test which
inspects the code to automatically check that any function which:
- handles a route
- accepts a service_id

For each function it checks that each of these routes have the
permissions decorator we’d expect.

Most of the introspection/AST code is adapted from here:
https://mvdwoord.github.io/exploration/2017/08/18/ast_explore.html

app/main/views/agreement.py

@@ -23,6 +23,7 @@ def agreement():
 
 @main.route('/services/<uuid:service_id>/agreement')
 @login_required
+@user_has_permissions('manage_service')
 def service_agreement(service_id):
     return render_template(
         'views/agreement/service-{}.html'.format(current_service.organisation.as_jinja_template),
@@ -41,6 +42,7 @@ def service_download_agreement(service_id):
 
 @main.route('/services/<uuid:service_id>/agreement/accept', methods=['GET', 'POST'])
 @login_required
+@user_has_permissions('manage_service')
 def service_accept_agreement(service_id):
 
     if not current_service.organisation:
@@ -64,6 +66,7 @@ def service_accept_agreement(service_id):
 
 @main.route('/services/<uuid:service_id>/agreement/confirm', methods=['GET', 'POST'])
 @login_required
+@user_has_permissions('manage_service')
 def service_confirm_agreement(service_id):
 
     if (

app/main/views/dashboard.py

@@ -43,6 +43,7 @@ from app.utils import (
 # to view history
 @main.route("/services/<service_id>/history")
 @login_required
+@user_has_permissions()
 def temp_service_history(service_id):
     data = service_api_client.get_service_history(service_id)['data']
     return render_template('views/temp-history.html',
@@ -78,6 +79,7 @@ def service_dashboard(service_id):
 
 
 @main.route("/services/<service_id>/dashboard.json")
+@login_required
 @user_has_permissions('view_activity')
 def service_dashboard_updates(service_id):
     return jsonify(**get_dashboard_partials(service_id))

app/main/views/jobs.py

@@ -163,6 +163,7 @@ def cancel_job(service_id, job_id):
 
 @main.route("/services/<service_id>/jobs/<job_id>.json")
 @user_has_permissions()
+@login_required
 def view_job_updates(service_id, job_id):
 
     job = job_api_client.get_job(service_id, job_id)['data']
@@ -205,6 +206,7 @@ def view_notifications(service_id, message_type=None):
 @main.route('/services/<service_id>/notifications.json', methods=['GET', 'POST'])
 @main.route('/services/<service_id>/notifications/<message_type>.json', methods=['GET', 'POST'])
 @user_has_permissions()
+@login_required
 def get_notifications_as_json(service_id, message_type=None):
     return jsonify(get_notifications(
         service_id, message_type, status_override=request.args.get('status')
@@ -213,6 +215,7 @@ def get_notifications_as_json(service_id, message_type=None):
 
 @main.route('/services/<service_id>/notifications/<message_type>.csv', endpoint="view_notifications_csv")
 @user_has_permissions()
+@login_required
 def get_notifications(service_id, message_type, status_override=None):
     # TODO get the api to return count of pages as well.
     page = get_page_from_request()

app/main/views/manage_users.py

@@ -269,6 +269,7 @@ def confirm_edit_user_mobile_number(service_id, user_id):
 
 @main.route("/services/<service_id>/cancel-invited-user/<uuid:invited_user_id>", methods=['GET'])
 @user_has_permissions('manage_service')
+@login_required
 def cancel_invited_user(service_id, invited_user_id):
     current_service.cancel_invite(invited_user_id)
 

app/main/views/notifications.py

@@ -220,6 +220,7 @@ def view_letter_notification_as_preview(service_id, notification_id, filetype):
 
 @main.route("/services/<service_id>/notification/<notification_id>.json")
 @user_has_permissions('view_activity', 'send_messages')
+@login_required
 def view_notification_updates(service_id, notification_id):
     return jsonify(**get_single_notification_partials(
         notification_api_client.get_notification(service_id, notification_id)

app/main/views/platform_admin.py

@@ -33,6 +33,7 @@ from app.utils import (
     generate_next_dict,
     generate_previous_dict,
     get_page_from_request,
+    user_has_permissions,
     user_is_platform_admin,
 )
 
@@ -339,6 +340,7 @@ def platform_admin_letter_validation_preview():
 
 @main.route("/services/<service_id>/letter-validation-preview", methods=["GET", "POST"])
 @login_required
+@user_has_permissions()
 def service_letter_validation_preview(service_id):
     return letter_validation_preview(from_platform_admin=False)
 

tests/app/main/test_permissions.py

@@ -1,3 +1,6 @@
+import ast
+import inspect
+
 import pytest
 from flask import request
 from werkzeug.exceptions import Forbidden, Unauthorized
@@ -369,3 +372,72 @@ def test_service_navigation_for_org_user(
         'Team members',
         'Usage',
     ]
+
+
+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
+
+
+def get_decorators_for_function(function):
+    for node in ast.walk(ast.parse(inspect.getsource(function))):
+        if isinstance(node, ast.FunctionDef):
+            for decorator in node.decorator_list:
+                yield get_name_of_decorator_from_ast_node(decorator)
+
+
+SERVICE_ID_ARGUMENT = 'service_id'
+ORGANISATION_ID_ARGUMENT = 'org_id'
+
+
+def get_routes_and_decorators_with_argument(argument_name):
+    import app.main.views as views
+    for module_name, module in inspect.getmembers(views):
+        for function_name, function in inspect.getmembers(module):
+            if (
+                inspect.isfunction(function) and
+                argument_name in inspect.signature(function).parameters.keys()
+            ):
+                decorators = list(get_decorators_for_function(function))
+                if 'route' in decorators:
+                    yield '{}.{}'.format(module_name, function_name), decorators
+
+
+def test_code_to_extract_decorators_works_with_known_examples():
+    assert (
+        'templates.choose_template',
+        ['route', 'route', 'route', 'route', 'login_required', 'user_has_permissions'],
+    ) in list(
+        get_routes_and_decorators_with_argument(SERVICE_ID_ARGUMENT)
+    )
+    assert (
+        'organisations.organisation_dashboard',
+        ['route', 'login_required', 'user_has_permissions'],
+    ) in list(
+        get_routes_and_decorators_with_argument(ORGANISATION_ID_ARGUMENT)
+    )
+
+
+def test_service_routes_have_decorator():
+
+    for endpoint, decorators in (
+        list(get_routes_and_decorators_with_argument(SERVICE_ID_ARGUMENT)) +
+        list(get_routes_and_decorators_with_argument(ORGANISATION_ID_ARGUMENT))
+    ):
+        if 'user_is_platform_admin' in decorators:
+            required_decorators = {'login_required'}
+        else:
+            required_decorators = {'login_required', 'user_has_permissions'}
+
+        for required_decorator in required_decorators:
+            assert required_decorator in decorators, (
+                'Missing {} decorator on app/main/views/{}.py::{}'
+            ).format(required_decorator, *endpoint.split('.'))

tests/app/main/views/test_platform_admin.py

@@ -19,7 +19,7 @@ from app.main.views.platform_admin import (
     sum_service_usage,
 )
 from tests import service_json
-from tests.conftest import mock_get_user, normalize_spaces
+from tests.conftest import SERVICE_ONE_ID, mock_get_user, normalize_spaces
 
 
 @pytest.mark.parametrize('endpoint', [
@@ -767,7 +767,7 @@ def test_service_letter_validation_preview_renders_correctly(
         mock_has_no_jobs
 
 ):
-    page = client_request.get('main.service_letter_validation_preview', service_id="service_1")
+    page = client_request.get('main.service_letter_validation_preview', service_id=SERVICE_ONE_ID)
 
     assert page.find('h1').text.strip() == "Letter validation preview"
     assert page.find_all('input', class_='file-upload-field')
@@ -780,7 +780,7 @@ def test_service_letter_validation_preview_returns_400_if_file_is_too_big(
 
 ):
     with open('tests/test_pdf_files/big.pdf', 'rb') as file:
-        page = client_request.post('main.service_letter_validation_preview', service_id="service_1",
+        page = client_request.post('main.service_letter_validation_preview', service_id=SERVICE_ONE_ID,
                                    _data=dict(
                                        pdf_file=file,
                                    ),