feat: 회원탈퇴 기능 구현 - #52
Conversation
📝 WalkthroughWalkthrough회원탈퇴 기능을 구현하며, 토큰 블랙리스트 저장소를 리팩토링합니다. 멤버 삭제 시 관련된 모든 엔티티(건강 리포트, 추천 식단 등)를 일괄 삭제하는 인프라를 추가합니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant MemberController
participant WithdrawalService
participant MemberDeletionExecutor
participant HealthReportRepository
participant RecommendedMealRepository
participant MemberRepository
participant TokenBlacklistStore
Client->>MemberController: POST /withdrawal (memberId, tokens)
MemberController->>WithdrawalService: withdrawal(memberId, accessToken, refreshToken)
WithdrawalService->>MemberDeletionExecutor: deleteAll(memberId)
MemberDeletionExecutor->>HealthReportRepository: deleteAllByMemberEntityId(memberId)
HealthReportRepository-->>MemberDeletionExecutor: success
MemberDeletionExecutor->>RecommendedMealRepository: deleteAllByMemberEntityId(memberId)
RecommendedMealRepository-->>MemberDeletionExecutor: success
MemberDeletionExecutor->>MemberRepository: delete(memberId)
MemberRepository-->>MemberDeletionExecutor: success
MemberDeletionExecutor-->>WithdrawalService: completion
WithdrawalService->>TokenBlacklistStore: blacklist(refreshToken, REFRESH, ttl)
TokenBlacklistStore-->>WithdrawalService: success
WithdrawalService->>TokenBlacklistStore: blacklist(accessToken, ACCESS, ttl)
TokenBlacklistStore-->>WithdrawalService: success
WithdrawalService-->>MemberController: withdrawal completed
MemberController->>Client: 200 WITHDRAWAL_SUCCESS (refreshToken cookie cleared)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/main/java/org/sopt/carena/recommend/adapter/out/persistence/repository/RecommendedMealRepository.java (1)
10-11: 파라미터 타입 일관성 확인 필요기존 메서드
findTopByMemberEntityIdOrderByCreatedAtDesc는long을 사용하지만, 새로 추가된 메서드는Long을 사용합니다.♻️ 제안된 수정
- void deleteAllByMemberEntityId(Long memberId); + void deleteAllByMemberEntityId(long memberId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/carena/recommend/adapter/out/persistence/repository/RecommendedMealRepository.java` around lines 10 - 11, The parameter type for deleteAllByMemberEntityId is inconsistent with findTopByMemberEntityIdOrderByCreatedAtDesc: change deleteAllByMemberEntityId(Long memberId) to use the primitive type long (deleteAllByMemberEntityId(long memberId)) so both repository methods use the same parameter type and avoid unnecessary boxing/unboxing; update the method signature in RecommendedMealRepository accordingly and adjust any callers if needed.src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportRepository.java (1)
47-48: 파라미터 타입 일관성 확인 필요기존 메서드들은
long(primitive)을 사용하지만, 새로 추가된 메서드는Long(wrapper)을 사용합니다. 일관성을 위해 기존 패턴에 맞추는 것을 권장합니다.♻️ 제안된 수정
- void deleteAllByMemberEntityId(Long memberId); + void deleteAllByMemberEntityId(long memberId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportRepository.java` around lines 47 - 48, The method signature deleteAllByMemberEntityId currently uses the wrapper type Long; update it to use the primitive long to match existing repository method signatures and maintain parameter type consistency in HealthReportRepository (change deleteAllByMemberEntityId(Long memberId) to deleteAllByMemberEntityId(long memberId)).src/main/java/org/sopt/carena/member/application/port/in/WithdrawalUseCase.java (1)
3-5: LGTM!인터페이스 설계가 적절합니다. 쉼표 뒤 공백 추가를 권장합니다 (코드 가독성).
♻️ 선택적 포맷팅 수정
- void withdrawal(Long memberId,String accessToken,String refreshToken); + void withdrawal(Long memberId, String accessToken, String refreshToken);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/carena/member/application/port/in/WithdrawalUseCase.java` around lines 3 - 5, The method signature in interface WithdrawalUseCase has no space after the comma in withdrawal(Long memberId,String accessToken,String refreshToken); — update the signature in WithdrawalUseCase so there is a space after each comma (e.g., between memberId and String accessToken, and between accessToken and String refreshToken) to match Java formatting conventions and improve readability.src/main/java/org/sopt/carena/member/adapter/out/persistence/MemberDeletionExecutor.java (1)
18-23: 명시적 멤버 존재 여부 검증은 선택사항
memberJpaRepository.deleteById(memberId)호출 시 멤버가 존재하지 않아도 Spring Data JPA 3.x에서는 예외를 발생시키지 않습니다(silent no-op). 다만, 명시적인 존재 여부 검증을 통해 더 명확한 에러 처리를 원한다면, 검증 로직은MemberDeletionExecutor보다는WithdrawalService서비스 계층에 추가하는 것이 더 적절합니다.현재 구현은 안전하고 멱등성을 가지고 있습니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/carena/member/adapter/out/persistence/MemberDeletionExecutor.java` around lines 18 - 23, The deleteAll method in MemberDeletionExecutor currently relies on memberJpaRepository.deleteById(memberId) which in Spring Data JPA 3.x is a silent no-op for non-existent IDs; if you want explicit existence validation, do not add it here—instead add a presence check in the service layer (WithdrawalService) before calling MemberDeletionExecutor.deleteAll: call memberJpaRepository.existsById(memberId) or load the Member in WithdrawalService and throw the appropriate domain exception if missing, then invoke MemberDeletionExecutor.deleteAll(memberId) to perform the deletions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/carena/member/adapter/out/persistence/RefreshTokenBlacklistStoreAdapter.java`:
- Line 23: In RefreshTokenBlacklistStoreAdapter, update the incorrect log
message in the log.debug call that currently says "AccessToken 블랙리스트 등록
(TTL={}ms)" to correctly reference RefreshToken (e.g., "RefreshToken 블랙리스트 등록
(TTL={}ms)"), keeping the TTL placeholder and log context intact so the debug
output accurately reflects the adapter's behavior.
In
`@src/main/java/org/sopt/carena/member/application/service/WithdrawalService.java`:
- Around line 30-35: In WithdrawalService, when handling refreshToken do the
same TTL validation as for accessToken: after computing refreshremainingMillis
via jwtTokenParser.getRemainingValidityMillis(refreshToken) check that
refreshremainingMillis > 0 before calling
refreshTokenBlacklistStore.blacklist(refreshToken, refreshremainingMillis); if
not positive, skip blacklisting to avoid attempting to store a negative TTL in
Redis.
- Around line 26-47: In the withdrawal method of WithdrawalService, move the DB
deletion (memberRepository.deleteMemberAggregate(memberId)) to execute before
any Redis blacklist operations (refreshTokenStore.delete,
refreshTokenBlacklistStore.blacklist, accessTokenBlacklistStore.blacklist) so
the database removal occurs first; after successful delete, then compute
remaining validity via
jwtTokenParser.getRemainingValidityMillis(accessToken/refreshToken) and perform
the blacklist calls only when tokens are non-null and remainingMillis > 0, and
add error handling around memberRepository.deleteMemberAggregate to avoid
blacklisting if the DB delete fails (or implement a compensation step to remove
blacklist entries on later DB failure).
- Around line 38-39: WithdrawalService currently calls
jwtTokenParser.getRemainingValidityMillis(...) (which calls parseSignedClaims())
for accessToken and refreshToken without catching
io.jsonwebtoken.ExpiredJwtException, so an expired token will throw an unchecked
exception and roll back the `@Transactional` method; wrap each call to
getRemainingValidityMillis (both the accessToken and refreshToken usages in
WithdrawalService) in a try-catch that catches ExpiredJwtException, treat an
expired token as having zero (or minimal) remaining validity (e.g., set
remainingMillis = 0) and optionally log/debug the expiration, and do not rethrow
so the transaction is not rolled back; apply the same fix pattern to
LogoutService where the same method is used.
---
Nitpick comments:
In
`@src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportRepository.java`:
- Around line 47-48: The method signature deleteAllByMemberEntityId currently
uses the wrapper type Long; update it to use the primitive long to match
existing repository method signatures and maintain parameter type consistency in
HealthReportRepository (change deleteAllByMemberEntityId(Long memberId) to
deleteAllByMemberEntityId(long memberId)).
In
`@src/main/java/org/sopt/carena/member/adapter/out/persistence/MemberDeletionExecutor.java`:
- Around line 18-23: The deleteAll method in MemberDeletionExecutor currently
relies on memberJpaRepository.deleteById(memberId) which in Spring Data JPA 3.x
is a silent no-op for non-existent IDs; if you want explicit existence
validation, do not add it here—instead add a presence check in the service layer
(WithdrawalService) before calling MemberDeletionExecutor.deleteAll: call
memberJpaRepository.existsById(memberId) or load the Member in WithdrawalService
and throw the appropriate domain exception if missing, then invoke
MemberDeletionExecutor.deleteAll(memberId) to perform the deletions.
In
`@src/main/java/org/sopt/carena/member/application/port/in/WithdrawalUseCase.java`:
- Around line 3-5: The method signature in interface WithdrawalUseCase has no
space after the comma in withdrawal(Long memberId,String accessToken,String
refreshToken); — update the signature in WithdrawalUseCase so there is a space
after each comma (e.g., between memberId and String accessToken, and between
accessToken and String refreshToken) to match Java formatting conventions and
improve readability.
In
`@src/main/java/org/sopt/carena/recommend/adapter/out/persistence/repository/RecommendedMealRepository.java`:
- Around line 10-11: The parameter type for deleteAllByMemberEntityId is
inconsistent with findTopByMemberEntityIdOrderByCreatedAtDesc: change
deleteAllByMemberEntityId(Long memberId) to use the primitive type long
(deleteAllByMemberEntityId(long memberId)) so both repository methods use the
same parameter type and avoid unnecessary boxing/unboxing; update the method
signature in RecommendedMealRepository accordingly and adjust any callers if
needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37103315-df45-4c92-bd1a-32c9da9e9376
📒 Files selected for processing (13)
src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportEmbeddingRepository.javasrc/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportRepository.javasrc/main/java/org/sopt/carena/member/adapter/in/web/code/MemberSuccessCode.javasrc/main/java/org/sopt/carena/member/adapter/in/web/controller/MemberApiDocs.javasrc/main/java/org/sopt/carena/member/adapter/in/web/controller/MemberController.javasrc/main/java/org/sopt/carena/member/adapter/out/persistence/MemberDeletionExecutor.javasrc/main/java/org/sopt/carena/member/adapter/out/persistence/MemberPersistenceAdapter.javasrc/main/java/org/sopt/carena/member/adapter/out/persistence/RefreshTokenBlacklistStoreAdapter.javasrc/main/java/org/sopt/carena/member/application/port/in/WithdrawalUseCase.javasrc/main/java/org/sopt/carena/member/application/port/out/MemberPersistencePort.javasrc/main/java/org/sopt/carena/member/application/port/out/RefreshTokenBlacklistStore.javasrc/main/java/org/sopt/carena/member/application/service/WithdrawalService.javasrc/main/java/org/sopt/carena/recommend/adapter/out/persistence/repository/RecommendedMealRepository.java
| long remainingMillis = | ||
| jwtTokenParser.getRemainingValidityMillis(accessToken); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "WithdrawalService.java" -type fRepository: team-carena/carena-server
Length of output: 150
🏁 Script executed:
find . -name "JwtTokenParser.java" -type fRepository: team-carena/carena-server
Length of output: 152
🏁 Script executed:
find . -name "*Jwt*.java" -type f | head -20Repository: team-carena/carena-server
Length of output: 493
🏁 Script executed:
cat -n ./src/main/java/org/sopt/carena/member/application/service/WithdrawalService.javaRepository: team-carena/carena-server
Length of output: 2396
🏁 Script executed:
cat -n ./src/main/java/org/sopt/carena/member/application/service/util/JwtTokenParser.javaRepository: team-carena/carena-server
Length of output: 1773
🏁 Script executed:
rg "ExpiredJwtException" --type javaRepository: team-carena/carena-server
Length of output: 229
🏁 Script executed:
rg "parseSignedClaims|getRemainingValidityMillis" -A 3 --type javaRepository: team-carena/carena-server
Length of output: 2878
🏁 Script executed:
rg "catch.*ExpiredJwtException|catch.*JwtException" --type javaRepository: team-carena/carena-server
Length of output: 489
🏁 Script executed:
cat -n ./src/main/java/org/sopt/carena/member/application/service/util/JwtTokenValidator.javaRepository: team-carena/carena-server
Length of output: 1830
🏁 Script executed:
cat -n ./src/main/java/org/sopt/carena/member/application/service/LogoutService.javaRepository: team-carena/carena-server
Length of output: 1793
🏁 Script executed:
rg "withdrawal|logout" -i --type java | grep -i "service\|usecase" | head -10Repository: team-carena/carena-server
Length of output: 1448
만료된 토큰 파싱 시 ExpiredJwtException 예외 처리 누락
JwtTokenParser.getRemainingValidityMillis()는 내부적으로 parseSignedClaims()를 호출하며, 만료된 토큰에 대해 io.jsonwebtoken.ExpiredJwtException을 던집니다. 현재 코드에서 이 예외가 처리되지 않아 AccessToken 뿐만 아니라 RefreshToken 처리 시에도 (31-32줄) 예외가 발생하면 회원탈퇴 전체가 실패합니다. @Transactional이 선언된 메서드에서 unchecked exception이 발생하면 트랜잭션이 롤백됩니다.
AccessToken과 RefreshToken 모두에 대해 예외 처리를 추가하세요:
제안된 수정
// Access Token 블랙리스트 처리
- long remainingMillis =
- jwtTokenParser.getRemainingValidityMillis(accessToken);
-
- if (remainingMillis > 0) {
- accessTokenBlacklistStore
- .blacklist(accessToken, remainingMillis);
- }
+ try {
+ long remainingMillis =
+ jwtTokenParser.getRemainingValidityMillis(accessToken);
+ if (remainingMillis > 0) {
+ accessTokenBlacklistStore.blacklist(accessToken, remainingMillis);
+ }
+ } catch (io.jsonwebtoken.ExpiredJwtException e) {
+ // 만료된 토큰은 블랙리스트 불필요
+ log.debug("Access token already expired, skipping blacklist");
+ }참고: LogoutService도 동일한 문제를 가지고 있습니다.
📝 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.
| long remainingMillis = | |
| jwtTokenParser.getRemainingValidityMillis(accessToken); | |
| // Access Token 블랙리스트 처리 | |
| try { | |
| long remainingMillis = | |
| jwtTokenParser.getRemainingValidityMillis(accessToken); | |
| if (remainingMillis > 0) { | |
| accessTokenBlacklistStore.blacklist(accessToken, remainingMillis); | |
| } | |
| } catch (io.jsonwebtoken.ExpiredJwtException e) { | |
| // 만료된 토큰은 블랙리스트 불필요 | |
| log.debug("Access token already expired, skipping blacklist"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/sopt/carena/member/application/service/WithdrawalService.java`
around lines 38 - 39, WithdrawalService currently calls
jwtTokenParser.getRemainingValidityMillis(...) (which calls parseSignedClaims())
for accessToken and refreshToken without catching
io.jsonwebtoken.ExpiredJwtException, so an expired token will throw an unchecked
exception and roll back the `@Transactional` method; wrap each call to
getRemainingValidityMillis (both the accessToken and refreshToken usages in
WithdrawalService) in a try-catch that catches ExpiredJwtException, treat an
expired token as having zero (or minimal) remaining validity (e.g., set
remainingMillis = 0) and optionally log/debug the expiration, and do not rethrow
so the transaction is not rolled back; apply the same fix pattern to
LogoutService where the same method is used.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/carena/member/application/service/WithdrawalService.java`:
- Around line 31-45: The current single try-catch in WithdrawalService wraps
refresh token deletion/parsing and access token blacklisting so an exception
during refresh handling can skip accessTokenBlacklistStore.blacklist; change to
handle each token operation independently by adding separate try-catch blocks
around refreshTokenStore.delete /
jwtTokenParser.getRemainingValidityMillis(refreshToken) /
refreshTokenBlacklistStore.blacklist and another try-catch around
jwtTokenParser.getRemainingValidityMillis(accessToken) /
accessTokenBlacklistStore.blacklist (referencing methods
refreshTokenStore.delete, jwtTokenParser.getRemainingValidityMillis,
refreshTokenBlacklistStore.blacklist, accessTokenBlacklistStore.blacklist) so
failures in refresh-token processing don’t prevent access-token blacklisting and
each failure is logged individually.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23f243a3-8157-466c-ba0d-cd2f737d841e
📒 Files selected for processing (2)
src/main/java/org/sopt/carena/member/adapter/out/persistence/RefreshTokenBlacklistStoreAdapter.javasrc/main/java/org/sopt/carena/member/application/service/WithdrawalService.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/org/sopt/carena/member/adapter/out/persistence/RefreshTokenBlacklistStoreAdapter.java
| try { | ||
| refreshTokenStore.delete(memberId); | ||
|
|
||
| if (refreshToken != null) { | ||
| long remaining = jwtTokenParser.getRemainingValidityMillis(refreshToken); | ||
| if (remaining > 0) { | ||
| refreshTokenBlacklistStore.blacklist(refreshToken, remaining); | ||
| } | ||
| } | ||
|
|
||
| long remaining = jwtTokenParser.getRemainingValidityMillis(accessToken); | ||
| if (remaining > 0) { | ||
| accessTokenBlacklistStore.blacklist(accessToken, remaining); | ||
| } | ||
| } catch (Exception e) { |
There was a problem hiding this comment.
토큰 처리 전체를 하나의 try-catch로 묶어 Access 블랙리스트가 누락될 수 있습니다.
Line 31-45에서 앞 단계(예: refresh 토큰 파싱/삭제) 예외가 나면 뒤 단계(access 토큰 블랙리스트)가 실행되지 않습니다. 토큰 무효화는 단계별로 독립 실패 처리하는 편이 안전합니다.
♻️ 제안 수정
- try {
- refreshTokenStore.delete(memberId);
-
- if (refreshToken != null) {
- long remaining = jwtTokenParser.getRemainingValidityMillis(refreshToken);
- if (remaining > 0) {
- refreshTokenBlacklistStore.blacklist(refreshToken, remaining);
- }
- }
-
- long remaining = jwtTokenParser.getRemainingValidityMillis(accessToken);
- if (remaining > 0) {
- accessTokenBlacklistStore.blacklist(accessToken, remaining);
- }
- } catch (Exception e) {
- // Redis 장애 시에도 탈퇴는 완료로 처리 (토큰은 만료 시 자연 무효화, 탈퇴 회원 토큰은 API에서 차단되기 때문에 이렇게 판단함)
- log.warn("토큰 블랙리스트 처리 실패. memberId={}", memberId, e);
- }
+ try {
+ refreshTokenStore.delete(memberId);
+ } catch (Exception e) {
+ log.warn("refresh token 저장소 정리 실패. memberId={}", memberId, e);
+ }
+
+ if (refreshToken != null) {
+ try {
+ long remaining = jwtTokenParser.getRemainingValidityMillis(refreshToken);
+ if (remaining > 0) {
+ refreshTokenBlacklistStore.blacklist(refreshToken, remaining);
+ }
+ } catch (Exception e) {
+ log.warn("refresh token 블랙리스트 처리 실패. memberId={}", memberId, e);
+ }
+ }
+
+ if (accessToken != null) {
+ try {
+ long remaining = jwtTokenParser.getRemainingValidityMillis(accessToken);
+ if (remaining > 0) {
+ accessTokenBlacklistStore.blacklist(accessToken, remaining);
+ }
+ } catch (Exception e) {
+ log.warn("access token 블랙리스트 처리 실패. memberId={}", memberId, e);
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/sopt/carena/member/application/service/WithdrawalService.java`
around lines 31 - 45, The current single try-catch in WithdrawalService wraps
refresh token deletion/parsing and access token blacklisting so an exception
during refresh handling can skip accessTokenBlacklistStore.blacklist; change to
handle each token operation independently by adding separate try-catch blocks
around refreshTokenStore.delete /
jwtTokenParser.getRemainingValidityMillis(refreshToken) /
refreshTokenBlacklistStore.blacklist and another try-catch around
jwtTokenParser.getRemainingValidityMillis(accessToken) /
accessTokenBlacklistStore.blacklist (referencing methods
refreshTokenStore.delete, jwtTokenParser.getRemainingValidityMillis,
refreshTokenBlacklistStore.blacklist, accessTokenBlacklistStore.blacklist) so
failures in refresh-token processing don’t prevent access-token blacklisting and
each failure is logged individually.
Kyoung-M1N
left a comment
There was a problem hiding this comment.
deleteAllByMemberEntityId로 삭제 과정에서 어떤 쿼리문이 발생하는지 확인해보면 좋을 것 같아요!
현재 삭제 대상 테이블에 memberId가 존재하지만 JPA의 인터페이스가 객체를 대상으로 쿼리를 작성하기 때문에 삭제 과정에서 memberId를 찾기 위해 불필요한 join이 발생할 수 있어요!
There was a problem hiding this comment.
기존 기능들의 구현에서 adapter에 JPA나 Redis 같은 외부 벤더의 흐름이 드러나도록 되어있는데 deletionExecutor.deleteAll이 재사용될 여지가 없는데도 불구하고 별도의 Executor로 분리하신 이유가 궁금합니다!
There was a problem hiding this comment.
여러 연관 테이블을 삭제 순서를 고려하여 함께 정리해야 하는 로직이라 단일 Repository 호출과 성격이 조금 다르다고 생각했습니다. 그래서 Adapter 내부에 직접 구현하기보다는, 여러 Repository를 조합해 삭제 흐름을 관리하는 삭제하는 별도의 executor로 분리하는게 어떨까라고 생각을 하였습니다..!
There was a problem hiding this comment.
Access token과 Refresh token의 블랙리스팅을 별도로 처리하는 것보다 같은 객체가 블랙리스팅이라는 행위를 토큰에 대해 공통으로 하도록 하여 재사용성을 높이는 방법은 어떨까요?
객체가 내부적으로 어떤 종류의 토큰인지 PREFIX 등을 추가하여 구분하거나, 메서드 자체를 토큰의 종류별로 분리하는 방법, 외부에서 토큰의 종류를 파라미터로 전닫받는 방법에 대해서도 고민해보시면 좋을 것 같아요!
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportRepository.java (1)
50-52:@Param("memberId")를 명시적으로 추가하여 다른 메서드들과 일관성 유지프로젝트의 다른
@Query메서드들(예:RecommendedCategoryRepository,HealthReportEmbeddingRepository)은 모두 named parameter에 대해 명시적으로@Param어노테이션을 사용합니다. 이 메서드도 같은 패턴을 따르면 일관성이 있습니다.참고로 현재 Spring Boot 3.5.9의 Gradle 플러그인은 기본적으로 컴파일러
-parameters옵션을 활성화하므로 현재 코드도 작동하지만, 명시적 표기는 코드 의도를 명확히 하고 나중의 빌드 설정 변경에 대한 방어책이 됩니다.수정 예시
+import org.springframework.data.repository.query.Param; ... `@Modifying` `@Query`("DELETE FROM HealthReportEntity h WHERE h.memberEntity.id = :memberId") - void deleteAllByMemberEntityId(Long memberId); + void deleteAllByMemberEntityId(`@Param`("memberId") Long memberId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportRepository.java` around lines 50 - 52, Add an explicit `@Param`("memberId") annotation to the deleteAllByMemberEntityId method parameter to match the project's named-parameter style and other repository methods; locate the method deleteAllByMemberEntityId(Long memberId) in HealthReportRepository and annotate its parameter with `@Param`("memberId") so the `@Query` named parameter :memberId is bound explicitly and keeps consistency with RecommendedCategoryRepository and HealthReportEmbeddingRepository.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportRepository.java`:
- Around line 50-52: Add an explicit `@Param`("memberId") annotation to the
deleteAllByMemberEntityId method parameter to match the project's
named-parameter style and other repository methods; locate the method
deleteAllByMemberEntityId(Long memberId) in HealthReportRepository and annotate
its parameter with `@Param`("memberId") so the `@Query` named parameter :memberId is
bound explicitly and keeps consistency with RecommendedCategoryRepository and
HealthReportEmbeddingRepository.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12c7d1cd-bb07-4ab8-92b5-9623a7855661
📒 Files selected for processing (3)
src/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportEmbeddingRepository.javasrc/main/java/org/sopt/carena/healthreport/adapter/out/persistence/repository/HealthReportRepository.javasrc/main/java/org/sopt/carena/recommend/adapter/out/persistence/repository/RecommendedMealRepository.java
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 (2)
src/main/java/org/sopt/carena/global/config/security/JwtAuthenticationFilter.java (1)
75-88:⚠️ Potential issue | 🟠 Major블랙리스트 조회는 로컬 JWT 검증 뒤로 미루세요.
Line 75가 Line 86보다 먼저 실행돼서, 형식이 깨졌거나 이미 만료된 토큰도 먼저 Redis round-trip을 발생시킵니다. 인증 실패를 로컬에서 바로 걸러낼 수 있는 요청까지 외부 저장소에 의존하게 되므로, 악성 트래픽 시 Redis 부하와 장애 전파 범위가 불필요하게 커집니다.
♻️ 순서 조정 예시
- if (tokenBlacklistStore.isBlacklisted(accessToken, TokenType.ACCESS)) { - log.warn("블랙리스트 처리된 액세스 토큰"); - handlerExceptionResolver.resolveException( - request, - response, - null, - new InvalidTokenException() - ); - return; - } - if (!jwtTokenValidator.isValid(accessToken)) { log.warn("유효하지 않은 액세스 토큰"); handlerExceptionResolver.resolveException(request, response, null, new InvalidTokenException()); return; // 필터 체인 중단 } + + if (tokenBlacklistStore.isBlacklisted(accessToken, TokenType.ACCESS)) { + log.warn("블랙리스트 처리된 액세스 토큰"); + handlerExceptionResolver.resolveException( + request, + response, + null, + new InvalidTokenException() + ); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/carena/global/config/security/JwtAuthenticationFilter.java` around lines 75 - 88, In JwtAuthenticationFilter, reverse the current checks so jwtTokenValidator.isValid(accessToken) runs before tokenBlacklistStore.isBlacklisted(accessToken, TokenType.ACCESS); i.e., perform local JWT validation first and return/resolve InvalidTokenException on failure, and only if the token passes local validation call tokenBlacklistStore.isBlacklisted to avoid unnecessary Redis round-trips for malformed or expired tokens; update the control flow around jwtTokenValidator and tokenBlacklistStore checks (and corresponding handlerExceptionResolver.resolveException calls) accordingly.src/main/java/org/sopt/carena/member/application/service/LogoutService.java (1)
29-34:⚠️ Potential issue | 🟠 Major로그아웃 성공 여부를 Redis 블랙리스트 쓰기에 종속시키지 마세요.
Line 26에서 refresh token은 이미 삭제되는데, Line 30의 TTL 계산이나 Line 34의 Redis 저장이 실패하면 로그아웃 API 전체가 실패로 끝납니다. 이 PR의
src/main/java/org/sopt/carena/member/application/service/WithdrawalService.java처럼 블랙리스트 처리는 별도 예외 처리로 분리해서 best-effort로 두는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/carena/member/application/service/LogoutService.java` around lines 29 - 34, The logout flow in LogoutService currently depends on jwtTokenParser.getRemainingValidityMillis(...) and tokenBlacklistStore.blacklist(...) succeeding, which can cause the whole logout to fail if TTL calc or Redis write fails; change this to a best-effort blacklist by moving the remainingMillis calculation and tokenBlacklistStore.blacklist(accessToken, TokenType.ACCESS, remainingMillis) into a separate try-catch that catches and logs any exception (do not rethrow), so refresh-token deletion and the primary logout result are not affected by Redis failures; keep references to jwtTokenParser.getRemainingValidityMillis and tokenBlacklistStore.blacklist/TokenType.ACCESS to locate and update the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/carena/member/adapter/out/persistence/TokenBlacklistStoreAdapter.java`:
- Around line 24-27: The Redis key currently stores the raw JWT in
TokenBlacklistStoreAdapter (variable key built from PREFIX +
tokenType.name().toLowerCase() + ":" + token), which leaks credentials; instead
compute a fixed-length secure hash (e.g., SHA-256 hex) of the token and use
PREFIX + tokenType.name().toLowerCase() + ":" + <token-hash> as the key before
calling redisTemplate.opsForValue().set(..., Duration.ofMillis(ttlMillis));
apply the same hashing change to the other occurrence around the lines
referenced (the lookup/delete paths that build keys at lines ~35-37) so all key
construction consistently uses the token hash rather than the raw token.
---
Outside diff comments:
In
`@src/main/java/org/sopt/carena/global/config/security/JwtAuthenticationFilter.java`:
- Around line 75-88: In JwtAuthenticationFilter, reverse the current checks so
jwtTokenValidator.isValid(accessToken) runs before
tokenBlacklistStore.isBlacklisted(accessToken, TokenType.ACCESS); i.e., perform
local JWT validation first and return/resolve InvalidTokenException on failure,
and only if the token passes local validation call
tokenBlacklistStore.isBlacklisted to avoid unnecessary Redis round-trips for
malformed or expired tokens; update the control flow around jwtTokenValidator
and tokenBlacklistStore checks (and corresponding
handlerExceptionResolver.resolveException calls) accordingly.
In `@src/main/java/org/sopt/carena/member/application/service/LogoutService.java`:
- Around line 29-34: The logout flow in LogoutService currently depends on
jwtTokenParser.getRemainingValidityMillis(...) and
tokenBlacklistStore.blacklist(...) succeeding, which can cause the whole logout
to fail if TTL calc or Redis write fails; change this to a best-effort blacklist
by moving the remainingMillis calculation and
tokenBlacklistStore.blacklist(accessToken, TokenType.ACCESS, remainingMillis)
into a separate try-catch that catches and logs any exception (do not rethrow),
so refresh-token deletion and the primary logout result are not affected by
Redis failures; keep references to jwtTokenParser.getRemainingValidityMillis and
tokenBlacklistStore.blacklist/TokenType.ACCESS to locate and update the code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 281812ab-bd48-4f0f-8a69-0b5f0f4664aa
📒 Files selected for processing (8)
src/main/java/org/sopt/carena/global/config/security/JwtAuthenticationFilter.javasrc/main/java/org/sopt/carena/member/adapter/out/persistence/AccessTokenBlacklistStoreAdapter.javasrc/main/java/org/sopt/carena/member/adapter/out/persistence/TokenBlacklistStoreAdapter.javasrc/main/java/org/sopt/carena/member/application/port/out/AccessTokenBlacklistStore.javasrc/main/java/org/sopt/carena/member/application/port/out/TokenBlacklistStore.javasrc/main/java/org/sopt/carena/member/application/service/LogoutService.javasrc/main/java/org/sopt/carena/member/application/service/WithdrawalService.javasrc/main/java/org/sopt/carena/member/domain/TokenType.java
💤 Files with no reviewable changes (2)
- src/main/java/org/sopt/carena/member/adapter/out/persistence/AccessTokenBlacklistStoreAdapter.java
- src/main/java/org/sopt/carena/member/application/port/out/AccessTokenBlacklistStore.java
| String key = PREFIX + tokenType.name().toLowerCase() + ":" + token; | ||
|
|
||
| redisTemplate.opsForValue() | ||
| .set(key, "blacklisted", Duration.ofMillis(ttlMillis)); |
There was a problem hiding this comment.
원문 JWT를 Redis 키로 저장하지 마세요.
Line 24와 Line 35가 bearer credential 전체를 Redis key에 그대로 넣고 있습니다. 키는 운영 도구, dump/AOF, 모니터링에서 쉽게 노출되므로 유출 시 블랙리스트 자체가 토큰 저장소가 됩니다. 조회 가능성만 필요하니 키는 토큰 원문 대신 SHA-256 같은 고정 길이 해시로 구성하는 편이 안전합니다.
🔐 키 생성 방식 예시
- String key = PREFIX + tokenType.name().toLowerCase() + ":" + token;
+ String key = PREFIX + tokenType.name().toLowerCase() + ":" + hash(token);Also applies to: 35-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/sopt/carena/member/adapter/out/persistence/TokenBlacklistStoreAdapter.java`
around lines 24 - 27, The Redis key currently stores the raw JWT in
TokenBlacklistStoreAdapter (variable key built from PREFIX +
tokenType.name().toLowerCase() + ":" + token), which leaks credentials; instead
compute a fixed-length secure hash (e.g., SHA-256 hex) of the token and use
PREFIX + tokenType.name().toLowerCase() + ":" + <token-hash> as the key before
calling redisTemplate.opsForValue().set(..., Duration.ofMillis(ttlMillis));
apply the same hashing change to the other occurrence around the lines
referenced (the lookup/delete paths that build keys at lines ~35-37) so all key
construction consistently uses the token hash rather than the raw token.
🚀 Related issue
closes #51
#️⃣ Summary
🎯 Work Description
👍 동작 확인
💬 To Reviewers
회원 관련 데이터를 삭제하는 로직에서 불필요하게 발생하던
SELECT및JOIN쿼리를 제거했습니다.기존에는 엔티티를 조회한 후 삭제하는 방식으로 인해 추가적인 조회 쿼리가 발생했지만,
@Modifying+DELETE을 사용하여 바로 삭제 쿼리를 실행하도록 변경했습니다.이를 통해 불필요한 DB 조회를 제거하고 쿼리를 단순화했으며, 회원 삭제 시 DB 성능을 개선했습니다.
변경 전
SELECT)JOIN발생DELETE수행변경 후
DELETE쿼리를 사용하여 조회 없이 바로 삭제Summary by CodeRabbit
릴리스 노트
새로운 기능
리팩토링