[Django] #21495: Add a setting for CSRF Header name
Django
-------------------------------+-----------------------------------
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Severity: Normal | Keywords: csrf,header,angularjs
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 1 | UI/UX: 0
-------------------------------+-----------------------------------
CSRF includes a few customizations in settings:
https://github.com/django/django/blob/master/django/conf/global_settings.py#L544
but neglects allowing the user to set the Header name used by the server.
It would be very helpful to have this setting to use with AngularJS. While
AngularJS allows overriding the cookie and header name, it would be better
for my workflow (and I'm sure others) to set this on the server side and
then AngularJS's CSRF functionality will "just work".
Details on the AngularJS CSRF workings:
http://docs.angularjs.org/api/ng.$http § Cross Site Request Forgery (XSRF)
Protection
Pull request:
https://github.com/django/django/pull/1958
--
Ticket URL: <https://code.djangoproject.com/ticket/21495>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by zerok):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Since also the cookie name is also configurable it definitely makes sense
to also make the header name configurable since it might be used from the
Django-context. The same is not necessarily true for the name of the form-
field so keeping this one hard-coded is IMO valid.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:1>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by susan):
I made a separate PR that addresses other people's suggestions here:
https://github.com/django/django/pull/1989 Feel free to code review; I'm
unsure what test(s) to add.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:2>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by susan):
I made a separate PR that addresses other people's suggestions here:
https://github.com/django/django/pull/1989 Feel free to code review; I'm
unsure what test(s) to add.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:3>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Comment (by anonymous):
YAPR (Yet Another PR) with a different interpretation for this change:
https://github.com/django/django/pull/1995
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:4>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 1 | Patch needs improvement: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by unaizalakain):
* needs_docs: 0 => 1
* easy: 1 => 0
* needs_tests: 0 => 1
Comment:
I'm +1 on the last implementation
(https://github.com/django/django/pull/1995) but this needs tests and
docs. I would also propose to the mailing list to follow a deprecation
timeline with the settings moved inside `CsrfViewMiddleware`. I'd be glad
if an issue initially requiring Yet An Other Setting turned out in an
issue removing 6 settings.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:5>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by unaizalakain):
* cc: unai@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:6>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by WesAlvaro):
Sure. If we're good on the idea, I'll create tests and mail the list. I
much prefer the idea of configuring the middleware vs adding settings!
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:7>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by timo):
As I noted on the [https://groups.google.com/d/topic/django-
developers/8bqbJ_6YArc/discussion mailing list thread]: "The main problem
I see with this approach is that it would no longer be straightforward for
3rd party code to access these settings. You'd need something akin to
get_user_model() to retrieve the currently installed CSRF middleware so
you could access its settings."
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:8>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by anonymous):
Thanks for the comment, I've replied to the thread. An interesting
concern, but one that I don't think we should be accommodating.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:9>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 1
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
The idea of moving settings into middleware has been thoroughly rejected
on [https://groups.google.com/d/topic/django-
developers/t8ybImtdnpM/discussion a django-developers thread] discussing
integrating django-secure's middleware. If it can be done in a backwards
compatible way, maybe it would make sense to group the CSRF settings into
a dictionary as suggested in the thread. See #22734 which discusses doing
this for the SMTP & email settings and also has a patch with proof of
concept code.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:10>
Django
Reporter: hello@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs | Needs documentation: 1
Has patch: 1 | Patch needs improvement: 1
Needs tests: 1 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by carljm):
Noting here that that mailing list thread also eventually reached a fairly
negative conclusion about the benefits of grouping related settings into
dictionaries. So I think an implementation of this ticket would best just
add a new setting.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:11>
Django
Reporter: hello@… | Owner: gregorth
Type: New feature | Status: assigned
Component: HTTP handling | Version: 1.6
Keywords: | Triage Stage: Accepted
csrf,header,angularjs |
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by gregorth):
* status: new => assigned
* needs_better_patch: 1 => 0
* owner: nobody => gregorth
Comment:
created pr here:
https://github.com/django/django/pull/4183
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:12>
Django
Reporter: hello@… | Owner: gregorth
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
csrf,header,angularjs |
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* version: 1.6 => master
* component: HTTP handling => CSRF
Comment:
Thanks, but tests and documentation are also required. See our
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/submitting-patches/#patch-review-checklist patch review checklist] for
tips. Please uncheck the appropriate flags on this ticket when that's been
added.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:13>
Django
Reporter: hello@… | Owner: gregorth
Type: New feature | Status: assigned
Component: CSRF | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
csrf,header,angularjs |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_docs: 1 => 0
* needs_tests: 1 => 0
Comment:
pr fixed, tests and documentation added
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:14>
Django
Reporter: hello@… | Owner: gregorth
Type: New feature | Status: assigned
Component: CSRF | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
csrf,header,angularjs |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
I've added comments for improvement on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:15>
Django
Reporter: hello@… | Owner: gregorth
Type: New feature | Status: assigned
Component: CSRF | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
csrf,header,angularjs |
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 1 => 0
Comment:
improved patch, squashed changes, updated PR
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:16>
Django
Reporter: hello@… | Owner: gregorth
Component: CSRF | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
csrf,header,angularjs |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"668d53cd125175eb708cc0af143f47b42cd42153"]:
{{{
#!CommitTicketReference repository=""
revision="668d53cd125175eb708cc0af143f47b42cd42153"
Fixed #21495 -- Added settings.CSRF_HEADER_NAME
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21495#comment:17>