Skip to content

Commit f4b93cc

Browse files
Fixes naive vs aware datetime comparison crashes on SQLite (#3562)
* Addessed .expires_at naive datetime error Signed-off-by: cafalchio <mcafalchio@gmail.com> * fix(tests): improve datetime normalization test coverage Expand test coverage for the naive vs aware datetime fix: - Add missing None expires_at path tests for PasswordResetToken, UserRole, EmailApiToken - Add naive not-expired and aware expired cross-case tests - Add _lookup_api_token_sync naive datetime tests for auth.py fix - Fix PEP 8 style: use 'is True/False' instead of '== True/False' Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: cafalchio <mcafalchio@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 8cc5cc9 commit f4b93cc

4 files changed

Lines changed: 205 additions & 21 deletions

File tree

mcpgateway/auth.py

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -453,28 +453,31 @@ def _lookup_api_token_sync(token_hash: str) -> Optional[Dict[str, Any]]:
453453
result = db.execute(select(EmailApiToken).where(EmailApiToken.token_hash == token_hash, EmailApiToken.is_active.is_(True)))
454454
api_token = result.scalar_one_or_none()
455455

456-
if api_token:
457-
# Check expiration
458-
if api_token.expires_at and api_token.expires_at < datetime.now(timezone.utc):
456+
if not api_token:
457+
return None
458+
459+
# Check expiration
460+
if api_token.expires_at:
461+
expires_at = api_token.expires_at.replace(tzinfo=timezone.utc) if api_token.expires_at.tzinfo is None else api_token.expires_at
462+
if utc_now() > expires_at:
459463
return {"expired": True}
460464

461-
# Check revocation
462-
# First-Party
463-
from mcpgateway.db import TokenRevocation # pylint: disable=import-outside-toplevel
465+
# Check revocation
466+
# First-Party
467+
from mcpgateway.db import TokenRevocation # pylint: disable=import-outside-toplevel
464468

465-
revoke_result = db.execute(select(TokenRevocation).where(TokenRevocation.jti == api_token.jti))
466-
if revoke_result.scalar_one_or_none() is not None:
467-
return {"revoked": True}
469+
revoke_result = db.execute(select(TokenRevocation).where(TokenRevocation.jti == api_token.jti))
470+
if revoke_result.scalar_one_or_none() is not None:
471+
return {"revoked": True}
468472

469-
# Update last_used timestamp
470-
api_token.last_used = utc_now()
471-
db.commit()
473+
# Update last_used timestamp
474+
api_token.last_used = utc_now()
475+
db.commit()
472476

473-
return {
474-
"user_email": api_token.user_email,
475-
"jti": api_token.jti,
476-
}
477-
return None
477+
return {
478+
"user_email": api_token.user_email,
479+
"jti": api_token.jti,
480+
}
478481

479482

480483
def _get_sync_redis_client():

mcpgateway/db.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -989,9 +989,13 @@ def is_expired(self) -> bool:
989989
Returns:
990990
True if assignment has expired, False otherwise
991991
"""
992-
if not self.expires_at:
992+
if self.expires_at is None:
993993
return False
994-
return utc_now() > self.expires_at
994+
expires_at = self.expires_at
995+
if expires_at.tzinfo is None:
996+
expires_at = expires_at.replace(tzinfo=timezone.utc)
997+
998+
return utc_now() > expires_at
995999

9961000

9971001
class PermissionAuditLog(Base):
@@ -1591,7 +1595,13 @@ def is_expired(self) -> bool:
15911595
Returns:
15921596
bool: True when `expires_at` is in the past.
15931597
"""
1594-
return self.expires_at <= utc_now()
1598+
if self.expires_at is None:
1599+
return False
1600+
expires_at = self.expires_at
1601+
if expires_at.tzinfo is None:
1602+
expires_at = expires_at.replace(tzinfo=timezone.utc)
1603+
1604+
return expires_at <= utc_now()
15951605

15961606
def is_used(self) -> bool:
15971607
"""Return whether the reset token was already consumed.
@@ -5188,7 +5198,10 @@ def is_expired(self) -> bool:
51885198
"""
51895199
if not self.expires_at:
51905200
return False
5191-
return utc_now() > self.expires_at
5201+
expires_at = self.expires_at
5202+
if expires_at.tzinfo is None:
5203+
expires_at = expires_at.replace(tzinfo=timezone.utc)
5204+
return utc_now() > expires_at
51925205

51935206
def is_valid(self) -> bool:
51945207
"""Check if token is valid (active and not expired).

tests/unit/mcpgateway/test_auth_helpers.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,35 @@ def test_lookup_api_token_sync_expired(monkeypatch):
272272
assert auth._lookup_api_token_sync("hash") == {"expired": True}
273273

274274

275+
def test_lookup_api_token_sync_expired_naive_datetime(monkeypatch):
276+
"""Naive datetime from SQLite is correctly detected as expired."""
277+
expired_token = SimpleNamespace(
278+
expires_at=datetime(2026, 3, 8, 12, 0, 0), # naive (no tzinfo)
279+
jti="jti-1",
280+
user_email="user@example.com",
281+
last_used=None,
282+
)
283+
session = DummySession(results=[expired_token])
284+
monkeypatch.setattr(auth, "fresh_db_session", lambda: _session_ctx(session))
285+
monkeypatch.setattr("mcpgateway.db.utc_now", lambda: datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc))
286+
assert auth._lookup_api_token_sync("hash") == {"expired": True}
287+
288+
289+
def test_lookup_api_token_sync_not_expired_naive_datetime(monkeypatch):
290+
"""Naive datetime from SQLite in the future is correctly detected as not expired."""
291+
active_token = SimpleNamespace(
292+
expires_at=datetime(2026, 3, 10, 12, 0, 0), # naive, in the future
293+
jti="jti-1",
294+
user_email="user@example.com",
295+
last_used=None,
296+
)
297+
session = DummySession(results=[active_token, None])
298+
monkeypatch.setattr(auth, "fresh_db_session", lambda: _session_ctx(session))
299+
monkeypatch.setattr("mcpgateway.db.utc_now", lambda: datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc))
300+
result = auth._lookup_api_token_sync("hash")
301+
assert result["user_email"] == "user@example.com"
302+
303+
275304
def test_lookup_api_token_sync_revoked(monkeypatch):
276305
api_token = SimpleNamespace(
277306
expires_at=None,

tests/unit/mcpgateway/test_db.py

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2851,3 +2851,142 @@ def __init__(self, name: str):
28512851
team_with_slug.slug = "workspace-alice-example-com"
28522852
db.set_email_team_slug(None, None, team_with_slug)
28532853
assert team_with_slug.slug == "workspace-alice-example-com"
2854+
2855+
2856+
def test_password_reset_is_expired_naive_expired():
2857+
"""Naive datetime in the past is detected as expired."""
2858+
with patch("mcpgateway.db.utc_now", return_value=datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc)):
2859+
token = db.PasswordResetToken(
2860+
user_email="user@example.com",
2861+
token_hash="a" * 64,
2862+
expires_at=datetime(2026, 3, 8, 12, 0, 0),
2863+
)
2864+
assert token.is_expired() is True
2865+
2866+
2867+
def test_password_reset_is_expired_naive_not_expired():
2868+
"""Naive datetime in the future is detected as not expired."""
2869+
with patch("mcpgateway.db.utc_now", return_value=datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc)):
2870+
token = db.PasswordResetToken(
2871+
user_email="user@example.com",
2872+
token_hash="a" * 64,
2873+
expires_at=datetime(2026, 3, 10, 12, 0, 0),
2874+
)
2875+
assert token.is_expired() is False
2876+
2877+
2878+
def test_password_reset_is_expired_aware():
2879+
"""Aware datetime comparison works without normalization."""
2880+
with patch("mcpgateway.db.utc_now", return_value=datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc)):
2881+
token = db.PasswordResetToken(
2882+
user_email="user@example.com",
2883+
token_hash="a" * 64,
2884+
expires_at=datetime(2026, 3, 8, 12, 0, 0, tzinfo=timezone.utc),
2885+
)
2886+
assert token.is_expired() is True
2887+
2888+
2889+
def test_password_reset_is_expired_none():
2890+
"""None expires_at is treated as not expired."""
2891+
token = db.PasswordResetToken(
2892+
user_email="user@example.com",
2893+
token_hash="a" * 64,
2894+
expires_at=None,
2895+
)
2896+
assert token.is_expired() is False
2897+
2898+
2899+
def test_user_role_is_expired_naive_expired():
2900+
"""Naive datetime in the past is detected as expired."""
2901+
with patch("mcpgateway.db.utc_now", return_value=datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc)):
2902+
user_role = db.UserRole(
2903+
user_email="user@example.com",
2904+
role_id="role_id",
2905+
scope="scope",
2906+
granted_by="user1@example.com",
2907+
granted_at=datetime(2026, 3, 7, 12, 0, 0),
2908+
expires_at=datetime(2026, 3, 8, 12, 0, 0),
2909+
)
2910+
assert user_role.is_expired() is True
2911+
2912+
2913+
def test_user_role_is_expired_aware_not_expired():
2914+
"""Aware datetime in the future is detected as not expired."""
2915+
with patch("mcpgateway.db.utc_now", return_value=datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc)):
2916+
user_role = db.UserRole(
2917+
user_email="user@example.com",
2918+
role_id="role_id",
2919+
scope="scope",
2920+
granted_by="user1@example.com",
2921+
granted_at=datetime(2026, 3, 7, 12, 0, 0),
2922+
expires_at=datetime(2026, 3, 10, 12, 0, 0, tzinfo=timezone.utc),
2923+
)
2924+
assert user_role.is_expired() is False
2925+
2926+
2927+
def test_user_role_is_expired_none():
2928+
"""None expires_at means role never expires."""
2929+
user_role = db.UserRole(
2930+
user_email="user@example.com",
2931+
role_id="role_id",
2932+
scope="scope",
2933+
granted_by="user1@example.com",
2934+
granted_at=datetime(2026, 3, 7, 12, 0, 0),
2935+
expires_at=None,
2936+
)
2937+
assert user_role.is_expired() is False
2938+
2939+
2940+
def test_email_api_token_is_expired_naive_expired():
2941+
"""Naive datetime in the past is detected as expired."""
2942+
with patch("mcpgateway.db.utc_now", return_value=datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc)):
2943+
token = db.EmailApiToken(
2944+
user_email="alice@example.com",
2945+
name="Production API Access",
2946+
server_id="prod-server-123",
2947+
resource_scopes=["tools.read", "resources.read"],
2948+
description="Read-only access to production tools",
2949+
expires_at=datetime(2026, 3, 8, 12, 0, 0),
2950+
)
2951+
assert token.is_expired() is True
2952+
2953+
2954+
def test_email_api_token_is_expired_naive_not_expired():
2955+
"""Naive datetime in the future is detected as not expired."""
2956+
with patch("mcpgateway.db.utc_now", return_value=datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc)):
2957+
token = db.EmailApiToken(
2958+
user_email="alice@example.com",
2959+
name="Production API Access",
2960+
server_id="prod-server-123",
2961+
resource_scopes=["tools.read", "resources.read"],
2962+
description="Read-only access to production tools",
2963+
expires_at=datetime(2026, 3, 13, 12, 0, 0),
2964+
)
2965+
assert token.is_expired() is False
2966+
2967+
2968+
def test_email_api_token_is_expired_aware_not_expired():
2969+
"""Aware datetime in the future is detected as not expired."""
2970+
with patch("mcpgateway.db.utc_now", return_value=datetime(2026, 3, 9, 12, 0, 0, tzinfo=timezone.utc)):
2971+
token = db.EmailApiToken(
2972+
user_email="alice@example.com",
2973+
name="Production API Access",
2974+
server_id="prod-server-123",
2975+
resource_scopes=["tools.read", "resources.read"],
2976+
description="Read-only access to production tools",
2977+
expires_at=datetime(2026, 3, 10, 12, 0, 0, tzinfo=timezone.utc),
2978+
)
2979+
assert token.is_expired() is False
2980+
2981+
2982+
def test_email_api_token_is_expired_none():
2983+
"""None expires_at means token never expires."""
2984+
token = db.EmailApiToken(
2985+
user_email="alice@example.com",
2986+
name="Production API Access",
2987+
server_id="prod-server-123",
2988+
resource_scopes=["tools.read", "resources.read"],
2989+
description="Read-only access to production tools",
2990+
expires_at=None,
2991+
)
2992+
assert token.is_expired() is False

0 commit comments

Comments
 (0)