[Refactor] redis 적용 #91
Conversation
- 리플레시 토큰 저장을 db가 아닌 레디스 이용
- 레디스 적용 - 레디스 사용으로 인한 코드 수정
WalkthroughAdds Redis configuration and dependencies, updates JWT key handling, migrates refresh-token storage from DB to Redis, adjusts AuthService to use Redis for refresh token validation and issuance, and modifies AuthController to return the token DTO in reissue responses. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant AC as AuthController
participant AS as AuthService
participant R as Redis
participant J as JwtProvider
C->>AC: POST /auth/reissue (refreshToken)
AC->>AS: reissueToken(request)
AS->>J: validate & parse refresh token
J-->>AS: claims (userId, version, exp)
AS->>R: GET refreshToken:{userId}
alt token missing or mismatch
AS-->>AC: throw TOKEN_INVALID
AC-->>C: error response
else valid
AS->>J: generate new access/refresh tokens
J-->>AS: TokenDto
AS->>R: SET refreshToken:{userId} = newToken EX ms TTL
AS-->>AC: TokenDto
AC-->>C: onSuccess(code, TokenDto)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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: 2
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/naughty/tuzamate/auth/service/AuthService.java (2)
36-52: Logout: (1) isValid result ignored (expiration not enforced), (2) deletes from DB instead of Redis
- Ignoring isValid means expired tokens may proceed until getUserId; parsing doesn’t enforce exp. Check and throw on invalid.
- After migration, delete the Redis key, not the DB row.
public void logout(String bearer) { @@ - jwtProvider.isValid(token); + if (!jwtProvider.isValid(token)) { + throw new CustomException(JwtErrorCode.TOKEN_INVALID); + } @@ - refreshTokenRepository.deleteById(userId); + redisTemplate.delete("refreshToken:" + userId); }Also remove the unused repository dependency/import:
-import naughty.tuzamate.auth.repository.RefreshTokenRepository; @@ - private final RefreshTokenRepository refreshTokenRepository;
54-64: Reissue: enforce expiration check (isValid result ignored)Ensure expired/invalid refresh tokens are rejected.
- jwtProvider.isValid(token); // 토큰 유효성 검사 + if (!jwtProvider.isValid(token)) { // 토큰 유효성 검사 + throw new CustomException(JwtErrorCode.TOKEN_INVALID); + }
🧹 Nitpick comments (7)
build.gradle (1)
37-39: Redis dep: good; JSR-310 module likely redundant under Spring Boot 3.3Boot already brings jackson-datatype-jsr310 via spring-boot-starter-json. Consider removing the explicit dependency to avoid duplication.
- implementation 'com.fasterxml.jackson.datatype:jackson-datatype-jsr310'src/main/java/naughty/tuzamate/auth/jwt/JwtProvider.java (1)
31-31: BASE64 secret requirement: fail-fast and clearer errorDecoding assumes JWT_SECRET is Base64. Add validation to throw a clear startup error if misconfigured.
- this.secret = Keys.hmacShaKeyFor(Decoders.BASE64.decode(secret)); + try { + this.secret = Keys.hmacShaKeyFor(Decoders.BASE64.decode(secret)); + } catch (IllegalArgumentException ex) { + throw new IllegalStateException("JWT_SECRET must be a Base64-encoded HMAC key (e.g., 256-bit).", ex); + }src/main/java/naughty/tuzamate/global/config/RedisConfig.java (2)
10-11: Remove unused importValueOperations isn’t used here.
-import org.springframework.data.redis.core.ValueOperations;
16-21: Consider password/SSL supportIf targeting managed Redis (e.g., ElastiCache), add optional password and SSL config.
src/main/java/naughty/tuzamate/auth/service/AuthService.java (1)
65-71: Key prefix duplicationExtract "refreshToken:" into a constant to avoid typos and ease future changes.
+ private static final String RT_PREFIX = "refreshToken:"; @@ - String storedRefreshToken = redisTemplate.opsForValue().get("refreshToken:" + userId); + String storedRefreshToken = redisTemplate.opsForValue().get(RT_PREFIX + userId); @@ - "refreshToken:" + userId, + RT_PREFIX + userId,src/main/java/naughty/tuzamate/auth/service/RefreshTokenService.java (2)
3-4: Clean up unused/internal importsRemove unused imports and the Netty internal class.
-import io.netty.util.internal.StringUtil; -import jakarta.annotation.PostConstruct; +import jakarta.annotation.PostConstruct; @@ -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.data.redis.core.RedisTemplate; -import org.springframework.data.redis.core.ValueOperations; +import org.springframework.data.redis.core.RedisTemplate; +import org.springframework.data.redis.core.ValueOperations; @@ -import org.springframework.util.StringUtils; +import org.springframework.util.StringUtils; @@ -import javax.swing.text.html.Option; -import java.time.Duration; +import java.time.Duration;Also applies to: 9-11, 14-16, 16-18
35-39: Unify key prefix across servicesDefine a shared constant (e.g., in a Keys class) to avoid drift with AuthService.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
build.gradle(2 hunks)src/main/java/naughty/tuzamate/auth/controller/AuthController.java(1 hunks)src/main/java/naughty/tuzamate/auth/jwt/JwtProvider.java(3 hunks)src/main/java/naughty/tuzamate/auth/service/AuthService.java(3 hunks)src/main/java/naughty/tuzamate/auth/service/RefreshTokenService.java(1 hunks)src/main/java/naughty/tuzamate/global/config/RedisConfig.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/naughty/tuzamate/auth/controller/AuthController.java (2)
src/main/java/naughty/tuzamate/auth/dto/TokenResponse.java (3)
TokenResponse(5-19)TokenDto(7-17)TokenDto(13-16)src/main/java/naughty/tuzamate/auth/controller/KakaoController.java (1)
RestController(22-71)
src/main/java/naughty/tuzamate/auth/service/AuthService.java (3)
src/main/java/naughty/tuzamate/auth/repository/RefreshTokenRepository.java (1)
RefreshTokenRepository(8-15)src/main/java/naughty/tuzamate/auth/entity/RefreshToken.java (2)
RefreshToken(8-34)update(30-32)src/main/java/naughty/tuzamate/domain/user/service/UserServiceImpl.java (1)
UserServiceImpl(24-85)
src/main/java/naughty/tuzamate/auth/service/RefreshTokenService.java (4)
src/main/java/naughty/tuzamate/auth/jwt/JwtProvider.java (1)
Slf4j(20-98)src/main/java/naughty/tuzamate/domain/user/service/UserServiceImpl.java (1)
Service(24-85)src/main/java/naughty/tuzamate/auth/repository/RefreshTokenRepository.java (3)
RefreshTokenRepository(8-15)findByToken(10-10)deleteByUserId(12-12)src/main/java/naughty/tuzamate/auth/entity/RefreshToken.java (2)
RefreshToken(8-34)update(30-32)
🔇 Additional comments (7)
src/main/java/naughty/tuzamate/auth/jwt/JwtProvider.java (1)
75-84: Parser changes LGTMverifyWith/parseSignedClaims usage matches JJWT 0.12.x.
src/main/java/naughty/tuzamate/global/config/RedisConfig.java (1)
22-46: Bean setup looks correct for String key/value opsLettuce + String serializers are appropriate for the current usage.
src/main/java/naughty/tuzamate/auth/controller/AuthController.java (1)
52-58: Cookie attributes: verify they match your client’s cross-site needsIf the app is cross-site (separate frontend domain), you’ll likely need SameSite=None; Secure and client fetch with credentials. Current Lax may block cookie on XHR.
src/main/java/naughty/tuzamate/auth/service/AuthService.java (1)
27-31: StringRedisTemplate is auto-configured
Spring Boot’s RedisAutoConfiguration will still register a defaultStringRedisTemplatebean (it only skips it if there’s already a bean named “stringRedisTemplate”), so your customRedisTemplatebean doesn’t interfere.src/main/java/naughty/tuzamate/auth/service/RefreshTokenService.java (3)
27-33: ValueOperations init: OKPostConstruct initialization is fine.
41-46: Getter: concise and safeReturning Optional based on hasText is good.
49-52: Delete by key: LGTMMatches the chosen key strategy.
|
|
||
| response.addHeader(HttpHeaders.SET_COOKIE, cookie.toString()); | ||
| return CustomResponse.onSuccess(AuthSuccessCode.ACCESS_TOKEN_REISSUE_SUCCESS_CODE); | ||
| return CustomResponse.onSuccess(AuthSuccessCode.ACCESS_TOKEN_REISSUE_SUCCESS_CODE, tokenDto); |
There was a problem hiding this comment.
Bug: maxAge expects seconds (you’re passing millis) → cookie lives 1000× longer
Pass a Duration based on milliseconds.
- return CustomResponse.onSuccess(AuthSuccessCode.ACCESS_TOKEN_REISSUE_SUCCESS_CODE, tokenDto);
+ return CustomResponse.onSuccess(AuthSuccessCode.ACCESS_TOKEN_REISSUE_SUCCESS_CODE, tokenDto);Apply this diff above where the cookie is built:
+import java.time.Duration;
@@
- ResponseCookie cookie = ResponseCookie.from("refreshToken", tokenDto.getRefreshToken())
+ ResponseCookie cookie = ResponseCookie.from("refreshToken", tokenDto.getRefreshToken())
.httpOnly(true)
.secure(true) // 테스트 시 false
.path("/")
.sameSite("Lax")
- .maxAge(jwtProvider.getRefreshExpiration())
+ .maxAge(Duration.ofMillis(jwtProvider.getRefreshExpiration()))
.build();📝 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.
| return CustomResponse.onSuccess(AuthSuccessCode.ACCESS_TOKEN_REISSUE_SUCCESS_CODE, tokenDto); | |
| ++ b/src/main/java/naughty/tuzamate/auth/controller/AuthController.java | |
| @@ | |
| import java.time.Duration; | |
| @@ | |
| ResponseCookie cookie = ResponseCookie.from("refreshToken", tokenDto.getRefreshToken()) | |
| .httpOnly(true) | |
| .secure(true) // 테스트 시 false | |
| .path("/") | |
| .sameSite("Lax") | |
| .maxAge(Duration.ofMillis(jwtProvider.getRefreshExpiration())) | |
| .build(); | |
| @@ | |
| return CustomResponse.onSuccess(AuthSuccessCode.ACCESS_TOKEN_REISSUE_SUCCESS_CODE, tokenDto); |
🤖 Prompt for AI Agents
In src/main/java/naughty/tuzamate/auth/controller/AuthController.java around
line 61, the cookie maxAge is being set using milliseconds (so it ends up 1000×
too long); change the cookie builder to pass a java.time.Duration created from
the millisecond value (e.g., use Duration.ofMillis(yourMillis) for the maxAge
parameter) so the cookie lifetime is interpreted correctly.
| public void saveRefreshToken(Long userId, String token, LocalDateTime expire) { | ||
| String key = "refreshToken:" + userId; | ||
| Duration ttl = Duration.between(LocalDateTime.now(), expire); | ||
| valueOperations.set(key, token, ttl); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
TTL calc may be wrong across time zones; add guard for negative/zero TTL
expire is LocalDateTime (no zone). If server default zone differs from where expire was computed (e.g., Asia/Seoul), TTL can skew. Also guard negative TTL.
- Duration ttl = Duration.between(LocalDateTime.now(), expire);
- valueOperations.set(key, token, ttl);
+ Duration ttl = Duration.between(LocalDateTime.now(), expire);
+ if (!ttl.isNegative() && !ttl.isZero()) {
+ valueOperations.set(key, token, ttl);
+ } else {
+ // Expired or invalid TTL: ensure token isn’t stored
+ redisTemplate.delete(key);
+ }Follow-up (recommended): change the method to accept Instant expireAt and compute Duration.between(Instant.now(), expireAt) to avoid zone issues.
📝 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.
| public void saveRefreshToken(Long userId, String token, LocalDateTime expire) { | |
| String key = "refreshToken:" + userId; | |
| Duration ttl = Duration.between(LocalDateTime.now(), expire); | |
| valueOperations.set(key, token, ttl); | |
| } | |
| public void saveRefreshToken(Long userId, String token, LocalDateTime expire) { | |
| String key = "refreshToken:" + userId; | |
| Duration ttl = Duration.between(LocalDateTime.now(), expire); | |
| if (!ttl.isNegative() && !ttl.isZero()) { | |
| valueOperations.set(key, token, ttl); | |
| } else { | |
| // Expired or invalid TTL: ensure token isn’t stored | |
| redisTemplate.delete(key); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/naughty/tuzamate/auth/service/RefreshTokenService.java around
lines 35 to 39, the TTL is computed from LocalDateTime which is zone-ambiguous
and may produce negative/incorrect durations; update the method to use a
zoned/instant-aware calculation and guard against non-positive TTLs. Replace
Duration.between(LocalDateTime.now(), expire) with a computation that converts
the provided expire to an Instant (or accept an Instant expireAt parameter) and
use Duration.between(Instant.now(), expireAt), then check if the resulting
duration is positive before calling valueOperations.set; if duration is zero or
negative, skip saving (or delete/expire immediately) and log/handle the case
appropriately. Consider the follow-up: change the method signature to accept
Instant expireAt to avoid zone issues entirely.
1 participant
📍 PR 타입 (하나 이상 선택)
🏷️ 관련 이슈
Close #
📌 개요
🔁 변경 사항
📸 스크린샷
✅ 체크리스트
💡 추가 사항 (리뷰어가 참고해야 할 것)
Summary by CodeRabbit
New Features
Refactor
Chores