Conversation
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 22개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 19개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
jychoi0831
left a comment
There was a problem hiding this comment.
PR #34 리뷰입니다.
Findings
P0: 최신 develop 기준으로 머지 불가 및 컴파일 불가 가능성이 큽니다.
PR #34의 PhotoClothesRegistrationService.java는 .wardrobe(), .color(), .isFavorite()를 쓰는 예전 Clothes builder를 기준으로 작성되어 있습니다. 하지만 현재 develop은 #35 반영 후 해당 필드가 없습니다. 또한 ClothesStyleTag.create(clothes, style) 호출도 현재 develop의 styleRole, sortOrder 인자 요구사항과 맞지 않습니다.
PR을 #35 모델에 맞춰 primaryColor/secondaryColors, ClothingColor, 새 ClothesStyleTag 생성 방식으로 리베이스해야 합니다.
P1: 업로드된 사용자 이미지가 인증 없이 공개됩니다.
SecurityConfig.java에서 /uploads/**를 public으로 열고, LocalImageStorageService.java는 사용자 사진 URL을 그대로 반환합니다. 의류 사진은 개인 이미지일 수 있으므로 URL만 알면 누구나 접근 가능한 구조는 위험합니다.
인증된 이미지 조회 API나 signed URL 방식이 필요합니다.
P2: AI 결과 검증 실패 메시지가 잘못 분류됩니다.
AiService.java에서 AI 결과 검증 중 잘못된 category/color/style 때문에 IllegalArgumentException이 나도, catch 블록이 모두 “업로드된 이미지를 찾을 수 없습니다.”로 처리합니다. 파일 없음과 AI 응답값 오류는 다른 실패 원인이므로 메시지와 로깅을 분리하는 편이 좋습니다.
Verification
gh pr view 34:mergeable=CONFLICTINGgit merge-tree:WardrobeClothes,RegistrationSource,WardrobeClothesRepositoryadd/add conflict 확인- PR 브랜치 단독
./gradlew compileJava --offline: 성공
현재 상태로는 머지 전 #35 색상/옷장 관계 모델에 맞춘 리베이스가 선행되어야 합니다.
본 리뷰는 Codex를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 26개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 26개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
There was a problem hiding this comment.
PR #34 리뷰 결과입니다. 현재 열린 PR은 feat/#33 -> develop 기준이며, 리뷰는 head commit 8e375fc 기준으로 확인했습니다.
P1: path userId만으로 사진 등록/분석/저장이 가능해 보입니다.
PhotoClothesRegistrationController.java:36에 TODO로 남아 있듯이, 현재 API는 인증 사용자와 path의 userId가 같은지 확인하지 않습니다.
PhotoClothesRegistrationController.java:39-44PhotoClothesRegistrationController.java:49-53PhotoClothesRegistrationController.java:66-73
위 엔드포인트들이 모두 path userId를 그대로 서비스에 전달합니다. 인증된 사용자가 다른 userId를 넣으면 타 사용자 명의로 사진 업로드/분석/옷 저장이 가능할 수 있습니다.
해결 방향: SecurityContext/JWT의 사용자 ID와 path userId를 비교하거나, path에서 userId를 제거하고 인증 주체 기준으로 처리하는 방식이 안전해 보입니다.
P1: 이미지 조회도 사용자 소유권 검증이 없습니다.
ImageController.java:26에도 TODO가 남아 있고, ImageController.java:31-48의 GET /clothes/{userId}/{filename}는 path의 userId/filename으로 파일을 바로 읽습니다.
/uploads/** 공개 허용은 제거되어 이전보다 나아졌지만, 인증된 사용자가 다른 사용자의 이미지 URL을 알면 조회할 수 있는 구조입니다. 이미지 응답 전에 인증 사용자와 path userId 일치 여부를 확인해야 합니다.
P2: Gemini 응답 필드명이 프롬프트 안에서 서로 달라 분석 실패 가능성이 있습니다.
CategoryCatalogService.java:136-145의 예시는 item_type을 안내하지만, GeminiService.java:62-70의 최종 JSON 형식은 itemType을 요구합니다. 그런데 GeminiClothingClassificationResult.java:9-16에는 item_type alias가 없습니다.
모델이 예시를 따라 item_type으로 반환하면 itemType이 비어 검증 실패가 날 수 있습니다.
해결 방향: @JsonAlias("item_type")를 추가하거나 프롬프트 예시를 모두 itemType으로 통일하는 것이 좋겠습니다.
P2: 같은 사진을 동시에 저장하면 중복 옷 생성 가능성이 있습니다.
PhotoClothesRegistrationService.java:91-94에서 isAlreadySaved()를 확인한 뒤 PhotoClothesRegistrationService.java:114-127에서 옷과 wardrobe row를 저장합니다. 하지만 ClothingAiPhotoRepository.java:8-10는 일반 조회라 동시 요청 두 개가 모두 false를 보고 각각 옷을 만들 수 있습니다.
해결 방향: 저장 플로우에는 pessimistic lock, optimistic version, 또는 상태 전이 조건을 DB 레벨에서 보장하는 방식이 필요해 보입니다.
검증: ./gradlew compileJava --offline 성공.
본 리뷰는 Codex를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 26개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 36개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
❌ 테스트 결과: 테스트 실패
실패한 테스트
|
jychoi0831
left a comment
jychoi0831
left a comment
There was a problem hiding this comment.
PR #34 재리뷰 결과입니다. 마지막 Codex 리뷰 이후 새 커밋이 올라온 80e3277 기준으로 확인했습니다.
P1: AI 분석 완료가 이미 저장된 사진 상태를 다시 SUCCESS/FAILED로 되돌릴 수 있습니다.
AiService.java:52-113에서 분석은 3단계로 나뉩니다. Phase 1에서 ANALYZING으로 바꾼 뒤 외부 Gemini 호출을 수행하고, Phase 3에서 다시 photo를 조회해 applyAnalysisSuccess 또는 applyAnalysisFailure를 호출합니다.
문제는 Gemini 호출 중 사용자가 PhotoClothesRegistrationService.java:91-129의 저장 API를 호출할 수 있다는 점입니다. 저장 API는 비관적 락으로 save-save 중복은 막지만, 현재 상태가 ANALYZING이어도 저장을 허용하고 markSaved()로 SAVED 상태를 만듭니다. 이후 늦게 끝난 AI 분석 Phase 3가 applyAnalysisSuccess/applyAnalysisFailure를 호출하면 ClothingAiPhoto.java:118-133에서 상태가 다시 SUCCESS 또는 FAILED로 덮입니다. 그러면 ClothingAiPhoto.java:146-148의 isAlreadySaved()가 다시 false가 되어 같은 사진으로 추가 저장이 가능해질 수 있습니다.
해결 방향: 저장 API에서 ANALYZING 상태 저장을 막거나, 분석 Phase 3에서 이미 SAVED인 photo는 결과를 덮어쓰지 않도록 해야 합니다. 더 안전하게는 상태 전이를 DB 레벨에서 보장하도록 optimistic lock/version 또는 상태 조건부 업데이트를 함께 고려하는 편이 좋겠습니다.
P1: OAuth 이메일 추출 실패 시 서로 다른 계정이 email = "null" 사용자로 묶일 수 있습니다.
OAuth2UserService.java:79-90에서 provider별 이메일을 꺼내는데, Kakao는 account_email을 읽고 있습니다. Kakao 사용자 정보 응답의 이메일 필드는 kakao_account.email이며, 공식 문서의 property key도 kakao_account.email입니다.
또한 Kakao/Naver/Google 모두 값이 없을 때 String.valueOf(null)이 "null" 문자열을 만들 수 있습니다. 그 값이 OAuth2UserService.java:41-55의 createAccount()로 들어가면 findByEmail("null") 기준으로 기존 사용자를 찾거나 새 사용자를 만들게 됩니다. User.java:25-26의 email은 unique라서, 이메일 동의가 없거나 필드명이 맞지 않는 여러 OAuth 계정이 같은 "null" 사용자에 연결될 수 있습니다.
해결 방향: Kakao는 email 키로 읽고, 모든 provider에서 추출한 이메일은 StringUtils.hasText()로 검증한 뒤 없으면 provider + "_" + providerId + "@noreply.invalid"처럼 provider/providerId 기반의 고유 fallback을 사용하는 것이 안전합니다.
참고: https://developers.kakao.com/docs/latest/en/kakaologin/rest-api
P2: JWT secret 형식이 .env.example 안내와 구현이 맞지 않습니다.
JwtTokenProvider.java:20-25는 jwt.secret을 Decoders.BASE64.decode(secret)로 디코딩한 뒤 Keys.hmacShaKeyFor()에 넘깁니다. 반면 .env.example:21-22는 JWT_SECRET을 "32자 이상 임의 문자열"로 안내합니다.
일반 32자 문자열을 넣으면 Base64 디코딩에 실패하거나, 디코딩은 되더라도 32바이트보다 짧아져 WeakKeyException으로 애플리케이션 시작이 실패할 수 있습니다.
해결 방향: 구현을 일반 문자열 기반으로 바꿔 secret.getBytes(StandardCharsets.UTF_8)를 사용하고 32바이트 이상을 검증하거나, 문서를 Base64 인코딩된 256-bit 이상 secret으로 명확히 바꾸는 편이 좋겠습니다.
검증:
git diff --check origin/develop..origin/feat/#33: 통과./gradlew compileJava --offline: 성공./gradlew test --offline: 실패. 임시 archive 환경에서.env가 없어 test profile의${DB_USERNAME}이 그대로 사용되며contextLoads()가 DB 접속 단계에서 실패했습니다.
본 리뷰는 Codex를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 37개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
jychoi0831
left a comment
There was a problem hiding this comment.
PR #34 재리뷰 결과입니다. 마지막 Codex 리뷰 이후 새 커밋 73cd696이 올라온 상태라 현재 head 기준으로 다시 확인했습니다.
P1: AI 분석 완료가 이미 저장된 사진 상태를 다시 SUCCESS/FAILED로 되돌릴 수 있습니다.
이전 리뷰의 상태 전이 문제가 현재 head에도 남아 있습니다.
AiService.java:52-113은 Phase 1에서 photo를 ANALYZING으로 바꾼 뒤 외부 Gemini 호출을 수행하고, Phase 3에서 다시 photo를 조회해 applyAnalysisSuccess 또는 applyAnalysisFailure를 호출합니다. 그런데 PhotoClothesRegistrationService.java:91-129의 저장 API는 ANALYZING 상태에서도 저장을 허용하고 markSaved()로 SAVED 상태를 만듭니다.
이 사이에 사용자가 저장을 완료하면, 늦게 끝난 분석 Phase 3가 ClothingAiPhoto.java:118-133에서 상태를 다시 SUCCESS 또는 FAILED로 덮을 수 있습니다. 그러면 ClothingAiPhoto.java:146-148의 isAlreadySaved()가 다시 false가 되어 같은 사진으로 추가 저장이 가능해질 수 있습니다.
해결 방향: 저장 API에서 ANALYZING 상태 저장을 막거나, 분석 Phase 3에서 이미 SAVED인 photo는 결과를 덮어쓰지 않도록 해야 합니다. 가능하면 optimistic lock/version 또는 상태 조건부 업데이트로 상태 전이를 DB 레벨에서 보장하는 편이 안전합니다.
P1: OAuth 이메일 추출 실패 시 서로 다른 계정이 email = "null" 사용자로 묶일 수 있습니다.
이전 리뷰의 OAuth 이메일 문제도 현재 head에 남아 있습니다.
OAuth2UserService.java:79-90에서 Kakao 이메일을 account_email로 읽고 있는데, Kakao 사용자 정보 응답의 이메일 property key는 kakao_account.email입니다. 또한 Kakao/Naver/Google 모두 이메일 값이 없으면 String.valueOf(null)이 "null" 문자열을 만들 수 있습니다.
그 값이 OAuth2UserService.java:41-55의 createAccount()로 들어가면 findByEmail("null") 기준으로 기존 사용자를 찾거나 새 사용자를 만들게 됩니다. User.email은 unique이므로, 이메일 동의가 없거나 필드명이 맞지 않는 여러 OAuth 계정이 같은 "null" 사용자에 연결될 수 있습니다.
해결 방향: Kakao는 email 키로 읽고, 모든 provider에서 추출한 이메일은 StringUtils.hasText()로 검증한 뒤 없으면 provider + "_" + providerId + "@noreply.invalid"처럼 provider/providerId 기반의 고유 fallback을 사용하는 것이 안전합니다.
참고: https://developers.kakao.com/docs/latest/en/kakaologin/rest-api
P2: 짧은 JWT secret을 32바이트로 패딩하면 약한 키 설정을 조용히 허용합니다.
이전 리뷰의 Base64 불일치 문제는 일반 문자열 기반으로 바뀌었지만, JwtTokenProvider.java:25-30에서 32바이트보다 짧은 secret을 Arrays.copyOf(keyBytes, 32)로 패딩하고 있습니다.
이렇게 하면 JWT_SECRET=a 같은 잘못된 설정도 애플리케이션이 정상 기동합니다. 하지만 실제 엔트로피는 1글자에 가깝고 나머지는 0 패딩이라 약한 서명 키가 됩니다. 인증 토큰 서명 키는 설정 오류를 숨기기보다 fail-fast로 막는 편이 안전합니다.
해결 방향: keyBytes.length < 32이면 IllegalArgumentException으로 기동을 실패시키고, .env.example의 "32자 이상" 안내와 맞추는 편이 좋겠습니다.
P2: 테스트 프로필이 아직 OAuth/Naver 환경변수에 의존해 clean checkout 테스트가 실패합니다.
이번 커밋에서 H2와 @AutoConfigureTestDatabase를 추가해 DB 의존성은 줄였지만, src/test/resources/application-test.yml:27-40의 OAuth client id/secret은 여전히 ${KAKAO_CLIENT_ID}, ${GOOGLE_CLIENT_ID}, ${NAVER_CLIENT_ID} 같은 placeholder를 기본값 없이 사용합니다.
실제로 clean archive에서 ./gradlew test를 실행하면 H2 DataSource는 잡히지만, NaverApiService.java:16-20이 ${spring.security.oauth2.client.registration.naver.client-id}를 주입받는 과정에서 ${NAVER_CLIENT_ID}를 해석하지 못해 contextLoads()가 실패합니다.
해결 방향: test profile에는 OAuth/Naver 값도 test-client-id, test-client-secret 같은 기본값을 제공하거나, 해당 외부 API bean을 test slice/mock/profile로 분리해 .env 없이도 테스트가 통과하도록 만드는 편이 좋겠습니다.
검증:
git diff --check origin/develop..origin/feat/#33: 통과./gradlew compileJava --offline: 성공./gradlew test --offline: 실패. 새 H2 의존성이 로컬 캐시에 없어 offline 모드에서 해석 불가./gradlew test: 실패.NAVER_CLIENT_IDplaceholder 미해결로contextLoads()실패
본 리뷰는 Codex를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 37개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
jychoi0831
left a comment
There was a problem hiding this comment.
PR #34 재리뷰 결과입니다. 마지막 Codex 리뷰 이후 새 커밋 1953f4c 기준으로 확인했습니다.
이번 커밋에서 JWT secret 검증, OAuth 이메일 fallback, test profile placeholder 문제는 개선되었고, clean archive 기준 ./gradlew test --offline도 통과했습니다.
P1: AI 분석 Phase 3가 락 없이 상태를 읽어 SAVED 상태를 다시 덮을 수 있는 경쟁 조건이 남아 있습니다.
AiService.java:96-116에 photo.isAlreadySaved() guard가 추가되어, 분석 결과 저장 시점에 이미 SAVED로 보이면 덮어쓰지 않도록 개선되었습니다. 다만 Phase 3의 조회는 AiService.java:129-131의 일반 findByIdAndUser_Id()를 사용합니다.
반면 저장 API는 PhotoClothesRegistrationService.java:91-129에서 findByIdAndUser_IdForUpdate()로 row lock을 잡고 저장합니다. 이때 분석 Phase 3가 저장 트랜잭션과 겹치면, 일반 SELECT가 저장 트랜잭션의 커밋 전 상태(ANALYZING)를 읽고 isAlreadySaved() == false를 통과할 수 있습니다. 이후 저장 트랜잭션이 markSaved()로 커밋한 뒤, 늦게 도착한 Phase 3 update가 applyAnalysisSuccess/applyAnalysisFailure로 상태를 다시 SUCCESS/FAILED로 덮을 수 있습니다.
결과적으로 이전에 지적했던 “저장된 사진이 다시 미저장 상태처럼 보이고 추가 저장 가능” 문제가 동시성 타이밍에 따라 여전히 재현될 수 있습니다.
해결 방향: Phase 3에서도 findByIdAndUser_IdForUpdate()를 사용해 저장 트랜잭션과 같은 row lock 규칙을 따르거나, @Version 기반 optimistic locking, 또는 where analysis_status <> 'SAVED' 조건부 업데이트처럼 상태 전이를 DB 레벨에서 보장하는 방식이 필요해 보입니다.
검증:
git diff --check origin/develop..origin/feat/#33: 통과./gradlew compileJava --offline: 성공./gradlew test --offline: 성공
본 리뷰는 Codex를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 37개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
jychoi0831
left a comment
There was a problem hiding this comment.
PR #34 재리뷰 결과입니다. 마지막 Codex 리뷰 이후 새 커밋 1322e8b 기준으로 확인했습니다.
이번 커밋에서 AiService.java의 Phase 3가 findByIdAndUser_IdForUpdate()를 사용하도록 바뀌어, 이전에 지적한 “분석 결과 저장 시점에 SAVED 상태를 덮어쓰는 문제”는 일부 개선되었습니다. 다만 같은 상태 전이 경로에 아직 남은 경쟁 조건이 있습니다.
P1: AI 분석 Phase 1이 lock 없이 ANALYZING으로 바꿔 SAVED 상태를 덮을 수 있습니다.
AiService.java:55-61의 Phase 1은 getOwnedPhoto()로 일반 조회를 한 뒤 photo.markAnalyzing()을 호출합니다. 이 조회는 AiService.java:129-131의 findByIdAndUser_Id()라서 row lock을 잡지 않습니다.
동시에 저장 API는 PhotoClothesRegistrationService.java:91-129에서 findByIdAndUser_IdForUpdate()로 lock을 잡고 markSaved()를 호출합니다. 하지만 분석 Phase 1은 같은 lock 규칙을 따르지 않기 때문에 아래 순서가 가능합니다.
- 분석 Phase 1이 일반 조회로
SUCCESS상태를 읽음 - 저장 API가 lock을 잡고
SAVED로 커밋 - 분석 Phase 1이 뒤늦게
markAnalyzing()을 flush/commit하면서SAVED를ANALYZING으로 덮음 - 이후 Phase 3가 lock을 잡아도 이미
SAVED상태가 사라져 있어SUCCESS/FAILED로 다시 바뀔 수 있음
즉, Phase 3에 lock을 추가한 현재 수정만으로는 “저장된 사진이 다시 미저장 상태처럼 보이고 추가 저장 가능”한 문제를 완전히 막기 어렵습니다.
해결 방향: Phase 1에서도 findByIdAndUser_IdForUpdate()를 사용해 저장 API와 같은 row lock 규칙을 따르거나, @Version 기반 optimistic locking, 또는 where analysis_status <> 'SAVED' 조건부 업데이트처럼 상태 전이를 DB 레벨에서 보장하는 방식이 필요해 보입니다.
검증:
git diff --check origin/develop..origin/feat/#33: 통과./gradlew compileJava --offline: 성공./gradlew test --offline: 성공
본 리뷰는 Codex를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 42개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
jychoi0831
left a comment
There was a problem hiding this comment.
PR #34 재리뷰 결과입니다. 마지막 Codex 리뷰 이후 올라온 head commit 66ef59e 기준으로 확인했습니다.
이번 커밋에서 AiService.java의 Phase 1도 findByIdAndUser_IdForUpdate()를 사용하도록 바뀌어, 이전에 지적했던 “저장된 사진의 SAVED 상태가 분석 흐름에 의해 다시 덮이는 문제”는 같은 row lock 규칙으로 정리된 것으로 보입니다.
P2: 새 사용자의 옷장 최초 생성은 여전히 동시 요청에서 한쪽이 실패할 수 있습니다.
WardrobeService.java:35-38에서 findByUser_IdForUpdate()로 조회한 뒤 없으면 createWardrobeEntity()를 호출하도록 바뀌었습니다. 그런데 WardrobeRepository.java:18-20의 pessimistic lock은 이미 존재하는 wardrobe row에만 걸립니다. wardrobe가 아직 없는 새 사용자라면 동시에 들어온 두 요청이 모두 빈 결과를 보고 각각 insert를 시도할 수 있고, 이 경우 Wardrobe의 user_id unique constraint 때문에 한쪽 요청은 DataIntegrityViolationException으로 실패할 수 있습니다.
사진 저장/일반 옷 등록처럼 첫 옷장 생성이 자연스럽게 발생하는 API에서는 사용자가 더블 클릭하거나 병렬 요청이 들어왔을 때 간헐적인 409가 될 수 있습니다.
해결 방향은 사용자 row를 기준으로 lock을 잡고 wardrobe를 생성하거나, unique constraint 충돌을 잡아 다시 findByUser_Id()로 조회해 반환하는 방식입니다. 또는 회원 생성 시점에 wardrobe를 미리 생성하면 이 경로의 경쟁 조건을 피할 수 있습니다.
검증:
git diff --check origin/develop..origin/feat/#33: 통과./gradlew compileJava --offline: 성공./gradlew test --offline: 성공
본 리뷰는 Codex를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 42개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
jychoi0831
left a comment
There was a problem hiding this comment.
PR #34 재리뷰 결과입니다. 마지막 Codex 리뷰 이후 올라온 head commit a1da790 기준으로 확인했습니다.
이번 커밋에서 getOrCreateWardrobe()가 존재하지 않는 wardrobe row 대신 항상 존재하는 users row를 FOR UPDATE로 잠그도록 바뀌어, 직전 리뷰에서 지적했던 “새 사용자의 옷장 최초 생성 동시 요청 시 한쪽이 unique constraint로 실패할 수 있는 문제”는 의도대로 개선된 것으로 보입니다.
P1: 옷장 API는 여전히 path userId 소유권 검증 없이 다른 사용자의 옷장을 조회/생성할 수 있습니다.
#34에서 JWT 필터와 SecurityUtils가 추가되었고, 옷/위시리스트/사진 API에는 path userId 검증이 붙었습니다. 하지만 WardrobeController.java:24-35의 GET/POST /api/wardrobes/users/{userId}는 여전히 path의 userId를 그대로 WardrobeService에 전달합니다.
SecurityConfig.java 기준으로 인증은 필요하지만, 인증된 사용자 A가 사용자 B의 userId로 요청하면 B의 옷장을 조회하거나, 아직 없다면 생성할 수 있습니다. 특히 이번 커밋에서 WardrobeService.getOrCreateWardrobe()는 안전해졌지만, 공개 컨트롤러 경로인 createWardrobe()는 여전히 요청자가 해당 userId의 소유자인지 확인하지 않습니다.
해결 방향은 다른 사용자 리소스 API와 동일하게 SecurityUtils.verifyUserIdMatch(userId) 또는 verifyOwnership(userId)를 WardrobeController에도 적용하는 것입니다. 가능하면 /api/v1 prefix와 ApiResponse<T> 래퍼도 함께 맞추면 새 인증 흐름과 응답 규격이 더 일관됩니다.
검증:
git diff --check origin/develop..origin/feat/#33: 통과./gradlew compileJava --offline: 성공./gradlew test --offline: 성공
본 리뷰는 Codex를 사용해 작성했습니다.
|
ℹ️ Claude 코드 리뷰 생략: 변경된 파일이 43개로 너무 많습니다. 10개 이하로 분리하는 것을 권장합니다. |
✅ 테스트 결과: 모든 테스트 통과
|
jychoi0831
left a comment
jychoi0831
left a comment
There was a problem hiding this comment.
PR #34 재리뷰 결과입니다. 마지막 Codex 리뷰 이후 올라온 head commit 562b1f3 기준으로 확인했습니다.
이번 커밋에서 WardrobeController도 /api/v1/wardrobes로 이동했고, GET/POST /users/{userId} 양쪽에 SecurityUtils.verifyUserIdMatch(userId)가 추가되었습니다. 응답도 ApiResponse<WardrobeResponse>로 감싸져 직전 리뷰에서 지적했던 “옷장 API가 path userId 소유권 검증 없이 조회/생성을 허용하는 문제”는 해결된 것으로 보입니다.
추가 blocking finding은 발견하지 못했습니다. 다만 이번 PR이 인증, 이미지 저장, AI 분석, 옷장/옷 등록 흐름을 함께 넓게 건드리고 있어서, 현재 contextLoads() 1개 외에 컨트롤러 권한 검증과 사진 저장/분석 상태 전이를 다루는 통합 테스트가 보강되면 이후 회귀를 더 잘 잡을 수 있습니다.
검증:
git diff --check origin/develop..origin/feat/#33: 통과./gradlew compileJava --offline: 성공./gradlew test --offline: 성공
본 리뷰는 Codex를 사용해 작성했습니다.
2 participants
📌 관련 이슈
Closes #33
🛠️ 작업 내용
✅ 변경 사항
🔍 테스트 내용
📷 스크린샷 (선택사항)
💬 리뷰어에게
📋 PR 체크리스트
develop브랜치를 base로 설정했나요?