feat(csrf): Adds a setting for the CSRF header name. #1958

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
7 participants
@wesalvaro

This is useful for integrating with AngularJS.

AngularJS uses a default cookie and header name to magically just work. It would be nice to set the CSRF cookie and header name in my Django app and not have to worry about setting the CSRF values in my AngularJS code.

Details on the values here:
http://docs.angularjs.org/api/ng.$http § Cross Site Request Forgery (XSRF) Protection

@umazalakain

This comment has been minimized.

Show comment Hide comment
@umazalakain

umazalakain Nov 22, 2013

Contributor

It's a nice feature but I'd discus this on the mailing list first, needing Yet An Other Setting is always a handicap. Anyway, a ticket is a must.

Contributor

umazalakain commented Nov 22, 2013

It's a nice feature but I'd discus this on the mailing list first, needing Yet An Other Setting is always a handicap. Anyway, a ticket is a must.

@wesalvaro

This comment has been minimized.

Show comment Hide comment
@wesalvaro

wesalvaro Nov 22, 2013

Cool. Sounds good to me. I've posted to the list and created a ticket.

Cool. Sounds good to me. I've posted to the list and created a ticket.

@charettes

This comment has been minimized.

Show comment Hide comment
@charettes

charettes Nov 22, 2013

Member

What about making CSRF_HEADER_NAME a class attribute of CsrfViewMiddleware instead of introducing another setting?

One could then easily subclass CsrfViewMiddleware and replace its MIDDLEWARE_CLASSES 'django.middleware.csrf.CsrfViewMiddleware' entry with 'path.to.my.AngularJSCsrfViewMiddleware'.

Member

charettes commented Nov 22, 2013

What about making CSRF_HEADER_NAME a class attribute of CsrfViewMiddleware instead of introducing another setting?

One could then easily subclass CsrfViewMiddleware and replace its MIDDLEWARE_CLASSES 'django.middleware.csrf.CsrfViewMiddleware' entry with 'path.to.my.AngularJSCsrfViewMiddleware'.

@wesalvaro

This comment has been minimized.

Show comment Hide comment
@wesalvaro

wesalvaro Nov 22, 2013

That's a great idea, @charettes. What about the other CSRF settings at that point? Seems like that would be ideal for all of them to adopt.

That's a great idea, @charettes. What about the other CSRF settings at that point? Seems like that would be ideal for all of them to adopt.

@shelldweller

This comment has been minimized.

Show comment Hide comment
@shelldweller

shelldweller Nov 22, 2013

Can you not just set the header name in Angular via xsrfHeaderName?

Can you not just set the header name in Angular via xsrfHeaderName?

@umazalakain

This comment has been minimized.

Show comment Hide comment
@umazalakain

umazalakain Nov 22, 2013

Contributor

Specifying csrf related settings in the CsrfViewMiddleware seems the right thing to me though if we decide to move all csrf settings there we ought to follow a pretty slow deprecation timeline.

Contributor

umazalakain commented Nov 22, 2013

Specifying csrf related settings in the CsrfViewMiddleware seems the right thing to me though if we decide to move all csrf settings there we ought to follow a pretty slow deprecation timeline.

@@ -556,6 +556,9 @@
CSRF_COOKIE_SECURE = False
CSRF_COOKIE_HTTPONLY = False
+# Settings for CSRF header.

This comment has been minimized.

Show comment Hide comment
@zerok

zerok Nov 23, 2013

Contributor

The comment is IMHO not really necessary since the name speaks for itself and it kind of separates this from the rest of the CSRF related settings.

@zerok

zerok Nov 23, 2013

Contributor

The comment is IMHO not really necessary since the name speaks for itself and it kind of separates this from the rest of the CSRF related settings.

@ArcTanSusan

This comment has been minimized.

Show comment Hide comment
@ArcTanSusan

ArcTanSusan Nov 25, 2013

Contributor

I made a separate PR that implements people's suggestions to set the CSRF_HEADER_NAME header as a class attribute #1989 Feel free to comment there; I'm unsure what test(s) to add.

Contributor

ArcTanSusan commented Nov 25, 2013

I made a separate PR that implements people's suggestions to set the CSRF_HEADER_NAME header as a class attribute #1989 Feel free to comment there; I'm unsure what test(s) to add.

@wesalvaro

This comment has been minimized.

Show comment Hide comment
@wesalvaro

wesalvaro Nov 25, 2013

I'm not a fan of #1989 so here is my interpretation of the comments above: #1995

I'm not a fan of #1989 so here is my interpretation of the comments above: #1995

@timgraham

This comment has been minimized.

Show comment Hide comment
@timgraham

timgraham Nov 26, 2013

Member

Closing in lieu of #1995

Member

timgraham commented Nov 26, 2013

Closing in lieu of #1995

@timgraham timgraham closed this Nov 26, 2013

@wesalvaro wesalvaro deleted the wesalvaro:patch-1 branch Nov 26, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment