Feat: [FN-67] 내 비밀번호 재설정 #13
Conversation
|
""" Walkthrough비밀번호 변경 기능이 추가되었습니다. 사용자 컨트롤러에 비밀번호 변경 엔드포인트가 도입되었고, 관련 서비스와 엔티티, 요청 모델, 저장소, 테스트 코드가 확장되었습니다. 토큰 버전 관리 로직이 추가되어 비밀번호 변경 시 토큰 버전이 증가하도록 구현되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UserController
participant UserService
participant AuthService
participant TokenVersionService
participant UserRepository
User->>UserController: PATCH /v1/users/me/password (ChangePasswordRequest)
UserController->>UserService: changePassword(userId, req)
UserService->>UserRepository: findActiveUserById(userId)
UserService->>AuthService: validatePasswordMatch(req.currentPassword, user.password)
AuthService-->>UserService: (성공/실패)
UserService->>User: changePassword(암호화된 newPassword)
UserService->>TokenVersionService: incrementTokenVersion(userId)
TokenVersionService->>UserRepository: incrementTokenVersion(userId)
TokenVersionService->>TokenVersionRedisRepository: delete(userId)
UserService-->>UserController: 완료(204 No Content)
UserController-->>User: 응답 반환
Estimated code review effort3 (~40 minutes) Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/project/flipnote/auth/service/AuthService.java (1)
88-92: 비밀번호 검증 메서드의 public 변경이 적절합니다.비밀번호 변경 기능에서 재사용하기 위해 public으로 변경한 것은 합리적입니다. passwordEncoder.matches()를 사용하여 안전하게 구현되었습니다.
보안상 더 나은 접근을 위해 별도의 패스워드 검증 서비스를 고려해볼 수 있지만, 현재 구현도 충분히 안전합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/project/flipnote/auth/service/AuthService.java(1 hunks)src/main/java/project/flipnote/auth/service/TokenVersionService.java(1 hunks)src/main/java/project/flipnote/user/controller/UserController.java(3 hunks)src/main/java/project/flipnote/user/entity/User.java(1 hunks)src/main/java/project/flipnote/user/model/ChangePasswordRequest.java(1 hunks)src/main/java/project/flipnote/user/repository/UserRepository.java(2 hunks)src/main/java/project/flipnote/user/service/UserService.java(3 hunks)src/test/java/project/flipnote/user/service/UserServiceTest.java(4 hunks)
🔇 Additional comments (12)
src/main/java/project/flipnote/user/entity/User.java (1)
102-104: 비밀번호 변경 메서드가 적절히 구현되었습니다.엔티티 메서드가 단순하게 필드만 업데이트하는 것은 좋은 설계입니다. 비밀번호 인코딩은 서비스 계층에서 처리되어야 합니다.
src/main/java/project/flipnote/auth/service/TokenVersionService.java (1)
29-32: 토큰 버전 증가 로직이 올바르게 구현되었습니다.데이터베이스 업데이트 후 Redis 캐시를 무효화하는 패턴이 적절합니다. 비밀번호 변경 시 기존 토큰들을 무효화하는 보안 요구사항을 잘 충족합니다.
src/main/java/project/flipnote/user/repository/UserRepository.java (2)
6-6: @Modifying 어노테이션 임포트가 적절합니다.JPQL 업데이트 쿼리에 필요한 어노테이션입니다.
26-28: 트랜잭션 컨텍스트 내 호출 확인 완료
토큰 버전 증가 쿼리가 효율적으로 구현되었습니다. 해당incrementTokenVersion메서드는UserService.changePassword에 적용된@Transactional(기본 readOnly 해제) 내에서 호출되어 트랜잭션 컨텍스트가 보장됩니다.
추가 조치가 필요 없으므로 이대로 머지해도 좋습니다.src/main/java/project/flipnote/user/controller/UserController.java (2)
8-8: 필요한 임포트가 적절히 추가되었습니다.@PatchMapping과 ChangePasswordRequest 임포트가 정확합니다.
Also applies to: 20-20
72-79: 비밀번호 변경 엔드포인트 검토 및 유효성 검증 확인 완료
비밀번호 변경 기능이 RESTful 원칙에 맞게 잘 구현되었습니다.
ChangePasswordRequest 모델의currentPassword,newPassword필드에@ValidPassword어노테이션이 적용되어 있어 입력값 유효성 검증이 정상적으로 동작합니다.추가 보안 강화를 위해 다음 사항을 고려해 보세요:
- 비밀번호 변경 요청에 대한 Rate Limiting 적용
- 성공적인 비밀번호 변경 시 사용자에게 알림 이메일 발송
- 비밀번호 변경 이력 및 시도 로그 기록
src/main/java/project/flipnote/user/model/ChangePasswordRequest.java (1)
5-12: 구현이 잘 되었습니다레코드 클래스를 DTO로 사용하는 것이 적절하며, 두 비밀번호 필드 모두에
@ValidPassword검증 어노테이션을 적용한 것이 보안상 좋습니다.src/main/java/project/flipnote/user/service/UserService.java (2)
12-13: 새로운 의존성 추가가 적절합니다비밀번호 변경 기능을 위해
AuthService와TokenVersionService를 추가한 것이 적절하며, 기존 코드 스타일과 일치합니다.Also applies to: 19-19, 36-37
101-110: 비밀번호 변경 로직이 보안적으로 잘 구현되었습니다구현이 우수합니다:
- 트랜잭션 범위 적용
- 현재 비밀번호 검증 후 변경 허용
- 새 비밀번호 암호화 저장
- 토큰 버전 증가로 기존 토큰 무효화
보안 모범 사례를 잘 따르고 있습니다.
src/test/java/project/flipnote/user/service/UserServiceTest.java (3)
18-18: 새로운 Mock 객체 추가가 적절합니다비밀번호 변경 기능 테스트를 위해
AuthService와TokenVersionServiceMock을 추가한 것이 적절합니다.Also applies to: 21-22, 29-29, 56-61
167-168: Spy 객체를 사용한 검증 방식이 개선되었습니다상태 검증 대신 행위 검증을 사용하는 것이 더 나은 테스트 방식입니다.
329-386: 비밀번호 변경 테스트가 포괄적으로 잘 작성되었습니다테스트 커버리지가 우수합니다:
- 성공 시나리오: 모든 상호작용 검증
- 사용자 없음: 예외 처리 및 부작용 없음 확인
- 잘못된 현재 비밀번호: 검증 실패 처리 및 부작용 없음 확인
각 테스트가 명확하고 적절한 Mock과 검증 패턴을 사용하고 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/project/flipnote/auth/service/EmailVerificationService.java (2)
16-20: 메소드명과 동작의 일관성을 확인해 주세요.
validateVerified메소드는 검증이 실패할 때 예외를 던지는 동작을 합니다. 메소드명이 boolean 반환을 암시할 수 있으므로,ensureVerified또는checkVerified와 같은 명명을 고려해보시기 바랍니다.- public void validateVerified(String email) { + public void ensureVerified(String email) {
17-18: 입력값 검증 추가를 고려해 주세요.
public void validateVerified(String email) { + if (email == null || email.trim().isEmpty()) { + throw new BizException(AuthErrorCode.INVALID_EMAIL); + } if (!emailVerificationRedisRepository.isVerified(email)) { throw new BizException(AuthErrorCode.UNVERIFIED_EMAIL); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/project/flipnote/auth/exception/AuthErrorCode.java(1 hunks)src/main/java/project/flipnote/auth/service/EmailVerificationService.java(1 hunks)src/main/java/project/flipnote/user/exception/UserErrorCode.java(1 hunks)src/main/java/project/flipnote/user/service/UserService.java(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/project/flipnote/auth/exception/AuthErrorCode.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/project/flipnote/user/service/UserService.java
🔇 Additional comments (2)
src/main/java/project/flipnote/auth/service/EmailVerificationService.java (1)
12-21: 이메일 검증 서비스 구현이 잘 설계되었습니다.단일 책임 원칙을 잘 따르고 있으며, 인증 도메인으로 로직을 분리한 것이 적절합니다. 코드가 간결하고 이해하기 쉽습니다.
src/main/java/project/flipnote/user/exception/UserErrorCode.java (1)
11-15: 도메인 분리 리팩토링이 적절합니다.
UNVERIFIED_EMAIL오류 코드를AuthErrorCode로 이동시킨 것이 도메인 책임 분리 관점에서 올바른 접근입니다. 남은 오류 코드들은 사용자 도메인에 적합합니다.
| DUPLICATE_EMAIL(HttpStatus.CONFLICT, "USER_001", "이미 사용 중인 이메일입니다."), | ||
| DUPLICATE_PHONE(HttpStatus.CONFLICT, "USER_002", "이미 사용 중인 휴대전화 번호입니다."), | ||
| USER_NOT_FOUND(HttpStatus.NOT_FOUND, "USER_003", "회원이 존재하지 않습니다."), | ||
| UNVERIFIED_EMAIL(HttpStatus.FORBIDDEN, "USER_004", "인증되지 않은 이메일입니다. 이메일 인증을 완료해 주세요."); | ||
|
|
||
| USER_NOT_FOUND(HttpStatus.NOT_FOUND, "USER_003", "회원이 존재하지 않습니다."); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
기존 코드에서 UNVERIFIED_EMAIL 사용 여부를 확인해 주세요.
UserErrorCode.UNVERIFIED_EMAIL을 사용하던 기존 코드가 모두 AuthErrorCode.UNVERIFIED_EMAIL로 업데이트되었는지 확인이 필요합니다.
다음 스크립트로 기존 사용처를 확인해 주세요:
🏁 Script executed:
#!/bin/bash
# Description: UserErrorCode.UNVERIFIED_EMAIL 사용처 확인
# UserErrorCode.UNVERIFIED_EMAIL 사용처 검색
rg -A 3 -B 3 "UserErrorCode\.UNVERIFIED_EMAIL"
# AuthErrorCode.UNVERIFIED_EMAIL로 변경된 부분 확인
rg -A 3 -B 3 "AuthErrorCode\.UNVERIFIED_EMAIL"Length of output: 1574
테스트 코드에서 UserErrorCode.UNVERIFIED_EMAIL 참조 수정 필요
UserServiceTest에서 여전히 UserErrorCode.UNVERIFIED_EMAIL을 사용하고 있으므로, AuthErrorCode.UNVERIFIED_EMAIL로 변경해 주세요.
- src/test/java/project/flipnote/user/service/UserServiceTest.java
- assertThat(exception.getErrorCode()).isEqualTo(UserErrorCode.UNVERIFIED_EMAIL); + assertThat(exception.getErrorCode()).isEqualTo(AuthErrorCode.UNVERIFIED_EMAIL);
- 필요한 경우 import 구문도 함께 수정하세요.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/test/java/project/flipnote/user/service/UserServiceTest.java, update all
references of UserErrorCode.UNVERIFIED_EMAIL to AuthErrorCode.UNVERIFIED_EMAIL
to reflect the correct error code usage. Also, modify the import statements
accordingly to import AuthErrorCode instead of UserErrorCode where necessary.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/project/flipnote/user/service/UserServiceTest.java(5 hunks)
🔇 Additional comments (6)
src/test/java/project/flipnote/user/service/UserServiceTest.java (6)
18-23: 새로운 서비스 계층 의존성 추가가 적절합니다.서비스 계층 추상화로의 리팩토링을 반영하여
AuthService,EmailVerificationService,TokenVersionService를 추가한 것이 좋습니다. 이는 테스트가 구현 세부사항보다는 비즈니스 로직에 집중할 수 있게 해줍니다.
30-32: 새로운 모델 임포트가 적절합니다.비밀번호 변경 기능을 위한
ChangePasswordRequest모델 임포트가 추가되었습니다.
57-64: 서비스 계층 Mock 객체 추가가 적절합니다.리팩토링된 아키텍처에 맞춰 서비스 계층의 Mock 객체들을 추가한 것이 좋습니다. 이는 테스트의 관심사 분리를 개선합니다.
146-151: 이메일 검증 테스트 로직 개선이 적절합니다.직접적인 repository mocking에서 service layer의 예외 처리로 변경된 것이 좋습니다. 또한 에러 코드가
AuthErrorCode.UNVERIFIED_EMAIL로 이동된 것도 적절한 도메인 분리를 반영합니다.
170-171: 회원 탈퇴 테스트 개선이 적절합니다.엔티티의
unregister()메서드 호출과 토큰 버전 서비스의incrementTokenVersion()호출을 검증하는 것이 좋습니다. 이는 비즈니스 로직의 정확한 실행을 보장합니다.
331-388: 비밀번호 변경 테스트 구현이 우수합니다.새로운
ChangePassword테스트 클래스가 다음과 같은 우수한 특징을 보여줍니다:
- 포괄적인 시나리오 커버리지: 성공 케이스와 두 가지 실패 케이스를 모두 포함
- 적절한 Mock 활용:
spy()를 사용하여 엔티티 메서드 호출 검증- 서비스 계층 테스트:
AuthService.validatePasswordMatch()검증- 부작용 방지 검증: 실패 시 불필요한 연산이 수행되지 않음을 확인
특히 라인 364-366과 385-386에서 실패 케이스에 대해 불필요한 연산이 수행되지 않음을 검증하는 것이 좋은 테스트 관행입니다.
|
|
||
| verify(passwordEncoder, never()).matches(anyString(), anyString()); | ||
| verify(passwordEncoder, never()).encode(anyString()); | ||
| verify(tokenVersionRedisRepository, never()).deleteTokenVersion(anyLong()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
토큰 버전 서비스 사용으로 업데이트가 필요합니다.
테스트에서 tokenVersionRedisRepository.deleteTokenVersion() 대신 tokenVersionService.incrementTokenVersion()을 검증해야 합니다. 현재 코드가 서비스 계층을 사용하도록 리팩토링되었기 때문입니다.
다음과 같이 수정하세요:
- verify(tokenVersionRedisRepository, never()).deleteTokenVersion(anyLong());
+ verify(tokenVersionService, never()).incrementTokenVersion(anyLong());Also applies to: 386-386
🤖 Prompt for AI Agents
In src/test/java/project/flipnote/user/service/UserServiceTest.java at lines 366
and 386, the test incorrectly verifies
tokenVersionRedisRepository.deleteTokenVersion() calls, but the code now uses
tokenVersionService.incrementTokenVersion(). Update the verify statements to
check that tokenVersionService.incrementTokenVersion() is called or not called
as appropriate, replacing all instances of deleteTokenVersion verification with
incrementTokenVersion verification to reflect the service layer refactor.
📝 변경 내용
✅ 체크리스트
💬 기타 참고 사항
Summary by CodeRabbit
신규 기능
/v1/users/me/password)버그 수정
테스트