Skip to content

Commit

Permalink
Return both Identity and AuthTicket from AuthTicketCookieHelper.ident…
Browse files Browse the repository at this point in the history
…ity()
  • Loading branch information
seanh committed Sep 10, 2024
1 parent 68ab9f1 commit a2f6e52
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 22 deletions.
2 changes: 1 addition & 1 deletion h/security/policy/_api_cookie.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def handles(request: Request) -> bool:
) in COOKIE_AUTHENTICATABLE_API_REQUESTS

def identity(self, request: Request) -> Identity | None:
identity = self.helper.identity(self.cookie, request)
identity, _ = self.helper.identity(self.cookie, request)

if identity is None:
return identity
Expand Down
2 changes: 1 addition & 1 deletion h/security/policy/_cookie.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, _ = 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,
Expand Down
6 changes: 3 additions & 3 deletions h/security/policy/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ def is_api_request(request) -> bool:
class AuthTicketCookieHelper:
def identity(
self, cookie: SignedCookieProfile, request: Request
) -> Identity | None:
) -> tuple[Identity, AuthTicket] | tuple[None, None]:
userid, ticket_id = self.get_cookie_value(cookie)

ticket = request.find_service(AuthTicketService).verify_ticket(
userid, ticket_id
)

if (not ticket) or ticket.user.deleted:
return None
return (None, None)

return Identity.from_models(user=ticket.user)
return (Identity.from_models(user=ticket.user), ticket)

def remember(self, cookie: SignedCookieProfile, request: Request, userid: str):
ticket_id = AuthTicket.generate_ticket_id()
Expand Down
18 changes: 12 additions & 6 deletions tests/unit/h/security/policy/_api_cookie_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pyramid.csrf import SessionCSRFStoragePolicy
from pyramid.exceptions import BadCSRFOrigin, BadCSRFToken

from h.security.identity import Identity as Identity_
from h.security.policy._api_cookie import APICookiePolicy
from h.security.policy.helpers import AuthTicketCookieHelper

Expand Down Expand Up @@ -31,12 +32,12 @@ def test_identity(self, api_cookie_policy, helper, pyramid_csrf_request):

helper.add_vary_by_cookie.assert_called_once_with(pyramid_csrf_request)
helper.identity.assert_called_once_with(sentinel.cookie, pyramid_csrf_request)
assert identity == helper.identity.return_value
assert identity == helper.identity.return_value[0]

def test_identity_with_no_auth_cookie(
self, api_cookie_policy, helper, pyramid_request
):
helper.identity.return_value = None
helper.identity.return_value = (None, None)

assert api_cookie_policy.identity(pyramid_request) is None

