#36819 closed Cleanup/optimization (wontfix)
Async login does not reuse authenticated user instance
| Reported by: | Mykhailo Havelia | Owned by: | |
|---|---|---|---|
| Component: | contrib.auth | Version: | 6.0 |
| Severity: | Normal | Keywords: | async auser alogin cache |
| Cc: | Mykhailo Havelia | Triage Stage: | Unreviewed |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
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
Change History (4)
follow-up: 2 comment:1 by , 3 days ago
| Keywords: | auser alogin cache added |
|---|---|
| Resolution: | → wontfix |
| Status: | new → closed |
follow-up: 3 comment:2 by , 2 days ago
Replying to 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.
follow-up: 4 comment:3 by , 2 days ago
Replying to Mykhailo Havelia:
Replying to 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!
comment:4 by , 42 hours ago
Replying to 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.
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:
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.