Skip to content

Conversation

@ghtjr410
Copy link

@ghtjr410 ghtjr410 commented Feb 5, 2026

📌 Summary

  • 배경:
    • Volume 1 과제 - 회원 기능 구현 요구사항 및 Claude Code 개발 환경 세팅
  • 목표:
    • 회원가입, 내 정보 조회, 비밀번호 변경 API 구현 및 모든 테스트 케이스 통과
  • 결과:
    • 역할과 책임이 명확한 User 도메인 완성
    • E2E/통합/단위 테스트 전체 통과
    • Claude Code 스킬 체계 구축

🧭 Context & Decision

문제 정의

  • 현재 동작/제약: 기존 프로젝트 템플릿에 User 도메인 없음
  • 문제(또는 리스크): 회원 정보 관리 기능 부재, 비밀번호 보안 요구사항 충족 필요
  • 성공 기준(완료 정의): 3개 API 정상 동작 + 모든 테스트 통과 + 비밀번호 규칙 검증

선택지와 결정

1. 입력값 검증 위치

  • 고려한 대안:
    • A: Controller에서 모든 검증 수행
    • B: Domain Entity에서 모든 검증 수행
    • C: 2단계 분리 (Request DTO - 형식 검증, Domain - 불변식 검증)
  • 최종 결정: C안 채택
  • 트레이드오프: 검증 로직이 분산되지만 관심사 분리로 유지보수성 향상
  • 추후 개선 여지: Custom Validator로 공통 패턴 추출 가능

2. 비밀번호 인코더 의존성 주입

  • 고려한 대안:
    • A: Service에서 인코딩 후 Entity에 전달
    • B: Entity 메서드 파라미터로 전달
  • 최종 결정: C안 채택 - User.create(..., PasswordEncoder encoder)
  • 트레이드오프:
    • 호출 시 매번 encoder를 전달해야 하지만, "User가 존재하면 비밀번호는 반드시 인코딩된 상태"라는 불변식이 구조적으로 보장.
    • Service에서 인코딩 후 전달하는 방식은 인코딩 누락 시 평문이 그대로 저장될 수 있고, 이를 컴파일 타임에 잡을 수 없음.

🏗️ Design Overview

변경 범위

  • 영향 받는 모듈/도메인: apps/commerce-api
  • 신규 추가:
    • User 도메인 전체 (Entity, Service, Repository, Facade, Controller)
    • PasswordEncoder 인터페이스 및 BCrypt 구현체
    • PasswordValidator 도메인 유틸
    • Claude Code 스킬/규칙 체계 (.claude/)
  • 제거/대체: 없음

주요 컴포넌트 책임

  • UserV1Controller: HTTP 요청/응답 변환, 헤더 인증 검증
  • UserFacade: Service 호출 오케스트레이션, Entity→Info 변환
  • UserService: 비즈니스 로직 (중복 체크, 인증, 비밀번호 변경)
  • User: 도메인 불변식 검증, 이름 마스킹, 비밀번호 변경 로직
  • PasswordValidator: 비밀번호 규칙 검증 (8~16자, 생년월일 포함 불가)
  • BcryptPasswordEncoder: 비밀번호 암호화/매칭

🔁 Flow Diagram

핵심 플로우

1. 회원가입 (POST /api/v1/users)

sequenceDiagram
  autonumber
  participant Client
  participant Controller
  participant Facade
  participant Service
  participant Repository
  participant Entity

  Client->>Controller: POST /api/v1/users (SignUpRequest)
  Controller->>Controller: @Valid 입력값 형식 검증
  Controller->>Facade: signUp(loginId, password, name, birthDate, email)
  Facade->>Service: signUp(...)
  Service->>Repository: existsByLoginId(loginId)
  Repository-->>Service: false
  Service->>Repository: existsByEmail(email)
  Repository-->>Service: false
  Service->>Entity: User.create(..., encoder)
  Entity->>Entity: 도메인 불변식 검증 + 비밀번호 암호화
  Entity-->>Service: User
  Service->>Repository: save(user)
  Repository-->>Service: User (with ID)
  Service-->>Facade: User
  Facade-->>Controller: UserInfo (maskedName 포함)
  Controller-->>Client: ApiResponse<UserResponse>
Loading

