-
Notifications
You must be signed in to change notification settings - Fork 0
[feat] 리프레쉬 토큰으로 액세스 토큰 자동 발급 로직 추가 #20
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
Conversation
WalkthroughGitHub Actions 배포 워크플로우의 브랜치 트리거를 변경했다. DiscordUserService에 Google OAuth 액세스 토큰 갱신 메서드를 추가했다. GcpService의 getProjectIds에서 만료 검사와 자동 갱신, DB 반영 로직을 도입했다. DiscordUserRepository는 공백 정리만 수행했다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Caller
participant GcpService
participant Repo as DiscordUserRepository
participant DUSvc as DiscordUserService
participant Google as Google OAuth
participant DB as Database
participant GCP as GCP API
Client->>GcpService: getProjectIds(userId, guildId)
GcpService->>Repo: findAccessTokenExpByUserIdAndGuildId(userId, guildId)
Repo-->>GcpService: accessToken, expiresAt, refreshToken
alt 토큰 만료됨
GcpService->>DUSvc: refreshAccessToken(refreshToken)
DUSvc->>Google: POST /token (refresh_token, client_id, client_secret)
Google-->>DUSvc: access_token, expires_in
DUSvc-->>GcpService: refreshed token data
GcpService->>Repo: update access_token, expires_at
Repo->>DB: persist changes
end
GcpService->>GCP: 호출 with valid access_token
GCP-->>GcpService: project IDs
GcpService-->>Client: project IDs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
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 (
|
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
🔭 Outside diff range comments (1)
src/main/java/com/gcp/domain/gcp/service/GcpService.java (1)
216-248: 다른 메서드에도 토큰 갱신 로직 적용 필요
getProjectIds메서드에만 토큰 자동 갱신 로직이 추가되었습니다. 다른 GCP API를 호출하는 메서드들(startVM,stopVM,getVmList등)에도 동일한 로직이 필요합니다.토큰 갱신 로직을 별도의 private 메서드로 추출하여 재사용하는 것이 좋습니다. 예를 들어:
private String getValidAccessToken(String userId, String guildId) { LocalDateTime tokenExp = discordUserRepository.findAccessTokenExpByUserIdAndGuildId(userId, guildId).orElseThrow(); if(tokenExp.isBefore(LocalDateTime.now().plusMinutes(5))){ DiscordUser discordUser = discordUserRepository.findByUserIdAndGuildId(userId, guildId).orElseThrow(); Map<String, Object> reissued = discordUserService.refreshAccessToken(discordUser.getGoogleRefreshToken()); discordUser.updateAccessToken((String) reissued.get("access_token")); Integer expiresIn = (Integer) reissued.get("expires_in"); if (expiresIn != null) { discordUser.updateAccessTokenExpiration(LocalDateTime.now().plusSeconds(expiresIn)); } discordUserRepository.save(discordUser); } return discordUserRepository.findAccessTokenByUserIdAndGuildId(userId, guildId).orElseThrow(); }그리고 각 메서드에서:
String accessToken = getValidAccessToken(userId, guildId);이 리팩토링을 수행하는 코드를 생성해드릴까요?
🧹 Nitpick comments (1)
src/main/java/com/gcp/domain/discord/service/DiscordUserService.java (1)
60-60: 제네릭 타입 대신 구체적인 타입 사용 권장
Map대신 구체적인 제네릭 타입을 사용하면 타입 안정성이 향상됩니다.- ResponseEntity<Map> response = restTemplate.postForEntity(url, request, Map.class); + ResponseEntity<Map<String, Object>> response = restTemplate.postForEntity(url, request, Map.class);
📜 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 (4)
.github/workflows/deploy.yml(1 hunks)src/main/java/com/gcp/domain/discord/repository/DiscordUserRepository.java(0 hunks)src/main/java/com/gcp/domain/discord/service/DiscordUserService.java(2 hunks)src/main/java/com/gcp/domain/gcp/service/GcpService.java(5 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/gcp/domain/discord/repository/DiscordUserRepository.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/com/gcp/domain/discord/service/DiscordUserService.java (3)
src/main/java/com/gcp/domain/gcp/service/GcpService.java (1)
Service(30-501)src/main/java/com/gcp/domain/discord/entity/DiscordUser.java (4)
updateRefreshToken(56-58)updateAccessToken(52-54)Entity(16-69)updateAccessTokenExpiration(60-62)src/main/java/com/gcp/domain/discord/repository/DiscordUserRepository.java (1)
Repository(12-37)
src/main/java/com/gcp/domain/gcp/service/GcpService.java (2)
src/main/java/com/gcp/domain/discord/service/DiscordUserService.java (1)
Service(23-76)src/main/java/com/gcp/domain/discord/entity/DiscordUser.java (3)
Entity(16-69)updateRefreshToken(56-58)updateAccessToken(52-54)
🔇 Additional comments (1)
.github/workflows/deploy.yml (1)
5-5: 기능 브랜치가 배포 워크플로우를 트리거하고 있습니다현재
feat/#19-access-token-reissue브랜치가 프로덕션 배포를 트리거하도록 설정되어 있습니다. 이는 의도적인 테스트 목적일 수 있지만, PR이 머지되기 전에main브랜치로 다시 변경해야 합니다.- branches: [feat/#19-access-token-reissue] + branches: [main]
| import static org.springframework.security.oauth2.core.OAuth2TokenIntrospectionClaimNames.CLIENT_ID; | ||
| import static org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames.CLIENT_SECRET; |
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.
🛠️ Refactor suggestion
잘못된 import 사용
OAuth2 관련 상수를 import하고 있지만 실제로는 문자열 리터럴을 사용하고 있습니다. 이 import들은 사용되지 않고 있으며 혼란을 야기할 수 있습니다.
-import static org.springframework.security.oauth2.core.OAuth2TokenIntrospectionClaimNames.CLIENT_ID;
-import static org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames.CLIENT_SECRET;📝 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.
| import static org.springframework.security.oauth2.core.OAuth2TokenIntrospectionClaimNames.CLIENT_ID; | |
| import static org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames.CLIENT_SECRET; |
🤖 Prompt for AI Agents
In src/main/java/com/gcp/domain/discord/service/DiscordUserService.java around
lines 20-21: the static imports for OAuth2TokenIntrospectionClaimNames.CLIENT_ID
and OAuth2ParameterNames.CLIENT_SECRET are unused and misleading because the
code uses string literals instead; remove these two unused import lines, run
your IDE or mvn/gradle organize-imports to ensure no other unused imports
remain, and verify compilation and tests pass.
|
|
||
| public Map<String, Object> refreshAccessToken(String refreshToken) { | ||
| String url = "https://oauth2.googleapis.com/token"; | ||
| log.info("{}", refreshToken); |
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.info("{}", refreshToken);
+ log.debug("Refreshing access token for user");📝 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.
| log.info("{}", refreshToken); | |
| log.debug("Refreshing access token for user"); |
🤖 Prompt for AI Agents
In src/main/java/com/gcp/domain/discord/service/DiscordUserService.java around
line 48, the code logs the raw refresh token which exposes sensitive
credentials; remove the direct log of refreshToken and either (a) delete the log
statement entirely, or (b) replace it with a non-sensitive message such as
"refresh token present" or a masked value (e.g., show only last 4 chars) and
guard it behind a debug check so tokens are never written to production logs.
| if (response.getStatusCode().is2xxSuccessful()) { | ||
| Map<String, Object> result = new HashMap<>(); | ||
| Map<String, Object> bodyMap = response.getBody(); | ||
|
|
||
| result.put("access_token", bodyMap.get("access_token")); | ||
| result.put("expires_in", bodyMap.get("expires_in")); // 보통 초 단위 (ex: 3599) | ||
| result.put("scope", bodyMap.get("scope")); | ||
| result.put("token_type", bodyMap.get("token_type")); | ||
|
|
||
| return result; | ||
| } else { | ||
| throw new RuntimeException("리프레시 토큰으로 액세스 토큰 재발급 실패"); | ||
| } |
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.
🛠️ Refactor suggestion
토큰 갱신 실패 시 상세한 오류 처리 필요
현재 2xx가 아닌 응답에 대해 단순히 RuntimeException을 던지고 있습니다. HTTP 상태 코드에 따른 구체적인 오류 처리가 필요합니다.
if (response.getStatusCode().is2xxSuccessful()) {
Map<String, Object> result = new HashMap<>();
Map<String, Object> bodyMap = response.getBody();
result.put("access_token", bodyMap.get("access_token"));
result.put("expires_in", bodyMap.get("expires_in")); // 보통 초 단위 (ex: 3599)
result.put("scope", bodyMap.get("scope"));
result.put("token_type", bodyMap.get("token_type"));
return result;
} else {
- throw new RuntimeException("리프레시 토큰으로 액세스 토큰 재발급 실패");
+ log.error("Failed to refresh access token. Status: {}", response.getStatusCode());
+ throw new RuntimeException("리프레시 토큰으로 액세스 토큰 재발급 실패. 상태 코드: " + response.getStatusCode());
}또한 RestTemplate의 기본 오류 핸들러가 4xx/5xx 응답에 대해 예외를 던지므로, try-catch 블록으로 HttpClientErrorException과 HttpServerErrorException을 처리하는 것이 좋습니다:
try {
ResponseEntity<Map<String, Object>> response = restTemplate.postForEntity(url, request, Map.class);
// ... 성공 처리
} catch (HttpClientErrorException e) {
log.error("Client error during token refresh: {}", e.getResponseBodyAsString());
throw new RuntimeException("토큰 갱신 실패: " + e.getMessage(), e);
} catch (HttpServerErrorException e) {
log.error("Server error during token refresh: {}", e.getResponseBodyAsString());
throw new RuntimeException("서버 오류로 토큰 갱신 실패", e);
}🤖 Prompt for AI Agents
In src/main/java/com/gcp/domain/discord/service/DiscordUserService.java around
lines 62 to 74, wrap the RestTemplate.postForEntity call in a try-catch and
handle non-2xx results more explicitly: catch HttpClientErrorException and
HttpServerErrorException, log the HTTP status and response body
(e.getStatusCode(), e.getResponseBodyAsString()) and rethrow a RuntimeException
with a clear message and the original exception as the cause; also handle a
general RestClientException fallback; if you still check
response.getStatusCode().is2xxSuccessful(), add a null check for
response.getBody() and for non-2xx responses log the status and body before
throwing a RuntimeException so callers get detailed error context.
| LocalDateTime tokenExp = discordUserRepository.findAccessTokenExpByUserIdAndGuildId(userId, guildId).orElseThrow(); | ||
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
토큰 갱신 로직의 트랜잭션 처리 및 동시성 문제
토큰 갱신 로직에 몇 가지 개선이 필요합니다:
- 토큰 갱신 후
discordUserRepository.save(discordUser)호출이 누락되어 변경사항이 DB에 저장되지 않습니다. - 동시에 여러 요청이 들어올 경우 중복 갱신이 발생할 수 있습니다.
expires_in이 Integer로 캐스팅되는데, null 체크가 필요합니다.
LocalDateTime tokenExp = discordUserRepository.findAccessTokenExpByUserIdAndGuildId(userId, guildId).orElseThrow();
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")));
+ Integer expiresIn = (Integer) reissued.get("expires_in");
+ if (expiresIn != null) {
+ discordUser.updateAccessTokenExpiration(LocalDateTime.now().plusSeconds(expiresIn));
+ }
+ discordUserRepository.save(discordUser);
}📝 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.
| LocalDateTime tokenExp = discordUserRepository.findAccessTokenExpByUserIdAndGuildId(userId, guildId).orElseThrow(); | |
| 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"))); | |
| } | |
| LocalDateTime tokenExp = discordUserRepository | |
| .findAccessTokenExpByUserIdAndGuildId(userId, guildId) | |
| .orElseThrow(); | |
| 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")); | |
| Integer expiresIn = (Integer) reissued.get("expires_in"); | |
| if (expiresIn != null) { | |
| discordUser.updateAccessTokenExpiration( | |
| LocalDateTime.now().plusSeconds(expiresIn) | |
| ); | |
| } | |
| discordUserRepository.save(discordUser); | |
| } |
| try { | ||
| String url = "https://cloudresourcemanager.googleapis.com/v1/projects"; | ||
| LocalDateTime tokenExp = discordUserRepository.findAccessTokenExpByUserIdAndGuildId(userId, guildId).orElseThrow(); | ||
| if(tokenExp.isBefore(LocalDateTime.now())){ |
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.
🛠️ Refactor suggestion
토큰 만료 시간에 여유 시간 추가 권장
정확히 만료 시간에 도달했을 때 갱신하면, 네트워크 지연 등으로 인해 실제 API 호출 시 토큰이 만료될 수 있습니다. 5분 정도의 여유 시간을 두는 것이 좋습니다.
- if(tokenExp.isBefore(LocalDateTime.now())){
+ if(tokenExp.isBefore(LocalDateTime.now().plusMinutes(5))){📝 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.
| if(tokenExp.isBefore(LocalDateTime.now())){ | |
| if (tokenExp.isBefore(LocalDateTime.now().plusMinutes(5))) { |
🤖 Prompt for AI Agents
In src/main/java/com/gcp/domain/gcp/service/GcpService.java around line 220, the
token expiry check currently compares tokenExp to the exact current time; change
it to include a safety buffer (e.g., 5 minutes) so you treat tokens that will
expire within that buffer as expired. Replace the condition to compare tokenExp
against LocalDateTime.now().plusMinutes(5) (or subtract the buffer from
tokenExp) so renewal happens earlier and avoids failures due to network latency.
📌 PR 개요
✅ 변경사항
🔍 체크리스트
📎 관련 이슈
Closes #19
💬 기타 참고사항
Summary by CodeRabbit