Skip to content

Commit

Permalink
Merge pull request #674 from nationalarchives/AYR-1348/page-view-logs
Browse files Browse the repository at this point in the history
AYR-1348 - Page view logs
  • Loading branch information
ltrasca authored Nov 29, 2024
2 parents 999c970 + e449f2b commit 083293b
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 40 deletions.
Empty file.
20 changes: 20 additions & 0 deletions app/main/middlewares/log_page_view.py
Original file line number Diff line number Diff line change
@@ -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
18 changes: 14 additions & 4 deletions app/main/routes.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import io
import json
import uuid

import boto3
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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"])
Expand All @@ -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(
Expand All @@ -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")
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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)

Expand Down
41 changes: 5 additions & 36 deletions app/tests/test_download.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import json
from unittest.mock import MagicMock, patch

import boto3
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
66 changes: 66 additions & 0 deletions app/tests/test_middlewares.py
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 083293b

Please sign in to comment.