From bfd2e1a16b84a4be94bea3729da4c36334ea2a22 Mon Sep 17 00:00:00 2001 From: Sean Hammond Date: Fri, 6 Sep 2024 15:32:54 +0100 Subject: [PATCH] Fix `BadCSRFToken` error Fix a `BadCSRFToken` error that occurs when users leave a page open for more than an hour and then try to continue using it. Fixes https://github.com/hypothesis/h/issues/8940 (see the issue for details of the bug and steps to reproduce). --- h/security/policy/top_level.py | 8 ++++---- h/session.py | 18 +++++++++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/h/security/policy/top_level.py b/h/security/policy/top_level.py index 6f1cc446009..bfa2c9dd591 100644 --- a/h/security/policy/top_level.py +++ b/h/security/policy/top_level.py @@ -10,6 +10,8 @@ from h.security.policy._cookie import CookiePolicy from h.security.policy.helpers import AuthTicketCookieHelper, is_api_request +HTML_AUTHCOOKIE_MAX_AGE = 30 * 24 * 3600 # 30 days. + class TopLevelPolicy: """The top-level security policy. Delegates to subpolicies.""" @@ -42,8 +44,6 @@ def _load_identity(self, request): def get_subpolicy(request): """Return the subpolicy for TopLevelSecurityPolicy to delegate to for `request`.""" - html_authcookie_max_age = 30 * 24 * 3600 # 30 days. - # The cookie that's used to authenticate API requests. api_authcookie = webob.cookies.SignedCookieProfile( secret=request.registry.settings["h_api_auth_cookie_secret"], @@ -55,7 +55,7 @@ def get_subpolicy(request): # Make the API authcookie stay fresh for longer than the HTML one. # This is to make it less likely that a browser will have an unexpired HTML # authcookie but an expired API one, which can lead to confusing results. - max_age=2 * html_authcookie_max_age, + max_age=2 * HTML_AUTHCOOKIE_MAX_AGE, ) api_authcookie = api_authcookie.bind(request) @@ -64,7 +64,7 @@ def get_subpolicy(request): secret=request.registry.settings["h_auth_cookie_secret"], salt="authsanity", cookie_name="auth", - max_age=html_authcookie_max_age, + max_age=HTML_AUTHCOOKIE_MAX_AGE, httponly=True, secure=request.scheme == "https", ) diff --git a/h/session.py b/h/session.py index 9b86a67be61..86d1963dafc 100644 --- a/h/session.py +++ b/h/session.py @@ -4,6 +4,7 @@ from pyramid.session import JSONSerializer, SignedCookieSessionFactory from h.security import derive_key +from h.security.policy.top_level import HTML_AUTHCOOKIE_MAX_AGE def model(request): @@ -110,7 +111,22 @@ def includeme(config): # pragma: no cover httponly=True, secure=secure, serializer=JSONSerializer(), - timeout=3600, + # One thing h uses the session for is to store CSRF tokens (see + # SessionCSRFStoragePolicy() below). + # + # The auth cookies that keep users logged in to h web pages have a + # lifetime of HTML_AUTHCOOKIE_MAX_AGE so in theory a user can leave a + # tab open (say a page containing a form) for up to + # HTML_AUTHCOOKIE_MAX_AGE and then return to the tab and expect to be + # able to submit the form. + # + # However, even if the user's auth cookie is still valid their form + # submission will still fail if the form's CSRF token has expired and + # the user will see a BadCSRFToken error. + # + # To avoid this we make sure that the lifetime of CSRF tokens is always + # longer than the lifetimes of auth cookies. + timeout=2 * HTML_AUTHCOOKIE_MAX_AGE, ) config.set_session_factory(factory) config.set_csrf_storage_policy(SessionCSRFStoragePolicy())