2. 내 정보 조회 (GET /api/v1/users/me)

sequenceDiagram
  autonumber
  participant Client
  participant Controller
  participant Facade
  participant Service
  participant Repository

  Client->>Controller: GET /api/v1/users/me (with auth headers)
  Controller->>Controller: 헤더 필수값 검증
  Controller->>Facade: getMyInfo(loginId, password)
  Facade->>Service: authenticate(loginId, password)
  Service->>Repository: findByLoginId(loginId)
  Repository-->>Service: User
  Service->>Service: passwordEncoder.matches(password, user.password)
  Service-->>Facade: User
  Facade-->>Controller: UserInfo (maskedName)
  Controller-->>Client: ApiResponse<UserResponse>
Loading

3. 비밀번호 변경 (PATCH /api/v1/users/me/password)

sequenceDiagram
  autonumber
  participant Client
  participant Controller
  participant Facade
  participant Service
  participant Entity

  Client->>Controller: PATCH /api/v1/users/me/password
  Controller->>Controller: 헤더 필수값 검증
  Controller->>Facade: changePassword(loginId, password, newPassword)
  Facade->>Service: changePassword(...)
  Service->>Service: authenticate(loginId, password)
  Service->>Entity: user.changePassword(newPassword, encoder)
  Entity->>Entity: 동일 비밀번호 체크 + 규칙 검증 + 암호화
  Entity-->>Service: (dirty checking)
  Service-->>Facade: void
  Facade-->>Controller: void
  Controller-->>Client: ApiResponse<null>
Loading

예외 플로우

회원가입 - 비밀번호 규칙 위반 (도메인 불변식)

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
Loading

회원가입 - 로그인ID 중복

sequenceDiagram
  autonumber
  participant Client
  participant Controller
  participant Facade
  participant Service
  participant Repository

  Client->>Controller: POST /api/v1/users
  Controller->>Facade: signUp(...)
  Facade->>Service: signUp(...)
  Service->>Repository: existsByLoginId(loginId)
  Repository-->>Service: true
  Service--xFacade: CoreException(CONFLICT)
  Facade--xController: CoreException 전파
  Controller-->>Client: 409 Conflict
Loading

내 정보 조회 - 인증 실패

sequenceDiagram
  autonumber
  participant Client
  participant Controller
  participant Facade
  participant Service

  Client->>Controller: GET /api/v1/users/me
  Controller->>Facade: getMyInfo(loginId, wrongPassword)
  Facade->>Service: authenticate(...)
  Service->>Service: passwordEncoder.matches() = false
  Service--xFacade: CoreException(UNAUTHORIZED)
  Facade--xController: CoreException 전파
  Controller-->>Client: 401 Unauthorized
Loading

비밀번호 변경 - 동일 비밀번호

sequenceDiagram
  autonumber
  participant Client
  participant Controller
  participant Facade
  participant Service
  participant Entity

  Client->>Controller: PATCH /api/v1/users/me/password
  Controller->>Facade: changePassword(loginId, password, samePassword)
  Facade->>Service: changePassword(...)
  Service->>Service: authenticate(...)
  Service->>Entity: user.changePassword(samePassword, encoder)
  Entity->>Entity: encoder.matches(newPassword, currentPassword) = true
  Entity--xService: IllegalArgumentException
  Service--xFacade: 예외 전파
  Facade--xController: 예외 전파
  Controller-->>Client: 400 Bad Request
Loading

Test Strategy

계층 테스트 유형 검증 대상
Entity 단위 테스트 도메인 불변식, 마스킹, 비밀번호 변경
PasswordValidator 단위 테스트 비밀번호 규칙 (8~16자, 생년월일 포함 불가)
Service 통합 테스트 중복 체크, 인증, 트랜잭션
Controller E2E 테스트 전체 API 흐름, HTTP 상태 코드

🔍 리뷰 포인트

1. 도메인 엔티티의 PasswordEncoder 의존성

User.create(..., PasswordEncoder encoder) 방식으로 "User가 존재하면 비밀번호는 반드시 인코딩된 상태"라는 불변식을 구조적으로 보장했습니다.
Service에서 인코딩 후 전달하는 방식은 인코딩 누락 시 평문 저장 가능성이 있고 이를 컴파일 타임에 잡을 수 없어서 현재 방식을 선택했는데, 이 선택에 대해 의견 부탁드립니다!

