feat: 비밀번호 변경 API 구현 #448
feat: 비밀번호 변경 API 구현 #448
Conversation
whqtker
requested review from
Gyuhyeok99,
lsy1307,
nayonsoso and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 2
🧹 Nitpick comments (5)
src/main/java/com/example/solidconnection/siteuser/controller/MyPageController.java (1)
44-51: 3) PATCH /my/password 응답 상태코드: 204 No Content 권장.
- 바디 없는 성공 응답은 204가 더 적합합니다. 200도 문제는 없으나, REST 관점에서 204가 의도를 더 잘 드러냅니다.
아래처럼 변경을 제안드립니다.
@PatchMapping("/password") public ResponseEntity<Void> updatePassword( @AuthorizedUser long siteUserId, @RequestBody @Valid PasswordUpdateRequest request ) { myPageService.updatePassword(siteUserId, request); - return ResponseEntity.ok().build(); + return ResponseEntity.noContent().build(); }추가 제안(선택):
- 컨트롤러 레벨 통합 테스트로 DTO 검증 실패 시 400을 반환하는 케이스를 한 건 정도 보강하면, 엔드포인트 품질을 더 확실히 보장할 수 있습니다. 원하시면 테스트 스켈레톤을 제공하겠습니다.
src/main/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmation.java (1)
1-20: 4) 커스텀 검증 어노테이션에 @documented 추가를 권장합니다.
- Javadoc 등 문서화 시 어노테이션 정보가 노출되어 유지보수에 유리합니다.
아래처럼 @documented를 추가해 주세요.
package com.example.solidconnection.siteuser.dto.validation; import jakarta.validation.Constraint; import jakarta.validation.Payload; import java.lang.annotation.ElementType; +import java.lang.annotation.Documented; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; @Constraint(validatedBy = PasswordConfirmationValidator.class) @Target({ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) +@Documented public @interface PasswordConfirmation {src/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java (1)
12-13: TODO 주석 처리 확인 필요#435 PR이 머지되면 @password 어노테이션을 추가하기로 되어 있습니다. 해당 PR의 상태를 확인하고 적절한 시점에 추가해 주세요.
src/test/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidatorTest.java (2)
44-57: 테스트 메서드명 오타 수정 필요메서드명에 '새로운'이 '새로은'으로 잘못 입력되어 있습니다.
- void 새로은_비밀번호와_확인_비밀번호가_일치하지_않으면_검증에_실패한다() { + void 새로운_비밀번호와_확인_비밀번호가_일치하지_않으면_검증에_실패한다() {
29-39: @notblank 검증도 추가 테스트 권장현재는 교차 필드 검증만 테스트하고 있는데, 각 필드의 @notblank 검증도 테스트하면 더 완전한 테스트 커버리지를 확보할 수 있습니다.
@notblank 검증을 위한 추가 테스트 케이스를 생성해 드릴까요?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/controller/MyPageController.java(2 hunks)src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmation.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidator.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java(4 hunks)src/test/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidatorTest.java(1 hunks)src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/com/example/solidconnection/siteuser/controller/MyPageController.java (2)
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (1)
RequiredArgsConstructor(25-111)src/main/java/com/example/solidconnection/mentor/controller/MentorMyPageController.java (1)
RequiredArgsConstructor(18-43)
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (2)
src/main/java/com/example/solidconnection/siteuser/controller/MyPageController.java (1)
RequiredArgsConstructor(19-52)src/main/java/com/example/solidconnection/auth/service/AuthService.java (1)
RequiredArgsConstructor(17-74)
src/test/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidatorTest.java (1)
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (3)
Nested(98-145)Nested(147-184)Nested(186-225)
🔇 Additional comments (10)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
59-61: 2) 에러 코드 추가 배치와 상태코드가 적절합니다.
- PASSWORD_MISMATCH, PASSWORD_NOT_CHANGED, PASSWORD_NOT_CONFIRMED 모두 400(BAD_REQUEST)로 합리적입니다.
- AUTH 섹션 내 배치도 일관성을 유지합니다.
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java (2)
4-4: 5) 테스트 의존성(import) 추가는 적절합니다.
- PASSWORD_MISMATCH, assertThatThrownBy, assertAll, PasswordUpdateRequest, PasswordEncoder 모두 본 테스트에 필요한 보강입니다. 잘 추가되었습니다.
Also applies to: 9-10, 25-25, 41-41
68-70: 6) PasswordEncoder 주입은 좋은 선택입니다.
- 인코딩/매칭을 실제 구현과 동일하게 검증할 수 있어 통합 테스트의 신뢰도를 높입니다.
src/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java (1)
6-17: 전체적으로 잘 구현되었습니다!DTO 레벨의 검증이 적절하게 구성되어 있고, 각 필드별 검증 메시지도 명확합니다. @PasswordConfirmation을 통한 교차 필드 검증도 좋은 접근입니다.
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (3)
95-104: 비밀번호 변경 로직이 명확하게 구현되었습니다트랜잭션 처리와 검증 로직이 적절하게 분리되어 있고, 도메인 메서드를 활용한 비밀번호 업데이트도 좋은 접근입니다.
100-101: 서비스 레이어 검증 로직 확인현재 비밀번호 검증은 서비스 레이어에서 처리하고 있는데, 이는 적절한 접근입니다. DB에 저장된 암호화된 비밀번호와 비교해야 하므로 DTO 레벨에서는 처리할 수 없는 검증입니다.
107-107: passwordEncoder.matches() 메서드의 인자 순서 확인 필요
passwordEncoder.matches(rawPassword, encodedPassword)형식이 일반적인데, 현재 코드는matches(userPassword, currentPassword)로 되어 있습니다. 첫 번째 인자가 평문 비밀번호, 두 번째 인자가 인코딩된 비밀번호여야 합니다.- private void validateCurrentPasswordSame(String userPassword, String currentPassword) { - if (!passwordEncoder.matches(userPassword, currentPassword)) { + private void validateCurrentPasswordSame(String userPassword, String currentPassword) { + if (!passwordEncoder.matches(currentPassword, userPassword)) {Likely an incorrect or invalid review comment.
src/main/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidator.java (2)
11-30: 교차 필드 검증이 깔끔하게 구현되었습니다검증 로직이 명확하고, 각 케이스별로 적절한 프로퍼티에 에러 메시지를 바인딩하는 구조가 좋습니다. 헬퍼 메서드로 로직을 분리한 것도 가독성 측면에서 우수합니다.
17-21: 조기 반환 패턴 활용이 좋습니다검증 실패 시 즉시 false를 반환하는 구조로 불필요한 검증을 피하고 있습니다. 효율적인 접근입니다.
src/test/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidatorTest.java (1)
19-74: 테스트 케이스가 충실하게 작성되었습니다
- 유효한 비밀번호 변경 케이스
- 새 비밀번호와 확인 비밀번호 불일치 케이스
- 현재 비밀번호와 새 비밀번호 동일 케이스
모든 중요한 시나리오를 커버하고 있고, 에러 메시지와 프로퍼티 경로까지 검증하는 것이 좋습니다.
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java
Show resolved
Hide resolved
Gyuhyeok99
left a comment
Gyuhyeok99
left a comment
There was a problem hiding this comment.
고생하셨습니다! 깔끔하게 잘 작성해주셔서 잘 읽혔습니다!
몇 가지 의견만 남겨놓았어요!!
@password 관련 pr도 머지되었으니 주석해제해주시면 될 거 같습니다!
| if (!passwordEncoder.matches(userPassword, currentPassword)) { | ||
| throw new CustomException(PASSWORD_MISMATCH); | ||
| } | ||
| } |
There was a problem hiding this comment.
passwordEncoder.matches를 들어가보면 파라미터 순서가
(rawPassword(평문), encodedPassword(암호화))
입니다!
순서를 변경하는 게 좋을 거 같습니다~
validateCurrentPasswordSame의 인자 이름을 rawPassword, encodedPassword로 맞춰도 좋을 거 같아요!(이건 선택사항으로 해도 될 거 같아요!)
사실 동작을 할까?란 의문을 품고 bruno로 호출해봤는데 지금 상태에서도 작동이 되긴 합니다!
| // then | ||
| assertAll( | ||
| () -> assertThat(violations) | ||
| .isNotEmpty() | ||
| .extracting("message") | ||
| .contains(PASSWORD_NOT_CONFIRMED.getMessage()), | ||
| () -> assertThat(violations) | ||
| .extracting(v -> v.getPropertyPath().toString()) | ||
| .containsExactly("confirmNewPassword") | ||
| ); | ||
| } |
There was a problem hiding this comment.
"message"는 따로 상수로 빼도 좋겠네요!
추가로 저는 개인적으로
assertThat(violations)
.isNotEmpty()
.extracting("message")
.contains(PASSWORD_NOT_CONFIRMED.getMessage()
만 있어도 충분히 검증이 되는 거 같다는 생각이긴 한데 어떻게 생각하시나요?
There was a problem hiding this comment.
validator 역할을 테스트하는 것에 초점을 맞추자면 제거해도 무방할 것 같습니다 !
message도 상수로 분리하였습니다 ~
src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java
Show resolved
Hide resolved
nayonsoso
left a comment
nayonsoso
left a comment
There was a problem hiding this comment.
빠른 머지를 위헤 우선 approve 드립니다~ 사소한 부분 코멘트 남겨뒀습니다 🙇🏻♀️
| String newPassword, | ||
|
|
||
| @NotBlank(message = "새 비밀번호를 다시 한번 입력해주세요.") | ||
| String confirmNewPassword |
There was a problem hiding this comment.
정말 아주 사소한 부분이긴 한데요..! 😅
confirmNewPassword를 그대로 해석하자면 "새로운 비밀번호를 확인한다"라는 동사구이므로, 필드의 이름으로는 적합하지 않은 것 같아요.
- 필드, 인자명은 명사구
- 함수명은 동사구
로 알고 있습니다!
그래서 이 필드 이름을 newPasswordConfirmation 으로 하는게 어떨지 조심스럽게 제안드립니다..
There was a problem hiding this comment.
앗 ... 변수명 고민하다가 동사형으로 써 버렸네요
제안하신 변수명 말고 좋은 이름은 없는 것 같아 그대로 사용하였습니다 ~!!
| if (!passwordEncoder.matches(userPassword, currentPassword)) { | ||
| throw new CustomException(PASSWORD_MISMATCH); | ||
| } | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (1)
106-110: 2) 메서드/파라미터 네이밍을 의도에 맞게 명확화하면 가독성이 상승합니다
- 메서드명은 validateCurrentPasswordMatches처럼 “무엇을 검증하는지” 드러나면 좋습니다.
- 파라미터명도 rawPassword, encodedPassword로 맞추면 오용 가능성을 더 낮출 수 있습니다.
적용 diff:
- private void validatePasswordMatch(String currentPassword, String userPassword) { - if (!passwordEncoder.matches(currentPassword, userPassword)) { + private void validateCurrentPasswordMatches(String rawPassword, String encodedPassword) { + if (!passwordEncoder.matches(rawPassword, encodedPassword)) { throw new CustomException(PASSWORD_MISMATCH); } }호출부 정합성 맞추기:
- validatePasswordMatch(request.currentPassword(), user.getPassword()); + validateCurrentPasswordMatches(request.currentPassword(), user.getPassword());
🧹 Nitpick comments (1)
src/test/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidatorTest.java (1)
25-29: 2) ValidatorFactory 재사용 및 자원 정리를 권장합니다
- 테스트마다 ValidatorFactory를 생성하고 폐기하지 않으면 불필요한 리소스 점유가 발생할 수 있습니다.
- 필드로 승격해 @AfterEach에서 close하면 깔끔합니다.
적용 diff:
- @BeforeEach - void setUp() { - ValidatorFactory factory = Validation.buildDefaultValidatorFactory(); - validator = factory.getValidator(); - } + private ValidatorFactory factory; + + @BeforeEach + void setUp() { + factory = Validation.buildDefaultValidatorFactory(); + validator = factory.getValidator(); + }추가: @AfterEach 메서드와 import
@@ -import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.AfterEach; @@ void setUp() { factory = Validation.buildDefaultValidatorFactory(); validator = factory.getValidator(); } + + @AfterEach + void tearDown() { + if (factory != null) { + factory.close(); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidator.java(1 hunks)src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java(4 hunks)src/test/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidatorTest.java(1 hunks)src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java
- src/main/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidator.java
- src/test/java/com/example/solidconnection/siteuser/service/MyPageServiceTest.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#448
File: src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java:118-121
Timestamp: 2025-08-12T21:43:45.717Z
Learning: SiteUser 도메인의 updatePassword 메서드에서는 파라미터 이름을 newEncodedPassword로 하거나 Javadoc을 추가해서 인코딩된 비밀번호가 들어와야 한다는 것을 명시해야 합니다.
📚 Learning: 2025-08-12T21:43:45.717Z
Learnt from: nayonsoso
PR: solid-connection/solid-connect-server#448
File: src/main/java/com/example/solidconnection/siteuser/domain/SiteUser.java:118-121
Timestamp: 2025-08-12T21:43:45.717Z
Learning: SiteUser 도메인의 updatePassword 메서드에서는 파라미터 이름을 newEncodedPassword로 하거나 Javadoc을 추가해서 인코딩된 비밀번호가 들어와야 한다는 것을 명시해야 합니다.
Applied to files:
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.javasrc/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java
🧬 Code Graph Analysis (1)
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (1)
src/main/java/com/example/solidconnection/siteuser/controller/MyPageController.java (1)
RequiredArgsConstructor(19-52)
🪛 GitHub Actions: CI with Gradle
src/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java
[error] 12-12: Cannot find symbol: Password annotation used in PasswordUpdateRequest. Ensure the annotation is defined and imported.
🔇 Additional comments (4)
src/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java (1)
15-16: 3) 네이밍 합의 반영 잘 되었습니다: newPasswordConfirmation 가독성 양호
- 과거 논의된 내용대로 명사구(newPasswordConfirmation)로 정리되어 일관성이 좋아졌습니다.
src/main/java/com/example/solidconnection/siteuser/service/MyPageService.java (2)
100-104: 1) PasswordEncoder.matches 인자 순서 적합합니다
- matches(rawPassword, encodedPassword) 순서가 정확합니다.
- 기존 비밀번호 검증 후 인코딩된 새 비밀번호를 저장하는 흐름도 적절합니다.
103-104: 도메인 메서드 파라미터명 확인 완료
- 검증 결과
- SiteUser.updatePassword의 파라미터명이
newEncodedPassword로 이미 인코딩된 비밀번호임을 명시하고 있어 추가적인 변경이 필요하지 않습니다.src/test/java/com/example/solidconnection/siteuser/dto/validation/PasswordConfirmationValidatorTest.java (1)
31-41: 1) 정상 플로우 검증이 명확합니다
- 유효한 요청에서 violations가 비어 있음을 확인해, 교차 필드 검증의 성공 케이스를 잘 커버합니다.
src/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/siteuser/dto/PasswordUpdateRequest.java
Show resolved
Hide resolved
- 비밀번호는 영문, 숫자, 특수문자를 포함한 8자리 이상
…tker/local-solid-connect-server into feat/446-change-password-api
3 participants
관련 이슈
작업 내용
비밀번호 변경 API를 구현하였습니다.
피그마와 기획 상으론 결정되지 않은 것 같지만, 일반적인 비밀번호 변경 플로우에서 받는 값이 '기존 비밀번호', '새 비밀번호', '새 비밀번호 확인(재확인 비밀번호)' 이기에, 일단 이렇게 값을 받도록 구현하였습니다.
#435 PR이 머지되면 DTO의 새 비밀번호 필드에
@Password어노테이션 추가하겠습니다. ddf8c9b크게 DTO, 서비스 계층에서 값 검증을 진행하였습니다.
@NotBlank, 기존 비밀번호가 새 비밀번호와 다른지, 새 비밀번호가 재확인 비밀번호와 같은지기존 비밀번호가 새 비밀번호와 다른지, 새 비밀번호가 재확인 비밀번호와 같은지 는 컴팩트 생성자를 사용할까 하다 DTO 본연의 역할에 집중하는 것이 좋다고 생각하여 커스텀 어노테이션을 작성하여 구현하였습니다.
해당 PR이 머지되면 bruno 작성하도록 하겠습니다.
특이 사항
리뷰 요구사항 (선택)