[Django] #31360: Is `request.csrf_processing_done` part of the public CSRF protection API?
Django
------------------------------------------------+------------------------
Reporter: Jaap Roes | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: CSRF | Version: 3.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
I've used a combination of `@csrf_exempt` and `@requires_csrf_token` to
customize CSRF protection for a specific view in my project (as
recommended in the docs:
https://docs.djangoproject.com/en/3.0/ref/csrf/#scenarios).
By looking at the implementation of the `CsrfViewMiddleware` I noticed
that `csrf_processing_done` is set to `True` on the `request` object when
a requests is "accepted" (i.e. passes the `CSRF` tests).
I've used this implementation detail in my custom handling of a specif
case of CSRF failure. When the result `getattr(request,
'csrf_processing_done', False)` is `False` I assume that the request has
failed the CSRF test.
The Django docs do not mention this special `request` attribute, yet it's
not prefixed with an underscore. So I'm unsure if I can rely on this
attribute to be available after a Django upgrade.
Should I consider `csrf_processing_done` as an implementation detail that
can change at any time? Or is it part of the public API that is it missing
documentation?
If it's not part of the public API, is there some other way a view can be
made to handle CSRF failure in a custom way?
--
Ticket URL: <https://code.djangoproject.com/ticket/31360>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Jaap Roes | Owner: nobody
Cleanup/optimization |
Component: CSRF | Version: 3.0
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => invalid
Comment:
Hi Jaap.
Wowser. That's a goodie. :)
> When the result `getattr(request, 'csrf_processing_done', False)` is
`False` I assume that the request has failed the CSRF test.
This is not a safe assumption. If `csrf_processing_done` is just used as a
flag to avoid repeating work within `process_view()`. If the view is
`csrf_exempt` the it will never be set: process_view exits early. It's not
that the request failed the test: rather, that the test was never
performed.
In all cases, where the CSRF test is failed, the failure view response
(4XX type error view normally) is returned.
As an internal flag `csrf_processing_done` is not intended to be
documented. I don't imagine it's going anywhere, but, equally, I wouldn't
recommend you rely on it.
This question is probably better targeted at a support channel.
(Interesting as it is...)
--
Ticket URL: <https://code.djangoproject.com/ticket/31360#comment:1>
Django
Reporter: Jaap Roes | Owner: nobody
Cleanup/optimization |
Component: CSRF | Version: 3.0
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jaap Roes):
Hi Carlton,
As I was typing that last sentence I considered just posting it on Stack
Overflow instead. But the fist bit seemed appropriate as a Django ticket,
sorry for the noise.
As a note; the `requires_csrf_token` decorator only overrides `_reject` so
the entire CSRF machinery is kept intact. The only difference is that
`_reject` doesn't result in a 400 response. This makes it "safe" for me to
make the assumption about `csrf_processing_done`.
The issue now being that this relies on the implementation
`requires_csrf_token` and `CsrfViewMiddleware` not changing too much
between Django releases. Let's see what SO can come up with as a "safer"
alternative ;)
--
Ticket URL: <https://code.djangoproject.com/ticket/31360#comment:2>
Django
Reporter: Jaap Roes | Owner: nobody
Cleanup/optimization |
Component: CSRF | Version: 3.0
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Hi Jaap. Thanks for the follow-up. No problem.
> The issue now being that this relies on the implementation ... not
changing too much between Django releases.
I think that's a reasonable bet.
* Write a test case that checks it.
* Check against the pre-release versions for each major release.
It's not likely to change, and you'd have multiple months warning if it
did.
--
Ticket URL: <https://code.djangoproject.com/ticket/31360#comment:3>