2. DTO 검증 vs 도메인 불변식 검증의 에러 응답 형식

입력값 검증을 Request DTO의 @Valid(형식 검증)와 도메인 내부 불변식 검증(비즈니스 규칙)으로 분리했는데, GlobalExceptionHandler에서 처리하는 예외 타입이 MethodArgumentNotValidException vs IllegalArgumentException으로 달라집니다.
클라이언트에게 내려가는 에러 응답 형식을 통일하는 게 좋을지, 검증 레이어가 다르니 형식이 다른 게 자연스러운 건지 고민입니다.
아니면 현재 과제 scope에 해당하지 않아 넘어가도 좋을지 의견 부탁드립니다!

3. 로그인ID 중복 체크의 동시성 처리

현재 existsByLoginId() 체크 후 save()하는 구조인데, 동시 요청 시 race condition 가능성이 있습니다. 락보다는 DB unique 제약조건을 최종 방어선으로 두고 DataIntegrityViolationException을 GlobalExceptionHandler에서 처리하는 게 현실적이라 판단했는데, 이 접근이 적절한지 의견 부탁드립니다!

Checklist

  • 회원가입 API 구현
  • 내 정보 조회 API 구현 (이름 마스킹)
  • 비밀번호 변경 API 구현
  • 비밀번호 규칙 검증 (8~16자, 생년월일 포함 불가)
  • 단위 테스트 작성 (User, PasswordValidator)
  • 통합 테스트 작성 (UserService)
  • E2E 테스트 작성 (UserApi)
  • Claude Code 스킬/규칙 체계 구축

변경 목적:
Volume 1 과제 구현으로 회원가입(POST /api/v1/users), 내 정보 조회(GET /api/v1/users/me, 이름 마스킹 적용), 비밀번호 변경(PATCH /api/v1/users/me/password) 3개 API 완성.

핵심 변경점:

  • User 엔티티 및 Password 값 객체 추가로 비밀번호 검증(8~16자, 생년월일 포함 불가) 및 Bcrypt 인코딩 보장
  • DTO 검증(@Valid) + 도메인 비즈니스 규칙 검증 2단계 검증 적용
  • @authuser ArgumentResolver 기반 인증 시스템 도입으로 X-Loopers-LoginId/LoginPw 헤더에서 사용자 추출
  • UserFacade → UserService → User/UserRepository 계층적 아키텍처 구성

리스크/주의사항:
CodeRabbit 자동 리뷰에서 지적한 프로덕션 배포 전 우선 보완 사항: (1) 평문 비밀번호를 HTTP 헤더로 전송하는 현재 방식의 보안 위험 → JWT/세션 기반 인증 전환 필요, (2) Rate limiting/계정 잠금 정책 부재로 무차별 대입 공격 취약, (3) 보안 감사 로그 부재, (4) 동시 비밀번호 변경 시 동시성 제어 미흡. 현 구현이 기능적 요구사항은 충족하나, 보안/운영 관점의 보강이 필요한 상태로 보임.

테스트/검증 방법:
User/Password 단위 테스트(~270줄, 21개 케이스), UserService 통합 테스트(~136줄), UserApiE2ETest E2E 테스트(~258줄)로 회원가입/인증/비밀번호 변경/내 정보 조회의 성공/실패 시나리오 모두 검증 완료. HTTP 헤더 기반 인증 동작도 E2E에서 확인되었음을 알 수 있음.

- 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 제외 추가
Copilot AI review requested due to automatic review settings February 5, 2026 15:50
@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

사용자 회원가입, 인증, 비밀번호 변경 및 정보 조회 기능을 제공하는 사용자 관리 시스템을 완전히 구현한다. BCrypt를 통한 비밀번호 암호화와 커스텀 헤더 기반 인증 메커니즘을 포함한다.

Changes

