Opened 8 years ago

Closed 8 years ago

#4015 closed (fixed)

login and logout should update request.user

Reported by: ubernostrum Owned by: adrian
Component: Contrib apps Version: master
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by ubernostrum)

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 (because request.user is still a User 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 (because request.user is still an AnonymousUser 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)

request_user.diff (686 bytes) - added by Vinay Sajip <vinay_sajip@…> 8 years ago.
Patch to update request.user in login and logout

Download all attachments as: .zip

Change History (10)

comment:1 Changed 8 years ago by ubernostrum

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 8 years ago by SmileyChris

Perhaps add a function to the LazyUser class in django.contrib.auth.middleware which clears request._cached_user (and shouldn't it really just store _cached_user in its own class rather than on request)?

comment:3 Changed 8 years ago by SmileyChris

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.

Changed 8 years ago by Vinay Sajip <vinay_sajip@…>

Patch to update request.user in login and logout

comment:4 Changed 8 years ago by Vinay Sajip <vinay_sajip@…>

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

comment:5 Changed 8 years ago by Gary Wilson <gary.wilson@…>

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:6 Changed 8 years ago by Vinay Sajip <vinay_sajip@…>

Sorry, I don't understand. The patch is to the login/logout functions.

comment:7 Changed 8 years ago by Gary Wilson <gary.wilson@…>

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 Changed 8 years ago by Vinay Sajip <vinay_sajip@…>

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 Changed 8 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

(In [5472]) Fixed #4015 -- Changed login() and logout() messages to update request.user if
it is relevant. Thanks James Bennett, Vinay Sajip and Gary Wilson.

Note: See TracTickets for help on using tickets.
Back to Top