[volume-1] 회원가입, 내 정보 조회, 비밀번호 변경 테스트코드 작성 및 기능 구현 #42
Conversation
📝 WalkthroughWalkthrough사용자 가입, 인증, 비밀번호 변경 기능을 포함하는 사용자 관리 시스템이 추가된다. Spring Security 기반 인증 인터셉터, 도메인 검증 로직, JPA 리포지토리, 서비스 계층이 함께 구성되며 광범위한 단위 및 통합 테스트가 포함된다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthInterceptor
participant UserV1Controller
participant UserFacade
participant UserService
participant PasswordEncoder
participant UserRepository
participant Database
Client->>AuthInterceptor: POST /api/v1/users/signup<br/>(headers: LoginId, LoginPw)
AuthInterceptor->>AuthInterceptor: Validate headers not blank
AuthInterceptor->>UserService: authenticate(loginId, password)
UserService->>UserRepository: findByLoginId(loginId)
UserRepository->>Database: SELECT * FROM users WHERE loginId=?
Database-->>UserRepository: UserModel
UserService->>PasswordEncoder: matches(password, stored)
PasswordEncoder-->>UserService: true/false
UserService-->>AuthInterceptor: UserModel or throw UNAUTHORIZED
AuthInterceptor->>AuthInterceptor: Store loginId in request attribute
AuthInterceptor->>UserV1Controller: proceed with handler
Client->>UserV1Controller: POST /api/v1/users/signup<br/>(SignupRequest)
UserV1Controller->>UserFacade: signUp(SignupCommand)
UserFacade->>UserService: signup(loginId, password, name, birthday, email)
UserService->>UserService: Password.validate(password, birthday)
UserService->>UserRepository: existsByLoginId(loginId)
UserRepository->>Database: SELECT COUNT FROM users WHERE loginId=?
Database-->>UserRepository: count
UserService->>PasswordEncoder: encode(password)
PasswordEncoder-->>UserService: encrypted
UserService->>UserRepository: save(UserModel)
UserRepository->>Database: INSERT INTO users
Database-->>UserRepository: UserModel (saved)
UserService-->>UserFacade: UserModel
UserFacade->>UserFacade: UserInfo.from(UserModel)
UserFacade-->>UserV1Controller: UserInfo
UserV1Controller->>UserV1Controller: UserResponse.from(UserInfo)
UserV1Controller-->>Client: ApiResponse<UserResponse>
sequenceDiagram
participant Client
participant AuthInterceptor
participant UserV1Controller
participant UserFacade
participant UserService
participant PasswordEncoder
participant UserRepository
participant Database
Client->>AuthInterceptor: PATCH /api/v1/users/password<br/>(headers: LoginId, LoginPw)
AuthInterceptor->>UserService: authenticate(loginId, password)
UserService->>UserRepository: findByLoginId(loginId)
UserRepository->>Database: SELECT * FROM users
Database-->>UserRepository: UserModel
UserService->>PasswordEncoder: matches(password, stored)
PasswordEncoder-->>UserService: true/false
UserService-->>AuthInterceptor: UserModel or throw UNAUTHORIZED
AuthInterceptor->>AuthInterceptor: Store loginId in request attribute
AuthInterceptor->>UserV1Controller: proceed with handler
Client->>UserV1Controller: PATCH /api/v1/users/password<br/>(ChangePasswordRequest)
UserV1Controller->>UserFacade: changePassword(ChangePasswordCommand)
UserFacade->>UserService: changePassword(loginId, currentPassword, newPassword)
UserService->>UserRepository: findByLoginId(loginId)
UserRepository->>Database: SELECT * FROM users
Database-->>UserRepository: UserModel
UserService->>PasswordEncoder: matches(currentPassword, stored)
PasswordEncoder-->>UserService: true/false (validate current)
UserService->>UserService: validate newPassword != currentPassword
UserService->>UserService: Password.validate(newPassword, birthday)
UserService->>PasswordEncoder: encode(newPassword)
PasswordEncoder-->>UserService: encrypted
UserService->>UserModel: changePassword(encrypted)
UserService->>UserRepository: save(UserModel)
UserRepository->>Database: UPDATE users SET password=?
Database-->>UserRepository: success
UserRepository-->>UserService: UserModel (updated)
UserService-->>UserFacade: void
UserFacade-->>UserV1Controller: void
UserV1Controller-->>Client: ApiResponse<Object>
운영 관점의 주요 검토 항목
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@apps/commerce-api/build.gradle.kts`:
- Around line 13-14: Add a new gradle.properties entry named
springSecurityCryptoVersion and update the implementation dependency in
build.gradle.kts to reference that property (use
project.properties["springSecurityCryptoVersion"] or the Kotlin DSL property
accessor) instead of a hard-coded coordinate; specifically, add the
springSecurityCryptoVersion key to gradle.properties and replace the
implementation("org.springframework.security:spring-security-crypto") line in
apps/commerce-api/build.gradle.kts to use the property reference; also ensure CI
dependency-vulnerability scanning is added/updated to regularly check the
project's dependencies.
In
`@apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java`:
- Around line 8-28: The UserService currently lacks transaction boundaries
causing potential data inconsistency; add `@Transactional` to the UserService
(class-level or on the signup and changePassword methods) so the signup and
changePassword flows (which perform read-then-write: UserService.signup and
UserService.changePassword) run in a single transaction and roll back on
exceptions, ensure the annotation uses default rollback-on-runtime-exception
semantics (or explicitly set rollbackFor if checked exceptions are used), and
add unit/integration tests that simulate an exception during the
persistence/save step to verify the operations rollback and no partial state is
persisted.
In `@apps/commerce-api/src/main/java/com/loopers/config/AuthInterceptor.java`:
- Around line 16-17: 현재 AuthInterceptor가 HEADER_LOGIN_ID/HEADER_LOGIN_PW을 통해 매요청
평문 비밀번호를 전송받아 authenticate()를 호출하는 방식은 보안·성능상 문제이므로, AuthInterceptor를 JWT 또는 세션
기반으로 전환하도록 수정하세요: 로그인 엔드포인트에서 사용자 인증 후 JWT 토큰(또는 세션 ID)을 발급하고 클라이언트는 이후 요청에
HEADER_LOGIN_ID 대신 Authorization: Bearer <token>을 전송하도록 변경하며, AuthInterceptor는
HEADER_LOGIN_PW와 직접 비밀번호 비교를 하지 않고 토큰의 서명/만료를 검증하거나 세션 스토어 조회만 수행하도록 구현하세요; 학습
목적으로 단기 유지 시에도 authenticate()를 매요청 호출하지 않도록 토큰 캐시/세션 캐시를 도입하여 DB 조회를 줄이도록 조치하세요.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java`:
- Around line 12-27: Password.validate currently calls birthday.replace("-","")
and will NPE if birthday is null; update Password.validate to first handle
null/blank birthday by either throwing CoreException(ErrorType.BAD_REQUEST) when
birthday is null/blank or by skipping the birthdate containment check when
birthday is null; specifically guard the birthday variable before calling
birthday.replace(...) and before checking value.contains(...), and add unit
tests for Password.validate covering birthday == null and birthday == "" to
assert BAD_REQUEST (or the chosen behavior) so the method no longer throws an
NPE.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java`:
- Around line 39-84: Remove the plaintext password validation from object
construction: delete or simplify validatePassword so the UserModel constructor
no longer rejects non-BCrypt-looking values (i.e., remove the
validatePassword(...) call in the UserModel(String loginId, String password,
...) constructor), retain a null-only check if you want to guard against nulls
in changePassword(String encryptedPassword) and/or constructor, and add a clear
comment on the UserModel constructor and changePassword method stating "this
method accepts a BCrypt hash only"; finally update unit tests that call new
UserModel(..., "otherPw@123", ...) to pass a BCrypt-hashed test constant (e.g.,
VALID_ENCRYPTED_PASSWORD) instead of a plaintext password so tests honor the
contract.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`:
- Around line 11-13: The UserService class is missing transactional boundaries
causing race conditions and inconsistency for signup and changePassword; add
`@Transactional` to the UserService class or annotate the specific methods
signup(...) and changePassword(...) so the read+write sequences run in a single
transaction (ensure appropriate propagation/isolation if needed), and also
ensure the loginId column has a DB-level UNIQUE constraint as a final defense
against concurrent duplicate inserts.
- Around line 36-46: The authenticate method in UserService leaks timing via
immediate exception when user not found; to fix, always perform a BCrypt
comparison regardless of user existence by obtaining a constant dummy hash
(e.g., a hard-coded valid BCrypt hash or one generated once at startup) and
calling passwordEncoder.matches(password, actualUser != null ?
user.getPassword() : dummyHash) before throwing the same CoreException on
mismatch; keep the userRepository.findByLoginId lookup but delay throwing until
after the comparison so response timing is consistent and retain the existing
ErrorType.UNAUTHORIZED behavior.
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordTest.java`:
- Around line 18-100: Password.validate currently calls birthday.replace(...)
and can NPE when birthday is null; add a null check at the start of
Password.validate (the static validate method in Password) and throw the same
CoreException with ErrorType.BAD_REQUEST used by other invalid-password cases
when birthday is null, then add a unit test in PasswordTest (e.g.,
throwsException_whenBirthdayIsNull) that asserts Password.validate(null, null)
or Password.validate(VALID_PASSWORD, null) throws CoreException with
ErrorType.BAD_REQUEST so the behavior is covered.
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java`:
- Around line 94-104: The test
throwsBadRequestException_whenBirthDateisAfterToday in UserModelTest uses a
hardcoded future date ("2027-01-01") which will become stale; change the test to
construct the future birth date dynamically (e.g., use
LocalDate.now().plusYears(1).toString() or equivalent) when calling new
UserModel(...) so the assertion on ErrorType.BAD_REQUEST remains
time-independent and stable across runs.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java`:
- Around line 59-71: Update the test expectation in
UserServiceIntegrationTest::throwsException_whenLoginIdAlreadyExists to expect
ErrorType.CONFLICT instead of ErrorType.BAD_REQUEST; locate the assertion that
checks result.getErrorType() and change the expected value to
ErrorType.CONFLICT. Also add (or plan) an API/E2E test that asserts the HTTP
response for duplicate signup maps to 409 to lock the contract between service
and clients.
🧹 Nitpick comments (12)
apps/commerce-api/src/main/java/com/loopers/config/SecurityConfig.java (1)
8-14: BCryptPasswordEncoder의 강도 설정을 운영 환경에 맞게 조정하기 바란다현재 코드는 BCryptPasswordEncoder의 기본값(strength 10)을 사용하고 있다. BCrypt 강도는 password 검증 소요 시간을 결정하므로, 실제 프로덕션 하드웨어에서 성능 테스트를 거쳐 검증이 약 1초 내에 완료되도록 strength 값을 조정해야 한다. CPU 성능에 따라 strength(4~31 범위)를 결정하고 부하 테스트를 수행할 것을 권장한다.
수정 예시:
return new BCryptPasswordEncoder(12);apps/commerce-api/src/main/java/com/loopers/domain/user/UserRepository.java (1)
5-8: 도메인 레포지토리 인터페이스가 적절히 정의되었다.헥사고날 아키텍처에 맞게 도메인 계층에서 인프라 의존성 없이 인터페이스를 정의하였다.
운영 관점에서 고려할 사항:
UserService.signup()에서 중복 체크 시findByLoginId().isPresent()를 사용하는데, 대량 트래픽 환경에서는existsByLoginId(String loginId)메서드를 추가하면 불필요한 엔티티 로딩을 피할 수 있다. 현재 규모에서는 문제가 되지 않으므로 선택적으로 고려하면 된다.apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java (1)
20-105: null 입력에 대한 테스트가 누락되었다.
loginId,name,birthday,birthday.replace("-", "")같은 코드가 Password 검증에서 사용되므로, null birthday가 전달되면 NPE가 발생할 수 있다.추가 테스트 케이스:
throwsBadRequestException_whenLoginIdIsNullthrowsBadRequestException_whenNameIsNullthrowsBadRequestException_whenBirthdayIsNullthrowsBadRequestException_whenEmailIsNullapps/commerce-api/src/main/java/com/loopers/infrastructure/user/UserRepositoryImpl.java (1)
10-12:@Component대신@Repository사용을 권장한다.레포지토리 구현체에
@Repository를 사용하면 다음과 같은 이점이 있다:
- 의미적 명확성: 해당 빈이 데이터 접근 계층임을 명시한다
- 예외 변환: Spring이 persistence 예외를
DataAccessException으로 자동 변환한다♻️ 수정 제안
-import org.springframework.stereotype.Component; +import org.springframework.stereotype.Repository; `@RequiredArgsConstructor` -@Component +@Repository public class UserRepositoryImpl implements UserRepository {apps/commerce-api/src/main/java/com/loopers/application/user/UserInfo.java (1)
14-22:from()메서드에서 null 입력 처리가 없다.
UserModel이 null로 전달되면 NPE가 발생한다. 호출자가 항상 non-null을 보장하더라도, 방어적 코딩 관점에서 명시적인 null 체크 또는Objects.requireNonNull()을 추가하면 디버깅이 용이해진다.♻️ 수정 제안
+import java.util.Objects; + public record UserInfo( // ... ) { public static UserInfo from(UserModel userModel) { + Objects.requireNonNull(userModel, "userModel must not be null"); return new UserInfo( userModel.getId(), userModel.getLoginId(), userModel.getName(), userModel.getBirthday(), userModel.getEmail() ); } }apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordTest.java (1)
41-63: 길이 검증의 경계값 테스트가 누락되었다.현재 7자(미만)와 17자(초과)만 테스트하고 있다. 정확히 8자와 16자인 비밀번호가 유효한지 확인하는 경계값 테스트를 추가해야 테스트 신뢰성이 높아진다.
추가 테스트 케이스:
doesNotThrowException_whenPasswordIs8Characters(예:"qwer@123")doesNotThrowException_whenPasswordIs16Characters(예:"qwer@12345678901")apps/commerce-api/src/main/java/com/loopers/config/AuthInterceptor.java (1)
22-35: 인터셉터 예외 발생 시 로깅 추가를 권장한다.인증 실패 시
CoreException이 발생하지만, 실패 원인에 대한 로깅이 없다. 운영 환경에서 인증 실패 패턴 모니터링, 무차별 대입 공격 탐지 등을 위해 로깅이 필요하다.단, 로그에
loginPw는 절대 포함하지 않아야 한다.♻️ 로깅 추가 제안
+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + `@RequiredArgsConstructor` `@Component` public class AuthInterceptor implements HandlerInterceptor { + private static final Logger log = LoggerFactory.getLogger(AuthInterceptor.class); // ... existing code ... `@Override` public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) { String loginId = request.getHeader(HEADER_LOGIN_ID); String loginPw = request.getHeader(HEADER_LOGIN_PW); if (loginId == null || loginId.isBlank() || loginPw == null || loginPw.isBlank()) { + log.warn("인증 헤더 누락 - URI: {}, RemoteAddr: {}", request.getRequestURI(), request.getRemoteAddr()); throw new CoreException(ErrorType.UNAUTHORIZED, "인증 헤더가 누락되었습니다."); } - userService.authenticate(loginId, loginPw); + try { + userService.authenticate(loginId, loginPw); + } catch (CoreException e) { + log.warn("인증 실패 - loginId: {}, URI: {}", loginId, request.getRequestURI()); + throw e; + } request.setAttribute(ATTR_LOGIN_ID, loginId); return true; } }apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java (2)
223-242: "새 비밀번호가 현재 비밀번호와 동일" 테스트에 stub 누락 가능성이 있다.
UserService.changePassword의 동일 비밀번호 검증 로직(Line 57-59)을 보면:if (passwordEncoder.matches(newPassword, user.getPassword())) { throw new CoreException(ErrorType.BAD_REQUEST, ...); }테스트에서
passwordEncoder.matches(VALID_PASSWORD, VALID_ENCRYPTED_PASSWORD)를true로 stub했지만, 이는 현재 비밀번호 검증용이다. 새 비밀번호(VALID_PASSWORD)와 저장된 암호화 비밀번호(VALID_ENCRYPTED_PASSWORD) 비교를 위한 두 번째matches호출에 대한 stub이 필요하다.현재 동작하는 이유는 Mockito의 lenient stubbing 또는 동일한 인자 조합이기 때문이다. 명시적으로 의도를 표현하는 것이 테스트 가독성에 좋다.
♻️ 명시적 stub 추가 제안
when(userRepository.findByLoginId(VALID_LOGIN_ID)) .thenReturn(Optional.of(user)); when(passwordEncoder.matches(VALID_PASSWORD, VALID_ENCRYPTED_PASSWORD)) .thenReturn(true); + // 새 비밀번호가 현재 비밀번호와 동일한지 확인하는 stub (동일하므로 true) + // Note: 위 stub과 동일한 인자이므로 이미 적용됨. 명시적 주석으로 의도 표현.
70-91: 중복 loginId 테스트에서 사용된 비밀번호가 Password 검증 규칙을 위반할 수 있다.Line 75의
existingUser생성 시"anonymous@123"비밀번호를 사용하는데,UserModel생성자가 암호화된 비밀번호를 받는다면 문제없다. 그러나 만약Password.validate()가 호출되는 경로라면 규칙 위반으로 테스트가 실패할 수 있다.학습된 내용에 따르면
UserModel생성자는 BCrypt 해시를 받으므로 현재 구현은 문제없다. 다만 테스트 데이터 일관성을 위해VALID_ENCRYPTED_PASSWORD상수 사용을 권장한다.apps/commerce-api/src/test/java/com/loopers/config/AuthInterceptorTest.java (1)
57-71: 빈 문자열 헤더에 대한 테스트 케이스가 누락되었다.
AuthInterceptor는loginId.isBlank()및loginPw.isBlank()검증을 수행하지만, 현재 테스트는null헤더만 검증한다. 빈 문자열("") 또는 공백만 있는 문자열(" ")에 대한 테스트가 필요하다.운영 환경에서 클라이언트가 빈 헤더를 전송하는 경우가 있으므로, 이 경계 케이스를 검증해야 한다.
💚 빈 문자열 헤더 테스트 추가 제안
`@DisplayName`("빈 문자열 인증 헤더로 요청하면, 예외가 발생한다.") `@Test` void throwsException_whenHeadersAreBlank() { // arrange HttpServletRequest request = mock(HttpServletRequest.class); HttpServletResponse response = mock(HttpServletResponse.class); when(request.getHeader(HEADER_LOGIN_ID)).thenReturn(" "); when(request.getHeader(HEADER_LOGIN_PW)).thenReturn(""); // act & assert CoreException result = assertThrows(CoreException.class, () -> { authInterceptor.preHandle(request, response, new Object()); }); assertThat(result.getErrorType()).isEqualTo(ErrorType.UNAUTHORIZED); verify(userService, never()).authenticate(any(), any()); }apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (2)
82-107: 중복 loginId 테스트에서 예상 HTTP 상태 코드 검증이 필요하다.
UserService.signup에서 중복 loginId 시ErrorType.CONFLICT를 던지는데, 테스트는is4xxClientError()로만 검증한다.CONFLICT가 HTTP 409로 매핑되는지, 또는 다른 4xx 코드인지 명시적으로 검증하면 API 계약이 명확해진다.♻️ 명시적 상태 코드 검증 제안
// assert assertAll( - () -> assertTrue(response.getStatusCode().is4xxClientError()), + () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT), () -> assertThat(response.getBody().meta().result()).isEqualTo(ApiResponse.Metadata.Result.FAIL) );
144-157: 인증 헤더 누락 테스트에서HttpEntity에 null 전달 검토가 필요하다.Line 150에서
HttpEntity에null을 전달하고 있다. 이는 요청 본문과 헤더가 모두 없는 상태를 의미한다. 명시적으로 빈 헤더를 전달하는 것이 테스트 의도를 더 명확하게 표현한다.♻️ 명시적 빈 HttpEntity 사용 제안
// act ParameterizedTypeReference<ApiResponse<UserV1Dto.UserResponse>> responseType = new ParameterizedTypeReference<>() {}; ResponseEntity<ApiResponse<UserV1Dto.UserResponse>> response = - testRestTemplate.exchange(ME_ENDPOINT, HttpMethod.GET, null, responseType); + testRestTemplate.exchange(ME_ENDPOINT, HttpMethod.GET, HttpEntity.EMPTY, responseType);
| // security (PasswordEncoder 사용) | ||
| implementation("org.springframework.security:spring-security-crypto") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 파일 존재 확인 및 내용 조회
find . -name "build.gradle.kts" -type f | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 429
🏁 Script executed:
# 특정 파일 내용 확인
if [ -f "apps/commerce-api/build.gradle.kts" ]; then
cat -n "apps/commerce-api/build.gradle.kts"
else
echo "File not found at apps/commerce-api/build.gradle.kts"
fiRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1290
🏁 Script executed:
# 버전 관리 관련 파일 검색
find . -type f \( -name "gradle.properties" -o -name "libs.versions.toml" -o -name "versions.gradle" -o -name "*.bom" \) | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 104
🏁 Script executed:
# spring-security 관련 의존성 검색
rg "spring-security" -A 2 -B 2 --type gradleRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 233
🏁 Script executed:
# gradle.properties 파일 내용 확인
cat -n gradle.propertiesRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 747
🏁 Script executed:
# 상위 build.gradle.kts 파일 확인
cat -n build.gradle.kts | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4157
🏁 Script executed:
# spring-security 관련 의존성 검색 (파일별)
echo "=== 모든 build.gradle.kts 파일에서 spring-security 검색 ==="
grep -r "spring-security" --include="build.gradle.kts" . 2>/dev/null | head -50Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 247
🏁 Script executed:
# springSecurityCryptoVersion이 정의되어 있는지 확인
grep -r "springSecurityCryptoVersion\|spring-security" gradle.properties 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 67
🏁 Script executed:
# Spring Boot 버전 확인 및 spring-security 포함 여부 검색
echo "=== Spring Boot Version ==="
grep -A 2 "springBootVersion" gradle.properties
echo -e "\n=== 다른 build.gradle.kts 파일들의 의존성 관리 패턴 ==="
for file in ./apps/*/build.gradle.kts ./modules/*/build.gradle.kts ./supports/*/build.gradle.kts; do
if [ -f "$file" ]; then
echo "=== $file ==="
grep "implementation\|annotationProcessor" "$file" | head -10
fi
doneRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3251
🏁 Script executed:
# Spring Security 관련 의존성 관리 확인
echo "=== Spring Security 관련 의존성 전체 검색 ==="
grep -r "spring-security" . --include="*.gradle" --include="*.gradle.kts" --include="*.properties" --include="*.toml" 2>/dev/nullRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 232
spring-security-crypto 의존성의 버전을 gradle.properties에서 명시적으로 관리해야 한다.
현재 버전이 고정되지 않아 빌드 시점에 따라 해상되는 버전이 달라질 수 있고, 운영 환경에서 보안 취약점 발생 시 대응이 지연된다. 프로젝트의 다른 외부 라이브러리(springdoc-openapi 등)는 gradle.properties에서 버전을 관리하고 있으므로 spring-security-crypto도 동일한 패턴으로 통일해야 한다.
gradle.properties에 springSecurityCryptoVersion= 항목을 추가하고, build.gradle.kts에서 ${project.properties["springSecurityCryptoVersion"]}로 참조하도록 수정해야 한다. 추가로 CI 파이프라인에 의존성 취약점 스캔 단계를 추가하여 정기적으로 의존성 버전의 보안 상태를 점검하는 편이 낫다.
수정 예시
gradle.properties에 추가:
springSecurityCryptoVersion=6.4.2
build.gradle.kts 수정:
- implementation("org.springframework.security:spring-security-crypto")
+ implementation("org.springframework.security:spring-security-crypto:${project.properties["springSecurityCryptoVersion"]}")🤖 Prompt for AI Agents
In `@apps/commerce-api/build.gradle.kts` around lines 13 - 14, Add a new
gradle.properties entry named springSecurityCryptoVersion and update the
implementation dependency in build.gradle.kts to reference that property (use
project.properties["springSecurityCryptoVersion"] or the Kotlin DSL property
accessor) instead of a hard-coded coordinate; specifically, add the
springSecurityCryptoVersion key to gradle.properties and replace the
implementation("org.springframework.security:spring-security-crypto") line in
apps/commerce-api/build.gradle.kts to use the property reference; also ensure CI
dependency-vulnerability scanning is added/updated to regularly check the
project's dependencies.
| @RequiredArgsConstructor | ||
| @Component | ||
| public class UserFacade { | ||
| private final UserService userService; | ||
|
|
||
| public UserInfo signUp(SignupCommand command) { | ||
| UserModel user = userService.signup( | ||
| command.loginId(), command.password(), command.name(), command.birthday(), command.email() | ||
| ); | ||
| return UserInfo.from(user); | ||
| } | ||
|
|
||
| public UserInfo getMyInfo(String loginId) { | ||
| UserModel user = userService.findByLoginId(loginId); | ||
| return UserInfo.from(user); | ||
| } | ||
|
|
||
| public void changePassword(ChangePasswordCommand command) { | ||
| userService.changePassword(command.loginId(), command.currentPassword(), command.newPassword()); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "UserService.java" -type fRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 159
🏁 Script executed:
rg -n "@Transactional|@Service|public.*signup|public.*changePassword|public.*findByLoginId" --type=java -A 2 -B 1 -g "**/UserService.java"Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1622
🏁 Script executed:
cat -n apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java | head -100Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3451
트랜잭션 경계가 없어 데이터 정합성 위험이 존재한다.
UserService에 @Transactional 어노테이션이 전혀 적용되지 않았다. signup 메서드는 중복 체크(읽기) 후 사용자를 저장(쓰기)하고, changePassword 메서드도 사용자 조회(읽기) 후 비밀번호를 변경하고 저장(쓰기)한다. 이 과정에서 예외 발생 시 데이터 정합성이 보장되지 않는다.
예를 들어 signup에서 중복 체크는 통과했으나 저장 중 DB 오류가 발생하면 부분적으로 성공한 상태가 남을 수 있다. changePassword에서도 검증은 완료했으나 저장 실패 시 불일치 상태가 된다.
개선안:
UserService에@Transactional적용 (클래스 레벨 또는signup,changePassword메서드 레벨)- 테스트: 각 메서드에서 저장 단계 예외 발생 시 변경사항이 롤백되는지 검증
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/application/user/UserFacade.java`
around lines 8 - 28, The UserService currently lacks transaction boundaries
causing potential data inconsistency; add `@Transactional` to the UserService
(class-level or on the signup and changePassword methods) so the signup and
changePassword flows (which perform read-then-write: UserService.signup and
UserService.changePassword) run in a single transaction and roll back on
exceptions, ensure the annotation uses default rollback-on-runtime-exception
semantics (or explicitly set rollbackFor if checked exceptions are used), and
add unit/integration tests that simulate an exception during the
persistence/save step to verify the operations rollback and no partial state is
persisted.
| private static final String HEADER_LOGIN_ID = "X-Loopers-LoginId"; | ||
| private static final String HEADER_LOGIN_PW = "X-Loopers-LoginPw"; |
There was a problem hiding this comment.
보안 및 운영 관점에서 인증 방식 검토가 필요하다.
매 요청마다 헤더에 비밀번호를 평문으로 전송하는 방식은 다음 문제가 있다:
- 보안 위험: 비밀번호가 매 요청마다 네트워크를 통해 전송되어 노출 위험이 증가한다. HTTPS 환경이더라도 로그, 프록시, CDN 등에서 헤더가 기록될 수 있다.
- 운영 부담: 매 요청마다
authenticate()가 호출되어 DB 조회 + 비밀번호 해시 비교가 발생한다. 트래픽 증가 시 DB 부하가 급증할 수 있다.
권장 수정안: JWT 토큰 또는 세션 기반 인증으로 전환하여, 로그인 시 한 번만 인증하고 이후 토큰/세션으로 검증하는 방식을 고려해야 한다.
학습/프로토타입 목적이라면 현재 방식을 유지하되, 프로덕션 배포 전 반드시 개선이 필요하다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/config/AuthInterceptor.java`
around lines 16 - 17, 현재 AuthInterceptor가 HEADER_LOGIN_ID/HEADER_LOGIN_PW을 통해
매요청 평문 비밀번호를 전송받아 authenticate()를 호출하는 방식은 보안·성능상 문제이므로, AuthInterceptor를 JWT 또는
세션 기반으로 전환하도록 수정하세요: 로그인 엔드포인트에서 사용자 인증 후 JWT 토큰(또는 세션 ID)을 발급하고 클라이언트는 이후 요청에
HEADER_LOGIN_ID 대신 Authorization: Bearer <token>을 전송하도록 변경하며, AuthInterceptor는
HEADER_LOGIN_PW와 직접 비밀번호 비교를 하지 않고 토큰의 서명/만료를 검증하거나 세션 스토어 조회만 수행하도록 구현하세요; 학습
목적으로 단기 유지 시에도 authenticate()를 매요청 호출하지 않도록 토큰 캐시/세션 캐시를 도입하여 DB 조회를 줄이도록 조치하세요.
| public static void validate(String value, String birthday) { | ||
| if (value == null || value.isBlank()) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다."); | ||
| } | ||
|
|
||
| if (value.length() < MIN_LENGTH || value.length() > MAX_LENGTH) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8자리 이상, 16자리 이하이어야 합니다."); | ||
| } | ||
|
|
||
| if (!value.matches(PASSWORD_PATTERN)) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호 형식이 올바르지 않습니다."); | ||
| } | ||
|
|
||
| String birthdayWithoutDash = birthday.replace("-", ""); | ||
| if (value.contains(birthday) || value.contains(birthdayWithoutDash)) { | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호에 생년월일을 포함할 수 없습니다."); |
There was a problem hiding this comment.
birthday가 null이면 NPE로 500이 발생할 수 있다
birthday가 null일 때 replace 호출로 NPE가 발생하면 운영에서 400이 아니라 500으로 떨어져 모니터링 지표와 장애 원인 분석이 왜곡된다.
birthday null/blank를 먼저 검증해 CoreException(BAD_REQUEST)로 처리하거나, null이면 생일 포함 검사를 건너뛰도록 분기하는 방식으로 수정하는 편이 안전하다.
추가 테스트로 birthday가 null 또는 공백인 경우 BAD_REQUEST가 반환되는 단위 테스트를 추가하는 편이 낫다.
🔧 예시 수정안
public static void validate(String value, String birthday) {
if (value == null || value.isBlank()) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다.");
}
+ if (birthday == null || birthday.isBlank()) {
+ throw new CoreException(ErrorType.BAD_REQUEST, "생년월일은 비어있을 수 없습니다.");
+ }
if (value.length() < MIN_LENGTH || value.length() > MAX_LENGTH) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 8자리 이상, 16자리 이하이어야 합니다.");
}
if (!value.matches(PASSWORD_PATTERN)) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호 형식이 올바르지 않습니다.");
}
String birthdayWithoutDash = birthday.replace("-", "");
if (value.contains(birthday) || value.contains(birthdayWithoutDash)) {
throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호에 생년월일을 포함할 수 없습니다.");
}
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java` around
lines 12 - 27, Password.validate currently calls birthday.replace("-","") and
will NPE if birthday is null; update Password.validate to first handle
null/blank birthday by either throwing CoreException(ErrorType.BAD_REQUEST) when
birthday is null/blank or by skipping the birthdate containment check when
birthday is null; specifically guard the birthday variable before calling
birthday.replace(...) and before checking value.contains(...), and add unit
tests for Password.validate covering birthday == null and birthday == "" to
assert BAD_REQUEST (or the chosen behavior) so the method no longer throws an
NPE.
| public UserModel(String loginId, String password, String name, String birthday, String email){ | ||
| validateLoginId(loginId); | ||
| validateName(name); | ||
| validateEmail(email); | ||
| validatePassword(password); | ||
|
|
||
| this.loginId = loginId; | ||
| this.name = name; | ||
| this.email = email; | ||
| this.password = password; | ||
| this.birthday = parseBirthday(birthday); | ||
| } | ||
|
|
||
| private void validateLoginId(String loginId){ | ||
| if(loginId == null || loginId.isBlank()){ | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "로그인 ID는 비어있을 수 없습니다."); | ||
| } | ||
| if(!loginId.matches(LOGIN_ID_PATTERN)){ | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "로그인 ID는 영문과 숫자만 허용됩니다."); | ||
| } | ||
| } | ||
|
|
||
| private void validateName(String name){ | ||
| if(name == null || name.isBlank()){ | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "이름은 비어있을 수 없습니다."); | ||
| } | ||
| } | ||
|
|
||
| private void validateEmail(String email){ | ||
| if(email == null || email.isBlank()){ | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "이메일은 비어있을 수 없습니다."); | ||
| } | ||
| if(!email.matches(EMAIL_PATTERN)){ | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "이메일 형식이 올바르지 않습니다."); | ||
| } | ||
| } | ||
|
|
||
| private void validatePassword(String password){ | ||
| if(password == null || password.isBlank()){ | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호는 비어있을 수 없습니다."); | ||
| } | ||
| } | ||
|
|
||
| public void changePassword(String encryptedPassword) { | ||
| validatePassword(encryptedPassword); | ||
| this.password = encryptedPassword; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find User.java and UserService
find . -type f -name "User.java" -o -name "UserService.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 159
🏁 Script executed:
# Check UserModel.java location and structure
fd -t f "UserModel.java" -x cat -n {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3974
🏁 Script executed:
# Search for how UserModel is instantiated in service layer
rg "new UserModel" -A 3 -B 3 --max-count=5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 10314
생성자 비밀번호 검증 불필요 - 테스트에서 계약 위반
학습 자료에 따르면 UserModel 생성자는 이미 암호화된 BCrypt 해시만 받아야 하며, 생성자 단계에서 null/공백 검증은 불필요하다. 실제 UserService 코드도 passwordEncoder.encode(password) 후 암호화된 값을 전달하므로 아키텍처는 올바르다.
문제는 다음 두 가지다:
-
validatePassword() 메서드가 불필요함: 생성자가 이미 암호화된 값만 받는다면 공백 검증은 무의미하다. 제거하거나 null만 검사하도록 단순화하되, 생성자/changePassword 메서드에 "이 메서드는 BCrypt 해시만 받음" 주석을 추가해야 한다.
-
테스트 계약 위반: 유닛 테스트에서
new UserModel(VALID_LOGIN_ID, "otherPw@123", ...)처럼 평문 비밀번호를 직접 전달하고 있다. 이는 설계 계약을 무시하는 것이므로, 테스트를 수정해 BCrypt 해시(예:VALID_ENCRYPTED_PASSWORD)로 변경해야 한다.
validatePassword() 제거와 테스트 수정을 진행하라.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java`
around lines 39 - 84, Remove the plaintext password validation from object
construction: delete or simplify validatePassword so the UserModel constructor
no longer rejects non-BCrypt-looking values (i.e., remove the
validatePassword(...) call in the UserModel(String loginId, String password,
...) constructor), retain a null-only check if you want to guard against nulls
in changePassword(String encryptedPassword) and/or constructor, and add a clear
comment on the UserModel constructor and changePassword method stating "this
method accepts a BCrypt hash only"; finally update unit tests that call new
UserModel(..., "otherPw@123", ...) to pass a BCrypt-hashed test constant (e.g.,
VALID_ENCRYPTED_PASSWORD) instead of a plaintext password so tests honor the
contract.
| @RequiredArgsConstructor | ||
| @Component | ||
| public class UserService { |
There was a problem hiding this comment.
@Transactional 어노테이션이 누락되었다.
signup과 changePassword 메서드에서 여러 데이터베이스 작업(조회 + 저장)을 수행하지만 트랜잭션 경계가 설정되지 않았다. 이로 인해 다음 문제가 발생할 수 있다:
- Race Condition: 동시 회원가입 요청 시 중복 체크와 저장 사이에 다른 요청이 끼어들어 중복 loginId가 생성될 수 있다
- 데이터 불일치: changePassword에서 조회 후 저장 전 예외 발생 시 일관성 문제
🔒️ 수정 제안
+import org.springframework.transaction.annotation.Transactional;
+
`@RequiredArgsConstructor`
`@Component`
public class UserService {
private final UserRepository userRepository;
private final PasswordEncoder passwordEncoder;
// 회원가입
+ `@Transactional`
public UserModel signup(String loginId, String password, String name, String birthday, String email) {
// ...
}
// 인증
+ `@Transactional`(readOnly = true)
public UserModel authenticate(String loginId, String password) {
// ...
}
// 비밀번호 변경
+ `@Transactional`
public void changePassword(String loginId, String currentPassword, String newPassword) {
// ...
}
// 로그인 ID로 조회
+ `@Transactional`(readOnly = true)
public UserModel findByLoginId(String loginId) {
// ...
}
}추가로 loginId 컬럼에 DB 레벨 유니크 제약을 설정하면 race condition에 대한 최종 방어선이 된다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 11 - 13, The UserService class is missing transactional boundaries
causing race conditions and inconsistency for signup and changePassword; add
`@Transactional` to the UserService class or annotate the specific methods
signup(...) and changePassword(...) so the read+write sequences run in a single
transaction (ensure appropriate propagation/isolation if needed), and also
ensure the loginId column has a DB-level UNIQUE constraint as a final defense
against concurrent duplicate inserts.
| // 인증 | ||
| public UserModel authenticate(String loginId, String password) { | ||
| UserModel user = userRepository.findByLoginId(loginId) | ||
| .orElseThrow(() -> new CoreException(ErrorType.UNAUTHORIZED, "회원 정보가 올바르지 않습니다.")); | ||
|
|
||
| if (!passwordEncoder.matches(password, user.getPassword())) { | ||
| throw new CoreException(ErrorType.UNAUTHORIZED, "회원 정보가 올바르지 않습니다."); | ||
| } | ||
|
|
||
| return user; | ||
| } |
There was a problem hiding this comment.
타이밍 공격에 취약할 수 있다.
사용자가 존재하지 않으면 즉시 예외가 발생하고, 존재하면 BCrypt 비교(약 100ms)가 수행된다. 이 응답 시간 차이로 공격자가 유효한 loginId를 추측할 수 있다.
완화 방법: 사용자가 존재하지 않아도 더미 해시와 비교하여 일정한 응답 시간을 유지한다.
🛡️ 수정 제안
+ // 타이밍 공격 방지용 더미 해시
+ private static final String DUMMY_HASH = "$2a$10$dummyhashfortimingattak";
+
// 인증
public UserModel authenticate(String loginId, String password) {
- UserModel user = userRepository.findByLoginId(loginId)
- .orElseThrow(() -> new CoreException(ErrorType.UNAUTHORIZED, "회원 정보가 올바르지 않습니다."));
+ Optional<UserModel> userOpt = userRepository.findByLoginId(loginId);
+
+ // 사용자 존재 여부와 무관하게 항상 BCrypt 비교 수행
+ String hashToCompare = userOpt.map(UserModel::getPassword).orElse(DUMMY_HASH);
+ boolean matches = passwordEncoder.matches(password, hashToCompare);
- if (!passwordEncoder.matches(password, user.getPassword())) {
+ if (userOpt.isEmpty() || !matches) {
throw new CoreException(ErrorType.UNAUTHORIZED, "회원 정보가 올바르지 않습니다.");
}
- return user;
+ return userOpt.get();
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java`
around lines 36 - 46, The authenticate method in UserService leaks timing via
immediate exception when user not found; to fix, always perform a BCrypt
comparison regardless of user existence by obtaining a constant dummy hash
(e.g., a hard-coded valid BCrypt hash or one generated once at startup) and
calling passwordEncoder.matches(password, actualUser != null ?
user.getPassword() : dummyHash) before throwing the same CoreException on
mismatch; keep the userRepository.findByLoginId lookup but delay throwing until
after the comparison so response timing is consistent and retain the existing
ErrorType.UNAUTHORIZED behavior.
| @DisplayName("비밀번호를 검증할 때, ") | ||
| @Nested | ||
| class Validate { | ||
|
|
||
| @DisplayName("정상적인 비밀번호면 예외가 발생하지 않는다.") | ||
| @Test | ||
| void doesNotThrowException_whenValid() { | ||
| // act & assert | ||
| assertDoesNotThrow(() -> Password.validate(VALID_PASSWORD, VALID_BIRTHDAY)); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호가 비어있으면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenPasswordIsBlank() { | ||
| // act | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| Password.validate("", VALID_BIRTHDAY); | ||
| }); | ||
|
|
||
| // assert | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호가 8자 미만이면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenPasswordIsShorterThan8() { | ||
| // act | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| Password.validate("qwer@12", VALID_BIRTHDAY); | ||
| }); | ||
|
|
||
| // assert | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호가 16자 초과이면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenPasswordIsLongerThan16() { | ||
| // act | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| Password.validate("qwer@123412341234", VALID_BIRTHDAY); | ||
| }); | ||
|
|
||
| // assert | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호에 허용되지 않은 문자가 포함되면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenPasswordContainsInvalidCharacters() { | ||
| // act | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| Password.validate("qwer@1234한글", VALID_BIRTHDAY); | ||
| }); | ||
|
|
||
| // assert | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호에 생년월일(대시 포함)이 포함되면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenPasswordContainsBirthdayWithDash() { | ||
| // act | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| Password.validate("qwer@1994-05-25", VALID_BIRTHDAY); | ||
| }); | ||
|
|
||
| // assert | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호에 생년월일(대시 미포함)이 포함되면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenPasswordContainsBirthdayWithoutDash() { | ||
| // act | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| Password.validate("qwer19940525", VALID_BIRTHDAY); | ||
| }); | ||
|
|
||
| // assert | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
| } |
There was a problem hiding this comment.
null birthday 입력에 대한 테스트가 누락되었다.
Password.validate()에서 birthday.replace("-", "")를 호출하므로 birthday가 null이면 NPE가 발생한다. 이 시나리오에 대한 테스트를 추가하거나, Password 클래스에서 null birthday를 명시적으로 처리해야 한다.
#!/bin/bash
# Password.validate에서 birthday null 처리 확인
rg -A 10 'public static void validate' --type java -g '**/Password.java'🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordTest.java`
around lines 18 - 100, Password.validate currently calls birthday.replace(...)
and can NPE when birthday is null; add a null check at the start of
Password.validate (the static validate method in Password) and throw the same
CoreException with ErrorType.BAD_REQUEST used by other invalid-password cases
when birthday is null, then add a unit test in PasswordTest (e.g.,
throwsException_whenBirthdayIsNull) that asserts Password.validate(null, null)
or Password.validate(VALID_PASSWORD, null) throws CoreException with
ErrorType.BAD_REQUEST so the behavior is covered.
| @DisplayName("생년월일이 현재보다 이후의 날짜면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsBadRequestException_whenBirthDateisAfterToday(){ | ||
| // act | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| new UserModel(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, "2027-01-01", VALID_EMAIL); | ||
| }); | ||
|
|
||
| // assert | ||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } |
There was a problem hiding this comment.
미래 날짜 테스트가 시간 의존적이다.
"2027-01-01" 하드코딩된 날짜는 2027년 이후 테스트 실패를 유발한다. LocalDate.now().plusYears(1).toString() 등 동적으로 계산된 미래 날짜를 사용하면 테스트 안정성이 향상된다.
🛠️ 수정 제안
+import java.time.LocalDate;
+
class UserModelTest {
+ private static final String FUTURE_BIRTHDAY = LocalDate.now().plusYears(1).toString();
// ...
`@DisplayName`("생년월일이 현재보다 이후의 날짜면, 예외가 발생한다.")
`@Test`
void throwsBadRequestException_whenBirthDateisAfterToday(){
// act
CoreException result = assertThrows(CoreException.class, () -> {
- new UserModel(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, "2027-01-01", VALID_EMAIL);
+ new UserModel(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, FUTURE_BIRTHDAY, VALID_EMAIL);
});
// assert
assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST);
}🤖 Prompt for AI Agents
In `@apps/commerce-api/src/test/java/com/loopers/domain/user/UserModelTest.java`
around lines 94 - 104, The test
throwsBadRequestException_whenBirthDateisAfterToday in UserModelTest uses a
hardcoded future date ("2027-01-01") which will become stale; change the test to
construct the future birth date dynamically (e.g., use
LocalDate.now().plusYears(1).toString() or equivalent) when calling new
UserModel(...) so the assertion on ErrorType.BAD_REQUEST remains
time-independent and stable across runs.
| @DisplayName("이미 가입된 로그인 ID로 가입하면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenLoginIdAlreadyExists() { | ||
| // arrange - 먼저 회원 하나 저장 | ||
| UserModel existingUser = new UserModel(VALID_LOGIN_ID, "otherPw@123", "기존회원", "1990-01-01", "other@test.com"); | ||
| userJpaRepository.save(existingUser); | ||
|
|
||
| // act & assert | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| userService.signup(VALID_LOGIN_ID, "newPw@1234", "신규회원", "1995-05-05", "new@test.com"); | ||
| }); | ||
|
|
||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); |
There was a problem hiding this comment.
중복 가입 오류 타입 불일치 수정 필요
Line 71에서 BAD_REQUEST를 기대하지만, 서비스는 중복 로그인 ID에 CONFLICT를 던지므로 테스트가 실패하고 운영에서 409/400 계약이 뒤바뀌어 클라이언트 재시도·알람 분류가 어긋날 수 있다. 수정안으로 기대값을 ErrorType.CONFLICT로 변경한다. 추가 테스트로 API/E2E에서 중복 가입 시 409로 매핑되는지 검증하는 케이스를 넣어 계약을 고정하는 것이 좋다.
수정안
- assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST);
+ assertThat(result.getErrorType()).isEqualTo(ErrorType.CONFLICT);📝 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.
| @DisplayName("이미 가입된 로그인 ID로 가입하면, 예외가 발생한다.") | |
| @Test | |
| void throwsException_whenLoginIdAlreadyExists() { | |
| // arrange - 먼저 회원 하나 저장 | |
| UserModel existingUser = new UserModel(VALID_LOGIN_ID, "otherPw@123", "기존회원", "1990-01-01", "other@test.com"); | |
| userJpaRepository.save(existingUser); | |
| // act & assert | |
| CoreException result = assertThrows(CoreException.class, () -> { | |
| userService.signup(VALID_LOGIN_ID, "newPw@1234", "신규회원", "1995-05-05", "new@test.com"); | |
| }); | |
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | |
| `@DisplayName`("이미 가입된 로그인 ID로 가입하면, 예외가 발생한다.") | |
| `@Test` | |
| void throwsException_whenLoginIdAlreadyExists() { | |
| // arrange - 먼저 회원 하나 저장 | |
| UserModel existingUser = new UserModel(VALID_LOGIN_ID, "otherPw@123", "기존회원", "1990-01-01", "other@test.com"); | |
| userJpaRepository.save(existingUser); | |
| // act & assert | |
| CoreException result = assertThrows(CoreException.class, () -> { | |
| userService.signup(VALID_LOGIN_ID, "newPw@1234", "신규회원", "1995-05-05", "new@test.com"); | |
| }); | |
| assertThat(result.getErrorType()).isEqualTo(ErrorType.CONFLICT); |
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java`
around lines 59 - 71, Update the test expectation in
UserServiceIntegrationTest::throwsException_whenLoginIdAlreadyExists to expect
ErrorType.CONFLICT instead of ErrorType.BAD_REQUEST; locate the assertion that
checks result.getErrorType() and change the expected value to
ErrorType.CONFLICT. Also add (or plan) an API/E2E test that asserts the HTTP
response for duplicate signup maps to 409 to lock the contract between service
and clients.
… existsByLoginId 메서드로 중복된 데이터가 존재하는지 여부만 확인
…만 하고 있으므로 혼란을 줄 수가 있음. 이해를 위한 주석 추가
…적으로 전달. httpEntity로 null을 전달하면 본문 및 헤더 둘 다 없다는 의미이기 때문
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@apps/commerce-api/src/main/java/com/loopers/config/AuthInterceptor.java`:
- Around line 38-39: AuthInterceptor currently logs the raw loginId on
authentication failure (see the catch block in AuthInterceptor where
log.warn("인증 실패 - loginId: {}, URI: {}", loginId, request.getRequestURI()) is
called); change this to log a non-PII-safe representation by masking or hashing
the loginId before logging (implement a helper like maskLoginId(String loginId)
or hashLoginId(String loginId) and use it in the catch block) so the log call
records only the masked/hashed value and not the plain identifier.
- Around line 36-41: The catch block in AuthInterceptor around
userService.authenticate currently logs the sensitive loginId; update the
log.warn call in the CoreException handler to remove the loginId and instead log
non-sensitive request details (use request.getRequestURI() and
request.getRemoteAddr()) so no user identifiers are written; keep the catch
throwing the exception after logging. Ensure the change is applied to the
log.warn line referenced in the CoreException catch within AuthInterceptor.
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java`:
- Around line 18-19: UserModel currently has a public no-arg constructor via
`@NoArgsConstructor` which allows creating empty entities; change it to a
protected no-arg constructor by updating the annotation on class UserModel to
use access = AccessLevel.PROTECTED (and add the lombok.AccessLevel import) so
JPA can instantiate while preventing accidental external new UserModel() calls;
update the annotation on the UserModel class and ensure imports/reference to
`@NoArgsConstructor` and AccessLevel are correct.
- Line 19: BaseEntity lacks equals/hashCode causing reference-equality issues
for all subclasses (e.g., UserModel); implement equals(Object) and hashCode() in
BaseEntity to use the identifier field (the `@Id` 'id'): equals should return true
when the other object is the same class (or compatible proxy-aware check) and
has a non-null matching id, and hashCode should derive from the id (with a
stable fallback for null id); update or add unit tests verifying that two entity
instances (including JPA proxy/mock) with the same id are considered equal and
behave correctly in Set/Map lookups.
- Around line 92-96: The DateTimeParseException cause is lost when creating
CoreException in UserModel (parseBirthday); add a constructor to CoreException
that accepts a Throwable cause (e.g., CoreException(ErrorType, String,
Throwable)) and then rethrow preserving the original exception by throwing new
CoreException(ErrorType.BAD_REQUEST, "생년월일 형식이 올바르지 않습니다.", e) inside the catch
block; ensure user-facing message stays the friendly string while the cause is
attached for logging/debugging.
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java`:
- Around line 47-52: The maskLastCharacter method currently returns
single-character names unchanged because of the check value.length() <= 1;
update maskLastCharacter to treat a one-character string as sensitive: keep the
null check, and if value.length() == 1 return "*" (or otherwise replace the
whole character with an asterisk), while preserving the existing behavior for
longer strings (return substring(0, value.length()-1) + "*"); this ensures
one-letter names (e.g., "김") are masked consistently for PII protection.
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java`:
- Around line 17-213: Add an integration test in UserServiceIntegrationTest that
simulates concurrent signups against UserService.signup using the same loginId
(e.g., run multiple tasks in an ExecutorService) to exercise the TOCTOU window
despite UserModel having `@Column`(unique = true); assert that one task succeeds
and the other(s) throw a CoreException (converted from the underlying
DataIntegrityViolationException) with the appropriate ErrorType (e.g.,
CONFLICT), or alternatively convert the test to run with SERIALIZABLE isolation;
reference UserService.signup, UserModel(loginId unique),
DataIntegrityViolationException and CoreException when implementing the
assertion and test orchestration.
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java`:
- Around line 83-108: The test's `@DisplayName` wrongly says "400 BAD_REQUEST"
while the assertion expects HttpStatus.CONFLICT (409); update the `@DisplayName`
on the throwsBadRequest_whenLoginIdAlreadyExists test to reflect 409 CONFLICT
(e.g., "이미 존재하는 로그인 ID로 가입하면, 409 CONFLICT 응답을 받는다.") so the annotation matches
the actual assertion and the behavior from UserService.signup which throws
ErrorType.CONFLICT; keep the test method name and assertions unchanged.
🧹 Nitpick comments (15)
apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java (1)
52-59:loginId길이 제한이 없다.현재 영문/숫자 패턴만 검증하고 길이 제한이 없어, 1자 또는 수백 자 loginId가 허용된다.
@Column기본 길이는 VARCHAR(255)이므로 DB 레벨에서는 제한이 있지만, 도메인 규칙으로 명시적 길이 범위(예: 4~20자)를 설정하는 것이 운영 안정성과 사용자 경험 측면에서 바람직하다.예시
private void validateLoginId(String loginId){ if(loginId == null || loginId.isBlank()){ throw new CoreException(ErrorType.BAD_REQUEST, "로그인 ID는 비어있을 수 없습니다."); } + if(loginId.length() < 4 || loginId.length() > 20){ + throw new CoreException(ErrorType.BAD_REQUEST, "로그인 ID는 4자 이상 20자 이하여야 합니다."); + } if(!loginId.matches(LOGIN_ID_PATTERN)){ throw new CoreException(ErrorType.BAD_REQUEST, "로그인 ID는 영문과 숫자만 허용됩니다."); } }apps/commerce-api/src/test/java/com/loopers/domain/user/PasswordTest.java (1)
18-100: 경계값 테스트가 누락되어 있다.현재 테스트는 7자(미만)와 17자(초과)만 검증하고 있다. 비밀번호 길이 검증의 경계값인 정확히 8자(유효 최소)와 정확히 16자(유효 최대)에 대한 테스트가 없다. 경계값 테스트는 off-by-one 오류를 방지하는 데 핵심적이다.
또한,
null비밀번호에 대한 테스트도 추가하면value == null분기가 검증된다. As per coding guidelines, 단위 테스트는 경계값/실패 케이스/예외 흐름을 포함하는지 점검한다.테스트 추가 예시
+ `@DisplayName`("비밀번호가 정확히 8자이면, 예외가 발생하지 않는다.") + `@Test` + void doesNotThrowException_whenPasswordIsExactly8Characters() { + assertDoesNotThrow(() -> Password.validate("qwer@123", VALID_BIRTHDAY)); + } + + `@DisplayName`("비밀번호가 정확히 16자이면, 예외가 발생하지 않는다.") + `@Test` + void doesNotThrowException_whenPasswordIsExactly16Characters() { + assertDoesNotThrow(() -> Password.validate("qwer@12345678901", VALID_BIRTHDAY)); + } + + `@DisplayName`("비밀번호가 null이면, 예외가 발생한다.") + `@Test` + void throwsException_whenPasswordIsNull() { + CoreException result = assertThrows(CoreException.class, () -> { + Password.validate(null, VALID_BIRTHDAY); + }); + assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); + }apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceTest.java (1)
157-179:changePassword성공 테스트에서 비밀번호 변경 결과 상태 검증이 누락되어 있다.현재
verify(userRepository, times(1)).save(any(UserModel.class))로 행위만 검증하고, 실제로 저장된UserModel의 비밀번호가NEW_ENCRYPTED_PASSWORD로 변경되었는지 상태 검증이 없다.save가 호출되더라도 잘못된 비밀번호가 설정된 채 저장될 수 있으므로,ArgumentCaptor로 저장된 객체의 비밀번호를 검증하면 테스트 신뢰도가 높아진다.상태 검증 추가 예시
+ ArgumentCaptor<UserModel> captor = ArgumentCaptor.forClass(UserModel.class); + // act userService.changePassword(VALID_LOGIN_ID, VALID_PASSWORD, NEW_PASSWORD); // assert - verify(userRepository, times(1)).save(any(UserModel.class)); + verify(userRepository, times(1)).save(captor.capture()); + assertThat(captor.getValue().getPassword()).isEqualTo(NEW_ENCRYPTED_PASSWORD);apps/commerce-api/src/main/java/com/loopers/domain/user/Password.java (1)
10-10:value.matches()는 호출 시마다 정규식을 컴파일한다.
String.matches()는 내부적으로 매번Pattern.compile()을 수행한다. 현재 회원가입/비밀번호 변경 시에만 호출되므로 성능 영향은 미미하지만,Pattern을 상수로 미리 컴파일하면 의도가 명확해지고 향후 대량 처리 시에도 안전하다.수정안
+import java.util.regex.Pattern; + public class Password { private static final int MIN_LENGTH = 8; private static final int MAX_LENGTH = 16; - private static final String PASSWORD_PATTERN = "^[a-zA-Z0-9!@#$%^&*()_+\\-=\\[\\]{};':\",./<>?]*$"; + private static final Pattern PASSWORD_PATTERN = Pattern.compile("^[a-zA-Z0-9!@#$%^&*()_+\\-=\\[\\]{};':\",./<>?]*$"); public static void validate(String value, String birthday) { // ... - if (!value.matches(PASSWORD_PATTERN)) { + if (!PASSWORD_PATTERN.matcher(value).matches()) { throw new CoreException(ErrorType.BAD_REQUEST, "비밀번호 형식이 올바르지 않습니다."); }apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java (2)
74-80:response.getBody()가 null일 경우 NPE가 발생할 수 있다.모든 테스트에서
response.getBody().meta(),response.getBody().data()패턴을 사용하고 있다. 서버 오류 등으로 body가 null로 반환되면 테스트가 NPE로 실패하여 실제 원인 파악이 어렵다.assertAll블록 내에서 body null 체크를 선행하거나, 블록 밖에서assertThat(response.getBody()).isNotNull()을 먼저 수행하는 것을 권장한다.
197-223: 회원가입 + 인증 헤더 설정 로직이 반복되고 있다.
ChangePassword내 4개 테스트와GetMe내 테스트들에서 회원가입 호출 및 헤더 설정이 동일하게 반복된다. 테스트 가독성과 유지보수를 위해 private helper 메서드로 추출하는 것을 고려할 수 있다.예시
private void signupValidUser() { UserV1Dto.SignupRequest request = new UserV1Dto.SignupRequest( VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL ); testRestTemplate.exchange(SIGNUP_ENDPOINT, HttpMethod.POST, new HttpEntity<>(request), new ParameterizedTypeReference<ApiResponse<UserV1Dto.UserResponse>>() {}); } private HttpHeaders createAuthHeaders(String loginId, String password) { HttpHeaders headers = new HttpHeaders(); headers.set(HEADER_LOGIN_ID, loginId); headers.set(HEADER_LOGIN_PW, password); headers.set("Content-Type", "application/json"); return headers; }apps/commerce-api/src/test/java/com/loopers/config/AuthInterceptorTest.java (2)
98-115: 빈 문자열 헤더 테스트에서getRequestURI()/getRemoteAddr()스터빙이 누락되었다.
AuthInterceptor.preHandle은 헤더가 누락/빈 문자열일 때request.getRequestURI()와request.getRemoteAddr()를 로그에 기록한다. Line 60-65의throwsException_whenHeadersMissing테스트에서는 이를 스터빙하고 있으나, 이 테스트에서는 누락되어 있다. 동일한 코드 경로를 타므로 일관성을 맞추는 것이 좋다. 현재는 Mockito 기본값(null)이 반환되어 테스트 자체는 통과하지만, 향후STRICT_STUBS모드 전환 시 문제가 될 수 있다.
36-116: 헤더가 부분적으로 누락된 경우의 경계값 테스트가 없다.현재 테스트는 두 헤더 모두 누락되거나, 두 헤더 모두 빈 문자열인 경우만 검증한다.
loginId는 유효하지만loginPw만 누락된 경우, 또는 그 반대의 경우에 대한 경계값 테스트를 추가하면 인터셉터의 조건 분기를 더 정밀하게 검증할 수 있다. 코딩 가이드라인에 따르면 단위 테스트는 경계값 케이스를 포함해야 한다.apps/commerce-api/src/main/java/com/loopers/config/WebMvcConfig.java (1)
14-19: 향후 공개 엔드포인트 추가 시excludePathPatterns관리에 주의가 필요하다.현재는
/api/v1/users/signup만 제외되어 있다. 향후 비인증 엔드포인트(예: 비밀번호 초기화, 헬스체크 등)가/api/**하위에 추가될 경우 인증 실패가 발생할 수 있으므로, 공개 경로 목록을 상수 배열로 관리하거나 주석으로 의도를 명시하는 것을 권장한다.apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java (1)
62-64: 테스트 데이터 준비 시UserModel을 JPA 리포지토리에 직접 저장하여 도메인 검증을 우회하고 있다.Line 63에서
new UserModel(...)로 생성 후userJpaRepository.save()를 호출한다. 이 경우 비밀번호 암호화 없이 평문이 저장되며,UserModel생성자의 검증 로직에만 의존한다. 중복 loginId 체크 테스트 목적으로는 문제없으나, 다른 테스트들은userService.signup()을 통해 데이터를 준비하고 있어 방식이 혼재되어 있다. 데이터 준비 방식을 통일하면 테스트 일관성이 높아진다.apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1ApiSpec.java (1)
1-27: Swagger 문서에 에러 응답 명세가 없다.
@Operation에summary/description만 있고@ApiResponse(4xx, 5xx) 정의가 없다. 클라이언트 개발자가 에러 응답 형식을 Swagger 문서만으로 파악할 수 없으므로, 주요 에러 케이스(401, 409, 400 등)에 대한 응답 명세를 추가하면 API 계약이 명확해진다.apps/commerce-api/src/main/java/com/loopers/domain/user/UserService.java (1)
12-14:@Component대신@Service가 의미적으로 더 적합하다.
UserService는 도메인 서비스 역할을 수행하므로@Service어노테이션이 Spring의 스테레오타입 계층 구조에 부합한다. 기능적 차이는 없으나, 코드 의도를 명확히 전달하고 AOP 기반 예외 변환 등 향후 확장 시 이점이 있다.수정안
-import org.springframework.stereotype.Component; +import org.springframework.stereotype.Service; `@RequiredArgsConstructor` -@Component +@Service public class UserService {apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Controller.java (2)
23-31: 회원가입 성공 시 HTTP 201(Created) 상태 코드를 반환해야 한다.현재
ApiResponse.success(response)는 기본적으로 HTTP 200을 반환한다. 리소스 생성 API(POST /signup)는 REST 관례상 201 Created를 반환하는 것이 표준이다. 클라이언트 및 모니터링 도구가 200과 201을 구분하여 동작할 수 있으므로, 상태 코드가 부정확하면 운영 시 혼란을 초래할 수 있다.
@ResponseStatus(HttpStatus.CREATED)또는ResponseEntity를 활용하여 201을 명시적으로 반환하도록 수정하는 것을 권장한다.♻️ 수정 제안
+import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; `@PostMapping`("/signup") + `@ResponseStatus`(HttpStatus.CREATED) `@Override` public ApiResponse<UserV1Dto.UserResponse> signup(
43-51:ApiResponse<Object>대신ApiResponse<Void>사용을 권장한다.
changePassword는 응답 본문이 없는 엔드포인트인데 반환 타입이ApiResponse<Object>로 선언되어 있다.ApiResponse<Void>가 의미적으로 정확하며, API 스펙(Swagger/OpenAPI 문서 생성 시) 에서도 응답 본문이 없음을 명확히 전달할 수 있다.apps/commerce-api/src/main/java/com/loopers/config/AuthInterceptor.java (1)
26-44:config패키지에서 도메인 계층(UserService)을 직접 참조하고 있다.현재
AuthInterceptor가com.loopers.domain.user.UserService를 직접 주입받고 있다. 헥사고널 아키텍처 관점에서, infrastructure/config 계층은 application 계층을 통해 도메인에 접근하는 것이 의존 방향 원칙에 부합한다.UserFacade에 인증 메서드를 추가하거나, 별도의 application-level 인증 포트를 두는 것을 고려할 수 있다.
| try{ | ||
| userService.authenticate(loginId, loginPw); | ||
| }catch(CoreException e){ | ||
| log.warn("인증 실패 - loginId: {}, URI: {}", loginId, request.getRequestURI()); | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "ApiControllerAdvice.java" -o -name "*ControllerAdvice*.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 170
🏁 Script executed:
find . -name "ErrorType.java" -o -name "*ErrorType*.java" | head -20Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 159
🏁 Script executed:
# Also search for AuthInterceptor to confirm its exact location and structure
find . -name "AuthInterceptor.java" -exec head -50 {} +Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 1821
🏁 Script executed:
cat ./apps/commerce-api/src/main/java/com/loopers/interfaces/api/ApiControllerAdvice.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 5938
🏁 Script executed:
cat ./apps/commerce-api/src/main/java/com/loopers/support/error/ErrorType.javaRepository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 917
🏁 Script executed:
# Verify Spring MVC interceptor exception handling in Spring Boot context
# Check if there are any other exception handlers that might affect interceptor exceptions
rg "ExceptionHandler|@ControllerAdvice|@RestControllerAdvice" --type java -A 2 | head -40Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 3854
🏁 Script executed:
# Check if there are any tests or documentation about interceptor exception handling
fd "test" --type f -name "*Test.java" -o -name "*Tests.java" | xargs grep -l "AuthInterceptor\|preHandle" | head -5Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 315
인증 헤더 누락 시 원격 주소 대신 loginId를 로그에 남기는 것은 민감정보 노출 위험이다.
라인 39에서 log.warn("인증 실패 - loginId: {}, URI: {}", loginId, request.getRequestURI());로 사용자 식별자(loginId)를 직접 로그에 기록한다. loginId는 사용자 개인정보이므로 운영 환경 로그에 남길 경우 보안 정책 위반 위험이 있다.
로그를 log.warn("인증 실패 - URI: {}, RemoteAddr: {}", request.getRequestURI(), request.getRemoteAddr());로 수정하여 사용자 식별 정보는 제외하고, 요청 발원 정보만 기록하도록 수정한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/config/AuthInterceptor.java`
around lines 36 - 41, The catch block in AuthInterceptor around
userService.authenticate currently logs the sensitive loginId; update the
log.warn call in the CoreException handler to remove the loginId and instead log
non-sensitive request details (use request.getRequestURI() and
request.getRemoteAddr()) so no user identifiers are written; keep the catch
throwing the exception after logging. Ensure the change is applied to the
log.warn line referenced in the CoreException catch within AuthInterceptor.
| }catch(CoreException e){ | ||
| log.warn("인증 실패 - loginId: {}, URI: {}", loginId, request.getRequestURI()); |
There was a problem hiding this comment.
로그에 loginId(사용자 식별자)가 기록되고 있다.
loginId는 사용자 식별 정보(PII)에 해당할 수 있다. 인증 실패 시 디버깅 목적으로 유용하지만, 로그 수집 시스템에 평문으로 적재되면 개인정보 보호 규정(GDPR/CCPA 등) 위반 소지가 있다. 운영 환경에서는 loginId를 마스킹하거나 해시 처리하여 기록하는 것을 권장한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/config/AuthInterceptor.java`
around lines 38 - 39, AuthInterceptor currently logs the raw loginId on
authentication failure (see the catch block in AuthInterceptor where
log.warn("인증 실패 - loginId: {}, URI: {}", loginId, request.getRequestURI()) is
called); change this to log a non-PII-safe representation by masking or hashing
the loginId before logging (implement a helper like maskLoginId(String loginId)
or hashLoginId(String loginId) and use it in the catch block) so the log call
records only the masked/hashed value and not the plain identifier.
| @NoArgsConstructor | ||
| public class UserModel extends BaseEntity { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
@NoArgsConstructor의 접근 수준을 PROTECTED로 제한해야 한다.
현재 @NoArgsConstructor가 기본값인 public 접근 수준으로 생성되어, 검증 로직을 우회한 빈 UserModel 인스턴스 생성이 가능하다. 운영 환경에서 다른 개발자가 실수로 new UserModel()을 호출하면 모든 필드가 null인 엔티티가 영속화될 수 있고, NOT NULL 제약조건 위반으로 런타임 예외가 발생한다. JPA는 protected 접근 수준만 요구하므로 접근 수준을 제한해야 한다.
수정안
+import lombok.AccessLevel;
-@NoArgsConstructor
+@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class UserModel extends BaseEntity {As per coding guidelines: **/domain/**/*.java — "엔티티/값 객체/DTO 경계를 명확히 하고, 불변성과 캡슐화를 점검한다."
📝 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.
| @NoArgsConstructor | |
| public class UserModel extends BaseEntity { | |
| import lombok.AccessLevel; | |
| `@NoArgsConstructor`(access = AccessLevel.PROTECTED) | |
| public class UserModel extends BaseEntity { |
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java`
around lines 18 - 19, UserModel currently has a public no-arg constructor via
`@NoArgsConstructor` which allows creating empty entities; change it to a
protected no-arg constructor by updating the annotation on class UserModel to
use access = AccessLevel.PROTECTED (and add the lombok.AccessLevel import) so
JPA can instantiate while preventing accidental external new UserModel() calls;
update the annotation on the UserModel class and ensure imports/reference to
`@NoArgsConstructor` and AccessLevel are correct.
| @Table(name = "users") | ||
| @Getter | ||
| @NoArgsConstructor | ||
| public class UserModel extends BaseEntity { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# BaseEntity 파일 검색 및 확인
echo "=== BaseEntity.java 검색 ==="
fd -t f "BaseEntity.java" 2>/dev/null
echo ""
echo "=== BaseEntity.java 내용 확인 ==="
fd -t f "BaseEntity.java" --exec cat -n {} \;Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 2410
BaseEntity에 equals/hashCode 구현이 누락되어 있다. 식별자 기반으로 즉시 추가해야 한다.
BaseEntity에 equals/hashCode가 정의되지 않아 모든 상속 엔티티(UserModel 포함)에서 Object의 기본 구현(참조 동일성)을 사용하게 된다. 이는 JPA 프록시 객체 생성 시 실제 엔티티와 다른 객체로 인식되어 Set/Map 컬렉션 조회, 지연 로딩 후 동일성 검사 등에서 예상치 못한 동작을 유발한다. 운영 중 데이터 무결성 문제와 성능 저하를 초래할 수 있다.
BaseEntity의 @Id 필드(id)를 기반으로 equals/hashCode를 구현하고, 데이터베이스 조회 후 같은 식별자를 가진 엔티티를 올바르게 식별하는지 확인하는 테스트를 추가해야 한다.
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java` at
line 19, BaseEntity lacks equals/hashCode causing reference-equality issues for
all subclasses (e.g., UserModel); implement equals(Object) and hashCode() in
BaseEntity to use the identifier field (the `@Id` 'id'): equals should return true
when the other object is the same class (or compatible proxy-aware check) and
has a non-null matching id, and hashCode should derive from the id (with a
stable fallback for null id); update or add unit tests verifying that two entity
instances (including JPA proxy/mock) with the same id are considered equal and
behave correctly in Set/Map lookups.
| try{ | ||
| parsed = LocalDate.parse(birthday); | ||
| }catch(DateTimeParseException e){ | ||
| throw new CoreException(ErrorType.BAD_REQUEST, "생년월일 형식이 올바르지 않습니다."); | ||
| } |
There was a problem hiding this comment.
DateTimeParseException의 원인(cause)이 유실된다.
CoreException 생성 시 원래 예외가 전달되지 않아, 로그에서 잘못된 입력값이나 파싱 실패의 구체적 원인을 확인할 수 없다. 운영 환경에서 "2024-13-01" 같은 입력이 들어왔을 때 디버깅이 어렵다. CoreException에 cause를 받는 생성자를 추가하고, 여기서 원본 예외를 전달해야 한다.
CoreException에 cause 지원 추가 후 적용 예시
CoreException에 생성자 추가:
public CoreException(ErrorType errorType, String customMessage, Throwable cause) {
super(customMessage != null ? customMessage : errorType.getMessage(), cause);
this.errorType = errorType;
this.customMessage = customMessage;
}parseBirthday 수정:
try{
parsed = LocalDate.parse(birthday);
- }catch(DateTimeParseException e){
- throw new CoreException(ErrorType.BAD_REQUEST, "생년월일 형식이 올바르지 않습니다.");
+ }catch(DateTimeParseException e){
+ throw new CoreException(ErrorType.BAD_REQUEST, "생년월일 형식이 올바르지 않습니다.", e);
}As per coding guidelines: **/*.java — "예외 처리 시 cause를 보존하고, 사용자 메시지와 로그 메시지를 분리하도록 제안한다."
🤖 Prompt for AI Agents
In `@apps/commerce-api/src/main/java/com/loopers/domain/user/UserModel.java`
around lines 92 - 96, The DateTimeParseException cause is lost when creating
CoreException in UserModel (parseBirthday); add a constructor to CoreException
that accepts a Throwable cause (e.g., CoreException(ErrorType, String,
Throwable)) and then rethrow preserving the original exception by throwing new
CoreException(ErrorType.BAD_REQUEST, "생년월일 형식이 올바르지 않습니다.", e) inside the catch
block; ensure user-facing message stays the friendly string while the cause is
attached for logging/debugging.
| private static String maskLastCharacter(String value) { | ||
| if (value == null || value.length() <= 1) { | ||
| return value; | ||
| } | ||
| return value.substring(0, value.length() - 1) + "*"; | ||
| } |
There was a problem hiding this comment.
단일 문자 이름에 대한 마스킹 동작을 확인해야 한다.
value.length() <= 1 조건에 의해, 한 글자 이름(예: "김")은 마스킹 없이 그대로 반환된다. 의도된 동작이라면 문제 없으나, PII 보호 관점에서 한 글자 이름도 "*"로 대체하는 것이 일관성 있다. 운영 환경에서 이름이 1자인 사용자가 존재할 경우, 마스킹이 적용되지 않아 개인정보 노출 가능성이 있다.
♻️ 수정 제안 (1자 이름도 마스킹)
private static String maskLastCharacter(String value) {
- if (value == null || value.length() <= 1) {
+ if (value == null || value.isEmpty()) {
return value;
}
+ if (value.length() == 1) {
+ return "*";
+ }
return value.substring(0, value.length() - 1) + "*";
}🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/main/java/com/loopers/interfaces/api/user/UserV1Dto.java`
around lines 47 - 52, The maskLastCharacter method currently returns
single-character names unchanged because of the check value.length() <= 1;
update maskLastCharacter to treat a one-character string as sensitive: keep the
null check, and if value.length() == 1 return "*" (or otherwise replace the
whole character with an asterisk), while preserving the existing behavior for
longer strings (return substring(0, value.length()-1) + "*"); this ensures
one-letter names (e.g., "김") are masked consistently for PII protection.
| @SpringBootTest | ||
| class UserServiceIntegrationTest { | ||
|
|
||
| private static final String VALID_LOGIN_ID = "namjin123"; | ||
| private static final String VALID_PASSWORD = "qwer@1234"; | ||
| private static final String VALID_NAME = "namjin"; | ||
| private static final String VALID_BIRTHDAY = "1994-05-25"; | ||
| private static final String VALID_EMAIL = "test@gmail.com"; | ||
|
|
||
| @Autowired | ||
| private UserService userService; | ||
|
|
||
| @Autowired | ||
| private UserJpaRepository userJpaRepository; | ||
|
|
||
| @Autowired | ||
| private DatabaseCleanUp databaseCleanUp; | ||
|
|
||
| @AfterEach | ||
| void tearDown() { | ||
| databaseCleanUp.truncateAllTables(); | ||
| } | ||
|
|
||
| @DisplayName("회원가입을 할 때, ") | ||
| @Nested | ||
| class Signup { | ||
|
|
||
| @DisplayName("정상적인 정보로 회원가입이 성공한다.") | ||
| @Test | ||
| void signupSucceeds_whenInfoIsValid() { | ||
| // act | ||
| UserModel result = userService.signup(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL); | ||
|
|
||
| // assert | ||
| assertThat(result).isNotNull(); | ||
| assertThat(result.getId()).isNotNull(); | ||
| assertThat(result.getLoginId()).isEqualTo(VALID_LOGIN_ID); | ||
|
|
||
| // DB에 실제로 저장됐는지 확인 | ||
| assertThat(userJpaRepository.findById(result.getId())).isPresent(); | ||
| } | ||
|
|
||
| @DisplayName("이미 가입된 로그인 ID로 가입하면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenLoginIdAlreadyExists() { | ||
| // arrange - 먼저 회원 하나 저장 | ||
| UserModel existingUser = new UserModel(VALID_LOGIN_ID, "otherPw@123", "기존회원", "1990-01-01", "other@test.com"); | ||
| userJpaRepository.save(existingUser); | ||
|
|
||
| // act & assert | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| userService.signup(VALID_LOGIN_ID, "newPw@1234", "신규회원", "1995-05-05", "new@test.com"); | ||
| }); | ||
|
|
||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.CONFLICT); | ||
| } | ||
| } | ||
|
|
||
| @DisplayName("인증을 할 때, ") | ||
| @Nested | ||
| class Authenticate { | ||
|
|
||
| @DisplayName("올바른 자격증명으로 인증이 성공한다.") | ||
| @Test | ||
| void authenticateSucceeds_whenCredentialsAreValid() { | ||
| // arrange - 회원가입 (BCrypt 암호화 포함) | ||
| userService.signup(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL); | ||
|
|
||
| // act | ||
| UserModel result = userService.authenticate(VALID_LOGIN_ID, VALID_PASSWORD); | ||
|
|
||
| // assert | ||
| assertThat(result).isNotNull(); | ||
| assertThat(result.getLoginId()).isEqualTo(VALID_LOGIN_ID); | ||
| } | ||
|
|
||
| @DisplayName("존재하지 않는 로그인 ID로 인증하면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenLoginIdNotFound() { | ||
| // act & assert | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| userService.authenticate("nonexistent", VALID_PASSWORD); | ||
| }); | ||
|
|
||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.UNAUTHORIZED); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호가 일치하지 않으면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenPasswordDoesNotMatch() { | ||
| // arrange - 회원가입 | ||
| userService.signup(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL); | ||
|
|
||
| // act & assert | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| userService.authenticate(VALID_LOGIN_ID, "wrongPw@123"); | ||
| }); | ||
|
|
||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.UNAUTHORIZED); | ||
| } | ||
| } | ||
|
|
||
| @DisplayName("비밀번호를 변경할 때, ") | ||
| @Nested | ||
| class ChangePassword { | ||
|
|
||
| private static final String NEW_PASSWORD = "newPw@1234"; | ||
|
|
||
| @DisplayName("정상적인 정보로 비밀번호 변경이 성공한다.") | ||
| @Test | ||
| void changePasswordSucceeds_whenInfoIsValid() { | ||
| // arrange - 회원가입 | ||
| userService.signup(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL); | ||
|
|
||
| // act | ||
| userService.changePassword(VALID_LOGIN_ID, VALID_PASSWORD, NEW_PASSWORD); | ||
|
|
||
| // assert - 새 비밀번호로 인증 성공 | ||
| UserModel result = userService.authenticate(VALID_LOGIN_ID, NEW_PASSWORD); | ||
| assertThat(result).isNotNull(); | ||
| assertThat(result.getLoginId()).isEqualTo(VALID_LOGIN_ID); | ||
| } | ||
|
|
||
| @DisplayName("비밀번호 변경 후 기존 비밀번호로 인증하면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenAuthenticatingWithOldPassword() { | ||
| // arrange - 회원가입 및 비밀번호 변경 | ||
| userService.signup(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL); | ||
| userService.changePassword(VALID_LOGIN_ID, VALID_PASSWORD, NEW_PASSWORD); | ||
|
|
||
| // act & assert - 기존 비밀번호로 인증 실패 | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| userService.authenticate(VALID_LOGIN_ID, VALID_PASSWORD); | ||
| }); | ||
|
|
||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.UNAUTHORIZED); | ||
| } | ||
|
|
||
| @DisplayName("현재 비밀번호가 일치하지 않으면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenCurrentPasswordDoesNotMatch() { | ||
| // arrange - 회원가입 | ||
| userService.signup(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL); | ||
|
|
||
| // act & assert | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| userService.changePassword(VALID_LOGIN_ID, "wrongPw@123", NEW_PASSWORD); | ||
| }); | ||
|
|
||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
|
|
||
| @DisplayName("새 비밀번호가 현재 비밀번호와 동일하면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenNewPasswordIsSameAsCurrent() { | ||
| // arrange - 회원가입 | ||
| userService.signup(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL); | ||
|
|
||
| // act & assert | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| userService.changePassword(VALID_LOGIN_ID, VALID_PASSWORD, VALID_PASSWORD); | ||
| }); | ||
|
|
||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.BAD_REQUEST); | ||
| } | ||
| } | ||
|
|
||
| @DisplayName("로그인 ID로 조회할 때, ") | ||
| @Nested | ||
| class FindByLoginId { | ||
|
|
||
| @DisplayName("존재하는 로그인 ID로 조회하면, 사용자를 반환한다.") | ||
| @Test | ||
| void returnsUser_whenLoginIdExists() { | ||
| // arrange - 회원가입 | ||
| userService.signup(VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL); | ||
|
|
||
| // act | ||
| UserModel result = userService.findByLoginId(VALID_LOGIN_ID); | ||
|
|
||
| // assert | ||
| assertThat(result).isNotNull(); | ||
| assertThat(result.getLoginId()).isEqualTo(VALID_LOGIN_ID); | ||
| } | ||
|
|
||
| @DisplayName("존재하지 않는 로그인 ID로 조회하면, 예외가 발생한다.") | ||
| @Test | ||
| void throwsException_whenLoginIdNotFound() { | ||
| // act & assert | ||
| CoreException result = assertThrows(CoreException.class, () -> { | ||
| userService.findByLoginId("nonexistent"); | ||
| }); | ||
|
|
||
| assertThat(result.getErrorType()).isEqualTo(ErrorType.NOT_FOUND); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. UserModel 엔티티에서 unique 제약 조건 확인
echo "=== UserModel Entity ==="
find . -name "UserModel.java" -type f | head -5 | xargs -I {} cat -n {}
echo -e "\n=== UserService.signup 메서드 ==="
find . -name "UserService.java" -path "*/user/*" -type f | head -5 | xargs -I {} grep -A 20 "signup" {} | head -40Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 4836
🏁 Script executed:
#!/bin/bash
# UserService 클래스에서 signup 메서드의 전체 선언부 확인
find . -name "UserService.java" -path "*/user/*" -type f | xargs -I {} grep -B 5 "public.*signup" {}Repository: Loopers-dev-lab/loop-pack-be-l2-vol3-java
Length of output: 323
동시 가입 시나리오에 대한 통합 테스트가 부재하고, 중복 요청 시 예외 처리 검증이 필요하다.
UserService.signup은 existsByLoginId 체크 후 save를 수행하므로 동시 요청 시 TOCTOU 윈도우가 존재한다. 다행히 DB 레벨의 unique 제약 조건이 이미 UserModel의 loginId 컬럼에 적용(@Column(nullable = false, unique = true))되어 있어 데이터 중복은 방지되지만, 두 번째 요청은 DB 제약 위반으로 인해 예외를 발생시킨다. 현재 테스트는 이 시나리오를 검증하지 않으므로, 동시 요청 시 DB unique 제약 위반 예외(DataIntegrityViolationException)가 CoreException으로 적절히 변환되는지 확인하는 테스트를 추가하거나, 격리 수준을 SERIALIZABLE로 명시하여 TOCTOU를 원천 차단하는 방안을 검토하기를 권장한다.
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/domain/user/UserServiceIntegrationTest.java`
around lines 17 - 213, Add an integration test in UserServiceIntegrationTest
that simulates concurrent signups against UserService.signup using the same
loginId (e.g., run multiple tasks in an ExecutorService) to exercise the TOCTOU
window despite UserModel having `@Column`(unique = true); assert that one task
succeeds and the other(s) throw a CoreException (converted from the underlying
DataIntegrityViolationException) with the appropriate ErrorType (e.g.,
CONFLICT), or alternatively convert the test to run with SERIALIZABLE isolation;
reference UserService.signup, UserModel(loginId unique),
DataIntegrityViolationException and CoreException when implementing the
assertion and test orchestration.
| @DisplayName("이미 존재하는 로그인 ID로 가입하면, 400 BAD_REQUEST 응답을 받는다.") | ||
| @Test | ||
| void throwsBadRequest_whenLoginIdAlreadyExists() { | ||
| // arrange - 먼저 회원가입 | ||
| UserV1Dto.SignupRequest firstRequest = new UserV1Dto.SignupRequest( | ||
| VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL | ||
| ); | ||
| testRestTemplate.exchange(SIGNUP_ENDPOINT, HttpMethod.POST, new HttpEntity<>(firstRequest), | ||
| new ParameterizedTypeReference<ApiResponse<UserV1Dto.UserResponse>>() {}); | ||
|
|
||
| // 같은 loginId로 다시 가입 시도 | ||
| UserV1Dto.SignupRequest duplicateRequest = new UserV1Dto.SignupRequest( | ||
| VALID_LOGIN_ID, "other@1234", "다른사람", "1990-01-01", "other@gmail.com" | ||
| ); | ||
|
|
||
| // act | ||
| ParameterizedTypeReference<ApiResponse<UserV1Dto.UserResponse>> responseType = new ParameterizedTypeReference<>() {}; | ||
| ResponseEntity<ApiResponse<UserV1Dto.UserResponse>> response = | ||
| testRestTemplate.exchange(SIGNUP_ENDPOINT, HttpMethod.POST, new HttpEntity<>(duplicateRequest), responseType); | ||
|
|
||
| // assert | ||
| assertAll( | ||
| () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT), | ||
| () -> assertThat(response.getBody().meta().result()).isEqualTo(ApiResponse.Metadata.Result.FAIL) | ||
| ); | ||
| } |
There was a problem hiding this comment.
@DisplayName과 실제 검증 상태 코드가 불일치한다.
Line 83의 DisplayName에는 "400 BAD_REQUEST"라고 기술되어 있으나, Line 105의 assertion은 HttpStatus.CONFLICT(409)를 기대한다. UserService.signup에서 중복 loginId 시 ErrorType.CONFLICT를 던지므로 assertion이 정확하고, DisplayName이 잘못된 것이다. 테스트 리포트에서 혼동을 유발할 수 있으므로 수정이 필요하다.
수정안
- `@DisplayName`("이미 존재하는 로그인 ID로 가입하면, 400 BAD_REQUEST 응답을 받는다.")
+ `@DisplayName`("이미 존재하는 로그인 ID로 가입하면, 409 CONFLICT 응답을 받는다.")📝 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.
| @DisplayName("이미 존재하는 로그인 ID로 가입하면, 400 BAD_REQUEST 응답을 받는다.") | |
| @Test | |
| void throwsBadRequest_whenLoginIdAlreadyExists() { | |
| // arrange - 먼저 회원가입 | |
| UserV1Dto.SignupRequest firstRequest = new UserV1Dto.SignupRequest( | |
| VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL | |
| ); | |
| testRestTemplate.exchange(SIGNUP_ENDPOINT, HttpMethod.POST, new HttpEntity<>(firstRequest), | |
| new ParameterizedTypeReference<ApiResponse<UserV1Dto.UserResponse>>() {}); | |
| // 같은 loginId로 다시 가입 시도 | |
| UserV1Dto.SignupRequest duplicateRequest = new UserV1Dto.SignupRequest( | |
| VALID_LOGIN_ID, "other@1234", "다른사람", "1990-01-01", "other@gmail.com" | |
| ); | |
| // act | |
| ParameterizedTypeReference<ApiResponse<UserV1Dto.UserResponse>> responseType = new ParameterizedTypeReference<>() {}; | |
| ResponseEntity<ApiResponse<UserV1Dto.UserResponse>> response = | |
| testRestTemplate.exchange(SIGNUP_ENDPOINT, HttpMethod.POST, new HttpEntity<>(duplicateRequest), responseType); | |
| // assert | |
| assertAll( | |
| () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT), | |
| () -> assertThat(response.getBody().meta().result()).isEqualTo(ApiResponse.Metadata.Result.FAIL) | |
| ); | |
| } | |
| `@DisplayName`("이미 존재하는 로그인 ID로 가입하면, 409 CONFLICT 응답을 받는다.") | |
| `@Test` | |
| void throwsBadRequest_whenLoginIdAlreadyExists() { | |
| // arrange - 먼저 회원가입 | |
| UserV1Dto.SignupRequest firstRequest = new UserV1Dto.SignupRequest( | |
| VALID_LOGIN_ID, VALID_PASSWORD, VALID_NAME, VALID_BIRTHDAY, VALID_EMAIL | |
| ); | |
| testRestTemplate.exchange(SIGNUP_ENDPOINT, HttpMethod.POST, new HttpEntity<>(firstRequest), | |
| new ParameterizedTypeReference<ApiResponse<UserV1Dto.UserResponse>>() {}); | |
| // 같은 loginId로 다시 가입 시도 | |
| UserV1Dto.SignupRequest duplicateRequest = new UserV1Dto.SignupRequest( | |
| VALID_LOGIN_ID, "other@1234", "다른사람", "1990-01-01", "other@gmail.com" | |
| ); | |
| // act | |
| ParameterizedTypeReference<ApiResponse<UserV1Dto.UserResponse>> responseType = new ParameterizedTypeReference<>() {}; | |
| ResponseEntity<ApiResponse<UserV1Dto.UserResponse>> response = | |
| testRestTemplate.exchange(SIGNUP_ENDPOINT, HttpMethod.POST, new HttpEntity<>(duplicateRequest), responseType); | |
| // assert | |
| assertAll( | |
| () -> assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CONFLICT), | |
| () -> assertThat(response.getBody().meta().result()).isEqualTo(ApiResponse.Metadata.Result.FAIL) | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
`@apps/commerce-api/src/test/java/com/loopers/interfaces/api/UserV1ApiE2ETest.java`
around lines 83 - 108, The test's `@DisplayName` wrongly says "400 BAD_REQUEST"
while the assertion expects HttpStatus.CONFLICT (409); update the `@DisplayName`
on the throwsBadRequest_whenLoginIdAlreadyExists test to reflect 409 CONFLICT
(e.g., "이미 존재하는 로그인 ID로 가입하면, 409 CONFLICT 응답을 받는다.") so the annotation matches
the actual assertion and the behavior from UserService.signup which throws
ErrorType.CONFLICT; keep the test method name and assertions unchanged.
2 participants
리뷰포인트
Password는 검증 규칙이 복잡해서 별도 클래스로 분리했는데, loginId, email 등은 UserModel 생성자에 남겨뒀습니다. "규칙이 복잡하면 분리한다"는 기준이 적절한지, 아니면 다른 기준이 있는지 궁금합니다.
Command 객체를 application 레이어로 옮기면서, UserService는 개별 파라미터를 받도록 변경했습니다. 도메인 레이어의 외부 의존성을 제거하기 위한 선택이었는데, 파라미터가 많아질 경우에도 이 방식이 적절한지 궁금합니다.
충분히 고민하지 못하고 도메인 엔티티에서만 데이터를 검증하고 있는데, 실무에서는 어떤 기준으로 검증 레이어를 나누시는지 궁금합니다.
현재는 Facade가 Service를 한 번만 호출하는 구조라 차이가 없지만, 유스케이스가 복잡해질 경우 트랜잭션 경계를 어디에 두는 것이 좋은지 궁금합니다.
📌 Summary
interfaces → application → domain ← infrastructure)이 지켜지고, 도메인 레이어는 외부 레이어의 타입에 의존하지 않는다.🧭 Context & Decision
결정 1: 도메인 엔티티의 검증 책임 — 엔티티 자체 검증 vs Value Object 분리
UserModel생성자에서 loginId, name, email, birthday 등의 필드를 직접 검증하고 있다.Password는 별도 클래스로 분리되어 있다.birthday와의 교차 검증이 필요한 경우 엔티티 생성자 시그니처가 복잡해진다.Password클래스로 분리하면 독립적으로 단위 테스트 가능하고, 엔티티의 책임이 줄어든다.Password클래스로 분리하고, loginId·name·email 등 단순 포맷 검증은 엔티티 내부에 유지했다.Password.validate()를 독립적으로 테스트할 수 있어 테스트 작성이 훨씬 수월해졌다. 특히 비밀번호는 길이, 패턴, 생년월일 포함 여부 등 검증 규칙이 7개나 되므로 분리 효과가 크다.결정 2: Command 객체의 위치와 필요성 — domain vs application / Command vs 개별 파라미터
SignupCommand,ChangePasswordCommand가 처음에는 domain 패키지에 있었다.domain이application개념을 알게됨)이 역전된다. 또한 "Command 없이 그냥 파라미터로 넘기면 안 되나?"라는 근본적 의문이 있었다.UserService는 개별 파라미터를 받도록 리팩토링했다.UserFacade에서command.loginId(), command.password(), ...로 분해하는 보일러플레이트가 생기지만, domain 레이어가 어떤 외부 타입에도 의존하지 않게 되었다.결정 3: Interfaces(Presentation) 레이어의 검증 책임 — 이중 검증 vs 도메인 검증만 신뢰
UserV1Controller에서는 별도의 입력 검증(@Valid,@NotBlank등)을 하지 않고, 도메인 레이어(UserModel생성자,Password.validate())에서 모든 검증을 수행한다.@Valid로 형식 검증, 도메인에서 비즈니스 검증: 관심사 분리가 명확하지만 검증 규칙이 두 곳에 분산되어 동기화 부담이 생겼다.UserModel과Password가 자기 보호(Self-Validation) 패턴을 충실히 구현하고 있기 때문이다.@Valid검증을 추가하는 것을 고려할 수 있다.🏗️ Design Overview
변경 범위
apps/commerce-api— user 도메인 전체 (신규)UserModel,Password,UserService,UserRepositoryUserFacade,UserInfo,SignupCommand,ChangePasswordCommandUserRepositoryImpl,UserJpaRepositoryUserV1Controller,UserV1ApiSpec,UserV1DtoAuthInterceptor,SecurityConfig,WebMvcConfigUserModelTest,PasswordTest,UserServiceTest,UserServiceIntegrationTest,AuthInterceptorTest,UserV1ApiE2ETest주요 컴포넌트 책임
UserModel: 회원 엔티티. 생성 시 loginId/name/email/birthday 자체 검증 (Self-Validating Entity)Password: 비밀번호 검증 VO. 길이, 패턴, 생년월일 포함 여부 등 비밀번호 규칙 검증 전담UserService: 도메인 서비스. 회원가입(중복 체크 + 암호화 + 저장), 인증, 비밀번호 변경, 조회 비즈니스 로직UserFacade: 유스케이스 조율. Command 분해 → Service 호출 → Info 변환AuthInterceptor: 인증 인터셉터.X-Loopers-LoginId/Pw헤더 기반 인증 처리UserV1Controller: API 엔드포인트. DTO ↔ Command 변환, 응답 포맷팅UserV1Dto: Request/Response DTO.toCommand()팩토리 메서드로 Command 변환 캡슐화🔁 Flow Diagram
회원가입 Flow
sequenceDiagram autonumber participant Client participant Controller as UserV1Controller participant Facade as UserFacade participant Service as UserService participant Password as Password (VO) participant Model as UserModel participant Repo as UserRepository Client->>Controller: POST /api/v1/users/signup (SignupRequest) Controller->>Controller: request.toCommand() Controller->>Facade: signUp(SignupCommand) Facade->>Service: signup(loginId, password, name, birthday, email) Service->>Password: validate(password, birthday) Password-->>Service: OK / CoreException Service->>Repo: findByLoginId(loginId) Repo-->>Service: Optional<UserModel> alt 이미 존재하는 loginId Service-->>Facade: CoreException(CONFLICT) end Service->>Service: passwordEncoder.encode(password) Service->>Model: new UserModel(loginId, encryptedPw, name, birthday, email) Note over Model: 자체 검증 (loginId, name, email, birthday) Service->>Repo: save(newUser) Repo-->>Service: UserModel Service-->>Facade: UserModel Facade-->>Controller: UserInfo Controller-->>Client: ApiResponse<UserResponse>인증 + 내 정보 조회 Flow
sequenceDiagram autonumber participant Client participant Interceptor as AuthInterceptor participant Controller as UserV1Controller participant Facade as UserFacade participant Service as UserService Client->>Interceptor: GET /api/v1/users/me (+ X-Loopers-LoginId/Pw 헤더) Interceptor->>Service: authenticate(loginId, password) alt 인증 실패 Service-->>Interceptor: CoreException(UNAUTHORIZED) Interceptor-->>Client: 401 Unauthorized end Service-->>Interceptor: UserModel Interceptor->>Interceptor: request.setAttribute("loginId", loginId) Interceptor->>Controller: pass Controller->>Facade: getMyInfo(loginId) Facade->>Service: findByLoginId(loginId) Service-->>Facade: UserModel Facade-->>Controller: UserInfo Controller-->>Client: ApiResponse<UserResponse>비밀번호 변경 Flow
sequenceDiagram autonumber participant Client participant Interceptor as AuthInterceptor participant Controller as UserV1Controller participant Facade as UserFacade participant Service as UserService participant Password as Password (VO) Client->>Interceptor: PATCH /api/v1/users/password (+ 인증 헤더 + ChangePasswordRequest) Interceptor->>Service: authenticate(loginId, password) Service-->>Interceptor: OK Interceptor->>Controller: pass (loginId in attribute) Controller->>Controller: request.toCommand(loginId) Controller->>Facade: changePassword(ChangePasswordCommand) Facade->>Service: changePassword(loginId, currentPw, newPw) Service->>Service: 사용자 조회 + 현재 비밀번호 확인 alt 현재 비밀번호 불일치 Service-->>Facade: CoreException(BAD_REQUEST) end alt 새 비밀번호 == 현재 비밀번호 Service-->>Facade: CoreException(BAD_REQUEST) end Service->>Password: validate(newPassword, birthday) Password-->>Service: OK / CoreException Service->>Service: passwordEncoder.encode(newPw) Service->>Service: user.changePassword(encryptedPw) Service->>Service: userRepository.save(user) Service-->>Facade: void Facade-->>Controller: void Controller-->>Client: ApiResponse (SUCCESS)🧪 테스트 전략
TDD로 안에서 바깥(탑다운)으로(도메인 → 서비스 → API) 순서로 작성했다.
UserModelTestPasswordTestUserServiceTestAuthInterceptorTestUserServiceIntegrationTestUserV1ApiE2ETest📝 회고
TDD와 레이어드 아키텍처를 처음 적용하면서 어떤 것에 내가 집중하고 어떤 것을 AI에게 맡겨서 효율성을 높여야 하는지 감잡기가 어려웠다. 처음부터 완벽한 설계를 하고 싶은 생각이 무의식적으로 있었는지, 첫 회원가입 테스트 코드를 작성하는데도 꽤 많은 시간이 걸렸다. 아쉬운 부분은 처음부터 내 설계가 아닌 AI가 해준 설계에 내가 점점 내 의도를 붙여나갔다는 점이다. 다음부터는 내가 먼저 설계하고 그것을 기반으로 AI와 함께 수정 및 구현을 해나가는 방식으로 해야겠다.
코드래빗
변경 목적
TDD 기반의 계층화된 아키텍처로 사용자 도메인을 구현하여 회원가입, 로그인 인증, 내 정보 조회, 비밀번호 변경 기능을 제공합니다.
핵심 변경점
도메인층에 UserModel, Password(VO), UserService, UserRepository를 신규 작성했고, 애플리케이션층에 UserFacade와 SignupCommand/ChangePasswordCommand를 추가했으며, 인프라층에 UserRepositoryImpl과 UserJpaRepository를 구현했습니다. 인터페이스층은 UserV1Controller와 관련 DTO/ApiSpec으로, 설정층은 AuthInterceptor(인증 헤더 검증), SecurityConfig(BCryptPasswordEncoder), WebMvcConfig(인터셉터 등록)로 구성됩니다. 또한 ErrorType에 UNAUTHORIZED 상수를 추가했습니다.
리스크/주의사항
@transactional이 도메인 서비스 메서드(UserService)에 적용되었는데, 일반적으로 애플리케이션 계층(UserFacade)에 적용하는 것이 권장되는 패턴입니다. 컨트롤러에서 @Valid 검증을 수행하지 않고 도메인층이 자체 검증하는 방식이 명확한지 확인이 필요하며, Password 검증이 VO로 분리된 반면 다른 필드는 UserModel 내부에서 검증되는 일관성도 검토해야 합니다.
테스트/검증 방법
단위 테스트(UserModelTest, PasswordTest, UserServiceTest, AuthInterceptorTest), 통합 테스트(UserServiceIntegrationTest), E2E 테스트(UserV1ApiE2ETest)를 통해 3단계 피라미드 검증을 구현했으며, 통합 테스트에서는 Testcontainers와 DatabaseCleanUp을 활용하여 데이터베이스 상태를 관리합니다.