Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always call set_actor #484

Merged
merged 2 commits into from
Jan 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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", 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)
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