fix: auth hardening -- domain exceptions + null user_id safety (OPE-76, OPE-77)#259
Conversation
…6, OPE-77) OPE-76: Domain exceptions replace HTTPException in auth service. - New services/exceptions.py with AuthenticationError hierarchy: TokenExpiredError, InvalidTokenError, TokenMissingClaimError, InvalidCredentialsError, SignupError, SessionError, UserIdRequiredError. - auth.py raises domain exceptions, not HTTPException. - middleware/auth.py catches domain exceptions and translates to HTTP. InvalidTokenError returns None (allows API key fallback). TokenExpiredError/TokenMissingClaimError raise 401 directly. OPE-77: Null user_id safety. - get_repo_or_404 and verify_repo_access accept Optional[str]. - None user_id returns 401 'User ID required' instead of confusing 404. - Affects all API key auth users without user_id. Tests: 5 new tests covering null safety + exception hierarchy. 289 tests pass (284 existing + 5 new). Closes OPE-76, OPE-77
|
@DevanshuNEU is attempting to deploy a commit to the Dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces a custom exception hierarchy for authentication errors and refactors authentication validation layers to raise domain-specific exceptions instead of HTTPExceptions, while adding explicit null-user_id checks to prevent unauthorized access to repository resources. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Middleware as Middleware<br/>_validate_jwt
participant Services as Services<br/>verify_jwt
participant APIKey as API Key Auth<br/>(Fallback)
participant API as API Handler
participant DB as Database
Client->>Middleware: Request with Authorization header
Middleware->>Services: Validate JWT token
alt Token is JWT
Services->>Services: Decode & verify token
alt Token valid
Services-->>Middleware: AuthContext
Middleware->>API: Request authorized
else Token expired
Services-->>Middleware: TokenExpiredError
Middleware-->>Client: HTTP 401
else Missing claim
Services-->>Middleware: TokenMissingClaimError
Middleware-->>Client: HTTP 401
else Invalid token
Services-->>Middleware: InvalidTokenError
Middleware->>APIKey: Return None (fallback)
end
else Not a JWT token
Services-->>Middleware: Return None
Middleware->>APIKey: Try API key authentication
APIKey->>DB: Validate API key
APIKey-->>Middleware: AuthContext (or None)
end
alt AuthContext obtained
Middleware->>API: Proceed with request
API->>DB: Process request
DB-->>API: Response
API-->>Client: HTTP 200
else No valid auth
Middleware-->>Client: HTTP 401
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/middleware/auth.py (1)
194-204:⚠️ Potential issue | 🟠 Major
get_current_userlegacy function will now return 500 instead of 401 on JWT failures.
verify_jwt()no longer raisesHTTPException— it raises domain exceptions (TokenExpiredError,InvalidTokenError, etc.). This legacy function has no exception handling, so domain exceptions will propagate unhandled to FastAPI's default handler, producing a 500 Internal Server Error instead of the expected 401.If this function is still in use, it needs a try/except that translates domain exceptions to
HTTPException(401), similar to_validate_jwt. If it's dead code, consider removing it.Proposed fix (if still in use)
async def get_current_user( credentials: HTTPAuthorizationCredentials = Depends(_bearer_required) ) -> dict: """ [LEGACY] Get current user from JWT token Prefer using require_auth() for new code. """ - from services.auth import get_auth_service - auth_service = get_auth_service() - return auth_service.verify_jwt(credentials.credentials) + try: + from services.auth import get_auth_service + auth_service = get_auth_service() + return auth_service.verify_jwt(credentials.credentials) + except AuthenticationError as e: + raise HTTPException(status_code=401, detail=str(e))#!/bin/bash # Check if get_current_user is still referenced anywhere (excluding its own definition) rg -n "get_current_user" --type=py -g '!backend/middleware/auth.py' -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/middleware/auth.py` around lines 194 - 204, The legacy get_current_user function calls auth_service.verify_jwt but does not catch domain exceptions (e.g., TokenExpiredError, InvalidTokenError), so these bubble up as 500s; update get_current_user to wrap the verify_jwt call in a try/except that catches the domain exceptions and raises fastapi.HTTPException(status_code=401, detail="Invalid or expired token") (mirroring the behavior in _validate_jwt), or if get_current_user is unused, remove it after confirming no references.
🧹 Nitpick comments (4)
backend/services/exceptions.py (1)
1-49: Well-structured exception hierarchy.Clean design that properly decouples business logic from the HTTP layer.
One observation:
UserIdRequiredError(Line 47) is defined here but never raised anywhere in this PR —backend/dependencies.pyraisesHTTPException(status_code=401)directly instead. Consider either usingUserIdRequiredErrorindependencies.py(and catching it in the middleware/route layer) to stay consistent with the domain-exception pattern, or removing it if it's not planned for near-term use.#!/bin/bash # Check if UserIdRequiredError is actually used anywhere in the codebase rg -n "UserIdRequiredError" --type=py -C2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/exceptions.py` around lines 1 - 49, The file defines UserIdRequiredError but dependencies.py currently raises HTTPException(status_code=401) directly; either remove the unused UserIdRequiredError class or replace the direct HTTPException in backend/dependencies.py with raising UserIdRequiredError so the middleware/route layer can translate it consistently—locate the UserIdRequiredError class in services/exceptions.py and the place in backend/dependencies.py where HTTPException(status_code=401) is raised, then choose and apply one of the two fixes and update tests/docs accordingly.backend/tests/test_auth_hardening.py (1)
9-23: Integration test mostly verifies its own mocks.The test patches
verify_repo_accessto raiseHTTPException(401)and then asserts the response is 401. This validates route wiring but doesn't exercise the actual null-guard logic independencies.py. The real logic is covered by the unit tests at Lines 25–41, so this isn't a gap — just something to be aware of if you want to evolve this into a true integration test later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/test_auth_hardening.py` around lines 9 - 23, The test test_search_with_null_user_id_returns_401 is currently asserting a 401 by patching routes.search.verify_repo_access to raise HTTPException, so it only verifies the mock, not the real null-guard in dependencies.py; remove or adjust the patch of routes.search.verify_repo_access (leave routes.search.require_auth mocked to return AuthContext(api_key_name="test-key", user_id=None)) so the actual verify_repo_access implementation in dependencies.py runs and produces the 401 when user_id is None, or alternatively change mock_verify to call through to the real function instead of raising to exercise the real logic. Ensure references remain to routes.search.require_auth, routes.search.verify_repo_access, and dependencies.py when making the change.backend/services/auth.py (2)
13-21: Unused imports:InvalidCredentialsError,SignupError,SessionError.These three exceptions are imported but never raised or referenced in this file.
signup,login,refresh_session, andlogoutstill raiseHTTPExceptiondirectly.Remove the unused imports now, or convert those methods to raise domain exceptions (and catch them at the route/middleware layer) as a follow-up.
Proposed fix
from services.exceptions import ( AuthenticationError, TokenExpiredError, InvalidTokenError, TokenMissingClaimError, - InvalidCredentialsError, - SignupError, - SessionError, )#!/bin/bash # Verify that InvalidCredentialsError, SignupError, SessionError are not referenced in auth.py rg -n "InvalidCredentialsError|SignupError|SessionError" --type=py -g 'backend/services/auth.py'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/auth.py` around lines 13 - 21, The file imports three unused exceptions (InvalidCredentialsError, SignupError, SessionError) — remove these unused imports from the top-level import list in backend/services/auth.py to clean up unused symbols; locate the import block that currently lists AuthenticationError, TokenExpiredError, InvalidTokenError, TokenMissingClaimError, InvalidCredentialsError, SignupError, SessionError and delete only InvalidCredentialsError, SignupError, and SessionError (or alternatively refactor signup, login, refresh_session, logout to raise those domain exceptions and update route/middleware to catch them if you prefer that pattern).
78-84: Consider exception chaining withraise ... fromfor debuggability.The new domain exceptions discard the original PyJWT traceback. Using
raise X from epreserves the cause chain, which is helpful when debugging token issues in logs.Proposed fix
- except jwt.ExpiredSignatureError: - raise TokenExpiredError("Token expired") - except jwt.InvalidAudienceError: - raise InvalidTokenError("Invalid token audience") + except jwt.ExpiredSignatureError as e: + raise TokenExpiredError("Token expired") from e + except jwt.InvalidAudienceError as e: + raise InvalidTokenError("Invalid token audience") from e except jwt.InvalidTokenError as e: logger.debug("JWT decode failed", error=str(e)) - raise InvalidTokenError("Invalid token") + raise InvalidTokenError("Invalid token") from e🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/auth.py` around lines 78 - 84, The custom domain exceptions in backend/services/auth.py currently discard the original PyJWT tracebacks; update the except handlers for jwt.ExpiredSignatureError, jwt.InvalidAudienceError and jwt.InvalidTokenError (the latter currently bound as e) to capture the original exception and re-raise using exception chaining (e.g., raise TokenExpiredError("Token expired") from e and raise InvalidTokenError("Invalid token audience") from e / raise InvalidTokenError("Invalid token") from e) so the original cause is preserved for debuggability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/middleware/auth.py`:
- Around line 194-204: The legacy get_current_user function calls
auth_service.verify_jwt but does not catch domain exceptions (e.g.,
TokenExpiredError, InvalidTokenError), so these bubble up as 500s; update
get_current_user to wrap the verify_jwt call in a try/except that catches the
domain exceptions and raises fastapi.HTTPException(status_code=401,
detail="Invalid or expired token") (mirroring the behavior in _validate_jwt), or
if get_current_user is unused, remove it after confirming no references.
---
Nitpick comments:
In `@backend/services/auth.py`:
- Around line 13-21: The file imports three unused exceptions
(InvalidCredentialsError, SignupError, SessionError) — remove these unused
imports from the top-level import list in backend/services/auth.py to clean up
unused symbols; locate the import block that currently lists
AuthenticationError, TokenExpiredError, InvalidTokenError,
TokenMissingClaimError, InvalidCredentialsError, SignupError, SessionError and
delete only InvalidCredentialsError, SignupError, and SessionError (or
alternatively refactor signup, login, refresh_session, logout to raise those
domain exceptions and update route/middleware to catch them if you prefer that
pattern).
- Around line 78-84: The custom domain exceptions in backend/services/auth.py
currently discard the original PyJWT tracebacks; update the except handlers for
jwt.ExpiredSignatureError, jwt.InvalidAudienceError and jwt.InvalidTokenError
(the latter currently bound as e) to capture the original exception and re-raise
using exception chaining (e.g., raise TokenExpiredError("Token expired") from e
and raise InvalidTokenError("Invalid token audience") from e / raise
InvalidTokenError("Invalid token") from e) so the original cause is preserved
for debuggability.
In `@backend/services/exceptions.py`:
- Around line 1-49: The file defines UserIdRequiredError but dependencies.py
currently raises HTTPException(status_code=401) directly; either remove the
unused UserIdRequiredError class or replace the direct HTTPException in
backend/dependencies.py with raising UserIdRequiredError so the middleware/route
layer can translate it consistently—locate the UserIdRequiredError class in
services/exceptions.py and the place in backend/dependencies.py where
HTTPException(status_code=401) is raised, then choose and apply one of the two
fixes and update tests/docs accordingly.
In `@backend/tests/test_auth_hardening.py`:
- Around line 9-23: The test test_search_with_null_user_id_returns_401 is
currently asserting a 401 by patching routes.search.verify_repo_access to raise
HTTPException, so it only verifies the mock, not the real null-guard in
dependencies.py; remove or adjust the patch of routes.search.verify_repo_access
(leave routes.search.require_auth mocked to return
AuthContext(api_key_name="test-key", user_id=None)) so the actual
verify_repo_access implementation in dependencies.py runs and produces the 401
when user_id is None, or alternatively change mock_verify to call through to the
real function instead of raising to exercise the real logic. Ensure references
remain to routes.search.require_auth, routes.search.verify_repo_access, and
dependencies.py when making the change.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/dependencies.pybackend/middleware/auth.pybackend/services/auth.pybackend/services/exceptions.pybackend/tests/test_auth_hardening.pybackend/tests/test_jwt_local_decode.py
📝 WalkthroughWalkthroughThis PR introduces domain-specific authentication exceptions and hardens null user_id handling. Custom exception classes replace HTTPException-based errors in auth services, middleware adds granular exception handling, and dependency helpers now guard against missing user_id with 401 responses. New tests verify null user_id safety and exception inheritance. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…xception 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.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
backend/dependencies.py (1)
47-57: Consider checkinguser_id is None(vs falsy) to match intent precisely.Line 52 uses
if not user_id:which will also reject"". If “missing user id” truly meansNone, prefer an explicitis Nonecheck (and keep the message stable for clients).Proposed tweak
- if not user_id: + if user_id is None: raise HTTPException(status_code=401, detail="User ID required for this operation")As per coding guidelines, PEP 8 style, max 120 char lines in Python backend.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/dependencies.py` around lines 47 - 57, Replace the falsy check in get_repo_or_404 with an explicit None check: change the conditional to test "if user_id is None:" (rather than "if not user_id:") so empty-string user IDs are not treated as missing; keep the HTTPException message unchanged and leave repo lookup via repo_manager.get_repo_for_user(repo_id, user_id) as-is; ensure the modified line follows PEP8 (≤120 chars) and update the docstring if necessary to reflect that only None denotes a missing user_id.backend/tests/test_auth_hardening.py (1)
47-69: Prefer normal imports over__import__for readability.Line 51’s
__import__('fastapi').HTTPExceptionworks, but is unconventional; a simplefrom fastapi import HTTPException(orimport fastapi) keeps it clearer.Suggested adjustment
- assert not issubclass(TokenExpiredError, __import__('fastapi').HTTPException) + from fastapi import HTTPException + assert not issubclass(TokenExpiredError, HTTPException)As per coding guidelines, PEP 8 style, max 120 char lines in Python backend.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/test_auth_hardening.py` around lines 47 - 69, In test_expired_token_raises_domain_exception replace the dynamic __import__('fastapi').HTTPException usage with an explicit import (e.g., add "from fastapi import HTTPException" at the top of the test or inside the test) and then compare TokenExpiredError against HTTPException; update the test_expired_token_raises_domain_exception function to use HTTPException for clarity and ensure the changed lines respect PEP8 line-length limits.backend/services/auth.py (1)
57-85: Name collision risk:jwt.InvalidTokenErrorvs yourservices.exceptions.InvalidTokenError.In
_verify_local, you catchjwt.InvalidTokenErrorand raise your ownInvalidTokenError. This is correct but easy to misread. Consider aliasing the domain exception on import for clarity.Example refactor
-from services.exceptions import ( +from services.exceptions import ( AuthenticationError, TokenExpiredError, - InvalidTokenError, + InvalidTokenError as AuthInvalidTokenError, TokenMissingClaimError, InvalidCredentialsError, SignupError, SessionError, ) @@ - except jwt.InvalidTokenError as e: + except jwt.InvalidTokenError as e: logger.debug("JWT decode failed", error=str(e)) - raise InvalidTokenError("Invalid token") + raise AuthInvalidTokenError("Invalid token")As per coding guidelines, PEP 8 style, max 120 char lines in Python backend.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/services/auth.py` around lines 57 - 85, Rename or alias the external PyJWT exception at import (e.g., from jwt import InvalidTokenError as PyJWTInvalidTokenError) and update the _verify_local exception handlers to catch PyJWTInvalidTokenError (and other jwt.* exceptions) while still raising your domain InvalidTokenError from services.exceptions; also update the logger.debug call in _verify_local to use a keyword-safe message (e.g., logger.debug("JWT decode failed: %s", str(e))) and wrap long lines to comply with PEP8 max 120 chars.backend/middleware/auth.py (1)
68-95: Don’t silently swallow unexpected JWT verification failures without at least logging.Line 93-94 (
except Exception: return None) can convert genuine server-side issues (e.g., misconfig, supabase outage) into “auth fallback” behavior, which likely becomes a 401 later. That makes incidents harder to detect and can mislead clients.Recommendation: log + rethrow for truly unexpected exceptions (or at minimum log at warning/error and keep fallback only for the specific “non-JWT”/invalid-token cases you intend).
As per coding guidelines, Use async/await for I/O operations in Python backend.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/middleware/auth.py` around lines 68 - 95, The _validate_jwt function currently swallows unexpected exceptions (the bare except Exception: return None), which hides server-side failures and prevents proper observability; update _validate_jwt to be async, await auth_service.verify_jwt (since verify_jwt is I/O), introduce a module-level logger, and in the broad exception path log the full exception with context (e.g., include token or user info redacted) and re-raise an HTTPException (500) or the original exception instead of returning None; keep the specific InvalidTokenError path as the only case that returns None to allow API-key fallback and preserve existing TokenExpiredError/TokenMissingClaimError/AuthenticationError handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/services/auth.py`:
- Around line 13-21: The signup/login/refresh_session/logout functions in
auth.py currently raise HTTPException (mixing HTTP layer into the service);
update these methods to raise the domain exceptions already imported (e.g.,
InvalidCredentialsError, SignupError, SessionError, TokenExpiredError,
InvalidTokenError, TokenMissingClaimError) instead of HTTPException, and ensure
any HTTP-specific mapping is moved to the route layer or middleware so
routes/middleware translate those domain exceptions into
HTTPException/responses; locate and change the raise sites inside signup, login,
refresh_session and logout to throw the appropriate imported domain exception
types and keep the service layer free of FastAPI errors.
- Around line 86-104: The synchronous network call in _verify_via_api is
blocking the event loop; make _verify_via_api async, wrap the blocking call
self.client.auth.get_user in fastapi.concurrency.run_in_threadpool and await it,
preserve the existing checks/errors (raise InvalidTokenError when response.user
is falsy and re-raise AuthenticationError), and chain the original exception
when raising AuthenticationError (raise ... from e); then make verify_jwt async
so it can await _verify_via_api and update any callers (e.g., require_auth) to
await verify_jwt to keep the async context consistent.
---
Nitpick comments:
In `@backend/dependencies.py`:
- Around line 47-57: Replace the falsy check in get_repo_or_404 with an explicit
None check: change the conditional to test "if user_id is None:" (rather than
"if not user_id:") so empty-string user IDs are not treated as missing; keep the
HTTPException message unchanged and leave repo lookup via
repo_manager.get_repo_for_user(repo_id, user_id) as-is; ensure the modified line
follows PEP8 (≤120 chars) and update the docstring if necessary to reflect that
only None denotes a missing user_id.
In `@backend/middleware/auth.py`:
- Around line 68-95: The _validate_jwt function currently swallows unexpected
exceptions (the bare except Exception: return None), which hides server-side
failures and prevents proper observability; update _validate_jwt to be async,
await auth_service.verify_jwt (since verify_jwt is I/O), introduce a
module-level logger, and in the broad exception path log the full exception with
context (e.g., include token or user info redacted) and re-raise an
HTTPException (500) or the original exception instead of returning None; keep
the specific InvalidTokenError path as the only case that returns None to allow
API-key fallback and preserve existing
TokenExpiredError/TokenMissingClaimError/AuthenticationError handling.
In `@backend/services/auth.py`:
- Around line 57-85: Rename or alias the external PyJWT exception at import
(e.g., from jwt import InvalidTokenError as PyJWTInvalidTokenError) and update
the _verify_local exception handlers to catch PyJWTInvalidTokenError (and other
jwt.* exceptions) while still raising your domain InvalidTokenError from
services.exceptions; also update the logger.debug call in _verify_local to use a
keyword-safe message (e.g., logger.debug("JWT decode failed: %s", str(e))) and
wrap long lines to comply with PEP8 max 120 chars.
In `@backend/tests/test_auth_hardening.py`:
- Around line 47-69: In test_expired_token_raises_domain_exception replace the
dynamic __import__('fastapi').HTTPException usage with an explicit import (e.g.,
add "from fastapi import HTTPException" at the top of the test or inside the
test) and then compare TokenExpiredError against HTTPException; update the
test_expired_token_raises_domain_exception function to use HTTPException for
clarity and ensure the changed lines respect PEP8 line-length limits.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/dependencies.pybackend/middleware/auth.pybackend/services/auth.pybackend/services/exceptions.pybackend/tests/test_auth_hardening.pybackend/tests/test_jwt_local_decode.py
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.
Self-audit caught: _verify_local had 'from e' on all 3 except handlers, but _verify_via_api's catch-all was missing it. Original Supabase error traceback was being lost on API fallback failures. 289 tests pass.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
1 participant
What
Two auth improvements in one focused PR.
OPE-76: Domain exceptions
Auth service was tightly coupled to FastAPI by raising
HTTPExceptiondirectly. Services should raise domain exceptions; middleware translates them.Before:
After:
New
services/exceptions.pywith clean hierarchy:AuthenticationError(base)TokenExpiredErrorInvalidTokenErrorTokenMissingClaimErrorInvalidCredentialsErrorSignupErrorSessionErrorUserIdRequiredErrorOPE-77: Null user_id safety
API key users without
user_idwere hittingget_repo_or_404(repo_id, None)which passedNoneto the DB and returned confusing 404s.Now
get_repo_or_404andverify_repo_accessacceptOptional[str]and return 401 when user_id is None with a clear message.Changes
289 tests pass (284 existing + 5 new).
Closes OPE-76, OPE-77
Summary by CodeRabbit
Bug Fixes
Tests