[FEAT] 비밀번호 찾기 기능 구현#110
Merged
Hidden character warning
The head ref may contain hidden characters: "feat/#109/\ube44\ubc00\ubc88\ud638-\ucc3e\uae30-api-\uae30\ub2a5-\uad6c\ud604"
Merged
Conversation
|
Caution Review failedPull request was closed or merged during review Walkthrough비밀번호 재설정(발송 → 코드 검증 → 재설정) 엔드투엔드 흐름과 이메일 인증 상태 저장/확인 기능을 추가하며, Redis 저장소 구현, API 스펙·컨트롤러, DTO·검증, 관련 설정(CI·애플리케이션) 및 달력 이벤트 모델 확장을 포함합니다. 변경사항비밀번호 재설정 기능
달력 이벤트 확장
Sequence Diagram(s)sequenceDiagram
participant Client
participant SendPWResetCtrl as SendPasswordResetEmailController
participant SendPWResetUC as SendPasswordResetEmailUseCase
participant EmailSvc as EmailVerificationService
participant PWCodeRepo as PasswordResetCodeRepository
participant VerifyCtrl as VerifyPasswordResetCodeController
participant VerifyUC as VerifyPasswordResetCodeUseCase
participant PWVerifiedRepo as PasswordResetVerifiedRepository
participant ResetCtrl as ResetPasswordController
participant ResetUC as ResetPasswordUseCase
participant UserSvc as UserService
Client->>SendPWResetCtrl: POST /api/email/password-reset/send (email)
SendPWResetCtrl->>SendPWResetUC: send(email)
SendPWResetUC->>EmailSvc: sendCode(email, code)
EmailSvc->>PWCodeRepo: save(email, code) [TTL=10m]
Client->>VerifyCtrl: POST /api/email/password-reset/verify (email, code)
VerifyCtrl->>VerifyUC: execute(email, code)
VerifyUC->>EmailSvc: verifyCode(email, code)
VerifyUC->>PWVerifiedRepo: save(email) [verified TTL=10m]
Client->>ResetCtrl: POST /api/users/password/reset (ResetPasswordRequest)
ResetCtrl->>ResetUC: execute(request)
ResetUC->>PWVerifiedRepo: existsByEmail(email)
ResetUC->>UserSvc: findByEmail(email) -> User
ResetUC->>UserSvc: updatePassword(encodedNewPassword)
ResetUC->>PWVerifiedRepo: delete(email)
코드 리뷰 예상 시간🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/resources/application.yml (1)
135-135:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win속성명 일관성 문제가 있습니다.
Line 135에서
pathPattern(camelCase)을 사용하고 있는데, 나머지 모든 항목들은path-pattern(kebab-case)을 사용하고 있습니다.YAML 파서가 두 형식을 모두 처리할 수 있더라도, 일관성을 위해
path-pattern으로 통일하시는 것을 권장드립니다.🔧 일관성 개선을 위한 제안
- - pathPattern: /api/users/login + - path-pattern: /api/users/login method: POST참고: 이 문제는 기존 코드에 존재하던 것으로 현재 PR의 범위는 아니지만, 향후 개선하시면 좋을 것 같습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/application.yml` at line 135, Replace the inconsistent camelCase property name pathPattern with the kebab-case variant path-pattern to match the rest of the YAML keys; locate the occurrence of pathPattern in the configuration (the property named pathPattern) and rename it to path-pattern so property naming is consistent across the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/project/dorumdorum/domain/user/application/dto/request/ResetPasswordRequest.java`:
- Around line 6-10: ResetPasswordRequest currently only enforces `@NotBlank` on
newPassword and newPasswordCheck; add a password strength validation (e.g., a
`@Pattern` on the newPassword field or apply a custom constraint annotation) to
enforce minimum length and required character classes (uppercase, lowercase,
digit, special char) and update imports accordingly; ensure the validation is
applied to the record component named newPassword (and consider mirroring or
separately validating newPasswordCheck in the same DTO or via a cross-field
validator in the ResetPasswordRequest to ensure both fields match).
In
`@src/main/java/com/project/dorumdorum/domain/user/application/usecase/ResetPasswordUseCase.java`:
- Around line 40-42: The current operation order in ResetPasswordUseCase calls
passwordResetVerifiedRepository.delete(request.email()) before updating the
user's password, which can leave the Redis verification state deleted if
user.updatePassword/passwordEncoder.encode fails (not rolled back); change the
sequence so that you first perform the transactional password update via
user.updatePassword(passwordEncoder.encode(request.newPassword())) after
locating the user with userService.findByEmail(request.email()), and only after
the update succeeds call passwordResetVerifiedRepository.delete(request.email())
to remove the verification state.
In
`@src/main/java/com/project/dorumdorum/domain/user/application/usecase/SendPasswordResetEmailUseCase.java`:
- Around line 36-37: Add per-email rate limiting to
SendPasswordResetEmailUseCase so callers cannot trigger unlimited sendCode
calls; before calling secureRandomGenerator.generate() and
emailVerificationService.sendCode(), check a rate-limit marker (e.g., via a
RateLimitRepository or Redis) keyed by the email and if it exists throw a
TOO_MANY_REQUESTS/RestApiException, and after sending store the marker with a
short TTL (e.g., 1 minute). Apply the same pattern optionally for IP-based
limits or use a decorator/Resilience4j `@RateLimiter` on the use case if
preferred.
- Around line 27-30: The current SendPasswordResetEmailUseCase.send method
exposes user existence by throwing RestApiException(EMAIL_NOT_FOUND) when
userService.isAlreadyRegistered(email) is false; change this to always return
the same success response regardless of registration status and only trigger
sending an auth code/email when userService.isAlreadyRegistered(email) is true
(e.g., call the existing email-sending logic inside the true branch), while
removing or replacing the EMAIL_NOT_FOUND throw; optionally log non-sensitive
info for ops but do not alter the outward response so clients cannot enumerate
users.
In
`@src/main/java/com/project/dorumdorum/domain/user/infra/repository/RedisEmailVerifiedRepository.java`:
- Around line 19-21: 현재 RedisEmailVerifiedRepository.save에서 EMAIL_VERIFIED +
email 키를 Duration.ofMinutes(10)으로 설정해 VerifyEmailUseCase/SignUpUseCase 흐름에서 인증
완료 상태가 회원가입 완료까지 제한될 수 있으니, 인증 코드 TTL과 인증 완료 상태 TTL을 분리하거나 인증 완료 TTL을 더 늘리거나 설정
가능하게 변경하세요; 구체적으로
RedisEmailVerifiedRepository.save/EMAIL_VERIFIED/VERIFIED_VALUE를 수정해 TTL 값을 설정에서
주입받도록(예: config 프로퍼티로) 대체하고 VerifyEmailUseCase와 SignUpUseCase는 기존 키 존재 검사 로직은
유지하되 TTL 정책이 변경되어도 동작하도록 검증하세요.
In
`@src/main/java/com/project/dorumdorum/domain/user/infra/repository/RedisPasswordResetCodeRepository.java`:
- Line 16: The PASSWORD_RESET key prefix and hardcoded TTL in
RedisPasswordResetCodeRepository should be externalized to configuration: add
properties (e.g. password-reset.code.ttl-minutes and
password-reset.code.key-prefix) in application.yml, create either
`@ConfigurationProperties` or `@Value` injections in
RedisPasswordResetCodeRepository to populate two fields (keyPrefix and
ttlMinutes), replace the private static final String PASSWORD_RESET =
"PASSWORD_RESET:" with the injected keyPrefix, and update save (and all other
methods that reference PASSWORD_RESET) to build keys using keyPrefix and to set
expiry using ttlMinutes (converted to Duration or seconds) instead of the
literal 10 minutes; ensure constructor or field injection and appropriate types
are used so tests and usages still compile.
- Around line 24-26: In RedisPasswordResetCodeRepository.findByEmail validate
the email parameter before using it: check for null or blank (e.g.,
Objects.requireNonNull(email) and String#isBlank or StringUtils.hasText) and
either throw an IllegalArgumentException with a clear message or return
Optional.empty() for invalid input; perform this defensive check at the top of
the findByEmail method so redisTemplate.opsForValue().get(PASSWORD_RESET +
email) is never called with a null/empty key.
- Around line 19-21: The save method in RedisPasswordResetCodeRepository (public
void save(String email, String code)) lacks input validation; add checks to
ensure email and code are non-null and non-blank (trimmed) before calling
redisTemplate.opsForValue().set, and if validation fails throw an
IllegalArgumentException (or a custom BadRequest/ValidationException) with a
clear message so you never store keys like "PASSWORD_RESET:null" or perform
Redis ops on invalid inputs; validate both parameters at the top of save, trim
them, and only proceed to build the key and call opsForValue().set when they
pass validation.
- Around line 29-31: In RedisPasswordResetCodeRepository::delete, validate the
email parameter (null or blank) before using it to build the key; if email is
null/empty/blank, either throw an IllegalArgumentException with a clear message
or simply return without calling redisTemplate.delete to avoid a
NullPointerException—check the input at the start of the delete(String email)
method, reference the PASSWORD_RESET key prefix and redisTemplate.delete call,
and ensure consistent behavior with other repository methods (throw vs no-op) in
this class.
In
`@src/main/java/com/project/dorumdorum/domain/user/infra/repository/RedisPasswordResetVerifiedRepository.java`:
- Around line 15-16: The constants PASSWORD_RESET_VERIFIED, VERIFIED_VALUE and
the hardcoded TTL (10 minutes) in RedisPasswordResetVerifiedRepository should be
externalized to configuration; add properties (e.g.,
password-reset.verified.ttl-minutes, key-prefix, value) in application.yml,
inject them into RedisPasswordResetVerifiedRepository via a
`@ConfigurationProperties` or `@Value` fields (e.g., keyPrefix, ttlMinutes,
verifiedValue) replacing the static constants, and update save(String userId)
and all other methods that reference PASSWORD_RESET_VERIFIED/VERIFIED_VALUE to
use keyPrefix, verifiedValue and compute TTL from ttlMinutes when calling Redis
operations.
- Around line 19-21: The save method in RedisPasswordResetVerifiedRepository
accepts an email without validation which can cause NullPointerException or
create keys like "PASSWORD_RESET_VERIFIED:null"; update the save(String email)
method to validate the parameter (e.g., check for null or blank after trim using
Objects.requireNonNull and String.isBlank or a utility like
StringUtils.hasText), and if invalid throw an IllegalArgumentException (or
return early) with a clear message; ensure you use the validated/trimmed email
when building the key (PASSWORD_RESET_VERIFIED + email) so no invalid keys are
written to Redis.
- Around line 29-31: The delete method
(RedisPasswordResetVerifiedRepository.delete) currently calls
redisTemplate.delete(PASSWORD_RESET_VERIFIED + email) without validating the
email parameter; add a null/empty check for the email argument and handle
invalid input by either throwing an IllegalArgumentException with a clear
message or returning early so you don't concatenate a null and avoid
NullPointerException; locate the method and the constants
PASSWORD_RESET_VERIFIED and redisTemplate to implement the check before calling
redisTemplate.delete.
- Around line 24-26: In RedisPasswordResetVerifiedRepository.existsByEmail
validate the email parameter before calling redisTemplate.hasKey: check for null
or blank (e.g., using Objects.requireNonNull or StringUtils.hasText) and either
throw an IllegalArgumentException with a clear message or return false for
invalid input; then use the existing
Boolean.TRUE.equals(redisTemplate.hasKey(PASSWORD_RESET_VERIFIED +
email.trim())) pattern so the redis key lookup remains null-safe and uses a
trimmed email.
In
`@src/main/java/com/project/dorumdorum/domain/user/ui/SendPasswordResetEmailController.java`:
- Around line 16-22: Add rate limiting to the unauthenticated
SendPasswordResetEmailController.send flow to prevent abuse: before calling
sendPasswordResetEmailUseCase.send(email) check/record request counts for the
requesting IP and the target email (e.g. with a shared RateLimiterService or
cache-backed counters) and enforce limits and a cooldown window (reject with 429
when exceeded); ensure the controller consults methods like
RateLimiterService.allowByIp(ip) and allowByEmail(email) or a combined
token-bucket check and returns an appropriate HTTP 429 response instead of
invoking sendPasswordResetEmailUseCase.send when limits are hit.
- Around line 16-22: 컨트롤러 SendPasswordResetEmailController의 send 메서드가 현재
SendPasswordResetEmailUseCase.send 호출에서 발생하는
EMAIL_NOT_FOUND/INVALID_EMAIL_DOMAIN 예외에 따라 다른 오류(계정 존재 여부 노출)를 만들 수 있으니, send
내부에서 발생 가능한 예외를 잡아 HTTP 응답을 항상 동일하게 반환하도록 변경하세요:
SendPasswordResetEmailUseCase.send 호출을 try-catch로 감싸고 이메일이 존재하지 않거나 도메인이 유효하지 않은
경우에도 ResponseEntity.ok().build()를 반환하며 실제 실패 사유는 로깅만 하고 클라이언트에 노출하지 않도록 처리하고(예:
log.warn/error), 추가로 해당 엔드포인트에 대한 레이트리밋 적용 여부를 확인하거나 적용해 동일한 응답 정책이 다른 인증/계정 관련
엔드포인트에도 일관되도록 점검하세요.
In
`@src/main/java/com/project/dorumdorum/domain/user/ui/VerifyPasswordResetCodeController.java`:
- Around line 16-23: Change VerifyPasswordResetCodeController.verify to accept a
request body DTO instead of query params: create a
VerifyPasswordResetCodeRequest record (fields email and code with validation
annotations), update the controller method signature to take `@RequestBody`
VerifyPasswordResetCodeRequest request (and keep it as a POST endpoint), then
pass request.email() and request.code() into
verifyPasswordResetCodeUseCase.execute; also update any caller/tests to send
JSON body.
In `@src/main/resources/application-dev.yml`:
- Line 16: The current Hibernate property ddl-auto is set to "update", which
allows runtime schema changes; change the ddl-auto value from "update" back to
"validate" for development (or "create-drop" only in test environments) to
prevent unintended schema modifications, and instead add/configure a migration
tool (e.g., Flyway or Liquibase) in the project to manage schema changes via
explicit migration scripts; ensure any CI/deploy docs describe running
migrations before starting the app.
---
Outside diff comments:
In `@src/main/resources/application.yml`:
- Line 135: Replace the inconsistent camelCase property name pathPattern with
the kebab-case variant path-pattern to match the rest of the YAML keys; locate
the occurrence of pathPattern in the configuration (the property named
pathPattern) and rename it to path-pattern so property naming is consistent
across the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 73c4d74e-6b1d-4052-85b1-c9b046f7d1ef
📒 Files selected for processing (22)
src/main/java/com/project/dorumdorum/domain/user/application/dto/request/ResetPasswordRequest.javasrc/main/java/com/project/dorumdorum/domain/user/application/usecase/ResetPasswordUseCase.javasrc/main/java/com/project/dorumdorum/domain/user/application/usecase/SendPasswordResetEmailUseCase.javasrc/main/java/com/project/dorumdorum/domain/user/application/usecase/SignUpUseCase.javasrc/main/java/com/project/dorumdorum/domain/user/application/usecase/VerifyEmailUseCase.javasrc/main/java/com/project/dorumdorum/domain/user/application/usecase/VerifyPasswordResetCodeUseCase.javasrc/main/java/com/project/dorumdorum/domain/user/domain/entity/User.javasrc/main/java/com/project/dorumdorum/domain/user/domain/repository/EmailVerifiedRepository.javasrc/main/java/com/project/dorumdorum/domain/user/domain/repository/PasswordResetCodeRepository.javasrc/main/java/com/project/dorumdorum/domain/user/domain/repository/PasswordResetVerifiedRepository.javasrc/main/java/com/project/dorumdorum/domain/user/infra/repository/RedisEmailVerifiedRepository.javasrc/main/java/com/project/dorumdorum/domain/user/infra/repository/RedisPasswordResetCodeRepository.javasrc/main/java/com/project/dorumdorum/domain/user/infra/repository/RedisPasswordResetVerifiedRepository.javasrc/main/java/com/project/dorumdorum/domain/user/ui/ResetPasswordController.javasrc/main/java/com/project/dorumdorum/domain/user/ui/SendPasswordResetEmailController.javasrc/main/java/com/project/dorumdorum/domain/user/ui/VerifyPasswordResetCodeController.javasrc/main/java/com/project/dorumdorum/domain/user/ui/spec/ResetPasswordApiSpec.javasrc/main/java/com/project/dorumdorum/domain/user/ui/spec/SendPasswordResetEmailApiSpec.javasrc/main/java/com/project/dorumdorum/domain/user/ui/spec/VerifyPasswordResetCodeApiSpec.javasrc/main/java/com/project/dorumdorum/global/exception/code/status/UserErrorStatus.javasrc/main/resources/application-dev.ymlsrc/main/resources/application.yml
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.
📝 Pull Request Template
📌 제목
📢 요약
🔗 연관 이슈: Resolves #109
🚀 PR 유형
✅ PR 체크리스트
📜 기타
Summary by CodeRabbit
New Features
Security & Validation
Configuration
Infrastructure & CI