Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 47 minutes and 21 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
WalkthroughUser 스키마에 Changes네이버 OAuth 소셜 로그인
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
필수 리뷰 체크포인트1. JWT_SECRET 환경변수 강제Why: 서명 없는 토큰 발급으로 인증 우회. function getSecret(): Uint8Array {
const secret = process.env.JWT_SECRET;
if (!secret) throw new Error("JWT_SECRET not set");
return new TextEncoder().encode(secret);
}✅ 현재 구현 통과, 테스트 (range_edaf87c73507) 확인됨. 2. CSRF state 검증 필수성Why: 공격자가 임의의 redirect_uri로 인가 코드 탈취. const stateCookie = req.cookies.get(STATE_COOKIE)?.value;
if (!stateCookie || stateCookie !== state) {
return NextResponse.redirect(...failUrl);
}✅ 구현됨, 쿠키 삭제(range_faea74365bea) 포함. 3. 신규 사용자 생성 시 닉네임 충돌 처리Why: 기존 로컬 회원과 닉네임 충돌 → 에러 또는 덮어쓰기. const existingUser = await prisma.user.findUnique({
where: { nickname: extractedUser.nickname }
});
if (existingUser) {
newUser.nickname = `naver_${naverId}`;
}
4. profileCompleted 상태 관리Why: 소셜 로그인 사용자는 추가 정보(나이, 약관) 미입력 → 기능 제한. // 신규 유저
user = await prisma.user.create({
data: { profileCompleted: false, ... }
});
// callback에서
const redirectUrl = user.profileCompleted
? successUrl
: additionalInfoUrl;✅ 구현됨, 로직 명확. 5. JWT payload provider 필드 선택 처리Why: provider 필드는 선택(provider?) 정의되지만 callback에서 export interface JwtPayload {
userId: string;
email?: string;
nickname: string;
provider?: string;
}✅ 선택 필드 → callback에서 "naver" 지정 가능. 로컬 로그인 (provider 생략 또는 "local") 구분 가능. 6. Naver access_token 비노출Why: access_token을 JWT에 포함하면 클라이언트에 노출 → 네이버 API 직접 호출 위험. const token = await signAccessToken({
userId: user.id,
email: user.email,
nickname: user.nickname,
provider: "naver"
});
// naverAccessToken은 포함 안 함✅ 구현됨, 보안 통과. 7. 환경변수 누락 시 graceful errorWhy: NAVER_CLIENT_ID, NAVER_REDIRECT_URI 누락 시 500 에러. try {
const authUrl = getNaverAuthUrl(state);
// ...
} catch (error) {
console.error("Naver login error:", error);
const baseUrl = process.env.NEXT_PUBLIC_BASE_URL || "http://localhost:3000";
return NextResponse.redirect(`${baseUrl}/login?error=naver_login_failed`);
}✅ 구현됨, 오류 처리 적절. 8. 테스트 커버리지 검증범위별 검사:
9. HTTP 응답 상태 코드 검증코드 (range_7bc8cab61933): const response = await fetch(naverTokenEndpoint, { method: "POST", ... });
if (!response.ok) {
throw new Error("Failed to get Naver token");
}
const data = await response.json();
if (data.error) {
throw new Error(`Naver error: ${data.error}`);
}✅ response.ok 및 JSON error 필드 검증, 명확한 에러 메시지. 10. State 쿠키 만료 시간code: range_fe3068c4c3e4 & range_39bb71753742 maxAge: 10 * 60 // 10분✅ OAuth 2.0 권장 범위(5~10분), 적절함. 최종 지적
Approve 조건: 닉네임 충돌 처리 재설계 + callback 통합 테스트 추가. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/api/auth/naver/callback/route.ts`:
- Around line 96-123: The current nickname uniqueness check using
prisma.user.findUnique then prisma.user.create can race and raise a Prisma
unique constraint error (P2002); update the user creation flow around
prisma.user.create to catch Prisma.PrismaClientKnownRequestError, detect
error.code === "P2002" related to the nickname, and then retry create with a
fallback nickname (e.g., `naver_${naverId}` or
`naver_${naverId}_${randomSuffix}`) before propagating the error; reference the
variables/fns finalNickname, prisma.user.findUnique, prisma.user.create and
ensure the P2002 branch produces a successful user or a deterministic fallback
redirect rather than letting the generic catch treat it as a fatal failure.
In `@src/app/api/auth/naver/login/route.ts`:
- Line 9: The GET handler currently builds error redirect URLs using
NEXT_PUBLIC_BASE_URL (in the GET function and the code around the other redirect
on lines 29-30); change it to derive the origin from the incoming request by
constructing new URL(request.url).origin and use that origin to build
same-origin redirect targets (e.g., new URL('/auth/naver/error?reason=...', new
URL(request.url).origin)). Replace any usage of NEXT_PUBLIC_BASE_URL in this
file with this request-based origin so error redirects are forced to the request
origin.
In `@src/lib/auth/__tests__/jwt.test.ts`:
- Around line 16-72: Add a boundary test for the JWT exp field by issuing a
token with signAccessToken, using jest fake timers (jest.useFakeTimers /
jest.setSystemTime or advanceTimersByTime) to simulate verification before and
just after the expiry, and assert verifyAccessToken returns the payload before
expiry and null after expiry; locate this new test alongside existing tests in
jwt.test.ts and ensure timers are restored (jest.useRealTimers) after the test.
In `@src/lib/auth/__tests__/naver.test.ts`:
- Around line 121-174: Add a new test in the getNaverUser suite that mocks
global.fetch to return ok: true and resultcode: "00" but with response.id
missing or empty, then call getNaverUser("access_token") and assert it rejects
with the same error you expect for missing id (e.g., "네이버 사용자 정보 조회 오류" or the
specific message thrown by getNaverUser). Use vi.spyOn(global,
"fetch").mockResolvedValueOnce to supply the mocked Response.json payload and
mirror the pattern used in the other tests so the edge-case of a success code
but missing response.id is covered.
In `@src/lib/auth/jwt.ts`:
- Around line 31-34: The current jwtVerify usage casts payload directly to
JwtPayload, which bypasses runtime checks and can allow tokens missing required
fields; update the verification in the function that calls jwtVerify(token,
getSecret()) to perform a runtime guard on the returned payload (check that
payload.userId is a string/number as expected and payload.nickname is a
non-empty string) and only return payload as JwtPayload when those checks pass,
otherwise return null; also ensure the catch block of the same try/catch returns
null (not throwing) so invalid tokens are closed out.
In `@src/lib/auth/naver.ts`:
- Around line 68-74: The fetch POST to "https://nid.naver.com/oauth2.0/token"
has no timeout or robust error handling; wrap this external call (and the
similar fetch at lines 94-99) with a shared timeout wrapper (e.g., a utility
like fetchWithTimeout) that aborts via AbortController after a configured short
timeout, and surface a clear error when aborted so callers can handle it; update
the token-exchange function(s) to use that wrapper and catch/translate
network/timeout errors into meaningful exceptions/logs instead of hanging
indefinitely.
- Around line 107-116: After verifying data.resultcode === "00", add a defensive
check that data.response and data.response.id exist and are non-empty before
returning; if missing, throw a clear exception (e.g., "네이버 사용자 정보 조회 오류: id 누락")
so callers of this function (the code that uses data.response.id for account
linking/upsert) never receive a user object without an identifier. Update the
logic around resultcode/data.response.id (referencing data.resultcode and
data.response.id in this file) to validate presence and throw immediately when
id is absent.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f2b962ed-87e3-4f5f-a16d-9d68c5e3e70f
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.json,!**/*.json,!package-lock.jsonpackage.jsonis excluded by!**/*.json
📒 Files selected for processing (9)
.env.exampleprisma/migrations/20260525000000_add_social_account/migration.sqlprisma/schema.prismasrc/app/api/auth/naver/callback/route.tssrc/app/api/auth/naver/login/route.tssrc/lib/auth/__tests__/jwt.test.tssrc/lib/auth/__tests__/naver.test.tssrc/lib/auth/jwt.tssrc/lib/auth/naver.ts
- naver.ts: fetchWithTimeout으로 외부 API 타임아웃 처리 - naver.ts: response.id 누락 시 명시적 에러 처리 - callback/route.ts: nickname P2002 race condition 방어 처리 - login/route.ts: 에러 리다이렉트를 NEXT_PUBLIC_BASE_URL 대신 request origin 기반으로 변경 - jwt.ts: verifyAccessToken 런타임 페이로드 필드 검증 추가 - jwt.test.ts: JWT 만료 경계 테스트 추가 - naver.test.ts: response.id 누락 케이스 테스트 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🚀 작업 내용 (What)
📣 핵심 변경 이유 (Why)
📸 스크린샷 (Visuals, 선택)
🔗 관련 이슈 (Issue)
Close #33