[Django] #36819: Async login does not reuse authenticated user instance

[Django]

#36819: Async login does not reuse authenticated user instance
-------------------------------------+-------------------------------------
Reporter: Mykhailo Havelia | Type:
| Cleanup/optimization
Status: new | Component:
| contrib.auth
Version: 6.0 | Severity: Normal
Keywords: async | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
**Problem**

When using alogin, calling `request.auser` does not reuse the already
authenticated user instance, resulting in redundant authentication
requests.

**Details**

**Middleware Setup**

{{{
class AuthenticationMiddleware(MiddlewareMixin):
def process_request(self, request):
...
request.user = SimpleLazyObject(lambda: get_user(request))
request.auser = partial(auser, request)
}}}

**Sync Login**

{{{
def login(request, user, backend=None):
...
request.user = user
}}}

**Async Login**

{{{
def alogin(request, user, backend=None):
...
request.user = user
}}}

**Problematic Behavior**

After calling `alogin(request, user)`, `request.user` is set to the
authenticated user instance. However, `request.auser()` does not use this
instance and instead performs authentication again, relying on its own
`_acached_user` cache.

**Expected Behavior**

After `alogin`, both `request.user` and `request.auser()` should reference
the same user instance, avoiding duplicate authentication.

**Suggested Solution**

Set a shared cache. `request._cached_user = user`, in `alogin`, so both
sync and async accessors use the same instance.

{{{
async def alogin(request, user, backend=None):
...
request.user = user
request._cached_user = user # <-- suggested addition

async def auser(request):
if not hasattr(request, "_cached_user"): # <-- suggested change
(_acached_user -> _cached_user)
request._cached_user = await auth.aget_user(request)
return request._cached_user
}}}

See related discussion:
https://github.com/django/django/pull/16552#discussion_r1122929933
--
Ticket URL: <https://code.djangoproject.com/ticket/36819>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36819: Async login does not reuse authenticated user instance
-------------------------------------+-------------------------------------

Reporter: Mykhailo Havelia | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution: wontfix
Keywords: async auser alogin | Triage Stage:
cache | Unreviewed

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

Changes (by Natalia Bidart):

* keywords: async => async auser alogin cache
* resolution: => wontfix
* status: new => closed

Comment:

Hello Mykhailo Havelia, thank you for your ticket and for linking to the
PR conversation.

I gotta say, as soon as I read this ticket description, my first gut
reaction was twofold:

1. why a project would mix async and sync in the same flow, and
2. a shared cache feels like the perfect recipe for a debugging nightmare

Reading Mariusz and Carlton's comment in the PR, seems like we are in
agreement. Closing following that rationale, if you feel strongly that
this caching is key for a given use case, please share specific details in
how a single request would benefit from it.
--
Ticket URL: <https://code.djangoproject.com/ticket/36819#comment:1>

[Django]

#36819: Async login does not reuse authenticated user instance
-------------------------------------+-------------------------------------
Reporter: Mykhailo Havelia | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution: wontfix
Keywords: async auser alogin | Triage Stage:
cache | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mykhailo Havelia):

Replying to [comment:1 Natalia Bidart]:

> why a project would mix async and sync in the same flow, and

There is no mixing of sync and async within the same flow. The issue is
that after calling alogin and awaiting `request.auser()`, we end up
performing redundant authentication requests (at least against the
database).

> a shared cache feels like the perfect recipe for a debugging nightmare

It's just one of the options how to solve it. Btw 🙂 that "perfect recipe
for a debugging nightmare" already exists in the current codebase. For
example:

{{{
async def alogin(request, user, backend=None):
...
request.user = user
}}}

Here, `alogin` is an async function that mutates request.user, which is
part of the synchronous API.
--
Ticket URL: <https://code.djangoproject.com/ticket/36819#comment:2>

[Django]

#36819: Async login does not reuse authenticated user instance
-------------------------------------+-------------------------------------
Reporter: Mykhailo Havelia | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution: wontfix
Keywords: async auser alogin | Triage Stage:
cache | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Natalia Bidart):

Replying to [comment:2 Mykhailo Havelia]:

> Replying to [comment:1 Natalia Bidart]:
>
> > why a project would mix async and sync in the same flow, and
>
> There is no mixing of sync and async within the same flow. The issue is
that after calling alogin and awaiting `request.auser()`, we end up
performing redundant authentication requests (at least against the
database).

Could you provide exact details and a complete description of this use
case? So we could evaluate further. Thank you!
--
Ticket URL: <https://code.djangoproject.com/ticket/36819#comment:3>

[Django]

#36819: Async login does not reuse authenticated user instance
-------------------------------------+-------------------------------------
Reporter: Mykhailo Havelia | Owner: (none)
Type: | Status: closed
Cleanup/optimization |
Component: contrib.auth | Version: 6.0
Severity: Normal | Resolution: wontfix
Keywords: async auser alogin | Triage Stage:
cache | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mykhailo Havelia):

Replying to [comment:3 Natalia Bidart]:

I found a related ticket — https://code.djangoproject.com/ticket/36540 —
which appears to have been fixed a few months ago 😌

{{{
async def auser():
return user

request.auser = auser
}}}

Sorry for taking your time. I'll make sure to check against the current
master next time.
--
Ticket URL: <https://code.djangoproject.com/ticket/36819#comment:4>