feat(web): 인증 refresh 및 라우트 보호 연결 - #76
Conversation
…to feature/67-auth-session-me # Conflicts: # apps/web/src/features/auth/components/login-form.tsx # apps/web/src/features/auth/components/signup-form.tsx
…ssion-me # Conflicts: # apps/web/src/app/(onboarding)/layout.tsx # apps/web/src/features/auth/components/login-form.tsx # apps/web/src/features/auth/components/signup-form.tsx
|
Warning Review limit reached
More reviews will be available in 58 minutes and 7 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. 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 ignored due to path filters (1)
📒 Files selected for processing (5)
Walkthrough모듈 스코프 액세스 토큰 유틸( ChangesAuth Context 기반 세션 관리 리팩터
Pretendard 폰트 로딩 방식 변경
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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 |
…ute-refresh # Conflicts: # apps/web/src/app/layout.tsx # apps/web/src/app/providers.tsx # apps/web/src/features/auth/components/login-form.tsx # apps/web/src/features/auth/components/signup-form.tsx # apps/web/src/features/auth/queries/auth-session.ts # apps/web/src/lib/api-client.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/app/providers.tsx`:
- Around line 26-29: The app-wide wrapping in `AuthProvider` is causing the
initial render to return null and blank the whole page until configuration
completes. Update `AuthProvider` so its initialization does not block rendering
for all routes, or limit the provider to only the minimal subtree that actually
needs it. Use the `AuthProvider` component and its `if (!configured) return
null;` behavior as the main place to adjust so first paint/SSR is preserved.
In `@apps/web/src/features/auth/components/auth-provider.tsx`:
- Around line 52-75: The session-clearing flow in auth-provider.tsx allows an
in-flight refreshAccessToken() started via refreshPromiseRef to still resolve
and call setAccessToken after clearSession() has run. Update clearSession() and
refreshAccessToken() so that logout/401 cleanup invalidates any pending refresh
(for example by bumping a generation/abort flag or resetting the ref state) and
make the refresh resolution path verify it is still current before writing
session state. Use the existing clearSession, refreshAccessToken, and
refreshPromiseRef symbols to ensure stale refresh results cannot restore
accessToken after session cleanup.
In `@apps/web/src/features/auth/components/require-guest-auth.tsx`:
- Around line 19-42: The guest-auth guard in require-guest-auth.tsx currently
returns children immediately, which lets `/login` and `/signup` render before
refreshAccessToken() and getMe() finish. Update RequireGuestAuth so it tracks an
authentication-checking state in the useEffect flow around
redirectIfAuthenticated, and render null or a loading shell until the
restoration attempt completes. Only render children once the user is confirmed
unauthenticated; keep the existing router.replace logic and ignore flag behavior
in place.
In `@apps/web/src/lib/api-client.ts`:
- Around line 156-159: The refresh retry path in `sendRequest` is bypassing the
same network-error normalization as the initial request, so a failed retry can
leak a raw TypeError instead of ApiError(0). Update the retry logic inside
`api-client.ts` so the second `sendRequest()` after
`authHandlers.refreshAccessToken()` goes through the same try/catch or shared
error-handling path as the first request, keeping the error contract consistent
in the 401 refresh flow.
🪄 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: 7cd7c763-1dbc-4bc2-9f13-0a227a2fbc10
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/web/package.jsonapps/web/src/app/(auth)/layout.tsxapps/web/src/app/globals.cssapps/web/src/app/layout.tsxapps/web/src/app/providers.tsxapps/web/src/features/auth/api/auth-api.tsapps/web/src/features/auth/components/auth-provider.tsxapps/web/src/features/auth/components/login-form.tsxapps/web/src/features/auth/components/require-guest-auth.tsxapps/web/src/features/auth/components/signup-form.tsxapps/web/src/features/auth/queries/auth-query-keys.tsapps/web/src/features/auth/queries/auth-session.tsapps/web/src/lib/access-token.tsapps/web/src/lib/api-client.ts
💤 Files with no reviewable changes (2)
- apps/web/src/lib/access-token.ts
- apps/web/src/app/globals.css
| const pretendard = localFont({ | ||
| display: "swap", | ||
| src: "../../../../node_modules/pretendard/dist/web/variable/woff2/PretendardVariable.woff2", |
There was a problem hiding this comment.
[제안]
next build에서 Pretendard 폰트 파일을 resolve하지 못해 production build가 실패합니다. pretendard는 apps/web/package.json의 dependency라서 root node_modules를 가정하는 ../../../../node_modules/... 경로 대신 apps/web package 기준으로 resolve되는 경로를 사용하거나, 폰트 파일을 앱 내부 asset으로 두는 방식은 어떠신가요?
There was a problem hiding this comment.
폰트 파일을 앱 내부 asset으로 두는 방식으로 수정하겠습니다!
| useEffect(() => { | ||
| let ignore = false; | ||
|
|
||
| async function redirectIfAuthenticated() { | ||
| try { | ||
| await refreshAccessToken(); | ||
| const authMe = await getMe({ redirectOnUnauthorized: false }); | ||
|
|
||
| if (!ignore) { | ||
| router.replace(authMe.organization ? "/dashboard" : "/organization/new"); | ||
| } | ||
| } catch { | ||
| // Guest routes should stay usable when no session can be restored. | ||
| } finally { | ||
| if (!ignore) { | ||
| setIsCheckingSession(false); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| redirectIfAuthenticated(); |
There was a problem hiding this comment.
[제안]
현재 guest/auth 라우트와 admin/onboarding 라우트의 redirect가 모두 client guard에서 처리되고 있습니다. 조직 유무처럼 /auth/me 조회가 필요한 분기는 현재 guard에 남기는 게 자연스럽지만, refresh token cookie 존재 여부만으로 판단 가능한 1차 접근 제어는 Next middleware에서 처리하는 것도 검토해볼 수 있을 것 같습니다.
예를 들어 미인증 사용자의 admin/onboarding 접근은 렌더링 전에 /login으로 보내고, 이미 세션 쿠키가 있는 사용자의 /login//signup 접근은 auth 화면 렌더링 전에 한 번 걸러낼 수 있습니다. 이렇게 나누면 초기 렌더링 전 redirect가 가능하고 불필요한 client-side refresh 시도를 줄일 수 있어 프로젝트 인증 컨벤션과도 더 잘 맞습니다.
다만 조직 유무에 따른 /dashboard vs /organization/new 분기는 서버 사용자 정보가 필요하므로 지금처럼 client guard에서 처리해도 괜찮아 보입니다.
There was a problem hiding this comment.
cookie 존재 여부만으로 판단 가능한 1차 redirect를 middleware로 분리하는 방향에는 동의합니다.
다만 이번 PR은 AuthProvider/useAuth 기반 세션 복원과 guest guard 연결을 범위로 잡고 있어서, middleware 도입은 admin/onboarding 보호까지 함께 정리하는 후속 작업으로 분리하겠습니다.
| throw createInvalidJsonResponseError(response); | ||
| } | ||
|
|
||
| return parsedResponse as TResponse; |
There was a problem hiding this comment.
[질문]
성공 응답은 현재 JSON 존재 여부만 확인한 뒤 TResponse로 타입 단언하고 있는 것으로 보입니다. 에러 응답은 errorResponseSchema.safeParse()로 런타임 검증을 하고 있는데, 성공 응답은 shared schema 검증 없이 호출부의 제네릭 타입에만 의존하고 있어서요. 특히 auth 흐름에서는 accessToken, hasOrganization, organization 값이 토큰 저장과 라우팅에 바로 사용되다 보니 서버 응답 계약이 깨졌을 때 감지가 늦어질 수 있을 것 같습니다.
authSessionResponseSchema, authMeResponseSchema 같은 shared schema를 API wrapper에서 parse하거나 apiClient가 선택적으로 response schema를 받아 검증하는 방식도 가능해 보이는데, 현재처럼 성공 응답 검증을 생략한 이유가 있을까요?
There was a problem hiding this comment.
API 응답 검증 정책을 어디에 둘지 정한 후에 후속 반영하겠습니다!
선택지는 각 API 함수에서 검증하는 것과 apiClient가 optional schema를 받게 하는 방법이 있을 것 같습니다.
지금은 각 API 함수에서 검증하는 것이 더 나아보여요. 공통 apiClient를 키우는 것보다는 auth API wrapper에서만 먼저 shared schema로 parse하는 것이 안전할 것 같다고 생각했습니다.
2 participants
작업 내용
확인
참고
Closes #68
Summary by CodeRabbit
New Features
Bug Fixes