-
Notifications
You must be signed in to change notification settings - Fork 44
[Volume - 1] 회원가입/내 정보 조회/비밀번호 수정 기능 작성 #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: ghtjr410
Are you sure you want to change the base?
Conversation
- API URL에 버전 prefix 적용 (/api/v1/users) - Request DTO에 Bean Validation 추가 - SignUpRequest: loginId, password, name, birthDate, email 필수값 검증 - ChangePasswordRequest: newPassword 필수값 검증 - Controller에서 @Valid 어노테이션으로 검증 활성화
- rules/core: 설계 원칙, DTO 패턴, 예외 처리, 계층 패턴, 네이밍 컨벤션, 워크플로우 규칙 추가 - rules/layer: API, Domain, Service 계층별 상세 규칙 추가 - skills: controller, entity, repository, service, test 패턴 스킬 추가 - skills: PRD 작성, 구현(implement) 워크플로우 스킬 추가 - CLAUDE.md: 프로젝트 개요 및 아키텍처 가이드 추가 - .gitignore: settings.local.json 제외 추가
📝 WalkthroughWalkthrough사용자 회원가입, 인증, 비밀번호 변경 및 정보 조회 기능을 제공하는 사용자 관리 시스템을 완전히 구현한다. BCrypt를 통한 비밀번호 암호화와 커스텀 헤더 기반 인증 메커니즘을 포함한다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as UserV1Controller
participant Facade as UserFacade
participant Service as UserService
participant Repo as UserRepository
participant Encoder as PasswordEncoder
participant DB as Database
Client->>Controller: POST /api/v1/users<br/>{loginId, password, name, birthDate, email}
Controller->>Facade: signUp(...)
Facade->>Service: signUp(...)
Service->>Repo: existsByLoginId(loginId)
Repo->>DB: SELECT loginId
DB-->>Repo: null
Repo-->>Service: false
Service->>Service: User.create(..., encoder)
Note over Service: validate inputs,<br/>encode password
Service->>Encoder: encode(rawPassword)
Encoder-->>Service: encodedPassword
Service->>Repo: save(user)
Repo->>DB: INSERT user
DB-->>Repo: saved user
Repo-->>Service: user
Service-->>Facade: user
Facade->>Facade: UserInfo.from(user)
Facade-->>Controller: userInfo
Controller-->>Client: ApiResponse<UserResponse>
sequenceDiagram
participant Client
participant Resolver as AuthUserResolver
participant Service as UserService
participant Repo as UserRepository
participant Encoder as PasswordEncoder
participant DB as Database
participant Controller as UserV1Controller
Client->>Resolver: GET /api/v1/users/me<br/>Headers: X-Loopers-LoginId, X-Loopers-LoginPw
Resolver->>Resolver: extract headers
Resolver->>Service: authenticate(loginId, rawPassword)
Service->>Repo: findByLoginId(loginId)
Repo->>DB: SELECT * FROM users
DB-->>Repo: user
Repo-->>Service: user
Service->>Service: user.matchesPassword(rawPassword, encoder)
Service->>Encoder: matches(rawPassword, encodedPassword)
Encoder-->>Service: true/false
alt password match
Service-->>Resolver: user
Resolver->>Resolver: AuthenticatedUser.from(user)
Resolver-->>Controller: authUser (injected)
else password mismatch
Service-->>Resolver: CoreException(UNAUTHORIZED)
Resolver-->>Client: 401 Unauthorized
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements the foundational user management domain for Volume 1, including user signup, profile retrieval, and password change functionality. The implementation follows a clean DDD architecture with proper layering and comprehensive test coverage.
Changes:
- Implemented complete User domain with Entity, Service, Repository, Facade, and Controller layers
- Added password validation and BCrypt encryption support
- Created comprehensive test suite including unit, integration, and E2E tests
- Established Claude Code skill and rule system for development guidance
Reviewed changes
Copilot reviewed 58 out of 59 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/commerce-api/src/main/java/com/loopers/domain/user/* | User domain layer with Entity, Service, Repository, PasswordEncoder, PasswordValidator |
| apps/commerce-api/src/main/java/com/loopers/application/user/* | Application layer with UserFacade and UserInfo |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/* | API layer with Controller, DTO, and Swagger spec |
| apps/commerce-api/src/main/java/com/loopers/infrastructure/user/* | Infrastructure implementations for Repository and PasswordEncoder |
| apps/commerce-api/src/main/java/com/loopers/support/error/ErrorType.java | Added UNAUTHORIZED error type |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java | Added IllegalArgumentException handler |
| apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiHeaders.java | API header constants for authentication |
| apps/commerce-api/src/test/java/com/loopers/domain/user/* | Unit tests for User entity and PasswordValidator |
| apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java | Integration tests for UserService |
| apps/commerce-api/src/test/java/com/loopers/interfaces/api/user/UserApiE2ETest.java | E2E tests for User API |
| apps/commerce-api/build.gradle.kts | Added spring-security-crypto dependency |
| http/commerce-api/user.http | HTTP test file for manual API testing |
| CLAUDE.md | Project overview and development guidelines |
| .claude/skills/* | Development skill patterns and examples |
| .claude/rules/* | Project rules and conventions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In
`@apps/commerce-api/src/main/java/com/loopers/domain/user/PasswordValidator.java`:
- Around line 41-49: The validateNotContainsBirthday method in PasswordValidator
should defensively handle a null birthDate: update
PasswordValidator.validateNotContainsBirthday to immediately return (or throw a
clear IllegalArgumentException) when birthDate is null to make the method
contract explicit, and add a unit test in PasswordValidatorTest that covers the
null-birthDate case; reference the existing callers User.create and
User.changePassword to ensure behavior remains compatible with those flows.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java`:
- Around line 93-99: The changePassword method currently calls
encoder.matches(newRawPassword, password) before validating newRawPassword,
which causes an NPE and 500 when newRawPassword is null/blank; move the input
validation (call PasswordValidator.validate(newRawPassword, birthDate) or an
explicit null/blank check) to run before encoder.matches, then perform the
same-password check using encoder.matches; update changePassword to encode only
after validation and the check. Also add unit tests for User.changePassword: one
asserting IllegalArgumentException for null/blank newRawPassword and another
asserting the existing IllegalArgumentException is still thrown when attempting
to reuse the same password.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`:
- Around line 19-27: Handle concurrent signup DB constraint violations by
catching DataIntegrityViolationException around the call to userRepository.save
in the signUp method and rethrowing a CoreException(ErrorType.CONFLICT, "이미 사용
중인 로그인 ID입니다"); specifically, wrap the save(user) invocation in a try/catch that
catches org.springframework.dao.DataIntegrityViolationException (or the
underlying JDBC exception) and maps it to the same Conflict CoreException used
for existsByLoginId, leaving User.create and the existsByLoginId check intact;
also add an integration test that fires concurrent signUp calls with the same
loginId and asserts one success and other responses map to CONFLICT (409).
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BcryptPasswordEncoder.java`:
- Around line 15-17: The matches method in BcryptPasswordEncoder currently calls
BCrypt.checkpw(rawPassword, encodedPassword) which can throw
IllegalArgumentException for malformed/truncated hashes and cause a 500; wrap
the BCrypt.checkpw call in a try-catch that catches IllegalArgumentException and
returns false instead of propagating, leaving existing null/blank assumptions
intact, and add unit tests for malformed hashes (e.g., truncated hash, invalid
prefix) to assert matches(...) returns false.
In `@apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiHeaders.java`:
- Around line 8-9: The ApiHeaders constant LOGIN_PW (value "X-Loopers-LoginPw")
exposes a practice of sending plaintext passwords in HTTP headers; remove or
deprecate this header and migrate authentication to a safer mechanism (e.g.,
accept password in the HTTPS request body for login endpoints or use standard
Authorization Bearer tokens after exchange). Update all usages of
ApiHeaders.LOGIN_PW across services/clients to stop setting the header and
instead send credentials in the login request body (or implement an OAuth-style
token exchange), modify the server-side login handler(s) to read password from
the request body (or validate tokens), and add a short audit to ensure
intermediaries/loggers are not recording the header if you must keep it (search
for LOGIN_PW / "X-Loopers-LoginPw" and update callers in controllers, clients,
and tests). Ensure backward-compatibility is handled (deprecate header, document
migration) and remove the LOGIN_PW constant once callers are updated.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java`:
- Around line 41-58: The controller currently accepts raw passwords via custom
headers (ApiHeaders.LOGIN_PW) in getMyInfo and changePassword; replace this
pattern by using an Authorization header (e.g., Bearer token) and update
validateAuthHeaders, userFacade.getMyInfo and userFacade.changePassword to
accept and validate a token instead of a raw password, or implement
session/token lookup inside those methods; remove usage of ApiHeaders.LOGIN_PW
from the controller methods and validateAuthHeaders call, ensure missing/invalid
token results in a 401 response path, and add a security filter that
masks/excludes any sensitive header (including legacy LOGIN_PW) from access logs
and the logging pipeline with tests asserting 401 for missing/invalid tokens and
that sensitive headers are masked in logs.
- Line 9: Controller에서 사용 중인 `@Valid` 사용을 제거하고 관련 Jakarta Validation import을 삭제하여
Spring의 MethodArgumentNotValidException 발생을 막으세요 (파일: UserV1Controller.java — 제거
대상: import jakarta.validation.Valid 및 메서드 파라미터의 `@Valid`). 또한 DTO(UserV1Dto)의 Bean
Validation 애너테이션들(`@NotBlank`, `@NotNull` 등)을 모두 제거하여 모든 검증이 도메인(User.create,
PasswordValidator)에서 IllegalArgumentException으로 일어나게 하세요. 변경 후
ApiControllerAdvice(기존 라인 111-114)에 의해 일관된 CoreException 포맷(errorCode,
message)으로 응답되는지 확인하는 E2E 테스트를 UserApiE2ETest에 추가하여 signUp 및 changePassword에 대해
빈 loginId, 잘못된 email 형식, 짧은 password 등의 케이스를 검증하세요.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java`:
- Around line 11-22: Remove Bean Validation annotations from the SignUpRequest
record (remove `@NotBlank/`@NotNull on loginId, password, name, birthDate, email)
and remove `@Valid` usage in the controller that accepts SignUpRequest so Spring's
MethodArgumentNotValidException is not thrown; rely on domain-layer validation
by calling User.create(...) and PasswordValidator.validate(...) from the
controller/service path and ensure ApiControllerAdvice handles the resulting
domain validation exceptions so all validation failures return the
ApiControllerAdvice error format (error.code, error.message) with HTTP 400;
after changes, run E2E tests sending invalid inputs to verify consistent 400
responses and ApiControllerAdvice formatting.
🧹 Nitpick comments (2)
apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordValidatorTest.java (2)
44-56: 경계값 테스트가 정확하지 않다.Line 53의
"Abcd1234!Abcd1234!"는 18자이다. 16자 초과 경계값 테스트는 정확히 17자(16+1)로 테스트하는 것이 경계 조건을 명확히 검증한다.🔧 경계값 테스트 개선 제안
`@Test` void 비밀번호가_16자_초과면_예외() { - assertThatThrownBy(() -> PasswordValidator.validate("Abcd1234!Abcd1234!", BIRTHDAY)) + assertThatThrownBy(() -> PasswordValidator.validate("Abcd1234!Abcd123", BIRTHDAY)) // 17자 .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("비밀번호는 8~16자여야 합니다"); }
65-71: birthDate가 null인 경우의 테스트가 누락되었다.
PasswordValidator.validate()의birthDate파라미터가 null일 때의 동작을 검증하는 테스트가 필요하다. 구현체에 null 방어 코드가 추가된다면 해당 케이스도 테스트해야 한다.🧪 birthDate null 테스트 추가 제안
`@Test` void 생년월일이_null이면_생년월일_검증을_스킵한다() { assertThatCode(() -> PasswordValidator.validate("Abcd1234!", null)) .doesNotThrowAnyException(); }
| private static void validateNotContainsBirthday(String rawPassword, LocalDate birthDate) { | ||
| String yyyyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyyyMMdd")); | ||
| String yyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyMMdd")); | ||
| String MMdd = birthDate.format(DateTimeFormatter.ofPattern("MMdd")); | ||
|
|
||
| if (rawPassword.contains(yyyyMMdd) || rawPassword.contains(yyMMdd) || rawPassword.contains(MMdd)) { | ||
| throw new IllegalArgumentException("비밀번호에 생년월일을 포함할 수 없습니다"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. PasswordValidator.java 전체 내용 조회
echo "=== PasswordValidator.java 내용 ==="
find apps -name "PasswordValidator.java" -type f | head -1 | xargs cat -n
echo ""
echo "=== validate() 메서드 호출부 검색 ==="
rg -n "PasswordValidator\.validate" --type=java -A3 -B1 apps/commerce-api/src/main/java/Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3392
🏁 Script executed:
#!/bin/bash
echo "=== User.java 전체 내용 ==="
find apps -name "User.java" -path "*/domain/user/*" -type f | head -1 | xargs cat -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4026
🏁 Script executed:
#!/bin/bash
echo "=== PasswordValidator 테스트 파일 찾기 ==="
find apps -name "*PasswordValidator*Test.java" -o -name "*PasswordValidatorTest.java" | head -5
echo ""
echo "=== User 테스트 파일에서 validate 관련 테스트 ==="
find apps -name "*UserTest.java" -o -name "*UserTests.java" | xargs ls -la 2>/dev/null | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 361
🏁 Script executed:
#!/bin/bash
echo "=== PasswordValidatorTest.java 내용 ==="
cat -n apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordValidatorTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3390
PasswordValidator.validateNotContainsBirthday()의 메서드 계약을 명확히 하기 위해 null 방어 코드를 추가하자.
현재 호출 컨텍스트(User.create(), User.changePassword())에서 birthDate는 validateBirthDate()로 검증된 후 또는 이미 생성된 엔티티에서 전달되므로 null이 아니다. 그러나 PasswordValidator는 static utility 클래스로 외부에서도 호출될 수 있으므로, 메서드의 의도를 명확히 하기 위해 null 체크를 추가하는 것이 좋은 관행이다. birthDate가 null인 경우의 테스트 케이스도 PasswordValidatorTest에 추가해야 한다.
제안: null 방어 코드 추가
private static void validateNotContainsBirthday(String rawPassword, LocalDate birthDate) {
+ if (birthDate == null) {
+ return; // 생년월일 정보가 없으면 해당 검증 스킵
+ }
String yyyyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyyyMMdd"));
String yyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyMMdd"));
String MMdd = birthDate.format(DateTimeFormatter.ofPattern("MMdd"));📝 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.
| private static void validateNotContainsBirthday(String rawPassword, LocalDate birthDate) { | |
| String yyyyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyyyMMdd")); | |
| String yyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyMMdd")); | |
| String MMdd = birthDate.format(DateTimeFormatter.ofPattern("MMdd")); | |
| if (rawPassword.contains(yyyyMMdd) || rawPassword.contains(yyMMdd) || rawPassword.contains(MMdd)) { | |
| throw new IllegalArgumentException("비밀번호에 생년월일을 포함할 수 없습니다"); | |
| } | |
| } | |
| private static void validateNotContainsBirthday(String rawPassword, LocalDate birthDate) { | |
| if (birthDate == null) { | |
| return; // 생년월일 정보가 없으면 해당 검증 스킵 | |
| } | |
| String yyyyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyyyMMdd")); | |
| String yyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyMMdd")); | |
| String MMdd = birthDate.format(DateTimeFormatter.ofPattern("MMdd")); | |
| if (rawPassword.contains(yyyyMMdd) || rawPassword.contains(yyMMdd) || rawPassword.contains(MMdd)) { | |
| throw new IllegalArgumentException("비밀번호에 생년월일을 포함할 수 없습니다"); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/domain/user/PasswordValidator.java`
around lines 41 - 49, The validateNotContainsBirthday method in
PasswordValidator should defensively handle a null birthDate: update
PasswordValidator.validateNotContainsBirthday to immediately return (or throw a
clear IllegalArgumentException) when birthDate is null to make the method
contract explicit, and add a unit test in PasswordValidatorTest that covers the
null-birthDate case; reference the existing callers User.create and
User.changePassword to ensure behavior remains compatible with those flows.
| public void changePassword(String newRawPassword, PasswordEncoder encoder) { | ||
| if (encoder.matches(newRawPassword, password)) { | ||
| throw new IllegalArgumentException("현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다"); | ||
| } | ||
| PasswordValidator.validate(newRawPassword, birthDate); | ||
| password = encoder.encode(newRawPassword); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
비밀번호 변경 입력 검증 순서로 500이 발생할 수 있다
운영에서 newRawPassword가 null/blank로 들어오면 encoder.matches에서 NPE가 발생해 400 대신 500이 반환되고 장애 분석이 어려워진다.
수정안: PasswordValidator.validate 또는 null/blank 체크를 matches 호출 이전으로 이동하라.
추가 테스트: null/blank 입력 시 IllegalArgumentException이 발생하고 동일 비밀번호 재사용 시 에러가 유지되는지 단위 테스트를 추가하라.
🛠️ 수정안
public void changePassword(String newRawPassword, PasswordEncoder encoder) {
- if (encoder.matches(newRawPassword, password)) {
- throw new IllegalArgumentException("현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다");
- }
PasswordValidator.validate(newRawPassword, birthDate);
+ if (encoder.matches(newRawPassword, password)) {
+ throw new IllegalArgumentException("현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다");
+ }
password = encoder.encode(newRawPassword);
}📝 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 void changePassword(String newRawPassword, PasswordEncoder encoder) { | |
| if (encoder.matches(newRawPassword, password)) { | |
| throw new IllegalArgumentException("현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다"); | |
| } | |
| PasswordValidator.validate(newRawPassword, birthDate); | |
| password = encoder.encode(newRawPassword); | |
| } | |
| public void changePassword(String newRawPassword, PasswordEncoder encoder) { | |
| PasswordValidator.validate(newRawPassword, birthDate); | |
| if (encoder.matches(newRawPassword, password)) { | |
| throw new IllegalArgumentException("현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다"); | |
| } | |
| password = encoder.encode(newRawPassword); | |
| } |
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java` around
lines 93 - 99, The changePassword method currently calls
encoder.matches(newRawPassword, password) before validating newRawPassword,
which causes an NPE and 500 when newRawPassword is null/blank; move the input
validation (call PasswordValidator.validate(newRawPassword, birthDate) or an
explicit null/blank check) to run before encoder.matches, then perform the
same-password check using encoder.matches; update changePassword to encode only
after validation and the check. Also add unit tests for User.changePassword: one
asserting IllegalArgumentException for null/blank newRawPassword and another
asserting the existing IllegalArgumentException is still thrown when attempting
to reuse the same password.
| @Transactional | ||
| public User signUp(String loginId, String rawPassword, String name, LocalDate birthDate, String email) { | ||
| if (userRepository.existsByLoginId(loginId)) { | ||
| throw new CoreException(ErrorType.CONFLICT, "이미 사용 중인 로그인 ID입니다"); | ||
| } | ||
|
|
||
| User user = User.create(loginId, rawPassword, name, birthDate, email, passwordEncoder); | ||
| return userRepository.save(user); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -name "UserService.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 159
🏁 Script executed:
find . -name "User.java" -type f | grep -E "(domain|entity)" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 152
🏁 Script executed:
find . -name "ErrorType.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 159
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1951
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/domain/user/User.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4002
🏁 Script executed:
cat -n ./apps/commerce-api/src/main/java/com/loopers/support/error/ErrorType.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1056
🏁 Script executed:
rg "DataIntegrityViolationException" --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
rg "catch.*Exception" apps/commerce-api/src/main/java/com/loopers/domain --type java -A 3 | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
grep -r "DataIntegrityViolationException\|ConstraintViolationException" apps/commerce-api/src --include="*.java" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
rg "import.*Violation" apps/commerce-api/src --type javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
동시 가입 요청 시 DataIntegrityViolationException을 적절히 처리해야 한다
로그인 ID에 유니크 제약이 이미 존재하므로, 동시 가입 요청 시 existsByLoginId 검사와 저장 사이에 경쟁이 발생하면 먼저 저장한 요청은 성공하고 나중 요청은 DataIntegrityViolationException이 발생한다. 현재는 이 예외를 처리하지 않아 500 오류로 응답되는데, 이를 CONFLICT(409)로 적절히 매핑하여 운영 안정성을 확보해야 한다.
saveMethod에서 DataIntegrityViolationException을 catch하여 CoreException(ErrorType.CONFLICT, "이미 사용 중인 로그인 ID입니다")로 변환하도록 수정한다.
추가 테스트로 동일 loginId를 동시에 signUp하는 통합 테스트를 작성해 하나만 성공하고 나머지가 CONFLICT(409)로 끝나는지 확인한다.
수정 제안
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import lombok.RequiredArgsConstructor;
+import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
`@Transactional`
public User signUp(String loginId, String rawPassword, String name, LocalDate birthDate, String email) {
if (userRepository.existsByLoginId(loginId)) {
throw new CoreException(ErrorType.CONFLICT, "이미 사용 중인 로그인 ID입니다");
}
User user = User.create(loginId, rawPassword, name, birthDate, email, passwordEncoder);
- return userRepository.save(user);
+ try {
+ return userRepository.save(user);
+ } catch (DataIntegrityViolationException e) {
+ throw new CoreException(ErrorType.CONFLICT, "이미 사용 중인 로그인 ID입니다", e);
+ }
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 19 - 27, Handle concurrent signup DB constraint violations by
catching DataIntegrityViolationException around the call to userRepository.save
in the signUp method and rethrowing a CoreException(ErrorType.CONFLICT, "이미 사용
중인 로그인 ID입니다"); specifically, wrap the save(user) invocation in a try/catch that
catches org.springframework.dao.DataIntegrityViolationException (or the
underlying JDBC exception) and maps it to the same Conflict CoreException used
for existsByLoginId, leaving User.create and the existsByLoginId check intact;
also add an integration test that fires concurrent signUp calls with the same
loginId and asserts one success and other responses map to CONFLICT (409).
| @Override | ||
| public boolean matches(String rawPassword, String encodedPassword) { | ||
| return BCrypt.checkpw(rawPassword, encodedPassword); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
BCrypt.checkpw IllegalArgumentException invalid hash format
💡 Result:
BCrypt.checkpw(plain, hashed) throws IllegalArgumentException: Invalid hash format (or similar) when the stored “hashed” value is not a valid BCrypt hash string for that library.
Common causes + fixes:
- You’re not passing a BCrypt hash at all
hashedmust look like:$2a$10$...(or$2b$...,$2y$...) and be ~60 chars.- If it’s empty, a different algorithm (PBKDF2/SCrypt/Argon2/SHA*), or modified,
checkpwcan’t parse it. (docs.spring.io)
- Hash got truncated/corrupted in storage
- Very common: DB column too short (e.g.,
VARCHAR(50)), so the 60-char BCrypt string is cut → invalid format. - Fix: make the column big enough (often
VARCHAR(255)), then re-hash and re-save passwords. (stackoverflow.com)
- Spring Security
{bcrypt}prefix mismatch
- If you use Spring’s
DelegatingPasswordEncoder, stored passwords are typically prefixed like{bcrypt}$2a$.... Spring uses the prefix to pick the encoder. (docs.spring.io)- If you call jBCrypt /
org.mindrot.jbcrypt.BCrypt.checkpwdirectly, pass only the$2a$...part (no{bcrypt}). - If you call Spring’s
PasswordEncoder.matches, keep{bcrypt}(or configure the right encoder explicitly). (docs.spring.io)
- If you call jBCrypt /
- Incompatible BCrypt variant / library mismatch
- Some online generators / other implementations produce variants/prefixes that a given Java BCrypt library may reject (often seen as “invalid salt/version/format”).
- Fix: ensure hashes are generated and verified with the same library, or upgrade/switch to a BCrypt implementation that supports your prefix. (stackoverflow.com)
Quick sanity check (Java):
System.out.println(hashed);
System.out.println(hashed.length()); // should be ~60 for raw "$2x$..." hashesIf you paste (1) the exact exception text, (2) which BCrypt you’re using (Spring BCryptPasswordEncoder vs org.mindrot.jbcrypt.BCrypt), and (3) an example of the stored hash prefix (mask the middle), I can tell you which of the above is happening.
Citations:
- 1: https://docs.spring.io/spring-security/site/docs/5.1.4.RELEASE/api/org/springframework/security/crypto/bcrypt/BCrypt.html?utm_source=openai
- 2: https://stackoverflow.com/questions/49582971/encoded-password-does-not-look-like-bcrypt?utm_source=openai
- 3: https://docs.spring.io/spring-security/reference/features/authentication/password-storage.html?utm_source=openai
- 4: https://docs.spring.io/spring-security/reference/features/authentication/password-storage.html?utm_source=openai
- 5: https://stackoverflow.com/questions/16478604/bcrypt-checkpw-invalid-salt-version-exception?utm_source=openai
🏁 Script executed:
# BcryptPasswordEncoder.java 전체 내용 확인
find . -name "BcryptPasswordEncoder.java" -type f | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 177
🏁 Script executed:
# 비밀번호 검증 흐름 확인
rg "PasswordPolicy|password.*validate" --type java -A 3 -B 3 | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# matches 메서드 호출 지점 확인
rg "\.matches\(" --type java | grep -i password | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 501
🏁 Script executed:
# BcryptPasswordEncoder 전체 코드 확인
cat -n apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BcryptPasswordEncoder.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 776
🏁 Script executed:
# UserService의 matches 호출 지점 확인
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java | grep -A 5 -B 5 "matches"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 577
🏁 Script executed:
# PasswordValidator 확인
find . -name "PasswordValidator.java" -type f | head -1 | xargs cat -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2344
🏁 Script executed:
# User 엔티티에서 password 필드 확인
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/User.java | head -60Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2384
손상된 해시 입력 시 500으로 전파될 위험이 있다
운영 관점에서 DB 손상이나 마이그레이션 오류로 잘못된 형식의 해시가 저장되면 BCrypt.checkpw가 IllegalArgumentException을 던져 인증 API가 500으로 실패한다. 실제 해시 값이 DB 컬럼 길이 제한으로 잘린 경우가 가장 흔한 원인이다.
수정안: matches 메서드에서 BCrypt.checkpw를 try-catch로 래핑하여 IllegalArgumentException을 false로 흡수한다. rawPassword나 encodedPassword의 null/공백 검사는 데이터 흐름상 필요하지 않다(정상 경로에서는 항상 유효한 값이 전달됨).
🛠 수정안 예시
`@Override`
public boolean matches(String rawPassword, String encodedPassword) {
- return BCrypt.checkpw(rawPassword, encodedPassword);
+ try {
+ return BCrypt.checkpw(rawPassword, encodedPassword);
+ } catch (IllegalArgumentException ex) {
+ return false;
+ }
}추가 테스트: 잘못된 해시 형식(예: 截断된 해시, 잘못된 접두사)에 대해 matches가 false를 반환하는 단위 테스트를 추가한다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BcryptPasswordEncoder.java`
around lines 15 - 17, The matches method in BcryptPasswordEncoder currently
calls BCrypt.checkpw(rawPassword, encodedPassword) which can throw
IllegalArgumentException for malformed/truncated hashes and cause a 500; wrap
the BCrypt.checkpw call in a try-catch that catches IllegalArgumentException and
returns false instead of propagating, leaving existing null/blank assumptions
intact, and add unit tests for malformed hashes (e.g., truncated hash, invalid
prefix) to assert matches(...) returns false.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiHeaders.java
Outdated
Show resolved
Hide resolved
| import com.loopers.interfaces.api.ApiResponse; | ||
| import com.loopers.support.error.CoreException; | ||
| import com.loopers.support.error.ErrorType; | ||
| import jakarta.validation.Valid; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check `@Valid` usage in the controller file
rg -n "@Valid" apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 271
🏁 Script executed:
#!/bin/bash
# Read the entire controller file to understand the structure
wc -l apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 173
🏁 Script executed:
#!/bin/bash
# Check CoreException and ApiControllerAdvice existence
fd -e java | xargs rg -l "class ApiControllerAdvice|class CoreException" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 326
🏁 Script executed:
#!/bin/bash
# Look for domain layer validation - User entity and PasswordPolicy
fd -e java -path "*/domain/*" | xargs rg -l "class User|class PasswordPolicy|class PasswordValidator" 2>/dev/null | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1143
🏁 Script executed:
#!/bin/bash
# Check if ApiControllerAdvice handles MethodArgumentNotValidException
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 7126
🏁 Script executed:
#!/bin/bash
# Check User domain entity validation
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/User.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4002
🏁 Script executed:
#!/bin/bash
# Check PasswordValidator
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/PasswordValidator.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2344
🏁 Script executed:
#!/bin/bash
# Check UserV1Dto for Bean Validation annotations
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1602
@Valid과 Bean Validation 어노테이션을 제거하여 일관된 에러 응답 포맷을 유지하라
운영 환경에서 @Valid 검증 실패 시 Spring이 MethodArgumentNotValidException을 발생시키는데, ApiControllerAdvice에 이를 처리하는 핸들러가 없어 기본 Spring 에러 응답이 반환된다. 이로 인해 클라이언트의 CoreException 기반 응답 파싱 로직이 실패한다.
도메인 레이어(User.create, PasswordValidator)에서 이미 모든 검증이 구현되어 있으므로, Controller는 @Valid를 제거하고 DTO의 Bean Validation 어노테이션(@NotBlank, @NotNull)도 제거해야 한다. 이렇게 하면 모든 검증 실패가 도메인에서 IllegalArgumentException으로 발생하며, 이는 ApiControllerAdvice의 라인 111-114에서 일관되게 CoreException 포맷으로 변환된다.
추가 테스트: 잘못된 요청(빈 loginId, 부정형식 email, 짧은 password 등)으로 signUp/changePassword를 호출할 때, 응답이 일관되게 CoreException 포맷(errorCode, message)으로 반환되는지 확인하는 E2E 테스트를 UserApiE2ETest에 추가하라.
🛠️ 수정안
-import jakarta.validation.Valid;
@@
- public ApiResponse<UserV1Dto.UserResponse> signUp(`@Valid` `@RequestBody` UserV1Dto.SignUpRequest request) {
+ public ApiResponse<UserV1Dto.UserResponse> signUp(`@RequestBody` UserV1Dto.SignUpRequest request) {
@@
- `@Valid` `@RequestBody` UserV1Dto.ChangePasswordRequest request
+ `@RequestBody` UserV1Dto.ChangePasswordRequest requestUserV1Dto.java에서도 다음과 같이 Bean Validation 어노테이션을 제거:
public record SignUpRequest(
- `@NotBlank`(message = "로그인 ID는 필수입니다")
String loginId,
- `@NotBlank`(message = "비밀번호는 필수입니다")
String password,
- `@NotBlank`(message = "이름은 필수입니다")
String name,
- `@NotNull`(message = "생년월일은 필수입니다")
LocalDate birthDate,
- `@NotBlank`(message = "이메일은 필수입니다")
String email
) {}
public record ChangePasswordRequest(
- `@NotBlank`(message = "새 비밀번호는 필수입니다")
String newPassword
) {}🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java`
at line 9, Controller에서 사용 중인 `@Valid` 사용을 제거하고 관련 Jakarta Validation import을 삭제하여
Spring의 MethodArgumentNotValidException 발생을 막으세요 (파일: UserV1Controller.java — 제거
대상: import jakarta.validation.Valid 및 메서드 파라미터의 `@Valid`). 또한 DTO(UserV1Dto)의 Bean
Validation 애너테이션들(`@NotBlank`, `@NotNull` 등)을 모두 제거하여 모든 검증이 도메인(User.create,
PasswordValidator)에서 IllegalArgumentException으로 일어나게 하세요. 변경 후
ApiControllerAdvice(기존 라인 111-114)에 의해 일관된 CoreException 포맷(errorCode,
message)으로 응답되는지 확인하는 E2E 테스트를 UserApiE2ETest에 추가하여 signUp 및 changePassword에 대해
빈 loginId, 잘못된 email 형식, 짧은 password 등의 케이스를 검증하세요.
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java
Outdated
Show resolved
Hide resolved
| public record SignUpRequest( | ||
| @NotBlank(message = "로그인 ID는 필수입니다") | ||
| String loginId, | ||
| @NotBlank(message = "비밀번호는 필수입니다") | ||
| String password, | ||
| @NotBlank(message = "이름은 필수입니다") | ||
| String name, | ||
| @NotNull(message = "생년월일은 필수입니다") | ||
| LocalDate birthDate, | ||
| @NotBlank(message = "이메일은 필수입니다") | ||
| String email | ||
| ) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# UserV1Dto.java 파일 확인
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1602
🏁 Script executed:
# User 엔티티와 PasswordPolicy 구현 확인
find apps/commerce-api -type f -name "User.java" -o -name "PasswordPolicy.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 150
🏁 Script executed:
# CoreException 및 ApiControllerAdvice 확인
find apps/commerce-api -type f \( -name "CoreException.java" -o -name "ApiControllerAdvice.java" \) | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 245
🏁 Script executed:
# interfaces/api 디렉토리 아래 다른 DTO 검색
find apps/commerce-api/src/main/java/com/loopers/interfaces/api -type f -name "*Dto.java" -o -name "*Request.java" -o -name "*Response.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 324
🏁 Script executed:
# User entity 검증 로직 확인
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/User.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4002
🏁 Script executed:
# PasswordPolicy 검색
find apps/commerce-api -type f -name "*PasswordPolicy*"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# ApiControllerAdvice 확인
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 7126
🏁 Script executed:
# CoreException 확인
cat -n apps/commerce-api/src/main/java/com/loopers/support/error/CoreException.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 741
🏁 Script executed:
# ExampleV1Dto 패턴 확인
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/example/ExampleV1Dto.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 620
🏁 Script executed:
# Verify PasswordValidator exists
find apps/commerce-api -type f -name "*PasswordValidator*"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 246
🏁 Script executed:
# Check if there are any controller methods using SignUpRequest/ChangePasswordRequest
grep -r "SignUpRequest\|ChangePasswordRequest" apps/commerce-api/src/main/java --include="*.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 968
🏁 Script executed:
# Check UserV1Controller to see how SignUpRequest is used
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3254
DTO의 Bean Validation 애너테이션을 제거하고 도메인 계층에서 검증하도록 수정하다.
컨트롤러의 @Valid 애너테이션이 Bean Validation을 먼저 실행하면 MethodArgumentNotValidException이 발생하는데, ApiControllerAdvice에 해당 예외 핸들러가 없어 CoreException 흐름을 우회한다. 결과적으로 필드 검증 오류는 Spring 기본 응답 형식으로, 다른 오류는 ApiControllerAdvice 표준 형식으로 반환되어 클라이언트 파싱이 불안정해진다.
User.create()와 PasswordValidator.validate()에 이미 동일한 검증 규칙이 구현되어 있으므로, DTO의 @NotBlank/@NotNull을 제거하고 컨트롤러의 @Valid를 삭제한다. 이후 잘못된 입력(빈 값, null, 형식 오류)을 보내 ApiControllerAdvice의 표준 에러 포맷(error.code, error.message)과 HTTP 400이 모든 검증 실패 시나리오에서 일관되게 유지되는지 E2E 테스트로 검증한다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java`
around lines 11 - 22, Remove Bean Validation annotations from the SignUpRequest
record (remove `@NotBlank/`@NotNull on loginId, password, name, birthDate, email)
and remove `@Valid` usage in the controller that accepts SignUpRequest so Spring's
MethodArgumentNotValidException is not thrown; rely on domain-layer validation
by calling User.create(...) and PasswordValidator.validate(...) from the
controller/service path and ensure ApiControllerAdvice handles the resulting
domain validation exceptions so all validation failures return the
ApiControllerAdvice error format (error.code, error.message) with HTTP 400;
after changes, run E2E tests sending invalid inputs to verify consistent 400
responses and ApiControllerAdvice formatting.
|
@coderabbitai 아쉬웠던점 요약 |
|
Tip For best results, initiate chat on the files or code changes.
운영 및 보안 관점에서의 개선이 필요한 사항1. 인증 방식의 보안 취약점
2. 보안 이벤트 감사 로깅 부재
3. Rate Limiting 및 계정 잠금 정책 부재
4. 운영 관찰성(Observability) 부족
5. 동시성 및 트랜잭션 경합 미검증
6. 성능: N+1 쿼리 및 캐싱 전략 부재
7. 에러 응답 일관성 및 클라이언트 지침 부족
8. 국제화(i18n) 미지원
9. 테스트: 실패 시나리오 및 경계값 보강 필요
10. API 버전 관리 전략 불명확
위 항목들은 프로덕션 배포 전 반드시 보완해야 할 사항이다. 특히 1~5번은 보안 및 운영 안정성에 직결되므로 우선순위를 높여 처리한다. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java`:
- Around line 105-117: The MMdd (4-digit) birthdate check in
User.validatePasswordNotContainsBirthDate is overly broad and can wrongly reject
many passwords; update this method to remove or guard the MMdd check—keep
yyyyMMdd and yyMMdd checks only, or make the MMdd check configurable via a
flag/constant so policy can be toggled. Locate
validatePasswordNotContainsBirthDate in the User class, remove the MMdd pattern
from the contains checks (or wrap its creation/usage behind a boolean like
CHECK_MMDD) and update any related tests/validation messages accordingly.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`:
- Around line 29-36: In authenticate (UserService.authenticate) replace the two
distinct client-facing error messages with a single generic response (e.g. throw
new CoreException(ErrorType.UNAUTHORIZED, "인증 정보가 올바르지 않습니다")) for both missing
loginId (userRepository.findByLoginId) and bad password (User.matchesPassword
using passwordEncoder), and move the specific reason into server logs (use the
class logger to log whether the user was not found or password mismatch) so
client-facing output no longer enables user enumeration while preserving
diagnostic logs.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java`:
- Around line 98-113: Extend the 비밀번호_변경 nested tests in
UserServiceIntegrationTest to cover failure cases: add a test that after
userService.changePassword(loginId, rawPassword, newPassword) authenticating
with the old rawPassword via userService.authenticate(loginId, rawPassword)
throws the UNAUTHORIZED exception (verifies DB update); add a test that calling
userService.changePassword(loginId, rawPassword, rawPassword) (new password same
as current) throws the expected validation exception; and add a test that
calling userService.changePassword(loginId, "wrongCurrent", "SomeNew1!") throws
UNAUTHORIZED; use the existing signUp setup, and assertions like
assertThatThrownBy / assertThatExceptionOfType against
userService.changePassword and userService.authenticate to validate the expected
exceptions.
🧹 Nitpick comments (4)
apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java (1)
8-62: 값 객체(Value Object)에equals/hashCode가 누락되었다.
Password는@Embeddable값 객체이므로value필드 기반의equals/hashCode구현이 필요하다. 현재 누락되어 있으면, 컬렉션 비교나 테스트 단언에서 동등성 판단이 참조 비교(Object.equals)로 동작하여 의도치 않은 결과가 발생할 수 있다.toString()은 해시 노출 방지를 위해 재정의하지 않거나, 마스킹 처리하는 것이 적절하다. As per coding guidelines, "equals/hashCode는 식별자 기반 또는 값 기반으로 일관되게 설계한다."♻️ 수정안
+ `@Override` + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Password password = (Password) o; + return java.util.Objects.equals(value, password.value); + } + + `@Override` + public int hashCode() { + return java.util.Objects.hash(value); + } }apps/commerce-api/src/main/java/com/loopers/domain/user/User.java (1)
14-17:@Getter가 클래스 레벨에 적용되어Password객체가 외부로 노출된다.
@Getter가 클래스 전체에 적용되어getPassword()가 공개된다.Password.getValue()를 통해 인코딩된 해시가 외부 계층(예: 직렬화, 로깅, 응답 매핑)으로 유출될 수 있다.password필드는@Getter대상에서 제외하고, 필요한 동작은matchesPassword,changePassword같은 도메인 메서드로 위임하는 것이 캡슐화 관점에서 안전하다. As per coding guidelines, "불변성과 캡슐화를 점검한다."♻️ 수정안
`@Entity` `@Table`(name = "users") -@Getter public class User extends BaseEntity { + + // password 필드를 제외하고 필요한 필드에만 `@Getter` 적용 + `@Getter` + `@Column`(nullable = false, unique = true) + private String loginId; + + `@Embedded` + private Password password; + + `@Getter` + `@Column`(nullable = false) + private String name; + + `@Getter` + `@Column`(nullable = false) + private LocalDate birthDate; + + `@Getter` + `@Column`(nullable = false) + private String email;apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java (2)
126-147:비밀번호_변경테스트에서 동일 비밀번호 재사용 시나리오가 누락되었다.
User.changePassword가Password.change에 위임하므로,Password단위 테스트에서 동일 비밀번호 재사용 예외는 검증된다. 그러나User수준에서validatePasswordNotContainsBirthDate와Password.change의 조합이 올바르게 동작하는지 확인하려면, 동일 비밀번호로 변경 시 예외가 발생하는 테스트를User단위 테스트에도 추가하는 것이 안전하다.🧪 추가 테스트
+ `@Test` + void 현재_비밀번호와_동일하면_예외() { + User user = User.create("testuser", "Test1234!", "홍길동", + LocalDate.of(2000, 1, 15), "test@example.com", PASSWORD_ENCODER); + + assertThatThrownBy(() -> user.changePassword("Test1234!", PASSWORD_ENCODER)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다"); + }
92-98: 이메일 형식 검증 테스트에서 RFC 경계 케이스가 포함되면 좋다.현재
"user@domain"(TLD 없음)이 거부되는 것을 검증하고 있다. 추가로.연속 사용("user@domain..com"), 특수문자 위치(".user@domain.com") 등 실무에서 빈번한 잘못된 입력을 포함하면 회귀 방지에 유리하다.
| private static void validatePasswordNotContainsBirthDate(String rawPassword, LocalDate birthDate) { | ||
| if (rawPassword == null || birthDate == null) { | ||
| return; | ||
| } | ||
|
|
||
| String yyyyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyyyMMdd")); | ||
| String yyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyMMdd")); | ||
| String MMdd = birthDate.format(DateTimeFormatter.ofPattern("MMdd")); | ||
|
|
||
| if (rawPassword.contains(yyyyMMdd) || rawPassword.contains(yyMMdd) || rawPassword.contains(MMdd)) { | ||
| throw new IllegalArgumentException("비밀번호에 생년월일을 포함할 수 없습니다"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
validatePasswordNotContainsBirthDate의 MMdd 패턴이 의도치 않은 거부를 유발할 수 있다.
MMdd 패턴은 4자리로, 일반적인 숫자 조합과 충돌할 가능성이 있다. 예를 들어 생년월일이 2000-12-10인 사용자는 "1210"을 포함하는 모든 비밀번호가 거부된다(Abcd1210! 등). 이는 보안 정책상 의도된 것일 수 있으나, 비밀번호 선택 범위를 과도하게 제한하여 사용자 경험을 저하시킬 수 있다. 정책 의도를 확인하고, 필요시 yyyyMMdd와 yyMMdd만 검사하는 방안을 검토하라.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java` around
lines 105 - 117, The MMdd (4-digit) birthdate check in
User.validatePasswordNotContainsBirthDate is overly broad and can wrongly reject
many passwords; update this method to remove or guard the MMdd check—keep
yyyyMMdd and yyMMdd checks only, or make the MMdd check configurable via a
flag/constant so policy can be toggled. Locate
validatePasswordNotContainsBirthDate in the User class, remove the MMdd pattern
from the contains checks (or wrap its creation/usage behind a boolean like
CHECK_MMDD) and update any related tests/validation messages accordingly.
apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java
Show resolved
Hide resolved
| @Nested | ||
| class 비밀번호_변경 { | ||
|
|
||
| @Test | ||
| void 유효한_새_비밀번호로_변경하면_성공한다() { | ||
| String loginId = "testuser"; | ||
| String rawPassword = "Test1234!"; | ||
| String newPassword = "NewPass123!"; | ||
| userService.signUp(loginId, rawPassword, "홍길동", LocalDate.of(2000, 1, 15), "test@example.com"); | ||
|
|
||
| userService.changePassword(loginId, rawPassword, newPassword); | ||
|
|
||
| assertThatCode(() -> userService.authenticate(loginId, newPassword)) | ||
| .doesNotThrowAnyException(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
비밀번호_변경 통합 테스트에서 실패 시나리오와 이전 비밀번호 무효화 검증이 누락되었다.
현재 성공 경로만 검증하고 있다. 통합 테스트는 트랜잭션 경계와 도메인 규칙이 결합된 상태에서 정상 동작하는지 확인하는 것이 목적이므로, 다음 시나리오를 추가하라:
- 변경 후 이전 비밀번호로 인증 시
UNAUTHORIZED예외 발생 - 현재 비밀번호와 동일한 새 비밀번호로 변경 시 예외 발생
- 잘못된 현재 비밀번호로 변경 시도 시
UNAUTHORIZED예외 발생
특히 (1)은 비밀번호 변경이 실제로 DB에 반영되었는지 확인하는 핵심 검증이다.
🧪 추가 테스트
+ `@Test`
+ void 변경_후_이전_비밀번호로_인증하면_예외() {
+ String loginId = "testuser";
+ String rawPassword = "Test1234!";
+ String newPassword = "NewPass123!";
+ userService.signUp(loginId, rawPassword, "홍길동", LocalDate.of(2000, 1, 15), "test@example.com");
+
+ userService.changePassword(loginId, rawPassword, newPassword);
+
+ assertThatThrownBy(() -> userService.authenticate(loginId, rawPassword))
+ .isInstanceOf(CoreException.class)
+ .satisfies(e -> assertThat(((CoreException) e).getErrorType()).isEqualTo(ErrorType.UNAUTHORIZED));
+ }
+
+ `@Test`
+ void 현재_비밀번호와_동일한_비밀번호로_변경하면_예외() {
+ String loginId = "testuser";
+ String rawPassword = "Test1234!";
+ userService.signUp(loginId, rawPassword, "홍길동", LocalDate.of(2000, 1, 15), "test@example.com");
+
+ assertThatThrownBy(() -> userService.changePassword(loginId, rawPassword, rawPassword))
+ .isInstanceOf(IllegalArgumentException.class);
+ }
+
+ `@Test`
+ void 잘못된_현재_비밀번호로_변경_시도하면_예외() {
+ String loginId = "testuser";
+ userService.signUp(loginId, "Test1234!", "홍길동", LocalDate.of(2000, 1, 15), "test@example.com");
+
+ assertThatThrownBy(() -> userService.changePassword(loginId, "WrongPass1!", "NewPass123!"))
+ .isInstanceOf(CoreException.class)
+ .satisfies(e -> assertThat(((CoreException) e).getErrorType()).isEqualTo(ErrorType.UNAUTHORIZED));
+ }🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java`
around lines 98 - 113, Extend the 비밀번호_변경 nested tests in
UserServiceIntegrationTest to cover failure cases: add a test that after
userService.changePassword(loginId, rawPassword, newPassword) authenticating
with the old rawPassword via userService.authenticate(loginId, rawPassword)
throws the UNAUTHORIZED exception (verifies DB update); add a test that calling
userService.changePassword(loginId, rawPassword, rawPassword) (new password same
as current) throws the expected validation exception; and add a test that
calling userService.changePassword(loginId, "wrongCurrent", "SomeNew1!") throws
UNAUTHORIZED; use the existing signUp setup, and assertions like
assertThatThrownBy / assertThatExceptionOfType against
userService.changePassword and userService.authenticate to validate the expected
exceptions.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java`:
- Around line 72-74: The getMaskedName() method in User throws
StringIndexOutOfBoundsException for empty or null name values; update
User.getMaskedName() to defensively handle null/empty and single-character names
by checking name == null or name.isEmpty() first (return empty string or a
single "*" as appropriate), handle name.length() == 1 by returning "*" and
otherwise return name.substring(0, name.length() - 1) + "*"; this ensures legacy
DB-loaded or JPA-proxied instances that bypass validateName() won't crash.
In
`@apps/commerce-api/src/main/java/com/loopers/support/auth/AuthUserResolver.java`:
- Around line 30-45: AuthUserResolver.resolveArgument currently calls
userService.authenticate(loginId, password) on every request (using
HEADER_LOGIN_ID/HEADER_LOGIN_PW) which forces repeated bcrypt checks; change
this to avoid per-request bcrypt by either switching to session/token-based auth
(perform bcrypt once at login and issue a session or JWT) or add a short-lived
in-memory authentication cache keyed by loginId+password hash (TTL ~30s) so
resolveArgument first checks the cache and only calls userService.authenticate
on a miss, then populate cache with the AuthenticatedUser (used by
AuthenticatedUser.from) to serve subsequent requests; add tests simulating rapid
repeated auth calls to verify latency/CPU improvement.
🧹 Nitpick comments (5)
apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java (1)
30-36:change메서드에서 동일 비밀번호 검증 시 타이밍 공격(timing attack) 가능성을 인지해야 한다.
encoder.matches가 BCrypt 기반이면 내부적으로 constant-time 비교를 수행하므로 현재 구현은 안전하다. 다만,PasswordEncoder구현체가 교체될 경우를 대비해 인터페이스 계약에 constant-time 비교 요구사항을 문서화하는 것을 권장한다.apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordTest.java (1)
67-74: 변경 후 새 비밀번호로 매칭되는지 기능 검증이 누락되어 있다.
isNotSameAs는 참조 동일성만 확인한다. 변경된 Password가 실제로 새 비밀번호와 매칭되는지, 이전 비밀번호와는 매칭되지 않는지 검증해야 변경 로직의 정합성을 보장할 수 있다.추가 검증 제안
Password changed = password.change("Efgh5678!", PASSWORD_ENCODER); assertThat(changed).isNotSameAs(password); + assertThat(changed.matches("Efgh5678!", PASSWORD_ENCODER)).isTrue(); + assertThat(changed.matches("Abcd1234!", PASSWORD_ENCODER)).isFalse();apps/commerce-api/src/main/java/com/loopers/domain/user/User.java (1)
114-116:DateTimeFormatter.ofPattern()을 매 호출마다 생성하고 있다.
DateTimeFormatter는 불변이며 스레드 안전하므로static final상수로 추출하면 불필요한 객체 생성을 제거할 수 있다.수정 제안
+ private static final DateTimeFormatter YYYYMMDD = DateTimeFormatter.ofPattern("yyyyMMdd"); + private static final DateTimeFormatter YYMMDD = DateTimeFormatter.ofPattern("yyMMdd"); + private static final DateTimeFormatter MMDD = DateTimeFormatter.ofPattern("MMdd"); + private static void validatePasswordNotContainsBirthDate(String rawPassword, LocalDate birthDate) { if (rawPassword == null || birthDate == null) { return; } - String yyyyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyyyMMdd")); - String yyMMdd = birthDate.format(DateTimeFormatter.ofPattern("yyMMdd")); - String MMdd = birthDate.format(DateTimeFormatter.ofPattern("MMdd")); + String yyyyMMdd = birthDate.format(YYYYMMDD); + String yyMMdd = birthDate.format(YYMMDD); + String MMdd = birthDate.format(MMDD);apps/commerce-api/src/test/java/com/loopers/domain/user/UserTest.java (1)
127-148: 비밀번호 변경 테스트에서 동일 비밀번호 거부 및 변경 후 매칭 검증이 누락되어 있다.
User.changePassword는 내부적으로Password.change를 호출하므로, User 레벨에서도 동일 비밀번호 변경 거부와 변경 후 새 비밀번호 매칭 여부를 검증하는 테스트가 있어야 도메인 계층 간 통합 정합성을 확인할 수 있다.추가 테스트 제안
+ `@Test` + void 현재_비밀번호와_동일하면_예외() { + User user = User.create("testuser", "Test1234!", "홍길동", + LocalDate.of(2000, 1, 15), "test@example.com", PASSWORD_ENCODER); + + assertThatThrownBy(() -> user.changePassword("Test1234!", PASSWORD_ENCODER)) + .isInstanceOf(CoreException.class) + .hasMessageContaining("현재 비밀번호와 동일한 비밀번호"); + } + + `@Test` + void 변경_후_새_비밀번호로_매칭된다() { + User user = User.create("testuser", "Test1234!", "홍길동", + LocalDate.of(2000, 1, 15), "test@example.com", PASSWORD_ENCODER); + + user.changePassword("NewPass123!", PASSWORD_ENCODER); + + assertThat(user.matchesPassword("NewPass123!", PASSWORD_ENCODER)).isTrue(); + assertThat(user.matchesPassword("Test1234!", PASSWORD_ENCODER)).isFalse(); + }apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
29-33:changePassword에서 현재 비밀번호 확인 없이 변경이 가능하다.현재 흐름은
AuthUserResolver에서X-Loopers-LoginPw헤더로 인증을 수행하므로, 인증 시점의 비밀번호가 곧 현재 비밀번호 확인 역할을 한다. 그러나 이 설계는 다음 문제를 내포한다:
- 인증 헤더의 비밀번호와 "현재 비밀번호 확인"이 암묵적으로 결합되어 있어, 향후 JWT/세션 기반 인증으로 전환 시 현재 비밀번호 검증이 누락될 수 있다.
changePassword메서드 자체만 보면 어떤 인증도 요구하지 않으므로, 다른 진입점에서 호출될 경우 무인증 비밀번호 변경이 가능하다.현재 아키텍처에서는 동작하지만, 메서드에 현재 비밀번호 파라미터를 추가하거나 Javadoc으로 인증 전제 조건을 명시하는 것을 권장한다.
| public String getMaskedName() { | ||
| return name.substring(0, name.length() - 1) + "*"; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getMaskedName()에서 빈 문자열 입력 시 StringIndexOutOfBoundsException이 발생한다.
create 시점에는 validateName이 빈 값을 차단하지만, DB에서 직접 로딩된 레거시 데이터나 JPA 프록시 상태에서 name이 빈 문자열일 경우 substring(0, -1)로 런타임 예외가 발생한다. 방어적 처리를 권장한다.
수정 제안
public String getMaskedName() {
+ if (name == null || name.isEmpty()) {
+ return "*";
+ }
return name.substring(0, name.length() - 1) + "*";
}📝 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 String getMaskedName() { | |
| return name.substring(0, name.length() - 1) + "*"; | |
| } | |
| public String getMaskedName() { | |
| if (name == null || name.isEmpty()) { | |
| return "*"; | |
| } | |
| return name.substring(0, name.length() - 1) + "*"; | |
| } |
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/User.java` around
lines 72 - 74, The getMaskedName() method in User throws
StringIndexOutOfBoundsException for empty or null name values; update
User.getMaskedName() to defensively handle null/empty and single-character names
by checking name == null or name.isEmpty() first (return empty string or a
single "*" as appropriate), handle name.length() == 1 by returning "*" and
otherwise return name.substring(0, name.length() - 1) + "*"; this ensures legacy
DB-loaded or JPA-proxied instances that bypass validateName() won't crash.
| @Override | ||
| public Object resolveArgument(MethodParameter parameter, | ||
| ModelAndViewContainer mavContainer, | ||
| NativeWebRequest webRequest, | ||
| WebDataBinderFactory binderFactory) { | ||
| HttpServletRequest request = (HttpServletRequest) webRequest.getNativeRequest(); | ||
| String loginId = request.getHeader(HEADER_LOGIN_ID); | ||
| String password = request.getHeader(HEADER_LOGIN_PW); | ||
|
|
||
| if (loginId == null || loginId.isBlank() || password == null || password.isBlank()) { | ||
| throw new CoreException(ErrorType.UNAUTHORIZED, "인증 헤더가 필요합니다"); | ||
| } | ||
|
|
||
| User user = userService.authenticate(loginId, password); | ||
| return AuthenticatedUser.from(user); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
매 요청마다 bcrypt 인증을 수행하는 것은 성능 병목이 될 수 있다.
userService.authenticate()가 매 @AuthUser 파라미터 해석 시 호출되므로, bcrypt 비교가 모든 인증 요청에서 반복된다. bcrypt는 의도적으로 느린 해시이므로, 트래픽 증가 시 응답 지연과 CPU 사용률 상승의 원인이 된다.
수정안: 세션 또는 토큰 기반 인증으로 전환하여 bcrypt 비교를 로그인 시점에만 수행하도록 제한하라. 당장 전환이 어렵다면, 짧은 TTL(예: 30초)의 인증 캐시를 도입하여 동일 요청자의 반복 인증 비용을 줄여라.
추가 테스트: 동일 사용자가 짧은 시간 내에 다수의 인증 요청을 보내는 부하 시나리오에서 응답 시간이 허용 범위 내인지 확인하는 성능 테스트를 추가하라.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/support/auth/AuthUserResolver.java`
around lines 30 - 45, AuthUserResolver.resolveArgument currently calls
userService.authenticate(loginId, password) on every request (using
HEADER_LOGIN_ID/HEADER_LOGIN_PW) which forces repeated bcrypt checks; change
this to avoid per-request bcrypt by either switching to session/token-based auth
(perform bcrypt once at login and issue a session or JWT) or add a short-lived
in-memory authentication cache keyed by loginId+password hash (TTL ~30s) so
resolveArgument first checks the cache and only calls userService.authenticate
on a miss, then populate cache with the AuthenticatedUser (used by
AuthenticatedUser.from) to serve subsequent requests; add tests simulating rapid
repeated auth calls to verify latency/CPU improvement.
📌 Summary
🧭 Context & Decision
문제 정의
선택지와 결정
1. 입력값 검증 위치
2. 비밀번호 인코더 의존성 주입
User.create(..., PasswordEncoder encoder)🏗️ Design Overview
변경 범위
apps/commerce-api.claude/)주요 컴포넌트 책임
UserV1Controller: HTTP 요청/응답 변환, 헤더 인증 검증UserFacade: Service 호출 오케스트레이션, Entity→Info 변환UserService: 비즈니스 로직 (중복 체크, 인증, 비밀번호 변경)User: 도메인 불변식 검증, 이름 마스킹, 비밀번호 변경 로직PasswordValidator: 비밀번호 규칙 검증 (8~16자, 생년월일 포함 불가)BcryptPasswordEncoder: 비밀번호 암호화/매칭🔁 Flow Diagram
핵심 플로우
1. 회원가입 (POST /api/v1/users)
2. 내 정보 조회 (GET /api/v1/users/me)
3. 비밀번호 변경 (PATCH /api/v1/users/me/password)
예외 플로우
회원가입 - 비밀번호 규칙 위반 (도메인 불변식)
sequenceDiagram autonumber participant Client participant Controller participant Facade participant Service participant Entity participant PasswordValidator Client->>Controller: POST /api/v1/users Controller->>Facade: signUp(loginId, "1234", name, birthDate, email) Facade->>Service: signUp(...) Service->>Repository: existsByLoginId() → false Service->>Entity: User.create(..., encoder) Entity->>PasswordValidator: validate("1234", birthDate) PasswordValidator->>PasswordValidator: 8자 미만 PasswordValidator--xEntity: IllegalArgumentException Entity--xService: 예외 전파 Service--xFacade: 예외 전파 Facade--xController: 예외 전파 Controller-->>Client: 400 Bad Request회원가입 - 로그인ID 중복
내 정보 조회 - 인증 실패
비밀번호 변경 - 동일 비밀번호
Test Strategy
🔍 리뷰 포인트
1. 도메인 엔티티의 PasswordEncoder 의존성
User.create(..., PasswordEncoder encoder)방식으로 "User가 존재하면 비밀번호는 반드시 인코딩된 상태"라는 불변식을 구조적으로 보장했습니다.Service에서 인코딩 후 전달하는 방식은 인코딩 누락 시 평문 저장 가능성이 있고 이를 컴파일 타임에 잡을 수 없어서 현재 방식을 선택했는데, 이 선택에 대해 의견 부탁드립니다!
2. DTO 검증 vs 도메인 불변식 검증의 에러 응답 형식
입력값 검증을 Request DTO의
@Valid(형식 검증)와 도메인 내부 불변식 검증(비즈니스 규칙)으로 분리했는데, GlobalExceptionHandler에서 처리하는 예외 타입이MethodArgumentNotValidExceptionvsIllegalArgumentException으로 달라집니다.클라이언트에게 내려가는 에러 응답 형식을 통일하는 게 좋을지, 검증 레이어가 다르니 형식이 다른 게 자연스러운 건지 고민입니다.
아니면 현재 과제 scope에 해당하지 않아 넘어가도 좋을지 의견 부탁드립니다!
3. 로그인ID 중복 체크의 동시성 처리
현재
existsByLoginId()체크 후save()하는 구조인데, 동시 요청 시 race condition 가능성이 있습니다. 락보다는 DB unique 제약조건을 최종 방어선으로 두고DataIntegrityViolationException을 GlobalExceptionHandler에서 처리하는 게 현실적이라 판단했는데, 이 접근이 적절한지 의견 부탁드립니다!Checklist
변경 목적:
Volume 1 과제 구현으로 회원가입(POST /api/v1/users), 내 정보 조회(GET /api/v1/users/me, 이름 마스킹 적용), 비밀번호 변경(PATCH /api/v1/users/me/password) 3개 API 완성.
핵심 변경점:
리스크/주의사항:
CodeRabbit 자동 리뷰에서 지적한 프로덕션 배포 전 우선 보완 사항: (1) 평문 비밀번호를 HTTP 헤더로 전송하는 현재 방식의 보안 위험 → JWT/세션 기반 인증 전환 필요, (2) Rate limiting/계정 잠금 정책 부재로 무차별 대입 공격 취약, (3) 보안 감사 로그 부재, (4) 동시 비밀번호 변경 시 동시성 제어 미흡. 현 구현이 기능적 요구사항은 충족하나, 보안/운영 관점의 보강이 필요한 상태로 보임.
테스트/검증 방법:
User/Password 단위 테스트(~270줄, 21개 케이스), UserService 통합 테스트(~136줄), UserApiE2ETest E2E 테스트(~258줄)로 회원가입/인증/비밀번호 변경/내 정보 조회의 성공/실패 시나리오 모두 검증 완료. HTTP 헤더 기반 인증 동작도 E2E에서 확인되었음을 알 수 있음.