[IMP] http: remove default CSRF tokens expiration
Our CSRF tokens are based on the current user session, and automatically expire as soon as the session does. However, they also come with a default 1h expiration delay. This proves to be a frequent annoyance for users who pause more than 1h on a form before submitting it (e.g. user logs out and browser sits on login page until the next day). It can even lead to blocking bugs, e.g. when the 1h expiration occurs in the middle of taking a survey exam, and the user is never able to post the answers that are only present in the state of the form they need to post. More generally, users have a hard time understanding those CSRF expiration errors, and don't know how to react. Longer default expiration times have been considered (e.g. 1 day or 1 week) but those would not bring any identified benefit in terms of security, while still giving a chance that some users would experience the incomprehensible HTTP 400 errors). Attacks that can typically compromise the CSRF token (XSS, RCE) can achieve as much, or more, on the system or user account than what is possible with the token. And nothing generally prevents the attacker from using the token immediately after capturing it, during the initial attack, making the expiration delay rather irrelevant. Given there seems to be no significant benefit in expiring the tokens before the session itself, let's just keep them valid as long as the session. Note: sessions are GC'd automatically after 7 days of inactivity, which gives an effective 1 week expiry for abandoned web forms anyway, as the token expires with the session. Additionally, fix `survey` module tests, that were using an incorrect regex for extracting CSRF tokens. closes odoo/odoo#51499 Signed-off-by: Olivier Dony (odo) <odo@openerp.com>
This commit is contained in:
@@ -167,7 +167,7 @@ class SurveyCase(common.SavepointCase):
|
||||
return self.opener.post(url=url, json={'params': post_data})
|
||||
|
||||
def _find_csrf_token(self, text):
|
||||
csrf_token_re = re.compile("(input.+csrf_token.+value=\")([_a-zA-Z0-9]{51})", re.MULTILINE)
|
||||
csrf_token_re = re.compile("(input.+csrf_token.+value=\")([a-f0-9]{40}o[0-9]*)", re.MULTILINE)
|
||||
return csrf_token_re.search(text).groups()[1]
|
||||
|
||||
def _prepare_post_data(self, question, answers, post_data):
|
||||
|
||||
+3
-4
@@ -381,13 +381,12 @@ class WebRequest(object):
|
||||
"""
|
||||
return self.session.db if not self.disable_db else None
|
||||
|
||||
def csrf_token(self, time_limit=3600):
|
||||
def csrf_token(self, time_limit=None):
|
||||
""" Generates and returns a CSRF token for the current session
|
||||
|
||||
:param time_limit: the CSRF token should only be valid for the
|
||||
specified duration (in second), by default 1h,
|
||||
:param time_limit: the CSRF token validity period (in seconds), or
|
||||
``None`` for the token to be valid as long as the
|
||||
current user's session is.
|
||||
current user session is (the default)
|
||||
:type time_limit: int | None
|
||||
:returns: ASCII token string
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user