-
Notifications
You must be signed in to change notification settings - Fork 1
[Feat] 토큰 재발급 API 구현 #149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feat] 토큰 재발급 API 구현 #149
Conversation
- 컨트롤러 메서드 작성
- 토큰 재발급 서비스 인터페이스와 구현체 작성
- 토큰 재발급 서비스 테스트 작성
- userId로 리프레시토큰을 찾는 메서드를 구현
- 토큰 재발급 서비스 주석 수정
- 게스트 로그인에서 리프레시 토큰을 반환하도록 수정
- 카카오 로그인에서 리프레시 토큰을 반환하도록 수정
- 통합테스트에 필요한 설정값 추가
- 리프레시 토큰 발급 기능을 추가함에 따라 테스트를 수정
- AuthService 이름을 LoginService로 변경
- AuthServiceFacade로 AuthController에서 LoginService와 ReissueTokenService로의 의존성을 제거
- 리프레시토큰 캐싱을 위한 키와 TTL을 환경변수로 주입하도록 수정
- 잘못 설계된 메서드를 제거하고 로직에서는 validateJwt 메서드로 대체
- 서비스 로직 변경에 따라서 테스트를 수정
- 리프레시 토큰에 jti 추가
- 토큰 재발급 API에 대한 테스트 작성 - 기존 테스트 이름 수정
- 토큰 재발급 API에 대한 명세 작성
- 새로운 토큰을 발급할 때 캐시에 저장된 리프레시 토큰도 갱신하도록 수정
- 캐시에 저장된 값을 변경하는지 검증하는 코드를 추가
- 카카오 로그인 성공 시 리프레시 토큰을 반환하는지 검증하는 코드 추가
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Walkthrough이 PR은 토큰 재발급 API를 구현하며, Redis 기반 리프레시 토큰 저장소를 추가했습니다. 기존 AuthService를 LoginService로 분리하고 AuthServiceFacade를 도입하여 서비스 계층을 재구조화했습니다. 로그인 및 게스트 로그인 흐름에 리프레시 토큰 생성 및 저장 기능을 추가했습니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as Client
participant Controller as AuthController
participant Facade as AuthServiceFacade
participant ReissueService as ReissueTokenService
participant JwtUtil as JwtUtil
participant Redis as Redis
participant UserRepo as UserRepository
User->>Client: POST /reissue/token (refreshToken)
Client->>Controller: reissueToken(ReissueTokenRequest)
Controller->>Facade: reissueToken(request)
Facade->>ReissueService: reissueToken(request)
ReissueService->>JwtUtil: validateJwt(refreshToken)
alt Invalid/Expired Token
JwtUtil-->>ReissueService: throw JwtException
ReissueService-->>Controller: CustomException
Controller-->>Client: 401/404 Error
else Valid Token
JwtUtil-->>ReissueService: userId extracted
ReissueService->>Redis: findByUserId(userId)
alt No Stored Token
Redis-->>ReissueService: Optional.empty()
ReissueService-->>Controller: REFRESH_TOKEN_NOT_FOUND
Controller-->>Client: 404 Error
else Token Exists
Redis-->>ReissueService: stored refreshToken
alt Token Mismatch
ReissueService-->>Controller: REFRESH_TOKEN_NOT_FOUND
Controller-->>Client: 404 Error
else Token Matches
ReissueService->>UserRepo: findById(userId)
UserRepo-->>ReissueService: User
ReissueService->>JwtUtil: createAccessJwt(userId, role)
JwtUtil-->>ReissueService: newAccessToken
ReissueService->>JwtUtil: createRefreshJwt(userId)
JwtUtil-->>ReissueService: newRefreshToken
ReissueService->>Redis: save(userId, newRefreshToken)
Redis-->>ReissueService: ✓ saved with TTL
ReissueService-->>Facade: ReissueTokenResponse
Facade-->>Controller: ReissueTokenResponse
Controller-->>Client: 200 OK (accessToken, refreshToken)
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 주의 깊게 검토할 영역:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Test Results379 tests 379 ✅ 1m 8s ⏱️ Results for commit f038010. ♻️ This comment has been updated with latest results. |
📊 JaCoCo Coverage
|
- 테스트를 실행하기 위해서 AuthControllerTest에 테스트용 레디스 컨테이너 설정 적용
- 저장된 리프레시 토큰이 없는 경우 테스트를 추가
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (9)
src/main/java/com/kuit/findyou/domain/auth/dto/ReissueTokenRequest.java (1)
3-5: 입력 유효성 검증 어노테이션 추가를 고려해 주세요.
refreshToken필드에@NotBlank같은 Bean Validation 어노테이션을 추가하면 컨트롤러 레벨에서 null이나 빈 문자열을 조기에 거부할 수 있습니다.🔎 제안하는 수정
package com.kuit.findyou.domain.auth.dto; +import jakarta.validation.constraints.NotBlank; + public record ReissueTokenRequest( - String refreshToken + @NotBlank String refreshToken ) { }src/main/java/com/kuit/findyou/domain/auth/repository/RedisRefreshTokenRepository.java (1)
5-9: 향후 로그아웃 기능을 위해delete메서드 추가를 고려해 주세요.현재 인터페이스에는
findByUserId와save만 있습니다. 로그아웃 시 리프레시 토큰 무효화나 명시적 삭제가 필요하다면delete(Long userId)메서드가 필요할 수 있습니다. 현재 PR 범위에서는 필수가 아니니 참고만 해주세요.src/main/java/com/kuit/findyou/domain/auth/service/LoginServiceImpl.java (1)
34-39: 토큰 생성 및 저장 로직이 중복됩니다.
kakaoLogin과guestLogin에서 동일한 토큰 생성/저장 패턴이 반복됩니다. 헬퍼 메서드로 추출하면 유지보수성이 향상됩니다.🔎 제안하는 리팩토링
private TokenPair generateAndSaveTokens(User user) { String accessToken = jwtUtil.createAccessJwt(user.getId(), user.getRole()); String refreshToken = jwtUtil.createRefreshJwt(user.getId()); redisRefreshTokenRepository.save(user.getId(), refreshToken); return new TokenPair(accessToken, refreshToken); }src/test/java/com/kuit/findyou/domain/auth/service/LoginServiceTest.java (1)
61-61: 복사-붙여넣기 오류: REFRESH_TOKEN 값이 "accessToken"으로 설정됨테스트가 동작하는 데는 문제가 없지만, 변수명과 값이 일치하지 않아 혼란을 줄 수 있어요.
🔎 수정 제안
- final String REFRESH_TOKEN = "accessToken"; + final String REFRESH_TOKEN = "refreshToken";src/main/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImpl.java (1)
38-42: 주석 오타 수정 및 예외 코드 분리 고려
- Line 38 주석에 오타가 있어요: "일차히지" → "일치하지"
- 토큰 불일치와 토큰 미존재에 동일한 예외 코드(
REFRESH_TOKEN_NOT_FOUND)를 사용하고 있는데, 디버깅 시 원인 파악이 어려울 수 있어요. 보안상 의도적으로 동일하게 처리한 거라면 괜찮아요~🔎 오타 수정 제안
- // 토큰이 일차히지 않으면 에러 + // 토큰이 일치하지 않으면 에러 if(!foundRefreshToken.equals(request.refreshToken())){ log.info("[reissueToken] 토큰이 일치하지 않음"); throw new CustomException(REFRESH_TOKEN_NOT_FOUND); }src/main/java/com/kuit/findyou/domain/auth/repository/RedisRefreshTokenRepositoryImpl.java (2)
20-22: userId가 null인 경우를 고려해보세요.현재 userId가 null이면 "prefix:null" 형태의 키가 생성됩니다. 방어적 코딩을 위해 null 체크를 추가하는 것을 권장합니다.
🔎 개선 제안
private String key(Long userId) { + if (userId == null) { + throw new IllegalArgumentException("userId cannot be null"); + } return refreshTokenKeyPrefix + userId; }
30-33: 파라미터 검증을 추가하는 것을 권장합니다.refreshToken이 null이거나 빈 문자열인 경우에 대한 검증을 추가하면 더 안전한 코드가 됩니다.
🔎 개선 제안
@Override public void save(Long userId, String refreshToken) { + if (userId == null) { + throw new IllegalArgumentException("userId cannot be null"); + } + if (refreshToken == null || refreshToken.isBlank()) { + throw new IllegalArgumentException("refreshToken cannot be null or empty"); + } redisTemplate.opsForValue().set(key(userId), refreshToken, Duration.ofMillis(refreshTokenExpireMs)); }src/test/java/com/kuit/findyou/domain/auth/controller/AuthControllerTest.java (2)
170-193: 토큰 재발급 성공 케이스가 잘 구현되었습니다.기본적인 검증은 충분하지만, 반환된 토큰의 userId 검증이나 Redis에 새 토큰이 저장되었는지 확인하는 검증을 추가하면 더욱 견고한 테스트가 될 수 있습니다.
🔎 선택적 개선 제안
// then assertThat(response.accessToken()).isNotBlank(); assertThat(response.refreshToken()).isNotBlank(); +assertThat(jwtUtil.getUserId(response.accessToken())).isEqualTo(user.getId()); +assertThat(redisRefreshTokenRepository.findByUserId(user.getId())).isPresent();
195-218: 만료된 토큰 테스트가 잘 작성되었습니다.테스트 로직은 올바르지만, 메서드 이름
reissueToken_shouldReturnUnauthorized_WhenRefreshTokenIsNotMatched가 실제로는 만료된 토큰을 테스트하므로WhenRefreshTokenIsExpired로 변경하면 더 명확할 것 같습니다.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/main/java/com/kuit/findyou/domain/auth/controller/AuthController.java(2 hunks)src/main/java/com/kuit/findyou/domain/auth/dto/ReissueTokenRequest.java(1 hunks)src/main/java/com/kuit/findyou/domain/auth/dto/ReissueTokenResponse.java(1 hunks)src/main/java/com/kuit/findyou/domain/auth/dto/response/GuestLoginResponse.java(1 hunks)src/main/java/com/kuit/findyou/domain/auth/dto/response/KakaoLoginResponse.java(2 hunks)src/main/java/com/kuit/findyou/domain/auth/repository/RedisRefreshTokenRepository.java(1 hunks)src/main/java/com/kuit/findyou/domain/auth/repository/RedisRefreshTokenRepositoryImpl.java(1 hunks)src/main/java/com/kuit/findyou/domain/auth/service/AuthServiceFacade.java(1 hunks)src/main/java/com/kuit/findyou/domain/auth/service/LoginService.java(1 hunks)src/main/java/com/kuit/findyou/domain/auth/service/LoginServiceImpl.java(3 hunks)src/main/java/com/kuit/findyou/domain/auth/service/ReissueTokenService.java(1 hunks)src/main/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImpl.java(1 hunks)src/main/java/com/kuit/findyou/global/common/response/status/BaseExceptionResponseStatus.java(1 hunks)src/main/java/com/kuit/findyou/global/common/swagger/SwaggerResponseDescription.java(1 hunks)src/main/java/com/kuit/findyou/global/jwt/util/JwtUtil.java(3 hunks)src/main/resources/application.yml(2 hunks)src/test/java/com/kuit/findyou/domain/auth/controller/AuthControllerTest.java(5 hunks)src/test/java/com/kuit/findyou/domain/auth/service/LoginServiceTest.java(5 hunks)src/test/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImplTest.java(1 hunks)src/test/resources/application-test.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImpl.java (2)
src/main/java/com/kuit/findyou/domain/auth/repository/RedisRefreshTokenRepositoryImpl.java (1)
RequiredArgsConstructor(11-34)src/main/java/com/kuit/findyou/global/jwt/util/JwtUtil.java (1)
Slf4j(23-89)
src/test/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImplTest.java (1)
src/main/java/com/kuit/findyou/global/jwt/exception/JwtExpiredException.java (1)
JwtExpiredException(6-10)
src/main/java/com/kuit/findyou/domain/auth/controller/AuthController.java (2)
src/main/java/com/kuit/findyou/domain/auth/service/LoginServiceImpl.java (1)
Slf4j(21-76)src/main/java/com/kuit/findyou/domain/auth/service/AuthServiceFacade.java (1)
RequiredArgsConstructor(12-29)
🔇 Additional comments (27)
src/test/resources/application-test.yml (1)
31-35: LGTM! 테스트 환경 설정 변경이 잘 되어 있습니다.새로운 계층적 JWT 만료 시간 구조(
expiration-ms.access-token,expiration-ms.refresh-token)와 Redis 키 프리픽스 설정이 적절하게 추가되었습니다. 테스트 환경에서 사용하기에 적합한 구성입니다.src/main/java/com/kuit/findyou/global/jwt/util/JwtUtil.java (2)
64-73: LGTM! 리프레시 토큰 생성 로직이 잘 구현되어 있습니다.
UUID.randomUUID()를 사용한jti클레임으로 토큰의 고유성을 보장합니다.- 리프레시 토큰에
ROLE클레임을 포함하지 않은 것은 좋은 설계입니다. 토큰 재발급 시 최신 역할 정보를 조회하게 됩니다.- 기존
createAccessJwt와 일관된 패턴을 유지하고 있습니다.
28-32: 설정 속성 경로가 운영 환경에도 제대로 적용되었습니다.운영 환경의
application.yml(156-158줄)과 테스트 설정 파일이 동일한 JWT 만료 시간 계층 구조를 가지고 있습니다. 운영 환경에서는 환경 변수(${JWT_ACCESS_EXPIRE_MS},${JWT_REFRESH_EXPIRE_MS})를 사용하고 있어 배포 유연성도 잘 유지되어 있네요.src/main/java/com/kuit/findyou/domain/auth/dto/response/GuestLoginResponse.java (1)
6-13: LGTM! 게스트 로그인 응답에 리프레시 토큰 필드가 잘 추가되었습니다.PR 목표에 맞게 게스트 로그인에서도 리프레시 토큰을 반환하도록 수정되었습니다. Swagger 문서화도 적절하게 되어 있습니다.
src/main/java/com/kuit/findyou/domain/auth/dto/response/KakaoLoginResponse.java (3)
13-16: LGTM! 팩토리 메서드 리네이밍과 리프레시 토큰 추가가 잘 되었습니다.
fromUserAndAccessToken→fromUserAndTokens변경이 새로운 파라미터를 잘 반영하고, 메서드 의도를 명확하게 표현합니다.
18-20:notFound()→firstLogin()리네이밍이 의미를 더 명확하게 합니다.비즈니스 로직의 의도가 더 잘 드러나는 좋은 네이밍 변경입니다. 👍
22-31: UserInfoDto에 리프레시 토큰 필드가 잘 추가되었습니다.Swagger
@Schema어노테이션과 함께 적절하게 문서화되어 있습니다.src/main/java/com/kuit/findyou/domain/auth/service/LoginService.java (1)
8-12: LGTM!AuthService→LoginService리네이밍이 적절합니다.
AuthServiceFacade패턴 도입과 함께 서비스 책임이 명확하게 분리되었습니다.LoginService는 로그인만 담당하고, 토큰 재발급은 별도 서비스에서 처리하는 구조가 SRP(Single Responsibility Principle)를 잘 따르고 있습니다.src/main/java/com/kuit/findyou/domain/auth/service/ReissueTokenService.java (1)
1-8: LGTM!인터페이스가 깔끔하게 정의되어 있고, 단일 책임 원칙을 잘 따르고 있습니다. 토큰 재발급 기능을 명확하게 분리한 설계가 좋습니다.
src/main/resources/application.yml (2)
9-9: 활성 프로파일이local로 변경되었습니다.개발 브랜치(develop)에 머지 시
active: local설정이 그대로 반영되면 의도치 않게 로컬 환경 설정이 적용될 수 있습니다. 배포 환경에서는 환경 변수나 외부 설정으로 프로파일을 오버라이드하는지 확인해 주세요.
156-160: JWT 설정 구조화가 잘 되었습니다.
expiration-ms하위에access-token과refresh-token을 분리하고, Redis 키 프리픽스를 추가한 구조가 명확합니다. 관련 코드(JwtUtil, RedisRefreshTokenRepositoryImpl)에서 이 설정값들을 올바르게 참조하는지 확인해 주세요.src/main/java/com/kuit/findyou/domain/auth/dto/ReissueTokenResponse.java (1)
1-7: LGTM!Java record를 활용한 불변 DTO 정의가 깔끔합니다.
src/main/java/com/kuit/findyou/global/common/response/status/BaseExceptionResponseStatus.java (1)
38-40: LGTM!새로운 예외 상태가 적절하게 추가되었습니다. 404 코드와 메시지가 리프레시 토큰을 찾을 수 없는 상황을 잘 표현합니다.
src/main/java/com/kuit/findyou/global/common/swagger/SwaggerResponseDescription.java (1)
33-40: LGTM!토큰 재발급 API에서 발생할 수 있는 예외 상태들이 적절하게 정의되었습니다. 기존 Swagger 문서화 패턴을 잘 따르고 있네요.
src/main/java/com/kuit/findyou/domain/auth/service/LoginServiceImpl.java (1)
69-74: Redis 저장 실패 시 DB와 불일치가 발생할 수 있습니다.
guestLogin은@Transactional로 감싸져 있지만, Redis 저장(line 72)은 트랜잭션 범위 밖에서 동작합니다. Redis 저장이 실패하면 DB에는 유저가 저장되었지만 리프레시 토큰은 없는 상태가 됩니다.현재 구조에서는 다음 로그인 시 토큰이 재발급되므로 큰 문제는 아니지만, Redis 저장 실패에 대한 예외 처리나 로깅을 추가하는 것이 좋을 수 있습니다.
src/test/java/com/kuit/findyou/domain/auth/service/LoginServiceTest.java (2)
30-38: 테스트 클래스 리팩토링 잘 되었네요~
LoginServiceImpl로의 이름 변경과RedisRefreshTokenRepositorymock 추가가 깔끔하게 적용되었습니다. 리프레시 토큰 플로우를 검증하기 위한 구조가 잘 갖춰졌네요.
73-79: 리프레시 토큰 저장 및 응답 검증 LGTM!
redisRefreshTokenRepository.save()호출 검증과 응답에refreshToken포함 여부 확인이 잘 추가되었습니다.src/main/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImpl.java (1)
24-54: 토큰 재발급 로직 잘 구현되었습니다!전체적인 흐름이 깔끔하네요:
- 리프레시 토큰 유효성 검증
- Redis에서 저장된 토큰 조회 및 비교
- 새 토큰 쌍 생성 및 저장
Redis TTL이 리프레시 토큰 만료 시간과 동기화되어 있어서 토큰 회전(rotation)도 제대로 처리될 거예요.
src/main/java/com/kuit/findyou/domain/auth/service/AuthServiceFacade.java (1)
14-28: Facade 패턴 깔끔하게 적용되었네요~
LoginService와ReissueTokenService를 하나의 진입점으로 통합해서 컨트롤러 레이어에서 관리하기 편해졌어요. 역할 분리도 명확하고 좋습니다!src/main/java/com/kuit/findyou/domain/auth/controller/AuthController.java (1)
53-62: 토큰 재발급 API 엔드포인트 잘 추가되었어요!Swagger 문서화와
@CustomExceptionDescription어노테이션 적용이 일관성 있게 되어있네요.src/test/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImplTest.java (1)
42-68: 정상 케이스 테스트 LGTM!토큰 재발급 성공 시나리오가 잘 검증되고 있어요. mock 설정과 assertion이 명확합니다.
src/main/java/com/kuit/findyou/domain/auth/repository/RedisRefreshTokenRepositoryImpl.java (2)
11-18: 클래스 구성이 깔끔합니다.생성자 주입과 외부 설정 값 주입이 적절하게 구현되어 있습니다.
24-28: 조회 로직이 적절합니다.Optional을 사용하여 null 값을 안전하게 처리하고 있습니다.
src/test/java/com/kuit/findyou/domain/auth/controller/AuthControllerTest.java (4)
46-46: 테스트 설정이 적절합니다.Redis TestContainer 설정을 추가하여 리프레시 토큰 저장소 테스트 환경을 구성했습니다.
54-55: Redis 저장소 의존성 주입이 올바릅니다.테스트에서 리프레시 토큰 저장 및 검증에 필요한 repository가 적절히 주입되었습니다.
220-230: 만료된 JWT 생성 헬퍼가 적절합니다.테스트를 위한 만료된 토큰 생성 로직이 명확하게 구현되어 있습니다.
232-257: 토큰 불일치 테스트가 잘 구현되었습니다.Redis에 저장된 토큰과 요청 토큰이 다른 경우를 적절히 테스트하고 있습니다.
| public BaseResponse<ReissueTokenResponse> reissueToken(@RequestBody ReissueTokenRequest request){ | ||
| log.debug("[reissueToken] request.refreshToken = {}", request.refreshToken()); | ||
| return BaseResponse.ok(authServiceFacade.reissueToken(request)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리프레시 토큰 로깅은 보안상 위험해요
log.debug라도 리프레시 토큰 값을 로그에 기록하면 로그 파일에 민감한 정보가 노출될 수 있어요. 토큰 탈취 시 계정 접근이 가능해지니 로깅을 제거하거나 마스킹 처리가 필요합니다.
🔎 수정 제안
@PostMapping("/reissue/token")
public BaseResponse<ReissueTokenResponse> reissueToken(@RequestBody ReissueTokenRequest request){
- log.debug("[reissueToken] request.refreshToken = {}", request.refreshToken());
+ log.info("[reissueToken] 토큰 재발급 요청");
return BaseResponse.ok(authServiceFacade.reissueToken(request));
}📝 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 BaseResponse<ReissueTokenResponse> reissueToken(@RequestBody ReissueTokenRequest request){ | |
| log.debug("[reissueToken] request.refreshToken = {}", request.refreshToken()); | |
| return BaseResponse.ok(authServiceFacade.reissueToken(request)); | |
| public BaseResponse<ReissueTokenResponse> reissueToken(@RequestBody ReissueTokenRequest request){ | |
| log.info("[reissueToken] 토큰 재발급 요청"); | |
| return BaseResponse.ok(authServiceFacade.reissueToken(request)); |
🤖 Prompt for AI Agents
In src/main/java/com/kuit/findyou/domain/auth/controller/AuthController.java
around lines 59 to 61, the debug log prints the raw refresh token which is
sensitive; remove the direct logging of request.refreshToken() or replace it
with a masked/boolean indicator. Update the code to either delete the log
statement entirely or log a non-sensitive placeholder (e.g., "refreshToken=***"
or "refreshToken=present") and keep only non-sensitive context, then run
tests/compile to ensure no references remain to the removed value.
| @DisplayName("리프레시 토큰이 일치하지 않으면 404 코드를 응답한다") | ||
| @Test | ||
| void reissueToken_shouldReturnNotFound_WhenRefreshTokenIsNotMatched(){ | ||
| // given |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
들여쓰기를 수정해주세요.
주석의 들여쓰기가 누락되어 코드 가독성이 떨어집니다.
🔎 수정 제안
-// given
+ // given📝 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.
| // given | |
| // given |
🤖 Prompt for AI Agents
In src/test/java/com/kuit/findyou/domain/auth/controller/AuthControllerTest.java
around line 235, the line containing the single-line comment "// given" is
misindented; adjust its leading whitespace so it matches the indentation level
of the surrounding test section comments (align with other section comments like
"// when" and "// then") to improve readability.
| // when & then | ||
| verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString()); | ||
|
|
||
| assertThatThrownBy(() -> reissueTokenService.reissueToken(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .hasMessage(EXPIRED_JWT.getMessage()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
verify() 호출 위치가 잘못되었어요
verify(redisRefreshTokenRepository, never()) 가 실제 테스트 액션(assertThatThrownBy) 전에 호출되고 있어서, 항상 통과하게 됩니다. 메서드가 아직 실행되지 않았으니 당연히 save()도 호출되지 않았겠죠.
🔎 수정 제안
@DisplayName("만료된 리프레시 토큰이면 예외를 발생시킨다")
@Test
void reissueToken_shouldThrowException_whenInvalidRefreshToken(){
// given
ReissueTokenRequest request = new ReissueTokenRequest("expired refresh");
doThrow(new JwtExpiredException(EXPIRED_JWT)).when(jwtUtil).validateJwt(anyString());
// when & then
- verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString());
-
assertThatThrownBy(() -> reissueTokenService.reissueToken(request))
.isInstanceOf(CustomException.class)
.hasMessage(EXPIRED_JWT.getMessage());
+
+ verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString());
}📝 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.
| // when & then | |
| verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString()); | |
| assertThatThrownBy(() -> reissueTokenService.reissueToken(request)) | |
| .isInstanceOf(CustomException.class) | |
| .hasMessage(EXPIRED_JWT.getMessage()); | |
| } | |
| // when & then | |
| assertThatThrownBy(() -> reissueTokenService.reissueToken(request)) | |
| .isInstanceOf(CustomException.class) | |
| .hasMessage(EXPIRED_JWT.getMessage()); | |
| verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString()); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImplTest.java
around lines 78 to 84, the verify(redisRefreshTokenRepository,
never()).save(...) assertion is placed before the action under test so it always
passes; move the verify call to after the assertThatThrownBy(() ->
reissueTokenService.reissueToken(request)) block (i.e., perform the reissueToken
call and exception assertion first, then verify that
redisRefreshTokenRepository.save was never called) to properly validate
interactions after the method execution.
| // when & then | ||
| verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString()); | ||
|
|
||
| assertThatThrownBy(() -> reissueTokenService.reissueToken(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .hasMessage(REFRESH_TOKEN_NOT_FOUND.getMessage()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동일한 verify() 위치 문제
이 테스트에서도 verify()가 테스트 액션 전에 호출되고 있어요. 위와 같이 assertThatThrownBy 이후로 이동해주세요.
🔎 수정 제안
// when & then
- verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString());
-
assertThatThrownBy(() -> reissueTokenService.reissueToken(request))
.isInstanceOf(CustomException.class)
.hasMessage(REFRESH_TOKEN_NOT_FOUND.getMessage());
+
+ verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString());
}📝 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.
| // when & then | |
| verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString()); | |
| assertThatThrownBy(() -> reissueTokenService.reissueToken(request)) | |
| .isInstanceOf(CustomException.class) | |
| .hasMessage(REFRESH_TOKEN_NOT_FOUND.getMessage()); | |
| } | |
| // when & then | |
| assertThatThrownBy(() -> reissueTokenService.reissueToken(request)) | |
| .isInstanceOf(CustomException.class) | |
| .hasMessage(REFRESH_TOKEN_NOT_FOUND.getMessage()); | |
| verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString()); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImplTest.java
around lines 97 to 103, the verify(redisRefreshTokenRepository,
never()).save(anyLong(), anyString()) call is placed before the
assertThatThrownBy assertion; move that verify call to after the
assertThatThrownBy(...).isInstanceOf(...).hasMessage(...) block so the
verification runs after the action and exception assertion.
| // when & then | ||
| verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString()); | ||
|
|
||
| assertThatThrownBy(() -> reissueTokenService.reissueToken(request)) | ||
| .isInstanceOf(CustomException.class) | ||
| .hasMessage(REFRESH_TOKEN_NOT_FOUND.getMessage()); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동일한 verify() 위치 문제
여기서도 마찬가지로 수정이 필요해요.
🔎 수정 제안
// when & then
- verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString());
-
assertThatThrownBy(() -> reissueTokenService.reissueToken(request))
.isInstanceOf(CustomException.class)
.hasMessage(REFRESH_TOKEN_NOT_FOUND.getMessage());
+
+ verify(redisRefreshTokenRepository, never()).save(anyLong(), anyString());
}🤖 Prompt for AI Agents
In
src/test/java/com/kuit/findyou/domain/auth/service/ReissueTokenServiceImplTest.java
around lines 116 to 122, the verify(redisRefreshTokenRepository,
never()).save(...) call is placed before the method under test is invoked; move
that verify so it runs after the assertThatThrownBy(...) (i.e., after invoking
reissueTokenService.reissueToken(request)) so the test asserts the exception
first and then verifies that save(...) was never called.
- verify 메서드의 잘못된 위치를 수정
- 주석에 들여쓰기 추가
- 보안을 위해 리프레시 토큰 로깅을 삭제
- 카카오 로그인과 게스트 로그인에 이벤트별로 로그를 남기도록 추가
JJUYAAA
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
깔끔한 PR인 것 같습니다!
수고하셨습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오오 테스트 엄청 세세하고 좋습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
파사드 전략 좋습니다! (+ auth -> login으로 네이밍 변환한 것도 더 직관적이고 좋은 것 같아요)
근데 별건 아니지만 개인적으로 ReissueTokenService라는 네이밍이 조금 어색하게 느껴지는데, Reissue 경로에서 Refresh token을 발급하는 흐름이니 차라리 RefreshTokenService가 더 낫지 않나 생각하긴 했어요!
혹시 Reissue라고 하신 다른 이유가 있는거면 알려주세욥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
엑세스토큰과 리프레시 토큰을 같이 재발급해서 ReissueTokenService라는 이름으로 지었습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 여기 코드만 보고 두 로직이 따로 있는 줄 알았는데 같이 있었네요! 좋습니다
ksg1227
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 깔끔한 것 같습니다! 크게 리뷰남길 부분이 없군여 굿굿
| dev: dev-db, dev-port, common | ||
| prod: prod-db, prod-port, common | ||
| active: prod | ||
| active: local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분 dev 로 바꿔야할 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ksg1227 엇 이렇게 하면 문제가 발생하나요? 도커 컴포즈 설정 파일을 봤을 때는 개발환경이나 운영환경에서 active profile을 지정하던데...
- 레디스 키가 변경될 이유가 없고 로직과 밀접한 관련이 있으므로 키를 상수로 변경
- 설정파일에서 레디스키 변수를 삭제
d2a80ce to
f038010
Compare
Related issue 🛠
Work Description 📝
Screenshot 📸
Uncompleted Tasks 😅
To Reviewers 📢
Summary by CodeRabbit
새로운 기능
✏️ Tip: You can customize this high-level summary in your review settings.