From d62a5689826f4b8ace81c25d4003e0cb5b73590a Mon Sep 17 00:00:00 2001 From: Rustem Saiargaliev Date: Tue, 22 Feb 2022 16:52:11 +0100 Subject: [PATCH 1/4] Fix #64 -- ability to not log user attribute Also, this PR adds session check to not trigger patching response headers with Vary: Cookie in case of empty session. See #64 for details. --- log_request_id/middleware.py | 17 ++++++++++------- log_request_id/tests.py | 37 +++++++++++++++++++++++++++++++----- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/log_request_id/middleware.py b/log_request_id/middleware.py index 6629276..aebdd77 100644 --- a/log_request_id/middleware.py +++ b/log_request_id/middleware.py @@ -21,18 +21,21 @@ def process_request(self, request): request.id = request_id def get_log_message(self, request, response): + message = 'method=%s path=%s status=%s' % (request.method, request.path, response.status_code) + user = getattr(request, 'user', None) - user_attribute = getattr(settings, LOG_USER_ATTRIBUTE_SETTING, False) + # avoid accessing session if it is empty + if (getattr(request, 'session', None) and request.session.is_empty()) or not user: + return message + + # `LOG_USER_ATTRIBUTE_SETTING` accepts False/None to skip setting attribute + # but falls back to 'pk' if value is not set + user_attribute = getattr(settings, LOG_USER_ATTRIBUTE_SETTING, 'pk') if user_attribute: - user_id = getattr(user, user_attribute, None) - else: - user_id = getattr(user, 'pk', None) or getattr(user, 'id', None) - message = 'method=%s path=%s status=%s' % (request.method, request.path, response.status_code) - if user_id: + user_id = getattr(user, user_attribute, None) or getattr(user, 'id', None) message += ' user=' + str(user_id) return message - def process_response(self, request, response): if getattr(settings, REQUEST_ID_RESPONSE_HEADER_SETTING, False) and getattr(request, 'id', None): response[getattr(settings, REQUEST_ID_RESPONSE_HEADER_SETTING)] = request.id diff --git a/log_request_id/tests.py b/log_request_id/tests.py index 707154e..c685914 100644 --- a/log_request_id/tests.py +++ b/log_request_id/tests.py @@ -5,6 +5,7 @@ except ImportError: async_to_sync = None +from django.contrib.sessions.backends.file import SessionStore from django.core.exceptions import ImproperlyConfigured from django.test import RequestFactory, TestCase, override_settings @@ -13,6 +14,11 @@ from testproject.views import test_view, test_async_view +class DummyUser(object): + pk = 'fake_pk' + username = 'fake_username' + + class RequestIDLoggingTestCase(TestCase): url = "/" @@ -84,14 +90,10 @@ class DummyUser(object): self.assertTrue('fake_pk' in self.handler.messages[1]) def test_log_user_attribute(self): - - class DummyUser(object): - pk = 'fake_pk' - username = 'fake_username' - with self.settings(LOG_REQUESTS=True, LOG_USER_ATTRIBUTE='username'): request = self.factory.get(self.url) request.user = DummyUser() + request.session = SessionStore("session_key") middleware = RequestIDMiddleware(get_response=lambda request: None) middleware.process_request(request) response = self.call_view(request) @@ -99,6 +101,31 @@ class DummyUser(object): self.assertEqual(len(self.handler.messages), 2) self.assertTrue('fake_username' in self.handler.messages[1]) + def test_log_user_attribute_anonymous_user(self): + with self.settings(LOG_REQUESTS=True, LOG_USER_ATTRIBUTE='username'): + request = self.factory.get(self.url) + request.session = SessionStore() + middleware = RequestIDMiddleware(get_response=lambda request: None) + middleware.process_request(request) + response = self.call_view(request) + middleware.process_response(request, response) + self.assertEqual(len(self.handler.messages), 2) + self.assertFalse('fake_username' in self.handler.messages[1]) + self.assertFalse(request.session.accessed) + + def test_log_user_attribute_unset(self): + with self.settings(LOG_REQUESTS=True, LOG_USER_ATTRIBUTE=None): + request = self.factory.get(self.url) + request.user = DummyUser() + request.session = SessionStore("session_key") + middleware = RequestIDMiddleware(get_response=lambda request: None) + middleware.process_request(request) + response = self.call_view(request) + middleware.process_response(request, response) + self.assertEqual(len(self.handler.messages), 2) + self.assertFalse('fake_username' in self.handler.messages[1]) + self.assertFalse(request.session.accessed) + def test_response_header_unset(self): with self.settings(LOG_REQUEST_ID_HEADER='REQUEST_ID_HEADER'): request = self.factory.get(self.url) From 461340be64117b088aeca823d4fb9339ff71dc5f Mon Sep 17 00:00:00 2001 From: Rustem Saiargaliev Date: Wed, 23 Feb 2022 15:31:48 +0100 Subject: [PATCH 2/4] Separate session and user checks --- log_request_id/middleware.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/log_request_id/middleware.py b/log_request_id/middleware.py index aebdd77..71dc822 100644 --- a/log_request_id/middleware.py +++ b/log_request_id/middleware.py @@ -23,9 +23,12 @@ def process_request(self, request): def get_log_message(self, request, response): message = 'method=%s path=%s status=%s' % (request.method, request.path, response.status_code) + if getattr(request, 'session', None) and request.session.is_empty(): + # avoid accessing session if it is empty + return message + user = getattr(request, 'user', None) - # avoid accessing session if it is empty - if (getattr(request, 'session', None) and request.session.is_empty()) or not user: + if not user: return message # `LOG_USER_ATTRIBUTE_SETTING` accepts False/None to skip setting attribute From 81e80738630577604a16fb3500b7c1bb0ad5ec97 Mon Sep 17 00:00:00 2001 From: Rustem Saiargaliev Date: Mon, 7 Mar 2022 09:28:04 +0100 Subject: [PATCH 3/4] Check for the setting first --- log_request_id/middleware.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/log_request_id/middleware.py b/log_request_id/middleware.py index 71dc822..ca1a117 100644 --- a/log_request_id/middleware.py +++ b/log_request_id/middleware.py @@ -23,20 +23,22 @@ def process_request(self, request): def get_log_message(self, request, response): message = 'method=%s path=%s status=%s' % (request.method, request.path, response.status_code) + # `LOG_USER_ATTRIBUTE_SETTING` accepts False/None to skip setting attribute + # but falls back to 'pk' if value is not set + user_attribute = getattr(settings, LOG_USER_ATTRIBUTE_SETTING, 'pk') + if not user_attribute: + return message + + # avoid accessing session if it is empty if getattr(request, 'session', None) and request.session.is_empty(): - # avoid accessing session if it is empty return message user = getattr(request, 'user', None) if not user: return message - # `LOG_USER_ATTRIBUTE_SETTING` accepts False/None to skip setting attribute - # but falls back to 'pk' if value is not set - user_attribute = getattr(settings, LOG_USER_ATTRIBUTE_SETTING, 'pk') - if user_attribute: - user_id = getattr(user, user_attribute, None) or getattr(user, 'id', None) - message += ' user=' + str(user_id) + user_id = getattr(user, user_attribute, None) or getattr(user, 'id', None) + message += ' user=' + str(user_id) return message def process_response(self, request, response): From 51eee8766c06f670c7c3cd597343ecb128d3cc76 Mon Sep 17 00:00:00 2001 From: Jamie Matthews Date: Sat, 21 Jan 2023 20:35:11 +0000 Subject: [PATCH 4/4] Add note about None LOG_USER_ATTRIBUTE setting to README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index fc57fe3..216e330 100644 --- a/README.md +++ b/README.md @@ -130,7 +130,7 @@ The `RequestIDMiddleware` also has the ability to log all requests received by t Logging other user attributes -------------------- -If you would like to log another user attribute instead of user ID, this can be specified with the `LOG_USER_ATTRIBUTE` setting. Eg. to log the username, use: `LOG_USER_ATTRIBUTE = "username"` +If you would like to log another user attribute instead of user ID, this can be specified with the `LOG_USER_ATTRIBUTE` setting. Eg. to log the username, use: `LOG_USER_ATTRIBUTE = "username"`. If this setting is set to `None`, no user attribute will be logged. License -------