toastdriven / restless

A lightweight REST miniframework for Python.
http://restless.readthedocs.org/en/latest/
BSD 3-Clause "New" or "Revised" License
831 stars 107 forks source link

DjangoResource.as_view should call csrf_exempt #21

Open selectnull opened 10 years ago

selectnull commented 10 years ago

DjangoResource as_list and as_detail call csrf_exempt on their super, but as_view is not overriden any custom endpoint method is protected with Django CSRF (and it shouldn't be).

toastdriven commented 10 years ago

Resolution is in #22, but needs tests before it can be merged.

schmitch commented 10 years ago

I'm still not a fan of this, since I even think that csrf_exempt shouldn't be called.

The OWASP Cheat Sheet is pretty clear about CSRF, too. It's still a big security issue. Also it's pretty easy to use the Django CSRF token in every Client Based (JavaScript) application. If somebody needs the csrf_exempt he could easily roll out his own "Resource" where he calls csrf_exempt explicitly.

toastdriven commented 10 years ago

@c-schmitt I'm confused. For an everyday site leaning on cookies/sessions for web form POSTs, CSRF is essential. But the very nature of an API is that you should be able to access it from other domains/places (think a mobile app) & we're handling authentication explicitly/separately. OWASP's guideline also seem to require performing additional requests in order to get a token.

The typical example (in my head) is that of a mobile app. Pre-OWASP, it looks like:

Post-OWASP, I think this looks like:

The post-OWASP process looks way more complex/fallible (not to mention, Restless has no cross-everything way of storing/generating such tokens), for the gain of shoving authentication to a different place. Perhaps I'm incorrect or overlooking something about this & would love to be corrected on where I'm incorrect.

selectnull commented 10 years ago

I must admit confusion on my part as well as I agree with @toastdriven's description of how APIs should work, so after thinking and googling a bit I would like to present my findings, hopefully be less confused and initiate more conversation. These are few links that I found relevant:

It appears that people are of the opinion that APIs, restfull or not, should be protected from CSRF.

Initially, my thoughts on the topic were that any API resource should be csrf exempt and therefor my pull request 306c8fb3978256c6b113a7aead88836b0b50b8c2. One of the reasons was similarity with as_list and as_detail methods. The reason for not wanting protection from CSRF is that I am using JWT for auth and don't need another token (which csrf_token in that scenario effectively is, just one more token along side jwt one).

Now, I'm leaning towards agreeing with @c-schmitt. Frameworks should not make the decision if API views should or shouldn't be csrf exempt. There are APIs that use cookie based auth and those are suspectible to CSRF. My conlusion is that if an application is using Django and is configured to be protected against CSRF then framework such as restless should not use csrf_exempt. If the developer uses JWT auth only (and not django cookie based auth), they should remove CsrfViewMiddleware from settings but that choice must not be decided in advance by the framework for him/her.

I see two possible ways this can be resolved:

  1. we agree that #22 is good for restless and I write those tests needed
  2. we agree that csrf_exempt should be removed from as_list and as_detail (in dj.py)

I still want to write those tests for #22 and plan to do so in the following weeks as those will be useful in any case this goes forward. I would definitely want to continue this discussion in the meantime and hear your opinions.