[Phase 3] feat(auth): 라우터 가드·헤더 로그인 상태 반영·세션 복원 구현 - #87
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe frontend adds auth API helpers, session restoration, route protection, header logout controls, and a shared 401 refresh queue for access-token renewal. ChangesAuthentication bootstrap, routing, and tokens
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/App.vue (1)
39-42: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider guarding
auth.logout()against rejection.If
auth.logout()rejects (e.g., a network error during server-side logout),router.push('/')is skipped and the user is left without feedback. Atry/finallyensures the redirect still happens after local state is cleared.Optional refactor
async function handleLogout() { - await auth.logout() - router.push('/') + try { + await auth.logout() + } finally { + router.push('/') + } }🤖 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 `@frontend/src/App.vue` around lines 39 - 42, The logout flow in handleLogout currently lets a rejection from auth.logout() prevent the redirect, so update it to ensure router.push('/') still runs even if the logout request fails. Use a try/finally around auth.logout() in App.vue’s handleLogout function, keeping the redirect in the finally block so the user is always sent back to the home route after local cleanup.
🤖 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.
Nitpick comments:
In `@frontend/src/App.vue`:
- Around line 39-42: The logout flow in handleLogout currently lets a rejection
from auth.logout() prevent the redirect, so update it to ensure router.push('/')
still runs even if the logout request fails. Use a try/finally around
auth.logout() in App.vue’s handleLogout function, keeping the redirect in the
finally block so the user is always sent back to the home route after local
cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 775114f7-d1a0-420c-8999-74ea0640e9a9
📒 Files selected for processing (3)
frontend/src/App.vuefrontend/src/main.jsfrontend/src/router/index.js
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
frontend/src/store/authStore.js (1)
4-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winToken storage keys duplicated across
authStore.jsandhttp.js.
ACCESS_KEY/REFRESH_KEYare redefined here with the same literal values as infrontend/src/api/http.js(lines 6-7). If one side changes a key string, the store and the HTTP refresh flow silently disagree on storage. Extract them into a shared module and import from both.🤖 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 `@frontend/src/store/authStore.js` around lines 4 - 5, The token storage keys are duplicated between authStore and the HTTP refresh flow, so both places can drift if one literal changes. Move ACCESS_KEY and REFRESH_KEY into a shared module and update authStore.js and http.js to import those shared constants instead of redefining them locally, keeping the token names consistent across the authStore and refresh logic.frontend/src/api/http.js (1)
90-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHard redirect via
window.location.hrefdrops SPA state and the original destination.A full page reload discards the in-memory app and doesn't preserve where the user was. Other navigation in this stack uses the router and passes
query.redirect(seefrontend/src/router/index.js). Prefer a router push (router.push({ path: '/login', query: { redirect: ... } })) for consistency, falling back tolocation.hrefonly if the router isn't reachable here.🤖 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 `@frontend/src/api/http.js` at line 90, The auth redirect in the HTTP handler uses a hard reload via window.location.href, which drops SPA state and loses the user’s original destination. Update the redirect logic in the HTTP client flow to use the router-based navigation pattern used elsewhere, following the router push approach in router/index.js and preserving the current destination in query.redirect; only fall back to location.href if the router instance is unavailable in this context.
🤖 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 `@frontend/src/api/http.js`:
- Around line 63-69: Queued retries in the http interceptor are not being marked
as retried, so they can re-enter the 401 refresh flow and trigger another
refresh cycle. Update the pendingQueue resolution path in the http request
interceptor so the retried request from http(original) also sets original._retry
= true before reissuing it, matching the behavior used for the initial
refresh-triggering request and preventing repeat refresh attempts.
- Around line 76-85: The token refresh flow in http’s axios interceptor updates
localStorage and axios defaults but leaves the auth store stale, so useAuthStore
state and downstream auth checks stay out of sync. Update the refresh handling
in the response interceptor to also invoke the auth store’s token persistence
action (for example the store method that saves both accessToken and
refreshToken) right after parsing res.data.data, alongside the existing
localStorage and http.defaults updates. Keep the existing flushQueue and
original request retry logic in place, but make sure the store update happens in
the same refresh path so isLoggedIn, the router guard, and the header all read
the same tokens.
In `@frontend/src/store/authStore.js`:
- Around line 19-22: restoreSession currently rehydrates only accessToken and
refreshToken, leaving user null so authenticated UI can render without profile
data. Update authStore’s restoreSession to also restore the user state when
tokens are present, or trigger the existing profile fetch flow from the auth
store after restoring a valid token. Keep the change centered around
restoreSession and any related user-loading action in authStore.
---
Nitpick comments:
In `@frontend/src/api/http.js`:
- Line 90: The auth redirect in the HTTP handler uses a hard reload via
window.location.href, which drops SPA state and loses the user’s original
destination. Update the redirect logic in the HTTP client flow to use the
router-based navigation pattern used elsewhere, following the router push
approach in router/index.js and preserving the current destination in
query.redirect; only fall back to location.href if the router instance is
unavailable in this context.
In `@frontend/src/store/authStore.js`:
- Around line 4-5: The token storage keys are duplicated between authStore and
the HTTP refresh flow, so both places can drift if one literal changes. Move
ACCESS_KEY and REFRESH_KEY into a shared module and update authStore.js and
http.js to import those shared constants instead of redefining them locally,
keeping the token names consistent across the authStore and refresh logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76edc980-8daf-46c8-b696-e3fd8404c740
📒 Files selected for processing (3)
frontend/src/api/auth.jsfrontend/src/api/http.jsfrontend/src/store/authStore.js
| if (isRefreshing) { | ||
| return new Promise((resolve, reject) => { | ||
| pendingQueue.push({ resolve, reject }) | ||
| }).then((token) => { | ||
| original.headers.Authorization = `Bearer ${token}` | ||
| return http(original) | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Queued retries are not marked _retry, allowing repeat refresh attempts.
The request that triggers the refresh sets original._retry = true (line 72), but requests resolved from pendingQueue retry via http(original) without ever setting _retry. If the freshly issued token is still rejected with 401, those retried requests re-enter the interceptor, pass the !original._retry gate, and kick off another refresh cycle. Mark queued retries as retried as well.
Proposed guard
}).then((token) => {
+ original._retry = true
original.headers.Authorization = `Bearer ${token}`
return http(original)
})📝 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.
| if (isRefreshing) { | |
| return new Promise((resolve, reject) => { | |
| pendingQueue.push({ resolve, reject }) | |
| }).then((token) => { | |
| original.headers.Authorization = `Bearer ${token}` | |
| return http(original) | |
| }) | |
| if (isRefreshing) { | |
| return new Promise((resolve, reject) => { | |
| pendingQueue.push({ resolve, reject }) | |
| }).then((token) => { | |
| original._retry = true | |
| original.headers.Authorization = `Bearer ${token}` | |
| return http(original) | |
| }) |
🤖 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 `@frontend/src/api/http.js` around lines 63 - 69, Queued retries in the http
interceptor are not being marked as retried, so they can re-enter the 401
refresh flow and trigger another refresh cycle. Update the pendingQueue
resolution path in the http request interceptor so the retried request from
http(original) also sets original._retry = true before reissuing it, matching
the behavior used for the initial refresh-triggering request and preventing
repeat refresh attempts.
| const res = await axios.post(`${baseURL}/api/v1/auth/refresh`, { | ||
| refreshToken: storedRefresh, | ||
| }) | ||
| const { accessToken, refreshToken } = res.data.data | ||
| localStorage.setItem(ACCESS_KEY, accessToken) | ||
| localStorage.setItem(REFRESH_KEY, refreshToken) | ||
| http.defaults.headers.common.Authorization = `Bearer ${accessToken}` | ||
| flushQueue(null, accessToken) | ||
| original.headers.Authorization = `Bearer ${accessToken}` | ||
| return http(original) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find direct consumers of store token state that could observe a stale value
rg -nP '\.(accessToken|refreshToken)\b' frontend/src -g '!**/http.js' -C2
rg -nP '\bisLoggedIn\b' frontend/src -C2Repository: ssafy-salman/salmanhae
Length of output: 2939
🏁 Script executed:
#!/bin/bash
set -e
# Inspect the refresh flow and auth store to see whether tokens are synchronized.
sed -n '1,220p' frontend/src/api/http.js
printf '\n--- authStore ---\n'
sed -n '1,220p' frontend/src/store/authStore.js
printf '\n--- router ---\n'
sed -n '1,120p' frontend/src/router/index.js
printf '\n--- App.vue ---\n'
sed -n '1,120p' frontend/src/App.vueRepository: ssafy-salman/salmanhae
Length of output: 8093
🏁 Script executed:
#!/bin/bash
set -e
# Look for any refresh-specific store updates elsewhere.
rg -n "refreshToken|_saveTokens|restoreSession|flushQueue|window.location.href|location.href" frontend/src -C3Repository: ssafy-salman/salmanhae
Length of output: 5465
🏁 Script executed:
#!/bin/bash
set -e
# Check whether anything outside the store reads accessToken/refreshToken directly.
rg -n "useAuthStore\(\)|auth\.(accessToken|refreshToken)|store\.accessToken|store\.refreshToken" frontend/src -C2Repository: ssafy-salman/salmanhae
Length of output: 1532
🏁 Script executed:
#!/bin/bash
set -e
rg -n "useAuthStore\(\)|auth\.(accessToken|refreshToken)|store\.accessToken|store\.refreshToken" frontend/src -C2Repository: ssafy-salman/salmanhae
Length of output: 1532
🏁 Script executed:
#!/bin/bash
set -e
sed -n '1,160p' frontend/src/api/auth.js
printf '\n--- main.js ---\n'
sed -n '1,80p' frontend/src/main.jsRepository: ssafy-salman/salmanhae
Length of output: 1130
Sync the auth store during token refresh
frontend/src/api/http.js:76-85 updates localStorage and http.defaults, but useAuthStore().accessToken / refreshToken stay stale for the rest of the session. Call the store’s token save action here (for example _saveTokens(accessToken, refreshToken)) so isLoggedIn, the router guard, and the header stay in sync.
🤖 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 `@frontend/src/api/http.js` around lines 76 - 85, The token refresh flow in
http’s axios interceptor updates localStorage and axios defaults but leaves the
auth store stale, so useAuthStore state and downstream auth checks stay out of
sync. Update the refresh handling in the response interceptor to also invoke the
auth store’s token persistence action (for example the store method that saves
both accessToken and refreshToken) right after parsing res.data.data, alongside
the existing localStorage and http.defaults updates. Keep the existing
flushQueue and original request retry logic in place, but make sure the store
update happens in the same refresh path so isLoggedIn, the router guard, and the
header all read the same tokens.
| restoreSession() { | ||
| this.accessToken = localStorage.getItem(ACCESS_KEY) || '' | ||
| this.refreshToken = localStorage.getItem(REFRESH_KEY) || '' | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
restoreSession rehydrates tokens but never restores user.
After a reload, accessToken is restored (so isLoggedIn is true) yet user stays null. Any header/profile UI that reads user will render an authenticated-but-empty state until a fresh fetch. Consider persisting/rehydrating user too, or fetching the profile when a token is restored.
🤖 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 `@frontend/src/store/authStore.js` around lines 19 - 22, restoreSession
currently rehydrates only accessToken and refreshToken, leaving user null so
authenticated UI can render without profile data. Update authStore’s
restoreSession to also restore the user state when tokens are present, or
trigger the existing profile fetch flow from the auth store after restoring a
valid token. Keep the change centered around restoreSession and any related
user-loading action in authStore.
1 participant
변경 내용
router/index.js:/chat라우트에meta: { requiresAuth: true }추가, 비로그인 접근 시/login리다이렉트 가드App.vue: 헤더에 로그인 상태 반영 — 비로그인 시 "로그인" 버튼, 로그인 시 "로그아웃" 버튼main.js: 앱 초기화 시authStore.restoreSession()호출 (localStorage 토큰 복원)연결 이슈
closes #86
테스트
Summary by CodeRabbit