[FEAT] 네이버 캘린더 연동 API 추가 - #43
Conversation
|
Warning Review limit reached
More reviews will be available in 4 minutes and 39 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
WalkthroughNaver OAuth 2.0 기반 캘린더 연동 기능 구현. OAuth 상태 관리, 토큰 교환/갱신/저장(HttpOnly 쿠키), 사용자 프로필 조회, iCalendar 포맷 일정 생성 API 호출, zod 기반 요청 검증, 엔드-투-엔드 테스트. ChangesNaver 캘린더 OAuth 및 일정 생성 연동
예상 코드 리뷰 난이도🎯 4 (복잡) | ⏱️ ~60분 관련 이슈
추천 라벨
시니어 리뷰 코멘트CSRF State 관리 방식 재검토 필요 // 현재 방식
validateAndClearOAuthState(state) // 실패 시 다시 시작해야 함
// 개선안
validateAndClearOAuthState(state, { retryCount: 3, ttl: 300000 })토큰 갱신 자동화 시 경합 조건 미처리 // src/lib/naver/auth.ts getValidTokens() 에 추가 필요
if (isRefreshing) await refreshingPromise;
else { refreshingPromise = refreshAccessToken(...); isRefreshing = true; }POST /api/naver/events/create에 타임존 검증 부재 const CreateEventSchema = z.object({
timeZone: z.enum(['Asia/Seoul', 'UTC', /* ... */]).optional(),
// ...
});이벤트 API 응답 매핑 불충분 if (data.result === 'failure') {
throw new Error(`Naver API error [${data.code}]: ${data.message}`);
}테스트 환경변수 누수 위험 afterEach(() => {
delete process.env.NAVER_CLIENT_ID;
delete process.env.NAVER_CLIENT_SECRET;
vi.clearAllMocks();
});쿠키 보안 옵션 불완전 const isProduction = process.env.NODE_ENV === 'production';
cookies().set(NAVER_TOKENS_COOKIE_NAME, JSON.stringify(tokens), {
secure: isProduction,
httpOnly: true,
sameSite: 'lax',
// ...
});🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 15
🤖 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 @.env.example:
- Around line 12-14: The .env.example currently lists NAVER_REDIRECT_URI as if
required; update the example to mark NAVER_REDIRECT_URI as optional (or comment
it out) and add a short note that getRedirectUri() in src/lib/naver/auth.ts will
auto-generate the redirect URI from NEXT_PUBLIC_BASE_URL when NAVER_REDIRECT_URI
is not set; specifically reference NAVER_REDIRECT_URI and getRedirectUri() so
developers understand the variable is optional and avoid redundant
configuration.
In `@src/app/api/naver/callback/route.ts`:
- Around line 52-54: The current OAuth callback returns NextResponse.redirect to
the API route `/api/naver/calendars`, which causes the browser to show raw JSON;
change the redirect target from
`${baseUrl}/api/naver/calendars?connected=true&user=${encodeURIComponent(identifier)}`
to the frontend scheduling UI route (for example
`${baseUrl}/scheduling?connected=true&user=${encodeURIComponent(identifier)}` or
the app's canonical scheduling path) so the browser lands on the UI page instead
of the API; keep the query params (connected and user/identifier) and update the
redirect call that uses NextResponse.redirect and variables baseUrl and
identifier accordingly.
In `@src/app/api/naver/events/create/route.ts`:
- Line 9: Zod schema for the request currently sets calendarId as optional but
uses an error message saying "calendarId는 필수입니다.", which conflicts with the
optional behavior and the fallback to DEFAULT_CALENDAR_ID in createEvent; update
the calendarId z.string().min(...) message to reflect that the field is optional
(e.g., "calendarId는 선택 사항입니다." or "제공할 경우 빈 문자열이 될 수 없습니다.") or change
validation to only run when a value is present so that empty/missing calendarId
does not produce a "required" error; ensure references are to the calendarId
schema entry and the createEvent logic that falls back to DEFAULT_CALENDAR_ID.
- Around line 71-87: Replace fragile string-based error checks in the Naver
calendar error handling block with explicit error-type or error-code checks:
introduce or import custom error classes (e.g., NaverAuthError,
NaverPermissionError) or use a standardized error.code from the upstream handler
(events.ts), then change the conditionals in create/route.ts to test err
instanceof NaverAuthError and err instanceof NaverPermissionError (or err.code
=== 'NAVER_AUTH' / 'NAVER_PERMISSION') and return the 401/403 NextResponse
accordingly; update the producer in events.ts to throw the corresponding custom
error types or set the canonical error.code so the router’s branching is
resilient to message changes.
In `@src/lib/naver/__tests__/auth.test.ts`:
- Around line 60-83: Add tests covering null/undefined received state and
missing cookie for OAuth state validation: update the "oauth state cookie" suite
to call validateAndClearOAuthState with null (expect false) and to simulate
mockCookieStore.get returning undefined while calling validateAndClearOAuthState
with a valid string (expect false). Ensure these tests reference
validateAndClearOAuthState and the mockCookieStore.get behavior so the code path
that returns false when receivedState is falsy or when the cookie is absent is
exercised.
- Around line 160-196: Add two tests to src/lib/naver/__tests__/auth.test.ts for
getUserProfile to cover API failure and network errors: one that stubs mockFetch
to resolve with ok: false (e.g., status 401 and text returning "Unauthorized")
and asserts getUserProfile rejects, and one that stubs
mockFetch.mockRejectedValueOnce(new Error("Network error")) and asserts
getUserProfile rejects with the network error message; place them alongside the
existing getUserProfile tests so they exercise the same call path and header
check.
- Around line 36-58: Add a test to validate that buildAuthUrl correctly
URL-encodes special characters in the state parameter: call buildAuthUrl with a
state containing spaces and reserved characters (e.g., "state with spaces &
special=chars") and assert the resulting URL includes the percent-encoded state
substring ("state%20with%20spaces%20%26%20special%3Dchars"); place this new test
alongside the existing buildAuthUrl tests in auth.test.ts and reference
buildAuthUrl so the encoding behavior is explicitly verified.
In `@src/lib/naver/__tests__/events.test.ts`:
- Around line 60-170: Add two tests to cover the missing error scenarios: one
that mocks fetch returning ok: false with status: 403 and text "Forbidden" and
asserts createEvent rejects with a "권한" (permission) error, and another that
mocks a successful response (ok: true) whose JSON has result: "success" but no
returnValue and asserts createEvent rejects (throws) — update the test file's
describe("createEvent"... ) to include these cases so the createEvent function's
403 handling and the success-without-returnValue path are verified; reference
the existing createEvent calls and mockFetch.mockResolvedValueOnce usage to
mirror the other tests.
- Around line 13-58: Tests for buildNaverScheduleIcal are missing key edge
cases; add unit tests that (1) call buildNaverScheduleIcal without uid and
assert UID matches /UID:[a-f0-9-]+@moim\.app/, (2) omit optional
location/description and assert the resulting iCal string does NOT contain
"LOCATION:" or "DESCRIPTION:", (3) pass an invalid ISO date (e.g.,
"invalid-date") and assert the call throws an error mentioning date/format, and
(4) pass startDateTime >= endDateTime and assert the function throws or rejects
with a validation error; target the buildNaverScheduleIcal symbol and make
assertions using toMatch/toContain/not.toContain/toThrow as in existing tests.
In `@src/lib/naver/auth.ts`:
- Around line 187-215: In getValidTokens, add error logging for both the
JSON.parse failure and the refreshAccessToken failure: when JSON.parse(raw)
throws, catch the error and log a descriptive message including
TOKEN_COOKIE_NAME and the raw cookie value (or a safely truncated/redacted
version) plus the error stack; likewise, in the catch around
refreshAccessToken(tokens.refreshToken) log a message that includes which token
refresh failed, the (redacted/truncated) refreshToken and the caught error/stack
before returning null; implement these logs inside getValidTokens (referencing
JSON.parse, refreshAccessToken, saveTokensToCookie, and TOKEN_COOKIE_NAME) and
keep the existing behavior of returning null after logging.
- Around line 199-211: getValidTokens() currently updates only
accessToken/tokenType/expiresAt after calling
refreshAccessToken(tokens.refreshToken) and ignores a possible new refresh
token; update getValidTokens() to also set tokens.refreshToken =
refreshed.refreshToken ?? tokens.refreshToken and then call
saveTokensToCookie(tokens), and ensure refreshAccessToken() returns
refreshed.refreshToken in its response shape so the new refresh token can be
persisted for future rotations.
- Around line 105-112: Extract a shared, type-safe parser (e.g.,
parseTokenResponse) and use it from exchangeCodeForTokens and
refreshAccessToken: implement parseTokenResponse(data) to validate and parse
expires_in (use parseInt(String(data.expires_in), 10)), throw on missing/invalid
expires_in, and return { accessToken, tokenType, expiresAt } with expiresAt =
Date.now() + expiresIn*1000; then replace the inline Number(data.expires_in)
logic in exchangeCodeForTokens and refreshAccessToken to call parseTokenResponse
and merge in refreshToken where needed (keep function names
exchangeCodeForTokens and refreshAccessToken to locate replacements) to remove
duplication and ensure type-safe expiry calculation.
In `@src/lib/naver/events.ts`:
- Around line 34-53: The toNaverLocalDateTime function can produce "undefined"
segments because Intl.DateTimeFormat.formatToParts may omit fields; update
toNaverLocalDateTime to validate that byType.year, month, day, hour, minute, and
second are present and non-empty after building parts, and if any are missing
throw a clear Error (e.g. "Invalid date parts from formatToParts") instead of
returning a malformed string; locate the formatToParts call and parts→byType
mapping in toNaverLocalDateTime and add the existence checks (or
fallback/zero-pad logic if you prefer) before composing and returning the final
timestamp string.
- Around line 60-105: The VTIMEZONE block in buildNaverScheduleIcal currently
hardcodes TZOFFSETFROM/TZOFFSETTO to +0900 while TZID uses the dynamic timeZone;
to fix this, enforce that input.timeZone is the expected Asia/Seoul (or
DEFAULT_TIME_ZONE) by validating timeZone at the top of buildNaverScheduleIcal
and throw or return an error if a different zone (e.g., "America/New_York") is
provided, or alternatively remove the VTIMEZONE block entirely if you verify
Naver accepts only DTSTART;TZID=... without VTIMEZONE—update the function’s
timeZone handling and add a clear validation/error path so UID, DTSTART/DTEND
and VTIMEZONE remain consistent.
In `@src/types/naver-calendar.ts`:
- Around line 40-46: Split NaverCreateScheduleResponse into two discriminated
types (e.g., NaverCreateScheduleSuccess with result: "success" and required
returnValue: NaverEvent, and NaverCreateScheduleFailure with result: "failure"
and optional message/code) and export their union as
NaverCreateScheduleResponse; update call sites (notably the code in events.ts
that reads data.result and then accesses data.returnValue) to rely on the
narrowed union so TypeScript infers returnValue as NaverEvent after checking
result === "success".
🪄 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: 5622f3dc-c52d-4528-9a62-311a1e81be17
⛔ 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 (12)
.env.examplesrc/app/api/naver/auth/route.tssrc/app/api/naver/calendars/route.tssrc/app/api/naver/callback/route.tssrc/app/api/naver/events/create/route.tssrc/app/api/naver/events/query/route.tssrc/lib/naver/__tests__/auth.test.tssrc/lib/naver/__tests__/events.test.tssrc/lib/naver/auth.tssrc/lib/naver/events.tssrc/types/calendar-event.tssrc/types/naver-calendar.ts
- OAuth 콜백을 API(JSON) 대신 /schedule/create UI 페이지로 리다이렉트 - 토큰 응답 파싱을 parseTokenResponse로 통합하고 expires_in 검증 추가 - 토큰 갱신 시 새 refresh_token 로테이션 반영 및 실패 로깅 추가 - createEvent 에러를 커스텀 에러 클래스(NaverAuthError/NaverPermissionError)로 분기 - NaverCreateScheduleResponse를 discriminated union으로 분리 - formatToParts 필수 필드 검증 및 Asia/Seoul 외 타임존 입력 차단 - calendarId zod 에러 메시지를 optional 동작에 맞게 수정 - .env.example의 NAVER_REDIRECT_URI를 선택값으로 주석 처리 - state 인코딩/null 처리, getUserProfile 실패, ical 엣지 케이스 테스트 추가 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
🚀 작업 내용 (What)
📣 핵심 변경 이유 (Why)
📸 스크린샷 (Visuals, 선택)
🔗 관련 이슈 (Issue)
Related #42
🧪 검증 결과
node node_modules/vitest/vitest.mjs run통과: 25 files, 209 testsnode node_modules/eslint/bin/eslint.js src/lib/naver src/app/api/naver src/types/naver-calendar.ts src/types/calendar-event.ts통과node node_modules/next/dist/bin/next build통과/api/naver/calendars인증 응답은 확인했습니다./api/naver/events/create호출은 네이버 Gateway에서401 / errorCode 024인증 실패가 발생했습니다.