tornadoweb / tornado

Tornado is a Python web framework and asynchronous networking library, originally developed at FriendFeed.
http://www.tornadoweb.org/
Apache License 2.0
21.68k stars 5.5k forks source link

XSRF cookie expiration issues #865

Open bdarnell opened 11 years ago

bdarnell commented 11 years ago

The xsrf cookie is not refreshed gracefully when it expires. Form-based applications will usually be OK since they will request a page with a fresh token before any form submission, but long-lived AJAXy pages can have trouble if a session crosses the 30-day boundary. There should be some way to refresh the token before it expires.

Additionally, the cache expiration of any page that includes xsrf_form_html should not be greater than the remaining time on the xsrf cookie.

Discussion: https://groups.google.com/forum/#!searchin/python-tornado/xsrf/python-tornado/1aN84IYC7h8/cW9-J9JbxcUJ

z0u commented 8 years ago

This affects me too: I'm writing an AJAX application, and the only time the XSRF cookie gets set is when the user logs in. But at that point _current_user is None, so the _xsrf cookie is set to expire with the browser session. I needed to override xsrf_token in my login handler to set the expiry to the same time as my user session cookie:

    @property
    def xsrf_token(self):
        token = super().xsrf_token
        self.set_cookie( '_xsrf', token, expires_days=30)
        return token
z0u commented 8 years ago

... Or should I be calling xsrf_token for every GET request, so that it gets refreshed?

bdarnell commented 8 years ago

I think the original idea was that you'd delay the initial creation of the xsrf_token until the user had logged in. I'm not sure why it sets a session-scoped cookie when there is no user, instead of just being an error or using the same 30-day expiration. (Maybe it was a gesture towards privacy, so there were no long-term cookies on anonymous users? But browser sessions are longer than they used to be so I'm not sure that session-scoped cookies ever make sense any more). Even if that was the original idea, though, it's not really correct since login forms need XSRF protection too.

The current system is really designed for traditional HTML forms, and it's not a great fit for modern javascript apps. I'm not sure what the right design for that environment is. The simplest workaround I've used is to make GET requests (periodically and after any failure) just to refresh the xsrf_token.

bdarnell commented 1 year ago

With the introduction of SameSite cookies, Tornado's xsrf_cookies system is obsolete. I intend to deprecate it which means that issues like this one will go unfixed (I do not intend to delete the feature entirely, though). Feedback is welcome in #3226.