탈퇴 유저 tombstone 누수 차단 - 활성 유저 전용 조회 분리#773
Conversation
findById 가 soft-deleted(tombstone) 유저도 반환해, 활성 검증이 호출부마다 제각각(throw / isActive / 미검증) 흩어져 있었다. GET /me 와 위시리스트 회원 가드는 검증이 아예 없어 탈퇴 유저가 그대로 통과했다. - UserService.findActiveById 추가 - tombstone 이면 deletedUser(409) - 활성 전용 7곳을 findActiveById 로 통일 (getMyProfile · updateProfile · updateProfileImageUrl · promoteToMember · AuthService.issueTokenForExistingUser · ProfileUpdateService.updateMe · WishlistService.requireMember) - 의도적 tombstone 읽기 5곳은 findById 유지 + 이유 주석 (AuthService.refresh 는 401 · SocialAccountService/Writer 는 재로그인 판정 · WithdrawalService · softDelete 는 멱등 탈퇴) - getPriceHistory 에 누락돼 있던 requireMember 추가 - 문서엔 "MEMBER 필요"로 적혀 있었으나 실제 가드가 없어 위시 엔드포인트 중 유일하게 활성 검증을 빠져나갔다. 부정확했던 403 description·example 도 실제 응답에 맞춤 - 새로 도달 가능해진 409 를 GET /me · 위시리스트 11개 엔드포인트 문서에 반영 동작 변경: GET /me 와 위시리스트가 tombstone 에 200 대신 409(USER-003). 정상 운영에선 denylist 가 401 로 먼저 끊어 도달하지 않고, 토큰 무효화가 부분 실패한 창(#689)에서만 뜨는 2차 방어선이다.
|
Discord 스레드 연동용 메타데이터입니다. discord-pr-bot 워크플로가 자동 생성하며, 수정·삭제하면 PR 과 Discord 알림 연동이 끊깁니다. |
Walkthrough탈퇴 사용자와 활성 사용자 조회 계약을 분리하고, 인증·사용자·위시리스트 서비스에서 활성 사용자 검증을 적용했습니다. 관련 API의 409 응답 문서와 예시를 추가했으며, 탈퇴 사용자 접근 차단과 프로필 이미지 보상 삭제를 통합 테스트로 검증했습니다. Changes탈퇴 사용자 차단
Estimated code review effort: 3 (Moderate) | ~20 minutes Assessment against linked issues
Sequence Diagram(s)sequenceDiagram
participant AccessToken
participant AuthService
participant UserService
participant WishlistService
AccessToken->>AuthService: 사용자 식별
AuthService->>UserService: findActiveById
UserService-->>AuthService: 활성 사용자 또는 USER-003
AuthService->>WishlistService: 위시리스트 요청 전달
WishlistService->>UserService: findActiveById
UserService-->>WishlistService: 활성 회원 또는 USER-003
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt (1)
32-39: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win최종 저장 실패 시 S3 고아 객체를 정리해야 합니다.
활성 확인 후 S3 업로드가 먼저 수행되고, 탈퇴가 그 사이에 커밋되면 마지막
updateProfile이USER-003으로 실패하지만 업로드된 프로필 이미지가 남습니다. 닉네임 충돌 등 후속 영속화 실패에도 같은 문제가 발생하므로, 객체 단위 보상 삭제나 정리 outbox를 추가해야 합니다. 프로필 이미지는 PII이므로 lifecycle 정리만 의존하면 안 됩니다.As per path instructions, Kotlin Service 계층의 외부 호출과 상태 변경 타이밍을 엄격히 검토해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt` around lines 32 - 39, Update ProfileUpdateService around imageStorage.upload and userService.updateProfile so any persistence failure after a successful upload immediately compensates by deleting the uploaded object using the storage abstraction’s object-delete operation; preserve the original exception and avoid cleanup failures masking it. Ensure cleanup covers deactivation races and other updateProfile failures, while retaining the existing pre-upload validation.Source: Path instructions
src/main/kotlin/com/depromeet/piki/user/service/UserService.kt (1)
297-324: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift활성 여부 체크는 쓰기와 같은 트랜잭션/락으로 묶어야 합니다.
UserService.findActiveById()뒤의save/saveAndFlush,WishlistService.requireMember()뒤의WishPersistenceService.persist*()는 모두 check-then-use라서, 중간에softDelete()가 커밋되면 탈퇴한 사용자에 프로필 변경이 반영되거나 위시가 생성될 수 있습니다.BaseEntity에@Version이 없으니, 사용자 상태를 쓰기 전에SELECT ... FOR UPDATE로 잡거나 optimistic locking을 추가하고, 위시 생성도 persistence 경로 안에서 다시 active 상태를 재검증하세요. Spring Data JPA locking / Hibernate optimistic locking 문서 참고.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/depromeet/piki/user/service/UserService.kt` around lines 297 - 324, 활성 사용자 확인과 쓰기를 동일 트랜잭션의 비관적 행 잠금 또는 낙관적 잠금으로 보호해 softDelete와의 경쟁을 차단하세요. src/main/kotlin/com/depromeet/piki/user/service/UserService.kt 297-324의 findActiveById 이후 saveAndFlush 경로와 334-343의 관련 사용자 쓰기 경로에 잠금/버전 검증을 적용하고, src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt 42-47의 requireMember 확인은 WishPersistenceService.persist* 내부에서 active 상태를 다시 검증하도록 수정하세요.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt`:
- Around line 32-39: Update ProfileUpdateService around imageStorage.upload and
userService.updateProfile so any persistence failure after a successful upload
immediately compensates by deleting the uploaded object using the storage
abstraction’s object-delete operation; preserve the original exception and avoid
cleanup failures masking it. Ensure cleanup covers deactivation races and other
updateProfile failures, while retaining the existing pre-upload validation.
In `@src/main/kotlin/com/depromeet/piki/user/service/UserService.kt`:
- Around line 297-324: 활성 사용자 확인과 쓰기를 동일 트랜잭션의 비관적 행 잠금 또는 낙관적 잠금으로 보호해
softDelete와의 경쟁을 차단하세요.
src/main/kotlin/com/depromeet/piki/user/service/UserService.kt 297-324의
findActiveById 이후 saveAndFlush 경로와 334-343의 관련 사용자 쓰기 경로에 잠금/버전 검증을 적용하고,
src/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.kt 42-47의
requireMember 확인은 WishPersistenceService.persist* 내부에서 active 상태를 다시 검증하도록
수정하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: fe418bfa-e383-4052-a7a6-fce2d80abd0f
📒 Files selected for processing (12)
src/main/kotlin/com/depromeet/piki/auth/service/AuthService.ktsrc/main/kotlin/com/depromeet/piki/user/controller/UserApi.ktsrc/main/kotlin/com/depromeet/piki/user/controller/UserApiExamples.ktsrc/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.ktsrc/main/kotlin/com/depromeet/piki/user/service/UserService.ktsrc/main/kotlin/com/depromeet/piki/user/service/WithdrawalService.ktsrc/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApi.ktsrc/main/kotlin/com/depromeet/piki/wishlist/controller/WishlistApiExamples.ktsrc/main/kotlin/com/depromeet/piki/wishlist/service/WishlistService.ktsrc/test/kotlin/com/depromeet/piki/user/controller/UserControllerIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/wishlist/controller/WishPriceHistoryIntegrationTest.ktsrc/test/kotlin/com/depromeet/piki/wishlist/controller/WishlistCrudIntegrationTest.kt
업로드는 트랜잭션 밖에서 먼저 끝나므로, 뒤이은 updateProfile 이 떨어지면 아무도 안 가리키는 객체가 S3 에 남는다. 닉네임 중복(409)이 흔한 트리거이고, 활성 확인과 영속화 사이에 탈퇴가 커밋된 경우엔 탈퇴 cascade 의 prefix 파기가 이미 지나간 뒤라 프로필 사진(PII)이 계속 남는다. registerFromImages 의 raw 회수와 같은 패턴으로 보상 삭제를 건다. 회수 자체는 best-effort 라 실패해도 삼키고 warn 만 남겨 원래 예외를 가리지 않는다. CodeRabbit 리뷰 지적 반영.
|
CodeRabbit 리뷰의 outside-diff 코멘트 2건 처리 결과입니다. 1.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt (1)
32-45: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift탈퇴와 프로필 수정을 쓰기 시점에 막아야 합니다.
지금은findActiveById()가 읽기 시점만 확인하고,updateProfile()이saveAndFlush()로 같은User를 다시 쓰기 때문에 그 사이WithdrawalPersistenceService가 커밋되면 탈퇴 행을 마지막 커밋이 덮어쓸 수 있습니다.deleted_at조건이 없고@Version도 없어, 프로필 수정으로 탈퇴 계정이 되살아나는 race가 남습니다.@Version(JPA 낙관적 락)이나UPDATE ... WHERE deleted_at IS NULL로 바꾸고, 0건이면deletedUser()로 처리하세요.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt` around lines 32 - 45, Update the persistence path used by UserService.updateProfile so profile writes cannot overwrite a concurrently committed withdrawal: add optimistic locking with `@Version` or make the update conditional on deleted_at being null, and treat a zero-row update (or optimistic-lock failure) as deletedUser(). Keep findActiveById’s existing validation and ensure the race results in a deletion error with no profile update persisted.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt`:
- Around line 39-45: 프로필 이미지 보상 삭제를 즉시 실패 처리에서 분리해 업로드와 DB 반영 결과를 안전하게 추적하세요.
ProfileUpdateService의 updateProfile 흐름에서 새 S3 객체를 pending asset/outbox로 기록하고,
업로드 응답 유실·timeout을 포함한 미확정 결과도 추적되도록 하며, 비동기 정리 작업은 DB에 해당 URL 참조가 없음을 확인한 객체만
deleteQuietly로 회수하도록 변경하세요.
---
Outside diff comments:
In `@src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt`:
- Around line 32-45: Update the persistence path used by
UserService.updateProfile so profile writes cannot overwrite a concurrently
committed withdrawal: add optimistic locking with `@Version` or make the update
conditional on deleted_at being null, and treat a zero-row update (or
optimistic-lock failure) as deletedUser(). Keep findActiveById’s existing
validation and ensure the race results in a deletion error with no profile
update persisted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 467168fc-e6ea-4e79-9a8f-367df56e6739
📒 Files selected for processing (2)
src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.ktsrc/test/kotlin/com/depromeet/piki/user/controller/UserControllerIntegrationTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/kotlin/com/depromeet/piki/user/controller/UserControllerIntegrationTest.kt
| // 영속화가 떨어지면 방금 올린 객체가 아무도 안 가리키는 orphan 으로 남는다 — 닉네임 중복(409)이 흔한 | ||
| // 트리거이고, 활성 확인과 영속화 사이에 탈퇴가 커밋되면 탈퇴 cascade 의 prefix 파기가 이미 지나간 뒤라 | ||
| // 프로필 사진(얼굴 등 PII)이 S3 에 계속 남는다. 그래서 lifecycle 에 맡기지 않고 즉시 회수한다 | ||
| // (registerFromImages 의 raw 회수와 같은 패턴). | ||
| return runCatching { userService.updateProfile(userId, nickname, url) } | ||
| .onFailure { deleteQuietly(key) } | ||
| .getOrThrow() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
rg -n -A35 -B10 \
'fun (upload|delete|updateProfile)|`@Transactional`' \
src/main/kotlinRepository: TeamPiKi/core
Length of output: 50370
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# 후보 파일과 관련 심볼 위치만 좁게 확인
fd -a 'ProfileUpdateService.kt|UserService.kt|ImageStorage.kt|.*Storage.*\.kt' src/main/kotlin
echo '--- ProfileUpdateService ---'
ast-grep outline src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt --view expanded || true
cat -n src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt | sed -n '1,140p'
echo '--- UserService ---'
fd -a 'UserService.kt' src/main/kotlin
ast-grep outline src/main/kotlin/com/depromeet/piki/user/service/UserService.kt --view expanded || true
cat -n src/main/kotlin/com/depromeet/piki/user/service/UserService.kt | sed -n '1,220p'
echo '--- ImageStorage impl/interface ---'
rg -n -A40 -B10 'interface ImageStorage|class .*ImageStorage|fun upload\(|fun delete\(' src/main/kotlinRepository: TeamPiKi/core
Length of output: 50369
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo '--- UserService.updateProfile ---'
cat -n src/main/kotlin/com/depromeet/piki/user/service/UserService.kt | sed -n '280,360p'
echo '--- ProfileUpdateService context ---'
cat -n src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt | sed -n '1,120p'
echo '--- search for commit/transaction note around user profile ---'
rg -n -A6 -B6 'updateProfile\(|duplicateNickname|DataIntegrityViolationException|`@Transactional`' src/main/kotlin/com/depromeet/piki/user/service/UserService.ktRepository: TeamPiKi/core
Length of output: 16987
프로필 이미지 보상 삭제를 업로드 단계와 커밋 결과에 맞게 분리하세요.
지금은 updateProfile 실패에만 deleteQuietly(key)가 걸려 있어서, 업로드 중 응답 유실/timeout처럼 “S3에는 올라갔지만 예외가 난” 경우는 회수되지 않습니다. 반대로 커밋이 메서드 반환 뒤에 실패할 수 있는 예외까지 즉시 삭제하면, DB가 이미 새 URL을 가리키는 상태와 어긋날 수 있습니다. 업로드/DB 반영을 pending asset(outbox)로 추적하고, 참조 없음이 확인된 객체만 비동기로 회수하는 쪽이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/kotlin/com/depromeet/piki/user/service/ProfileUpdateService.kt`
around lines 39 - 45, 프로필 이미지 보상 삭제를 즉시 실패 처리에서 분리해 업로드와 DB 반영 결과를 안전하게 추적하세요.
ProfileUpdateService의 updateProfile 흐름에서 새 S3 객체를 pending asset/outbox로 기록하고,
업로드 응답 유실·timeout을 포함한 미확정 결과도 추적되도록 하며, 비동기 정리 작업은 DB에 해당 URL 참조가 없음을 확인한 객체만
deleteQuietly로 회수하도록 변경하세요.
1 participant
Situation
UserService.findById는 탈퇴(soft-delete)한 유저도 그대로 돌려준다. 그래서 "이 계정이 아직 살아있나" 를 확인하는 책임이 호출부 12곳에 흩어졌고, 각자 다르게 처리하고 있었다: 예외를 던지는 곳, 활성 여부만 보고 건너뛰는 곳, 아예 확인하지 않는 곳.GET /me)는 탈퇴한 계정 정보를 200 으로 내려줬고, 위시리스트 회원 가드는 회원 종류만 보고 탈퇴 여부를 안 봐서 죽은 계정이 위시리스트를 그대로 쓸 수 있었다.Task
Action
조회 메서드를 둘로 나눔
findActiveById(신설)USER-003으로 끊음findById(기존)호출부마다 흩어져 있던 탈퇴 확인을
findActiveById한 자리로 모았다. 두 메서드 모두 왜 그 쪽을 골라야 하는지 주석으로 박아, 다음 호출부가 이름만 보고도 고를 수 있게 했다.활성 전용으로 통일한 경로
getMyProfile(GET /me)WishlistService.requireMemberupdateProfileupdateProfileImageUrlpromoteToMemberAuthService.issueTokenForExistingUserProfileUpdateService.updateMe기존 동작을 유지한 경로
탈퇴 상태를 읽는 게 목적이라 그대로 뒀고, 왜 활성 조회를 안 쓰는지 각 자리에 주석을 남겼다.
AuthService.refreshSocialAccountService.loginExistingSocialAccountWriter.linkGuestAndPromoteWithdrawalService.withdraw/softDelete작업 중 발견한 누락
위시리스트 가격 이력 조회만 회원 가드를 안 타고 있었다. 나머지 10개 엔드포인트는 전부 첫 줄에서 가드를 호출하는데 이것만 빠져 있었고, 왜 없는지 주석도 없었다. 문서에는 "회원 필요" 라고 적혀 있어 문서와 코드가 어긋난 상태였다.
검토했으나 채택하지 않은 안
@SQLRestriction으로 탈퇴 계정 일괄 제외USER-003을 쓰고 있어 도메인 안에서 응답이 갈린다문서
새로 도달 가능해진 409 를 마이페이지 조회와 위시리스트 11개 엔드포인트에 반영했다. 이미 다른 의미의 409(항목 상태 충돌)를 쓰던 두 엔드포인트는 상태 코드당 블록이 하나라, 기존 설명에 사유를 덧붙이고 예시만 추가했다. 예시는 문구를 손으로 박지 않고 예외 정의에서 끌어와 실제 응답과 어긋나지 않게 했다.
리뷰에서 나온 보강 - 업로드 객체 회수
프로필 이미지 업로드는 트랜잭션 밖에서 먼저 끝나므로, 뒤이은 영속화가 떨어지면 아무도 안 가리키는 객체가 S3 에 남는다. 닉네임 중복(409)이 가장 흔한 트리거이고, 활성 확인과 영속화 사이에 탈퇴가 커밋된 경우엔 탈퇴 정리의 프로필 파기가 이미 지나간 뒤라 사진이 계속 남는다.
Result
USER-003을 내려준다. 클라이언트 대응은 401 과 같다(토큰 폐기 후 로그인 화면).연관 이슈
Summary by CodeRabbit
내 정보및 위시리스트 관련 기능에 접근하면 일관되게 409(CONFLICT) 이 반환됩니다.