search

maxcountryman / flask-seasurf

SeaSurf is a Flask extension for preventing cross-site request forgery (CSRF).
http://readthedocs.org/docs/flask-seasurf/
Other
190 stars 49 forks source link

Remove Jinja2 dependency for better AJAX support #29

Closed zdexter closed 11 years ago

zdexter commented 11 years ago

Currently, flask-seasurf will only send the client a cookie containing a CSRF token if someone has called the _get_token() function, usually from a Jinja2 view.

If you are doing AJAX requests and don't use the Jinja2 tag {{ csrf_token }} - that is, if you just want to use the value in the cookie for doing some AJAX after loading a CSRF-protected view, this change lets you do that.

It adds a bit of the request context into the application global g so that the response can know whether or not it should include a cookie. I thought that it would be reasonable to set the cookie on the response if the page should be protected per the app config.

Because all views that are protected per the config can now get the cookie upon render, there is no need to check and see if someone actually used the csrf_token tag. Previously, the 'used' flag was never set, meaning the cookie never got set.

zdexter commented 11 years ago

Let me know what you think! Hopefully this will fit most use cases. Mine was: If you want to, say, use AngularJS's built-in CSRF prevention (on all POST requests, Angular will send a header containing the value of the XSRF-TOKEN cookie if it exists), you have no need to write the csrf_token value to the template. This seems to be a very common use case these days as not everyone is using forms; also, javascript frameworks generally do not make you render the CSRF token.

maxcountryman commented 11 years ago

It looks like this is breaking a number of unit tests. Otherwise, I like the idea!

zdexter commented 11 years ago

Oops, sorry about that; added some commits to tidy things up and pass tests.

zdexter commented 11 years ago

Sorry for the delay; cleaned up, tested, and pushed.

texuf commented 11 years ago

Is this project still supported? This issue is also causing a "WARNING: Forbidden (CSRF token missing or incorrect.)" in my project. I fixed it by commenting out lines 246 and 247 in my own fork, but it cost me time and users. Why not merge the pull request?

maxcountryman commented 11 years ago

@texuf yes, it's supported. Always bear in mind you can fork a project, make changes to it, and then incorporate those into your project. You needn't rely on releases.

When I have time to review this I plan to merge it in and do a release. Until then, you may want to pull this in directly yourself, depending on your use case.

texuf commented 11 years ago

Thanks for the quick reply.

Just for the sake of anyone else with a similar problem, the current functionality seems to go against a note in the django doc you reference ( https://docs.djangoproject.com/en/dev/ref/contrib/csrf/#ajax)

The note below the getCookie example states, "Regardless, you’re guaranteed to have the cookie if the token is present in the DOM, so you should use the cookie!"

In the latest version of Flask-Seasurf, if a token is present in a jinja template, it is not set in the cookie.

Again thanks for your time! You do great work.

-A

On Thu, May 30, 2013 at 12:42 PM, Max Countryman notifications@github.comwrote:

@texuf https://github.com/texuf yes, it's supported. Always bear in mind you can fork a project, make changes to it, and then incorporate those into your project. You needn't rely on releases.

When I have time to review this I plan to merge it in and do a release. Until then, you may want to pull this in directly yourself, depending on your use case.

— Reply to this email directly or view it on GitHubhttps://github.com/maxcountryman/flask-seasurf/pull/29#issuecomment-18703252 .

zdexter commented 11 years ago

Update: This PR works with Flask 0.10.1.

Also, I just want to note for anyone wondering about the internals of this PR: It's significant that frameworks like AngularJS use JavaScript to set a custom header containing the value of the CSRF token. (Extracting the a token from the request cookies and comparing it against the expected value provides no security benefit.) Unless a browser has security flaws, an attacker will be unable to set a request header containing the value of the CSRF cookie because the attacker's JavaScript cannot access the cookie value.

In other words, an attacker cannot get the victim's browser to construct a cross-site request with the value of the CRSF cookie inserted into an HTTP header.

That's why Flask-SeaSurf gets the submitted CSRF token from the form or the headers only, and never from the cookies.

maxcountryman commented 11 years ago

@zdexter I'm ready to merge this if you feel comfortable with it.

maxcountryman commented 11 years ago

Merged with 6c974c713ee1eec43ad8d3808e37436992316234.