-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CSRF protection #8
Comments
You can use the method proposed by @rrb5068 in #6. To summarize it: it involves passing a 3rd parameter to You have to validate the CSRF token's authenticity on the server yourself - Sijax doesn't handle that for you. The Sijax documentation doesn't mention CSRF tokens, because Sijax is more low-level and doesn't concern itself with that. |
In my (admittedly limited) experience, if security features like CSRF are not explicitly documented then they simply are not used in a majority of cases. So if the lack of documentation is simply a time issue rather than a decision cut in stone I'd be happy to contribute documentation myself. Maybe @rrb5068 could also provide an illuminating use-case. |
I'm not at all against documenting the CSRF and Sijax situation. I agree that it'd be nice if CSRF is mentioned (the least we could do). If a few ways to protect against it are mentioned as well, even better. If you feel like contributing these additions, I'd be happy to merge them in. |
Sorry, I haven’t checked this in quite a while. With a Flask app, I’m going to turn on CSRF protection because it’s going to keep my users safe. I could decorate my Sijax method and tell the app to not require the CSRF token just to make it work, but I wouldn’t want to do that because the protection is doing an important job. You said: "In my (admittedly limited) experience, if security features like CSRF are not explicitly documented then they simply are not used in a majority of cases.” @spantaleev may not have documented it for his Flask extension yet, but CSRF protection is widely documented for Flask and several of its extensions, as well as for web applications in general. If you want to talk about it more, I’d be happy to. |
Hi, thanks for checking in 😄 I've been swamped with other things but this will become relevant to me again in about two to three weeks which is when I will try to improve the documentation as well. My comment on documentation is mostly due to reading that "proper implementation of a security key" is left as an exercise to the reader, or that passwords need to be salted and hashed and properly checked but that is not shown. I understand the need to convey basic principles first but I think that a full example with best security practices should always be available in addition. Rant over 😃 I'll try to do my part to improve it. |
@spantaleev since I started working with Sijax again, I would like to make additions to the documentation now. I'm currently thinking about inserting another section after Setting up the client (browser) and maybe extending the chat, comet and upload examples. What do you think? |
@Midnighter, that's great. Any changes are welcome! |
So I've started with #10. Comments, questions, corrections are very welcome. For now I've changed the chat example only but I was also considering simply creating separate versions of each example with CSRF protection included. I would also like to expand on dealing with Ajax errors. I'm not sure how Sijax handles that at the moment. What do you think? |
Looks good. Thanks for your contribution - it points people in the right direction. We can possibly make it easier to pass the token for all requests and then make Flask-Sijax automatically validate it. The best way to do it may be to patch Sijax itself a bit, so that it could take some custom headers or jQuery AJAX request options and automatically add them to all requests. If the CSRF-protection feature is enabled, the initialization can be done by the The server-side code can then validate the token for every request and not let it go through at all. What happens whenever an invalid token is encountered needs to be decided. Do we return some JSON back and fire an event on the JS side that the developer can catch? Do we just create an In any case, this sounds like a major change and there's a lot of things to be decided and done about it.. |
It's definitely a lot to consider.
From what I could see, many people like to return an error 400 code. From my experience Sijax silently fails when something goes wrong. It'd be great if at least that could change in the near future, considering that the full range of changes is probably something for the far future. With a view, a |
What about adding a Sijax.request_include_in_data method in the sijax.js file. |
I see that you made an implementation of this in issue #6 but I find nothing about the topic of CSRF protection in the documentation of either Sijax or Flask-Sijax. Which method of using a token of those mentioned in #6 can I now use?
The text was updated successfully, but these errors were encountered: