Merged
Conversation
- 테스트 프로필에 oauth 설정 추가 - 로컬에서 sonar task 스킵되도록 수정
WalkthroughGoogle OAuth 코드 교환 및 리프레시 토큰 저장 흐름을 도입합니다. 클라이언트에서 받은 authorizationCode를 여러 계층(GoogleApiManager → GoogleApi → GoogleRestClient)을 통해 Google 토큰 엔드포인트로 교환하고, 결과(GoogleTokenResponse)를 도메인 레이어로 전달해 사용자에 리프레시 토큰을 저장합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthUseCase
participant GoogleApiManager
participant GoogleApi
participant GoogleRestClient
participant GoogleOAuth as Google OAuth Server
Client->>AuthUseCase: Sign-in request (authorizationCode)
activate AuthUseCase
AuthUseCase->>GoogleApiManager: fetchGoogleRefreshTokenIfNeeded(authorizationCode)
activate GoogleApiManager
GoogleApiManager->>GoogleApiManager: validate clientSecret/redirectUri/tokenUri
GoogleApiManager->>GoogleApi: exchangeAuthorizationCode(code, clientId, clientSecret, redirectUri, tokenUri)
deactivate GoogleApiManager
activate GoogleApi
GoogleApi->>GoogleRestClient: exchangeAuthorizationCode(code, clientId, clientSecret, redirectUri, url)
deactivate GoogleApi
activate GoogleRestClient
GoogleRestClient->>GoogleOAuth: POST form (code, client_id, client_secret, redirect_uri, grant_type=authorization_code)
activate GoogleOAuth
GoogleOAuth-->>GoogleRestClient: GoogleTokenResponse (access_token, refresh_token, id_token, ...)
deactivate GoogleOAuth
GoogleRestClient-->>GoogleApi: Result<GoogleTokenResponse>
deactivate GoogleRestClient
activate GoogleApi
GoogleApi-->>GoogleApiManager: Result<GoogleTokenResponse>
deactivate GoogleApi
activate GoogleApiManager
GoogleApiManager->>GoogleApiManager: map/validate response, log success
GoogleApiManager-->>AuthUseCase: GoogleTokenResponse (or null/throw)
deactivate GoogleApiManager
AuthUseCase->>UserSignInService: processSignIn(..., googleRefreshToken)
deactivate AuthUseCase
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infra/src/main/kotlin/org/yapp/infra/external/oauth/google/GoogleRestClient.kt (1)
74-80: 🧹 Nitpick | 🔵 Trivialrevoke 엔드포인트 URL이 하드코딩되어 있습니다.
exchangeAuthorizationCode와exchangeIdToken은 URL을 파라미터로 받는 반면,revoke는https://oauth2.googleapis.com/revoke가 하드코딩되어 있어 일관성이 없습니다. 설정을 통해 관리하는 것이 좋습니다.
🤖 Fix all issues with AI agents
In `@apis/src/main/kotlin/org/yapp/apis/auth/manager/GoogleApiManager.kt`:
- Around line 29-31: clientSecret이 null일 때 빈 문자열로 대체하는 대신 명시적 예외를 던져 문제를 빨리
발견하도록 수정하세요: GoogleApiManager.kt에서 googleOauthProperties.clientSecret에 대해 현재
사용중인 "?: \"\"" 패턴을 제거하고 tokenUri 처리와 동일하게 누락 시 IllegalStateException(또는 적절한 런타임
예외)으로 실패하도록 변경해 clientSecret 파라미터를 설정하는 코드(예: clientSecret = ...)가 올바른 값만 전달되게
하세요.
In `@infra/src/main/kotlin/org/yapp/infra/external/oauth/google/GoogleApi.kt`:
- Around line 26-47: Remove the unused exchangeAuthorizationCode function from
GoogleApi.kt: delete the entire exchangeAuthorizationCode(...) declaration
(including its parameters and runCatching call) and any now-unused imports or
references; ensure there are no remaining callers (the codebase and tests use
only exchangeIdToken and GoogleApiManager), then run a quick build to confirm no
compilation or test breakage.
In
`@infra/src/main/kotlin/org/yapp/infra/external/oauth/google/GoogleRestClient.kt`:
- Around line 52-72: The exchangeIdToken function in GoogleRestClient is using
the JWT-bearer grant to try to exchange an ID token at Google's token endpoint,
which is unsupported; replace this flow with a correct approach: either (A) if
you intended to exchange an authorization code, rename exchangeIdToken to
something like exchangeAuthorizationCode and change the request to use
grant_type=authorization_code with parameters code, client_id, client_secret and
redirect_uri and return GoogleTokenResponse; or (B) if you really have an ID
token from the client, remove the token-exchange call and instead validate/parse
the ID token server-side (e.g., with GoogleIdTokenVerifier or a JWT library) to
extract user info and avoid calling the token endpoint; update callers of
exchangeIdToken accordingly (or implement a new method for code exchange and a
separate method for ID token validation).
Comment on lines
26
to
47
| fun exchangeAuthorizationCode( | ||
| code: String, | ||
| clientId: String, | ||
| clientSecret: String, | ||
| redirectUri: String, | ||
| tokenExchangeUrl: String, | ||
| ): Result<GoogleTokenResponse> { | ||
| return runCatching { | ||
| googleRestClient.exchangeAuthorizationCode(code, clientId, clientSecret, redirectUri, tokenExchangeUrl) | ||
| } | ||
| } | ||
|
|
||
| fun exchangeIdToken( | ||
| idToken: String, | ||
| clientId: String, | ||
| clientSecret: String, | ||
| tokenExchangeUrl: String, | ||
| ): Result<GoogleTokenResponse> { | ||
| return runCatching { | ||
| googleRestClient.exchangeIdToken(idToken, clientId, clientSecret, tokenExchangeUrl) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'exchangeAuthorizationCode' --type=kotlinRepository: YAPP-Github/Reed-Server
Length of output: 484
🏁 Script executed:
rg -n 'GoogleApiManager' --type=kotlin -A 10 | head -50Repository: YAPP-Github/Reed-Server
Length of output: 3243
🏁 Script executed:
rg -n 'exchangeIdToken|exchangeAuthorizationCode' --type=kotlin | grep -E '(Manager|Test|test)'Repository: YAPP-Github/Reed-Server
Length of output: 179
🏁 Script executed:
# Final verification - check test files for any usage
find . -type f -name "*Test*.kt" -o -name "*test*.kt" | xargs rg -l 'exchangeAuthorizationCode' 2>/dev/null || echo "No test files found using exchangeAuthorizationCode"Repository: YAPP-Github/Reed-Server
Length of output: 118
exchangeAuthorizationCode 메서드는 사용되지 않으므로 제거하세요.
코드베이스 검색 결과 이 메서드는 정의되기만 하고 외부에서 호출되지 않습니다. GoogleApiManager에서는 exchangeIdToken만 사용 중이며, 테스트 파일에서도 호출되지 않습니다. YAGNI 원칙에 따라 제거를 권장합니다.
🤖 Prompt for AI Agents
In `@infra/src/main/kotlin/org/yapp/infra/external/oauth/google/GoogleApi.kt`
around lines 26 - 47, Remove the unused exchangeAuthorizationCode function from
GoogleApi.kt: delete the entire exchangeAuthorizationCode(...) declaration
(including its parameters and runCatching call) and any now-unused imports or
references; ensure there are no remaining callers (the codebase and tests use
only exchangeIdToken and GoogleApiManager), then run a quick build to confirm no
compilation or test breakage.
infra/src/main/kotlin/org/yapp/infra/external/oauth/google/GoogleRestClient.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt (1)
57-73: 🧹 Nitpick | 🔵 TrivialApple과 Google의
authorizationCode검증 로직이 중복됩니다.두 분기의 null 체크 및 예외 처리 패턴이 동일합니다. 현재는 두 케이스뿐이라 허용 가능하지만, 향후 provider가 추가될 경우 헬퍼 함수 추출을 고려해보세요.
infra/src/main/kotlin/org/yapp/infra/external/oauth/google/GoogleRestClient.kt (1)
55-61: 🧹 Nitpick | 🔵 Trivial
revoke메서드의 URL이 하드코딩되어 있습니다.
exchangeAuthorizationCode는 URL을 파라미터로 받아 설정에서 관리하지만,revoke는"https://oauth2.googleapis.com/revoke"가 직접 하드코딩되어 있습니다. 이 PR 범위 밖이지만, 향후 일관성을 위해 설정으로 외부화하는 것을 권장합니다.apis/src/main/kotlin/org/yapp/apis/auth/usecase/AuthUseCase.kt (1)
77-93: 🧹 Nitpick | 🔵 TrivialApple과 Google의 fetch 헬퍼가 동일 패턴 — 추후 통합 가능
fetchAppleRefreshTokenIfNeeded와fetchGoogleRefreshTokenIfNeeded가 구조적으로 동일합니다(타입 체크 → 외부 API 호출 → refreshToken 반환). 현재 2개 프로바이더이므로 큰 문제는 아니지만,UserSignInService리팩토링과 함께 전략 패턴이나 공통 인터페이스로 통합하면 프로바이더 추가 시 이 파일의 변경을 줄일 수 있습니다.
🤖 Fix all issues with AI agents
In `@apis/src/main/kotlin/org/yapp/apis/auth/manager/GoogleApiManager.kt`:
- Around line 50-52: onSuccess 콜백에서 전달받은 tokenResponse 파라미터가 사용되고 있지 않습니다;
GoogleApiManager.kt 내부의 .onSuccess { tokenResponse -> ... }를 찾아 파라미터 이름을 사용하지
않음을 명시하기 위해 언더스코어로 바꾸거나 파라미터를 완전히 생략하여 .onSuccess { _ -> ... } 또는 .onSuccess {
... } 형식으로 수정하세요.
In `@apis/src/main/kotlin/org/yapp/apis/auth/service/UserSignInService.kt`:
- Around line 18-45: 현재 processSignIn의 시그니처가 appleRefreshToken: String? ,
googleRefreshToken: String? 처럼 프로바이더별 nullable 파라미터로 되어 있어 프로바이더가 늘어나면 시그니처가 계속
커집니다; 이를 고치려면 FindOrCreateUserRequest를 그대로 받고 프로바이더별 refresh token을 하나의 인자(예:
sealed class SocialRefreshToken 또는 Map<String,String> socialRefreshTokens)로 통합한
뒤 processSignIn 내부에서 initialUserResponse를 받은 후 socialRefreshTokens를 검사하여 각
프로바이더에 맞는 update 메서드(userAccountService.updateAppleRefreshToken,
updateGoogleRefreshToken 등)를 호출하도록 분기 또는 반복 처리로 변경하세요; 새로운 프로바이더가 추가되면 sealed
class에 케이스를 추가하거나 맵 키만 추가하면 되도록 구현을 변경하면 됩니다.
- Around line 25-43: The code currently reassigns userResponse from
initialUserResponse after the Apple update and then uses that potentially
modified value for the Google update (symbols: userResponse,
initialUserResponse, appleRefreshToken, googleRefreshToken,
userAccountService.updateAppleRefreshToken, updateGoogleRefreshToken), which
causes two sequential DB updates if both tokens are provided; change this by
adding a defensive check that disallows both appleRefreshToken and
googleRefreshToken being non-blank (throw a clear IllegalArgumentException or
custom BadRequestException) before performing any updates, or explicitly
document and retain the dual-update behavior—implement the guard near the start
of the method to validate tokens and return/throw before calling
updateAppleRefreshToken/updateGoogleRefreshToken.
In
`@apis/src/main/kotlin/org/yapp/apis/auth/strategy/signin/SignInCredentials.kt`:
- Around line 22-29: GoogleAuthCredentials uses a block-bodied override for
getProviderType(), which is inconsistent with KakaoAuthCredentials and
AppleAuthCredentials that use expression-body overrides; change
GoogleAuthCredentials.getProviderType() to an expression body returning
ProviderType.GOOGLE so the style matches (update the override in class
GoogleAuthCredentials to use = ProviderType.GOOGLE).
In `@apis/src/main/kotlin/org/yapp/apis/auth/usecase/AuthUseCase.kt`:
- Around line 86-93: Verify the frontend's Google Sign-In flow includes
access_type=offline and prompt=consent so a refresh_token can be issued, and
then update fetchGoogleRefreshTokenIfNeeded to handle nullable
GoogleTokenResponse.refreshToken from
googleApiManager.exchangeAuthorizationCode: when credentials is
GoogleAuthCredentials call exchangeAuthorizationCode as now, but if
tokenResponse.refreshToken is null emit a warning via the existing logger
(include identifying info like user id or authorizationCode) and explicitly
return null (or throw a documented AuthException if your flow requires a refresh
token); reference fetchGoogleRefreshTokenIfNeeded, GoogleAuthCredentials,
googleApiManager.exchangeAuthorizationCode, and GoogleTokenResponse.refreshToken
when making the change.
In
`@apis/src/main/kotlin/org/yapp/apis/user/dto/request/SaveGoogleRefreshTokenRequest.kt`:
- Around line 21-26: The `@Schema` description for SaveGoogleRefreshTokenRequest
is inconsistent with the `@field`:NotBlank validation on googleRefreshToken and
the of() factory that creates non-null values; update the `@Schema` on
googleRefreshToken to indicate it is required/non-blank (remove "nullable if not
issued yet") and reflect that it contains a valid Google refresh token, so the
documentation matches the `@field`:NotBlank constraint and the of() factory
behavior.
- Around line 9-12: Remove the unnecessary Swagger annotation from the internal
DTO by deleting the `@Schema` annotation on the SaveGoogleRefreshTokenRequest
class; this DTO is constructed via its of() factory and not exposed as an API
request body, so simply remove the `@Schema`(...) declaration (leave class,
properties, and the of() factory intact).
Comment on lines
+50
to
+52
| .onSuccess { tokenResponse -> | ||
| log.info { "Successfully exchanged Google authorization code for tokens" } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
onSuccess 콜백에서 사용하지 않는 파라미터를 _로 변경하세요.
tokenResponse 파라미터가 사용되지 않습니다.
♻️ 수정 제안
- .onSuccess { tokenResponse ->
+ .onSuccess { _ ->
log.info { "Successfully exchanged Google authorization code for tokens" }
}또는 파라미터를 완전히 생략:
- .onSuccess { tokenResponse ->
+ .onSuccess {
log.info { "Successfully exchanged Google authorization code for tokens" }
}📝 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
| .onSuccess { tokenResponse -> | |
| log.info { "Successfully exchanged Google authorization code for tokens" } | |
| } | |
| .onSuccess { _ -> | |
| log.info { "Successfully exchanged Google authorization code for tokens" } | |
| } |
🤖 Prompt for AI Agents
In `@apis/src/main/kotlin/org/yapp/apis/auth/manager/GoogleApiManager.kt` around
lines 50 - 52, onSuccess 콜백에서 전달받은 tokenResponse 파라미터가 사용되고 있지 않습니다;
GoogleApiManager.kt 내부의 .onSuccess { tokenResponse -> ... }를 찾아 파라미터 이름을 사용하지
않음을 명시하기 위해 언더스코어로 바꾸거나 파라미터를 완전히 생략하여 .onSuccess { _ -> ... } 또는 .onSuccess {
... } 형식으로 수정하세요.
Comment on lines
18
to
+45
| fun processSignIn( | ||
| @Valid request: FindOrCreateUserRequest, appleRefreshToken: String? | ||
| @Valid request: FindOrCreateUserRequest, | ||
| appleRefreshToken: String?, | ||
| googleRefreshToken: String? | ||
| ): CreateUserResponse { | ||
| val initialUserResponse = userAccountService.findOrCreateUser(request) | ||
|
|
||
| return appleRefreshToken.takeIf { !it.isNullOrBlank() } | ||
| ?.let { token -> | ||
| userAccountService.updateAppleRefreshToken( | ||
| SaveAppleRefreshTokenRequest.of( | ||
| initialUserResponse, token | ||
| ) | ||
| var userResponse = initialUserResponse | ||
|
|
||
| // Update Apple refresh token if provided | ||
| if (!appleRefreshToken.isNullOrBlank()) { | ||
| userResponse = userAccountService.updateAppleRefreshToken( | ||
| SaveAppleRefreshTokenRequest.of( | ||
| userResponse, appleRefreshToken | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| // Update Google refresh token if provided | ||
| if (!googleRefreshToken.isNullOrBlank()) { | ||
| userResponse = userAccountService.updateGoogleRefreshToken( | ||
| SaveGoogleRefreshTokenRequest.of( | ||
| userResponse, googleRefreshToken | ||
| ) | ||
| } ?: initialUserResponse | ||
| ) | ||
| } | ||
|
|
||
| return userResponse |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
확장성을 고려한 리팩토링 제안: 소셜 프로바이더 추가 시 파라미터 증가 문제
현재 Apple/Google 두 개 프로바이더에 대해 각각 nullable 파라미터를 추가하는 방식은 명확하고 동작에 문제가 없습니다. 다만, 향후 프로바이더가 추가될 때마다 processSignIn의 시그니처가 계속 늘어나게 됩니다.
프로바이더별 refresh token을 하나의 sealed class 또는 map 기반 파라미터로 통합하면 확장 시 시그니처 변경 없이 대응할 수 있습니다.
예시: sealed class 활용
sealed class SocialRefreshToken {
data class Apple(val refreshToken: String) : SocialRefreshToken()
data class Google(val refreshToken: String) : SocialRefreshToken()
}
fun processSignIn(
`@Valid` request: FindOrCreateUserRequest,
socialRefreshToken: SocialRefreshToken?
): CreateUserResponse {
val initialUserResponse = userAccountService.findOrCreateUser(request)
return when (socialRefreshToken) {
is SocialRefreshToken.Apple -> userAccountService.updateAppleRefreshToken(
SaveAppleRefreshTokenRequest.of(initialUserResponse, socialRefreshToken.refreshToken)
)
is SocialRefreshToken.Google -> userAccountService.updateGoogleRefreshToken(
SaveGoogleRefreshTokenRequest.of(initialUserResponse, socialRefreshToken.refreshToken)
)
null -> initialUserResponse
}
}🤖 Prompt for AI Agents
In `@apis/src/main/kotlin/org/yapp/apis/auth/service/UserSignInService.kt` around
lines 18 - 45, 현재 processSignIn의 시그니처가 appleRefreshToken: String? ,
googleRefreshToken: String? 처럼 프로바이더별 nullable 파라미터로 되어 있어 프로바이더가 늘어나면 시그니처가 계속
커집니다; 이를 고치려면 FindOrCreateUserRequest를 그대로 받고 프로바이더별 refresh token을 하나의 인자(예:
sealed class SocialRefreshToken 또는 Map<String,String> socialRefreshTokens)로 통합한
뒤 processSignIn 내부에서 initialUserResponse를 받은 후 socialRefreshTokens를 검사하여 각
프로바이더에 맞는 update 메서드(userAccountService.updateAppleRefreshToken,
updateGoogleRefreshToken 등)를 호출하도록 분기 또는 반복 처리로 변경하세요; 새로운 프로바이더가 추가되면 sealed
class에 케이스를 추가하거나 맵 키만 추가하면 되도록 구현을 변경하면 됩니다.
Comment on lines
+25
to
+43
| var userResponse = initialUserResponse | ||
|
|
||
| // Update Apple refresh token if provided | ||
| if (!appleRefreshToken.isNullOrBlank()) { | ||
| userResponse = userAccountService.updateAppleRefreshToken( | ||
| SaveAppleRefreshTokenRequest.of( | ||
| userResponse, appleRefreshToken | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| // Update Google refresh token if provided | ||
| if (!googleRefreshToken.isNullOrBlank()) { | ||
| userResponse = userAccountService.updateGoogleRefreshToken( | ||
| SaveGoogleRefreshTokenRequest.of( | ||
| userResponse, googleRefreshToken | ||
| ) | ||
| } ?: initialUserResponse | ||
| ) | ||
| } |
There was a problem hiding this comment.
var 재할당 체인이 정확하게 동작하는지 확인 필요
현재 userResponse가 Apple 토큰 업데이트 결과를 받고, 이어서 Google 토큰 업데이트에도 동일한 userResponse를 사용합니다. 실제 로그인 플로우에서는 Apple 또는 Google 중 하나만 non-null이므로 문제 없지만, 만약 두 토큰이 동시에 전달되면 두 번의 DB 업데이트가 순차 실행됩니다.
이 동작이 의도된 것인지 확인하거나, 방어적으로 둘 다 전달된 경우를 차단하는 것을 고려해 주세요.
방어 코드 예시
fun processSignIn(
`@Valid` request: FindOrCreateUserRequest,
appleRefreshToken: String?,
googleRefreshToken: String?
): CreateUserResponse {
+ require(!(appleRefreshToken != null && googleRefreshToken != null)) {
+ "Apple과 Google refresh token이 동시에 전달될 수 없습니다."
+ }
val initialUserResponse = userAccountService.findOrCreateUser(request)🤖 Prompt for AI Agents
In `@apis/src/main/kotlin/org/yapp/apis/auth/service/UserSignInService.kt` around
lines 25 - 43, The code currently reassigns userResponse from
initialUserResponse after the Apple update and then uses that potentially
modified value for the Google update (symbols: userResponse,
initialUserResponse, appleRefreshToken, googleRefreshToken,
userAccountService.updateAppleRefreshToken, updateGoogleRefreshToken), which
causes two sequential DB updates if both tokens are provided; change this by
adding a defensive check that disallows both appleRefreshToken and
googleRefreshToken being non-blank (throw a clear IllegalArgumentException or
custom BadRequestException) before performing any updates, or explicitly
document and retain the dual-update behavior—implement the guard near the start
of the method to validate tokens and return/throw before calling
updateAppleRefreshToken/updateGoogleRefreshToken.
Comment on lines
22
to
29
| data class GoogleAuthCredentials( | ||
| val idToken: String, | ||
| val authorizationCode: String, | ||
| ) : SignInCredentials() { | ||
| override fun getProviderType(): ProviderType { | ||
| return ProviderType.GOOGLE | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
getProviderType() 스타일이 다른 credentials 클래스와 일관되지 않습니다.
KakaoAuthCredentials와 AppleAuthCredentials는 expression body(= ProviderType.X)를 사용하지만, GoogleAuthCredentials만 block body를 사용합니다.
♻️ 일관성을 위한 수정 제안
data class GoogleAuthCredentials(
val idToken: String,
val authorizationCode: String,
) : SignInCredentials() {
- override fun getProviderType(): ProviderType {
- return ProviderType.GOOGLE
- }
+ override fun getProviderType(): ProviderType = ProviderType.GOOGLE
}📝 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
| data class GoogleAuthCredentials( | |
| val idToken: String, | |
| val authorizationCode: String, | |
| ) : SignInCredentials() { | |
| override fun getProviderType(): ProviderType { | |
| return ProviderType.GOOGLE | |
| } | |
| } | |
| data class GoogleAuthCredentials( | |
| val idToken: String, | |
| val authorizationCode: String, | |
| ) : SignInCredentials() { | |
| override fun getProviderType(): ProviderType = ProviderType.GOOGLE | |
| } |
🤖 Prompt for AI Agents
In `@apis/src/main/kotlin/org/yapp/apis/auth/strategy/signin/SignInCredentials.kt`
around lines 22 - 29, GoogleAuthCredentials uses a block-bodied override for
getProviderType(), which is inconsistent with KakaoAuthCredentials and
AppleAuthCredentials that use expression-body overrides; change
GoogleAuthCredentials.getProviderType() to an expression body returning
ProviderType.GOOGLE so the style matches (update the override in class
GoogleAuthCredentials to use = ProviderType.GOOGLE).
Comment on lines
+86
to
+93
| private fun fetchGoogleRefreshTokenIfNeeded(credentials: SignInCredentials): String? { | ||
| if (credentials is GoogleAuthCredentials) { | ||
| val tokenResponse = googleApiManager.exchangeAuthorizationCode(credentials.authorizationCode) | ||
| return tokenResponse.refreshToken | ||
| } | ||
|
|
||
| return null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for GoogleApiManager implementation
echo "=== Searching for GoogleApiManager.kt ==="
fd "GoogleApiManager.kt" -type f
echo ""
echo "=== Searching for GoogleRestClient.kt ==="
fd "GoogleRestClient.kt" -type f
echo ""
echo "=== Searching for GoogleTokenResponse.kt ==="
fd "GoogleTokenResponse.kt" -type f
echo ""
echo "=== Searching for related API manager files ==="
fd -e ".kt" | grep -i "google.*api\|api.*manager" | head -20Repository: YAPP-Github/Reed-Server
Length of output: 1050
🌐 Web query:
Google OAuth2 refresh_token response requirements access_type offline prompt consent
💡 Result:
To receive a refresh_token from Google’s OAuth 2.0 token response (when exchanging an authorization code):
- Request offline access in the authorization request by setting
access_type=offline. This instructs Google to return a refresh token on the first code exchange for that user+client. [1][2] - Expect
refresh_tokenonly on the first authorization for a given user ↔ OAuth client pairing; subsequent authorizations are often auto-approved and won’t include a new refresh token unless the user revoked access. [1][3] - If you must obtain a new refresh token (e.g., you lost it), force a new consent screen with
prompt=consentin the authorization request. (prompt=select_accountalone is not sufficient.) [1][4]
Notes:
include_granted_scopes=trueis for incremental authorization; it’s commonly recommended but isn’t the key requirement for getting a refresh token. [1]
🏁 Script executed:
#!/bin/bash
echo "=== GoogleApiManager.kt ==="
cat "apis/src/main/kotlin/org/yapp/apis/auth/manager/GoogleApiManager.kt"
echo ""
echo "=== GoogleApi.kt ==="
cat "infra/src/main/kotlin/org/yapp/infra/external/oauth/google/GoogleApi.kt"
echo ""
echo "=== Searching for GoogleTokenResponse ==="
fd -e "kt" -path "*/google/*" | xargs grep -l "GoogleTokenResponse\|class.*Token.*Response" 2>/dev/null | head -10Repository: YAPP-Github/Reed-Server
Length of output: 5652
🏁 Script executed:
#!/bin/bash
# Search for GoogleTokenResponse definition
echo "=== Searching for token response classes ==="
rg "data class|class.*TokenResponse" -A 5 -i --type kt | grep -i google -A 5 | head -40Repository: YAPP-Github/Reed-Server
Length of output: 155
🏁 Script executed:
#!/bin/bash
echo "=== Searching for GoogleTokenResponse definition ==="
find . -name "*.kt" -path "*/google/*" -exec grep -l "GoogleTokenResponse" {} \;
echo ""
echo "=== Searching for GoogleRestClient ==="
find . -name "*.kt" | xargs grep -l "GoogleRestClient" | head -5Repository: YAPP-Github/Reed-Server
Length of output: 562
🏁 Script executed:
#!/bin/bash
echo "=== Find all files in google folder ==="
find . -path "*/google/*" -name "*.kt" | head -20
echo ""
echo "=== Search for response classes in google oauth folder ==="
find . -path "*/google/*" -name "*.kt" -exec grep -H "data class\|class.*Response" {} \;Repository: YAPP-Github/Reed-Server
Length of output: 824
🏁 Script executed:
#!/bin/bash
echo "=== GoogleTokenResponse.kt ==="
cat "./infra/src/main/kotlin/org/yapp/infra/external/oauth/google/response/GoogleTokenResponse.kt"
echo ""
echo "=== GoogleRestClient.kt ==="
cat "./infra/src/main/kotlin/org/yapp/infra/external/oauth/google/GoogleRestClient.kt"Repository: YAPP-Github/Reed-Server
Length of output: 2745
🏁 Script executed:
#!/bin/bash
echo "=== Searching for GoogleOauthProperties ==="
find . -name "*.kt" | xargs grep -l "GoogleOauthProperties"
echo ""
echo "=== Searching for authorization request construction ==="
find . -name "*.kt" | xargs grep -B 5 -A 5 "access_type\|prompt.*consent" | head -30Repository: YAPP-Github/Reed-Server
Length of output: 453
🏁 Script executed:
#!/bin/bash
echo "=== GoogleOauthProperties.kt ==="
cat "./apis/src/main/kotlin/org/yapp/apis/config/GoogleOauthProperties.kt"
echo ""
echo "=== Searching for authorization request/login flow ==="
find . -name "*.kt" | xargs grep -l "authorize\|authorization.*url\|oauth.*login" | head -10Repository: YAPP-Github/Reed-Server
Length of output: 647
🏁 Script executed:
#!/bin/bash
echo "=== Search for Google OAuth authorize endpoint construction ==="
rg -i "authorize|/o/oauth" --max-count 20
echo ""
echo "=== Search for OAuth login controller ==="
find . -name "*Controller.kt" | xargs grep -l "google\|oauth" | head -5Repository: YAPP-Github/Reed-Server
Length of output: 1772
🏁 Script executed:
#!/bin/bash
echo "=== Search for authentication controller ==="
find . -path "*/auth/*" -name "*Controller.kt" -o -path "*/auth/*" -name "*Endpoint.kt"
echo ""
echo "=== Search for Google authorization URL construction ==="
rg -i "google.*authorize|createAuthorizationRequestUri|getAuthorizationUri" --max-count 20
echo ""
echo "=== Check AuthController ==="
find . -name "*AuthController.kt" -o -name "*LoginController.kt"Repository: YAPP-Github/Reed-Server
Length of output: 341
🏁 Script executed:
#!/bin/bash
echo "=== Look at auth usecase and controller ==="
find ./apis/src/main/kotlin/org/yapp/apis/auth -name "*.kt" | head -20
echo ""
echo "=== Search for Google OAuth login/authorize endpoint ==="
rg -B 3 -A 10 "fun.*google.*login|fun.*sign.*google" -i --max-count 15Repository: YAPP-Github/Reed-Server
Length of output: 4513
🏁 Script executed:
#!/bin/bash
echo "=== GoogleSignInStrategy.kt ==="
cat "./apis/src/main/kotlin/org/yapp/apis/auth/strategy/signin/GoogleSignInStrategy.kt"
echo ""
echo "=== AuthController.kt ==="
cat "./apis/src/main/kotlin/org/yapp/apis/auth/controller/AuthController.kt"Repository: YAPP-Github/Reed-Server
Length of output: 3874
🏁 Script executed:
#!/bin/bash
echo "=== Check SocialLoginRequest to understand credentials ==="
cat "./apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt"
echo ""
echo "=== Search for GoogleAuthCredentials ==="
find . -name "*.kt" | xargs grep -l "GoogleAuthCredentials" | head -5Repository: YAPP-Github/Reed-Server
Length of output: 3400
🏁 Script executed:
#!/bin/bash
echo "=== SignInCredentials.kt and related ==="
cat "./apis/src/main/kotlin/org/yapp/apis/auth/strategy/signin/SignInCredentials.kt"
echo ""
echo "=== Full AuthUseCase.kt to understand context ==="
cat "./apis/src/main/kotlin/org/yapp/apis/auth/usecase/AuthUseCase.kt"Repository: YAPP-Github/Reed-Server
Length of output: 5116
Google OAuth 토큰 교환 시 에러 핸들링과 refresh_token 가능성 검토 필요
현재 코드는 googleApiManager.exchangeAuthorizationCode에서 AuthException으로 에러를 변환하고 있어 기본 에러 처리는 구현되어 있습니다. 다만 두 가지 확인이 필요합니다:
-
클라이언트 인증 요청 확인 - Google OAuth에서 refresh_token을 받으려면 초기 인증 요청(authorization request)에
access_type=offline&prompt=consent이 포함되어야 합니다. 현재 HTML의 Google Sign-In 라이브러리가 이 파라미터를 설정하고 있는지 확인하세요. -
null refresh_token 처리 -
GoogleTokenResponse.refreshToken은 nullable입니다. 클라이언트가 오프라인 접근을 요청하지 않으면 null이 반환될 수 있으며, 현재 코드는 이를 그대로 전달합니다. 이것이 의도된 동작인지, 아니면 null인 경우 로그 경고를 추가해야 하는지 검토 부탁드립니다.
🤖 Prompt for AI Agents
In `@apis/src/main/kotlin/org/yapp/apis/auth/usecase/AuthUseCase.kt` around lines
86 - 93, Verify the frontend's Google Sign-In flow includes access_type=offline
and prompt=consent so a refresh_token can be issued, and then update
fetchGoogleRefreshTokenIfNeeded to handle nullable
GoogleTokenResponse.refreshToken from
googleApiManager.exchangeAuthorizationCode: when credentials is
GoogleAuthCredentials call exchangeAuthorizationCode as now, but if
tokenResponse.refreshToken is null emit a warning via the existing logger
(include identifying info like user id or authorizationCode) and explicitly
return null (or throw a documented AuthException if your flow requires a refresh
token); reference fetchGoogleRefreshTokenIfNeeded, GoogleAuthCredentials,
googleApiManager.exchangeAuthorizationCode, and GoogleTokenResponse.refreshToken
when making the change.
Comment on lines
+9
to
+12
| @Schema( | ||
| name = "SaveGoogleRefreshTokenRequest", | ||
| description = "Request DTO for saving Google refresh token with user ID and authorization code" | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
내부 전용 DTO에 @Schema 어노테이션이 불필요해 보입니다.
이 클래스는 API 요청 바디로 직접 노출되지 않고, of() 팩토리를 통해 내부적으로만 생성됩니다. Swagger 문서에 노출할 필요가 없다면 @Schema 어노테이션을 제거하는 것이 좋습니다.
🤖 Prompt for AI Agents
In
`@apis/src/main/kotlin/org/yapp/apis/user/dto/request/SaveGoogleRefreshTokenRequest.kt`
around lines 9 - 12, Remove the unnecessary Swagger annotation from the internal
DTO by deleting the `@Schema` annotation on the SaveGoogleRefreshTokenRequest
class; this DTO is constructed via its of() factory and not exposed as an API
request body, so simply remove the `@Schema`(...) declaration (leave class,
properties, and the of() factory intact).
Comment on lines
+21
to
+26
| @field:Schema( | ||
| description = "Google refresh token, nullable if not issued yet", | ||
| example = "1//0g_xxxxxxxxxxxxxxxxxxxxxx" | ||
| ) | ||
| @field:NotBlank(message = "googleRefreshToken must not be blank") | ||
| val googleRefreshToken: String? = null |
There was a problem hiding this comment.
@Schema description과 @field:NotBlank 검증이 모순됩니다.
Line 22의 description에는 "nullable if not issued yet"이라고 되어 있지만, Line 25의 @field:NotBlank는 blank/null을 허용하지 않습니다. 이 DTO는 of() 팩토리 메서드를 통해 non-null 값으로만 생성되므로, description을 수정하는 것이 맞습니다.
📝 description 수정 제안
`@field`:Schema(
- description = "Google refresh token, nullable if not issued yet",
+ description = "Google refresh token",
example = "1//0g_xxxxxxxxxxxxxxxxxxxxxx"
)🤖 Prompt for AI Agents
In
`@apis/src/main/kotlin/org/yapp/apis/user/dto/request/SaveGoogleRefreshTokenRequest.kt`
around lines 21 - 26, The `@Schema` description for SaveGoogleRefreshTokenRequest
is inconsistent with the `@field`:NotBlank validation on googleRefreshToken and
the of() factory that creates non-null values; update the `@Schema` on
googleRefreshToken to indicate it is required/non-blank (remove "nullable if not
issued yet") and reflect that it contains a valid Google refresh token, so the
documentation matches the `@field`:NotBlank constraint and the of() factory
behavior.
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
새로운 기능
변경사항