Feat(#77): Access Token 블랙리스트 (Redis 기반) - #79
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds Redis configuration and runtime wiring, introduces a Redis-backed access-token blacklist, registers access tokens during logout, and rejects blacklisted tokens in ChangesRedis token blacklist
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AuthController
participant AuthService
participant TokenBlacklistService
participant JwtFilter
AuthController->>AuthService: pass logout request and refresh token
AuthService-->>AuthController: return bearer access token
AuthController->>TokenBlacklistService: store access token with remaining TTL
JwtFilter->>TokenBlacklistService: check access token membership
TokenBlacklistService-->>JwtFilter: return blacklist status
JwtFilter-->>JwtFilter: skip authentication for blacklisted token
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
ch0412
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/java/com/gold/safefam/domain/auth/service/AuthService.java`:
- Around line 139-148: Update logout() so tokenBlacklistService.blacklist() is
not executed before the refresh-token database transaction commits; register the
access-token blacklist operation for after-commit processing using the project’s
transaction synchronization, durable outbox, or retry mechanism, and ensure
failed post-commit processing is retried reliably while preserving both
revocations.
In `@src/main/java/com/gold/safefam/global/security/JwtFilter.java`:
- Around line 49-53: Update JwtFilter’s
tokenBlacklistService.isBlacklisted(token) path so Redis failures are handled
intentionally rather than escaping before the existing try block and
filterChain.doFilter flow. Configure explicit Redis connection/command timeouts
in the relevant Redis client configuration, and apply one consistent policy for
blacklist lookup failures—preferably fail-closed by rejecting the request, or
preserve a documented fail-open behavior—without allowing the exception to abort
valid JWT processing unexpectedly.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f0d539a8-9828-454a-bb2e-28241e801df4
📒 Files selected for processing (10)
.env.example.env.prod.examplebuild.gradledocker-compose.ymlsrc/main/java/com/gold/safefam/domain/auth/controller/AuthController.javasrc/main/java/com/gold/safefam/domain/auth/service/AuthService.javasrc/main/java/com/gold/safefam/global/security/JwtFilter.javasrc/main/java/com/gold/safefam/global/security/TokenBlacklistService.javasrc/main/resources/application-prod.ymlsrc/main/resources/application.yml
|
|
||
| // Access Token 블랙리스트 등록 | ||
| String bearerToken = httpRequest.getHeader("Authorization"); | ||
| if (bearerToken != null && bearerToken.startsWith("Bearer ")) { | ||
| String accessToken = bearerToken.substring(7); | ||
| if (jwtUtil.validateToken(accessToken)) { | ||
| long remaining = jwtUtil.getExpiration(accessToken).getTime() - System.currentTimeMillis(); | ||
| tokenBlacklistService.blacklist(accessToken, remaining); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== locate target files ==="
fd -a 'AuthService\.java$|TokenBlacklistService\.java$|RefreshToken|RefreshToken.*|JwtUtil\.java$' . | sed 's#^\./##' | head -100
echo "=== AuthService outline and relevant lines ==="
if [ -f src/main/java/com/gold/safefam/domain/auth/service/AuthService.java ]; then
wc -l src/main/java/com/gold/safefam/domain/auth/service/AuthService.java
sed -n '1,220p' src/main/java/com/gold/safefam/domain/auth/service/AuthService.java | cat -n
fi
echo "=== transactional/blacklist refresh token usages ==="
rg -n "blacklist|refresh|refreshToken|delete|remove|logout|`@Transactional`|TokenBlacklistService|jwtUtil" src/main/java || true
echo "=== service files with transaction annotations ==="
rg -n "`@Transactional`|class .*AuthService|interface TokenBlacklistService|class TokenBlacklistService" src/main/java || trueRepository: SafeFam/SafeFam_BE
Length of output: 33338
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== TokenBlacklistService ==="
sed -n '1,120p' src/main/java/com/gold/safefam/global/security/TokenBlacklistService.java | cat -n
echo "=== JwtFilter blacklist usage ==="
sed -n '1,120p' src/main/java/com/gold/safefam/global/security/JwtFilter.java | cat -n
echo "=== Outbox related files ==="
for f in $(fd -a 'Outbox.*|Event.*Service|Messaging.*' src/main/java/src/main/java 2>/dev/null || fd -a 'Outbox.*|Event.*Service|Messaging.*' src/main/java 2>/dev/null | head -50); do
echo "--- $f"
wc -l "$f"
rg -n "Outbox|publish|event|TransactionalEventListener|blacklist|logout|refresh" "$f" || true
done
echo "=== refresh token entity/repository ==="
sed -n '1,120p' src/main/java/com/gold/safefam/domain/auth/entity/RefreshToken.java | cat -n
sed -n '1,120p' src/main/java/com/gold/safefam/domain/auth/repository/RefreshTokenRepository.java | cat -n
echo "=== build deps for Redis/transaction outbox ==="
find . -maxdepth 4 \( -name 'pom.xml' -o -name 'build.gradle' -o, -name 'build.gradle.kts' \) -print | while read -r f; do
echo "--- $f"
rg -n "spring-data-redis|spring-boot-starter-data-redis|redis|transaction|outbox|messaging|kafka|oracle" "$f" || true
doneRepository: SafeFam/SafeFam_BE
Length of output: 15496
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== TokenBlacklistService ==="
sed -n '1，120p' src/main/java/com/gold/safefam/global/security/TokenBlacklist/TokenBlacklistService.java | cat -n
echo "=== Outbox related files ==="
fd -a 'Outbox.*|Event.*Service|Messaging.*' src/main/java 2>/dev/null | while read -r f; do
echo "--- $f"
rg -n "Outbox|publish|event|TransactionalEventListener|blacklist|logout|refresh" "$f" || true
done
echo "=== build deps for Redis/transaction outbox ==="
find . -maxdepth 4 \( -name 'pom.xml' -o -name 'build.gradle' -o -name 'build.gradle.kts' \) -print | while read -r f; do
echo "--- $f"
rg -n "spring-data-redis|spring-boot-starter-data-redis|redis|transaction|outbox|messaging|kafka|oracle" "$f" || true
doneRepository: SafeFam/SafeFam_BE
Length of output: 238
Define an atomic logout policy across Redis and the database.
logout() deletes the refresh token in the DB transaction, but tokenBlacklistService.blacklist() writes Redis before commit. If the database commit fails after the Redis write, the refresh token can remain usable while the access token is blacklisted. Use an after-commit mechanism, such as a durable outbox or explicit retry policy, so both revocation records can be processed reliably.
🤖 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/java/com/gold/safefam/domain/auth/service/AuthService.java` around
lines 139 - 148, Update logout() so tokenBlacklistService.blacklist() is not
executed before the refresh-token database transaction commits; register the
access-token blacklist operation for after-commit processing using the project’s
transaction synchronization, durable outbox, or retry mechanism, and ensure
failed post-commit processing is retried reliably while preserving both
revocations.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/backend-ci.yml (1)
22-32: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winTest the authenticated Redis contract in CI.
docker-compose.ymlstarts Redis with--requirepass, but this workflow starts Redis without authentication and setsREDIS_PASSWORDto an empty value. CI therefore cannot detect an incorrect or missing Redis password insrc/main/resources/application-prod.yml. Configure the CI Redis instance and health check with a CI-only password, then pass the same value to the Gradle step, or add a separate authenticated integration test.Also applies to: 53-56
🤖 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 @.github/workflows/backend-ci.yml around lines 22 - 32, Update the services.redis CI configuration to start Redis with a non-empty CI-only password and make its health check authenticate using that same password. Pass the identical value as REDIS_PASSWORD to the Gradle test step so application-prod.yml is validated against authenticated Redis.
🤖 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/java/com/gold/safefam/domain/auth/controller/AuthController.java`:
- Around line 116-120: Update the logout flow around AuthController and
tokenBlacklistService.blacklist so blacklist registration is durable after
authService.logout commits. Enqueue the access-token blacklist operation in an
after-commit outbox or equivalent reliable retry mechanism, and process retries
until success so a blacklist failure cannot leave the token usable or require
the client to retry logout.
- Line 119: The TokenBlacklistService must not use the raw bearer token in Redis
keys. Update both blacklist and isBlacklisted to hash the token and construct
the key with the resulting digest, preserving consistent hashing so blacklist
lookups continue to work; the AuthController call should remain unchanged.
---
Nitpick comments:
In @.github/workflows/backend-ci.yml:
- Around line 22-32: Update the services.redis CI configuration to start Redis
with a non-empty CI-only password and make its health check authenticate using
that same password. Pass the identical value as REDIS_PASSWORD to the Gradle
test step so application-prod.yml is validated against authenticated Redis.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b54184e-bd90-4705-9320-528beeef5d66
📒 Files selected for processing (5)
.github/workflows/backend-ci.ymlsrc/main/java/com/gold/safefam/domain/auth/controller/AuthController.javasrc/main/java/com/gold/safefam/domain/auth/service/AuthService.javasrc/main/java/com/gold/safefam/global/security/JwtFilter.javasrc/test/resources/application-test.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gold/safefam/global/security/JwtFilter.java
| String accessToken = authService.logout(userId, request.refreshToken(), httpRequest); | ||
| if (accessToken != null && jwtUtil.validateToken(accessToken)) { | ||
| long remaining = jwtUtil.getExpiration(accessToken).getTime() - System.currentTimeMillis(); | ||
| tokenBlacklistService.blacklist(accessToken, remaining); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
Make blacklist registration recoverable after the database commit.
When this code receives the result from authService.logout(...), the refresh-token deletion has already committed. If tokenBlacklistService.blacklist(...) fails, the access token remains usable. The deleted refresh token prevents the client from retrying logout to create the missing blacklist entry. Use a durable after-commit outbox or a reliable retry mechanism.
🤖 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/java/com/gold/safefam/domain/auth/controller/AuthController.java`
around lines 116 - 120, Update the logout flow around AuthController and
tokenBlacklistService.blacklist so blacklist registration is durable after
authService.logout commits. Enqueue the access-token blacklist operation in an
after-commit outbox or equivalent reliable retry mechanism, and process retries
until success so a blacklist failure cannot leave the token usable or require
the client to retry logout.
There was a problem hiding this comment.
Known limitation, outbox 패턴으로 개선 예정
There was a problem hiding this comment.
@ch0412, 확인했습니다. 이 PR의 범위에서는 해당 제한 사항을 유지하겠습니다.
후속 작업으로 GitHub 이슈를 생성할까요? 이슈에는 트랜잭션 커밋 후 블랙리스트 등록을 위한 outbox 처리, 재시도, 그리고 Redis 장애 후 복구 검증을 포함하겠습니다.
You are interacting with an AI system.
1 participant
📋 작업 내용
🧪 테스트 결과
🔗 관련 이슈
Closes #77
✅ 체크리스트
Summary by CodeRabbit
New Features
Bug Fixes