Feat: goolgle login 페이지 추가 및 연결 #204
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough온보딩에 구글 소셜 로그인 추가: SocialLoginStep이 Google OAuth로 리다이렉트하고 콜백 경로( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Browser
participant Client as SocialLoginStep
participant Google
participant Callback as GoogleCallback
participant API as Backend (/api/v2/auth/google)
participant Storage as LocalStorage
User->>Client: 구글 로그인 버튼 클릭
Client->>Browser: Google OAuth URL로 리다이렉트
Browser->>Google: 인증/동의 요청
Google->>Browser: 리다이렉트 콜백 (/onboarding/callback?code=...)
Browser->>Callback: 콜백 페이지 로드
Callback->>API: POST /api/v2/auth/google (code)
API-->>Callback: { email, userId, accessToken?, isUser }
Callback->>Storage: email, userId (및 accessToken if provided)
alt isUser = true
Callback->>Browser: "/"로 네비게이션
else isUser = false
Callback->>Browser: "/onboarding?step=ALARM"로 네비게이션
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
✅ Storybook chromatic 배포 확인: |
![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
🧹 Nitpick comments (2)
apps/client/src/shared/apis/setting/axiosInstance.ts (1)
36-40: noAuthNeeded 배열 중복 정의 → 상수 추출 고려요청/응답 인터셉터에서
noAuthNeeded배열을 각각 정의하고 있어서, 이후 예외 경로를 추가/수정할 때 한쪽만 수정되는 실수 가능성이 있습니다. 파일 상단에 예를 들어:const NO_AUTH_PATHS = [ '/api/v1/auth/token', '/api/v1/auth/signup', '/api/v2/auth/google', ];처럼 상수로 추출해 두고 두 인터셉터에서
NO_AUTH_PATHS를 재사용하면 유지보수성이 좋아질 것 같습니다.Also applies to: 71-75
apps/client/src/pages/onBoarding/GoogleCallback.tsx (1)
8-19: OAuth 콜백에서 state 검증 및 불필요한 리로드 제거 제안검증 결과, 리뷰 댓글 중 일부는 정확하지만 보완이 필요합니다.
SocialLoginStep의
state값 검증
- OAuth 보안 관점에서 권장됩니다.
state파라미터를 추가했다면 여기서도sessionStorage의 저장값과 비교해 검증하고, 불일치 시 로그인을 중단하세요.- 예시:
useEffect(() => { const params = new URLSearchParams(window.location.search); const code = params.get('code'); const state = params.get('state'); const storedState = sessionStorage.getItem('google_oauth_state'); if (!code || !state || !storedState || state !== storedState) { alert('로그인 실패. 다시 시도해주세요.'); navigate('/onboarding?step=3'); return; } sessionStorage.removeItem('google_oauth_state'); loginWithCode(code); }, [navigate]);온보딩 path 일관성 개선
- React Router는 경로 매칭 시 기본적으로 대소문자를 무시하므로
/onboarding과/onBoarding의 대소문자 차이는 실제 매칭 문제를 일으키지 않습니다. 다만, 코드베이스 일관성을 위해ROUTES_CONFIG.onBoarding.path를 import해서 사용하는 방식으로 통일하면 유지보수성이 개선됩니다.
window.location.reload()제거 권장
- SPA에서
window.location.reload()는 전체 페이지를 재로딩해 클라이언트 라우팅, 애플리케이션 상태, 폼 데이터 등이 손실되고 성능 저하 및 UX가 악화됩니다.- 토큰/유저 정보를 최상단에서
localStorage로부터 초기화하는 구조라면,navigate호출만으로도 충분합니다. 실제 필요성을 재점검하고 불필요하면 제거하세요.
console.log(res);제거
- 배포 전 디버깅용 로그를 제거하세요.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/client/src/assets/chippi_extension_popup.svgis excluded by!**/*.svgapps/client/src/assets/onBoarding/icons/googleLogo.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
apps/client/src/pages/onBoarding/GoogleCallback.tsx(1 hunks)apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx(7 hunks)apps/client/src/pages/onBoarding/components/funnel/step/SocialLoginStep.tsx(1 hunks)apps/client/src/routes/router.tsx(2 hunks)apps/client/src/routes/routesConfig.ts(1 hunks)apps/client/src/shared/apis/setting/axiosInstance.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/client/src/routes/router.tsx (1)
apps/client/src/routes/routesConfig.ts (1)
ROUTES_CONFIG(1-26)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (2)
apps/client/src/constants/alarms.ts (1)
AlarmsType(11-15)apps/client/src/pages/onBoarding/utils/formatRemindTime.ts (1)
normalizeTime(1-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (3)
apps/client/src/routes/routesConfig.ts (1)
22-25: 구글 OAuth 콜백 라우트 엔트리 추가 확인
onBoardingCallback라우트가title: '구글 OAuth 콜백',path: '/onboarding/callback'으로 정의되어 있고,router.tsx에서 이 값을 그대로 사용해GoogleCallback을 매핑하고 있어 설정이 잘 맞춰져 있습니다.apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
186-187: CardStyle overflow 조건 변경 확인
CardStyle({ overflow: step === 4 && alarmSelected === 3 })로 조건을 좁혀서 AlarmStep의 '사용자 설정'(3번)인 경우에만 overflow를 풀어주는 구조는 해당 step에서의 UI 높이 문제만 해결하고 다른 단계에는 영향을 주지 않아 보입니다. 이 변경 자체는 무리 없어 보입니다.apps/client/src/routes/router.tsx (1)
4-4: Google OAuth 콜백 라우트와 환경 변수 설정 정합성 재확인 필요React Router 7에서 path에 leading slash(
/)가 있으면 중첩 라우트 내에서도 절대 경로로 처리되어, URL이 정확히/onboarding/callback일 때만 매칭됩니다. 따라서ROUTES_CONFIG.onBoardingCallback.path = '/onboarding/callback'은 의도대로 동작합니다. 또한 경로 매칭은 기본적으로 대소문자 비구분이므로 케이싱 차이는 문제가 되지 않습니다.다만 Google OAuth 콜백이 정상 작동하려면 배포 도메인 +
/onboarding/callback전체 URL이 Google OAuth 설정에서 등록한VITE_GOOGLE_REDIRECT_URI와 정확히 일치(프로토콜, 도메인, path 포함)해야 합니다. 환경 변수 설정을 다시 한 번 확인해 주세요.
| case 3: | ||
| return <SocialLoginStep />; | ||
| case 4: | ||
| return ( | ||
| <AlarmStep selected={alarmSelected} setSelected={setAlarmSelected} /> | ||
| ); | ||
| case 4: | ||
| case 5: | ||
| if (isMac) return <MacStep />; | ||
| return <FinalStep />; | ||
| case 5: | ||
| case 6: | ||
| if (isMac) return <FinalStep />; | ||
| return null; | ||
|
|
There was a problem hiding this comment.
온보딩 nextStep 로직 때문에 마지막 단계/회원가입으로 진행이 막히는 문제
현재 nextStep 로직 때문에 Mac/비Mac 사용자 모두 온보딩 마지막 단계에 도달하지 못하고, postSignData도 호출되지 않는 흐름이 됩니다.
- 단계 진행 조건 버그
if ((isMac && step < 5) || (!isMac && step < 4)) {
setDirection(1);
setStep(next);
navigate(`/onboarding?step=${next}`);
return;
}
if ((isMac && step === 6) || (!isMac && step === 5)) {
postSignData(...);
}- Mac 사용자:
- step 0~4 →
< 5조건으로 5까지는 정상 진행. - step 5일 때는 위 조건이 false라
setStep(6)이 호출되지 않아서 case 6(최종 단계)로는 절대 진입하지 못합니다. - 따라서
step === 6분기 역시 도달 불가입니다.
- step 0~4 →
- 비Mac 사용자:
- step 0~3 →
< 4조건으로 4까지는 진행. - step 4(AlarmStep)에서
< 4가 false라 step 5(최종 단계)로 이동이 막혀step === 5분기도 도달 불가입니다.
- step 0~3 →
의도대로라면 아마 다음과 같이 조건을 조정하는 것이 자연스러워 보입니다:
- if ((isMac && step < 5) || (!isMac && step < 4)) {
+ if ((isMac && step < 6) || (!isMac && step < 5)) {
setDirection(1);
setStep(next);
navigate(`/onboarding?step=${next}`);
return;
}
- if ((isMac && step === 6) || (!isMac && step === 5)) {
+ if ((isMac && step === 6) || (!isMac && step === 5)) {
postSignData(...);
}이렇게 하면:
- Mac: 0→1→2→3(Social)→4(Alarm)→5(MacStep)→6(FinalStep) 후 Next 클릭 시
postSignData호출. - 비Mac: 0→1→2→3(Social)→4(Alarm)→5(FinalStep) 후 Next 클릭 시
postSignData호출.
- 알람 '사용자 설정' 시간 값이 비어 있는 경우 처리
if (step === 4) {
if (alarmSelected === 1) setRemindTime('09:00');
else if (alarmSelected === 2) setRemindTime('20:00');
else {
const raw = AlarmsType[alarmSelected - 1].time;
setRemindTime(normalizeTime(raw));
}
}AlarmsType[2].time초기값이''인 상태에서 사용자가 유효한 시간을 설정하지 않고 그대로 진행하면,normalizeTime('')호출 시'NaN:NaN'같은 문자열이 나올 수 있습니다.- 최소한
raw가 빈 문자열이거나 정규식에 매칭되지 않는 경우에는 기존remindTime을 유지하거나, 기본값으로 fallback 하는 방어 로직이 있으면 좋겠습니다. 예:
else {
const raw = AlarmsType[alarmSelected - 1].time;
if (!raw) {
// 기본값 유지 또는 안전한 기본값 지정
setRemindTime(remindTime);
} else {
setRemindTime(normalizeTime(raw));
}
}위 두 부분이 수정되지 않으면 온보딩 완료 및 알람 설정 값 전송(remindDefault)에 문제가 생길 수 있어, 우선순위 높게 봐야 할 것 같습니다.
Also applies to: 135-151, 154-174
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
115-127 (also check 135-151 and 154-174), the nextStep branching prevents
reaching the final onboarding step and calling postSignData because the step
ceiling checks use strict < thresholds; change the progression logic so Mac
users allow advancing when step <= 5 (so step 5 can advance to 6) and non-Mac
users allow advancing when step <= 4 (so step 4 can advance to 5), and adjust
the postSignData trigger to fire when (isMac && step === 6) or (!isMac && step
=== 5) after incrementing; additionally, when handling alarmSelected in step ===
4, add a guard for empty or invalid AlarmsType[alarmSelected - 1].time (check
falsy or regex mismatch) and in that case preserve the existing remindTime or
set a safe default instead of calling normalizeTime(''), only calling
normalizeTime when raw is non-empty and valid.
| const SocialLoginStep = () => { | ||
| const handleGoogleLogin = () => { | ||
| const clientId = import.meta.env.VITE_GOOGLE_CLIENT_ID; | ||
| const redirectUri = import.meta.env.VITE_GOOGLE_REDIRECT_URI; | ||
|
|
||
| if (!clientId || !redirectUri) { | ||
| alert('Google OAuth 설정이 누락되었습니다.'); | ||
| return; | ||
| } | ||
|
|
||
| const googleAuthUrl = | ||
| `https://accounts.google.com/o/oauth2/v2/auth?` + | ||
| `client_id=${clientId}` + | ||
| `&redirect_uri=${redirectUri}` + | ||
| `&response_type=code` + | ||
| `&scope=email profile`; | ||
|
|
||
| window.location.href = googleAuthUrl; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Google OAuth URL 구성 시 보안(state) 및 인코딩 개선 필요
현재 handleGoogleLogin에서 Google OAuth URL을 직접 문자열로 이어 붙이고 있고, state 파라미터가 빠져 있습니다. 두 가지 개선을 강하게 추천드립니다.
-
state파라미터 추가 및 콜백에서 검증- 랜덤한
state값을 생성해sessionStorage등에 저장하고, OAuth URL에 함께 실어 보내야 CSRF 공격 및 콜백 URL 오염을 방지할 수 있습니다. - 이후 콜백 컴포넌트(GoogleCallback)에서
state쿼리값과 저장된 값을 비교해 일치하지 않으면 에러 처리해야 안전합니다.
- 랜덤한
-
URL 파라미터 인코딩
redirectUri,scope등의 값은 공백/특수문자를 포함할 수 있으므로URLSearchParams를 사용해 안전하게 인코딩하는 편이 좋습니다.
예시는 아래처럼 구현할 수 있습니다.
- const googleAuthUrl =
- `https://accounts.google.com/o/oauth2/v2/auth?` +
- `client_id=${clientId}` +
- `&redirect_uri=${redirectUri}` +
- `&response_type=code` +
- `&scope=email profile`;
-
- window.location.href = googleAuthUrl;
+ const state = crypto.randomUUID();
+ sessionStorage.setItem('google_oauth_state', state);
+
+ const params = new URLSearchParams({
+ client_id: clientId,
+ redirect_uri: redirectUri,
+ response_type: 'code',
+ scope: 'email profile',
+ state,
+ });
+
+ window.location.href =
+ `https://accounts.google.com/o/oauth2/v2/auth?${params.toString()}`;(콜백 쪽에서 state 검증 로직도 같이 추가해 주셔야 합니다.)
Also applies to: 40-50
🌐 Web query:
Google OAuth 2.0 authorization endpoint에서 `state` 파라미터의 역할과 SPA에서 사용 시 권장되는 보안 패턴(PKCE 등)을 확인해 주세요.
💡 Result:
요약 — 목적과 권장 패턴
- state 파라미터 역할
- OAuth 요청과 응답 사이에 임의의 문자열을 유지해 "요청 ↔ 응답"이 같은 브라우저/세션에서 발생했음을 검증(특히 CSRF 방지용). 서버가 받은 state가 요청할 때 보낸 값과 일치하는지 확인해야 함. 또한 리다이렉트 후 원래 위치 복원, nonce 전달 등에도 사용 가능. [1][2].
- SPA(브라우저 앱)에서 권장 보안 패턴
- Authorization Code flow + PKCE 사용: 브라우저 앱은 Authorization Code 흐름에 PKCE(code_challenge/code_verifier)를 결합해 사용해야 하며(implicit flow 사용 권장 안 함), PKCE는 공용 클라이언트에서 인증 코드 탈취를 방지. Google도 코드 모델(PKCE 포함) 및 팝업 UX 권장을 제시. [3][4].
- state(또는 OIDC nonce) 생성·검증: 요청 시 충분히 랜덤한 state(권장: 고비트 랜덤, 일회용) 생성→ 로컬(세션 쿠키/메모리/secure storage)에 보관→ 리다이렉트 응답에서 비교 검증. CSRF 방지 목적이라면 state를 쿠키 해시나 세션 식별자와 결합해 검증. [1][2].
- code_verifier 보관 방식: PKCE의 code_verifier는 교환 전까지 안전하게 보관(권장: 메모리 또는 짧은 수명 세션 스토리지). 장기 저장 금지. [3].
- 토큰 취급: 액세스/리프레시 토큰을 브라우저에 장기 저장하지 말 것(가능하면 HttpOnly SameSite 쿠키나 백엔드로 토큰 보관). 브라우저에 리프레시 토큰을 발급할 경우 리프레시 토큰 회전(rotation)과 제한된 수명 적용 권장. [5].
- 리다이렉트 검증: 등록된 redirect_uri와 정확히 일치하는지 확인(리다이렉트 URI 검증 엄격 적용). [5].
- 라이브러리 사용: 구현 실수 위험이 있으므로 Google Identity Services 등 검증된 라이브러리 사용 권장. [4].
참고(주요 출처)
- Google OAuth 문서(상태 파라미터 설명, 클라이언트 측/서버 측 권장사항). [1][2][4]
- Google 권장: Authorization Code model (팝업 UX 등) 및 PKCE 사용 가이드. [3]
- IETF 권고(“OAuth 2.0 for Browser-Based Apps”) — 브라우저 앱에 PKCE, 리프레시 토큰 회전, 정확한 redirect URI, 액세스 토큰 응답 금지 등 요구사항. [5]
원하시면 샘플 구현 절차(요청 예시, state/PKCE 생성·검증 코드 스니펫, 안전한 토큰 저장 방안)를 한국어 코드 예제로 바로 드리겠습니다.
웹 검색 결과를 검토했습니다. Google OAuth 공식 문서와 IETF 권고에서 확인된 내용:
✓ state 파라미터는 필수 — CSRF 공격 방지 및 요청-응답 매칭 검증용
✓ SPA에서는 Authorization Code + PKCE 조합 권장 — 인증 코드 탈취 방지
✓ URLSearchParams 사용 — 안전한 파라미터 인코딩
✓ 콜백에서 state 검증 필수 — 일치 확인 후 진행
원본 리뷰의 보안 지적이 모두 검증되었습니다. 최종 리뷰 코멘트를 생성하겠습니다.
Google OAuth URL 구성 시 CSRF 방지(state) 및 보안 개선 필수
현재 handleGoogleLogin에서 Google OAuth URL을 문자열 연결로 구성하고 있으며, state 파라미터가 누락되어 CSRF 공격 및 콜백 리다이렉트 오염에 취약합니다. 다음 개선이 필수입니다.
-
state파라미터 생성 및 검증 (CSRF 방지)- 랜덤한
state값을 생성해sessionStorage에 저장하고 OAuth URL에 포함 - 콜백 컴포넌트(GoogleCallback)에서 쿼리의
state값과 저장된 값을 비교해 일치 확인 후 진행
- 랜덤한
-
URL 파라미터 안전한 인코딩
URLSearchParams사용으로redirectUri,scope등 특수문자 안전 처리
-
PKCE 검토 (권장)
- SPA에서는 Authorization Code + PKCE 조합이 권장 패턴입니다. 라이브러리(Google Identity Services) 사용 권장.
구현 예시:
- const googleAuthUrl =
- `https://accounts.google.com/o/oauth2/v2/auth?` +
- `client_id=${clientId}` +
- `&redirect_uri=${redirectUri}` +
- `&response_type=code` +
- `&scope=email profile`;
-
- window.location.href = googleAuthUrl;
+ const state = crypto.randomUUID();
+ sessionStorage.setItem('google_oauth_state', state);
+
+ const params = new URLSearchParams({
+ client_id: clientId,
+ redirect_uri: redirectUri,
+ response_type: 'code',
+ scope: 'email profile',
+ state,
+ });
+
+ window.location.href =
+ `https://accounts.google.com/o/oauth2/v2/auth?${params.toString()}`;콜백 컴포넌트에서도 state 검증 로직을 추가하세요.
(Also applies to: lines 40-50)
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/step/SocialLoginStep.tsx
around lines 4-22 (and also apply the same fix to lines 40-50), the Google OAuth
URL is built by string concatenation and omits a required state parameter;
generate a cryptographically random state, save it to sessionStorage, build the
auth URL with URLSearchParams (including client_id, redirect_uri,
response_type=code, scope, and state) to ensure proper encoding, and redirect to
that URL; additionally, plan to implement state verification in the callback
component to compare the incoming state against the stored value and consider
adding PKCE (or a Google Identity library) for SPA-safe authorization code flow.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
146-153: 알람 시간 빈 값 처리 로직이 누락되었습니다.사용자가 "사용자 설정" 옵션(alarmSelected === 3)을 선택했지만 시간을 설정하지 않은 경우,
AlarmsType[2].time은 빈 문자열입니다. 이 상태에서normalizeTime('')을 호출하면'NaN:NaN'이 반환되어 서버로 전송됩니다.다음과 같이 빈 값 검증을 추가하세요:
if (step === 4) { if (alarmSelected === 1) setRemindTime('09:00'); else if (alarmSelected === 2) setRemindTime('20:00'); else { const raw = AlarmsType[alarmSelected - 1].time; - setRemindTime(normalizeTime(raw)); + if (!raw || raw.trim() === '') { + // 기본값 설정 또는 기존 값 유지 + setRemindTime('09:00'); // 또는 기존 remindTime 유지 + } else { + setRemindTime(normalizeTime(raw)); + } } }또는 AlarmStep 컴포넌트에서 시간이 설정되지 않았을 경우 다음 단계로 진행을 막는 것도 고려해보세요.
🧹 Nitpick comments (2)
apps/client/src/pages/onBoarding/GoogleCallback.tsx (2)
13-13: 사용자 경험 개선: alert 대신 UI 컴포넌트를 사용하세요.
alert는 브라우저의 기본 다이얼로그로, 현대적인 웹 앱에서는 사용자 경험이 좋지 않습니다. 디자인 시스템의 Toast나 Modal 컴포넌트를 사용하는 것이 좋습니다.예를 들어:
if (!code) { - alert('로그인 실패. 다시 시도해주세요.'); + // Toast.error('로그인 실패. 다시 시도해주세요.'); navigate('/onboarding?step=3'); return; }} catch (error) { console.error('로그인 오류:', error); - alert('로그인 중 오류가 발생했습니다.'); + // Toast.error('로그인 중 오류가 발생했습니다.'); navigate('/onboarding?step=3'); }Also applies to: 43-43
47-47: TODO 주석이 있습니다. 로딩 컴포넌트로 교체가 필요합니다.로딩 상태를 표시하는 TODO 주석이 있습니다. 디자인 시스템의 Spinner나 Skeleton 컴포넌트로 교체하면 사용자 경험이 개선됩니다.
로딩 컴포넌트를 적용한 코드를 생성하거나 별도 이슈를 생성할까요?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/pages/onBoarding/GoogleCallback.tsx(1 hunks)apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-15T20:00:13.756Z
Learnt from: constantly-dev
Repo: Pinback-Team/pinback-client PR: 80
File: apps/client/src/shared/components/ui/modalPop/ModalPop.tsx:36-41
Timestamp: 2025-07-15T20:00:13.756Z
Learning: In apps/client/src/shared/components/ui/modalPop/ModalPop.tsx, the InfoBox component uses hardcoded values for title, location, and icon URL as temporary test data. These should be replaced with dynamic data from props when implementing actual functionality and should be marked with TODO comments for future changes.
Applied to files:
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx
🧬 Code graph analysis (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (2)
apps/client/src/constants/alarms.ts (1)
AlarmsType(11-15)apps/client/src/pages/onBoarding/utils/formatRemindTime.ts (1)
normalizeTime(1-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (4)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (4)
65-68: URL 파라미터 기반 step 초기화 로직이 추가되었습니다.URL의
step쿼리 파라미터를 읽어 초기 step 상태를 설정하는 로직이 잘 구현되었습니다. 브라우저 새로고침이나 직접 URL 접근 시에도 올바른 step이 표시됩니다.
155-160: Step 진행 로직이 올바르게 수정되었습니다.이전 리뷰에서 지적된 step 진행 조건이 올바르게 수정되었습니다:
- Mac 사용자: step 0~5에서 진행 가능 → step 6에서 회원가입 완료
- 비Mac 사용자: step 0~4에서 진행 가능 → step 5에서 회원가입 완료
이제 모든 사용자가 온보딩을 정상적으로 완료하고
postSignData가 호출됩니다.Also applies to: 162-181
231-240: SocialLoginStep에서 Next 버튼을 숨기는 로직이 올바릅니다.Step 3(SocialLoginStep)에서는 Google 로그인 버튼을 통해 외부로 리다이렉트되므로, Next 버튼을 렌더링하지 않는 것이 적절합니다.
194-194: Overflow 조건이 올바르게 설정되었습니다.검증 결과,
step === 4 && alarmSelected === 3조건은 정확합니다:
- alarmSelected === 3: AlarmBox에서 사용자 정의 알람(custom alarm) 옵션을 의미하며, 이 옵션이 선택될 때만 TimePicker 컴포넌트가 표시됩니다.
- step === 4: AlarmStep이 렌더링되는 단계로, 알람 선택 UI가 표시됩니다.
- TimePicker는 AlarmBox 내부에서 렌더링되며, MainCard의 경계를 벗어나기 때문에
overflow-visible이 필요합니다.조건은 TimePicker가 실제로 표시되는 경우에만 overflow를 허용하도록 정확하게 설정되어 있습니다.
| useEffect(() => { | ||
| const params = new URLSearchParams(window.location.search); | ||
| const code = params.get('code'); | ||
|
|
||
| if (!code) { | ||
| alert('로그인 실패. 다시 시도해주세요.'); | ||
| navigate('/onboarding?step=3'); | ||
| return; | ||
| } | ||
|
|
||
| loginWithCode(code); | ||
| }, []); |
There was a problem hiding this comment.
useEffect 의존성 배열에 누락된 항목이 있습니다.
useEffect 내부에서 navigate와 loginWithCode를 사용하고 있지만 의존성 배열에 포함되지 않았습니다. React 19에서는 이를 경고하며, 예상치 못한 동작이 발생할 수 있습니다.
다음과 같이 수정하세요:
useEffect(() => {
const params = new URLSearchParams(window.location.search);
const code = params.get('code');
if (!code) {
alert('로그인 실패. 다시 시도해주세요.');
navigate('/onboarding?step=3');
return;
}
loginWithCode(code);
- }, []);
+ }, [navigate, loginWithCode]);단, loginWithCode를 의존성에 추가하면 매 렌더링마다 재생성되므로 무한 루프가 발생할 수 있습니다. loginWithCode를 useCallback으로 감싸거나, useEffect 내부로 이동시키는 것을 권장합니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/GoogleCallback.tsx around lines 8 to 19, the
useEffect uses navigate and loginWithCode but omits them from the dependency
array, which causes React warnings and potential bugs; update the effect to
include navigate and loginWithCode in the dependency array and to avoid
re-creation loops wrap loginWithCode in useCallback (so it is stable) or move
the loginWithCode logic directly inside the useEffect callback, then add
navigate to the deps as well (and run lint/verify no infinite loop).
| const loginWithCode = async (code: string) => { | ||
| try { | ||
| const res = await apiRequest.post('/api/v2/auth/google', { | ||
| code, | ||
| }); | ||
| console.log(res); | ||
| const { isUser, userId, email, accessToken } = res.data.data; | ||
| console.log({ isUser, userId, email, accessToken }); | ||
| // 공통 저장 | ||
| localStorage.setItem('email', email); | ||
| localStorage.setItem('userId', userId); | ||
|
|
||
| if (isUser) { | ||
| // 기존 유저 | ||
| localStorage.setItem('token', accessToken); | ||
| navigate('/'); | ||
| } else { | ||
| // 신규 유저 | ||
| navigate('/onboarding?step=4'); | ||
| } | ||
| } catch (error) { | ||
| console.error('로그인 오류:', error); | ||
| alert('로그인 중 오류가 발생했습니다.'); | ||
| navigate('/onboarding?step=3'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
보안 취약점: accessToken을 localStorage에 저장하고 있습니다.
accessToken을 localStorage에 저장하는 것은 XSS 공격에 취약합니다. 공격자가 악성 스크립트를 주입하면 토큰을 탈취할 수 있습니다.
다음 중 하나의 방법을 고려하세요:
- httpOnly 쿠키 사용 (권장): 백엔드에서 httpOnly 쿠키로 토큰을 설정하여 JavaScript에서 접근 불가능하게 만듭니다.
- 메모리에만 저장: 새로고침 시 재로그인이 필요하지만 가장 안전합니다.
- sessionStorage 사용: localStorage보다는 약간 더 안전하지만 여전히 XSS에 취약합니다.
추가로, email과 userId도 민감한 정보이므로 저장 방식을 재검토해야 합니다.
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/GoogleCallback.tsx around lines 21-46, the
code stores accessToken (and also email/userId) in localStorage which is
vulnerable to XSS; remove localStorage.setItem('token', accessToken') and stop
persisting the token in JS-accessible storage, instead have the backend set an
httpOnly secure cookie with the token on the /api/v2/auth/google response (or
keep the token only in memory/session if you accept re-login on refresh), and
change the client flow to rely on that cookie for auth checks (adjust navigation
and any places that read localStorage token); also reassess whether email and
userId must be persisted — if not, avoid storing them or store minimal
non-sensitive data in a safer place (or use sessionStorage temporarily) and
update related code paths accordingly.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
164-182: postSignData 호출 전 email 값을 검증하세요.
userEmail이 URL 파라미터에서 초기화되지만, 파라미터가 없거나 비어있을 경우 빈 문자열로 서버에 전송될 수 있습니다.onError핸들러에서localStorage.getItem('email')을 사용하는 것으로 보아, localStorage도 대체 소스로 고려할 수 있습니다.다음과 같이 검증을 추가하세요:
if ((isMac && step === 6) || (!isMac && step === 5)) { + const emailToUse = userEmail || localStorage.getItem('email') || ''; + if (!emailToUse) { + alert('이메일 정보가 없습니다. 다시 로그인해주세요.'); + navigate('/onboarding?step=3'); + return; + } postSignData( { - email: userEmail, + email: emailToUse, remindDefault: remindTime, fcmToken: fcmToken, },
♻️ Duplicate comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
147-154: '사용자 설정' 알람 시간이 비어있을 때 방어 로직을 추가하세요.
AlarmsType[2].time의 초기값은 빈 문자열('')입니다. 사용자가 시간을 설정하지 않고 다음 단계로 진행하면normalizeTime('')이 호출되어'NaN:NaN'같은 잘못된 값이 생성됩니다. 이는 서버로 전송되는remindDefault값을 오염시킵니다.빈 문자열 체크를 추가하세요:
if (step === 4) { if (alarmSelected === 1) setRemindTime('09:00'); else if (alarmSelected === 2) setRemindTime('20:00'); else { const raw = AlarmsType[alarmSelected - 1].time; - setRemindTime(normalizeTime(raw)); + if (!raw) { + // 기본값 유지 또는 안전한 기본값 지정 + setRemindTime('09:00'); + } else { + setRemindTime(normalizeTime(raw)); + } } }
🧹 Nitpick comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
9-9: SocialLoginStep을 lazy import로 변경하는 것을 고려하세요.다른 Step 컴포넌트들(StoryStep, AlarmStep, MacStep, FinalStep)은 모두
lazy()를 사용하여 코드 스플리팅을 적용하고 있습니다. SocialLoginStep도 동일하게 처리하면 초기 번들 크기를 줄일 수 있습니다.다음과 같이 변경할 수 있습니다:
-import SocialLoginStep from './step/SocialLoginStep'; +const SocialLoginStep = lazy(() => import('./step/SocialLoginStep'));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/client/public/assets/onBoarding/icons/googleLogo.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-15T20:00:13.756Z
Learnt from: constantly-dev
Repo: Pinback-Team/pinback-client PR: 80
File: apps/client/src/shared/components/ui/modalPop/ModalPop.tsx:36-41
Timestamp: 2025-07-15T20:00:13.756Z
Learning: In apps/client/src/shared/components/ui/modalPop/ModalPop.tsx, the InfoBox component uses hardcoded values for title, location, and icon URL as temporary test data. These should be replaced with dynamic data from props when implementing actual functionality and should be marked with TODO comments for future changes.
Applied to files:
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx
🧬 Code graph analysis (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (2)
apps/client/src/constants/alarms.ts (1)
AlarmsType(11-15)apps/client/src/pages/onBoarding/utils/formatRemindTime.ts (1)
normalizeTime(1-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (4)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (4)
156-163: 이전 리뷰의 critical issue가 수정되었습니다!단계 진행 조건이
< 6/< 5로 올바르게 수정되어, Mac/비Mac 사용자 모두 온보딩 마지막 단계에 도달하고postSignData가 정상적으로 호출됩니다.
- Mac 사용자: 0→1→2→3(Social)→4(Alarm)→5(Mac)→6(Final) → postSignData 호출 ✓
- 비Mac 사용자: 0→1→2→3(Social)→4(Alarm)→5(Final) → postSignData 호출 ✓
185-192: URL 기반 네비게이션 구현이 적절합니다.
navigate()를 사용하여 step을 URL 쿼리 파라미터로 동기화한 것은 좋은 접근입니다. 브라우저의 뒤로가기/앞으로가기 버튼과도 호환되어 사용자 경험이 개선됩니다.
195-195: 오버플로우 조건이 올바르게 업데이트되었습니다.AlarmStep이 index 3에서 4로 이동함에 따라 오버플로우 조건도
step === 4로 정확하게 수정되었습니다.
232-241: SocialLoginStep에서 Next 버튼을 숨긴 것이 적절합니다.SocialLoginStep(step 3)에서는 사용자가 Google OAuth로 리다이렉트되므로 Next 버튼이 불필요합니다. Back 버튼은 정상적으로 표시되어 사용자가 이전 단계로 돌아갈 수 있습니다.
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx
Outdated
Show resolved
Hide resolved
into feat/#200/goolgle-login
…Team/pinback-client into feat/#200/goolgle-login
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/client/src/pages/onBoarding/GoogleCallback.tsx (2)
10-20: useEffect 의존성 배열이 여전히 불완전합니다.이전 리뷰에서 지적된 문제가 여전히 남아있습니다.
useEffect내부에서navigate,searchParams,loginWithCode를 사용하고 있지만 의존성 배열이 비어있습니다. React 19에서는 이를 경고하며, 예상치 못한 동작이 발생할 수 있습니다.다음과 같이 수정하세요:
useEffect(() => { const code = searchParams.get('code'); if (!code) { alert('로그인 실패. 다시 시도해주세요.'); navigate('/onboarding?step=3'); return; } loginWithCode(code); - }, []); + }, [navigate, searchParams, loginWithCode]);단,
loginWithCode를 의존성에 추가하면 매 렌더링마다 재생성되므로 무한 루프가 발생합니다. 가장 권장하는 해결책은loginWithCode로직을useEffect내부로 이동시키는 것입니다:useEffect(() => { const code = searchParams.get('code'); if (!code) { alert('로그인 실패. 다시 시도해주세요.'); navigate('/onboarding?step=3'); return; } + const loginWithCode = async (code: string) => { + try { + const res = await apiRequest.post('/api/v2/auth/google', { code }); + const { isUser, userId, email, accessToken } = res.data.data; + + localStorage.setItem('email', email); + localStorage.setItem('userId', userId); + + handleUserLogin(isUser, accessToken); + } catch (error) { + console.error('로그인 오류:', error); + alert('로그인 중 오류가 발생했습니다.'); + navigate('/onboarding?step=3'); + } + }; + loginWithCode(code); - }, []); + }, [navigate, searchParams]);또한
handleUserLogin도 의존성으로 추가하거나, 함께 useEffect 내부로 이동시켜야 합니다.
36-50: 여러 문제들이 있습니다. 우선순위별로 정리합니다:1. [Critical - 이전 리뷰에서 지적됨] localStorage 보안 취약점
accessToken,userId를 localStorage에 저장하는 것은 XSS 공격에 취약합니다. 공격자가 악성 스크립트를 주입하면 토큰과 사용자 정보를 탈취할 수 있습니다.다음 중 하나의 방법을 고려하세요:
- httpOnly 쿠키 사용 (권장): 백엔드에서 httpOnly, Secure, SameSite 속성을 가진 쿠키로 토큰을 설정하여 JavaScript에서 접근 불가능하게 만듭니다.
- 메모리에만 저장: 새로고침 시 재로그인이 필요하지만 가장 안전합니다.
- sessionStorage 사용: localStorage보다는 약간 더 안전하지만 여전히 XSS에 취약합니다.
2. [Minor] 프로덕션 환경에서 console.error 제거
Line 46의
console.error는 프로덕션에서 민감한 정보를 노출할 수 있습니다. 적절한 로깅 서비스(예: Sentry)를 사용하거나 개발 환경에서만 로그를 출력하도록 수정하세요.3. [Recommended] API 응답 타입 정의
Line 39에서
res.data.data의 타입이 명시되지 않아 타입 안정성이 떨어집니다. API 응답 인터페이스를 정의하면 좋습니다.interface GoogleAuthResponse { isUser: boolean; userId: string; email: string; accessToken?: string; } const loginWithCode = async (code: string) => { try { const res = await apiRequest.post<{ data: GoogleAuthResponse }>( '/api/v2/auth/google', { code } ); const { isUser, userId, email, accessToken } = res.data.data; // ... } };
🧹 Nitpick comments (2)
apps/client/src/pages/onBoarding/GoogleCallback.tsx (2)
22-34: 함수 재생성을 방지하기 위해 useCallback을 사용하세요.
handleUserLogin함수가 컴포넌트가 렌더링될 때마다 재생성됩니다.navigate를 사용하므로useCallback으로 감싸서 메모이제이션하는 것이 좋습니다.+ import { useEffect, useCallback } from 'react'; const GoogleCallback = () => { const navigate = useNavigate(); const [searchParams] = useSearchParams(); - const handleUserLogin = ( + const handleUserLogin = useCallback(( isUser: boolean, accessToken: string | undefined - ) => { + ) => { if (isUser) { if (accessToken) { localStorage.setItem('token', accessToken); } navigate('/'); } else { navigate('/onboarding?step=4'); } - }; + }, [navigate]);
14-14: 사용자 경험 개선을 위해 alert 대신 Toast 알림을 사용하세요.
alert()는 브라우저 UI를 블로킹하고 스타일링이 불가능하여 현대적인 웹 애플리케이션에서는 권장되지 않습니다. Toast 라이브러리(예: react-hot-toast, sonner)나 프로젝트의 디자인 시스템에서 제공하는 알림 컴포넌트를 사용하는 것이 좋습니다.예시 (react-hot-toast 사용):
import toast from 'react-hot-toast'; // Line 14 if (!code) { toast.error('로그인 실패. 다시 시도해주세요.'); navigate('/onboarding?step=3'); return; } // Line 47 catch (error) { console.error('로그인 오류:', error); toast.error('로그인 중 오류가 발생했습니다.'); navigate('/onboarding?step=3'); }Also applies to: 47-47
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/src/pages/onBoarding/GoogleCallback.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/onBoarding/GoogleCallback.tsx (1)
apps/client/src/shared/components/loadingChippi/LoadingChippi.tsx (1)
LoadingChippi(3-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (2)
apps/client/src/pages/onBoarding/GoogleCallback.tsx (2)
1-4: LGTM! 필요한 의존성들이 적절하게 임포트되어 있습니다.
52-62: LGTM! 로딩 UI가 명확하고 사용자 친화적입니다.LoadingChippi 컴포넌트를 활용한 로딩 상태 표시가 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
169-176: '사용자 설정' 알람 시간이 빈 값일 때NaN:NaN이 발생합니다.
AlarmsType[2].time의 초기값이''입니다. 사용자가 시간을 설정하지 않고 진행하면normalizeTime('')호출 시'NaN:NaN'이 반환되어 API에 잘못된 값이 전송됩니다.이전 리뷰에서 지적된 문제이므로 방어 로직이 필요합니다.
else { const raw = AlarmsType[alarmSelected - 1].time; - setRemindTime(normalizeTime(raw)); + if (raw && raw.trim() !== '') { + setRemindTime(normalizeTime(raw)); + } + // raw가 빈 값이면 기존 remindTime('09:00') 유지 }
🧹 Nitpick comments (4)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (4)
4-8:SocialLoginStep만 직접 import되어 있어 일관성이 없습니다.다른 Step 컴포넌트들(
StoryStep,AlarmStep,MacStep,FinalStep)은 모두lazy()로 동적 로딩되는데,SocialLoginStep만 직접 import되어 있습니다. 번들 사이즈 최적화를 위해 일관되게 lazy loading을 적용하는 것을 권장합니다.-import SocialLoginStep from './step/SocialLoginStep'; const StoryStep = lazy(() => import('./step/StoryStep')); const AlarmStep = lazy(() => import('./step/AlarmStep')); const MacStep = lazy(() => import('./step/MacStep')); const FinalStep = lazy(() => import('./step/FinalStep')); +const SocialLoginStep = lazy(() => import('./step/SocialLoginStep'));
95-96: Firebase 초기화가 렌더링마다 실행됩니다.
initializeApp과getMessaging이 컴포넌트 함수 본문에서 호출되어 매 렌더링마다 실행됩니다. Firebase SDK가 중복 초기화를 내부적으로 처리하지만, 모듈 스코프로 이동하거나useMemo를 사용하는 것이 효율적입니다.+// 컴포넌트 외부 (모듈 스코프) +const app = initializeApp(firebaseConfig); +const messaging = getMessaging(app); const MainCard = () => { // ... - const app = initializeApp(firebaseConfig); - const messaging = getMessaging(app);
125-140: FCM 토큰 요청이 컴포넌트 마운트 시 즉시 실행됩니다.알림 권한 요청이 사용자 인터랙션 없이 컴포넌트 마운트 시 즉시 실행됩니다. 이는 다음과 같은 문제가 있을 수 있습니다:
- 브라우저가 사용자 제스처 없는 권한 요청을 차단할 수 있음
- 사용자가 왜 알림 권한이 필요한지 이해하기 전에 프롬프트가 표시됨
알림 설정과 관련된
ALARM단계에서 요청하는 것이 UX 측면에서 더 적절할 수 있습니다.
144-149: StoryStep 인덱스 추출 방식을 개선할 수 있습니다.
step.replace('STORY_', '')로 문자열을 파싱하는 것보다 이미 정의된storySteps배열을 활용하면 더 안전합니다.case Step.STORY_0: case Step.STORY_1: case Step.STORY_2: return ( - <StoryStep step={Number(step.replace('STORY_', '')) as 0 | 1 | 2} /> + <StoryStep step={storySteps.indexOf(step) as 0 | 1 | 2} /> );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (6)
apps/client/src/shared/apis/queries.ts (1)
usePostSignUp(65-90)apps/client/public/firebase-messaging-sw.js (2)
firebaseConfig(4-12)messaging(35-35)apps/client/src/firebase-config.ts (1)
firebaseConfig(1-9)apps/client/src/pages/onBoarding/utils/registerServiceWorker.ts (1)
registerServiceWorker(1-11)apps/client/src/constants/alarms.ts (1)
AlarmsType(11-15)apps/client/src/pages/onBoarding/utils/formatRemindTime.ts (1)
normalizeTime(1-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (3)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (3)
89-92: Step 파라미터 검증이 개선되었습니다.
Object.values(Step).includes(stepParam)로 유효한 Step 값인지 검증하는 로직이 추가되어 이전 리뷰에서 지적된 문제가 해결되었습니다.
223-229: Progress 표시 로직이 적절하게 구현되었습니다.
storySteps.includes(step)로 스토리 단계에서만 Progress를 표시하고,storySteps.indexOf(step)로 진행률을 계산하는 방식이 명확합니다.
249-269: 버튼 표시 조건이 적절합니다.
STORY_0과SOCIAL_LOGIN에서 뒤로가기 버튼 숨김SOCIAL_LOGIN에서 다음 버튼 숨김 (Google 로그인 버튼으로 진행)사용자 플로우에 맞게 구현되었습니다.
| // 마지막 스텝(Final)인 경우 → API 호출 | ||
| if (step === Step.FINAL) { | ||
| postSignData( | ||
| { email: userEmail, remindDefault: remindTime, fcmToken }, | ||
| { | ||
| email: userEmail, | ||
| remindDefault: remindTime, | ||
| fcmToken: fcmToken, | ||
| }, | ||
| { | ||
| onSuccess: () => { | ||
| window.location.href = '/'; | ||
| }, | ||
| onSuccess: () => (window.location.href = '/'), | ||
| onError: () => { | ||
| const savedEmail = localStorage.getItem('email'); | ||
| if (savedEmail) { | ||
| window.location.href = '/'; | ||
| } | ||
| if (savedEmail) window.location.href = '/'; | ||
| }, | ||
| } | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the MainCard.tsx file
fd -t f "MainCard.tsx" --exec wc -l {} \;Repository: Pinback-Team/pinback-client
Length of output: 140
🏁 Script executed:
#!/bin/bash
# Read the full MainCard.tsx file with line numbers
cat -n ./apps/client/src/pages/onBoarding/components/funnel/MainCard.tsxRepository: Pinback-Team/pinback-client
Length of output: 9775
🏁 Script executed:
#!/bin/bash
# Find the usePostSignUp hook implementation
fd -t f -name "*.ts" -o -name "*.tsx" | xargs rg -l "usePostSignUp" | head -5Repository: Pinback-Team/pinback-client
Length of output: 421
🏁 Script executed:
#!/bin/bash
# Find and read the queries.ts file
fd -t f "queries.ts" -path "*/shared/apis/*" --exec wc -l {} \;Repository: Pinback-Team/pinback-client
Length of output: 239
🏁 Script executed:
#!/bin/bash
# Read the queries.ts file
cat -n ./apps/client/src/shared/apis/queries.tsRepository: Pinback-Team/pinback-client
Length of output: 4209
🏁 Script executed:
#!/bin/bash
# Find axios.ts file in shared/apis
fd -type f "axios.ts" -path "*/shared/apis/*"Repository: Pinback-Team/pinback-client
Length of output: 241
🏁 Script executed:
#!/bin/bash
# Find axios.ts file
find ./apps/client/src/shared/apis -name "axios.ts" -o -name "axios.tsx"Repository: Pinback-Team/pinback-client
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Read the axios.ts file
wc -l ./apps/client/src/shared/apis/axios.tsRepository: Pinback-Team/pinback-client
Length of output: 112
🏁 Script executed:
#!/bin/bash
# Read the entire axios.ts file
cat -n ./apps/client/src/shared/apis/axios.tsRepository: Pinback-Team/pinback-client
Length of output: 2634
🏁 Script executed:
#!/bin/bash
# Let's also check if there are other validation patterns in the codebase
# Search for validation patterns related to empty strings or null checks before API calls
rg -A 5 -B 2 "if.*!.*email.*\|.*if.*!.*userEmail" --type=tsx --type=ts | head -40Repository: Pinback-Team/pinback-client
Length of output: 98
Based on my verification of the codebase, I found important information about the API contract and the concerns raised in the review comment.
Key Findings:
-
fcmToken null handling - The
postSignUpRequestinterface at line 35 ofapps/client/src/shared/apis/axios.tsexplicitly definesfcmToken: string | null, which means the backend API is designed to accept null fcmToken. This is not an error condition. -
userEmail validation - The
userEmailcan indeed be an empty string (initialized at line 68 of MainCard.tsx, set from URL params at lines 81-93 without validation). This IS a valid concern that needs addressing. -
Error handling - The
onErrorcallback (lines 192-195) silently redirects to home page ifsavedEmailexists, which could hide actual errors from the user. This concern is valid but not critical since the API likely validates the email server-side.
✓ Validate userEmail before API call, but fcmToken: null is acceptable per API design
The backend API explicitly accepts fcmToken as string | null (see line 35 of apps/client/src/shared/apis/axios.ts), so passing null is not an error. However, userEmail should be validated since it can be an empty string if not provided via URL parameters.
if (step === Step.FINAL) {
+ if (!userEmail) {
+ alert('이메일 정보가 없습니다. 다시 로그인해주세요.');
+ navigate('/onboarding');
+ return;
+ }
postSignData(
{ email: userEmail, remindDefault: remindTime, fcmToken },Additionally, consider improving error feedback in the onError callback instead of silently redirecting:
onError: () => {
- const savedEmail = localStorage.getItem('email');
- if (savedEmail) window.location.href = '/';
+ alert('회원가입 처리 중 오류가 발생했습니다. 다시 시도해주세요.');📝 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.
| // 마지막 스텝(Final)인 경우 → API 호출 | |
| if (step === Step.FINAL) { | |
| postSignData( | |
| { email: userEmail, remindDefault: remindTime, fcmToken }, | |
| { | |
| email: userEmail, | |
| remindDefault: remindTime, | |
| fcmToken: fcmToken, | |
| }, | |
| { | |
| onSuccess: () => { | |
| window.location.href = '/'; | |
| }, | |
| onSuccess: () => (window.location.href = '/'), | |
| onError: () => { | |
| const savedEmail = localStorage.getItem('email'); | |
| if (savedEmail) { | |
| window.location.href = '/'; | |
| } | |
| if (savedEmail) window.location.href = '/'; | |
| }, | |
| } | |
| ); | |
| return; | |
| } | |
| // 마지막 스텝(Final)인 경우 → API 호출 | |
| if (step === Step.FINAL) { | |
| if (!userEmail) { | |
| alert('이메일 정보가 없습니다. 다시 로그인해주세요.'); | |
| navigate('/onboarding'); | |
| return; | |
| } | |
| postSignData( | |
| { email: userEmail, remindDefault: remindTime, fcmToken }, | |
| { | |
| onSuccess: () => (window.location.href = '/'), | |
| onError: () => { | |
| alert('회원가입 처리 중 오류가 발생했습니다. 다시 시도해주세요.'); | |
| }, | |
| } | |
| ); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
186-199, the code calls postSignData when step === Step.FINAL but does not
validate userEmail (which can be an empty string) and silently redirects on
error; fcmToken:null is acceptable per API. Fix by validating that userEmail is
a non-empty, well-formed email before calling postSignData and abort/notify the
user if invalid; if calling postSignData, replace the silent onError redirect
with visible error handling (display an error toast/modal or log the error with
details) and only redirect after user acknowledgement or successful fallback
logic.
| const prevStep = () => { | ||
| if (step > 0) { | ||
| const idx = stepOrder.indexOf(step); | ||
| if (idx > 0) { | ||
| const previous = stepOrder[idx - 1]; | ||
| setDirection(-1); | ||
| setStep((prev) => prev - 1); | ||
| setStep(previous); | ||
| navigate(`/onboarding?step=${previous}`); | ||
| } | ||
| }; |
There was a problem hiding this comment.
비Mac 사용자가 뒤로가기 시 MAC 단계에 도달할 수 있습니다.
nextStep에서는 비Mac 사용자일 때 MAC 단계를 건너뛰지만, prevStep에서는 이 로직이 없습니다. FINAL 단계에서 뒤로가기 하면 비Mac 사용자도 MAC 단계가 표시됩니다.
const prevStep = () => {
const idx = stepOrder.indexOf(step);
if (idx > 0) {
- const previous = stepOrder[idx - 1];
+ let previous = stepOrder[idx - 1];
+ // 비Mac 사용자가 MAC 단계를 건너뛰도록 처리
+ if (previous === Step.MAC && !isMac) {
+ previous = Step.ALARM;
+ }
setDirection(-1);
setStep(previous);
navigate(`/onboarding?step=${previous}`);
}
};🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
207 to 215, prevStep currently just moves one index back in stepOrder which
causes non-Mac users to land on the MAC step when going back; update prevStep to
mirror nextStep's logic by skipping the MAC step for non-Mac users — determine
the candidate previous step, if it is the MAC step and the user is not on a Mac
(using the same isMac check or prop used in nextStep) then step back one more
index (or find the previous valid step) before calling setDirection(-1),
setStep(...) and navigate(`/onboarding?step=...`) so non-Mac users never
transition into the MAC step on back navigation.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (3)
189-201: FINAL 단계에서 userEmail 유효성 검증 없이 회원가입 API를 호출합니다
userEmail은 query string에서 가져와 세팅하고 있는데, 어떤 경로에서는 비어 있거나 잘못된 값일 수 있습니다. 현재는 값 검증 없이 그대로postSignData에 전달하고 있어, 백엔드에서 에러가 나도 사용자는 원인을 파악하기 어렵습니다. FINAL 단계 진입 시점 혹은isFinalStep분기 안에서 최소한 “비어 있지 않은 이메일인지” 정도는 체크하는 것이 안전해 보입니다. (이전 리뷰에서 제안된 내용과 동일한 방향입니다.)- if (isFinalStep) { - postSignData( - { email: userEmail, remindDefault: remindTime, fcmToken }, + if (isFinalStep) { + if (!userEmail) { + alert('이메일 정보가 없습니다. 다시 로그인해주세요.'); + navigate('/onboarding'); + return; + } + + postSignData( + { email: userEmail, remindDefault: remindTime, fcmToken }, { onSuccess: () => (window.location.href = '/'), onError: () => { const savedEmail = localStorage.getItem('email'); if (savedEmail) window.location.href = '/'; }, } ); return; }추가로,
onError에서도 단순 리다이렉트 대신 에러 안내 토스트/모달 등을 노출해 주면 디버깅과 UX 측면에서 도움이 될 것 같습니다.
173-180: ‘사용자 설정’ 알람에서 빈 time 문자열로 인해 잘못된 remindTime이 저장될 수 있습니다
AlarmsType[2].time이 빈 문자열인 상태에서(사용자 설정 선택 시) 그대로normalizeTime(raw)에 넘기면"NaN:NaN"같은 값이 되어 서버에 잘못된remindDefault가 전송될 수 있습니다. 최소한 빈 문자열일 때는 이전 값 유지나 안전한 기본값으로 fallback 하는 방어 로직이 필요해 보입니다.- if (isAlarmStep) { - if (alarmSelected === 1) setRemindTime('09:00'); - else if (alarmSelected === 2) setRemindTime('20:00'); - else { - const raw = AlarmsType[alarmSelected - 1].time; - setRemindTime(normalizeTime(raw)); - } - } + if (isAlarmStep) { + if (alarmSelected === 1) { + setRemindTime('09:00'); + } else if (alarmSelected === 2) { + setRemindTime('20:00'); + } else { + const raw = AlarmsType[alarmSelected - 1].time; + if (!raw) { + // 사용자 설정 시간이 비어 있으면 이전 값 유지 또는 안전한 기본값으로 대체 + setRemindTime(remindTime); + } else { + setRemindTime(normalizeTime(raw)); + } + } + }
209-214: 비Mac 사용자가 뒤로가기 시 MAC 단계로 진입하는 문제
nextStep에서는 비Mac에서 MAC 단계를 건너뛰지만,prevStep에서는 그대로stepOrder[idx - 1]를 사용해서 비Mac 사용자가 FINAL → 뒤로 가기 시 MAC 스텝으로 진입할 수 있습니다. 이전 리뷰에서 언급된 것과 동일한 문제로, 뒤로가기에서도 MAC 스텝을 스킵하는 로직이 필요해 보입니다.- const prevStep = () => { - const idx = stepOrder.indexOf(step); - if (idx > 0) { - const previous = stepOrder[idx - 1]; - setDirection(-1); - setStep(previous); - navigate(`/onboarding?step=${previous}`); - } - }; + const prevStep = () => { + const idx = stepOrder.indexOf(step); + if (idx > 0) { + let previous = stepOrder[idx - 1]; + // 비Mac 사용자는 MAC 스텝을 건너뛰도록 처리 + if (previous === Step.MAC && !isMac) { + previous = Step.ALARM; + } + setDirection(-1); + setStep(previous); + navigate(`/onboarding?step=${previous}`); + } + };
🧹 Nitpick comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
98-140: FCM 토큰 요청 실패 시 alert가 중복으로 노출될 수 있습니다
requestFCMToken내부에서 권한 거부/토큰 생성 실패/예외 케이스마다alert를 이미 띄우고 있는데, 바깥useEffect의 IIFE에서token이 falsy일 때 다시 한 번alert('푸시 알람 설정 에러')를 띄우고 있어 동일 상황에서 팝업이 두 번 보일 수 있습니다. UX 상 불필요한 중복이라 보입니다.(async () => { const token = await requestFCMToken(); - if (token) { - setFcmToken(token); - localStorage.setItem('FcmToken', token); - } else { - alert('푸시 알람 설정 에러'); - } + if (token) { + setFcmToken(token); + localStorage.setItem('FcmToken', token); + } })();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (5)
apps/client/src/shared/apis/queries.ts (1)
usePostSignUp(65-90)apps/client/src/firebase-config.ts (1)
firebaseConfig(1-9)apps/client/src/pages/onBoarding/utils/registerServiceWorker.ts (1)
registerServiceWorker(1-11)apps/client/src/constants/alarms.ts (1)
AlarmsType(11-15)apps/client/src/pages/onBoarding/utils/formatRemindTime.ts (1)
normalizeTime(1-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
20-32: 문자열 기반 Step 모델과 stepOrder/버튼 분기 구조가 이전 숫자 기반 로직보다 명확합니다
Step을 문자열 상수로 정의하고StepType,storySteps,stepOrder로 흐름을 구성한 덕분에, STORY/SOCIAL_LOGIN/ALARM/MAC/FINAL 단계가 각각 어떤 화면인지 훨씬 읽기 쉬워졌습니다.
renderStep에서의 switch,- story 스텝에만 Progress를 노출하는 조건,
- SOCIAL_LOGIN에서 뒤로/다음 버튼을 숨기는 분기 등이 전반적으로 자연스럽고 의도도 잘 드러납니다.
forward 진행(특히 MAC 스텝 스킵) 로직도
stepOrder+shouldSkipMacStep로 깔끔하게 표현된 것 같아요.Also applies to: 142-163, 224-227, 250-271
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (3)
156-163: 알람 사용자 설정 시간(AlarmsType[..].time)이 비어 있거나 이상한 형식일 때 방어 로직이 없습니다
alarmSelected === 3인 경우:const raw = AlarmsType[alarmSelected - 1].time; setRemindTime(normalizeTime(raw));
raw가''이거나 정규식에 매칭되지 않는 문자열이면normalizeTime결과가NaN:NaN같은 잘못된 값이 될 수 있고,- 그대로
remindDefault로 서버에 전송될 수 있습니다.이전에 지적된 부분이 아직 동일하게 남아 있는 것 같아서(조건만
step === Step.ALARM으로 바뀜) 한 번 더 남깁니다.- if (isAlarmStep) { - if (alarmSelected === 1) setRemindTime('09:00'); - else if (alarmSelected === 2) setRemindTime('20:00'); - else { - const raw = AlarmsType[alarmSelected - 1].time; - setRemindTime(normalizeTime(raw)); - } - } + if (isAlarmStep) { + if (alarmSelected === 1) { + setRemindTime('09:00'); + } else if (alarmSelected === 2) { + setRemindTime('20:00'); + } else { + const raw = AlarmsType[alarmSelected - 1].time; + if (!raw) { + // 이전 값 유지 또는 안전한 기본값으로 fallback + setRemindTime(remindTime); + } else { + setRemindTime(normalizeTime(raw)); + } + } + }이 정도 방어만 있어도 잘못된 시간 문자열이 저장·전송되는 리스크를 많이 줄일 수 있을 것 같습니다.
172-181: FINAL 단계에서userEmail검증 없이 가입 API를 호출합니다if (isFinalStep) { postSignData( { email: userEmail, remindDefault: remindTime, fcmToken }, { ... } ); }현재
userEmail은 쿼리 파라미터가 없으면 기본값''인 상태로 남을 수 있고, 이 경우에도 그대로 서버로 전송됩니다. 이전 리뷰에서 제안되었던 이메일 유효성 체크가 아직 반영되지 않은 상태로 보입니다.최소한 비어 있는 이메일에 대해서는 방어 로직이 필요해 보입니다:
- if (isFinalStep) { - postSignData( - { email: userEmail, remindDefault: remindTime, fcmToken }, + if (isFinalStep) { + if (!userEmail) { + alert('이메일 정보가 없습니다. 다시 로그인해주세요.'); + navigate('/onboarding'); + return; + } + + postSignData( + { email: userEmail, remindDefault: remindTime, fcmToken }, { - onSuccess: () => (window.location.href = '/'), - onError: () => { - const savedEmail = localStorage.getItem('email'); - if (savedEmail) window.location.href = '/'; - }, + onSuccess: () => (window.location.href = '/'), + onError: () => { + alert('회원가입 처리 중 오류가 발생했습니다. 다시 시도해주세요.'); + }, } ); return; }실제 백엔드에서 허용하는 이메일 형식에 맞춰 더 엄격한 검증(정규식 등)을 추가하는 것도 고려해 주세요.
191-199: 비Mac 사용자가 뒤로가기 시 MAC 단계에 진입할 수 있는 문제
nextStep에서는!isMac일 때 MAC 단계를 건너뛰도록 잘 처리하셨는데,prevStep에는 동일한 로직이 없어 비Mac 사용자가 FINAL 단계에서 뒤로가기를 누르면 MAC 단계 화면을 보게 됩니다.const prevStep = () => { const idx = stepOrder.indexOf(step); if (idx > 0) { const previous = stepOrder[idx - 1]; setDirection(-1); setStep(previous); navigate(`/onboarding?step=${previous}`); } };이전에 지적되었던 내용이 string step 기반으로 리팩터링된 뒤에도 그대로 남아 있는 상황이라, 아래처럼 한 단계 더 건너뛰도록 보완하는 게 좋겠습니다:
const prevStep = () => { const idx = stepOrder.indexOf(step); if (idx > 0) { - const previous = stepOrder[idx - 1]; + let previous = stepOrder[idx - 1]; + if (previous === Step.MAC && !isMac) { + // 비Mac 사용자는 MAC 단계를 건너뛴다 + previous = Step.ALARM; + } setDirection(-1); setStep(previous); navigate(`/onboarding?step=${previous}`); } };이렇게 하면 비Mac 유저는 앞으로/뒤로 어디서도 MAC 단계에 진입하지 않게 됩니다.
🧹 Nitpick comments (3)
apps/client/src/pages/onBoarding/constants/onboardingSteps.ts (1)
1-27: 문자열 기반 Step 정의와 타입 설계 좋습니다숫자 인덱스 대신
Step/StepType을 도입해서 온보딩 흐름을 따라가기 훨씬 수월해졌습니다.storySteps/stepOrder로 초기 스토리 단계와 전체 플로우를 분리해 둔 것도 구조적으로 깔끔해 보여요.추후 스토리 단계가 늘어나면
storySteps와stepProgress를 같이 업데이트해야 하니, 아주 나중에는storySteps.length를 기반으로 진행률을 계산하는 방향도 고려해볼 수 있을 것 같습니다만, 지금 구현으로도 충분히 명확합니다.apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (2)
19-24: StepType 기반 플로우로 리팩터링된 부분 전체적으로 잘 정리되었습니다
- URL 쿼리의
step을Step값으로 검증해서만 세팅하고,stepOrder/storySteps를 기준으로 렌더링·네비게이션·Progress까지 일관되게 묶은 구조가 이전 숫자 기반 구현보다 훨씬 읽기 좋습니다.storySteps.includes(step)로 Progress 표시 여부를 제어하는 패턴도 명확하네요.추가로, 나중에 스토리 단계가 늘어날 경우를 생각하면
stepProgress를 하드코딩하기보다는storySteps.indexOf(step)기반으로(index + 1) / storySteps.length * 100같은 계산식으로 만들어 두면 유지보수가 조금 더 수월해질 것 같습니다.Also applies to: 56-75, 201-213, 233-253
125-145: SocialLoginStep에서 기본 ‘뒤로/다음’ 버튼을 숨기는 UX 확인 필요
renderStep에서Step.SOCIAL_LOGIN일 때는SocialLoginStep만 렌더링하고, 하단 버튼 영역에서는:
뒤로버튼:Step.STORY_0,Step.SOCIAL_LOGIN에서 숨김다음버튼:Step.SOCIAL_LOGIN에서 숨김으로 처리하고 있어서, SocialLoginStep 내부 UI 외에는 앞으로/뒤로 이동 수단이 없는 상태입니다.
SocialLoginStep 안에서:
- 사용자가 구글 로그인 플로우를 취소하거나,
- 오류가 났을 때 온보딩으로 되돌아갈 수 있는 명시적인 액션(예: “뒤로”, “건너뛰기”)이 이미 있다면 괜찮겠지만,
그렇지 않다면 최소한 뒤로가기 액션 하나는 제공해 주는 게 UX 측면에서 안전해 보입니다. SocialLoginStep 내부에 이런 컨트롤이 있는지 한 번만 더 확인해 주세요.
Also applies to: 233-253
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx(6 hunks)apps/client/src/pages/onBoarding/constants/onboardingSteps.ts(1 hunks)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (3)
155-162: 사용자 지정 알람 시간(AlarmsType[...]?.time)이 비어 있거나 잘못된 경우를 방어하지 않으면 잘못된remindTime이 전송될 수 있습니다if (isAlarmStep) { if (alarmSelected === 1) setRemindTime('09:00'); else if (alarmSelected === 2) setRemindTime('20:00'); else { const raw = AlarmsType[alarmSelected - 1].time; setRemindTime(normalizeTime(raw)); } }지금은
raw가 빈 문자열이거나, 예상하지 않은 포맷이어도 그대로normalizeTime에 넘기고 있습니다. 이 경우:
normalizeTime내부에서 파싱 에러/NaN → 이상한 문자열이remindDefault로 서버에 전달될 수 있고,- 사용자가 시간을 제대로 입력하지 않은 상태로도 온보딩이 진행됩니다.
최소한
raw가 falsy이거나 정규식에 매칭되지 않는 경우에는 기존 값을 유지하거나 안전한 기본값으로 fallback 하는 방어 로직이 있는 편이 좋겠습니다. 예:else { const raw = AlarmsType[alarmSelected - 1].time; if (!raw) { // 이전 remindTime 유지 또는 기본값으로 설정 // setRemindTime(remindTime); setRemindTime('09:00'); } else { setRemindTime(normalizeTime(raw)); } }
171-182: FINAL 단계에서userEmail검증 없이 가입 API를 호출하고, 에러 시 조용히 리다이렉트되어 문제가 숨겨질 수 있습니다if (isFinalStep) { postSignData( { email: userEmail, remindDefault: remindTime, fcmToken }, { onSuccess: () => (window.location.href = '/'), onError: () => { const savedEmail = localStorage.getItem('email'); if (savedEmail) window.location.href = '/'; }, } ); return; }우려되는 부분은 두 가지입니다.
userEmail이 빈 문자열일 수 있음
- 이 컴포넌트 안에서
userEmail은 오직 localStorage(''그대로입니다.- 사용자가 직접 URL로
/onboarding?step=FINAL에 진입하거나, 이메일이 저장되지 않은 상태에서 진행되면 백엔드로email: ''를 보내게 됩니다.- 최소한
!userEmail인 경우에는 가입 요청을 막고, 다시 로그인/소셜 로그인으로 유도하는 방어가 필요해 보입니다.에러 시 사용자에게 아무 메시지 없이 홈으로 보내는 동작
onError에서savedEmail이 있으면 곧바로/로 보내는데, 이 경우 가입이 실패했는지, 이미 가입된 계정인지 등을 사용자가 전혀 알 수 없습니다.예상 수정 예시는 다음과 같습니다.
if (isFinalStep) { + if (!userEmail) { + alert('이메일 정보가 없습니다. 다시 로그인 후 온보딩을 진행해주세요.'); + navigate('/onboarding'); + return; + } + postSignData( { email: userEmail, remindDefault: remindTime, fcmToken }, { - onSuccess: () => (window.location.href = '/'), - onError: () => { - const savedEmail = localStorage.getItem('email'); - if (savedEmail) window.location.href = '/'; - }, + onSuccess: () => (window.location.href = '/'), + onError: () => { + alert('회원가입 처리 중 오류가 발생했습니다. 잠시 후 다시 시도해주세요.'); + }, } ); return; }이렇게 하면 잘못된 payload를 보내는 것을 막고, 실패 시에도 최소한의 피드백을 줄 수 있습니다.
190-197: 비Mac 사용자가 뒤로가기를 눌렀을 때 MAC 스텝으로 이동하는 문제가 여전히 남아 있습니다현재
prevStep은 단순히stepOrder에서 이전 인덱스로만 이동합니다.const prevStep = () => { const idx = stepOrder.indexOf(step); if (idx > 0) { const previous = stepOrder[idx - 1]; setDirection(-1); setStep(previous); navigate(`/onboarding?step=${previous}`); } };
nextStep에서는!isMac일 때 MAC 스텝을 건너뛰어 바로 FINAL로 가지만,prevStep에는 같은 예외 처리가 없어:
- 비Mac 사용자가 FINAL에서 뒤로가기를 누르면
- 한 단계 이전인 MAC 스텝으로 이동하게 됩니다.
의도대로라면 비Mac 사용자는 MAC 스텝을 전혀 보지 않는 것이 자연스러워 보입니다. 예전 리뷰에서 제안되었던 것처럼, 이전 스텝이 MAC이고
!isMac이면 한 번 더 건너뛰도록 보정하는 게 좋겠습니다.const prevStep = () => { const idx = stepOrder.indexOf(step); if (idx > 0) { - const previous = stepOrder[idx - 1]; + let previous = stepOrder[idx - 1]; + if (previous === Step.MAC && !isMac) { + previous = Step.ALARM; + } setDirection(-1); setStep(previous); navigate(`/onboarding?step=${previous}`); } };
🧹 Nitpick comments (3)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (3)
64-75: 알림 권한 거부/토큰 실패 시 유저에게 alert가 두 번 뜰 수 있습니다현재 흐름에서는:
requestFCMToken내부에서
- 권한 거부 →
'알림 권한 허용이 필요합니다!'- 토큰 생성 실패 →
'토큰 생성 실패. 다시 시도해주세요.'- 기타 예외 →
'알림 설정 중 오류가 발생했습니다. 다시 시도해주세요.'- 그리고
useEffect안에서token이 falsy일 때 다시
'푸시 알람 설정 에러'를 띄우고 있어서, 한 번의 실패에 대해 서로 다른 alert가 연달아 나올 수 있습니다.
UX 측면에서 한 번만 명확한 메시지를 보여주는 편이 좋습니다. 예를 들어:
requestFCMToken에서만 alert를 띄우고,useEffect의else { alert('푸시 알람 설정 에러'); }는 제거하거나 로그만 남기도록 바꾸는 식으로 정리해 보시면 좋겠습니다.Also applies to: 107-122
124-144: STORY 단계 인덱스를 문자열 파싱으로 계산하는 부분은 살짝 취약해서, 매핑 테이블로 분리하면 더 안전합니다case Step.STORY_0: case Step.STORY_1: case Step.STORY_2: return ( <StoryStep step={Number(step.replace('STORY_', '')) as 0 | 1 | 2} /> );처럼 문자열을 직접 파싱하면,
Stepenum 이름이 바뀌거나 STORY 계열이 늘어났을 때 쉽게 깨질 수 있습니다. 작은 매핑을 두고 쓰는 편이 조금 더 명시적입니다.const storyIndexByStep: Record<Step.STORY_0 | Step.STORY_1 | Step.STORY_2, 0 | 1 | 2> = { [Step.STORY_0]: 0, [Step.STORY_1]: 1, [Step.STORY_2]: 2, }; ... case Step.STORY_0: case Step.STORY_1: case Step.STORY_2: return <StoryStep step={storyIndexByStep[step]} />;필수는 아니지만, 나중에 스텝이 늘어날 때 안전망이 되어 줄 수 있습니다.
200-212: story 단계 개수와stepProgress배열 길이가 암묵적으로 묶여 있어, 나중에 단계 추가 시 같이 관리할 수 있는 장치가 있으면 좋겠습니다{storySteps.includes(step) && ( <Progress value={stepProgress[storySteps.indexOf(step)].progress} variant="profile" className="w-[15.6rem]" /> )}현재는:
storySteps에 STORY 단계들이 정의되어 있고,stepProgress는[33, 66, 100]세 값만 하드코딩되어 있습니다.지금 구조 자체는 문제 없지만, 나중에 STORY 단계가 추가되면
storySteps와stepProgress를 반드시 같이 수정해야 해서 실수 여지가 있습니다. 예를 들어:
storySteps에 바로{ step, progress }를 들고 있도록 하거나,stepProgress를Record<StepType, number>형태로 바꾸는 식으로단일 소스에 progress 값을 두면 유지보수가 조금 더 편할 것 같습니다. (반드시 지금 바꿀 필요는 없고, 스텝이 더 복잡해지면 고려해볼 만한 수준입니다.)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/client/src/pages/onBoarding/GoogleCallback.tsx(1 hunks)apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx(5 hunks)apps/client/src/shared/apis/axios.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/client/src/pages/onBoarding/GoogleCallback.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: storybook
🔇 Additional comments (1)
apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx (1)
231-252: 소셜 로그인 스텝에서 버튼 노출을 분리한 구조가 명확해서 좋습니다{!([Step.STORY_0, Step.SOCIAL_LOGIN] as StepType[]).includes(step) && ( <Button ... onClick={prevStep}>뒤로</Button> )} {step !== Step.SOCIAL_LOGIN && ( <Button ... onClick={nextStep}>다음</Button> )}
- 첫 스토리(
STORY_0)와 소셜 로그인에서는 뒤로 가기 버튼을 숨기고,- 소셜 로그인에서는
다음버튼도 숨긴 채 스텝 내부에서만 흐름을 제어하는 구조라온보딩 흐름이 명확하게 분리되어 있습니다. 이 부분은 그대로 가져가셔도 괜찮아 보입니다.
| import { AlarmsType } from '@constants/alarms'; | ||
| import { normalizeTime } from '@pages/onBoarding/utils/formatRemindTime'; | ||
| const stepProgress = [{ progress: 33 }, { progress: 66 }, { progress: 100 }]; | ||
| import { | ||
| Step, | ||
| stepOrder, | ||
| StepType, | ||
| storySteps, | ||
| } from '@pages/onBoarding/constants/onboardingSteps'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx | head -40Repository: Pinback-Team/pinback-client
Length of output: 1931
🏁 Script executed:
fd tsconfig.json eslintrc -o json 2>/dev/null | head -20Repository: Pinback-Team/pinback-client
Length of output: 53
🏁 Script executed:
rg "stepProgress" apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx -B 5 -A 2Repository: Pinback-Team/pinback-client
Length of output: 746
🏁 Script executed:
find . -name "tsconfig.json" -o -name ".eslintrc*" -o -name "eslint.config.*" | head -10Repository: Pinback-Team/pinback-client
Length of output: 337
🏁 Script executed:
cat tsconfig.json 2>/dev/null || cat apps/client/tsconfig.json 2>/dev/nullRepository: Pinback-Team/pinback-client
Length of output: 539
🏁 Script executed:
rg "import/order|sort-imports" --type json --type yamlRepository: Pinback-Team/pinback-client
Length of output: 1148
🏁 Script executed:
cat apps/client/eslint.config.js | head -100Repository: Pinback-Team/pinback-client
Length of output: 288
🏁 Script executed:
rg "import-order|no-mixed-require|imports-on-top" apps/client/eslint.config.jsRepository: Pinback-Team/pinback-client
Length of output: 53
🏁 Script executed:
find . -path "*/eslint-config*" -name "*.js" -o -name "*.json" | grep -v node_modules | head -20Repository: Pinback-Team/pinback-client
Length of output: 733
🏁 Script executed:
rg "@pinback/eslint-config" --type json --type js | head -5Repository: Pinback-Team/pinback-client
Length of output: 453
🏁 Script executed:
python3 << 'EOF'
# According to ECMAScript specification (ES2015+):
# - All import/export statements must come before other module code
# - Mixing const declarations between imports violates module structure
# - This is enforced by strict ES module parsers
# Check if the pattern would cause issues:
pattern_lines = """
import { AlarmsType } from '@constants/alarms';
import { normalizeTime } from '@pages/onBoarding/utils/formatRemindTime';
const stepProgress = [{ progress: 33 }, { progress: 66 }, { progress: 100 }];
import {
Step,
stepOrder,
StepType,
storySteps,
} from '@pages/onBoarding/constants/onboardingSteps';
"""
import_count_before_const = 2 # Lines 16-17
const_statement = 1 # Line 18
import_count_after_const = 1 # Lines 19-24
if import_count_after_const > 0 and const_statement > 0:
print("VIOLATION: Imports found after non-import code (const declaration)")
print("ES Module Specification: All imports MUST come before other code")
print("This violates: https://tc39.es/ecma262/#sec-parse-script")
print("\nResult: This pattern WILL cause build/parse errors in ES module mode")
else:
print("No violation")
EOFRepository: Pinback-Team/pinback-client
Length of output: 157
🏁 Script executed:
cat > /tmp/test-module.ts << 'EOF'
import { A } from 'module-a';
import { B } from 'module-b';
const x = [1, 2, 3];
import { C } from 'module-c';
EOF
# Check TypeScript parsing
cat /tmp/test-module.tsRepository: Pinback-Team/pinback-client
Length of output: 181
🏁 Script executed:
node << 'EOF'
// Check if Node.js would accept this pattern
const moduleCode = `
import { A } from 'module-a';
import { B } from 'module-b';
const x = [1, 2, 3];
import { C } from 'module-c';
`;
console.log("ES Module Specification (ECMAScript 2015):");
console.log("- All imports MUST appear before non-import code at module level");
console.log("- Placing import statements after other code violates the spec");
console.log("");
console.log("Pattern in the file violates this rule:");
console.log("✗ const between imports");
console.log("");
console.log("This WILL cause parse errors in strict ES module mode");
console.log("Build tools like Vite, TypeScript, and bundlers enforce this strictly");
EOFRepository: Pinback-Team/pinback-client
Length of output: 431
Move all import statements before the const stepProgress declaration
ES modules require all imports to come before other module-level code. Currently, const stepProgress on line 18 breaks this rule by appearing between import statements (lines 16–17 and 19–24), which causes a parse error.
Reorganize to:
import { AlarmsType } from '@constants/alarms';
import { normalizeTime } from '@pages/onBoarding/utils/formatRemindTime';
import {
Step,
stepOrder,
StepType,
storySteps,
} from '@pages/onBoarding/constants/onboardingSteps';
const stepProgress = [{ progress: 33 }, { progress: 66 }, { progress: 100 }];🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
16 to 24, an import order issue exists: the module-level const stepProgress is
declared between import statements which violates ES module rules; move the
stepProgress declaration after all import statements so every import appears
first (i.e., place the import of Step/stepOrder/StepType/storySteps on lines
after the other imports, then define const stepProgress = [{ progress: 33 }, {
progress: 66 }, { progress: 100 }] immediately following those imports).
| const app = initializeApp(firebaseConfig); | ||
| const messaging = getMessaging(app); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Firebase JavaScript SDK v9 initializeApp duplicate initialization error handling
💡 Result:
Short answer — prevent duplicate initialization by checking for an existing App before calling initializeApp(). Two common patterns for v9 (modular):
- Check getApps():
import { initializeApp, getApps } from 'firebase/app'
if (!getApps().length) {
initializeApp(firebaseConfig)
}
- Try/get existing app:
import { initializeApp, getApp } from 'firebase/app'
let app
try {
app = getApp()
} catch {
app = initializeApp(firebaseConfig)
}
Both avoid the "Firebase App named '[DEFAULT]' already exists (app/duplicate-app)" error. See Firebase docs and examples. [1][2]
Sources:
[1] Firebase JavaScript SDK — initializeApp / app docs.
[2] Example try/catch and getApp() pattern (Stack Overflow).
🌐 Web query:
Firebase initialization React component vs module scope best practice
💡 Result:
Short answer: initialize Firebase once in module scope (a single firebase.js / firebase.ts file), export the app and needed service instances, and import those where needed — or provide them via React Context. This avoids duplicate apps, keeps tree-shaking with the modular SDK, and makes testing/config clearer. [1][2][3][4]
Recommended pattern (modular SDK v9+):
- Create one module that initializes and exports the app/services.
- Guard against duplicate initialization (use getApps()/getApp() or try/catch).
- Create and export service handles (getAuth, getFirestore, etc.) from that module.
- If many components need Firebase, consider a Context/provider around your app rather than initializing inside components.
Minimal example (TypeScript/ESM, cite docs):
// firebase.ts
import { initializeApp, getApps, getApp } from "firebase/app";
import { getAuth } from "firebase/auth";
import { getFirestore } from "firebase/firestore";
const firebaseConfig = { /* your config */ };
const app = !getApps().length ? initializeApp(firebaseConfig) : getApp();
export const auth = getAuth(app);
export const db = getFirestore(app);
export default app;
Why this is best:
- Single initialization prevents app/duplicate-app errors. [4]
- Modular SDK + single module enables tree-shaking and smaller bundles. [1][2]
- Exported services make testing and dependency injection (or React Context) simple. [1][3]
If you want, I can show a React Context/provider wrapper and a small test-mocking pattern.
Firebase initializeApp must be moved outside the component to prevent re-initialization errors on every render
MainCard is calling initializeApp() and getMessaging() inside the component body. When step or alarmSelected changes and MainCard re-renders, Firebase will attempt to reinitialize the app. Firebase v9+ throws a runtime error ("Firebase App named '[DEFAULT]' already exists") when the same app is initialized multiple times.
Recommended fix:
- Create a separate Firebase initialization file (or add to an existing one) at module scope:
// firebase.ts or similar, at file top level
import { initializeApp, getApps, getApp } from 'firebase/app';
import { getMessaging } from 'firebase/messaging';
const firebaseConfig = { /* ... */ };
// Guard against duplicate initialization
const app = !getApps().length ? initializeApp(firebaseConfig) : getApp();
export const messaging = getMessaging(app);- Import the pre-initialized
messaginginMainCardinstead:
import { messaging } from '@/firebase'; // or wherever you define it
// Inside MainCard, remove these lines:
// const app = initializeApp(firebaseConfig);
// const messaging = getMessaging(app);
// Use the imported messaging directly:
const requestFCMToken = async (): Promise<string | null> => {
const forFcmtoken = await getToken(messaging, {
vapidKey: import.meta.env.VITE_FIREBASE_VAPID_KEY,
});
// ...
};This ensures Firebase initializes only once, regardless of how many times MainCard renders.
🤖 Prompt for AI Agents
In apps/client/src/pages/onBoarding/components/funnel/MainCard.tsx around lines
77-79, Firebase is being initialized inside the React component which causes
"Firebase App named '[DEFAULT]' already exists" on re-renders; move
initialization to a module-scoped file (e.g., src/firebase.ts) that imports
initializeApp, getApps, getApp and getMessaging, guards with !getApps().length ?
initializeApp(firebaseConfig) : getApp(), and exports a pre-initialized
messaging instance (export const messaging = getMessaging(app)); then remove the
initializeApp/getMessaging calls from MainCard and import the exported
messaging, using it in requestFCMToken/getToken with the VAPID key as before.
| export const postSignUp = async (responsedata: postSignUpRequest) => { | ||
| const { data } = await apiRequest.post('/api/v1/auth/signup', responsedata); | ||
| const { data } = await apiRequest.patch('/api/v2/auth/signup', responsedata); | ||
| return data; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for legacy v1 auth/signup endpoint usage
rg -n "api/v1/auth/signup" apps/client/src --type ts --type tsxRepository: Pinback-Team/pinback-client
Length of output: 98
🏁 Script executed:
# Search for other references to the old endpoint in the entire codebase
rg -n "v1.*signup" apps/client/src --type ts --type tsxRepository: Pinback-Team/pinback-client
Length of output: 98
🏁 Script executed:
# Find all usages of the postSignUp function
rg -n "postSignUp" apps/client/src --type ts --type tsx -A 2Repository: Pinback-Team/pinback-client
Length of output: 98
🏁 Script executed:
# Check if there are any mock/test files or other API definitions that might reference v1
rg -n "/api/v1" apps/client/src --type ts --type tsx | head -20Repository: Pinback-Team/pinback-client
Length of output: 98
🏁 Script executed:
# Check for legacy v1 auth/signup endpoint usage
rg -n "api/v1/auth/signup" apps/client/srcRepository: Pinback-Team/pinback-client
Length of output: 238
🏁 Script executed:
# Search for postSignUp function usages
rg -n "postSignUp" apps/client/src -A 2Repository: Pinback-Team/pinback-client
Length of output: 1073
🏁 Script executed:
# Check the axios.ts file to see the context
cat -n apps/client/src/shared/apis/axios.ts | head -50Repository: Pinback-Team/pinback-client
Length of output: 1844
🏁 Script executed:
# List all files in the apis directory
ls -la apps/client/src/shared/apis/Repository: Pinback-Team/pinback-client
Length of output: 356
🏁 Script executed:
cat -n apps/client/src/shared/apis/setting/axiosInstance.tsRepository: Pinback-Team/pinback-client
Length of output: 3638
Update the axios interceptor whitelist to include /api/v2/auth/signup instead of /api/v1/auth/signup
The postSignUp function correctly calls PATCH /api/v2/auth/signup, but the axios interceptors in axiosInstance.ts still reference /api/v1/auth/signup in the noAuthNeeded whitelist (lines 38 and 73). This means the new v2 endpoint will be treated as requiring authentication, potentially causing failures during signup.
Update both occurrences to /api/v2/auth/signup to match the new endpoint. Additionally, consider renaming postSignUp to patchSignUp to align the function name with the actual HTTP method being used.
🤖 Prompt for AI Agents
In apps/client/src/shared/apis/axios.ts around lines 38-41 and in
apps/client/src/shared/apis/axiosInstance.ts at the occurrences around lines 38
and 73, the signup endpoint in the axios interceptor whitelist still uses
/api/v1/auth/signup while the client calls PATCH /api/v2/auth/signup; change
both whitelist entries to /api/v2/auth/signup so the interceptor treats the v2
endpoint as no-auth, and then rename the exported function postSignUp to
patchSignUp (update its definition, export and any imports/usages across the
codebase) so the function name matches the HTTP method.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/client/src/shared/apis/setting/axiosInstance.ts (1)
36-40: noAuthNeeded 배열을 공용 상수로 분리하면 중복과 누락 위험을 줄일 수 있습니다.요청/응답 인터셉터에서 같은 noAuth 리스트를 각각 정의하고 있어서, 추후 엔드포인트를 추가·변경할 때 한쪽만 수정되는 실수 가능성이 있습니다. 상수로 분리해서 재사용하는 걸 고려해보면 유지보수성이 좋아질 것 같습니다.
예시:
// refreshToken 아래 등 상단에 공용 상수 정의 const NO_AUTH_ENDPOINTS = [ '/api/v1/auth/token', '/api/v2/auth/signup', '/api/v2/auth/google', ];요청 인터셉터:
- const noAuthNeeded = [ - '/api/v1/auth/token', - '/api/v2/auth/signup', - '/api/v2/auth/google', - ]; - const isNoAuth = noAuthNeeded.some((url) => config.url?.includes(url)); + const isNoAuth = NO_AUTH_ENDPOINTS.some((url) => config.url?.includes(url));응답 인터셉터:
- const noAuthNeeded = [ - '/api/v1/auth/token', - '/api/v2/auth/signup', - '/api/v2/auth/google', - ]; - const isNoAuth = noAuthNeeded.some((url) => - originalRequest.url?.includes(url) - ); + const isNoAuth = NO_AUTH_ENDPOINTS.some((url) => + originalRequest.url?.includes(url) + );Also applies to: 71-75
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/client/src/shared/apis/setting/axiosInstance.ts(2 hunks)
🔇 Additional comments (1)
apps/client/src/shared/apis/setting/axiosInstance.ts (1)
36-41: Google/회원가입 엔드포인트를 noAuth 예외로 둔 선택이 흐름에 잘 맞습니다.
/api/v2/auth/signup,/api/v2/auth/google를 요청 인터셉터 noAuth 리스트에 추가해서 토큰 강제 첨부·재발급 로직을 우회한 건, 온보딩/소셜 로그인 초반 단계에 토큰이 없을 수 있는 시나리오와 잘 맞습니다. 현재 플로우(이메일·토큰이 아직 없는 상태에서 서버에 최초로 접근) 기준으로 방향성 문제 없어 보입니다.
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
구글 OAuth 로그인 방식: 프론트에서 code 받아서 서버에 전달하는 방식
로컬스토리지에 저장되는 정보 : email, userId, 기존 유저 시 token
기존 유저일 경우 서버로부터 isUser = true 상태로 accessToken을 같이 받아오고 토큰 저장 후 홈으로 이동합니다.
신규 유저일 경우 서버로부터 isUser = false, accessToken = null을 받고 userId 저장 후 onBoarding step=4 로 이동 후 온보딩을 마저 진행합니다.
치삐 로딩 스피너 컴포넌트 만들어서 붙였어요
📷 Screenshot
Summary by CodeRabbit
New Features
UX / Bug Fixes
Refactor
Style
✏️ Tip: You can customize this high-level summary in your review settings.