diff --git a/h/security/policy/_cookie.py b/h/security/policy/_cookie.py index a11949897f7..fbde0f3e65a 100644 --- a/h/security/policy/_cookie.py +++ b/h/security/policy/_cookie.py @@ -30,7 +30,7 @@ def __init__( def identity(self, request): self.helper.add_vary_by_cookie(request) - identity, _ = self.helper.identity(self.html_authcookie, request) + identity, ticket_id = self.helper.identity(self.html_authcookie, request) # If a request was successfully authenticated using the HTML auth # cookie and that request did *not* also contain the API auth cookie, @@ -47,7 +47,7 @@ def identity(self, request): # this won't happen but it could happen if the API auth cookie (but not # the HTML one) was deleted somehow, for example by the user manually # deleting the cookie in the browser's developer tools, or another way. - self._issue_api_authcookie(identity, request) + self._issue_api_authcookie(identity, request, ticket_id) return identity @@ -76,9 +76,11 @@ def remember(self, request, userid, **kw): # pylint:disable=unused-argument # would set the same cookie twice. request.h_api_authcookie_headers_added = True + ticket_id = self.helper.add_ticket(request, userid) + return [ - *self.helper.remember(self.html_authcookie, request, userid), - *self.helper.remember(self.api_authcookie, request, userid), + *self.helper.remember(self.html_authcookie, userid, ticket_id), + *self.helper.remember(self.api_authcookie, userid, ticket_id), ] def forget(self, request): @@ -91,7 +93,7 @@ def forget(self, request): def permits(self, request, context, permission) -> Allowed | Denied: return identity_permits(self.identity(request), context, permission) - def _issue_api_authcookie(self, identity, request): + def _issue_api_authcookie(self, identity, request, ticket_id): if not identity: return @@ -105,7 +107,7 @@ def _issue_api_authcookie(self, identity, request): return headers = self.helper.remember( - self.api_authcookie, request, identity.user.userid + self.api_authcookie, identity.user.userid, ticket_id ) def add_api_authcookie_headers( diff --git a/h/security/policy/helpers.py b/h/security/policy/helpers.py index 7e68201ca39..e7139698c79 100644 --- a/h/security/policy/helpers.py +++ b/h/security/policy/helpers.py @@ -28,9 +28,17 @@ def identity( return (Identity.from_models(user=ticket.user), ticket) - def remember(self, cookie: SignedCookieProfile, request: Request, userid: str): + def add_ticket(self, request: Request, userid): + """ + Add a new auth ticket for the given user to the DB. + + Returns the ID of the newly-created auth ticket. + """ ticket_id = AuthTicket.generate_ticket_id() request.find_service(AuthTicketService).add_ticket(userid, ticket_id) + return ticket_id + + def remember(self, cookie: SignedCookieProfile, userid: str, ticket_id): return cookie.get_headers([userid, ticket_id]) def forget(self, cookie: SignedCookieProfile, request: Request): diff --git a/h/security/policy/top_level.py b/h/security/policy/top_level.py index bc007ddf62c..c32f0b9be46 100644 --- a/h/security/policy/top_level.py +++ b/h/security/policy/top_level.py @@ -48,7 +48,7 @@ def get_subpolicy(request): api_authcookie = webob.cookies.SignedCookieProfile( secret=request.registry.settings["h_api_auth_cookie_secret"], salt=request.registry.settings["h_api_auth_cookie_salt"], - cookie_name="h_api_authcookie", + cookie_name="h_api_authcookie.v2", httponly=True, secure=request.scheme == "https", samesite="strict", diff --git a/tests/unit/h/security/policy/_cookie_test.py b/tests/unit/h/security/policy/_cookie_test.py index 0d74e39d5c3..59c30727545 100644 --- a/tests/unit/h/security/policy/_cookie_test.py +++ b/tests/unit/h/security/policy/_cookie_test.py @@ -31,7 +31,9 @@ def test_identity_issues_api_authcookie( pyramid_request.response ) helper.remember.assert_called_once_with( - api_authcookie, pyramid_request, helper.identity.return_value[0].user.userid + api_authcookie, + helper.identity.return_value[0].user.userid, + helper.identity.return_value[1], ) for header in helper.remember.return_value: assert header in pyramid_request.response.headerlist @@ -122,9 +124,10 @@ def remember(cookie, *_args, **_kwargs): headers = cookie_policy.remember(pyramid_request, sentinel.userid, foo="bar") assert not pyramid_request.session + helper.add_ticket.assert_called_once_with(pyramid_request, sentinel.userid) assert helper.remember.call_args_list == [ - call(html_authcookie, pyramid_request, sentinel.userid), - call(api_authcookie, pyramid_request, sentinel.userid), + call(html_authcookie, sentinel.userid, helper.add_ticket.return_value), + call(api_authcookie, sentinel.userid, helper.add_ticket.return_value), ] assert headers == [ sentinel.html_authcookie_header_1, @@ -203,7 +206,7 @@ def html_authcookie(self, pyramid_request): @pytest.fixture def api_authcookie(self, pyramid_request): api_authcookie = SignedCookieProfile( - "secret", "salt", cookie_name="h_api_authcookie" + "secret", "salt", cookie_name="h_api_authcookie.v2" ) # Add the API auth cookie to the request. diff --git a/tests/unit/h/security/policy/helpers_test.py b/tests/unit/h/security/policy/helpers_test.py index 1a953de5550..ce0438bb105 100644 --- a/tests/unit/h/security/policy/helpers_test.py +++ b/tests/unit/h/security/policy/helpers_test.py @@ -54,15 +54,21 @@ def test_identity_when_user_deleted( assert helper.identity(cookie, pyramid_request) == (None, None) - def test_remember( - self, auth_ticket_service, cookie, helper, pyramid_request, AuthTicket - ): - headers = helper.remember(cookie, pyramid_request, "test_userid") + def test_add_ticket(self, helper, auth_ticket_service, pyramid_request, AuthTicket): + ticket_id = helper.add_ticket(pyramid_request, sentinel.userid) AuthTicket.generate_ticket_id.assert_called_once_with() - ticket_id = AuthTicket.generate_ticket_id.return_value - auth_ticket_service.add_ticket.assert_called_once_with("test_userid", ticket_id) - cookie.get_headers.assert_called_once_with(["test_userid", ticket_id]) + auth_ticket_service.add_ticket.assert_called_once_with( + sentinel.userid, AuthTicket.generate_ticket_id.return_value + ) + assert ticket_id == AuthTicket.generate_ticket_id.return_value + + def test_remember(self, cookie, helper): + headers = helper.remember(cookie, sentinel.userid, sentinel.ticket_id) + + cookie.get_headers.assert_called_once_with( + [sentinel.userid, sentinel.ticket_id] + ) assert headers == cookie.get_headers.return_value def test_forget(self, auth_ticket_service, cookie, helper, pyramid_request): diff --git a/tests/unit/h/security/policy/top_level_test.py b/tests/unit/h/security/policy/top_level_test.py index 5c905e7506e..b965b40ccc2 100644 --- a/tests/unit/h/security/policy/top_level_test.py +++ b/tests/unit/h/security/policy/top_level_test.py @@ -82,7 +82,7 @@ def test_api_request( 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", + cookie_name="h_api_authcookie.v2", max_age=5184000, httponly=True, secure=True, @@ -129,7 +129,7 @@ def test_non_api_request( call( secret="test_h_api_auth_cookie_secret", salt="test_h_api_auth_cookie_salt", - cookie_name="h_api_authcookie", + cookie_name="h_api_authcookie.v2", max_age=5184000, httponly=True, secure=True,