Skip to content

Commit

Permalink
Fix BadCSRFToken error
Browse files Browse the repository at this point in the history
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 #8940 (see the issue for
details of the bug and steps to reproduce).
  • Loading branch information
seanh committed Sep 19, 2024
1 parent 0a183d8 commit 8595594
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 7 deletions.
8 changes: 4 additions & 4 deletions h/security/policy/top_level.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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"],
Expand All @@ -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=HTML_AUTHCOOKIE_MAX_AGE + 3600,
)
api_authcookie = api_authcookie.bind(request)

Expand All @@ -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",
)
Expand Down
18 changes: 17 additions & 1 deletion h/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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=HTML_AUTHCOOKIE_MAX_AGE + 3600,
)
config.set_session_factory(factory)
config.set_csrf_storage_policy(SessionCSRFStoragePolicy())
4 changes: 2 additions & 2 deletions tests/unit/h/security/policy/top_level_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def test_api_request(
secret="test_h_api_auth_cookie_secret",
salt="test_h_api_auth_cookie_salt",
cookie_name="h_api_authcookie",
max_age=5184000,
max_age=2595600,
httponly=True,
secure=True,
samesite="strict",
Expand Down Expand Up @@ -147,7 +147,7 @@ def test_non_api_request(
secret="test_h_api_auth_cookie_secret",
salt="test_h_api_auth_cookie_salt",
cookie_name="h_api_authcookie",
max_age=5184000,
max_age=2595600,
httponly=True,
secure=True,
samesite="strict",
Expand Down

0 comments on commit 8595594

Please sign in to comment.