Skip to content

Conversation

@katiekim17
Copy link

@katiekim17 katiekim17 commented Feb 4, 2026

Title

회원 도메인 구현 (가입, 조회, 비밀번호 변경, 헤더 인증)


📌 Summary

배경: 회원 도메인의 핵심 기능 (가입, 조회, 비밀번호 변경) 구현 필요

목표: 회원 가입, 내 정보 조회, 비밀번호 수정 API 구현 및 헤더 기반 인증 적용

결과: 3개 API 엔드포인트 구현 완료, E2E 테스트 포함


🧭 Context & Decision

문제 정의

현재 동작/제약:

  • 회원 관련 API가 존재하지 않음
  • 인증 메커니즘 부재

문제(또는 리스크):

  • 회원 정보 관리 불가
  • API 보안 취약

성공 기준(완료 정의):

  • 회원가입/조회/비밀번호 변경 API 동작
  • 헤더 인증 (X-Loopers-LoginId, X-Loopers-LoginPw) 적용
  • E2E 테스트 통과

선택지와 결정

고려한 대안:

  • A: Filter/Interceptor 기반 인증
  • B: Facade/Service에서 명시적 인증 호출

최종 결정: B (Facade/Service에서 명시적 인증)

트레이드오프:

  • 장점: 코드 흐름이 명시적, 디버깅 용이
  • 단점: 매 메서드마다 인증 호출 필요

추후 개선 여지:

  • API 확장 시 Interceptor로 리팩토링 고려
  • 입력값 포맷 검증 (이름, 이메일, 생년월일) 추가 필요

🏗️ Design Overview

변경 범위

영향 받는 모듈/도메인:

  • commerce-api (interfaces, application, domain, infrastructure)

신규 추가:

  • MemberModel, MemberService, MemberRepository
  • MemberFacade, MemberInfo
  • MemberV1Controller, MemberV1ApiSpec, MemberV1Dto
  • SecurityConfig (PasswordEncoder)

제거/대체: 없음

주요 컴포넌트 책임

Component 책임
MemberV1Controller HTTP 요청 수신, 헤더 파싱, 응답 반환
MemberFacade DTO 변환, Service 조합, 인증 호출
MemberService 비즈니스 로직 (가입, 조회, 인증, 비밀번호 변경)
MemberModel 도메인 엔티티, 검증 로직 (비밀번호 규칙 등)

📝 API Endpoints

Method Endpoint 설명 인증
POST /api/v1/members 회원가입
GET /api/v1/members/me 내 정보 조회 ✅ 헤더
PATCH /api/v1/members/me/password 비밀번호 변경 ✅ 헤더

✅ Test Coverage

  • MemberModelTest (단위 테스트)
  • MemberServiceIntegrationTest (통합 테스트)
  • MemberV1ApiE2ETest (E2E 테스트)

🔁 Flow Diagram

Flow Diagrams

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

sequenceDiagram
    autonumber
    participant Client
    participant Controller
    participant Facade
    participant Service
    participant DB

    Client->>Controller: POST /api/v1/members (SignUpRequest)
    Controller->>Facade: signupMember(request)
    Facade->>Facade: Request to MemberModel 변환
    Facade->>Service: saveMember(memberModel)
    Service->>DB: findByLoginId (중복 체크)
    DB-->>Service: Optional.empty()
    Service->>Service: passwordEncoder.encode()
    Service->>DB: save(memberModel)
    DB-->>Service: savedMember
    Service-->>Facade: MemberModel
    Facade->>Facade: MemberModel to MemberInfo 변환
    Facade-->>Controller: MemberInfo
    Controller-->>Client: 201 Created (SignUpResponse)
Loading

예외 흐름

sequenceDiagram
    autonumber
    participant Client
    participant Controller
    participant Facade
    participant Service
    participant DB

    Client->>Controller: POST /api/v1/members (중복 ID)
    Controller->>Facade: signupMember(request)
    Facade->>Service: saveMember(memberModel)
    Service->>DB: findByLoginId
    DB-->>Service: Optional.of(existingMember)
    Service-->>Controller: CoreException (CONFLICT)
    Controller-->>Client: 409 Conflict
Loading

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

sequenceDiagram
    autonumber
    participant Client
    participant Controller
    participant Facade
    participant Service
    participant DB

    Client->>Controller: GET /api/v1/members/me
    Note over Client,Controller: Headers: X-Loopers-LoginId, X-Loopers-LoginPw
    Controller->>Facade: getMyInfo(loginId, password)
    Facade->>Service: authenticate(loginId, password)
    Service->>DB: findByLoginId
    DB-->>Service: MemberModel
    Service->>Service: passwordEncoder.matches()
    Service-->>Facade: MemberModel (인증 성공)
    Facade->>Facade: MemberModel to MemberInfo 변환 (이름 마스킹)
    Facade-->>Controller: MemberInfo
    Controller-->>Client: 200 OK (MemberInfoResponse)
Loading

예외 흐름 - 인증 실패

sequenceDiagram
    autonumber
    participant Client
    participant Controller
    participant Facade
    participant Service
    participant DB

    Client->>Controller: GET /api/v1/members/me (틀린 비밀번호)
    Controller->>Facade: getMyInfo(loginId, wrongPassword)
    Facade->>Service: authenticate(loginId, wrongPassword)
    Service->>DB: findByLoginId
    DB-->>Service: MemberModel
    Service->>Service: passwordEncoder.matches() = false
    Service-->>Controller: CoreException (UNAUTHORIZED)
    Controller-->>Client: 401 Unauthorized
Loading

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

sequenceDiagram
    autonumber
    participant Client
    participant Controller
    participant Facade
    participant Service
    participant DB

    Client->>Controller: PATCH /api/v1/members/me/password
    Note over Client,Controller: Headers: X-Loopers-LoginId, X-Loopers-LoginPw
    Note over Client,Controller: Body: oldPassword, newPassword
    Controller->>Facade: changePassword(loginId, headerPw, oldPw, newPw)
    Facade->>Service: authenticate(loginId, headerPw)
    Service->>DB: findByLoginId
    DB-->>Service: MemberModel
    Service->>Service: passwordEncoder.matches(headerPw)
    Service-->>Facade: 인증 성공
    Facade->>Facade: new MemberModel(loginId, oldPw)
    Facade->>Service: changePassword(memberModel, newPw)
    Service->>Service: passwordEncoder.matches(oldPw) 검증
    Service->>Service: newPw != oldPw 검증
    Service->>Service: validatePassword(newPw) 규칙 검증
    Service->>Service: passwordEncoder.encode(newPw)
    Service->>DB: Dirty Checking (자동 저장)
    Service-->>Facade: void
    Facade-->>Controller: void
    Controller-->>Client: 200 OK
Loading

예외 흐름 - Body의 기존 비밀번호 불일치

sequenceDiagram
    autonumber
    participant Client
    participant Controller
    participant Facade
    participant Service

    Client->>Controller: PATCH (헤더 인증 OK, Body oldPw 틀림)
    Controller->>Facade: changePassword(...)
    Facade->>Service: authenticate() 성공
    Facade->>Service: changePassword(wrongOldPw, newPw)
    Service->>Service: passwordEncoder.matches(wrongOldPw) = false
    Service-->>Controller: CoreException (UNAUTHORIZED)
    Controller-->>Client: 401 Unauthorized
Loading

예외 흐름 - 새 비밀번호가 기존과 동일

sequenceDiagram
    autonumber
    participant Client
    participant Controller
    participant Facade
    participant Service

    Client->>Controller: PATCH (newPw == oldPw)
    Controller->>Facade: changePassword(...)
    Facade->>Service: authenticate() 성공
    Facade->>Service: changePassword(oldPw, samePassword)
    Service->>Service: oldPw 검증 성공
    Service->>Service: newPw == oldPw 체크
    Service-->>Controller: CoreException (BAD_REQUEST)
    Controller-->>Client: 400 Bad Request
Loading
sequenceDiagram
  autonumber
  participant Client
  participant API
  participant Service
  participant DB
  Client->>API: request
  API->>Service: command/query
  Service->>DB: read/write
  DB-->>Service: result
  Service-->>API: response
  API-->>Client: response
Loading

목적

회원 도메인의 핵심 기능(회원가입, 내 정보 조회, 비밀번호 변경)을 구현하고 HTTP 헤더 기반 인증 방식을 도입합니다.

핵심 변경점

  • MemberModel, MemberService, MemberRepository 등 도메인 및 애플리케이션 계층 구현
  • MemberV1Controller로 3개 REST 엔드포인트 노출 (POST /api/v1/members, GET /api/v1/members/me, PATCH /api/v1/members/me/password)
  • X-Loopers-LoginId, X-Loopers-LoginPw 헤더를 통한 명시적 인증 처리 (Facade/Service 레벨)
  • BCryptPasswordEncoder를 통한 비밀번호 암호화 및 SecurityConfig 추가
  • 중복 ID(409), 인증 실패(401), 비밀번호 검증 실패(400) 등 에러 케이스 처리

리스크/주의사항

  • 선택된 아키텍처: 필터/인터셉터 대신 Facade/Service에서 직접 인증 호출
  • 이메일, 생년월일, 이메일 입력 형식에 대한 검증은 아직 실행하지 않았음

테스트/검증 방법

  • 단위 테스트: MemberModelTest (입력 검증, 비밀번호 변경 등)
  • 통합 테스트: MemberServiceIntegrationTest (회원 저장, 조회, 인증, 비밀번호 변경)
  • E2E 테스트: MemberV1ApiE2ETest (회원가입, 조회, 비밀번호 변경 엔드포인트 전 시나리오)

@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

  • ✅ Full review completed - (🔄 Check again to review again)
✨ Finishing touches
🧪 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

@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/member/MemberModel.java`:
- Around line 57-59: The public getter getPassword in MemberModel exposes the
password field and must be restricted: either remove the public getPassword
method or change its visibility to package-private (default) so it is not
accessible for external DTO mappers/serializers; update any internal callers to
use package-private access or introduce a safe accessor in the same package if
needed (reference MemberModel::getPassword and the private field password), and
add/adjust API response serialization tests to assert that the password property
is not present in serialized outputs (e.g., verify DTO/JSON responses do not
contain "password"). Ensure you also search for and update any usages of
MemberModel.getPassword to avoid compilation breaks.
- Around line 21-50: The MemberModel constructor currently accepts and stores a
plain-text password (MemberModel(String loginId, String password, ...)) which
risks leaking secrets; change the entity contract so the constructor only
accepts an already-hashed password (e.g., MemberModel(String loginId, String
hashedPassword, String name, ...)) and remove any code paths that persist plain
text; move plain-text password validation (validatePassword) out of the entity
into the service or a domain/factory where the raw password is validated and
then encoded via PasswordEncoder before calling the MemberModel constructor;
update or add a factory method or service method that performs
validatePassword(password, birthDate) then encodes and invokes the new
constructor; and add integration tests for signup/password-change that assert
the DB-stored password != raw input and that
PasswordEncoder.matches(rawPassword, storedHash) returns true.

In
`@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`:
- Around line 20-24: The current TOCTOU check in MemberService.saveMember (using
memberRepository.findByLoginId) can still allow duplicate inserts under
concurrency; wrap the memberRepository.save call in a try/catch that catches
DataIntegrityViolationException and rethrows a CoreException(ErrorType.CONFLICT,
"이미 존재하는 아이디입니다.", e) so DB unique-constraint violations are mapped to a
CONFLICT error; ensure the existing pre-check and password encoding
(passwordEncoder.encode / memberModel.encryptPassword) remain before the save
and keep the method transactional (e.g., `@Transactional`) so the catch handles
the integrity exception correctly.
- Around line 18-32: The saveMember method performs an unnecessary DB read by
calling getMember(loginId) after memberRepository.save(memberModel); change it
to use the entity returned by memberRepository.save(...) and return that result
directly (i.e., capture the return value of memberRepository.save and return it)
instead of calling getMember(...). Ensure the password encryption step
(passwordEncoder.encode and memberModel.encryptPassword) happens before calling
memberRepository.save so the saved entity contains the encrypted password.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberJpaRepository.java`:
- Line 9: The loginId column lacks an index/unique constraint, causing repeated
findByLoginId calls (used by MemberJpaRepository.findByLoginId, saveMember and
getMember) to potentially trigger full table scans; add the index either by
annotating the entity MemberModel with `@Table`(indexes = `@Index`(name =
"idx_login_id", columnList = "login_id", unique = true)) or by creating a DB
migration that runs CREATE UNIQUE INDEX idx_login_id ON member(login_id); verify
the index exists in production before deploy.

In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberRepositoryImpl.java`:
- Around line 20-23: 메서드 update가 현재 Optional.empty()로 빈 구현되어 있어 호출 시 무시되는 실패가
발생하므로, 당장 구현하지 않는다면 MemberRepositoryImpl.update(MemberModel memberModel)에서
Optional.empty()를 반환하는 대신 UnsupportedOperationException을 던지도록 변경하거나(예: throw new
UnsupportedOperationException("MemberRepositoryImpl.update not implemented")),
실제 동작을 구현할 준비가 되었다면 MemberModel을 받아 DB를 업데이트하고 업데이트된 MemberModel을
Optional.of(...)로 반환하도록 구현하세요; 대상 식별자는 클래스 MemberRepositoryImpl와 메서드
update(MemberModel memberModel)입니다.

In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java`:
- Around line 68-72: The test can NPE if response.getBody() is null; update the
assertions to first assert the response body is non-null before calling data(),
e.g., add an assertion on response.getBody() (or
assertThat(response.getBody()).isNotNull()) as the first check in assertAll so
subsequent access to response.getBody().data() (and the check of "loginId") is
safe; locate the assertion block using response.getBody(), data(), and assertAll
to apply this change.

In `@build.gradle.kts`:
- Around line 43-46: Un-comment and remove the TODO: re-enable the apply(plugin
= "jacoco") logic (the commented apply(plugin = "jacoco") block that gated
jacoco for non-container projects) and the second commented JaCoCo-related block
(the commented section around jacocoTestReport / JacocoReport configuration),
then add an explicit JaCoCo toolVersion setting (e.g., configure the jacoco {
toolVersion = "0.8.14" } or set in the Jacoco plugin extension) so the build
uses JaCoCo 0.8.14+ and ensure any jacocoTestReport/JacocoReport task
configuration is active. Ensure the TODO about JDK 24 is removed and the plugin
application and report task names (apply(plugin = "jacoco"), jacoco {
toolVersion = ... }, jacocoTestReport / JacocoReport) are present and configured
to produce coverage in CI.
🧹 Nitpick comments (13)
modules/jpa/src/main/resources/jpa.yml (1)

45-45: 로컬 프로파일 JDBC URL도 환경변수 기본값으로 통일하는 편이 안전하다
로컬 포트를 하드코딩하면 팀원/CI에서 포트가 다를 때 연결 실패로 개발·테스트 장애가 발생할 수 있다. 운영 유연성을 위해 기본값을 둔 환경변수 형태로 통일하는 수정이 적절하다. 예시는 아래와 같다. 추가로 로컬 프로파일에서 (1) 기본값 사용, (2) MYSQL_PORT 오버라이드 두 경우에 대해 DB 연결 스모크 테스트를 수행해 동작을 확인하는 것이 필요하다.

제안 변경안
-      jdbc-url: jdbc:mysql://localhost:3307/loopers
+      jdbc-url: jdbc:mysql://${MYSQL_HOST:localhost}:${MYSQL_PORT:3307}/loopers
docker/infra-compose.yml (1)

6-6: MySQL 호스트 포트도 기본값 포함 환경변수로 노출하는 편이 운영 리스크를 줄인다
호스트 포트가 고정되면 로컬 스크립트·문서·CI가 3306을 가정한 경우 접속 실패로 장애가 발생할 수 있다. 기본값을 둔 환경변수로 매핑해 포트 변경을 유연하게 만드는 수정이 적절하다. 예시는 아래와 같다. 추가로 MYSQL_PORT 미설정/설정 각각에 대해 docker compose up 후 DB 연결 스모크 테스트(예: mysqladmin ping 또는 애플리케이션 연결 테스트)를 수행하는 것이 필요하다.

제안 변경안
-      - "3307:3306"
+      - "${MYSQL_PORT:-3307}:3306"
apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberJpaRepository.java (1)

10-10: 불필요한 메서드 선언 - save 메서드는 JpaRepository에서 이미 제공된다.

JpaRepository<MemberModel, Long>를 상속받으면 save(S entity) 메서드가 자동 제공되므로 별도 선언이 불필요하다. 코드 혼란을 줄이기 위해 제거를 권장한다.

♻️ 수정 제안
 public interface MemberJpaRepository extends JpaRepository<MemberModel, Long> {

   Optional<MemberModel> findByLoginId(String loginId);
-  MemberModel save(MemberModel memberModel);
 }
build.gradle.kts (1)

64-65: 코드 스타일 불일치 - 괄호 앞 공백 제거가 필요하다.

다른 의존성 선언과 일관성을 위해 implementation 뒤 공백을 제거해야 한다.

♻️ 수정 제안
         // 암호화
-        implementation ("org.springframework.security:spring-security-crypto")
+        implementation("org.springframework.security:spring-security-crypto")
apps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.java (1)

40-117: 테스트 커버리지 보완 필요 - 경계값 테스트가 누락되었다.

비밀번호 유효성 검증에서 8자(최소 경계)와 16자(최대 경계) 성공 케이스가 누락되었다. 경계값 테스트 없이는 off-by-one 오류를 탐지할 수 없다.

추가 권장 테스트 케이스:

  • 8자 비밀번호 (성공 기대)
  • 16자 비밀번호 (성공 기대)
  • null 비밀번호 (예외 기대)
  • 빈 문자열 비밀번호 (예외 기대)
  • 특수문자 미포함 (예외 기대 여부 확인 필요)
  • 숫자 미포함 (예외 기대 여부 확인 필요)
💚 경계값 테스트 추가 예시
`@DisplayName`("(성공케이스) 비밀번호가 8자일 때 → 정상 생성")
`@Test`
void createsMemberModel_whenPwIs8Chars() {
    // arrange
    String password = "Test123!"; // exactly 8 chars
    
    // act & assert
    assertDoesNotThrow(() -> 
        new MemberModel("testuser", password, "홍길동", "19900101", "test@example.com")
    );
}

`@DisplayName`("(성공케이스) 비밀번호가 16자일 때 → 정상 생성")
`@Test`
void createsMemberModel_whenPwIs16Chars() {
    // arrange
    String password = "Test12345678901!"; // exactly 16 chars
    
    // act & assert
    assertDoesNotThrow(() -> 
        new MemberModel("testuser", password, "홍길동", "19900101", "test@example.com")
    );
}
apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1ApiSpec.java (2)

6-8: 사용되지 않는 import 문이 존재한다.

SchemaPostMapping이 import되었으나 이 인터페이스에서 사용되지 않는다. 불필요한 import는 코드 가독성을 저해하고 IDE 경고를 발생시킨다.

♻️ 수정 제안
 package com.loopers.interfaces.api.member;

 import com.loopers.interfaces.api.ApiResponse;
 import com.loopers.interfaces.api.member.MemberV1Dto.SignUpResponse;
 import io.swagger.v3.oas.annotations.Operation;
-import io.swagger.v3.oas.annotations.media.Schema;
 import io.swagger.v3.oas.annotations.tags.Tag;
-import org.springframework.web.bind.annotation.PostMapping;

17-18: 코드 내 개발자 메모 제거를 권장한다.

Lines 17-18의 주석은 API 문서화 목적이 아닌 개발자 간 소통용 메모이다. PR 머지 전 제거하거나 필요 시 별도 문서(README, Wiki)로 이동하는 것이 바람직하다.

♻️ 수정 제안
   `@Operation`(
       summary = "회원 가입 요청",
       description = "주어진 정보를 가지고 회원 가입을 실행한다"
   )
-    // `@Schema는` Swagger API 문서에서 파라미터 설명을 보여주는 용도
-    // 예제에서는 Long exampleId 같은 단일 파라미터에 붙였는데, 지금은 SignUpRequest로 통째로 받으니까 여기엔 필요 없음
   ApiResponse<MemberV1Dto.SignUpResponse> signUp(
       MemberV1Dto.SignUpRequest request
   );
apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java (1)

44-99: 유효하지 않은 입력에 대한 테스트 케이스 부재

MemberModelTest에서 비밀번호 정책 검증(길이, 한글 포함, 생년월일 포함 등)이 BAD_REQUEST를 반환하는 것을 확인했다. E2E 테스트에서도 이러한 경계값 케이스를 포함하여 API 레벨에서 400 Bad Request 응답이 올바르게 반환되는지 검증해야 한다.

추가 테스트 케이스 예시:

  • 비밀번호가 너무 짧은 경우 (7자 미만)
  • 비밀번호에 한글이 포함된 경우
  • 비밀번호에 생년월일이 포함된 경우
  • 필수 필드가 누락된 경우
apps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.java (1)

35-53: 저장 후 비밀번호 암호화 검증 누락

MemberService.saveMember는 비밀번호를 암호화한 후 저장한다. 저장된 회원의 비밀번호가 원본과 다르게 암호화되었는지 검증하는 assertion이 필요하다. 이를 통해 암호화 로직이 의도대로 동작하는지 확인할 수 있다.

추가 assertion 제안
       // assert
       assertAll(
           () -> assertThat(result).isNotNull(),
           () -> assertThat(result.getLoginId()).isEqualTo(memberModel.getLoginId()),
           () -> assertThat(result.getName()).isEqualTo(memberModel.getName()),
           () -> assertThat(result.getBirthDate()).isEqualTo(memberModel.getBirthDate()),
-          () -> assertThat(result.getEmail()).isEqualTo(memberModel.getEmail())
+          () -> assertThat(result.getEmail()).isEqualTo(memberModel.getEmail()),
+          () -> assertThat(result.getPassword()).isNotEqualTo("Test1234!")
       );
apps/commerce-api/src/main/java/com/loopers/domain/member/MemberRepository.java (2)

7-7: update 메서드가 구현되지 않은 상태로 인터페이스에 정의됨

MemberRepositoryImpl에서 update 메서드가 Optional.empty()를 반환하고 있다. 사용되지 않는 메서드가 인터페이스에 정의되어 있으면 향후 유지보수 시 혼란을 야기할 수 있다. 현재 PR 범위에서 필요 없다면 제거하거나, 구현 예정이라면 TODO 주석을 추가하는 것을 권장한다.


8-8: 파라미터명이 메서드 의미와 불일치

findByLoginId(String id)에서 파라미터명이 id로 되어 있어 PK id와 혼동될 수 있다. loginId로 변경하면 의도가 명확해진다.

수정 제안
-    Optional<MemberModel> findByLoginId(String id);
+    Optional<MemberModel> findByLoginId(String loginId);
apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java (1)

11-13: @service 대신 @Component 사용

서비스 계층임을 명시적으로 나타내려면 @Service 어노테이션이 적합하다. 기능상 차이는 없지만 코드의 의도를 명확히 전달한다.

apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberRepositoryImpl.java (1)

9-11: @repository 대신 @Component 사용

@Repository는 persistence layer 예외를 Spring의 DataAccessException으로 변환하는 기능을 제공한다. JPA 예외가 발생할 경우 일관된 예외 처리를 위해 @Repository 사용을 권장한다.

수정 제안
 `@RequiredArgsConstructor`
-@Component
+@Repository
 public class MemberRepositoryImpl implements MemberRepository {

Comment on lines 21 to 50
public MemberModel(String loginId, String password, String name, String birthDate, String email) {

// 모든 항목은 비어 있을 수 없다
if (loginId == null || loginId.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "아이디는 비어있을 수 없습니다.");
}
if (password == null || password.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다.");
}
if (name == null || name.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "이름은 비어있을 수 없습니다.");
}
if (birthDate == null || birthDate.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 비어있을 수 없습니다.");
}
if (email == null || email.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "이메일은 비어있을 수 없습니다.");
}

// 가입된 아이디로는 가입이 불가능하다 -> 디비에서 검증. 서비스에서 하기
// 비밀번호 8~16자의 영문 대소문자, 숫자, 특수문자만 가능합니다.
// 비밀번호 생년월일은 비밀번호 내에 포함될 수 없습니다.
// 비밀번호 규칙 검증
validatePassword(password, birthDate);

this.loginId = loginId;
this.password = password;
this.name = name;
this.birthDate = birthDate;
this.email = 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

평문 비밀번호가 엔티티에 남아 운영/보안 사고로 이어질 수 있다
현재 생성자가 평문 비밀번호를 필드에 저장한 뒤 별도 encryptPassword로 교체하는 구조라, 서비스 호출 누락/예외 시 DB에 평문 저장 또는 로그·덤프 유출로 보안 사고 위험이 크다.
수정안으로 엔티티에는 암호화된 값만 보관하도록 계약을 바꾸고, 평문 검증은 서비스/도메인 서비스에서 수행 후 암호화 값을 생성자에 전달하거나 팩토리로 일원화하는 방식으로 구조를 변경해야 한다.
추가로 회원가입/비밀번호 변경 통합 테스트에서 DB 저장 값이 평문과 다르고 PasswordEncoder.matches가 true인지 검증해야 한다.

🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`
around lines 21 - 50, The MemberModel constructor currently accepts and stores a
plain-text password (MemberModel(String loginId, String password, ...)) which
risks leaking secrets; change the entity contract so the constructor only
accepts an already-hashed password (e.g., MemberModel(String loginId, String
hashedPassword, String name, ...)) and remove any code paths that persist plain
text; move plain-text password validation (validatePassword) out of the entity
into the service or a domain/factory where the raw password is validated and
then encoded via PasswordEncoder before calling the MemberModel constructor;
update or add a factory method or service method that performs
validatePassword(password, birthDate) then encodes and invokes the new
constructor; and add integration tests for signup/password-change that assert
the DB-stored password != raw input and that
PasswordEncoder.matches(rawPassword, storedHash) returns true.

Comment on lines 57 to 59
public String getPassword() {
return password;
}
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

비밀번호 getter는 외부 노출 경로가 되어 보안 사고 위험이 크다
public getter가 존재하면 DTO 매핑/직렬화/로그 경로를 통해 비밀번호(해시 포함)가 노출될 수 있어 운영 리스크가 크다.
수정안으로 접근 범위를 축소해 패키지 내부로 제한하거나 제거해 외부 노출 경로를 차단해야 한다.
추가로 API 응답 직렬화 테스트에서 password 필드가 포함되지 않는지 검증해야 한다.

🔧 수정 예시
-    public String getPassword() {
-      return password;
-    }
+    String getPassword() {
+      return password;
+    }
📝 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 getPassword() {
return password;
}
String getPassword() {
return password;
}
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`
around lines 57 - 59, The public getter getPassword in MemberModel exposes the
password field and must be restricted: either remove the public getPassword
method or change its visibility to package-private (default) so it is not
accessible for external DTO mappers/serializers; update any internal callers to
use package-private access or introduce a safe accessor in the same package if
needed (reference MemberModel::getPassword and the private field password), and
add/adjust API response serialization tests to assert that the password property
is not present in serialized outputs (e.g., verify DTO/JSON responses do not
contain "password"). Ensure you also search for and update any usages of
MemberModel.getPassword to avoid compilation breaks.

Comment on lines 18 to 32
@Transactional(readOnly = false)
public MemberModel saveMember(MemberModel memberModel) {
//저장하기 전에 이미 같은 loginId가 있는지 확인
Optional<MemberModel> existing = memberRepository.findByLoginId(memberModel.getLoginId());
if (existing.isPresent()) {
throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.");
}

// 비밀번호 암호화 후 저장
String encrypted = passwordEncoder.encode(memberModel.getPassword());
memberModel.encryptPassword(encrypted);
memberRepository.save(memberModel);

return getMember(memberModel.getLoginId());
}
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

저장 후 불필요한 조회 쿼리 발생

memberRepository.save(memberModel)getMember(memberModel.getLoginId())를 호출하여 DB를 다시 조회하고 있다. save()가 이미 저장된 엔티티를 반환하므로 추가 조회는 불필요한 I/O이다.

수정 제안
     `@Transactional`(readOnly = false)
       public MemberModel saveMember(MemberModel memberModel) {
         //저장하기 전에 이미 같은 loginId가 있는지 확인
         Optional<MemberModel> existing = memberRepository.findByLoginId(memberModel.getLoginId());
         if (existing.isPresent()) {
           throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.");
         }
 
         // 비밀번호 암호화 후 저장
         String encrypted = passwordEncoder.encode(memberModel.getPassword());
         memberModel.encryptPassword(encrypted);
-        memberRepository.save(memberModel);
-
-        return getMember(memberModel.getLoginId());
+        return memberRepository.save(memberModel);
       }
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`
around lines 18 - 32, The saveMember method performs an unnecessary DB read by
calling getMember(loginId) after memberRepository.save(memberModel); change it
to use the entity returned by memberRepository.save(...) and return that result
directly (i.e., capture the return value of memberRepository.save and return it)
instead of calling getMember(...). Ensure the password encryption step
(passwordEncoder.encode and memberModel.encryptPassword) happens before calling
memberRepository.save so the saved entity contains the encrypted password.

Comment on lines 20 to 24
//저장하기 전에 이미 같은 loginId가 있는지 확인
Optional<MemberModel> existing = memberRepository.findByLoginId(memberModel.getLoginId());
if (existing.isPresent()) {
throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.");
}
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

동시성 환경에서 중복 체크 우회 가능 (TOCTOU)

findByLoginId 조회 후 save 사이에 다른 트랜잭션이 동일한 loginId로 저장할 수 있다. DB에 unique constraint가 있다면 DataIntegrityViolationException이 발생하는데, 이를 CoreException(CONFLICT)로 변환하는 예외 처리가 필요하다. 운영 환경에서 동시 가입 요청 시 500 에러가 발생할 수 있다.

예외 처리 추가 제안
`@Transactional`(readOnly = false)
public MemberModel saveMember(MemberModel memberModel) {
    Optional<MemberModel> existing = memberRepository.findByLoginId(memberModel.getLoginId());
    if (existing.isPresent()) {
        throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.");
    }

    String encrypted = passwordEncoder.encode(memberModel.getPassword());
    memberModel.encryptPassword(encrypted);
    
    try {
        return memberRepository.save(memberModel);
    } catch (DataIntegrityViolationException e) {
        throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.", e);
    }
}
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`
around lines 20 - 24, The current TOCTOU check in MemberService.saveMember
(using memberRepository.findByLoginId) can still allow duplicate inserts under
concurrency; wrap the memberRepository.save call in a try/catch that catches
DataIntegrityViolationException and rethrows a CoreException(ErrorType.CONFLICT,
"이미 존재하는 아이디입니다.", e) so DB unique-constraint violations are mapped to a
CONFLICT error; ensure the existing pre-check and password encoding
(passwordEncoder.encode / memberModel.encryptPassword) remain before the save
and keep the method transactional (e.g., `@Transactional`) so the catch handles
the integrity exception correctly.


public interface MemberJpaRepository extends JpaRepository<MemberModel, Long> {

Optional<MemberModel> findByLoginId(String loginId);
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:

# MemberModel 엔티티 찾기
fd -e java -type f | xargs grep -l "class MemberModel" | head -5

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

Length of output: 255


🏁 Script executed:

# MemberModel 엔티티 찾기
fd "MemberModel" --extension java

# MemberModel 엔티티 전체 내용 확인
fd "MemberModel" --extension java --exec cat {} \;

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

Length of output: 6967


🏁 Script executed:

# SQL 마이그레이션 파일 찾기 (flyway, liquibase, schema.sql 등)
fd -e sql . | head -20

# 각 SQL 파일에서 login_id 또는 loginId 검색
fd -e sql . --exec grep -l -i "login_id\|loginId" {} \;

# schema.sql 파일이 있는지 확인
find . -name "schema.sql" -o -name "V*.sql" -o -name "db.changelog*.xml" 2>/dev/null

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

Length of output: 67


🏁 Script executed:

# migration 관련 디렉토리 찾기
find . -type d -name "*migration*" -o -name "*flyway*" -o -name "*liquibase*" -o -name "*db*" 2>/dev/null | grep -v node_modules

# properties/yml 파일에서 DDL 생성 설정 확인
fd -e properties -e yml . --exec grep -l "jpa\|ddl\|hibernate" {} \;

# MemberModel 엔티티의 전체 애노테이션 확인 (특히 `@Index`, `@Table`)
cat apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java | grep -E "@|private String" | head -20

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

Length of output: 775


🏁 Script executed:

# jpa.yml 내용 확인
cat ./modules/jpa/src/main/resources/jpa.yml

# commerce-api application.yml 내용 확인
cat ./apps/commerce-api/src/main/resources/application.yml

# MemberModel이 참조된 다른 파일들 (설정, 테스트 등) 확인
fd -e java . --exec grep -l "MemberModel\|loginId" {} \; | grep -v test | head -10

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

Length of output: 3956


🏁 Script executed:

# MemberRepositoryImpl 내용 확인
cat ./apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberRepositoryImpl.java

# MemberJpaRepository 전체 내용 확인
cat ./apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberJpaRepository.java

# MemberService에서 findByLoginId 사용 여부 확인
cat ./apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java

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

Length of output: 2634


loginId 컬럼에 인덱스 및 유니크 제약조건을 정의해야 한다.

findByLoginId()는 회원가입 시 중복 체크(saveMember)와 로그인 시 조회(getMember)에서 반복 호출된다. 현재 MemberModel 엔티티에 @Index 애노테이션이 없고, 프로덕션 환경에서 ddl-auto: none으로 설정되어 있어 수동 스키마 관리가 필요하다. 사용자 수 증가 시 인덱스 부재로 인한 full table scan이 발생하여 성능 저하를 초래한다.

수정안: MemberModel에 @Table(indexes = @Index(name = "idx_login_id", columnList = "login_id", unique = true))를 추가하거나, 데이터베이스 마이그레이션 파일에서 CREATE UNIQUE INDEX idx_login_id ON member(login_id);를 정의한다. 프로덕션 배포 전 인덱스 생성 여부를 데이터베이스에서 직접 확인한다.

🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberJpaRepository.java`
at line 9, The loginId column lacks an index/unique constraint, causing repeated
findByLoginId calls (used by MemberJpaRepository.findByLoginId, saveMember and
getMember) to potentially trigger full table scans; add the index either by
annotating the entity MemberModel with `@Table`(indexes = `@Index`(name =
"idx_login_id", columnList = "login_id", unique = true)) or by creating a DB
migration that runs CREATE UNIQUE INDEX idx_login_id ON member(login_id); verify
the index exists in production before deploy.

Comment on lines +20 to +23
@Override
public Optional<MemberModel> update(MemberModel memberModel) {
return Optional.empty();
}
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

update 메서드가 빈 구현으로 존재

Optional.empty()를 반환하는 stub 구현이다. 실수로 호출 시 데이터가 업데이트되지 않고 silent failure가 발생한다. 구현 전까지 UnsupportedOperationException을 던지거나 인터페이스에서 제거하는 것을 권장한다.

수정 제안 (미구현 명시)
   `@Override`
   public Optional<MemberModel> update(MemberModel memberModel) {
-    return Optional.empty();
+    throw new UnsupportedOperationException("update is not implemented yet");
   }
📝 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
@Override
public Optional<MemberModel> update(MemberModel memberModel) {
return Optional.empty();
}
`@Override`
public Optional<MemberModel> update(MemberModel memberModel) {
throw new UnsupportedOperationException("update is not implemented yet");
}
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberRepositoryImpl.java`
around lines 20 - 23, 메서드 update가 현재 Optional.empty()로 빈 구현되어 있어 호출 시 무시되는 실패가
발생하므로, 당장 구현하지 않는다면 MemberRepositoryImpl.update(MemberModel memberModel)에서
Optional.empty()를 반환하는 대신 UnsupportedOperationException을 던지도록 변경하거나(예: throw new
UnsupportedOperationException("MemberRepositoryImpl.update not implemented")),
실제 동작을 구현할 준비가 되었다면 MemberModel을 받아 DB를 업데이트하고 업데이트된 MemberModel을
Optional.of(...)로 반환하도록 구현하세요; 대상 식별자는 클래스 MemberRepositoryImpl와 메서드
update(MemberModel memberModel)입니다.

Comment on lines 68 to 72
// assert
assertAll(
() -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED),
() -> assertThat(response.getBody().data().get("loginId")).isNotNull()
);
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

response.getBody() null 체크 누락으로 테스트 실패 시 NPE 발생 가능

테스트 실패 시 응답 본문이 null일 수 있어 response.getBody().data()에서 NullPointerException이 발생할 수 있다. 테스트 디버깅이 어려워지는 원인이 된다.

수정 제안
             // assert
             assertAll(
                 () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED),
-                () -> assertThat(response.getBody().data().get("loginId")).isNotNull()
+                () -> assertThat(response.getBody()).isNotNull(),
+                () -> assertThat(response.getBody().data()).isNotNull(),
+                () -> assertThat(response.getBody().data().get("loginId")).isNotNull()
             );
📝 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
// assert
assertAll(
() -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED),
() -> assertThat(response.getBody().data().get("loginId")).isNotNull()
);
// assert
assertAll(
() -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED),
() -> assertThat(response.getBody()).isNotNull(),
() -> assertThat(response.getBody().data()).isNotNull(),
() -> assertThat(response.getBody().data().get("loginId")).isNotNull()
);
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java`
around lines 68 - 72, The test can NPE if response.getBody() is null; update the
assertions to first assert the response body is non-null before calling data(),
e.g., add an assertion on response.getBody() (or
assertThat(response.getBody()).isNotNull()) as the first check in assertAll so
subsequent access to response.getBody().data() (and the check of "loginId") is
safe; locate the assertion block using response.getBody(), data(), and assertAll
to apply this change.

build.gradle.kts Outdated
Comment on lines 43 to 46
// TODO: JDK 24 환경에서 JacocoReport 태스크 생성 오류 발생 - JDK 21로 전환 후 활성화 필요
// if (name !in containerProjects) {
// apply(plugin = "jacoco")
// }
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

🌐 Web query:

JaCoCo JDK 24 support latest version compatibility

💡 Result:

JaCoCo officially supports JDK 24 starting with version 0.8.13 (released 2025-04-02). If you see Unsupported class file major version 68 on Java 24, you’re likely on 0.8.12 or older. [1], [2]

Latest release: JaCoCo 0.8.14 (released 2025-10-11)—it still supports Java 24 and additionally adds official Java 25 support. [1]

Recommendation: use JaCoCo ≥ 0.8.13 for JDK 24; preferably 0.8.14 unless you need a newer snapshot build. [1]

Sources:
[1] https://www.jacoco.org/jacoco/trunk/doc/changes
[2] jacoco/jacoco#1858


🏁 Script executed:

# Check build.gradle.kts file for JaCoCo configuration
find . -name "build.gradle.kts" -type f | head -5

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

Length of output: 252


🏁 Script executed:

# Check the build.gradle.kts file content, especially around lines 43-46 and 96-116
cat -n ./build.gradle.kts | head -120

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

Length of output: 5014


JaCoCo를 즉시 활성화하여 테스트 커버리지 수집을 재개해야 한다.

JDK 21로 이미 구성되어 있음에도 불구하고(line 22) JaCoCo가 비활성화되어 있어 CI/CD 파이프라인의 커버리지 리포팅이 중단된 상태다. TODO 주석의 "JDK 24 환경에서" 언급은 현재 설정과 맞지 않으며, JaCoCo 0.8.13 이상(현재 최신 버전 0.8.14)이 JDK 24를 공식 지원하므로 버전 업그레이드로도 문제 해결이 가능하다.

lines 43-46과 96-116의 주석 처리된 코드를 즉시 활성화하고, TODO 주석을 제거해야 한다. 추가적으로 JaCoCo 버전을 명시적으로 지정하거나 최신 버전(0.8.14 이상)으로 업그레이드할 것을 권장한다.

🤖 Prompt for AI Agents
In `@build.gradle.kts` around lines 43 - 46, Un-comment and remove the TODO:
re-enable the apply(plugin = "jacoco") logic (the commented apply(plugin =
"jacoco") block that gated jacoco for non-container projects) and the second
commented JaCoCo-related block (the commented section around jacocoTestReport /
JacocoReport configuration), then add an explicit JaCoCo toolVersion setting
(e.g., configure the jacoco { toolVersion = "0.8.14" } or set in the Jacoco
plugin extension) so the build uses JaCoCo 0.8.14+ and ensure any
jacocoTestReport/JacocoReport task configuration is active. Ensure the TODO
about JDK 24 is removed and the plugin application and report task names
(apply(plugin = "jacoco"), jacoco { toolVersion = ... }, jacocoTestReport /
JacocoReport) are present and configured to produce coverage in CI.

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

🤖 Fix all issues with AI agents
In
`@apps/commerce-api/src/main/java/com/loopers/application/member/MemberFacade.java`:
- Around line 39-47: The Facade currently authenticates with the header password
then calls memberService.changePassword with the body prevPassword, which can
cause mismatches; update MemberFacade.changePassword so authentication uses the
body prevPassword (i.e., call memberService.authenticate(loginId, prevPassword)
instead of memberService.authenticate(loginId, password)) and construct
MemberModel with the same prevPassword before calling
memberService.changePassword(memberModel, newPassword), ensuring the header
password is not used for the change flow (or add a clarifying comment if the
header must be preserved for another purpose).

In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`:
- Around line 68-71: The constructor MemberModel(String loginId, String
prevPassword) bypasses validation and can create invalid entities; update it to
perform at least the loginId validation (reusing existing validation logic used
elsewhere) or replace the constructor with a clearly named static factory (e.g.,
forPasswordChange) that documents/limits its intent, and ensure password
validation rules are enforced where appropriate; update usages such as
MemberFacade.changePassword to call the new validated factory or constructor,
and add tests asserting that instances created via this path cannot be persisted
to the DB if they violate invariants.
- Around line 141-145: MemberModel.changePassword currently stores the plaintext
password after validation which risks saving unhashed values if callers forget
to call MemberService.encryptPassword; either make changePassword perform
hashing itself using the PasswordEncoder (e.g., call encoder.encode(...) inside
MemberModel.changePassword) or change its signature to accept only an
already-encrypted password (e.g., changePasswordHashed) and enforce that
contract; update MemberService.changePassword to use the new API accordingly and
add a test that calls the flow and asserts the DB-stored password is not equal
to the plaintext and that PasswordEncoder.matches(plaintext, storedValue)
returns true.

In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java`:
- Around line 41-50: The controller method getMyInfo in MemberV1Controller
currently reads a plaintext password from the X-Loopers-LoginPw header which
risks logging secrets; remove usage of that header and switch to token-based
authentication instead (e.g., accept an Authorization: Bearer <token> header and
resolve the user from the token or SecurityContext), update
memberFacade.getMyInfo signature to accept the authenticated principal or
token-derived user id rather than login+password, and ensure any request/access
logging excludes the X-Loopers-LoginPw header (or remove the header entirely).
Add tests to verify authentication works with a token and that server logs do
not contain the X-Loopers-LoginPw value.
🧹 Nitpick comments (16)
apps/commerce-api/src/main/java/com/loopers/application/member/MemberInfo.java (1)

6-14: MemberInfo 레코드 구현이 적절하다.

getMaskedName() 사용으로 개인정보 마스킹이 적용되어 있다. Record의 불변성으로 데이터 무결성이 보장된다.

단, from() 메서드에서 null MemberModel 전달 시 NPE가 발생할 수 있다. 현재 호출 경로에서는 null이 전달될 가능성이 낮으나, 방어적 프로그래밍 관점에서 null 체크를 고려할 수 있다.

방어적 null 체크 예시
 public static MemberInfo from(MemberModel model) {
+    if (model == null) {
+        throw new IllegalArgumentException("MemberModel cannot be null");
+    }
     return new MemberInfo(
         model.getLoginId(),
         model.getMaskedName(),
         model.getBirthDate(),
         model.getEmail()
       );
 }
apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberJpaRepository.java (1)

10-10: save 메서드 선언이 중복이다.

JpaRepository가 이미 save(S entity) 메서드를 제공하므로 명시적 선언이 불필요하다. 제거하여 코드 간결성을 유지하는 것을 권장한다.

수정 제안
 public interface MemberJpaRepository extends JpaRepository<MemberModel, Long> {
 
   Optional<MemberModel> findByLoginId(String loginId);
-  MemberModel save(MemberModel memberModel);
 }
apps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.java (2)

105-131: 비밀번호 변경 테스트의 검증 로직이 불완전하다

Line 130에서 updatedMember.getPassword()가 이전과 다른지만 확인하고 있다. 운영 환경에서 비밀번호 변경 후 실제로 새 비밀번호로 인증이 가능한지 검증하지 않으면, 암호화 로직 오류나 저장 실패를 감지하지 못한다.

수정안:

  1. 변경된 비밀번호로 재인증 테스트 추가
  2. 기존 비밀번호로 인증 실패 테스트 추가
검증 강화 제안
      // assert
      MemberModel updatedMember = memberService.getMember("testuser");
      // 비밀번호가 변경되었는지 확인 (암호화된 비밀번호 비교)
      assertThat(updatedMember.getPassword()).isNotEqualTo(insertedMember.getPassword());
+
+     // 새 비밀번호로 인증 가능한지 확인
+     MemberModel reAuthenticated = memberService.authenticate(loginId, newPassword);
+     assertThat(reAuthenticated).isNotNull();
    }

45-47: assertAll 내 assertion 가독성 개선 필요

한 줄에 여러 assertion을 나열하면 테스트 실패 시 어느 assertion이 실패했는지 파악하기 어렵고, 코드 리뷰 시에도 검증 항목을 놓치기 쉽다.

포맷팅 제안
      // assert
      assertAll(
-        () -> assertThat(result).isNotNull(), () -> assertThat(result.getLoginId()).isEqualTo(memberModel.getLoginId()), () -> assertThat(result.getName()).isEqualTo(memberModel.getName()), () -> assertThat(result.getBirthDate()).isEqualTo(memberModel.getBirthDate()), () -> assertThat(result.getEmail()).isEqualTo(memberModel.getEmail()));
+        () -> assertThat(result).isNotNull(),
+        () -> assertThat(result.getLoginId()).isEqualTo(memberModel.getLoginId()),
+        () -> assertThat(result.getName()).isEqualTo(memberModel.getName()),
+        () -> assertThat(result.getBirthDate()).isEqualTo(memberModel.getBirthDate()),
+        () -> assertThat(result.getEmail()).isEqualTo(memberModel.getEmail())
+      );
apps/commerce-api/src/main/java/com/loopers/domain/member/MemberRepository.java (1)

8-8: 미구현 update 메서드 존재

update 메서드가 인터페이스에 선언되어 있으나, MemberRepositoryImpl에서 Optional.empty()를 반환한다. 현재 비밀번호 변경은 JPA dirty checking으로 처리되므로 이 메서드는 사용되지 않는다.

운영 관점: 미구현 메서드는 향후 개발자가 잘못 사용할 위험이 있다. 사용하지 않는다면 제거하거나, 구현 계획이 있다면 @Deprecated 또는 UnsupportedOperationException을 던지도록 변경해야 한다.

제거 또는 명시적 미구현 처리 제안
 public interface MemberRepository {
     MemberModel save(MemberModel memberModel);
     Optional<MemberModel> findByLoginId(String id);
-    Optional<MemberModel> update(MemberModel memberModel);
 }

또는 구현체에서:

`@Override`
public Optional<MemberModel> update(MemberModel memberModel) {
    throw new UnsupportedOperationException("Not implemented yet");
}
apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java (2)

38-49: authenticate 메서드에 @Transactional 누락

authenticategetMember를 호출하여 내부적으로 트랜잭션이 적용되지만, 메서드 자체에 @Transactional(readOnly = true)을 명시하는 것이 일관성 있고, 향후 로직 추가 시 트랜잭션 범위 문제를 예방한다.

@transactional 추가 제안
+  `@Transactional`(readOnly = true)
   public MemberModel authenticate(String loginId, String password) {
     // 1. 회원 조회
     MemberModel member = getMember(loginId);  // 없으면 NOT_FOUND

51-56: getMember에서 불필요한 MemberModel 생성

Line 53에서 new MemberModel(loginId)를 생성한 후 model.getLoginId()만 사용한다. 파라미터 loginId를 직접 사용하면 불필요한 객체 생성을 피할 수 있다.

불필요한 객체 생성 제거
   `@Transactional`(readOnly = true)
   public MemberModel getMember(String loginId) {
-    MemberModel model = new MemberModel(loginId);  // 객체 먼저 생성해야 함
-    return memberRepository.findByLoginId(model.getLoginId())
+    return memberRepository.findByLoginId(loginId)
         .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "[id = " + loginId + "] 회원을 찾을 수 없습니다."));
   }
apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java (3)

70-81: 중복된 null 체크 assertion

assertThat(response.getBody()).isNotNull() 이후 다시 Assertions.assertNotNull(response.getBody())를 호출하고 있다. 같은 검증을 반복하여 테스트 가독성이 떨어진다.

중복 제거 제안
            // assert
            assertAll(
                () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED),
-               () -> assertThat(response.getBody()).isNotNull(),
-               () -> {
-                 Assertions.assertNotNull(response.getBody());
-                 assertThat(response.getBody().data()).isNotNull();
-               },
-               () -> {
-                 Assertions.assertNotNull(response.getBody());
-                 assertThat(response.getBody().data().get("loginId")).isNotNull();
-               }
+               () -> {
+                 assertThat(response.getBody()).isNotNull();
+                 assertThat(response.getBody().data()).isNotNull();
+                 assertThat(response.getBody().data().get("loginId")).isNotNull();
+               }
            );

51-59: 테스트 데이터 생성 중복

signUpRequest Map 생성이 여러 테스트에서 반복된다. 테스트 유지보수성을 위해 helper method 또는 상수로 추출하는 것을 권장한다.

Helper method 추출 제안
private Map<String, String> createSignUpRequest(String loginId, String password) {
    return Map.of(
        "loginId", loginId,
        "password", password,
        "name", "홍길동",
        "birthDate", "19900101",
        "email", "test@example.com"
    );
}

208-251: 비밀번호 변경 후 새 비밀번호로 로그인 검증 누락

E2E 테스트에서 비밀번호 변경 성공 응답(200 OK)만 확인하고, 실제로 새 비밀번호로 /me 조회가 가능한지 검증하지 않는다. 운영 환경에서 비밀번호 변경 후 즉시 세션/인증 문제가 발생할 수 있으므로, 변경 후 재인증 테스트가 필요하다.

재인증 검증 추가 제안
            // assert
            assertAll(
                () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK),
                () -> assertThat(response.getBody()).isNotNull(),
                () -> {
                  Assertions.assertNotNull(response.getBody());
                  assertThat(response.getBody().data()).isEqualTo("비밀번호가 변경되었습니다.");
                }
            );
+
+           // 새 비밀번호로 재인증 검증
+           HttpHeaders newHeaders = new HttpHeaders();
+           newHeaders.set("X-Loopers-LoginId", "testuser");
+           newHeaders.set("X-Loopers-LoginPw", "NewPass123!");
+           ResponseEntity<ApiResponse<Map<String, String>>> meResponse = testRestTemplate.exchange(
+               ENDPOINT + "/me",
+               HttpMethod.GET,
+               new HttpEntity<>(newHeaders),
+               new ParameterizedTypeReference<>() {}
+           );
+           assertThat(meResponse.getStatusCode()).isEqualTo(HttpStatus.OK);
        }
apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1ApiSpec.java (1)

25-28: 보안 관점: 비밀번호 헤더 전송 방식 검토

X-Loopers-LoginPw 헤더로 비밀번호를 전송하는 방식은 다음 보안 위험이 있다:

  • HTTP 접근 로그에 헤더가 기록될 수 있음
  • 프록시/로드밸런서에서 헤더가 노출될 수 있음
  • 브라우저 개발자 도구에서 쉽게 확인 가능

운영 환경에서는 JWT/세션 기반 인증 또는 Authorization 헤더(Bearer token) 방식으로 전환을 권장한다. 현재 학습/개발 단계라면 주석으로 보안 한계를 명시해야 한다.

apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java (1)

52-60: @Override 누락 및 들여쓰기 불일치

changePassword 메서드에 @Override 어노테이션이 누락되어 있고, 다른 메서드와 들여쓰기가 일관되지 않다. MemberV1ApiSpec 인터페이스 계약 준수를 컴파일 타임에 보장하려면 @Override를 추가해야 한다.

🔧 수정 예시
   `@PatchMapping`("/me/password")
+  `@Override`
   public ApiResponse<String> changePassword(
       `@RequestHeader`("X-Loopers-LoginId") String loginId,
       `@RequestHeader`("X-Loopers-LoginPw") String password,
       `@RequestBody` ChangePasswordRequest request
   ) {
-      memberFacade.changePassword(loginId, password, request.oldPassword(), request.newPassword());
-      return ApiResponse.success("비밀번호가 변경되었습니다.");
-    }
+    memberFacade.changePassword(loginId, password, request.oldPassword(), request.newPassword());
+    return ApiResponse.success("비밀번호가 변경되었습니다.");
+  }
apps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.java (2)

153-167: @DisplayName 중복으로 테스트 의도 구분 불가

Line 137과 Line 153의 @DisplayName이 동일한 "이름 마지막 글자를 마스킹한다"로 설정되어 있다. 테스트 실패 시 어떤 케이스인지 식별이 어렵고, 테스트 리포트 가독성이 저하된다.

🔧 수정 예시
-    `@DisplayName`("이름 마지막 글자를 마스킹한다")
+    `@DisplayName`("한 글자 이름은 전체가 마스킹된다")
     `@Test`
     void single_character_name_is_fully_masked() {

175-192: 비밀번호 변경 테스트에서 기존 비밀번호와 동일한 경우 누락

ChangePassword 테스트 그룹에서 새 비밀번호가 기존 비밀번호와 동일할 때의 검증이 누락되어 있다. MemberServiceIntegrationTest에서는 커버되지만, 단위 테스트 수준에서 MemberModel.changePassword가 동일 비밀번호를 거부하는지 검증이 필요하다.

추가 테스트: 기존 비밀번호와 새 비밀번호가 동일할 때 예외 발생 여부를 확인하는 테스트를 추가해야 한다.

apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java (2)

117-120: encryptPassword 메서드의 public 접근 제한 필요

encryptPassword가 public이면 외부에서 임의의 값으로 비밀번호를 덮어쓸 수 있다. 도메인 무결성 보호를 위해 패키지 프라이빗 또는 서비스 계층에서만 접근 가능하도록 제한해야 한다.

🔧 수정 예시
- public void encryptPassword(String encryptedPassword) {
+ void encryptPassword(String encryptedPassword) {
     this.password = encryptedPassword;
   }

122-133: maskLastChar 메서드를 static 유틸리티로 분리 가능

maskLastChar는 인스턴스 상태에 의존하지 않으므로 static 메서드로 변경하거나, 별도 유틸리티 클래스로 분리하면 재사용성과 테스트 용이성이 향상된다.

🔧 수정 예시
- public String maskLastChar(String name) {
+ public static String maskLastChar(String name) {
     if (name == null || name.isBlank()) {
       throw new IllegalArgumentException("이름은 비어 있을 수 없습니다.");
     }

Comment on lines +39 to +47
public void changePassword(String loginId, String password, String prevPassword, String newPassword) {
// 헤더 인증
memberService.authenticate(loginId, password);

MemberModel memberModel = new MemberModel(loginId, prevPassword); // raw prevPassword

// Service 호출
memberService.changePassword(memberModel, newPassword);

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:

# MemberV1Controller에서 changePassword 호출 방식 확인
rg -n -A10 'changePassword' apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java

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

Length of output: 506


🏁 Script executed:

# MemberService.changePassword 구현 확인
rg -n -A15 'changePassword' apps/commerce-api/src/main/java/com/loopers/application/member/MemberService.java

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

Length of output: 206


🏁 Script executed:

# MemberService.authenticate 구현 확인
rg -n -A10 'void authenticate' apps/commerce-api/src/main/java/com/loopers/application/member/MemberService.java

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

Length of output: 206


🏁 Script executed:

# MemberService 파일 위치 검색
fd -n 'MemberService' apps/commerce-api --type f

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

Length of output: 315


🏁 Script executed:

# 디렉토리 구조 확인
tree -L 4 apps/commerce-api/src/main/java/com/loopers/application/member/

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

Length of output: 212


🏁 Script executed:

# MemberService 인터페이스 또는 클래스 정의 찾기
rg 'class MemberService|interface MemberService' apps/commerce-api --type java

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

Length of output: 319


🏁 Script executed:

# MemberFacade.java 전체 내용 확인하여 의존성 파악
cat -n apps/commerce-api/src/main/java/com/loopers/application/member/MemberFacade.java

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

Length of output: 1818


🏁 Script executed:

# MemberService.java의 changePassword와 authenticate 메서드 확인
cat -n apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java

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

Length of output: 3494


changePassword에서 헤더 비밀번호와 body 이전 비밀번호 불일치 발생 가능

MemberV1Controller에서 @RequestHeader("X-Loopers-LoginPw") String password(헤더)와 request.oldPassword()(body)를 분리하여 전달하므로 두 값이 다를 수 있다. 현재 Facade의 설계에서는 헤더 password로 authenticate를 수행한 후, body의 prevPassword로 changePassword를 검증하므로 두 비밀번호가 일치하지 않으면 "기존 비밀번호가 일치하지 않습니다" 예외 발생한다.

운영 관점 문제:

  • 클라이언트가 헤더와 body에 동일한 비밀번호를 전송해야 한다는 요구사항이 명확하지 않음
  • 실패 시 에러 메시지가 모호하여 디버깅 어려움
  • 의도적인 이중 확인이라면 주석으로 명시 필요

개선안:

  1. body의 oldPassword 하나로 통일하고 헤더에서 제거하거나
  2. 헤더 비밀번호 하나만 사용하여 단일 인증으로 단순화하거나
  3. 현재 설계 유지 시 Facade에서 authenticate(loginId, prevPassword) 호출로 변경하고, 헤더 password는 검증 생략 또는 목적 명시
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/application/member/MemberFacade.java`
around lines 39 - 47, The Facade currently authenticates with the header
password then calls memberService.changePassword with the body prevPassword,
which can cause mismatches; update MemberFacade.changePassword so authentication
uses the body prevPassword (i.e., call memberService.authenticate(loginId,
prevPassword) instead of memberService.authenticate(loginId, password)) and
construct MemberModel with the same prevPassword before calling
memberService.changePassword(memberModel, newPassword), ensuring the header
password is not used for the change flow (or add a clarifying comment if the
header must be preserved for another purpose).

