Django

Code

Ticket #3393 (closed: fixed)

Opened 2 years ago

Last modified 3 months ago

login view assumes 'set_test_cookie' has been called

Reported by: ubernostrum Assigned to: jacob
Milestone: 1.0 Component: Contrib apps
Version: SVN Keywords: auth login set_test_cookie
Cc: Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 1

Description

Currently, django.contrib.auth.views.login instantiates AuthenticationForm by passing it the request, which is optional; when this happens, AuthenticationForm will fail on valid logins unless set_test_cookie has been called previously. The login view itself does this, but this prevents easy reusability -- it's a fairly common use case to want to display a login box on each page for non-authenticated users, which means that each view in use must call set_test_cookie to ensure that the POST to login won't fail.

A simple solution to this would be to stop passing the request to AuthenticationForm. Perhaps a more flexible solution would be for login to begin taking a keyword argument to determine whether it should pass the request or not (and hence, whether set_test_cookie will be required or not)

Attachments

3393.diff (0.6 kB) - added by ubernostrum on 10/11/07 14:40:25.
Simpler patch
3393_2.diff (1.0 kB) - added by lcordier on 12/10/07 08:27:11.

Change History

01/29/07 13:50:41 changed by ubernostrum

And Jacob has suggested just yanking the test out of AuthenticationForm; I'll work on a patch for that, since it sounds like a better idea (though it means that any code which uses AuthenticationForm will now need to remember to test for cookies on its own).

01/30/07 10:14:34 changed by Gary Wilson <gary.wilson@gmail.com>

  • needs_better_patch set to 1.
  • has_patch set to 1.
  • stage changed from Unreviewed to Accepted.

10/11/07 14:40:25 changed by ubernostrum

  • attachment 3393.diff added.

Simpler patch

10/11/07 14:45:25 changed by jacob

  • owner changed from nobody to jacob.
  • status changed from new to assigned.

12/10/07 08:26:39 changed by lcordier

3393.diff have problems with the deletion of the test-cookie, when it has not been set.

12/10/07 08:27:11 changed by lcordier

  • attachment 3393_2.diff added.

03/31/08 16:05:11 changed by MiguelFilho <miguel.filho@gmail.com>

I have just faced this problem. I was doing a post from a custom view and had the exactly same problem described by ubernostrum.

I've applied 3393_2.diff against trunk and it worked fine for me.

Any chance to get this merged?

04/14/08 15:48:29 changed by kcarnold

I was seeing odd problems of a single-page login only working in weird circumstances. This patch fixed it immediately. +1 by me.

(I don't bother testing for cookie support anywhere in my app. Maybe I should?)

05/29/08 03:49:21 changed by lcordier

I am not sure, but maybe we should un-tick the "Patch needs improvement" flag for the triagers to evaluate the new patch ? Otherwise it would help if they can indicate what types of improvements that are needed.

06/16/08 14:37:22 changed by jacob

  • milestone set to 1.0.

06/17/08 20:05:06 changed by SmileyChris

I get the point of this ticket, but it is killing existing functionality.

Perhaps a better solution would be to have an attribute on the view which, if set, avoids the set_test_cookie option. Then people can just pass this arg to the login view in their url conf.

06/18/08 11:13:15 changed by jacob

  • status changed from assigned to closed.
  • resolution set to fixed.

(In [7692]) Fixed #3393: login view no longer assumes that set_test_cookie has been called. This is mildly backwards-incompatible, but in the "now it works the way it should have all along" sense. Thanks to James and lcordier for the patches.


Add/Change #3393 (login view assumes 'set_test_cookie' has been called)




Change Properties
Action