diff --git a/CHANGELOG.md b/CHANGELOG.md index 6576baf3..febc4c78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,20 +1,21 @@ # Changes +## Next Release + #### Breaking Changes - feat: Change `LogEntry.change` field type to `JSONField` rather than `TextField`. This change include a migration that may take time to run depending on the number of records on your `LogEntry` table ([#407](https://github.com/jazzband/django-auditlog/pull/407)) -## Next Release - #### Improvements - feat: Added support for Correlation ID. ([#481](https://github.com/jazzband/django-auditlog/pull/481)) - feat: Added pre-log and post-log signals. ([#483](https://github.com/jazzband/django-auditlog/pull/483)) +- feat: Make timestamp in LogEntry overwritable. ([#476](https://github.com/jazzband/django-auditlog/pull/476)) #### Fixes - fix: Make sure `LogEntry.changes_dict()` returns an empty dict instead of `None` when `json.loads()` returns `None`. ([#472](https://github.com/jazzband/django-auditlog/pull/472)) -- feat: Make timestamp in LogEntry overwritable. ([#476](https://github.com/jazzband/django-auditlog/pull/476)) +- fix: Always set remote_addr even if the request has no authenticated user. ([#484](https://github.com/jazzband/django-auditlog/pull/484)) ## 2.2.1 (2022-11-28) diff --git a/auditlog/context.py b/auditlog/context.py index de2c0cad..80d2fae5 100644 --- a/auditlog/context.py +++ b/auditlog/context.py @@ -3,7 +3,6 @@ import time from functools import partial -from django.contrib.auth import get_user_model from django.db.models.signals import pre_save from auditlog.models import LogEntry @@ -55,12 +54,8 @@ def _set_actor(user, sender, instance, signal_duid, **kwargs): else: if signal_duid != auditlog["signal_duid"]: return - auth_user_model = get_user_model() - if ( - sender == LogEntry - and isinstance(user, auth_user_model) - and instance.actor is None - ): + + if sender == LogEntry and instance.actor is None: instance.actor = user instance.remote_addr = auditlog["remote_addr"] diff --git a/auditlog/middleware.py b/auditlog/middleware.py index 9d07ed79..e3274ee1 100644 --- a/auditlog/middleware.py +++ b/auditlog/middleware.py @@ -1,4 +1,4 @@ -import contextlib +from django.contrib.auth import get_user_model from auditlog.cid import set_cid from auditlog.context import set_actor @@ -30,15 +30,18 @@ def _get_remote_addr(request): return remote_addr + @staticmethod + def _get_actor(request): + user = getattr(request, "user", None) + if isinstance(user, get_user_model()) and user.is_authenticated: + return user + return None + def __call__(self, request): remote_addr = self._get_remote_addr(request) + user = self._get_actor(request) set_cid(request) - if hasattr(request, "user") and request.user.is_authenticated: - context = set_actor(actor=request.user, remote_addr=remote_addr) - else: - context = contextlib.nullcontext() - - with context: + with set_actor(actor=user, remote_addr=remote_addr): return self.get_response(request) diff --git a/auditlog_tests/tests.py b/auditlog_tests/tests.py index cf8f0dfb..487c7d13 100644 --- a/auditlog_tests/tests.py +++ b/auditlog_tests/tests.py @@ -433,7 +433,7 @@ def test_request_anonymous(self): request = self.factory.get("/") request.user = AnonymousUser() - self.get_response_mock.side_effect = self.side_effect(self.assert_no_listeners) + self.get_response_mock.side_effect = self.side_effect(self.assert_has_listeners) response = self.middleware(request) @@ -518,6 +518,38 @@ def test_cid(self): self.assertEqual(history.cid, expected_result) self.assertEqual(get_cid(), expected_result) + def test_set_actor_anonymous_request(self): + """ + The remote address will be set even when there is no actor + """ + remote_addr = "123.213.145.99" + actor = None + + with set_actor(actor=actor, remote_addr=remote_addr): + obj = SimpleModel.objects.create(text="I am not difficult.") + + history = obj.history.get() + self.assertEqual( + history.remote_addr, + remote_addr, + msg=f"Remote address is {remote_addr}", + ) + self.assertIsNone(history.actor, msg="Actor is `None` for anonymous user") + + def test_get_actor(self): + params = [ + (AnonymousUser(), None, "The user is anonymous so the actor is `None`"), + (self.user, self.user, "The use is authenticated so it is the actor"), + (None, None, "There is no actor"), + ("1234", None, "The value of request.user is not a valid user model"), + ] + for user, actor, msg in params: + with self.subTest(msg): + request = self.factory.get("/") + request.user = user + + self.assertEqual(self.middleware._get_actor(request), actor) + class SimpleIncludeModelTest(TestCase): """Log only changes in include_fields"""