Test cookie isn't deleted if persistent data in Newforms-Admin login

[Michael Newman]

The test is probably the most simple way to see this. Right now a test cookie might be left in the users session due to it only being removed if there is no persistent data. The fix is simply moving the delete_test_cookie up a few lines and I added to the admin_view tests to check to make sure there isn't a cookie left after the client is logged in.

[Rajesh Dhawan]

session.test_cookie_delete() will raise an Exception if testcookie does not exist in the session. So, we should test that the cookie worked before calling test_cookie_delete:

if request.session.test_cookie_worked():
    request.session.delete_test_cookie()

[anonymous]

rajeshdhawan: You can see on line 244:

if not request.session.test_cookie_worked():
     message = _("Looks like your browser isn't configured to accept cookies. Please enable cookies, reload this page, and try again.")
     return self.display_login_form(request, message)

which does the same check you are purposing and short circuits long before it reaches this code.

This is a bug in django and I am marking for 1.0 so it doesn't get forgotten, correct me if I am wrong.

[Rajesh Dhawan]

anonymous: Yes, I see that it does the same check. It's strange, though, because I have been lately seeing the following trace (I got a few of these this morning using the latest svn checkout):

Traceback:
File "/home/rajesh/Development/default-django/django/core/handlers/base.py"
in get_response 86. response = callback(request, *callback_args, **callback_kwargs)
File "/home/rajesh/Development/default-django/django/contrib/admin/sites.py"
in root 156. return self.login(request)
File "/home/rajesh/Development/django-svn/django/views/decorators/cache.py"
in _wrapped_view_func 44. response = view_func(request, *args, **kwargs)
File "/home/rajesh/Development/default-django/django/contrib/admin/sites.py"
in login 280. request.session.delete_test_cookie()
File "/home/rajesh/Development/default-django/django/contrib/sessions/backends/base.py"
in delete_test_cookie 84. del self[self.TEST_COOKIE_NAME]
File "/home/rajesh/Development/default-django/django/contrib/sessions/backends/base.py"
in __delitem__ 53. del self._session[key]
Exception Type: KeyError at /admin/ Exception Value: 'testcookie'

I haven't traced this down to the session.delete_test_cookie function to be able to confirm that it's a bug in the sessions module (it happens very intermittently, so it's hard to debug).

[Russell Keith-Magee]

(In [8484]) Fixed #7776: Ensured that the test cookie is always deleted once a login has succeeded. Thanks for the report and fix, Mnewman.

[Rajesh Dhawan]

Russell: This still throws the error I mentioned above. Previously, I thought it was an intermittent problem but now I have been able to narrow it down to the case where an already logged in user is logging in to admin as a different user. In such a case, the login call on line 274 has the side effect of flushing/clearing out the session of the previously logged in user (as of [8343]). So, this empty session causes the delete_test_cookie call to fail with a KeyError.

I think that the test cookie should only be deleted after checking request.session.test_cookie_worked() again. The test_cookie_worked call at line 244 is not sufficient.

[Michael Newman]

rajeshdhawan: The error you are having is clearly a different problem. If you are having a problem please open another ticket. the bug that I reported and described has been fixed. Thanks Russell

[Jacob]

Milestone 1.0 deleted