Expand Down Expand Up @@ -64,7 +65,7 @@ def test_authenticated_userid(
helper.add_vary_by_cookie.assert_called_once_with(pyramid_csrf_request)
helper.identity.assert_called_once_with(sentinel.cookie, pyramid_csrf_request)
Identity.authenticated_userid.assert_called_once_with(
helper.identity.return_value
helper.identity.return_value[0]
)
assert authenticated_userid == Identity.authenticated_userid.return_value

Expand All @@ -78,13 +79,18 @@ def test_permits(
helper.add_vary_by_cookie.assert_called_once_with(pyramid_csrf_request)
helper.identity.assert_called_once_with(sentinel.cookie, pyramid_csrf_request)
identity_permits.assert_called_once_with(
helper.identity.return_value, sentinel.context, sentinel.permission
helper.identity.return_value[0], sentinel.context, sentinel.permission
)
assert permits == identity_permits.return_value

@pytest.fixture
def helper(self):
return create_autospec(AuthTicketCookieHelper, instance=True, spec_set=True)
def helper(self, factories):
helper = create_autospec(AuthTicketCookieHelper, instance=True, spec_set=True)
helper.identity.return_value = (
create_autospec(Identity_, instance=True, spec_set=True),
factories.AuthTicket(),
)
return helper

@pytest.fixture
def api_cookie_policy(self, helper):
Expand Down
21 changes: 13 additions & 8 deletions tests/unit/h/security/policy/_cookie_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def test_identity(self, cookie_policy, helper, pyramid_request, html_authcookie)

helper.add_vary_by_cookie.assert_called_once_with(pyramid_request)
helper.identity.assert_called_once_with(html_authcookie, pyramid_request)
assert identity == helper.identity.return_value
assert identity == helper.identity.return_value[0]

def test_identity_issues_api_authcookie(
self, cookie_policy, helper, pyramid_request, api_authcookie
Expand All @@ -31,16 +31,16 @@ def test_identity_issues_api_authcookie(
pyramid_request.response
)
helper.remember.assert_called_once_with(
api_authcookie, pyramid_request, helper.identity.return_value.user.userid
api_authcookie, pyramid_request, helper.identity.return_value[0].user.userid
)
for header in helper.remember.return_value:
assert header in pyramid_request.response.headerlist

@pytest.mark.parametrize(
"identity",
[
None,
Identity_(user=None, auth_client=None),
(None, None),
(Identity_(user=None, auth_client=None), sentinel.auth_ticket),
],
)
def test_identity_doesnt_issue_api_authcookie_if_user_not_authenticated(
Expand Down Expand Up @@ -94,7 +94,7 @@ def test_authenticated_userid(
helper.add_vary_by_cookie.assert_called_once_with(pyramid_request)
helper.identity.assert_called_once_with(html_authcookie, pyramid_request)
Identity.authenticated_userid.assert_called_once_with(
helper.identity.return_value
helper.identity.return_value[0]
)
assert authenticated_userid == Identity.authenticated_userid.return_value

Expand Down Expand Up @@ -190,7 +190,7 @@ def test_permits(
helper.add_vary_by_cookie.assert_called_once_with(pyramid_request)
helper.identity.assert_called_once_with(html_authcookie, pyramid_request)
identity_permits.assert_called_once_with(
helper.identity.return_value, sentinel.context, sentinel.permission
helper.identity.return_value[0], sentinel.context, sentinel.permission
)
assert permits == identity_permits.return_value

Expand All @@ -215,8 +215,13 @@ def api_authcookie(self, pyramid_request):
return api_authcookie.bind(pyramid_request)

@pytest.fixture
def helper(self):
return create_autospec(AuthTicketCookieHelper, instance=True, spec_set=True)
def helper(self, factories):
helper = create_autospec(AuthTicketCookieHelper, instance=True, spec_set=True)
helper.identity.return_value = (
create_autospec(Identity_, instance=True, spec_set=True),
factories.AuthTicket(),
)
return helper

@pytest.fixture
def cookie_policy(self, html_authcookie, api_authcookie, helper):
Expand Down
7 changes: 4 additions & 3 deletions tests/unit/h/security/policy/helpers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class TestAuthTicketCookieHelper:
def test_identity(
self, auth_ticket_service, cookie, helper, pyramid_request, Identity
):
identity = helper.identity(cookie, pyramid_request)
identity, auth_ticket = helper.identity(cookie, pyramid_request)

auth_ticket_service.verify_ticket.assert_called_once_with(
sentinel.userid, sentinel.ticket_id
Expand All @@ -38,20 +38,21 @@ def test_identity(
user=auth_ticket_service.verify_ticket.return_value.user
)
assert identity == Identity.from_models.return_value
assert auth_ticket == auth_ticket_service.verify_ticket.return_value

def test_identity_when_no_user(
self, auth_ticket_service, cookie, helper, pyramid_request
):
auth_ticket_service.verify_ticket.return_value = None

assert helper.identity(cookie, pyramid_request) is None
assert helper.identity(cookie, pyramid_request) == (None, None)

def test_identity_when_user_deleted(
self, auth_ticket_service, cookie, helper, pyramid_request
):
auth_ticket_service.verify_ticket.return_value.user.deleted = True

assert helper.identity(cookie, pyramid_request) is None
assert helper.identity(cookie, pyramid_request) == (None, None)

def test_remember(
self, auth_ticket_service, cookie, helper, pyramid_request, AuthTicket
Expand Down

0 comments on commit a2f6e52

Please sign in to comment.