Skip to content

🔒 fix: secure card API endpoint with NextAuth #372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
is0692vs wants to merge 1 commit into
base: main
Choose a base branch
from
+15 −1
Open

🔒 fix: secure card API endpoint with NextAuth #372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/app/api/card/[username]/route.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
import { describe, expect, it, vi } from "vitest";
import { getAuthenticatedUser } from "@/lib/apiUtils";

Check warning on line 2 in src/app/api/card/[username]/route.test.ts

View workflow job for this annotation

GitHub Actions / Lint 

'getAuthenticatedUser' is defined but never used

vi.mock("@/lib/apiUtils", async (importOriginal) => {
const actual = await importOriginal<typeof import("@/lib/apiUtils")>();
return {
...actual,
getAuthenticatedUser: vi.fn().mockResolvedValue({ username: "alice", token: "token" }),
};
});
Comment on lines +4 to +10

@gemini-code-assist gemini-code-assist Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to the repository guidelines, mock implementations returning Promises should use async functions and have explicit return types to maintain type safety and readability.

Suggested change
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
  1. 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.

Comment on lines +4 to +10

@greptile-apps greptile-apps Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 401 パスのテストが欠落している

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.


vi.mock("@/lib/cardDataFetcher", () => ({
fetchCardData: vi.fn(),
Expand Down
7 changes: 6 additions & 1 deletion src/app/api/card/[username]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import { RateLimiter } from "@/lib/rateLimit";
import { fetchCardData } from "@/lib/cardDataFetcher";
import { parseCardQueryParams, renderCardResponse, renderErrorCardResponse } from "@/lib/cardRenderer";
import { getClientIp } from "@/lib/rateLimit";
import { getAuthenticatedUser } from "@/lib/apiUtils";


export const runtime = "edge";
const rateLimiter = new RateLimiter(50, 60 * 1000); // 50 requests per minute


Expand All @@ -15,6 +16,10 @@ export async function GET(
{ params }: { params: Promise<{ username: string }> }
): Promise<Response> {
const { username } = await params;
const user = await getAuthenticatedUser();
if (!user) {
return new Response("Unauthorized", { status: 401 });
}
Comment on lines +19 to +22

@greptile-apps greptile-apps Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 user.token が取得されているが一切使用されていない

getAuthenticatedUser() は { username, token } を返しますが、token はその後まったく利用されていません。fetchCardData は process.env.GITHUB_TOKEN を直接参照するため、認証済みユーザーの OAuth トークンが使われていない状態です。意図的であれば問題ありませんが、_user として命名しておくと「意図的に未使用」であることが明示されます。

Suggested change
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!

const url = new URL(request.url);
const options = parseCardQueryParams(url.searchParams);
const allowedOrigin = process.env.APP_URL || "http://localhost:3000";
Expand Down
Loading