[Django] #37017: alogout() doesn't clear request.user

[Django]

#37017: alogout() doesn't clear request.user
-------------------------------------------+------------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Keywords: not-security
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+------------------------------
As of Django 6.0, `alogout()` no longer clears `request.user`, only
`request.auser`. If code accesses `request.user` before
`RemoteUserMiddleware` (or similar) runs `alogout()`, then it is possible
for `request.user` to be stale, and for a resource behind authentication
to be visible to a logged-out user.

`user` and `auser` aren't really two concepts: just two getters for the
same underlying concept. (Thanks, function color problem!)


(I'm suggesting this was an oversight in
31a43c571f4d036827d4fd7a5f615591637dc1be. This was
[https://github.com/django/django/pull/19709#issuecomment-3162977011
discussed] during development, but it may not have been clear how this
would arise in practice.)

The security team considered a report about this suggesting the following
order of middlewares:

{{{#!py
MIDDLEWARE = [
"django.middleware.security.SecurityMiddleware",
"django.contrib.sessions.middleware.SessionMiddleware",
"django.contrib.auth.middleware.AuthenticationMiddleware",
"app.middleware.MaterializeUserMiddleware", # e.g. a logging
middleware like Sentry
"django.contrib.auth.middleware.RemoteUserMiddleware",
"django.contrib.auth.middleware.LoginRequiredMiddleware",
]
}}}

The problem does not reproduce if `RemoteUserMiddleware` is moved one
position earlier. The security team closed the report on this basis (that
is, anything responsible for logout should happen before other code that
might be interested in that logout). Our
[https://docs.djangoproject.com/en/6.0/howto/auth-remote-
user/#configuration docs say] `RemoteUserMiddleware` should be placed
"after" `AuthenticationMiddleware`, but does not clarify whether this
entails ''directly'' after.

Still seems like something to fix to make auth easier to reason about.

Thanks Peng Zhou for the report.
--
Ticket URL: <https://code.djangoproject.com/ticket/37017>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37017: alogout() doesn't clear request.user

---------------------------------+---------------------------------------

Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0

Severity: Release blocker | Resolution:

Keywords: not-security | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

---------------------------------+---------------------------------------
Changes (by Jacob Walls):

* cc: Michael Manfre (added)

Comment:

It's likely we should have `alogin()` also set `request.user` for
parallelism.
--
Ticket URL: <https://code.djangoproject.com/ticket/37017#comment:1>

[Django]

#37017: alogout() doesn't clear request.user
---------------------------------+---------------------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution:

Keywords: not-security | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+---------------------------------------

Changes (by Sarah Boyce):

* stage: Unreviewed => Accepted

Comment:

Thank you for the report! Apologies that my review was problematic
I suppose `request.user` will still be used in async contexts by anything
that has been made async by a simple `sync_to_async` call and therefore
needs to stay valid.
--
Ticket URL: <https://code.djangoproject.com/ticket/37017#comment:2>

[Django]

#37017: alogout() doesn't clear request.user
---------------------------------+---------------------------------------
Reporter: Jacob Walls | Owner: Jacob Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: not-security | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+---------------------------------------

Changes (by Jacob Walls):

* has_patch: 0 => 1

Comment:

No worries, here's a patch: [https://github.com/django/django/pull/21045
PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/37017#comment:3>

[Django]

#37017: alogout() doesn't clear request.user

-------------------------------------+-------------------------------------

Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: assigned
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution:

Keywords: not-security | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Sarah Boyce):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/37017#comment:4>

[Django]

#37017: alogout() doesn't clear request.user
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls

Type: Bug | Status: closed
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution: fixed

Keywords: not-security | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls <jacobtylerwalls@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"a32c7075cf634aee1f4f3deecd27f194097ec0c2" a32c707]:
{{{#!CommitTicketReference repository=""
revision="a32c7075cf634aee1f4f3deecd27f194097ec0c2"
Fixed #37017 -- Fixed setting or clearing of request.user after
alogin/alogout().

Regression in 31a43c571f4d036827d4fd7a5f615591637dc1be.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37017#comment:5>

[Django]

#37017: alogout() doesn't clear request.user
-------------------------------------+-------------------------------------
Reporter: Jacob Walls | Owner: Jacob
| Walls
Type: Bug | Status: closed
Component: contrib.auth | Version: 6.0
Severity: Release blocker | Resolution: fixed
Keywords: not-security | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls <jacobtylerwalls@…>):

In [changeset:"a5c28dc1d7606f11adb932d0bd4dba899a028007" a5c28dc]:
{{{#!CommitTicketReference repository=""
revision="a5c28dc1d7606f11adb932d0bd4dba899a028007"
[6.0.x] Fixed #37017 -- Fixed setting or clearing of request.user after
alogin/alogout().

Regression in 31a43c571f4d036827d4fd7a5f615591637dc1be.

Backport of a32c7075cf634aee1f4f3deecd27f194097ec0c2 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/37017#comment:6>