-
Notifications
You must be signed in to change notification settings - Fork 44
[volume-1] 회원가입, 내 정보 조회, 비밀번호 변경 기능 구현 #49
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: kimjunyoung90
Are you sure you want to change the base?
[volume-1] 회원가입, 내 정보 조회, 비밀번호 변경 기능 구현 #49
Conversation
|
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
✨ 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.
Actionable comments posted: 14
🤖 Fix all issues with AI agents
In `@apps/commerce-api/build.gradle.kts`:
- Around line 14-15: The build currently depends on
"org.springframework.security:spring-security-crypto" without an explicit
resolved version or SCA in CI; enable Gradle dependency locking by generating
and committing gradle.lockfile, add a springBootVersion property in
gradle.properties (e.g. 3.4.4) so BOM resolution is explicit, record the
resolved version of spring-security-crypto in your project docs or a
DEPENDENCIES.md, and add an SCA step (Snyk or OWASP Dependency-Check) to the CI
pipeline to fail or warn on known vulnerabilities so the dependency and its
versions are regularly scanned and validated.
In
`@apps/commerce-api/src/main/java/com/loopers/user/controller/UserController.java`:
- Around line 41-43: The controller currently accepts currentPassword via
request header (see UserController method signature using
`@RequestHeader`(LOGIN_PW_HEADER) and LOGIN_ID_HEADER) which risks leaking
credentials; change the API to require currentPassword inside
ChangePasswordRequest (add a validated field currentPassword to the
ChangePasswordRequest DTO), remove the `@RequestHeader` parameters and any LOGIN_*
header constants usage from the controller method (update UserController to read
currentPassword from the request body instead), add validation annotations so
missing currentPassword triggers 4xx, and add a controller test that posts a
ChangePasswordRequest with and without currentPassword verifying success and 4xx
failure respectively.
In
`@apps/commerce-api/src/main/java/com/loopers/user/dto/ChangePasswordRequest.java`:
- Around line 3-7: Change ChangePasswordRequest by removing the `@NotBlank`
annotation on newPassword and remove the `@Valid` usage from the relevant
UserController endpoint so DTO-level JSR validation is not used; update
PasswordValidator.validate (or the caller in UserController) to catch
IllegalArgumentException (or any validation exception) and rethrow a
CoreException.badRequest("비밀번호 검증 실패") (or construct CoreException with
BadRequest semantics) so all password validation failures are mapped to a
unified CoreException flow; ensure references to ChangePasswordRequest,
PasswordValidator.validate, CoreException and UserController/@Valid are updated
and add tests verifying null/blank/policy-violating newPassword each return
BAD_REQUEST with the unified ApiResponse format.
In `@apps/commerce-api/src/main/java/com/loopers/user/dto/CreateUserRequest.java`:
- Around line 3-20: Remove all Jakarta Bean Validation annotations from the
CreateUserRequest DTO and centralize format checks in the domain layer:
implement LoginIdValidator, BirthDateValidator, and EmailValidator in
domain/validator (modeled after the existing PasswordValidator) and call them
from UserService.createUser(...) before any repository operations; on validation
failure throw IllegalArgumentException or a domain-specific exception (e.g.,
InvalidFormatException). Also delete CreateUserRequestValidationTest and move
its assertions into UserServiceTest to assert that invalid loginId, birthDate,
email, and password inputs cause the expected exceptions. Ensure method names
referenced are LoginIdValidator.validate(...), BirthDateValidator.validate(...),
EmailValidator.validate(...), and PasswordValidator.validate(...), and that
UserService.createUser(...) invokes them in that order prior to checking
duplicates.
In
`@apps/commerce-api/src/main/java/com/loopers/user/exception/DuplicateLoginIdException.java`:
- Around line 3-13: Replace DuplicateLoginIdException to extend CoreException
(not RuntimeException) and add constructors that accept (String errorCode,
HttpStatus httpStatus, String message) or at minimum preserve the
DEFAULT_MESSAGE while mapping the appropriate error code/HTTP status for
duplicate-login (409); add an additional constructor that accepts a Throwable
cause to preserve original exceptions. Ensure the class references CoreException
and ApiControllerAdvice handling conventions used elsewhere so the common error
body is returned; update/ensure any places that throw new
DuplicateLoginIdException(...) pass the required error code/status signature.
Finally, add an integration test that triggers a duplicate sign-up and asserts a
409 response and that the response body matches the ApiControllerAdvice common
error format.
In
`@apps/commerce-api/src/main/java/com/loopers/user/exception/GlobalExceptionHandler.java`:
- Around line 17-49: GlobalExceptionHandler is returning Map responses and
overriding ApiControllerAdvice due to `@Order`(HIGHEST_PRECEDENCE), causing
inconsistent response shapes; fix by removing GlobalExceptionHandler entirely
(or at minimum delete its MethodArgumentNotValidException handler) and ensure
all custom exceptions (DuplicateLoginIdException, InvalidCredentialsException,
SamePasswordException) either extend CoreException or are wrapped into
CoreException so they flow through ApiControllerAdvice which emits
ApiResponse<?>; after changes add integration tests asserting each exception
yields the ApiResponse<?> standard format and proper HTTP status.
In
`@apps/commerce-api/src/main/java/com/loopers/user/exception/InvalidCredentialsException.java`:
- Around line 3-7: InvalidCredentialsException currently extends
RuntimeException and bypasses the standard error routing; change its superclass
to CoreException so ApiControllerAdvice will handle it, update its constructor
to call super(...) with the existing message and the proper error code/HTTP
mapping per CoreException conventions, and then add/adjust controller or service
tests to assert that authentication failures return the standardized error code
and response schema handled by ApiControllerAdvice (reference:
InvalidCredentialsException, CoreException, ApiControllerAdvice).
In `@apps/commerce-api/src/main/java/com/loopers/user/service/UserService.java`:
- Around line 24-45: Wrap the userRepository.save(user) call in createUser with
a try/catch that catches DataIntegrityViolationException and rethrows
DuplicateLoginIdException so unique-constraint violations during concurrent
signups are mapped to a 409; keep the pre-check using existsByLoginId, but
modify createUser to catch DataIntegrityViolationException around
userRepository.save(user) and throw new DuplicateLoginIdException(), ensuring
PasswordValidator.validate(...) and passwordEncoder.encode(...) call sites
remain unchanged.
In
`@apps/commerce-api/src/main/java/com/loopers/user/validator/PasswordValidator.java`:
- Around line 8-20: The validate method in PasswordValidator should guard
against null or blank passwords: at the start of
PasswordValidator.validate(String password, String birthDate) add a null/blank
check and throw a clear IllegalArgumentException (e.g., "비밀번호는 비어 있을 수 없습니다.")
instead of allowing an NPE; keep this explicit check before length/regex checks
and also ensure birthDate handling remains conditional; update unit tests to
cover password == null and password == "" to assert the exception type and
message.
In
`@apps/commerce-api/src/test/java/com/loopers/user/controller/CreateUserRequestValidationTest.java`:
- Around line 3-115: Remove DTO-level Jakarta Bean Validation and the failing
test: delete validation annotations and this test class
CreateUserRequestValidationTest, and replace it with domain/service-focused
tests that assert the same rules are enforced by the User constructor and
PasswordValidator and that violations surface as the project's CoreException
through the service layer (e.g., tests exercising User (or UserFactory)
construction, PasswordValidator, and the UserService signup method to verify
missing/invalid loginId, password, name, birthDate, email all produce
CoreException); ensure new tests include both unit tests for domain validators
(User constructor, PasswordValidator) and a service-level test (UserService) to
confirm the unified error path.
In `@apps/commerce-api/src/test/java/com/loopers/user/domain/UserTest.java`:
- Around line 12-79: Tests are passing plain-text passwords to
User.builder().password(...) and to changePassword(), but User expects
BCrypt-hashed passwords; update tests to hash all test passwords using
BCryptPasswordEncoder (or a test helper like hashPassword(String)) before
passing them into User.builder() and before calling user.changePassword(...),
and assert against the hashed value or adapt assertions to verify BCrypt matches
(e.g., use BCryptPasswordEncoder.matches) so the tests exercise the same
hashed-password invariants as the production flow (targets: UserTest,
User.builder(), changePassword(), getPassword()).
In `@apps/commerce-api/src/test/java/com/loopers/user/UserE2ETest.java`:
- Around line 126-255: Replace the time-based loginId generation with a
UUID-based unique id to avoid flakiness; update each test that currently builds
loginId like "pwchg" + (System.currentTimeMillis() % ...) to use
UUID.randomUUID() (or a small helper method e.g., createUniqueLoginId()) and run
the same CreateUserRequest flow unchanged; optionally add a short additional
test that creates many users in quick succession to assert no collisions occur
when using the new UUID-based ids.
- Around line 22-25: Remove class-level `@Transactional` from UserE2ETest and
implement explicit test data cleanup to guarantee isolation: add an `@AfterEach`
method that calls userRepository.deleteAll() (or annotate tests with `@Sql`(...,
executionPhase = AFTER_TEST_METHOD)) so data created by the embedded server
threads is removed after each test; also replace all usages that generate
loginId via System.currentTimeMillis() % N with a collision-safe generator
(e.g., UUID.randomUUID().toString() or an atomic sequence) in the tests that
create users so duplicate loginId races (seen where loginId is computed with
System.currentTimeMillis()) are eliminated and tests like the duplicate-loginId
check and "my profile" retrieval become stable.
🧹 Nitpick comments (3)
modules/jpa/src/main/java/com/loopers/config/jpa/JpaConfig.java (1)
10-11:@EnableJpaRepositories패키지 스캔 범위의 유지보수성 문제
@EntityScan은com.loopers전체를 스캔하지만,@EnableJpaRepositories는 개별 패키지를 명시적으로 나열하고 있다. 이 방식은 새로운 도메인 리포지토리(예:com.loopers.order.repository,com.loopers.product.repository)가 추가될 때마다 이 설정을 수동으로 변경해야 하므로 누락 실수로 인한 런타임 오류 위험이 있다.운영 관점에서 설정 누락은 배포 후 특정 기능 장애로 이어질 수 있으므로, 일관된 패키지 스캔 전략을 권장한다.
♻️ 권장 수정안: 베이스 패키지 통일
`@EntityScan`({"com.loopers"}) -@EnableJpaRepositories({"com.loopers.infrastructure", "com.loopers.user.repository"}) +@EnableJpaRepositories({"com.loopers"})또는 명시적 제어가 필요한 경우, 리포지토리 패키지 명명 규칙을 문서화하고 통합 테스트에서 리포지토리 빈 등록 여부를 검증하는 테스트를 추가하는 것을 권장한다.
`@Test` void contextLoads_allRepositoriesRegistered() { assertThat(applicationContext.getBean(UserRepository.class)).isNotNull(); // 새 리포지토리 추가 시 여기에도 검증 추가 }apps/commerce-api/src/test/java/com/loopers/user/validator/PasswordValidatorTest.java (1)
13-60: 경계값 ‘정상 통과’ 케이스가 빠져 있다운영 관점에서 성공 경계 테스트가 없으면 정책 변경 시 허용 범위가 조용히 깨져 실제 사용자 비밀번호가 불필요하게 거절/허용되는 장애로 이어질 수 있다.
수정안으로 최소(8자)·최대(16자) 길이와 허용 문자 조합에서 예외가 발생하지 않는 성공 테스트를 추가하는 것이 필요하다.
추가 테스트로 생년월일 미포함 시 정상 통과하는 케이스를 명시적으로 검증하는 것이 필요하다.🧪 테스트 보강 예시
@@ public class PasswordValidatorTest { @@ `@Test` void 비밀번호에_생년월일_포함시_IllegalArgumentException이_발생한다() { @@ assertThat(thrown).isInstanceOf(IllegalArgumentException.class); } + + `@ParameterizedTest` + `@ValueSource`(strings = {"Ab1!abcd", "Ab1!abcdEf2@Gh3X"}) + void 비밀번호_경계값_정상_통과한다(String password) { + //when + Throwable thrown = catchThrowable(() -> PasswordValidator.validate(password, "1990-04-27")); + + //then + assertThat(thrown).isNull(); + } + + `@Test` + void 비밀번호에_생년월일_미포함시_정상_통과한다() { + //when + Throwable thrown = catchThrowable(() -> PasswordValidator.validate("Ab1!abcd", "1990-04-27")); + + //then + assertThat(thrown).isNull(); + } }apps/commerce-api/src/main/java/com/loopers/user/exception/SamePasswordException.java (1)
3-13: cause 보존 생성자를 추가하는 것이 좋다운영 관점에서 비밀번호 변경 실패가 다른 내부 예외를 래핑하는 경우 cause가 사라지면 로그 상관관계가 끊겨 장애 분석 시간이 늘어난다.
수정안으로 (String message, Throwable cause) 생성자를 추가해 상위 레이어에서 원인 예외를 보존하도록 개선하는 것이 필요하다.
추가 테스트로 UserServiceTest에서 cause 전달 후 getCause가 유지되는지 검증하는 단위 테스트를 보강하는 것이 필요하다.As per coding guidelines "예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다."🔧 개선 예시
public class SamePasswordException extends RuntimeException { @@ public SamePasswordException(String message) { super(message); } + + public SamePasswordException(String message, Throwable cause) { + super(message, cause); + } }
| // security | ||
| implementation("org.springframework.security:spring-security-crypto") |
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:
# 1. build.gradle.kts 파일 위치 및 내용 확인
find . -name "build.gradle.kts" -type f | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 429
🏁 Script executed:
# 2. apps/commerce-api 디렉토리 구조 확인
ls -la apps/commerce-api/ 2>/dev/null || echo "Directory not found"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 314
🏁 Script executed:
# 3. 프로젝트 루트 build.gradle.kts 확인
cat -n build.gradle.kts 2>/dev/null || echo "Not found in root"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4538
🏁 Script executed:
# 4. libs.versions.toml 파일 검색
find . -name "libs.versions.toml" -o -name "gradle.properties" -o -name "settings.gradle.kts" | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 126
🏁 Script executed:
# gradle.properties 확인
cat -n gradle.propertiesRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 747
🏁 Script executed:
# apps/commerce-api/build.gradle.kts 전체 내용 확인
cat -n apps/commerce-api/build.gradle.ktsRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1269
🏁 Script executed:
# settings.gradle.kts 확인
cat -n settings.gradle.ktsRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1230
🏁 Script executed:
# gradle.lockfile 확인
find . -name "gradle.lockfile" -o -name "*.lockfile" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# CI 설정 확인
find . -type f \( -name "*.yml" -o -name "*.yaml" \) -path "*/.github/*" | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 110
🏁 Script executed:
# 다른 apps의 보안 관련 의존성 버전 확인
rg -n "spring-security|spring-boot-starter-security" apps/ --type gradleRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# 프로젝트 전체에서 명시적으로 버전을 지정한 의존성 패턴 확인
rg -n 'implementation\(.*:[^"]*:[^"]*"\)' root build.gradle.kts apps/ modules/ supports/ --type gradle | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 129
🏁 Script executed:
# Spring Boot 3.4.4 BOM에서 spring-security 버전 확인
curl -s https://repo.maven.org/maven2/org/springframework/boot/spring-boot-dependencies/3.4.4/spring-boot-dependencies-3.4.4.pom 2>/dev/null | grep -A 2 "spring-security" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# 다른 모듈/앱에서 보안 관련 의존성이 어떻게 관리되는지 확인
find . -name "build.gradle.kts" -type f -exec grep -l "spring-security\|crypto" {} \;Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 121
🏁 Script executed:
# .github 디렉토리 전체 구조 확인
find .github -type f 2>/dev/null | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 141
명시적 버전 관리와 보안 검증 프로세스 추가
spring-security-crypto는 Spring Boot BOM을 통해 관리되는 표준 방식이나, 실제 해석되는 버전을 추적하고 보안 패치 적용을 보증하기 위해 다음을 수행해야 한다.
- Gradle 의존성 잠금 파일(gradle.lockfile) 도입으로 의존성 버전을 명시적으로 고정하고, 정기적으로 갱신 검증을 수행한다.
- CI 파이프라인에 의존성 취약점 스캔(SCA, 예: Snyk, OWASP Dependency-Check)을 추가하여 보안 취약점을 자동으로 감지한다.
- gradle.properties에서 Spring Boot 버전(3.4.4)을 명시하고, 별도 문서에 spring-security-crypto의 결정된 버전을 기록한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/build.gradle.kts` around lines 14 - 15, The build currently
depends on "org.springframework.security:spring-security-crypto" without an
explicit resolved version or SCA in CI; enable Gradle dependency locking by
generating and committing gradle.lockfile, add a springBootVersion property in
gradle.properties (e.g. 3.4.4) so BOM resolution is explicit, record the
resolved version of spring-security-crypto in your project docs or a
DEPENDENCIES.md, and add an SCA step (Snyk or OWASP Dependency-Check) to the CI
pipeline to fail or warn on known vulnerabilities so the dependency and its
versions are regularly scanned and validated.
| private final UserService userService; | ||
|
|
||
| @PostMapping | ||
| public ResponseEntity<CreateUserResponse> createUser(@Valid @RequestBody CreateUserRequest request) { |
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 apps/commerce-api -name "UserController.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 164
🏁 Script executed:
find apps/commerce-api -name "*ApiControllerAdvice*" -o -name "*ControllerAdvice*" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 168
🏁 Script executed:
find apps/commerce-api -path "*/config/*" -name "*.java" | grep -i adviceRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/user/controller/UserController.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2195
🏁 Script executed:
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: 6820
🏁 Script executed:
find apps/commerce-api -name "CreateUserRequest.java" -o -name "ChangePasswordRequest.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 240
🏁 Script executed:
find apps/commerce-api -path "*/test/*" -name "*UserController*Test*.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 168
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/user/dto/CreateUserRequest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 765
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/user/dto/ChangePasswordRequest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 315
🏁 Script executed:
cat -n apps/commerce-api/src/test/java/com/loopers/user/controller/UserControllerTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5536
🏁 Script executed:
find apps/commerce-api -name "GlobalExceptionHandler.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 171
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/user/exception/GlobalExceptionHandler.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2606
🏁 Script executed:
find apps/commerce-api -name "pom.xml" -o -name "build.gradle" -o -name "build.gradle.kts" | head -1 | xargs cat | grep -A 5 -B 5 "spring-boot\|spring-webmvc" | head -40Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 606
🏁 Script executed:
rg "MethodArgumentNotValidException" apps/commerce-apiRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 576
@Valid 제거하고 응답 포맷 일관성 확보 필요
@Valid 사용으로 MethodArgumentNotValidException이 GlobalExceptionHandler를 통해 Map 형식으로 응답하나, 다른 에러는 ApiControllerAdvice의 ApiResponse 형식으로 반환되어 클라이언트 파싱 일관성이 깨진다. 응답 포맷을 ApiResponse로 통일하거나, 요청값 검증을 서비스/도메인 계층(User 생성자, PasswordPolicy)으로 이동하여 모든 에러가 CoreException을 거쳐 ApiControllerAdvice로 처리되도록 개선해야 한다.
또한 비밀번호를 X-Loopers-LoginPw 헤더로 전달하면 로그, 프록시, APM에서 민감정보가 노출될 수 있으므로 요청 본문으로 이동하는 것이 필요하다.
수정 제안
-import jakarta.validation.Valid;
@@
- public ResponseEntity<CreateUserResponse> createUser(`@Valid` `@RequestBody` CreateUserRequest request) {
+ public ResponseEntity<CreateUserResponse> createUser(`@RequestBody` CreateUserRequest request) {
@@
- public ResponseEntity<Void> changePassword(
+ public ResponseEntity<Void> changePassword(
`@RequestHeader`(LOGIN_ID_HEADER) String loginId,
- `@RequestHeader`(LOGIN_PW_HEADER) String currentPassword,
- `@Valid` `@RequestBody` ChangePasswordRequest request
+ `@RequestBody` ChangePasswordRequest request
) {
- userService.changePassword(loginId, currentPassword, request.newPassword());
+ userService.changePassword(loginId, request.currentPassword(), request.newPassword());| @RequestHeader(LOGIN_ID_HEADER) String loginId, | ||
| @RequestHeader(LOGIN_PW_HEADER) String currentPassword, | ||
| @Valid @RequestBody ChangePasswordRequest request |
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.
비밀번호를 헤더로 전달하면 노출 위험이 큼
운영 관점에서 요청 헤더는 게이트웨이/프록시/접근 로그/APM에 기본 수집되는 경우가 많아 비밀번호가 평문으로 남을 위험이 크다.
수정안으로 ChangePasswordRequest에 currentPassword를 추가해 바디로 전달하고 헤더 파라미터 및 상수를 제거하는 방식이 안전하다.
추가 테스트로 currentPassword가 바디에 있을 때만 성공하고 누락 시 4xx로 실패하는지 컨트롤러 테스트를 추가하는 것이 필요하다.
수정 제안
- public ResponseEntity<Void> changePassword(
- `@RequestHeader`(LOGIN_ID_HEADER) String loginId,
- `@RequestHeader`(LOGIN_PW_HEADER) String currentPassword,
- `@RequestBody` ChangePasswordRequest request
- ) {
- userService.changePassword(loginId, currentPassword, request.newPassword());
+ public ResponseEntity<Void> changePassword(
+ `@RequestHeader`(LOGIN_ID_HEADER) String loginId,
+ `@RequestBody` ChangePasswordRequest request
+ ) {
+ userService.changePassword(loginId, request.currentPassword(), request.newPassword());
return ResponseEntity.ok().build();
}As per coding guidelines '로깅 시 민감정보 노출 가능성을 점검한다.'를 따랐다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/user/controller/UserController.java`
around lines 41 - 43, The controller currently accepts currentPassword via
request header (see UserController method signature using
`@RequestHeader`(LOGIN_PW_HEADER) and LOGIN_ID_HEADER) which risks leaking
credentials; change the API to require currentPassword inside
ChangePasswordRequest (add a validated field currentPassword to the
ChangePasswordRequest DTO), remove the `@RequestHeader` parameters and any LOGIN_*
header constants usage from the controller method (update UserController to read
currentPassword from the request body instead), add validation annotations so
missing currentPassword triggers 4xx, and add a controller test that posts a
ChangePasswordRequest with and without currentPassword verifying success and 4xx
failure respectively.
| import jakarta.validation.constraints.NotBlank; | ||
|
|
||
| public record ChangePasswordRequest( | ||
| @NotBlank | ||
| String newPassword |
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. ChangePasswordRequest 파일 확인
find . -name "ChangePasswordRequest.java" -type f | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 166
🏁 Script executed:
#!/bin/bash
# 2. 프로젝트 전체 DTO의 Bean Validation 사용 패턴 확인
rg -n '@NotBlank|@Email|@Pattern|@Valid|jakarta\.validation' -g '*.java' -A 2 -B 2 | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 6330
🏁 Script executed:
#!/bin/bash
# 3. 프로젝트 구조와 User 관련 서비스/도메인 파일 확인
find . -path '*/user/*' -name '*.java' -type f | grep -E '(Service|Domain|Policy)' | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 331
🏁 Script executed:
#!/bin/bash
# 4. UserService의 changePassword 메서드 확인
cat -n ./apps/commerce-api/src/main/java/com/loopers/user/service/UserService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3194
🏁 Script executed:
#!/bin/bash
# 5. PasswordPolicy 파일 찾기 및 내용 확인
find . -name "*PasswordPolicy*" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
#!/bin/bash
# 6. 에러 핸들링 구조 확인 - CoreException과 ApiControllerAdvice 패턴
find . -name "*ControllerAdvice*" -o -name "*CoreException*" | head -10Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 332
🏁 Script executed:
#!/bin/bash
# 7. ApiControllerAdvice 확인 - 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: 6820
🏁 Script executed:
#!/bin/bash
# 8. PasswordValidator 확인
find . -name "PasswordValidator.java" -type f | xargs cat -nRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1039
ChangePasswordRequest의 @NotBlank를 제거하고, PasswordValidator 예외를 CoreException으로 매핑해야 한다
@NotBlank는 MethodArgumentNotValidException을 유발하는데, ApiControllerAdvice에 이를 처리하는 핸들러가 없어서 generic Throwable 핸들러에 의해 INTERNAL_ERROR로 처리된다. 또한 도메인 계층의 PasswordValidator.validate()도 IllegalArgumentException을 던져서 동일하게 INTERNAL_ERROR로 처리되므로, 검증 에러의 응답 상태 코드와 형식이 일관되지 않는다.
수정안으로 다음 항목이 필요하다:
- DTO에서
@NotBlank제거 - PasswordValidator 예외를 CoreException(BadRequest, "비밀번호 검증 실패") 등으로 래핑하여 통일된 에러 핸들링 흐름으로 변경
- UserController의
@Valid제거
추가 테스트로 newPassword가 null/blank/정책 위배인 경우 각각 BAD_REQUEST 상태 코드와 통일된 ApiResponse 형식으로 응답되는지 검증해야 한다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/user/dto/ChangePasswordRequest.java`
around lines 3 - 7, Change ChangePasswordRequest by removing the `@NotBlank`
annotation on newPassword and remove the `@Valid` usage from the relevant
UserController endpoint so DTO-level JSR validation is not used; update
PasswordValidator.validate (or the caller in UserController) to catch
IllegalArgumentException (or any validation exception) and rethrow a
CoreException.badRequest("비밀번호 검증 실패") (or construct CoreException with
BadRequest semantics) so all password validation failures are mapped to a
unified CoreException flow; ensure references to ChangePasswordRequest,
PasswordValidator.validate, CoreException and UserController/@Valid are updated
and add tests verifying null/blank/policy-violating newPassword each return
BAD_REQUEST with the unified ApiResponse format.
| import jakarta.validation.constraints.Email; | ||
| import jakarta.validation.constraints.NotBlank; | ||
| import jakarta.validation.constraints.Pattern; | ||
|
|
||
| public record CreateUserRequest( | ||
| @NotBlank | ||
| @Pattern(regexp = "^[a-zA-Z0-9]+$") | ||
| String loginId, | ||
| @NotBlank | ||
| String password, | ||
| @NotBlank | ||
| String name, | ||
| @NotBlank | ||
| @Pattern(regexp = "\\d{4}-\\d{2}-\\d{2}") | ||
| String birthDate, | ||
| @NotBlank | ||
| 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:
# User 엔티티 찾기
fd -e java User | grep -E '(entity|domain)' | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 220
🏁 Script executed:
# CreateUserRequest 파일 확인
fd CreateUserRequest -e javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 257
🏁 Script executed:
# UserService 찾기
fd UserService -e javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 325
🏁 Script executed:
# 프로젝트의 Bean Validation 사용 현황 전체 파악
rg 'jakarta\.validation|@NotBlank|@Email|@Pattern' -g '*.java' -A 2 -B 2 | head -80Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4839
🏁 Script executed:
# 다른 DTO들의 검증 패턴 확인
fd -e java -path '*/dto/*' | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 253
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/user/domain/User.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2253
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/user/service/UserService.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3194
🏁 Script executed:
cat -n apps/commerce-api/src/test/java/com/loopers/user/controller/CreateUserRequestValidationTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5154
🏁 Script executed:
fd PasswordValidator -e javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 252
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/user/validator/PasswordValidator.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1039
DTO의 Bean Validation 어노테이션을 제거하고 도메인 레이어 검증으로 일원화해야 한다
현재 @NotBlank, @Pattern, @Email은 @Valid 경로에서만 MethodArgumentNotValidException을 발생시키므로, 컨트롤러를 거치지 않고 UserService를 직접 호출하는 다른 진입점에서는 loginId·email·birthDate 형식 검증이 누락된다. 이는 CoreException 통일 경로를 벗어나 일관성 없는 에러 응답을 야기한다.
수정안으로 다음을 진행해야 한다:
- DTO에서 검증 어노테이션 제거
- PasswordValidator 패턴을 따라 LoginIdValidator, BirthDateValidator, EmailValidator를 domain/validator 패키지에 작성하고 UserService.createUser()에서 호출
- 형식 검증 실패 시 IllegalArgumentException 또는 도메인 예외 발생
- CreateUserRequestValidationTest는 제거하고, UserServiceTest에서 잘못된 형식 입력 시 예외 발생 여부를 검증
🛠️ 수정 예시
-import jakarta.validation.constraints.Email;
-import jakarta.validation.constraints.NotBlank;
-import jakarta.validation.constraints.Pattern;
-
public record CreateUserRequest(
- `@NotBlank`
- `@Pattern`(regexp = "^[a-zA-Z0-9]+$")
String loginId,
- `@NotBlank`
String password,
- `@NotBlank`
String name,
- `@NotBlank`
- `@Pattern`(regexp = "\\d{4}-\\d{2}-\\d{2}")
String birthDate,
- `@Email`
- `@NotBlank`
String email
) {
}UserService.createUser()에 검증 로직 추가:
public User createUser(CreateUserRequest request) {
LoginIdValidator.validate(request.loginId()); // 추가
BirthDateValidator.validate(request.birthDate()); // 추가
EmailValidator.validate(request.email()); // 추가
if(userRepository.existsByLoginId(request.loginId())){
throw new DuplicateLoginIdException();
}
PasswordValidator.validate(request.password(), request.birthDate());
// ...
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/user/dto/CreateUserRequest.java`
around lines 3 - 20, Remove all Jakarta Bean Validation annotations from the
CreateUserRequest DTO and centralize format checks in the domain layer:
implement LoginIdValidator, BirthDateValidator, and EmailValidator in
domain/validator (modeled after the existing PasswordValidator) and call them
from UserService.createUser(...) before any repository operations; on validation
failure throw IllegalArgumentException or a domain-specific exception (e.g.,
InvalidFormatException). Also delete CreateUserRequestValidationTest and move
its assertions into UserServiceTest to assert that invalid loginId, birthDate,
email, and password inputs cause the expected exceptions. Ensure method names
referenced are LoginIdValidator.validate(...), BirthDateValidator.validate(...),
EmailValidator.validate(...), and PasswordValidator.validate(...), and that
UserService.createUser(...) invokes them in that order prior to checking
duplicates.
| public static void validate(String password, String birthDate) { | ||
| if (password.length() < 8) { | ||
| throw new IllegalArgumentException("비밀번호는 8자 이상이어야 합니다."); | ||
| } | ||
| if (password.length() > 16) { | ||
| throw new IllegalArgumentException("비밀번호는 16자 이하여야 합니다."); | ||
| } | ||
| String pattern = "^[a-zA-Z0-9!@#$%^&*()_+\\-=\\[\\]{};':\"\\\\|,.<>/?]+$"; | ||
| if (!password.matches(pattern)) { | ||
| throw new IllegalArgumentException("비밀번호는 영문 대소문자, 숫자, 특수문자만 허용됩니다."); | ||
| } | ||
| if (birthDate != null && password.contains(birthDate)) { | ||
| 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.
null 비밀번호 입력 시 NPE가 발생하므로 가드가 필요하다
운영 관점에서 null 입력이 들어오면 NPE로 500 응답이 발생해 장애 분석이 어려워지고 클라이언트가 올바른 피드백을 받지 못한다.
수정안으로 null/blank 체크를 가장 앞에서 수행하고 명시적 예외를 던져야 한다.
추가 테스트로 password가 null 또는 빈 문자열인 경우 예외 메시지와 상태가 기대대로인지 검증해야 한다.
🛠️ 수정 예시
public static void validate(String password, String birthDate) {
+ if (password == null || password.isBlank()) {
+ throw new IllegalArgumentException("비밀번호는 필수값이다.");
+ }
if (password.length() < 8) {
throw new IllegalArgumentException("비밀번호는 8자 이상이어야 합니다.");
}As per coding guidelines: Java + Spring Boot 리뷰 기준으로 null 처리와 예외 흐름의 명확성을 점검해야 한다고 명시되어 있다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/user/validator/PasswordValidator.java`
around lines 8 - 20, The validate method in PasswordValidator should guard
against null or blank passwords: at the start of
PasswordValidator.validate(String password, String birthDate) add a null/blank
check and throw a clear IllegalArgumentException (e.g., "비밀번호는 비어 있을 수 없습니다.")
instead of allowing an NPE; keep this explicit check before length/regex checks
and also ensure birthDate handling remains conditional; update unit tests to
cover password == null and password == "" to assert the exception type and
message.
| import com.loopers.user.dto.CreateUserRequest; | ||
| import jakarta.validation.ConstraintViolation; | ||
| import jakarta.validation.Validation; | ||
| import jakarta.validation.Validator; | ||
| import jakarta.validation.constraints.Email; | ||
| import jakarta.validation.constraints.Pattern; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
| import org.junit.jupiter.params.provider.Arguments; | ||
| import org.junit.jupiter.params.provider.MethodSource; | ||
|
|
||
| import java.util.Set; | ||
| import java.util.stream.Stream; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| public class CreateUserRequestValidationTest { | ||
|
|
||
| private Validator validator; | ||
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| validator = Validation.buildDefaultValidatorFactory().getValidator(); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @MethodSource("필수값_누락_케이스") | ||
| void 회원가입시_필수정보를_입력하지_않으면_실패한다(CreateUserRequest request, String expectedField) { | ||
| //given | ||
|
|
||
| //when | ||
| Set<ConstraintViolation<CreateUserRequest>> violations = validator.validate(request); | ||
|
|
||
| //then | ||
| assertThat(violations).hasSize(1); | ||
| assertThat(violations.iterator().next().getPropertyPath().toString()).isEqualTo(expectedField); | ||
| } | ||
|
|
||
| static Stream<Arguments> 필수값_누락_케이스() { | ||
| return Stream.of( | ||
| Arguments.of(new CreateUserRequest(null, "pw", "name", "1990-01-01", "a@a.com"), "loginId"), | ||
| Arguments.of(new CreateUserRequest("test", null, "name", "1990-01-01", "a@a.com"), "password"), | ||
| Arguments.of(new CreateUserRequest("test", "pw", null, "1990-01-01", "a@a.com"), "name"), | ||
| Arguments.of(new CreateUserRequest("test", "pw", "name", null, "a@a.com"), "birthDate"), | ||
| Arguments.of(new CreateUserRequest("test", "pw", "name", "1990-01-01", null), "email") | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void 이메일_형식_불일치_시_실패() { | ||
| //given | ||
| String id = "test"; | ||
| String password = "pw"; | ||
| String name = "name"; | ||
| String birthDate = "1990-01-01"; | ||
| String email = "test123"; | ||
| CreateUserRequest request = new CreateUserRequest(id, password, name, birthDate, email); | ||
|
|
||
| //when | ||
| Set<ConstraintViolation<CreateUserRequest>> violations = validator.validate(request); | ||
|
|
||
| //then | ||
| assertThat(violations).hasSize(1); | ||
| ConstraintViolation<CreateUserRequest> violation = violations.iterator().next(); | ||
| assertThat(violation.getPropertyPath().toString()).isEqualTo("email"); | ||
| assertThat(violation.getConstraintDescriptor() | ||
| .getAnnotation() | ||
| .annotationType()) | ||
| .isEqualTo(Email.class); | ||
| } | ||
|
|
||
| @Test | ||
| void 생년월일_형식_불일치_시_실패() { | ||
| //given | ||
| String id = "test"; | ||
| String password = "pw"; | ||
| String name = "name"; | ||
| String birthDate = "19900427"; | ||
| String email = "test123@test.com"; | ||
| CreateUserRequest request = new CreateUserRequest(id, password, name, birthDate, email); | ||
|
|
||
| //when | ||
| Set<ConstraintViolation<CreateUserRequest>> violations = validator.validate(request); | ||
|
|
||
| //then | ||
| assertThat(violations).hasSize(1); | ||
| ConstraintViolation<CreateUserRequest> violation = violations.iterator().next(); | ||
| assertThat(violation.getPropertyPath().toString()).isEqualTo("birthDate"); | ||
| } | ||
|
|
||
| @Test | ||
| void 로그인ID에_영문_숫자_외_문자_포함_시_실패() { | ||
| //given | ||
| String id = "test@123"; | ||
| String password = "pw"; | ||
| String name = "name"; | ||
| String birthDate = "1990-01-01"; | ||
| String email = "test@test.com"; | ||
| CreateUserRequest request = new CreateUserRequest(id, password, name, birthDate, email); | ||
|
|
||
| //when | ||
| Set<ConstraintViolation<CreateUserRequest>> violations = validator.validate(request); | ||
|
|
||
| //then | ||
| assertThat(violations).hasSize(1); | ||
| ConstraintViolation<CreateUserRequest> violation = violations.iterator().next(); | ||
| assertThat(violation.getPropertyPath().toString()).isEqualTo("loginId"); | ||
| assertThat(violation.getConstraintDescriptor() | ||
| .getAnnotation() | ||
| .annotationType()) | ||
| .isEqualTo(Pattern.class); | ||
| } |
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.
DTO Bean Validation 테스트는 프로젝트 검증 규약과 충돌한다
운영 관점에서 DTO 레벨 Bean Validation에 의존하면 서비스/배치 등 다른 진입점에서 검증이 누락되어 동일 입력이 서로 다른 응답 포맷으로 처리되는 문제가 발생할 수 있다.
수정안으로 CreateUserRequest의 Jakarta Validation 의존과 본 테스트를 제거하고, User/PasswordValidator/서비스 레이어에서 검증이 수행되도록 테스트를 도메인/서비스 단으로 재작성하는 것이 필요하다.
추가 테스트로 회원가입 시 필수값 누락/형식 오류가 도메인 또는 서비스에서 동일한 CoreException 경로로 처리되는지 서비스 단위 테스트와 API 통합 테스트를 보강하는 것이 필요하다.
Based on learnings "This project (loop-pack-be-l2-vol3-java) intentionally avoids Bean Validation annotations (Valid, NotBlank, Email, etc.) in DTOs. All validation is performed in the domain layer (User entity constructor, PasswordPolicy) following hexagonal architecture principles."
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/user/controller/CreateUserRequestValidationTest.java`
around lines 3 - 115, Remove DTO-level Jakarta Bean Validation and the failing
test: delete validation annotations and this test class
CreateUserRequestValidationTest, and replace it with domain/service-focused
tests that assert the same rules are enforced by the User constructor and
PasswordValidator and that violations surface as the project's CoreException
through the service layer (e.g., tests exercising User (or UserFactory)
construction, PasswordValidator, and the UserService signup method to verify
missing/invalid loginId, password, name, birthDate, email all produce
CoreException); ensure new tests include both unit tests for domain validators
(User constructor, PasswordValidator) and a service-level test (UserService) to
confirm the unified error path.
| User user = User.builder() | ||
| .loginId("testId") | ||
| .password("password123!") | ||
| .name("홍길동") | ||
| .birthDate("1990-01-01") | ||
| .email("test@test.com") | ||
| .build(); | ||
|
|
||
| // when | ||
| String maskedName = user.getMaskedName(); | ||
|
|
||
| // then | ||
| assertThat(maskedName).isEqualTo("홍길*"); | ||
| } | ||
|
|
||
| @Test | ||
| void 한_글자_이름은_마스킹_문자로_반환된다() { | ||
| // given | ||
| User user = User.builder() | ||
| .loginId("testId") | ||
| .password("password123!") | ||
| .name("김") | ||
| .birthDate("1990-01-01") | ||
| .email("test@test.com") | ||
| .build(); | ||
|
|
||
| // when | ||
| String maskedName = user.getMaskedName(); | ||
|
|
||
| // then | ||
| assertThat(maskedName).isEqualTo("*"); | ||
| } | ||
|
|
||
| @Test | ||
| void 두_글자_이름의_마지막_글자가_마스킹된다() { | ||
| // given | ||
| User user = User.builder() | ||
| .loginId("testId") | ||
| .password("password123!") | ||
| .name("이순") | ||
| .birthDate("1990-01-01") | ||
| .email("test@test.com") | ||
| .build(); | ||
|
|
||
| // when | ||
| String maskedName = user.getMaskedName(); | ||
|
|
||
| // then | ||
| assertThat(maskedName).isEqualTo("이*"); | ||
| } | ||
|
|
||
| @Test | ||
| void changePassword로_비밀번호가_변경된다() { | ||
| // given | ||
| User user = User.builder() | ||
| .loginId("testId") | ||
| .password("oldPassword123!") | ||
| .name("홍길동") | ||
| .birthDate("1990-01-01") | ||
| .email("test@test.com") | ||
| .build(); | ||
| String newPassword = "newPassword456!"; | ||
|
|
||
| // when | ||
| user.changePassword(newPassword); | ||
|
|
||
| // then | ||
| assertThat(user.getPassword()).isEqualTo(newPassword); |
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:
# User.java 파일 찾기 및 내용 확인
fd -t f "User.java" --exclude "*Test*" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 150
🏁 Script executed:
# UserTest.java 파일 전체 내용 확인
fd -t f "UserTest.java" apps/commerce-apiRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 154
🏁 Script executed:
# UserService.java 파일 찾기
fd -t f "UserService.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 158
🏁 Script executed:
# 생성자 또는 password 관련 로직 검색
rg "password" -A 5 -B 2 apps/commerce-api/src/main/java/com/loopers/domain/user/User.java | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 190
🏁 Script executed:
# User.java 파일 라인 수 확인 후 내용 읽기
wc -l apps/commerce-api/src/main/java/com/loopers/user/domain/User.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 153
🏁 Script executed:
# UserTest.java 파일 라인 수 확인 후 내용 읽기
wc -l apps/commerce-api/src/test/java/com/loopers/user/domain/UserTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 157
🏁 Script executed:
# UserService.java에서 password 암호화 관련 로직 검색
rg "password|encode|encrypt" -A 3 -B 3 apps/commerce-api/src/main/java/com/loopers/user/service/UserService.java | head -150Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1648
🏁 Script executed:
# User.java 전체 내용 읽기
cat -n apps/commerce-api/src/main/java/com/loopers/user/domain/User.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2253
🏁 Script executed:
# UserTest.java 전체 내용 읽기
cat -n apps/commerce-api/src/test/java/com/loopers/user/domain/UserTest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2652
테스트 데이터도 BCrypt 해시 불변식을 따라야 한다
User 엔티티의 생성자와 changePassword()는 항상 BCrypt 해시된 비밀번호를 받으며, 운영 환경의 UserService에서도 PasswordValidator로 검증한 후 반드시 암호화하여 전달한다. 그러나 테스트 코드는 평문 비밀번호로 작성되어 실제 운영 경로와 다른 검증 흐름을 테스트하므로, 향후 해시 포맷이나 암호화 정책이 추가되면 테스트가 실패하거나 무의미해질 수 있다.
BCryptPasswordEncoder를 이용해 테스트 데이터를 해시하거나, 테스트 헬퍼 메서드로 해시 생성 로직을 분리해 모든 password 파라미터에 적용해야 한다.
🛠️ 수정 예시
+import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
+
class UserTest {
+
+ private static final BCryptPasswordEncoder encoder = new BCryptPasswordEncoder();
+
+ private static String hashed(String raw) {
+ return encoder.encode(raw);
+ }
@@
- .password("password123!")
+ .password(hashed("password123!"))
@@
- .password("oldPassword123!")
+ .password(hashed("oldPassword123!"))
@@
- String newPassword = "newPassword456!";
+ String newPassword = hashed("newPassword456!");🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/user/domain/UserTest.java` around
lines 12 - 79, Tests are passing plain-text passwords to
User.builder().password(...) and to changePassword(), but User expects
BCrypt-hashed passwords; update tests to hash all test passwords using
BCryptPasswordEncoder (or a test helper like hashPassword(String)) before
passing them into User.builder() and before calling user.changePassword(...),
and assert against the hashed value or adapt assertions to verify BCrypt matches
(e.g., use BCryptPasswordEncoder.matches) so the tests exercise the same
hashed-password invariants as the production flow (targets: UserTest,
User.builder(), changePassword(), getPassword()).
| @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) | ||
| @Import(MySqlTestContainersConfig.class) | ||
| @Transactional | ||
| public class UserE2ETest { |
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:
# 파일 존재 여부 확인 및 크기 체크
ls -la apps/commerce-api/src/test/java/com/loopers/user/UserE2ETest.java
wc -l apps/commerce-api/src/test/java/com/loopers/user/UserE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 270
🏁 Script executed:
# 전체 파일 내용 읽기 (크기 확인 후)
cat -n apps/commerce-api/src/test/java/com/loopers/user/UserE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 11597
🏁 Script executed:
# `@Transactional과` TestRestTemplate 사용 확인
rg -n '@Transactional|TestRestTemplate' -A 3 apps/commerce-api/src/test/java/com/loopers/user/UserE2ETest.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 442
@transactional이 E2E HTTP 요청 데이터 롤백을 보장하지 않으므로 테스트 간 데이터 격리를 명시적으로 처리해야 한다.
@SpringBootTest(webEnvironment = RANDOM_PORT)는 별도 스레드에서 실제 임베디드 서버를 실행하기 때문에, 클래스 레벨 @Transactional이 적용되어 있어도 HTTP 요청을 처리하는 서버 스레드의 데이터는 롤백되지 않는다. 이로 인해 테스트 종료 후 데이터가 데이터베이스에 남아 플래키와 테스트 간 상호 간섭이 발생할 수 있다.
추가로 라인 128, 161, 195, 227에서 System.currentTimeMillis() % N으로 loginId를 생성하고 있는데, 이 방식은 밀리초 단위 충돌 가능성이 높아 테스트가 동시에 또는 빠르게 연속 실행될 때 중복 loginId가 발생할 수 있다. 특히 라인 52-74의 중복 loginId 체크 테스트나 라인 76-104의 내 정보 조회 테스트가 불안정해질 수 있다.
수정안: @Transactional을 제거하고 @AfterEach에서 UserRepository.deleteAll()로 테이블을 비우거나 @Sql(scripts = "...", executionPhase = AFTER_TEST_METHOD)을 사용하여 격리를 보장한다. 아울러 loginId를 UUID나 고유 시퀀스로 생성하도록 변경하여 충돌을 제거한다.
추가 테스트: 테스트 실행 순서를 랜덤으로 여러 회 반복 실행해도 모든 테스트가 안정적으로 통과하는지 검증하여 격리 수준을 확인한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/user/UserE2ETest.java` around
lines 22 - 25, Remove class-level `@Transactional` from UserE2ETest and implement
explicit test data cleanup to guarantee isolation: add an `@AfterEach` method that
calls userRepository.deleteAll() (or annotate tests with `@Sql`(...,
executionPhase = AFTER_TEST_METHOD)) so data created by the embedded server
threads is removed after each test; also replace all usages that generate
loginId via System.currentTimeMillis() % N with a collision-safe generator
(e.g., UUID.randomUUID().toString() or an atomic sequence) in the tests that
create users so duplicate loginId races (seen where loginId is computed with
System.currentTimeMillis()) are eliminated and tests like the duplicate-loginId
check and "my profile" retrieval become stable.
| void 비밀번호_변경_API_요청시_200_OK_반환() { | ||
| // given - 사용자 생성 | ||
| String loginId = "pwchg" + (System.currentTimeMillis() % 10000); | ||
| String currentPassword = "Password1!"; | ||
| String newPassword = "NewPassword2@"; | ||
|
|
||
| CreateUserRequest createRequest = new CreateUserRequest( | ||
| loginId, currentPassword, "홍길동", "1990-01-01", "test@example.com" | ||
| ); | ||
| ResponseEntity<CreateUserResponse> createResponse = restTemplate.postForEntity("/api/v1/users", createRequest, CreateUserResponse.class); | ||
| assertThat(createResponse.getStatusCode()).isEqualTo(HttpStatus.CREATED); | ||
|
|
||
| // when - 비밀번호 변경 | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.set(LOGIN_ID_HEADER, loginId); | ||
| headers.set(LOGIN_PW_HEADER, currentPassword); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| ChangePasswordRequest changeRequest = new ChangePasswordRequest(newPassword); | ||
| HttpEntity<ChangePasswordRequest> entity = new HttpEntity<>(changeRequest, headers); | ||
|
|
||
| ResponseEntity<Void> response = restTemplate.exchange( | ||
| "/api/v1/users/password", | ||
| HttpMethod.PATCH, | ||
| entity, | ||
| Void.class | ||
| ); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK); | ||
| } | ||
|
|
||
| @Test | ||
| void 비밀번호_변경시_기존_비밀번호가_일치하지_않으면_401_Unauthorized_반환() { | ||
| // given - 사용자 생성 | ||
| String loginId = "pwfail" + (System.currentTimeMillis() % 1000); | ||
| String currentPassword = "Password1!"; | ||
| String wrongPassword = "WrongPassword!"; | ||
| String newPassword = "NewPassword2@"; | ||
|
|
||
| CreateUserRequest createRequest = new CreateUserRequest( | ||
| loginId, currentPassword, "홍길동", "1990-01-01", "test@example.com" | ||
| ); | ||
| ResponseEntity<CreateUserResponse> createResponse = restTemplate.postForEntity("/api/v1/users", createRequest, CreateUserResponse.class); | ||
| assertThat(createResponse.getStatusCode()).isEqualTo(HttpStatus.CREATED); | ||
|
|
||
| // when - 잘못된 비밀번호로 변경 시도 | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.set(LOGIN_ID_HEADER, loginId); | ||
| headers.set(LOGIN_PW_HEADER, wrongPassword); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| ChangePasswordRequest changeRequest = new ChangePasswordRequest(newPassword); | ||
| HttpEntity<ChangePasswordRequest> entity = new HttpEntity<>(changeRequest, headers); | ||
|
|
||
| ResponseEntity<String> response = restTemplate.exchange( | ||
| "/api/v1/users/password", | ||
| HttpMethod.PATCH, | ||
| entity, | ||
| String.class | ||
| ); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED); | ||
| } | ||
|
|
||
| @Test | ||
| void 비밀번호_변경시_새_비밀번호가_기존과_동일하면_400_Bad_Request_반환() { | ||
| // given - 사용자 생성 | ||
| String loginId = "pwsame" + (System.currentTimeMillis() % 1000); | ||
| String currentPassword = "Password1!"; | ||
|
|
||
| CreateUserRequest createRequest = new CreateUserRequest( | ||
| loginId, currentPassword, "홍길동", "1990-01-01", "test@example.com" | ||
| ); | ||
| ResponseEntity<CreateUserResponse> createResponse = restTemplate.postForEntity("/api/v1/users", createRequest, CreateUserResponse.class); | ||
| assertThat(createResponse.getStatusCode()).isEqualTo(HttpStatus.CREATED); | ||
|
|
||
| // when - 동일한 비밀번호로 변경 시도 | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.set(LOGIN_ID_HEADER, loginId); | ||
| headers.set(LOGIN_PW_HEADER, currentPassword); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| ChangePasswordRequest changeRequest = new ChangePasswordRequest(currentPassword); | ||
| HttpEntity<ChangePasswordRequest> entity = new HttpEntity<>(changeRequest, headers); | ||
|
|
||
| ResponseEntity<String> response = restTemplate.exchange( | ||
| "/api/v1/users/password", | ||
| HttpMethod.PATCH, | ||
| entity, | ||
| String.class | ||
| ); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } | ||
|
|
||
| @Test | ||
| void 비밀번호_변경시_8자_미만_규칙_위반하면_400_Bad_Request_반환() { | ||
| // given - 사용자 생성 | ||
| String loginId = "pwrule" + (System.currentTimeMillis() % 1000); | ||
| String currentPassword = "Password1!"; | ||
| String invalidPassword = "short"; // 8자 미만 | ||
|
|
||
| CreateUserRequest createRequest = new CreateUserRequest( | ||
| loginId, currentPassword, "홍길동", "1990-01-01", "test@example.com" | ||
| ); | ||
| ResponseEntity<CreateUserResponse> createResponse = restTemplate.postForEntity("/api/v1/users", createRequest, CreateUserResponse.class); | ||
| assertThat(createResponse.getStatusCode()).isEqualTo(HttpStatus.CREATED); | ||
|
|
||
| // when - 규칙 위반 비밀번호로 변경 시도 | ||
| HttpHeaders headers = new HttpHeaders(); | ||
| headers.set(LOGIN_ID_HEADER, loginId); | ||
| headers.set(LOGIN_PW_HEADER, currentPassword); | ||
| headers.setContentType(MediaType.APPLICATION_JSON); | ||
|
|
||
| ChangePasswordRequest changeRequest = new ChangePasswordRequest(invalidPassword); | ||
| HttpEntity<ChangePasswordRequest> entity = new HttpEntity<>(changeRequest, headers); | ||
|
|
||
| ResponseEntity<String> response = restTemplate.exchange( | ||
| "/api/v1/users/password", | ||
| HttpMethod.PATCH, | ||
| entity, | ||
| String.class | ||
| ); | ||
|
|
||
| // then | ||
| assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); | ||
| } |
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.
시간 기반 loginId 축약은 충돌로 플래키 테스트를 유발할 수 있다
운영 관점에서 병렬 실행 또는 빠른 연속 실행 시 동일한 ID가 생성되어 409/401이 비의도적으로 발생할 수 있다. 수정안으로 UUID 기반 ID를 사용해 충돌 가능성을 제거하는 것이 안전하다. 추가 테스트로 짧은 시간 내 다수 회원가입을 반복해도 충돌 없이 성공하는지 검증하는 테스트를 추가하는 것이 좋다.
🔧 수정안 예시
@@
- String loginId = "pwchg" + (System.currentTimeMillis() % 10000);
+ String loginId = "pwchg-" + UUID.randomUUID();
@@
- String loginId = "pwfail" + (System.currentTimeMillis() % 1000);
+ String loginId = "pwfail-" + UUID.randomUUID();
@@
- String loginId = "pwsame" + (System.currentTimeMillis() % 1000);
+ String loginId = "pwsame-" + UUID.randomUUID();
@@
- String loginId = "pwrule" + (System.currentTimeMillis() % 1000);
+ String loginId = "pwrule-" + UUID.randomUUID();As per coding guidelines: 통합 테스트는 격리 수준, 플래키 가능성, 테스트 데이터 준비/정리 전략을 점검한다를 반영한 지적이다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/user/UserE2ETest.java` around
lines 126 - 255, Replace the time-based loginId generation with a UUID-based
unique id to avoid flakiness; update each test that currently builds loginId
like "pwchg" + (System.currentTimeMillis() % ...) to use UUID.randomUUID() (or a
small helper method e.g., createUniqueLoginId()) and run the same
CreateUserRequest flow unchanged; optionally add a short additional test that
creates many users in quick succession to assert no collisions occur when using
the new UUID-based ids.
📌 Summary
🧭 Context & Decision
문제 정의
선택지와 결정
🏗️ Design Overview
변경 범위
UserControllerUserServiceUserCreateUserRequest,CreateUserResponse,GetMyInfoResponse,ChangePasswordRequestPasswordValidatorUserRepositoryDuplicateLoginIdException,InvalidCredentialsException,SamePasswordException,GlobalExceptionHandler주요 컴포넌트 책임
UserControllerUserServiceUserPasswordValidatorCreateUserRequestChangePasswordRequestGetMyInfoResponseGlobalExceptionHandler🔁 Flow Diagram
1. 회원가입 Flow
2. 내 정보 조회 Flow
3. 비밀번호 변경 Flow
Exception Flow
변경 목적: 회원가입, 내 정보 조회, 비밀번호 변경 기능을 TDD(Inside-Out) 방식으로 구현하여 서비스의 기본 인증/사용자 관리 기능 구성.
핵심 변경점: (1) BaseEntity를 상속하는 User 도메인 추가 - 이름 마스킹(마지막 문자 제외), 비밀번호 변경 메서드 포함; (2) UserController에 3개 엔드포인트 구현 - 회원가입(POST), 내 정보 조회(GET /me), 비밀번호 변경(PATCH /password); (3) PasswordValidator 유틸리티로 비밀번호 규칙 검증(8~16자, 지정 특수문자, 생년월일 미포함) 분리; (4) BCryptPasswordEncoder를 통한 비밀번호 암호화 저장; (5) 4개 커스텀 예외(DuplicateLoginIdException, InvalidCredentialsException, SamePasswordException)와 GlobalExceptionHandler로 HTTP 상태코드 매핑.
리스크/주의사항: PasswordValidator가 도메인이 아닌 별도 static 유틸리티로 분리되어, User 엔티티 생성 시점에 비밀번호 규칙 유효성을 보장하지 못할 수 있음. User 클래스의 필수값 검증이 Assert.hasText로 처리되는데, BaseEntity의 guard() 메서드와의 검증 전략 통합 여부를 확인해야 함.
테스트/검증: 12개 테스트 파일 포함 - UserControllerTest, CreateUserRequestValidationTest, UserTest(도메인), UserServiceTest, UserServiceIntegrationTest(통합테스트), PasswordValidatorTest, UserE2ETest 등으로 단위/통합/E2E 테스트 커버; 성공 케이스와 중복ID, 잘못된 자격증명, 동일 비밀번호 변경 등 예외 시나리오 포함.