Conversation
Walkthrough로그아웃 기능이 추가되었습니다. 액세스 토큰 블랙리스트(레디스) 저장·조회 로직과 토큰 폐기 흐름(revoke), 컨트롤러 엔드포인트(DELETE /api/v1/auth/logout), 보안 설정 및 필터의 블랙리스트 검사 통합이 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as AuthController
participant AuthSvc as AuthService
participant TokenSvc as TokenService
participant Redis as RedisService
participant Repo as RefreshTokenRepo
User->>Controller: DELETE /api/v1/auth/logout (Bearer accessToken, Cookie refreshToken)
activate Controller
Controller->>AuthSvc: logout(userId, request, response)
activate AuthSvc
AuthSvc->>TokenSvc: revokeTokens(request, response, userId)
activate TokenSvc
TokenSvc->>TokenSvc: resolveToken(request) -> accessToken
alt accessToken present & valid
TokenSvc->>TokenSvc: calc remainingMillis
alt remainingMillis > 0
TokenSvc->>Redis: addAccessTokenToBlacklist(accessToken, remainingMillis)
activate Redis
Redis-->>Redis: set blacklist key with TTL
deactivate Redis
end
end
TokenSvc->>Repo: delete refresh token by userId
TokenSvc->>TokenSvc: clearRefreshTokenCookie(response)
deactivate TokenSvc
AuthSvc-->>Controller: LogoutResponse(true)
deactivate AuthSvc
Controller-->>User: 200 OK + ApiResponse(LOGOUT_SUCCESS)
deactivate Controller
Note over TokenSvc,Redis: 향후 요청 시 TokenAuthenticationFilter가 Redis에서 블랙리스트 조회 후 인증 거부
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 추가로 주의해서 살필 항목:
Possibly related PRs
시 🎭
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/test/java/com/almang/inventory/user/auth/service/AuthServiceTest.java (1)
150-161: 로그아웃 테스트 구조가 잘 작성되었습니다.기존 테스트 패턴을 따르며 핵심 동작을 검증합니다. 다만, 토큰 폐기 중 예외 발생 시나리오도 고려하면 더 견고한 테스트가 될 수 있습니다. 예를 들어,
tokenService.revokeTokens()가 예외를 던질 때의 동작을 검증하는 테스트를 추가하는 것이 좋습니다.src/main/java/com/almang/inventory/user/auth/dto/response/LogoutResponse.java (1)
3-5: 간결한 DTO 구현입니다.Java record 사용은 적절하지만,
success필드가 항상true를 반환한다면 HTTP 상태 코드만으로 충분할 수 있습니다. 만약 향후 로그아웃 응답에 추가 정보(예: 로그아웃된 세션 수, 타임스탬프 등)를 포함할 계획이 있다면 현재 구조가 확장에 유리합니다. 그렇지 않다면ApiResponse<Void>형태도 고려해볼 만합니다.참고: 성공/실패를 명시적으로 표현하는 현재 방식도 API 일관성 측면에서 충분히 합리적입니다.
src/main/java/com/almang/inventory/global/security/jwt/TokenAuthenticationFilter.java (1)
73-90: 블랙리스트 검증 로직이 올바른 위치에 배치되었습니다.토큰 유효성 검증 후, 사용자 조회 전에 블랙리스트를 확인하는 것은 보안 측면에서 적절합니다. 다만, 모든 인증 요청마다 Redis 조회가 발생하므로 성능에 영향을 줄 수 있습니다.
개선 제안:
- Redis 응답 시간을 모니터링하여 병목이 발생하는지 확인하세요.
- 향후 트래픽이 증가하면 로컬 캐시 레이어(예: Caffeine Cache)를 추가로 도입하여 최근 검증된 토큰을 짧은 시간(예: 10초) 동안 캐싱하는 방안을 고려할 수 있습니다.
참고: Spring Cache와 Redis를 조합한 다층 캐싱 전략에 대해서는 Spring Cache Abstraction 문서를 참고하세요.
src/main/java/com/almang/inventory/user/auth/service/AuthService.java (1)
53-60: 로그아웃 로직이 명확하게 구현되었습니다.다만,
@Transactional어노테이션의 필요성을 재검토해보세요. 현재logout메서드는:
tokenService.revokeTokens()호출 (Redis 작업 수행)- 로그 출력 및 응답 반환
고려사항:
- Redis 작업은 Spring의 트랜잭션 관리 대상이 아닙니다.
- DB 작업이 없다면
@Transactional은 불필요한 트랜잭션 오버헤드를 발생시킵니다.- 향후 DB 작업(예: 로그아웃 이력 저장)을 추가할 계획이 있다면 유지하는 것이 좋습니다.
참고: Spring 트랜잭션 관리에 대해서는 Transaction Management 문서를 참고하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java(1 hunks)src/main/java/com/almang/inventory/global/config/security/SecurityConfig.java(2 hunks)src/main/java/com/almang/inventory/global/security/jwt/TokenAuthenticationFilter.java(3 hunks)src/main/java/com/almang/inventory/user/auth/controller/AuthController.java(3 hunks)src/main/java/com/almang/inventory/user/auth/dto/response/LogoutResponse.java(1 hunks)src/main/java/com/almang/inventory/user/auth/service/AuthService.java(2 hunks)src/main/java/com/almang/inventory/user/auth/service/RedisService.java(3 hunks)src/main/java/com/almang/inventory/user/auth/service/TokenService.java(1 hunks)src/test/java/com/almang/inventory/user/auth/controller/AuthControllerTest.java(3 hunks)src/test/java/com/almang/inventory/user/auth/service/AuthServiceTest.java(3 hunks)src/test/java/com/almang/inventory/user/auth/service/RedisServiceTest.java(2 hunks)src/test/java/com/almang/inventory/user/auth/service/TokenServiceTest.java(1 hunks)
🔇 Additional comments (11)
src/main/java/com/almang/inventory/global/api/SuccessMessage.java (1)
13-13: 깔끔한 메시지 추가입니다! 👍기존 패턴과 일관성 있게 로그아웃 성공 메시지가 추가되었습니다.
src/test/java/com/almang/inventory/user/auth/service/RedisServiceTest.java (1)
122-186: 블랙리스트 기능에 대한 테스트 커버리지가 우수합니다! 🎯새로 추가된 Redis 작업들에 대한 테스트가 포괄적으로 작성되었습니다:
- 정상 케이스와 엣지 케이스(TTL ≤ 0) 모두 커버
verifyNoInteractions사용으로 불필요한 호출이 없음을 명시적으로 검증- 블랙리스트 존재 여부에 대한 true/false 케이스 모두 검증
테스트 코드의 가독성과 완결성이 뛰어납니다.
src/test/java/com/almang/inventory/user/auth/controller/AuthControllerTest.java (1)
220-260: 로그아웃 엔드포인트 테스트가 완벽합니다! ✨성공 시나리오와 예외 처리 시나리오 모두를 검증하며, 기존 테스트 패턴과의 일관성도 유지하고 있습니다. DELETE 메서드 사용, 인증 컨텍스트 설정, 응답 구조 검증이 모두 적절합니다.
src/main/java/com/almang/inventory/user/auth/service/TokenService.java (1)
103-113: 쿠키 삭제 로직이 정확합니다.
Max-Age=0설정으로 쿠키를 삭제하며,setRefreshTokenCookie와 동일한 속성(httpOnly, secure, sameSite, path)을 사용하여 일관성을 유지합니다. 이는 브라우저가 올바른 쿠키를 식별하고 삭제할 수 있도록 보장합니다.참고: HTTP 쿠키 속성에 대한 자세한 내용은 MDN Cookie 문서를 참고하세요.
src/main/java/com/almang/inventory/global/config/security/SecurityConfig.java (1)
30-30: 깔끔한 의존성 주입입니다! 👍RedisService를 TokenAuthenticationFilter에 전달하여 블랙리스트 검증을 가능하게 하는 구조가 명확합니다. 생성자 주입 패턴을 일관되게 사용하고 있어 좋습니다.
Also applies to: 72-72
src/test/java/com/almang/inventory/user/auth/service/TokenServiceTest.java (3)
149-182: 탁월한 테스트 커버리지입니다! 🎯토큰 폐기 로직의 모든 주요 동작을 검증하고 있습니다:
- 액세스 토큰 블랙리스트 등록 (TTL 포함)
- 리프레시 토큰 삭제
- 쿠키 속성 검증 (Max-Age=0, HttpOnly, Secure, SameSite)
ArgumentCaptor를 활용한 Set-Cookie 헤더 검증도 정확합니다.
184-203: 중요한 엣지 케이스를 잘 포착했습니다! 🛡️Authorization 헤더가 없는 경우에도 리프레시 토큰 삭제와 쿠키 클리어가 정상 동작하는지 확인하는 것은 매우 중요합니다. 이를 통해 로그아웃이 부분적으로만 실패하는 상황을 방지할 수 있습니다.
205-224: 성능 최적화를 검증하는 스마트한 테스트입니다! ⚡만료된 토큰을 블랙리스트에 추가하지 않는 로직은 불필요한 Redis 연산을 줄이는 좋은 최적화입니다.
Mockito.never()를 사용하여 메서드가 호출되지 않음을 명확하게 검증하고 있습니다.src/main/java/com/almang/inventory/user/auth/controller/AuthController.java (1)
82-95: REST 원칙을 잘 따르는 로그아웃 엔드포인트입니다! 🎯구현이 깔끔합니다:
- DELETE 메서드 사용이 RESTful 관행에 부합
- 인증된 사용자만 로그아웃 가능하도록 보안 처리
- userId 로깅으로 감사 추적(audit trail) 확보
- 서비스 레이어로의 적절한 위임
다른 엔드포인트들과 일관된 응답 구조를 유지하고 있어 API 사용성이 좋습니다.
src/main/java/com/almang/inventory/user/auth/service/RedisService.java (2)
15-15: 일관된 네이밍 컨벤션을 유지하고 있습니다! 📝기존 코드의 패턴을 따라
ACCESS_BLACKLIST_PREFIX상수와blacklistAccessTokenKey헬퍼 메서드를 구현했습니다. 키 생성 로직의 캡슐화가 잘 되어 있어 유지보수성이 높습니다.Also applies to: 30-32
78-86: 똑똑한 TTL 관리 전략입니다! ⏱️블랙리스트 구현이 매우 효율적입니다:
- 자동 만료: 토큰의 남은 수명만큼만 블랙리스트에 유지하여 Redis 메모리를 효율적으로 사용
- 조기 종료:
remainingMillis <= 0체크로 이미 만료된 토큰에 대한 불필요한 Redis 연산 방지- 단순 검증:
hasKey를 사용한 간결한 블랙리스트 확인이 방식은 Spring Security와 Redis를 함께 사용하는 JWT 무효화의 모범 사례입니다.
💡 참고: Spring Data Redis의
hasKey()메서드는 키가 존재하지 않으면false를 반환하므로, 블랙리스트에 없는 토큰을 올바르게 처리합니다.
| public void revokeTokens(HttpServletRequest request, HttpServletResponse response, Long userId) { | ||
| String accessToken = resolveToken(request); | ||
|
|
||
| // 액세스 토큰 무효화 | ||
| if (accessToken != null && !accessToken.isBlank()) { | ||
| long remainMillis = jwtTokenProvider.getRemainingMillis(accessToken); | ||
| if (remainMillis > 0) { | ||
| redisService.addAccessTokenToBlacklist(accessToken, remainMillis); | ||
| } | ||
| } | ||
|
|
||
| // 리프레시 토큰 삭제 | ||
| redisService.deleteByUserId(userId.toString()); | ||
|
|
||
| // 리프레시 쿠키 삭제 | ||
| clearRefreshTokenCookie(response); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
토큰 폐기 로직이 전반적으로 잘 구현되었습니다.
다만, 액세스 토큰을 추출할 수 없는 경우(Line 86에서 accessToken이 null인 경우)에도 리프레시 토큰은 삭제됩니다. 이는 다음 시나리오를 고려해야 합니다:
잠재적 문제:
- 사용자가 유효한 액세스 토큰을 보유하고 있지만 요청 헤더에 포함하지 않고 로그아웃을 시도하는 경우
- 리프레시 토큰은 삭제되지만 액세스 토큰은 블랙리스트에 추가되지 않아 만료 시까지 사용 가능
개선 방안:
- 로그아웃 엔드포인트를 인증 필수로 만들어 항상 액세스 토큰이 존재하도록 보장
- 액세스 토큰이 없을 경우 예외를 발생시켜 명시적으로 실패 처리
현재 AuthController의 로그아웃 엔드포인트가 인증이 필요한지 확인이 필요합니다.
다음 스크립트로 로그아웃 엔드포인트의 인증 요구 사항을 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: Verify if logout endpoint requires authentication
# Check AuthController logout endpoint and SecurityConfig
echo "=== AuthController logout endpoint ==="
rg -nP -A10 'DELETE.*logout|@DeleteMapping.*logout' --type=java
echo -e "\n=== SecurityConfig authentication requirements ==="
rg -nP -A5 -B5 '/logout|permitAll|authenticated' --type=java -g '*SecurityConfig*'Length of output: 4190
보안 설정 모순으로 인한 토큰 폐기 로직 실패 위험
검증 결과, AuthController의 logout 엔드포인트는 @AuthenticationPrincipal로 인증을 요구하려 하지만, SecurityConfig의 .anyRequest().permitAll() (43줄)이 이를 무효화합니다. 결과적으로 인증되지 않은 사용자가 logout을 호출하면 userPrincipal이 null이 되어 88줄에서 NullPointerException 발생하고, 더 심각하게는 해당 사용자의 토큰이 전혀 폐기되지 않습니다.
즉시 조치 필요:
SecurityConfig.java의.anyRequest().permitAll()→.requestMatchers("/logout").authenticated()로 변경하여 logout 엔드포인트를 인증 필수로 강제 (또는 Spring Security의 인증 필터가 401을 반환하도록 설정)- 또는 명시적 에러 핸들링: userPrincipal이 null일 경우
BadCredentialsException발생시켜 실패 처리
참고: 현재 SecurityConfig의 주석 "추후 변경 예정"과 함께 이 보안 설정은 개발/테스트 환경 전용입니다. 프로덕션 배포 전에 필수로 수정하세요.
🤖 Prompt for AI Agents
In src/main/java/com/almang/inventory/user/auth/service/TokenService.java around
lines 85–101, the revokeTokens flow assumes an authenticated principal and can
NPE if logout is invoked without authentication; fix by enforcing authentication
on the logout endpoint in SecurityConfig (replace .anyRequest().permitAll() with
a rule that requires .requestMatchers("/logout").authenticated() or otherwise
ensure the security filter returns 401 for unauthenticated requests), and/or add
an explicit null check in revokeTokens to throw a BadCredentialsException (or
similar authentication exception) when the user principal is null so the method
fails safely without proceeding to token revocation; implement one or both fixes
before production.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/java/com/almang/inventory/store/global/config/TestSecurityConfig.java (1)
17-18: 로그아웃 엔드포인트 인증 강제 설정, 방향 좋습니다 👍
/api/v1/auth/logout에 대해authenticated()를 먼저 두고, 그 뒤에anyRequest().permitAll()을 배치한 구조가 Spring Security 매처 우선순위(먼저 매칭되는 룰이 적용)에 잘 맞습니다. 실제 보안 설정과 “로그아웃은 반드시 인증된 사용자만 호출”이라는 정책을 테스트 환경에서도 일관되게 가져가고 있어서 괜찮습니다.다만 개선 여지는 하나 있습니다:
- 현재 TestSecurityConfig 에서는 로그아웃만 인증을 요구하고 나머지는
permitAll()이라, 실제SecurityConfig와 전체 보안 정책이 다를 가능성이 큽니다. 이 PR 목적(로그아웃 로직 검증)에는 충분하지만, 장기적으로는
- 테스트에서 실제
SecurityConfig를 그대로 import 하거나,- 공용 보안 설정을 별도 설정 클래스로 추출한 뒤 prod/test 양쪽에서 재사용
하는 식으로 “테스트 환경의 보안 룰과 운영 환경의 룰을 최대한 동일하게 유지”하는 것이 좋습니다. 이렇게 해야 나중에 다른 인증/인가 관련 버그가 테스트에서 더 잘 잡힙니다. 관련 개념은 Spring Security 공식 문서의authorizeHttpRequests및 Testing 섹션을 참고하면 좋습니다.테스트에서 실제
SecurityConfig또는 공용 보안 설정을 재사용하기 어렵지는 않은지 한 번 검토해 주세요. 필요하다면 해당 구조 리팩터링 방향도 함께 논의해 볼 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/almang/inventory/global/config/security/SecurityConfig.java(3 hunks)src/test/java/com/almang/inventory/store/global/config/TestSecurityConfig.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/almang/inventory/global/config/security/SecurityConfig.java
1 participant
✨ 작업 내용
📝 적용 범위
/user/auth📌 참고 사항
Summary by CodeRabbit
새로운 기능
보안
테스트