routerの作成#8
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 53 seconds. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughウォークスルーFastAPIアプリケーションの構造を再編成し、環境設定、JWT認証、依存性注入を導入しました。6つの新しいルーターモジュール（ユーザー、トリップ、トリップメンバー、候補スポット、反応、イテラリティ）を追加し、Supabaseおよび Google Places API統合のための新しい環境変数を設定しました。 変更内容
推定コードレビュー時間🎯 3 (Moderate) | ⏱️ ~25 minutes 関連の可能性があるPR
詩
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request establishes the core backend architecture by introducing environment configuration management, JWT-based authentication, and a modular router structure for users, trips, and itineraries. The review feedback highlights a few areas for improvement: moving inline imports to the top of the file to comply with PEP 8, renaming a variable in the candidates router to prevent shadowing the status module, and addressing an unused path parameter in the trip member joining endpoint.
| import uuid | ||
| from fastapi import APIRouter, Depends, Query, status |
There was a problem hiding this comment.
PEP 8に基づき、import 文はファイルの先頭にまとめて配置してください。下部の関数内で math が使用されているため、ここでインポートを追加します。
| import uuid | |
| from fastapi import APIRouter, Depends, Query, status | |
| import uuid | |
| import math | |
| from fastapi import APIRouter, Depends, Query, status |
| status: str | None = Query(default=None), | ||
| current_user_id: uuid.UUID = Depends(get_current_user_id), | ||
| usecase: CandidateSpotUsecase = Depends(get_candidate_spot_usecase), | ||
| ): | ||
| items = await usecase.get_list(trip_id, current_user_id, status) |
There was a problem hiding this comment.
引数名 status は、 fastapi からインポートされた status モジュールをシャドウイングしています。これにより、関数内で status.HTTP_... などの定数にアクセスできなくなる可能性があります。 candidate_status などに名称を変更し、 alias="status" を使用してAPIのインターフェースを維持することを推奨します。
| status: str | None = Query(default=None), | |
| current_user_id: uuid.UUID = Depends(get_current_user_id), | |
| usecase: CandidateSpotUsecase = Depends(get_candidate_spot_usecase), | |
| ): | |
| items = await usecase.get_list(trip_id, current_user_id, status) | |
| candidate_status: str | None = Query(default=None, alias="status"), | |
| current_user_id: uuid.UUID = Depends(get_current_user_id), | |
| usecase: CandidateSpotUsecase = Depends(get_candidate_spot_usecase), | |
| ): | |
| items = await usecase.get_list(trip_id, current_user_id, candidate_status) |
|
|
||
| @router.post("/{trip_id}/join", response_model=TripMemberResponse, status_code=status.HTTP_201_CREATED) | ||
| async def join_trip( | ||
| trip_id: uuid.UUID, |
There was a problem hiding this comment.
パスパラメータ trip_id が関数内で使用されていません。招待コードによる参加処理であれば、パスから {trip_id} を削除するか、提供された trip_id が招待コードに対応する旅行と一致するかを検証するロジックを追加することを検討してください。
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/config/jwt.py`:
- Around line 8-23: Remove the insecure options={"verify_aud": False} from
jwt.decode in get_current_user_id and instead pass
audience=settings.SUPABASE_JWT_AUDIENCE and issuer=settings.SUPABASE_JWT_ISSUER
to enable aud/iss checks (keep using settings.SUPABASE_JWT_SECRET and
algorithms=["HS256"]); change _bearer = HTTPBearer() to _bearer =
HTTPBearer(auto_error=False) and add a guard in get_current_user_id to raise
UnauthorizedException when credentials is None or credentials.credentials is
missing; also add SUPABASE_JWT_ISSUER and SUPABASE_JWT_AUDIENCE to your settings
(app/config/env.py) and .env.example with example values so the new
audience/issuer parameters can be populated.
In `@app/main.py`:
- Around line 13-15: The DB engine disposal must be guaranteed even if an
exception occurs during lifespan; modify the lifespan/context manager around
init_db() and yield so that engine.dispose() is called from a finally block.
Concretely, in the function that currently calls await init_db(); yield; await
engine.dispose() (referencing init_db and engine.dispose), wrap the yield inside
try/finally: call await init_db() before the try, perform yield inside the try,
and call await engine.dispose() in the finally to ensure disposal on errors or
shutdown.
In `@app/routers/itinerary.py`:
- Around line 39-68: The PATCH /{trip_id}/itinerary/reorder route is shadowed by
the more general PATCH /{trip_id}/itinerary/{activity_id} because FastAPI
matches routes in declaration order; move the reorder_activities route (function
name reorder_activities) so it is declared before the activity-specific routes
(update_activity and delete_activity) so the static "/reorder" path is matched
first and the handler receives requests correctly.
In `@requirements.txt`:
- Line 4: requirements.txt currently allows PyJWT >=2.8.0 which permits
vulnerable <2.12.0 versions; update the PyJWT constraint to a fixed minimum of
at least 2.12.0 (change the line containing "PyJWT" to "PyJWT>=2.12.0"), then
regenerate your lockfile/constraints (pip-compile, poetry lock, or pip install
-r ...) and run the test suite to ensure no breakage from the dependency
upgrade.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6eec3c28-5414-4ed8-ac95-a99e69cfa34e
📒 Files selected for processing (13)
.env.exampleapp/config/dependency.pyapp/config/env.pyapp/config/jwt.pyapp/infrastructure/database.pyapp/main.pyapp/routers/candidates.pyapp/routers/itinerary.pyapp/routers/reactions.pyapp/routers/trip_members.pyapp/routers/trips.pyapp/routers/users.pyrequirements.txt
| _bearer = HTTPBearer() | ||
|
|
||
|
|
||
| async def get_current_user_id( | ||
| credentials: HTTPAuthorizationCredentials = Depends(_bearer), | ||
| ) -> uuid.UUID: | ||
| try: | ||
| payload = jwt.decode( | ||
| credentials.credentials, | ||
| settings.SUPABASE_JWT_SECRET, | ||
| algorithms=["HS256"], | ||
| options={"verify_aud": False}, | ||
| ) | ||
| return uuid.UUID(payload["sub"]) | ||
| except (jwt.InvalidTokenError, KeyError, ValueError): | ||
| raise UnauthorizedException() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and examine its content
find . -type f -name "jwt.py" -o -name "env.py" -o -name "dependency.py" | head -20Repository: Onzoro-rento/Travel-App-Backend
Length of output: 141
🏁 Script executed:
# List the app/config directory structure
ls -la app/config/ 2>/dev/null || echo "app/config/ not found"Repository: Onzoro-rento/Travel-App-Backend
Length of output: 366
🏁 Script executed:
# Check the git structure to understand the repo layout
git ls-files app/ | grep -E "(jwt|config|env)" | head -20Repository: Onzoro-rento/Travel-App-Backend
Length of output: 135
🏁 Script executed:
cat -n app/config/jwt.pyRepository: Onzoro-rento/Travel-App-Backend
Length of output: 937
🏁 Script executed:
cat -n app/config/env.pyRepository: Onzoro-rento/Travel-App-Backend
Length of output: 481
🏁 Script executed:
cat -n app/config/dependency.pyRepository: Onzoro-rento/Travel-App-Backend
Length of output: 2629
🏁 Script executed:
find . -name ".env*" -o -name "env*example*" | head -10Repository: Onzoro-rento/Travel-App-Backend
Length of output: 89
🏁 Script executed:
# Check how HTTPBearer and jwt.decode are used in the codebase
rg "HTTPBearer\|verify_aud" -A 2 -B 2Repository: Onzoro-rento/Travel-App-Backend
Length of output: 57
🌐 Web query:
FastAPI HTTPBearer auto_error default behavior missing header
💡 Result:
By default, FastAPI's HTTPBearer (with auto_error=True, the default) raises an HTTPException when the Authorization header is missing or lacks a valid Bearer token/credentials. Prior to FastAPI 0.122.0 (released 2025-11-24), this was a 403 Forbidden with detail "Not authenticated". Since FastAPI 0.122.0, it raises a 401 Unauthorized with detail "Not authenticated", aligning with HTTP standards (RFC 6750) for missing authentication information. If auto_error=False, it returns None instead of raising an error, allowing manual handling (useful for optional auth). Official docs confirm: "By default, if the HTTP Bearer token is not provided (in an Authorization header), HTTPBearer will automatically cancel the request and send the client an error." Source code (current master): if no authorization/scheme/credentials and auto_error=True, raise HTTPException(status_code=HTTP_401_UNAUTHORIZED, detail="Not authenticated").
Citations:
- 1: https://github.com/fastapi/fastapi/blob/e94028ab/fastapi/security/http.py
- 2: https://github.com/FastAPI/FastAPI/blob/5ba94612/fastapi/security/http.py
- 3: https://fastapi.tiangolo.com/reference/security/
- 4: HTTPBearer security scheme is returning 403 instead or 401 fastapi/fastapi#2026
- 5: https://github.com/fastapi/fastapi/releases/tag/0.122.0
- 6: HTTPBearer security scheme is returning 403 instead or 401 fastapi/fastapi#10177
- 7: https://github.com/tiangolo/fastapi/blob/master/fastapi/security/http.py
🌐 Web query:
Supabase JWT claims aud iss requirements verification
💡 Result:
Supabase JWTs require specific aud (audience) and iss (issuer) claims that must be verified for authentication and authorization, particularly for Row Level Security (RLS) and API access. Required Claims: - iss: string, always present, format "https://.supabase.co/auth/v1". Must match the project's issuer URL. - aud: string or string[] (e.g., "authenticated", "anon", or array), identifies the intended recipient. Verification Requirements: When validating JWTs: 1. Verify iss matches your Supabase project's issuer (e.g., https://project-ref.supabase.co/auth/v1). 2. Validate aud matches expected values like "authenticated" (user tokens), "anon" (public), or "service_role" (admin). 3. Use Supabase's JWKS endpoint (https://.supabase.co/auth/v1/.well-known/jwks.json) for signature verification. 4. Check expiration (exp), issued at (iat), and other required claims. Supabase services automatically verify these claims. For custom validation, use libraries like supabase.auth.getClaims or language-specific JWT verifiers ensuring iss, aud, signature, and expiration. Note: aud can be a string or array; code must handle both.
Citations:
- 1: https://supabase.com/docs/guides/auth/jwt-fields
- 2: https://supabase.io/docs/guides/auth/jwt-fields
- 3: https://supabase.com/docs/learn/auth-deep-dive/auth-deep-dive-jwts
- 4: https://supabase.com/docs/guides/auth/jwts
- 5: https://supabase.com/docs/reference/javascript/auth-getclaims
🏁 Script executed:
cat -n .env.exampleRepository: Onzoro-rento/Travel-App-Backend
Length of output: 777
Supabase JWT の aud / iss クレーム検証を有効にしてください。
verify_aud=False により audience クレーム検証が無効化されており、署名が有効な別用途のトークン（例：anon スコープ）も受け入れる可能性があります。Supabase では JWT に必須の iss と aud クレームが含まれ、これらの検証はセキュリティ上必須です。
修正が必要な箇所
app/config/jwt.py: verify_aud=False を削除し、audience と issuer パラメータで Supabase の要件に合わせた検証を追加してください。HTTPBearer(auto_error=False) への変更と credentials is None チェックの追加により、エラーハンドリングを統一できます。
app/config/env.py: SUPABASE_JWT_ISSUER と SUPABASE_JWT_AUDIENCE 設定を追加してください。
.env.example: 上記設定項目を追加し、値の例を示してください（ SUPABASE_JWT_ISSUER=https://your-project-ref.supabase.co/auth/v1、SUPABASE_JWT_AUDIENCE=authenticated など）。
参考: Supabase JWT Claims Reference, Supabase JWT verification
🧰 Tools
🪛 Ruff (0.15.10)
[warning] 12-12: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
[warning] 23-23: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/config/jwt.py` around lines 8 - 23, Remove the insecure
options={"verify_aud": False} from jwt.decode in get_current_user_id and instead
pass audience=settings.SUPABASE_JWT_AUDIENCE and
issuer=settings.SUPABASE_JWT_ISSUER to enable aud/iss checks (keep using
settings.SUPABASE_JWT_SECRET and algorithms=["HS256"]); change _bearer =
HTTPBearer() to _bearer = HTTPBearer(auto_error=False) and add a guard in
get_current_user_id to raise UnauthorizedException when credentials is None or
credentials.credentials is missing; also add SUPABASE_JWT_ISSUER and
SUPABASE_JWT_AUDIENCE to your settings (app/config/env.py) and .env.example with
example values so the new audience/issuer parameters can be populated.
| await init_db() | ||
| yield | ||
| await engine.dispose() |
There was a problem hiding this comment.
DB エンジン破棄を finally で保証してください。
lifespan 中に例外が流れると、yield 後の engine.dispose() が実行されない可能性があります。シャットダウン時の接続解放は finally に入れておく方が安全です。
修正案
async def lifespan(app: FastAPI):
await init_db()
- yield
- await engine.dispose()
+ try:
+ yield
+ finally:
+ await engine.dispose()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await init_db() | |
| yield | |
| await engine.dispose() | |
| await init_db() | |
| try: | |
| yield | |
| finally: | |
| await engine.dispose() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/main.py` around lines 13 - 15, The DB engine disposal must be guaranteed
even if an exception occurs during lifespan; modify the lifespan/context manager
around init_db() and yield so that engine.dispose() is called from a finally
block. Concretely, in the function that currently calls await init_db(); yield;
await engine.dispose() (referencing init_db and engine.dispose), wrap the yield
inside try/finally: call await init_db() before the try, perform yield inside
the try, and call await engine.dispose() in the finally to ensure disposal on
errors or shutdown.
1 participant
Summary by CodeRabbit
リリースノート