Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📋 Walkthrough이 변경은 애플리케이션 전역에서 토큰 만료 여부를 검증하는 로직을 추가하여 인증 흐름을 강화합니다. 기존에는 토큰의 존재 여부만 확인했다면, 이제는 만료된 토큰을 유효하지 않은 것으로 취급합니다. 주요 변경 사항:
🎯 Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 변경이 여러 파일에 걸쳐 있고, 각 파일마다 토큰 만료 검증 로직을 상이한 문맥에서 적용했습니다. 특히 👥 Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: 1d17417492
ℹ️ 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".
| if (accessToken && isTokenExpired(accessToken)) { | ||
| clearAccessToken(); |
There was a problem hiding this comment.
Calling clearAccessToken() as soon as an access token is detected expired logs the client into a temporary unauthenticated state before refresh has even been attempted. In this codebase, UI/query flows are gated by isAuthenticated (for example in university and mentor widgets), so a normal silent refresh can now transiently disable authenticated behavior and reset auth-dependent UI even when refresh succeeds moments later. Keep the old auth state until reissue actually fails to avoid this regression.
Useful? React with 👍 / 👎.
| if (isTokenExpired(newAccessToken)) { | ||
| throw new Error("재발급된 토큰이 이미 만료되었습니다."); |
There was a problem hiding this comment.
Rejecting a freshly reissued access token based on isTokenExpired() can force unnecessary logout loops when client and auth-server clocks are skewed (for example, a user device clock ahead of server time). In that case /auth/reissue succeeds server-side but this client check throws and clears auth state anyway. Add skew tolerance or trust the server-issued token instead of hard-failing immediately.
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: 1
🧹 Nitpick comments (1)
apps/web/src/utils/axiosInstance.ts (1)
38-64: 1.tryReissueAccessToken()의 실패 계약을 한쪽으로 정리해주세요.지금 helper는 내부에서 에러를 삼키고
null만 반환하는데, 두 호출부는try/catch를 두고 있어서 reject도 가능한 것처럼 읽힙니다. 실패를 값으로 표현할 거면catch를 걷어내고null체크만 남기는 편이 흐름을 더 빨리 읽게 해줍니다.Also applies to: 99-110, 147-157
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/utils/axiosInstance.ts` around lines 38 - 64, Unify the failure contract by treating tryReissueAccessToken() as a value-returning failure (it returns null on failure) and remove the surrounding try/catch blocks in its callers so they check the returned value for null instead; specifically keep the behavior in tryReissueAccessToken (setRefreshStatus/clearAccessToken and return accessToken or null) and change the caller sites (the blocks around lines 99-110 and 147-157 that currently wrap calls to tryReissueAccessToken() in try/catch) to simply await the call and branch on a null result rather than catching errors.
🤖 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/web/src/lib/zustand/useAuthStore.ts`:
- Around line 91-104: The hydration callback is directly mutating the Zustand
state (state.accessToken, state.userRole, state.isAuthenticated,
state.refreshStatus, state.isInitialized) which can break subscriber
notifications and be overwritten by sync storages; change the onRehydrateStorage
callback signature to (state, error) and perform all updates via the store's
setState API instead of assigning to state directly: call setState with a new
partial state that clears an expired token (use
isTokenExpired(state.accessToken)), sets userRole via parseUserRoleFromToken
when valid, toggles refreshStatus to "idle" or "success", and marks
isInitialized true so subscribers receive the change atomically.
---
Nitpick comments:
In `@apps/web/src/utils/axiosInstance.ts`:
- Around line 38-64: Unify the failure contract by treating
tryReissueAccessToken() as a value-returning failure (it returns null on
failure) and remove the surrounding try/catch blocks in its callers so they
check the returned value for null instead; specifically keep the behavior in
tryReissueAccessToken (setRefreshStatus/clearAccessToken and return accessToken
or null) and change the caller sites (the blocks around lines 99-110 and 147-157
that currently wrap calls to tryReissueAccessToken() in try/catch) to simply
await the call and branch on a null result rather than catching errors.
🪄 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: 07194c86-797d-443f-b900-4d8f093abb90
📒 Files selected for processing (7)
apps/web/src/apis/Auth/server/postReissueToken.tsapps/web/src/app/mentor/_ui/MentorClient/index.tsxapps/web/src/lib/web-socket/useConnectWebSocket.tsapps/web/src/lib/zustand/useAuthStore.tsapps/web/src/middleware.tsapps/web/src/utils/axiosInstance.tsdocs/auth-refresh-edge-cases.md
| const hasValidToken = Boolean(state.accessToken && !isTokenExpired(state.accessToken)); | ||
|
|
||
| if (!hasValidToken) { | ||
| state.accessToken = null; | ||
| state.userRole = null; | ||
| state.isAuthenticated = false; | ||
| state.refreshStatus = "idle"; | ||
| } else { | ||
| state.userRole = parseUserRoleFromToken(state.accessToken); | ||
| state.isAuthenticated = true; | ||
| state.refreshStatus = "success"; | ||
| } | ||
|
|
||
| state.isInitialized = true; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the actual file
find . -type f -name "useAuthStore.ts" | head -5Repository: solid-connection/solid-connect-web
Length of output: 120
🏁 Script executed:
# Also check the file structure
git ls-files apps/web/src/lib/zustand/ | head -20Repository: solid-connection/solid-connect-web
Length of output: 354
🏁 Script executed:
# Read the useAuthStore.ts file to see the actual implementation
cat -n ./apps/web/src/lib/zustand/useAuthStore.tsRepository: solid-connection/solid-connect-web
Length of output: 3820
🏁 Script executed:
# Let's check if there are any other usages or tests of this store
grep -r "useAuthStore" apps/web/src --include="*.ts" --include="*.tsx" | head -20Repository: solid-connection/solid-connect-web
Length of output: 1972
🏁 Script executed:
# Also check if there's documentation or examples about Zustand persist behavior
find . -type f \( -name "*.md" -o -name "*.txt" \) -path "*/zustand*" | head -10Repository: solid-connection/solid-connect-web
Length of output: 60
🌐 Web query:
Zustand persist middleware onRehydrateStorage direct state mutation propagation subscribers
💡 Result:
In Zustand's persist middleware, the onRehydrateStorage callback is invoked before the persisted state is loaded from storage and applied to the store. It receives the current (initial) state as an argument and can return an optional post-rehydration callback that runs after the state is merged in or if an error occurs. State mutations (via set) within onRehydrateStorage or its returned callback use the standard set function, which does trigger subscribers because Zustand's core setState always notifies listeners/subscribers to state changes. However, for synchronous storages like localStorage: - Mutations in onRehydrateStorage can be overwritten by the final persisted state assignment. - This was a known issue (e.g., GitHub #1688), fixed in later versions where updates during rehydration are preserved properly. The rehydration itself (merging persisted state) happens via internal set calls after storage retrieval, so subscribers are notified of the hydration state changes. React components using useStore will re-render accordingly once hydrated. Use store.persist.hasHydrated, onHydrate, or onFinishHydration to detect/track hydration status reactively. Example usage: onRehydrateStorage: (state) => { // Pre-rehydration, mutations here may be overwritten (sync storage) return (newState, error) => { if (!error) { // Post-rehydration mutations propagate normally via set state.someAction; // Triggers subscribers } }; } Direct state mutation (e.g., store.setState({ ... })) during rehydration also notifies subscribers, as it's the same mechanism.
Citations:
- 1: https://docs.pmnd.rs/zustand/integrations/persisting-store-data
- 2: State updates within the 'onRehydrateStorage' callback using a synchronous storage API get dropped pmndrs/zustand#1688
- 3: https://github.com/pmndrs/zustand/blob/main/src/middleware/persist.ts
- 4: Is it possible to immediately load persisted data? pmndrs/zustand#346
🏁 Script executed:
# Check if there's a post-rehydration pattern used elsewhere or documentation
grep -r "onRehydrateStorage" apps/web --include="*.ts" --include="*.tsx" -A 5 -B 2Repository: solid-connection/solid-connect-web
Length of output: 710
🌐 Web query:
Zustand persist middleware best practices token validation hydration pattern
💡 Result:
Zustand persist middleware best practices for token validation and hydration, particularly for authentication stores, involve several key patterns grounded in official documentation and community implementations. ## Store Setup Use persist middleware with partialize to store only serializable token data (avoid functions/actions). Store tokens as strings. Example auth store: import { create } from 'zustand'; import { persist, createJSONStorage } from 'zustand/middleware'; import jwtDecode from 'jwt-decode'; interface AuthState { accessToken: string | null; refreshToken: string | null; user: any | null; isAuthenticated: boolean; setTokens: (access: string, refresh?: string, user?: any) => void; logout: => void; init: => void; // For validation } export const useAuthStore = create( persist( (set, get) => ({ accessToken: null, refreshToken: null, user: null, isAuthenticated: false, setTokens: (accessToken, refreshToken, user) => { set({ accessToken, refreshToken, user, isAuthenticated: true }); }, logout: => { set({ accessToken: null, refreshToken: null, user: null, isAuthenticated: false }); }, init: async => { const { accessToken, refreshToken } = get; if (accessToken) { try { // Validate JWT expiration const decoded = jwtDecode<{ exp: number }>(accessToken); if (decoded.exp * 1000 > Date.now) { // Valid, fetch user if needed set({ isAuthenticated: true }); } else if (refreshToken) { // Attempt refresh } else { get.logout; } } catch { get.logout; } } } }), { name: 'auth-storage', storage: createJSONStorage( => localStorage), partialize: (state) => ({ accessToken: state.accessToken, refreshToken: state.refreshToken }), // Custom merge for functions/serialization issues merge: (persistedState, currentState) => ({ ...currentState, ...persistedState }), } ) ); {value: "Example auth store code", citations: [1,5,6,7], confidence: "high"} ## Hydration Pattern Track hydration to prevent rendering mismatches (critical in SSR like Next.js). 1. Add hasHydrated flag: onRehydrateStorage: (state) => (state, error) => { if (!error && state) state.setHasHydrated(true); } In store: _hasHydrated: false, setHasHydrated: (hydrated) => set({ _hasHydrated: hydrated }), Use: const hydrated = useAuthStore((s) => s._hasHydrated); if (!hydrated) return ; {value: "Hydration tracking with onRehydrateStorage", citations: [1,3,14], confidence: "high"} For SSR/Next.js: - skipHydration: true - Manual rehydrate in client component: 'use client'; useEffect( => { useAuthStore.persist.rehydrate.then( => useAuthStore.getState.init); }, []); {value: "SSR hydration handling", citations: [1,3,12,21], confidence: "high"} Alternative for hydration mismatches: useSyncExternalStore for selectors differing server/client. {value: "useSyncExternalStore for external stores", citations: [4], confidence: "medium"} ## Token Validation - Do not trust persisted tokens blindly. Always validate on hydration/init. - Decode JWT, check exp claim. - If expired, attempt refresh with refreshToken, else logout/clearStorage. - Call init after rehydrate. - Use onRehydrateStorage to conditionally clear invalid state (e.g., delete localStorage if no valid JWT). {value: "Token validation on init/hydration", citations: [6,16,19], confidence: "high"} ## Best Practices 1. partialize: Persist only tokens, not user data or functions. Fetch user on validation. 2. Actions separate: Store data only; validate in actions/getters. 3. SSR: Use skipHydration + manual rehydrate + hydration flag. 4. Security: Tokens in localStorage ok for SPA but consider httpOnly cookies for refreshToken. 5. Refresh logic: Intercept API calls, refresh if 401. 6. Selectors: Use shallow for performance. 7. Versioning: Use version for breaking changes. This pattern ensures tokens persist reliably, validate on load, and handle hydration without mismatches. {value: "Summary of best practices", citations: [1,6,7,9], confidence: "high"}
Citations:
- 1: https://docs.pmnd.rs/zustand/integrations/persisting-store-data
- 2: https://stackoverflow.com/questions/79733899/zustand-persist-in-next-js-resets-state-after-navigation-and-causes-hydration-mi
- 3: https://medium.com/@judemiracle/fixing-react-hydration-errors-when-using-zustand-persist-with-usesyncexternalstore-b6d7a40f2623
- 4: https://dev.to/atsyot/solving-zustand-persisted-store-re-hydtration-merging-state-issue-1abk
- 5: https://doichevkostia.dev/blog/authentication-store-with-zustand
- 6: https://medium.com/@jkc5186/managing-user-sessions-with-zustand-in-react-5bf30f6bc536
- 7: Persist middleware and SSR/SSG pmndrs/zustand#1145
- 8: How to restore persisted state on page refresh pmndrs/zustand#1569
- 9: Add ability to delete, or choose not to rehydrate the store in onRehydrateStorage pmndrs/zustand#405
- 10: https://stackoverflow.com/questions/76650255/how-to-use-zustand-for-user-auth-global-state-in-next-js-13-4-9
- 11: https://stackoverflow.com/questions/72649255/nextjs-zustand-persist-state
hydration 콜백 구조를 수정하여 직접 mutation 대신 setState로 반영해주세요.
현재 코드는 onRehydrateStorage에서 state를 직접 변경하고 있어서 Zustand의 구독자 알림이 제대로 전파되지 않을 수 있습니다. 특히 localStorage 같은 동기 저장소에서는 이 변경사항이 최종 hydration 완료 전에 덮어씌워질 위험이 있습니다. 만료된 토큰을 정리하려면 다음처럼 콜백 구조를 바꾸어 setState를 통해 업데이트하는 것이 안전합니다:
✅ 개선된 패치
onRehydrateStorage: () => (state, error) => {
- // hydration 완료 후 isInitialized를 true로 설정
- if (state) {
+ if (!error && state) {
const hasValidToken = Boolean(state.accessToken && !isTokenExpired(state.accessToken));
- if (!hasValidToken) {
- state.accessToken = null;
- state.userRole = null;
- state.isAuthenticated = false;
- state.refreshStatus = "idle";
- } else {
- state.userRole = parseUserRoleFromToken(state.accessToken);
- state.isAuthenticated = true;
- state.refreshStatus = "success";
- }
-
- state.isInitialized = true;
+ useAuthStore.setState(
+ hasValidToken
+ ? {
+ userRole: parseUserRoleFromToken(state.accessToken),
+ isAuthenticated: true,
+ refreshStatus: "success",
+ isInitialized: true,
+ }
+ : {
+ accessToken: null,
+ userRole: null,
+ isAuthenticated: false,
+ refreshStatus: "idle",
+ isInitialized: true,
+ },
+ );
}
},변경 요점:
- 콜백 함수에 error 파라미터 추가 → hydration 오류 상황 대응
- 콜백 내부에서 setState로 상태 업데이트 → 모든 구독자에게 변경 알림 전파
- 동기 저장소에서도 올바른 순서로 업데이트 처리 → 최종 hydration 상태 덮어씌워지지 않음
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/zustand/useAuthStore.ts` around lines 91 - 104, The
hydration callback is directly mutating the Zustand state (state.accessToken,
state.userRole, state.isAuthenticated, state.refreshStatus, state.isInitialized)
which can break subscriber notifications and be overwritten by sync storages;
change the onRehydrateStorage callback signature to (state, error) and perform
all updates via the store's setState API instead of assigning to state directly:
call setState with a new partial state that clears an expired token (use
isTokenExpired(state.accessToken)), sets userRole via parseUserRoleFromToken
when valid, toggles refreshStatus to "idle" or "success", and marks
isInitialized true so subscribers receive the change atomically.
1 participant
변경 내용
docs/auth-refresh-edge-cases.md)배경
검증
pnpm --filter @solid-connect/web run typecheck