[feature] 비밀번호 변경 시 아이디/이전 비밀번호 동일 여부 검증 추가하고 초기화 api 비인증 사용자에게 제공한다#728
Conversation
- 아이디와 동일한지, 이전 비밀번호와 동일한지 체크 - 각각 702-1, 702-2 ErrorCode 설정
- 로그인 여부와 상관없이 userId 만으로 초기화 하도록 수정 - 브라우저의 쿠키 제거 로직 삭제
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
Error codesbackend/src/main/java/moadong/global/exception/ErrorCode.java |
에러 코드 2종 추가: PASSWORD_SAME_AS_USERID, PASSWORD_SAME_AS_OLD. import com.google.api.Http; 불필요한 import 추가. |
User API Controllerbackend/src/main/java/moadong/user/controller/UserController.java |
/reset 엔드포인트 시그니처 변경: 인증 컨텍스트/쿠키 제거, @RequestBody @Validated UserResetRequest 수신으로 변경 및 서비스 반환값 바로 응답. 보안 어노테이션 제거. |
Request DTObackend/src/main/java/moadong/user/payload/request/UserResetRequest.java |
UserResetRequest 레코드 신설: @NotNull @UserId String userId. |
User service validationbackend/src/main/java/moadong/user/service/UserCommandService.java |
비밀번호 변경 시 검증 추가: 비밀번호==아이디 경우 PASSWORD_SAME_AS_USERID 예외, 비밀번호==기존 비밀번호 경우 PASSWORD_SAME_AS_OLD 예외 발생. 기존 흐름 이후에 암호화·저장 및 토큰 처리 유지. |
Response DTObackend/src/main/java/moadong/user/payload/response/TempPasswordResponse.java |
tempPassword 컴포넌트에서 @Password 검증 제거(현재는 @NotNull String tempPassword). 관련된 사용되지 않는 import 잔존 가능. |
Sequence Diagram(s)
sequenceDiagram
autonumber
actor Client
participant Controller as UserController
participant Service as UserCommandService
rect rgb(245,248,255)
Note over Client,Controller: 비밀번호 초기화 (변경된 흐름)
Client->>Controller: POST /users/reset { userId }
Controller->>Service: reset(userId)
Service-->>Controller: TempPasswordResponse
Controller-->>Client: 200 TempPasswordResponse
end
sequenceDiagram
autonumber
actor Client
participant Controller as UserController
participant Service as UserCommandService
participant Repo as UserRepository
rect rgb(245,255,245)
Note over Client,Service: 비밀번호 변경 검증 추가
Client->>Controller: PATCH /users/me/password { newPassword }
Controller->>Service: update(userId, newPassword, ...)
alt newPassword == userId
Service-->>Controller: throw PASSWORD_SAME_AS_USERID
Controller-->>Client: 400 702-1
else newPassword == oldPassword
Service-->>Controller: throw PASSWORD_SAME_AS_OLD
Controller-->>Client: 400 702-2
else 변경 진행
Service->>Repo: save(encryptedPassword)
Repo-->>Service: OK
Service-->>Controller: UpdateResponse
Controller-->>Client: 200 UpdateResponse
end
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| 비밀번호 변경 시 동일 비밀번호 방지(아이디/이전 비밀번호) MOA-220 | ✅ | |
| 비밀번호 초기화 API 개선(요청 본문 기반으로 수정) MOA-220 | ✅ |
Assessment against linked issues: Out-of-scope changes
| Code Change | Explanation |
|---|---|
불필요한 import 추가: import com.google.api.Http; (backend/src/main/java/moadong/global/exception/ErrorCode.java) |
사용되지 않는 import로, MOA-220 요구사항과 무관함. |
@Password 검증 제거: TempPasswordResponse.tempPassword (backend/src/main/java/moadong/user/payload/response/TempPasswordResponse.java) |
linked issue에서 명시되지 않은 응답 DTO의 검증 주석 제거로 의도된 범위인지 불분명함. |
Possibly related issues
- 비밀번호 변경 시 동일 비밀번호를 방지하고 초기화 api를 개선한다 #727: 비밀번호 동일성 검증 및 초기화 API 변경과 내용이 일치함.
- [feature] MOA-214 관리자 계정 비밀번호 변경 및 초기화한다 #719: 사용자 비밀번호 변경/초기화 관련 요구사항과 영역이 겹침.
Possibly related PRs
- [feature] 상시 모집 상태 추가 및 모집 정보 변경 시 반영되도록 변경 #485:
ErrorCodeenum에 상수 추가 관련 변경을 포함함. - [feature] 관리자 계정의 비밀번호 변경 및 초기화한다 #722: UserController.reset 및 비밀번호 처리 로직 관련 변경을 다룸.
Suggested labels
✨ Feature
Suggested reviewers
- lepitaaar
- Zepelown
- PororoAndFriends
📜 Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backend/src/main/java/moadong/user/payload/response/TempPasswordResponse.java(0 hunks)
💤 Files with no reviewable changes (1)
- backend/src/main/java/moadong/user/payload/response/TempPasswordResponse.java
✨ Finishing Touches
- 📝 Generate Docstrings
🧪 Generate unit tests
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feature/#727-password-enhancement-MOA-220
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 help to get the list of available commands and usage tips.
Test Results80 tests 77 ✅ 7s ⏱️ Results for commit 99505db. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/moadong/user/service/UserCommandService.java (1)
131-134: 치명적: Refresh 토큰 DB 미갱신으로 재발급 검증 실패
UserCommandService.java:131–134에서 쿠키만 설정하고user.refreshToken을 갱신하지 않아 이후 재발급 검증이 불가능합니다.user.updateRefreshToken(...)호출 후userRepository.save(user)를 수행한 뒤 쿠키를 설정해야 합니다.@@ -131,4 +131,8 @@ - userRepository.save(user); - - String newRefreshToken = jwtProvider.generateRefreshToken(user.getUsername()).getToken(); - ResponseCookie cookie = cookieMaker.makeRefreshTokenCookie(newRefreshToken); - response.addHeader("Set-Cookie", cookie.toString()); + String newRefreshToken = jwtProvider.generateRefreshToken(user.getUserId()).getToken(); + user.updateRefreshToken(new RefreshToken(newRefreshToken, new Date())); + userRepository.save(user); + + ResponseCookie cookie = cookieMaker.makeRefreshTokenCookie(newRefreshToken); + response.addHeader("Set-Cookie", cookie.toString());
🧹 Nitpick comments (3)
backend/src/main/java/moadong/global/exception/ErrorCode.java (1)
3-3: 불필요한 import 제거
import com.google.api.Http;는 사용되지 않습니다. 정리하지 않으면 정적 분석/포매터에서 경고됩니다.-import com.google.api.Http;backend/src/main/java/moadong/user/service/UserCommandService.java (1)
117-126: 아이디 비교는 영속 엔티티 기준으로, 입력값 유효성 확인 권장
userId.equals(...)대신user.getUserId()로 비교해 신뢰 가능한 소스 사용 권장.- DTO에서
password가 null/blank가 아닌지 검증되어 있는지 확인 필요.- if (userId.equals(userUpdateRequest.password())) { + if (user.getUserId().equals(userUpdateRequest.password())) { throw new RestApiException(ErrorCode.PASSWORD_SAME_AS_USERID); }backend/src/main/java/moadong/user/payload/request/UserResetRequest.java (1)
6-11: userId 공백 문자열 방지용 검증 보강 제안
@NotNull만으로는 빈 문자열을 막지 못합니다.@NotBlank(또는@Size) 추가 권장.-import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.NotBlank; @@ - @NotNull + @NotBlank @UserId String userId
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
backend/src/main/java/moadong/global/exception/ErrorCode.java(2 hunks)backend/src/main/java/moadong/user/controller/UserController.java(2 hunks)backend/src/main/java/moadong/user/payload/request/UserResetRequest.java(1 hunks)backend/src/main/java/moadong/user/service/UserCommandService.java(1 hunks)
🔇 Additional comments (1)
backend/src/main/java/moadong/global/exception/ErrorCode.java (1)
38-40: 비밀번호 검증용 에러 코드 추가 적절메시지와 상태코드 선택이 목적에 부합합니다. 클라이언트/문서에 신규 코드(
702-1,702-2) 반영만 확인해주세요.
| public ResponseEntity<?> reset(@RequestBody @Validated UserResetRequest userResetRequest) { | ||
| TempPasswordResponse tempPwdResponse = userCommandService.reset(userResetRequest.userId()); | ||
| return Response.ok(tempPwdResponse); |
There was a problem hiding this comment.
치명적: 비인증 사용자의 비밀번호 초기화 결과(임시 비밀번호)를 응답 본문으로 반환
누구나 타인의 userId로 임시 비밀번호를 받아 계정 탈취가 가능합니다. 최소한 응답에 임시 비밀번호를 포함하지 말고, 검증된 채널(등록된 이메일/SMS)로만 전달해야 합니다. 또한 다음 보강을 권장합니다:
- 사용자 확인(이메일 토큰/OTP), CAPTCHA, IP/계정 단위 rate limiting 및 감사 로깅
- 응답은 일반 메시지(202/200)로 한정, 재시도 쿨다운
- 여러 기기 세션 무효화(이미
updateRefreshToken(null)사용 중이나, 초기화 시에도 재검토)
- public ResponseEntity<?> reset(@RequestBody @Validated UserResetRequest userResetRequest) {
- TempPasswordResponse tempPwdResponse = userCommandService.reset(userResetRequest.userId());
- return Response.ok(tempPwdResponse);
- }
+ public ResponseEntity<?> reset(@RequestBody @Validated UserResetRequest userResetRequest) {
+ // 서비스는 임시 비밀번호를 외부 채널로 전송하도록 변경하고(추가 작업 필요),
+ // API 응답에는 비밀번호를 포함하지 않습니다.
+ userCommandService.reset(userResetRequest.userId());
+ return Response.ok("비밀번호 초기화 요청이 접수되었습니다.");
+ }📝 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.
| public ResponseEntity<?> reset(@RequestBody @Validated UserResetRequest userResetRequest) { | |
| TempPasswordResponse tempPwdResponse = userCommandService.reset(userResetRequest.userId()); | |
| return Response.ok(tempPwdResponse); | |
| public ResponseEntity<?> reset(@RequestBody @Validated UserResetRequest userResetRequest) { | |
| // 서비스는 임시 비밀번호를 외부 채널로 전송하도록 변경하고(추가 작업 필요), | |
| // API 응답에는 비밀번호를 포함하지 않습니다. | |
| userCommandService.reset(userResetRequest.userId()); | |
| return Response.ok("비밀번호 초기화 요청이 접수되었습니다."); | |
| } |
🤖 Prompt for AI Agents
In backend/src/main/java/moadong/user/controller/UserController.java around
lines 99-101, do not return the temporary password in the HTTP response body;
instead change the endpoint to perform server-side verification (e.g., require
email token/OTP or CAPTCHA before reset), trigger delivery of the temporary
password only via a verified channel (registered email/SMS) and log the action
for audit, apply IP/account rate-limiting and retry cooldown, invalidate all
sessions/refresh tokens as part of the reset, and finally respond with a generic
success status/message (202 or 200) without any secret material in the response
body.
Zepelown
left a comment
Zepelown
left a comment
There was a problem hiding this comment.
비밀번호 검증에 @ Password과 같은 비밀번호 형식 검증도 있으면 좋을 거 같아요.
아마 global annotation에 있을 겁니다.
이를 이용해서 dto에서 검증을 하면 될 것 같아요.
아니면 서비스에서 Password Validator 클래스를 이용해도 될 겁니다.
그 UserUpdateRequest에서 @password로 검증하고 있다고 판단해서 따로 추가하진 않았습니다. 이걸론 검증이 부족할까요? |
Zepelown
left a comment
Zepelown
left a comment
There was a problem hiding this comment.
비밀번호 검증 로직이 없는 줄 알았네요.
죄송합니다.
고생하셨어요!
3 participants
#️⃣연관된 이슈
📝작업 내용
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit