-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT/#292] 구글 로그인 구현합니다. #310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- DataStore Preferences 라이브러리 의존성 추가
- `attemptAutoLogin` 메서드가 로그인 상태를 반환하도록 수정했습니다.
- OAuthDataStore 추가
- ID 토큰 및 플랫폼 정보 저장, 조회, 삭제 기능 구현
- DataStore를 사용하여 OAuth 관련 정보 관리
- OAuthDataStoreKeys 추가
- DataStore 키 정의
- AuthService에 구글 회원가입 API 함수 추가 - AuthInterceptor에 구글 회원가입 경로 추가 (헤더 미포함 처리) - GoogleSignUpRequestDto 추가 - AuthDataSource 및 AuthDataSourceImpl에 구글 회원가입 관련 함수 추가
AuthRepository와 AuthRepositoryImpl에 `signUpWithGoogle` 함수를 추가
- 구글 로그인 기능을 구현했습니다. - GoogleSignInHelper 구현 - 404면 회원가입 프로세스 진행 - 추후 회원가입 때 어떤 타입의 회원가입(구글/카카오)인지 판단하기 위해 프로파이더로 분기
회원가입 API 연동 - 회원가입 API에 구글 로그인 로직 추가 - Kakao와 Google 플랫폼에 따른 회원가입 분기 처리
WalkthroughThis update introduces Google OAuth login to the application. It adds new data storage and authentication modules, integrates Google sign-in with token handling, updates dependency management, and refactors login and sign-up flows to support Google and Kakao providers. UI and ViewModel logic are adjusted to unify and streamline OAuth handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginScreen
participant GoogleSignInHelper
participant LoginViewModel
participant OAuthDataStore
participant AuthRepository
participant AuthService
User->>LoginScreen: Clicks Google Login
LoginScreen->>GoogleSignInHelper: requestSignIn()
GoogleSignInHelper->>LoginScreen: onSuccess(idToken) / onFailure
LoginScreen->>LoginViewModel: LoginOAuth(provider=GOOGLE, idToken)
LoginViewModel->>AuthRepository: signUpWithGoogle(GoogleSignUpRequestDto)
AuthRepository->>AuthService: signUpWithGoogle()
AuthService-->>AuthRepository: ApiResponse<SignUpResponseDto>
AuthRepository-->>LoginViewModel: Result<SignUpResponseDto>
LoginViewModel->>OAuthDataStore: save tokens / clear OAuth data
LoginViewModel->>LoginScreen: Navigate to Home or SignUp
Estimated code review effort🎯 4 (Complex) | ⏱️ ~90 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected relative to the linked issue objectives. Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🔭 Outside diff range comments (1)
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt (1)
1-3: Fix import order to comply with KtLint.Imports must be ordered lexicographically. Move the
android.content.Contextimport before thecom.*imports.package com.sopt.clody.presentation.ui.auth.signup +import android.content.Context import com.airbnb.mvrx.MavericksViewModel -import android.content.Context
🧹 Nitpick comments (3)
app/src/main/java/com/sopt/clody/data/remote/dto/request/GoogleSignUpRequestDto.kt (1)
6-12: LGTM! Well-structured DTO with proper serialization.The implementation follows good practices with immutable properties, appropriate nullability, and proper kotlinx.serialization annotations.
Consider using an enum for the
platformfield instead ofStringfor better type safety:@Serializable data class GoogleSignUpRequestDto( @SerialName("idToken") val idToken: String, - @SerialName("platform") val platform: String, + @SerialName("platform") val platform: OAuthProvider, @SerialName("fcmToken") val fcmToken: String, @SerialName("name") val name: String?, )This would leverage the existing
OAuthProviderenum and prevent potential runtime errors from invalid platform values.app/src/main/java/com/sopt/clody/presentation/ui/login/LoginContract.kt (1)
15-15: Good unification of OAuth login intents.The unified
LoginOAuthintent withOAuthProviderenum provides a clean, type-safe approach to handling different OAuth providers.Consider making the intent parameters more explicit about when they're required:
sealed class LoginIntent { data class LoginOAuth( val provider: OAuthProvider, val context: Context, // Required for launching OAuth flow val idToken: String? = null // Optional for cases where token is already available ) : LoginIntent() data object ClearError : LoginIntent() }This makes it clearer that
contextis required for OAuth flows whileidTokenis truly optional.app/src/main/java/com/sopt/clody/presentation/ui/login/LoginViewModel.kt (1)
85-105: Consider more robust error handling patterns.The core logic for Kakao user validation is sound, but the error handling could be improved:
The string-based error checking (lines 98-99) is fragile and mixes HTTP status codes with Korean error messages:
if (msg.contains("404") || msg.contains("유저가 없습니다")) {Consider using structured error handling with specific error types or codes from the repository layer instead of parsing error messages. This would make the code more maintainable and less prone to breaking when error messages change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
app/build.gradle.kts(2 hunks)app/src/main/java/com/sopt/clody/data/datastore/DataStoreModule.kt(1 hunks)app/src/main/java/com/sopt/clody/data/datastore/OAuthDataStore.kt(1 hunks)app/src/main/java/com/sopt/clody/data/datastore/OAuthDataStoreKeys.kt(1 hunks)app/src/main/java/com/sopt/clody/data/datastore/OAuthProvider.kt(1 hunks)app/src/main/java/com/sopt/clody/data/remote/api/AuthService.kt(2 hunks)app/src/main/java/com/sopt/clody/data/remote/datasource/AuthDataSource.kt(2 hunks)app/src/main/java/com/sopt/clody/data/remote/datasourceimpl/AuthDataSourceImpl.kt(2 hunks)app/src/main/java/com/sopt/clody/data/remote/dto/request/GoogleSignUpRequestDto.kt(1 hunks)app/src/main/java/com/sopt/clody/data/remote/util/AuthInterceptor.kt(1 hunks)app/src/main/java/com/sopt/clody/data/repositoryimpl/AuthRepositoryImpl.kt(2 hunks)app/src/main/java/com/sopt/clody/domain/repository/AuthRepository.kt(2 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpContract.kt(2 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpScreen.kt(2 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt(3 hunks)app/src/main/java/com/sopt/clody/presentation/ui/login/GoogleSignInHelper.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/login/LoginContract.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/login/LoginScreen.kt(9 hunks)app/src/main/java/com/sopt/clody/presentation/ui/login/LoginViewModel.kt(3 hunks)app/src/main/java/com/sopt/clody/presentation/ui/splash/SplashViewModel.kt(2 hunks)gradle/libs.versions.toml(2 hunks)
🧬 Code Graph Analysis (1)
app/src/main/java/com/sopt/clody/presentation/ui/login/LoginScreen.kt (1)
app/src/main/java/com/sopt/clody/presentation/ui/auth/component/button/KaKaoButton.kt (1)
KaKaoButton(24-54)
🪛 GitHub Actions: Clody CI
app/src/main/java/com/sopt/clody/presentation/ui/login/GoogleSignInHelper.kt
[error] 4-4: KtLint: Unused import detected.
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt
[error] 3-3: KtLint: Imports must be ordered in lexicographic order without any empty lines in-between with "java", "javax", "kotlin" and aliases in the end.
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/sopt/clody/presentation/ui/login/LoginScreen.kt (1)
app/src/main/java/com/sopt/clody/presentation/ui/auth/component/button/KaKaoButton.kt (1)
KaKaoButton(24-54)
🪛 GitHub Actions: Clody CI
app/src/main/java/com/sopt/clody/presentation/ui/login/GoogleSignInHelper.kt
[error] 4-4: KtLint: Unused import detected.
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt
[error] 3-3: KtLint: Imports must be ordered in lexicographic order without any empty lines in-between with "java", "javax", "kotlin" and aliases in the end.
🔇 Additional comments (33)
gradle/libs.versions.toml (1)
143-145: LGTM! Library declarations are correctly structured.The library entries properly reference the version variables and use the correct module coordinates for the OAuth and DataStore dependencies.
app/build.gradle.kts (2)
34-34: LGTM! Proper handling of sensitive OAuth configuration.The Google OAuth client ID is correctly sourced from
local.propertiesand exposed as a build config field, following security best practices for handling sensitive API keys.Also applies to: 41-41
159-161: LGTM! Dependencies correctly added.The new OAuth and DataStore dependencies are properly declared and match the library definitions in
libs.versions.toml.app/src/main/java/com/sopt/clody/data/remote/util/AuthInterceptor.kt (1)
52-52: LGTM! Correct exclusion for Google OAuth endpoint.Adding the Google OAuth endpoint to the authorization exclusion list is appropriate, as OAuth flows use their own authentication mechanism and don't require existing authorization headers.
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpScreen.kt (1)
83-85: LGTM! Code formatting improved.The multi-line lambda formatting enhances readability without changing the logic.
app/src/main/java/com/sopt/clody/data/datastore/OAuthProvider.kt (1)
1-6: LGTM! Well-designed OAuth provider enum.The enum provides type-safe representation of OAuth providers with clear API value mapping. The design is clean, extensible, and follows good Kotlin practices.
app/src/main/java/com/sopt/clody/data/remote/api/AuthService.kt (2)
4-4: Import addition looks good.The import for
GoogleSignUpRequestDtois properly added to support the new Google OAuth endpoint.
26-29: Google OAuth endpoint implementation follows good practices.The new
signUpWithGooglemethod is well-structured:
- Uses appropriate REST endpoint for OAuth2 (
api/v1/auth/oauth2/google)- Correctly omits authorization header since OAuth tokens are handled in the request body
- Maintains consistency with existing API response patterns
- Clear method naming distinguishes it from regular signup
app/src/main/java/com/sopt/clody/data/remote/datasourceimpl/AuthDataSourceImpl.kt (2)
6-6: Import addition is appropriate.The import for
GoogleSignUpRequestDtosupports the new method implementation.
22-23: Implementation follows established patterns.The
signUpWithGooglemethod implementation correctly delegates to theauthServiceand maintains consistency with other methods in the class. The single-expression function style is clean and appropriate.app/src/main/java/com/sopt/clody/data/remote/datasource/AuthDataSource.kt (2)
4-4: Import addition supports new interface method.The import for
GoogleSignUpRequestDtois correctly added to support the new method signature.
13-13: Interface extension is well-designed.The new
signUpWithGooglemethod signature is consistent with existing patterns and appropriately typed. The method clearly distinguishes Google OAuth signup from regular signup by acceptingGoogleSignUpRequestDtoinstead of requiring separate authorization parameters.app/src/main/java/com/sopt/clody/domain/repository/AuthRepository.kt (2)
3-3: Import addition is properly placed.The import for
GoogleSignUpRequestDtois correctly positioned and supports the new method signature.
12-12: Repository method follows domain layer patterns.The
signUpWithGooglemethod signature is well-designed:
- Uses
Result<T>wrapper for consistent error handling across the repository- Clear parameter typing with
GoogleSignUpRequestDto- Maintains consistency with existing repository methods
- Proper separation of concerns at the domain layer
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpContract.kt (2)
5-5: Import addition supports OAuth platform tracking.The import for
OAuthProviderenables the new platform property in the state.
21-21: Platform property addition enhances OAuth support.The new
platformproperty withOAuthProvider.KAKAOas default is well-designed:
- Enables tracking of the OAuth provider during signup flow
- Maintains backward compatibility with existing Kakao-based flows
- Provides clear typing for OAuth provider selection
- Logically placed within the state data class
app/src/main/java/com/sopt/clody/data/datastore/OAuthDataStoreKeys.kt (1)
5-8: LGTM! Clean DataStore keys implementation.The implementation follows Android DataStore best practices with properly typed preference keys and consistent naming conventions.
app/src/main/java/com/sopt/clody/data/repositoryimpl/AuthRepositoryImpl.kt (1)
26-29: LGTM! Consistent implementation pattern.The new
signUpWithGooglemethod follows the same error handling and response processing pattern as existing methods. The absence of an authorization parameter is appropriate for OAuth flows.app/src/main/java/com/sopt/clody/data/datastore/DataStoreModule.kt (1)
11-22: LGTM! Standard Dagger Hilt module implementation.The module properly provides a singleton
OAuthDataStorewith correct scoping and context injection. Follows Dagger Hilt best practices.app/src/main/java/com/sopt/clody/data/datastore/OAuthDataStore.kt (2)
10-12: LGTM! Good use of DataStore delegation pattern.The implementation correctly uses the preferencesDataStore delegate and properly scopes it to the application context.
32-37: LGTM! Efficient cleanup implementation.Good practice using a single edit transaction to remove both values atomically.
app/src/main/java/com/sopt/clody/presentation/ui/login/GoogleSignInHelper.kt (3)
15-26: LGTM! Well-configured sign-in request.The sign-in request is properly configured with appropriate settings for OAuth flow.
28-42: LGTM! Clean callback-based implementation.Good error handling and proper use of the Google Sign-In API.
44-49: LGTM! Safe token extraction.Good use of
runCatchingfor defensive error handling.app/src/main/java/com/sopt/clody/presentation/ui/splash/SplashViewModel.kt (2)
55-63: LGTM! Cleaner separation of concerns.Good refactoring - determining login state before navigation improves code readability and flow.
79-98: LGTM! Improved data flow.The explicit parameter passing makes the code more predictable and reduces coupling between methods.
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt (2)
194-209: LGTM! Clean result handling.Good centralization of sign-up result handling with appropriate platform-specific cleanup.
180-180: Confirm OAuthProvider.apiValue is defined—no action needed.The
OAuthProviderenum inapp/src/main/java/com/sopt/clody/data/datastore/OAuthProvider.ktis declared as:enum class OAuthProvider(val apiValue: String) { GOOGLE("google"), KAKAO("kakao"), }Therefore, the usage of
OAuthProvider.KAKAO.apiValueon line 180 ofSignUpViewModel.ktis valid.app/src/main/java/com/sopt/clody/presentation/ui/login/LoginScreen.kt (2)
52-62: LGTM! Clean Google Sign-In integration.Good use of the Activity Result API with proper token extraction and intent dispatching.
78-91: LGTM! Well-structured OAuth callbacks.Both OAuth providers are handled appropriately with proper error handling.
app/src/main/java/com/sopt/clody/presentation/ui/login/LoginViewModel.kt (3)
9-11: LGTM! Clean import additions for OAuth functionality.The new imports for
OAuthDataStore,OAuthProvider, andGoogleSignUpRequestDtoproperly support the unified OAuth implementation. The constructor injection ofoauthDataStorefollows proper dependency injection patterns.Also applies to: 32-32
35-35: Good refactoring to unify intent handling.The simplification of the intent channel type and consolidation into a single
LoginOAuthintent improves code maintainability and reduces duplication.Also applies to: 46-46, 50-55
107-135: Review Google authentication flow naming and API call consistency.The
validateGoogleUsermethod (LoginViewModel.kt:107–135) actually invokesauthRepository.signUpWithGoogle, while the Kakao flow invalidateKakaoUser(lines 85–93) usessignIn. This mismatch can lead to confusion about whether Google should be signing up or signing in.• Rename
validateGoogleUserto something likesignUpGoogleUser(or split into separate sign-in/sign-up methods) to reflect the actual API call.
• Consider unifying the naming convention for both providers: e.g.,signInWithKakao/signInWithGoogleandsignUpWithKakao/signUpWithGoogleif both flows exist.
• Revisit error handling on failure: matching"500"or"유저가 없습니다"in the message is brittle. Prefer explicit HTTP status checks or domain-specific exception types for “user not found.”Please verify:
- Is there a dedicated Google sign-in endpoint, or is the sign-up call intended to cover both cases?
- Should user-not-found be surfaced via a 404 or a custom error code instead of a 500?
- Update method names and documentation to clearly distinguish between sign-in and sign-up flows.
| suspend fun saveIdToken(token: String) { | ||
| dataStore.edit { it[OAuthDataStoreKeys.GOOGLE_ID_TOKEN] = token } | ||
| } | ||
|
|
||
| suspend fun getIdToken(): String? { | ||
| return dataStore.data.first()[OAuthDataStoreKeys.GOOGLE_ID_TOKEN] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security concern: Consider encrypting sensitive OAuth tokens.
The ID token is stored in plain text. OAuth tokens are sensitive data that should be encrypted at rest. Consider using EncryptedSharedPreferences or Android Keystore for secure token storage.
For example, you could use EncryptedSharedPreferences:
val masterKey = MasterKey.Builder(context)
.setKeyScheme(MasterKey.KeyScheme.AES256_GCM)
.build()
val encryptedPrefs = EncryptedSharedPreferences.create(
context,
"oauth_encrypted_prefs",
masterKey,
EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
)🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/data/datastore/OAuthDataStore.kt around
lines 14 to 20, the OAuth ID token is currently stored in plain text using
DataStore, which poses a security risk. To fix this, replace the current storage
mechanism with encrypted storage by integrating EncryptedSharedPreferences or
Android Keystore. Initialize a MasterKey and create an
EncryptedSharedPreferences instance, then update the saveIdToken and getIdToken
functions to store and retrieve the token securely using this encrypted
preferences instance.
| is SignUpContract.SignUpSideEffect.NavigateToWebView -> { | ||
| navigateToWebView(effect.url) // ✅ WebView 이동 처리 | ||
| } | ||
| is SignUpContract.SignUpSideEffect.ShowMessage -> {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Address the empty error message handler.
The ShowMessage side effect handler is currently empty, meaning error messages won't be displayed to users. This could result in silent failures and poor user experience.
Consider implementing proper error message display:
-is SignUpContract.SignUpSideEffect.ShowMessage -> {}
+is SignUpContract.SignUpSideEffect.ShowMessage -> {
+ // TODO: Show toast, snackbar, or other user feedback
+ // Example: Toast.makeText(context, effect.message, Toast.LENGTH_SHORT).show()
+}📝 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.
| is SignUpContract.SignUpSideEffect.ShowMessage -> {} | |
| is SignUpContract.SignUpSideEffect.ShowMessage -> { | |
| // TODO: Show toast, snackbar, or other user feedback | |
| // Example: Toast.makeText(context, effect.message, Toast.LENGTH_SHORT).show() | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpScreen.kt
at line 35, the handler for the ShowMessage side effect is empty, so error
messages are not shown to users. Update this handler to display the error
message appropriately, for example by showing a Toast, Snackbar, or dialog with
the message content to ensure users are informed of errors.
| private suspend fun signUp(context: Context) { | ||
| val state = withState(this@SignUpViewModel) { it } | ||
|
|
||
| if (!networkUtil.isNetworkAvailable()) { | ||
| setState { copy(errorMessage = "네트워크 연결을 확인해주세요.") } | ||
| return | ||
| } | ||
|
|
||
| setState { copy(isLoading = true) } | ||
|
|
||
| val platform = oAuthDataStore.getPlatform() | ||
| val fcmToken = fcmTokenProvider.getToken().orEmpty() | ||
|
|
||
| if (platform == OAuthProvider.GOOGLE) { | ||
| val idToken = oAuthDataStore.getIdToken() | ||
| if (idToken.isNullOrBlank()) { | ||
| setState { copy(errorMessage = "Google ID Token이 없습니다.", isLoading = false) } | ||
| return | ||
| } | ||
| setState { copy(isLoading = true) } | ||
|
|
||
| val request = GoogleSignUpRequestDto( | ||
| idToken = idToken, | ||
| platform = "Android", | ||
| name = state.nickname, | ||
| fcmToken = fcmToken, | ||
| ) | ||
|
|
||
| val result = authRepository.signUpWithGoogle(request) | ||
| handleSignUpResult(result, isGoogle = true) | ||
| } else { | ||
| loginSdk.login(context).fold( | ||
| onSuccess = { accessToken -> | ||
| launch { | ||
| val fcm = fcmTokenProvider.getToken().orEmpty() | ||
| val req = SignUpRequestDto("kakao", state.nickname, fcm) | ||
| authRepository.signUp("Bearer ${accessToken.value}", req).fold( | ||
| onSuccess = { | ||
| tokenRepository.setTokens(it.accessToken, it.refreshToken) | ||
| _sideEffects.send(SignUpContract.SignUpSideEffect.NavigateToTimeReminder) | ||
| }, | ||
| onFailure = { | ||
| setState { copy(errorMessage = it.message ?: "알 수 없는 오류") } | ||
| }, | ||
| ) | ||
|
|
||
| setState { copy(isLoading = false) } | ||
| } | ||
| onSuccess = { token -> | ||
| val request = SignUpRequestDto( | ||
| platform = OAuthProvider.KAKAO.apiValue, | ||
| name = state.nickname, | ||
| fcmToken = fcmToken, | ||
| ) | ||
| val result = authRepository.signUp("Bearer ${token.value}", request) | ||
| handleSignUpResult(result, isGoogle = false) | ||
| }, | ||
| onFailure = { | ||
| setState { copy(errorMessage = it.message ?: "로그인 실패", isLoading = false) } | ||
| setState { copy(errorMessage = "로그인에 실패했어요~", isLoading = false) } | ||
| }, | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix error message inconsistency.
The error message at line 188 says "로그인에 실패했어요~" (Login failed) but this is in the sign-up flow. It should say "회원가입에 실패했어요~" (Sign-up failed) for consistency.
- setState { copy(errorMessage = "로그인에 실패했어요~", isLoading = false) }
+ setState { copy(errorMessage = "회원가입에 실패했어요~", isLoading = false) }Otherwise, the refactoring looks good with proper network checking and loading state management.
🤖 Prompt for AI Agents
In
app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt
between lines 147 and 192, update the error message in the onFailure block of
the loginSdk.login call from "로그인에 실패했어요~" to "회원가입에 실패했어요~" to correctly
reflect that the failure occurred during sign-up, ensuring message consistency.
app/src/main/java/com/sopt/clody/presentation/ui/login/GoogleSignInHelper.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/sopt/clody/presentation/ui/login/LoginScreen.kt
Outdated
Show resolved
Hide resolved
| private suspend fun handleLoginOAuth(intent: LoginIntent.LoginOAuth) { | ||
| setState { copy(isLoading = true, errorMessage = null) } | ||
|
|
||
| loginSdk.login(context).fold( | ||
| onSuccess = { accessToken -> | ||
| validateUser("Bearer ${accessToken.value}") | ||
| when (intent.provider) { | ||
| OAuthProvider.KAKAO -> { | ||
| loginSdk.login(intent.context!!).fold( | ||
| onSuccess = { token -> | ||
| validateKakaoUser(token.value) | ||
| }, | ||
| onFailure = { error -> | ||
| setState { copy(isLoading = false) } | ||
| _sideEffects.send(LoginContract.LoginSideEffect.ShowError("로그인에 실패했습니다")) | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
| OAuthProvider.GOOGLE -> { | ||
| val idToken = intent.idToken | ||
| if (idToken.isNullOrBlank()) { | ||
| _sideEffects.send(LoginContract.LoginSideEffect.ShowError("로그인에 실패했습니다.")) | ||
| return | ||
| } | ||
|
|
||
| validateGoogleUser(idToken) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Address potential null pointer exception with context handling.
While the unified OAuth handling approach is well-structured, there's a critical issue with force unwrapping the context:
loginSdk.login(intent.context!!) // Line 62 - potential crashConsider safe handling of the context:
- loginSdk.login(intent.context!!).fold(
+ val context = intent.context
+ if (context == null) {
+ setState { copy(isLoading = false) }
+ _sideEffects.send(LoginContract.LoginSideEffect.ShowError("로그인에 실패했습니다"))
+ return
+ }
+ loginSdk.login(context).fold(Additionally, consider extracting hardcoded error messages to string resources for better localization support.
🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/presentation/ui/login/LoginViewModel.kt
between lines 57 and 83, the code force unwraps intent.context with '!!' which
can cause a null pointer exception if context is null. To fix this, safely check
if intent.context is null before calling loginSdk.login, and handle the null
case gracefully, such as sending an error side effect and returning early. Also,
replace hardcoded error message strings with references to string resources to
support localization.
| androidxCredentials = "1.5.0" | ||
| googleAuth = "21.3.0" | ||
| datastore = "1.1.7" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the dependency versions are current and compatible.
The added OAuth and DataStore dependency versions should be validated to ensure they're the latest stable versions and are compatible with the existing AndroidX and Compose ecosystem.
🌐 Web query:
What are the latest stable versions of androidx.credentials:credentials-play-services-auth, com.google.android.gms:play-services-auth, and androidx.datastore:datastore-preferences as of January 2025?
💡 Result:
As of January 2025, the latest stable versions of the requested libraries are:
- androidx.credentials:credentials-play-services-auth: 1.5.0 (released March 12, 2025)[1][5].
- com.google.android.gms:play-services-auth: 21.3.0 (released December 9, 2024)[6].
- androidx.datastore:datastore-preferences: 1.1.2 (released January 15, 2025)[3][7].
Supporting details:
- For androidx.credentials:credentials-play-services-auth, version 1.5.0 is the latest stable, with further 1.6.x versions available only as alpha releases as of June–July 2025[1][5][8].
- For com.google.android.gms:play-services-auth, 21.3.0 is marked as the most recent stable release prior to January 2025[6]. Documentation does not indicate any subsequent releases up to and through January 2025.
- For androidx.datastore:datastore-preferences, 1.1.2 is the latest stable version, with 1.1.3 in development (but not yet generally released) as of January 2025[3][7].
If your build targets January 2025, the versions above are considered latest stable and production-ready.
Citations:
- 1: https://developer.android.com/jetpack/androidx/releases/credentials
- 2: https://developer.android.com/games/pgs/migration_overview
- 3: https://developer.android.com/jetpack/androidx/releases/datastore
- 4: https://mvnrepository.com/artifact/androidx.credentials/credentials-play-services-auth/1.5.0-alpha04
- 5: https://mvnrepository.com/artifact/androidx.credentials/credentials-play-services-auth
- 6: https://mvnrepository.com/artifact/com.google.android.gms/play-services-auth
- 7: https://mvnrepository.com/artifact/androidx.datastore/datastore-preferences-android/1.1.2
- 8: https://mahozad.ir/androidx-release-notes/
- 9: https://developers.google.com/android/guides/releases
- 10: https://www.apkmirror.com/apk/google-inc/google-play-services/google-play-services-25-02-61-release/google-play-services-25-02-61-100300-717007275-android-apk-download/
Update Datastore Dependency to Latest Stable
Two of the three newly added versions are already the latest stable releases, but the DataStore entry needs correction:
• androidxCredentials = "1.5.0" ⇒ ✅ matches the current stable (released Mar 12, 2025)
• googleAuth = "21.3.0" ⇒ ✅ matches the current stable (released Dec 9, 2024)
• datastore = "1.1.7" ⇒ ❌ no stable 1.1.7 exists; the latest GA is 1.1.2 (released Jan 15, 2025)
Please update your gradle/libs.versions.toml to:
datastore = "1.1.2"
If you actually need an upcoming or alpha DataStore version, explicitly reference that pre-release coordinate instead of 1.1.7.
🤖 Prompt for AI Agents
In gradle/libs.versions.toml at lines 56 to 58, the datastore version is
incorrectly set to 1.1.7, which is not a stable release. Update the datastore
dependency version to the latest stable release by changing it to 1.1.2. If a
pre-release or alpha version is needed, specify that explicitly instead of using
1.1.7.
로그인 API 요청 시 불필요한 platform, name 값 제거 GoogleSignUpRequestDto의 platform, name 제거
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/sopt/clody/data/remote/dto/request/GoogleSignUpRequestDto.kt (1)
6-10: Well-structured DTO with proper serialization setup.The data class follows Kotlin best practices:
- Immutable properties with
val- Proper kotlinx serialization annotations
- Clean, focused design for the Google sign-up request payload
Consider adding KDoc documentation to clarify the purpose and sensitivity of the tokens:
+/** + * Request DTO for Google OAuth2 sign-up. + * @param idToken Google ID token obtained from OAuth2 flow + * @param fcmToken Firebase Cloud Messaging token for push notifications + */ @Serializable data class GoogleSignUpRequestDto(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/java/com/sopt/clody/data/remote/dto/request/GoogleSignUpRequestDto.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt(3 hunks)app/src/main/java/com/sopt/clody/presentation/ui/login/GoogleSignInHelper.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/login/LoginContract.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/login/LoginScreen.kt(7 hunks)app/src/main/java/com/sopt/clody/presentation/ui/login/LoginViewModel.kt(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/src/main/java/com/sopt/clody/presentation/ui/login/GoogleSignInHelper.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/main/java/com/sopt/clody/presentation/ui/auth/signup/SignUpViewModel.kt
- app/src/main/java/com/sopt/clody/presentation/ui/login/LoginContract.kt
- app/src/main/java/com/sopt/clody/presentation/ui/login/LoginViewModel.kt
- app/src/main/java/com/sopt/clody/presentation/ui/login/LoginScreen.kt
🔇 Additional comments (1)
app/src/main/java/com/sopt/clody/data/remote/dto/request/GoogleSignUpRequestDto.kt (1)
1-4: LGTM! Clean package structure and minimal imports.The package follows Android conventions and imports are exactly what's needed for kotlinx serialization.
| private fun attemptAutoLogin(): Boolean { | ||
| val isLoggedIn = tokenRepository.getAccessToken().isNotBlank() && | ||
| tokenRepository.getRefreshToken().isNotBlank() | ||
| setState { copy(isUserLoggedIn = isLoggedIn) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[p3]
현재 로직 수정으로 인해 SplashContract.State.isLoggedIn 부분이 필요없어진 것 같습니다. 확인부탁드립니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[p5]
이제 스플래시 뷰모델 로직은
디버그/릴리즈 버전 체크 && 점검시간 체크 -> 토큰 유뮤로 자동 로그인 체크 -> 앱 버전 업데이트 체크 -> 최신 버전이고 자동 로그인 가능하면 홈 아니면 로그인 화면으로 이동이겠네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[p4]
preferences 기반 토큰 저장 방식이 아닌 preferences datastore 로 사용한 것 같은데, 추후에 카카오 로그인도 이것과 합치나요? 동일하게 preferences가 아닌 해당 방식을 채택한 이유가 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음. SharedPreferences는 Sync I/O 기반이라 UI thread를 막을 수 있어서 구조적으로 위험할 수 잇죠.
반면, DataStore는 Coroutine 기반으로 설계되어 IO 안정성이 좋습니다.
그래서 공식문서에서도 장기적으로 Datastore을 지향하고 있슴다.
카카오같은 경우는 로그인/회원가입에 loginsdk를 통해서 auth token을 발급하는 구조라 구글 로그인과는 조금 다를 수 있습니다.
구글 로그인은 요청하면 또 이메일 선택 UI가 뜨기 때문에 로컬에 캐싱했다가 회원가입 때 쏘는 구조로 했슴다이.
- LanguageProvider를 통해 로그인 타입 결정 - LanguageProviderImpl에서 시스템 언어 설정에 따라 로그인 타입 반환
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/src/main/java/com/sopt/clody/presentation/utils/language/LanguageProviderImpl.kt (1)
18-23: LGTM: Clean refactor to when expression with OAuth provider mapping.The implementation correctly maps Korean locale to
KAKAOand defaults towhenexpression is more idiomatic than the previous if-else structure.Consider market-specific OAuth provider preferences: While the current logic is reasonable, you may want to consider if other regional markets have preferred OAuth providers (e.g., specific providers for other Asian markets) as the application scales internationally.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/sopt/clody/presentation/ui/login/LoginViewModel.kt(2 hunks)app/src/main/java/com/sopt/clody/presentation/utils/language/LanguageProvider.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/utils/language/LanguageProviderImpl.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/sopt/clody/presentation/ui/login/LoginViewModel.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
app/src/main/java/com/sopt/clody/presentation/utils/language/LanguageProvider.kt (2)
3-3: LGTM: Clean import update for OAuth refactor.The import statement correctly reflects the transition from
LoginTypetoOAuthProvideras part of the OAuth consolidation effort.
7-7: LGTM: Interface method signature updated correctly.The method signature change from
LoginTypetoOAuthProvidermaintains the same contract semantics while aligning with the new OAuth provider abstraction.app/src/main/java/com/sopt/clody/presentation/utils/language/LanguageProviderImpl.kt (1)
4-4: LGTM: Import updated consistently with interface changes.The import statement correctly reflects the transition to
OAuthProviderenum, maintaining consistency with the interface changes.
- UserInfoResponseDto의 platform 타입을 OAuthProvider로 변경 - AccountManagementViewModel에서 platform Enum 처리 - AccountManagementLogoutOption에서 로그인 플랫폼에 따라 아이콘을 다르게 표시
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/sopt/clody/presentation/ui/setting/component/AccountManagementLogoutOption.kt (1)
28-32: Improve fallback handling for unknown platforms.The current implementation defaults to Google icon for both
elsecase, which might be confusing. Consider using a generic OAuth icon or a question mark icon for unknown platforms.val platformIconRes = when (platform) { OAuthProvider.KAKAO -> R.drawable.img_account_management_kakao OAuthProvider.GOOGLE -> R.drawable.img_google_button_logo - else -> R.drawable.img_google_button_logo // 서버에서 google을 어떻게 내려줄까요? + null -> R.drawable.ic_unknown_platform // Use a generic icon for unknown platforms }Also, the Korean comment suggests uncertainty about server behavior. Consider coordinating with the backend team to clarify the expected platform values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/sopt/clody/data/remote/dto/response/UserInfoResponseDto.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/setting/component/AccountManagementLogoutOption.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/setting/screen/AccountManagementScreen.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/setting/screen/AccountManagementViewModel.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
app/src/main/java/com/sopt/clody/presentation/ui/setting/screen/AccountManagementScreen.kt (1)
app/src/main/java/com/sopt/clody/presentation/ui/setting/component/AccountManagementLogoutOption.kt (1)
AccountManagementLogoutOption(22-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
app/src/main/java/com/sopt/clody/presentation/ui/setting/screen/AccountManagementScreen.kt (1)
138-142: LGTM! Improved consistency by always showing logout option.The removal of conditional rendering based on platform string and addition of the
platformparameter improves code consistency. Now the logout option is always displayed with proper platform-specific icon handling delegated to the component itself.app/src/main/java/com/sopt/clody/data/remote/dto/response/UserInfoResponseDto.kt (2)
3-3: Import added for OAuthProvider enum.Good addition to support the new type-safe platform representation.
11-11: Null-safety of OAuthProvider? verified
The only consumer ofplatformisAccountManagementLogoutOption(AccountManagementLogoutOption.kt), where thewhen(platform)expression covers bothnulland unknown cases with theelsebranch. No other code dereferencesplatform, so there’s no risk of an NPE.If defaulting to the Google icon when
platformisnullis your intended UX, you’re all set—no further changes required.app/src/main/java/com/sopt/clody/presentation/ui/setting/component/AccountManagementLogoutOption.kt (4)
19-19: Import added for OAuthProvider.Good addition to support the new parameter type.
25-25: Parameter type improved with nullable OAuthProvider.The nullable
OAuthProvider?type properly handles cases where platform information might be unavailable.
35-35: LGTM! Padding modifier simplified.The padding declaration is more concise while maintaining the same spacing values.
38-39: Dynamic icon selection implemented correctly.The image now uses the dynamically selected platform icon resource, which properly supports multiple OAuth providers.
| val platformEnum = it.platform | ||
| retryCount = 0 | ||
| UserInfoState.Success(it) | ||
| UserInfoState.Success(it.copy(platform = platformEnum)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Simplify redundant platform assignment.
The code extracts platformEnum = it.platform and then creates a copy with platform = platformEnum, which is redundant since the platform is already of the correct type. This can be simplified.
- val platformEnum = it.platform
retryCount = 0
- UserInfoState.Success(it.copy(platform = platformEnum))
+ UserInfoState.Success(it)Or if there's a specific reason for the copy operation, please clarify in a comment.
🤖 Prompt for AI Agents
In
app/src/main/java/com/sopt/clody/presentation/ui/setting/screen/AccountManagementViewModel.kt
around lines 70 to 72, the code redundantly assigns platformEnum = it.platform
and then copies it with platform = platformEnum, which is unnecessary since the
platform is already correct. Simplify by removing the intermediate platformEnum
variable and the copy operation, directly using it as needed. If the copy is
required for some reason, add a clarifying comment explaining why.
📌 ISSUE
closed #292
📄 Work Description
구글 로그인 구현
✨ PR Point
📸 ScreenShot/Video
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores