feat: #259 로그인 상태에서 비밀번호 변경#269
Merged
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Walkthrough로그인 상태에서 비밀번호를 변경할 수 있는 기능을 추가합니다. 백엔드에 JWT 인증 기반의 PATCH 엔드포인트를 구현하고, 프론트엔드의 Settings 페이지에 비밀번호 변경 모달 UI를 추가합니다. Changes
Sequence DiagramsequenceDiagram
participant User as 사용자
participant Client as 프론트엔드
participant Auth as AuthController
participant Service as AuthService
participant DB as Database
User->>Client: 현재/신규 비밀번호 입력 후 제출
Client->>Client: 클라이언트 측 검증 (8자, 영문+숫자, 일치 확인)
Client->>Auth: PATCH /auth/change-password + JWT<br/>(currentPassword, newPassword, confirmPassword)
Auth->>Auth: JWT 인증 & userId 검증
Auth->>Service: changePassword(userId, dto)
Service->>Service: 신규/확인 비밀번호 일치 검증
Service->>DB: userId로 email 인증 레코드 조회
Service->>Service: 현재 비밀번호 해싱값과 DB값 비교
alt 비밀번호 일치
Service->>Service: 신규 비밀번호 해싱 (bcrypt)
Service->>DB: 레코드 업데이트 (새 해시)
Service->>DB: 사용자의 모든 refresh token 폐기
Service-->>Auth: { message: "재로그인 필요" }
Auth-->>Client: 200 + message
Client->>Client: 성공 알림 표시
Client->>Client: 로그아웃 처리
Client->>User: 로그인 페이지로 리다이렉트
else 비밀번호 불일치
Service-->>Auth: 401 Unauthorized
Auth-->>Client: 401
Client->>User: 오류 메시지 표시
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/pages/__tests__/Settings.test.tsx (1)
157-201: 비밀번호 변경 “제출 성공 경로” 단언을 추가하면 더 안전합니다.현재는 버튼/모달 표시까지만 검증합니다.
authApi.changePassword호출,logout호출,/login이동까지 확인하는 테스트를 추가하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/__tests__/Settings.test.tsx` around lines 157 - 201, Add a test that verifies the successful password-change flow: mock authApi.changePassword to resolve, fill in and submit the password-change form in the Settings component (use the existing renderWithRouter and userEvent setup), then assert authApi.changePassword was called with the current and new password values, assert mockLogout was called, and assert the app navigated to '/login' (check router history or the presence of the login route) to confirm logout+redirect. Locate the form and submit button via labels/headings already used in the tests and reuse useAuth/mockLogout to validate side effects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/auth/auth.service.ts`:
- Around line 285-287: Wrap the password update and token revocation in a single
database transaction so both succeed or both roll back: inside the AuthService
method that sets auth.credential, calls this.userAuthRepository.save(auth) and
this.revokeAllRefreshTokens(userId), start a transaction (e.g., via a
QueryRunner or TypeORM transactionalEntityManager), perform the bcrypt hash
assignment to auth.credential, save using the transactional manager (not
this.userAuthRepository directly), call revokeAllRefreshTokens(userId) using the
same transaction context (or convert it to accept the transactional manager),
and commit; on error rollback and rethrow the error to ensure atomicity.
In `@backend/test/app.e2e-spec.ts`:
- Around line 15-17: The test bootstrap imports the same suite twice
("./suites/cellar.e2e-spec"), causing duplicate test execution; remove the
redundant import so each suite is imported only once (keep one import of
"./suites/cellar.e2e-spec" and delete the other), ensuring the remaining imports
(including "./suites/auth-change-password.e2e-spec") stay unchanged.
In `@backend/test/suites/auth-change-password.e2e-spec.ts`:
- Around line 101-150: Add an E2E that verifies refresh-token revocation on
password change: create a user with context.testHelper.createUser and capture
its issued refresh token (from the login response or cookie) before calling
PATCH /auth/change-password (as in the existing tests), then after asserting 200
on the password change attempt, call the refresh endpoint (e.g., POST
/auth/refresh or the route your app exposes) using the previously captured
refresh token and assert it fails (401/403 or the app's expected error). Use the
same helper methods generateUniqueEmail/createUser and existing routes
'/auth/change-password' and '/auth/login' to locate where to add the test and
ensure you reuse the original user.token if needed to authorize the change.
In `@src/pages/Settings.tsx`:
- Around line 534-535: The password form state is only cleared on success,
leaving sensitive inputs in memory when the Dialog is closed by the user; update
the Dialog with open={isChangePasswordOpen} onOpenChange={...} to detect when it
closes (open becomes false) and call the form-reset routine (e.g.,
resetChangePasswordForm or setCurrentPassword/setNewPassword/setConfirmPassword
to empty strings) so all password-related state is cleared immediately; apply
the same change to the other modal referenced (the Dialog at the earlier block
around lines 277-278) so both modals clear sensitive inputs on close.
---
Nitpick comments:
In `@src/pages/__tests__/Settings.test.tsx`:
- Around line 157-201: Add a test that verifies the successful password-change
flow: mock authApi.changePassword to resolve, fill in and submit the
password-change form in the Settings component (use the existing
renderWithRouter and userEvent setup), then assert authApi.changePassword was
called with the current and new password values, assert mockLogout was called,
and assert the app navigated to '/login' (check router history or the presence
of the login route) to confirm logout+redirect. Locate the form and submit
button via labels/headings already used in the tests and reuse
useAuth/mockLogout to validate side effects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2b54c8aa-c5ed-4359-a9b3-b338beed9d50
📒 Files selected for processing (8)
backend/src/auth/auth.controller.tsbackend/src/auth/auth.service.tsbackend/src/auth/dto/change-password.dto.tsbackend/test/app.e2e-spec.tsbackend/test/suites/auth-change-password.e2e-spec.tssrc/lib/api/auth.api.tssrc/pages/Settings.tsxsrc/pages/__tests__/Settings.test.tsx
Comment on lines
+285
to
+287
| auth.credential = await bcrypt.hash(dto.newPassword, 10); | ||
| await this.userAuthRepository.save(auth); | ||
| await this.revokeAllRefreshTokens(userId); |
Contributor
There was a problem hiding this comment.
비밀번호 갱신과 토큰 폐기는 트랜잭션으로 묶어야 합니다.
현재는 비밀번호 저장 후 토큰 폐기가 실패하면 부분 반영 상태가 됩니다(비밀번호는 바뀌었는데 기존 리프레시 토큰은 유효). 두 작업을 원자적으로 처리해 주세요.
🔒 제안 수정안
- auth.credential = await bcrypt.hash(dto.newPassword, 10);
- await this.userAuthRepository.save(auth);
- await this.revokeAllRefreshTokens(userId);
+ await this.userAuthRepository.manager.transaction(async (manager) => {
+ auth.credential = await bcrypt.hash(dto.newPassword, 10);
+ await manager.save(UserAuthentication, auth);
+ await manager.update(
+ RefreshToken,
+ { userId, isRevoked: false },
+ { isRevoked: true },
+ );
+ });📝 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.
Suggested change
| auth.credential = await bcrypt.hash(dto.newPassword, 10); | |
| await this.userAuthRepository.save(auth); | |
| await this.revokeAllRefreshTokens(userId); | |
| await this.userAuthRepository.manager.transaction(async (manager) => { | |
| auth.credential = await bcrypt.hash(dto.newPassword, 10); | |
| await manager.save(UserAuthentication, auth); | |
| await manager.update( | |
| RefreshToken, | |
| { userId, isRevoked: false }, | |
| { isRevoked: true }, | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/auth/auth.service.ts` around lines 285 - 287, Wrap the password
update and token revocation in a single database transaction so both succeed or
both roll back: inside the AuthService method that sets auth.credential, calls
this.userAuthRepository.save(auth) and this.revokeAllRefreshTokens(userId),
start a transaction (e.g., via a QueryRunner or TypeORM
transactionalEntityManager), perform the bcrypt hash assignment to
auth.credential, save using the transactional manager (not
this.userAuthRepository directly), call revokeAllRefreshTokens(userId) using the
same transaction context (or convert it to accept the transactional manager),
and commit; on error rollback and rethrow the error to ensure atomicity.
Comment on lines
15
to
+17
| import './suites/cellar.e2e-spec'; | ||
| import './suites/cellar.e2e-spec'; | ||
| import './suites/auth-change-password.e2e-spec'; |
Contributor
There was a problem hiding this comment.
중복된 테스트 스위트 import를 제거해주세요.
./suites/cellar.e2e-spec가 두 번 import되어 해당 스위트가 중복 실행될 수 있습니다.
🔧 제안 수정안
import './suites/notes-schemas.e2e-spec';
import './suites/cellar.e2e-spec';
-import './suites/cellar.e2e-spec';
import './suites/auth-change-password.e2e-spec';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/test/app.e2e-spec.ts` around lines 15 - 17, The test bootstrap
imports the same suite twice ("./suites/cellar.e2e-spec"), causing duplicate
test execution; remove the redundant import so each suite is imported only once
(keep one import of "./suites/cellar.e2e-spec" and delete the other), ensuring
the remaining imports (including "./suites/auth-change-password.e2e-spec") stay
unchanged.
Comment on lines
+101
to
+150
| it('PATCH /auth/change-password - 정상 변경 후 200 및 새 비밀번호로 로그인 성공', async () => { | ||
| const uniqueEmail = context.testHelper.generateUniqueEmail('change-pw'); | ||
| const user = await context.testHelper.createUser('Change Password Success', uniqueEmail, 'password123'); | ||
|
|
||
| await request(context.app.getHttpServer()) | ||
| .patch('/auth/change-password') | ||
| .set('Authorization', `Bearer ${user.token}`) | ||
| .send({ | ||
| currentPassword: 'password123', | ||
| newPassword: 'newPassword456', | ||
| confirmPassword: 'newPassword456', | ||
| }) | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).toHaveProperty('message'); | ||
| }); | ||
|
|
||
| // 새 비밀번호로 로그인 성공 확인 | ||
| return request(context.app.getHttpServer()) | ||
| .post('/auth/login') | ||
| .send({ | ||
| email: uniqueEmail, | ||
| password: 'newPassword456', | ||
| }) | ||
| .expect(201); | ||
| }); | ||
|
|
||
| it('PATCH /auth/change-password - 변경 후 이전 비밀번호로 로그인 실패', async () => { | ||
| const uniqueEmail = context.testHelper.generateUniqueEmail('change-pw-old'); | ||
| const user = await context.testHelper.createUser('Change Password Old', uniqueEmail, 'password123'); | ||
|
|
||
| await request(context.app.getHttpServer()) | ||
| .patch('/auth/change-password') | ||
| .set('Authorization', `Bearer ${user.token}`) | ||
| .send({ | ||
| currentPassword: 'password123', | ||
| newPassword: 'newPassword456', | ||
| confirmPassword: 'newPassword456', | ||
| }) | ||
| .expect(200); | ||
|
|
||
| // 이전 비밀번호로 로그인 실패 확인 | ||
| return request(context.app.getHttpServer()) | ||
| .post('/auth/login') | ||
| .send({ | ||
| email: uniqueEmail, | ||
| password: 'password123', | ||
| }) | ||
| .expect(401); | ||
| }); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
리프레시 토큰 전체 폐기 요구사항을 검증하는 E2E가 필요합니다.
현재 성공/실패 로그인은 검증하지만, “비밀번호 변경 후 기존 refresh token 사용 불가”는 직접 검증하지 않습니다. 이 케이스를 추가해 주세요.
🧪 추가 테스트 예시
+ it('PATCH /auth/change-password - 변경 후 기존 refresh token으로 재발급 실패', async () => {
+ const email = context.testHelper.generateUniqueEmail('change-pw-refresh');
+ await context.testHelper.createUser('Change Password Refresh', email, 'password123');
+
+ const loginRes = await request(context.app.getHttpServer())
+ .post('/auth/login')
+ .send({ email, password: 'password123' })
+ .expect(201);
+
+ const accessToken = loginRes.body.access_token;
+ const refreshCookie = (loginRes.headers['set-cookie'] || []).find((c: string) =>
+ c.startsWith('refresh_token='),
+ );
+ expect(refreshCookie).toBeDefined();
+
+ await request(context.app.getHttpServer())
+ .patch('/auth/change-password')
+ .set('Authorization', `Bearer ${accessToken}`)
+ .send({
+ currentPassword: 'password123',
+ newPassword: 'newPassword456',
+ confirmPassword: 'newPassword456',
+ })
+ .expect(200);
+
+ await request(context.app.getHttpServer())
+ .post('/auth/refresh')
+ .set('Cookie', refreshCookie as string)
+ .expect(401);
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/test/suites/auth-change-password.e2e-spec.ts` around lines 101 - 150,
Add an E2E that verifies refresh-token revocation on password change: create a
user with context.testHelper.createUser and capture its issued refresh token
(from the login response or cookie) before calling PATCH /auth/change-password
(as in the existing tests), then after asserting 200 on the password change
attempt, call the refresh endpoint (e.g., POST /auth/refresh or the route your
app exposes) using the previously captured refresh token and assert it fails
(401/403 or the app's expected error). Use the same helper methods
generateUniqueEmail/createUser and existing routes '/auth/change-password' and
'/auth/login' to locate where to add the test and ensure you reuse the original
user.token if needed to authorize the change.
Comment on lines
+534
to
+535
| <Dialog open={isChangePasswordOpen} onOpenChange={setIsChangePasswordOpen}> | ||
| <DialogContent> |
Contributor
There was a problem hiding this comment.
모달 닫기 시 비밀번호 입력값을 즉시 초기화해주세요.
현재는 성공 시에만 폼을 비우고, 사용자가 닫기(X/외부 클릭)로 종료하면 민감 입력값이 state에 남습니다.
🧹 제안 수정안
- <Dialog open={isChangePasswordOpen} onOpenChange={setIsChangePasswordOpen}>
+ <Dialog
+ open={isChangePasswordOpen}
+ onOpenChange={(open) => {
+ setIsChangePasswordOpen(open);
+ if (!open) {
+ setChangePasswordForm({ currentPassword: '', newPassword: '', confirmPassword: '' });
+ }
+ }}
+ >Also applies to: 277-278
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Settings.tsx` around lines 534 - 535, The password form state is
only cleared on success, leaving sensitive inputs in memory when the Dialog is
closed by the user; update the Dialog with open={isChangePasswordOpen}
onOpenChange={...} to detect when it closes (open becomes false) and call the
form-reset routine (e.g., resetChangePasswordForm or
setCurrentPassword/setNewPassword/setConfirmPassword to empty strings) so all
password-related state is cleared immediately; apply the same change to the
other modal referenced (the Dialog at the earlier block around lines 277-278) so
both modals clear sensitive inputs on close.
This was referenced Mar 16, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
Backend
ChangePasswordDto: 현재/새/확인 비밀번호 유효성 검사 (8자 이상, 영문+숫자)AuthService.changePassword(): 현재 비밀번호 검증 → 해시 저장 → 리프레시 토큰 revokeAuthController:PATCH /auth/change-passwordJWT 인증 필요 엔드포인트 추가Frontend
authApi.changePassword(): 새 API 메서드 추가Test plan
npm run build) 성공npm run test:run)Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트