Skip to content

Commit

Permalink
Revert to HTML auth cookie
Browse files Browse the repository at this point in the history
Revert to using the HTML auth cookie (the original cookie that's used to
authenticate HTML page loads) instead of the new API auth cookie for
authenticated create-group and edit-group API requests.

This is a hopefully temporary workaround for an undiagnosed issue we've
been seeing with users getting 404s from the create-group API when
trying to use the create-group page, see:
https://hypothes-is.slack.com/archives/C2BLQDKHA/p1725041696040459
  • Loading branch information
seanh committed Aug 30, 2024
1 parent 3003d47 commit 87b8133
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 20 deletions.
27 changes: 18 additions & 9 deletions h/security/policy/top_level.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,6 @@ def get_subpolicy(request):
)
api_authcookie = api_authcookie.bind(request)

if is_api_request(request):
return APIPolicy(
[
BearerTokenPolicy(),
AuthClientPolicy(),
APICookiePolicy(api_authcookie, AuthTicketCookieHelper()),
]
)

# The cookie that's used to authenticate HTML page requests.
html_authcookie = webob.cookies.SignedCookieProfile(
secret=request.registry.settings["h_auth_cookie_secret"],
Expand All @@ -78,4 +69,22 @@ def get_subpolicy(request):
secure=request.scheme == "https",
)
html_authcookie = html_authcookie.bind(request)

if is_api_request(request):
return APIPolicy(
[
BearerTokenPolicy(),
AuthClientPolicy(),
APICookiePolicy(
# Use html_authcookie rather than api_authcookie to
# authenticate API requests, at least for now. This is a
# hopefully temporary workaround for an undiagnosed issue
# we've been seeing in production, see:
# https://hypothes-is.slack.com/archives/C2BLQDKHA/p1725041696040459
html_authcookie,
AuthTicketCookieHelper(),
),
]
)

return CookiePolicy(html_authcookie, api_authcookie, AuthTicketCookieHelper())
39 changes: 28 additions & 11 deletions tests/unit/h/security/policy/top_level_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,28 +70,45 @@ def test_api_request(
AuthTicketCookieHelper,
):
is_api_request.return_value = True
html_authcookie = create_autospec(
SignedCookieProfile, instance=True, spec_set=True
)
api_authcookie = create_autospec(
SignedCookieProfile, instance=True, spec_set=True
)
webob.cookies.SignedCookieProfile.return_value = api_authcookie
webob.cookies.SignedCookieProfile.side_effect = [
api_authcookie,
html_authcookie,
]

policy = get_subpolicy(pyramid_request)

BearerTokenPolicy.assert_called_once_with()
AuthClientPolicy.assert_called_once_with()
webob.cookies.SignedCookieProfile.assert_called_once_with(
secret="test_h_api_auth_cookie_secret",
salt="test_h_api_auth_cookie_salt",
cookie_name="h_api_authcookie",
max_age=5184000,
httponly=True,
secure=True,
samesite="strict",
)
assert webob.cookies.SignedCookieProfile.call_args_list == [
call(
secret="test_h_api_auth_cookie_secret",
salt="test_h_api_auth_cookie_salt",
cookie_name="h_api_authcookie",
max_age=5184000,
httponly=True,
secure=True,
samesite="strict",
),
call(
secret="test_h_auth_cookie_secret",
salt="authsanity",
cookie_name="auth",
max_age=2592000,
httponly=True,
secure=True,
),
]
api_authcookie.bind.assert_called_once_with(pyramid_request)
html_authcookie.bind.assert_called_once_with(pyramid_request)
AuthTicketCookieHelper.assert_called_once_with()
APICookiePolicy.assert_called_once_with(
api_authcookie.bind.return_value,
html_authcookie.bind.return_value,
AuthTicketCookieHelper.return_value,
)
APIPolicy.assert_called_once_with(
Expand Down

0 comments on commit 87b8133

Please sign in to comment.