From 8caf7e1b24124cfcdac80d21b42b718ac9b5fe17 Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Tue, 7 Nov 2023 01:00:38 +0000
Subject: [PATCH 01/11] fix(backups): do not infinitely retry automatic backup
 if it errors out

---
 selfprivacy_api/backup/__init__.py | 38 +++++++++++++++++++++++++-----
 selfprivacy_api/backup/jobs.py     | 10 ++++++++
 tests/test_graphql/test_backup.py  | 32 +++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py
index aa11f7f..a5fe066 100644
--- a/selfprivacy_api/backup/__init__.py
+++ b/selfprivacy_api/backup/__init__.py
@@ -1,7 +1,8 @@
 """
 This module contains the controller class for backups.
 """
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, timezone
+import time
 import os
 from os import statvfs
 from typing import Callable, List, Optional
@@ -37,6 +38,7 @@ from selfprivacy_api.backup.providers import get_provider
 from selfprivacy_api.backup.storage import Storage
 from selfprivacy_api.backup.jobs import (
     get_backup_job,
+    get_backup_fail,
     add_backup_job,
     get_restore_job,
     add_restore_job,
@@ -292,9 +294,9 @@ class Backups:
     def back_up(
         service: Service, reason: BackupReason = BackupReason.EXPLICIT
     ) -> Snapshot:
-        """The top-level function to back up a service"""
-        folders = service.get_folders()
-        service_name = service.get_id()
+        """The top-level function to back up a service
+        If it fails for any reason at all, it should both mark job as
+        errored and re-raise an error"""
 
         job = get_backup_job(service)
         if job is None:
@@ -302,6 +304,10 @@ class Backups:
         Jobs.update(job, status=JobStatus.RUNNING)
 
         try:
+            if service.can_be_backed_up() is False:
+                raise ValueError("cannot backup a non-backuppable service")
+            folders = service.get_folders()
+            service_name = service.get_id()
             service.pre_backup()
             snapshot = Backups.provider().backupper.start_backup(
                 folders,
@@ -692,23 +698,43 @@ class Backups:
         """Get a timezone-aware time of the last backup of a service"""
         return Storage.get_last_backup_time(service.get_id())
 
+    @staticmethod
+    def get_last_backup_error_time(service: Service) -> Optional[datetime]:
+        """Get a timezone-aware time of the last backup of a service"""
+        job = get_backup_fail(service)
+        if job is not None:
+            datetime_created = job.created_at
+            if datetime_created.tzinfo is None:
+                # assume it is in localtime
+                offset = timedelta(seconds=time.localtime().tm_gmtoff)
+                datetime_created = datetime_created - offset
+                return datetime.combine(datetime_created.date(), datetime_created.time(),timezone.utc)
+            return datetime_created
+        return None
+
     @staticmethod
     def is_time_to_backup_service(service: Service, time: datetime):
         """Returns True if it is time to back up a service"""
         period = Backups.autobackup_period_minutes()
-        service_id = service.get_id()
         if not service.can_be_backed_up():
             return False
         if period is None:
             return False
 
-        last_backup = Storage.get_last_backup_time(service_id)
+        last_error = Backups.get_last_backup_error_time(service)
+
+        if last_error is not None:
+            if time < last_error + timedelta(seconds=AUTOBACKUP_JOB_EXPIRATION_SECONDS):
+                return False
+
+        last_backup = Backups.get_last_backed_up(service)
         if last_backup is None:
             # queue a backup immediately if there are no previous backups
             return True
 
         if time > last_backup + timedelta(minutes=period):
             return True
+
         return False
 
     # Helpers
diff --git a/selfprivacy_api/backup/jobs.py b/selfprivacy_api/backup/jobs.py
index ab4eaca..0aacd86 100644
--- a/selfprivacy_api/backup/jobs.py
+++ b/selfprivacy_api/backup/jobs.py
@@ -80,9 +80,19 @@ def get_job_by_type(type_id: str) -> Optional[Job]:
             return job
 
 
+def get_failed_job_by_type(type_id: str) -> Optional[Job]:
+    for job in Jobs.get_jobs():
+        if job.type_id == type_id and job.status == JobStatus.ERROR:
+            return job
+
+
 def get_backup_job(service: Service) -> Optional[Job]:
     return get_job_by_type(backup_job_type(service))
 
 
+def get_backup_fail(service: Service) -> Optional[Job]:
+    return get_failed_job_by_type(backup_job_type(service))
+
+
 def get_restore_job(service: Service) -> Optional[Job]:
     return get_job_by_type(restore_job_type(service))
diff --git a/tests/test_graphql/test_backup.py b/tests/test_graphql/test_backup.py
index 1903fba..27a2879 100644
--- a/tests/test_graphql/test_backup.py
+++ b/tests/test_graphql/test_backup.py
@@ -14,6 +14,8 @@ import secrets
 
 import tempfile
 
+from selfprivacy_api.utils.huey import huey
+
 import selfprivacy_api.services as services
 from selfprivacy_api.services import Service, get_all_services
 from selfprivacy_api.services.service import ServiceStatus
@@ -119,6 +121,10 @@ def dummy_service(tmpdir, backups, raw_dummy_service) -> Service:
     # register our service
     services.services.append(service)
 
+    # make sure we are in immediate mode because this thing is non pickleable to store on queue.
+    huey.immediate = True
+    assert huey.immediate is True
+
     assert get_service_by_id(service.get_id()) is not None
     yield service
 
@@ -996,6 +1002,32 @@ def test_autobackup_timing(backups, dummy_service):
     assert Backups.is_time_to_backup_service(dummy_service, future)
 
 
+def test_backup_unbackuppable(backups, dummy_service):
+    dummy_service.set_backuppable(False)
+    assert dummy_service.can_be_backed_up() is False
+    with pytest.raises(ValueError):
+        Backups.back_up(dummy_service)
+
+
+def test_failed_autoback_prevents_more_autobackup(backups, dummy_service):
+    backup_period = 13  # minutes
+    now = datetime.now(timezone.utc)
+
+    Backups.set_autobackup_period_minutes(backup_period)
+    assert Backups.is_time_to_backup_service(dummy_service, now)
+
+    # artificially making an errored out backup job
+    dummy_service.set_backuppable(False)
+    with pytest.raises(ValueError):
+        Backups.back_up(dummy_service)
+    dummy_service.set_backuppable(True)
+
+    assert Backups.get_last_backed_up(dummy_service) is None
+    assert Backups.get_last_backup_error_time(dummy_service) is not None
+
+    assert Backups.is_time_to_backup_service(dummy_service, now) is False
+
+
 # Storage
 def test_snapshots_caching(backups, dummy_service):
     Backups.back_up(dummy_service)

From b545a400c3657848bb570aed96d047514dc8c133 Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Fri, 10 Nov 2023 11:47:48 +0000
Subject: [PATCH 02/11] doc(jobs): document that we are tz-naive when storing
 jobs

---
 selfprivacy_api/jobs/__init__.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/selfprivacy_api/jobs/__init__.py b/selfprivacy_api/jobs/__init__.py
index 05b5ab8..7310016 100644
--- a/selfprivacy_api/jobs/__init__.py
+++ b/selfprivacy_api/jobs/__init__.py
@@ -8,8 +8,8 @@ A job is a dictionary with the following keys:
     - name: name of the job
     - description: description of the job
     - status: status of the job
-    - created_at: date of creation of the job
-    - updated_at: date of last update of the job
+    - created_at: date of creation of the job, naive localtime
+    - updated_at: date of last update of the job, naive localtime
     - finished_at: date of finish of the job
     - error: error message if the job failed
     - result: result of the job

From 73a847f28849a181a52353a0eb7346787370af8b Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Fri, 10 Nov 2023 12:19:32 +0000
Subject: [PATCH 03/11] feature(time): timestamp parsers

---
 selfprivacy_api/utils/time.py | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 selfprivacy_api/utils/time.py

diff --git a/selfprivacy_api/utils/time.py b/selfprivacy_api/utils/time.py
new file mode 100644
index 0000000..5eb7e04
--- /dev/null
+++ b/selfprivacy_api/utils/time.py
@@ -0,0 +1,30 @@
+from datetime import datetime, timezone
+
+
+def tzaware_parse_time(iso_timestamp: str) -> datetime:
+    """
+    parse an iso8601 timestamp into timezone-aware datetime
+    assume utc if no timezone in stamp
+    example of timestamp:
+    2023-11-10T12:07:47.868788+00:00
+
+    """
+    dt = datetime.fromisoformat(iso_timestamp)
+    if dt.tzinfo is None:
+        dt = dt.astimezone(timezone.utc)
+    return dt
+
+
+def tzaware_parse_time_strict(iso_timestamp: str) -> datetime:
+    """
+    parse an iso8601 timestamp into timezone-aware datetime
+    raise an error if no timezone in stamp
+    example of timestamp:
+    2023-11-10T12:07:47.868788+00:00
+
+    """
+    dt = datetime.fromisoformat(iso_timestamp)
+    if dt.tzinfo is None:
+        raise ValueError("no timezone in timestamp", iso_timestamp)
+        dt = dt.astimezone(timezone.utc)
+    return dt

From 4d893d56b24dfa9f0bb837d9faa846a9efa214a5 Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Fri, 10 Nov 2023 12:29:25 +0000
Subject: [PATCH 04/11] test(common): add forced utc times for tests

---
 tests/common.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/common.py b/tests/common.py
index 97d0d7a..55b95a6 100644
--- a/tests/common.py
+++ b/tests/common.py
@@ -11,6 +11,10 @@ def five_minutes_into_future_naive():
     return datetime.now() + timedelta(minutes=5)
 
 
+def five_minutes_into_future_naive_utc():
+    return datetime.utcnow() + timedelta(minutes=5)
+
+
 def five_minutes_into_future():
     return datetime.now(timezone.utc) + timedelta(minutes=5)
 
@@ -19,6 +23,10 @@ def five_minutes_into_past_naive():
     return datetime.now() - timedelta(minutes=5)
 
 
+def five_minutes_into_past_naive_utc():
+    return datetime.utcnow() - timedelta(minutes=5)
+
+
 def five_minutes_into_past():
     return datetime.now(timezone.utc) - timedelta(minutes=5)
 

From e78bcca9f2f9978136f4e80e11cb854ac87bad4c Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Fri, 10 Nov 2023 12:49:30 +0000
Subject: [PATCH 05/11] test(auth): forced utc in recovery tests

---
 tests/test_graphql/test_api_recovery.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_graphql/test_api_recovery.py b/tests/test_graphql/test_api_recovery.py
index 19f8a3d..593c50b 100644
--- a/tests/test_graphql/test_api_recovery.py
+++ b/tests/test_graphql/test_api_recovery.py
@@ -10,8 +10,8 @@ from tests.common import (
 )
 
 # Graphql API's output should be timezone-naive
-from tests.common import five_minutes_into_future_naive as five_minutes_into_future
-from tests.common import five_minutes_into_past_naive as five_minutes_into_past
+from tests.common import five_minutes_into_future_naive_utc as five_minutes_into_future
+from tests.common import five_minutes_into_past_naive_utc as five_minutes_into_past
 
 from tests.test_graphql.api_common import (
     assert_empty,

From 8453f62c746251c11a74aa4acd2e4517f35aa415 Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Fri, 10 Nov 2023 13:05:38 +0000
Subject: [PATCH 06/11] refactor(time): more time functions

---
 selfprivacy_api/utils/time.py | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/selfprivacy_api/utils/time.py b/selfprivacy_api/utils/time.py
index 5eb7e04..36871c3 100644
--- a/selfprivacy_api/utils/time.py
+++ b/selfprivacy_api/utils/time.py
@@ -1,6 +1,29 @@
 from datetime import datetime, timezone
 
 
+def ensure_tz_aware(dt: datetime) -> datetime:
+    """
+    returns timezone-aware datetime
+    assumes utc on naive datetime input
+    """
+    if dt.tzinfo is None:
+        dt = dt.astimezone(timezone.utc)
+    return dt
+
+
+def ensure_tz_aware_strict(dt: datetime) -> datetime:
+    """
+    returns timezone-aware datetime
+    raises error if input is a naive datetime
+    """
+    if dt.tzinfo is None:
+        raise ValueError(
+            "no timezone in datetime (tz-aware datetime is required for this operation)",
+            dt,
+        )
+    return dt
+
+
 def tzaware_parse_time(iso_timestamp: str) -> datetime:
     """
     parse an iso8601 timestamp into timezone-aware datetime
@@ -10,8 +33,7 @@ def tzaware_parse_time(iso_timestamp: str) -> datetime:
 
     """
     dt = datetime.fromisoformat(iso_timestamp)
-    if dt.tzinfo is None:
-        dt = dt.astimezone(timezone.utc)
+    dt = ensure_tz_aware(dt)
     return dt
 
 
@@ -24,7 +46,5 @@ def tzaware_parse_time_strict(iso_timestamp: str) -> datetime:
 
     """
     dt = datetime.fromisoformat(iso_timestamp)
-    if dt.tzinfo is None:
-        raise ValueError("no timezone in timestamp", iso_timestamp)
-        dt = dt.astimezone(timezone.utc)
+    dt = ensure_tz_aware_strict(dt)
     return dt

From 8badb9aaaf79fd20ff3311b936bfcf9a10d21766 Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Fri, 10 Nov 2023 13:31:12 +0000
Subject: [PATCH 07/11] refactor(auth): tz_aware expiration comparison

---
 selfprivacy_api/actions/api_tokens.py           | 6 ++++--
 selfprivacy_api/utils/{time.py => timeutils.py} | 0
 2 files changed, 4 insertions(+), 2 deletions(-)
 rename selfprivacy_api/utils/{time.py => timeutils.py} (100%)

diff --git a/selfprivacy_api/actions/api_tokens.py b/selfprivacy_api/actions/api_tokens.py
index 37b7631..3746c57 100644
--- a/selfprivacy_api/actions/api_tokens.py
+++ b/selfprivacy_api/actions/api_tokens.py
@@ -7,6 +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.repositories.tokens.redis_tokens_repository import (
     RedisTokensRepository,
 )
@@ -121,8 +122,9 @@ def get_new_api_recovery_key(
 ) -> str:
     """Get new recovery key"""
     if expiration_date is not None:
-        current_time = datetime.now().timestamp()
-        if expiration_date.timestamp() < current_time:
+        expiration_date = ensure_tz_aware(expiration_date)
+        current_time = datetime.now(timezone.utc)
+        if expiration_date < current_time:
             raise InvalidExpirationDate("Expiration date is in the past")
     if uses_left is not None:
         if uses_left <= 0:
diff --git a/selfprivacy_api/utils/time.py b/selfprivacy_api/utils/timeutils.py
similarity index 100%
rename from selfprivacy_api/utils/time.py
rename to selfprivacy_api/utils/timeutils.py

From dd6f37a17d918e4ea92f3cc0959982c3d7e5c6ed Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Fri, 10 Nov 2023 17:10:01 +0000
Subject: [PATCH 08/11] 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,
     }
 

From 1bbb804919a76d827f621a70f7069e52de12a474 Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Fri, 10 Nov 2023 17:40:52 +0000
Subject: [PATCH 09/11] test(auth): token tests clearer about timezone
 assumptions

---
 selfprivacy_api/models/tokens/new_device_key.py |  4 ++--
 selfprivacy_api/models/tokens/recovery_key.py   |  1 +
 tests/test_models.py                            | 15 +++++++++++----
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/selfprivacy_api/models/tokens/new_device_key.py b/selfprivacy_api/models/tokens/new_device_key.py
index 9fbd23b..241cbd3 100644
--- a/selfprivacy_api/models/tokens/new_device_key.py
+++ b/selfprivacy_api/models/tokens/new_device_key.py
@@ -22,7 +22,7 @@ class NewDeviceKey(BaseModel):
 
     def is_valid(self) -> bool:
         """
-        Check if the recovery key is valid.
+        Check if key is valid.
         """
         if is_past(self.expires_at):
             return False
@@ -30,7 +30,7 @@ class NewDeviceKey(BaseModel):
 
     def as_mnemonic(self) -> str:
         """
-        Get the recovery key as a mnemonic.
+        Get the key as a mnemonic.
         """
         return Mnemonic(language="english").to_mnemonic(bytes.fromhex(self.key))
 
diff --git a/selfprivacy_api/models/tokens/recovery_key.py b/selfprivacy_api/models/tokens/recovery_key.py
index 3b81398..3f52735 100644
--- a/selfprivacy_api/models/tokens/recovery_key.py
+++ b/selfprivacy_api/models/tokens/recovery_key.py
@@ -47,6 +47,7 @@ class RecoveryKey(BaseModel):
     ) -> "RecoveryKey":
         """
         Factory to generate a random token.
+        If passed naive time as expiration, assumes utc
         """
         creation_date = datetime.now(timezone.utc)
         if expiration is not None:
diff --git a/tests/test_models.py b/tests/test_models.py
index 2263e82..f01bb4f 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -1,18 +1,25 @@
 import pytest
-from datetime import datetime, timedelta
+from datetime import datetime, timedelta, timezone
 
 from selfprivacy_api.models.tokens.recovery_key import RecoveryKey
 from selfprivacy_api.models.tokens.new_device_key import NewDeviceKey
 
 
-def test_recovery_key_expired():
-    expiration = datetime.now() - timedelta(minutes=5)
+def test_recovery_key_expired_utcnaive():
+    expiration = datetime.utcnow() - timedelta(minutes=5)
+    key = RecoveryKey.generate(expiration=expiration, uses_left=2)
+    assert not key.is_valid()
+
+
+def test_recovery_key_expired_tzaware():
+    expiration = datetime.now(timezone.utc) - timedelta(minutes=5)
     key = RecoveryKey.generate(expiration=expiration, uses_left=2)
     assert not key.is_valid()
 
 
 def test_new_device_key_expired():
-    expiration = datetime.now() - timedelta(minutes=5)
+    # key is supposed to be tzaware
+    expiration = datetime.now(timezone.utc) - timedelta(minutes=5)
     key = NewDeviceKey.generate()
     key.expires_at = expiration
     assert not key.is_valid()

From e414f3b8fd46be99b2dfe4b0fb750086cb73271a Mon Sep 17 00:00:00 2001
From: Houkime <>
Date: Mon, 13 Nov 2023 09:15:12 -0700
Subject: [PATCH 10/11] fix(auth): fix timezone issues with recovery tokens

---
 selfprivacy_api/utils/timeutils.py      |  4 +++-
 tests/common.py                         |  4 ++++
 tests/test_graphql/api_common.py        | 12 ++++++------
 tests/test_graphql/test_api_recovery.py | 12 ++++++------
 tests/test_rest_endpoints/test_auth.py  |  6 +++---
 5 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/selfprivacy_api/utils/timeutils.py b/selfprivacy_api/utils/timeutils.py
index 36871c3..b6494c6 100644
--- a/selfprivacy_api/utils/timeutils.py
+++ b/selfprivacy_api/utils/timeutils.py
@@ -7,7 +7,9 @@ def ensure_tz_aware(dt: datetime) -> datetime:
     assumes utc on naive datetime input
     """
     if dt.tzinfo is None:
-        dt = dt.astimezone(timezone.utc)
+        # astimezone() is dangerous, it makes an implicit assumption that
+        # the time is localtime
+        dt = dt.replace(tzinfo=timezone.utc)
     return dt
 
 
diff --git a/tests/common.py b/tests/common.py
index c327ae9..5199899 100644
--- a/tests/common.py
+++ b/tests/common.py
@@ -36,6 +36,10 @@ class NearFuture(datetime):
     def now(cls, tz=None):
         return datetime.now(tz) + timedelta(minutes=13)
 
+    @classmethod
+    def utcnow(cls):
+        return datetime.utcnow() + timedelta(minutes=13)
+
 
 def read_json(file_path):
     with open(file_path, "r", encoding="utf-8") as file:
diff --git a/tests/test_graphql/api_common.py b/tests/test_graphql/api_common.py
index bfac767..4e4aec2 100644
--- a/tests/test_graphql/api_common.py
+++ b/tests/test_graphql/api_common.py
@@ -6,16 +6,16 @@ ORIGINAL_DEVICES = TOKENS_FILE_CONTENTS["tokens"]
 
 def assert_ok(response, request):
     data = assert_data(response)
-    data[request]["success"] is True
-    data[request]["message"] is not None
-    data[request]["code"] == 200
+    assert data[request]["success"] is True
+    assert data[request]["message"] is not None
+    assert data[request]["code"] == 200
 
 
 def assert_errorcode(response, request, code):
     data = assert_data(response)
-    data[request]["success"] is False
-    data[request]["message"] is not None
-    data[request]["code"] == code
+    assert data[request]["success"] is False
+    assert data[request]["message"] is not None
+    assert data[request]["code"] == code
 
 
 def assert_empty(response):
diff --git a/tests/test_graphql/test_api_recovery.py b/tests/test_graphql/test_api_recovery.py
index b0155e7..629bac0 100644
--- a/tests/test_graphql/test_api_recovery.py
+++ b/tests/test_graphql/test_api_recovery.py
@@ -177,9 +177,9 @@ def test_graphql_generate_recovery_key_with_expiration_date(
     assert_recovery_recent(status["creationDate"])
 
     # timezone-aware comparison. Should pass regardless of server's tz
-    assert datetime.fromisoformat(
-        status["expirationDate"]
-    ) == expiration_date.astimezone(timezone.utc)
+    assert datetime.fromisoformat(status["expirationDate"]) == expiration_date.replace(
+        tzinfo=timezone.utc
+    )
 
     assert status["usesLeft"] is None
 
@@ -208,9 +208,9 @@ def test_graphql_use_recovery_key_after_expiration(
     assert_recovery_recent(status["creationDate"])
 
     # timezone-aware comparison. Should pass regardless of server's tz
-    assert datetime.fromisoformat(
-        status["expirationDate"]
-    ) == expiration_date.astimezone(timezone.utc)
+    assert datetime.fromisoformat(status["expirationDate"]) == expiration_date.replace(
+        tzinfo=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 8565143..4d0d2ed 100644
--- a/tests/test_rest_endpoints/test_auth.py
+++ b/tests/test_rest_endpoints/test_auth.py
@@ -12,8 +12,8 @@ from tests.common import (
     NearFuture,
     assert_recovery_recent,
 )
-from tests.common import five_minutes_into_future_naive as five_minutes_into_future
-from tests.common import five_minutes_into_past_naive as five_minutes_into_past
+from tests.common import five_minutes_into_future_naive_utc as five_minutes_into_future
+from tests.common import five_minutes_into_past_naive_utc as five_minutes_into_past
 
 DATE_FORMATS = [
     "%Y-%m-%dT%H:%M:%S.%fZ",
@@ -338,7 +338,7 @@ def test_generate_recovery_token_with_expiration_date(
         "exists": True,
         "valid": True,
         "date": time_generated,
-        "expiration": expiration_date.astimezone(timezone.utc).isoformat(),
+        "expiration": expiration_date.replace(tzinfo=timezone.utc).isoformat(),
         "uses_left": None,
     }
 

From c3cec36ad4f331a5397681f414e655f775ed7a34 Mon Sep 17 00:00:00 2001
From: Inex Code <inex.code@selfprivacy.org>
Date: Mon, 13 Nov 2023 19:36:12 +0300
Subject: [PATCH 11/11] style: formatting

---
 selfprivacy_api/backup/__init__.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/selfprivacy_api/backup/__init__.py b/selfprivacy_api/backup/__init__.py
index a5fe066..f575ac0 100644
--- a/selfprivacy_api/backup/__init__.py
+++ b/selfprivacy_api/backup/__init__.py
@@ -708,7 +708,9 @@ class Backups:
                 # assume it is in localtime
                 offset = timedelta(seconds=time.localtime().tm_gmtoff)
                 datetime_created = datetime_created - offset
-                return datetime.combine(datetime_created.date(), datetime_created.time(),timezone.utc)
+                return datetime.combine(
+                    datetime_created.date(), datetime_created.time(), timezone.utc
+                )
             return datetime_created
         return None