Skip to content

Commit 3c8003a

Browse files
committed
fix: review findings -- get_current_user catches domain exceptions, exception chaining, test exercises real code
1. get_current_user (legacy, used by logout + get_user_info routes) now catches domain exceptions. Was letting TokenExpiredError etc bubble up as 500s after our domain exception migration. 2. Exception chaining: jwt.ExpiredSignatureError etc now raise with 'from e' so original PyJWT traceback is preserved for debugging. 3. Removed unused imports (InvalidCredentialsError, SignupError, SessionError) from auth.py. Will re-add when signup/login are migrated to domain exceptions. 4. test_search_with_null_user_id: now exercises the REAL null guard in dependencies.py instead of mocking verify_repo_access to return the answer. Overrides require_auth to return AuthContext with user_id=None, lets real verify_repo_access catch it. 289 tests pass.
1 parent 25edad1 commit 3c8003a

3 files changed

Lines changed: 27 additions & 18 deletions

File tree

backend/middleware/auth.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,12 @@ async def get_current_user(
201201
"""
202202
from services.auth import get_auth_service
203203
auth_service = get_auth_service()
204-
return auth_service.verify_jwt(credentials.credentials)
204+
try:
205+
return auth_service.verify_jwt(credentials.credentials)
206+
except (TokenExpiredError, TokenMissingClaimError) as e:
207+
raise HTTPException(status_code=401, detail=str(e))
208+
except AuthenticationError:
209+
raise HTTPException(status_code=401, detail="Invalid or expired token")
205210

206211

207212
async def get_optional_user(

backend/services/auth.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
TokenExpiredError,
1616
InvalidTokenError,
1717
TokenMissingClaimError,
18-
InvalidCredentialsError,
19-
SignupError,
20-
SessionError,
2118
)
2219

2320

@@ -75,13 +72,13 @@ def _verify_local(self, token: str) -> Dict[str, Any]:
7572
"metadata": payload.get("user_metadata") or {},
7673
}
7774

78-
except jwt.ExpiredSignatureError:
79-
raise TokenExpiredError("Token expired")
80-
except jwt.InvalidAudienceError:
81-
raise InvalidTokenError("Invalid token audience")
75+
except jwt.ExpiredSignatureError as e:
76+
raise TokenExpiredError("Token expired") from e
77+
except jwt.InvalidAudienceError as e:
78+
raise InvalidTokenError("Invalid token audience") from e
8279
except jwt.InvalidTokenError as e:
8380
logger.debug("JWT decode failed", error=str(e))
84-
raise InvalidTokenError("Invalid token")
81+
raise InvalidTokenError("Invalid token") from e
8582

8683
def _verify_via_api(self, token: str) -> Dict[str, Any]:
8784
"""Fallback: verify via Supabase API call (requires network)."""

backend/tests/test_auth_hardening.py

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,27 @@ class TestNullUserIdSafety:
77
"""API key users (no user_id) get 401, not confusing 404."""
88

99
def test_search_with_null_user_id_returns_401(self, client, valid_headers):
10-
"""Search should reject None user_id, not pretend repo doesn't exist."""
11-
with patch("routes.search.require_auth") as mock_auth, \
12-
patch("routes.search.verify_repo_access") as mock_verify:
13-
from middleware.auth import AuthContext
14-
mock_auth.return_value = AuthContext(api_key_name="test-key", user_id=None)
15-
# verify_repo_access should raise 401 when user_id is None
16-
from fastapi import HTTPException
17-
mock_verify.side_effect = HTTPException(status_code=401, detail="User ID required")
10+
"""Search should reject None user_id via the real verify_repo_access null guard."""
11+
from fastapi.testclient import TestClient
12+
from main import app
13+
from middleware.auth import require_auth, AuthContext
14+
15+
# Mock auth to return a context with no user_id (API key user)
16+
async def mock_auth_no_user():
17+
return AuthContext(api_key_name="test-key", user_id=None)
18+
19+
app.dependency_overrides[require_auth] = mock_auth_no_user
20+
try:
1821
resp = client.post(
1922
"/api/v1/search",
2023
json={"query": "auth", "repo_id": "test"},
2124
headers=valid_headers,
2225
)
23-
assert resp.status_code == 401
26+
# verify_repo_access in dependencies.py should catch None user_id
27+
assert resp.status_code == 401
28+
assert "User ID required" in resp.json()["detail"]
29+
finally:
30+
app.dependency_overrides.pop(require_auth, None)
2431

2532
def test_get_repo_or_404_rejects_none_user_id(self):
2633
"""get_repo_or_404 should raise 401 when user_id is None."""

0 commit comments

Comments
 (0)