Always call set_actor (#484)

CHANGELOG.md

@@ -1,21 +1,22 @@
 # 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
 
 - Changes the view when it has changes in fields `JSONField`. The `JSONField.encoder` is assigned to `json.dumps`. ([#489](https://github.com/jazzband/django-auditlog/pull/489))
 - 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)
 

auditlog/context.py

@@ -3,7 +3,6 @@ import threading
 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"]

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 @@ class AuditlogMiddleware:
 
         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)

auditlog_tests/tests.py

@@ -433,7 +433,7 @@ class MiddlewareTest(TestCase):
         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 @@ class MiddlewareTest(TestCase):
                     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"""