From 3251bdc22062cb19e6255bf5c162ef2ece134302 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Thu, 1 Dec 2022 15:56:28 +0200 Subject: [PATCH] events: improve handling creation of events with non-pickleable objects Signed-off-by: Jens Langhammer --- authentik/events/models.py | 3 +-- authentik/events/utils.py | 11 +++++++++-- authentik/policies/tests/test_process.py | 4 +++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/authentik/events/models.py b/authentik/events/models.py index 9977c3381..f067a7ba2 100644 --- a/authentik/events/models.py +++ b/authentik/events/models.py @@ -1,7 +1,6 @@ """authentik events models""" import time from collections import Counter -from copy import deepcopy from datetime import timedelta from inspect import currentframe from smtplib import SMTPException @@ -211,7 +210,7 @@ class Event(SerializerModel, ExpiringModel): current = currentframe() parent = current.f_back app = parent.f_globals["__name__"] - cleaned_kwargs = cleanse_dict(sanitize_dict(deepcopy(kwargs))) + cleaned_kwargs = cleanse_dict(sanitize_dict(kwargs)) event = Event(action=action, app=app, context=cleaned_kwargs) return event diff --git a/authentik/events/utils.py b/authentik/events/utils.py index 23265fd9f..306897697 100644 --- a/authentik/events/utils.py +++ b/authentik/events/utils.py @@ -1,5 +1,6 @@ """event utilities""" import re +from copy import copy from dataclasses import asdict, is_dataclass from pathlib import Path from types import GeneratorType @@ -87,9 +88,15 @@ def sanitize_item(value: Any) -> Any: """Sanitize a single item, ensure it is JSON parsable""" if is_dataclass(value): # Because asdict calls `copy.deepcopy(obj)` on everything that's not tuple/dict, - # and deepcopy doesn't work with HttpRequests (neither django nor rest_framework). + # and deepcopy doesn't work with HttpRequest (neither django nor rest_framework). + # (more specifically doesn't work with ResolverMatch) + # rest_framework's custom Request class makes this more complicated as it also holds a + # thread lock. + # Since this class is mainly used for Events which already hold the http request context + # we just remove the http_request from the shallow policy request # Currently, the only dataclass that actually holds an http request is a PolicyRequest - if isinstance(value, PolicyRequest): + if isinstance(value, PolicyRequest) and value.http_request is not None: + value: PolicyRequest = copy(value) value.http_request = None value = asdict(value) if isinstance(value, dict): diff --git a/authentik/policies/tests/test_process.py b/authentik/policies/tests/test_process.py index 3d55a4c70..33cd56350 100644 --- a/authentik/policies/tests/test_process.py +++ b/authentik/policies/tests/test_process.py @@ -2,6 +2,7 @@ from django.contrib.auth.models import AnonymousUser from django.core.cache import cache from django.test import RequestFactory, TestCase +from django.urls import resolve, reverse from guardian.shortcuts import get_anonymous_user from authentik.core.models import Application, Group, User @@ -129,8 +130,9 @@ class TestPolicyProcess(TestCase): ) binding = PolicyBinding(policy=policy, target=Application.objects.create(name="test")) - http_request = self.factory.get("/") + http_request = self.factory.get(reverse("authentik_core:impersonate-end")) http_request.user = self.user + http_request.resolver_match = resolve(reverse("authentik_core:impersonate-end")) request = PolicyRequest(self.user) request.set_http_request(http_request)