CsrfMiddleware needs to reset ETag header
|Reported by:||carljm||Owned by:||lukeplant|
|Cc:||Triage Stage:||Design decision needed|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||yes||Patch needs improvement:||no|
If you enable CsrfMiddleware with ConditionalGetMiddleware (or CommonMiddleware + USE_ETAGS setting), spurious "Cross Site Request Forgery" 403 errors will be generated if a user logs out and then logs back in again.
The logout and re-login will cause the user to have a new session id, so their CSRF token will change. If the user then visits any form-containing page that they had previously visited in their old session, where the ETag header for the page was set before the CsrfMiddleware had a chance to add its token to the form, Django will incorrectly tell the browser that the page has not been modified (based on the ETag), even though the page should now contain a different CSRF token. Submission of the form on that page will then cause a 403 because the browser submits the old CSRF token with the form.
Ironically, this is _most_ likely to happen on a view with the never_cache decorator (such as the login view), because never_cache sets the ETag before CsrfMiddleware gets to mangle the form. It can also happen on any other form, if CsrfMiddleware is before ConditionalGetMiddleware/CommonMiddleware in the MIDDLEWARE_CLASSES (so it processes the response later).
The fix is two-fold: 1) CsrfMiddleware should always either recalculate or trash any ETag header it finds on a response where it adds a CSRF token, because that ETag's MD5 sum is no longer correct after the CSRF token is added, and 2) it should be documented that CsrfMiddleware should be placed after ConditionalGetMiddleware and CommonMiddleware in the MIDDLEWARE_CLASSES (otherwise the first fix will result in ETags not working or being double-calculated every time).
It might not be a bad idea to also add a warning in the middleware documentation that any middleware which touches the response content in a significant way needs to take the same precautions in order to work with ETags.
Change History (11)
Changed 7 years ago by carljm
comment:1 Changed 7 years ago by carljm
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
comment:6 Changed 7 years ago by carljm
- Triage Stage changed from Unreviewed to Design decision needed