feat: 로그인 안한 상태로 페이지 접근 시 제한#42
Conversation
📝 WalkthroughWalkthrough클라이언트 측 인증 훅 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Layout as app/(with-sidebar)/layout.tsx
participant Hook as hooks/useAuthGuard.ts
participant Firebase as Firebase Auth
participant Router as Next.js Router
participant Sidebar as components/common/Sidebar.tsx
User->>Layout: 사이드바 페이지 접근
Layout->>Hook: useAuthGuard() 호출
Hook->>Firebase: onAuthStateChanged 구독
Firebase-->>Hook: 인증 상태 이벤트
alt 로딩 중
Hook-->>Layout: { loading: true }
Layout-->>User: "로딩중..." 표시
else 미인증
Hook-->>Router: router.replace('/login')
Router-->>User: 로그인 페이지로 리다이렉트
else 인증됨
Hook-->>Layout: { isAuthed: true, user }
Layout-->>Sidebar: 사이드바 및 자식 렌더링
User->>Sidebar: 로그아웃 클릭
Sidebar->>Firebase: signOut()
Firebase-->>Sidebar: signOut 완료
Sidebar-->>User: alert("로그아웃 되었습니다.")
Sidebar-->>Router: router.replace('/landing')
Router-->>User: 랜딩 페이지로 리다이렉트
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
시
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 3
🤖 Fix all issues with AI agents
In `@app/`(with-sidebar)/layout.tsx:
- Around line 17-20: The layout currently calls alert('로그인이 필요합니다.') directly
during render when isAuthed is false, which is an anti-pattern; remove the alert
from the render path and either (a) simply return null when isAuthed is false
(since useAuthGuard already handles redirect), or (b) if you must show a
one-time alert, trigger it inside a useEffect in the layout component (or inside
useAuthGuard) so it runs only once on change of isAuthed; update the conditional
in layout.tsx (the component using isAuthed) accordingly and ensure no side
effects occur during render.
In `@hooks/useAuthGuard.ts`:
- Line 22: The default redirectTo in useAuthGuard ('/login') conflicts with
Sidebar.tsx which redirects to '/landing' on logout, causing a second redirect
when onAuthStateChanged fires; fix by making redirect behavior consistent—either
change useAuthGuard's default redirectTo to '/landing' or add an escape flag
(e.g., skipRedirectOnLogout) to useAuthGuard and have Sidebar set that flag when
calling signOut so onAuthStateChanged does not trigger the guard redirect;
update the useAuthGuard logic (and the Sidebar logout call) to check this new
flag or new default and only perform redirect when appropriate.
- Around line 28-41: The onAuthStateChanged handler currently sets
setLoading(true) when u is null which leaves loading stuck true and causes
premature redirects; update the effect (the onAuthStateChanged callback in
useEffect) to always setLoading(false) after processing the auth result and
avoid redirecting on the initial transient null by introducing an initialization
guard (e.g., a ref or an "initialized" state): when the callback runs, setUser(u
?? null), setLoading(false), and if u is null only call
router.replace(redirectTo) after the initialization flag is true (set the flag
to true on the first callback) so transient nulls don’t trigger redirects (refer
to the onAuthStateChanged callback, setUser, setLoading, router.replace, and
redirectTo).
🧹 Nitpick comments (2)
components/common/Sidebar.tsx (1)
26-35: 로그아웃 로직 변경 확인됨로그아웃 성공 시 사용자에게 피드백을 제공하는 것은 좋습니다. 다만
alert()은 브라우저를 블로킹하고 UX 측면에서 다소 구식입니다. 추후 toast 알림(예: react-hot-toast, sonner)으로 개선을 고려해 보세요.현재 기능적으로는 문제가 없으므로 승인합니다.
app/(with-sidebar)/layout.tsx (1)
16-16: 로딩 UI 개선 고려현재 단순 텍스트 "로딩중..."을 표시하고 있습니다. UX 향상을 위해 스피너나 스켈레톤 UI 사용을 고려해 보세요.
| useEffect(() => { | ||
| const unsub = onAuthStateChanged(auth, (u) => { | ||
| if (!u) { | ||
| setUser(null); | ||
| setLoading(true); | ||
| router.replace(redirectTo); | ||
| return; | ||
| } | ||
| setUser(u); | ||
| setLoading(false); | ||
| }); | ||
|
|
||
| return () => unsub(); | ||
| }, [router, redirectTo]); |
There was a problem hiding this comment.
loading 상태가 미인증 시 false로 전환되지 않는 문제
사용자가 미인증일 때 setLoading(true)를 호출하고 있어 loading이 계속 true인 채로 남습니다. 이로 인해:
layout.tsx에서if (!isAuthed)블록의 alert가 절대 실행되지 않음 (loading이true이므로 먼저 반환됨)- 리다이렉트 중 사용자가 "로딩중..." 화면만 계속 보게 됨
또한 Firebase onAuthStateChanged는 초기에 persisted auth state를 확인하기 전에 null로 콜백될 수 있어, 실제로 로그인된 사용자도 잠시 미인증으로 판단되어 불필요한 리다이렉트가 발생할 수 있습니다.
🔧 수정 제안
useEffect(() => {
const unsub = onAuthStateChanged(auth, (u) => {
if (!u) {
setUser(null);
- setLoading(true);
+ setLoading(false);
router.replace(redirectTo);
return;
}
setUser(u);
setLoading(false);
});
return () => unsub();
}, [router, redirectTo]);🤖 Prompt for AI Agents
In `@hooks/useAuthGuard.ts` around lines 28 - 41, The onAuthStateChanged handler
currently sets setLoading(true) when u is null which leaves loading stuck true
and causes premature redirects; update the effect (the onAuthStateChanged
callback in useEffect) to always setLoading(false) after processing the auth
result and avoid redirecting on the initial transient null by introducing an
initialization guard (e.g., a ref or an "initialized" state): when the callback
runs, setUser(u ?? null), setLoading(false), and if u is null only call
router.replace(redirectTo) after the initialization flag is true (set the flag
to true on the first callback) so transient nulls don’t trigger redirects (refer
to the onAuthStateChanged callback, setUser, setLoading, router.replace, and
redirectTo).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@middleware.ts`:
- Around line 4-22: The middleware currently trusts the client-set isLoggedIn
cookie (set in LoginForm.tsx via document.cookie) allowing auth bypass; update
middleware(request: NextRequest) to validate authentication server-side instead
of reading this client cookie — either verify Firebase ID/session tokens using
the Firebase Admin SDK or switch to Firebase Auth createSessionCookie() and read
an HttpOnly, Secure session cookie in middleware, and replace the simple cookie
check with token verification logic before calling NextResponse.redirect/new
URL(...) or NextResponse.next(); also ensure every code path returns explicitly
(add an explicit NextResponse.next() at the end of middleware) so intent is
clear.
🧹 Nitpick comments (2)
components/auth/LoginForm.tsx (1)
25-29: 쿠키 설정 시 보안 속성 추가 권장쿠키에
SameSite와Secure속성이 누락되어 있습니다. 프로덕션 환경에서는 보안을 위해 이 속성들을 추가하는 것이 좋습니다.또한 중요한 보안 고려사항: 이 쿠키는 클라이언트에서 설정되고 HttpOnly가 아니므로, 악의적인 사용자가 수동으로
isLoggedIn=true쿠키를 설정하여 미들웨어 인증을 우회할 수 있습니다. 미들웨어에서 실제 Firebase 세션 토큰을 검증하거나, 서버 측에서 쿠키를 설정하는 방식을 고려해보세요.🔒 쿠키 속성 개선 제안
const setLoginCookie = () => { - // 7일간 유지되는 쿠키 설정 - document.cookie = 'isLoggedIn=true; path=/; max-age=' + 60 * 60 * 24 * 7; + // 7일간 유지되는 쿠키 설정 (보안 속성 포함) + const maxAge = 60 * 60 * 24 * 7; + document.cookie = `isLoggedIn=true; path=/; max-age=${maxAge}; SameSite=Lax${window.location.protocol === 'https:' ? '; Secure' : ''}`; };middleware.ts (1)
24-26: 보호 경로 범위 확인 필요현재 matcher는
/와/landing만 커버합니다. 다른 보호가 필요한 경로(예:/write,/profile등)는 클라이언트 측useAuthGuard에 의존하게 됩니다.서버 측에서도 이러한 경로들을 보호하려면 matcher 확장을 고려해보세요.
♻️ 확장된 matcher 예시
export const config = { - matcher: ['/', '/landing'], + matcher: ['/', '/landing', '/write/:path*', '/profile/:path*'], };
| export function middleware(request: NextRequest) { | ||
| const { pathname } = request.nextUrl; | ||
|
|
||
| // 쿠키에서 isLoggedIn 확인 | ||
| const isLoggedIn = request.cookies.get('isLoggedIn')?.value === 'true'; | ||
|
|
||
| if (pathname === '/') { | ||
| // 로그인이 안 된 경우에만 랜딩으로 보냄 | ||
| if (!isLoggedIn) { | ||
| return NextResponse.redirect(new URL('/landing', request.url)); | ||
| } | ||
| // 로그인 된 상태라면 그대로 루트('/')를 보여줌 | ||
| return NextResponse.next(); | ||
| } | ||
|
|
||
| if (pathname === '/landing' && isLoggedIn) { | ||
| return NextResponse.redirect(new URL('/', request.url)); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the middleware file and related auth configuration
git ls-files | grep -E "(middleware|auth|cookie|firebase)" | head -20Repository: DeveloperBlog-Devflow/devflow
Length of output: 428
🏁 Script executed:
# Look for where isLoggedIn cookie is set
rg "isLoggedIn" -t typescript -t javascript -B 3 -A 3Repository: DeveloperBlog-Devflow/devflow
Length of output: 107
🏁 Script executed:
# Search for cookie configuration and HttpOnly settings
rg "cookies\." -t typescript -t javascript -B 2 -A 2 | head -50Repository: DeveloperBlog-Devflow/devflow
Length of output: 107
🏁 Script executed:
# Check for Firebase configuration
rg -i "firebase" -t typescript -t javascript -t json | head -20Repository: DeveloperBlog-Devflow/devflow
Length of output: 107
🏁 Script executed:
# Look for auth-related setup and session configuration
fd -e "ts" -e "js" -e "json" | xargs grep -l -E "(auth|session|login)" 2>/dev/null | head -10Repository: DeveloperBlog-Devflow/devflow
Length of output: 160
🏁 Script executed:
# Read the middleware file completely
cat -n middleware.tsRepository: DeveloperBlog-Devflow/devflow
Length of output: 936
🏁 Script executed:
# Look for where isLoggedIn cookie is set (likely in LoginForm)
cat -n "components/auth/LoginForm.tsx"Repository: DeveloperBlog-Devflow/devflow
Length of output: 4636
🏁 Script executed:
# Check the Firebase configuration
cat -n "lib/firebase.ts"Repository: DeveloperBlog-Devflow/devflow
Length of output: 1373
🏁 Script executed:
# Check signup service for auth handling
cat -n "services/auth/signup.service.ts"Repository: DeveloperBlog-Devflow/devflow
Length of output: 1900
🏁 Script executed:
# Search for any server-side auth handling or API routes
fd -e ts -e js | xargs grep -l "NextRequest\|NextResponse\|route.ts" 2>/dev/null | head -10Repository: DeveloperBlog-Devflow/devflow
Length of output: 86
🏁 Script executed:
# Check if there are any server-side auth routes or session handling
fd -e ts -e js "api.*auth\|api.*session\|route" 2>/dev/null | head -10Repository: DeveloperBlog-Devflow/devflow
Length of output: 55
🏁 Script executed:
# Look for any logout functionality or cookie clearing
rg "isLoggedIn|cookies\|logout" --type ts --type js -B 2 -A 2 | head -40Repository: DeveloperBlog-Devflow/devflow
Length of output: 1324
보안 취약점: 클라이언트 쿠키 기반 라우팅 인증 우회 가능
현재 미들웨어는 isLoggedIn 쿠키의 값만으로 라우팅을 결정하고 있습니다. 이 쿠키는 LoginForm.tsx의 document.cookie = 'isLoggedIn=true'로 클라이언트에서 설정되므로, 악의적인 사용자가 브라우저 콘솔에서 document.cookie = 'isLoggedIn=true'를 실행하여 인증을 우회할 수 있습니다. 미들웨어는 이 쿠키 값을 검증 없이 신뢰하고 있습니다.
이미 Firebase Auth를 통해 실제 인증을 처리하고 있으므로, 다음 중 하나의 방식으로 개선할 것을 권장합니다:
- Firebase Admin SDK를 사용하여 미들웨어에서 서버 측 세션 토큰 검증
- Firebase Auth의
createSessionCookie()기능을 활용하여 HttpOnly + Secure 플래그가 적용된 안전한 쿠키 사용
더불어, 명시적인 NextResponse.next() 반환을 추가하면 코드의 의도가 더욱 명확해집니다.
🔒 명시적 반환 추가 제안
if (pathname === '/landing' && isLoggedIn) {
return NextResponse.redirect(new URL('/', request.url));
}
+
+ return NextResponse.next();
}🤖 Prompt for AI Agents
In `@middleware.ts` around lines 4 - 22, The middleware currently trusts the
client-set isLoggedIn cookie (set in LoginForm.tsx via document.cookie) allowing
auth bypass; update middleware(request: NextRequest) to validate authentication
server-side instead of reading this client cookie — either verify Firebase
ID/session tokens using the Firebase Admin SDK or switch to Firebase Auth
createSessionCookie() and read an HttpOnly, Secure session cookie in middleware,
and replace the simple cookie check with token verification logic before calling
NextResponse.redirect/new URL(...) or NextResponse.next(); also ensure every
code path returns explicitly (add an explicit NextResponse.next() at the end of
middleware) so intent is clear.
1 participant
1) 작업한 이슈번호
2) 변경 요약 (What & Why)
3) 스크린샷/동영상 (UI 변경 시)
4) 상세 변경사항
5) 참고사항
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.