refactor(IN-314): 유튜브 로그인, 개발서버 로그인 구현 및 로그인 로직 수정#136
Merged
Conversation
- 개발 서버 로그인: app\auth\callback\route.ts에 개발서버 라우팅 분기 처리 - 유튜브 로그인: app\auth\youtube\route.ts 생성
401 에러 발생 시 /로 리다이렉트 되는 것이 아니라 로그인 모달이 랜더링 되도록 코드 수정
화면 랜더링 시 최상단이 아닌 하단으로 내려가서 랜더링 되는 문제가 있음. auth여부와 채널 연동 여부 레이아웃에 children을 통해 랜더링하여 해결
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 24 minutes and 52 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
워크스루Google과 YouTube OAuth를 제공자 접두사( 변경 사항다중 제공자 OAuth 및 모달 기반 인증 플로우
예상 코드 리뷰 난이도🎯 3 (중간) | ⏱️ ~20 분 제안된 리뷰어
시
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/layouts/ChannelLinkedLayout.tsx (1)
32-37:⚠️ Potential issue | 🟠 Major | ⚡ Quick win인증/연동 미충족 상태에서 children을 마운트하지 마세요.
Line 37처럼
children을 그대로 렌더링하면router.replace('/')이전에 보호된 하위 트리가 실행되어 불필요한 API 호출/상태 갱신이 발생할 수 있습니다. 가드 실패 시에는 placeholder만 렌더링하고children마운트는 막는 쪽이 안전합니다.수정 예시
if ( isInitializing || !user?.userChannelDetails?.youtubeChannelName || !user?.userChannelDetails?.youtubeChannelId ) - return <div className='invisible'>{children}</div> + return <div className='invisible min-h-screen' aria-hidden='true' />🤖 Prompt for 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. In `@src/app/layouts/ChannelLinkedLayout.tsx` around lines 32 - 37, The current guard in ChannelLinkedLayout.tsx mounts children even when the auth/channel checks fail (the if checking isInitializing || !user?.userChannelDetails?.youtubeChannelName || !user?.userChannelDetails?.youtubeChannelId), which causes protected subtree effects before router.replace('/'). Change the early return so it does not render or mount children (e.g., return a placeholder like <div className='invisible' /> or null) instead of returning <div className='invisible'>{children}</div>, ensuring children are only mounted after the guard passes.app/auth/callback/route.ts (1)
59-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick win민감 정보(accessToken·사용자 PII) 로깅을 제거하세요.
JSON.stringify(data)에는accessToken과userDetails등 PII가 포함되어 서버 로그에 평문으로 남습니다. PR 설명에서는 디버그 로그를 제거했다고 했으나 이 라인이 남아 있습니다.🔒️ 제안 수정
const data: LoginResponse = await backendResponse.json() - console.log('[auth/callback] backend data:', JSON.stringify(data))🤖 Prompt for 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. In `@app/auth/callback/route.ts` at line 59, Remove the plaintext console.log('[auth/callback] backend data:', JSON.stringify(data)) in app/auth/callback/route.ts; either delete the call or replace it with a sanitized/log-safe message that does not include data.accessToken, data.userDetails, or any PII (only log non-sensitive status or an opaque request id). Locate the statement that references "data" (the console.log with '[auth/callback]') and ensure any logging uses redaction or explicit whitelisting of safe fields before emitting to logs.
🧹 Nitpick comments (1)
app/auth/youtube/route.ts (1)
4-36: ⚡ Quick win
app/auth/google/route.ts와 거의 동일한 로직 중복.
state접두사(youtube:vsgoogle:)만 다르고 쿠키 설정·URLSearchParams·리다이렉트 구성이 완전히 동일합니다. 제공자가 추가될수록 복사·붙여넣기가 늘어나므로 공통 헬퍼로 추출하는 것을 권장합니다.♻️ 제안: 공통 헬퍼 추출
// 예: app/auth/shared/startOAuth.ts import { NextResponse } from 'next/server' import { cookies } from 'next/headers' export async function startOAuth(provider: 'google' | 'youtube') { if (process.env.NEXT_PUBLIC_MOCK_ENABLED === 'true') { return NextResponse.redirect( `${process.env.NEXT_PUBLIC_APP_URL}/auth/mock-callback` ) } const state = `${provider}:${crypto.randomUUID()}` const cookieStore = await cookies() cookieStore.set('oauth_state', state, { httpOnly: true, secure: process.env.NODE_ENV === 'production', sameSite: 'lax', path: '/', maxAge: 60 * 10, }) const params = new URLSearchParams({ client_id: process.env.GOOGLE_CLIENT_ID!, redirect_uri: `${process.env.NEXT_PUBLIC_APP_URL}/auth/callback`, response_type: 'code', scope: 'openid email profile https://www.googleapis.com/auth/youtube.readonly', state, access_type: 'offline', prompt: 'consent', }) return NextResponse.redirect( `https://accounts.google.com/o/oauth2/v2/auth?${params.toString()}` ) }각 라우트는
export const GET = () => startOAuth('youtube')형태로 단순화됩니다.🤖 Prompt for 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. In `@app/auth/youtube/route.ts` around lines 4 - 36, There is duplicated OAuth-start logic in the GET handlers (only the state prefix differs); extract a shared startOAuth(provider: string) helper that encapsulates the mock redirect, cookie creation via cookies().set('oauth_state', ...), URLSearchParams construction, and NextResponse.redirect call; update the existing GET exports (e.g., the functions named GET in both provider routes) to simply return startOAuth('youtube') or startOAuth('google'), and make the helper accept a provider-specific state prefix and optional scope override so provider-specific scopes (like YouTube’s) can be passed in.
🤖 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/layouts/AuthGuardLayout.tsx`:
- Around line 21-22: The current AuthGuardLayout returns <div
className='invisible'>{children}</div> when isInitializing or not isLoggedIn,
which keeps children mounted and can trigger unwanted side effects; change the
guard so children are not mounted in that branch (e.g., return null or a
container without rendering children) and only render children when
isInitializing is false and isLoggedIn is true; update the logic inside
AuthGuardLayout (referencing isInitializing, isLoggedIn, and children) to avoid
mounting protected content while unauthenticated or initializing.
In `@src/features/auth/model/usePopupOAuth.ts`:
- Around line 26-31: The reset function currently nulls popupRef.current without
closing the window, leaving an open OAuth popup; update reset (the useCallback
named reset that calls stopPolling, manipulates popupRef, setIsLoading,
setError) to check if popupRef.current exists and is not closed (e.g.,
!popupRef.current.closed), call popupRef.current.close() inside a try/catch to
safely close the popup, then set popupRef.current = null and continue to call
stopPolling(), setIsLoading(false), and setError(null).
---
Outside diff comments:
In `@app/auth/callback/route.ts`:
- Line 59: Remove the plaintext console.log('[auth/callback] backend data:',
JSON.stringify(data)) in app/auth/callback/route.ts; either delete the call or
replace it with a sanitized/log-safe message that does not include
data.accessToken, data.userDetails, or any PII (only log non-sensitive status or
an opaque request id). Locate the statement that references "data" (the
console.log with '[auth/callback]') and ensure any logging uses redaction or
explicit whitelisting of safe fields before emitting to logs.
In `@src/app/layouts/ChannelLinkedLayout.tsx`:
- Around line 32-37: The current guard in ChannelLinkedLayout.tsx mounts
children even when the auth/channel checks fail (the if checking isInitializing
|| !user?.userChannelDetails?.youtubeChannelName ||
!user?.userChannelDetails?.youtubeChannelId), which causes protected subtree
effects before router.replace('/'). Change the early return so it does not
render or mount children (e.g., return a placeholder like <div
className='invisible' /> or null) instead of returning <div
className='invisible'>{children}</div>, ensuring children are only mounted after
the guard passes.
---
Nitpick comments:
In `@app/auth/youtube/route.ts`:
- Around line 4-36: There is duplicated OAuth-start logic in the GET handlers
(only the state prefix differs); extract a shared startOAuth(provider: string)
helper that encapsulates the mock redirect, cookie creation via
cookies().set('oauth_state', ...), URLSearchParams construction, and
NextResponse.redirect call; update the existing GET exports (e.g., the functions
named GET in both provider routes) to simply return startOAuth('youtube') or
startOAuth('google'), and make the helper accept a provider-specific state
prefix and optional scope override so provider-specific scopes (like YouTube’s)
can be passed in.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c26ce192-8173-4850-8a88-4b652f3ed69e
📒 Files selected for processing (18)
app/auth/callback/route.tsapp/auth/google/route.test.tsapp/auth/google/route.tsapp/auth/youtube/route.tssrc/app/layouts/AuthGuardLayout.tsxsrc/app/layouts/ChannelLinkedLayout.tsxsrc/app/styles/globals.csssrc/features/auth/model/useAuthInit.tssrc/features/auth/model/usePopupOAuth.tssrc/features/auth/model/useRequireAuth.test.tssrc/features/auth/model/useRequireAuth.tssrc/features/navigation/model/navItems.tssrc/features/navigation/model/types.tssrc/features/navigation/ui/NavMenuItem.tsxsrc/pages/home/ui/HomePage.tsxsrc/pages/main/ui/ChannelProfilePage.tsxsrc/shared/api/axiosInstance.tssrc/widgets/auth/ui/LoginModal.tsx
💤 Files with no reviewable changes (1)
- src/app/styles/globals.css
children이 마운트되면 그 안의 컴포넌트들이 즉시 실행되어 불필요한 호출이 발생하는 문제를 수정
reset은 로그인 취소용인데 기존 코드는 참조만 null로 비워서 OAuth 팝업 창이 화면에 그대로 남는 문제가 있었습니다. eset 함수 내 popupRef.current = null 앞에 팝업 닫기 로직을 추가했습니다.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/layouts/AuthGuardLayout.tsx (1)
15-19:⚠️ Potential issue | 🟠 Major | ⚡ Quick win모달 오픈과 동시에 redirect 레이스 가능—redirect 조건을 모달-오픈 이력 기반으로 보강 필요
useRequireAuth는!isInitializing && !isLoggedIn일 때 useEffect에서open()으로 로그인 모달을 여는데(src/features/auth/model/useRequireAuth.ts:12-17),AuthGuardLayout은 같은 커밋의 redirect useEffect에서 렌더 캡처된isModalOpen === false기준으로 즉시router.replace('/')를 실행할 수 있어 모달 기반 인증 플로우가 건너뛰어질 수 있습니다(src/app/layouts/AuthGuardLayout.tsx:15-19).가능한 수정 예시
-import { useEffect } from 'react' +import { useEffect, useRef } from 'react' @@ const { isLoggedIn, isInitializing } = useRequireAuth() const isModalOpen = useLoginModal((s) => s.isOpen) const router = useRouter() + const hasOpenedLoginModal = useRef(false) + + useEffect(() => { + if (isModalOpen) hasOpenedLoginModal.current = true + }, [isModalOpen]) @@ useEffect(() => { - if (!isInitializing && !isLoggedIn && !isModalOpen) { + if (!isInitializing && !isLoggedIn && hasOpenedLoginModal.current && !isModalOpen) { router.replace('/') } }, [isInitializing, isLoggedIn, isModalOpen, router])🤖 Prompt for 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. In `@src/app/layouts/AuthGuardLayout.tsx` around lines 15 - 19, The redirect race happens because AuthGuardLayout's effect uses the rendered isModalOpen (false) and immediately calls router.replace('/') while useRequireAuth may be about to open the login modal; fix by making the redirect guard aware of the modal-open request lifecycle: expose a flag from useRequireAuth (e.g., hasAttemptedToOpenModal or isModalOpenRequested) that is set when open() is invoked in useRequireAuth, then update AuthGuardLayout's effect to require !hasAttemptedToOpenModal (or !isModalOpenRequested) in addition to !isInitializing and !isLoggedIn and !isModalOpen before calling router.replace; use the existing symbols useRequireAuth, isInitializing, isLoggedIn, isModalOpen and router.replace to locate where to add the flag and change the effect condition.
🤖 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.
Outside diff comments:
In `@src/app/layouts/AuthGuardLayout.tsx`:
- Around line 15-19: The redirect race happens because AuthGuardLayout's effect
uses the rendered isModalOpen (false) and immediately calls router.replace('/')
while useRequireAuth may be about to open the login modal; fix by making the
redirect guard aware of the modal-open request lifecycle: expose a flag from
useRequireAuth (e.g., hasAttemptedToOpenModal or isModalOpenRequested) that is
set when open() is invoked in useRequireAuth, then update AuthGuardLayout's
effect to require !hasAttemptedToOpenModal (or !isModalOpenRequested) in
addition to !isInitializing and !isLoggedIn and !isModalOpen before calling
router.replace; use the existing symbols useRequireAuth, isInitializing,
isLoggedIn, isModalOpen and router.replace to locate where to add the flag and
change the effect condition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3617416a-f357-4e06-ab36-b3a4846ea098
📒 Files selected for processing (2)
src/app/layouts/AuthGuardLayout.tsxsrc/features/auth/model/usePopupOAuth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/features/auth/model/usePopupOAuth.ts
토큰 리프레시 성공 후 유저 정보를 반환하는 API 요청을 보내도록(/user/me)해 유저 상태를 업데이트 하도록 수정
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 작업 개요
🗂 작업 유형
✏️ 작업 내용
✅ 셀프 체크리스트
💬 리뷰어에게
Summary by CodeRabbit
New Features
Improvements
Bug Fixes