Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29946 closed Bug (needsinfo)

Weird session behavior after upgrading to Django 1.11.16

Reported by: Tim Abbott Owned by: nobody
Component: contrib.auth Version: 1.11
Severity: Release blocker Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

Upgrading from Django 1.11.14 to Django 1.11.16 last week seems to have introduced a weird and serious password reset + session bug issue in Zulip (https://zulipchat.com). What we experienced was at follows:

  • A user reset their password using the Django password reset form, and completes the flow. This part of the reproduction recipe seems to be essential.
  • That user then logs in with their new password. The login completes, and e.g. sends new-login emails from our processor for the user_logged_in signal.
  • Immediately after the login completes, all API requests fail with the server returning the standard 401 error for "no session", aka request.user.is_authenticated being False.
  • Looking at a browser console, the browser is sending cookies.
  • Inspecting the database using our standard session debugging code: https://github.com/zulip/zulip/blob/master/zerver/lib/sessions.py#L25, there are no sessions associated with the user after this login process.
  • This was highly reproducible for us.

While I'm not 100% sure that the Django upgrade is what caused the problem (just because I haven't been able to reproduce it in our development environment), but the following is very suspicious:

  • Reverting to 1.11.14 fixed the problem for all the previously affected users who we've talked to (still waiting to hear back from a few more for further confirmation).
  • We haven't made any changes to our own password reset code in months.
  • During the same period after we took the Django upgrade, we started seeing this new exception in our error emails, which is in the database code for session updates and thus could be triggered by the changes in 1.11.16:
Traceback (most recent call last):
  File "/home/zulip/deployments/2018-11-12-02-10-23/zulip-py3-venv/lib/python3.5/site-packages/django/contrib/sessions/backends/db.py", line 87, in save
    obj.save(force_insert=must_create, force_update=not must_create, using=using)
  File "/home/zulip/deployments/2018-11-12-02-10-23/zulip-py3-venv/lib/python3.5/site-packages/django/db/models/base.py", line 808, in save
    force_update=force_update, update_fields=update_fields)
  File "/home/zulip/deployments/2018-11-12-02-10-23/zulip-py3-venv/lib/python3.5/site-packages/django/db/models/base.py", line 838, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/home/zulip/deployments/2018-11-12-02-10-23/zulip-py3-venv/lib/python3.5/site-packages/django/db/models/base.py", line 907, in _save_table
    raise DatabaseError("Forced update did not affect any rows.")
django.db.utils.DatabaseError: Forced update did not affect any rows.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/zulip/deployments/2018-11-12-02-10-23/zulip-py3-venv/lib/python3.5/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/home/zulip/deployments/2018-11-12-02-10-23/zulip-py3-venv/lib/python3.5/site-packages/django/utils/deprecation.py", line 142, in __call__
    response = self.process_response(request, response)
  File "./zerver/middleware.py", line 389, in process_response
    request.session.save()
  File "/home/zulip/deployments/2018-11-12-02-10-23/zulip-py3-venv/lib/python3.5/site-packages/django/contrib/sessions/backends/cached_db.py", line 63, in save
    super(SessionStore, self).save(must_create)
  File "/home/zulip/deployments/2018-11-12-02-10-23/zulip-py3-venv/lib/python3.5/site-packages/django/contrib/sessions/backends/db.py", line 94, in save
    raise UpdateError
django.contrib.sessions.backends.base.UpdateError

I'm happy to do further debugging to help get to the bottom of this.

Change History (4)

comment:1 by Tim Graham, 6 years ago

Description: modified (diff)

There are two changes between 1.11.14 and 1.11.16

In 1.11.15: d6eaee092709aad477a9894598496c6deec532ff - Fixed CVE-2018-14574 -- Fixed open redirect possibility in CommonMiddleware.
In 1.11.16: 2668418d99b42599536d353705456cf5db718d48 - Fixed #29499 -- Fixed race condition in QuerySet.update_or_create().

It's not obvious how these fixes could be relevant to the code in question.

comment:2 by Tim Graham, 6 years ago

Resolution: needsinfo
Status: newclosed

Please reopen if you find Django at fault.

comment:3 by Tim Abbott, 6 years ago

Just a quick follow-up for now: We've been able to confirm that the problem is not resolved by the version downgrade (but was masked by the server restart effectively flushing memcached). I could repeatedly reproduce by changing a user's password (to get into bad state) and then flush memcached to leave it. The issue seems to be completely resolved by switching from the cached_db to the db session backend, so I think there is a Django bug somewhere here. I don't have time to finish isolating before Thanksgiving, but I expect to have more details on the exact Django issue after thanksgiving (will probably end up being a new issue to declutter the world). Thanks!

comment:4 by Carlton Gibson, 6 years ago

Thanks for the follow-up Tim. Please do open a new issue if/when you track down issue.

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