-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/#155 google login #159
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
Walkthrough๊ตฌ๊ธ ์น ํด๋ผ์ด์ธํธ ID๋ฅผ ๋น๋ ์ค์ ์ ์ถ๊ฐํ๊ณ , AndroidX CredentialManager ๊ธฐ๋ฐ์ GoogleLoginHelper์ OAuth UI/๋ทฐ๋ชจ๋ธ ํ์ฅ์ ํตํด ๊ธฐ์กด ์นด์นด์ค ๋ก๊ทธ์ธ์ ๊ตฌ๊ธ ๋ก๊ทธ์ธ์ ํตํฉํ์ต๋๋ค. ProGuard ๊ท์น๊ณผ ๋ผ์ด๋ธ๋ฌ๋ฆฌ ์์กด์ฑ๋ ์ถ๊ฐ๋์์ต๋๋ค. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant OauthScreen
participant OauthViewModel
participant GoogleLoginHelper
participant CredentialManager
participant Backend
User->>OauthScreen: ๊ตฌ๊ธ ๋ก๊ทธ์ธ ํด๋ฆญ
OauthScreen->>OauthViewModel: GoogleLoginClick ๋ฐ์ก
OauthViewModel->>OauthScreen: StartGoogleLogin ์ฌ์ด๋์ดํํธ (๋๋ ์ด๋ฒคํธ)
OauthScreen->>GoogleLoginHelper: requestGoogleLogin(onSuccess,onFailure)
GoogleLoginHelper->>CredentialManager: GetCredentialRequest (webClientId ํฌํจ)
CredentialManager->>User: ๊ตฌ๊ธ ๊ณ์ ์ ํ UI ํ์
User->>CredentialManager: ๊ณ์ ์ ํ
CredentialManager->>GoogleLoginHelper: GoogleIdTokenCredential ๋ฐํ
GoogleLoginHelper->>OauthScreen: onSuccess(idToken)
OauthScreen->>OauthViewModel: GoogleLoginSuccess(provider="GOOGLE", accessToken=idToken)
OauthViewModel->>Backend: postSocialLogin(provider, accessToken)
Backend->>OauthViewModel: JWT ์๋ต
OauthViewModel->>OauthViewModel: SaveJwtToken ์ฒ๋ฆฌ
Estimated code review effort๐ฏ 3 (Moderate) | โฑ๏ธ ~20 ๋ถ Possibly related PRs
Suggested labels
Suggested reviewers
์ ์ฒด ์ค๋ช๊ตฌ๊ธ ๋ก๊ทธ์ธ ๊ธฐ๋ฅ์ ์ถ๊ฐํฉ๋๋ค. ๊ตฌ๊ธ ์น ํด๋ผ์ด์ธํธ ID๋ฅผ ๋น๋ ์ค์ ์ ์ถ๊ฐํ๊ณ , AndroidX CredentialManager๋ฅผ ํ์ฉํ GoogleLoginHelper ํด๋์ค๋ฅผ ๊ตฌํํ์ฌ ๊ตฌ๊ธ ์ธ์ฆ ํ ํฐ์ ํ๋ํฉ๋๋ค. OAuth ํ๋ฉด๊ณผ ๋ทฐ๋ชจ๋ธ์ ํ์ฅํด ๊ธฐ์กด ์นด์นด์ค ๋ก๊ทธ์ธ๊ณผ ํจ๊ป ๊ตฌ๊ธ ๋ก๊ทธ์ธ ํ๋ฆ์ ํตํฉํฉ๋๋ค. ๋ณ๊ฒฝ์ฌํญ
์ํ์ค ๋ค์ด์ด๊ทธ๋จsequenceDiagram
actor User
participant OauthScreen
participant OauthViewModel
participant GoogleLoginHelper
participant CredentialManager
participant Backend
User->>OauthScreen: ๊ตฌ๊ธ ๋ก๊ทธ์ธ ํด๋ฆญ
OauthScreen->>OauthViewModel: GoogleLoginClick ๋ฐ์ก
OauthViewModel->>OauthViewModel: StartGoogleLogin ์ฌ์ด๋์ดํํธ ์์ฑ
OauthScreen->>GoogleLoginHelper: requestGoogleLogin ํธ์ถ
GoogleLoginHelper->>CredentialManager: ์๊ฒฉ์ฆ๋ช
์์ฒญ
CredentialManager->>User: ๊ตฌ๊ธ ๊ณ์ ์ ํ UI
User->>CredentialManager: ๊ณ์ ์ ํ
CredentialManager->>GoogleLoginHelper: GoogleIdTokenCredential ๋ฐํ
GoogleLoginHelper->>OauthScreen: onSuccess (ID ํ ํฐ)
OauthScreen->>OauthViewModel: GoogleLoginSuccess ๋ฐ์ก
OauthViewModel->>Backend: postSocialLogin (provider="GOOGLE", accessToken)
Backend->>OauthViewModel: JWT ํ ํฐ ๋ฐํ
OauthViewModel->>OauthViewModel: SaveJwtToken ๋ฐ์ก
์ฝ๋ ๋ฆฌ๋ทฐ ์์ ๋ ธ๋ ฅ๐ฏ 3 (Moderate) | โฑ๏ธ ~20 ๋ถ
๊ด๋ จ PR ๊ฐ๋ฅ์ฑ
์ ์ ๋ผ๋ฒจ
์ ์ ๊ฒํ ์
์
๐ฅ Pre-merge checks | โ 3 | โ 2โ Failed checks (2 warnings)
โ Passed checks (3 passed)
โ๏ธ Tip: You can configure your own custom pre-merge checks in the settings. โจ Finishing touches
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 |
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: 9
โป๏ธ Duplicate comments (2)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileEditScreen.kt (2)
140-140: ๋์ผํ ์ด์: ๊ณ ์ ๋์ด Spacer ๊ฒ์ฆ ํ์Line 129์ ๋์ผํ๊ฒ, ์ด Spacer๋ weight ๊ธฐ๋ฐ์์ ๊ณ ์ ๋์ด(32.dp)๋ก ๋ณ๊ฒฝ๋์์ต๋๋ค.
168-168: ๋์ผํ ์ด์: ๊ณ ์ ๋์ด Spacer ๊ฒ์ฆ ํ์Line 129, 140๊ณผ ๋์ผํ๊ฒ, ์ด Spacer๋ weight ๊ธฐ๋ฐ์์ ๊ณ ์ ๋์ด(24.dp)๋ก ๋ณ๊ฒฝ๋์์ต๋๋ค.
๐งน Nitpick comments (4)
feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthViewModel.kt (1)
56-76: ๋๋ฒ๊ทธ ๋ก๊ทธ ๋ฉ์์ง๋ฅผ ๊ฐ์ ํ์ธ์.Line 73์ ๋ก๊ทธ ํ๊ทธ
"asdasdasd"๋ ์๋ฏธ ์๋ ๋ฌธ์์ด์ ๋๋ค. ๋๋ฒ๊น ์ ๋ก๊ทธ๋ฅผ ์๋ณํ๊ธฐ ์ด๋ ต์ต๋๋ค.๋ค์๊ณผ ๊ฐ์ด ๊ฐ์ ํ์ธ์:
}.onFailure { - Log.d("asdasdasd", it.toString()) + Log.e("OauthViewModel", "Social login failed: ${it.message}", it) }feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthScreen.kt (1)
50-50: GoogleLoginHelper์ ์๋ช ์ฃผ๊ธฐ ๊ด๋ฆฌ๋ฅผ ๊ณ ๋ คํด๋ณด์ธ์.
GoogleLoginHelper๊ฐremember๋ก ์์ฑ๋์ง๋ง ๋ช ์์ ์ธ ์ ๋ฆฌ ๋ก์ง์ด ์์ต๋๋ค. ํ์ฌ๋ ํฐ ๋ฌธ์ ๊ฐ ์์ด ๋ณด์ด์ง๋ง, ํฅํ ๋ฆฌ์์ค ๊ด๋ฆฌ๊ฐ ํ์ํ ๊ฒฝ์ฐ๋ฅผ ๋๋นํ์ฌDisposableEffect๋ฅผ ์ฌ์ฉํ๋ ๊ฒ์ ๊ณ ๋ คํด๋ณด์ธ์.์์:
val googleLoginHelper = remember { GoogleLoginHelper(context = context) } DisposableEffect(Unit) { onDispose { // Clean up if needed in the future } }feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt (2)
25-25: ์ฃผ์ ์ฒ๋ฆฌ๋ ์ฝ๋๋ฅผ ์ ๋ฆฌํ์ธ์.์ฌ๋ฌ ๊ณณ์ ์ฃผ์ ์ฒ๋ฆฌ๋ ์ฝ๋(SERVER_URL, nonce, ๋ก๊ทธ ๋ฑ)๊ฐ ๋จ์์์ต๋๋ค. ํ์ํ ์ฝ๋๋ ํ์ฑํํ๊ณ , ๋ถํ์ํ ์ฝ๋๋ ์ ๊ฑฐํ์ฌ ์ฝ๋์ ๊ฐ๋ ์ฑ์ ๋์ด๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
์์:
- Line 25:
SERVER_URL์ด ํ์ํ๋ค๋ฉด ํ์ฑํํ๊ณ , ์๋๋ฉด ์ ๊ฑฐ- Lines 69-71: ๋๋ฒ๊ทธ ๋ก๊ทธ๊ฐ ํ์ํ๋ค๋ฉด BuildConfig.DEBUG๋ก ์กฐ๊ฑด๋ถ ํ์ฑํ
- Lines 74, 77, 83: ์๋ฌ ๋ก๊ทธ๋ ํ์ฑํ ๊ถ์ฅ
Also applies to: 33-33, 69-71, 74-74, 77-77, 83-83
24-24: BuildConfig๋ฅผ ์ง์ ์ฐธ์กฐํ๋ ๋์ ์ฃผ์ ๋ฐ๋ ๊ฒ์ ๊ณ ๋ คํด๋ณด์ธ์.ํ์ฌ
WEB_CLIENT_ID๋ฅผ companion object์์ BuildConfig๋ฅผ ์ง์ ์ฐธ์กฐํ๊ณ ์์ต๋๋ค. ํ ์คํธ ์ฉ์ด์ฑ๊ณผ ์์กด์ฑ ๋ช ์๋ฅผ ์ํด ์์ฑ์๋ฅผ ํตํด ์ฃผ์ ๋ฐ๋ ๊ฒ์ ๊ณ ๋ คํด๋ณด์ธ์.์์:
class GoogleLoginHelper( private val context: Context, private val webClientId: String = com.teamsolply.solply.buildconfig.BuildConfig.GOOGLE_WEB_CLIENT_ID ) { // ... private val googleIdOption: GetGoogleIdOption = GetGoogleIdOption.Builder() .setServerClientId(webClientId) // ... }
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
๐ Files selected for processing (12)
app/proguard-rules.pro(1 hunks)build-logic/convention/src/main/java/com/teamsolply/solply/convention/BuildConfig.kt(1 hunks)core/buildconfig/src/main/java/com/teamsolply/solply/buildconfig/impl/BuildConfigFieldsProviderImpl.kt(2 hunks)core/common/src/main/java/com/teamsolply/solply/common/buildconfig/BuildConfigFields.kt(1 hunks)feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileEditScreen.kt(3 hunks)feature/oauth/build.gradle.kts(2 hunks)feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt(1 hunks)feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthContract.kt(2 hunks)feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthScreen.kt(4 hunks)feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthViewModel.kt(1 hunks)feature/oauth/src/main/res/values/strings.xml(1 hunks)gradle/libs.versions.toml(2 hunks)
๐งฐ Additional context used
๐งฌ Code graph analysis (1)
feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthViewModel.kt (4)
core/ui/src/main/java/com/teamsolply/solply/ui/base/BaseViewModel.kt (1)
postSideEffect(48-52)remote/oauth/src/main/java/com/teamsolply/solply/oauth/service/OauthService.kt (2)
postSocialLogin(10-16)postSocialLogin(11-15)data/oauth/src/main/java/com/teamsolply/solply/oauth/repository/OauthRepositoryImpl.kt (1)
postSocialLogin(13-27)domain/oauth/src/main/java/com/teamsolply/solply/oauth/repository/OauthRepository.kt (2)
postSocialLogin(5-8)postSocialLogin(6-6)
๐ช detekt (1.23.8)
feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt
[warning] 73-73: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
๐ Additional comments (8)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileEditScreen.kt (1)
129-130: ๊ณ ์ ๋์ด๋ก ๋ณ๊ฒฝ๋ Spacer์ ๋ฐ์ํ ๋์์ ํ์ธํ์ธ์.์ด Spacer๊ฐ weight ๊ธฐ๋ฐ์์ ๊ณ ์ ๋์ด(16.dp)๋ก ๋ณ๊ฒฝ๋์์ต๋๋ค. ๊ณ ์ ๋์ด๋ ๋ค์ํ ํ๋ฉด ํฌ๊ธฐ์์ ์ ์ฐํ๊ฒ ๋์ํ์ง ๋ชปํ ์ ์์ต๋๋ค. ํนํ ์์ ํ๋ฉด์์๋ ๋ ์ด์์์ด ์๋ฆฌ๊ฑฐ๋ ์๋ํ์ง ์์ ์คํฌ๋กค์ด ๋ฐ์ํ ์ ์์ต๋๋ค.
๋ค์ํ ํ๋ฉด ํฌ๊ธฐ(ํนํ ์์ ๊ธฐ๊ธฐ)์์ UI๊ฐ ์ฌ๋ฐ๋ฅด๊ฒ ํ์๋๋์ง ํ์ธํด์ฃผ์ธ์.
feature/oauth/build.gradle.kts (1)
1-17: LGTM! Google Sign-In ์์กด์ฑ์ด ์ฌ๋ฐ๋ฅด๊ฒ ์ถ๊ฐ๋์์ต๋๋ค.Google Sign-In ํตํฉ์ ํ์ํ Credential Manager API ๋ผ์ด๋ธ๋ฌ๋ฆฌ๋ค(
google.id,credentials,credentials.play.auth)๊ณผ BuildConfig ์ ๊ทผ์ ์ํ ์์กด์ฑ์ด ์ ์ ํ๊ฒ ์ถ๊ฐ๋์์ต๋๋ค.feature/oauth/src/main/res/values/strings.xml (1)
5-5: PR ๋ชฉํ์์ ๋ถ์ผ์น ํ์ธ์ด ํ์ํฉ๋๋ค.์ด PR์ ๋ชฉํ๋ ๊ตฌ๊ธ ๋ก๊ทธ์ธ ๊ตฌํ์ธ๋ฐ,
apple_login๋ฌธ์์ด ๋ฆฌ์์ค๊ฐ ์ถ๊ฐ๋์์ต๋๋ค. Apple ๋ก๊ทธ์ธ์ด ์ด๋ฒ PR์ ๋ฒ์์ ํฌํจ๋๋ ๊ฒ์ด ๋ง๋์ง ํ์ธ์ด ํ์ํฉ๋๋ค.feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthContract.kt (2)
15-17: LGTM! Google ๋ก๊ทธ์ธ Intent๊ฐ ์ผ๊ด์ฑ ์๊ฒ ์ถ๊ฐ๋์์ต๋๋ค.๊ธฐ์กด Kakao ๋ก๊ทธ์ธ ํจํด์ ๋ฐ๋ผ Google ๋ก๊ทธ์ธ Intent๋ค์ด ์ ์ ํ๊ฒ ์ถ๊ฐ๋์์ต๋๋ค. Click -> Success/Failure ํ๋ฆ์ด ๋ช ํํฉ๋๋ค.
27-27: LGTM! Google ๋ก๊ทธ์ธ SideEffect๊ฐ ์ถ๊ฐ๋์์ต๋๋ค.Kakao ๋ก๊ทธ์ธ๊ณผ ๋์ผํ ํจํด์ผ๋ก
StartGoogleLoginSideEffect๊ฐ ์ถ๊ฐ๋์ด ์ผ๊ด์ฑ์ด ์ ์ง๋ฉ๋๋ค.core/common/src/main/java/com/teamsolply/solply/common/buildconfig/BuildConfigFields.kt (1)
6-6: LGTM! BuildConfig ํ๋๊ฐ ์ฌ๋ฐ๋ฅด๊ฒ ์ถ๊ฐ๋์์ต๋๋ค.
googleWebClientIdํ๋๊ฐ data class์ ์ ์ ํ๊ฒ ์ถ๊ฐ๋์์ต๋๋ค.build-logic/convention/src/main/java/com/teamsolply/solply/convention/BuildConfig.kt (1)
37-41: ํ๋ ์ด๋ฆ์ ์คํ๋ฅผ ์์ ํ์ธ์.BuildConfig ํ๋ ์ด๋ฆ์ ์คํ๊ฐ ์์ต๋๋ค:
GOOGLe_WEB_CLIENT_ID์e๊ฐ ์๋ฌธ์์ ๋๋ค.GOOGLE_WEB_CLIENT_ID๋ก ์์ ํด์ผ ํฉ๋๋ค.๋ค์ diff๋ฅผ ์ ์ฉํ์ธ์:
buildConfigField( "String", - "GOOGLE_WEB_CLIENT_ID", + "GOOGLE_WEB_CLIENT_ID", gradleLocalProperties(rootDir, providers).getProperty("google.web.client.id") )Likely an incorrect or invalid review comment.
core/buildconfig/src/main/java/com/teamsolply/solply/buildconfig/impl/BuildConfigFieldsProviderImpl.kt (1)
4-4: LGTM! Google Web Client ID๊ฐ ์ฌ๋ฐ๋ฅด๊ฒ ๊ตฌ์ฑ๋์์ต๋๋ค.BuildConfig์์
GOOGLE_WEB_CLIENT_ID๋ฅผ ๊ฐ์ ธ์BuildConfigFields์ ์ ์์ ์ผ๋ก ์ ๋ฌํ๊ณ ์์ต๋๋ค.Also applies to: 18-18
| -keep class * extends com.google.gson.TypeAdapter | ||
| -keep class com.google.googlesignin.** { *; } |
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.
๐งฉ Analysis chain
ProGuard ๊ท์น์ด ์ง๋์น๊ฒ ๊ด๋ฒ์ํ ์ ์์ต๋๋ค.
ํ์ฌ ๊ท์น์ด ๋งค์ฐ ๋์ ๋ฒ์๋ฅผ ์ ์งํ๊ณ ์์ต๋๋ค:
* extends TypeAdapter: ๋ชจ๋ TypeAdapter ์๋ธํด๋์ค ์ ์งcom.google.googlesignin.** { *; }: ๋ชจ๋ Google Sign-In ํด๋์ค์ ๋ฉค๋ฒ ์ ์ง
์ด๋ ๋๋ ํ๋ฅผ ๋ฐฉํดํ๊ณ APK ํฌ๊ธฐ๋ฅผ ์ฆ๊ฐ์ํฌ ์ ์์ต๋๋ค. Google Sign-In Credential Manager API๋ฅผ ์ฌ์ฉํ๋ ๊ฒฝ์ฐ, ๋ ๊ตฌ์ฒด์ ์ธ ๊ท์น์ผ๋ก ์ถฉ๋ถํ ์ ์์ต๋๋ค.
๋ค์ ์คํฌ๋ฆฝํธ๋ก ์ค์ ์ฌ์ฉ๋๋ Google Sign-In ํด๋์ค๋ฅผ ํ์ธํ์ธ์:
๐ Script executed:
#!/bin/bash
# Google Sign-In ๊ด๋ จ import ๋ฐ ์ฌ์ฉ ํ์ธ
rg -nP --type=kotlin -C3 'import.*google\.android\.gms\.auth|import.*googleid' feature/oauth/Length of output: 1223
๐ Script executed:
#!/bin/bash
# Search for actual usage of com.google.googlesignin package
rg -nP --type=kotlin --type=java 'com\.google\.googlesignin' .Length of output: 0
๐ Script executed:
#!/bin/bash
# Search for Gson TypeAdapter usage
rg -nP --type=kotlin --type=java 'TypeAdapter' .Length of output: 0
๐ Script executed:
#!/bin/bash
# Check if Credential Manager classes appear in ProGuard rules already
cat app/proguard-rules.proLength of output: 847
๐ Script executed:
#!/bin/bash
# Check build.gradle for dependencies
fd -e gradle | head -20Length of output: 0
๐ Script executed:
#!/bin/bash
# Search build files for google-signin or gson dependencies
rg -nP 'google.*signin|gson' build.gradleLength of output: 52
๐ Script executed:
#!/bin/bash
# Search for any Credential Manager ProGuard rules or missing rules
rg -nP 'credentials|credential' app/proguard-rules.proLength of output: 0
์ฌ์ฉ๋์ง ์๋ ProGuard ๊ท์น์ ์ ๊ฑฐํ์ธ์.
๊ฒ์ฆ ๊ฒฐ๊ณผ, ์ฝ๋๋ฒ ์ด์ค ์ด๋์์๋ ๋ค์์ด ์ฌ์ฉ๋์ง ์์ต๋๋ค:
com.google.googlesigninํจํค์ง (๊ฒ์ ๊ฒฐ๊ณผ: 0๊ฑด)TypeAdapterํด๋์ค (๊ฒ์ ๊ฒฐ๊ณผ: 0๊ฑด)
ํ์ฌ ๊ตฌํ์ com.google.android.libraries.identity.googleid (Credential Manager API)๋ฅผ ์ฌ์ฉํ๊ณ ์์ต๋๋ค. ๋ผ์ธ 23-24์ ๊ท์น์ ๋ ์ด์ ํ์ํ์ง ์์ ๋ ๊ฑฐ์ Google Sign-In SDK ๊ด๋ จ ๊ท์น์
๋๋ค.
์ด ๊ท์น๋ค์ ์ ๊ฑฐํ๋ฉด APK ํฌ๊ธฐ๋ฅผ ์ค์ด๊ณ ์ ์ ํ ๋๋ ํ๊ฐ ๊ฐ๋ฅํฉ๋๋ค:
-keep class * extends com.google.gson.TypeAdapter
-keep class com.google.googlesignin.** { *; }
๐ค Prompt for AI Agents
In app/proguard-rules.pro around lines 23 to 24, remove the two unused legacy
ProGuard rules for Google Sign-In and Gson TypeAdapter (the lines keeping
com.google.googlesignin.** and classes extending com.google.gson.TypeAdapter);
delete those two lines, run a clean build to confirm no missing symbol or
runtime obfuscation issues, and if build/tests pass, commit the updated proguard
file.
| private val nonce = UUID.randomUUID().toString() | ||
| private val googleIdOption: GetGoogleIdOption = GetGoogleIdOption.Builder() | ||
| .setServerClientId(WEB_CLIENT_ID) // ์น ํด๋ผ์ด์ธํธ ID | ||
| .setFilterByAuthorizedAccounts(false) | ||
| // .setNonce(nonce) | ||
| .build() |
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.
๐งฉ Analysis chain
Nonce ์ฌ์ฉ์ด ๊ถ์ฅ๋ฉ๋๋ค.
Line 33์์ setNonce(nonce)๊ฐ ์ฃผ์ ์ฒ๋ฆฌ๋์ด ์์ต๋๋ค. Nonce๋ ์ฌ์ ๊ณต๊ฒฉ(replay attack)์ ๋ฐฉ์งํ๋ ์ค์ํ ๋ณด์ ๋ฉ์ปค๋์ฆ์
๋๋ค. ํน๋ณํ ์ด์ ๊ฐ ์๋ค๋ฉด ํ์ฑํํ๋ ๊ฒ์ด ์ข์ต๋๋ค.
Nonce๋ฅผ ํ์ฑํํ์ธ์:
private val googleIdOption: GetGoogleIdOption = GetGoogleIdOption.Builder()
.setServerClientId(WEB_CLIENT_ID) // ์น ํด๋ผ์ด์ธํธ ID
.setFilterByAuthorizedAccounts(false)
-// .setNonce(nonce)
+ .setNonce(nonce)
.build()์๋ฒ ์ธก์์๋ ์ด nonce๋ฅผ ๊ฒ์ฆํ๋๋ก ๊ตฌํํด์ผ ํฉ๋๋ค.
์ด ๋ฆฌ๋ทฐ ์ฝ๋ฉํธ๋ฅผ ๊ฒ์ฆํ๊ธฐ ์ํด ์ ์ฅ์๋ฅผ ๋ถ์ํ๊ณ ์์ต๋๋ค. ์คํฌ๋ฆฝํธ๋ฅผ ์คํ ์ค์ ๋๋ค.
๐ Script executed:
#!/bin/bash
# Search for the GoogleLoginHelper file and examine its content
find . -name "GoogleLoginHelper.kt" -type f 2>/dev/null | head -5Length of output: 78
๐ Script executed:
#!/bin/bash
# Search for GetGoogleIdOption imports and usage patterns
rg -n "GetGoogleIdOption" --type kotlin -B3 -A3Length of output: 1907
๐ Script executed:
#!/bin/bash
# Search for nonce-related code and comments in the OAuth module
rg -n "nonce" --type kotlin -i -B2 -A2Length of output: 1167
๐ Script executed:
#!/bin/bash
# Search for server-side validation of nonce
rg -n "nonce" --type java --type kotlin -B2 -A2Length of output: 1167
๐ Script executed:
#!/bin/bash
# Check for build.gradle or pom.xml to identify Google Sign-In library version
find . -name "build.gradle*" -o -name "pom.xml" | xargs grep -l "google\|gms" 2>/dev/null | head -5Length of output: 96
๐ Script executed:
#!/bin/bash
# Search for any comments or documentation explaining why nonce was commented out
rg -n "replay\|setNonce" --type kotlin --type java -B5 -A5Length of output: 0
๐ Script executed:
#!/bin/bash
# Look for server-side token validation logic
rg -n "verify|validate" --type kotlin --type java -i | grep -i "token\|nonce\|jwt\|google" | head -10Length of output: 0
๐ Script executed:
#!/bin/bash
# Check Google Play Services dependency version
find . -name "build.gradle*" | xargs cat 2>/dev/null | grep -A2 -B2 "play.*services.*auth"Length of output: 0
๐ Script executed:
#!/bin/bash
# Search for any authentication verification controller or service
fd -e "\.kt$" -e "\.java$" | xargs grep -l "GoogleIdToken\|idToken\|verify" | head -10Length of output: 0
๐ Script executed:
#!/bin/bash
# Look at the full GoogleLoginHelper.kt file
find . -name "GoogleLoginHelper.kt" -type f 2>/dev/null -exec cat {} \;Length of output: 3518
๐ Script executed:
#!/bin/bash
# Search for server endpoint that might validate Google tokens
rg -n "google.*token\|id.*token" --type kotlin --type java -i | head -15Length of output: 0
๐ Script executed:
#!/bin/bash
# Check if there's documentation or configuration about security settings
find . -name "*.md" -o -name "*.txt" | xargs grep -l "nonce\|security" 2>/dev/null | head -5Length of output: 0
๐ Script executed:
#!/bin/bash
# Search for any existing issue tracker references or TODOs related to nonce
rg -n "TODO|FIXME|XXX" --type kotlin | grep -i "nonce\|security\|replay"Length of output: 0
๐ Script executed:
#!/bin/bash
# Look for Google Sign-In implementation details
rg -n "credentialResponse\|credential" --type kotlin -B3 -A3 | head -30Length of output: 0
๐ Script executed:
#!/bin/bash
# Check for any security-related tests or validation logic
fd -e "Test.kt" -e "Test.java" | xargs grep -l "nonce\|GoogleLogin" 2>/dev/nullLength of output: 0
๐ Script executed:
#!/bin/bash
# Look for the server-side implementation that receives the token
rg -n "id_token\|idToken" --type kotlin --type java | head -15Length of output: 0
๐ Script executed:
#!/bin/bash
# Search for GoogleIdTokenVerifier or similar verification
rg -n "Verifier\|Verify" --type java --type kotlin | grep -i googleLength of output: 0
๐ Script executed:
#!/bin/bash
# Check application properties or configuration for security settings
find . -name "application*.yml" -o -name "application*.properties" | xargs cat 2>/dev/null | grep -i nonceLength of output: 0
๐ Script executed:
#!/bin/bash
# Look at git history to see if nonce was commented intentionally
git log --oneline feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt 2>/dev/null | head -10Length of output: 104
๐ Script executed:
#!/bin/bash
# Check for any existing nonce validation on the backend
rg -n "setNonce\|getNonce\|validateNonce" --type java --type kotlinLength of output: 0
๐ Script executed:
#!/bin/bash
# Look for any comments or documentation files
find . -path "*oauth*" -name "*.md" -o -path "*oauth*" -name "README*" 2>/dev/nullLength of output: 0
๐ Script executed:
#!/bin/bash
# Get broader context about the OAuth implementation
find . -path "*oauth*" -type f \( -name "*.kt" -o -name "*.java" \) | head -20Length of output: 1603
๐ Script executed:
#!/bin/bash
# Check for ID token validation on server
rg -n "verify.*IdToken\|validateIdToken" --type java --type kotlinLength of output: 0
๐ Script executed:
#!/bin/bash
# Search for Google API client library usage
rg -n "GoogleIdTokenVerifier\|TokenVerifier" --type javaLength of output: 0
๐ Script executed:
#!/bin/bash
# Check if there's any exception handling related to nonce
rg -n "NonceException\|InvalidNonce" --type java --type kotlinLength of output: 0
๐ Script executed:
#!/bin/bash
# Look for the complete token exchange process
rg -n "exchange\|credential.*token" --type kotlin -B2 -A2 | head -30Length of output: 0
๐ Script executed:
#!/bin/bash
# Search for server endpoint path that handles token verification
rg -n "@PostMapping\|@GetMapping\|@PutMapping" --type kotlin | grep -i "token\|auth\|google\|login"Length of output: 0
๐ Script executed:
#!/bin/bash
# Check if nonce is being used elsewhere in the codebase
find . -type f \( -name "*.kt" -o -name "*.java" \) -exec grep -l "nonce" {} \;Length of output: 78
๐ Script executed:
#!/bin/bash
# Look for documentation on Google Sign-In requirements
find . -name "*.md" -o -name "*.txt" -o -name "*.adoc" 2>/dev/null | xargs grep -l "Google\|OAuth\|Sign-In" 2>/dev/null | head -5Length of output: 0
๐ Script executed:
#!/bin/bash
# Check for any vulnerability scanning or security audit files
find . -name "*.json" -o -name "*.yaml" | xargs grep -l "security\|vulnerability\|nonce" 2>/dev/null | head -5Length of output: 0
๐ Script executed:
#!/bin/bash
# Get imports from GoogleLoginHelper to understand the library version
rg -n "^import" feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt 2>/dev/null || rg -n "^import" . --type kotlin | grep -i google | head -10Length of output: 709
๐ Script executed:
#!/bin/bash
# Check the Google Sign-In Android library documentation reference
cat build.gradle.kts 2>/dev/null || cat build.gradle 2>/dev/null || find . -name "build.gradle*" -type f | head -1 | xargs catLength of output: 637
๐ Script executed:
#!/bin/bash
# Look for any recent commit messages about nonce
git log --all --grep="nonce" --oneline 2>/dev/null || echo "No git history available"Length of output: 0
๐ Script executed:
#!/bin/bash
# Final check: get the exact structure of the GoogleLoginHelper.kt file around the nonce area
find . -name "GoogleLoginHelper.kt" -type f 2>/dev/null -exec head -50 {} \;Length of output: 1954
๐ Script executed:
#!/bin/bash
# Look for server-side Google token verification configuration
find . -name "*.properties" -o -name "*.yml" -o -name "*.yaml" 2>/dev/null | xargs grep -i "google\|client.*id\|secret" 2>/dev/null | head -15Length of output: 0
๐ Script executed:
#!/bin/bash
# Check for Spring Security or other security framework configuration
rg -n "@EnableWebSecurity\|@Configuration\|securityFilterChain" --type java -B2 -A10 | head -40Length of output: 0
๐ Script executed:
#!/bin/bash
# Search for JWT or token parsing logic
rg -n "JwtDecoder\|verifyIdToken\|parseToken" --type java -B2 -A5Length of output: 0
๐ Script executed:
#!/bin/bash
# Look for Google client library initialization
rg -n "GoogleCredential\|GoogleAuthorizationCodeFlow\|GoogleClientSecrets" --type javaLength of output: 0
๐ Script executed:
#!/bin/bash
# Check if nonce validation happens in a filter or interceptor
rg -n "OncePerRequestFilter\|HandlerInterceptor" --type java -B2 -A10Length of output: 0
๐ Script executed:
#!/bin/bash
# Get comprehensive view of OAuth module structure
find feature/oauth -type f 2>/dev/null | head -30Length of output: 525
๐ Script executed:
#!/bin/bash
# Look at documentation or SECURITY.md files
find . -iname "*security*" -type f 2>/dev/null | head -10Length of output: 0
๐ Script executed:
#!/bin/bash
# Check for any architecture decision records or ADRs
find . -iname "*adr*" -o -iname "*decision*" -type f 2>/dev/null | head -10Length of output: 94
๐ Script executed:
#!/bin/bash
# Final comprehensive search for nonce implementation
rg -rn "nonce" --type-list 2>/dev/null | head -5
rg -rn "nonce" . 2>/dev/null | head -20Length of output: 229
๐ Script executed:
#!/bin/bash
# Check if setNonce is available in the GetGoogleIdOption API
rg -n "GetGoogleIdOption" . --type kotlin -A20 2>/dev/null | head -40Length of output: 4825
๐ Script executed:
#!/bin/bash
# Look for version constraints or documentation
cat README.md 2>/dev/null | grep -i "google\|oauth\|security" || echo "No README or no matches"Length of output: 71
๐ Script executed:
#!/bin/bash
# Check all files in the OAuth feature module
ls -la feature/oauth/ 2>/dev/null || find . -path "*oauth*" -type d | head -5Length of output: 289
๐ Script executed:
#!/bin/bash
# Last attempt - find the exact file and show context
find . -name "GoogleLoginHelper.kt" 2>/dev/null -exec echo "=== File: {} ===" \; -exec cat {} \;Length of output: 3611
๐ Script executed:
#!/bin/bash
# Check for Google Play Services version in dependencies
rg -n "com\.google\.android\.gms" . --type kotlin --type java | head -10Length of output: 306
๐ Script executed:
#!/bin/bash
# Look for version strings
rg -n "gms:play-services.*auth" . | head -5Length of output: 0
๐ Script executed:
#!/bin/bash
# Check if there's a docs folder
find . -type d -name "docs" -o -type d -name "documentation" 2>/dev/nullLength of output: 0
๐ Script executed:
#!/bin/bash
# Search for any CHANGELOG or release notes
find . -iname "CHANGELOG*" -o -iname "RELEASE*" 2>/dev/null | xargs grep -i nonce 2>/dev/nullLength of output: 0
๐ Script executed:
#!/bin/bash
# Verify the exact lines mentioned in the review
sed -n '29,34p' feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt 2>/dev/null || echo "File path may differ"Length of output: 278
๐ Web query:
Google Identity Services GetGoogleIdOption setNonce method API documentation
๐ก Result:
setNonce is a method on GetGoogleIdOption.Builder that sets the nonce used when generating a Google ID token. Use it when you build GetGoogleIdOption to have the ID token include the provided nonce.
Example (Kotlin):
val googleIdOption = GetGoogleIdOption.Builder()
.setServerClientId(WEB_CLIENT_ID)
.setNonce(nonce)
.build()
Docs: GetGoogleIdOption.Builder (method: setNonce). [1][2]
Sources
[1] GetGoogleIdOption.Builder โ Google Developers reference.
[2] Authenticate users with Sign in with Google (Android) โ example showing setNonce.
๐ Web query:
androidx.credentials GetGoogleIdOption.Builder setNonce API
๐ก Result:
Yes โ GetGoogleIdOption.Builder has a setNonce(...) method. It accepts a String nonce and sets the nonce used when generating the Google ID token.
Signatures / usage examples:
- Kotlin: .setNonce(nonce: String?) on GetGoogleIdOption.Builder. [1]
- Java: public @nonnull GetGoogleIdOption.Builder setNonce(String nonce). [2]
Example (Kotlin):
val googleIdOption = GetGoogleIdOption.Builder()
.setServerClientId(WEB_CLIENT_ID)
.setNonce(nonce)
.build()
Sources:
[1] Android Developers โ Credential Manager: GetGoogleIdOption.Builder (example).
[2] Google Developers reference โ GetGoogleIdOption.Builder API.
๐ Web query:
Google Sign-In nonce replay attack prevention Android 2024 2025
๐ก Result:
Short answer โ use Googleโs nonce support + server-side single-use verification and device attestation:
Key points and recommended implementation
- Include a cryptographically strong nonce in each Android Signโin with Google request (setNonce / setNonce(nonce)). This nonce is intended to prevent replay attacks. [1][2][3]
- Generate the nonce serverโside or in the app using a CSPRNG (unpredictable), encode URLโsafe Base64, and meet provider rules (recommend โฅ16 bytes). Treat each nonce as singleโuse. Store/hash it serverโside and mark consumed when an ID token/response is accepted. Reject reused or expired nonces. [2][4]
- Verify the ID token on your backend: verify signature, aud/client_id, exp/iat, and that the token contains the same nonce you issued. Only accept tokens whose nonce matches a currently valid, unused nonce. [1][3]
- Optionally harden highโvalue flows by combining nonce with a request hash or a serverโissued unique value and using Play Integrity / SafetyNet attestation (nonce field) to bind the response to the specific request and detect tampering. Check and verify attestation tokens serverโside. [4][5]
- For OAuth code flows, also use PKCE on top of the above; for OIDC ID tokens rely on the nonce claim to prevent replay. [3]
Quick checklist
- Generate CSPRNG nonce (server or app), URLโsafe Base64, โฅ16 bytes. [4]
- Call setNonce(...) when building the Google signโin request on Android. [1][2][6]
- Exchange/receive ID token on server; verify signature, aud, exp, iat, and nonce == stored value. Mark nonce used. [1][3]
- Consider Play Integrity / SafetyNet attestation and request-specific hashes for extra protection. [4][5]
Sources
[1] Google Identity Android docs โ BeginSignInRequest / GetSignInIntentRequest nonce docs.
[2] Android Developer: Authenticate users with Sign in with Google (nonce guidance).
[3] Google Signโin / OpenID Connect guidance (ID token verification / nonce role).
[4] Play Integrity and SafetyNet docs โ using nonce to prevent replay/tampering.
[5] Google Signโin Security bundle / claims (auth_time and additional trust signals).
Nonce ํ์ฑํ ๋ฐ ์๋ฒ ์ธก ๊ฒ์ฆ ๊ตฌํ ํ์
ํด๋ผ์ด์ธํธ: GetGoogleIdOption.Builder์ setNonce() ๋ฉ์๋๊ฐ Google ๊ณต์ API์์ ์ง์๋๋ฉฐ, nonce๋ ์ฌ์ ๊ณต๊ฒฉ ๋ฐฉ์ง๋ฅผ ์ํ ํ์ค ๋ณด์ ๋ฉ์ปค๋์ฆ์
๋๋ค. ํ์ฌ nonce๊ฐ ์์ฑ๋์์ผ๋(29์ค) ์ฃผ์ ์ฒ๋ฆฌ๋์ด ์ฌ์ฉ๋์ง ์๊ณ ์์ต๋๋ค.
์๋ฒ: ์ ์ฅ์ ๊ฒ์ ๊ฒฐ๊ณผ nonce ๊ฒ์ฆ ๋ก์ง์ด ์กด์ฌํ์ง ์์ต๋๋ค. Google ๋ฌธ์์ ๋ฐ๋ฅด๋ฉด ์๋ฒ๋ ID ํ ํฐ์ nonce ํด๋ ์์ ๋ฐ๊ธ๋ nonce์ ๋น๊ตํ์ฌ ๊ฒ์ฆํด์ผ ํ๋ฉฐ, ๊ฐ nonce๋ ๋จ์ผ ์ฌ์ฉ ์ฉ๋๋ก ์ทจ๊ธ๋์ด์ผ ํฉ๋๋ค.
private val googleIdOption: GetGoogleIdOption = GetGoogleIdOption.Builder()
.setServerClientId(WEB_CLIENT_ID) // ์น ํด๋ผ์ด์ธํธ ID
.setFilterByAuthorizedAccounts(false)
-// .setNonce(nonce)
+ .setNonce(nonce)
.build()์๋ฒ ์ธก์์ ์ด nonce๋ฅผ ID ํ ํฐ์ nonce ํด๋ ์๊ณผ ๋์กฐํ์ฌ ๊ฒ์ฆํ๋ ๋ก์ง์ ๊ตฌํํด์ผ ํฉ๋๋ค.
๐ 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.
| private val nonce = UUID.randomUUID().toString() | |
| private val googleIdOption: GetGoogleIdOption = GetGoogleIdOption.Builder() | |
| .setServerClientId(WEB_CLIENT_ID) // ์น ํด๋ผ์ด์ธํธ ID | |
| .setFilterByAuthorizedAccounts(false) | |
| // .setNonce(nonce) | |
| .build() | |
| private val nonce = UUID.randomUUID().toString() | |
| private val googleIdOption: GetGoogleIdOption = GetGoogleIdOption.Builder() | |
| .setServerClientId(WEB_CLIENT_ID) // ์น ํด๋ผ์ด์ธํธ ID | |
| .setFilterByAuthorizedAccounts(false) | |
| .setNonce(nonce) | |
| .build() |
๐ค Prompt for AI Agents
In feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt
around lines 29โ34, the client creates a nonce but it's commented out and also
generated as a single class-level val; enable nonce usage and make it
per-login-request by generating a fresh UUID for each auth attempt and pass it
to GetGoogleIdOption.setNonce(nonce). On the server side, implement persistent
single-use nonce handling: when starting an auth flow persist the nonce (e.g.,
DB/Redis) with an expiry, accept the ID token from the client and decode its
nonce claim, compare it to the stored nonce, reject mismatches or
expired/nonexistent nonces, and delete/mark the nonce as used so it cannot be
reused.
| private val request: GetCredentialRequest = GetCredentialRequest.Builder() | ||
| .addCredentialOption(googleIdOption) | ||
| .build() | ||
| private val coroutineScope: CoroutineScope = CoroutineScope(Dispatchers.Main) |
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.
์๋ช ์ฃผ๊ธฐ๋ฅผ ๊ณ ๋ คํ์ง ์์ CoroutineScope๊ฐ ๋ฉ๋ชจ๋ฆฌ ๋์๋ฅผ ์ ๋ฐํ ์ ์์ต๋๋ค.
CoroutineScope(Dispatchers.Main)์ ์ง์ ์์ฑํ๋ฉด Activity/Fragment๊ฐ ์ข
๋ฃ๋์ด๋ ์ฝ๋ฃจํด์ด ๊ณ์ ์คํ๋ ์ ์์ต๋๋ค.
๋ค์ ์ค ํ๋๋ฅผ ์ ์ฉํ์ธ์:
- ๊ถ์ฅ: Activity์ lifecycleScope๋ฅผ ํ๋ผ๋ฏธํฐ๋ก ๋ฐ๊ธฐ
class GoogleLoginHelper(
private val context: Context,
private val scope: CoroutineScope
)
// Usage: GoogleLoginHelper(context, lifecycleScope)- ๋์: SupervisorJob์ ์ฌ์ฉํ๊ณ ๋ช ์์ ์ผ๋ก ์ทจ์
private val job = SupervisorJob()
private val coroutineScope = CoroutineScope(Dispatchers.Main + job)
fun dispose() {
job.cancel()
}๐ค Prompt for AI Agents
In feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt
around line 38, the helper creates a raw CoroutineScope with Dispatchers.Main
which can outlive Activity/Fragment and cause memory leaks; change the class to
accept a CoroutineScope (preferably lifecycleScope) as a constructor parameter
and use that scope for launches, or alternatively create the scope with a
SupervisorJob combined with Dispatchers.Main and expose a dispose/clear method
that cancels the job; update usages to pass lifecycleScope from
Activities/Fragments when instantiating the helper.
| } catch (e: GetCredentialException) { | ||
| Log.e("Google Sign-in failed", " ${e.localizedMessage}") | ||
| } | ||
| } |
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.
์๋ฌ ๋ฐ์ ์ onFailure ์ฝ๋ฐฑ์ด ํธ์ถ๋์ง ์์ต๋๋ค.
GetCredentialException์ด ๋ฐ์ํ๋ฉด ๋ก๊ทธ๋ง ๋จ๊ธฐ๊ณ onFailure ์ฝ๋ฐฑ์ ํธ์ถํ์ง ์์ UI๊ฐ ๋ก๊ทธ์ธ ์คํจ๋ฅผ ๊ฐ์งํ์ง ๋ชปํฉ๋๋ค. ์ฌ์ฉ์๋ ๋ก๊ทธ์ธ์ด ์งํ ์ค์ธ์ง ์คํจํ๋์ง ์ ์ ์๊ฒ ๋ฉ๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ์์ ํ์ธ์:
} catch (e: GetCredentialException) {
Log.e("Google Sign-in failed", " ${e.localizedMessage}")
+ onFailure(e.localizedMessage ?: "๊ตฌ๊ธ ๋ก๊ทธ์ธ์ ์คํจํ์์ต๋๋ค.")
}๐ 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.
| } catch (e: GetCredentialException) { | |
| Log.e("Google Sign-in failed", " ${e.localizedMessage}") | |
| } | |
| } | |
| } catch (e: GetCredentialException) { | |
| Log.e("Google Sign-in failed", " ${e.localizedMessage}") | |
| onFailure(e.localizedMessage ?: "๊ตฌ๊ธ ๋ก๊ทธ์ธ์ ์คํจํ์์ต๋๋ค.") | |
| } | |
| } |
๐ค Prompt for AI Agents
In feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt
around lines 51โ54, the catch block for GetCredentialException currently only
logs the error so the UI never learns the login failed; update the catch to
invoke the provided onFailure callback with the exception (e.g., onFailure(e))
so failure is propagated to the caller, and if onFailure must run on the UI
thread, post/invoke it on the main thread before returning.
| } catch (e: GoogleIdTokenParsingException) { | ||
| // Log.e(TAG, "Received an invalid google id token response", e) | ||
| } |
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.
์์ธ๊ฐ ์ผ์ผ์ง๊ณ ์์ต๋๋ค.
GoogleIdTokenParsingException์ด ๋ฐ์ํด๋ ์๋ฌด ์ฒ๋ฆฌ๋ ํ์ง ์์ ์ฌ์ฉ์๊ฐ ์คํจ๋ฅผ ์ ์ ์์ต๋๋ค. Detekt๋ ์ด๋ฅผ ๊ฒฝ๊ณ ํ๊ณ ์์ต๋๋ค.
์์ธ ๋ฐ์ ์ ์คํจ ์ฝ๋ฐฑ์ ํธ์ถํ์ธ์:
} catch (e: GoogleIdTokenParsingException) {
-// Log.e(TAG, "Received an invalid google id token response", e)
+ Log.e(TAG, "Received an invalid google id token response", e)
+ onFailure("๊ตฌ๊ธ ๋ก๊ทธ์ธ ํ ํฐ ํ์ฑ์ ์คํจํ์์ต๋๋ค.")
}๐ 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.
| } catch (e: GoogleIdTokenParsingException) { | |
| // Log.e(TAG, "Received an invalid google id token response", e) | |
| } | |
| } catch (e: GoogleIdTokenParsingException) { | |
| Log.e(TAG, "Received an invalid google id token response", e) | |
| onFailure("๊ตฌ๊ธ ๋ก๊ทธ์ธ ํ ํฐ ํ์ฑ์ ์คํจํ์์ต๋๋ค.") | |
| } |
๐งฐ Tools
๐ช detekt (1.23.8)
[warning] 73-73: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
๐ค Prompt for AI Agents
In feature/oauth/src/main/java/com/teamsolply/solply/oauth/GoogleLoginHelper.kt
around lines 73-75, the catch block for GoogleIdTokenParsingException currently
swallows the exception; replace it so the exception is logged and the login
failure path is invoked (e.g., call the existing failure callback or listener
with the exception or an appropriate error object). Ensure you do not rethrow
unnecessarily, but propagate the error to the caller by invoking the module's
failure callback (and include the exception in the log message and callback
parameters).
| OauthSideEffect.StartGoogleLogin -> googleLoginHelper.requestGoogleLogin( | ||
| onSuccess = { accessToken -> | ||
| viewModel.sendIntent( | ||
| OauthIntent.GoogleLoginSuccess( | ||
| provider = "GOOGLE", | ||
| accessToken = accessToken | ||
| ) | ||
| ) | ||
| }, | ||
| onFailure = {} | ||
| ) | ||
|
|
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.
๊ตฌ๊ธ ๋ก๊ทธ์ธ ๋ก์ง์ด ์ค๋ณต๋์ด ์์ต๋๋ค.
๊ตฌ๊ธ ๋ก๊ทธ์ธ ๋ก์ง์ด ๋ ๊ณณ์ ์ค๋ณต ๊ตฌํ๋์ด ์์ต๋๋ค:
- Lines 68-79:
OauthSideEffect.StartGoogleLogin์ฌ์ด๋ ์ดํํธ ์ฒ๋ฆฌ - Lines 88-100:
googleLoginClick์ฝ๋ฐฑ์์ ์ง์ ํธ์ถ
์ด๋ ์๋ํ์ง ์์ ๋์์ ์ ๋ฐํ ์ ์์ต๋๋ค. ์นด์นด์ค ๋ก๊ทธ์ธ๊ณผ ๋์ผํ๊ฒ ViewModel์ ํตํ ๋จ์ผ ํ๋ก์ฐ๋ง ์ฌ์ฉํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
๋ค์๊ณผ ๊ฐ์ด ์์ ํ์ฌ ์นด์นด์ค ๋ก๊ทธ์ธ๊ณผ ์ผ๊ด๋ ํจํด์ ์ ์งํ์ธ์:
OauthScreen(
kakaoLoginClick = { viewModel.sendIntent(OauthIntent.KakaoLoginClick) },
- googleLoginClick = {
- googleLoginHelper.requestGoogleLogin(
- onSuccess = { accessToken ->
- viewModel.sendIntent(
- OauthIntent.GoogleLoginSuccess(
- provider = "GOOGLE",
- accessToken = accessToken
- )
- )
- },
- onFailure = {}
- )
- }
+ googleLoginClick = { viewModel.sendIntent(OauthIntent.GoogleLoginClick) }
)Also applies to: 88-100
๐ค Prompt for AI Agents
In feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthScreen.kt around
lines 68-79 and 88-100 there is duplicated Google login logic: the same
googleLoginHelper.requestGoogleLogin call is invoked both from the
OauthSideEffect.StartGoogleLogin handler and from the googleLoginClick callback.
Remove the direct request call from the googleLoginClick callback (lines
~88-100) and instead dispatch a single ViewModel intent from the click handler
(e.g., send an intent like OauthIntent.StartGoogleLogin or similar) so that the
side-effect handler (lines ~68-79) is the sole place that calls
googleLoginHelper.requestGoogleLogin; also propagate success/failure through the
existing ViewModel intents rather than handling login directly in the UI
callback.
| ) | ||
| ) | ||
| }, | ||
| onFailure = {} |
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.
์๋ฌ ์ฒ๋ฆฌ๊ฐ ๋๋ฝ๋์์ต๋๋ค.
onFailure ์ฝ๋ฐฑ์ด ๋น ๋๋ค {}๋ก ์ฒ๋ฆฌ๋์ด ์์ด ์ฌ์ฉ์๊ฐ ๋ก๊ทธ์ธ ์คํจ๋ฅผ ์ ์ ์์ต๋๋ค. ์ต์ํ ๋ก๊ทธ๋ฅผ ๋จ๊ธฐ๊ฑฐ๋ ์ฌ์ฉ์์๊ฒ ํ ์คํธ/์ค๋ต๋ฐ๋ก ์๋ ค์ฃผ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
์์:
onFailure = { errorMessage ->
Log.e("OauthScreen", "Google login failed: $errorMessage")
// TODO: Show error message to user
}Also applies to: 98-98
๐ค Prompt for AI Agents
In feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthScreen.kt around
lines 77 and 98, the onFailure callbacks are empty lambdas so login failures are
swallowed; update both callbacks to accept the error (or message) parameter, log
the error (e.g., Log.e with a clear tag and message) and surface feedback to the
user (show a Toast or Snackbar with a user-friendly message); ensure you handle
nullable/unknown error text safely and avoid crashing when showing the UI
feedback.
| OauthIntent.GoogleLoginClick -> postSideEffect(OauthSideEffect.StartGoogleLogin) | ||
|
|
||
| is OauthIntent.GoogleLoginSuccess -> postSocialLogin( | ||
| provider = intent.provider, | ||
| oauthAccessToken = intent.accessToken | ||
| ) | ||
|
|
||
| is OauthIntent.GoogleLoginFailure -> { | ||
| TODO() | ||
| } |
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.
Google ๋ก๊ทธ์ธ ์คํจ ์ฒ๋ฆฌ๋ฅผ ๊ตฌํํด์ผ ํฉ๋๋ค.
Google ๋ก๊ทธ์ธ ํ๋ฆ์ด ์ถ๊ฐ๋์์ผ๋ GoogleLoginFailure์ ์๋ฌ ์ฒ๋ฆฌ๊ฐ TODO()๋ก ๋จ์์์ต๋๋ค. ์ด๋ ๋ฐํ์ ์ NotImplementedError๋ฅผ ๋ฐ์์ํต๋๋ค.
์ฌ์ฉ์์๊ฒ ์ ์ ํ ์๋ฌ ๋ฉ์์ง๋ฅผ ํ์ํ๊ฑฐ๋ ๋ก๊ทธ๋ฅผ ๋จ๊ธฐ๋ ๋ฑ์ ์๋ฌ ์ฒ๋ฆฌ๊ฐ ํ์ํฉ๋๋ค.
Kakao ๋ก๊ทธ์ธ ์คํจ ์ฒ๋ฆฌ๋ ๋์ผํ๊ฒ TODO()๋ก ๋จ์์์ต๋๋ค(Line 24-26). ๋ ๊ฒฝ์ฐ ๋ชจ๋ ์๋ฌ ์ฒ๋ฆฌ ๊ตฌํ์ด ํ์ํฉ๋๋ค. ์๋ฌ ์ฒ๋ฆฌ ๊ตฌํ์ ๋์๋๋ฆด๊น์?
๐ค Prompt for AI Agents
feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthViewModel.kt around
lines 24-26 and 28-37: both KakaoLoginFailure and GoogleLoginFailure currently
call TODO() which throws NotImplementedError at runtime; replace each TODO with
concrete failure handling by posting an appropriate side effect (e.g.,
postSideEffect(OauthSideEffect.ShowError(...)) with a user-friendly message and
include/log the underlying error for diagnostics), and ensure any cleanup or
state reset needed after a failed social login is performed (e.g., clear loading
state or reset flags).
| credentials = "1.6.0-beta03" | ||
| googleid = "1.1.1" |
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.
๐งฉ Analysis chain
๋ผ์ด๋ธ๋ฌ๋ฆฌ ๋ฒ์ ํ์ธ์ด ํ์ํฉ๋๋ค.
credentials ๋ผ์ด๋ธ๋ฌ๋ฆฌ๊ฐ ๋ฒ ํ ๋ฒ์ (1.6.0-beta03)์ ์ฌ์ฉํ๊ณ ์์ต๋๋ค. ํ๋ก๋์
ํ๊ฒฝ์์๋ ์์ ๋ฒ์ ์ฌ์ฉ์ ๊ถ์ฅํฉ๋๋ค.
๋ค์ ๋ช ๋ น์ผ๋ก ์ต์ ์์ ๋ฒ์ ์ ํ์ธํ์ธ์:
๐ Web query:
What is the latest stable version of androidx.credentials library?
๐ก Result:
The latest stable release is androidx.credentials:credentials version 1.5.0 (released March 12, 2025). [1]
Sources
[1] Android Jetpack โ Credentials release notes.
์ต์ ์ ๋ณด๋ฅผ ํ์ธํ๊ธฐ ์ํด ์ถ๊ฐ ๊ฒ์์ด ํ์ํฉ๋๋ค.
ํ๋ก๋์
ํ๊ฒฝ์์๋ ์์ ๋ฒ์ (1.5.0)์ ์ฌ์ฉํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
์ฝ๋๊ฐ ์์ง ๋ฒ ํ ๋จ๊ณ์ธ androidx.credentials:credentials:1.6.0-beta03์ ์ฌ์ฉ ์ค์
๋๋ค. ์ต์ ์์ ๋ฒ์ ์ 1.5.0์
๋๋ค. ๋ฒ ํ ๋ฒ์ ์ ๋ฏธ์์ฑ ๊ธฐ๋ฅ๊ณผ ์ ์ฌ์ ๋ฒ๊ทธ๊ฐ ํฌํจ๋ ์ ์์ผ๋ฏ๋ก, ํ๋ก๋์
๋ฐฐํฌ ์ ์ ์์ ๋ฒ์ ์ผ๋ก ๋ณ๊ฒฝํ์ธ์.
credentials = "1.5.0"์ผ๋ก ๋ณ๊ฒฝํ๊ฑฐ๋, 1.6.0 ๋ฒ ํ ๋ฒ์ ์ฌ์ฉ์ด ํ์์ธ ๊ฒฝ์ฐ ๊ทธ ์ด์ ๋ฅผ ๋ช
์ํ์ธ์.
๐ค Prompt for AI Agents
gradle/libs.versions.toml lines 93-94: the project references the beta artifact
androidx.credentials:credentials 1.6.0-beta03 which is not recommended for
production; update the version to the latest stable release by changing
credentials = "1.6.0-beta03" to credentials = "1.5.0", or if you must keep the
beta, add a short comment next to the entry explaining why the beta is required
and what feature or fix depends on it.
88guri
left a comment
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.
๊ตฌ๊ธ ๋ก๊ทธ์ธ๐ Good~
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)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileEditScreen.kt (1)
129-131: ๊ณ ์ ๋์ด์ weight ํผ์ฉ์ผ๋ก ์ธํ ๋ ์ด์์ ๋ถ์ผ์น์ผ๋ถ Spacer๋ง ๊ณ ์ ๋์ด(
height)๋ก ๋ณ๊ฒฝ๋์๊ณ , Line 197์Spacer(modifier = Modifier.weight(12f))๋ ๊ทธ๋๋ก ์ ์ง๋์ด ์์ต๋๋ค. ์ด๋ก ์ธํด:
- ๊ณ ์ ๋์ด Spacer๋ค์ ํ๋ฉด ํฌ๊ธฐ์ ๊ด๊ณ์์ด ํญ์ ๋์ผํ ๊ณต๊ฐ์ ์ฐจ์ง
- weight Spacer๋ ๋จ์ ๊ณต๊ฐ์ ๋ชจ๋ ์ฐจ์ง
Arrangement.SpaceBetween๊ณผ ์กฐํฉ ์ ์๋์น ์์ ๋ ์ด์์ ๋ฐ์ ๊ฐ๋ฅํ๋ฉด ํฌ๊ธฐ๋ณ ๋์์ด ํ์ํ๋ค๋ฉด weight ๋ฐฉ์์ผ๋ก ํต์ผํ๊ฑฐ๋, ๊ณ ์ ๋ ์ด์์์ด ์๋๋ผ๋ฉด ๋ชจ๋ Spacer๋ฅผ ๊ณ ์ ๋์ด๋ก ๋ณ๊ฒฝํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.
โป๏ธ ์ผ๊ด๋ ๋ฐฉ์์ผ๋ก ์์ ์ ์ (๊ณ ์ ๋์ด ๋ฐฉ์)
- Spacer(modifier = Modifier.weight(12f)) + Spacer(modifier = Modifier.weight(1f))๋๋ ๋ชจ๋ spacer๋ฅผ weight ๊ธฐ๋ฐ์ผ๋ก ๋ณต์:
- modifier = Modifier.height(16.dp) + modifier = Modifier.weight(1f)- Spacer(modifier = Modifier.height(32.dp)) + Spacer(modifier = Modifier.weight(2f))- Spacer(modifier = Modifier.height(24.dp)) + Spacer(modifier = Modifier.weight(1.5f))Also applies to: 141-141, 169-169
๐ Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
๐ Files selected for processing (2)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileEditScreen.ktfeature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthViewModel.kt
๐ง Files skipped from review as they are similar to previous changes (1)
- feature/oauth/src/main/java/com/teamsolply/solply/oauth/OauthViewModel.kt
๐ Additional comments (1)
feature/mypage/src/main/java/com/teamsolply/solply/mypage/profile/ProfileEditScreen.kt (1)
117-124:Arrangement.SpaceBetween๊ณผ Spacer ์กฐํฉ ์ฌ๊ฒํ ํ์ํ์ฌ Column์ด
verticalArrangement = Arrangement.SpaceBetween์ ์ฌ์ฉํ๊ณ ์์ด ์์ ์์๋ค ์ฌ์ด์ ์๋์ผ๋ก ๊ท ๋ฑํ ๊ฐ๊ฒฉ์ด ๋ฐฐ๋ถ๋ฉ๋๋ค. ์ด ์ํ์์ ๊ณ ์ ๋์ด Spacer๋ฅผ ์ถ๊ฐํ๋ฉด Spacer ์์ฒด๊ฐ "์ฝํ ์ธ "๋ก ๊ฐ์ฃผ๋์ด ์๋ํ ๋ ์ด์์๊ณผ ๋ค๋ฅด๊ฒ ๋์ํ ์ ์์ต๋๋ค.
SpaceBetween์ ์ ์งํ๋ฉด์ ํน์ ๊ฐ๊ฒฉ์ ์กฐ์ ํ๋ ค๋ฉด Spacer ๋์ ํด๋น ์ปดํฌ๋ํธ์padding์ด๋Modifier.padding()์ ์ฌ์ฉํ๋ ๊ฒ์ด ๋ ๋ช ํํฉ๋๋ค.
๐ PR ์์ฝ
๐ฑ ์์ ํ ๋ด์ฉ
๐ฑ PR ํฌ์ธํธ
๐ธ ์คํฌ๋ฆฐ์ท
๐ฎ ๊ด๋ จ ์ด์
Summary by CodeRabbit
์๋ก์ด ๊ธฐ๋ฅ
๊ฐ์
์์
โ๏ธ Tip: You can customize this high-level summary in your review settings.