Skip to content

Commit

Permalink
fix: called set_actor even when is the user is anon
Browse files Browse the repository at this point in the history
  • Loading branch information
aqeelat committed Dec 22, 2022
1 parent 63c8882 commit 7d31538
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 16 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,14 @@

## Next Release

#### Improvements

- 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)

Expand Down
9 changes: 2 additions & 7 deletions auditlog/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"]
Expand Down
17 changes: 10 additions & 7 deletions auditlog/middleware.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import contextlib
from django.contrib.auth import get_user_model

from auditlog.context import set_actor

Expand Down Expand Up @@ -29,13 +29,16 @@ def _get_remote_addr(request):

return remote_addr

@staticmethod
def _get_actor(request):
user = getattr(request, "user")
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)

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)
34 changes: 33 additions & 1 deletion auditlog_tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,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)

Expand Down Expand Up @@ -481,6 +481,38 @@ def test_get_remote_addr(self):
self.middleware._get_remote_addr(request), expected_remote_addr
)

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"""
Expand Down

0 comments on commit 7d31538

Please sign in to comment.