Comment on lines +68 to +71
public MemberModel(String loginId, String prevPassword) {
this.loginId = loginId;
this.password = prevPassword;
}
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

검증 우회 생성자로 인한 무결성 위반 가능성

MemberModel(String loginId, String prevPassword) 생성자는 loginId 검증과 password 검증을 모두 생략한다. 이 생성자를 통해 잘못된 형식의 loginId나 규칙에 맞지 않는 비밀번호가 포함된 객체가 생성될 수 있다. 현재 MemberFacade.changePassword에서 사용되는데, 서비스 계층에서 인증 후 사용하더라도 엔티티 자체의 불변식(invariant)이 깨질 수 있다.

수정안: 최소한 loginId 검증은 수행하거나, 용도를 명확히 하는 정적 팩토리 메서드로 분리해야 한다.
추가 테스트: 해당 생성자로 생성된 객체가 DB에 저장되지 않는지 검증해야 한다.

🔧 수정 예시
- public MemberModel(String loginId, String prevPassword) {
-   this.loginId = loginId;
-   this.password = prevPassword;
- }
+ // 인증 전용 VO - DB 저장 불가
+ public static MemberModel forAuthentication(String loginId, String password) {
+   if (loginId == null || loginId.isBlank()) {
+     throw new CoreException(ErrorType.BAD_REQUEST, "아이디는 비어있을 수 없습니다.");
+   }
+   MemberModel model = new MemberModel();
+   model.loginId = loginId;
+   model.password = password;
+   return model;
+ }
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`
around lines 68 - 71, The constructor MemberModel(String loginId, String
prevPassword) bypasses validation and can create invalid entities; update it to
perform at least the loginId validation (reusing existing validation logic used
elsewhere) or replace the constructor with a clearly named static factory (e.g.,
forPasswordChange) that documents/limits its intent, and ensure password
validation rules are enforced where appropriate; update usages such as
MemberFacade.changePassword to call the new validated factory or constructor,
and add tests asserting that instances created via this path cannot be persisted
to the DB if they violate invariants.

Comment on lines +141 to +145
// 비밀번호 변경하기
public void changePassword(String newPassword, String birthDate) {
validatePassword(newPassword, birthDate);
this.password = newPassword;
}
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

changePassword에서 평문 비밀번호 저장

changePassword 메서드가 검증 후 평문 비밀번호를 그대로 필드에 저장한다. MemberService.changePassword에서 호출 후 encryptPassword를 별도 호출해야 하는 구조인데, 이 호출이 누락되면 DB에 평문이 저장된다.

수정안: changePassword에서 암호화까지 수행하거나, 암호화된 값만 받도록 메서드 시그니처를 변경해야 한다.
추가 테스트: changePassword 호출 후 DB에 저장된 값이 평문과 다르고 PasswordEncoder.matches가 true인지 검증해야 한다.

🔧 수정 예시
- public void changePassword(String newPassword, String birthDate) {
-   validatePassword(newPassword, birthDate);
-   this.password = newPassword;
- }
+ public void changePassword(String newPassword, String birthDate, String encryptedPassword) {
+   validatePassword(newPassword, birthDate);
+   this.password = encryptedPassword;
+ }
#!/bin/bash
# Description: MemberService.changePassword에서 encryptPassword 호출 여부 확인
rg -n -A 10 'changePassword' apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`
around lines 141 - 145, MemberModel.changePassword currently stores the
plaintext password after validation which risks saving unhashed values if
callers forget to call MemberService.encryptPassword; either make changePassword
perform hashing itself using the PasswordEncoder (e.g., call encoder.encode(...)
inside MemberModel.changePassword) or change its signature to accept only an
already-encrypted password (e.g., changePasswordHashed) and enforce that
contract; update MemberService.changePassword to use the new API accordingly and
add a test that calls the flow and asserts the DB-stored password is not equal
to the plaintext and that PasswordEncoder.matches(plaintext, storedValue)
returns true.

Comment on lines +41 to +50
@GetMapping("/me")
@Override
public ApiResponse<MemberInfoResponse> getMyInfo(
@RequestHeader("X-Loopers-LoginId") String loginId,
@RequestHeader("X-Loopers-LoginPw") String password
) {
MemberInfo info = memberFacade.getMyInfo(loginId, password);
MemberInfoResponse response = MemberInfoResponse.from(info);
return ApiResponse.success(response);
}
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

헤더 기반 인증 시 비밀번호 평문 전송 위험

X-Loopers-LoginPw 헤더로 비밀번호를 전송하면 서버 액세스 로그, 프록시 로그, CDN 캐시 등에 평문 비밀번호가 기록될 수 있다. HTTP 헤더는 일반적으로 로그에 포함되므로 운영 환경에서 비밀번호 유출 사고로 이어질 수 있다.

수정안: 토큰 기반 인증(JWT, 세션 토큰 등)으로 전환하거나, 최소한 로깅 설정에서 해당 헤더를 제외하도록 구성해야 한다.
추가 테스트: 서버 로그에 X-Loopers-LoginPw 헤더 값이 기록되지 않는지 검증해야 한다.

🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java`
around lines 41 - 50, The controller method getMyInfo in MemberV1Controller
currently reads a plaintext password from the X-Loopers-LoginPw header which
risks logging secrets; remove usage of that header and switch to token-based
authentication instead (e.g., accept an Authorization: Bearer <token> header and
resolve the user from the token or SecurityContext), update
memberFacade.getMyInfo signature to accept the authenticated principal or
token-derived user id rather than login+password, and ensure any request/access
logging excludes the X-Loopers-LoginPw header (or remove the header entirely).
Add tests to verify authentication works with a token and that server logs do
not contain the X-Loopers-LoginPw value.

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

🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`:
- Around line 123-126: Replace the IllegalArgumentException in
MemberModel.maskLastChar with a CoreException so errors flow through
ApiControllerAdvice: change the null/blank check to throw new
CoreException(ErrorType.BAD_REQUEST, "이름은 비어 있을 수 없습니다.") (or equivalent
localized message), ensuring CoreException and ErrorType.BAD_REQUEST are
imported/used; then add a unit test asserting that calling
MemberModel.maskLastChar(null) and maskLastChar("") results in a
BAD_REQUEST-mapped CoreException (or that the controller advice transforms it
into the standardized BAD_REQUEST response) to verify unified error handling
through ApiControllerAdvice.

In
`@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberRepository.java`:
- Around line 5-9: The MemberRepository.update method is a dead/non-standard
contract (MemberRepository.update) because MemberRepositoryImpl.update returns
Optional.empty() and no callers use updateMemberModel; remove the misleading API
by deleting the update declaration from MemberRepository and removing its stub
implementation from MemberRepositoryImpl, leaving save(MemberModel) as the
single write operation; alternatively, if you prefer to keep update, implement
MemberRepositoryImpl.update to perform a proper merge/update and return the
persisted MemberModel wrapped in Optional, but do not leave a no-op
Optional.empty() implementation.

In
`@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`:
- Around line 51-56: The getMember method unnecessarily instantiates a
MemberModel just to read its loginId; remove the new MemberModel(loginId) and
call memberRepository.findByLoginId(loginId) directly, keeping the existing
orElseThrow(...) that throws new CoreException(ErrorType.NOT_FOUND, "[id = " +
loginId + "] 회원을 찾을 수 없습니다."); this avoids needless object creation and any side
effects from MemberModel's constructor while preserving behavior in getMember.
- Around line 38-49: The authenticate method lacks a transactional boundary
which can leave the MemberModel detached after getMember(); add a read-only
transaction to authenticate by annotating the method with
`@Transactional`(readOnly = true) so lazy-loaded associations on MemberModel
remain accessible; update imports to include
org.springframework.transaction.annotation.Transactional and ensure the
authenticate method (and its signature in MemberService) is the one annotated,
leaving getMember() unchanged.
- Around line 33-35: When converting DataIntegrityViolationException to
CoreException in MemberService, preserve the original exception as the cause:
update the catch block that currently throws new
CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.") to include the caught
exception (e) as the cause (e.g., use a CoreException constructor or factory
that accepts Throwable) so the original stack trace and message are retained for
logging and debugging.

In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1ApiSpec.java`:
- Around line 26-28: The current API spec (MemberV1ApiSpec) accepts a plaintext
password via the X-Loopers-LoginPw RequestHeader (method parameter "password");
stop accepting passwords in headers and migrate this endpoint to token-based
authentication (e.g., issue/require JWTs from an authentication flow and remove
the X-Loopers-LoginPw header parameter from MemberV1ApiSpec and its
implementations). Until the token migration is complete, ensure the sensitive
header is never logged by adding a logback filter/masking rule for
"X-Loopers-LoginPw" at the appender level (apply to logback.xml used in
local/dev/prod), verify masking across environments, and add a short deprecation
note and TODO in the login controller/handler referencing removal of
X-Loopers-LoginPw.

In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java`:
- Around line 52-60: The changePassword method in MemberV1Controller is missing
the `@Override` annotation; add `@Override` immediately above the changePassword
method declaration to match other implemented methods (signUp, getMyInfo) and
ensure the method signature exactly matches the one declared in MemberV1ApiSpec
(changePassword(...)) so compilation will catch future interface changes.

In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java`:
- Around line 212-251: The test
returnsOk_whenHeaderAuthAndPasswordChangeAreValid only asserts HTTP 200 and
message; add real verification that the password changed by performing two
additional authenticated requests using testRestTemplate: (1) GET ENDPOINT +
"/me" with headers X-Loopers-LoginId="testuser" and
X-Loopers-LoginPw="NewPass123!" and assert a HttpStatus.OK response (and
non-null body), and (2) GET ENDPOINT + "/me" with X-Loopers-LoginPw="Test1234!"
and assert HttpStatus.UNAUTHORIZED; use the same
ParameterizedTypeReference<ApiResponse<Map<String,String>>>/ResponseEntity types
as the existing calls and keep assertions consistent with the existing style in
the test method returnsOk_whenHeaderAuthAndPasswordChangeAreValid.
- Around line 84-107: The test returnsConflict_whenDuplicateLoginIdIsProvided
does the first signup call via testRestTemplate.exchange but never checks its
result; capture that first ResponseEntity (the initial testRestTemplate.exchange
call) and assert its status is HttpStatus.CREATED (and optionally assert body is
non-null) before performing the duplicate signup, so the test reliably verifies
the precondition; update the method in MemberV1ApiE2ETest to store the first
response and add an assertion on its status (referencing ENDPOINT and
testRestTemplate.exchange).
🧹 Nitpick comments (8)
docker/infra-compose.yml (1)

1-1: docker-compose version 필드는 deprecated 상태이다.

Docker Compose V2에서는 version 필드가 무시되며, 향후 제거될 예정이다. 현재 동작에는 영향이 없으나, deprecation 경고를 방지하려면 해당 라인을 제거하거나 명시적 버전(예: '3.8')을 사용하는 것을 권장한다.

♻️ version 필드 제거 제안
-version: '3'
 services:
   mysql:
apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberJpaRepository.java (1)

10-10: save() 메서드 선언이 불필요하다.

JpaRepository가 이미 save(S entity) 메서드를 제공하므로 명시적 선언이 중복된다. 제거해도 동작에 영향이 없다.

♻️ 수정 제안
 public interface MemberJpaRepository extends JpaRepository<MemberModel, Long> {

   Optional<MemberModel> findByLoginId(String loginId);
-  MemberModel save(MemberModel memberModel);
 }
apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java (1)

74-79: 비밀번호가 두 번 설정되어 혼란을 유발한다.

Line 75에서 member.changePassword(newPassword, birthDate)를 호출하여 평문 비밀번호를 설정한 후, Line 78-79에서 다시 암호화된 비밀번호로 덮어쓴다. changePassword()가 내부적으로 비밀번호 규칙 검증만 수행하고 실제 설정은 encryptPassword()에서 한다면 문제없지만, 코드 가독성과 의도 파악이 어렵다. 주석이나 메서드명 개선이 필요하다.

♻️ 개선 제안
-    // 새 비밀번호 규칙 검증 + 암호화 + 저장 (Dirty Checking)
-    member.changePassword(newPassword, member.getBirthDate());
-
-    // 암호화 후 저장 (Dirty Checking)
+    // 새 비밀번호 규칙 검증
+    member.validateNewPassword(newPassword, member.getBirthDate());
+
+    // 암호화 후 저장 (Dirty Checking 활용)
     String encryptedPassword = passwordEncoder.encode(newPassword);
     member.encryptPassword(encryptedPassword);

MemberModel의 changePassword() 메서드가 검증만 수행한다면 메서드명을 validateNewPassword()로 변경하여 의도를 명확히 해야 한다.

apps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.java (2)

45-47: assertAll 내 assertion들의 가독성이 낮다.

단일 라인에 모든 assertion이 나열되어 있어 테스트 실패 시 원인 파악이 어렵다. 각 assertion을 개별 라인으로 분리하면 가독성과 디버깅 효율이 향상된다.

♻️ 포맷팅 개선 제안
       // assert
-      assertAll(() -> assertThat(result).isNotNull(), () -> assertThat(result.getLoginId()).isEqualTo(memberModel.getLoginId()), () -> assertThat(result.getName()).isEqualTo(memberModel.getName()), () -> assertThat(result.getBirthDate()).isEqualTo(memberModel.getBirthDate()), () -> assertThat(result.getEmail()).isEqualTo(memberModel.getEmail()));
+      assertAll(
+          () -> assertThat(result).isNotNull(),
+          () -> assertThat(result.getLoginId()).isEqualTo(memberModel.getLoginId()),
+          () -> assertThat(result.getName()).isEqualTo(memberModel.getName()),
+          () -> assertThat(result.getBirthDate()).isEqualTo(memberModel.getBirthDate()),
+          () -> assertThat(result.getEmail()).isEqualTo(memberModel.getEmail())
+      );

Also applies to: 80-82


105-131: 비밀번호 변경 후 새 비밀번호로 인증 가능한지 검증하는 테스트가 누락되었다.

현재 테스트는 updatedMember.getPassword()가 이전과 다른지만 확인한다. 운영 환경에서 중요한 것은 변경된 비밀번호로 실제 로그인이 가능한지이다. 비밀번호 변경 후 authenticate(loginId, newPassword)가 성공하는지 검증하는 assertion을 추가해야 한다.

💚 테스트 보완 제안
       // assert
       MemberModel updatedMember = memberService.getMember("testuser");
       // 비밀번호가 변경되었는지 확인 (암호화된 비밀번호 비교)
       assertThat(updatedMember.getPassword()).isNotEqualTo(insertedMember.getPassword());
+
+      // 새 비밀번호로 인증 가능한지 확인
+      MemberModel authenticated = memberService.authenticate(loginId, newPassword);
+      assertThat(authenticated).isNotNull();
     }
apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java (3)

45-108: 경계값 및 유효성 검증 테스트 누락

회원가입 테스트에서 다음 경계값/예외 케이스가 누락되어 있다:

  1. 필수 필드 누락: loginId, password, name 등이 null이거나 빈 문자열인 경우
  2. 잘못된 이메일 형식: "invalid-email" 형태의 입력
  3. 비밀번호 정책 위반: 최소 길이, 특수문자 요구사항 미충족
  4. birthDate 형식 오류: "1990-01-01" 또는 "abc" 등 잘못된 형식

운영 환경에서 잘못된 입력이 들어올 경우 적절한 400 Bad Request 응답을 반환하는지 검증해야 한다.

추가 테스트 케이스 예시
`@DisplayName`("필수 필드가 누락되면, 400 Bad Request를 반환한다.")
`@Test`
void returnsBadRequest_whenRequiredFieldIsMissing() {
    Map<String, String> request = Map.of(
        "loginId", "testuser",
        // password 누락
        "name", "홍길동",
        "birthDate", "19900101",
        "email", "test@example.com"
    );

    ResponseEntity<ApiResponse<Map<String, String>>> response = testRestTemplate.exchange(
        ENDPOINT,
        HttpMethod.POST,
        new HttpEntity<>(request),
        new ParameterizedTypeReference<>() {}
    );

    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
}

`@DisplayName`("잘못된 이메일 형식이면, 400 Bad Request를 반환한다.")
`@Test`
void returnsBadRequest_whenEmailFormatIsInvalid() {
    Map<String, String> request = Map.of(
        "loginId", "testuser",
        "password", "Test1234!",
        "name", "홍길동",
        "birthDate", "19900101",
        "email", "invalid-email"
    );

    ResponseEntity<ApiResponse<Map<String, String>>> response = testRestTemplate.exchange(
        ENDPOINT,
        HttpMethod.POST,
        new HttpEntity<>(request),
        new ParameterizedTypeReference<>() {}
    );

    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
}

110-206: 인증 헤더 누락 시나리오 테스트 필요

인증 헤더가 아예 없는 경우의 테스트가 누락되어 있다. 운영 환경에서 클라이언트 버그로 인해 헤더가 전송되지 않을 수 있으며, 이 경우 적절한 에러 응답을 반환하는지 검증해야 한다.

추가 테스트 케이스 예시
`@DisplayName`("인증 헤더가 없으면, 401 Unauthorized를 반환한다.")
`@Test`
void returnsUnauthorized_whenAuthHeadersAreMissing() {
    // act - 헤더 없이 조회
    ResponseEntity<ApiResponse<Map<String, String>>> response = testRestTemplate.exchange(
        ENDPOINT + "/me",
        HttpMethod.GET,
        null,
        new ParameterizedTypeReference<>() {}
    );

    // assert
    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
}

`@DisplayName`("X-Loopers-LoginId 헤더만 있으면, 401 Unauthorized를 반환한다.")
`@Test`
void returnsUnauthorized_whenOnlyLoginIdHeaderIsProvided() {
    HttpHeaders headers = new HttpHeaders();
    headers.set("X-Loopers-LoginId", "testuser");
    // X-Loopers-LoginPw 누락

    ResponseEntity<ApiResponse<Map<String, String>>> response = testRestTemplate.exchange(
        ENDPOINT + "/me",
        HttpMethod.GET,
        new HttpEntity<>(headers),
        new ParameterizedTypeReference<>() {}
    );

    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
}

116-125: 테스트 데이터 설정 중복

회원가입 요청 데이터(signUpRequest)가 여러 테스트 메서드에서 동일하게 반복된다. 테스트 유지보수성을 위해 공통 헬퍼 메서드 또는 상수로 추출하는 것을 권장한다.

리팩토링 예시
private static final String DEFAULT_LOGIN_ID = "testuser";
private static final String DEFAULT_PASSWORD = "Test1234!";

private Map<String, String> createSignUpRequest() {
    return Map.of(
        "loginId", DEFAULT_LOGIN_ID,
        "password", DEFAULT_PASSWORD,
        "name", "홍길동",
        "birthDate", "19900101",
        "email", "test@test.co.kr"
    );
}

private HttpHeaders createAuthHeaders(String loginId, String password) {
    HttpHeaders headers = new HttpHeaders();
    headers.set("X-Loopers-LoginId", loginId);
    headers.set("X-Loopers-LoginPw", password);
    return headers;
}

private void signUpDefaultMember() {
    ResponseEntity<ApiResponse<Map<String, String>>> response = testRestTemplate.exchange(
        ENDPOINT, HttpMethod.POST, 
        new HttpEntity<>(createSignUpRequest()), 
        new ParameterizedTypeReference<ApiResponse<Map<String, String>>>() {}
    );
    assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED);
}

Comment on lines +123 to +126
public String maskLastChar(String name) {
if (name == null || name.isBlank()) {
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

IllegalArgumentException 대신 CoreException으로 통일해야 한다
현재 예외는 ApiControllerAdvice의 통일 응답을 우회해 500/비표준 응답이 발생할 수 있어 운영 모니터링과 클라이언트 처리에 혼선을 준다.
수정안으로 CoreException(ErrorType.BAD_REQUEST, ...)으로 교체해 오류 포맷을 일관화해야 한다.
추가 테스트로 name이 null/blank일 때 BAD_REQUEST로 매핑되는지 단위 테스트를 추가해야 한다.

🔧 수정 예시
-      throw new IllegalArgumentException("이름은 비어 있을 수 없습니다.");
+      throw new CoreException(ErrorType.BAD_REQUEST, "이름은 비어 있을 수 없습니다.");

Based on learnings: In the loop-pack-be-l2-vol3-java project, enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format이다.

🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`
around lines 123 - 126, Replace the IllegalArgumentException in
MemberModel.maskLastChar with a CoreException so errors flow through
ApiControllerAdvice: change the null/blank check to throw new
CoreException(ErrorType.BAD_REQUEST, "이름은 비어 있을 수 없습니다.") (or equivalent
localized message), ensuring CoreException and ErrorType.BAD_REQUEST are
imported/used; then add a unit test asserting that calling
MemberModel.maskLastChar(null) and maskLastChar("") results in a
BAD_REQUEST-mapped CoreException (or that the controller advice transforms it
into the standardized BAD_REQUEST response) to verify unified error handling
through ApiControllerAdvice.

Comment on lines +5 to +9
public interface MemberRepository {
MemberModel save(MemberModel memberModel);
Optional<MemberModel> findByLoginId(String id);
Optional<MemberModel> update(MemberModel memberModel);

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:

fd -t f 'MemberRepository*.java' apps/

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

Length of output: 164


🏁 Script executed:

rg -n "class MemberRepository" apps/ -A 20

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

Length of output: 2369


🏁 Script executed:

rg -n "update\(" apps/commerce-api -B 2 -A 5 --type java

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

Length of output: 2504


🏁 Script executed:

find apps/ -type f -name "*Test.java" -o -name "*Tests.java" | head -20

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

Length of output: 905


🏁 Script executed:

rg -n "\.update\(" apps/commerce-api/src/main/java -B 3 -A 3 --type java

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

Length of output: 67


🏁 Script executed:

cat -n apps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.java

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

Length of output: 7871


🏁 Script executed:

cat -n apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java

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

Length of output: 16761


🏁 Script executed:

find apps/commerce-api -name "MemberService.java" -type f

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

Length of output: 161


🏁 Script executed:

cat -n apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java

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

Length of output: 3494


🏁 Script executed:

rg -n "repository\.update\(" apps/commerce-api/src/main/java --type java -B 2 -A 2

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

Length of output: 67


MemberRepository 인터페이스의 update 메서드는 비표준 계약으로 제거하거나 구현을 명확히 해야 한다

현재 MemberRepositoryImpl의 update 메서드는 Optional.empty()만 반환하며 실제 갱신 로직이 없다. 다행히 changePassword는 JPA의 Dirty Checking으로 자동 영속화되어 정상 작동하지만, 리포지토리 인터페이스에 정의된 update 메서드는 사용되지 않는 데드코드다.

이는 Repository 계약의 명확성을 해친다. save와 update의 역할을 구분하려 했다면 update는 완전히 구현하거나, 불필요하면 인터페이스에서 제거해야 한다. 현재 상태에서는 새로운 개발자가 update를 사용하려 할 때 의도 없이 데이터 반영 실패를 초래할 수 있다.

수정안:

  1. save로만 통일하고 update 메서드 제거 (추천)
  2. 또는 update를 진정한 merge/update 구현으로 채우기 (보조책)

updateMemberModel를 호출하는 곳이 없음을 확인했으므로, 인터페이스와 구현에서 update 메서드를 정리하는 것이 좋다.

🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberRepository.java`
around lines 5 - 9, The MemberRepository.update method is a dead/non-standard
contract (MemberRepository.update) because MemberRepositoryImpl.update returns
Optional.empty() and no callers use updateMemberModel; remove the misleading API
by deleting the update declaration from MemberRepository and removing its stub
implementation from MemberRepositoryImpl, leaving save(MemberModel) as the
single write operation; alternatively, if you prefer to keep update, implement
MemberRepositoryImpl.update to perform a proper merge/update and return the
persisted MemberModel wrapped in Optional, but do not leave a no-op
Optional.empty() implementation.

Comment on lines +33 to +35
} catch (DataIntegrityViolationException e) {
throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.");
}
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

예외 변환 시 원인(cause)을 보존해야 한다.

DataIntegrityViolationExceptionCoreException으로 변환할 때 원인 예외를 전달하지 않으면 운영 환경에서 장애 원인 파악이 어렵다. 로그에 원본 스택 트레이스가 남지 않아 DB 제약조건 위반의 정확한 원인을 추적할 수 없다.

🛡️ 수정 제안
     } catch (DataIntegrityViolationException e) {
-      throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.");
+      throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.", e);
     }
📝 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
} catch (DataIntegrityViolationException e) {
throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.");
}
} catch (DataIntegrityViolationException e) {
throw new CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.", e);
}
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`
around lines 33 - 35, When converting DataIntegrityViolationException to
CoreException in MemberService, preserve the original exception as the cause:
update the catch block that currently throws new
CoreException(ErrorType.CONFLICT, "이미 존재하는 아이디입니다.") to include the caught
exception (e) as the cause (e.g., use a CoreException constructor or factory
that accepts Throwable) so the original stack trace and message are retained for
logging and debugging.

Comment on lines +38 to +49
public MemberModel authenticate(String loginId, String password) {
// 1. 회원 조회
MemberModel member = getMember(loginId); // 없으면 NOT_FOUND

// 2. 비밀번호 일치 여부 확인
if (!passwordEncoder.matches(password, member.getPassword())) {
// 3. 불일치 시 UNAUTHORIZED 예외
throw new CoreException(ErrorType.UNAUTHORIZED, "인증 실패");
}

return member;
}
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

authenticate() 메서드에 @Transactional(readOnly = true)가 누락되었다.

getMember()를 호출하여 엔티티를 조회하는데, 트랜잭션 경계가 없으면 getMember()의 트랜잭션이 종료된 후 반환된 엔티티가 detached 상태가 된다. MemberModel에 lazy 연관관계가 추가되면 LazyInitializationException이 발생할 수 있다.

🛡️ 수정 제안
+  `@Transactional`(readOnly = true)
   public MemberModel authenticate(String loginId, String password) {
     // 1. 회원 조회
     MemberModel member = getMember(loginId);  // 없으면 NOT_FOUND
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`
around lines 38 - 49, The authenticate method lacks a transactional boundary
which can leave the MemberModel detached after getMember(); add a read-only
transaction to authenticate by annotating the method with
`@Transactional`(readOnly = true) so lazy-loaded associations on MemberModel
remain accessible; update imports to include
org.springframework.transaction.annotation.Transactional and ensure the
authenticate method (and its signature in MemberService) is the one annotated,
leaving getMember() unchanged.

Comment on lines +51 to +56
@Transactional(readOnly = true)
public MemberModel getMember(String loginId) {
MemberModel model = new MemberModel(loginId); // 객체 먼저 생성해야 함
return memberRepository.findByLoginId(model.getLoginId())
.orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "[id = " + loginId + "] 회원을 찾을 수 없습니다."));
}
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

getMember()에서 불필요한 MemberModel 객체 생성이 있다.

Line 53에서 new MemberModel(loginId)로 객체를 생성한 후 model.getLoginId()를 호출하는데, 파라미터 loginId를 직접 사용하면 된다. 불필요한 객체 생성은 GC 부담을 증가시키고, MemberModel 생성자에 검증 로직이 있다면 예상치 못한 예외가 발생할 수 있다.

♻️ 수정 제안
   `@Transactional`(readOnly = true)
   public MemberModel getMember(String loginId) {
-    MemberModel model = new MemberModel(loginId);  // 객체 먼저 생성해야 함
-    return memberRepository.findByLoginId(model.getLoginId())
+    return memberRepository.findByLoginId(loginId)
         .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "[id = " + loginId + "] 회원을 찾을 수 없습니다."));
   }
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`
around lines 51 - 56, The getMember method unnecessarily instantiates a
MemberModel just to read its loginId; remove the new MemberModel(loginId) and
call memberRepository.findByLoginId(loginId) directly, keeping the existing
orElseThrow(...) that throws new CoreException(ErrorType.NOT_FOUND, "[id = " +
loginId + "] 회원을 찾을 수 없습니다."); this avoids needless object creation and any side
effects from MemberModel's constructor while preserving behavior in getMember.

Comment on lines +26 to +28
@RequestHeader("X-Loopers-LoginId") String loginId,
@RequestHeader("X-Loopers-LoginPw") String password
);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# 대상 파일 확인
cat -n apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1ApiSpec.java | head -50

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

Length of output: 1601


🏁 Script executed:

# 로깅 관련 설정 파일 검색
find . -type f \( -name "*.yml" -o -name "*.yaml" -o -name "*.properties" \) | xargs grep -l -i "logging\|log" 2>/dev/null | head -20

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

Length of output: 333


🏁 Script executed:

# 토큰 기반 인증 관련 클래스 검색
fd -e java . | xargs grep -l "Token\|JWT\|Bearer" | head -10

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

Length of output: 67


🏁 Script executed:

# 민감 헤더 필터링 관련 설정 검색
find . -type f \( -name "*.java" -o -name "*.yml" -o -name "*.yaml" \) | xargs grep -i "X-Loopers\|LoggingFilter\|MDCFilter\|SensitiveHeader" 2>/dev/null | head -20

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

Length of output: 3011


🏁 Script executed:

# logging.yml 내용 확인
cat -n ./supports/logging/src/main/resources/logging.yml

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

Length of output: 137


🏁 Script executed:

# application.yml에서 logging 설정 확인
cat -n ./apps/commerce-api/src/main/resources/application.yml | grep -A 20 -B 2 "logging"

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

Length of output: 522


🏁 Script executed:

# 필터/미들웨어 관련 클래스 전체 검색
fd -e java . | xargs grep -l "Filter\|Interceptor" | grep -E "(Logging|Sensitive|Header|Security)" | head -10

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

Length of output: 67


🏁 Script executed:

# 모든 필터/인터셉터 클래스 확인
fd -e java . --path "*filter*" --path "*interceptor*" | head -20

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

Length of output: 318


🏁 Script executed:

# supports/logging 디렉토리 구조 확인
find ./supports/logging -type f -name "*.java" | head -20

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

Length of output: 67


🏁 Script executed:

# logback.xml 파일 위치 확인
find . -name "logback.xml" -o -name "logback-*.xml"

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

Length of output: 142


🏁 Script executed:

# logback.xml 내용 확인
find . -name "logback.xml" | head -1 | xargs cat

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

Length of output: 1904


🏁 Script executed:

# supports/logging 디렉토리의 Java 파일 확인
find ./supports/logging -type f -name "*.java"

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

Length of output: 67


🏁 Script executed:

# 비밀번호/민감 정보 관련 필터링 검색
rg -i "password|secret|sensitive|masking|filter|header" --type java ./supports/logging

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

Length of output: 67


🏁 Script executed:

# HTTP 요청/응답 로깅 관련 클래스 검색
rg -i "HttpServletRequest|RequestHeader|ResponseHeader|Filter|Interceptor" --type java | head -20

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

Length of output: 1860


🏁 Script executed:

# logback appender 파일들 확인
find ./supports/logging/src/main/resources/appenders -type f -name "*.xml"

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

Length of output: 300


🏁 Script executed:

# 각 appender 파일 내용 확인
cat ./supports/logging/src/main/resources/appenders/plain-console-appender.xml

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

Length of output: 317


🏁 Script executed:

# json-console-appender 확인
cat ./supports/logging/src/main/resources/appenders/json-console-appender.xml

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

Length of output: 398


🏁 Script executed:

# MemberV1Controller 구현 확인 - 비밀번호 처리 방식
cat -n ./apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java | head -80

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

Length of output: 3286


헤더 기반 비밀번호 전송은 보안 critical 문제이므로 즉시 개선이 필요하다.

X-Loopers-LoginPw 헤더로 비밀번호를 전송할 경우, 현재 로깅 설정에서 민감 헤더 필터링이 구현되지 않아 서버 액세스 로그, 프록시, CDN에 평문 비밀번호가 기록된다. logback.xml과 appender 설정에 헤더 마스킹 규칙이 없으므로 즉시 노출 위험이 있다.

다음을 필수로 수행해야 한다:

  • 토큰 기반 인증(JWT 등)으로 전환하여 헤더에 비밀번호를 전송하지 않도록 변경한다.
  • 토큰 전환 전까지는 logback 필터를 통해 X-Loopers-LoginPw 헤더를 반드시 마스킹해야 한다.
  • 로컬, 개발, 운영 모든 환경에서 민감 헤더가 로그에 노출되지 않는지 검증한다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1ApiSpec.java`
around lines 26 - 28, The current API spec (MemberV1ApiSpec) accepts a plaintext
password via the X-Loopers-LoginPw RequestHeader (method parameter "password");
stop accepting passwords in headers and migrate this endpoint to token-based
authentication (e.g., issue/require JWTs from an authentication flow and remove
the X-Loopers-LoginPw header parameter from MemberV1ApiSpec and its
implementations). Until the token migration is complete, ensure the sensitive
header is never logged by adding a logback filter/masking rule for
"X-Loopers-LoginPw" at the appender level (apply to logback.xml used in
local/dev/prod), verify masking across environments, and add a short deprecation
note and TODO in the login controller/handler referencing removal of
X-Loopers-LoginPw.

Comment on lines +52 to +60
@PatchMapping("/me/password")
public ApiResponse<String> changePassword(
@RequestHeader("X-Loopers-LoginId") String loginId,
@RequestHeader("X-Loopers-LoginPw") String password,
@RequestBody ChangePasswordRequest request
) {
memberFacade.changePassword(loginId, password, request.oldPassword(), request.newPassword());
return ApiResponse.success("비밀번호가 변경되었습니다.");
}
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

changePassword 메서드에 @Override 어노테이션이 누락되었다.

MemberV1ApiSpec 인터페이스를 구현하는 다른 메서드들(signUp, getMyInfo)은 @Override가 있으나 changePassword에는 없다. 일관성을 위해 추가해야 하며, 인터페이스 시그니처 변경 시 컴파일 타임에 오류를 감지할 수 있다.

♻️ 수정 제안
   `@PatchMapping`("/me/password")
+  `@Override`
     public ApiResponse<String> changePassword(
         `@RequestHeader`("X-Loopers-LoginId") String loginId,
         `@RequestHeader`("X-Loopers-LoginPw") String password,
         `@RequestBody` ChangePasswordRequest request
     ) {
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java`
around lines 52 - 60, The changePassword method in MemberV1Controller is missing
the `@Override` annotation; add `@Override` immediately above the changePassword
method declaration to match other implemented methods (signUp, getMyInfo) and
ensure the method signature exactly matches the one declared in MemberV1ApiSpec
(changePassword(...)) so compilation will catch future interface changes.

Comment on lines +84 to +107
@DisplayName("중복된 loginId로 가입하면, 409 Conflict를 반환한다.")
@Test
void returnsConflict_whenDuplicateLoginIdIsProvided() {
// arrange - 먼저 한 명 가입
Map<String, String> request = Map.of(
"loginId", "testuser",
"password", "Test1234!",
"name", "홍길동",
"birthDate", "19900101",
"email", "test@example.com"
);
testRestTemplate.exchange(ENDPOINT, HttpMethod.POST, new HttpEntity<>(request), new ParameterizedTypeReference<ApiResponse<Map<String, String>>>() {});

// act - 같은 ID로 다시 가입
ResponseEntity<ApiResponse<Map<String, String>>> response = testRestTemplate.exchange(
ENDPOINT,
HttpMethod.POST,
new HttpEntity<>(request),
new ParameterizedTypeReference<>() {}
);

// assert
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT);
}
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

첫 번째 회원가입 결과 검증 누락

Line 95에서 첫 번째 회원가입 요청의 결과를 검증하지 않는다. 만약 첫 번째 가입이 실패하면 중복 테스트가 잘못된 이유로 통과할 수 있어 테스트 신뢰성이 저하된다.

수정 제안
-            testRestTemplate.exchange(ENDPOINT, HttpMethod.POST, new HttpEntity<>(request), new ParameterizedTypeReference<ApiResponse<Map<String, String>>>() {});
+            ResponseEntity<ApiResponse<Map<String, String>>> firstResponse = testRestTemplate.exchange(
+                ENDPOINT, HttpMethod.POST, new HttpEntity<>(request), 
+                new ParameterizedTypeReference<ApiResponse<Map<String, String>>>() {});
+            assertThat(firstResponse.getStatusCode()).isEqualTo(HttpStatus.CREATED);
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java`
around lines 84 - 107, The test returnsConflict_whenDuplicateLoginIdIsProvided
does the first signup call via testRestTemplate.exchange but never checks its
result; capture that first ResponseEntity (the initial testRestTemplate.exchange
call) and assert its status is HttpStatus.CREATED (and optionally assert body is
non-null) before performing the duplicate signup, so the test reliably verifies
the precondition; update the method in MemberV1ApiE2ETest to store the first
response and add an assertion on its status (referencing ENDPOINT and
testRestTemplate.exchange).

Comment on lines +212 to +251
@DisplayName("헤더 인증 성공 + 유효한 비밀번호 변경 요청이면, 200 OK를 반환한다.")
@Test
void returnsOk_whenHeaderAuthAndPasswordChangeAreValid() {
// arrange - 먼저 회원 가입
Map<String, String> signUpRequest = Map.of(
"loginId", "testuser",
"password", "Test1234!",
"name", "홍길동",
"birthDate", "19900101",
"email", "test@test.co.kr"
);
testRestTemplate.exchange(ENDPOINT, HttpMethod.POST, new HttpEntity<>(signUpRequest), new ParameterizedTypeReference<ApiResponse<Map<String, String>>>() {});

// act - 헤더 인증 + 비밀번호 변경 요청
HttpHeaders headers = new HttpHeaders();
headers.set("X-Loopers-LoginId", "testuser");
headers.set("X-Loopers-LoginPw", "Test1234!");
headers.set("Content-Type", "application/json");

Map<String, String> changePasswordRequest = Map.of(
"oldPassword", "Test1234!",
"newPassword", "NewPass123!"
);
ResponseEntity<ApiResponse<String>> response = testRestTemplate.exchange(
ENDPOINT + "/me/password",
HttpMethod.PATCH,
new HttpEntity<>(changePasswordRequest, headers),
new ParameterizedTypeReference<>() {}
);

// assert
assertAll(
() -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.OK),
() -> assertThat(response.getBody()).isNotNull(),
() -> {
Assertions.assertNotNull(response.getBody());
assertThat(response.getBody().data()).isEqualTo("비밀번호가 변경되었습니다.");
}
);
}
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

비밀번호 변경 후 실제 변경 검증 누락

비밀번호 변경 API가 200 OK를 반환하는 것만 확인하고, 실제로 비밀번호가 변경되었는지 검증하지 않는다. 운영 환경에서 API는 성공 응답을 반환하지만 DB 반영이 실패하는 경우가 발생할 수 있다.

다음 검증을 추가해야 한다:

  1. 새 비밀번호로 인증이 성공하는지 확인
  2. 기존 비밀번호로 인증이 실패하는지 확인
추가 검증 코드 예시
            // 새 비밀번호로 인증 성공 확인
            HttpHeaders newPasswordHeaders = new HttpHeaders();
            newPasswordHeaders.set("X-Loopers-LoginId", "testuser");
            newPasswordHeaders.set("X-Loopers-LoginPw", "NewPass123!");
            
            ResponseEntity<ApiResponse<Map<String, String>>> verifyResponse = testRestTemplate.exchange(
                ENDPOINT + "/me",
                HttpMethod.GET,
                new HttpEntity<>(newPasswordHeaders),
                new ParameterizedTypeReference<>() {}
            );
            assertThat(verifyResponse.getStatusCode()).isEqualTo(HttpStatus.OK);
            
            // 기존 비밀번호로 인증 실패 확인
            HttpHeaders oldPasswordHeaders = new HttpHeaders();
            oldPasswordHeaders.set("X-Loopers-LoginId", "testuser");
            oldPasswordHeaders.set("X-Loopers-LoginPw", "Test1234!");
            
            ResponseEntity<ApiResponse<Map<String, String>>> oldPwResponse = testRestTemplate.exchange(
                ENDPOINT + "/me",
                HttpMethod.GET,
                new HttpEntity<>(oldPasswordHeaders),
                new ParameterizedTypeReference<>() {}
            );
            assertThat(oldPwResponse.getStatusCode()).isEqualTo(HttpStatus.UNAUTHORIZED);
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/MemberV1ApiE2ETest.java`
around lines 212 - 251, The test
returnsOk_whenHeaderAuthAndPasswordChangeAreValid only asserts HTTP 200 and
message; add real verification that the password changed by performing two
additional authenticated requests using testRestTemplate: (1) GET ENDPOINT +
"/me" with headers X-Loopers-LoginId="testuser" and
X-Loopers-LoginPw="NewPass123!" and assert a HttpStatus.OK response (and
non-null body), and (2) GET ENDPOINT + "/me" with X-Loopers-LoginPw="Test1234!"
and assert HttpStatus.UNAUTHORIZED; use the same
ParameterizedTypeReference<ApiResponse<Map<String,String>>>/ResponseEntity types
as the existing calls and keep assertions consistent with the existing style in
the test method returnsOk_whenHeaderAuthAndPasswordChangeAreValid.

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/member/MemberModel.java`:
- Around line 100-115: The validatePassword method can NPE when password is
null/blank; add an explicit null/blank check at the start of validatePassword
(the method in MemberModel used by changePassword) and throw new
CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다." or equivalent) so all
validation errors flow through CoreException/ApiControllerAdvice; update callers
(e.g., changePassword) tests to send null and blank newPassword and assert
BAD_REQUEST + the standard error response format in unit/integration tests to
verify behavior.

In
`@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`:
- Line 55: Remove the sensitive loginId from the exception message returned to
the client: add a logger to MemberService (e.g., annotate the class with `@Slf4j`)
and before throwing the CoreException from the lookup that currently uses
loginId, log the detailed context (including loginId) via log.warn or log.error,
then throw new CoreException(ErrorType.NOT_FOUND, "회원을 찾을 수 없습니다.") with the
generic message so clients do not see the id; keep references to MemberService,
the loginId variable, CoreException, and ErrorType.NOT_FOUND when making the
change.

In
`@apps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.java`:
- Around line 105-131: The test's assertion is weak because it compares
updatedMember.getPassword() to insertedMember.getPassword() which may be the
same value if change failed; update the test to capture the original stored
password before calling memberService.changePassword (e.g., call
memberService.getMember(loginId) after saveMember and store its getPassword()),
then call memberService.changePassword(insertedMember, newPassword) and assert
that the post-change password (memberService.getMember(loginId).getPassword())
differs from the original stored password and also that authenticating with
newPassword succeeds and authenticating with prevPassword fails (use
MemberService.changePassword, MemberService.getMember, MemberService.saveMember
and any authentication method you have) to ensure a true password change.
🧹 Nitpick comments (1)
apps/commerce-api/src/main/java/com/loopers/application/member/MemberInfo.java (1)

7-14: from() 메서드에서 null 체크가 누락되었다

model 파라미터가 null일 경우 NPE가 발생한다. Facade 계층에서 null을 전달할 가능성은 낮지만, 방어적 코딩 관점에서 명시적 검증을 추가하는 것이 운영 환경에서의 디버깅에 유리하다.

♻️ 수정 제안
 public static MemberInfo from(MemberModel model) {
+    if (model == null) {
+        throw new IllegalArgumentException("MemberModel cannot be null");
+    }
     return new MemberInfo(
         model.getLoginId(),
         model.getMaskedName(),
         model.getBirthDate(),
         model.getEmail()
       );
 }

Comment on lines +100 to +115
private void validatePassword(String password, String birthDate) {
// 1. 8~16자 길이 체크
if (password.length() < 8 || password.length() > 16) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8~16자여야 합니다.");
}

// 2. 영문 대소문자, 숫자, 특수문자만 허용 (한글, 공백 등 불가)
if (!password.matches("^[a-zA-Z0-9!@#$%^&*()_+\\-=\\[\\]{};':\"\\\\|,.<>/?]+$")) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 영문 대소문자, 숫자, 특수문자만 가능합니다.");
}

// 3. 생년월일이 비밀번호에 포함되면 안됨
if (password.contains(birthDate)) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호에 생년월일을 포함할 수 없습니다.");
}
}
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

null/blank 새 비밀번호가 NPE로 500을 유발할 수 있다.
changePassword 경로에서 newPassword가 null/blank이면 length/regex 접근 시 NPE가 발생해 표준 에러 포맷이 깨진다.
수정안: validatePassword 시작에 null/blank 체크를 추가하고 CoreException(ErrorType.BAD_REQUEST)으로 통일해야 한다.
추가 테스트: null/blank newPassword 요청이 BAD_REQUEST와 표준 응답 포맷으로 내려오는지 단위/통합 테스트로 검증해야 한다.

🛠️ 수정 예시
 private void validatePassword(String password, String birthDate) {
+  if (password == null || password.isBlank()) {
+    throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다.");
+  }
   // 1. 8~16자 길이 체크
   if (password.length() < 8 || password.length() > 16) {
     throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8~16자여야 합니다.");
   }
Based on learnings: In the loop-pack-be-l2-vol3-java project, enforce unified error handling by routing errors through CoreException to ApiControllerAdvice to ensure a consistent response format. Do not introduce MethodArgumentNotValidException handlers or Bean Validation handling, as that would create inconsistent error handling patterns라는 지침을 따르는 판단이다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`
around lines 100 - 115, The validatePassword method can NPE when password is
null/blank; add an explicit null/blank check at the start of validatePassword
(the method in MemberModel used by changePassword) and throw new
CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다." or equivalent) so all
validation errors flow through CoreException/ApiControllerAdvice; update callers
(e.g., changePassword) tests to send null and blank newPassword and assert
BAD_REQUEST + the standard error response format in unit/integration tests to
verify behavior.

public MemberModel getMember(String loginId) {
MemberModel model = new MemberModel(loginId); // 객체 먼저 생성해야 함
return memberRepository.findByLoginId(model.getLoginId())
.orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "[id = " + loginId + "] 회원을 찾을 수 없습니다."));
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

에러 메시지에 loginId 노출은 운영 로그에서 문제가 될 수 있다

"[id = " + loginId + "] 회원을 찾을 수 없습니다." 메시지가 클라이언트에게 그대로 전달될 경우, 공격자가 유효한 loginId를 열거(enumeration)할 수 있다. 사용자 메시지와 로그 메시지를 분리하여, 클라이언트에는 일반적인 메시지를, 로그에는 상세 정보를 남기는 것이 보안상 유리하다.

🛡️ 수정 제안
   `@Transactional`(readOnly = true)
   public MemberModel getMember(String loginId) {
-    MemberModel model = new MemberModel(loginId);  // 객체 먼저 생성해야 함
-    return memberRepository.findByLoginId(model.getLoginId())
-        .orElseThrow(() -> new CoreException(ErrorType.NOT_FOUND, "[id = " + loginId + "] 회원을 찾을 수 없습니다."));
+    return memberRepository.findByLoginId(loginId)
+        .orElseThrow(() -> {
+            // 로그에는 상세 정보, 클라이언트에는 일반 메시지
+            log.warn("회원 조회 실패: loginId={}", loginId);
+            return new CoreException(ErrorType.NOT_FOUND, "회원을 찾을 수 없습니다.");
+        });
   }

Logger 필드 추가:

`@Slf4j`
`@RequiredArgsConstructor`
`@Component`
public class MemberService {
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberService.java`
at line 55, Remove the sensitive loginId from the exception message returned to
the client: add a logger to MemberService (e.g., annotate the class with `@Slf4j`)
and before throwing the CoreException from the lookup that currently uses
loginId, log the detailed context (including loginId) via log.warn or log.error,
then throw new CoreException(ErrorType.NOT_FOUND, "회원을 찾을 수 없습니다.") with the
generic message so clients do not see the id; keep references to MemberService,
the loginId variable, CoreException, and ErrorType.NOT_FOUND when making the
change.

Comment on lines +105 to +131
@DisplayName("기존 비밀번호와 새 비밀번호가 유효하면, 비밀번호가 변경된다.")
@Test
void changesPassword_whenOldAndNewPasswordsAreValid() {
// arrange
String loginId = "testuser";
String prevPassword = "Test1234!";
String name = "홍길동";
String birthDate = "19900101";
String email = "test@test.co.kr";

// act
// 기존 등록된 디비 설정
MemberModel member = new MemberModel(loginId, prevPassword, name, birthDate, email);
memberService.saveMember(member);

// 클라에서 입력한 아이디와 기존 비밀번호, 새로운 비밀번호
MemberModel insertedMember = new MemberModel(loginId, prevPassword);
String newPassword = "NewPass123!";

// act
memberService.changePassword(insertedMember, newPassword);

// assert
MemberModel updatedMember = memberService.getMember("testuser");
// 비밀번호가 변경되었는지 확인 (암호화된 비밀번호 비교)
assertThat(updatedMember.getPassword()).isNotEqualTo(insertedMember.getPassword());
}
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

비밀번호 변경 성공 검증이 약해 거짓 양성이 가능함
현재 검증은 변경 전/후의 저장 비밀번호를 비교하지 않아, 변경이 실패해도 테스트가 통과할 수 있다. 수정안으로 변경 전 저장된 비밀번호를 조회해 두고 변경 후 값과 비교하거나, 새 비밀번호 인증 성공/기존 비밀번호 인증 실패를 추가해야 한다. 추가 테스트로 변경 전/후 저장 비밀번호 비교 또는 인증 시나리오를 포함해야 한다.

🛠️ 수정 예시
       // act
       memberService.changePassword(insertedMember, newPassword);

       // assert
-      MemberModel updatedMember = memberService.getMember("testuser");
-      // 비밀번호가 변경되었는지 확인 (암호화된 비밀번호 비교)
-      assertThat(updatedMember.getPassword()).isNotEqualTo(insertedMember.getPassword());
+      MemberModel beforeMember = memberService.getMember("testuser");
+      memberService.changePassword(insertedMember, newPassword);
+      MemberModel updatedMember = memberService.getMember("testuser");
+      assertThat(updatedMember.getPassword()).isNotEqualTo(beforeMember.getPassword());
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.java`
around lines 105 - 131, The test's assertion is weak because it compares
updatedMember.getPassword() to insertedMember.getPassword() which may be the
same value if change failed; update the test to capture the original stored
password before calling memberService.changePassword (e.g., call
memberService.getMember(loginId) after saveMember and store its getPassword()),
then call memberService.changePassword(insertedMember, newPassword) and assert
that the post-change password (memberService.getMember(loginId).getPassword())
differs from the original stored password and also that authenticating with
newPassword succeeds and authenticating with prevPassword fails (use
MemberService.changePassword, MemberService.getMember, MemberService.saveMember
and any authentication method you have) to ensure a true password change.

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

🤖 Fix all issues with AI agents
In @.claude/settings.local.json:
- Around line 3-6: Replace the non-standard Gradle patterns "Bash(./gradlew
test:*)" and "Bash(./gradlew :apps:commerce-api:test:*)" in the "allow" array
with the canonical task invocations "Bash(./gradlew test)" and "Bash(./gradlew
:apps:commerce-api:test)"; update the entries in .claude/settings.local.json
where those strings appear (look for the allow array) and then verify the
authorization system recognizes and permits those exact commands by testing
execution.

In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`:
- Around line 57-66: The single-arg MemberModel(String loginId) constructor
allows creation of entities missing required fields (name, birthDate, email) and
thus breaks domain invariants; remove this constructor or convert it into a
separate read-only VO/factory used only for queries, and instead provide a
constructor/factory for entity creation that requires and assigns all mandatory
fields (e.g., include name, birthDate, email alongside loginId and reuse
validateLoginId(loginId) there). Update MemberModel so required fields are
non-nullable and set at construction, and add a unit/integration test proving
instances from the old single-arg path cannot be persisted (or that the
constructor is not present for persistence).

In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1ApiSpec.java`:
- Around line 34-38: changePassword API currently requires both header
X-Loopers-LoginPw and request.body.oldPassword which duplicates credentials;
choose one approach and remove the other to avoid ambiguity. Either remove the
password header from MemberV1ApiSpec.changePassword (keep
MemberV1Dto.ChangePasswordRequest.oldPassword as the single source of truth) and
update callers/auth filters accordingly, or remove oldPassword from
MemberV1Dto.ChangePasswordRequest and rely solely on X-Loopers-LoginPw in the
changePassword signature and docs; ensure related validation and tests are
updated to reflect the chosen strategy.

In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Dto.java`:
- Around line 42-45: The ChangePasswordRequest record's autogenerated toString()
can expose sensitive fields oldPassword and newPassword; override the record's
toString() in ChangePasswordRequest to return a masked representation (e.g.,
replace characters with '*' or show only last 2 chars) for both oldPassword and
newPassword instead of printing raw values, ensuring any logging or
serialization uses the masked string; implement the toString() method inside the
ChangePasswordRequest record and reference the oldPassword and newPassword
fields when building the masked output.
- Around line 9-15: SignUpRequest (the record in MemberV1Dto) exposes password
via the auto-generated toString, so override SignUpRequest::toString to mask the
password (e.g., replace with fixed mask or show only length) and return the
other fields (loginId, name, birthDate, email) as before; alternatively ensure
callers never log the record directly—update any logging sites to avoid printing
the object—to prevent plaintext passwords from appearing in logs.

In
`@apps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.java`:
- Around line 20-38: The test currently assumes plain-text password storage;
update the test in MemberModelTest so it no longer asserts member.getPassword()
equals rawPassword; instead assert that member.getPassword() is not equal to
rawPassword and that member.matches(rawPassword) (or the equivalent password
verification method on MemberModel) returns true; reference the MemberModel
constructor and the changePassword() / encryptPassword() behavior so the test
will continue to pass when the domain starts storing encrypted passwords.

In `@build.gradle.kts`:
- Around line 63-65: Add a new property named spring-security.version (set to
6.4.4) in gradle.properties and update the spring-security-crypto declaration in
build.gradle.kts so it references that property instead of relying solely on the
BOM (i.e., change the
implementation("org.springframework.security:spring-security-crypto") entry to
use the property-based version); also add/enable a dependency security scan step
(DependencyCheck or Snyk) in the CI pipeline to monitor for vulnerabilities.
🧹 Nitpick comments (7)
docker/infra-compose.yml (2)

1-1: version 필드는 Docker Compose V2에서 더 이상 필요하지 않다.

Docker Compose V2 이상에서는 version 필드가 deprecated 되었다. 현재는 동작에 문제가 없으나, 향후 경고 메시지가 발생할 수 있다.

♻️ 제안: version 필드 제거
-version: '3'
 services:
   mysql:

3-15: MySQL 서비스에 healthcheck 설정이 누락되어 있다.

운영 관점에서 문제가 될 수 있다. depends_on을 사용하는 다른 서비스가 MySQL에 의존할 경우, 컨테이너가 시작되었더라도 MySQL이 실제로 연결을 수락할 준비가 되지 않은 상태에서 연결 시도가 발생할 수 있다. redis-master 서비스에는 healthcheck가 정의되어 있으나 MySQL에는 누락되어 있다.

🛡️ 제안: MySQL healthcheck 추가
   mysql:
     image: mysql:8.0
     ports:
       - "3307:3306"
     environment:
       - MYSQL_ROOT_PASSWORD=root
       - MYSQL_USER=application
       - MYSQL_PASSWORD=application
       - MYSQL_DATABASE=loopers
       - MYSQL_CHARACTER_SET=utf8mb4
       - MYSQL_COLLATE=utf8mb4_general_ci
     volumes:
       - mysql-8-data:/var/lib/mysql
+    healthcheck:
+      test: ["CMD", "mysqladmin", "ping", "-h", "localhost", "-u", "root", "-proot"]
+      interval: 10s
+      timeout: 5s
+      retries: 10
apps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.java (1)

137-167: 마스킹 로직의 실패 케이스 테스트가 부족하다
운영 관점에서 null/blank 입력 시 에러 포맷이 깨지면 API 일관성이 흔들릴 수 있다.
수정안으로 null/blank 이름에 대해 BAD_REQUEST가 발생하는지 테스트를 추가해야 한다.
추가 테스트로 공백 문자열과 null 모두에 대해 예외 흐름을 검증해야 한다.

As per coding guidelines: **/*Test*.java: 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.

apps/commerce-api/src/main/java/com/loopers/config/SecurityConfig.java (1)

11-14: BCrypt strength를 애플리케이션 설정으로 외부화하고 성능 특성을 명시적으로 관리해야 한다

BCryptPasswordEncoder의 기본 strength(10)는 Spring Security 전 버전에서 동일하게 유지되므로 버전 변동으로 인한 문제는 발생하지 않는다. 다만 운영 관점에서 strength 값을 애플리케이션 설정으로 외부화하면, 운영 환경의 하드웨어 특성에 맞춰 성능 튜닝이 가능하다. Spring은 password 검증이 ~1초가 되도록 strength를 벤치마킹할 것을 권장한다. 현재 strength 10에서 문제가 없다면 유지해도 되지만, 향후 성능 최적화 시 application.yml에서 security.bcrypt.strength 같은 설정값을 정의하고 new BCryptPasswordEncoder(strength) 형태로 주입하는 것이 좋은 실천이다. 변경 시 해당 strength 값으로 해시가 생성되고 matches가 정상 동작하는지 테스트하면 된다.

apps/commerce-api/src/main/java/com/loopers/infrastructure/member/MemberJpaRepository.java (1)

10-10: save 메서드 선언은 불필요하다.

JpaRepository<MemberModel, Long>을 상속하면 save(S entity) 메서드가 이미 제공된다. 중복 선언은 코드 혼란을 유발하며, 반환 타입 불일치 시 의도치 않은 동작이 발생할 수 있다.

♻️ 수정 제안
 public interface MemberJpaRepository extends JpaRepository<MemberModel, Long> {

   Optional<MemberModel> findByLoginId(String loginId);
-  MemberModel save(MemberModel memberModel);
 }
apps/commerce-api/src/test/java/com/loopers/domain/member/MemberServiceIntegrationTest.java (1)

46-46: assertAll 내 assertion을 개행하여 가독성을 개선할 수 있다.

한 줄에 모든 assertion이 있어 테스트 실패 시 어떤 조건이 실패했는지 파악하기 어렵다.

♻️ 수정 제안
       // assert
-      assertAll(() -> assertThat(result).isNotNull(), () -> assertThat(result.getLoginId()).isEqualTo(memberModel.getLoginId()), () -> assertThat(result.getName()).isEqualTo(memberModel.getName()), () -> assertThat(result.getBirthDate()).isEqualTo(memberModel.getBirthDate()), () -> assertThat(result.getEmail()).isEqualTo(memberModel.getEmail()));
+      assertAll(
+          () -> assertThat(result).isNotNull(),
+          () -> assertThat(result.getLoginId()).isEqualTo(memberModel.getLoginId()),
+          () -> assertThat(result.getName()).isEqualTo(memberModel.getName()),
+          () -> assertThat(result.getBirthDate()).isEqualTo(memberModel.getBirthDate()),
+          () -> assertThat(result.getEmail()).isEqualTo(memberModel.getEmail())
+      );
apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Controller.java (1)

32-60: 메서드 간 들여쓰기가 일관되지 않다.

signUp, getMyInfo는 4칸 들여쓰기를 사용하고, changePassword는 2칸 들여쓰기를 사용한다. 코드 일관성을 위해 동일한 들여쓰기 규칙을 적용해야 한다.

Comment on lines +3 to +6
"allow": [
"Bash(./gradlew test:*)",
"Bash(./gradlew :apps:commerce-api:test:*)"
]
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 . -type f -name "*.gradle*" -o -name "settings.gradle*" | head -20

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

Length of output: 451


🏁 Script executed:

cat -n ".claude/settings.local.json" 2>/dev/null || echo "File not found"

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

Length of output: 270


🏁 Script executed:

# 저장소 루트 구조 확인
ls -la | grep -E "^\." && echo "---" && ls -la | head -20

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

Length of output: 67


🏁 Script executed:

cat -n settings.gradle.kts

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

Length of output: 1230


🏁 Script executed:

head -50 build.gradle.kts

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

Length of output: 1361


🏁 Script executed:

head -50 ./apps/commerce-api/build.gradle.kts

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

Length of output: 1003


🏁 Script executed:

# Gradle에서 wildcard 패턴이 실제로 동작하는지 확인
# 프로젝트 구조와 가능한 태스크 확인
rg "test" --type gradle -A 2 | head -40

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

Length of output: 233


🌐 Web query:

Gradle wildcard task pattern syntax colon test:*

💡 Result:

In Gradle task paths, : is a separator, not a wildcard operator. So test:* (or :test:*) is not valid Gradle task-selection syntax—Gradle doesn’t support * wildcards for choosing tasks by name/path on the command line. Instead, Gradle supports task selectors (e.g., test runs the test task across subprojects) and name abbreviation (unique prefixes / camel-case abbreviations). [1]

What to use instead:

  • Run test for all subprojects (from the root):

    • ./gradlew test (task selector across the multi-project build) [1]
  • Run test in one subproject:

    • ./gradlew :subproject:test (or subproject:test) [1]
  • If you meant “wildcard” for tests inside the test task (not tasks):

    • ./gradlew test --tests '*SomePattern*' (test filtering supports *) [2]

Sources:
[1] Gradle User Manual – Command-Line Interface (multi-project task paths, task selectors, name abbreviation) (docs.gradle.org)
[2] Gradle User Manual – Testing in Java & JVM projects (test filtering, --tests, wildcard *) (docs.gradle.org)

Citations:


🌐 Web query:

Gradle task selector patterns documentation

💡 Result:

Gradle documents “task selector patterns” as task paths/selectors plus name abbreviation (pattern expansion) in the Gradle User Manual:

  • Task selectors / task paths (multi-project):

    • Run a task in a specific project with a path like :subproject:taskName (or subproject:taskName).
    • Run a task across subprojects by specifying only the task name from the root (e.g., gradle test runs test in all subprojects that have it).
      Source: Gradle Command-Line Interface → “Executing tasks in multi-project builds”. [1]
  • Name abbreviation / “patterns” (what most people mean by selector patterns):

    • You can type a unique prefix (e.g., gradle checheck).
    • You can use camel-case patterns that also match kebab-case (e.g., foBa or fB matches fooBar and foo-bar).
    • Also works for project names (e.g., gradle mAL:cT).
      Source: Gradle Command-Line Interface → “Name abbreviation”. [1]

If you want the shorter “basics” page for task paths (but it doesn’t cover the abbreviation patterns in as much detail), see: Command-Line Interface Basics → “Executing tasks”. [2]

Sources
[1] Gradle User Manual — Command-Line Interface: https://docs.gradle.org/current/userguide/command_line_interface.html
[2] Gradle User Manual — Command-Line Interface Basics: https://docs.gradle.org/current/userguide/command_line_interface_basics.html


🏁 Script executed:

# Gradle 공식 문서의 Task Selector 패턴 확인
# 실제 프로젝트에서 test로 시작하는 태스크들 확인
rg "test" -t kts build.gradle.kts | grep -E "(task|Test)" | head -20

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

Length of output: 112


Gradle 태스크 패턴 문법을 표준 형식으로 수정한다.

test:* 형식은 Gradle 공식 명령어가 아니다. Gradle은 태스크 경로에서 : 를 구분자로 사용하지만 * 와일드카드를 지원하지 않는다. 현재 패턴 ./gradlew test:*./gradlew :apps:commerce-api:test:* 는 표준 Gradle 문법이 아니므로 의도한 권한이 제대로 부여되지 않을 위험이 있다.

운영 관점에서 다음과 같이 수정한다:

  • ./gradlew test (모든 모듈의 테스트 태스크)
  • ./gradlew :apps:commerce-api:test (commerce-api 모듈만 테스트)

이렇게 수정하면 Claude가 정확한 Gradle 태스크를 실행할 수 있으며, 저장소 구조와의 명확한 대응이 보장된다. 추가로 권한 시스템이 실제로 이 패턴들을 인식하는지 테스트한다.

🤖 Prompt for AI Agents
In @.claude/settings.local.json around lines 3 - 6, Replace the non-standard
Gradle patterns "Bash(./gradlew test:*)" and "Bash(./gradlew
:apps:commerce-api:test:*)" in the "allow" array with the canonical task
invocations "Bash(./gradlew test)" and "Bash(./gradlew
:apps:commerce-api:test)"; update the entries in .claude/settings.local.json
where those strings appear (look for the allow array) and then verify the
authorization system recognizes and permits those exact commands by testing
execution.

Comment on lines +57 to +66
public MemberModel(String loginId) {
// 모든 항목은 비어 있을 수 없다
if (loginId == null || loginId.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "아이디는 비어있을 수 없습니다.");
}

// 가입하는 로그인 ID는 영문과 숫자만 허용한다
validateLoginId(loginId);
this.loginId = loginId;
}
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

필수 필드 누락 가능한 생성자는 도메인 불변식 훼손 위험이 있다
운영 관점에서 name/birthDate/email이 null인 엔티티가 저장되면 데이터 무결성이 깨지고 조회/직렬화 경로에서 NPE가 발생할 수 있다.
수정안으로 이 생성자를 제거하거나 조회 전용 VO/팩토리로 분리하고, 엔티티 생성은 필수 필드를 강제해야 한다.
추가 테스트로 해당 생성 경로로는 영속화가 불가능함을 보장하는 테스트를 추가해야 한다.

As per coding guidelines: **/domain/**/*.java: 엔티티/값 객체/DTO 경계를 명확히 하고, 불변성과 캡슐화를 점검한다.

🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java`
around lines 57 - 66, The single-arg MemberModel(String loginId) constructor
allows creation of entities missing required fields (name, birthDate, email) and
thus breaks domain invariants; remove this constructor or convert it into a
separate read-only VO/factory used only for queries, and instead provide a
constructor/factory for entity creation that requires and assigns all mandatory
fields (e.g., include name, birthDate, email alongside loginId and reuse
validateLoginId(loginId) there). Update MemberModel so required fields are
non-nullable and set at construction, and add a unit/integration test proving
instances from the old single-arg path cannot be persisted (or that the
constructor is not present for persistence).

