AYR-1348 - Page view logs

app/main/middlewares/log_page_view.py

@@ -0,0 +1,20 @@
+import json
+from functools import wraps
+
+from flask import current_app, request, session
+
+
+def log_page_view(route_function):
+    @wraps(route_function)
+    def wrapper(*args, **kwargs):
+        user_id = session.get("user_id", "anonymous")
+        log_data = {
+            "event": "api_request",
+            "user_id": user_id,
+            "route": request.path,
+            "method": request.method,
+        }
+        current_app.audit_logger.info(json.dumps(log_data))
+        return route_function(*args, **kwargs)
+
+    return wrapper

app/main/routes.py

@@ -1,5 +1,4 @@
 import io
-import json
 import uuid
 
 import boto3
@@ -33,6 +32,7 @@
 from app.main.flask_config_helpers import (
     get_keycloak_instance_from_flask_config,
 )
+from app.main.middlewares.log_page_view import log_page_view
 from app.main.util.date_filters_validator import validate_date_filters
 from app.main.util.filter_sort_builder import (
     build_browse_consignment_filters,
@@ -76,6 +76,7 @@ def index():
 
 @bp.route("/sign-out", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def sign_out():
     keycloak_openid = get_keycloak_instance_from_flask_config()
     keycloak_openid.logout(session["refresh_token"])
@@ -85,6 +86,7 @@ def sign_out():
 
 
 @bp.route("/sign-in", methods=["GET"])
+@log_page_view
 def sign_in():
     keycloak_openid = get_keycloak_instance_from_flask_config()
     auth_url = keycloak_openid.auth_url(
@@ -96,6 +98,7 @@ def sign_in():
 
 
 @bp.route("/callback", methods=["GET"])
+@log_page_view
 def callback():
     keycloak_openid = get_keycloak_instance_from_flask_config()
     code = request.args.get("code")
@@ -126,6 +129,7 @@ def accessibility():
 
 @bp.route("/browse", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def browse():
     form = SearchForm()
     page = int(request.args.get("page", 1))
@@ -205,6 +209,7 @@ def browse():
 
 @bp.route("/browse/transferring_body/<uuid:_id>", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def browse_transferring_body(_id: uuid.UUID):
     """
     Render the browse transferring body view page.
@@ -288,6 +293,7 @@ def browse_transferring_body(_id: uuid.UUID):
 
 @bp.route("/browse/series/<uuid:_id>", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def browse_series(_id: uuid.UUID):
     """
     Render the browse series view page.
@@ -376,6 +382,7 @@ def browse_series(_id: uuid.UUID):
 
 @bp.route("/browse/consignment/<uuid:_id>", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def browse_consignment(_id: uuid.UUID):
     """
     Render the browse consignment view page.
@@ -464,6 +471,7 @@ def browse_consignment(_id: uuid.UUID):
 
 @bp.route("/search", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def search():
     form_data = request.form.to_dict()
     args_data = request.args.to_dict()
@@ -495,6 +503,7 @@ def search():
 
 @bp.route("/search_results_summary", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def search_results_summary():
     ayr_user = AYRUser(session.get("user_groups"))
     if ayr_user.is_standard_user:
@@ -552,6 +561,7 @@ def search_results_summary():
 
 @bp.route("/search/transferring_body/<uuid:_id>", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def search_transferring_body(_id: uuid.UUID):
     body = db.session.get(Body, _id)
     validate_body_user_groups_or_404(body.Name)
@@ -652,6 +662,7 @@ def search_transferring_body(_id: uuid.UUID):
 
 @bp.route("/record/<uuid:record_id>", methods=["GET"])
 @access_token_sign_in_required
+@log_page_view
 def record(record_id: uuid.UUID):
     """
     Render the record details page.
@@ -711,6 +722,7 @@ def record(record_id: uuid.UUID):
 
 @bp.route("/download/<uuid:record_id>")
 @access_token_sign_in_required
+@log_page_view
 def download_record(record_id: uuid.UUID):
     file = db.session.get(File, record_id)
     render = request.args.get("render", False)
@@ -766,9 +778,6 @@ def download_record(record_id: uuid.UUID):
             as_attachment=True,
             download_name=download_filename,
         )
-    current_app.audit_logger.info(
-        json.dumps({"user_id": session["user_id"], "file": key})
-    )
     return response
 
 
@@ -804,6 +813,7 @@ def http_exception(error):
 
 @bp.route("/record/<uuid:record_id>/manifest")
 @access_token_sign_in_required
+@log_page_view
 def generate_manifest(record_id: uuid.UUID):
     file = db.session.get(File, record_id)
 

app/tests/test_download.py

@@ -1,4 +1,3 @@
-import json
 from unittest.mock import MagicMock, patch
 
 import boto3
@@ -40,7 +39,7 @@ def test_invalid_id_raises_404(self, client: FlaskClient):
 
     @mock_aws
     def test_download_record_standard_user_with_citable_reference_with_file_extension(
-        self, app, client, mock_standard_user, caplog
+        self, app, client, mock_standard_user
     ):
         """
         Given a File in the database with corresponding file in the s3 bucket
@@ -69,19 +68,9 @@ def test_download_record_standard_user_with_citable_reference_with_file_extensio
         )
         assert response.data == b"record"
 
-        with client.session_transaction() as session:
-            user_id = session["user_id"]
-
-        key = file.consignment.ConsignmentReference + "/" + str(file.FileId)
-
-        msg = json.dumps({"user_id": user_id, "file": key})
-
-        assert caplog.records[0].levelname == "INFO"
-        assert caplog.records[0].message == msg
-
     @mock_aws
     def test_download_record_standard_user_with_citable_reference_without_file_extension(
-        self, app, client, mock_standard_user, caplog
+        self, app, client, mock_standard_user
     ):
         """
         Given a File in the database with corresponding file in the s3 bucket
@@ -112,19 +101,9 @@ def test_download_record_standard_user_with_citable_reference_without_file_exten
         )
         assert response.data == b"record"
 
-        with client.session_transaction() as session:
-            user_id = session["user_id"]
-
-        key = file.consignment.ConsignmentReference + "/" + str(file.FileId)
-
-        msg = json.dumps({"user_id": user_id, "file": key})
-
-        assert caplog.records[0].levelname == "INFO"
-        assert caplog.records[0].message == msg
-
     @mock_aws
     def test_download_record_standard_user_without_citable_reference(
-        self, app, client, mock_standard_user, caplog
+        self, app, client, mock_standard_user
     ):
         """
         Given a File in the database with corresponding file in the s3 bucket
@@ -155,16 +134,6 @@ def test_download_record_standard_user_without_citable_reference(
         )
         assert response.data == b"record"
 
-        with client.session_transaction() as session:
-            user_id = session["user_id"]
-
-        key = file.consignment.ConsignmentReference + "/" + str(file.FileId)
-
-        msg = json.dumps({"user_id": user_id, "file": key})
-
-        assert caplog.records[0].levelname == "INFO"
-        assert caplog.records[0].message == msg
-
     @mock_aws
     def test_download_record_standard_user_get_file_errors(
         self, app, client, mock_standard_user
@@ -226,8 +195,8 @@ def test_download_record_standard_user_read_file_error(
         msg = "Error reading S3 file content: Read error"
 
         assert response.status_code == 500
-        assert caplog.records[0].levelname == "ERROR"
-        assert caplog.records[0].message == msg
+        assert caplog.records[1].levelname == "ERROR"
+        assert caplog.records[1].message == msg
 
     @mock_aws
     def test_raises_404_for_standard_user_without_access_to_files_transferring_body(

app/tests/test_middlewares.py

@@ -0,0 +1,66 @@
+import pytest
+from flask.testing import FlaskClient
+from moto import mock_aws
+
+from app.main.middlewares.log_page_view import log_page_view
+
+
+@pytest.mark.parametrize(
+    "route_path, method, session_object, route_function, expected_response, expected_log",
+    [
+        (
+            "/test_route",
+            "GET",
+            {"user_id": "test_user"},
+            lambda: "Test Response",
+            b"Test Response",
+            '{"event": "api_request", "user_id": "test_user", "route": "/test_route", "method": "GET"}',
+        ),
+        (
+            "/anonymous_route",
+            "GET",
+            {},
+            lambda: "Anonymous Response",
+            b"Anonymous Response",
+            '{"event": "api_request", "user_id": "anonymous", "route": "/anonymous_route", "method": "GET"}',
+        ),
+        (
+            "/post_route",
+            "POST",
+            {"user_id": "test_user"},
+            lambda: "Post Response",
+            b"Post Response",
+            '{"event": "api_request", "user_id": "test_user", "route": "/post_route", "method": "POST"}',
+        ),
+    ],
+)
+@mock_aws
+def test_log_page_view(
+    app,
+    client: FlaskClient,
+    route_path,
+    method,
+    session_object,
+    route_function,
+    expected_response,
+    expected_log,
+    caplog,
+):
+    """Test that the log_page_view middleware generates the correct log when an
+    endpoint decorated with it is requested"""
+
+    @app.route(route_path, methods=[method])
+    @log_page_view
+    def dynamic_route():
+        return route_function()
+
+    with client.session_transaction() as session:
+        session.update(session_object)
+
+    response = client.open(route_path, method=method)
+
+    assert response.status_code == 200
+    assert response.data == expected_response
+
+    assert caplog.records[0].levelname == "INFO"
+    assert caplog.records[0].message == expected_log