From dd6f37a17d918e4ea92f3cc0959982c3d7e5c6ed Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Fri, 10 Nov 2023 17:10:01 +0000 Subject: [PATCH] feature(auth): tz_aware recovery --- selfprivacy_api/actions/api_tokens.py | 14 +++++++---- .../graphql/queries/api_queries.py | 2 +- tests/common.py | 7 +++--- tests/test_graphql/test_api_recovery.py | 24 +++++++++++++++---- tests/test_rest_endpoints/test_auth.py | 3 ++- 5 files changed, 36 insertions(+), 14 deletions(-) diff --git a/selfprivacy_api/actions/api_tokens.py b/selfprivacy_api/actions/api_tokens.py index 3746c57..e93491f 100644 --- a/selfprivacy_api/actions/api_tokens.py +++ b/selfprivacy_api/actions/api_tokens.py @@ -7,7 +7,7 @@ from typing import Optional from pydantic import BaseModel from mnemonic import Mnemonic -from selfprivacy_api.utils.timeutils import ensure_tz_aware +from selfprivacy_api.utils.timeutils import ensure_tz_aware, ensure_tz_aware_strict from selfprivacy_api.repositories.tokens.redis_tokens_repository import ( RedisTokensRepository, ) @@ -95,16 +95,22 @@ class RecoveryTokenStatus(BaseModel): def get_api_recovery_token_status() -> RecoveryTokenStatus: - """Get the recovery token status""" + """Get the recovery token status, timezone-aware""" token = TOKEN_REPO.get_recovery_key() if token is None: return RecoveryTokenStatus(exists=False, valid=False) is_valid = TOKEN_REPO.is_recovery_key_valid() + + # New tokens are tz-aware, but older ones might not be + expiry_date = token.expires_at + if expiry_date is not None: + expiry_date = ensure_tz_aware_strict(expiry_date) + return RecoveryTokenStatus( exists=True, valid=is_valid, - date=_naive(token.created_at), - expiration=_naive(token.expires_at), + date=ensure_tz_aware_strict(token.created_at), + expiration=expiry_date, uses_left=token.uses_left, ) diff --git a/selfprivacy_api/graphql/queries/api_queries.py b/selfprivacy_api/graphql/queries/api_queries.py index cf56231..7052ded 100644 --- a/selfprivacy_api/graphql/queries/api_queries.py +++ b/selfprivacy_api/graphql/queries/api_queries.py @@ -38,7 +38,7 @@ class ApiRecoveryKeyStatus: def get_recovery_key_status() -> ApiRecoveryKeyStatus: - """Get recovery key status""" + """Get recovery key status, times are timezone-aware""" status = get_api_recovery_token_status() if status is None or not status.exists: return ApiRecoveryKeyStatus( diff --git a/tests/common.py b/tests/common.py index 55b95a6..c327ae9 100644 --- a/tests/common.py +++ b/tests/common.py @@ -67,8 +67,7 @@ def mnemonic_to_hex(mnemonic): return Mnemonic(language="english").to_entropy(mnemonic).hex() -def assert_recovery_recent(time_generated): - assert ( - datetime.strptime(time_generated, "%Y-%m-%dT%H:%M:%S.%f") - timedelta(seconds=5) - < datetime.now() +def assert_recovery_recent(time_generated: str): + assert datetime.fromisoformat(time_generated) - timedelta(seconds=5) < datetime.now( + timezone.utc ) diff --git a/tests/test_graphql/test_api_recovery.py b/tests/test_graphql/test_api_recovery.py index 593c50b..b0155e7 100644 --- a/tests/test_graphql/test_api_recovery.py +++ b/tests/test_graphql/test_api_recovery.py @@ -2,6 +2,10 @@ # pylint: disable=unused-argument # pylint: disable=missing-function-docstring +import pytest + +from datetime import datetime, timezone + from tests.common import ( generate_api_query, assert_recovery_recent, @@ -11,6 +15,7 @@ from tests.common import ( # Graphql API's output should be timezone-naive from tests.common import five_minutes_into_future_naive_utc as five_minutes_into_future +from tests.common import five_minutes_into_future as five_minutes_into_future_tz from tests.common import five_minutes_into_past_naive_utc as five_minutes_into_past from tests.test_graphql.api_common import ( @@ -158,17 +163,24 @@ def test_graphql_generate_recovery_key(client, authorized_client, tokens_file): graphql_use_recovery_key(client, key, "new_test_token2") +@pytest.mark.parametrize( + "expiration_date", [five_minutes_into_future(), five_minutes_into_future_tz()] +) def test_graphql_generate_recovery_key_with_expiration_date( - client, authorized_client, tokens_file + client, authorized_client, tokens_file, expiration_date: datetime ): - expiration_date = five_minutes_into_future() key = graphql_make_new_recovery_key(authorized_client, expires_at=expiration_date) status = graphql_recovery_status(authorized_client) assert status["exists"] is True assert status["valid"] is True assert_recovery_recent(status["creationDate"]) - assert status["expirationDate"] == expiration_date.isoformat() + + # timezone-aware comparison. Should pass regardless of server's tz + assert datetime.fromisoformat( + status["expirationDate"] + ) == expiration_date.astimezone(timezone.utc) + assert status["usesLeft"] is None graphql_use_recovery_key(client, key, "new_test_token") @@ -194,7 +206,11 @@ def test_graphql_use_recovery_key_after_expiration( assert status["exists"] is True assert status["valid"] is False assert_recovery_recent(status["creationDate"]) - assert status["expirationDate"] == expiration_date.isoformat() + + # timezone-aware comparison. Should pass regardless of server's tz + assert datetime.fromisoformat( + status["expirationDate"] + ) == expiration_date.astimezone(timezone.utc) assert status["usesLeft"] is None diff --git a/tests/test_rest_endpoints/test_auth.py b/tests/test_rest_endpoints/test_auth.py index d62fa18..8565143 100644 --- a/tests/test_rest_endpoints/test_auth.py +++ b/tests/test_rest_endpoints/test_auth.py @@ -2,6 +2,7 @@ # pylint: disable=unused-argument # pylint: disable=missing-function-docstring import datetime +from datetime import timezone import pytest from tests.conftest import TOKENS_FILE_CONTENTS @@ -337,7 +338,7 @@ def test_generate_recovery_token_with_expiration_date( "exists": True, "valid": True, "date": time_generated, - "expiration": expiration_date.isoformat(), + "expiration": expiration_date.astimezone(timezone.utc).isoformat(), "uses_left": None, }