[Debug] 회원탈퇴 외래키 제약으로 인한 오류 & 프로필 이미지 url 반환 코드 수정#133
Conversation
…into feat/change-password
…o fix/profileImage
📝 WalkthroughWalkthroughUser 엔티티에 탈퇴 관련 필드와 메서드가 추가되고, 서비스 레이어에서 탈퇴 검증 및 S3 기반 프로필 이미지 URL을 조회해 UserConverter로 전달하도록 변경되었습니다. SecurityConfig에 CORS 및 PasswordEncoder 빈이 추가/수정되었습니다. (≤50단어) Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant UserService as UserServiceImpl
participant S3 as S3Service
participant Repo as UserRepository
participant Converter as UserConverter
Client->>UserService: getMemberInfo(request)
UserService->>Repo: findById(userId)
Repo-->>UserService: User entity
UserService->>UserService: check isDeleted -> if true throw WITHDRAWN_USER
UserService->>S3: toUrl(user.getProfileImage())
S3-->>UserService: profileImageUrl
UserService->>Converter: toUserInfo(user, profileImageUrl)
Converter-->>UserService: UserInfoDto
UserService-->>Client: UserInfoDto
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/eatsfine/eatsfine/domain/user/service/userService/UserServiceImpl.java (2)
195-211:⚠️ Potential issue | 🟠 MajorS3 이미지 삭제를 트랜잭션 커밋 이후로 이동해야 합니다.
현재 S3 프로필 이미지 삭제(Line 202-206)가
user.withdraw()및userRepository.save()호출 전에 실행됩니다. DB 저장이 실패하면 이미지는 이미 삭제되었지만 사용자는 탈퇴 상태가 아닌 불일치가 발생합니다.
updateMemberInfo메서드(Line 147-160)에서 이미 사용하고 있는TransactionSynchronization.afterCommit()패턴을 동일하게 적용해야 합니다.🔧 수정 제안
`@Override` `@Transactional` public void withdraw(HttpServletRequest request) { User user = getCurrentUser(request); - String profileImage = user.getProfileImage(); - if (profileImage != null && !profileImage.isBlank()) { - try { - s3Service.deleteByKey(profileImage); - } catch (Exception e) { - log.warn("프로필 이미지 삭제 실패. key={}", profileImage, e); - } - } - user.withdraw(); userRepository.save(user); + + String profileImage = user.getProfileImage(); + if (profileImage != null && !profileImage.isBlank()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + `@Override` + public void afterCommit() { + try { + s3Service.deleteByKey(profileImage); + } catch (Exception e) { + log.warn("프로필 이미지 삭제 실패. key={}", profileImage, e); + } + } + }); + } }As per coding guidelines, "DB 일관성, 롤백 정책이 올바른지 검토."
242-243:⚠️ Potential issue | 🟡 Minor로그에 사용자 이메일(
user.getEmail())이 노출되고 있습니다.이메일은 개인식별정보(PII)에 해당합니다. 로그에는
userId만 남기고 이메일은 제외하는 것이 바람직합니다.As per coding guidelines, "비밀번호, 토큰 등 민감한 정보가 로깅되지 않는지."
src/main/java/com/eatsfine/eatsfine/domain/user/converter/UserConverter.java (1)
45-51:⚠️ Potential issue | 🟡 Minor
toUpdateResponse는 현재 호출되지 않는 미사용 메서드입니다.검증 결과,
toUpdateResponse메서드는 코드베이스 어디에서도 호출되고 있지 않습니다. 실제 흐름은UserService.updateMemberInfo에서 String 메시지를 반환하며,UpdateResponseDto와toUpdateResponse컨버터는 사용되지 않고 있습니다.다만 코드에는 여전히
user.getProfileImage()를 직접 사용하여 S3 key가 반환될 수 있는 문제가 잠재되어 있습니다. 이 메서드를 향후 사용할 계획이 있다면,toUserInfo와 동일하게 profileImageUrl 파라미터를 받아 처리하도록 수정하거나, 현재 불필요한 메서드라면 제거하는 것을 권장합니다.
🤖 Fix all issues with AI agents
In `@src/main/java/com/eatsfine/eatsfine/domain/user/entity/User.java`:
- Around line 52-56: The isDeleted field is initialized to false but Lombok's
`@Builder` ignores field initializers, causing Builder-created Users (e.g. in
UserConverter.toUser()) to get a null isDeleted and nullable DB writes; add
Lombok's `@Builder.Default` to the isDeleted field (keep the "= false" assignment)
so Builder sets a default false value, and verify uses like isDeleted() remain
null-safe; alternatively ensure a DB column default via columnDefinition if you
prefer DB-level enforcement.
- Around line 99-108: The withdraw() soft-delete only nulls refreshToken and
sets isDeleted/deletedAt but leaves issued accessTokens valid and related Term
entities intact; update withdraw() (and/or add a UserService method called from
withdraw()) to 1) record/emit an immediate token invalidation (e.g., add user
token blacklist entry or increment a tokenVersion field tied to JWTs so issued
accessTokens are rejected), 2) explicitly remove or anonymize related Term data
instead of relying on CascadeType.REMOVE (deleteRelatedTerms() or
termRepository.deleteByUser(this) as part of withdrawal), and 3) enforce the
deletion check earlier by adding isDeleted() validation into
JwtAuthenticationFilter so requests with deleted users are rejected at
authentication time rather than only in getCurrentUser().
In
`@src/main/java/com/eatsfine/eatsfine/domain/user/service/userService/UserServiceImpl.java`:
- Around line 88-94: getMemberInfo in UserServiceImpl is read-only but annotated
with `@Transactional`; change it to use a read-only transaction by updating the
annotation to `@Transactional`(readOnly = true) on the getMemberInfo method so
Hibernate uses MANUAL flush mode for better performance; ensure the method
signature (getMemberInfo), any calls to getCurrentUser, s3Service.toUrl, and
UserConverter.toUserInfo remain unchanged.
- Around line 229-235: The login() method currently calls
userRepository.findByEmail(...) directly and therefore allows withdrawn users to
log back in; update login() to either call getCurrentUser(email) or perform the
same deletion check: after obtaining the User from
userRepository.findByEmail(...), call user.isDeleted() and if true throw new
UserException(UserErrorStatus.WITHDRAWN_USER) (same behavior as getCurrentUser),
ensuring the login flow blocks deleted accounts.
In `@src/main/java/com/eatsfine/eatsfine/domain/user/status/UserErrorStatus.java`:
- Around line 20-21: The enum UserErrorStatus has a duplicate semicolon after
the enum constant WITHDRAWN_USER; remove the redundant semicolon so the enum
constant list ends with a single semicolon (i.e., keep the semicolon that
terminates the enum constant list if needed, but delete the extra one following
WITHDRAWN_USER) in the UserErrorStatus enum to eliminate the obvious typo.
| @Column(name = "deleted_at") | ||
| private LocalDateTime deletedAt; | ||
|
|
||
| @Column(name = "is_deleted") | ||
| private Boolean isDeleted = false; |
There was a problem hiding this comment.
isDeleted 필드에 @Builder.Default 누락 — Builder로 생성 시 null이 됩니다.
Lombok @Builder는 필드 초기화 값(= false)을 무시합니다. UserConverter.toUser() 등 Builder로 User를 생성하면 isDeleted가 null로 설정되어 DB에도 null이 저장됩니다.
isDeleted() 메서드에서 null-safe 처리를 하고 있어 런타임 오류는 없지만, DB 데이터 정합성을 위해 @Builder.Default를 추가하거나 columnDefinition으로 DB 기본값을 설정해야 합니다.
🔧 수정 제안
`@Column`(name = "is_deleted")
+ `@Builder.Default`
private Boolean isDeleted = false;📝 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.
| @Column(name = "deleted_at") | |
| private LocalDateTime deletedAt; | |
| @Column(name = "is_deleted") | |
| private Boolean isDeleted = false; | |
| `@Column`(name = "deleted_at") | |
| private LocalDateTime deletedAt; | |
| `@Column`(name = "is_deleted") | |
| `@Builder.Default` | |
| private Boolean isDeleted = false; |
🤖 Prompt for AI Agents
In `@src/main/java/com/eatsfine/eatsfine/domain/user/entity/User.java` around
lines 52 - 56, The isDeleted field is initialized to false but Lombok's `@Builder`
ignores field initializers, causing Builder-created Users (e.g. in
UserConverter.toUser()) to get a null isDeleted and nullable DB writes; add
Lombok's `@Builder.Default` to the isDeleted field (keep the "= false" assignment)
so Builder sets a default false value, and verify uses like isDeleted() remain
null-safe; alternatively ensure a DB column default via columnDefinition if you
prefer DB-level enforcement.
| // 회원 탈퇴 메서드 추가 | ||
| public void withdraw() { | ||
| this.isDeleted = true; | ||
| this.deletedAt = LocalDateTime.now(); | ||
| this.refreshToken = null; // refresh token도 null 처리 | ||
| } | ||
|
|
||
| public boolean isDeleted() { | ||
| return this.isDeleted != null && this.isDeleted; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find User.java and related entity files
find . -type f -name "User.java" -o -name "*Term*.java" -o -name "*Entity.java" | head -20Repository: Eatsfine/BE
Length of output: 337
🏁 Script executed:
#!/bin/bash
# Check repository structure
ls -la src/main/java/com/eatsfine/eatsfine/domain/ 2>/dev/null || ls -la . | head -20Repository: Eatsfine/BE
Length of output: 1033
🏁 Script executed:
#!/bin/bash
# Find all entity files and examine User references
fd "\.java$" src/main/java --type f -x grep -l "@Entity\|@Table" {} \; | head -30Repository: Eatsfine/BE
Length of output: 1225
🏁 Script executed:
#!/bin/bash
# Read User.java to see the withdraw() and isDeleted() methods and context
cat -n src/main/java/com/eatsfine/eatsfine/domain/user/entity/User.java | head -120Repository: Eatsfine/BE
Length of output: 3686
🏁 Script executed:
#!/bin/bash
# Read Term.java to check User relationship and cascade config
cat -n src/main/java/com/eatsfine/eatsfine/domain/term/entity/Term.javaRepository: Eatsfine/BE
Length of output: 1166
🏁 Script executed:
#!/bin/bash
# Search for getCurrentUser implementation
rg -n "getCurrentUser" --type java -A 5Repository: Eatsfine/BE
Length of output: 5362
🏁 Script executed:
#!/bin/bash
# Search for token invalidation or blacklist mechanism
rg -n "refreshToken|accessToken|token.*invalidat|blacklist|logout" --type java -i | head -40Repository: Eatsfine/BE
Length of output: 6367
🏁 Script executed:
#!/bin/bash
# Search for where isDeleted() method is actually called
rg -n "\.isDeleted\(\)" --type java -B 3 -A 3Repository: Eatsfine/BE
Length of output: 981
🏁 Script executed:
#!/bin/bash
# Check security configuration and filter/interceptor for user validation
rg -n "class.*Filter|class.*Interceptor|class.*Advice|SecurityContextHolder" --type java | head -20Repository: Eatsfine/BE
Length of output: 708
🏁 Script executed:
#!/bin/bash
# Search for batch jobs, scheduled tasks, or cleanup logic
rg -n "@Scheduled|@Transactional.*void|batch|cleanup|purge|delete" --type java -i | grep -i "term\|orphan\|delete\|batch" | head -20Repository: Eatsfine/BE
Length of output: 2488
🏁 Script executed:
#!/bin/bash
# Check if isDeleted filter exists in repository queries
rg -n "isDeleted|deleted_at" --type java src/main/java/com/eatsfine/eatsfine/domain/user/ -B 2 -A 2Repository: Eatsfine/BE
Length of output: 2606
🏁 Script executed:
#!/bin/bash
# Check JwtAuthenticationFilter to see if isDeleted is checked there
cat -n src/main/java/com/eatsfine/eatsfine/global/config/jwt/JwtAuthenticationFilter.java | head -80Repository: Eatsfine/BE
Length of output: 3602
🏁 Script executed:
#!/bin/bash
# Search for all entities that reference User to understand cascade impact scope
rg -n "User.*user|@ManyToOne.*User|@OneToOne.*User" --type java src/main/java/com/eatsfine/eatsfine/domain/ -B 1 -A 1 | head -60Repository: Eatsfine/BE
Length of output: 6335
🏁 Script executed:
#!/bin/bash
# Check if there's a withdraw service method and how it's implemented
rg -n "withdraw" --type java -B 5 -A 10Repository: Eatsfine/BE
Length of output: 9453
🏁 Script executed:
#!/bin/bash
# Check if there are any batch jobs or scheduled deletion tasks
fd ".*Batch.*\.java|.*Scheduler.*\.java|.*Job.*\.java" --type f | xargs cat 2>/dev/null | head -50Repository: Eatsfine/BE
Length of output: 1548
🏁 Script executed:
#!/bin/bash
# Verify if there are any other User references or repositories that do isDeleted filtering
rg -n "findByEmail|findById" --type java src/main/java/com/eatsfine/eatsfine/domain/user/repository/ -A 3Repository: Eatsfine/BE
Length of output: 559
🏁 Script executed:
#!/bin/bash
# Check if there's any aspect/annotation that globally checks isDeleted
rg -n "@Aspect|@Around|isDeleted" --type java src/main/java/com/eatsfine/eatsfine/global/ -B 2 -A 5Repository: Eatsfine/BE
Length of output: 37
withdraw() 및 isDeleted() — 소프트 딜리트 구현에 몇 가지 미흡한 점이 있습니다.
소프트 딜리트로 FK 제약을 해결한 점은 좋으나, 다음 사항들을 개선해야 합니다:
-
발급된 accessToken은 만료 시까지 유효합니다.
refreshToken을 null 처리하면 토큰 재발급을 차단하지만, 탈퇴 전에 발급된 JWT accessToken은 만료 전까지 여전히 사용 가능합니다. 토큰 블랙리스트나 즉시 무효화 메커니즘이 필요할 수 있습니다. -
getCurrentUser()에서isDeleted()검사가 있으나, JwtAuthenticationFilter는 검사하지 않습니다. 현재 서비스 계층에서만 삭제 여부를 확인하므로, 필터 단계에서 추가 검증을 고려하세요. -
Term(약관 동의) 데이터가 영구 잔존합니다.
withdraw()메서드는 필드를 업데이트할 뿐 엔티티를 삭제하지 않으므로, User의CascadeType.REMOVE(Line 110)가 작동하지 않습니다. GDPR/개인정보 보호 관점에서 탈퇴 시 관련 데이터(Term 등)를 명시적으로 삭제하는 로직이 필요합니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/eatsfine/eatsfine/domain/user/entity/User.java` around
lines 99 - 108, The withdraw() soft-delete only nulls refreshToken and sets
isDeleted/deletedAt but leaves issued accessTokens valid and related Term
entities intact; update withdraw() (and/or add a UserService method called from
withdraw()) to 1) record/emit an immediate token invalidation (e.g., add user
token blacklist entry or increment a tokenVersion field tied to JWTs so issued
accessTokens are rejected), 2) explicitly remove or anonymize related Term data
instead of relying on CascadeType.REMOVE (deleteRelatedTerms() or
termRepository.deleteByUser(this) as part of withdrawal), and 3) enforce the
deletion check earlier by adding isDeleted() validation into
JwtAuthenticationFilter so requests with deleted users are rejected at
authentication time rather than only in getCurrentUser().
| @Override | ||
| @Transactional | ||
| public UserResponseDto.UserInfoDto getMemberInfo(HttpServletRequest request) { | ||
| User user = getCurrentUser(request); | ||
| return UserConverter.toUserInfo(user); | ||
| String profileUrl = s3Service.toUrl(user.getProfileImage()); | ||
| return UserConverter.toUserInfo(user, profileUrl); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
getMemberInfo는 읽기 전용 — @Transactional(readOnly = true) 사용 권장.
이 메서드는 데이터를 조회만 하므로 readOnly = true를 설정하면 Hibernate flush 모드를 MANUAL로 전환하여 성능상 이점이 있습니다.
♻️ 수정 제안
`@Override`
- `@Transactional`
+ `@Transactional`(readOnly = true)
public UserResponseDto.UserInfoDto getMemberInfo(HttpServletRequest request) {As per coding guidelines, "읽기 전용 트랜잭션(readOnly = true)을 적절히 사용했는지."
📝 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.
| @Override | |
| @Transactional | |
| public UserResponseDto.UserInfoDto getMemberInfo(HttpServletRequest request) { | |
| User user = getCurrentUser(request); | |
| return UserConverter.toUserInfo(user); | |
| String profileUrl = s3Service.toUrl(user.getProfileImage()); | |
| return UserConverter.toUserInfo(user, profileUrl); | |
| } | |
| `@Override` | |
| `@Transactional`(readOnly = true) | |
| public UserResponseDto.UserInfoDto getMemberInfo(HttpServletRequest request) { | |
| User user = getCurrentUser(request); | |
| String profileUrl = s3Service.toUrl(user.getProfileImage()); | |
| return UserConverter.toUserInfo(user, profileUrl); | |
| } |
🤖 Prompt for AI Agents
In
`@src/main/java/com/eatsfine/eatsfine/domain/user/service/userService/UserServiceImpl.java`
around lines 88 - 94, getMemberInfo in UserServiceImpl is read-only but
annotated with `@Transactional`; change it to use a read-only transaction by
updating the annotation to `@Transactional`(readOnly = true) on the getMemberInfo
method so Hibernate uses MANUAL flush mode for better performance; ensure the
method signature (getMemberInfo), any calls to getCurrentUser, s3Service.toUrl,
and UserConverter.toUserInfo remain unchanged.
src/main/java/com/eatsfine/eatsfine/domain/user/service/userService/UserServiceImpl.java
Show resolved
Hide resolved
src/main/java/com/eatsfine/eatsfine/domain/user/status/UserErrorStatus.java
Outdated
Show resolved
Hide resolved
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 (3)
src/main/java/com/eatsfine/eatsfine/domain/user/service/userService/UserServiceImpl.java (1)
199-215:⚠️ Potential issue | 🟠 Major
withdraw()메서드에서 S3 이미지 삭제 순서 및 refresh token 무효화 누락 문제.두 가지 문제가 있습니다:
S3 삭제가 트랜잭션 커밋 전에 실행됩니다.
user.withdraw()또는userRepository.save()실패 시 프로필 이미지는 이미 S3에서 삭제된 상태가 되어 데이터 불일치가 발생합니다.updateMemberInfo에서와 동일하게TransactionSynchronization.afterCommit()을 사용하세요.Refresh token이 무효화되지 않습니다.
logout()과 달리withdraw()시에는user.updateRefreshToken(null)이 호출되지 않습니다.getCurrentUser의isDeleted()체크로 일부 보호되지만, refresh token으로 새 access token을 발급받는 별도의 경로가 있다면 보안 갭이 생길 수 있습니다.🔒 수정 제안
`@Override` `@Transactional` public void withdraw(HttpServletRequest request) { User user = getCurrentUser(request); - String profileImage = user.getProfileImage(); - if (profileImage != null && !profileImage.isBlank()) { - try { - s3Service.deleteByKey(profileImage); - } catch (Exception e) { - log.warn("프로필 이미지 삭제 실패. key={}", profileImage, e); - } - } - + user.updateRefreshToken(null); user.withdraw(); userRepository.save(user); + + String profileImage = user.getProfileImage(); + if (profileImage != null && !profileImage.isBlank()) { + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + `@Override` + public void afterCommit() { + try { + s3Service.deleteByKey(profileImage); + } catch (Exception e) { + log.warn("프로필 이미지 삭제 실패. key={}", profileImage, e); + } + } + }); + } }As per coding guidelines, "DB 일관성, 롤백 정책이 올바른지 검토."
src/main/java/com/eatsfine/eatsfine/global/config/SecurityConfig.java (2)
108-113:⚠️ Potential issue | 🟠 Major프로덕션 환경에 localhost origin이 포함되어 있습니다.
http://localhost:3000,http://localhost:5173이 프로덕션 CORS 허용 목록에 하드코딩되어 있으면, 공격자가 로컬에서 credentialed 요청을 보낼 수 있습니다. Spring Profile(@Profile또는@Value)을 활용하여 환경별로 분리하는 것을 권장합니다.♻️ 환경별 분리 예시
- config.setAllowedOriginPatterns(List.of( - "https://www.eatsfine.co.kr", - "https://eatsfine.co.kr", - "http://localhost:3000", - "http://localhost:5173")); + config.setAllowedOriginPatterns(allowedOrigins);
allowedOrigins를application.yml의 프로파일별 설정(app.cors.allowed-origins)에서@Value로 주입받도록 변경하세요.As per coding guidelines,
src/**: "매직 넘버, 하드코딩된 값이 존재하는지 점검" 및 "비밀번호, 토큰 등 민감한 정보가 로깅되지 않는지" (보안 관련).
126-129:⚠️ Potential issue | 🟡 Minor
PasswordEncoder빈 사용 현황 확인됨.
PasswordEncoder는 실제로UserServiceImpl에서 주입되어 다음 기능들에 사용됩니다:
- 회원가입 시 비밀번호 인코딩
- 로그인 시 비밀번호 검증
- 비밀번호 변경 시 현재/새 비밀번호 검증 및 인코딩
다만 이 PR의 명시된 목적(회원탈퇴, 프로필 이미지 URL 수정)과는 직접 연관이 없습니다. 이 빈이 이번 PR에서 새로 추가되는 것이라면, 추가 이유를 명확히 하거나 기존 인증 관련 기능 정리의 일환인지 확인이 필요합니다.
3 participants
💡 작업 개요
✅ 작업 내용
🧪 테스트 내용
📝 기타 참고 사항
Summary by CodeRabbit
새로운 기능
버그 수정 / 오류 처리