Opened 18 years ago
Closed 18 years ago
#4015 closed (fixed)
login and logout should update request.user
Reported by: | James Bennett | Owned by: | Adrian Holovaty |
---|---|---|---|
Component: | Contrib apps | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
Currently, django.contrib.auth.login
and django.contrib.auth.logout
don't update request.user
, which means that things which happen after those functions are called (e.g., templates which include {% if user.is_authenticated %}
) will not see that the authentication status has changed.
This causes some counterintuitive behavior:
- If you use
django.contrib.auth.views.logout
and have it return a template directly, the template may "think" you're still logged in even though you aren't (becauserequest.user
is still aUser
object). Having it return a redirect instead shows the expected behavior, because it ends up generating a new request). - If you use forms which subclass
django.contrib.auth.forms.AuthenticationForm
(e.g., the form for posting registered comments), the form may still think you're anonymous even after it's successfully logged you in (becauserequest.user
is still anAnonymousUser
object). This is why, for example, entering a username and password when previewing a registered comment seems to do nothing (the form will still think those fields are required, because it doesn't know you've successfully logged in during that request).
Having login
and logout
update request.user
would clear this up.
Attachments (1)
Change History (10)
comment:1 by , 18 years ago
Description: | modified (diff) |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 18 years ago
comment:3 by , 18 years ago
Answering my own question: no it shouldn't because user is an object on the class, not on the request instance. My suggestion of adding a function to it still seems a valid solution.
by , 18 years ago
Attachment: | request_user.diff added |
---|
Patch to update request.user in login and logout
comment:4 by , 18 years ago
Has patch: | set |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:5 by , 18 years ago
Although, isn't it possible that one may be using the login
/logout
functions and not the AuthenticationMiddleware
that is responsible for adding the user
attribute to the request
object?
comment:7 by , 18 years ago
I must not have been thinking too clearly last night.
I was thinking that the patch would raise an error for those not using the AuthenticationMiddleware
, since request.user
wouldn't exist for them. But I guess since the patch is just assigning to request.user
(and not accessing it beforehand) it wouldn't raise any errors. It would only slightly clutter the request
object for someone not using AuthenticationMiddleware
.
comment:8 by , 18 years ago
Well, AuthenticationMiddleware
is tiny. If they're using authentication, it (or some other approach which sticks a User
instance in the request
) would be the path of least resistance...and if it's a fully public site, with no authentication, then login
/logout
wouldn't (I presume) get called.
comment:9 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Perhaps add a function to the
LazyUser
class indjango.contrib.auth.middleware
which clearsrequest._cached_user
(and shouldn't it really just store_cached_user
in its own class rather than onrequest
)?