[Django] #22638: Form wizard may raise unreasonable exceptions in case of SECRET_KEY change
Django
-----------------------------------+--------------------
Reporter: erikr | Owner: nobody
Type: Bug | Status: new
Component: contrib.formtools | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------
Our form wizard has two storage options: sessions and cookies, with
`SessionWizardView` and `CookieWizardView`. To prevent manipulation, the
cookies storage uses the signed cookies from `django.core.signing`. This
creates a signature based on the `SECRET_KEY`. If the secret key is
changed, `request.get_signed_cookie` will raise an exception, in which
case the storage will raise `WizardViewCookieModified`, a subclass of
`SuspiciousOperation`.
The cookie is loaded very early in the rendering of a the form wizard
view. This means that if a user starts a form wizard, and the secret key
is changed, any requests to the wizard will result in an exception and
likely a 500 error. The user can only recover from this by deleting the
cookie or restarting the browser (it seems to only persist in the current
session).
It may appear sensible to raise a `SuspiciousOperation` for a possible
cookie manipulation, but we currently don't do this in any other place,
like sessions. Currently, user may suddenly get 500 errors for no clear
reason, and the developer of the project has no ability to help them.
Leaving this as is may also discourage people from rotating their secret
key when needed.
I suggest that in case of an invalid wizard cookie, we simply ignore the
value and thereby return the user to the first step.
--
Ticket URL: <https://code.djangoproject.com/ticket/22638>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: erikr | Owner: nobody
Type: Bug | Status: new
Component: contrib.formtools | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:1>
Django
Reporter: erikr | Owner: nobody
Type: Bug | Status: new
Component: contrib.formtools | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by nott):
WizardViewCookieModified, a subclass of SuspiciousOperation, is raised in
this situation since 1.6
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:2>
Django
Reporter: erikr | Owner: nott
Type: Bug | Status: assigned
Component: contrib.formtools | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by nott):
* owner: nobody => nott
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:3>
Django
Reporter: erikr | Owner: nott
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by erikr):
* has_patch: 0 => 1
Comment:
Apologies, forgot to assign this to myself before working on it. Wrote a
PR on https://github.com/django/django/pull/2673
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:4>
Django
Reporter: erikr | Owner:
Type: Bug | Status: new
Component: contrib.formtools | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by erikr):
* owner: nott =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:5>
Django
Reporter: erikr | Owner:
Type: Bug | Status: new
Component: contrib.formtools | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by syphar):
tested the behavior with a test-application, looks fine to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:6>
Django
Reporter: erikr | Owner:
Type: Bug | Status: new
Component: contrib.formtools | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by syphar):
* cc: denis.cornehl@… (added)
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:7>
Django
Reporter: erikr | Owner:
Type: Bug | Status: new
Component: contrib.formtools | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by erikr):
* needs_better_patch: 1 => 0
Comment:
Existing PR updated: https://github.com/django/django/pull/2673
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:8>
Django
Reporter: erikr | Owner:
Type: Bug | Status: new
Component: contrib.formtools | Version: 1.6
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by claudep):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:9>
Django
Reporter: erikr | Owner: erikr
Type: Bug | Status: assigned
Component: contrib.formtools | Version: 1.6
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* owner: => erikr
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:10>
Django
Reporter: erikr | Owner: erikr
Component: contrib.formtools | Version: 1.6
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ba5ddf7aed542d25f0fdb25a04d87305de0f3972"]:
{{{
#!CommitTicketReference repository=""
revision="ba5ddf7aed542d25f0fdb25a04d87305de0f3972"
Fixed #22638 -- Changed CookieWizardView to ignore invalid cookies
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22638#comment:11>