diff --git a/h/security/policy/_cookie.py b/h/security/policy/_cookie.py index 24d7d877c26..5f4c53969a4 100644 --- a/h/security/policy/_cookie.py +++ b/h/security/policy/_cookie.py @@ -25,9 +25,6 @@ def identity(self, request): userid, ticket_id = self._get_cookie_value() - if not ticket_id: - return None - user = request.find_service(AuthTicketService).verify_ticket(userid, ticket_id) if (not user) or user.deleted: diff --git a/h/services/auth_ticket.py b/h/services/auth_ticket.py index 4685e287ae0..bdd4f8ec8de 100644 --- a/h/services/auth_ticket.py +++ b/h/services/auth_ticket.py @@ -18,13 +18,12 @@ def __init__(self, session, user_service): self._user_service = user_service self._user = None - def verify_ticket(self, userid: str, ticket_id: str) -> User | None: + def verify_ticket(self, userid: str | None, ticket_id: str | None) -> User | None: """ Return the User object matching the given userid and ticket_id, or None. Verify that there is an unexpired AuthTicket in the DB matching the - given `userid` and `ticket_id` and if so return the User corresponding - User object. + given `userid` and `ticket_id` and if so return the corresponding User. """ if self._user: diff --git a/tests/unit/h/security/policy/_cookie_test.py b/tests/unit/h/security/policy/_cookie_test.py index ae79ca76294..ff2d2dd2ee4 100644 --- a/tests/unit/h/security/policy/_cookie_test.py +++ b/tests/unit/h/security/policy/_cookie_test.py @@ -21,13 +21,6 @@ def test_identity(self, pyramid_request, auth_ticket_service, cookie_policy, use user=auth_ticket_service.verify_ticket.return_value ) - def test_identity_when_no_ticket_in_cookie( - self, cookie, cookie_policy, pyramid_request - ): - cookie.get_value.return_value = None - - assert cookie_policy.identity(pyramid_request) is None - def test_identity_when_user_marked_as_deleted( self, pyramid_request, auth_ticket_service, cookie_policy ): diff --git a/tests/unit/h/services/auth_ticket_test.py b/tests/unit/h/services/auth_ticket_test.py index ac7f245a0ad..5674d849106 100644 --- a/tests/unit/h/services/auth_ticket_test.py +++ b/tests/unit/h/services/auth_ticket_test.py @@ -31,9 +31,19 @@ def test_verify_ticket_short_circuits_if_user_cache_is_set(self, service): service.verify_ticket(sentinel.userid, sentinel.ticket_id) == service._user ) - def test_verify_ticket_returns_None_if_there_is_no_ticket(self, service, user): + @pytest.mark.usefixtures("auth_ticket") + def test_verify_ticket_returns_None_if_theres_no_matching_ticket( + self, service, user + ): assert service.verify_ticket(user.userid, ticket_id="does_not_exist") is None + def test_verify_ticket_when_theres_no_userid(self, service, auth_ticket): + assert service.verify_ticket(None, ticket_id=auth_ticket.id) is None + + @pytest.mark.usefixtures("auth_ticket") + def test_verify_ticket_when_theres_no_ticket_id(self, service, user): + assert service.verify_ticket(user.userid, ticket_id=None) is None + def test_verify_ticket_returns_None_if_the_ticket_has_expired( self, service, auth_ticket ):