[Refactor] 인증 아키텍처 개선 - Axios 인스턴스 수정 및 refresh 주체 변경#388
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough이 변경사항은 인증(Auth) 시스템의 토큰 갱신 흐름을 재구조화합니다. 자동 갱신 로직을 제거하고, API 인스턴스를 분리하며, 프록시 미들웨어에서 제어된 갱신을 구현하며, 자격증명 기반 요청으로 전환합니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
🎭 Playwright Report✨ E2E Test가 성공적으로 완료되었습니다. Test 요약 내용을 확인해주세요.
📊 Test Summary
📜 Test Details✅ Passed Tests (3)
|
🎨 Storybook Report✅ 변경 사항이 없습니다 모든 Story가 이전 빌드와 동일합니다.
|
🚀 PR Preview Report✨ Build가 성공적으로 완료되었습니다. Preview에서 변경사항을 확인하세요.
|
📊 Coverage Report
📉 #388을 main에 병합하면 coverage가 Coverage 요약@@ Coverage Diff @@
## main #388 +/- ##
===========================================
- Coverage 35.68% 35.47% -0.21%
===========================================
Files 262 262 0
Lines 12140 12143 +3
Branches 477 468 -9
===========================================
- Hits 4332 4308 -24
+ Misses 7808 7835 +27 영향받은 파일
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 (1)
src/api/core/refresh/index.ts (1)
13-26:⚠️ Potential issue | 🔴 Critical미들웨어 컨텍스트에서
next/headerscookies()사용 불가 - 아키텍처 재설계 필요
next/headers의cookies()API는 Server Components, Route Handlers, Server Actions 전용으로 설계되었으며, 미들웨어 컨텍스트에서는 작동하지 않습니다. 미들웨어는 별도의 런타임(종종 Edge)에서 실행되어AsyncLocalStorage를 통한 요청 컨텍스트가 전파되지 않기 때문입니다.현재 코드 흐름:
proxy.ts미들웨어에서API.authService.refresh()호출 (line 22)refresh/index.ts의 axios 인터셉터에서next/headers의cookies()호출 시도 (line 17-18)- 미들웨어 컨텍스트에서 AsyncLocalStorage가 전파되지 않음
cookieStore.get('refreshToken')이undefined반환- 리프레시 요청이 쿠키 없이 전송되어
proxy.tsline 30의 catch 블록에서 조용히 실패해결 방안:
- 미들웨어에서 이미 접근 가능한
refreshToken을 axios 인터셉터에 전달하거나- 미들웨어 내에서 직접 새 accessToken을 획득한 후 refresh API 호출을 제거하거나
- axios 인터셉터를 클라이언트 환경에서만 작동하도록 제한 (
isServer조건일 때 쿠키 주입 제거)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/core/refresh/index.ts` around lines 13 - 26, The interceptor refreshInstance.interceptors.request.use is trying to call next/headers cookies() in a middleware context where AsyncLocalStorage isn't available, causing cookieStore.get('refreshToken') to be undefined; fix by removing server-side cookies() usage in the interceptor and instead have the middleware pass the refresh token into the request (e.g., set a header like X-Refresh-Token on the axios config in proxy.ts when calling API.authService.refresh()), or alternatively perform the refresh entirely inside the middleware (call the refresh endpoint from proxy.ts and set the new access token there) or scope the interceptor to client-only (remove the isServer branch and only inject cookies in client runtime). Ensure the change targets refreshInstance.interceptors.request.use and the proxy.ts call site (API.authService.refresh) so the refreshToken is provided from middleware rather than calling cookies() inside the interceptor.
🧹 Nitpick comments (4)
src/proxy.ts (2)
22-24: 불필요한 중간 변수data
const data = res;는 역할 없이 별칭만 추가합니다.res를 직접 사용하거나, 애초에 변수명을data로 선언하면 됩니다.♻️ 제안 수정
- const res = await API.authService.refresh(); - const data = res; - hasValidToken = true; - response.cookies.set('accessToken', data.accessToken, { - httpOnly: false, - maxAge: data.expiresIn, + const data = await API.authService.refresh(); + hasValidToken = true; + response.cookies.set('accessToken', data.accessToken, { + httpOnly: false, + maxAge: data.expiresIn,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/proxy.ts` around lines 22 - 24, The temporary alias const data = res is redundant; update the API.authService.refresh() handling in this block (where res, data, and hasValidToken are used) to remove the unnecessary data variable—either use res directly in subsequent logic or rename the original declaration to data (e.g., const data = await API.authService.refresh()) and eliminate the extra assignment—so that only one variable (res or data) holds the response before setting hasValidToken = true.
30-32: 리프레시 실패 시 에러 무음 처리 — 관찰 가능성 부재빈
catch블록은 네트워크 타임아웃, 서버 오류 등의 실패 원인을 모두 숨깁니다. 적어도console.error또는 구조화된 로그를 남겨 운영 환경에서 추적이 가능하도록 하는 것을 권장합니다.📋 제안 수정
} catch (err) { hasValidToken = false; + console.error('[proxy] token refresh failed:', err); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/proxy.ts` around lines 30 - 32, The empty catch that sets hasValidToken = false swallows the refresh error; update the catch to capture the exception (e) and log it (e.g., console.error or the app logger) before setting hasValidToken = false so failures like timeouts or server errors are observable; locate the try/catch around hasValidToken assignment in proxy.ts and replace the silent catch with a logged error including contextual info (token refresh attempt and error) so operators can trace issues.src/providers/provider-auth/index.tsx (1)
4-6:setIsAuthenticated컨텍스트 노출 — 의도적 설계인지 확인 필요
setIsAuthenticated를 컨텍스트로 외부에 노출하면 로그인·로그아웃·리프레시 흐름을 우회하여 임의의 하위 컴포넌트에서 인증 상태를 직접 변경할 수 있습니다. 현재 설계가 의도된 것이라면 최소한 사용 가이드를 주석으로 남기는 것이 좋습니다. 의도하지 않은 오용을 방지하려면setIsAuthenticated대신login()/logout()같은 명시적 액션 함수를 노출하는 방식을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/provider-auth/index.tsx` around lines 4 - 6, The context currently exposes setIsAuthenticated which allows arbitrary components to mutate auth state; change the context API to expose only isAuthenticated plus explicit actions (e.g., login(), logout(), refreshAuth()/checkAuth()) instead of setIsAuthenticated, update the provider implementation to keep internal state via setIsAuthenticated but only call it from those action functions, and update all consumers to call login()/logout() rather than calling setIsAuthenticated directly; if exposing setIsAuthenticated was intentional, add a clear comment in the context/type declaration documenting the intended usage and risks.src/api/service/auth-service/index.ts (1)
38-40:refreshAPI호출 시withCredentials: true중복
refreshInstance는 이미 인스턴스 수준에서withCredentials: true로 생성되어 있으므로(src/api/core/refresh/index.ts 10번째 줄), 요청별로 다시 전달하는 것은 불필요합니다.♻️ 제안 수정
- const data = await refreshAPI.post<RefreshResponse>('/api/v1/auth/refresh', null, { withCredentials: true }); + const data = await refreshAPI.post<RefreshResponse>('/api/v1/auth/refresh', null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/service/auth-service/index.ts` around lines 38 - 40, The request to refresh tokens redundantly passes { withCredentials: true } even though the axios instance refreshInstance (used as refreshAPI) already sets withCredentials at instance level; update the call in auth-service (the refreshAPI.post<RefreshResponse> invocation) to remove the per-request options object so it simply calls refreshAPI.post<RefreshResponse>('/api/v1/auth/refresh', null), relying on the refreshInstance configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/proxy.ts`:
- Around line 25-29: The cookie domain is hardcoded in the response.cookies.set
call for 'accessToken' and missing the secure flag; change
response.cookies.set(...) to read the domain from an environment variable (e.g.,
process.env.COOKIE_DOMAIN or fallback to undefined) instead of 'wego.monster',
and add secure: true to the options (or compute secure based on NODE_ENV/HTTPS
detection if you need local HTTP support), keeping httpOnly, maxAge:
data.expiresIn, and the same cookie name ('accessToken').
- Around line 20-38: The refresh flow sets the accessToken on the local response
via response.cookies.set(...) but then returns a new NextResponse.redirect(...)
which drops that Set-Cookie header; to fix, create the redirect response (e.g.,
const redirectRes = NextResponse.redirect(new URL('/', request.url))) and copy
or re-set the same cookie on that redirect response before returning it,
preserving the cookie attributes (httpOnly, maxAge/expiresIn, domain) so the
browser receives accessToken when redirected; update the branch handling
isPublic && hasValidToken to return the modified redirect response instead of
the original response.
---
Outside diff comments:
In `@src/api/core/refresh/index.ts`:
- Around line 13-26: The interceptor refreshInstance.interceptors.request.use is
trying to call next/headers cookies() in a middleware context where
AsyncLocalStorage isn't available, causing cookieStore.get('refreshToken') to be
undefined; fix by removing server-side cookies() usage in the interceptor and
instead have the middleware pass the refresh token into the request (e.g., set a
header like X-Refresh-Token on the axios config in proxy.ts when calling
API.authService.refresh()), or alternatively perform the refresh entirely inside
the middleware (call the refresh endpoint from proxy.ts and set the new access
token there) or scope the interceptor to client-only (remove the isServer branch
and only inject cookies in client runtime). Ensure the change targets
refreshInstance.interceptors.request.use and the proxy.ts call site
(API.authService.refresh) so the refreshToken is provided from middleware rather
than calling cookies() inside the interceptor.
---
Nitpick comments:
In `@src/api/service/auth-service/index.ts`:
- Around line 38-40: The request to refresh tokens redundantly passes {
withCredentials: true } even though the axios instance refreshInstance (used as
refreshAPI) already sets withCredentials at instance level; update the call in
auth-service (the refreshAPI.post<RefreshResponse> invocation) to remove the
per-request options object so it simply calls
refreshAPI.post<RefreshResponse>('/api/v1/auth/refresh', null), relying on the
refreshInstance configuration.
In `@src/providers/provider-auth/index.tsx`:
- Around line 4-6: The context currently exposes setIsAuthenticated which allows
arbitrary components to mutate auth state; change the context API to expose only
isAuthenticated plus explicit actions (e.g., login(), logout(),
refreshAuth()/checkAuth()) instead of setIsAuthenticated, update the provider
implementation to keep internal state via setIsAuthenticated but only call it
from those action functions, and update all consumers to call login()/logout()
rather than calling setIsAuthenticated directly; if exposing setIsAuthenticated
was intentional, add a clear comment in the context/type declaration documenting
the intended usage and risks.
In `@src/proxy.ts`:
- Around line 22-24: The temporary alias const data = res is redundant; update
the API.authService.refresh() handling in this block (where res, data, and
hasValidToken are used) to remove the unnecessary data variable—either use res
directly in subsequent logic or rename the original declaration to data (e.g.,
const data = await API.authService.refresh()) and eliminate the extra
assignment—so that only one variable (res or data) holds the response before
setting hasValidToken = true.
- Around line 30-32: The empty catch that sets hasValidToken = false swallows
the refresh error; update the catch to capture the exception (e) and log it
(e.g., console.error or the app logger) before setting hasValidToken = false so
failures like timeouts or server errors are observable; locate the try/catch
around hasValidToken assignment in proxy.ts and replace the silent catch with a
logged error including contextual info (token refresh attempt and error) so
operators can trace issues.
📝 변경 사항
인증 로직 리팩토링 변경사항
커밋 히스토리
119b45f51b60c5eb1353498f2c77aa134791. Refresh 로직 주체 변경 - AuthProvider → proxy.ts
페이지 새로고침 시 accessToken 재발급(refresh) 로직의 실행 주체를
AuthProvider(클라이언트)에서proxy.ts(미들웨어, 서버)로 변경.Before - AuthProvider에서 refresh 수행
After - proxy.ts에서 refresh 수행
변경 이유
useEffect에서 refresh를 수행하면 컴포넌트 마운트 이후에 실행되므로, 초기 렌더링 시 인증 상태가 불확실한 구간이 발생AuthProvider 변경사항
useEffect내 refresh 로직 전체 삭제js-cookieimport 제거APIimport 제거isAuthenticated상태 관리 역할은 유지 (외부에서setIsAuthenticated로 제어)2. LoginResponse 타입 수정
src/types/service/auth.ts의LoginResponse인터페이스에expiresAt필드 추가.3. authAPI → refreshAPI 변경
src/api/core/auth/index.ts→src/api/core/refresh/index.ts로 파일 이동 및 이름 변경.변경 이유
기존
authAPI는 인증 관련 API(login, signup, logout, refresh 등) 전용 인스턴스였으나, 실제로 별도의 axios 인스턴스가 필요한 것은 refresh API만 해당. refresh가 서버에서 실행될 경우refreshToken을 request header에 직접 주입해야 하기 때문.나머지 인증 관련 API(login, signup, logout, withdraw, google OAuth)는
baseAPI에withCredentials: true옵션만 추가하면 충분.refreshAPI 인스턴스 변경점
auth-service import 변경
4. proxy.ts 인증 판단 기준 변경
기존에는
accessToken || refreshToken존재 여부로 인증 상태를 판단했으나, 변경 후에는 refresh를 미들웨어에서 선처리하므로hasValidToken(accessToken 존재 또는 refresh 성공 여부) 단일 변수로 판단.변경된 파일 목록
src/api/core/auth/index.ts→src/api/core/refresh/index.tssrc/api/service/auth-service/index.tssrc/providers/provider-auth/index.tsxsrc/proxy.tssrc/types/service/auth.ts🔗 관련 이슈
Closes #
🧪 테스트 방법
📸 스크린샷 (선택)
📋 체크리스트
💬 추가 코멘트
CodeRabbit Review는 자동으로 실행되지 않습니다.
Review를 실행하려면 comment에 아래와 같이 작성해주세요
Summary by CodeRabbit
릴리스 노트
버그 수정
개선사항