[feat] 액세스 토큰 재발급 로직 AOP로 일괄 적용 #23
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
kamillcream
added
status: needs feedback
Walkthrough토큰 유효성 검증을 위한 커스텀 어노테이션과 AOP Aspect가 추가되었고, GcpService에 어노테이션을 적용하여 기존의 토큰 재발급 로직을 제거했습니다. 또한 GcpService에 방화벽 규칙 조회 메서드가 새로 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant GcpService as GcpService (@RequiredValidToken)
participant TokenAspect
participant Repo as DiscordUserRepository
participant UserSvc as DiscordUserService
Client->>GcpService: call method(userId, guildId, ...)
activate GcpService
TokenAspect-->>GcpService: Around advice intercept
activate TokenAspect
TokenAspect->>Repo: find token & expiry by userId, guildId
Repo-->>TokenAspect: token, expiry
alt token expired
TokenAspect->>Repo: load DiscordUser
Repo-->>TokenAspect: DiscordUser(refreshToken, ...)
TokenAspect->>UserSvc: refreshAccessToken(refreshToken)
UserSvc-->>TokenAspect: newAccessToken, expires_in
TokenAspect->>Repo: save updated token & expiry
end
TokenAspect-->>GcpService: proceed()
deactivate TokenAspect
GcpService-->>Client: result
deactivate GcpService
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 2
🧹 Nitpick comments (4)
src/main/java/com/gcp/domain/gcp/aop/RequiredValidToken.java (1)
6-10: 서브클래스에도 어노테이션 전파 필요 시 @inherited 추가를 제안합니다클래스 레벨에 부착했을 때 상속 구조에서의 일관된 적용을 원하시면 @inherited를 함께 사용하세요. 현재 AOP 포인트컷은 타입 레벨을 기준으로 동작하므로, 추후 확장을 고려해두면 안전합니다.
적용 예시:
@Target({ElementType.METHOD, ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) @Documented +@Inherited public @interface RequiredValidToken { }src/main/java/com/gcp/domain/gcp/service/GcpService.java (1)
382-426: 방화벽 규칙 응답 매핑을 강타입화하면 사용성과 안정성이 좋아집니다현재 map.put("sourceRanges", JsonNode), map.put("targetTags", JsonNode)로 JsonNode를 그대로 내보내고 있어, 후속 사용처에서 타입 캐스팅/직렬화 시 불편과 오류 가능성이 있습니다. 문자열 리스트로 변환해 주는 것을 권장합니다.
예시 수정안:
- map.put("sourceRanges", rule.path("sourceRanges")); - map.put("targetTags", rule.path("targetTags")); + List<String> sourceRanges = new ArrayList<>(); + rule.path("sourceRanges").forEach(n -> sourceRanges.add(n.asText())); + map.put("sourceRanges", sourceRanges); + + List<String> targetTags = new ArrayList<>(); + rule.path("targetTags").forEach(n -> targetTags.add(n.asText())); + map.put("targetTags", targetTags);추가 제안:
- allowed/denied 모두를 커버하거나, 필요한 프로토콜만 필터링하는 정책을 명시하세요(현재 tcp만 수집).
- 반복되는 ObjectMapper 생성은 필드 재사용으로 미세 최적화 가능합니다.
src/main/java/com/gcp/domain/gcp/aop/TokenAspect.java (2)
26-27: 메서드 레벨 @RequiredValidToken도 작동하도록 포인트컷 확장 제안현재 포인트컷은 @Within(...)만 매칭하여 타입 레벨 어노테이션만 동작합니다. 어노테이션 타겟에 METHOD가 포함되어 있으므로, 메서드 레벨 부착도 지원하려면 @annotation(...)을 함께 포함해 주세요.
- @Around("@within(com.gcp.domain.gcp.aop.RequiredValidToken) && args(userId, guildId, ..)") + @Around("(@within(com.gcp.domain.gcp.aop.RequiredValidToken) || " + + "@annotation(com.gcp.domain.gcp.aop.RequiredValidToken)) && args(userId, guildId, ..)") public Object validateAndRefreshToken(ProceedingJoinPoint joinPoint, String userId, String guildId) throws Throwable {확인 요청:
- 메서드 단위로 일부만 보호하고 싶은 사용 시나리오가 있는지요? 있다면 위 변경이 필요합니다.
29-31: 예외 메시지 보강으로 디버깅 가능성 향상Optional.orElseThrow()에 구체 메시지를 넣어 원인 파악을 쉽게 해주세요.
- LocalDateTime tokenExp = discordUserRepository.findAccessTokenExpByUserIdAndGuildId(userId, guildId) - .orElseThrow(); + LocalDateTime tokenExp = discordUserRepository.findAccessTokenExpByUserIdAndGuildId(userId, guildId) + .orElseThrow(() -> new IllegalStateException( + String.format("accessTokenExpiration 조회 실패: userId=%s, guildId=%s", userId, guildId)));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/main/java/com/gcp/domain/gcp/aop/RequiredValidToken.java(1 hunks)src/main/java/com/gcp/domain/gcp/aop/TokenAspect.java(1 hunks)src/main/java/com/gcp/domain/gcp/service/GcpService.java(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/gcp/domain/gcp/aop/TokenAspect.java (4)
src/main/java/com/gcp/domain/discord/entity/DiscordUser.java (4)
Entity(16-69)updateRefreshToken(56-58)updateAccessToken(52-54)updateAccessTokenExpiration(60-62)src/main/java/com/gcp/domain/discord/service/DiscordUserService.java (2)
Service(23-76)refreshAccessToken(46-75)src/main/java/com/gcp/domain/discord/repository/DiscordUserRepository.java (1)
Repository(12-35)src/main/java/com/gcp/domain/oauth2/handler/OAuth2AuthenticationSuccessHandler.java (1)
Override(48-110)
src/main/java/com/gcp/domain/gcp/service/GcpService.java (3)
src/main/java/com/gcp/domain/gcp/service/GcpProjectCommandServiceImpl.java (1)
Service(10-30)src/main/java/com/gcp/domain/gcp/service/GcpProjectCommandService.java (1)
GcpProjectCommandService(3-5)src/main/java/com/gcp/domain/discord/entity/DiscordUser.java (2)
Entity(16-69)updateRefreshToken(56-58)
🔇 Additional comments (1)
src/main/java/com/gcp/domain/gcp/service/GcpService.java (1)
8-34: GcpService 클래스 레벨에 @RequiredValidToken 적용 아주 적절합니다토큰 검증/재발급을 횡단 관심사로 이동하여 서비스 메서드의 중복을 제거한 점 좋습니다. 현재 TokenAspect 포인트컷은 args(userId, guildId, ..)에 의존하니, 본 클래스의 AOP 대상 메서드들이 계속 동일한 파라미터 순서를 유지하는지 정도만 유의해 주세요.
Comment on lines
+32
to
+42
| if (tokenExp.isBefore(LocalDateTime.now())) { | ||
| DiscordUser discordUser = discordUserRepository.findByUserIdAndGuildId(userId, guildId) | ||
| .orElseThrow(); | ||
|
|
||
| Map<String, Object> reissued = discordUserService.refreshAccessToken(discordUser.getGoogleRefreshToken()); | ||
|
|
||
| discordUser.updateAccessToken((String) reissued.get("access_token")); | ||
| discordUser.updateAccessTokenExpiration( | ||
| LocalDateTime.now().plusSeconds((Integer) reissued.get("expires_in")) | ||
| ); | ||
| } |
There was a problem hiding this comment.
토큰 업데이트 영속화 누락 및 expires_in 캐스팅 위험
- 문제 1: discordUser.updateAccessToken/Expiration 호출 후 저장(save)을 하지 않아, 트랜잭션 경계/어드바이스 순서에 따라 DB 반영이 누락될 수 있습니다. 이 경우 매 호출마다 재발급이 반복되거나, 서비스 메서드에서 여전히 만료된 토큰을 읽을 수 있습니다.
- 문제 2: (Integer) 캐스팅은 JSON 파서가 Long/Double 등으로 매핑할 때 ClassCastException을 유발할 수 있습니다. Number로 처리하세요.
- 문제 3: 만료 시각 계산 시 LocalDateTime.now()의 시스템 기본 타임존을 사용해, OAuth2AuthenticationSuccessHandler에서 Asia/Seoul로 저장한 시각과 불일치 가능성이 있습니다(서버 TZ가 다를 경우). 동일 TZ로 맞추거나 UTC로 일원화하세요.
- 문제 4: refreshToken null/공백 방어 로직이 없습니다.
아래와 같이 보완을 권장합니다.
@@
- if (tokenExp.isBefore(LocalDateTime.now())) {
+ if (tokenExp.isBefore(LocalDateTime.now())) {
DiscordUser discordUser = discordUserRepository.findByUserIdAndGuildId(userId, guildId)
.orElseThrow();
- Map<String, Object> reissued = discordUserService.refreshAccessToken(discordUser.getGoogleRefreshToken());
+ String refreshToken = discordUser.getGoogleRefreshToken();
+ if (refreshToken == null || refreshToken.isBlank()) {
+ throw new IllegalStateException(
+ String.format("리프레시 토큰이 없습니다: userId=%s, guildId=%s", userId, guildId)
+ );
+ }
+ Map<String, Object> reissued = discordUserService.refreshAccessToken(refreshToken);
- discordUser.updateAccessToken((String) reissued.get("access_token"));
- discordUser.updateAccessTokenExpiration(
- LocalDateTime.now().plusSeconds((Integer) reissued.get("expires_in"))
- );
+ discordUser.updateAccessToken((String) reissued.get("access_token"));
+ Number expiresInNum = (Number) reissued.get("expires_in");
+ long expiresInSec = (expiresInNum != null) ? expiresInNum.longValue() : 3600L;
+ discordUser.updateAccessTokenExpiration(
+ // OAuth2AuthenticationSuccessHandler와 동일하게 Asia/Seoul 기준으로 저장
+ LocalDateTime.now(java.time.ZoneId.of("Asia/Seoul")).plusSeconds(expiresInSec)
+ );
+ // 트랜잭션 경계/어드바이스 순서와 무관하게 영속화 보장
+ discordUserRepository.save(discordUser);
}추가 메모:
- 동시다발 호출 시 다중 재발급 경쟁이 발생할 수 있습니다. 사용자별 락(분산락/DB-Lock/Optimistic Lock) 도입이나 더블체크(재조회 후 여전히 만료인 경우에만 갱신)로 개선을 고려해보세요.
📝 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.
Suggested change
| if (tokenExp.isBefore(LocalDateTime.now())) { | |
| DiscordUser discordUser = discordUserRepository.findByUserIdAndGuildId(userId, guildId) | |
| .orElseThrow(); | |
| Map<String, Object> reissued = discordUserService.refreshAccessToken(discordUser.getGoogleRefreshToken()); | |
| discordUser.updateAccessToken((String) reissued.get("access_token")); | |
| discordUser.updateAccessTokenExpiration( | |
| LocalDateTime.now().plusSeconds((Integer) reissued.get("expires_in")) | |
| ); | |
| } | |
| if (tokenExp.isBefore(LocalDateTime.now())) { | |
| DiscordUser discordUser = discordUserRepository.findByUserIdAndGuildId(userId, guildId) | |
| .orElseThrow(); | |
| String refreshToken = discordUser.getGoogleRefreshToken(); | |
| if (refreshToken == null || refreshToken.isBlank()) { | |
| throw new IllegalStateException( | |
| String.format("리프레시 토큰이 없습니다: userId=%s, guildId=%s", userId, guildId) | |
| ); | |
| } | |
| Map<String, Object> reissued = discordUserService.refreshAccessToken(refreshToken); | |
| discordUser.updateAccessToken((String) reissued.get("access_token")); | |
| Number expiresInNum = (Number) reissued.get("expires_in"); | |
| long expiresInSec = (expiresInNum != null) ? expiresInNum.longValue() : 3600L; | |
| discordUser.updateAccessTokenExpiration( | |
| // OAuth2AuthenticationSuccessHandler와 동일하게 Asia/Seoul 기준으로 저장 | |
| LocalDateTime.now(java.time.ZoneId.of("Asia/Seoul")).plusSeconds(expiresInSec) | |
| ); | |
| // 트랜잭션 경계/어드바이스 순서와 무관하게 영속화 보장 | |
| discordUserRepository.save(discordUser); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/com/gcp/domain/gcp/aop/TokenAspect.java around lines 32-42, the
token refresh block needs these fixes: guard against null/blank refresh token
and bail/throw before calling refresh; when reading expires_in treat it as a
Number (Number expiresNum = (Number) reissued.get("expires_in")) and use
expiresNum.longValue() to avoid ClassCastException; compute the new expiration
using a consistent ZoneId (e.g., ZoneId.of("Asia/Seoul") or ZoneOffset.UTC)
instead of LocalDateTime.now() with system default; after updating discordUser
fields persist the change via repository.save(discordUser) (and consider
reloading the user and double-checking expiration to avoid race-condition double
refreshes or implement a user-level lock/optimistic check).
Comment on lines
+36
to
+37
| Map<String, Object> reissued = discordUserService.refreshAccessToken(discordUser.getGoogleRefreshToken()); | ||
|
|
There was a problem hiding this comment.
보안: 리프레시 토큰 로그 출력 금지 권고
해당 어드바이스는 만료 시 매번 refreshAccessToken(...)을 호출합니다. 현재 DiscordUserService.refreshAccessToken 내부에서 refreshToken을 로그(INFO)로 찍고 있어 민감정보 유출 위험이 큽니다. 즉시 제거를 권장합니다.
수정 제안 (다른 파일, 참고용):
// src/main/java/com/gcp/domain/discord/service/DiscordUserService.java
public Map<String, Object> refreshAccessToken(String refreshToken) {
String url = "https://oauth2.googleapis.com/token";
- log.info("{}", refreshToken); // 민감정보 로그 출력 금지
+ // 로그 미출력 또는 마스킹 처리 고려
...
}🤖 Prompt for AI Agents
In src/main/java/com/gcp/domain/gcp/aop/TokenAspect.java around lines 36 to 37,
the code calls discordUserService.refreshAccessToken(...) which currently logs
the raw refresh token inside DiscordUserService; remove any logging of the full
refresh token (or replace with a non-sensitive indicator/masked value) inside
DiscordUserService.refreshAccessToken and any other methods it calls, ensure
only non-sensitive info (e.g., "refresh token present" or masked substring) is
logged if needed, and run a quick grep to delete other occurrences of logging
the refresh token across the codebase.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌 PR 개요
✅ 변경사항
🔍 체크리스트
📎 관련 이슈
Closes #22
💬 기타 참고사항
Summary by CodeRabbit
신규 기능
리팩터