CSRF with AJAX documentation is out-of-date

[Idan Gazit]

Following the release of Django 1.2.5, we issued new guidelines on using CSRF protection with AJAX requests: ​https://www.djangoproject.com/weblog/2011/feb/08/security/

In that release, we included a JS snippet showing how to properly set the CSRF token header on AJAX requests, which never made it into the docs.

In addition, the existing docs on using CSRF with AJAX are not as good as they could be. Right now, we mix together discussion of how to get the CSRF token and how to use it—breaking these out into logical sections would make the docs easier to read.

Because the changes I'm making touch on security-related issues, I'd really like several pairs of practiced eyes to go over it before we make a change.

[Idan Gazit]

Docs patch with clearer AJAX & CSRF explanations.

[anonymous]

A minor comment on the docs part, you say:

 	100	If the CSRF token is not present in markup by use of the :ttag:`csrf_token` 
 	101	template tag, it must be supplied to the client by other means. This is common 
 	102	in cases where the form is dynamically added to the page, and Django supplies 
 	103	a decorator which will set a cookie containing the CSRF token: 
 	104	:func:`~django.views.decorators.csrf.ensure_csrf_cookie`. Use the decorator 
 	105	on any view which will need access to the CSRF token, but doesn't include the 
 	106	token in the actual markup sent to the client. 

Small clarification, but I think of view as referring to python code specifically, not the more abstract combination of view and content it renders for display.

So perhaps something like "If you need access to the CSRF on the client and it is not made available in the markup, use the CSRF decorator on the associated view to set the CSRF cookie"

Specifying that the need is on the client side, instead of in the "view" which I think of as the Django side.

[anonymous]

Also to highlight something I think PaulM saw and commented on in IRC, there is either a bug in the original/current snippet in the docs, or my logic is failing.

This line was removed from the current docs in the patch:

126	 	        if (!safeMethod(settings.type) && sameOrigin(settings.url)) { 

and this was added:

145	            if (!(/^https?:.*/.test(settings.url)) && safeMethod(settings.type) ) { 

the logic of safeMethod is reversed - but as I read it, the patch is correct, you only want to send the cookie if the method was safe. If that is not the case, that function should be renamed to be much clearer.

Also not clear in the current patch, nor the original security release notes, is why relative URLs should be enforced. A brief note explaining this in the docs would be beneficial.

[Preston Holmes]

doh - a noisy note to say, FWIW, that the above two comments are from me

[Luke Plant]

Is there a reason the patch reverts [16183] and removes sameOrigin?

[Luke Plant]

BTW, if I remember correctly, the snippet that gets the csrftoken from the DOM was deliberately rejected in favour of one that gets it from the cookie. If you have the token in the DOM, you are guaranteed to have it in the cookie too if you used the normal mechanism. So it seemed to make more sense to rely on the cookie.

@ptone: other parts of CSRF docs do mention the fact that if you send a CSRF token cross-domain you have a security issue - the other domain can use the token to do a CSRF attack on you.

[Preston Holmes]

I checked, and yes both the context_processor, and decorator call the middleware, so the cookie should be available. One thing to mention in this part of the docs, is that the cookie name may not be the default if the CSRF_COOKIE_NAME has been changed.

Also, the csrf input element will be present and set to "" if the context processor was run without either csrf decorator or middleware setting the cookie

@lukeplant my comment about enforcing relative URLs was basically the same one you made about removing same origin, that is, disallowing full, same origin URLs - I wasn't aware of how new r16183 was.

Either way - this should be accepted and improved

[Idan Gazit]

@lukeplant:

Regarding [16183]:

Looking at this again with fresh eyes, I see that I mixed up the date order — [16183] came after the blog post (linked in ticket description). Posting a new patch backing out that deletion shortly, my bad.

Regardless, there should be an explicit mention that the sameOrigin logic adheres to the recommendation of that blogpost, so it doesn't appear we're providing conflicting recommendations.

Regarding DOM vs. cookie for acquiring CSRF token.

Again, the blogpost cited above recommends a snippet which is getting the value out of the DOM. If this isn't a good way to get the snippet, then we should update the blogpost's example. Either way, will update patch to omit the DOM method.

@ptone

Good catch regarding safeMethod. I'm changing the method name to be a bit clearer about what it does.

[Idan Gazit]

Better CSRF docs patch, incorporates fixes from comments above.

[Luke Plant]

@idangazit:

As I remember there was a series of changes in quick succession. Relatively few people have access to the blog to change it (I don't), so it only got updated in the docs. For this reason I think we need to refrain from putting code and instructions like that on the blog where it is difficult to correct.

[Tim Graham]

Updated to apply cleanly to master

[Tim Graham]

I've updated the patch to apply cleanly to master. My only question is whether it was a conscious decision to remove the note: "Due to a bug introduced in jQuery 1.5, the example above will not work correctly on that version. Make sure you are running at least jQuery 1.5.1." Otherwise, I'll try to give this a final test and commit it sometime soon.

[Tim Graham <timograham@…>]

In [e376558ed257a4f8ce63779058f3d181ef49caa0]:

Fixed #16936 - Updated javascript for CSRF protection.

Thanks Idan Gazit for the patch.

[Tim Graham <timograham@…>]

In [c088a42670df867667401f131b865d9c7686667f]:

[1.4.X] Fixed #16936 - Updated javascript for CSRF protection.

Thanks Idan Gazit for the patch.

Backport of e376558ed2 from master