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 30, 2022
1 parent 7f8edd5 commit f164bdb
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 18 deletions.
7 changes: 4 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)

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.cid import set_cid
from auditlog.context import set_actor
Expand Down Expand Up @@ -30,15 +30,18 @@ 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)

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

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

0 comments on commit f164bdb

Please sign in to comment.