Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=209.85.219.43; helo=mail-qv1-f43.google.com; envelope-from=matt@jellyfish.co; receiver=<UNKNOWN> 
MIME-Version: 1.0
References: <CADiSC1=sr8LDkQRvjfL_VuC+Y66eeBeTnGQccyZMg99mwC4UOA@mail.gmail.com>
In-Reply-To: <CADiSC1=sr8LDkQRvjfL_VuC+Y66eeBeTnGQccyZMg99mwC4UOA@mail.gmail.com>
From: Matt Klein <matt@jellyfish.co>
Date: Mon, 16 Oct 2023 12:42:07 -0400
Message-ID: <CADiSC1k-yUQeJRFwVzpuT1uin5bYMiYXOvETuSWTGmQqtcquEQ@mail.gmail.com>
Subject: Re: [PATCH django-impersonate] Ensure an impersonation session is
 marked as ended if it's timed out
To: ~petersanchez/public-inbox@lists.code.netlandish.com
Content-Type: text/plain; charset="UTF-8"

We've noticed that, in the event an impersonation session ends by
timing out (i.e., reaching MAX_DURATION), the session's log record in
impersonate_impersonationlog isn't handled appropriately.
Specifically, the log row for that session is not given any value for
session_ended_at, making it appear as if the impersonation session
never ended. This behavior is also seen if we use code in our
CUSTOM_ALLOW function to directly call
impersonate.views.stop_impersonate.

In these cases we can see that the on_session_end signal handler logs
the message "Unfinished ImpersonationLog could not be found for
None...", indicating that the impersonator is assigned to "None"
rather than the actual user. This patch fixes that behavior by calling
the on_session_end signal handler using the request.user instead of
request.impersonator.

Thanks,
Matt Klein

--- a/vendor/django-impersonate-1.9.1/impersonate/views.py
+++ b/vendor/django-impersonate-1.9.1/impersonate/views.py
@@ -85,7 +85,12 @@ def stop_impersonate(request):
     if impersonating is not None:
         session_end.send(
             sender=None,
-            impersonator=request.impersonator,
+            # mpk 10/4/23 bug fix: set the impersonator to
request.user instead of request.impersonator,
+            # since request.impersonator might not have actually been
set for this request -- e.g., in
+            # the case where we're calling stop_impersonate directly,
or where the impersonation session
+            # is timing out because it's reached MAX_DURATION
+            # impersonator=request.impersonator,
+            impersonator=request.user,
             impersonating=impersonating,
             request=request,
         )