Cohort / File(s) Summary
설정 및 의존성
.gitignore, apps/commerce-api/build.gradle.kts
로컬 설정 파일 무시 규칙 추가 및 Spring Security 암호화 의존성 추가
도메인 - 비밀번호 가치 객체
apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java, apps/commerce-api/src/main/java/com/loopers/domain/user/PasswordEncoder.java
비밀번호 검증(길이 8-16, 허용된 문자), 암호화, 매칭 로직을 포함하는 Password 임베더블과 PasswordEncoder 인터페이스
도메인 - 사용자 엔티티
apps/commerce-api/src/main/java/com/loopers/domain/user/User.java
사용자 생성, 비밀번호 변경, 비밀번호 매칭, 이름 마스킹을 지원하는 JPA 엔티티
도메인 - 저장소 및 서비스
apps/commerce-api/src/main/java/com/loopers/domain/user/UserRepository.java, apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java
사용자 CRUD 인터페이스 및 회원가입, 인증, 비밀번호 변경 로직을 포함하는 트랜잭션 서비스
인프라 계층
apps/commerce-api/src/main/java/com/loopers/infrastructure/user/BcryptPasswordEncoder.java, apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserJpaRepository.java, apps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java
BCrypt 기반 비밀번호 암호화 구현 및 Spring Data JPA 저장소
애플리케이션 계층
apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java, apps/commerce-api/src/main/java/com/loopers/application/user/UserInfo.java
서비스에 위임하는 파사드 및 사용자 정보 DTO
API 명세 및 DTO
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserApiV1Spec.java, apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java
Swagger 주석이 적용된 API 명세 인터페이스 및 SignUpRequest, UserResponse, ChangePasswordRequest DTO
API 컨트롤러
apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java
/api/v1/users 엔드포인트 구현 (회원가입, 내 정보 조회, 비밀번호 변경)
인증 지원
apps/commerce-api/src/main/java/com/loopers/support/auth/AuthUser.java, apps/commerce-api/src/main/java/com/loopers/support/auth/AuthUserResolver.java, apps/commerce-api/src/main/java/com/loopers/support/auth/AuthenticatedUser.java, apps/commerce-api/src/main/java/com/loopers/interfaces/api/config/WebMvcConfig.java
X-Loopers-LoginId/X-Loopers-LoginPw 헤더 기반 커스텀 인증 처리
에러 처리 및 응답
apps/commerce-api/src/main/java/com/loopers/support/error/ErrorType.java, apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiResponse.java, apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.java
UNAUTHORIZED 에러 타입 추가, ApiResponse를 Void 타입으로 수정, IllegalArgumentException 핸들러 추가
단위 및 통합 테스트
apps/commerce-api/src/test/java/com/loopers/domain/user/Password*.java, apps/commerce-api/src/test/java/com/loopers/domain/user/User*.java, apps/commerce-api/src/test/java/com/loopers/domain/user/FakePasswordEncoder.java
Password 값 객체, User 엔티티, UserService에 대한 단위 및 통합 테스트
API E2E 테스트
apps/commerce-api/src/test/java/com/loopers/interfaces/api/user/UserApiE2ETest.java, apps/commerce-api/src/test/java/com/loopers/interfaces/api/example/ExampleV1ApiE2ETest.java
REST API 엔드포인트에 대한 E2E 테스트 및 기존 테스트 패키지 정리
HTTP 문서
http/commerce-api/user.http
회원가입, 내 정보 조회, 비밀번호 변경 API 호출 예제

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>
Loading
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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.97% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed 제목은 Pull Request의 주요 변경사항(회원가입/내 정보 조회/비밀번호 수정 기능)을 명확하게 요약하고 있다.
Description check ✅ Passed PR 설명은 템플릿의 주요 섹션(Summary, Context & Decision, Design Overview, Flow Diagram)을 완벽하게 충족하고 있으며, 아키텍처 결정 및 테스트 전략까지 상세히 기술하였다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a 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.

@incheol789 incheol789 assigned incheol789 and unassigned incheol789 Feb 5, 2026
@ghtjr410
Copy link
Author

ghtjr410 commented Feb 5, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Feb 5, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a 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();
}

Comment on lines 41 to 49
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("비밀번호에 생년월일을 포함할 수 없습니다");
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 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 -n

Repository: 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 -5

Repository: 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.java

Repository: 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.

Suggested change
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.

