feat: 로그인 끊김/실패 시 복구 UX 개선#387
Open
sumi-0011 wants to merge 2 commits into
Open
Conversation
- API 401 시 무음 signOut 대신 차단형 Dialog로 만료 사실 안내 - 보호 라우트 무음 리다이렉트를 ?session=expired&callbackUrl 보존 리다이렉트로 변경 - 재로그인 후 원래 페이지로 자동 복귀 (기존 login helper + LoginButton 콜백 재사용) - 다중 401 single-fire 가드와 401 발생 시 세션 캐시 무효화 포함 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- /auth/desktop: 자동 OAuth redirect 제거하고 명시적 'GitHub으로 계속하기' CTA로 변경 (무한 루프 차단) - /auth/desktop: ?error= 도착 시 안내 배너 추가 - /auth/error 페이지 신설: NextAuth가 보내는 인증 실패를 404 대신 친절히 처리, localStorage callbackUrl 기반으로 데스크톱 흐름 자동 복귀 - middleware: /auth/* 하위 경로 전체를 public으로 처리 (error/signOut 등 새 auth 페이지 보호) - 메시지: ko_KR/en_US Auth 섹션에 desktop·error 카피 추가 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 Walkthrough워크스루미들웨어에서 토큰 없는 요청을 감지하여 변경사항세션 만료 흐름
🎯 3 (보통) | ⏱️ ~25분관련된 가능성 있는 PR
추천 라벨
추천 검토자
시
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 5
🤖 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 `@apps/web/src/apis/interceptor.ts`:
- Around line 76-81: The 401 handling currently only clears session cache inside
the browser branch so server-side 401s keep using an expired token; move the
call to clearSessionCache() so it runs unconditionally when
error?.response?.status === 401 (before the typeof window check), keep
triggerSessionExpired(window.location.pathname + window.location.search) only
inside the client branch, and still throw the same
CustomException('TOKEN_EXPIRED', 'token expired') afterward; update the block
around the error?.response?.status === 401 check to call clearSessionCache() for
all environments and call triggerSessionExpired() only when typeof window !==
'undefined'.
In `@apps/web/src/app/`[locale]/auth/desktop/page.tsx:
- Around line 35-36: The hardcoded Korean strings for the error branch should be
replaced with translation keys via the existing useTranslations('Auth') hook:
swap the literal h1 and p contents with t('invalidRequest.title') and
t('invalidRequest.description') (or similar keys you add) while keeping the
existing className props (titleCss and descCss); ensure those keys are added to
the Auth locale files for both Korean and English so the error UI displays in
the current locale.
In `@apps/web/src/components/Global/SessionExpiredQueryWatcher.tsx`:
- Around line 8-20: The effect only runs once on mount so it misses later query
changes; update the watcher so it re-evaluates when the URL query changes by
basing the effect on a stable location search value (e.g. use the router's
useLocation().search or otherwise pass window.location.search as a dependency)
and/or listen for navigation events; modify the useEffect that currently calls
URLSearchParams, triggerSessionExpired, and window.history.replaceState to
depend on that location search value (or add a popstate listener) so
session=expired is handled whenever the query changes.
In `@apps/web/src/middleware.ts`:
- Around line 42-49: The redirect currently treats any missing token from
getToken({ req }) as "session=expired"; change the logic so only true
session-expiration cases set session=expired. Specifically, in the middleware
where getToken, extractLocale, routing.defaultLocale and NextResponse.redirect
are used: if getToken returns null (anonymous user) redirect to the login/locale
callback without setting session=expired (use a different param like
reason=anonymous or no param), and only set
redirectUrl.searchParams.set('session','expired') when you can detect an expired
token (e.g., token present but expired or contains an explicit error flag)
before calling NextResponse.redirect. Ensure callbackUrl handling remains the
same.
In `@apps/web/src/utils/sessionExpired.ts`:
- Around line 13-18: The isAuthPath helper is too permissive because
pathname.startsWith('/auth') matches /author and similar; change isAuthPath
(used by triggerSessionExpired) to use a boundary regex (e.g. /^\/auth(\/|$)/)
instead of startsWith so it only matches exactly /auth or /auth/*; update the
function definition for isAuthPath to combine that regex with the existing
/\/[^/]+\/auth(\/|$)/ test to preserve other auth patterns.
🪄 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: CHILL
Plan: Pro
Run ID: 96f86974-b83b-4193-a746-9f6fc93c7c83
📒 Files selected for processing (10)
apps/web/messages/en_US.jsonapps/web/messages/ko_KR.jsonapps/web/src/apis/interceptor.tsapps/web/src/app/[locale]/auth/desktop/page.tsxapps/web/src/app/[locale]/auth/error/page.tsxapps/web/src/components/Global/GlobalComponent.tsxapps/web/src/components/Global/SessionExpiredDialog.tsxapps/web/src/components/Global/SessionExpiredQueryWatcher.tsxapps/web/src/middleware.tsapps/web/src/utils/sessionExpired.ts
Comment on lines
76
to
+81
| if (error?.response?.status === 401) { | ||
| if (typeof window !== 'undefined') { | ||
| signOut(); | ||
| clearSessionCache(); | ||
| triggerSessionExpired(window.location.pathname + window.location.search); | ||
| } | ||
| throw new CustomException('TOKEN_EXPIRED', 'token expired and sign out success'); | ||
| throw new CustomException('TOKEN_EXPIRED', 'token expired'); |
There was a problem hiding this comment.
서버에서 난 401은 세션 캐시를 비우지 못합니다.
지금은 clearSessionCache() 가 브라우저 분기 안에만 있어서, 서버 렌더링/서버 호출에서 401이 발생하면 같은 워커가 최대 5분 동안 만료된 access token을 계속 재사용할 수 있습니다. 캐시 무효화는 환경과 무관하게 먼저 수행하고, triggerSessionExpired() 만 클라이언트에서 호출해야 합니다.
🔧 제안 수정
export const interceptorResponseRejected = async (error: AxiosError<ApiErrorScheme>) => {
if (error?.response?.status === 401) {
- if (typeof window !== 'undefined') {
- clearSessionCache();
+ clearSessionCache();
+ if (typeof window !== 'undefined') {
triggerSessionExpired(window.location.pathname + window.location.search);
}
throw new CustomException('TOKEN_EXPIRED', 'token expired');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (error?.response?.status === 401) { | |
| if (typeof window !== 'undefined') { | |
| signOut(); | |
| clearSessionCache(); | |
| triggerSessionExpired(window.location.pathname + window.location.search); | |
| } | |
| throw new CustomException('TOKEN_EXPIRED', 'token expired and sign out success'); | |
| throw new CustomException('TOKEN_EXPIRED', 'token expired'); | |
| if (error?.response?.status === 401) { | |
| clearSessionCache(); | |
| if (typeof window !== 'undefined') { | |
| triggerSessionExpired(window.location.pathname + window.location.search); | |
| } | |
| throw new CustomException('TOKEN_EXPIRED', 'token expired'); |
🤖 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 `@apps/web/src/apis/interceptor.ts` around lines 76 - 81, The 401 handling
currently only clears session cache inside the browser branch so server-side
401s keep using an expired token; move the call to clearSessionCache() so it
runs unconditionally when error?.response?.status === 401 (before the typeof
window check), keep triggerSessionExpired(window.location.pathname +
window.location.search) only inside the client branch, and still throw the same
CustomException('TOKEN_EXPIRED', 'token expired') afterward; update the block
around the error?.response?.status === 401 check to call clearSessionCache() for
all environments and call triggerSessionExpired() only when typeof window !==
'undefined'.
Comment on lines
35
to
+36
| <h1 className={titleCss}>잘못된 요청</h1> | ||
| <p className={descCss}> | ||
| redirect_uri가 허용 범위를 벗어났거나 필수 파라미터가 누락되었습니다. | ||
| </p> | ||
| <p className={descCss}>redirect_uri가 허용 범위를 벗어났거나 필수 파라미터가 누락되었습니다.</p> |
There was a problem hiding this comment.
유효성 실패 분기가 한국어 하드코딩이라 영어 로케일에서 안내가 깨집니다.
useTranslations('Auth')를 이미 쓰고 있으므로 이 분기도 번역 키를 사용해 로케일 일관성을 맞춰주세요.
수정 예시
- <h1 className={titleCss}>잘못된 요청</h1>
- <p className={descCss}>redirect_uri가 허용 범위를 벗어났거나 필수 파라미터가 누락되었습니다.</p>
+ <h1 className={titleCss}>{t('desktopInvalidRequestTitle')}</h1>
+ <p className={descCss}>{t('desktopInvalidRequestDescription')}</p>추가 키 예시:
// apps/web/messages/en_US.json
"Auth": {
+ "desktopInvalidRequestTitle": "Invalid request",
+ "desktopInvalidRequestDescription": "The redirect_uri is out of the allowed range or a required parameter is missing.",
...
}// apps/web/messages/ko_KR.json
"Auth": {
+ "desktopInvalidRequestTitle": "잘못된 요청",
+ "desktopInvalidRequestDescription": "redirect_uri가 허용 범위를 벗어났거나 필수 파라미터가 누락되었습니다.",
...
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <h1 className={titleCss}>잘못된 요청</h1> | |
| <p className={descCss}> | |
| redirect_uri가 허용 범위를 벗어났거나 필수 파라미터가 누락되었습니다. | |
| </p> | |
| <p className={descCss}>redirect_uri가 허용 범위를 벗어났거나 필수 파라미터가 누락되었습니다.</p> | |
| <h1 className={titleCss}>{t('desktopInvalidRequestTitle')}</h1> | |
| <p className={descCss}>{t('desktopInvalidRequestDescription')}</p> |
🤖 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 `@apps/web/src/app/`[locale]/auth/desktop/page.tsx around lines 35 - 36, The
hardcoded Korean strings for the error branch should be replaced with
translation keys via the existing useTranslations('Auth') hook: swap the literal
h1 and p contents with t('invalidRequest.title') and
t('invalidRequest.description') (or similar keys you add) while keeping the
existing className props (titleCss and descCss); ensure those keys are added to
the Auth locale files for both Korean and English so the error UI displays in
the current locale.
Comment on lines
+8
to
+20
| useEffect(() => { | ||
| const params = new URLSearchParams(window.location.search); | ||
| if (params.get('session') !== 'expired') return; | ||
|
|
||
| const callbackUrl = params.get('callbackUrl'); | ||
| triggerSessionExpired(callbackUrl); | ||
|
|
||
| params.delete('session'); | ||
| params.delete('callbackUrl'); | ||
| const query = params.toString(); | ||
| const nextUrl = window.location.pathname + (query ? `?${query}` : ''); | ||
| window.history.replaceState(null, '', nextUrl); | ||
| }, []); |
There was a problem hiding this comment.
쿼리 감시가 최초 1회만 실행되어 이후 session=expired 유입을 놓칠 수 있습니다.
현재 구현은 마운트 시점 이후의 URL 쿼리 변경을 감지하지 못합니다. 전역 watcher라면 쿼리 변경 시에도 재평가되도록 의존성을 연결하는 편이 안전합니다.
수정 예시
'use client';
import { useEffect } from 'react';
+import { useSearchParams } from 'next/navigation';
import { triggerSessionExpired } from '@/utils/sessionExpired';
export function SessionExpiredQueryWatcher() {
+ const searchParams = useSearchParams();
+
useEffect(() => {
- const params = new URLSearchParams(window.location.search);
+ const params = new URLSearchParams(searchParams.toString());
if (params.get('session') !== 'expired') return;
const callbackUrl = params.get('callbackUrl');
triggerSessionExpired(callbackUrl);
params.delete('session');
params.delete('callbackUrl');
const query = params.toString();
- const nextUrl = window.location.pathname + (query ? `?${query}` : '');
+ const nextUrl = window.location.pathname + (query ? `?${query}` : '') + window.location.hash;
window.history.replaceState(null, '', nextUrl);
- }, []);
+ }, [searchParams]);
return null;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| const params = new URLSearchParams(window.location.search); | |
| if (params.get('session') !== 'expired') return; | |
| const callbackUrl = params.get('callbackUrl'); | |
| triggerSessionExpired(callbackUrl); | |
| params.delete('session'); | |
| params.delete('callbackUrl'); | |
| const query = params.toString(); | |
| const nextUrl = window.location.pathname + (query ? `?${query}` : ''); | |
| window.history.replaceState(null, '', nextUrl); | |
| }, []); | |
| 'use client'; | |
| import { useEffect } from 'react'; | |
| import { useSearchParams } from 'next/navigation'; | |
| import { triggerSessionExpired } from '@/utils/sessionExpired'; | |
| export function SessionExpiredQueryWatcher() { | |
| const searchParams = useSearchParams(); | |
| useEffect(() => { | |
| const params = new URLSearchParams(searchParams.toString()); | |
| if (params.get('session') !== 'expired') return; | |
| const callbackUrl = params.get('callbackUrl'); | |
| triggerSessionExpired(callbackUrl); | |
| params.delete('session'); | |
| params.delete('callbackUrl'); | |
| const query = params.toString(); | |
| const nextUrl = window.location.pathname + (query ? `?${query}` : '') + window.location.hash; | |
| window.history.replaceState(null, '', nextUrl); | |
| }, [searchParams]); | |
| return null; | |
| } |
🤖 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 `@apps/web/src/components/Global/SessionExpiredQueryWatcher.tsx` around lines 8
- 20, The effect only runs once on mount so it misses later query changes;
update the watcher so it re-evaluates when the URL query changes by basing the
effect on a stable location search value (e.g. use the router's
useLocation().search or otherwise pass window.location.search as a dependency)
and/or listen for navigation events; modify the useEffect that currently calls
URLSearchParams, triggerSessionExpired, and window.history.replaceState to
depend on that location search value (or add a popstate listener) so
session=expired is handled whenever the query changes.
Comment on lines
+42
to
+49
| const token = await getToken({ req }); | ||
| if (!token) { | ||
| const locale = extractLocale(req.nextUrl.pathname) ?? routing.defaultLocale; | ||
| const callbackUrl = req.nextUrl.pathname + req.nextUrl.search; | ||
| const redirectUrl = new URL(`/${locale}`, req.url); | ||
| redirectUrl.searchParams.set('session', 'expired'); | ||
| redirectUrl.searchParams.set('callbackUrl', callbackUrl); | ||
| return NextResponse.redirect(redirectUrl); |
There was a problem hiding this comment.
비로그인 접근까지 session=expired 로 보내면 안내가 틀어집니다.
이 분기는 토큰이 없는 모든 요청을 session=expired 로 리다이렉트합니다. 그래서 보호 라우트를 처음 여는 익명 사용자도 “세션이 만료되었어요” 차단형 플로우를 보게 됩니다. session=expired 는 실제 만료가 확인된 경우로 한정하고, 단순 비로그인 접근은 별도 reason 값이나 일반 로그인 진입으로 분리하는 편이 맞습니다.
🤖 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 `@apps/web/src/middleware.ts` around lines 42 - 49, The redirect currently
treats any missing token from getToken({ req }) as "session=expired"; change the
logic so only true session-expiration cases set session=expired. Specifically,
in the middleware where getToken, extractLocale, routing.defaultLocale and
NextResponse.redirect are used: if getToken returns null (anonymous user)
redirect to the login/locale callback without setting session=expired (use a
different param like reason=anonymous or no param), and only set
redirectUrl.searchParams.set('session','expired') when you can detect an expired
token (e.g., token present but expired or contains an explicit error flag)
before calling NextResponse.redirect. Ensure callbackUrl handling remains the
same.
Comment on lines
+13
to
+18
| const isAuthPath = (pathname: string) => pathname.startsWith('/auth') || /^\/[^/]+\/auth(\/|$)/.test(pathname); | ||
|
|
||
| export const triggerSessionExpired = (callbackUrl?: string | null) => { | ||
| if (typeof window === 'undefined') return; | ||
|
|
||
| if (isAuthPath(window.location.pathname)) return; |
There was a problem hiding this comment.
/auth 경로 판별이 과하게 넓습니다.
Line 13의 pathname.startsWith('/auth') 는 /author, /authentic 같은 보호 경로도 인증 페이지로 오인합니다. 그러면 해당 경로에서는 세션 만료 다이얼로그가 열리지 않아 복구 플로우가 끊깁니다. 미들웨어와 동일하게 /auth(/|$) 경계 기준으로 맞추는 편이 안전합니다.
🔧 제안 수정
-const isAuthPath = (pathname: string) => pathname.startsWith('/auth') || /^\/[^/]+\/auth(\/|$)/.test(pathname);
+const isAuthPath = (pathname: string) =>
+ /^\/auth(\/|$)/.test(pathname) || /^\/[^/]+\/auth(\/|$)/.test(pathname);🤖 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 `@apps/web/src/utils/sessionExpired.ts` around lines 13 - 18, The isAuthPath
helper is too permissive because pathname.startsWith('/auth') matches /author
and similar; change isAuthPath (used by triggerSessionExpired) to use a boundary
regex (e.g. /^\/auth(\/|$)/) instead of startsWith so it only matches exactly
/auth or /auth/*; update the function definition for isAuthPath to combine that
regex with the existing /\/[^/]+\/auth(\/|$)/ test to preserve other auth
patterns.
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
기존에는 세션이 만료되거나
/auth/desktop인증이 실패하면 사용자가 "왜 멈췄는지" 모르고 화면이 끊겼습니다. 두 케이스 모두 명확한 안내와 복귀 동선을 추가했습니다.1. 세션 만료 UX
signOut(), 보호 라우트 비로그인 접근 →/로 무음 리다이렉트login()helper 재사용?session=expired&callbackUrl=...query를 붙여 같은 UX로 통일2.
/auth/desktop비로그인/실패 케이스login()호출 → 사용자에게 안내 없이 OAuth로 점프, 실패 시 무한 redirect 루프 위험?error=도착 시 안내 배너 표시/auth/error페이지 신설: 기존엔 NextAuth 인증 실패 시 404였음. 새 페이지가 친절히 안내하고 localStorage callbackUrl 기반으로 데스크톱 흐름으로 복귀 가능/auth/*하위 경로 전체를 public으로 처리 (error/signOut 등 보호)Test plan
pnpm --filter @gitanimals/web type-check통과pnpm --filter @gitanimals/web lint신규 경고 0건?error=배너 노출/auth/error가 404 아니고 에러 코드 표시/auth/desktop?...복귀 +redirect_uri보존authorize실패 시/auth/error도달하는지 수동 확인_mobile반응형 확인🤖 Generated with Claude Code
Summary by CodeRabbit
릴리스 노트
새로운 기능
다국어 지원