[Feat] 구글 로그인 구현 #27
[Feat] 구글 로그인 구현 #27
Conversation
github-actions
bot
added
the
size/L
📝 WalkthroughWalkthrough이 PR은 JWT 및 OAuth 인증 설정을 리팩토링합니다. 기존에 사용되던 Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant GUC as GoogleUserClient
participant GTP as GoogleTokenProvider
participant GUIP as GoogleUserInfoProvider
participant GA as Google OAuth 서버
U->>GUC: 인증 코드 전달
GUC->>GTP: fetchAccessToken(authCode)
GTP->>GA: POST 요청 (토큰 발급)
GA-->>GTP: GoogleTokenResponse 반환
GTP-->>GUC: GoogleTokenResponse 전달
GUC->>GUIP: fetchUserInfo(accessToken)
GUIP->>GA: GET 요청 (사용자 정보)
GA-->>GUIP: GoogleUserInfoResponse 반환
GUIP-->>GUC: GoogleUserInfoResponse 전달
GUC-->>U: 사용자 객체(User) 반환
Possibly related PRs
✨ Finishing Touches
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/main/java/com/evenly/took/feature/auth/client/google/GoogleAuthCodeRequestUrlProvider.java (1)
12-34: Google 인증 코드 요청 URL 생성 구현이 잘 되었습니다GoogleProperties를 주입받아 Google OAuth 인증 URL을 올바르게 구성하고 있습니다. UriComponentsBuilder를 사용한 URL 생성 방식도 적절합니다.
각 OAuth 매개변수의 의미를 설명하는 주석을 추가하면 코드 유지보수성이 향상될 것입니다:
@Override public String provide() { return UriComponentsBuilder.fromUriString(googleProperties.authorizationUri()) + // Google 애플리케이션의 클라이언트 ID .queryParam("client_id", googleProperties.clientId()) + // 인증 후 리디렉션될 URI .queryParam("redirect_uri", googleProperties.redirectUri()) + // OAuth 2.0 흐름에서 사용할 응답 타입 .queryParam("response_type", "code") + // 요청할 권한 범위 (예: profile, email) .queryParam("scope", googleProperties.scope()) + // 오프라인 접근을 위한 설정 (refresh token 발급 관련) .queryParam("access_type", "offline") .build(true) .toUriString(); }src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserClient.java (2)
35-53:fetch메서드에서 Map을 광범위하게 사용하는 방식은 타입 안정성을 저해할 수 있습니다.
사용자 정보를 담을 전용 DTO를 고려하면 유지보수가 한층 수월해질 것입니다.
65-75: Google OAuth 토큰 발급 로직을 분할하여 책임을 명확히 한 점이 좋습니다.
다만 HTTP 요청 시 서버 오류(5xx 상황 등)를 어떻게 처리할지 추가 고려가 필요해 보입니다.src/test/java/com/evenly/took/feature/auth/application/JwtTokenProviderTest.java (1)
26-27: TokenProperties를 사용해 JwtTokenProvider를 생성하는 로직이 정상적으로 반영되었습니다.
추가로 만료 시간이 다른 시나리오를 테스트하면, 보다 안정적인 검증이 가능할 것입니다.src/main/resources/application-dev.yml (1)
47-51: 인증 관련 설정 분리 및 기본값 검토
새로 추가된auth섹션에서 access token 관련 설정을 관리하는 방식은 구조적으로 깔끔합니다. 다만,
access-token-secret에 기본값secret이 지정되어 있는 점은 개발 환경에서는 문제가 없으나, 운영 환경에서는 보안상 반드시 환경 변수를 통해 고유한 값이 설정되어야 합니다.refresh-token-expiration-time의 기본값이 없으므로, 해당 값이 설정되지 않을 경우 발생할 수 있는 예기치 않은 동작을 미연에 방지할 필요가 있습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/main/java/com/evenly/took/feature/auth/application/JwtTokenProvider.java(3 hunks)src/main/java/com/evenly/took/feature/auth/application/UuidTokenProvider.java(2 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleAuthCodeRequestUrlProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserClient.java(1 hunks)src/main/java/com/evenly/took/feature/auth/exception/AuthErrorCode.java(1 hunks)src/main/java/com/evenly/took/global/config/properties/PropertiesConfig.java(1 hunks)src/main/java/com/evenly/took/global/config/properties/auth/GoogleProperties.java(1 hunks)src/main/java/com/evenly/took/global/config/properties/auth/TokenProperties.java(1 hunks)src/main/java/com/evenly/took/global/config/security/WebSecurityConfig.java(1 hunks)src/main/resources/application-dev.yml(1 hunks)src/main/resources/application-local.yml(1 hunks)src/main/resources/application-prod.yml(1 hunks)src/test/java/com/evenly/took/feature/auth/application/JwtTokenProviderTest.java(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/evenly/took/global/config/properties/auth/TokenProperties.java
🔇 Additional comments (15)
src/main/java/com/evenly/took/global/config/properties/auth/GoogleProperties.java (1)
5-14: Google OAuth 구성이 잘 구현되었습니다Java record를 사용하여 불변 속성을 정의한 접근 방식이 좋습니다. 모든 필요한 Google OAuth 속성이 포함되어 있습니다.
src/main/resources/application-local.yml (1)
62-70: 환경 변수를 활용한 OAuth 구성이 적절합니다Google OAuth를 위한 구성이 모두 환경 변수를 참조하여 구현되었습니다. 클라이언트 ID와 시크릿 같은 민감한 정보를 하드코딩하지 않고 환경 변수로 관리하는 접근 방식이 보안 측면에서 우수합니다.
src/main/java/com/evenly/took/feature/auth/application/JwtTokenProvider.java (1)
11-11: 토큰 관련 설정 변경이 일관되게 적용되었습니다.
AuthProperties에서TokenProperties로의 변경이 잘 이루어졌습니다. 이 변경은 인증 구성 관리의 일관성을 향상시키는 데 도움이 됩니다. JWT 토큰 생성 및 검증 로직은 그대로 유지되면서 설정 소스만 변경되었네요.Also applies to: 27-27, 32-32, 79-79
src/main/java/com/evenly/took/feature/auth/application/UuidTokenProvider.java (1)
10-10: TokenProperties로의 변경이 잘 적용되었습니다.
AuthProperties에서TokenProperties로의 변경이 리프레시 토큰 제공자에도 일관되게 적용되었습니다. 이는JwtTokenProvider의 변경 사항과 잘 맞추어져 있어 좋습니다.Also applies to: 19-19, 24-24
src/main/resources/application-prod.yml (2)
47-50: 토큰 관련 설정이 적절히 리팩토링되었습니다.기존
jwt섹션을auth섹션으로 대체하고 필요한 설정값들을 추가했습니다. 기본값 설정과 환경 변수 참조가 적절히 구성되어 있습니다.
52-60: Google OAuth 설정이 잘 구성되었습니다.Google 소셜 로그인을 위한 필요한 설정들이 적절히 추가되었습니다. 모든 중요 설정값들이 환경 변수를 통해 주입되도록 구성되어 있어 보안적으로도 좋은 접근입니다.
src/main/java/com/evenly/took/global/config/properties/PropertiesConfig.java (1)
6-6: 설정 클래스 참조가 일관되게 변경되었습니다.
AuthProperties에서TokenProperties로의 변경이 설정 클래스에도 잘 반영되었습니다. 이는 다른 클래스들의 변경 사항과 일치하며, 애플리케이션 전체에서 설정 일관성을 유지하는 데 중요합니다.Also applies to: 9-9
src/main/java/com/evenly/took/feature/auth/exception/AuthErrorCode.java (1)
18-22: Google OAuth 에러 코드를 명확히 분리하여 가독성이 향상되었습니다.
구글 연동 중 발생할 가능성이 있는 다양한 예외 상황에 대해 세분화된 처리가 가능해졌습니다.src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserClient.java (5)
1-33: GoogleUserClient 클래스를 통해 OAuthType.GOOGLE 지원이 명확하게 드러납니다.
GoogleProperties와 RestClient를 의존성 주입으로 받아 유지보수성과 확장성을 높인 점이 인상적입니다.
55-63: 토큰 요청 파라미터를 따로 메서드로 분리하여 코드 가독성이 좋습니다.
가독성과 재사용성을 모두 확보한 점이 돋보입니다.
77-86: 사용자 정보를 가져오는 로직도 일관된 구조로 구현되었습니다.
현재 HttpClientErrorException 하나만으로 클라이언트·서버 오류를 통합 처리하고 있으므로, 오류 유형을 더 세분화할 필요가 있을지 검토해 보세요.
88-92: 응답이 null이거나 특정 key가 누락된 상황을 잘 검증하여 예외 처리를 강화했습니다.
에러 상황을 명확히 식별할 수 있어 디버깅 시에도 도움이 될 것입니다.
94-105: 사용자 정보를 User 객체로 변환하는 부분이 단순·명료하여 유지보수에 용이합니다.
확장 시에도 구조를 쉽게 확장할 수 있어 보이므로 적절한 설계로 판단됩니다.src/test/java/com/evenly/took/feature/auth/application/JwtTokenProviderTest.java (1)
11-11: AuthProperties 대신 TokenProperties로 전환하여 코드 일관성이 개선되었습니다.
새로운 설정 클래스 사용으로 인해 인증 관련 구조가 보다 명확해졌습니다.src/main/resources/application-dev.yml (1)
52-61: 구글 OAuth 설정 검토
새롭게 추가된oauth섹션의
다만, 운영 환경에서 올바른 값들이 전달되는지 다시 한번 확인하고, 필요한 경우 개발 환경용 기본 설정 값을 제공하는 것도 고려해 볼 수 있습니다.
src/main/java/com/evenly/took/global/config/security/WebSecurityConfig.java
Show resolved
Hide resolved
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
src/main/java/com/evenly/took/feature/auth/client/google/dto/GoogleUserInfoResponse.java (1)
5-12: 추가 Google 사용자 정보 필드 고려 필요이 클래스는 Google에서 반환하는 사용자 정보 중 기본적인 식별자(
sub)와 이름(name)만 포함하고 있습니다. Google OAuth는 일반적으로 더 많은 사용자 정보(예: email, picture, locale, verified_email 등)를 제공합니다. 향후 확장성을 고려하여 이러한 필드들을 추가하는 것이 좋을 수 있습니다.추가 필드를 포함한 예시:
public record GoogleUserInfoResponse( @JsonProperty("sub") String sub, @JsonProperty("name") String name + + @JsonProperty("email") + String email, + + @JsonProperty("picture") + String picture, + + @JsonProperty("locale") + String locale, + + @JsonProperty("verified_email") + Boolean verifiedEmail ) { }src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderErrorHandler.java (1)
20-37: 에러 처리 구현이 적절히 되어 있으나 세부 에러 정보 추가 검토현재 구현은 기본적인 HTTP 상태 코드에 따른 에러 처리가 잘 되어 있습니다. 하지만 Google OAuth에서 발생할 수 있는 좀 더 구체적인 에러 케이스(예: invalid_grant, expired_token 등)에 대한 처리가 추가되면 더 명확한 에러 메시지를 제공할 수 있을 것입니다.
추가로, 디버깅을 용이하게 하기 위해 Google에서 제공하는 실제 에러 메시지를 예외에 포함시키는 것을 고려해보세요.
if (response.getStatusCode().is4xxClientError()) { - throw new TookException(AuthErrorCode.INVALID_GOOGLE_TOKEN_REQUEST); + throw new TookException(AuthErrorCode.INVALID_GOOGLE_TOKEN_REQUEST, "Google 토큰 요청 오류: " + responseBody); } log.error("Google 승인토큰 오류: {}", responseBody); -throw new TookException(AuthErrorCode.INVALID_GOOGLE_CONNECTION); +throw new TookException(AuthErrorCode.INVALID_GOOGLE_CONNECTION, "Google 연결 오류: " + responseBody);src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderErrorHandler.java (1)
31-33: 에러 응답 본문 활용 개선 필요현재 코드는 응답 본문을 읽어오지만, 클라이언트 측 오류(4xx)에 대해서는 이 정보를 활용하지 않고 있습니다. Google API는 문제의 정확한 원인을 응답 본문에 JSON 형태로 제공하므로, 이를 파싱하여 더 구체적인 에러 처리를 할 수 있습니다.
if (response.getStatusCode().is4xxClientError()) { + log.error("Google 사용자 정보 클라이언트 오류: {}", responseBody); throw new TookException(AuthErrorCode.INVALID_GOOGLE_USER_REQUEST); }src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserClient.java (2)
29-39: 예외 처리를 더 구체적으로 개선해 주세요.현재 모든 HTTP 클라이언트 오류에 대해 동일한
INVALID_GOOGLE_CONNECTION오류 코드를 사용하고 있습니다. Google OAuth 연결 실패의 다양한 상황(잘못된 인증 코드, 서버 오류 등)에 따라 더 구체적인 오류 코드를 제공하면 디버깅과 사용자 경험이 향상될 것입니다.try { GoogleTokenResponse tokenResponse = googleTokenProvider.fetchAccessToken(authCode); GoogleUserInfoResponse userInfoResponse = googleUserInfoProvider.fetchUserInfo(tokenResponse.accessToken()); return generateUser(userInfoResponse.sub(), userInfoResponse.name()); } catch (HttpClientErrorException ex) { - throw new TookException(AuthErrorCode.INVALID_GOOGLE_CONNECTION); + if (ex.getStatusCode().is4xxClientError()) { + throw new TookException(AuthErrorCode.INVALID_GOOGLE_AUTH_CODE); + } else if (ex.getStatusCode().is5xxServerError()) { + throw new TookException(AuthErrorCode.GOOGLE_SERVER_ERROR); + } else { + throw new TookException(AuthErrorCode.INVALID_GOOGLE_CONNECTION); + } }
24-39: 로깅 추가 권장외부 서비스와의 통신에 대한 로깅을 추가하면 디버깅과 모니터링에 도움이 됩니다. 특히 OAuth 인증 과정은 문제가 발생했을 때 추적하기 어려울 수 있으므로, 적절한 로깅이 중요합니다.
+ import org.slf4j.Logger; + import org.slf4j.LoggerFactory; @Component @RequiredArgsConstructor public class GoogleUserClient implements UserClient { + private final Logger log = LoggerFactory.getLogger(GoogleUserClient.class); private final GoogleTokenProvider googleTokenProvider; private final GoogleUserInfoProvider googleUserInfoProvider; @Override public User fetch(String authCode) { try { + log.info("Google 사용자 정보 조회 시작. Auth Code: {}", authCode.substring(0, 5) + "..."); GoogleTokenResponse tokenResponse = googleTokenProvider.fetchAccessToken(authCode); + log.debug("Google 액세스 토큰 획득 성공"); GoogleUserInfoResponse userInfoResponse = googleUserInfoProvider.fetchUserInfo(tokenResponse.accessToken()); + log.info("Google 사용자 정보 조회 성공. User ID: {}", userInfoResponse.sub()); return generateUser(userInfoResponse.sub(), userInfoResponse.name()); } catch (HttpClientErrorException ex) { + log.error("Google 연결 오류 발생: {}", ex.getMessage()); throw new TookException(AuthErrorCode.INVALID_GOOGLE_CONNECTION); } }src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProvider.java (1)
31-37: 로깅 추가 권장외부 API 호출에 대한 로깅을 추가하면 문제 해결 및 모니터링에 도움이 됩니다. 특히 Google API와 같은 외부 서비스 호출은 로깅이 중요합니다.
+ import org.slf4j.Logger; + import org.slf4j.LoggerFactory; @Component public class GoogleUserInfoProvider { + private final Logger log = LoggerFactory.getLogger(GoogleUserInfoProvider.class); private final RestClient restClient; private final GoogleProperties googleProperties; public GoogleUserInfoResponse fetchUserInfo(String accessToken) { + log.debug("Google 사용자 정보 요청 시작. Access Token: {}", accessToken.substring(0, 5) + "..."); + try { return restClient.get() .uri(googleProperties.userInfoUrl()) .header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken) .retrieve() .body(GoogleUserInfoResponse.class); + } catch (Exception e) { + log.error("Google 사용자 정보 요청 실패: {}", e.getMessage()); + throw e; + } }src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java (1)
61-61: 테스트 코드에서 lenient() 사용에 대한 검토
lenient().when()사용은 해당 모의 객체가 테스트에서 필요하지 않을 수 있음을 나타냅니다. 이는 테스트나 구현 설계에 문제가 있을 수 있습니다. 실제 필요한 모의 객체만 설정하고 불필요한 설정은 제거하는 것이 좋습니다.src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java (1)
19-92: 테스트 코드의 정확성 검증 보완테스트 코드는 성공 및 실패 케이스를 다루고 있어 좋습니다. 그러나 모의 객체 설정을 더 정확하게 하고, 더 많은 케이스(예: 다양한 HTTP 오류 코드에 대한 테스트)를 추가하면 좋을 것 같습니다. 또한 모의 객체 인터랙션 검증(verify 사용)을 추가하여 예상대로 호출되었는지 확인하는 것이 좋습니다.
성공 케이스에서 다음과 같이 검증 로직을 추가할 수 있습니다:
// when GoogleUserInfoResponse actualResponse = googleUserInfoProvider.fetchUserInfo(accessToken); // then assertThat(actualResponse).isNotNull(); assertThat(actualResponse.sub()).isEqualTo("dummySub"); assertThat(actualResponse.name()).isEqualTo("홍길동"); + verify(googleProperties).userInfoUrl(); + verify(restClient.get().uri(userInfoUrl)).header("Authorization", "Bearer " + accessToken); + verify(restClient.get().uri(userInfoUrl).header(anyString(), anyString()).retrieve()).body(GoogleUserInfoResponse.class);src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserClientTest.java (1)
1-71: 테스트 구조와 코드 품질이 우수합니다!이 테스트 클래스는 구조적으로 잘 설계되어 있으며, 성공 및 실패 케이스를 명확하게 분리하여 테스트하고 있습니다. 모킹과 검증이 적절하게 사용되었습니다.
다만, 아래와 같은 추가 개선사항을 고려해보세요:
- 경계 케이스(예: 빈 문자열 또는 null 인증 코드)에 대한 테스트 추가
- 유저 정보 응답이 불완전한 경우(예: 이름이 null인 경우)에 대한 테스트 추가
src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java (2)
33-40: RestClient 설정 중복 제거 필요fetchAccessToken 메서드에서 CONTENT_TYPE 헤더를 다시 설정하고 있는데, 이미 RestClient 빌더에서 defaultHeader로 설정했습니다(28-30줄). 헤더 중복 설정은 제거하는 것이 좋겠습니다.
public GoogleTokenResponse fetchAccessToken(String authCode) { return restClient.post() .uri(googleProperties.tokenUri()) - .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE) .body(getTokenRequestParams(authCode)) .retrieve() .body(GoogleTokenResponse.class); }
42-50: 입력 유효성 검사 및 로깅 추가 권장authCode의 null 또는 빈 문자열 검사가 없습니다. 또한 디버깅을 위한 로깅이 추가되면 좋겠습니다.
private MultiValueMap<String, String> getTokenRequestParams(String authCode) { + if (authCode == null || authCode.isBlank()) { + throw new IllegalArgumentException("인증 코드는 비어있을 수 없습니다."); + } MultiValueMap<String, String> tokenRequestParams = new LinkedMultiValueMap<>(); tokenRequestParams.add("code", authCode); tokenRequestParams.add("client_id", googleProperties.clientId()); tokenRequestParams.add("client_secret", googleProperties.clientSecret()); tokenRequestParams.add("redirect_uri", googleProperties.redirectUri()); tokenRequestParams.add("grant_type", "authorization_code"); return tokenRequestParams; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderErrorHandler.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserClient.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderErrorHandler.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/dto/GoogleTokenResponse.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/dto/GoogleUserInfoResponse.java(1 hunks)src/main/java/com/evenly/took/feature/auth/exception/AuthErrorCode.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserClientTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java(1 hunks)
🔇 Additional comments (2)
src/main/java/com/evenly/took/feature/auth/client/google/dto/GoogleTokenResponse.java (1)
5-21: Google OAuth 토큰 응답 구조가 잘 구현되었습니다.Google OAuth 토큰 응답에 필요한 모든 필드가 올바르게 포함되어 있으며,
@JsonProperty어노테이션을 사용하여 JSON 필드와 Java 필드를 적절히 매핑하고 있습니다. 필드명도 명확하고 표준 네이밍 컨벤션을 따르고 있습니다.src/main/java/com/evenly/took/feature/auth/exception/AuthErrorCode.java (1)
18-24: 구글 인증 관련 오류 코드 추가가 적절합니다구글 OAuth 인증 과정에서 발생할 수 있는 다양한 예외 상황에 대한 오류 코드가 적절하게 추가되었습니다. 각 오류 코드는 명확한 메시지와 적합한 HTTP 상태 코드를 가지고 있어 좋습니다.
...main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderErrorHandler.java
Outdated
Show resolved
Hide resolved
src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProvider.java
Show resolved
Hide resolved
src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProvider.java
Outdated
Show resolved
Hide resolved
src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java
Outdated
Show resolved
Hide resolved
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java (2)
42-43: 인증 헤더 설정 방식이 하드코딩되어 있습니다.인증 헤더 설정 시 하드코딩된 문자열 대신 Spring의 상수를 사용하는 것이 좋습니다.
-when(requestSpec.header("Authorization", "Bearer " + accessToken)).thenReturn(requestSpec); +when(requestSpec.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)).thenReturn(requestSpec);이를 위해 다음 import를 추가하세요:
import org.springframework.http.HttpHeaders;
67-68: 실패 케이스에서도 인증 헤더 설정 방식이 하드코딩되어 있습니다.성공 케이스와 동일한 방식으로 Spring의 상수를 사용하여 개선할 수 있습니다.
-when(requestSpec.header("Authorization", "Bearer " + accessToken)).thenReturn(requestSpec); +when(requestSpec.header(HttpHeaders.AUTHORIZATION, "Bearer " + accessToken)).thenReturn(requestSpec);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderErrorHandler.java(1 hunks)src/main/java/com/evenly/took/feature/auth/exception/AuthErrorCode.java(1 hunks)src/main/java/com/evenly/took/feature/common/exception/TookException.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/MockGoogleProviderTest.java(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI test run workflow
src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java
[error] 75-75: Test case '사용자정보_조회시_예외발생()' failed due to an AssertionError.
🔇 Additional comments (14)
src/test/java/com/evenly/took/feature/auth/client/google/MockGoogleProviderTest.java (1)
14-32: 추상 테스트 클래스 구현이 잘 되었습니다.GoogleProvider 관련 테스트 클래스들을 위한 공통 Mock 설정을 잘 구현하셨습니다. RestClient와 관련 빌더의 설정이 명확하게 되어 있어 하위 테스트 클래스에서 중복 코드를 제거할 수 있습니다.
src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderErrorHandler.java (4)
18-25: 에러 핸들러 구현이 적절합니다.ResponseErrorHandler 인터페이스를 구현한 에러 핸들러가 명확하게 작성되었습니다. HTTP 응답 상태 코드의 오류 여부를 확인하는 로직이 잘 구현되었습니다.
31-33: 클라이언트 오류 처리가 적절합니다.4xx 클라이언트 오류에 대해 INVALID_GOOGLE_USER_REQUEST 에러 코드를 사용하는 것이 적절합니다.
35-37: 서버 오류 처리가 개선되었습니다.이전 리뷰에서 제안된 대로 5xx 서버 오류에 대해 INVALID_GOOGLE_SERVER_ERROR 에러 코드를 사용하는 것으로 수정되었습니다. 이는 오류의 성격을 더 정확히 반영합니다.
39-41: 미분류 오류 처리가 적절합니다.알 수 없는 오류에 대해 로그를 남기고 INVALID_GOOGLE_CONNECTION 에러 코드를 사용하는 것이 적절합니다. 로그 메시지가 명확하게 작성되어 있습니다.
src/main/java/com/evenly/took/feature/common/exception/TookException.java (1)
10-13: 생성자 구현이 명확합니다.@AllArgsConstructor 어노테이션을 제거하고 명시적으로 생성자를 구현한 것이 좋습니다. 이렇게 하면 코드의 의도가 더 명확해집니다. 생성자에서 errorCode.getMessage()를 super()에 전달하여 예외 메시지로 사용하는 것도 적절합니다.
src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java (3)
25-28: 테스트 설정이 적절합니다.@beforeeach 메서드에서 GoogleTokenProvider를 초기화하는 것이 적절합니다. 모든 테스트에서 공통적으로 사용하는 객체를 준비하는 방식이 명확합니다.
30-57: 성공 케이스 테스트가 잘 구현되었습니다.성공 케이스를 위한 중첩 클래스와 테스트 메서드가 명확하게 구현되었습니다. given-when-then 구조를 따라 테스트 가독성이 좋습니다. 예상 응답과 실제 응답을 비교하는 assertion이 적절합니다.
59-81: 실패 케이스 테스트가 잘 구현되었습니다.실패 케이스를 위한 중첩 클래스와 테스트 메서드가 명확하게 구현되었습니다. 예외 발생을 테스트하는 방식(assertThatThrownBy)이 적절하며, 예외의 타입 및 속성 검증이 잘 이루어지고 있습니다.
src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java (4)
17-22: 테스트 클래스 구조가 잘 설계되었습니다.MockGoogleProviderTest를 상속받고 필요한 의존성을 모킹하는 방식이 적절합니다. 에러 핸들러를 별도로 모킹한 것도 좋은 접근입니다.
24-27: 테스트 셋업 메서드가 간결하게 작성되었습니다.
setUpUserInfoProvider메서드가 명확하고 책임이 분명합니다. 테스트 준비 코드가 간결하게 작성되었습니다.
29-54: 성공 케이스 테스트가 잘 구현되었습니다.성공 케이스에 대한 테스트 메서드가 명확하게 구현되었습니다. given-when-then 패턴을 따라 테스트 코드의 가독성이 좋습니다. 응답 값에 대한 검증도 적절합니다.
73-75: 예외 테스트 검증 부분은 잘 작성되었습니다.예외의 타입과 속성을 함께 검증하는 방식이 적절합니다. 다만 앞서 지적한 예외 코드 불일치 문제를 해결해야 합니다.
🧰 Tools
🪛 GitHub Actions: CI test run workflow
[error] 75-75: Test case '사용자정보_조회시_예외발생()' failed due to an AssertionError.
src/main/java/com/evenly/took/feature/auth/exception/AuthErrorCode.java (1)
18-24: 구글 인증 관련 오류 코드가 적절하게 추가되었습니다.구글 OAuth 인증 프로세스의 다양한 단계에서 발생할 수 있는 오류에 대한 코드가 명확하게 정의되어 있습니다. 각 오류 코드는 적절한 HTTP 상태 코드와 명확한 오류 메시지를 포함하고 있습니다.
INVALID_GOOGLE_TOKEN_REQUEST: 승인 코드 요청 과정의 오류INVALID_GOOGLE_USER_REQUEST: 사용자 정보 요청 과정의 클라이언트 측 오류INVALID_GOOGLE_SERVER_ERROR: 만료된 토큰/승인 코드로 인한 오류INVALID_GOOGLE_CONNECTION: 구글 서버와의 연결 오류오류 코드의 계층 구조와 분류가 논리적으로 잘 설계되어 있습니다.
| when(requestBodySpec.header(anyString(), (String[])any())).thenReturn(requestBodySpec); | ||
| lenient().when(requestBodySpec.body(any(MultiValueMap.class))).thenReturn(requestBodySpec); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Mock 설정과 실제 코드의 일치 확인 필요
이전 리뷰 코멘트에서 지적된 바와 같이, 테스트의 Mock 설정이 실제 GoogleTokenProvider 구현과 일치하지 않을 수 있습니다. 현재는 anyString()과 any()를 사용하여 헤더 설정을 모킹하고 있는데, 이는 구체적인 헤더 설정 방식과 일치하는지 확인이 필요합니다.
🏁 Script executed:
#!/bin/bash
# GoogleTokenProvider 클래스에서 헤더 설정 방식 확인
rg -A 5 "header\(" --type java src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.javaLength of output: 294
테스트 모킹 설정 개선 필요
현재 src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java 파일에서는 헤더 설정 모킹 시 anyString()과 any()를 사용하고 있습니다. 하지만 실제 GoogleTokenProvider 구현에서는 아래와 같이 구체적인 값으로 헤더를 설정하고 있습니다:
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE)
.body(getTokenRequestParams(authCode))
.retrieve()
.body(GoogleTokenResponse.class);이에 따라 테스트가 실제 코드와 일치하지 않을 가능성이 있습니다. 다음과 같이 개선을 요청드립니다:
- 테스트에서
anyString()대신eq(HttpHeaders.CONTENT_TYPE)를 사용하여 헤더 키가 올바른지 검증합니다. (String[])any()대신eq(MediaType.APPLICATION_FORM_URLENCODED_VALUE)를 사용하여 헤더 값도 정확히 매칭되도록 합니다.
이와 같이 수정하시면 테스트와 실제 구현 내용이 일치함을 보장할 수 있습니다.
| when(responseSpec.body(eq(GoogleUserInfoResponse.class))) | ||
| .thenThrow(new TookException(AuthErrorCode.INVALID_GOOGLE_SERVER_ERROR)); |
There was a problem hiding this comment.
예외 테스트에서 발생하는 예외와 검증하는 예외가 일치하지 않습니다.
현재 테스트는 INVALID_GOOGLE_SERVER_ERROR를 던지도록 모킹되어 있지만, 검증 부분에서는 INVALID_GOOGLE_USER_REQUEST를 기대하고 있습니다. 이로 인해 파이프라인 테스트가 실패하고 있습니다.
다음과 같이 수정하세요:
when(responseSpec.body(eq(GoogleUserInfoResponse.class)))
- .thenThrow(new TookException(AuthErrorCode.INVALID_GOOGLE_SERVER_ERROR));
+ .thenThrow(new TookException(AuthErrorCode.INVALID_GOOGLE_USER_REQUEST));또는 실제 GoogleUserInfoProvider 구현에서 서버 에러를 사용자 요청 에러로 변환하는 로직이 있다면, 그에 맞게 테스트를 수정해야 합니다.
📝 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.
| when(responseSpec.body(eq(GoogleUserInfoResponse.class))) | |
| .thenThrow(new TookException(AuthErrorCode.INVALID_GOOGLE_SERVER_ERROR)); | |
| when(responseSpec.body(eq(GoogleUserInfoResponse.class))) | |
| .thenThrow(new TookException(AuthErrorCode.INVALID_GOOGLE_USER_REQUEST)); |
github-actions
bot
removed
the
size/XL
github-actions
bot
added
the
size/XL
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java (2)
29-54: 성공 케이스 테스트 구현이 적절합니다.테스트 케이스가 given-when-then 패턴을 명확히 따르고 있으며, 유효한 액세스 토큰을 통해 사용자 정보를 정상적으로 가져오는 시나리오를 잘 테스트하고 있습니다.
다만, 상수 문자열을 사용하는 부분은 개선할 여지가 있습니다.
인증 헤더 형식과 같은 반복되는 문자열은 상수로 추출하면 가독성과 유지보수성이 향상됩니다:
+ private static final String AUTHORIZATION_HEADER = "Authorization"; + private static final String BEARER_PREFIX = "Bearer "; @Test void 유효한_액세스토큰_주어졌을때_사용자정보_반환() { // given String accessToken = "validAccessToken"; String userInfoUrl = "http://dummy-userinfo-url"; when(googleProperties.userInfoUrl()).thenReturn(userInfoUrl); GoogleUserInfoResponse expectedResponse = new GoogleUserInfoResponse("dummySub", "홍길동"); RestClient.RequestHeadersSpec requestSpec = restClient.get().uri(userInfoUrl); - when(requestSpec.header("Authorization", "Bearer " + accessToken)).thenReturn(requestSpec); + when(requestSpec.header(AUTHORIZATION_HEADER, BEARER_PREFIX + accessToken)).thenReturn(requestSpec); RestClient.ResponseSpec responseSpec = requestSpec.retrieve(); when(responseSpec.body(eq(GoogleUserInfoResponse.class))).thenReturn(expectedResponse);
41-44: Mockito verify를 통한 메서드 호출 검증 추가 고려.현재 테스트에서는 반환값에 대한 검증만 이루어지고 있습니다. RestClient의 메소드 호출 여부와 파라미터 값도 verify를 통해 검증하면 테스트의 견고성이 향상됩니다.
다음과 같이 메소드 호출 검증을 추가하는 것을 고려해보세요:
// then assertThat(actualResponse).isNotNull(); assertThat(actualResponse.sub()).isEqualTo("dummySub"); assertThat(actualResponse.name()).isEqualTo("홍길동"); + + // API 호출 검증 + verify(restClient).get(); + verify(requestSpec).header("Authorization", "Bearer " + accessToken); + verify(responseSpec).body(eq(GoogleUserInfoResponse.class));
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java
🔇 Additional comments (3)
src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java (3)
17-27: 테스트 클래스 구조가 잘 설계되었습니다.테스트 클래스가
MockGoogleProviderTest를 상속받아 기본 설정을 재사용하고,@BeforeEach를 통해 각 테스트마다 필요한 의존성을 초기화하는 구조가 좋습니다.
56-77: 예외 처리 테스트가 적절히 구현되었습니다.이전 리뷰에서 지적된 예외 코드 불일치 문제가 해결되었습니다. 올바른 에러 코드(
INVALID_GOOGLE_USER_REQUEST)로 테스트가 수정되어 있습니다.
1-16: 구글 로그인 기능 구현에 적합한 테스트 케이스.PR 목표인 구글 로그인 구현을 위한 적절한 테스트 케이스가 작성되었습니다. 유효한 토큰과 유효하지 않은 토큰에 대한 시나리오를 모두 테스트하여 신뢰성 있는 구현을 보장합니다.
src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java
Show resolved
Hide resolved
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/test/java/com/evenly/took/feature/auth/client/google/MockGoogleProviderTest.java (1)
1-36: Google 인증 테스트를 위한 기반 클래스 구조가 잘 설계되었습니다.테스트 관련 공통 모킹 로직을 추상 클래스로 분리한 접근 방식이 좋습니다. 이를 통해 Google 인증 관련 다양한 테스트 클래스에서 중복 코드 없이 일관된 테스트 환경을 구성할 수 있습니다.
RestClient 관련 모킹에서 Content-Type 헤더 설정 부분에 "UTF-8"이 세 번째 인자로 들어가는 것이 일반적인 패턴인지 확인해 보세요. 일반적으로 인코딩은 "application/x-www-form-urlencoded; charset=UTF-8"와 같이 Content-Type 값 자체에 포함되는 경우가 많습니다.
- when(restClientBuilder.defaultHeader("Content-Type", "application/x-www-form-urlencoded", "UTF-8")) + when(restClientBuilder.defaultHeader("Content-Type", "application/x-www-form-urlencoded; charset=UTF-8"))src/main/java/com/evenly/took/feature/auth/client/google/GoogleAuthCodeRequestUrlProvider.java (1)
17-18: GoogleProperties 및 GoogleUrlProperties에 대한 파라미터 검증 고려
현재는 외부 프로퍼티 값이 잘못 들어와도 별도의 검증 없이 그대로 사용될 수 있습니다. OAuth 프로세스에서 URL이 제대로 설정되지 않은 경우 치명적인 오류로 이어질 수 있으니, 유효성 검증 로직(예: 빈 값, 형식 오류 등)을 고려해 볼 수 있습니다.src/main/resources/application-prod.yml (1)
52-61: Google OAuth 설정이 환경 변수로 잘 분리되었습니다.
redirect-uri,client-id,client-secret, 그리고scope까지 외부에서 주입 가능하도록 설정하여 보안 및 배포 유연성이 향상되었습니다. URL 역시 별도 필드로 관리함으로써, OAuth 전체 플로우에서 주소가 변경될 때 유연하게 대응할 수 있을 것으로 보입니다.src/main/resources/application-local.yml (1)
62-71: Google OAuth 설정이 추가되었습니다.OAuth 구글 로그인을 위한 필요한 설정들이 잘 구성되어 있습니다. 환경 변수를 통해 민감한 정보를 관리하는 방식은 보안 측면에서 좋은 접근입니다.
다만, 환경 변수 명명 규칙에 약간의 불일치가 있습니다.
GOOGLE_AUTHORIZATION_URL과GOOGLE_OAUTH_TOKEN_URI에서 하나는 'URL', 다른 하나는 'URI'를 사용하고 있습니다. 일관성을 위해 통일하는 것이 좋을 것 같습니다.src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java (2)
16-35: GoogleTokenProvider 클래스 구조가 잘 설계되었습니다.Spring의
RestClient를 활용하여 Google OAuth 토큰을 가져오는 기능을 구현한 점이 좋습니다. 의존성 주입을 통해 필요한 속성들과 클라이언트 빌더를 주입받고 있으며, 에러 핸들링도 고려되어 있습니다.추가적으로 로깅을 구현하는 것을 고려해 보세요. OAuth 인증 과정에서 발생할 수 있는 문제를 디버깅하는데 도움이 될 것입니다.
import org.springframework.stereotype.Component; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.web.client.RestClient; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.evenly.took.feature.auth.client.google.dto.GoogleTokenResponse; import com.evenly.took.global.config.properties.auth.GoogleProperties; import com.evenly.took.global.config.properties.auth.GoogleUrlProperties; @Component public class GoogleTokenProvider { + + private static final Logger logger = LoggerFactory.getLogger(GoogleTokenProvider.class);
37-44: fetchAccessToken 메서드가 명확하게 구현되었습니다.RestClient API를 사용하여 Google의 토큰 URI에 요청을 보내고 응답을 처리하는 로직이 잘 구현되어 있습니다.
타임아웃 설정이 없습니다. 외부 서비스 호출 시 타임아웃을 설정하면 서비스 안정성을 향상시킬 수 있습니다.
public GoogleTokenResponse fetchAccessToken(String authCode) { + logger.debug("Fetching access token for auth code"); return restClient.post() .uri(googleUrlProperties.tokenUri()) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE) .body(getTokenRequestParams(authCode)) + .timeout(java.time.Duration.ofSeconds(10)) .retrieve() .body(GoogleTokenResponse.class); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/evenly/took/feature/auth/client/google/GoogleAuthCodeRequestUrlProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProvider.java(1 hunks)src/main/java/com/evenly/took/global/config/properties/auth/GoogleProperties.java(1 hunks)src/main/java/com/evenly/took/global/config/properties/auth/GoogleUrlProperties.java(1 hunks)src/main/resources/application-dev.yml(1 hunks)src/main/resources/application-local.yml(1 hunks)src/main/resources/application-prod.yml(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/MockGoogleProviderTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/java/com/evenly/took/global/config/properties/auth/GoogleProperties.java
- src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProvider.java
- src/main/resources/application-dev.yml
- src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java
- src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java
🔇 Additional comments (4)
src/main/java/com/evenly/took/global/config/properties/auth/GoogleUrlProperties.java (1)
5-11:@ConfigurationProperties사용 방식이 적절합니다.
Google OAuth에 필요한 URL 정보를 별도의 레코드로 관리함으로써 설정 변경시 코드 수정 없이 확장 가능하도록 설계한 점이 좋습니다. 향후 유지 보수와 확장성을 고려할 때, 관련 필드에 대한 간단한 JavaDoc 주석을 남겨두면 협업 시 이해를 높일 수 있으니 참고 부탁드립니다.src/main/java/com/evenly/took/feature/auth/client/google/GoogleAuthCodeRequestUrlProvider.java (2)
13-14: 의존성 주입과 빈 등록이 잘 구성되었습니다.
@Component와@RequiredArgsConstructor를 함께 사용하여 필요한 필드를 주입받는 구조는 간결하고 명확합니다.
25-35: URL 구성 메서드가 명확하고 간결합니다.
UriComponentsBuilder를 활용해 파라미터를 설정하는 방식이 읽기 쉽고 유지 보수에 용이합니다. 다만,scope값이 한 개 이상의 스코프가 필요한 경우 문자열 조합 방식에 주의해야 합니다. 여러 스코프가 필요하다면 공백이나+등 Google에서 요구하는 형식이 올바른지 확인 부탁드립니다.src/main/resources/application-prod.yml (1)
47-50: JWT 토큰 관련 설정이 명확하게 분리되었습니다.
액세스 토큰 및 리프레시 토큰 구분이 명확해져, 인증 관련 로직에서 혼동이 줄어들 것으로 보입니다. 기본값(secret,7200)이 배포 환경에 맞는지 다시 확인 부탁드립니다.
src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java
Outdated
Show resolved
Hide resolved
github-actions
bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java (1)
37-45: 🛠️ Refactor suggestion인증 코드 검증 로직이 필요합니다.
authCode파라미터에 대한 유효성 검증이 없습니다. 이전 리뷰 코멘트에서도 지적된 내용입니다.public GoogleTokenResponse fetchAccessToken(String authCode) { + if (authCode == null || authCode.isBlank()) { + throw new IllegalArgumentException("인증 코드는 null이거나 빈 값일 수 없습니다"); + } GoogleTokenRequest request = GoogleTokenRequest.of(googleProperties, authCode); return restClient.post() .uri(googleUrlProperties.tokenUri()) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE) .body(request.toMultiValueMap()) .retrieve() .body(GoogleTokenResponse.class); }
🧹 Nitpick comments (7)
src/main/java/com/evenly/took/feature/auth/client/google/dto/response/GoogleUserInfoResponse.java (1)
5-12: 사용자 정보 필드 확장 고려현재 레코드는
sub와name필드만 포함하고 있습니다. Google OAuth 사용자 정보 응답에는 일반적으로 이메일, 프로필 이미지 URL 등 추가 정보가 포함됩니다. 애플리케이션의 요구사항에 따라 다음 필드를 추가하는 것이 좋습니다:public record GoogleUserInfoResponse( @JsonProperty("sub") String sub, @JsonProperty("name") String name, + + @JsonProperty("email") + String email, + + @JsonProperty("picture") + String picture ) { }src/main/java/com/evenly/took/feature/auth/client/google/error/GoogleUserInfoProviderErrorHandler.java (1)
27-41: 오류 처리 로직의 보완 고려현재 오류 처리 로직은 4xx 및 5xx 오류를 적절히 구분하여 처리하고 있습니다. 하지만 다음 사항을 고려해 보세요:
- 로그 메시지에 상태 코드(
response.getStatusCode())를 포함하면 디버깅에 도움이 됩니다.- 가능하다면 특정 오류 코드(예: 401, 403)에 대한 더 구체적인 오류 메시지를 제공하는 것이 좋습니다.
@Override public void handleError(URI uri, HttpMethod method, ClientHttpResponse response) throws IOException { String responseBody = StreamUtils.copyToString(response.getBody(), StandardCharsets.UTF_8); if (response.getStatusCode().is4xxClientError()) { + log.error("Google 사용자정보 클라이언트 오류: {} - {}", response.getStatusCode(), responseBody); throw new TookException(AuthErrorCode.INVALID_GOOGLE_USER_REQUEST); } if (response.getStatusCode().is5xxServerError()) { + log.error("Google 서버 오류: {} - {}", response.getStatusCode(), responseBody); throw new TookException(AuthErrorCode.INVALID_GOOGLE_SERVER_ERROR); } - log.error("Google 사용자정보 알 수 없는 오류: {}", responseBody); + log.error("Google 사용자정보 알 수 없는 오류: {} - {}", response.getStatusCode(), responseBody); throw new TookException(AuthErrorCode.INVALID_GOOGLE_CONNECTION); }src/main/java/com/evenly/took/feature/auth/client/google/dto/request/GoogleTokenRequest.java (2)
16-22: 팩토리 메서드 구현이 깔끔합니다만, 코드 유효성 검증이 필요합니다.
of메서드는 가독성이 좋고 객체 생성을 단순화합니다. 하지만code파라미터에 대한 유효성 검증이 없습니다. 코드가 null이거나 빈 문자열일 경우 예외를 발생시키는 것이 좋습니다.public static GoogleTokenRequest of(GoogleProperties googleProperties, String code) { + if (code == null || code.isBlank()) { + throw new IllegalArgumentException("인증 코드는 null이거나 빈 값일 수 없습니다"); + } return new GoogleTokenRequest("authorization_code", googleProperties.clientId(), googleProperties.redirectUri(), code, googleProperties.clientSecret()); }
8-14: 상수 값을 상수 필드로 추출하는 것이 좋습니다."authorization_code"와 같은 문자열 리터럴은 상수로 추출하여 코드 유지보수성을 높이는 것이 좋습니다.
public record GoogleTokenRequest( String grantType, String clientId, String redirectUri, String code, String clientSecret ) { + private static final String GRANT_TYPE_AUTHORIZATION_CODE = "authorization_code"; public static GoogleTokenRequest of(GoogleProperties googleProperties, String code) { return new GoogleTokenRequest(GRANT_TYPE_AUTHORIZATION_CODE, googleProperties.clientId(), googleProperties.redirectUri(), code, googleProperties.clientSecret()); }Also applies to: 16-22
src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java (3)
37-45: Content-Type 헤더가 중복 설정되고 있습니다.RestClient 빌더에서 이미 기본 헤더로
CONTENT_TYPE을 설정했는데,fetchAccessToken메서드에서 다시 설정하고 있습니다. 중복 설정은 제거하는 것이 좋습니다.public GoogleTokenResponse fetchAccessToken(String authCode) { GoogleTokenRequest request = GoogleTokenRequest.of(googleProperties, authCode); return restClient.post() .uri(googleUrlProperties.tokenUri()) - .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE) .body(request.toMultiValueMap()) .retrieve() .body(GoogleTokenResponse.class); }
37-45: 로깅 추가를 고려해 보세요.토큰 요청 시작과 완료 시점에 로그를 남기는 것이 디버깅과 모니터링에 도움이 됩니다.
+private static final org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(GoogleTokenProvider.class); public GoogleTokenResponse fetchAccessToken(String authCode) { + log.debug("Google 액세스 토큰 요청 시작: authCode={}", authCode.substring(0, Math.min(authCode.length(), 10)) + "..."); GoogleTokenRequest request = GoogleTokenRequest.of(googleProperties, authCode); - return restClient.post() + GoogleTokenResponse response = restClient.post() .uri(googleUrlProperties.tokenUri()) .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_FORM_URLENCODED_VALUE) .body(request.toMultiValueMap()) .retrieve() .body(GoogleTokenResponse.class); + log.debug("Google 액세스 토큰 요청 완료"); + return response; }
19-21: 필드에 대한 JavaDoc 주석 추가를 고려해 보세요.각 필드가 무엇을 의미하는지 JavaDoc 주석을 추가하면 코드 가독성과 유지보수성이 향상됩니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserClient.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProvider.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/dto/request/GoogleTokenRequest.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/dto/response/GoogleTokenResponse.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/dto/response/GoogleUserInfoResponse.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/error/GoogleTokenProviderErrorHandler.java(1 hunks)src/main/java/com/evenly/took/feature/auth/client/google/error/GoogleUserInfoProviderErrorHandler.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserClientTest.java(1 hunks)src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/test/java/com/evenly/took/feature/auth/client/google/GoogleTokenProviderTest.java
- src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProviderTest.java
- src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserClient.java
- src/test/java/com/evenly/took/feature/auth/client/google/GoogleUserClientTest.java
- src/main/java/com/evenly/took/feature/auth/client/google/GoogleUserInfoProvider.java
🔇 Additional comments (7)
src/main/java/com/evenly/took/feature/auth/client/google/dto/response/GoogleTokenResponse.java (1)
5-21: 토큰 응답 구조 구현이 적절합니다Google OAuth 토큰 응답에 필요한 모든 표준 필드가 포함되어 있으며, 각 필드에 대한 JSON 매핑이 올바르게 설정되어 있습니다.
src/main/java/com/evenly/took/feature/auth/client/google/error/GoogleUserInfoProviderErrorHandler.java (1)
22-25: 오류 감지 로직이 적절합니다
hasError메서드는 HTTP 응답의 상태 코드를 확인하여, Spring의ResponseErrorHandler인터페이스를 올바르게 구현하고 있습니다.src/main/java/com/evenly/took/feature/auth/client/google/error/GoogleTokenProviderErrorHandler.java (1)
22-25: 오류 감지 로직이 적절합니다
hasError메서드는 응답 상태 코드를 확인하여 오류 여부를 적절히 판단하고 있습니다.src/main/java/com/evenly/took/feature/auth/client/google/dto/request/GoogleTokenRequest.java (2)
8-14: 레코드 타입을 적절히 활용한 DTO 설계입니다.불변 데이터를 표현하기 위한 레코드 타입을 사용하여 DTO를 효과적으로 구현했습니다.
24-32: MultiValueMap 변환 메서드가 잘 구현되어 있습니다.요청 파라미터를 MultiValueMap으로 변환하는 로직이 깔끔하게 구현되어 있습니다.
src/main/java/com/evenly/took/feature/auth/client/google/GoogleTokenProvider.java (2)
16-22: 적절한 컴포넌트 구조와 의존성 주입이 잘 설계되었습니다.
@Component어노테이션과 생성자 주입 방식을 사용하여 스프링 빈 구성이 잘 되어 있습니다.
23-35: RestClient 설정이 적절히 되어 있습니다.RestClient를 사용하여 HTTP 클라이언트를 구성하고, 기본 헤더와 에러 핸들러를 설정한 것이 좋습니다.
...n/java/com/evenly/took/feature/auth/client/google/error/GoogleTokenProviderErrorHandler.java
Show resolved
Hide resolved
| import com.evenly.took.feature.user.domain.User; | ||
| import com.evenly.took.global.service.MockTest; | ||
|
|
||
| public class GoogleUserClientTest extends MockTest { |
There was a problem hiding this comment.
우오 UserClient 테스트까지 너무 좋네요오오오 👍👍👍
| RestClient.RequestHeadersSpec requestSpec = restClient.get().uri(userInfoUrl); | ||
| when(requestSpec.header("Authorization", "Bearer " + accessToken)).thenReturn(requestSpec); | ||
| RestClient.ResponseSpec responseSpec = requestSpec.retrieve(); | ||
| when(responseSpec.body(eq(GoogleUserInfoResponse.class))).thenReturn(expectedResponse); |
There was a problem hiding this comment.
RestClient 전체 로직을 한번에 모킹하는 것이 아닌 각 메서드별로 모킹을 진행한 이유가 궁금합니다! 위 방식을 사용했을 때의 이점이 있었을 것 같아서 그냥 궁금해서 여쭈어봅니다!!
| @Mock(answer = Answers.RETURNS_DEEP_STUBS) | ||
| protected RestClient restClient; |
There was a problem hiding this comment.
위 설정은 불필요한 stub이 존재할 때 발생하는 오류를 방지하기 위한 설정이라고 보아도 될까요? 제가 잘 이해한 게 맞는지 궁금해서 여쭤봐요!!
| try { | ||
| GoogleTokenResponse tokenResponse = googleTokenProvider.fetchAccessToken(authCode); | ||
| GoogleUserInfoResponse userInfoResponse = googleUserInfoProvider.fetchUserInfo(tokenResponse.accessToken()); | ||
|
|
||
| return generateUser(userInfoResponse.sub(), userInfoResponse.name()); | ||
| } catch (HttpClientErrorException ex) { | ||
| throw new TookException(AuthErrorCode.INVALID_GOOGLE_CONNECTION); | ||
| } | ||
| } |
There was a problem hiding this comment.
혹시 각 GoogleXXXProvider에서 에러 핸들러를 통해 HttpClientErrorException을 잡고 있어도 잡히지 않는 예외가 있었나요?? 혹시 있었다면 저도 처리해야할 것 같아서 여쭤봐요!
| if (response.getStatusCode().is4xxClientError()) { | ||
| throw new TookException(AuthErrorCode.INVALID_GOOGLE_USER_REQUEST); | ||
| } |
There was a problem hiding this comment.
카카오 서버가 4xx 에러를 반환하여도 위 에러는 결국 저희가 유효하지 않은 Access Token으로 카카오 API를 요청했기 때문에 발생한 문제라고 생각하는데요! 따라서 툭의 클라이언트보다는 툭의 서버단 문제라고 생각해 저는 해당 상황에서 500 에러를 반환하도록 하였어요 이에 대해 어떻게 생각하시는지 궁금합니다! (저희끼리 클라이언트에 반환하는 예외 상태는 통일하면 좋을 것 같아 여쭤봐요) @Lim-Changi 저희가 아직 에러 반환 상태에 대해 논의해본 적이 없어서 함께 논의해보면 좋을 것 같슴다
| String responseBody = StreamUtils.copyToString(response.getBody(), StandardCharsets.UTF_8); | ||
|
|
||
| if (response.getStatusCode().is4xxClientError()) { | ||
| throw new TookException(AuthErrorCode.INVALID_GOOGLE_TOKEN_REQUEST); |
There was a problem hiding this comment.
옹 인증 코드 유효하지 않은 부분은 툭 클라이언트에서 잘못 전달했기 때문에 BAD_REQUEST 반환이 맞겠네요!! 저도 경호님 방식 따라야겠어요
관련 이슈가 있다면 적어주세요.
📌 개발 이유
💻 수정 사항
AuthCodeRequestUrlProvider의 구글 구현체인GoogleAuthCodeRequestUrlProvider추가UserClient의 구글 구현체인GoogleUserClient추가addAllowedOrigin->addAllowedOriginPattern변경🧪 테스트 방법
/api/auth/GOOGLE로 요청 후 redirect가 잘 오는지 확인⛳️ 고민한 점 || 궁굼한 점
/api/auth/GOOGLE이 공통응답 SuccessResponse를 사용 중이라 swagger에 "status": "CONTINUE" 로 예시가 나오는 중인데 RedirectResponse를 새롭게 만들어야할지 궁금합니다.📁 레퍼런스
🧪 테스트 설명
GoogleTokenProviderTest
restClient.post().uri(tokenUri)를 시작으로,header(),body()호출을 stub 처리한 후retrieve()단계에서 올바른 응답 객체를 반환하도록 모킹합니다.retrieve()호출 단계에서TookException을 던지도록 모킹하여,fetchAccessToken()메소드가 예외를 올바르게 처리하는지 확인합니다.GoogleUserInfoProviderTest
restClient.get().uri(userInfoUrl)로 시작해,header()호출을 모킹한 후retrieve()단계에서 올바른 사용자 정보 응답을 반환하도록 설정합니다.retrieve()단계에서 예외를 던지도록 모킹하여,fetchUserInfo()메소드가 예외를 적절하게 처리하는지 확인합니다.Stub과 Lenient 사용 이유
Stub 사용 이유:
post().uri(),header(),body(),retrieve(),body(Class<T>))을 모킹하기 위해 각 호출 단계에 대해 stub을 설정합니다.Lenient 사용 이유:
fetchAccessToken()나fetchUserInfo()메소드 내부에서 생성되는 인자(예: MultiValueMap)가 정확히 일치하지 않을 수 있습니다.lenient()를 적용하여 해당 stubbing이 테스트 흐름에 영향을 주지 않도록 처리했습니다.Summary by CodeRabbit
이외에도 환경별 설정 및 CORS 정책이 업데이트되어 전반적인 시스템 안정성이 강화되었습니다.