Comment on lines 93 to 99
public void changePassword(String newRawPassword, PasswordEncoder encoder) {
if (encoder.matches(newRawPassword, password)) {
throw new IllegalArgumentException("현재 비밀번호와 동일한 비밀번호는 사용할 수 없습니다");
}
PasswordValidator.validate(newRawPassword, birthDate);
password = encoder.encode(newRawPassword);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

비밀번호 변경 입력 검증 순서로 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.

Suggested change
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.

Comment on lines +19 to +27
@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);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "UserService.java" -type f

Repository: 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 -5

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 152


🏁 Script executed:

find . -name "ErrorType.java" -type f

Repository: 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.java

Repository: 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.java

Repository: 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.java

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 1056


🏁 Script executed:

rg "DataIntegrityViolationException" --type java

Repository: 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 -20

Repository: 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 -5

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


🏁 Script executed:

rg "import.*Violation" apps/commerce-api/src --type java

Repository: 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).

Comment on lines +15 to +17
@Override
public boolean matches(String rawPassword, String encodedPassword) {
return BCrypt.checkpw(rawPassword, encodedPassword);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 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:

  1. You’re not passing a BCrypt hash at all
  • hashed must look like: $2a$10$... (or $2b$..., $2y$...) and be ~60 chars.
  • If it’s empty, a different algorithm (PBKDF2/SCrypt/Argon2/SHA*), or modified, checkpw can’t parse it. (docs.spring.io)
  1. 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)
  1. 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.checkpw directly, 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)
  1. 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$..." hashes

If 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:


🏁 Script executed:

# BcryptPasswordEncoder.java 전체 내용 확인
find . -name "BcryptPasswordEncoder.java" -type f | head -5

Repository: 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 -100

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 67


🏁 Script executed:

# matches 메서드 호출 지점 확인
rg "\.matches\(" --type java | grep -i password | head -20

Repository: 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.java

Repository: 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 -n

Repository: 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 -60

Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java

Length of output: 2384


손상된 해시 입력 시 500으로 전파될 위험이 있다

운영 관점에서 DB 손상이나 마이그레이션 오류로 잘못된 형식의 해시가 저장되면 BCrypt.checkpwIllegalArgumentException을 던져 인증 API가 500으로 실패한다. 실제 해시 값이 DB 컬럼 길이 제한으로 잘린 경우가 가장 흔한 원인이다.

수정안: matches 메서드에서 BCrypt.checkpwtry-catch로 래핑하여 IllegalArgumentExceptionfalse로 흡수한다. rawPasswordencodedPassword의 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;
+        }
     }

추가 테스트: 잘못된 해시 형식(예: 截断된 해시, 잘못된 접두사)에 대해 matchesfalse를 반환하는 단위 테스트를 추가한다.

🤖 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.

import com.loopers.interfaces.api.ApiResponse;
import com.loopers.support.error.CoreException;
import com.loopers.support.error.ErrorType;
import jakarta.validation.Valid;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 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.java

Repository: 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.java

Repository: 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 -20

Repository: 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 -20

Repository: 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.java

Repository: 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.java

Repository: 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.java

Repository: 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.java

Repository: 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 request

UserV1Dto.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 등의 케이스를 검증하세요.

