Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#7364 closed (fixed)

Login view is cached when caching middleware is turned on

Reported by: clong@… Owned by: nobody
Component: Contrib apps Version: master
Severity: Keywords: auth, login, cache
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

This causes a problem, because the login view expects a cookie to be set when a user visits the login page. If it's not set it will fail the login, but will set the cookie.

Quick steps:

  1. Make a request to the login page without any GET or POST parameters.
  2. Web server returns the cached page. (Note: this doesn’t set the cookie because no Python code has been run, the page returned is static)
  3. The user enters in the login information and submits the information. This is sent to the server as a POST.
  4. The server, because of the POST, now runs the login view. This fails because there was no cookie previously set, but it does set the cookie that should have been previously set.
  5. The error message shown to the user is as if it was a failed attempt. If the user reenters the information, the login will now work as the cookie is now set.

I attached a diff that uses the 'never_cache' decorator to fix this.

Attachments (4)

auth-caching.diff (603 bytes) - added by clong@… 6 years ago.
admin_login_cache.diff (1.4 KB) - added by permon 6 years ago.
admin_login_cache-8161.diff (1.4 KB) - added by jcassee 6 years ago.
Updated patch to revision 8161
7364-admin_login_cache-8345.diff (1.7 KB) - added by jcassee 6 years ago.

Download all attachments as: .zip

Change History (14)

Changed 6 years ago by clong@…

Changed 6 years ago by permon

comment:1 Changed 6 years ago by permon

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Previous patch corrected general behaviour of login page. But this one was not the one used in contrib.admin.

Changed 6 years ago by jcassee

Updated patch to revision 8161

comment:2 Changed 6 years ago by ericholscher

  • milestone set to 1.0
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 6 years ago by jcassee

Diff updated to revision 8345. Also changed to older decorators syntax for compatibility with Python 2.3.

Changed 6 years ago by jcassee

comment:4 Changed 6 years ago by julianb

What about that: r7692?

comment:5 Changed 6 years ago by jcassee

Sorry, julianb, is this comment directed at the correct ticket? I'm not sure what the form has to do with the view being cached.

comment:6 Changed 6 years ago by julianb

This ticket #7364: "causes a problem, because the login view expects a cookie to be set when a user visits the login page"

Changeset [7692]: "login view no longer assumes that set_test_cookie has been called"

comment:7 Changed 6 years ago by jcassee

You are right, I guess. I thought it would be a bad idea to cache the login page in any case, as you may want to display a different text if the user is already logged in (the default template does not). The cookie thing was not my main concern.

comment:8 Changed 6 years ago by julianb

Okay, seems reasonable, sorry for thinking mainly about cookies ;)

I also see that there are some tickets open which aim to change how and when (test-)cookies are set. It's still important to see whether caching affects any new solution.

comment:9 Changed 6 years ago by gwilson

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

(In [8383]) Fixed #7364 -- Never cache the contrib.auth login view.

comment:10 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.