🔒 fix: secure card API endpoint with NextAuth#372
Conversation
Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 42 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ 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 introduces authentication checks to the card API route by calling getAuthenticatedUser and returning a 401 Unauthorized response if the user is not authenticated. It also removes the edge runtime configuration and updates the test suite to mock the authentication utility. The review feedback recommends enhancing the mock implementation in the test file by using an async function with an explicit return type to improve type safety and readability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| vi.mock("@/lib/apiUtils", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("@/lib/apiUtils")>(); | ||
| return { | ||
| ...actual, | ||
| getAuthenticatedUser: vi.fn().mockResolvedValue({ username: "alice", token: "token" }), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
According to the repository guidelines, mock implementations returning Promises should use async functions and have explicit return types to maintain type safety and readability.
| vi.mock("@/lib/apiUtils", async (importOriginal) => { | |
| const actual = await importOriginal<typeof import("@/lib/apiUtils")>(); | |
| return { | |
| ...actual, | |
| getAuthenticatedUser: vi.fn().mockResolvedValue({ username: "alice", token: "token" }), | |
| }; | |
| }); | |
| vi.mock("@/lib/apiUtils", async (importOriginal) => { | |
| const actual = await importOriginal<typeof import("@/lib/apiUtils")>(); | |
| return { | |
| ...actual, | |
| getAuthenticatedUser: vi.fn(async (): Promise<{ username: string; token: string } | null> => ({ username: "alice", token: "token" })), | |
| }; | |
| }); |
References
- In TypeScript, ensure functions and mock implementations have explicit return types and use async functions for mocks returning Promises to maintain type safety and readability.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const user = await getAuthenticatedUser(); | ||
| if (!user) { | ||
| return new Response("Unauthorized", { status: 401 }); | ||
| } |
There was a problem hiding this comment.
user.token が取得されているが一切使用されていない
getAuthenticatedUser() は { username, token } を返しますが、token はその後まったく利用されていません。fetchCardData は process.env.GITHUB_TOKEN を直接参照するため、認証済みユーザーの OAuth トークンが使われていない状態です。意図的であれば問題ありませんが、_user として命名しておくと「意図的に未使用」であることが明示されます。
| const user = await getAuthenticatedUser(); | |
| if (!user) { | |
| return new Response("Unauthorized", { status: 401 }); | |
| } | |
| const _user = await getAuthenticatedUser(); | |
| if (!_user) { | |
| return new Response("Unauthorized", { status: 401 }); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/card/[username]/route.ts
Line: 19-22
Comment:
**`user.token` が取得されているが一切使用されていない**
`getAuthenticatedUser()` は `{ username, token }` を返しますが、`token` はその後まったく利用されていません。`fetchCardData` は `process.env.GITHUB_TOKEN` を直接参照するため、認証済みユーザーの OAuth トークンが使われていない状態です。意図的であれば問題ありませんが、`_user` として命名しておくと「意図的に未使用」であることが明示されます。
```suggestion
const _user = await getAuthenticatedUser();
if (!_user) {
return new Response("Unauthorized", { status: 401 });
}
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| vi.mock("@/lib/apiUtils", async (importOriginal) => { | ||
| const actual = await importOriginal<typeof import("@/lib/apiUtils")>(); | ||
| return { | ||
| ...actual, | ||
| getAuthenticatedUser: vi.fn().mockResolvedValue({ username: "alice", token: "token" }), | ||
| }; | ||
| }); |
There was a problem hiding this comment.
getAuthenticatedUser は常に認証済みユーザーを返すようにモックされていますが、null を返した場合（未認証）に 401 Unauthorized が返されることを確認するテストケースがありません。この PR の主要な変更点であるにもかかわらず、そのパスがテストで検証されていない状態です。vi.mocked(getAuthenticatedUser).mockResolvedValueOnce(null) を使って未認証シナリオのテストを追加してください。
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/card/[username]/route.test.ts
Line: 4-10
Comment:
**401 パスのテストが欠落している**
`getAuthenticatedUser` は常に認証済みユーザーを返すようにモックされていますが、`null` を返した場合（未認証）に `401 Unauthorized` が返されることを確認するテストケースがありません。この PR の主要な変更点であるにもかかわらず、そのパスがテストで検証されていない状態です。`vi.mocked(getAuthenticatedUser).mockResolvedValueOnce(null)` を使って未認証シナリオのテストを追加してください。
How can I resolve this? If you propose a fix, please make it concise.1 participant
🎯 What: Added NextAuth validation to the
/api/card/[username]endpoint and updated associated tests.🛡️ Solution:
export const runtime = "edge";from the route asgetServerSession(which relies on Node crypto modules) is unsupported in the Edge runtime.getAuthenticatedUserfrom@/lib/apiUtils.await getAuthenticatedUser()to verify the session. If no user is returned, the endpoint immediately responds with a401 Unauthorized.getAuthenticatedUserappropriately so that existing tests continue to pass with the new authentication layer.PR created automatically by Jules for task 15887205752685383845 started by @is0692vs
Greptile Summary
このPRは
/api/card/[username]エンドポイントに NextAuth セッション検証を追加し、未認証アクセスを 401 で拒否します。Edge runtime の削除（getServerSessionがNodeのcrypto依存のため）も合わせて行われています。getAuthenticatedUser()を呼び出して未認証時に即401を返す認証ガードを追加。Edge runtime を Node.js runtime に切り替え。getAuthenticatedUserのモックを追加し既存のキャッシュ・エラーテストを維持。ただし認証がnullを返すケース（401パス）のテストが未追加。Cache-Control: public, s-maxage=1800ヘッダーが残っており、CDNがレスポンスをキャッシュすることで認証チェックがバイパスされるリスクがある。Confidence Score: 3/5
認証チェックを追加したものの、
publicキャッシュヘッダーが残っているため CDN 経由で認証をバイパスできる状態のままマージするのはリスクがある。追加された認証ガードは
Cache-Control: public, s-maxage=1800と組み合わさると実質的に機能しない。CDN にキャッシュされた後は未認証リクエストにもレスポンスが返り、意図したアクセス制御が守られない。このキャッシュヘッダーの修正が必須であり、マージ前に対応が必要。src/app/api/card/[username]/route.ts のキャッシュ制御ヘッダーを要確認。
Security Review
src/app/api/card/[username]/route.ts:Cache-Control: public, s-maxage=1800により、認証済みリクエストのレスポンスが CDN にキャッシュされ、その後の未認証リクエストにもキャッシュ済みレスポンスが返される。getAuthenticatedUser()のチェックが実行されないため、追加された認証ガードが実質的に機能しない。Important Files Changed
publicキャッシュヘッダーが残っており CDN 経由で認証をバイパスされる恐れがある。また認証ユーザーのトークンは取得されるが未使用。getAuthenticatedUserのモックを追加し既存テストを維持。401 パス（未認証時）のテストケースが欠落している。Sequence Diagram
sequenceDiagram participant Client as クライアント（未認証） participant CDN as CDN / プロキシ participant Route as /api/card/[username] participant Auth as getAuthenticatedUser() participant GH as GitHub API Note over Client,GH: 認証済みユーザーの初回リクエスト Client->>CDN: GET /api/card/alice（認証済み） CDN->>Route: キャッシュミス → 転送 Route->>Auth: getAuthenticatedUser() Auth-->>Route: "{ username, token }" Route->>GH: fetchCardData("alice") GH-->>Route: カードデータ Route-->>CDN: "200 OK (Cache-Control: public, s-maxage=1800)" CDN-->>Client: 200 OK（レスポンスをキャッシュ） Note over Client,GH: 未認証ユーザーの後続リクエスト（問題あり） Client->>CDN: GET /api/card/alice（未認証） CDN-->>Client: 200 OK（キャッシュ済みレスポンスを返す） Note right of CDN: Route の認証チェックは実行されない！Comments Outside Diff (1)
src/app/api/card/[username]/route.ts, line 11-12 (link)public, s-maxage=1800は CDN・リバースプロキシに「このレスポンスを全ユーザー共有でキャッシュしてよい」と指示します。認証済みユーザーが/api/card/aliceを取得すると CDN がキャッシュし、その後に未認証のリクエストが来ても CDN がキャッシュ済みレスポンスを返すため、getAuthenticatedUser()のチェックが一切実行されず認証が完全にバイパスされます。publicをprivateに変更するか、CDN キャッシュを無効にする必要があります。Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: add authentication to card API endp..." | Re-trigger Greptile