Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#16004 closed Bug (fixed)

csrf_protect does not send cookie if view returns TemplateResponse

Reported by: lukeplant Owned by: nobody
Component: CSRF Version: 1.3
Severity: Release blocker Keywords:
Cc: chris@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX:

Description

See http://groups.google.com/group/django-developers/browse_thread/thread/f96e982254fbe5c3S

The problem is that decorator_from_middleware does not render the response the way that normal handling does.

Note that problem will be hidden if CsrfViewMiddleware is in use.

See also #16003 which is likely related.

Attachments (3)

16004.fix.diff (795 bytes) - added by lukeplant 4 years ago.
Fix
16004.fix.alternative.diff (5.5 KB) - added by lukeplant 4 years ago.
16004.fix.alternative.2.diff (6.2 KB) - added by lukeplant 4 years ago.
Fixed issue found by Carl Meyer

Download all attachments as: .zip

Change History (12)

Changed 4 years ago by lukeplant

Fix

comment:1 Changed 4 years ago by lukeplant

I've attached a proposed fix. I've got full tests as well, but they depend on a refactoring of some existing tests that I haven't yet committed.

This would unfortunately render #15008 ([16087]) useless.

comment:2 Changed 4 years ago by jezdez

  • Patch needs improvement set

There *must* be a better way to fix this, I can't believe that CSRF prevents us from making it easier to customize the admin.

comment:3 Changed 4 years ago by lukeplant

It's not just CSRF, decorator_from_middleware is fundamentally broken w.r.t. TemplateResponse. This would be a bug in decorator_from_middleware even if csrf_protect didn't exist.

'gzip_page' is also affected (and would also be fixed by this), but the bigger issue is any 3rd party decorator that happens to use decorator_from_middleware.

And the bigger issue again is TemplateResponse, and the concept of lazy responses. I'm sure there are many other bugs lurking wherever rendering a template has side-effects - such as in django.contrib.messages and django.contrib.session.

comment:4 Changed 4 years ago by acdha

  • Cc chris@… added

Changed 4 years ago by lukeplant

comment:5 Changed 4 years ago by lukeplant

See my message on the thread for an explanation for the patch.

comment:6 Changed 4 years ago by jezdez

Amen! +1

Changed 4 years ago by lukeplant

Fixed issue found by Carl Meyer

comment:7 Changed 4 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from new to closed

In [16276]:

Fixed #16004 - csrf_protect does not send cookie if view returns TemplateResponse

The root bug was in decorator_from_middleware, and the fix also corrects
bugs with gzip_page and other decorators.

comment:8 Changed 4 years ago by lukeplant

In [16279]:

[1.3.X] Fixed #16004 - csrf_protect does not send cookie if view returns TemplateResponse

The root bug was in decorator_from_middleware, and the fix also corrects
bugs with gzip_page and other decorators.

Backport of [16276] from trunk.

comment:9 Changed 4 years ago by carljm

In [16961]:

Fixed #17035, #17036 -- Clarified documentation regarding TemplateResponse and middleware handling. Refs #16004. Thanks ptone.

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