CSRF/Ajax/JQuery - Token is set to be inserted on both GET and POST

[goran@…]

My understanding of the documentation is that a csrf token is only required for POST requests.

The JQuery code currently attaches (or attempts to insert the token) to XHR requests for both GET and POST actions to urls on the same domain.

I suggest that we need to insert an if statement to test if the request is a POST before an unnecessary loop of the browser cookies is called.

$('html').ajaxSend(function(event, xhr, settings) {
    xhr.setRequestHeader("x-testing1", 'testme1');
    function getCookie(name) {
        var cookieValue = null;
        if (document.cookie && document.cookie != '') {
            var cookies = document.cookie.split(';');
            for (var i = 0; i < cookies.length; i++) {
                var cookie = jQuery.trim(cookies[i]);
                // Does this cookie string begin with the name we want?
                if (cookie.substring(0, name.length + 1) == (name + '=')) {
                    cookieValue = decodeURIComponent(cookie.substring(name.length + 1));
                    break;
                }
            }
        }
        return cookieValue;
    }
    
    if (settings.type == 'POST') {
    	if (!(/^http:.*/.test(settings.url) || /^https:.*/.test(settings.url))) {
        	// Only send the token to relative URLs i.e. locally.
        	xhr.setRequestHeader("X-CSRFToken", getCookie('csrftoken'));
    	}
    }
});

Note the insertion of the (settings.type == 'POST') test (I understand this could be incorporated in the below if statement but thought it was more readable to present it this way.

[Luke Plant]

We decided to leave this check out for simplicity, but adding it would probably be better, due to the overhead of cookie parsing.

However, we would want to do settings.type != 'GET' instead of the above, for future compatibility with PUT and DELETE which will need the token. (Our CSRF protection doesn't handle them yet, but it should and hopefully will soon). I'd also like to find out whether case insensitive matching on the method name 'GET' might be required.

[Aymeric Augustin]

settings.type is uppercased at line 6674 of jQuery.js: s.type = s.type.toUpperCase();

The ajaxSend event is fired at line 6756: globalEventContext.trigger( "ajaxSend", [ jqXHR, s ] );

So we can safely assume that settings.type is uppercase.

---

I also noticed jQuery computes settings.hasContent like this: s.hasContent = !rnoContent.test( s.type ); where rnoContent = /^(?:GET|HEAD)$/.

This parameter is internal to jQuery; it is used, for instance, to determine whether to send the X-Requested-With header.

So the test for safe requests could be: if (!settings.hasContent) { /* send token on local requests */ }

---

Finally, since jQuery 1.5, settings.crossDomain tells whether the request is cross-domain — see around line 6647. Hence my suggestion:

$('html').ajaxSend(function(event, xhr, settings) {

    // ... define getCookie here ...

    // only send the CSRF token for local unsafe requests — safe requests are GET and HEAD
    if (!s.crossDomain && s.hasContent ) {
         xhr.setRequestHeader("X-CSRFToken", getCookie('csrftoken'));
    }
}

---

One last thought: we could mention the possibility to use ​http://plugins.jquery.com/project/Cookie and this code:

$('html').ajaxSend(function(event, xhr, settings) {
    // only send the CSRF token for local unsafe requests (safe requests are GET and HEAD)
    if (!s.crossDomain && s.hasContent ) {
         xhr.setRequestHeader("X-CSRFToken", $.cookie('csrftoken'));
    }
}

---

Any opinions on this before I propose a patch?

[Luke Plant]

I'm dubious about using settings.hasContent because it is only accidental that it does the right thing.

We also can't make the default implementation use things only available in jQuery 1.5

But we can mention some of these things in the docs.

[Luke Plant]

In [16190]:

Mentioned simplification of AJAX example code in CSRF docs.

Refs #15469. Thanks to aaugustin for the suggestion

[Luke Plant]

In [16191]:

Fixed #15469 - CSRF token is inserted on GET requests

Thanks to goran for report.

[Luke Plant]

In [16193]:

[1.3.X] Fixed #15469 - CSRF token is inserted on GET requests

Thanks to goran for report.

Backport of [16191] from trunk.

[Luke Plant]

In [16194]:

[1.2.X] Fixed #15469 - CSRF token is inserted on GET requests

Thanks to goran for report.

Backport of [16191] from trunk.