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

events: fix incorrect user logged when using API token authentication #9302

Merged
merged 1 commit into from
Apr 16, 2024
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
13 changes: 3 additions & 10 deletions authentik/enterprise/audit/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
from django.db.models.signals import post_init
from django.http import HttpRequest

from authentik.core.models import User
from authentik.events.middleware import AuditMiddleware, should_log_model
from authentik.events.utils import cleanse_dict, sanitize_item

Expand All @@ -28,13 +27,10 @@ def connect(self, request: HttpRequest):
super().connect(request)
if not self.enabled:
return
user = getattr(request, "user", self.anonymous_user)
if not user.is_authenticated:
user = self.anonymous_user
if not hasattr(request, "request_id"):
return
post_init.connect(
partial(self.post_init_handler, user=user, request=request),
partial(self.post_init_handler, request=request),
dispatch_uid=request.request_id,
weak=False,
)
Expand Down Expand Up @@ -76,7 +72,7 @@ def diff(self, before: dict, after: dict) -> dict:
diff[key] = {"previous_value": value, "new_value": after.get(key)}
return sanitize_item(diff)

def post_init_handler(self, user: User, request: HttpRequest, sender, instance: Model, **_):
def post_init_handler(self, request: HttpRequest, sender, instance: Model, **_):
"""post_init django model handler"""
if not should_log_model(instance):
return
Expand All @@ -90,7 +86,6 @@ def post_init_handler(self, user: User, request: HttpRequest, sender, instance:

def post_save_handler(
self,
user: User,
request: HttpRequest,
sender,
instance: Model,
Expand All @@ -112,6 +107,4 @@ def post_save_handler(
for field_set in ignored_field_sets:
if set(diff.keys()) == set(field_set):
return None
return super().post_save_handler(
user, request, sender, instance, created, thread_kwargs, **_
)
return super().post_save_handler(request, sender, instance, created, thread_kwargs, **_)
34 changes: 17 additions & 17 deletions authentik/events/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,26 +110,32 @@

self.anonymous_user = get_anonymous_user()

def get_user(self, request: HttpRequest) -> User:
user = _CTX_OVERWRITE_USER.get()
if user:
return user

Check warning on line 116 in authentik/events/middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/middleware.py#L116

Added line #L116 was not covered by tests
user = getattr(request, "user", self.anonymous_user)
if not user.is_authenticated:
return self.anonymous_user
return user

def connect(self, request: HttpRequest):
"""Connect signal for automatic logging"""
self._ensure_fallback_user()
user = getattr(request, "user", self.anonymous_user)
if not user.is_authenticated:
user = self.anonymous_user
if not hasattr(request, "request_id"):
return
post_save.connect(
partial(self.post_save_handler, user=user, request=request),
partial(self.post_save_handler, request=request),
dispatch_uid=request.request_id,
weak=False,
)
pre_delete.connect(
partial(self.pre_delete_handler, user=user, request=request),
partial(self.pre_delete_handler, request=request),
dispatch_uid=request.request_id,
weak=False,
)
m2m_changed.connect(
partial(self.m2m_changed_handler, user=user, request=request),
partial(self.m2m_changed_handler, request=request),
dispatch_uid=request.request_id,
weak=False,
)
Expand Down Expand Up @@ -174,7 +180,6 @@

def post_save_handler(
self,
user: User,
request: HttpRequest,
sender,
instance: Model,
Expand All @@ -187,22 +192,20 @@
return
if _CTX_IGNORE.get():
return
if _new_user := _CTX_OVERWRITE_USER.get():
user = _new_user
user = self.get_user(request)

action = EventAction.MODEL_CREATED if created else EventAction.MODEL_UPDATED
thread = EventNewThread(action, request, user=user, model=model_to_dict(instance))
thread.kwargs.update(thread_kwargs or {})
thread.run()

def pre_delete_handler(self, user: User, request: HttpRequest, sender, instance: Model, **_):
def pre_delete_handler(self, request: HttpRequest, sender, instance: Model, **_):
"""Signal handler for all object's pre_delete"""
if not should_log_model(instance): # pragma: no cover
return
if _CTX_IGNORE.get():
return
if _new_user := _CTX_OVERWRITE_USER.get():
user = _new_user
user = self.get_user(request)

EventNewThread(
EventAction.MODEL_DELETED,
Expand All @@ -211,18 +214,15 @@
model=model_to_dict(instance),
).run()

def m2m_changed_handler(
self, user: User, request: HttpRequest, sender, instance: Model, action: str, **_
):
def m2m_changed_handler(self, request: HttpRequest, sender, instance: Model, action: str, **_):
"""Signal handler for all object's m2m_changed"""
if action not in ["pre_add", "pre_remove", "post_clear"]:
return
if not should_log_m2m(instance):
return
if _CTX_IGNORE.get():
return
if _new_user := _CTX_OVERWRITE_USER.get():
user = _new_user
user = self.get_user(request)

Check warning on line 225 in authentik/events/middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/middleware.py#L225

Added line #L225 was not covered by tests

EventNewThread(
EventAction.MODEL_UPDATED,
Expand Down
44 changes: 35 additions & 9 deletions authentik/events/tests/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from django.urls import reverse
from rest_framework.test import APITestCase

from authentik.core.models import Application
from authentik.core.models import Application, Token, TokenIntents

Check warning on line 6 in authentik/events/tests/test_middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/tests/test_middleware.py#L6

Added line #L6 was not covered by tests
from authentik.core.tests.utils import create_test_admin_user
from authentik.events.middleware import audit_ignore, audit_overwrite_user
from authentik.events.models import Event, EventAction
Expand All @@ -27,14 +27,13 @@
data={"name": uid, "slug": uid},
)
self.assertTrue(Application.objects.filter(name=uid).exists())
self.assertTrue(
Event.objects.filter(
action=EventAction.MODEL_CREATED,
context__model__model_name="application",
context__model__app="authentik_core",
context__model__name=uid,
).exists()
)
event = Event.objects.filter(

Check warning on line 30 in authentik/events/tests/test_middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/tests/test_middleware.py#L30

Added line #L30 was not covered by tests
action=EventAction.MODEL_CREATED,
context__model__model_name="application",
context__model__app="authentik_core",
context__model__name=uid,
).first()
self.assertIsNotNone(event)

Check warning on line 36 in authentik/events/tests/test_middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/tests/test_middleware.py#L36

Added line #L36 was not covered by tests

def test_delete(self):
"""Test model creation event"""
Expand Down Expand Up @@ -88,3 +87,30 @@
user__username=new_user.username,
).exists()
)

def test_create_with_api(self):

Check warning on line 91 in authentik/events/tests/test_middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/tests/test_middleware.py#L91

Added line #L91 was not covered by tests
"""Test model creation event (with API token auth)"""
self.client.logout()
token = Token.objects.create(user=self.user, intent=TokenIntents.INTENT_API, expiring=False)
uid = generate_id()
self.client.post(

Check warning on line 96 in authentik/events/tests/test_middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/tests/test_middleware.py#L93-L96

Added lines #L93 - L96 were not covered by tests
reverse("authentik_api:application-list"),
data={"name": uid, "slug": uid},
HTTP_AUTHORIZATION=f"Bearer {token.key}",
)
self.assertTrue(Application.objects.filter(name=uid).exists())
event = Event.objects.filter(

Check warning on line 102 in authentik/events/tests/test_middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/tests/test_middleware.py#L101-L102

Added lines #L101 - L102 were not covered by tests
action=EventAction.MODEL_CREATED,
context__model__model_name="application",
context__model__app="authentik_core",
context__model__name=uid,
).first()
self.assertIsNotNone(event)
self.assertEqual(

Check warning on line 109 in authentik/events/tests/test_middleware.py

View check run for this annotation

Codecov / codecov/patch

authentik/events/tests/test_middleware.py#L108-L109

Added lines #L108 - L109 were not covered by tests
event.user,
{
"pk": self.user.pk,
"email": self.user.email,
"username": self.user.username,
},
)
Loading