Optionally turn off usage of CRSF token, implementation of USE_CSRF_TOKEN setting

[ntuckovic]

First of all, I want to thank to the author and contributors of this amazing project!
We've discovered it via LinkedIn discussions one year from now and shortly after started to consider an usage in actual production on heavily daily visited frontend project, mainly organised as VUEJS application.
We have discussed about Progressive Enhancement concept for the next re-factor phase, primarily because of simplification benefits and change of main goals of the project.

As we pushed first unicorn component into production we have shortly faced an issue with usage of CRSF token.
The problem is that the every request to the server (not just unicorn ones) includes X-CSRF token into header and Vary header is then changed to Cookie, Origin, because of {% csrf_token %} token placement in the template.

This resulted with impossible effective caching of requests for URLs whose responses do not changes so often by 3rd party caching mechanism before application (we use Varnish).

It's completely understandable why CSRF token is used by Unicorn (security, etc...), but since our project does not communicate with DB directly (It's just one of the services that consumes internal API requests) we have decided to fork the project and loosen up security. To be exact we have set the usage of CSRF token as an optional (by default it is enabled) with additional setting variable USE_CSRF_TOKEN, you can see changes in the files from forked project PR: https://github.com/Styria-Digital/django-unicorn-loosened/pull/1/files

We are almost 3 months in production with this altered version of package and there where zero critical situations or bugs in unicorn functionalities when USE_CSRF_TOKEN is set to False.

Does something like our use case and code changes sounds and looks interesting and helpful for other users/implementators? If so, we might do a PR to official package.

Of course, beside core code implementation of USE_CSRF_TOKEN setting, there should be written documentation added to use this with caution.

Thank you for all future feedback and discussion!

[adamghill]

Thanks for the message @ntuckovic! I'm inclined to accept a PR for this functionality even though I do think it should be used with caution -- I will definitely add warnings to the docs if it gets merged in. But, it seems similar to Django itself where CSRF can be disabled entirely if you want.

One thought as I read through the Django CSRF docs again: if django-unicorn made AJAX GETs instead of POSTs, the CSRF protection wouldn't kick in. If all you are doing is hitting the backend for a re-rendered component that doesn't change server state then maybe that would be ok. Each Unicorn element could have an attribute to say it should be a GET, e.g.

<div>
  <input unicorn:model="name" unicorn:get></input>
</div>

Or maybe a modifier:

<div>
  <input unicorn:model.get="name"></input>
</div>

One nice thing about this to me is that the HTML is pretty explicitly saying this is "safe" -- using a setting to turn off CSRF for all Unicorn calls is a little more hidden.

I'm wondering if explicitly marking elements as GETs would be a useful feature instead of "setting to turn off CSRF protection" or maybe in addition? Let me know what you think!

[ntuckovic]

Hi @adamghill, thank you for reply!

Your suggestion seems reasonable, specifically because of explicity reasons you've mention. Nothing is hidden in some settings but it is as defined in code. Love the idea, in addition, surely that approach would also consider some code alteration that remove CSRF usage from JS and PY side in case of get parameter.

What about Unicorn.call method?
We are using it a lot, especially now when we are in hybrid period between VUE application and unicorn components.
Would something like a new method Unicorn.get fit in?
So we would have that new get method that runs GET request instead of POST when called, in example load_next_page method from the-feed component:

Unicorn.get('the-feed', 'load_next_page');

Only thing that bothers me a little bit is that nothing can forbid implementation of method that interacts with DB, and it is called with get parameter/method.

But this bad practice situation is also possible with plain Django or Rest Framework view implementation, there is no explicit validation what you do in the view upon GET request.

[adamghill]

Hmm, interesting. Unicorn.get(...) might be a good idea as well.

Only thing that bothers me a little bit is that nothing can forbid implementation of method that interacts with DB, and it is called with get parameter/method. But this bad practice situation is also possible with plain Django or Rest Framework view implementation, there is no explicit validation what you do in the view upon GET request.

Yeah, this was my thought as well. However, looking at https://github.com/conformist-mw/django-query-counter gave me an idea: I wonder if I could somehow check that no database changes are happening when the action is a GET? It's theoretical because I haven't tried it out and I'm not sure it's worth it, to be honest, but maybe I could see if it's possible.