-
Notifications
You must be signed in to change notification settings - Fork 0
feat: 리프레쉬 토큰을 RDBMS에서 관리한다 #316
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
base: develop
Are you sure you want to change the base?
Conversation
- RefreshToken 엔티티에 token rotation 관련 필드 추가 - TokenStatus enum 추가 (ACTIVE, REVOKED, EXPIRED, ROTATED) - 원자적 업데이트를 위한 markAsRotatedIfActive 메서드 추가 - QueryDSL 기반 커스텀 Repository 구현 (revoke, hard delete) - 새로운 에러 코드 추가 (_TOKEN_REUSE_DETECTED, _DUPLICATED_REQUEST, _ALREADY_USED_REFRESH_TOKEN) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Race condition 방지를 위한 원자적 토큰 상태 업데이트 - 토큰 값 검증 로직 추가 (DB 토큰과 요청 토큰 비교) - Grace Period 내 중복 요청 처리 - 토큰 재사용 공격 탐지 시 family 전체 revoke Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- ClientInfoExtractor 유틸 클래스 추가 (IP, UserAgent 추출) - OAuth 로그인 및 온보딩 시 클라이언트 정보 저장 - 테스트 로그인에도 클라이언트 정보 저장 적용 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- 회원 탈퇴(withdraw) 시 해당 유저의 모든 ACTIVE 토큰을 REVOKED로 변경 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- calculateRefreshTokenExpiryAt 메서드 추가 - 토큰 만료 시간 계산 로직 중복 제거 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- 만료된 토큰 상태 업데이트 (ACTIVE → EXPIRED) 스케줄러 추가 - 오래된 비활성 토큰(REVOKED/EXPIRED/ROTATED) hard delete 스케줄러 추가 - 스케줄러 cron 및 retention-days 설정을 YAML로 외부화 - 기본값: 매일 03:00 만료 처리, 03:30 hard delete, 30일 보관 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Testcontainers MySQL 의존성 추가 (H2 제거) - Singleton Container Pattern으로 DatabaseTestConfig 구현 - 테스트 환경에서 실제 MySQL과 동일한 환경으로 테스트 가능 - @Serviceconnection과 @DynamicPropertySource로 DB 연결 자동화 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- RefreshTokenRepositoryTest: Repository 레이어 통합 테스트 - TokenServiceTest: 토큰 재발급, 검증, 토큰 탈취 감지 테스트 - TokenControllerTest: API 엔드포인트 테스트 업데이트 - UserControllerTest: HttpServletRequest 목 추가 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughRedis 기반 refresh token 저장소를 제거하고 JPA/MySQL 기반의 RefreshToken 엔티티로 전환했습니다. JTI 기반 토큰 로테이션(3초 그레이스)·재발급 흐름과 클라이언트 메타데이터(IP·User‑Agent) 수집이 도입되며, 관련 리포지토리·서비스·컨트롤러 시그니처, 스케줄러, 테스트 및 CI 설정이 함께 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as TokenController
participant Service as TokenService
participant Extractor as ClientInfoExtractor
participant Jwt as JwtUtil
participant Repo as RefreshTokenRepo
Client->>Controller: POST /api/v1/tokens/action-reissue (body + HttpServletRequest)
Controller->>Service: reissueToken(req, httpRequest)
Service->>Extractor: extractClientIp(httpRequest)
Extractor-->>Service: clientIp
Service->>Extractor: extractUserAgent(httpRequest)
Extractor-->>Service: userAgent
Service->>Jwt: parse provided refresh token -> jti
Jwt-->>Service: jti
Service->>Repo: findByJti(jti)
Repo-->>Service: refreshTokenEntity
alt token.status == ACTIVE
Service->>Repo: markAsRotatedIfActive(tokenId, lastUsedAt, lastUsedIp)
Repo-->>Service: updateCount
Service->>Jwt: generateAccessToken(...)
Jwt-->>Service: newAccessToken
Service->>Jwt: generateRefreshToken(..., newJti)
Jwt-->>Service: newRefreshToken
Service->>Repo: save(rotated/new RefreshToken entity)
Repo-->>Service: savedEntity
Service-->>Controller: return new tokens
else token.status == ROTATED
alt within grace period
Service-->>Controller: throw DUPLICATED_REQUEST
else
Service->>Repo: revokeAllByFamilyId(familyId)
Repo-->>Service: revokedCount
Service-->>Controller: throw TOKEN_REUSE_DETECTED
end
else token.status in (REVOKED, EXPIRED)
Service-->>Controller: throw INVALID_REFRESH_TOKEN
end
Controller-->>Client: HTTP response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
- PR 생성 시 작성자를 assignee로 자동 할당 - 팀원을 reviewer로 자동 할당 (작성자 제외) - 설정 파일명 변경: auto_assign.yaml → auto-assign-config.yaml Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- CI에서 openapi3 태스크 실행 시 디렉토리 없음 오류 방지 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
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/side/onetime/service/TestAuthService.java (1)
41-68: revoke+save가 원자적이지 않습니다 —@Transactional필요.
토큰 revoke 후 저장 실패 시 ACTIVE 토큰이 사라지는 불일치가 발생할 수 있습니다. 서비스 메서드에 트랜잭션을 걸어 원자성을 보장하세요.As per coding guidelines, ...🧩 제안 수정안
+import org.springframework.transaction.annotation.Transactional; ... - public OnboardUserResponse login(TestLoginRequest request) { + `@Transactional` + public OnboardUserResponse login(TestLoginRequest request) {
🤖 Fix all issues with AI agents
In @.github/workflows/commit-labeler.yaml:
- Around line 24-36: The current label logic uses substring checks like
msg.includes("ci") which false-positives on words such as "traffic"; change to
stricter commit-header regex matching on the msg variable (e.g. match the
conventional commit type at the start of the header) and replace the
msg.includes checks (the lines with msg.includes("feat"), msg.includes("fix"),
msg.includes("ci"), etc.) with regex tests that assert the type prefix (e.g.
/^feat(\(|:|\s)/i) so only actual commit types trigger labels via the labels.add
calls.
In `@src/main/java/side/onetime/domain/RefreshToken.java`:
- Around line 62-143: The entity currently stores the raw Refresh Token in the
tokenValue field and in create(...) and rotate(...), which is unsafe; change to
store and persist only a secure hash (e.g., SHA-256 or HMAC-SHA256) instead of
the JWT string: update the field name (tokenValue → tokenHash or keep tokenValue
but document it's a hash), change create(...) and rotate(...) to accept the raw
token for hashing (or accept the already-hashed value) and compute/assign the
hash before persisting, adjust the column definition to suit the hash length,
and ensure all token comparisons elsewhere use the same hash function to compare
incoming raw tokens against the stored hash.
In
`@src/main/java/side/onetime/repository/custom/RefreshTokenRepositoryImpl.java`:
- Around line 18-58: Bulk QueryDSL updates/delete bypass JPA auditing so you
must explicitly set refreshToken.updatedDate when changing statuses: in
revokeByUserIdAndBrowserId, revokeAllByUserId, revokeAllByFamilyId add a
.set(refreshToken.updatedDate, LocalDateTime.now()) in the update query; in
updateExpiredTokens use the provided now parameter and add
.set(refreshToken.updatedDate, now) alongside setting status; ensure the update
queries continue to call .execute() and import LocalDateTime if needed so
updatedDate reflects the status change.
In `@src/main/java/side/onetime/service/TokenService.java`:
- Around line 54-66: The reissue flow must explicitly validate expiration before
extracting claims and be protected by a distributed lock: call
jwtUtil.validateToken(refreshToken) immediately after obtaining
reissueTokenRequest.refreshToken() and before
jwtUtil.getClaimFromToken(refreshToken, "jti", String.class) so expired tokens
raise TokenErrorStatus._TOKEN_EXPIRED instead of a generic claim extraction
error; also annotate the TokenService reissue method with `@DistributedLock` (or
the appropriate lock annotation used in the project) to prevent race conditions
when accessing refreshTokenRepository.findByJti(jti) and updating/validating
RefreshToken records.
In `@src/main/java/side/onetime/util/JwtUtil.java`:
- Around line 148-156: The calculateRefreshTokenExpiryAt method currently
divides REFRESH_TOKEN_EXPIRATION_TIME by 1000 losing sub-second precision;
update JwtUtil.calculateRefreshTokenExpiryAt to add the expiration using
millisecond (or nanosecond) precision instead (e.g., use
Duration.ofMillis(REFRESH_TOKEN_EXPIRATION_TIME) or
issuedAt.plus(REFRESH_TOKEN_EXPIRATION_TIME, ChronoUnit.MILLIS)) so issuedAt is
incremented exactly by REFRESH_TOKEN_EXPIRATION_TIME without truncation.
🧹 Nitpick comments (9)
build.gradle (1)
98-102: Testcontainers 버전 정합성 확보 권장.
junit-jupiter(1.19.0)와mysql(1.20.1) 버전이 달라 충돌 가능성이 있습니다. BOM으로 정렬하는 편이 안전합니다.🔧 제안 수정안
+ testImplementation platform('org.testcontainers:testcontainers-bom:1.20.1') testImplementation 'org.springframework.boot:spring-boot-testcontainers' - testImplementation 'org.testcontainers:testcontainers' - testImplementation 'org.testcontainers:junit-jupiter:1.19.0' - testImplementation 'org.testcontainers:mysql:1.20.1' + testImplementation 'org.testcontainers:testcontainers' + testImplementation 'org.testcontainers:junit-jupiter' + testImplementation 'org.testcontainers:mysql'src/main/java/side/onetime/util/ClientInfoExtractor.java (1)
15-38: X-Forwarded-For 신뢰 경계 확인 필요.
헤더 직접 파싱은 스푸핑 위험이 있어, 신뢰 가능한 프록시 뒤에서만 사용되는지 확인하고 가능하면ForwardedHeaderFilter/request.getRemoteAddr()기반으로 일원화하는 편이 안전합니다.src/main/java/side/onetime/service/TestAuthService.java (1)
58-60: 시간 기준 일관성 개선 제안.
now를 한 번만 찍고 동일 값을 만료 계산에 재사용하면 미세한 시간 차이를 제거할 수 있습니다.🧩 제안 수정안
- LocalDateTime now = LocalDateTime.now(); - LocalDateTime expiryAt = jwtUtil.calculateRefreshTokenExpiryAt(LocalDateTime.now()); + LocalDateTime now = LocalDateTime.now(); + LocalDateTime expiryAt = jwtUtil.calculateRefreshTokenExpiryAt(now);src/test/java/side/onetime/configuration/DatabaseTestConfig.java (1)
28-43:@ServiceConnection과DynamicPropertySource가 중복됩니다.Spring Boot 3.1+ (현 프로젝트는 3.3.2)에서
@ServiceConnection은 자동으로 Testcontainers로부터 DataSource를 구성합니다.DynamicPropertySource와 함께 사용하면 중복 설정이 됩니다.@ServiceConnection만 사용하면 됩니다.♻️ `@ServiceConnection만` 사용하는 방식 (권장)
`@ServiceConnection` static final MySQLContainer<?> MYSQL_CONTAINER; static { MYSQL_CONTAINER = new MySQLContainer<>(MYSQL_IMAGE) .withCommand("--default-time-zone=+09:00") .withLogConsumer(new Slf4jLogConsumer(log)); MYSQL_CONTAINER.start(); } - - `@DynamicPropertySource` - static void configureProperties(DynamicPropertyRegistry registry) { - registry.add("spring.datasource.url", MYSQL_CONTAINER::getJdbcUrl); - registry.add("spring.datasource.username", MYSQL_CONTAINER::getUsername); - registry.add("spring.datasource.password", MYSQL_CONTAINER::getPassword); - }src/main/java/side/onetime/domain/RefreshToken.java (1)
30-38: 도메인 Soft Delete 패턴 미적용도메인 가이드라인은
@SQLDelete/@SQLRestriction+Status(ACTIVE/DELETED)적용을 요구합니다. RefreshToken은 하드 삭제 로직이 있어 예외라면 정책 문서화가 필요하고, 가능하면 Soft Delete + 스케줄러 purge로 정렬하는 방향을 검토해 주세요. 코딩 가이드라인에 근거함(As per coding guidelines).src/test/java/side/onetime/token/TokenServiceTest.java (2)
32-34: 테스트 유형(단위/통합)과@SpringBootTest사용 여부를 확인해주세요.현재 MockitoExtension 기반 단위 테스트인데, 가이드라인이 Spring Boot Test 사용을 요구하는지 정책 확인이 필요합니다. As per coding guidelines, ...
68-92: 리플렉션 기반 필드 조작은 테스트 취약점입니다.private 필드 조작이 깨지기 쉬워 유지보수에 불리합니다. 테스트 전용 팩토리/빌더 또는 패키지 스코프 설정자 추가를 고려해 주세요.
src/main/java/side/onetime/service/TokenService.java (1)
52-53: @distributedlock 제거가 정책적으로 의도된 것인지 확인이 필요합니다.현재 원자적 업데이트로 레이스를 방지하고 있지만, 서비스 메서드에
@DistributedLock사용을 요구하는 가이드라인과의 정합성을 확인해 주세요. As per coding guidelines, ...src/main/java/side/onetime/repository/RefreshTokenRepository.java (1)
34-46:@Modifying에clearAutomatically = true추가는 선택사항현재 코드는
markAsRotatedIfActive()호출 후 동일 트랜잭션 내에서 업데이트된 엔티티를 다시 조회하지 않으므로,clearAutomatically = true추가는 필수는 아닙니다. 다만 향후 이 메서드 호출 이후 같은 토큰을 재조회하는 로직이 추가될 가능성을 대비한 방어적 코딩으로 추가하는 것도 좋습니다.또한
RefreshToken.status필드에@Enumerated(EnumType.STRING)이 올바르게 설정되어 있으므로, JPQL에서 문자열 리터럴('ACTIVE','ROTATED')로 비교하는 것이 정상 작동합니다. ✅
src/main/java/side/onetime/repository/custom/RefreshTokenRepositoryImpl.java
Show resolved
Hide resolved
- RefreshTokenRepositoryImpl의 update/delete 메서드에 @transactional 추가 - TestAuthService.login()에 @transactional 추가 - Security Filter에서 호출 시 트랜잭션 없이 실행되는 문제 해결 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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: 1
🤖 Fix all issues with AI agents
In `@src/main/java/side/onetime/service/TestAuthService.java`:
- Around line 60-62: The code calls LocalDateTime.now() twice causing slight
inconsistencies; capture a single LocalDateTime (e.g., LocalDateTime now =
LocalDateTime.now()) and reuse that same variable when computing expiryAt via
jwtUtil.calculateRefreshTokenExpiryAt(now) so creation and expiry times are
consistent (update in TestAuthService where now and expiryAt are set).
🧹 Nitpick comments (1)
src/main/java/side/onetime/repository/custom/RefreshTokenRepositoryImpl.java (1)
21-29:LocalDateTime.now()사용에 대한 선택적 개선 제안revoke 메서드들에서
LocalDateTime.now()를 inline으로 호출하고 있습니다. 현재 구현도 동작에 문제가 없지만, 테스트 용이성과 일관성을 위해updateExpiredTokens처럼 timestamp를 파라미터로 받는 방식을 고려해 볼 수 있습니다.Also applies to: 33-39, 44-50
- /api/v1/users/logout도 만료된 액세스 토큰으로 요청 가능 - shouldNotFilter에 해당 경로 추가 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- TestAuthService에서 now 변수 재사용하도록 수정 - 미세한 시간 차이로 인한 테스트/로깅 불일치 방지 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- getClaimFromToken() 호출 전 토큰 검증 추가 - 만료된 토큰에 대해 명확한 에러 메시지 반환 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- DistributedLock 어노테이션 제거 - DistributedLockAop 제거 - CustomSpringELParser 제거 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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: 2
🤖 Fix all issues with AI agents
In `@build.gradle`:
- Around line 95-99: The Testcontainers dependencies specify mixed, hard-coded
versions (junit-jupiter:1.19.0 and mysql:1.20.1) which can cause
incompatibility; remove the explicit version qualifiers from the Testcontainers
module declarations in build.gradle (the testImplementation entries for
'org.testcontainers:testcontainers', 'org.testcontainers:junit-jupiter', and
'org.testcontainers:mysql') so Spring Boot's dependency management
(testcontainers.version) controls a single consistent version, and keep the
Spring Boot testcontainers artifact
('org.springframework.boot:spring-boot-testcontainers') as-is without adding
explicit Testcontainers versions.
In `@src/main/resources/application-local.yaml`:
- Around line 4-6: The YAML enables SQL initialization but no scripts exist:
create src/main/resources/schema.sql containing the required DDL (table
definitions) and optionally src/main/resources/data.sql for initial rows to
satisfy sql.init.mode: always; alternatively, if you prefer Hibernate to manage
schema, either remove generate-ddl and sql.init.mode or change
spring.jpa.hibernate.ddl-auto from validate to create/update (e.g., ddl-auto:
update) so startup succeeds; ensure the keys referenced (sql.init.mode,
generate-ddl, spring.jpa.hibernate.ddl-auto / ddl-auto) are consistent with the
chosen approach.
- generate-ddl 제거 (ddl-auto와 중복) - ddl-auto: validate → update - sql.init.mode: always → never Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
# Conflicts: # src/main/java/side/onetime/service/TestAuthService.java # src/main/java/side/onetime/util/JwtUtil.java
✅ PR 유형
변경)
🚀 작업 내용
RefreshToken MySQL 마이그레이션
family_id기반 토큰 계보 관리로 탈취 시 전체 무효화 가능@DistributedLock) 및 쿨다운 로직 제거토큰 재발급 API 개선
markAsRotatedIfActive) 패턴 적용클라이언트 정보 로깅
ClientInfoExtractor유틸리티 추가 (IP, User-Agent 추출)스케줄러 추가
테스트 환경 개선
CI/CD 개선
ci라벨 추가📝️ 관련 이슈
💬 기타 사항 or 추가 코멘트
참고 자료
Summary by CodeRabbit
새 기능
버그 수정
테스트
개선사항 / 기타
✏️ Tip: You can customize this high-level summary in your review settings.