[1주차] 회원가입 / 내 정보 조회 / 비밀번호 수정 기능 구현 - 정인철#9
[1주차] 회원가입 / 내 정보 조회 / 비밀번호 수정 기능 구현 - 정인철#9incheol789 merged 8 commits intoLoopers-dev-lab:incheol789from
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`:
- Around line 13-23: Add JPA `@Column` annotations to MemberModel's field
declarations so DB-level constraints are enforced: annotate loginId with
`@Column`(nullable = false, unique = true) and annotate password, name, birthDate,
and email with `@Column`(nullable = false); also add the javax.persistence.Column
(or jakarta.persistence.Column depending on project) import. Update the
MemberModel class fields (loginId, password, name, birthDate, email) accordingly
to match the BaseEntity pattern.
In
`@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`:
- Around line 25-27: Replace the insecure SHA-256 single-hash PasswordEncoder
with an adaptive, salted algorithm (e.g., BCrypt or Argon2) and update usages in
MemberService/MemberModel: change PasswordEncoder.encode(...) to produce a
properly salted/strength-configured hash and ensure
MemberModel.applyEncodedPassword(...) stores that encoded hash (not the raw or
single-sha256 value); also replace any PasswordEncoder.matches(...)
implementation that uses String.equals() with the algorithm's safe verify method
(or a constant-time comparison) so password checks are resistant to timing
attacks. Ensure configuration exposes work factor/params (e.g., BCrypt rounds)
and update tests/creation flows that call PasswordEncoder.encode and matches
accordingly.
- Around line 18-27: Add a DB-level uniqueness constraint on the loginId field
and handle concurrent-insert failures by mapping integrity exceptions to your
domain exception: add `@Column`(unique = true) on MemberModel.loginId, keep the
existing pre-check in MemberService.register (which calls PasswordEncoder.encode
and memberRepository.save), and wrap the save call (or the transaction boundary)
to catch DataIntegrityViolationException (or the JPA equivalent) and rethrow as
new CoreException(ErrorType.CONFLICT, "이미 존재하는 loginId입니다.") so concurrent
inserts fail cleanly and surface the same domain error.
In
`@apps/commerce-api/src/main/java/com/loopers/support/crypto/PasswordEncoder.java`:
- Around line 9-25: The current PasswordEncoder.encode method uses MessageDigest
SHA-256 (MessageDigest.getInstance("SHA-256")) which produces unsalted, fast
hashes and is vulnerable to rainbow-table and brute-force attacks; replace this
implementation with a password‑hashing algorithm that handles salts and work
factor for you (e.g., BCrypt or Argon2). Concretely, change
PasswordEncoder.encode to call a secure password hasher (for example Spring's
BCryptPasswordEncoder.encode or a libsodium/Argon2 wrapper) so salts are
generated and stored internally and a configurable strength/work-factor is
applied, and update any code that relies on the SHA-256 output format to accept
the new hashed string format. Ensure exceptions are propagated or wrapped
appropriately and add unit tests verifying verification via the chosen library
(e.g., BCryptPasswordEncoder.matches or Argon2 verify) rather than recomputing
raw hashes.
- Around line 27-29: The matches method in PasswordEncoder uses String.equals,
which is vulnerable to timing attacks; change matches(String rawPassword, String
encodedPassword) to compute the encoded value via encode(rawPassword) and then
perform a constant-time comparison (e.g. convert both to byte[] and use
MessageDigest.isEqual or a constant-time byte loop) instead of String.equals,
and handle nulls safely so comparisons never short-circuit.
🧹 Nitpick comments (2)
apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java (1)
35-49: 응답 매핑을 별도 메서드로 추출하는 것을 고려해 보세요.
MemberInfo에서MyInfoResponse로의 변환 로직이 인라인으로 작성되어 있습니다. 현재 코드도 작동하지만, 매핑 로직을MyInfoResponse의 정적 팩토리 메서드로 추출하면 재사용성과 가독성이 향상됩니다.♻️ 정적 팩토리 메서드 제안
MemberV1Dto.java에 추가:public record MyInfoResponse( String loginId, String maskedName, LocalDate birthDate, String email ) { public static MyInfoResponse from(MemberInfo info) { return new MyInfoResponse( info.loginId(), info.maskedName(), info.birthDate(), info.email() ); } }컨트롤러에서 사용:
MemberInfo info = memberFacade.getMyInfo(loginId, loginPw); -MemberV1Dto.MyInfoResponse response = new MemberV1Dto.MyInfoResponse( - info.loginId(), - info.maskedName(), - info.birthDate(), - info.email() -); +MemberV1Dto.MyInfoResponse response = MemberV1Dto.MyInfoResponse.from(info); return ApiResponse.success(response);apps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.java (1)
92-138: password와 email의 null/empty 케이스 테스트를 추가하세요.MemberModel 생성자는 password와 email에 대해 null 검증을 수행하지만(생성자 40줄, 65줄), loginId, name, birthDate와 달리 명시적인 null 테스트 케이스가 누락되어 있습니다. 테스트 일관성을 위해 다음 케이스들을 추가하는 것을 권장합니다:
- password가 null일 때 BAD_REQUEST 예외 발생
- email이 null일 때 BAD_REQUEST 예외 발생
refactor: UserRegisterService에서 레이어 간 책임 분리 안되어 있는것
📌 Summary
🧭 Context & Decision
문제 정의
선택지와 결정
1. 인증 방식
2. 비밀번호 암호화 방식
3. 테스트 전략 (TDD 적용 시 고민했던 부분)
🏗️ Design Overview
변경 범위
apps/commerce-apidomain/member/- MemberModel, MemberService, MemberRepositoryapplication/member/- MemberFacade, MemberInfointerfaces/api/member/- MemberV1Controller, MemberV1ApiSpec, MemberV1Dtoinfrastructure/member/- MemberRepositoryImpl, MemberJpaRepositorysupport/crypto/PasswordEncoder- SHA-256 암호화 유틸주요 컴포넌트 책임
MemberModel: 회원 엔티티. 생성 시 유효성 검증, 비밀번호 변경 로직 보유MemberService: 회원 비즈니스 로직. 가입/조회/비밀번호 변경 처리MemberFacade: Controller와 Service 사이 조율. DTO 변환 담당MemberV1Controller: REST API 엔드포인트. 요청/응답 처리PasswordEncoder: SHA-256 기반 비밀번호 단방향 암호화🔁 Flow Diagram
Main Flow - 회원가입
Main Flow - 내 정보 조회
Exception Flow - 중복 아이디
✅ Test Plan
단위 테스트 (MemberModelTest - 22 cases)
서비스 단위 테스트 (MemberServiceUnitTest - 16 cases)
통합 테스트 (MemberServiceIntegrationTest - 4 cases)
E2E 테스트 (MemberV1ApiE2ETest - 8 cases)
📝 추가 메모
TDD 진행하면서 어려웠던 점
Red 단계에서 테스트 범위 결정: 처음에 E2E 테스트부터 작성했는데, 실패 시 원인 파악이 어려웠음. 결국 도메인 모델 → 서비스 → E2E 순으로 작성하는 게 효율적이었음.
테스트 더블 선택: Mockito의 mock, spy, stub 개념이 헷갈렸음. 이번에 정리한 기준:
Given-When-Then 경계: Given에서 어디까지 준비해야 하는지 애매했음. 결론은 "테스트 대상 메서드 호출 직전까지가 Given".
리뷰어에게 질문
PasswordEncoder를support/crypto에 두었는데,domain/member안에 두는 게 더 적절할까요?@Nested클래스 사용이 과한 것 같기도 한데, 가독성 면에서 괜찮을까요?MemberServiceUnitTest에 테스트 더블 패턴별로 예시를 다 넣었는데, 실무에서는 이렇게 다양하게 쓰는 편인가요?Summary by CodeRabbit
릴리스 노트