~petersanchez/public-inbox

2 2

Re: [PATCH django-impersonate] Ensure an impersonation session is marked as ended if it's timed out

Matt Klein <matt@jellyfish.co>
Details
Message ID
<CADiSC1k-yUQeJRFwVzpuT1uin5bYMiYXOvETuSWTGmQqtcquEQ@mail.gmail.com>
DKIM signature
missing
Download raw message
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,
         )

Re: [PATCH django-impersonate] Ensure an impersonation session is marked as ended if it's timed out

Details
Message ID
<20231018201246.oxarujz2tdu7kgfi@thinkpad.my.domain>
In-Reply-To
<CADiSC1k-yUQeJRFwVzpuT1uin5bYMiYXOvETuSWTGmQqtcquEQ@mail.gmail.com> (view parent)
DKIM signature
missing
Download raw message
Matt, thanks for the second patch. Unfortunately it doesn't apply. It
looks like you may be trying to submit it directly from a parent repo
which you've included the impersonate code into. The error is:

`unable to find 'vendor/django-impersonate-1.9.1/impersonate/views.py' for patching`

There is no `vendor` directory in the repo. If you can clone the main
repo, make your changes, and resubmit the patch it should apply cleanly.

Thanks,

Peter

Re: [PATCH django-impersonate] Ensure an impersonation session is marked as ended if it's timed out

Matt Klein <matt@jellyfish.co>
Details
Message ID
<CADiSC1=u=sYiGm8QTE2hnXigHp+0DVZv4w0sv-oa=rfTu_uiUA@mail.gmail.com>
In-Reply-To
<20231018201246.oxarujz2tdu7kgfi@thinkpad.my.domain> (view parent)
DKIM signature
missing
Download raw message
Hi Peter, thanks for your response. Does this one work? (I took out
the comments too since they're not really necessary.)

--- a/impersonate/views.py
+++ b/impersonate/views.py
@@ -85,7 +85,7 @@ def stop_impersonate(request):
     if impersonating is not None:
         session_end.send(
             sender=None,
-            impersonator=request.impersonator,
+            impersonator=request.user,
             impersonating=impersonating,
             request=request,
         )


On Wed, Oct 18, 2023 at 4:12 PM Peter Sanchez <peter@netlandish.com> wrote:
>
> Matt, thanks for the second patch. Unfortunately it doesn't apply. It
> looks like you may be trying to submit it directly from a parent repo
> which you've included the impersonate code into. The error is:
>
> `unable to find 'vendor/django-impersonate-1.9.1/impersonate/views.py' for patching`
>
> There is no `vendor` directory in the repo. If you can clone the main
> repo, make your changes, and resubmit the patch it should apply cleanly.
>
> Thanks,
>
> Peter
Reply to thread Export thread (mbox)