Comment on lines +11 to +22
public record SignUpRequest(
@NotBlank(message = "로그인 ID는 필수입니다")
String loginId,
@NotBlank(message = "비밀번호는 필수입니다")
String password,
@NotBlank(message = "이름은 필수입니다")
String name,
@NotNull(message = "생년월일은 필수입니다")
LocalDate birthDate,
@NotBlank(message = "이메일은 필수입니다")
String email
) {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# UserV1Dto.java 파일 확인
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java

Repository: 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 -20

Repository: 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 -20

Repository: 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 -20

Repository: 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.java

Repository: 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.java

Repository: 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.java

Repository: 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.java

Repository: 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 -20

Repository: 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.java

Repository: 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.

@ghtjr410 ghtjr410 changed the title Volume 1 [Volume - 1] 회원가입/내 정보 조회/비밀번호 수정 기능 작성 Feb 5, 2026
@ghtjr410
Copy link
Author

ghtjr410 commented Feb 7, 2026

@coderabbitai 아쉬웠던점 요약

@coderabbitai
Copy link

coderabbitai bot commented Feb 7, 2026

Tip

For best results, initiate chat on the files or code changes.

@ghtjr410

운영 및 보안 관점에서의 개선이 필요한 사항

1. 인증 방식의 보안 취약점

  • 문제: 평문 비밀번호를 HTTP 헤더(X-Loopers-LoginPw)로 전송한다. HTTPS를 사용하더라도 로그, 캐시, 브라우저 히스토리에 노출될 위험이 있다.
  • 개선안: JWT 또는 세션 기반 인증으로 전환하고, 로그인 시에만 비밀번호를 Body로 전송한다. 이후 요청은 토큰으로 처리한다.
  • 추가 테스트: 토큰 만료, 갱신, 무효화 시나리오 테스트가 필요하다.

2. 보안 이벤트 감사 로깅 부재

  • 문제: 로그인 실패, 비밀번호 변경 등 보안 이벤트에 대한 로깅이 없다. 장애 발생 시 추적이 불가능하고, 무차별 대입 공격을 탐지할 수 없다.
  • 개선안: UserService.authenticate()changePassword()에서 성공/실패 이벤트를 구조화된 로그로 기록한다. IP, 타임스탬프, 실패 사유를 포함한다.
  • 추가 테스트: 로깅 통합 테스트를 추가한다.

3. Rate Limiting 및 계정 잠금 정책 부재

  • 문제: 연속된 로그인 실패 시 계정 잠금이나 요청 제한이 없어 무차별 대입 공격에 취약하다.
  • 개선안: Redis 기반 rate limiter를 도입하거나, 실패 횟수를 DB에 기록하여 5회 실패 시 계정을 일시 잠금한다.
  • 추가 테스트: 연속 실패 시나리오, 잠금 해제 로직 테스트가 필요하다.

4. 운영 관찰성(Observability) 부족

  • 문제: 메트릭, 트레이싱, 헬스체크 엔드포인트가 없다. 프로덕션에서 API 응답 시간, 에러율, DB 커넥션 상태를 모니터링할 수 없다.
  • 개선안: Micrometer를 사용하여 메트릭을 수집하고, /actuator/health 엔드포인트를 활성화한다. 인증 실패율, 비밀번호 변경 빈도를 메트릭으로 노출한다.
  • 추가 테스트: 헬스체크 엔드포인트 E2E 테스트를 추가한다.

5. 동시성 및 트랜잭션 경합 미검증

  • 문제: 동일 사용자에 대한 동시 비밀번호 변경 요청 시 데이터 정합성이 보장되지 않는다. 낙관적 락이나 비관적 락이 없다.
  • 개선안: User 엔티티에 @Version 필드를 추가하여 낙관적 락을 적용한다. 또는 changePassword() 호출 시 PESSIMISTIC_WRITE 락을 획득한다.
  • 추가 테스트: 멀티스레드 환경에서 동시 변경 시나리오 테스트가 필요하다.

6. 성능: N+1 쿼리 및 캐싱 전략 부재

  • 문제: 현재는 단순 조회이나, 향후 사용자 권한이나 프로필 확장 시 N+1 쿼리가 발생할 가능성이 있다. 또한 사용자 정보 조회 시 매번 DB를 호출한다.
  • 개선안: findByLoginId()@EntityGraph 또는 fetch join을 준비하고, 읽기 전용 조회는 Redis 캐시(@Cacheable)를 적용한다.
  • 추가 테스트: 캐시 히트/미스 시나리오, 캐시 무효화 테스트가 필요하다.

7. 에러 응답 일관성 및 클라이언트 지침 부족

  • 문제: IllegalArgumentExceptionBAD_REQUEST로 매핑하지만, 에러 코드가 없어 클라이언트가 특정 실패 사유(예: "비밀번호에 생년월일 포함")를 구분하기 어렵다.
  • 개선안: ErrorType에 세부 에러 코드(INVALID_PASSWORD_FORMAT, PASSWORD_CONTAINS_BIRTHDAY 등)를 추가하고, API 응답에 errorCode 필드를 포함한다.
  • 추가 테스트: 각 에러 코드별 E2E 테스트를 추가한다.

8. 국제화(i18n) 미지원

  • 문제: 에러 메시지가 한국어로 하드코딩되어 있어 다국어 서비스 확장 시 재작업이 필요하다.
  • 개선안: MessageSource를 사용하여 에러 메시지를 외부화하고, Accept-Language 헤더를 기반으로 다국어를 지원한다.
  • 추가 테스트: 다국어 에러 응답 테스트가 필요하다.

9. 테스트: 실패 시나리오 및 경계값 보강 필요

  • 문제: DB 연결 실패, 타임아웃, 외부 서비스(이메일 전송 등) 장애 시나리오가 테스트되지 않았다.
  • 개선안: Testcontainers로 DB 장애를 시뮬레이션하고, WireMock으로 외부 API 장애를 재현한다. Circuit Breaker 패턴을 도입하여 연쇄 장애를 방지한다.
  • 추가 테스트: 타임아웃, 재시도, 폴백 로직 테스트를 추가한다.

10. API 버전 관리 전략 불명확

  • 문제: URL 기반 버전(/api/v1)만 사용하며, 버전 간 마이그레이션, Deprecation 정책이 명시되지 않았다.
  • 개선안: API 변경 시 헤더 기반 버전(X-API-Version)도 지원하고, Deprecation 헤더를 통해 클라이언트에게 사전 공지한다.
  • 추가 테스트: 구 버전 API 호출 시 경고 응답 테스트를 추가한다.

위 항목들은 프로덕션 배포 전 반드시 보완해야 할 사항이다. 특히 1~5번은 보안 및 운영 안정성에 직결되므로 우선순위를 높여 처리한다.

@ghtjr410
Copy link
Author

ghtjr410 commented Feb 8, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Feb 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a 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.changePasswordPassword.change에 위임하므로, Password 단위 테스트에서 동일 비밀번호 재사용 예외는 검증된다. 그러나 User 수준에서 validatePasswordNotContainsBirthDatePassword.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") 등 실무에서 빈번한 잘못된 입력을 포함하면 회귀 방지에 유리하다.

Comment on lines 105 to 117
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("비밀번호에 생년월일을 포함할 수 없습니다");
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

validatePasswordNotContainsBirthDateMMdd 패턴이 의도치 않은 거부를 유발할 수 있다.

MMdd 패턴은 4자리로, 일반적인 숫자 조합과 충돌할 가능성이 있다. 예를 들어 생년월일이 2000-12-10인 사용자는 "1210"을 포함하는 모든 비밀번호가 거부된다(Abcd1210! 등). 이는 보안 정책상 의도된 것일 수 있으나, 비밀번호 선택 범위를 과도하게 제한하여 사용자 경험을 저하시킬 수 있다. 정책 의도를 확인하고, 필요시 yyyyMMddyyMMdd만 검사하는 방안을 검토하라.

🤖 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.

Comment on lines 98 to 113
@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();
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

비밀번호_변경 통합 테스트에서 실패 시나리오와 이전 비밀번호 무효화 검증이 누락되었다.

현재 성공 경로만 검증하고 있다. 통합 테스트는 트랜잭션 경계와 도메인 규칙이 결합된 상태에서 정상 동작하는지 확인하는 것이 목적이므로, 다음 시나리오를 추가하라:

  1. 변경 후 이전 비밀번호로 인증 시 UNAUTHORIZED 예외 발생
  2. 현재 비밀번호와 동일한 새 비밀번호로 변경 시 예외 발생
  3. 잘못된 현재 비밀번호로 변경 시도 시 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.

@ghtjr410
Copy link
Author

ghtjr410 commented Feb 8, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Feb 8, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a 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 헤더로 인증을 수행하므로, 인증 시점의 비밀번호가 곧 현재 비밀번호 확인 역할을 한다. 그러나 이 설계는 다음 문제를 내포한다:

  1. 인증 헤더의 비밀번호와 "현재 비밀번호 확인"이 암묵적으로 결합되어 있어, 향후 JWT/세션 기반 인증으로 전환 시 현재 비밀번호 검증이 누락될 수 있다.
  2. changePassword 메서드 자체만 보면 어떤 인증도 요구하지 않으므로, 다른 진입점에서 호출될 경우 무인증 비밀번호 변경이 가능하다.

현재 아키텍처에서는 동작하지만, 메서드에 현재 비밀번호 파라미터를 추가하거나 Javadoc으로 인증 전제 조건을 명시하는 것을 권장한다.

Comment on lines +72 to +74
public String getMaskedName() {
return name.substring(0, name.length() - 1) + "*";
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.

Comment on lines +30 to +45
@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);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

매 요청마다 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants