Skip to content

Commit 9e6d2c1

Browse files
committed
fix: review findings -- jwt alias, explicit None check, test cleanup
1. 'import jwt as pyjwt' to avoid name collision with our services.exceptions.InvalidTokenError. Now pyjwt.InvalidTokenError vs InvalidTokenError is unambiguous. 2. 'if user_id is None' instead of 'if not user_id' in dependencies.py. Empty string user_ids (unlikely but possible) no longer rejected. 3. Replaced __import__('fastapi').HTTPException hack in test with proper import. Skipped: signup/login HTTPException migration (scope creep, separate PR), async _verify_via_api (6th time raised), _validate_jwt bare except (intentional for API key fallback). 289 tests pass.
1 parent 3c8003a commit 9e6d2c1

3 files changed

Lines changed: 9 additions & 8 deletions

File tree

backend/dependencies.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def get_repo_or_404(repo_id: str, user_id: Optional[str]) -> dict:
4949
Get repository with ownership verification.
5050
Raises 401 if user_id is None, 404 if not found or user doesn't own it.
5151
"""
52-
if not user_id:
52+
if user_id is None:
5353
raise HTTPException(status_code=401, detail="User ID required for this operation")
5454
repo = repo_manager.get_repo_for_user(repo_id, user_id)
5555
if not repo:
@@ -62,7 +62,7 @@ def verify_repo_access(repo_id: str, user_id: Optional[str]) -> None:
6262
Verify user has access to repository.
6363
Raises 401 if user_id is None, 404 if no access.
6464
"""
65-
if not user_id:
65+
if user_id is None:
6666
raise HTTPException(status_code=401, detail="User ID required for this operation")
6767
if not repo_manager.verify_ownership(repo_id, user_id):
6868
raise HTTPException(status_code=404, detail="Repository not found")

backend/services/auth.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from fastapi import HTTPException, status
66
from typing import Optional, Dict, Any
77
import os
8-
import jwt
8+
import jwt as pyjwt
99
from datetime import datetime
1010
from supabase import create_client, Client
1111

@@ -54,7 +54,7 @@ def verify_jwt(self, token: str) -> Dict[str, Any]:
5454
def _verify_local(self, token: str) -> Dict[str, Any]:
5555
"""Decode and verify JWT locally with HS256 secret."""
5656
try:
57-
payload = jwt.decode(
57+
payload = pyjwt.decode(
5858
token,
5959
self.jwt_secret,
6060
algorithms=["HS256"],
@@ -72,11 +72,11 @@ def _verify_local(self, token: str) -> Dict[str, Any]:
7272
"metadata": payload.get("user_metadata") or {},
7373
}
7474

75-
except jwt.ExpiredSignatureError as e:
75+
except pyjwt.ExpiredSignatureError as e:
7676
raise TokenExpiredError("Token expired") from e
77-
except jwt.InvalidAudienceError as e:
77+
except pyjwt.InvalidAudienceError as e:
7878
raise InvalidTokenError("Invalid token audience") from e
79-
except jwt.InvalidTokenError as e:
79+
except pyjwt.InvalidTokenError as e:
8080
logger.debug("JWT decode failed", error=str(e))
8181
raise InvalidTokenError("Invalid token") from e
8282

backend/tests/test_auth_hardening.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Tests for auth hardening -- domain exceptions + null safety (OPE-76, OPE-77)."""
22
import pytest
33
from unittest.mock import patch, MagicMock
4+
from fastapi import HTTPException
45

56

67
class TestNullUserIdSafety:
@@ -55,7 +56,7 @@ def test_expired_token_raises_domain_exception(self):
5556
"""Auth service should raise TokenExpiredError, not HTTPException."""
5657
from services.exceptions import TokenExpiredError
5758
assert issubclass(TokenExpiredError, Exception)
58-
assert not issubclass(TokenExpiredError, __import__('fastapi').HTTPException)
59+
assert not issubclass(TokenExpiredError, HTTPException)
5960

6061
def test_exception_hierarchy(self):
6162
"""All auth exceptions inherit from AuthenticationError."""

0 commit comments

Comments
 (0)