Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이 변경사항은 사용자 비밀번호 업데이트 기능을 추가하는 작업입니다. API 계층(UserFacade, UserController, UserControllerImpl)에서는 비밀번호 업데이트를 위한 새로운 메서드와 엔드포인트가 구현되었으며, 요청 데이터를 캡슐화하는 Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Test Coverage Report
Files
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ @@ Coverage Diff @@
## develop #202 +/- ##
=============================================
+ Coverage 92.99% 93.73% +0.74%
- Complexity 129 136 +7
=============================================
Files 48 48
Lines 371 383 +12
Branches 4 4
=============================================
+ Hits 345 359 +14
+ Misses 21 20 -1
+ Partials 5 4 -1
Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
aics-api/src/main/java/kgu/developers/api/user/presentation/request/UserPasswordUpdateRequest.java (2)
14-15: 정규식 패턴의 보안성 강화 필요현재 비밀번호 정규식이 기본적인 요구사항은 충족하지만, 보안을 더욱 강화하기 위해 다음 사항들을 고려해보시기 바랍니다:
- 대문자 포함 필수
- 특수문자 최소 개수 지정
- 연속된 문자 제한
- regexp = "^(?=.*[A-Za-z])(?=.*\\d)(?=.*[!@#$%^&*(),.?\":{}|<>])[A-Za-z\\d!@#$%^&*(),.?\":{}|<>]{8,15}$", + regexp = "^(?=.*[A-Z])(?=.*[a-z])(?=.*\\d)(?=.*[!@#$%^&*(),.?\":{}|<>]){2,}(?!.*(.)\1{2})[A-Za-z\\d!@#$%^&*(),.?\":{}|<>]{8,15}$", - message = "비밀번호는 영문, 숫자, 특수문자를 포함하여 8~15자리여야 합니다." + message = "비밀번호는 대소문자, 숫자, 특수문자(최소 2개)를 포함하여 8~15자리여야 하며, 연속된 문자는 사용할 수 없습니다."
20-26: 중복 정규식 패턴 리팩토링 필요originalPassword와 newPassword에 동일한 정규식 패턴이 중복되어 있습니다. 상수로 추출하여 재사용하는 것이 좋습니다.
+ private static final String PASSWORD_PATTERN = "^(?=.*[A-Za-z])(?=.*\\d)(?=.*[!@#$%^&*(),.?\":{}|<>])[A-Za-z\\d!@#$%^&*(),.?\":{}|<>]{8,15}$"; + private static final String PASSWORD_MESSAGE = "비밀번호는 영문, 숫자, 특수문자를 포함하여 8~15자리여야 합니다."; @Pattern( - regexp = "^(?=.*[A-Za-z])(?=.*\\d)(?=.*[!@#$%^&*(),.?\":{}|<>])[A-Za-z\\d!@#$%^&*(),.?\":{}|<>]{8,15}$", - message = "비밀번호는 영문, 숫자, 특수문자를 포함하여 8~15자리여야 합니다." + regexp = PASSWORD_PATTERN, + message = PASSWORD_MESSAGE )aics-api/src/main/java/kgu/developers/api/user/application/UserFacade.java (1)
40-43: 트랜잭션 격리 수준 검토 필요비밀번호 변경과 같은 중요한 작업에는 적절한 트랜잭션 격리 수준이 필요합니다.
+ @Transactional(isolation = Isolation.REPEATABLE_READ) public void updatePassword(UserPasswordUpdateRequest request) { User user = userQueryService.me(); userCommandService.updatePassword(user, request.originalPassword(), request.newPassword()); }aics-api/src/main/java/kgu/developers/api/user/presentation/UserControllerImpl.java (1)
53-59: API 응답 및 문서화 개선 필요비밀번호 변경 API에 대한 개선사항:
- Swagger 문서화 누락
- 성공/실패 시나리오에 대한 응답 코드 명시 필요
+ @Operation(summary = "비밀번호 변경", description = "사용자의 비밀번호를 변경합니다.") + @ApiResponses(value = { + @ApiResponse(responseCode = "204", description = "비밀번호 변경 성공"), + @ApiResponse(responseCode = "400", description = "잘못된 비밀번호 형식"), + @ApiResponse(responseCode = "401", description = "인증 실패") + }) @Override @PatchMapping("/password") public ResponseEntity<Void> updatePassword( @Valid @RequestBody UserPasswordUpdateRequest request ) { userFacade.updatePassword(request); return ResponseEntity.noContent().build(); }aics-api/src/testFixtures/java/user/application/UserFacadeTest.java (1)
110-121: 비밀번호 업데이트 성공 테스트에 검증 로직을 보완하면 좋겠습니다.현재 테스트는 예외가 발생하지 않는 것만 확인하고 있습니다. 비밀번호가 실제로 변경되었는지 검증하는 로직을 추가하면 더 완성도 높은 테스트가 될 것 같습니다.
다음과 같이 검증 로직을 추가해보세요:
@Test @DisplayName("updatePassword는 주어진 형식으로 변경하는 경우 예외를 던지지 않는다") public void updatePassword_Success() { // given UserPasswordUpdateRequest request = UserPasswordUpdateRequest.builder() .originalPassword("password1234") .newPassword("newpass1234") .build(); + // when assertDoesNotThrow(() -> userFacade.updatePassword(request)); + + // then + // 변경된 비밀번호로 로그인이 가능한지 확인 + assertDoesNotThrow(() -> userFacade.login( + new UserLoginRequest("202411345", "newpass1234") + )); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
aics-api/src/main/java/kgu/developers/api/user/application/UserFacade.java(2 hunks)aics-api/src/main/java/kgu/developers/api/user/presentation/UserController.java(2 hunks)aics-api/src/main/java/kgu/developers/api/user/presentation/UserControllerImpl.java(3 hunks)aics-api/src/main/java/kgu/developers/api/user/presentation/request/UserPasswordUpdateRequest.java(1 hunks)aics-api/src/testFixtures/java/user/application/UserFacadeTest.java(3 hunks)aics-domain/src/main/java/kgu/developers/domain/user/application/command/UserCommandService.java(1 hunks)aics-domain/src/main/java/kgu/developers/domain/user/domain/User.java(1 hunks)aics-domain/src/testFixtures/java/user/application/UserCommandServiceTest.java(2 hunks)aics-domain/src/testFixtures/java/user/domain/UserDomainTest.java(1 hunks)
🔇 Additional comments (3)
aics-api/src/main/java/kgu/developers/api/user/presentation/UserController.java (1)
58-69: 비밀번호 업데이트 API 구현이 적절합니다.API 문서화와 입력 유효성 검증이 잘 구현되어 있습니다. 204 응답 코드는 비밀번호 업데이트 작업에 적합합니다.
aics-api/src/testFixtures/java/user/application/UserFacadeTest.java (2)
5-5: 비밀번호 관련 설정이 적절히 구현되었습니다!비밀번호 암호화를 위한
BCryptPasswordEncoder설정과 관련 import문이 잘 추가되었으며, 테스트 데이터의 비밀번호도 올바르게 인코딩되어 있습니다.Also applies to: 12-12, 21-21, 35-35, 43-43
123-136: 예외 처리 테스트가 잘 구현되었습니다!잘못된 원본 비밀번호를 입력했을 때
InvalidPasswordException이 발생하는지 적절히 검증하고 있습니다. 테스트 구조와 검증 방식이 명확합니다.
...-domain/src/main/java/kgu/developers/domain/user/application/command/UserCommandService.java
Show resolved
Hide resolved
aics-domain/src/testFixtures/java/user/application/UserCommandServiceTest.java
Show resolved
Hide resolved
aics-domain/src/main/java/kgu/developers/domain/user/domain/User.java
Outdated
Show resolved
Hide resolved
LeeHanEum
left a comment
There was a problem hiding this comment.
빠른 작업 대단히 감사합니다.
몇가지 코멘트만 확인 부탁드려요 👍
Summary
회원 비밀번호 변경 기능 구현
Tasks