Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Walkthrough이 변경사항은 애플리케이션의 인증 및 세션 관리 시스템을 근본적으로 리팩토링합니다. 새로운 세션 관리 모듈( Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
상세 변경사항1. 레이아웃 및 UI 구조 개편
2. 세션 관리 시스템 신설
3. 인증 API 및 토큰 관리 변경
4. 라우트 보호 메커니즘 통일
5. 컴포넌트 및 라우트 업데이트
🚥 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)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8f96269e0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const handleLogout = () => { | ||
| clearSession(); | ||
| toast.success("로그아웃되었습니다."); | ||
| void navigate({ to: "/auth/login" }); |
There was a problem hiding this comment.
handleLogout only removes the access token from localStorage and immediately navigates to /auth/login, but the refresh cookie is left intact. Because /auth/login now runs redirectIfAuthenticated, users with a still-valid refresh cookie will get a new access token via /auth/reissue and be redirected back to /scores, so logout does not actually terminate the session in normal authenticated flows.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| return reissueAccessTokenIfPossible(); |
There was a problem hiding this comment.
ensureSessionToken always falls through to reissueAccessTokenIfPossible, and protected routes now call this from beforeLoad. In this app (@tanstack/react-start), beforeLoad also runs during server rendering, where browser localStorage is unavailable and request cookies are not forwarded into this axios call, so direct loads/refreshes of /scores, /bruno, or /chat-socket can be misclassified as unauthenticated and redirected to /auth/login even for valid sessions.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
apps/admin/src/routes/chat-socket/index.tsx (1)
134-139:⚠️ Potential issue | 🟠 Major2. 소켓 연결 직전에 세션 토큰을 다시 동기화해주세요.
beforeLoad는 페이지 진입 시점만 보장합니다. 페이지를 오래 열어둔 뒤 Line 199에서 기존tokenstate로/connectURL을 만들면 만료된 access token으로 연결할 수 있습니다.loadAccessToken()도 만료/재발급 검사를 우회하므로, 토큰 불러오기와 연결 직전에는ensureSessionToken()을 통해 최신 토큰을 받아주세요.최신 세션 토큰을 사용하도록 조정하는 예시
-import { requireAdminSession } from "@/lib/auth/session"; -import { loadAccessToken } from "@/lib/utils/localStorage"; +import { ensureSessionToken, requireAdminSession } from "@/lib/auth/session"; @@ useEffect(() => { - const accessToken = loadAccessToken(); - if (accessToken) { - setToken(accessToken); - } + let ignore = false; + + void ensureSessionToken().then((accessToken) => { + if (!ignore && accessToken) { + setToken(accessToken); + } + }); + + return () => { + ignore = true; + }; }, []); @@ - const handleLoadStoredToken = () => { - const accessToken = loadAccessToken(); + const handleLoadStoredToken = async () => { + const accessToken = await ensureSessionToken(); setToken(accessToken ?? ""); @@ const handleConnect = async () => { if (!roomId.trim()) { toast.error("Room ID를 입력해주세요."); return; } const normalizedServerUrl = normalizeBaseUrl(serverUrl); + const accessToken = await ensureSessionToken(); + setToken(accessToken ?? ""); + const nextSocketUrl = - normalizedServerUrl && token.trim() - ? `${normalizedServerUrl}/connect?token=${encodeURIComponent(token.trim())}` + normalizedServerUrl && accessToken + ? `${normalizedServerUrl}/connect?token=${encodeURIComponent(accessToken)}` : "";Also applies to: 172-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/admin/src/routes/chat-socket/index.tsx` around lines 134 - 139, The current effect only sets token once via loadAccessToken() which can be stale; before creating the /connect URL or calling the socket connect logic (the code that builds the connect URL using token state around the connect handler near Line 199), call ensureSessionToken() to refresh/validate the session and then setToken() with the returned value; specifically replace the connect-creation path that uses token state (and any uses inside the effect or connect function) so it awaits ensureSessionToken(), assigns the fresh token (via setToken or a local variable) and then constructs the /connect URL and initiates the socket connection using that fresh token rather than the possibly stale token from loadAccessToken().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/admin/src/components/layout/AdminLayout.tsx`:
- Around line 17-20: When logging out from handleLogout, call the server logout
endpoint (adminLogoutApi) and await its success before calling clearSession();
on success expire the refresh cookie server-side so redirectIfAuthenticated
cannot reissue an access token, then call clearSession(), show
toast.success("로그아웃되었습니다.") and navigate to "/auth/login"; on failure do not
clear the session or show a success toast—show an error toast instead. Implement
this by replacing the current synchronous clearSession() call in handleLogout
with an async try/catch that awaits adminLogoutApi(), only clears session and
navigates on success, and handles errors (toast.error) on failure; reference
handleLogout, clearSession, adminLogoutApi and redirectIfAuthenticated when
making the change.
In `@apps/admin/src/lib/auth/session.ts`:
- Around line 22-24: clearSession only removes local access token but doesn't
invalidate the server refresh cookie, allowing a concurrent or subsequent
reissueAccessTokenIfPossible (called via
ensureSessionToken/redirectIfAuthenticated) to silently restore a session; add a
server-side logout endpoint that clears/invalidates the refresh cookie, and on
the client add a session-generation guard (e.g., a monotonically-incremented
sessionGeneration variable stored alongside clearSession) so clearSession
increments the generation and reissueAccessTokenIfPossible checks the generation
before persisting any new token (ignore/stash results if generation changed),
and ensure redirectIfAuthenticated/ensureSessionToken propagate that check so
stale reissues are not saved.
---
Outside diff comments:
In `@apps/admin/src/routes/chat-socket/index.tsx`:
- Around line 134-139: The current effect only sets token once via
loadAccessToken() which can be stale; before creating the /connect URL or
calling the socket connect logic (the code that builds the connect URL using
token state around the connect handler near Line 199), call ensureSessionToken()
to refresh/validate the session and then setToken() with the returned value;
specifically replace the connect-creation path that uses token state (and any
uses inside the effect or connect function) so it awaits ensureSessionToken(),
assigns the fresh token (via setToken or a local variable) and then constructs
the /connect URL and initiates the socket connection using that fresh token
rather than the possibly stale token from loadAccessToken().
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 582a1976-984f-4582-91ea-24661207d53f
📒 Files selected for processing (12)
apps/admin/src/components/layout/AdminLayout.tsxapps/admin/src/components/layout/AdminSidebar.tsxapps/admin/src/lib/api/auth.tsapps/admin/src/lib/api/client.tsapps/admin/src/lib/auth/session.tsapps/admin/src/lib/utils/localStorage.tsapps/admin/src/routes/auth/login.tsxapps/admin/src/routes/bruno/index.tsxapps/admin/src/routes/chat-socket/index.tsxapps/admin/src/routes/index.tsxapps/admin/src/routes/scores/index.tsxapps/admin/src/types/auth.ts
💤 Files with no reviewable changes (1)
- apps/admin/src/lib/utils/localStorage.ts
| const handleLogout = () => { | ||
| clearSession(); | ||
| toast.success("로그아웃되었습니다."); | ||
| void navigate({ to: "/auth/login" }); |
There was a problem hiding this comment.
1. 로그아웃 시 refresh cookie도 무효화해 주세요.
- 지금은 `clearSession()`이 access token만 지우기 때문에, `/auth/login` 진입 직후 `redirectIfAuthenticated()`가 `/auth/reissue`로 새 access token을 다시 받아 `/scores`로 되돌릴 수 있습니다.
- 쿠키 기반 refresh 모델에서는 서버 로그아웃 API로 refresh cookie를 만료시키거나, 명시적 로그아웃 상태에서는 reissue를 막는 처리가 필요합니다.
🐛 수정 방향 예시
- const handleLogout = () => {
- clearSession();
- toast.success("로그아웃되었습니다.");
- void navigate({ to: "/auth/login" });
- };
+ const handleLogout = async () => {
+ await adminLogoutApi(); // 서버에서 refresh cookie 만료
+ clearSession();
+ toast.success("로그아웃되었습니다.");
+ void navigate({ to: "/auth/login" });
+ };추가로 adminLogoutApi() 실패 시에는 성공 토스트와 세션 삭제를 하지 않도록 처리하면, 실제 로그아웃 실패와 UI 상태가 엇갈리는 일을 줄일 수 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/components/layout/AdminLayout.tsx` around lines 17 - 20, When
logging out from handleLogout, call the server logout endpoint (adminLogoutApi)
and await its success before calling clearSession(); on success expire the
refresh cookie server-side so redirectIfAuthenticated cannot reissue an access
token, then call clearSession(), show toast.success("로그아웃되었습니다.") and navigate
to "/auth/login"; on failure do not clear the session or show a success
toast—show an error toast instead. Implement this by replacing the current
synchronous clearSession() call in handleLogout with an async try/catch that
awaits adminLogoutApi(), only clears session and navigates on success, and
handles errors (toast.error) on failure; reference handleLogout, clearSession,
adminLogoutApi and redirectIfAuthenticated when making the change.
| export const clearSession = () => { | ||
| removeAccessToken(); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 설명: 로그아웃 경로가 refresh 쿠키를 서버에서 무효화하는지, clearSession 이후 reissue가 다시 저장될 수 있는지 확인합니다.
# 기대 결과: 로그아웃 성공 시 서버 refresh 쿠키 만료 API가 호출되고, 진행 중이던 reissue 결과는 저장되지 않아야 합니다.
rg -n -C3 --type=ts --type=tsx '\b(clearSession|redirectIfAuthenticated|reissueAccessTokenIfPossible|reissueAccessTokenApi|logout|Logout)\b'Repository: solid-connection/solid-connect-web
Length of output: 105
🏁 Script executed:
rg -n -C3 'clearSession|redirectIfAuthenticated|reissueAccessTokenIfPossible|reissueAccessTokenApi|logout|Logout' apps/admin/src/lib/auth/Repository: solid-connection/solid-connect-web
Length of output: 3060
🏁 Script executed:
# Read the session.ts file to understand current implementation
head -100 apps/admin/src/lib/auth/session.tsRepository: solid-connection/solid-connect-web
Length of output: 1795
🏁 Script executed:
# Find logout-related endpoints and handlers
fd -e ts -e tsx . apps/admin/src | head -30Repository: solid-connection/solid-connect-web
Length of output: 1310
🏁 Script executed:
cat apps/admin/src/components/layout/AdminLayout.tsxRepository: solid-connection/solid-connect-web
Length of output: 2519
🏁 Script executed:
cat apps/admin/src/lib/api/auth.tsRepository: solid-connection/solid-connect-web
Length of output: 820
🏁 Script executed:
# Search for logout endpoint or auth routes
rg -n 'logout|Logout|clearSession' apps/admin/src/ --type-list | head -20Repository: solid-connection/solid-connect-web
Length of output: 467
🏁 Script executed:
# Check auth routes if they exist
find apps -name '*auth*' -o -name '*logout*' | grep -E '\.(ts|tsx)$'Repository: solid-connection/solid-connect-web
Length of output: 234
🏁 Script executed:
# Search for logout-related endpoints
rg -n 'sign-out|logout|/auth' apps/ --type ts --max-count 30 | grep -E '(logout|sign-out|endpoint|route)'Repository: solid-connection/solid-connect-web
Length of output: 2081
🏁 Script executed:
# Check if there's a logout endpoint in the API or routes
find apps -name '*.ts' -o -name '*.tsx' | xargs grep -l 'sign-out\|logout.*endpoint\|auth.*logout' 2>/dev/null | head -10Repository: solid-connection/solid-connect-web
Length of output: 107
🏁 Script executed:
# Check the actual auth route configuration
cat apps/admin/src/routes/auth/login.tsxRepository: solid-connection/solid-connect-web
Length of output: 3636
로그아웃 후 refresh 쿠키로 인한 즉시 재인증 방지 필요합니다.
clearSession()은 localStorage의 access token만 삭제하며, 서버에서 refresh 쿠키를 무효화하지 않습니다. 다음과 같은 경로로 로그아웃이 무효화될 수 있습니다.
- 로그아웃 시 진행 중인
reissueAccessTokenIfPossible()호출이clearSession()이후에 완료되어 새 토큰을 다시 저장할 수 있습니다. - 로그인 페이지 진입 시
redirectIfAuthenticated()→ensureSessionToken()→reissueAccessTokenIfPossible()으로 이어지고, 서버의 유효한 refresh 쿠키로 토큰 재발급에 성공합니다. - 새 토큰이 저장되어 사용자가
scores페이지로 다시 리다이렉트됩니다.
아래 두 가지가 필요합니다.
- 서버 측: logout 엔드포인트 추가하여 refresh 쿠키 무효화
- 클라이언트 측: 로그아웃 후 stale reissue 결과를 저장하지 않도록 세션 generation guard 추가
구현 예시: session generation guard
let reissuePromise: Promise<string | null> | null = null;
+let sessionGeneration = 0;
@@
export const clearSession = () => {
+ sessionGeneration += 1;
removeAccessToken();
};
@@
export const reissueAccessTokenIfPossible = async (): Promise<string | null> => {
if (reissuePromise) {
return reissuePromise;
}
+ const generationAtStart = sessionGeneration;
reissuePromise = (async () => {
try {
const response = await reissueAccessTokenApi();
const nextAccessToken = response.data.accessToken;
@@
if (!nextAccessToken) {
clearSession();
return null;
}
+ if (generationAtStart !== sessionGeneration) {
+ return null;
+ }
+
saveAccessToken(nextAccessToken);
return nextAccessToken;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/admin/src/lib/auth/session.ts` around lines 22 - 24, clearSession only
removes local access token but doesn't invalidate the server refresh cookie,
allowing a concurrent or subsequent reissueAccessTokenIfPossible (called via
ensureSessionToken/redirectIfAuthenticated) to silently restore a session; add a
server-side logout endpoint that clears/invalidates the refresh cookie, and on
the client add a session-generation guard (e.g., a monotonically-incremented
sessionGeneration variable stored alongside clearSession) so clearSession
increments the generation and reissueAccessTokenIfPossible checks the generation
before persisting any new token (ignore/stash results if generation changed),
and ensure redirectIfAuthenticated/ensureSessionToken propagate that check so
stale reissues are not saved.
1 participant
작업 배경
주요 변경 사항
/scores,/bruno,/chat-socket)의beforeLoad를 공통 세션 유틸(requireAdminSession)로 통합했습니다./) 진입 시 세션 검사 후인증됨 -> /scores,미인증 -> /auth/login으로 즉시 리다이렉트하도록 고정했습니다./auth/login)에 역가드(redirectIfAuthenticated)를 추가해 인증 상태에서는/scores로 이동하도록 처리했습니다.scores,bruno,chat-socket).<a href>에서 라우터Link기반으로 바꿔 SPA 내비게이션 신뢰성을 높였습니다./chat-socket를 보호 라우트로 편입하고, 페이지 내부 즉시 로그인 블록을 제거해 인증 진입점을 단일화했습니다.AdminLayout을 실제 라우트에서 사용하도록 연결하고, 헤더 로그아웃 액션(세션 정리 +/auth/login이동)을 추가했습니다./auth/reissue)로 통일했습니다.reissueAccessTokenApi(refreshToken)->reissueAccessTokenApi()로 시그니처 변경refreshToken의존 제거(옵셔널 처리)검증
pnpm --filter @solid-connect/admin run ci:check통과pnpm --filter @solid-connect/admin run build통과비고
apps/admin/.output/*)은 커밋에 포함하지 않았습니다.