feature/#64 rtr 기반 refresh token 발행 기능 구현 #65
Conversation
|
Warning Rate limit exceeded@minjo-on has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
개요워크스루이 풀 리퀘스트는 애플리케이션의 인증 및 토큰 관리 시스템을 리팩토링하고 개선합니다. 주요 변경 사항은 Redis를 활용한 리프레시 토큰 회전(RTR) 메커니즘 도입, JWT 토큰 생성 및 관리 로직 재설계, 인증 관련 서비스 구조 변경을 포함합니다. 새로운 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client
participant AuthFacade
participant JwtService
participant UserCommandService
participant RefreshTokenService
participant JwtProvider
Client->>AuthFacade: 로그인 요청
AuthFacade->>JwtService: 로그인 처리
JwtService->>UserCommandService: 사용자 인증
UserCommandService-->>JwtService: 사용자 정보
JwtService->>JwtProvider: 토큰 생성
JwtProvider-->>JwtService: 액세스/리프레시 토큰
JwtService->>RefreshTokenService: 리프레시 토큰 저장
JwtService-->>AuthFacade: JWT 응답
AuthFacade-->>Client: 토큰 반환
이슈 대상 평가
관련 가능성 있는 PR
제안된 레이블
토끼의 시 🐰
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (18)
soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java (1)
46-48: 메서드 시그니처 개선이 잘 되었습니다!메서드의 책임이 더 명확해졌고, 의존성이 줄어들었습니다. 다만 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
public User processLogin(String idToken) { + if (idToken == null || idToken.isBlank()) { + throw new IllegalArgumentException("ID 토큰은 null이거나 비어있을 수 없습니다."); + } OidcIdToken oidcIdToken = validateAndDecodeIdToken(idToken); return findOrCreateUser(oidcIdToken); }soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/exception/RefreshTokenExceptionCode.java (2)
11-13: 클래스 선언이 적절합니다만, 문서화를 추가하면 좋겠습니다.Lombok 어노테이션을 통해 코드가 간결해졌습니다. 다만, 이 예외 코드의 용도와 사용 사례에 대한 JavaDoc 문서화를 추가하면 더 좋을 것 같습니다.
/** * Refresh Token과 관련된 예외 상황을 정의하는 열거형 클래스입니다. * ExceptionCode 인터페이스를 구현하여 일관된 예외 처리를 보장합니다. */
14-15: 추가적인 예외 케이스 고려가 필요합니다.현재는 토큰을 찾을 수 없는 경우만 처리하고 있습니다. 다음과 같은 추가적인 예외 상황도 고려해보시는 것은 어떨까요?
- 토큰 만료 (REFRESH_TOKEN_EXPIRED)
- 유효하지 않은 토큰 형식 (INVALID_REFRESH_TOKEN_FORMAT)
- 토큰 재발급 횟수 초과 (REFRESH_TOKEN_REISSUE_LIMIT_EXCEEDED)
soridam-api/src/main/java/sorisoop/soridam/api/noise/presentation/response/NoisePersistResponse.java (1)
Line range hint
9-12: UUID 형식 검증 로직 추가 제안Schema 예시에서 'nse_' 접두사가 포함된 UUID 형식을 보여주고 있습니다. 응답 전에 이 형식을 검증하는 것이 좋을 것 같습니다.
다음과 같은 검증 로직 추가를 제안드립니다:
public static NoisePersistResponse from(Noise noise){ + String uuid = noise.extractUuid(); + if (!uuid.startsWith("nse_") || uuid.length() != 27) { + throw new IllegalStateException("잘못된 UUID 형식입니다: " + uuid); + } return builder() - .id(noise.extractUuid()) + .id(uuid) .build(); }soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/RefreshTokenRepository.java (1)
10-10: 커스텀 쿼리 메서드 구현이 적절합니다.Spring Data의 명명 규칙을 잘 따르고 있으며, Optional을 사용하여 null 처리를 안전하게 하고 있습니다.
하나 제안드리고 싶은 점은 성능 최적화입니다.
Redis에서 refreshToken으로 조회 시 성능을 최적화하기 위해 @indexed 어노테이션 사용을 고려해보시기 바랍니다:
public class RefreshToken { @Indexed private String refreshToken; // ... }soridam-infra/src/main/java/sorisoop/soridam/infra/config/redis/RedisConfig.java (2)
16-18: Redis 저장소 구성의 범위 검토 필요
@EnableRedisRepositories의 basePackages가 "sorisoop.soridam"으로 설정되어 있어 범위가 너무 넓을 수 있습니다. Redis 저장소가 실제로 필요한 패키지로 범위를 좁히는 것이 좋습니다.-@EnableRedisRepositories(basePackages = "sorisoop.soridam") +@EnableRedisRepositories(basePackages = "sorisoop.soridam.infra.redis")
35-38: Redis 타임아웃 설정 최적화 필요현재 command 타임아웃이 5초로 설정되어 있습니다. Redis는 일반적으로 밀리초 단위의 응답 시간을 가지므로, 이 값이 너무 높을 수 있습니다. 성능 최적화를 위해 더 낮은 값을 고려해보세요.
- .commandTimeout(Duration.ofSeconds(5)) + .commandTimeout(Duration.ofMillis(500))soridam-auth/build.gradle (1)
26-27: 의존성 정렬 개선 제안가독성을 위해 Redis 의존성을 다른 Spring Boot 스타터 의존성들과 함께 그룹화하는 것이 좋습니다.
dependencies { implementation 'org.springframework.boot:spring-boot-starter-web' implementation 'org.springframework.boot:spring-boot-starter-oauth2-client' + implementation 'org.springframework.boot:spring-boot-starter-data-redis' // 도메인 및 공통 모듈 implementation project(':soridam-common') implementation project(':soridam-domain') implementation project(':soridam-global-util') implementation 'io.jsonwebtoken:jjwt-api:0.12.6' runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.12.6' runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.12.6' //swagger implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.8.1' - implementation 'org.springframework.boot:spring-boot-starter-data-redis' }soridam-domain/build.gradle (1)
31-31: 의존성 정렬 개선 제안가독성을 위해 Redis 의존성을 다른 Spring Boot 스타터 의존성들과 함께 그룹화하는 것이 좋습니다.
// 데이터베이스와 상호작용 (JPA 사용) + implementation 'org.springframework.boot:spring-boot-starter-data-redis' implementation 'org.springframework.boot:spring-boot-starter-data-jpa' - implementation 'org.springframework.boot:spring-boot-starter-data-redis'soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/exception/RefreshTokenNotFoundException.java (1)
7-11: 예외 처리 구현이 잘 되어있습니다!예외 클래스가 명확하게 구현되어 있습니다. 다만, 클래스 레벨에 Javadoc 주석을 추가하여 이 예외가 언제 발생하는지 문서화하면 더 좋을 것 같습니다.
예시 Javadoc:
/** * Redis에서 리프레시 토큰을 찾을 수 없을 때 발생하는 예외입니다. */ public class RefreshTokenNotFoundException extends CustomException {soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/application/RefreshTokenService.java (1)
12-14: 트랜잭션 관리 추가 필요여러 Redis 작업이 하나의 트랜잭션으로 처리되어야 하는 경우를 대비하여 @transactional 어노테이션 추가를 고려해주세요.
@Service @RequiredArgsConstructor +@Transactional public class RefreshTokenService {soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/RefreshToken.java (2)
11-15: TTL 값의 설정 방식 개선이 필요합니다.현재 리프레시 토큰의 만료 시간(TTL)이 코드에 하드코딩되어 있습니다. 환경에 따라 유연하게 변경할 수 있도록 설정 파일로 분리하는 것이 좋을 것 같습니다.
다음과 같이 수정하는 것을 제안드립니다:
-@RedisHash(value = "refreshToken", timeToLive = 60 * 60 * 24) +@RedisHash(value = "refreshToken", timeToLive = "${refresh.token.ttl:86400}")그리고 application.yml에 다음 설정을 추가:
refresh: token: ttl: 86400 # 24시간을 초 단위로 표현
22-27: 팩토리 메서드의 문서화와 유효성 검증이 필요합니다.팩토리 메서드가 잘 구현되어 있지만, 문서화와 매개변수 검증이 없습니다.
다음과 같이 개선하는 것을 제안드립니다:
+/** + * RefreshToken 객체를 생성합니다. + * + * @param userId 사용자 ID + * @param refreshToken 리프레시 토큰 문자열 + * @return 새로운 RefreshToken 객체 + * @throws IllegalArgumentException 매개변수가 null이거나 비어있는 경우 + */ public static RefreshToken of(String userId, String refreshToken) { + if (userId == null || userId.trim().isEmpty()) { + throw new IllegalArgumentException("사용자 ID는 필수입니다"); + } + if (refreshToken == null || refreshToken.trim().isEmpty()) { + throw new IllegalArgumentException("리프레시 토큰은 필수입니다"); + } return RefreshToken.builder() .userId(userId) .refreshToken(refreshToken) .build(); }soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/request/jwt/RefreshTokenRequest.java (1)
12-13: 토큰 유효성 검증 강화 필요@NotNull 외에도 토큰 형식에 대한 추가적인 유효성 검증이 필요합니다. @pattern 어노테이션을 사용하여 토큰 형식을 검증하는 것이 좋습니다.
@NotNull +@Pattern(regexp = "^[A-Za-z0-9-_=]+\\.[A-Za-z0-9-_=]+\\.?[A-Za-z0-9-_.+/=]*$", + message = "올바른 토큰 형식이 아닙니다") String refreshTokensoridam-domain/src/main/java/sorisoop/soridam/domain/user/application/UserCommandService.java (1)
16-16: 트랜잭션 범위 최적화 필요클래스 레벨의 @transactional은 모든 메서드에 트랜잭션이 적용되어 불필요한 오버헤드가 발생할 수 있습니다. 필요한 메서드에만 개별적으로 적용하는 것이 좋습니다.
-@Transactional public class UserCommandService {soridam-api/src/main/java/sorisoop/soridam/api/auth/application/AuthFacade.java (1)
28-39: 소셜 로그인 중복 코드 제거 필요kakaoLogin과 googleLogin 메서드에 중복된 코드가 있습니다. 공통 로직을 추출하여 재사용성을 높이는 것이 좋습니다.
+private JwtResponse processSocialLogin(User user) { + user.updateLastLoginTime(); + return jwtService.getToken(user); +} @Transactional public JwtResponse kakaoLogin(OidcLoginRequest idToken) { User user = kakaoOidcService.processLogin(idToken.idToken()); - user.updateLastLoginTime(); - return jwtService.getToken(user); + return processSocialLogin(user); } @Transactional public JwtResponse googleLogin(OidcLoginRequest idToken) { User user = googleOidcService.processLogin(idToken.idToken()); - user.updateLastLoginTime(); - return jwtService.getToken(user); + return processSocialLogin(user); }soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/JwtAuthenticationFilter.java (1)
14-14: 토큰 타입 검증 로직 추가를 고려해 보세요.현재는 모든 토큰을 액세스 토큰으로 간주하고 있습니다. 리프레시 토큰이 실수로 전달되는 경우를 방지하기 위해 토큰 타입 검증을 추가하면 좋을 것 같습니다.
soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/application/JwtProvider.java (1)
38-41: 액세스 토큰 생성 메서드 구현 검토액세스 토큰 생성 메서드가 적절히 구현되었습니다. 다만 다음 사항들을 고려해보시기 바랍니다:
- 토큰 만료 시간을 설정 파일로 관리하는 것이 더 유연할 수 있습니다.
- 토큰 생성 시 사용되는 시간값(now)이 메서드마다 중복 생성됩니다.
+private static final Duration DEFAULT_ACCESS_TOKEN_DURATION = Duration.ofDays(2); + public String generateAccessToken(String userId, Role role) { - Date now = new Date(); - return makeToken(new Date(now.getTime() + Duration.ofDays(2).toMillis()), userId, role); + return generateAccessToken(userId, role, DEFAULT_ACCESS_TOKEN_DURATION); }Also applies to: 43-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
soridam-api/src/main/java/sorisoop/soridam/api/auth/application/AuthFacade.java(1 hunks)soridam-api/src/main/java/sorisoop/soridam/api/auth/application/AuthService.java(0 hunks)soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/AuthApiController.java(2 hunks)soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/request/jwt/RefreshTokenRequest.java(1 hunks)soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/request/oauth/OidcLoginRequest.java(1 hunks)soridam-api/src/main/java/sorisoop/soridam/api/noise/presentation/response/NoisePersistResponse.java(1 hunks)soridam-api/src/main/resources/application.yml(1 hunks)soridam-auth/build.gradle(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/config/SecurityConfig.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/JwtAuthenticationFilter.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/application/JwtProvider.java(4 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/application/JwtService.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/response/JwtResponse.java(1 hunks)soridam-auth/src/main/java/sorisoop/soridam/auth/oauth/OidcService.java(1 hunks)soridam-domain/build.gradle(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/noise/application/NoiseCommandService.java(2 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/RefreshToken.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/RefreshTokenRepository.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/application/RefreshTokenService.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/exception/RefreshTokenExceptionCode.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/exception/RefreshTokenNotFoundException.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/application/UserCommandService.java(2 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/application/UserQueryService.java(2 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java(3 hunks)soridam-global-util/src/main/java/sorisoop/soridam/globalutil/logging/LoggingUtils.java(1 hunks)soridam-infra/build.gradle(1 hunks)soridam-infra/src/main/java/sorisoop/soridam/infra/config/redis/RedisConfig.java(1 hunks)
💤 Files with no reviewable changes (1)
- soridam-api/src/main/java/sorisoop/soridam/api/auth/application/AuthService.java
✅ Files skipped from review due to trivial changes (2)
- soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/response/JwtResponse.java
- soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/request/oauth/OidcLoginRequest.java
🔇 Additional comments (19)
soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/exception/RefreshTokenExceptionCode.java (2)
1-10: 패키지 구조와 임포트가 잘 구성되어 있습니다!도메인 주도 설계 원칙에 따라 패키지가 잘 구성되어 있으며, 필요한 임포트들이 깔끔하게 정리되어 있습니다.
17-23: 필드 선언과 메소드 구현이 깔끔합니다!상태와 메시지 필드가 적절히 캡슐화되어 있으며, getCode() 메소드의 구현이 간단명료합니다.
soridam-api/src/main/java/sorisoop/soridam/api/noise/presentation/response/NoisePersistResponse.java (1)
16-16: 새로운 extractUuid() 메서드 구현 확인 필요
getId()에서extractUuid()로의 변경이 확인되었습니다. UUID 추출 로직의 정확성을 검증해야 합니다.다음 스크립트를 실행하여
extractUuid()메서드의 구현을 확인하겠습니다:✅ Verification successful
extractUuid() 메서드 구현이 확인되었습니다
UuidExtractable 인터페이스의 기본 구현을 통해 UUID 추출이 안전하게 처리되고 있습니다. 접두사 처리와 널 안전성이 보장되어 있어 현재 구현이 적절합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of extractUuid() method in Noise class # Search for the Noise class implementation ast-grep --pattern 'class Noise { $$$ extractUuid() { $$$ } $$$ }' # Search for any test cases related to UUID extraction rg -A 5 "extractUuid|UUID" --type javaLength of output: 4447
soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/RefreshTokenRepository.java (2)
8-9: 리포지토리 구조가 잘 설계되었습니다!CrudRepository를 상속받아 기본적인 CRUD 작업과 함께 Redis 기반의 RefreshToken 관리가 가능하도록 잘 구현되었습니다.
1-11: Redis 설정 검증이 필요합니다.Redis 연결 설정이 올바르게 되어있는지 확인이 필요합니다.
아래 스크립트로 Redis 설정 파일의 존재 여부와 내용을 확인하겠습니다:
✅ Verification successful
Redis가 올바르게 구성되어 있습니다.
- application.properties에 Redis 연결 설정이 정의되어 있습니다
- RedisConfig 설정 클래스가 존재합니다 (soridam-infra/src/main/java/sorisoop/soridam/infra/config/redis/RedisConfig.java)
- 필요한 Redis 의존성이 모든 관련 모듈에 포함되어 있습니다
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Redis 설정 파일 검색 echo "Searching for Redis configuration files..." fd -e yml -e yaml -e properties | xargs rg -l "spring\.redis|redis\." # Redis 의존성 확인 echo "Checking Redis dependencies..." fd "build.gradle" | xargs rg "redis"Length of output: 584
Script:
#!/bin/bash # Redis 설정 클래스 검색 echo "Searching for Redis configuration classes..." ast-grep --pattern 'class $_ { $$$ @Bean $$$ Redis$_($$$) { $$$ } $$$ }' # application.properties/yml 파일 검색 (다른 패턴) echo "Searching for application properties..." fd "application" -e yml -e yaml -e properties | xargs cat # Redis 관련 Java 설정 클래스 검색 echo "Searching for Redis-related Java config files..." fd -e java | xargs rg -l "Redis.*Config|@EnableRedis|RedisTemplate"Length of output: 2750
soridam-infra/src/main/java/sorisoop/soridam/infra/config/redis/RedisConfig.java (1)
45-55: Redis 트랜잭션 설정 검토 필요
setEnableTransactionSupport(true)로 설정되어 있습니다. Redis 트랜잭션이 실제로 필요한지 검토해주세요. 트랜잭션이 필요하지 않은 경우 불필요한 오버헤드가 발생할 수 있습니다.soridam-infra/build.gradle (1)
23-23: Redis 의존성 추가 승인Spring Boot Redis 스타터 의존성이 적절하게 추가되었습니다.
soridam-domain/src/main/java/sorisoop/soridam/domain/noise/application/NoiseCommandService.java (2)
3-4: 깔끔한 static import 사용을 확인했습니다!코드의 가독성을 높이기 위해 UuidPrefix.NOISE를 static import로 처리한 것이 좋습니다.
33-34: UUID 접두사 추가로 인한 기존 데이터 호환성 검증이 필요합니다.기존 ID 조회 방식이 변경되어 데이터 접근에 영향을 줄 수 있습니다. 다음 사항들을 확인해 주시기 바랍니다:
- 기존 데이터의 마이그레이션 계획
- 롤백 전략
- 데이터 정합성 검증 방안
아래 스크립트를 실행하여 현재 저장된 Noise 엔티티들의 ID 형식을 확인해 주세요:
✅ Verification successful
UUID 접두사 처리가 프레임워크 레벨에서 안전하게 구현되어 있습니다.
다음과 같은 이유로 데이터 호환성 문제가 발생하지 않습니다:
@PrefixedUuid어노테이션으로 엔티티 생성 시점부터 접두사가 자동 적용됩니다.- 조회 시
NOISE.getPrefix()를 통해 일관된 접두사 처리가 이루어집니다.- UUID 생성과 처리가
PrefixedUuidGenerator를 통해 프레임워크 레벨에서 관리됩니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the ID format of existing Noise entities # Search for Noise entity ID usage patterns rg -A 2 "findById|save|delete.*Noise" # Check for any direct ID assignments or comparisons ast-grep --pattern 'noise.setId($$$)'Length of output: 8427
Script:
#!/bin/bash # Check for database schema files and entity definitions fd -e sql -e java "Noise.*" # Look for JPA entity annotations and ID field definitions rg -A 5 "@Entity.*Noise|@Id.*String" # Search for migration scripts fd -e sql -e xml "migration|schema|V[0-9]"Length of output: 2610
Script:
#!/bin/bash # Examine the Noise entity definition cat soridam-domain/src/main/java/sorisoop/soridam/domain/noise/domain/Noise.java # Look for any ID-related constants or prefix definitions rg -A 2 "PREFIX|ID_PREFIX|getPrefix"Length of output: 4019
soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/RefreshToken.java (2)
1-10: 패키지 구조와 임포트가 잘 구성되어 있습니다!도메인 계층에 적절하게 위치해 있으며, 필요한 임포트들이 잘 정리되어 있습니다.
1-28: 보안 관련 검증이 필요합니다.리프레시 토큰은 민감한 인증 데이터를 다루므로, 다음 사항들을 확인해주시기 바랍니다:
- Redis 연결이 TLS로 보호되어 있는지
- Redis 접근 제어가 적절히 설정되어 있는지
- 토큰 값이 충분한 엔트로피를 가지고 있는지
다음 스크립트로 Redis 설정을 확인할 수 있습니다:
soridam-api/src/main/java/sorisoop/soridam/api/auth/application/AuthFacade.java (1)
41-43: 토큰 재발급 예외 처리 강화 필요토큰 재발급 시 발생할 수 있는 다양한 예외 상황(만료된 토큰, 유효하지 않은 토큰 등)에 대한 처리가 필요합니다.
soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/application/JwtService.java (2)
21-26: 로그인 및 토큰 발행 로직이 잘 구현되었습니다!로그인 검증, 토큰 생성, 그리고 리프레시 토큰 저장의 순서가 적절합니다.
리프레시 토큰 저장 실패 시의 예외 처리가
refreshTokenService에 구현되어 있는지 확인해 주세요.
40-45: 토큰 생성 로직이 깔끔하게 구현되었습니다!사용자 UUID와 역할을 이용한 토큰 생성 방식이 적절하며, 응답 객체 생성이 잘 분리되어 있습니다.
soridam-global-util/src/main/java/sorisoop/soridam/globalutil/logging/LoggingUtils.java (1)
28-30: null 체크 추가가 적절합니다!객체가 null인 경우에 대한 방어적 처리가 잘 되었습니다.
soridam-domain/src/main/java/sorisoop/soridam/domain/user/domain/User.java (2)
73-75:@OneToMany관계에 LAZY 로딩 전략 적용에 대한 피드백성능 최적화를 위해
@OneToMany관계에 LAZY 로딩을 적용한 것은 좋은 선택입니다. 이는 N+1 문제를 방지하는데 도움이 됩니다.
94-94: 사용자 생성 시 기본 역할 설정 구현 검토모든 사용자 생성 메서드에서
Role.USER를 일관되게 설정하여 보안을 강화한 것이 좋습니다.Also applies to: 102-103, 112-113
soridam-auth/src/main/java/sorisoop/soridam/auth/config/SecurityConfig.java (1)
26-26: OAuth2 엔드포인트 보안 설정 확인 필요OAuth2 관련 패턴이 제거되었습니다. OAuth2 엔드포인트에 대한 보안이 적절히 처리되고 있는지 확인이 필요합니다.
Also applies to: 43-52
✅ Verification successful
OAuth2 엔드포인트 보안 설정 검증 완료
OAuth2 관련 엔드포인트들이
/api/auth/**패턴에 포함되어 있으며, 이는PUBLIC_ENDPOINTS를 통해 적절히 보호되고 있습니다. OAUTH2_PATTERNS 상수 제거는 보안에 영향을 미치지 않습니다.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # OAuth2 관련 엔드포인트 설정 확인 rg -l "oauth2" .Length of output: 510
Script:
#!/bin/bash # SecurityConfig 파일 전체 내용 확인 cat soridam-auth/src/main/java/sorisoop/soridam/auth/config/SecurityConfig.java # AuthApiController의 OAuth2 관련 엔드포인트 확인 rg "oauth2|oidc" ./soridam-api/src/main/java/sorisoop/soridam/api/auth/presentation/AuthApiController.java -B 2 -A 2Length of output: 4600
soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/application/JwtProvider.java (1)
111-111: 권한 검증 로직 강화기본 케이스에서
UnauthorizedException을 발생시키도록 변경한 것은 보안 강화에 도움이 됩니다.
| data: | ||
| redis: | ||
| host: ${REDIS_HOST:localhost} | ||
| port: ${REDIS_PORT:6379} | ||
| password: ${REDIS_PASSWORD} | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Redis 구성 경로 불일치 및 보안 설정 개선 필요
application.yml의 Redis 구성에 다음과 같은 문제가 있습니다:
- 구성 경로가
RedisConfig.java와 일치하지 않습니다. Java 코드는spring.data.redis를 참조하지만, YAML은data.redis로 설정되어 있습니다. - 프로덕션 환경을 위한 SSL/TLS 설정이 여전히 필요합니다.
다음과 같이 수정을 제안드립니다:
- data:
+ spring:
+ data:
redis:
host: ${REDIS_HOST:localhost}
port: ${REDIS_PORT:6379}
password: ${REDIS_PASSWORD}
+ ssl:
+ enabled: ${REDIS_SSL_ENABLED:false}🔗 Analysis chain
Redis 구성의 보안 관련 개선사항
Redis 구성에서 다음 사항들을 고려해주시기 바랍니다:
REDIS_PASSWORD환경 변수가 설정되지 않은 경우의 처리가 없습니다. 애플리케이션 시작 시 오류가 발생할 수 있습니다.- 프로덕션 환경에서는 SSL/TLS 설정이 필요할 수 있습니다.
다음과 같이 수정하는 것을 제안드립니다:
data:
redis:
host: ${REDIS_HOST:localhost}
port: ${REDIS_PORT:6379}
- password: ${REDIS_PASSWORD}
+ password: ${REDIS_PASSWORD:}
+ ssl:
+ enabled: ${REDIS_SSL_ENABLED:false}환경 변수가 설정되어 있는지 확인하기 위해 다음 스크립트를 실행해보세요:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if Redis-related environment variables are set in various config files
echo "Checking for Redis environment variables in configuration files..."
rg -l "REDIS_(HOST|PORT|PASSWORD)"
Length of output: 226
Script:
#!/bin/bash
# Search for Redis-related configurations in Java code and other config files
echo "Searching for Redis configuration in Java code..."
rg -t java "Redis" -A 5
echo -e "\nSearching for SSL/TLS configurations..."
rg "ssl" -i
Length of output: 6385
| public RefreshToken getToken(String token) { | ||
| return refreshTokenRepository.findByRefreshToken(token) | ||
| .orElseThrow(RefreshTokenNotFoundException::new); | ||
| } |
There was a problem hiding this comment.
토큰 검증 로직 추가 필요
getToken 메소드에서 입력된 토큰 값에 대한 기본적인 유효성 검사가 없습니다. 토큰이 null이거나 빈 문자열인 경우에 대한 처리가 필요합니다.
다음과 같이 수정을 제안드립니다:
public RefreshToken getToken(String token) {
+ if (token == null || token.trim().isEmpty()) {
+ throw new IllegalArgumentException("토큰 값이 비어있습니다");
+ }
return refreshTokenRepository.findByRefreshToken(token)
.orElseThrow(RefreshTokenNotFoundException::new);
}📝 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 RefreshToken getToken(String token) { | |
| return refreshTokenRepository.findByRefreshToken(token) | |
| .orElseThrow(RefreshTokenNotFoundException::new); | |
| } | |
| public RefreshToken getToken(String token) { | |
| if (token == null || token.trim().isEmpty()) { | |
| throw new IllegalArgumentException("토큰 값이 비어있습니다"); | |
| } | |
| return refreshTokenRepository.findByRefreshToken(token) | |
| .orElseThrow(RefreshTokenNotFoundException::new); | |
| } |
| public void delete(RefreshToken refreshToken) { | ||
| refreshTokenRepository.delete(refreshToken); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
삭제 작업 실패 처리 필요
delete 메소드에서 삭제 작업 실패 시에 대한 예외 처리가 필요합니다.
public void delete(RefreshToken refreshToken) {
+ if (refreshToken == null) {
+ throw new IllegalArgumentException("삭제할 토큰 정보가 없습니다");
+ }
refreshTokenRepository.delete(refreshToken);
}📝 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 delete(RefreshToken refreshToken) { | |
| refreshTokenRepository.delete(refreshToken); | |
| } | |
| public void delete(RefreshToken refreshToken) { | |
| if (refreshToken == null) { | |
| throw new IllegalArgumentException("삭제할 토큰 정보가 없습니다"); | |
| } | |
| refreshTokenRepository.delete(refreshToken); | |
| } |
| public void save(String userId, String token) { | ||
| refreshTokenRepository.save(RefreshToken.of(userId, token)); | ||
| }} |
There was a problem hiding this comment.
💡 Codebase verification
리프레시 토큰 보안 검토 결과
TTL은 이미 @RedisHash 어노테이션을 통해 24시간으로 적절히 설정되어 있습니다. 다만 다음 보안 개선사항들은 여전히 필요합니다:
public void save(String userId, String token) {
+ if (userId == null || token == null) {
+ throw new IllegalArgumentException("사용자 ID와 토큰은 필수값입니다");
+ }
+
+ // 기존 토큰이 있다면 삭제
+ refreshTokenRepository.findByUserId(userId)
+ .ifPresent(refreshTokenRepository::delete);
+
refreshTokenRepository.save(RefreshToken.of(userId, token));
}🔗 Analysis chain
토큰 저장 시 보안 개선 필요
save 메소드에서 다음 사항들을 고려해야 합니다:
- userId와 token의 유효성 검증
- 동일 사용자의 기존 토큰 처리 방안
- TTL(Time-To-Live) 설정
아래와 같은 개선을 제안드립니다:
public void save(String userId, String token) {
+ if (userId == null || token == null) {
+ throw new IllegalArgumentException("사용자 ID와 토큰은 필수값입니다");
+ }
+
+ // 기존 토큰이 있다면 삭제
+ refreshTokenRepository.findByUserId(userId)
+ .ifPresent(refreshTokenRepository::delete);
+
refreshTokenRepository.save(RefreshToken.of(userId, token));
}Redis TTL 설정이 되어있는지 확인이 필요합니다:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Redis 관련 설정 파일 검색
rg -l 'redis' --type yaml
Length of output: 72
Script:
#!/bin/bash
# Check Redis configuration in application.yml
cat soridam-api/src/main/resources/application.yml
# Check RefreshToken entity for TTL configuration
ast-grep --pattern 'class RefreshToken {
$$$
}'
Length of output: 1120
Script:
#!/bin/bash
# Search for Redis configuration classes
rg -l "RedisConfiguration|@EnableRedis|@TimeToLive"
# Check RefreshToken implementation
fd RefreshToken.java --exec cat {}
Length of output: 822
| @Id | ||
| private String userId; | ||
|
|
||
| @Indexed | ||
| private String refreshToken; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
필드 유효성 검증 추가가 필요합니다.
현재 userId와 refreshToken 필드에 대한 유효성 검증이 없습니다. null 체크나 형식 검증을 추가하면 좋을 것 같습니다.
다음과 같은 검증 어노테이션 추가를 제안드립니다:
@Id
+@NotBlank(message = "사용자 ID는 필수입니다")
private String userId;
@Indexed
+@NotBlank(message = "리프레시 토큰은 필수입니다")
+@Size(min = 1, message = "리프레시 토큰은 비어있을 수 없습니다")
private String refreshToken;📝 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.
| @Id | |
| private String userId; | |
| @Indexed | |
| private String refreshToken; | |
| @Id | |
| @NotBlank(message = "사용자 ID는 필수입니다") | |
| private String userId; | |
| @Indexed | |
| @NotBlank(message = "리프레시 토큰은 필수입니다") | |
| @Size(min = 1, message = "리프레시 토큰은 비어있을 수 없습니다") | |
| private String refreshToken; |
| public User login(String email, String password){ | ||
| User user = userRepository.findByEmail(email) | ||
| .orElseThrow(UserNotFoundException::new); | ||
| user.isPasswordMatching(password, bCryptPasswordEncoder); | ||
| user.updateLastLoginTime(); | ||
|
|
||
| return user; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
로그인 보안 강화 필요
- 로그인 시도 횟수 제한이 없어 무차별 대입 공격에 취약할 수 있습니다.
- 비밀번호 비교 시 일정 시간 비교(constant-time comparison)를 사용하는 것이 좋습니다.
- 실패한 로그인 시도를 로깅하는 것이 보안상 좋습니다.
다음과 같은 개선을 제안합니다:
public User login(String email, String password){
+ // 로그인 시도 횟수 확인
+ checkLoginAttempts(email);
User user = userRepository.findByEmail(email)
.orElseThrow(UserNotFoundException::new);
- user.isPasswordMatching(password, bCryptPasswordEncoder);
+ boolean isValid = user.isPasswordMatching(password, bCryptPasswordEncoder);
+ if (!isValid) {
+ // 실패한 로그인 시도 기록
+ recordFailedLoginAttempt(email);
+ throw new InvalidCredentialsException();
+ }
user.updateLastLoginTime();
return user;
}Committable suggestion skipped: line range outside the PR's diff.
| try{ | ||
| Object principal = SecurityContextHolder.getContext().getAuthentication().getPrincipal(); | ||
| String userId = ((UserDetails)principal).getUsername(); | ||
| return getById(userId); | ||
| return getById(USER.getPrefix() + userId); | ||
| } catch (Exception e){ | ||
| throw new UnauthorizedException(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
보안 컨텍스트 처리 개선 필요
- 현재 예외 처리가 너무 일반적입니다. 구체적인 예외 처리가 필요합니다.
- SecurityContext가 null인 경우에 대한 명시적인 처리가 필요합니다.
public User me() {
- try {
- Object principal = SecurityContextHolder.getContext().getAuthentication().getPrincipal();
+ Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
+ if (authentication == null) {
+ throw new UnauthorizedException("인증 정보가 없습니다");
+ }
+ try {
+ Object principal = authentication.getPrincipal();
String userId = ((UserDetails)principal).getUsername();
return getById(USER.getPrefix() + userId);
- } catch (Exception e) {
- throw new UnauthorizedException();
+ } catch (ClassCastException e) {
+ throw new UnauthorizedException("잘못된 인증 타입입니다");
}
}📝 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.
| try{ | |
| Object principal = SecurityContextHolder.getContext().getAuthentication().getPrincipal(); | |
| String userId = ((UserDetails)principal).getUsername(); | |
| return getById(userId); | |
| return getById(USER.getPrefix() + userId); | |
| } catch (Exception e){ | |
| throw new UnauthorizedException(); | |
| } | |
| Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); | |
| if (authentication == null) { | |
| throw new UnauthorizedException("인증 정보가 없습니다"); | |
| } | |
| try { | |
| Object principal = authentication.getPrincipal(); | |
| String userId = ((UserDetails)principal).getUsername(); | |
| return getById(USER.getPrefix() + userId); | |
| } catch (ClassCastException e) { | |
| throw new UnauthorizedException("잘못된 인증 타입입니다"); | |
| } |
| public JwtResponse reissue(String token) { | ||
| RefreshToken refreshToken = refreshTokenService.getToken(token); | ||
| String userId = refreshToken.getUserId(); | ||
| User user = userQueryService.getById(userId); | ||
| JwtResponse response = getToken(user); | ||
|
|
||
| refreshTokenService.delete(refreshToken); | ||
| refreshTokenService.save(userId, response.refreshToken()); | ||
|
|
||
| return response; | ||
| } |
There was a problem hiding this comment.
리프레시 토큰 재발행 시 동시성 문제 가능성이 있습니다.
현재 구현에서는 이전 토큰 삭제와 새 토큰 저장이 원자적으로 실행되지 않습니다. 동시에 여러 요청이 들어올 경우 문제가 발생할 수 있습니다.
다음과 같은 개선을 제안드립니다:
public JwtResponse reissue(String token) {
RefreshToken refreshToken = refreshTokenService.getToken(token);
String userId = refreshToken.getUserId();
User user = userQueryService.getById(userId);
JwtResponse response = getToken(user);
- refreshTokenService.delete(refreshToken);
- refreshTokenService.save(userId, response.refreshToken());
+ refreshTokenService.replaceToken(refreshToken, userId, response.refreshToken());
return response;
}RefreshTokenService에 원자적 작업을 위한 replaceToken 메소드를 추가하는 것을 추천드립니다.
Committable suggestion skipped: line range outside the PR's diff.
| @PostMapping("/reissue") | ||
| public ResponseEntity<JwtResponse> reissue( | ||
| @Valid | ||
| @RequestBody | ||
| RefreshTokenRequest request | ||
| ){ | ||
| JwtResponse response = authFacade.reissue(request); | ||
| return ResponseEntity.ok(response); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
리프레시 토큰 재발행 API에 대한 문서화가 필요합니다.
다른 엔드포인트들과 같이 @Operation과 @ApiResponse 어노테이션을 추가하여 API 문서화를 해주세요.
다음과 같이 추가하는 것을 제안드립니다:
+ @Operation(summary = "토큰 재발행 API", description = """
+ - Description : 이 API는 리프레시 토큰을 사용하여 새로운 액세스 토큰을 발급합니다.
+ """)
+ @ApiResponse(responseCode = "200")
@PostMapping("/reissue")
public ResponseEntity<JwtResponse> reissue(📝 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.
| @PostMapping("/reissue") | |
| public ResponseEntity<JwtResponse> reissue( | |
| @Valid | |
| @RequestBody | |
| RefreshTokenRequest request | |
| ){ | |
| JwtResponse response = authFacade.reissue(request); | |
| return ResponseEntity.ok(response); | |
| } | |
| @Operation(summary = "토큰 재발행 API", description = """ | |
| - Description : 이 API는 리프레시 토큰을 사용하여 새로운 액세스 토큰을 발급합니다. | |
| """) | |
| @ApiResponse(responseCode = "200") | |
| @PostMapping("/reissue") | |
| public ResponseEntity<JwtResponse> reissue( | |
| @Valid | |
| @RequestBody | |
| RefreshTokenRequest request | |
| ){ | |
| JwtResponse response = authFacade.reissue(request); | |
| return ResponseEntity.ok(response); | |
| } |
soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/application/JwtProvider.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/application/JwtService.java(1 hunks)soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/application/RefreshTokenService.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- soridam-auth/src/main/java/sorisoop/soridam/auth/jwt/application/JwtService.java
🔇 Additional comments (3)
soridam-domain/src/main/java/sorisoop/soridam/domain/refresh/application/RefreshTokenService.java (3)
1-13: 클래스 구조가 적절합니다!Spring 서비스 계층의 모범 사례를 잘 따르고 있으며, 의존성 주입이 올바르게 구현되어 있습니다.
15-18: 토큰 유효성 검증 로직 추가 필요토큰의 null 또는 빈 문자열 체크가 누락되어 있습니다.
public RefreshToken getToken(String token) { + if (token == null || token.trim().isEmpty()) { + throw new IllegalArgumentException("토큰 값이 비어있습니다"); + } return refreshTokenRepository.findByRefreshToken(token) .orElseThrow(RefreshTokenNotFoundException::new); }
20-24: 🛠️ Refactor suggestionsave 메소드 개선 필요사항
- 입력값 검증이 누락되어 있습니다.
findById대신 사용자 ID에 특화된 finder 메소드를 사용하는 것이 더 명확합니다.- 삭제 후 저장 작업이 하나의 트랜잭션으로 처리되어야 합니다.
다음과 같은 개선을 제안드립니다:
+ @Transactional public void save(String userId, String token) { + if (userId == null || token == null) { + throw new IllegalArgumentException("사용자 ID와 토큰은 필수값입니다"); + } - refreshTokenRepository.findById(userId) + refreshTokenRepository.findByUserId(userId) .ifPresent(refreshTokenRepository::delete); refreshTokenRepository.save(RefreshToken.of(userId, token)); }Likely invalid or redundant comment.
| package sorisoop.soridam.domain.refresh.application; | ||
|
|
||
| import org.springframework.stereotype.Service; | ||
|
|
||
| import lombok.RequiredArgsConstructor; | ||
| import sorisoop.soridam.domain.refresh.RefreshToken; | ||
| import sorisoop.soridam.domain.refresh.RefreshTokenRepository; | ||
| import sorisoop.soridam.domain.refresh.exception.RefreshTokenNotFoundException; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor | ||
| public class RefreshTokenService { | ||
| private final RefreshTokenRepository refreshTokenRepository; | ||
|
|
||
| public RefreshToken getToken(String token) { | ||
| return refreshTokenRepository.findByRefreshToken(token) | ||
| .orElseThrow(RefreshTokenNotFoundException::new); | ||
| } | ||
|
|
||
| public void save(String userId, String token) { | ||
| refreshTokenRepository.findById(userId) | ||
| .ifPresent(refreshTokenRepository::delete); | ||
| refreshTokenRepository.save(RefreshToken.of(userId, token)); | ||
| }} |
There was a problem hiding this comment.
💡 Codebase verification
Redis 보안 설정 미흡
다음과 같은 보안 관련 설정이 누락되어 있습니다:
- RefreshToken의 만료 시간(TTL) 설정이 없습니다. 영구 토큰은 보안상 위험할 수 있습니다.
- Redis 연결에 대한 기본적인 보안 설정만 되어있습니다.
권장 사항:
@TimeToLive어노테이션이나 Redis 설정을 통해 RefreshToken의 만료 시간을 설정하세요.- Redis 연결의 SSL/TLS 설정을 고려하세요.
- Redis 접속 재시도 정책과 타임아웃 설정을 추가하세요.
🔗 Analysis chain
Redis 설정 검증 필요
리프레시 토큰의 보안을 위해 다음 사항들의 검증이 필요합니다:
- Redis TTL(Time-To-Live) 설정이 적절히 되어 있는지 확인
- Redis 연결의 보안 설정 검토
다음 스크립트로 Redis 관련 설정을 확인하겠습니다:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Redis 설정 파일 검색
echo "=== Redis 설정 파일 확인 ==="
rg -l 'redis|lettuce|RedisConfiguration' --type-add 'config:*.{yml,yaml,properties,xml}' --type config
# RefreshToken 엔티티의 TTL 설정 확인
echo "=== RefreshToken TTL 설정 확인 ==="
ast-grep --pattern 'class RefreshToken {
$$$
@TimeToLive
$$$
}'
Length of output: 349
Script:
#!/bin/bash
# Redis 설정 내용 확인
echo "=== application.yml의 Redis 설정 확인 ==="
cat soridam-api/src/main/resources/application.yml
# TTL 관련 다른 설정 검색
echo "=== TTL 관련 설정 검색 ==="
rg -i "ttl|expir|timeout" --type-add 'config:*.{yml,yaml,properties,xml}' --type config -A 2
Length of output: 1500
minjo-on
deleted the
feature/#64-RTR-based-refresh-token-issuance-and-management
branch
Summary
Tasks
To Reviewer
Summary by CodeRabbit
릴리즈 노트
새로운 기능
개선 사항
기술적 변경