[BUG] 토큰 만료 API 수정 #281
[BUG] 토큰 만료 API 수정 #281
Conversation
📝 WalkthroughWalkthrough소셜 로그인(kakao, Apple, Naver) 엔드포인트와 로그아웃/탈퇴/연결해제(revoke) 로직이 추가되었고, Redis 기반 리프레시 토큰 저장소 및 토큰 블랙리스트가 도입되어 리프레시 재발급·로그아웃·탈퇴 흐름이 재구성되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User as "사용자"
participant Client as "클라이언트"
participant AuthService as "AuthService"
participant JwtService as "JwtService"
participant Redis as "Redis"
User->>Client: 리프레시로 토큰 재발급 요청 (Authorization header + refresh token)
Client->>AuthService: reissueToken(header)
AuthService->>JwtService: getUserIdFromToken(header) / validateToken
JwtService->>Redis: GET refresh_token:{userId}
Redis-->>JwtService: storedRefreshToken
JwtService-->>AuthService: storedRefreshToken 확인
AuthService->>JwtService: generateAccessToken(userId)
JwtService-->>AuthService: newAccessToken
AuthService->>JwtService: generateRefreshToken(userId)
JwtService->>Redis: SET refresh_token:{userId} = newRefreshToken (TTL)
Redis-->>JwtService: OK
AuthService->>JwtService: getRemainingExpiration(oldRefreshToken)
JwtService-->>AuthService: remainingTTL
alt remainingTTL > 0
AuthService->>JwtService: blacklistToken(oldRefreshToken, remainingTTL)
JwtService->>Redis: SET blacklist:{hash(oldRefreshToken)} = true (TTL)
Redis-->>JwtService: OK
end
AuthService-->>Client: { accessToken, refreshToken }
sequenceDiagram
actor User as "사용자"
participant Client as "클라이언트"
participant AuthService as "AuthService"
participant ExternalOAuth as "ExternalOAuth (Kakao/Apple/Naver)"
participant UserRepo as "UserRepo"
participant JwtService as "JwtService"
participant Redis as "Redis"
User->>Client: 소셜 로그인 요청 (code / provider token)
Client->>AuthService: socialLogin(...)
AuthService->>ExternalOAuth: 프로필/토큰 검증 요청
ExternalOAuth-->>AuthService: profile (id, email, name)
AuthService->>UserRepo: upsertUser(profile)
UserRepo-->>AuthService: User
AuthService->>JwtService: generateAccessToken(userId)
JwtService-->>AuthService: accessToken
AuthService->>JwtService: generateRefreshToken(userId)
JwtService->>Redis: SET refresh_token:{userId} = refreshToken (TTL)
Redis-->>JwtService: OK
AuthService-->>Client: { accessToken, refreshToken, nextStep }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/auth/service/AuthService.java`:
- Around line 196-213: Call jwtService.validateToken(token) explicitly at the
start of logout (after stripping "Bearer ") to make intent clear before any
parsing, then call jwtService.extractUserId(token) as before; after validation,
check for prior blacklisting (e.g., add and use a
jwtService.isBlacklisted(token) or equivalent) and return early if already
blacklisted to avoid duplicate logout work, otherwise proceed to compute
jwtService.getRemainingExpiration(token) and jwtService.blacklistToken(token,
remainingExpiration) and finally jwtService.deleteRefreshToken(userId).
In `@src/main/java/com/hrr/backend/domain/auth/service/JwtService.java`:
- Around line 154-158: The validateRefreshToken method can throw an NPE if
refreshToken is null; change the comparison to a null-safe check (e.g., use
Objects.equals(refreshToken, storedToken) or explicitly null-check both values)
inside validateRefreshToken so it returns false when either token is null or
they don't match; update references to REFRESH_TOKEN_PREFIX + userId and
storedToken as needed and ensure the method still returns a boolean.
In `@src/test/java/com/hrr/backend/domain/auth/service/JwtServiceTest.java`:
- Around line 71-80: The test JwtServiceTest.validateRefreshTokenFail declares
an unused variable realToken which is unnecessary; either remove the realToken
declaration or use it in the assertion to make the test intent clear. Update the
test method (validateRefreshTokenFail) to eliminate the unused realToken
variable if the test only needs fakeToken, or change the assertion to compare
behavior between realToken and fakeToken via
jwtService.validateRefreshToken(realToken, TEST_USER_ID) as appropriate.
- Around line 30-34: The test profile lacks Redis configuration so tests try to
connect to localhost:6379; add proper Redis test settings in
application-test.yml or spin up a Testcontainers/embedded Redis for tests and
point the test profile to it, remove the unused realToken variable in
JwtServiceTest, replace the hardcoded "30 minutes" usage with the configured
constant (use the same config field used by JwtService, e.g.,
accessTokenValidity / 60000) instead of a literal, and stop using
redisTemplate.keys("*") in JwtServiceTest#setUp — switch to a non-blocking
cleanup strategy or isolate tests to a dedicated test Redis instance and clear
only known test keys or use flushdb on the test instance.
🧹 Nitpick comments (5)
src/main/java/com/hrr/backend/domain/auth/service/JwtService.java (1)
127-129: Boolean 언박싱 주의
redisTemplate.hasKey()는Boolean객체를 반환하며, Redis 연결 문제 시null이 반환될 수 있습니다.Boolean.TRUE.equals()를 사용하면 더 안전합니다.♻️ 안전한 null 처리
public boolean isTokenBlacklisted(String token) { - return redisTemplate.hasKey(BLACKLIST_PREFIX + token); + return Boolean.TRUE.equals(redisTemplate.hasKey(BLACKLIST_PREFIX + token)); }src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
188-193: 예외 로깅 시 스택 트레이스 전체 노출 주의
log.error("네이버 로그인 중 오류 발생: ", e)에서 전체 예외 스택을 로깅합니다. 운영 환경에서는 민감한 정보가 포함될 수 있으니, 필요한 정보만 로깅하거나 로그 레벨 정책을 확인하세요.src/test/java/com/hrr/backend/domain/auth/service/JwtServiceTest.java (2)
96-105: 테스트 의도 중복 확인 필요
blacklistedTokenIsInvalid테스트가blacklistToken테스트와 동일한 검증을 수행합니다. 테스트명에 따르면 블랙리스트된 토큰이 "검증 시 무효 처리"되어야 하는데, 실제로는validateToken호출 없이isTokenBlacklisted만 확인합니다.♻️ 테스트 의도에 맞는 검증 추가
`@Test` `@DisplayName`("블랙리스트에 등록된 토큰은 검증 시 무효 처리") void blacklistedTokenIsInvalid() { // given String accessToken = jwtService.generateAccessToken(TEST_USER_ID); jwtService.blacklistToken(accessToken, Duration.ofMinutes(5)); // when & then assertThat(jwtService.isTokenBlacklisted(accessToken)).isTrue(); + // Note: validateToken 자체는 블랙리스트를 체크하지 않으므로, + // 실제 요청 처리 시 isTokenBlacklisted를 먼저 확인하는 로직이 필요합니다. }참고:
validateToken이 블랙리스트를 체크하지 않는다면, 이 테스트는 의미가 약합니다. Filter/Interceptor 레벨에서 블랙리스트 체크가 이루어지는지 확인하세요.
134-146: 하드코딩된 만료 시간 가정
assertThat(remaining.toMinutes()).isLessThanOrEqualTo(30)에서 30분이라는 값이 하드코딩되어 있습니다. 설정값이 변경되면 테스트가 실패할 수 있으니, 가능하다면 설정값을 주입받아 검증하세요.src/test/java/com/hrr/backend/domain/auth/service/AuthServiceTest.java (1)
112-123: 테스트 검증 범위 확장 고려
cannotUseBlacklistedAccessToken테스트는 블랙리스트 등록 여부만 확인합니다. 실제로 블랙리스트된 토큰으로 API 요청 시 거부되는지는 통합 테스트나 Filter 레벨 테스트에서 검증이 필요합니다.
src/test/java/com/hrr/backend/domain/auth/service/JwtServiceTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/auth/service/AuthService.java`:
- Around line 80-83: The blacklist check in AuthService is inverted: currently
it throws when a token is NOT blacklisted; change the condition so it throws
only when jwtService.isTokenBlacklisted(refreshToken) is true (i.e., remove the
negation). Update the conditional in the method containing that snippet
(AuthService token/refresh handling) to call
jwtService.isTokenBlacklisted(refreshToken) and throw new
GlobalException(ErrorCode.AUTH_INVALID_TOKEN) only when that returns true so
blacklisted refresh tokens are rejected and normal tokens pass.
In `@src/main/java/com/hrr/backend/domain/auth/service/JwtService.java`:
- Around line 127-129: The isTokenBlacklisted method currently returns the
result of redisTemplate.hasKey(...) which can be null and cause NPE on unboxing;
change the implementation to defensively handle a nullable Boolean by comparing
it to Boolean.TRUE (e.g., use
Boolean.TRUE.equals(redisTemplate.hasKey(BLACKLIST_PREFIX + token))) so the
method always returns a primitive boolean without risk of NPE and still
correctly indicates blacklist membership.
In `@src/test/java/com/hrr/backend/domain/auth/service/JwtServiceTest.java`:
- Around line 106-115: The test's DisplayName claims "blacklisted token is
invalid during validation" but the test only calls
jwtService.isTokenBlacklisted; fix by either (A) changing the test to actually
call jwtService.validateToken(accessToken) and assert it throws the expected
exception (and ensure JwtService.validateToken consults isTokenBlacklisted and
throws when true), referencing methods: JwtService.validateToken,
JwtService.isTokenBlacklisted, JwtService.blacklistToken, or (B) if you don't
want validation to check blacklist, update the test DisplayName to reflect that
it only verifies isTokenBlacklisted (and keep assertions against
jwtService.isTokenBlacklisted(accessToken)). Ensure the test method
name/displayName and assertions are consistent.
🧹 Nitpick comments (3)
src/main/java/com/hrr/backend/domain/auth/service/JwtService.java (1)
131-152:getRemainingExpiration에서 모든 예외를Duration.ZERO로 처리하면 디버깅이 어려울 수 있습니다.
catch (Exception e)가 만료된 토큰 외의 예외(서명 오류, 파싱 오류 등)도 조용히 삼키고 있습니다. 최소한 로깅을 추가하면 운영 시 문제 추적에 도움이 됩니다.🛠️ 제안된 수정
} catch (Exception e) { + log.warn("토큰 만료 시간 계산 실패: {}", e.getMessage()); return Duration.ZERO; }참고:
JwtService클래스에@Slf4j어노테이션이 없으므로 추가가 필요합니다.src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
107-196: 소셜 로그인 메서드들(kakaoLogin,appleLogin,naverLogin)에 상당한 코드 중복이 존재합니다.JWT 토큰 생성,
nextStep결정,LoginResponse구성 로직이 4개 메서드(socialLogin포함)에서 반복됩니다. 공통 로직을private헬퍼 메서드로 추출하면 유지보수성이 향상됩니다.예시:
private AuthResponseDto.LoginResponse buildLoginResponse(User user) { String accessToken = jwtService.generateAccessToken(user.getId()); String refreshToken = jwtService.generateRefreshToken(user.getId()); String nextStep = user.determineNextStep(); return new AuthResponseDto.LoginResponse( user.getId(), accessToken, refreshToken, user.getDisplayName(), user.getDisplayNickname(), user.getLoginStatus(), nextStep); }src/test/java/com/hrr/backend/domain/auth/service/JwtServiceTest.java (1)
81-90:generateRefreshToken호출 없이 테스트하면 "잘못된 토큰" 시나리오가 아닌 "저장된 토큰 없음" 시나리오입니다.현재 Redis에 토큰이 저장되지 않은 상태에서
fakeToken을 검증하므로,storedToken == null로false가 반환됩니다. "잘못된 Refresh Token 검증 실패"라는 의도에 맞추려면 실제 토큰을 먼저 저장한 후 다른 토큰으로 검증하는 것이 더 정확한 테스트입니다.🛠️ 개선 제안
void validateRefreshTokenFail() { // given + jwtService.generateRefreshToken(TEST_USER_ID); // Redis에 실제 토큰 저장 String fakeToken = "fake.refresh.token"; // when & then assertThat(jwtService.validateRefreshToken(fakeToken, TEST_USER_ID)).isFalse(); }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/auth/service/AuthService.java`:
- Around line 228-242: The revoke method only unlinks social accounts but does
not clear the app's refresh token, leaving RTs in Redis and causing
inconsistency with withdraw; update revoke (in AuthService.revoke) to call
jwtService.deleteRefreshToken(userId) after the social revocation/unlink switch
so the user's refresh token is removed the same way withdraw does, ensuring both
flows consistently invalidate RTs.
🧹 Nitpick comments (5)
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (3)
77-104:reissueToken에서 블랙리스트 중복 검사Line 78의
jwtService.validateToken(refreshToken)이 내부적으로 이미 블랙리스트 검사를 수행합니다 (JwtServiceLine 71-73). 따라서 Line 81-83의isTokenBlacklisted재검사는 중복입니다.코드 의도를 명확히 하기 위해 중복 검사를 제거하거나, 주석으로 방어적 검사임을 명시하세요.
♻️ 중복 제거 제안
// Refresh Token 유효성 검증 jwtService.validateToken(refreshToken); - // Redis에 저장된 Refresh Token과 비교 검증 - if (jwtService.isTokenBlacklisted(refreshToken)) { - throw new GlobalException(ErrorCode.AUTH_INVALID_TOKEN); - } - // userId 추출 Long userId = jwtService.extractUserId(refreshToken);
107-196: 로그인 메서드 간 코드 중복이 상당합니다
kakaoLogin,appleLogin,naverLogin,socialLogin모두 동일한 패턴을 반복합니다:
- 외부 API로 사용자 정보 조회
upsert처리- AT/RT 생성
LoginResponse구성 및 반환- 동일한 try-catch 구조
토큰 생성 + 응답 구성 로직을 private 헬퍼 메서드로 추출하면 중복을 줄이고, 향후 응답 구조 변경 시 수정 포인트가 하나로 줄어듭니다.
♻️ 헬퍼 메서드 추출 예시
private AuthResponseDto.LoginResponse buildLoginResponse(User user) { String accessToken = jwtService.generateAccessToken(user.getId()); String refreshToken = jwtService.generateRefreshToken(user.getId()); String nextStep = user.determineNextStep(); return new AuthResponseDto.LoginResponse( user.getId(), accessToken, refreshToken, user.getDisplayName(), user.getDisplayNickname(), user.getLoginStatus(), nextStep ); }
198-215:logout에서 이미 블랙리스트된 토큰의 중복 처리 가능성
extractUserId가 JWT 파싱 시 암묵적으로 토큰을 검증하지만, 블랙리스트 확인은 하지 않습니다. 따라서 이미 블랙리스트된 AT로 로그아웃을 다시 호출하면 중복으로 블랙리스트에 등록됩니다 (Redis SET은 덮어쓰므로 에러는 아니지만 불필요한 작업).방어적으로 처리하려면 블랙리스트 확인 후 early return을 추가할 수 있습니다:
♻️ 방어적 처리 제안
public void logout(String tokenHeader) { String token = tokenHeader.startsWith("Bearer ") ? tokenHeader.substring(7) : tokenHeader; + // 이미 블랙리스트된 토큰이면 중복 처리 방지 + if (jwtService.isTokenBlacklisted(token)) { + return; + } + Long userId = jwtService.extractUserId(token);src/main/java/com/hrr/backend/domain/auth/service/JwtService.java (2)
70-85:validateToken이false를 반환하지 않는 boolean 메서드이 메서드는 성공 시
true, 실패 시 항상 예외를 던지므로 절대false를 반환하지 않습니다.boolean리턴 타입은 호출자에게 "false가 반환될 수 있다"는 잘못된 기대를 줍니다.두 가지 개선 방향이 있습니다:
- 리턴 타입을
void로 변경 (검증 실패 시 예외만 던지는 패턴)- 예외 대신
false반환 (호출자가 분기 처리하는 패턴)현재 호출자들(
AuthService.reissueToken, 테스트 코드)을 보면 반환값을 사용하는 곳과 무시하는 곳이 혼재하므로, 일관성을 위해void로 통일하는 것을 권장합니다.
134-155:getRemainingExpiration에서 모든 예외를 무시하면 디버깅이 어려워질 수 있습니다Line 152의
catch (Exception e)가 모든 예외를 삼키고Duration.ZERO를 반환합니다. 만료된 토큰은Duration.ZERO가 적절하지만, 서명 오류나 파싱 오류 등 예상치 못한 문제도 동일하게 처리됩니다.최소한 예외를 로그로 남기면 운영 중 문제 추적에 도움이 됩니다.
♻️ 로깅 추가 제안
} catch (Exception e) { + log.warn("토큰 만료 시간 계산 실패: {}", e.getMessage()); return Duration.ZERO; }참고:
@Slf4j어노테이션이 이 클래스에 없으므로, 로깅 추가 시@Slf4j도 함께 추가해야 합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/auth/service/AuthService.java`:
- Around line 217-226: The withdraw method only deletes the refresh token and
doesn't blacklist the active access token, leaving a window where a deleted user
can still call APIs; update withdraw(Long userId) to accept the incoming token
header (same parameter used by logout) and apply the same access-token blacklist
logic used in logout (call the jwtService method that blacklists/saves the
access token to the blacklist) before deleting the refresh token and completing
user.withdraw(); reference the withdraw method, logout method, and
jwtService.deleteRefreshToken/use of the jwtService blacklist method to locate
where to add the tokenHeader handling and blacklist call.
- Around line 80-102: The refresh-token flow in AuthService has a TOCTOU race:
separate checks (jwtService.isTokenBlacklisted, jwtService.validateRefreshToken)
allow two concurrent requests to both succeed and issue duplicate token pairs;
change the logic to atomically validate-and-consume the stored refresh token
(e.g., implement jwtService.validateAndConsumeRefreshToken or similar) so that
Redis performs a single atomic get-and-delete (GETDEL or Lua script) for the key
tied to userId (use jwtService.extractUserId to locate it), throw
GlobalException(ErrorCode.AUTH_INVALID_TOKEN) if the consumed value doesn't
match the presented refreshToken, then proceed to
generateAccessToken/generateRefreshToken and blacklist the old token only if
needed; alternatively serialize issuance per userId with a distributed lock
(Redisson) to prevent concurrent issuance.
- Around line 139-147: The apple refresh_token from
appleTokens.get("refresh_token") is not validated and null can be passed into
socialUserService.upsertAppleUser and later into appleAuthService.revoke; update
AuthService to guard the refresh token: extract appleRefreshToken =
appleTokens.get("refresh_token"), check for null/empty and normalize it (e.g.,
use Optional.empty / empty string / don't overwrite existing DB token) before
calling upsertAppleUser(socialId, appleRefreshToken, request.getName()), and
ensure any revoke logic in appleAuthService.revoke is only called when a
non-null token is present.
🧹 Nitpick comments (3)
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (3)
107-196: 4개의 로그인 메서드에서 토큰 생성 및 응답 구성 로직이 중복됩니다.
socialLogin,kakaoLogin,appleLogin,naverLogin모두 동일한 패턴을 반복합니다:
generateAccessToken+generateRefreshTokendetermineNextStepLoginResponse생성이를
private헬퍼로 추출하면 변경 지점이 하나로 수렴되어 유지보수성이 크게 향상됩니다. 특히 토큰 발급 로직이 변경될 때(예: 추가 클레임, 새 필드) 4곳을 수정할 필요가 없어집니다.♻️ 헬퍼 메서드 추출 예시
+ private AuthResponseDto.LoginResponse buildLoginResponse(User user) { + String accessToken = jwtService.generateAccessToken(user.getId()); + String refreshToken = jwtService.generateRefreshToken(user.getId()); + String nextStep = user.determineNextStep(); + return new AuthResponseDto.LoginResponse( + user.getId(), accessToken, refreshToken, + user.getDisplayName(), user.getDisplayNickname(), + user.getLoginStatus(), nextStep); + }
72-75:refreshHeader가null일 경우NullPointerException이 발생합니다.
startsWith("Bearer ")는null입력에서 NPE를 던집니다. 컨트롤러의@RequestHeader가 기본적으로 필수이므로 보통null이 전달되지 않지만, 서비스 레이어에서 방어적 검증을 두는 것이 안전합니다.logout(Line 200)에서도 동일합니다.🛡️ 공통 유틸 추출 제안
+ private String resolveToken(String header) { + if (header == null || header.isBlank()) { + throw new GlobalException(ErrorCode.AUTH_INVALID_TOKEN); + } + return header.startsWith("Bearer ") ? header.substring(7) : header; + }
38-38:userChallengeRepository필드를 제거하세요.이 필드는 어떤 메서드에서도 사용되지 않습니다.
@RequiredArgsConstructor를 통해 주입되고 있지만 실제로 참조되지 않으므로 불필요한 Spring 빈 와이어링을 유발하고, 클래스의 책임 범위를 모호하게 만듭니다.개선 방법:
- 선언되지 않은 의존성은 제거해야 하며, 향후 필요하다면 그때 추가하세요. Spring 프레임워크의 dependency injection 가이드에서도 필요한 의존성만 주입할 것을 권장합니다.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/hrr/backend/domain/auth/service/JwtService.java (1)
70-85:⚠️ Potential issue | 🟡 MinorRedis 블랙리스트 검증에 Redis 연결 실패 처리 추가 필요
Line 130의
isTokenBlacklisted메서드는redisTemplate.hasKey()를 직접 호출하며, Redis 연결 실패 시RedisConnectionFailureException등이 처리되지 않은 채 전파되어 모든 토큰 검증이 실패합니다. 이는 Redis 장애가 전체 인증 시스템을 차단하는 단일 장애 지점(SPOF)이 됩니다.토큰 블랙리스트는 보안상 중요한 상태이므로 fail-closed(예외 전파)가 안전하지만, Redis 타임아웃으로 인한 무한 대기나 서비스 정지를 피하려면 다음을 권장합니다:
- Redis 클라이언트 타임아웃 설정:
LettuceConnectionFactory에서setTimeout()및setConnectTimeout()으로 빠른 실패(fast-fail) 구현- Circuit Breaker 추가: Resilience4j
@CircuitBreaker로 연속 실패 시 Redis 호출을 일시 중단하고 503 응답 반환- 선택사항: Redis 장애 시 제한된 시간 동안만 토큰 검증을 건너뛰는 하이브리드 방식(모니터링 필수)
현재는 설정이 보이지 않으므로, Spring Boot 환경설정에서 Redis 타임아웃을 명시적으로 설정하는 것부터 시작하세요.
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/auth/service/AuthService.java`:
- Around line 206-223: The logout method currently calls
jwtService.extractUserId(token) which throws on expired tokens; catch
ExpiredJwtException around that call and extract the userId from
exception.getClaims() (or fallback to a safe parse method) so logout proceeds
for expired ATs; then continue to compute remainingExpiration, blacklistToken
(if any) and deleteRefreshToken(userId) as before, ensuring you only rethrow
other JWT errors. Reference: method logout, jwtService.extractUserId,
jwtService.getRemainingExpiration, jwtService.blacklistToken,
jwtService.deleteRefreshToken.
- Around line 253-268: The revoke method currently lets exceptions from external
social calls (naverAuthService.revoke, appleAuthService.revoke,
kakaoAuthService.unlink) prevent jwtService.deleteRefreshToken(userId) from
running; modify AuthService.revoke to ensure refresh-token deletion always
occurs by wrapping the social-service call(s) in a try-catch or try-finally
block: call the appropriate branch (based on socialAuth.getSocialType()) inside
a try, log any exception with context (including socialAuth identifiers) and
suppress or rethrow as appropriate, and then call
jwtService.deleteRefreshToken(userId) in the finally (or after catching) so
token cleanup always runs even when social unlink/revoke fails.
In
`@src/test/java/com/hrr/backend/domain/auth/service/AuthServiceWithdrawTest.java`:
- Around line 81-106: The test withdraw_success_no_active_challenges is missing
verification that jwtService.blacklistToken is called when
jwtService.getRemainingExpiration returns Duration.ofMinutes(10); update the
test (method withdraw_success_no_active_challenges) to verify that
authService.withdraw invokes jwtService.blacklistToken with the same token
(dummyToken) and the expected duration (Duration.ofMinutes(10)), e.g., add a
verify(jwtService).blacklistToken(...) after calling authService.withdraw
alongside the existing verify(jwtService).deleteRefreshToken(userId) and
verify(user).withdraw().
🧹 Nitpick comments (3)
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (1)
113-138: 로그인 메서드들의 중복 코드 — 공통 헬퍼 추출을 고려해 주세요
kakaoLogin,appleLogin,naverLogin세 메서드 모두 사용자 조회 후 토큰 생성 →nextStep결정 →LoginResponse빌드 패턴이 동일합니다. 소셜 사용자 조회 부분만 다르고 나머지 로직은 완전히 동일하므로, 공통 부분을 별도 private 메서드로 추출하면 유지보수성이 크게 향상됩니다.♻️ 리팩토링 예시
private AuthResponseDto.LoginResponse buildLoginResponse(User user) { String accessToken = jwtService.generateAccessToken(user.getId()); String refreshToken = jwtService.generateRefreshToken(user.getId()); String nextStep = user.determineNextStep(); return new AuthResponseDto.LoginResponse( user.getId(), accessToken, refreshToken, user.getDisplayName(), user.getDisplayNickname(), user.getLoginStatus(), nextStep ); }각 로그인 메서드에서:
public AuthResponseDto.LoginResponse kakaoLogin(String kakaoAccessToken) { try { KakaoUserResponse kakaoUser = kakaoAuthService.fetchUser(kakaoAccessToken); User user = socialUserService.upsertKakaoUser(kakaoUser); return buildLoginResponse(user); } catch (GlobalException e) { throw e; } catch (Exception e) { throw new GlobalException(ErrorCode.AUTH_EXTERNAL_API_ERROR); } }Also applies to: 140-176, 178-204
src/main/java/com/hrr/backend/domain/auth/service/JwtService.java (1)
134-155:getRemainingExpiration이 모든 예외를Duration.ZERO로 무시합니다현재 로직은 만료된 토큰뿐 아니라 변조된(malformed) 토큰, 서명 불일치 등의 경우에도
Duration.ZERO를 반환합니다. 호출부(logout/withdraw)에서는 이미 유효성 검증을 거친 후 호출하므로 실질적 문제는 아니지만, 예상치 못한 경로에서 호출될 경우 오류를 조용히 삼키게 됩니다.최소한
catch블록에서 경고 로그를 남기는 것을 권장합니다:🔧 로깅 추가 제안
} catch (Exception e) { + log.warn("토큰 만료 시간 계산 실패: {}", e.getMessage()); return Duration.ZERO; }src/test/java/com/hrr/backend/domain/auth/service/AuthServiceWithdrawTest.java (1)
27-38:AuthService의 모든 의존성이 Mock 선언되지 않았습니다 — 현재는 동작하지만 취약한 구조
AuthService는 8개의final필드를 가지고 있지만, 이 테스트에서는 3개(userRepository,userChallengeRepository,jwtService)만@Mock으로 선언되어 있습니다. Mockito의@InjectMocks는 생성자 주입 실패 시 필드 주입으로 fallback하여 동작하지만, 이는 명시적이지 않습니다.나머지 의존성도
@Mock으로 선언하면 의도가 명확해지고, 향후withdraw메서드가 다른 의존성을 사용하게 될 때 테스트가 깨지는 것을 방지할 수 있습니다.
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java
Outdated
Show resolved
Hide resolved
src/test/java/com/hrr/backend/domain/auth/service/AuthServiceWithdrawTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/auth/service/AuthService.java`:
- Around line 150-155: Currently appleRefreshToken is converted from null to an
empty string before calling socialUserService.upsertAppleUser, which can later
cause appleAuthService.revoke(socialAuth.getSocialRefreshToken()) to send an
empty string to Apple; instead preserve null (do not replace with "") and update
the revoke flow and/or upsert logic: leave appleRefreshToken as null when
absent, modify socialUserService.upsertAppleUser(socialId, appleRefreshToken,
...) to treat null as “no change” (preserve existing DB token) or ensure revoke
callers (appleAuthService.revoke and any use of
socialAuth.getSocialRefreshToken()) perform null/empty checks before calling the
Apple API to avoid sending empty strings.
In `@src/main/java/com/hrr/backend/domain/auth/service/JwtService.java`:
- Around line 173-182: The validateRefreshToken method in JwtService is dead
code (used only by tests) because AuthService.reissueToken performs the same
inline comparison after fetching the token via getAndDeleteRefreshToken; either
remove JwtService.validateRefreshToken to eliminate the duplicate check, or
refactor AuthService.reissueToken to call JwtService.validateRefreshToken (pass
refreshToken and userId) and rely on that centralized null-safe comparison
instead of repeating storedRefreshToken.equals(refreshToken); update tests
accordingly if you remove or relocate the method.
🧹 Nitpick comments (6)
src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (2)
75-111:reissueToken에서 블랙리스트 검사가 중복 수행됩니다.Line 81의
validateToken(refreshToken)은 내부적으로isTokenBlacklisted를 이미 호출합니다 (JwtService Line 71-73). Line 84에서 다시isTokenBlacklisted를 호출하므로 동일한 Redis 조회가 2회 발생합니다. 또한 같은 토큰에 대해 JWT 파싱이 4회(validateToken,isTokenBlacklisted후extractUserId, 그리고getRemainingExpiration) 수행됩니다.
validateToken에서 이미 블랙리스트+유효성 검증을 모두 처리하므로, Line 83-86의 중복 블랙리스트 검사를 제거하고,extractUserId대신 이미 파싱된 claims를 재사용하는 방식을 고려해 보세요.♻️ 중복 블랙리스트 검사 제거 제안
// Refresh Token 유효성 검증 jwtService.validateToken(refreshToken); - // Redis에 저장된 Refresh Token과 비교 검증 - if (jwtService.isTokenBlacklisted(refreshToken)) { - throw new GlobalException(ErrorCode.AUTH_INVALID_TOKEN); - } - // userId 추출 Long userId = jwtService.extractUserId(refreshToken);
43-72:socialLogin과kakaoLogin의 로직이 거의 동일합니다.두 메서드 모두 카카오 사용자 조회 → upsert → 토큰 생성 →
LoginResponse반환 패턴이 동일합니다.socialLogin은 추가로exchangeToken을 통해 authorization code → access token 변환을 수행하는 차이만 있습니다. DRY 원칙에 따라 공통 로직을 private 헬퍼 메서드로 추출하면 4개 login 메서드 모두 간결해집니다.Also applies to: 113-138
src/main/java/com/hrr/backend/domain/auth/service/JwtService.java (4)
101-128:extractUserId와getUserIdFromToken은 거의 동일한 역할을 합니다.두 메서드의 유일한 차이는
ExpiredJwtException처리뿐입니다.extractUserId는 만료된 토큰에서 예외를 던지고,getUserIdFromToken은 만료된 토큰에서도 userId를 추출합니다. 현재extractUserId는reissueToken에서,getUserIdFromToken은logout에서 사용되는데, 호출자가 어떤 메서드를 써야 하는지 이름만으로는 구분이 어렵습니다.하나의 메서드로 통합하거나, 메서드 이름을 의도가 명확하게 드러나도록 변경하는 것을 권장합니다 (예:
extractUserIdAllowExpired등).
150-171:getRemainingExpiration에서 모든 예외를 무시하고Duration.ZERO를 반환합니다.Line 168-170에서
catch (Exception e)로 모든 예외를 삼키면, 서명 불일치·변조된 토큰 등 유효하지 않은 토큰에 대해서도 조용히Duration.ZERO를 반환합니다. 호출자(reissueToken,logout,withdraw)는 이 경우 블랙리스트 등록을 건너뛰게 되는데, 변조된 토큰이라면 이미 앞단에서 걸러지므로 현재 흐름상 큰 문제는 아닙니다.다만 디버깅 편의를 위해 최소한
log.warn을 추가하면 운영 시 추적이 용이합니다.🔍 로깅 추가 제안
} catch (Exception e) { + log.warn("토큰 만료 시간 계산 실패: {}", e.getMessage()); return Duration.ZERO; }
70-85:validateToken은 항상true를 반환하거나 예외를 던집니다.
boolean반환 타입이지만false를 반환하는 경로가 없어, 호출자가 반환값을 검사하는 것이 무의미합니다. 현재AuthService.reissueToken에서도 반환값을 사용하지 않고 있습니다.void로 변경하면 메서드의 계약(contract)이 더 명확해집니다.
36-38:getSigningKey()가 호출마다 새로운SecretKey인스턴스를 생성합니다.JWT 파싱/서명 시마다
Keys.hmacShaKeyFor()를 호출하여 바이트 변환 + 키 생성을 반복합니다.secret값은 변하지 않으므로, 키를 한 번만 생성하여 캐싱하면 불필요한 객체 할당을 줄일 수 있습니다.@PostConstruct로 초기화하거나lazy패턴을 사용하세요.♻️ 키 캐싱 제안
+import jakarta.annotation.PostConstruct; + `@Service` -@RequiredArgsConstructor public class JwtService { `@Value`("${jwt.secret}") private String secret; + private SecretKey signingKey; + + `@PostConstruct` + void init() { + this.signingKey = Keys.hmacShaKeyFor(secret.getBytes(StandardCharsets.UTF_8)); + } + - private SecretKey getSigningKey() { - return Keys.hmacShaKeyFor(secret.getBytes(StandardCharsets.UTF_8)); - } + private SecretKey getSigningKey() { + return signingKey; + }참고:
@PostConstruct를 사용하면@RequiredArgsConstructor와 함께 사용할 때 생성자 주입 순서와 무관하게@Value필드가 초기화된 후 호출됩니다.
src/main/java/com/hrr/backend/domain/auth/service/JwtService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/main/java/com/hrr/backend/domain/auth/service/AuthService.java`:
- Around line 203-220: The logout method lacks a null check for tokenHeader and
can NPE when tokenHeader is null; mirror the guard used in withdraw by first
verifying tokenHeader != null (or handling empty/invalid headers) and return
early before calling jwtService.getUserIdFromToken,
jwtService.getRemainingExpiration, jwtService.blacklistToken, and
jwtService.deleteRefreshToken; ensure the method extracts the token only after
the null/empty check and preserves existing blacklist and refresh-token deletion
behavior.
- Around line 259-262: The APPLE branch in AuthService currently calls
appleAuthService.revoke(socialAuth.getSocialRefreshToken()) without guarding
against a null refresh token which causes Apple API errors; before invoking
appleAuthService.revoke(...) in the switch (or inside AuthService's APPLE case),
check if socialAuth.getSocialRefreshToken() is non-null and only call revoke
when present (otherwise call kakao-like unlink with socialAuth.getSocialId() or
skip revoke and log a specific warning that the token is null), and additionally
add a null guard inside AppleAuthService.revoke(...) to no-op and emit a clear
log entry when given a null token so the external API is never called with
"token": null.
🧹 Nitpick comments (5)
src/main/java/com/hrr/backend/domain/auth/service/JwtService.java (2)
70-85:validateToken에 블랙리스트 검증이 추가되었으나,AuthService.reissueToken과 중복 체크됩니다.
validateToken내부에서isTokenBlacklisted를 호출하고(Line 71),AuthService.reissueToken에서도 동일한 체크를 수행합니다(AuthService Line 84). 현재 동작상 문제는 없지만, 방어 로직이 두 레이어에 분산되어 있어 유지보수 시 혼란의 소지가 있습니다. 한쪽으로 통일하거나,reissueToken측 체크에 의도를 명시하는 주석을 추가하면 좋겠습니다.
150-171:getRemainingExpiration에서 모든 예외를 무시하면 디버깅이 어려울 수 있습니다.
catch (Exception e)에서Duration.ZERO를 반환하는 것은 만료된 토큰에 대해 올바른 동작이지만, 예상치 못한 파싱 오류까지 조용히 삼키면 문제 추적이 어렵습니다. 최소한 debug/warn 레벨 로그를 남기는 것을 권장합니다.🔍 로깅 추가 제안
} catch (Exception e) { + log.debug("토큰 만료 시간 계산 실패: {}", e.getMessage()); return Duration.ZERO; }참고:
JwtService에 현재@Slf4j가 없으므로 클래스 레벨에 추가가 필요합니다.src/main/java/com/hrr/backend/domain/auth/service/AuthService.java (3)
75-111:reissueToken흐름이 견고하게 개선되었습니다. 원자적getAndDeleteRefreshToken+ 비교 검증 + 블랙리스트 처리의 3중 방어가 훌륭합니다.한 가지 작은 포인트: Line 84의
isTokenBlacklisted체크는 Line 81의validateToken내부에서 이미 수행됩니다(JwtService Line 71). 블랙리스트된 토큰은validateToken에서 먼저 거부되므로 Line 84-86은 도달 불가능한 코드입니다. 의도적인 방어 계층이라면 주석으로 명시하면 좋겠습니다.
89-89:extractUserId대신getUserIdFromToken사용을 권장합니다.JwtService에서
extractUserId와getUserIdFromToken통합을 제안드렸는데, 이 호출부도 함께 교체 대상입니다. 현재validateToken을 통과한 후이므로 동작상 문제는 없지만, API 일관성을 위해 통일하는 것이 좋습니다.
113-201: 로그인 메서드 4개(socialLogin,kakaoLogin,appleLogin,naverLogin)에 토큰 발급 및 응답 생성 로직이 중복됩니다.각 메서드에서
generateAccessToken→generateRefreshToken→determineNextStep→new LoginResponse(...)패턴이 반복되고 있습니다. 공통 부분을 헬퍼 메서드로 추출하면 변경 포인트를 줄이고 유지보수성을 높일 수 있습니다. 예를 들어LoginResponse필드가 변경되면 4곳을 모두 수정해야 하는 상황을 방지할 수 있습니다.♻️ 헬퍼 메서드 추출 예시
private AuthResponseDto.LoginResponse buildLoginResponse(User user) { String accessToken = jwtService.generateAccessToken(user.getId()); String refreshToken = jwtService.generateRefreshToken(user.getId()); String nextStep = user.determineNextStep(); return new AuthResponseDto.LoginResponse( user.getId(), accessToken, refreshToken, user.getDisplayName(), user.getDisplayNickname(), user.getLoginStatus(), nextStep ); }각 로그인 메서드에서는
return buildLoginResponse(user);로 호출하면 됩니다.
1 participant
#️⃣ 연관된 이슈
✨ 작업 내용 (Summary)
✅ 변경 사항 체크리스트
🧪 테스트 결과
📸 스크린샷
💬 리뷰 요구사항
📎 참고 자료
Summary by CodeRabbit
새로운 기능
보안 개선
테스트