Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: CI / CD

on:
push:
branches: [main]
branches: [feat/#19-access-token-reissue]

jobs:
CI:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,4 @@ Optional<String> findAccessTokenByUserIdAndGuildId(@Param("userId") String userI
@Query("SELECT u.accessTokenExpiration FROM DiscordUser u WHERE u.userId = :userId AND u.guildId = :guildId")
Optional<LocalDateTime> findAccessTokenExpByUserIdAndGuildId(@Param("userId") String userId,
@Param("guildId") String guildId);


}
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,35 @@
import com.gcp.domain.discord.entity.DiscordUser;
import com.gcp.domain.discord.repository.DiscordUserRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
import org.springframework.stereotype.Service;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.web.client.RestTemplate;

import java.util.HashMap;
import java.util.Map;

import static org.springframework.security.oauth2.core.OAuth2TokenIntrospectionClaimNames.CLIENT_ID;
import static org.springframework.security.oauth2.core.endpoint.OAuth2ParameterNames.CLIENT_SECRET;
Comment on lines +20 to +21
Copy link

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.

Suggested change
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.


@Service
@RequiredArgsConstructor
@Slf4j
public class DiscordUserService {
private final DiscordUserRepository discordUserRepository;
private final RestTemplate restTemplate;

@Value("${GOOGLE_CLIENT_ID}")
private String googleClientId;

@Value("${GOOGLE_CLIENT_SECRET}")
private String googleClientSecret;


public boolean insertDiscordUser(String userId, String userName, String guildId, String guildName){
Expand All @@ -19,4 +42,35 @@ public boolean insertDiscordUser(String userId, String userName, String guildId,
}
return false;
}

public Map<String, Object> refreshAccessToken(String refreshToken) {
String url = "https://oauth2.googleapis.com/token";
log.info("{}", refreshToken);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

민감한 정보가 로그에 노출됩니다

리프레시 토큰은 민감한 보안 정보입니다. 이를 로그에 그대로 출력하는 것은 보안 위험을 초래할 수 있습니다.

-        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.

Suggested change
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.


MultiValueMap<String, String> body = new LinkedMultiValueMap<>();
body.add("client_id", googleClientId);
body.add("client_secret", googleClientSecret);
body.add("refresh_token", refreshToken);
body.add("grant_type", "refresh_token");

HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);

HttpEntity<MultiValueMap<String, String>> request = new HttpEntity<>(body, headers);
ResponseEntity<Map> response = restTemplate.postForEntity(url, request, Map.class);

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("리프레시 토큰으로 액세스 토큰 재발급 실패");
}
Comment on lines +62 to +74
Copy link

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.

}
}
21 changes: 16 additions & 5 deletions src/main/java/com/gcp/domain/gcp/service/GcpService.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.gcp.domain.discord.entity.DiscordUser;
import com.gcp.domain.discord.repository.DiscordUserRepository;
import com.gcp.domain.discord.service.DiscordUserService;
import com.gcp.domain.gcp.dto.ProjectZoneDto;
import com.gcp.domain.gcp.repository.GcpProjectRepository;

import com.gcp.domain.gcp.util.GcpImageUtil;
import com.google.auth.oauth2.GoogleCredentials;
import com.google.cloud.compute.v1.Project;
import com.gcp.domain.oauth2.util.TokenEncryptConverter;
import lombok.RequiredArgsConstructor;
import lombok.SneakyThrows;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -18,18 +18,19 @@
import org.json.JSONObject;
import org.springframework.http.*;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestTemplate;


import java.io.IOException;
import java.time.LocalDateTime;
import java.util.*;

import static org.json.XMLTokener.entity;

@Service
@RequiredArgsConstructor
@Slf4j
@Transactional
public class GcpService {

private final RestTemplate restTemplate = new RestTemplate();
Expand All @@ -39,6 +40,8 @@ public class GcpService {
private static final String PROJECT_ID = "sincere-elixir-464606-j1";
private final GcpImageUtil gcpImageUtil;

private final DiscordUserService discordUserService;


public String startVM(String userId, String guildId, String vmName) {
try {
Expand Down Expand Up @@ -129,7 +132,7 @@ public List<String> getVmLogs(String userId, String guildId, String vmName) {
"resource.type=\"gce_instance\" AND resource.labels.instance_id=\"%s\" AND severity>=ERROR",
vmId
);

Map<String, Object> body = Map.of(
"resourceNames", List.of("projects/sincere-elixir-464606-j1"),
"pageSize", 50,
Expand Down Expand Up @@ -213,8 +216,16 @@ public List<Map<String, String>> getVmList(String userId, String guildId) {
public List<String> getProjectIds(String userId, String guildId) {
try {
String url = "https://cloudresourcemanager.googleapis.com/v1/projects";
LocalDateTime tokenExp = discordUserRepository.findAccessTokenExpByUserIdAndGuildId(userId, guildId).orElseThrow();
if(tokenExp.isBefore(LocalDateTime.now())){
Copy link

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.

Suggested change
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.

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")));
}
Comment on lines +219 to +225
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

토큰 갱신 로직의 트랜잭션 처리 및 동시성 문제

토큰 갱신 로직에 몇 가지 개선이 필요합니다:

  1. 토큰 갱신 후 discordUserRepository.save(discordUser) 호출이 누락되어 변경사항이 DB에 저장되지 않습니다.
  2. 동시에 여러 요청이 들어올 경우 중복 갱신이 발생할 수 있습니다.
  3. 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.

Suggested change
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);
}

String accessToken = discordUserRepository.findAccessTokenByUserIdAndGuildId(userId, guildId).orElseThrow();


HttpHeaders headers = new HttpHeaders();
headers.setBearerAuth(accessToken);
headers.setContentType(MediaType.APPLICATION_JSON);
Expand Down