Comment on lines +34 to +38
ApiResponse<String> changePassword(
@RequestHeader("X-Loopers-LoginId") String loginId,
@RequestHeader("X-Loopers-LoginPw") String password,
MemberV1Dto.ChangePasswordRequest request
);
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

changePassword에서 헤더 password와 body oldPassword가 중복된다.

헤더의 X-Loopers-LoginPw와 body의 oldPassword가 모두 존재하여 클라이언트가 두 값을 동일하게 전송해야 하는지 불명확하다. 인증 흐름을 단순화하여 하나의 비밀번호만 사용하도록 API 설계를 재검토해야 한다.

수정안:

  1. body의 oldPassword만 사용하고 헤더에서 비밀번호 제거
  2. 또는 헤더 인증만 사용하고 body에서 oldPassword 제거
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1ApiSpec.java`
around lines 34 - 38, changePassword API currently requires both header
X-Loopers-LoginPw and request.body.oldPassword which duplicates credentials;
choose one approach and remove the other to avoid ambiguity. Either remove the
password header from MemberV1ApiSpec.changePassword (keep
MemberV1Dto.ChangePasswordRequest.oldPassword as the single source of truth) and
update callers/auth filters accordingly, or remove oldPassword from
MemberV1Dto.ChangePasswordRequest and rely solely on X-Loopers-LoginPw in the
changePassword signature and docs; ensure related validation and tests are
updated to reflect the chosen strategy.

Comment on lines +9 to +15
public record SignUpRequest(
String loginId,
String password,
String name,
String birthDate,
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 | 🟡 Minor

SignUpRequest record의 toString()에서 비밀번호가 노출될 수 있다.

Java record는 자동으로 toString()을 생성하며, password 필드가 평문으로 출력된다. 로깅 프레임워크나 예외 메시지에서 request 객체가 출력될 경우 비밀번호가 로그에 기록될 수 있다.

수정안: toString()을 오버라이드하여 password 필드를 마스킹하거나, 로깅 시 해당 객체를 직접 출력하지 않도록 주의해야 한다.

🛡️ 수정 제안
     public record SignUpRequest(
         String loginId,
         String password,
         String name,
         String birthDate,
         String email
-    ) {}
+    ) {
+      `@Override`
+      public String toString() {
+        return "SignUpRequest[loginId=" + loginId + ", password=*****, name=" + name + 
+               ", birthDate=" + birthDate + ", email=" + email + "]";
+      }
+    }
📝 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 record SignUpRequest(
String loginId,
String password,
String name,
String birthDate,
String email
) {}
public record SignUpRequest(
String loginId,
String password,
String name,
String birthDate,
String email
) {
`@Override`
public String toString() {
return "SignUpRequest[loginId=" + loginId + ", password=*****, name=" + name +
", birthDate=" + birthDate + ", email=" + email + "]";
}
}
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Dto.java`
around lines 9 - 15, SignUpRequest (the record in MemberV1Dto) exposes password
via the auto-generated toString, so override SignUpRequest::toString to mask the
password (e.g., replace with fixed mask or show only length) and return the
other fields (loginId, name, birthDate, email) as before; alternatively ensure
callers never log the record directly—update any logging sites to avoid printing
the object—to prevent plaintext passwords from appearing in logs.

Comment on lines +42 to +45
public record ChangePasswordRequest(
String oldPassword,
String newPassword
) {}
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

ChangePasswordRequest record의 toString()에서도 비밀번호가 노출될 수 있다.

oldPasswordnewPassword 모두 민감정보이므로 동일한 마스킹 처리가 필요하다.

🛡️ 수정 제안
     public record ChangePasswordRequest(
       String oldPassword,
       String newPassword
-    ) {}
+    ) {
+      `@Override`
+      public String toString() {
+        return "ChangePasswordRequest[oldPassword=*****, newPassword=*****]";
+      }
+    }
📝 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 record ChangePasswordRequest(
String oldPassword,
String newPassword
) {}
public record ChangePasswordRequest(
String oldPassword,
String newPassword
) {
`@Override`
public String toString() {
return "ChangePasswordRequest[oldPassword=*****, newPassword=*****]";
}
}
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/member/MemberV1Dto.java`
around lines 42 - 45, The ChangePasswordRequest record's autogenerated
toString() can expose sensitive fields oldPassword and newPassword; override the
record's toString() in ChangePasswordRequest to return a masked representation
(e.g., replace characters with '*' or show only last 2 chars) for both
oldPassword and newPassword instead of printing raw values, ensuring any logging
or serialization uses the masked string; implement the toString() method inside
the ChangePasswordRequest record and reference the oldPassword and newPassword
fields when building the masked output.

Comment on lines +20 to +38
void createsMemberModel_whenAllFieldsAreProvided() {
// arrange
String loginId = "testuser";
String rawPassword = "Test1234!";
String name = "홍길동";
String birthDate = "19900101";
String email = "test@example.com";

// act
MemberModel member = new MemberModel(loginId, rawPassword, name, birthDate, email);

// assert
assertThat(member.getLoginId()).isEqualTo(loginId);
assertThat(member.getPassword()).isEqualTo(rawPassword);
assertThat(member.getName()).isEqualTo(name);
assertThat(member.getBirthDate()).isEqualTo(birthDate);
assertThat(member.getEmail()).isEqualTo(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:

fd "MemberModel.java" --type f

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

Length of output: 159


🏁 Script executed:

head -n 200 apps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.java

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

Length of output: 5923


🏁 Script executed:

wc -l apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java

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

Length of output: 163


🏁 Script executed:

cat -n apps/commerce-api/src/main/java/com/loopers/domain/member/MemberModel.java

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

Length of output: 4989


생성과 변경 시점에 비밀번호 평문 저장이 고정되어 암호화 적용을 차단한다

현재 MemberModel 생성자(2255줄)와 changePassword() 메서드(142145줄)는 비밀번호를 검증만 수행하고 평문으로 저장한다. 테스트가 이 평문 저장을 정상 동작으로 기대하므로, 향후 생성자나 변경 메서드에 암호화 로직을 추가할 때 테스트가 실패하며 보안 개선을 차단한다.

encryptPassword() 메서드(118~120줄)는 존재하나 서비스 계층에서 호출되는 구조이므로, 도메인 모델 자체가 암호화 검증을 하지 않는 상태다. 테스트는 비밀번호가 평문과 다르고 matches() 검증을 통과하는지 확인하도록 변경해야 한다.

🔧 수정 예시
-      assertThat(member.getPassword()).isEqualTo(rawPassword);
+      // 비밀번호는 평문과 달라야 하고, 인코더로 검증되어야 함
+      assertThat(member.getPassword()).isNotEqualTo(rawPassword);
-      assertThat(member.getPassword()).isEqualTo(newPassword);
+      // 비밀번호 변경 후에도 평문과 달라야 하고, 인코더로 검증되어야 함
+      assertThat(member.getPassword()).isNotEqualTo(newPassword);
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/domain/member/MemberModelTest.java`
around lines 20 - 38, The test currently assumes plain-text password storage;
update the test in MemberModelTest so it no longer asserts member.getPassword()
equals rawPassword; instead assert that member.getPassword() is not equal to
rawPassword and that member.matches(rawPassword) (or the equivalent password
verification method on MemberModel) returns true; reference the MemberModel
constructor and the changePassword() / encryptPassword() behavior so the test
will continue to pass when the domain starts storing encrypted passwords.

Comment on lines +63 to 65
// 암호화
implementation ("org.springframework.security:spring-security-crypto")
// Test
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 "build.gradle.kts" -type f | head -20

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

Length of output: 429


🏁 Script executed:

head -n 100 build.gradle.kts

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

Length of output: 3531


🏁 Script executed:

sed -n '55,75p' build.gradle.kts

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

Length of output: 1313


🏁 Script executed:

find . -name "gradle.properties" -type f

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

Length of output: 104


🏁 Script executed:

cat gradle.properties

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

Length of output: 621


🏁 Script executed:

grep -r "springCloudDependenciesVersion" . --include="*.properties" --include="*.gradle.kts"

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

Length of output: 295


🏁 Script executed:

# Spring Cloud BOM 정보 조회 (웹 검색 필요)
cat <<'EOF' > /tmp/check_bom.py
import json
import subprocess

# gradle의 dependencyInsight 실행 시뮬레이션
# Spring Cloud BOM 2024.0.1에 포함된 spring-security-crypto 버전 확인

# Spring Cloud 2024.0.1 BOM의 spring-security-crypto 버전
# 공식 문서: https://spring.io/projects/spring-cloud
# Spring Cloud 2024.0.1은 Spring Boot 3.4.x를 지원하고
# spring-security 버전을 상속받음

print("Spring Cloud BOM 2024.0.1 분석:")
print("- Spring Boot 3.4.4를 사용")
print("- Spring Cloud 2024.0.1 = 2024.0.1 (latest stable)")
print("- spring-security-crypto는 spring-boot-starter-security로부터 전이 의존성")
print("- 명시적 버전 선언 없음 = BOM에서 암묵적 관리")
EOF
python /tmp/check_bom.py

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

Length of output: 284


🏁 Script executed:

# build.gradle.kts에서 spring-security 관련 의존성 전체 확인
grep -n "security" build.gradle.kts

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

Length of output: 166


🏁 Script executed:

# dependencyManagement 설정 전체 확인
sed -n '48,52p' build.gradle.kts

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

Length of output: 251


🌐 Web query:

Spring Cloud BOM 2024.0.1 spring-security-crypto version dependency management

💡 Result:

With Spring Cloud BOM 2024.0.1, the release train is based on Spring Boot 3.4.3 [1]. Spring Cloud itself does not independently manage spring-security-crypto; that version comes from Spring Boot’s dependency management (via spring-boot-dependencies).

In Spring Boot 3.4.3, the managed Spring Security BOM is org.springframework.security:spring-security-bom:6.4.3 [2], which in turn manages org.springframework.security:spring-security-crypto:6.4.3 [3].

So, effectively, Spring Cloud 2024.0.1 ⇒ Spring Boot 3.4.3spring-security-crypto 6.4.3. [1][2][3]

Sources:
[1] Spring Cloud 2024.0.1 release blog (based on Spring Boot 3.4.3) (spring.io)
[2] spring-boot-dependencies 3.4.3 managed dependencies show spring-security-bom 6.4.3 (mvnrepository.com)
[3] spring-security-crypto artifact version 6.4.3 (mvnrepository.com)

Citations:


🌐 Web query:

Spring Boot 3.4.4 spring-security-crypto BOM version management

💡 Result:

Spring Boot 3.4.4 manages org.springframework.security:* (including spring-security-crypto) via its dependency-management BOM (org.springframework.boot:spring-boot-dependencies). In that BOM, the managed Spring Security BOM version is 6.4.4, so spring-security-crypto is effectively managed at 6.4.4 as well. [1]

Use the managed version (recommended)

Maven

<dependency>
  <groupId>org.springframework.security</groupId>
  <artifactId>spring-security-crypto</artifactId>
</dependency>

Override Spring Security (all modules, incl. crypto)

Spring Security documents that, when using Spring Boot’s BOM, you override by setting the spring-security.version property. [2]

Maven

<properties>
  <spring-security.version>6.4.6</spring-security.version>
</properties>

Gradle

ext['spring-security.version'] = '6.4.6'

Don’t import spring-security-bom yourself unless you’re intentionally taking over version alignment from Boot—otherwise you can end up with mixed Spring Security versions.

Sources: [1] Spring Boot 3.4.4 managed dependencies show spring-security-bom at 6.4.4; [2] Spring Security reference docs on Spring Boot BOM usage and overriding via spring-security.version.


spring-security-crypto 버전을 gradle.properties에 명시적으로 정의해야 한다

현재 구현은 Spring Cloud BOM 2024.0.1 → Spring Boot 3.4.4 → spring-security-bom 6.4.4를 통해 spring-security-crypto 6.4.4로 관리되고 있으나, gradle.properties에 명시되지 않아 버전 관리 일관성이 떨어진다. 운영 관점에서 암호화 라이브러리 버전을 gradle.properties에 spring-security.version=6.4.4 형태로 명시하면 의존성 추적과 보안 감시가 용이해진다. 동시에 CI 파이프라인에 보안 스캔(DependencyCheck/Snyk)을 포함하여 취약점 변동을 모니터링해야 한다.

🤖 Prompt for AI Agents
In `@build.gradle.kts` around lines 63 - 65, Add a new property named
spring-security.version (set to 6.4.4) in gradle.properties and update the
spring-security-crypto declaration in build.gradle.kts so it references that
property instead of relying solely on the BOM (i.e., change the
implementation("org.springframework.security:spring-security-crypto") entry to
use the property-based version); also add/enable a dependency security scan step
(DependencyCheck or Snyk) in the CI pipeline to monitor for vulnerabilities.

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.

3 participants