[FIX] 로그인 추가 기능 및 로직 수정, modal 스크롤 프리징 처리 - #115
Conversation
…into fix/fix-login-error
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 68 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough로그인 역할 타입과 OAuth 변환을 정리했습니다. Soft UX 쿠키와 이미지 URL을 정제합니다. 비밀번호 변경 후 재인증과 Toast를 추가했습니다. 프로필 폼의 제출 조건과 Enter 동작을 수정했습니다. 중첩 가능한 본문 스크롤 잠금 훅을 도입했습니다. Changes인증 및 프로필 UX
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🔵 Low · up to The profile forms now block accidental Enter submission, but the shared helper performs the event mutation directly instead of returning a decision for the form to apply. This is a bounded implementation risk that should be fixed or explicitly accepted; no broader merge-blocking risk remains. Sequence Diagram(s)sequenceDiagram
participant ProfileEditForm
participant Reauth
participant SessionStorage
participant LoginForm
participant Toast
ProfileEditForm->>Reauth: 비밀번호 변경 후 재인증 요청
Reauth->>SessionStorage: Toast 플래그 저장
Reauth->>LoginForm: 로그아웃 후 로그인 경로 이동
LoginForm->>SessionStorage: Toast 플래그 소비
LoginForm->>Toast: 변경 결과 메시지 렌더링
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/lib/auth/passwordChangedToast.ts (1)
22-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff브라우저 부수효과를
src/lib밖으로 이동하세요.
src/lib경로 규칙은 순수 함수만 허용합니다. 현재 두 모듈이 저장소, Zustand 상태, 브라우저 이동을 직접 변경합니다.
src/lib/auth/passwordChangedToast.ts#L22-L50: sessionStorage 읽기·쓰기·삭제를 Client Hook 또는 UI 경계로 이동하세요.src/lib/auth/reauthAfterPasswordChange.ts#L11-L23: logout 및window.location.assign()처리를 Client Hook 또는 호출 컴포넌트로 이동하세요.As per path instructions, "
src/lib/**/*.ts: 순수 함수로 작성하고 부수효과를 두지 않습니다."🤖 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 `@src/lib/auth/passwordChangedToast.ts` around lines 22 - 50, Move the sessionStorage read, write, and removal out of markPasswordChangedToast and consumePasswordChangedToast in src/lib/auth/passwordChangedToast.ts into a Client Hook or UI boundary, keeping the toast flag selection and message mapping as pure logic. Also move logout and window.location.assign() out of src/lib/auth/reauthAfterPasswordChange.ts into a Client Hook or calling component; update both affected files accordingly.Source: Path instructions
src/lib/utils/preventEnterSubmitOnInput.ts (1)
7-11: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win이벤트 차단 부수효과를 컴포넌트 경계로 분리해 주세요.
Line 11에서
event.preventDefault()를 직접 호출합니다. 입력 대상과 Enter 여부를 boolean으로 판정하는 순수 함수와, 반환값에 따라preventDefault()를 호출하는 form handler를 분리하세요. 이렇게 하면src/lib유틸리티가 순수 함수 규칙을 지키고 테스트도 단순해집니다.As per path instructions,
src/lib/**/*.ts는 "순수 함수로 작성하고 부수효과를 두지 않습니다." 규칙을 따릅니다.🤖 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 `@src/lib/utils/preventEnterSubmitOnInput.ts` around lines 7 - 11, Refactor preventEnterSubmitOnInput so the input-target and Enter/composition checks live in a pure boolean-returning helper, while the form event handler invokes event.preventDefault() only when that helper returns true. Keep the existing behavior unchanged and ensure the src/lib utility itself has no side effects.Source: Path instructions
🤖 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 `@src/components/profile/MoverProfileEditForm.tsx`:
- Around line 45-47: Update the reset handling in MoverProfileEditForm to use
the submitted formValues rather than the current getValues() result, so only
values actually sent to the server become the new baseline. Alternatively,
ensure reset runs only when the current values still match formValues; preserve
accurate isDirty behavior when inputs change during submission.
In `@src/hooks/useBodyScrollLock.ts`:
- Around line 21-24: Update the body scroll-lock logic around scrollbarWidth so
it preserves the existing computed padding-right and adds scrollbarWidth instead
of replacing it. Set document.body.style.paddingRight to the combined value
while retaining the current overflow and positive-width behavior.
In `@src/lib/auth/role.ts`:
- Around line 35-39: saveRole에서 parseSoftUxAuthRole(role)가 반환되지 않는 경우에도 ADMIN이면
기존 ROLE_STORAGE_KEY 쿠키를 제거한 뒤 반환하도록 수정하세요. CUSTOMER/MOVER 저장 동작은 유지하고, 기존 클라이언트
저장소의 제거 API를 사용해 moving_role 힌트가 남지 않게 하세요.
In `@src/lib/utils/moverProfileImage.ts`:
- Line 5: Update isLocalPublicPath to reject backslash characters before
treating a value as a local public path, so inputs such as "/\\attacker.example"
fail validation while normal single-slash local paths remain accepted.
In `@src/lib/utils/preventEnterSubmitOnInput.ts`:
- Around line 8-11: Update the input check in preventEnterSubmitOnInput so Enter
is prevented only for text-entry input types such as text, password, email, tel,
search, and url; allow file and other non-text input types to proceed normally
while preserving the existing composition and non-input guards.
---
Nitpick comments:
In `@src/lib/auth/passwordChangedToast.ts`:
- Around line 22-50: Move the sessionStorage read, write, and removal out of
markPasswordChangedToast and consumePasswordChangedToast in
src/lib/auth/passwordChangedToast.ts into a Client Hook or UI boundary, keeping
the toast flag selection and message mapping as pure logic. Also move logout and
window.location.assign() out of src/lib/auth/reauthAfterPasswordChange.ts into a
Client Hook or calling component; update both affected files accordingly.
In `@src/lib/utils/preventEnterSubmitOnInput.ts`:
- Around line 7-11: Refactor preventEnterSubmitOnInput so the input-target and
Enter/composition checks live in a pure boolean-returning helper, while the form
event handler invokes event.preventDefault() only when that helper returns true.
Keep the existing behavior unchanged and ensure the src/lib utility itself has
no side effects.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 504da944-ab7a-4f7e-8895-ca7bad780716
📒 Files selected for processing (30)
src/app/(auth)/oauth/[provider]/callback/page.tsxsrc/app/layout.tsxsrc/app/movers/layout.tsxsrc/components/auth/LoginForm.tsxsrc/components/auth/RoleGuard.tsxsrc/components/common/Header/HeaderSideNav.tsxsrc/components/common/Header/ProfileMenuTrigger.tsxsrc/components/common/Modal/ModalMain.tsxsrc/components/profile/CustomerProfileEditForm.tsxsrc/components/profile/CustomerProfileForm.tsxsrc/components/profile/MoverBasicInfoEditForm.tsxsrc/components/profile/MoverProfileEditForm.tsxsrc/components/profile/MoverProfileForm.tsxsrc/hooks/auth/useLoginMutation.tssrc/hooks/useBodyScrollLock.tssrc/lib/api/auth.tssrc/lib/auth/clientStorageHint.tssrc/lib/auth/nickname.tssrc/lib/auth/oauth.tssrc/lib/auth/passwordChangedToast.tssrc/lib/auth/profileCompleted.tssrc/lib/auth/profileImage.tssrc/lib/auth/reauthAfterPasswordChange.tssrc/lib/auth/redirect.tssrc/lib/auth/role.tssrc/lib/auth/startOAuthLogin.tssrc/lib/schemas/passwordChangeFields.tssrc/lib/utils/moverProfileImage.tssrc/lib/utils/preventEnterSubmitOnInput.tssrc/stores/useAuthStore.ts
juengseulki
left a comment
juengseulki
left a comment
There was a problem hiding this comment.
📋 PR 리뷰
전체 변경사항 확인했습니다!
이번 PR은 로그인 Soft UX 힌트 정리, 이메일 로그인 role 검증을 BE 기준으로 정리, 비밀번호 변경 후 재로그인 흐름, 프로필 폼 UX, Modal/SideNav 스크롤 잠금 공통화를 중심으로 확인했습니다.
👍 잘 반영된 부분
- Soft UX 힌트를 cookie 단일 저장 방식으로 정리하고, 기존 localStorage 값은 clear 시 마이그레이션 정리하도록 한 점이 좋습니다.
- nickname은 trim + 제어문자 제거 + 길이 제한을 적용하고 있어 Header 표시용 힌트로 사용하기에 적절합니다.
- role 힌트는
CUSTOMER | MOVER만 허용하고ADMIN이나 임의 문자열은 저장하지 않도록 분리해 실제 AuthRole과 Soft UX 힌트 역할이 명확해졌습니다. - profile image 역시 로컬 경로와 allowlist 기반 https 원격 URL만 허용하고
javascript:,data:,http:, protocol-relative URL을 차단하도록 보완한 점이 좋습니다. - 이메일 로그인은 현재 BE 계약에 맞게
role을 요청에 포함하고, FE에서 로그인 성공 후 audience mismatch를 다시 logout하는 로직을 제거한 방향이 맞습니다. - 반대로 OAuth는 아직 BE에서 기존 계정 role mismatch를 토큰 발급 전에 차단하지 않으므로 FE 롤백 로직을 유지해 이메일/OAuth 정책 차이를 잘 구분했습니다.
- 비밀번호 변경 후 서버에서 기존 Refresh Token이 폐기되는 흐름에 맞춰 로컬 세션도 정리하고 hard navigation으로 로그인 페이지로 이동하도록 한 점이 자연스럽습니다.
- 비밀번호 변경 안내는
sessionStorageone-shot flag로 넘겨 로그인 페이지에서 한 번만 Toast를 표시하도록 구성되어 새 document navigation 이후에도 안내가 유지됩니다. useBodyScrollLock에 참조 카운트를 두어 Modal과 SideNav가 동시에 열리는 경우 한쪽이 닫혀도 다른 쪽의 scroll lock이 풀리지 않도록 한 점이 좋습니다.- Modal은
isVisible이 아니라isRendered기준으로 잠금을 유지해 exit animation 동안 배경이 먼저 스크롤되는 문제를 방지했습니다. - SideNav도 동일 훅을 사용하도록 정리해 body overflow 제어 로직이 중복되지 않게 됐습니다.
🔍 확인 및 제안
1. Soft UX cookie는 현재 역할로 사용한다면 보안상 문제 없어 보입니다
nickname, role, profile image cookie는 클라이언트가 직접 수정할 수 있는 값이므로 인증/인가의 신뢰값으로 사용하면 안 됩니다.
현재 구현은 Header 첫 페인트 등의 Soft UX 힌트 용도로만 사용하고,
실제 세션 및 권한은 access token / 서버 응답을 기준으로 확정하는 구조이므로 현재 사용 범위에서는 괜찮아 보입니다.
특히 role도 parseSoftUxAuthRole()을 별도로 두어 허용값만 사용하는 점이 좋습니다.
2. sanitizeSoftUxProfileImageUrl과 resolveMoverProfileImageSrc는 지금은 분리 유지가 더 자연스러워 보입니다
둘 다 이미지 URL 검증을 한다는 점은 비슷하지만 책임이 조금 다릅니다.
-
sanitizeSoftUxProfileImageUrl- cookie에 저장하거나 cookie에서 읽을 값을 검증
- 유효하지 않으면
null - 길이 제한이나
.., 역슬래시 등 Soft UX 저장값 자체의 방어까지 포함
-
resolveMoverProfileImageSrc- 실제 카드/목록에서 렌더링할 이미지 src 결정
- 유효하지 않으면 기본 이미지 fallback
따라서 완전히 하나로 합치기보다는,
공통으로 쓰는 isSafeProfileImageUrl 같은 작은 predicate만 추출하고
각 함수는 현재 책임을 유지하는 쪽이 더 명확해 보입니다.
3. 이메일 로그인 role 처리
LoginForm에서 audience를 role로 바꿔 BE로 전달하고,
성공 응답 이후에는 FE가 다시 role mismatch를 판단하지 않는 구조로 변경된 점 확인했습니다.
BE가 mismatch 시 토큰 발급 전에 401을 반환한다는 현재 계약과 맞기 때문에
기존처럼 성공 후 logout까지 호출할 필요는 없어 보입니다.
4. 비밀번호 변경 후 재로그인
reauthAfterPasswordChange()에서 Toast flag를 먼저 남긴 뒤
logout({ deferUiClear: true })을 시도하고 실패 여부와 관계없이 로그인 페이지로 hard navigate하도록 되어 있습니다.
이미 BE에서 password 변경 시 Refresh Token을 폐기하는 구조이므로
logout API가 실패하더라도 클라이언트 세션 정리 후 재로그인으로 보내는 현재 정책이 자연스럽습니다.
💬 To Reviewer
말씀해주신 Soft UX cookie 검증과 이미지 helper 분리 여부를 중심으로 확인했습니다.
Soft UX cookie는 사용자가 임의 변경 가능한 값이지만,
현재 코드에서는 실제 인증·인가 결정을 위한 값이 아니라 Header SSR/첫 페인트를 위한 힌트로 사용되고 있습니다.
또 nickname, role, profile image 각각 load/save 시 sanitize를 수행하고 있어
깨진 값이나 예상하지 않은 문자열이 그대로 UI에 사용되는 범위도 줄였습니다.
특히 profile image는 로컬 /...와 허용된 https host만 통과시키고,
protocol-relative URL과 잘못된 scheme을 제외하도록 한 점이 적절합니다.
sanitizeSoftUxProfileImageUrl과 resolveMoverProfileImageSrc는
검증 기준 일부는 공유하지만 반환 정책과 사용 목적이 달라 현재는 분리 유지하는 편이 더 읽기 좋아 보입니다.
필요하다면 나중에 공통 URL 판별 함수만 추출하는 정도가 적절할 것 같습니다.
이메일 로그인은 현재 BE에서 요청 role과 DB role mismatch를 토큰 발급 전에 차단하므로,
FE에서 별도 mismatch logout을 제거한 것도 맞는 방향입니다.
OAuth는 아직 같은 서버 정책이 적용되지 않아 기존 롤백을 유지하고 있어 두 인증 흐름도 잘 구분되어 있습니다.
스크롤 잠금 역시 참조 카운트 기반 공통 훅으로 바꾸면서
Modal exit 기간과 중첩 overlay 상황까지 고려되어 있습니다.
이번 PR에서 설명한 기능들은 전반적으로 빠짐없이 잘 반영된 것으로 보입니다.
수고하셨습니다! 👍
…into fix/fix-login-error
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lib/utils/preventEnterSubmitOnInput.ts`:
- Around line 3-15: Update preventEnterSubmitOnInput to be a pure predicate that
only returns whether the Enter key should be blocked, without calling
preventDefault; move the event.preventDefault() call into each profile form’s
onKeyDown handler when the utility returns true, preserving the existing
composition, target, and input-type checks.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f25a02bb-339c-49b4-8e46-f51e577c56ef
📒 Files selected for processing (9)
src/components/profile/CustomerProfileEditForm.tsxsrc/components/profile/MoverBasicInfoEditForm.tsxsrc/components/profile/MoverProfileEditForm.tsxsrc/components/profile/ProfileChipGroup.tsxsrc/components/profile/ProfileImageUploader.tsxsrc/hooks/useBodyScrollLock.tssrc/lib/auth/role.tssrc/lib/utils/moverProfileImage.tssrc/lib/utils/preventEnterSubmitOnInput.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/lib/utils/moverProfileImage.ts
- src/hooks/useBodyScrollLock.ts
- src/components/profile/MoverProfileEditForm.tsx
- src/lib/auth/role.ts
- src/components/profile/CustomerProfileEditForm.tsx
|
두 이미지 검증 함수가 //, \, ..을 같은 기준으로 막아서, cookie로 들어오든 DB 값으로 들어오든 우회 지점이 갈라지지 않게 짜신 점이 좋은 것 같습니다. Soft UX cookie 검증 관련 이미지 함수 통합 관련 고생 많으셨습니다 👍 |
3 participants
📋 작업 내용
🔥 변경 사항
localStorage이중 저장 제거,cookie만 사용, nickname/role/image sanitize)role전달 후 BE 검증, FE audience mismatchlogout제거role을 BE가 강제하지 않으므로 FE 세션 롤백 유지Enter submit방지,isDirty없을 때 수정 버튼disableduseBodyScrollLock훅 생성,Modal은exit(isRendered)까지 스크롤 잠금✅ 체크리스트
📷 스크린샷 (선택)
💬 To Reviewer
sanitizeSoftUxProfileImageUrl)과resolveMoverProfileImageSrc부분을 합쳐도 괜찮을까요? 둘의 역할이 살짝 다르지만 같은 선이어서 고민하다가 따로 정리하지는 않았습니다.Summary by CodeRabbit