fix: 앱 로그인 풀림 현상 해결 - #371
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
|
Warning Review limit reached
Next review available in: 58 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review 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 refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughJWT 처리를 Changes인증 토큰 동기화
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Page
participant WebView
participant CookieSync
participant SecureStore
Page->>WebView: about:blank 워밍업 로드
WebView-->>Page: onLoadEnd
Page->>CookieSync: WebView 준비 완료 전달
CookieSync->>WebView: 쿠키 조회 및 설정
CookieSync->>SecureStore: 최신 토큰 저장
Page->>WebView: APP_REQ_NAVIGATE 전달
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/web/src/apis/postTokenRefresh.ts (1)
6-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win응답 타입을 이름 있는
ResponseT로 추출 권장.
ApiResponseT<{ accessToken: string | null; refreshToken: string | null }>가 인라인으로 선언되어 있습니다. 동일 shape가proxy.ts에서도 별도로 가정되고 있어(그래프 컨텍스트:postTokenRefreshServer소비부), 추후 필드가 바뀔 때 두 곳을 동기화해야 하는 드리프트 위험이 있습니다.PostTokenRefreshServerResponseT같은 이름으로 추출해 재사용하는 것을 권장합니다.As per coding guidelines, "Name API request and response types as
{PascalCaseFunctionName}RequestTand{PascalCaseFunctionName}ResponseT."♻️ 제안 diff
+export type PostTokenRefreshServerResponseT = ApiResponseT<{ + accessToken: string | null; + refreshToken: string | null; +}>; + export const postTokenRefreshServer = async (cookies: string) => { - const response = await serverApi.post< - ApiResponseT<{ accessToken: string | null; refreshToken: string | null }> - >(ENDPOINTS.AUTH_TOKEN_REFRESH, void 0, { + const response = await serverApi.post<PostTokenRefreshServerResponseT>( + ENDPOINTS.AUTH_TOKEN_REFRESH, + void 0, + {🤖 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 `@apps/web/src/apis/postTokenRefresh.ts` around lines 6 - 15, Extract the inline token-refresh payload type used by postTokenRefreshServer into a named PostTokenRefreshServerResponseT type following the API naming convention, and use that type in the serverApi.post generic while preserving the existing nullable accessToken and refreshToken fields.Source: Coding guidelines
apps/app/utils/tokenStorage.ts (1)
15-21: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win동시 호출 시 TOCTOU 레이스 가능성.
setTokens의 최신성 검사는 read-then-write 구조로 원자적이지 않습니다.useWebviewCookieSync.ts의 초기 동기화 effect와AppState리스너 effect가 거의 동시에setTokens를 호출하면(예: 콜드 스타트 시), 둘 다 같은 오래된storedRefreshToken을 읽은 뒤 나중에 쓰는 호출이 최신 토큰을 덮어쓸 수 있습니다. 이슈#367이목표로 하는 "최신 토큰만 저장" 보장이 동시성 상황에서는 완전하지 않습니다.간단한 in-memory 프라미스 기반 락으로
setTokens호출을 직렬화하면 이 창을 닫을 수 있습니다.♻️ 제안 방향
+let setTokensLock: Promise<void> = Promise.resolve(); + async setTokens(accessToken: string, refreshToken: string) { + const run = async () => { const storedRefreshToken = await TokenStorage.getRefreshToken(); if (storedRefreshToken === refreshToken) return; if (storedRefreshToken && !isFresherToken(refreshToken, storedRefreshToken)) return; const options = getSecureStoreOptions(); await Promise.all([...]); + }; + setTokensLock = setTokensLock.then(run); + return setTokensLock; },🤖 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 `@apps/app/utils/tokenStorage.ts` around lines 15 - 21, setTokens의 storedRefreshToken 조회와 토큰 저장을 in-memory Promise 기반 락으로 직렬화해 동시 호출 시에도 한 번에 하나만 최신성 검사를 수행하도록 수정하세요. 각 호출은 이전 작업 완료를 기다린 뒤 기존 isFresherToken 검사를 실행하고, 저장 작업이 성공하거나 실패해도 락이 반드시 해제되도록 하며, 최신 토큰만 저장하는 현재 동작은 유지하세요.apps/web/src/proxy.ts (1)
46-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win쿠키 Max-Age append 로직 중복 — 헬퍼로 추출 권장.
handleGuestLogin과handleTokenRefresh에서cookieOptions구성,getTokenMaxAge계산,set-cookieappend 로직이 거의 동일하게 중복되어 있습니다. 공용 헬퍼(예:appendAppAuthCookies(response, bodyAccess, bodyRefresh))로 추출하면 유지보수성이 좋아지고, 두 곳의 Max-Age 처리 방식이 향후 드리프트되는 것을 방지할 수 있습니다.♻️ 제안 방향
+const appendAppAuthCookies = ( + response: NextResponse, + bodyAccess: string | null | undefined, + bodyRefresh: string | null | undefined +) => { + if (!bodyAccess || !bodyRefresh) return; + const cookieOptions = `Path=/; SameSite=Lax${process.env.NODE_ENV === 'production' ? '; Secure' : ''}`; + const accessMaxAge = getTokenMaxAge(bodyAccess); + const refreshMaxAge = getTokenMaxAge(bodyRefresh); + response.headers.append( + 'set-cookie', + `access_token=${bodyAccess}; ${cookieOptions}${accessMaxAge === null ? '' : `; Max-Age=${accessMaxAge}`}` + ); + response.headers.append( + 'set-cookie', + `refresh_token=${bodyRefresh}; ${cookieOptions}${refreshMaxAge === null ? '' : `; Max-Age=${refreshMaxAge}`}` + ); +};Also applies to: 150-164
🤖 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 `@apps/web/src/proxy.ts` around lines 46 - 60, Extract the duplicated app authentication cookie logic from handleGuestLogin and handleTokenRefresh into a shared helper such as appendAppAuthCookies(response, bodyAccess, bodyRefresh). Move cookieOptions construction, getTokenMaxAge calls, and both set-cookie appends into the helper, then replace both inline implementations with helper calls while preserving the existing conditional behavior.
🤖 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/app/app/index.tsx`:
- Around line 147-158: Update the warmup flow around handleWebViewLoadEnd so
isWarmupLoaded transitions even when the initial about:blank onLoadEnd event is
delayed or missing. Add a bounded timeout fallback for the html: '' WebView
load, ensure it is cleared when the warmup load completes, and preserve the
existing normal event-driven transition.
In `@apps/app/hooks/useWebviewCookieSync.ts`:
- Around line 57-62: Handle nullable token fields at both refresh-response
sites: in apps/app/hooks/useWebviewCookieSync.ts lines 57-62, validate both
tokens before TokenStorage.setTokens and follow the existing 401
cleanup/cookie-deletion path when either is missing; in
apps/app/utils/postWishLinkFromShare.ts lines 45-51, validate both tokens before
storing them or retrying postWishLink, preserving the existing failure behavior.
In `@packages/core/src/utils/jwt.ts`:
- Around line 33-37: Separate isTokenValid into an expiration-only helper and
ensure it is not used for authorization decisions. Update the protected-route
logic in proxy.ts to require server-side JWT signature, issuer, and audience
validation or a trusted backend authentication result before returning
NextResponse.next(), while preserving isTokenValid’s expiration-check behavior
for non-authorization uses.
- Around line 78-87: Align getTokenMaxAge with its documented expired-token
contract by returning null when exp has elapsed, rather than clamping the result
to 0. Preserve null for malformed tokens, and update related tests and the
getTokenMaxAge comment to reflect the chosen behavior consistently with the
proxy cookie handling.
---
Nitpick comments:
In `@apps/app/utils/tokenStorage.ts`:
- Around line 15-21: setTokens의 storedRefreshToken 조회와 토큰 저장을 in-memory Promise
기반 락으로 직렬화해 동시 호출 시에도 한 번에 하나만 최신성 검사를 수행하도록 수정하세요. 각 호출은 이전 작업 완료를 기다린 뒤 기존
isFresherToken 검사를 실행하고, 저장 작업이 성공하거나 실패해도 락이 반드시 해제되도록 하며, 최신 토큰만 저장하는 현재 동작은
유지하세요.
In `@apps/web/src/apis/postTokenRefresh.ts`:
- Around line 6-15: Extract the inline token-refresh payload type used by
postTokenRefreshServer into a named PostTokenRefreshServerResponseT type
following the API naming convention, and use that type in the serverApi.post
generic while preserving the existing nullable accessToken and refreshToken
fields.
In `@apps/web/src/proxy.ts`:
- Around line 46-60: Extract the duplicated app authentication cookie logic from
handleGuestLogin and handleTokenRefresh into a shared helper such as
appendAppAuthCookies(response, bodyAccess, bodyRefresh). Move cookieOptions
construction, getTokenMaxAge calls, and both set-cookie appends into the helper,
then replace both inline implementations with helper calls while preserving the
existing conditional behavior.
🪄 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 Plus
Run ID: b8ad9c41-cdc8-470f-b268-06e65f1960fc
📒 Files selected for processing (11)
apps/app/app/index.tsxapps/app/hooks/useWebviewCookieSync.tsapps/app/utils/postWishLinkFromShare.tsapps/app/utils/tokenStorage.tsapps/web/src/apis/postTokenRefresh.tsapps/web/src/hooks/useFcmTokenSync.tsapps/web/src/proxy.tsapps/web/src/utils/auth.tsapps/web/src/utils/refreshClientToken.tspackages/core/src/index.tspackages/core/src/utils/jwt.ts
* fix: 내 토너먼트 페이지 게스트도 접근 가능하도록 변경 * fix: 위시 아카이브 로그인 유도 렌더를 try/catch 밖으로 분리
작업 요약
WKWebView쿠키 저장 특성으로 인한 쿠키 동기화 타이밍 문제 동반배경
작업 내용
1. 토큰 갱신 응답 body를 camelCase로 파싱
{ accessToken, refreshToken }인데 여러 곳에서access_token(snake_case)으로 파싱 → 새 토큰을undefined로 받아 저장 실패useWebviewCookieSync,postWishLinkFromShare,proxy,refreshClientToken,postTokenRefresh2. 웹뷰 워밍업 후 쿠키 동기화 + 양방향 동기화 통합
WKHTTPCookieStore는 살아있는WKWebView없이 쿠키 읽기/쓰기가 유실 → 기존엔 웹뷰 렌더 전 동기화로 시딩 누락sharedCookiesEnabled+ iOS 쿠키 이중 저장(WKHTTPCookieStore / NSHTTPCookieStorage)useWebviewCookieSync로 통합3. JWT 유틸
@piki/core이전 + iat 기반 최신 토큰 판별packages/core/src/utils/jwt.ts신규 —decodeJwtPayload/isTokenValid/isFresherToken/getTokenExpiresIso/getTokenMaxAgeiat가 최신인 토큰만 채택 → 오래된 사본이 최신 토큰을 덮어쓰지 않도록4. proxy 세션 쿠키 → 만료 시간 지정
Max-Age가 없어 앱 종료 시 증발 → 토큰exp기반Max-Age부여5. 토큰 갱신 응답 타입
AuthTokensT추출packages/core/src/types/auth.ts신규 —AuthTokensT = { accessToken: string | null; refreshToken: string | null }postTokenRefresh,refreshClientToken,proxy(반환 타입 경유) 적용연관 이슈
closes #367
Summary by CodeRabbit