[feat] #12 카카오 로그인 구현 및 JWT 토큰 갱신 로직 구성#20
Merged
Merged
Conversation
…m-2-Android into feat/#12-auth # Conflicts: # app/build.gradle.kts # app/src/main/java/com/neki/android/app/NekiApplication.kt # settings.gradle.kts
Walkthrough카카오 로그인 통합(앱 초기화·매니페스트 콜백), 서버 연동 로그인·토큰 갱신 엔드포인트·모델 추가, DataStore 토큰 API 리네이밍·확장, Auth 이벤트 브로드캐스트 및 자동 토큰 갱신 로직, auth 모듈(API/impl) 신규 도입. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자(UI)
participant App as 앱(LoginScreen/KakaoLoginHelper)
participant Kakao as Kakao SDK
participant AuthRepo as AuthRepository (impl)
participant API as 서버(API)
participant Store as DataStoreRepository
participant Event as AuthEventManager
User->>App: ClickKakaoLogin
App->>Kakao: loginWithKakao()
Kakao-->>App: idToken
App->>AuthRepo: loginWithKakao(idToken)
AuthRepo->>API: POST /api/auth/kakao/login (idToken)
API-->>AuthRepo: BasicResponse<AuthResponse>
AuthRepo-->>App: Result<Auth>
App->>Store: saveJwtTokens(accessToken, refreshToken)
App-->>User: NavigateToHome
Note over API,App: 이후 요청 시 자동 Bearer 첨부/갱신
App->>API: 요청 (Bearer)
API-->>App: 401 (토큰만료)
App->>AuthRepo: updateAccessToken(refreshToken)
AuthRepo->>API: POST /api/auth/refresh
API-->>AuthRepo: BasicResponse<AuthResponse> / 실패
alt 갱신 성공
AuthRepo-->>Store: saveJwtTokens(newTokens)
App->>API: 원본 요청 재시도
else 갱신 실패
App->>Event: emitTokenExpired()
Event-->>App: (브로드캐스트)
end
sequenceDiagram
participant UI as LoginScreen
participant VM as LoginViewModel
participant Store as DataStoreRepository
participant Repo as AuthRepository
participant Event as AuthEventManager
UI->>VM: EnterLoginScreen
VM->>Store: isSavedJwtTokens()
Store-->>VM: Boolean
alt 저장된 토큰 있음
VM->>Store: getRefreshToken()
VM->>Repo: updateAccessToken(refreshToken)
Repo-->>VM: Result<Auth>
alt 성공
VM->>Store: saveJwtTokens(...)
VM-->>UI: NavigateToHome
else 실패
VM->>Event: emitTokenExpired()
VM-->>UI: ShowToast / Login 화면
end
else 없음
VM-->>UI: Show Login
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Fix all issues with AI agents
In @app/build.gradle.kts:
- Around line 25-29: buildConfigField(...) is being assigned to
manifestPlaceholders incorrectly: call buildConfigField("String",
"KAKAO_NATIVE_APP_KEY", ...) separately (it returns Unit) to add the BuildConfig
entry, and then set manifestPlaceholders["KAKAO_NATIVE_APP_KEY"] =
properties["KAKAO_NATIVE_APP_KEY"].toString().removeSurrounding("\"") to assign
the manifest placeholder (use removeSurrounding to strip quotes).
In @core/data/build.gradle.kts:
- Around line 24-26: The buildConfigField call inside defaultConfig uses
properties["BASE_URL"].toString(), which yields the literal "null" if the
property is missing; change it to safely handle null and produce a valid Java
String literal: compute a value like val baseUrl = (properties["BASE_URL"] as?
String)?.takeIf { it.isNotBlank() } ?: "<default-url-or-throw>" and then call
buildConfigField("String", "BASE_URL", "\"$baseUrl\"") so BASE_URL is never set
to the string "null" (update the default value or throw as appropriate).
In
@core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt:
- Around line 81-89: The current loadTokens lambda uses !! on getAccessToken()
and getRefreshToken(), risking NPE if tokens are removed between
isSavedJwtTokens() and the individual gets; change loadTokens to fetch both
tokens in a null-safe way (call dataStoreRepository.getAccessToken().first() and
getRefreshToken().first() into local vals and only construct BearerTokens if
both are non-null), otherwise return null; reference the loadTokens lambda and
dataStoreRepository.getAccessToken/getRefreshToken/isSavedJwtTokens and ensure
you avoid the !! operator.
- Around line 96-102: The null-forcing operator on
dataStoreRepository.getRefreshToken().first() in the RefreshTokenRequest
creation risks an NPE; update the refresh token retrieval inside the
refreshTokens block to safely use oldTokens?.refreshToken if available or
perform an explicit null check before calling
client.post(...).body<BasicResponse<AuthResponse>>() so you either bail out or
handle the missing token (e.g., log/error/throw) instead of using !!; ensure
RefreshTokenRequest receives a non-null refreshToken value.
- Around line 116-121: The current catch-all in NetworkModule.kt that logs the
exception, calls dataStoreRepository.clearTokens() and
authEventManager.emitTokenExpired() must be replaced with exception-specific
handling: catch HttpException and if e.code() == 401 (or other token-expiry
indicators) then clear tokens via dataStoreRepository.clearTokens() and call
authEventManager.emitTokenExpired(); catch network-related exceptions (e.g.,
IOException, SocketTimeoutException, ConnectException) and trigger a
retry/backoff mechanism or rethrow so callers can retry instead of clearing
tokens; catch other unexpected Exceptions to log and surface a user-friendly
error without forcing logout. Locate the try/catch around the network call in
NetworkModule.kt and replace the single catch(Exception) with multiple catches
for HttpException, IOException (and its timeout/connect subclasses), and a final
generic catch that avoids token deletion.
In @detekt-config.yml:
- Around line 52-54: Replace the broad catch (e: Exception) in NetworkModule.kt
(around the token refresh logic) with explicit catches for Ktor/serialization
exceptions such as ClientRequestException, ServerResponseException, and
SerializationException, handling each by calling Timber.e(e),
dataStoreRepository.clearTokens(), and authEventManager.emitTokenExpired() (and
keep a final generic catch for any other Exception if needed); after updating
the catch blocks, re-enable the TooGenericExceptionCaught rule in
detekt-config.yml.
In @feature/auth/impl/build.gradle.kts:
- Around line 9-14: The dependency declaration exposes the Kakao SDK from
auth.impl even though it's only used internally; change the dependency from
api(libs.kakao.user) to implementation(libs.kakao.user) in the build.gradle.kts
so Kakao types (used only by KakaoLoginHelper) are not propagated to consumers
and module boundaries remain intact.
In
@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/component/LoginContent.kt:
- Around line 21-28: Button lacks an accessibility label; update the Button call
that uses onClickKakaoLogin to include a Modifier.semantics with a
contentDescription (use stringResource for localization, e.g. a new
R.string.kakao_login_label) so screen readers can announce the button purpose.
Locate the Button in LoginContent.kt (the one with onClick = onClickKakaoLogin)
and add the semantics modifier to the composable's Modifier, importing
androidx.compose.ui.semantics.contentDescription and
androidx.compose.ui.semantics.semantics, and add the localized stringResource
message for the contentDescription.
- Line 25: Replace the hardcoded UI string in LoginContent.kt (the text
parameter currently set to "카카오 로그인") with a string resource reference using
stringResource(R.string.kakao_login), add the import
androidx.compose.ui.res.stringResource at the top of the file, and add the
corresponding entry <string name="kakao_login">카카오 로그인</string> to strings.xml
so the label is localized.
In
@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginScreen.kt:
- Around line 40-42: The onFailure lambda currently only logs the error via
Timber.d("로그인 실패 $message") and doesn't provide user feedback; update the
failure handling to post a side effect so users see a toast: inside the
onFailure callback call postSideEffect(LoginSideEffect.ShowToastMessage(message
?: "카카오 로그인에 실패했습니다.")) (or use a default message), and also ensure the
LoginIntent.FailLogin branch posts the same side effect (e.g.,
LoginIntent.FailLogin -> postSideEffect(LoginSideEffect.ShowToastMessage("카카오
로그인에 실패했습니다."))) so both paths surface the error to the user.
In
@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt:
- Around line 46-49: The current block can cause an NPE and a race condition
because isSavedJwtTokens() may be true while getRefreshToken() returns null and
because NetworkModule.refreshTokens may already handle refresh; update the code
to first obtain the refresh token safely (e.g., val refresh =
dataStoreRepository.getRefreshToken().first() and check for null) and only call
authRepository.updateAccessToken(refreshToken = refresh) if refresh is non-null;
additionally confirm whether NetworkModule.refreshTokens already performs token
refresh and remove this duplicate updateAccessToken() call if so to avoid
double-refresh/race conditions, referencing isSavedJwtTokens(),
getRefreshToken(), authRepository.updateAccessToken(), and
NetworkModule.refreshTokens to locate the logic.
- Line 38: The FailLogin branch incorrectly navigates to
LoginSideEffect.NavigateToKakaoRedirectingUri; change it to emit an error side
effect instead by calling postSideEffect with LoginSideEffect.ShowError (or the
existing error-displaying side effect) and pass the failure message/throwable
from LoginIntent.FailLogin if available; update the LoginIntent.FailLogin
handling to use postSideEffect(LoginSideEffect.ShowError(...)) rather than
NavigateToKakaoRedirectingUri so failures display an error to the user.
- Around line 65-79: The login flow sets isLoading = true but never resets it on
failure, causing the UI to stay loading; update the coroutine in LoginViewModel
(the launch that calls authRepository.loginWithKakao) to always reset isLoading
to false after the call completes—e.g., call reduce { copy(isLoading = false) }
in both the onSuccess and onFailure handlers (or use a
finally/invokeOnCompletion equivalent) so that after
saveJwtTokens/postSideEffect or after Timber.d(...) the UI loading state is
cleared.
In
@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/util/KakaoLoginHelper.kt:
- Around line 25-31: The callback in
UserApiClient.Companion.instance.loginWithKakaoAccount is force-unwrapping
token.idToken which can cause an NPE and also doesn’t handle the case where both
error and token are null; change the flow to safely null-check token and
token.idToken (e.g., if (token?.idToken != null) call onSuccess with that value,
else call onFailure with a descriptive message) and ensure any branch where both
error and token are null falls back to onFailure rather than doing nothing or
using !!.
- Around line 16-22: The callback in
UserApiClient.Companion.instance.loginWithKakaoTalk force-unwraps token.idToken
causing potential NPE and also fails to handle the edge case where both error
and token are null; change the callback to safely check error first, then use
token?.idToken?.let { onSuccess(it) } ?: onFailure("idToken is null or missing")
(or a localized message) so you never force-unwrap, and ensure that if both
error and token (or idToken) are null you call onFailure with a clear message;
update the lambda in KakaoLoginHelper.kt accordingly.
In @gradle/libs.versions.toml:
- Line 32: Update the kakao dependency version in the versions catalog: locate
the kakao entry in gradle/libs.versions.toml (the line currently set as kakao =
"2.23.1") and change its value to "2.23.2" so it reads kakao = "2.23.2", then
save and run a quick build to ensure dependency resolution succeeds.
🧹 Nitpick comments (11)
core/model/src/main/java/com/neki/android/core/model/Auth.kt (1)
3-6: 빈 문자열 기본값 대신 nullable 타입 사용을 고려하세요.현재 구현은 유효하지 않은
Auth()객체 생성을 허용합니다. 토큰이 없는 상태를 명시적으로 표현하려면 nullable 타입(String?)을 사용하거나 기본값 없이 필수 매개변수로 만드는 것이 좋습니다.♻️ 제안하는 리팩토링
data class Auth( - val accessToken: String = "", - val refreshToken: String = "", + val accessToken: String, + val refreshToken: String, )또는 nullable 타입 사용:
data class Auth( - val accessToken: String = "", - val refreshToken: String = "", + val accessToken: String?, + val refreshToken: String?, )app/src/main/java/com/neki/android/app/NekiApplication.kt (1)
26-28:KAKAO_NATIVE_APP_KEY빈 값 대비 guard/로깅 추가 권장Kakao SDK의
KakaoSdk.init()은 빈 값이나 잘못된 키로 호출되면NullPointerException,IllegalArgumentException, 또는KOE101서버 오류를 발생시킵니다. 현재 코드는 guard 없이 직접 초기화하므로, CI/로컬 설정 누락 시 오류 메시지가 명확하지 않을 수 있습니다. 디버그 빌드에서 최소한 명시적 검증을 추가하면 초기화 단계에서 문제를 빠르게 파악할 수 있습니다.제안 수정
NaverMapSdk.getInstance(this).client = NaverMapSdk.NcpKeyClient(BuildConfig.NAVER_MAP_CLIENT_ID) - KakaoSdk.init(this, BuildConfig.KAKAO_NATIVE_APP_KEY) + val kakaoKey = BuildConfig.KAKAO_NATIVE_APP_KEY + check(kakaoKey.isNotBlank()) { "KAKAO_NATIVE_APP_KEY is blank. Check buildConfigField/local.properties/CI secrets." } + KakaoSdk.init(this, kakaoKey)core/data-api/src/main/java/com/neki/android/core/dataapi/repository/DataStoreRepository.kt (1)
11-11: 메서드명을 더 관용적으로 개선 고려새로운
isSavedJwtTokens()메서드는 유용한 기능을 제공하지만, Kotlin의 관용적인 네이밍 컨벤션을 고려하면 다음과 같은 이름이 더 자연스럽습니다:
hasJwtTokens()areJwtTokensSaved()hasStoredJwtTokens()현재 이름인 "isSaved"는 다소 어색하게 읽힙니다.
♻️ 제안하는 리팩토링
- fun isSavedJwtTokens(): Flow<Boolean> + fun hasJwtTokens(): Flow<Boolean>core/data/src/main/java/com/neki/android/core/data/remote/model/response/AuthResponse.kt (1)
7-16: LGTM! 간결한 DTO 구조
AuthResponse데이터 클래스와toModel()매핑이 올바르게 구현되었습니다. kotlinx.serialization을 적절히 활용하고 있으며, 도메인 모델로의 변환도 명확합니다.선택적으로, 토큰 값에 대한 기본적인 검증(예: 빈 문자열 체크)을 추가하는 것을 고려할 수 있지만, DTO 레이어에서는 현재 구조도 적절합니다. 검증은 도메인 레이어나 사용처에서 수행하는 것이 일반적입니다.
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/component/LoginContent.kt (1)
26-26: 폰트 크기를 테마 타이포그래피로 변경 권장폰트 크기가
22.sp로 하드코딩되어 있습니다. Material3의 타이포그래피 스케일을 사용하면 일관된 디자인 시스템을 유지할 수 있습니다.♻️ 제안하는 리팩토링
Text( text = "카카오 로그인", - fontSize = 22.sp, + style = MaterialTheme.typography.titleLarge, )또한 파일 상단에 import 추가:
import androidx.compose.material3.MaterialThemefeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginContract.kt (1)
7-12:FailLogin에 에러 정보 추가를 고려해 보세요.현재
FailLogin이 data object로 선언되어 있어 실패 원인을 전달할 수 없습니다. 사용자에게 더 구체적인 에러 메시지를 보여주거나 로깅을 위해 에러 정보를 포함하는 것이 유용할 수 있습니다.💡 선택적 개선안
sealed interface LoginIntent { data object EnterLoginScreen : LoginIntent data object ClickKakaoLogin : LoginIntent data class SuccessLogin(val idToken: String) : LoginIntent - data object FailLogin : LoginIntent + data class FailLogin(val error: Throwable? = null) : LoginIntent }feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginScreen.kt (1)
19-21: 사용되지 않는navigateBack파라미터
navigateBack파라미터가 선언되었지만 함수 내에서 사용되지 않습니다. 향후 Navigation 3 리팩토링 시 사용할 예정이라면@Suppress("UNUSED_PARAMETER")어노테이션을 추가하거나, 현재 사용하지 않는다면 제거를 고려해주세요.core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt (2)
46-49:sendWithoutJwtUrls를private으로 변경 권장모듈 외부에서 접근할 필요가 없는 내부 상수입니다.
private으로 선언하여 캡슐화를 강화하세요.♻️ 제안된 수정
- val sendWithoutJwtUrls = listOf( + private val sendWithoutJwtUrls = listOf( "/api/auth/kakao/login", "/api/auth/refresh", )
125-132: 로그 메시지와 실제 로직이 불일치
shouldNotJwt변수명과 로그 메시지"Bearer 인증 필요 API 여부"가 혼란을 줍니다. 변수명을requiresAuth로 변경하고 로직을 명확하게 표현하는 것이 좋습니다.♻️ 제안된 수정
sendWithoutRequest { request -> - val shouldNotJwt = sendWithoutJwtUrls.any { + val isAuthExcluded = sendWithoutJwtUrls.any { request.url.encodedPath == it } - Timber.d("Bearer 인증 필요 API 여부 : $shouldNotJwt") - !shouldNotJwt + Timber.d("Bearer 인증 제외 API 여부 : $isAuthExcluded") + !isAuthExcluded }core/data/src/main/java/com/neki/android/core/data/remote/api/ApiService.kt (1)
9-9: 사용되지 않는 import
io.ktor.client.request.getimport가 사용되지 않습니다. 이전getPosts(),getPost()메서드 제거 후 남은 것으로 보입니다.♻️ 제안된 수정
import io.ktor.client.call.body -import io.ktor.client.request.get import io.ktor.client.request.postfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginActivity.kt (1)
35-48: Lifecycle 인식 없이 이벤트 수집
lifecycleScope.launch로 수집하면 Activity가 백그라운드에 있을 때도 이벤트를 처리합니다.repeatOnLifecycle을 사용하여STARTED상태에서만 수집하는 것이 권장됩니다.♻️ 제안된 수정
+import androidx.lifecycle.Lifecycle +import androidx.lifecycle.repeatOnLifecycle private fun observeAuthEvents() { lifecycleScope.launch { - authEventManager.authEvent.collect { event -> - when (event) { - AuthEvent.RefreshTokenExpired -> { - Toast.makeText( - this@LoginActivity, - "RefreshToken이 만료되었습니다.", - Toast.LENGTH_SHORT, - ).show() + repeatOnLifecycle(Lifecycle.State.STARTED) { + authEventManager.authEvent.collect { event -> + when (event) { + AuthEvent.RefreshTokenExpired -> { + Toast.makeText( + this@LoginActivity, + "RefreshToken이 만료되었습니다.", + Toast.LENGTH_SHORT, + ).show() + } } } } } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/com/neki/android/app/NekiApplication.ktcore/data-api/src/main/java/com/neki/android/core/dataapi/auth/AuthEventManager.ktcore/data-api/src/main/java/com/neki/android/core/dataapi/repository/AuthRepository.ktcore/data-api/src/main/java/com/neki/android/core/dataapi/repository/DataStoreRepository.ktcore/data/build.gradle.ktscore/data/src/main/java/com/neki/android/core/data/auth/AuthEventManagerImpl.ktcore/data/src/main/java/com/neki/android/core/data/remote/api/ApiService.ktcore/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.ktcore/data/src/main/java/com/neki/android/core/data/remote/model/request/KakaoLoginRequest.ktcore/data/src/main/java/com/neki/android/core/data/remote/model/request/RefreshTokenRequest.ktcore/data/src/main/java/com/neki/android/core/data/remote/model/response/AuthResponse.ktcore/data/src/main/java/com/neki/android/core/data/remote/model/response/BasicResponse.ktcore/data/src/main/java/com/neki/android/core/data/remote/model/response/PostResponse.ktcore/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/AuthRepositoryImpl.ktcore/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.ktcore/model/src/main/java/com/neki/android/core/model/Auth.ktcore/model/src/main/java/com/neki/android/core/model/Post.ktdetekt-config.ymlfeature/auth/api/.gitignorefeature/auth/api/build.gradle.ktsfeature/auth/api/src/main/kotlin/com/neki/android/feature/auth/api/LoginNavKey.ktfeature/auth/impl/.gitignorefeature/auth/impl/build.gradle.ktsfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginActivity.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginContract.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginScreen.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/component/LoginContent.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/di/LoginEntryProvider.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/util/KakaoLoginHelper.ktgradle/libs.versions.tomlsettings.gradle.kts
💤 Files with no reviewable changes (2)
- core/data/src/main/java/com/neki/android/core/data/remote/model/response/PostResponse.kt
- core/model/src/main/java/com/neki/android/core/model/Post.kt
🧰 Additional context used
🧬 Code graph analysis (4)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/di/LoginEntryProvider.kt (1)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginScreen.kt (1)
LoginRoute(18-60)
core/data/src/main/java/com/neki/android/core/data/repository/impl/AuthRepositoryImpl.kt (1)
core/data/src/main/java/com/neki/android/core/data/util/RunSuspendCatching.kt (1)
runSuspendCatching(8-20)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginActivity.kt (2)
core/designsystem/src/main/java/com/neki/android/core/designsystem/ui/theme/Theme.kt (1)
NekiTheme(22-37)feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginScreen.kt (1)
LoginRoute(18-60)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt (1)
core/ui/src/main/java/com/neki/android/core/ui/MviIntentStore.kt (2)
mviIntentStore(53-60)postSideEffect(39-41)
⏰ 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: ci-build
🔇 Additional comments (27)
feature/auth/impl/.gitignore (1)
1-1: 표준 .gitignore 규칙입니다.빌드 디렉토리 무시 규칙이 올바르게 설정되었습니다.
feature/auth/api/.gitignore (1)
1-1: 표준 .gitignore 규칙입니다.빌드 디렉토리 무시 규칙이 올바르게 설정되었습니다.
feature/auth/api/src/main/kotlin/com/neki/android/feature/auth/api/LoginNavKey.kt (1)
7-15: 네비게이션 구조가 올바르게 구현되었습니다.Navigation 3의 타입 안전 네비게이션을 위한 sealed interface와 @serializable 애노테이션이 적절하게 사용되었습니다.
feature/auth/api/build.gradle.kts (1)
1-7: 구성 깔끔합니다 (api 모듈 최소 설정 유지)core/data/src/main/java/com/neki/android/core/data/remote/model/response/BasicResponse.kt (1)
5-11: 백엔드 응답에서data필드가 null이거나 누락될 때 디코딩 실패 위험kotlinx.serialization에서 generic type
T를 사용하는 경우, 기본값이 없는 non-nullable 필드는 누락되거나 null인 데이터를 받으면MissingFieldException을 발생시킬 수 있습니다. 특히 에러 응답에서data필드가 생략되거나 null로 반환되는 경우 문제가 될 수 있으므로, nullable 기본값을 추가하는 것이 안전합니다.@Serializable data class BasicResponse<T>( val resultCode: String, val message: String, val success: Boolean, - val data: T, + val data: T? = null, )app/src/main/AndroidManifest.xml (1)
29-39: 이 설정은 Kakao SDK 공식 가이드와 정확히 일치하므로 추가 조정이 불필요합니다공식 Kakao Android SDK 문서에서는 AuthCodeHandlerActivity 설정 시
launchMode나noHistory속성을 요구하지 않습니다. 필요한 속성은 현재 코드에 모두 포함되어 있습니다:
- ✓
android:exported="true"(Android 12+)- ✓ intent-filter:
android.intent.action.VIEW- ✓ categories:
DEFAULT,BROWSABLE- ✓ data:
kakao${KAKAO_NATIVE_APP_KEY}://oauth또한
app/build.gradle.kts에서manifestPlaceholders["KAKAO_NATIVE_APP_KEY"]가 올바르게 설정되어 있어 빌드 시점에 값이 정상 주입됩니다. 현재 구성은 공식 가이드를 정확히 따르고 있습니다.Likely an incorrect or invalid review comment.
app/build.gradle.kts (1)
42-43: LGTM! 의존성 추가 적절새로운 auth 기능 모듈의 api와 impl 의존성이 올바르게 추가되었습니다. 기존 패턴(api/impl 분리)을 잘 따르고 있습니다.
core/data/src/main/java/com/neki/android/core/data/remote/model/request/KakaoLoginRequest.kt (1)
6-9: LGTM! 간결한 요청 모델
KakaoLoginRequest데이터 클래스가 올바르게 구현되었습니다. kotlinx.serialization을 적절히 활용하고 있으며, 단일 필드를 가진 요청 모델로서 적절한 구조입니다.core/data-api/src/main/java/com/neki/android/core/dataapi/repository/DataStoreRepository.kt (1)
6-9:saveJwtTokens메서드명 변경 완료 - 명확성 개선
saveTokens에서saveJwtTokens로 변경하여 토큰 타입을 명시적으로 표현했습니다. 검증 결과 모든 호출 위치가 올바르게 업데이트되었습니다.
- 인터페이스 정의 (DataStoreRepository.kt:6) ✓
- 구현 클래스 (DataStoreRepositoryImpl.kt:21) ✓
- 호출 위치 (LoginViewModel.kt:69, NetworkModule.kt:104) ✓
- 기존 메서드명 호출 없음 ✓
Breaking change가 완벽하게 적용되었습니다.
core/data/src/main/java/com/neki/android/core/data/repository/di/RepositoryModule.kt (2)
3-7: LGTM!AuthRepository와 AuthEventManager 관련 import가 올바르게 추가되었습니다.
25-35: LGTM!AuthRepository와 AuthEventManager의 DI 바인딩이 올바르게 구성되었습니다. Singleton 스코프가 적절하게 적용되어 있으며, 기존 DataStoreRepository 바인딩과 일관된 패턴을 따르고 있습니다.
gradle/libs.versions.toml (2)
70-70: LGTM!ktor-client-auth 라이브러리가 올바르게 추가되었습니다. 기존 ktor 버전 참조를 사용하여 버전 일관성이 유지됩니다.
85-85: LGTM!kakao-user 라이브러리가 올바르게 추가되었습니다.
core/data-api/src/main/java/com/neki/android/core/dataapi/repository/AuthRepository.kt (1)
1-8: LGTM!AuthRepository 인터페이스가 잘 설계되었습니다. Result 반환 타입을 사용하여 에러 핸들링이 명확하고, suspend 함수로 비동기 작업을 적절하게 처리합니다. 책임이 명확하게 분리되어 있습니다.
settings.gradle.kts (2)
22-22: LGTM!카카오 Maven 저장소가 올바르게 추가되었습니다.
36-37: LGTM!feature:auth:api와 feature:auth:impl 모듈이 올바르게 추가되었습니다. 기존 feature 모듈들과 일관된 네이밍 패턴을 따르고 있습니다.
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/di/LoginEntryProvider.kt (2)
15-24: LGTM!LoginEntryProvider 모듈이 올바르게 구성되었습니다. ActivityRetainedComponent 스코프는 설정 변경 시에도 유지되어야 하는 navigation entry에 적합합니다. Navigator 인스턴스를 주입받아 entry installer 람다로 제공하는 패턴이 올바릅니다.
26-32: LGTM!loginEntry 확장 함수가 올바르게 구현되었습니다. LoginNavKey.Login을 등록하고 navigator::goBack을 navigateBack 콜백으로 연결하는 구조가 적절합니다.
core/data-api/src/main/java/com/neki/android/core/dataapi/auth/AuthEventManager.kt (1)
5-12: LGTM! 깔끔한 인터페이스 설계입니다.
SharedFlow를 사용한 인증 이벤트 브로드캐스팅과 sealed class를 통한 이벤트 타입 정의가 적절합니다. 향후 다른 인증 관련 이벤트(예: 로그아웃, 세션 만료 등)를 쉽게 확장할 수 있는 구조입니다.core/data/src/main/java/com/neki/android/core/data/repository/impl/DataStoreRepositoryImpl.kt (1)
31-38: LGTM! 토큰 존재 여부 확인 로직이 적절합니다.복호화 후 두 토큰의 유효성을 확인하는 방식이 올바릅니다. 성능이 중요한 경우, 복호화 없이 단순 존재 여부만 먼저 확인하는 방법도 고려할 수 있지만, 현재 구현도 데이터 무결성 측면에서 더 안전합니다.
core/data/src/main/java/com/neki/android/core/data/repository/impl/AuthRepositoryImpl.kt (1)
11-28: LGTM! 깔끔한 Repository 구현입니다.
runSuspendCatching을 사용하여CancellationException을 적절히 전파하고, API 호출 결과를 도메인 모델로 변환하는 패턴이 잘 적용되어 있습니다.core/data/src/main/java/com/neki/android/core/data/remote/model/request/RefreshTokenRequest.kt (1)
6-9: LGTM!
@SerialName이 필드명과 동일하여 생략 가능하지만, 명시적으로 지정하는 것도 일관성 측면에서 나쁘지 않습니다.core/data/src/main/java/com/neki/android/core/data/auth/AuthEventManagerImpl.kt (1)
13-18: LGTM! 적절한 SharedFlow 구성입니다.
extraBufferCapacity = 1과tryEmit()의 조합이 적절합니다. 토큰 만료 이벤트는 상태적 특성을 가지므로, 버퍼가 가득 찬 경우 이벤트가 드롭되더라도 큰 문제가 되지 않습니다. 다만, 이벤트 손실이 절대 발생하면 안 되는 경우라면replay = 1을 고려할 수 있습니다.core/data/build.gradle.kts (1)
39-39: LGTM! ktor-client-auth 의존성 추가JWT 토큰 갱신 로직 구현을 위한 적절한 의존성입니다.
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginScreen.kt (1)
62-70: LGTM!
LoginScreencomposable이 stateless하게 잘 구현되었으며, preview와 테스트가 용이한 구조입니다.core/data/src/main/java/com/neki/android/core/data/remote/api/ApiService.kt (1)
16-24: LGTM!API 엔드포인트가 명확하게 정의되었으며, 요청/응답 모델 사용이 적절합니다.
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginActivity.kt (1)
28-30:navigateBack빈 람다 처리현재
navigateBack = {}로 빈 람다를 전달하고 있습니다. PR 설명에 따르면 Navigation 3로 리팩토링 예정이므로 현재는 괜찮지만,LoginRoute에서 해당 파라미터를 사용하지 않는다면 optional로 변경하는 것도 고려해볼 수 있습니다.
Comment on lines
+116
to
+121
| } catch (e: Exception) { | ||
| Timber.e(e) | ||
| dataStoreRepository.clearTokens() | ||
| authEventManager.emitTokenExpired() | ||
| null | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the NetworkModule.kt file
find . -name "NetworkModule.kt" -type fRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 158
🏁 Script executed:
# Get file size to determine read strategy
wc -l core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 160
🏁 Script executed:
# Read the specific section and context around lines 116-121
sed -n '100,140p' core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt | cat -nRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 2135
🏁 Script executed:
# Search for TooGenericExceptionCaught in the repository
rg "TooGenericExceptionCaught" --type kotlin --type javaRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 61
🏁 Script executed:
# Check git log for PR description or commit messages mentioning exception handling
git log --oneline --all --grep="TooGenericExceptionCaught" | head -20Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 141
🏁 Script executed:
# Look for any lint suppression annotations around the catch block
sed -n '110,125p' core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt | cat -nRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 929
🏁 Script executed:
# Get more context about the token refresh flow - read from line 80 onwards
sed -n '80,130p' core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt | cat -nRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 3020
🏁 Script executed:
# Check the git commit details for context about why TooGenericExceptionCaught was disabled
git show 015d8ff --stat | head -30Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 1791
🏁 Script executed:
# Look for retry logic or error handling patterns in the codebase
rg "refreshToken\|tokenExpired" --type kotlin -B 2 -A 2 | head -60Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 61
예외 타입별 처리 필요
네트워크 오류, 서버 오류, 토큰 만료 등 모든 예외를 catch (e: Exception)으로 동일하게 처리하고 있습니다. 이로 인해 일시적인 네트워크 오류(타임아웃, 연결 실패)가 발생해도 토큰을 삭제하고 사용자에게 재로그인을 강제하게 됩니다.
예외 타입을 구분하여 처리해야 합니다:
HttpException(401)또는 토큰 만료 관련 예외 → 토큰 삭제, 재로그인 유도- 네트워크 예외(ConnectionException, TimeoutException 등) → 재시도 로직 적용
- 기타 예외 → 적절한 사용자 안내
🤖 Prompt for AI Agents
In
@core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt
around lines 116 - 121, The current catch-all in NetworkModule.kt that logs the
exception, calls dataStoreRepository.clearTokens() and
authEventManager.emitTokenExpired() must be replaced with exception-specific
handling: catch HttpException and if e.code() == 401 (or other token-expiry
indicators) then clear tokens via dataStoreRepository.clearTokens() and call
authEventManager.emitTokenExpired(); catch network-related exceptions (e.g.,
IOException, SocketTimeoutException, ConnectException) and trigger a
retry/backoff mechanism or rethrow so callers can retry instead of clearing
tokens; catch other unexpected Exceptions to log and surface a user-friendly
error without forcing logout. Locate the try/catch around the network call in
NetworkModule.kt and replace the single catch(Exception) with multiple catches
for HttpException, IOException (and its timeout/connect subclasses), and a final
generic catch that avoids token deletion.
Comment on lines
+52
to
+54
| exceptions: | ||
| TooGenericExceptionCaught: | ||
| active: false |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Explore repository structure first
echo "=== Repository structure ==="
git ls-files | head -20
echo ""
# Check if there are Kotlin files
echo "=== Finding Kotlin files ==="
fd -t f -e kt -e kts | head -20Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 2215
🏁 Script executed:
#!/bin/bash
# Find detekt-config.yml
echo "=== Finding detekt-config.yml ==="
fd -t f -i detekt-config.yml
echo ""
echo "=== Finding all config files ==="
fd -t f \( -name "*.yml" -o -name "*.yaml" \) | grep -i detekt
echo ""
echo "=== Looking for auth/token related Kotlin files ==="
fd -t f -e kt | grep -i "auth\|token\|login" | head -20Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 1850
🏁 Script executed:
#!/bin/bash
# Read detekt-config.yml
echo "=== detekt-config.yml (lines 40-70) ==="
cat -n detekt-config.yml | sed -n '40,70p'
echo ""
echo "=== Searching for catch blocks with Exception or Throwable ==="
rg "catch\s*\(\s*\w+\s*:\s*(Exception|Throwable)" --type kotlin -n
echo ""
echo "=== Checking AuthRepositoryImpl for exception handling ==="
cat -n core/data/src/main/java/com/neki/android/core/data/repository/impl/AuthRepositoryImpl.ktRepository: YAPP-Github/27th-App-Team-2-Android
Length of output: 2575
🏁 Script executed:
#!/bin/bash
# Check NetworkModule.kt for the catch block
echo "=== NetworkModule.kt around line 116 ==="
cat -n core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt | sed -n '110,125p'
echo ""
echo "=== Find runSuspendCatching definition ==="
rg -A 15 "fun runSuspendCatching" --type kotlin
echo ""
echo "=== Search for all catch statements more broadly ==="
rg -n "catch\s*\(" --type kotlin | head -20Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 1203
🏁 Script executed:
#!/bin/bash
# Get more context from NetworkModule.kt
echo "=== NetworkModule.kt around token refresh (lines 100-135) ==="
cat -n core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt | sed -n '100,135p'
echo ""
echo "=== Search for runSuspendCatching file ==="
fd -t f -e kt | xargs grep -l "fun runSuspendCatching" 2>/dev/null
echo ""
echo "=== Search for HttpException or similar exception types ==="
rg "import.*Exception" --type kotlin | grep -i "http\|retrofit\|exception" | head -10Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 2193
🏁 Script executed:
#!/bin/bash
# Read RunSuspendCatching.kt
echo "=== RunSuspendCatching.kt ==="
cat -n core/data/src/main/java/com/neki/android/core/data/util/RunSuspendCatching.kt
echo ""
echo "=== NetworkModule.kt token refresh full context (lines 85-125) ==="
cat -n core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt | sed -n '85,125p'
echo ""
echo "=== Search for imports in NetworkModule.kt ==="
head -50 core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt | grep "^import"Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 5188
🏁 Script executed:
#!/bin/bash
# Check if Ktor exception types are already available
echo "=== Search for existing Ktor exception imports/usage ==="
rg "ClientRequestException|ServerResponseException|SerializationException" --type kotlin
echo ""
echo "=== Look for full NetworkModule.kt to understand structure ==="
wc -l core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt
echo ""
echo "=== Check Ktor version in dependencies ==="
grep -r "ktor" build-logic/src/main/java/com/neki/android/buildlogic/extensions/LibsExtension.kt || grep -r "ktor" app/build.gradle.kts || grep -r "ktor" core/data/build.gradle.kts || find . -name "*.toml" -o -name "*.gradle.kts" | xargs grep -l "ktor" 2>/dev/null | head -5Repository: YAPP-Github/27th-App-Team-2-Android
Length of output: 617
토큰 갱신 실패 처리에서 특정 예외 타입을 명시적으로 캐치하세요.
NetworkModule.kt 라인 116의 catch (e: Exception) 블록에서 Ktor 클라이언트의 구체적인 예외 타입(ClientRequestException, ServerResponseException, SerializationException 등)을 캐치하면 TooGenericExceptionCaught 규칙을 활성화할 수 있습니다. 현재 구현은 모든 예외를 동일하게 처리하고 있으므로, 특정 예외 타입으로 변경해도 동작은 변하지 않습니다.
수정 예시
try {
// ... token refresh logic
} catch (e: ClientRequestException) {
Timber.e(e)
dataStoreRepository.clearTokens()
authEventManager.emitTokenExpired()
null
} catch (e: ServerResponseException) {
Timber.e(e)
dataStoreRepository.clearTokens()
authEventManager.emitTokenExpired()
null
} catch (e: Exception) {
Timber.e(e)
dataStoreRepository.clearTokens()
authEventManager.emitTokenExpired()
null
}이후 detekt-config.yml의 TooGenericExceptionCaught 규칙을 다시 활성화하세요.
🤖 Prompt for AI Agents
In @detekt-config.yml around lines 52 - 54, Replace the broad catch (e:
Exception) in NetworkModule.kt (around the token refresh logic) with explicit
catches for Ktor/serialization exceptions such as ClientRequestException,
ServerResponseException, and SerializationException, handling each by calling
Timber.e(e), dataStoreRepository.clearTokens(), and
authEventManager.emitTokenExpired() (and keep a final generic catch for any
other Exception if needed); after updating the catch blocks, re-enable the
TooGenericExceptionCaught rule in detekt-config.yml.
Ojongseok
commented
Jan 11, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt (1)
59-77:expectSuccess = true가 토큰 갱신 실패 처리를 “예외”로 몰아갈 수 있음현재 refresh 로직이
try/catch(Exception)에 의존하는 구조라, 서버 5xx/타임아웃 같은 “일시 장애”도 토큰 클리어 + 강제 재로그인으로 이어질 수 있습니다. 의도라면 OK지만, 아니라면 refresh 실패 분기(특히 401/400 vs 네트워크)를 분리하는 쪽이 안정적입니다.
🤖 Fix all issues with AI agents
In
@core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt:
- Around line 81-89: Avoid using !! and separate .first() calls inside the
loadTokens block: instead fetch access and refresh in a single atomic snapshot
(e.g., combine(dataStoreRepository.getAccessToken(),
dataStoreRepository.getRefreshToken()).first() or add a getTokens() that returns
both) and then null/blank-check both before constructing BearerTokens; if either
is null/blank return null. Update the loadTokens lambda to remove forced !!, use
the combined result to prevent mismatched snapshots, and validate token contents
before returning BearerTokens.
- Around line 91-123: Remove the Timber logs that print tokens in the
refreshTokens block, stop re-reading refreshToken from dataStoreRepository and
instead use the provided oldTokens.refreshToken inside refreshTokens, and mark
the refresh HTTP call as a refresh request by calling
markAsRefreshTokenRequest() on the request created in
client.post("/api/auth/refresh") to avoid recursive interception; additionally,
improve error handling in the catch: only clear tokens and call
authEventManager.emitTokenExpired() for definitive auth failures (e.g., 401/403
or explicit auth error from the response) and for network/5xx errors propagate
or retry appropriately instead of forcing logout (use specific exception checks
or inspect the response status), and keep references to
dataStoreRepository.saveJwtTokens, dataStoreRepository.clearTokens, and
authEventManager.emitTokenExpired() when implementing these changes.
🧹 Nitpick comments (2)
core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt (2)
43-49:BASE_URL,sendWithoutJwtUrls는 외부 노출 최소화(privacy) 추천
NetworkModule내부에서만 쓰는 값이면private로 제한하는 게 안전합니다(특히 allowlist는 실수로 다른 곳에서 재사용/변경되는 걸 막는 효과). 또한sendWithoutJwtUrls는 “JWT 제외 경로” 의미가 더 드러나는 이름이 가독성이 좋습니다.Proposed diff
- val BASE_URL = BuildConfig.BASE_URL + private val BASE_URL = BuildConfig.BASE_URL - val sendWithoutJwtUrls = listOf( + private val sendWithoutJwtUrls = setOf( "/api/auth/kakao/login", "/api/auth/refresh", )
125-132:sendWithoutRequest경로 매칭/로깅이 혼동 소지 + 정확히 일치(==)는 취약할 수 있음
shouldNotJwt는 “JWT 제외”인데 로그 문구가 “Bearer 인증 필요”라 의미가 뒤집혀 보입니다.encodedPath == it는 trailing slash, 하위 경로(예:/api/auth/kakao/login/...)에서 누락될 수 있어startsWith같은 정책이 더 안전할 때가 많습니다(의도에 따라).Proposed diff
sendWithoutRequest { request -> - val shouldNotJwt = sendWithoutJwtUrls.any { - request.url.encodedPath == it - } - - Timber.d("Bearer 인증 필요 API 여부 : $shouldNotJwt") - !shouldNotJwt + val jwtExcluded = sendWithoutJwtUrls.any { excluded -> + request.url.encodedPath.startsWith(excluded) + } + Timber.d("JWT 제외 경로 여부: $jwtExcluded (${request.url.encodedPath})") + !jwtExcluded }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.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: ci-build
🔇 Additional comments (1)
core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt (1)
79-134: 토큰 로깅 및 refresh 실패 처리 정책 검토 필요토큰 값이 DEBUG 레벨의 Timber 로그에 직접 기록되고 있습니다(93, 109, 110번 줄):
Timber.d("RefreshToken : ${...}")Timber.d("New AccessToken : ${...}")Timber.d("New RefreshToken : ${...}")현재 Timber 설정상 RELEASE 빌드에서는 로그가 억제되므로 프로덕션 환경은 안전하지만, 개발/스테이징 환경에서 디버그 로그 노출 시 민감정보 유출 위험이 있습니다. 토큰 값을 마스킹하거나 민감한 정보 로깅을 제거하는 것을 권장합니다.
또한 refresh 실패 시 모든 예외 유형(
catch (e: Exception))에 대해 동일하게 토큰 클리어와 TokenExpired 이벤트를 발생시키고 있습니다(118-119번 줄). 네트워크 타임아웃, 서버 5xx 에러 등 일시적 장애도 재로그인 필요 상태로 처리되는지 확인하고, 필요시 예외 유형별로 다른 처리 로직(재시도, 대기 등)을 적용하는 것을 검토해주세요.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt:
- Line 38: The handler for LoginIntent.FailLogin in LoginViewModel currently
posts a NavigateToKakaoRedirectingUri side effect automatically; change this to
surface an error state/side effect instead of auto-redirecting by replacing the
postSideEffect(LoginSideEffect.NavigateToKakaoRedirectingUri) call with a call
that exposes an error message (e.g.,
postSideEffect(LoginSideEffect.ShowLoginError or update a LiveData/State like
loginError) and ensure the UI shows the error and presents an explicit retry
action; keep NavigateToKakaoRedirectingUri only for explicit user-triggered
retry flows.
- Around line 52-55: The onFailure block currently only logs the exception via
Timber.d and calls authEventManager.emitTokenExpired; add a user-facing UI
update so users see the token refresh failure (e.g., emit a SideEffect or update
view state to show an error/toast). Inside the onFailure of the token refresh
call, send a UI event (for example emit a LoginSideEffect.ShowError or update a
LiveData/StateFlow error field) alongside authEventManager.emitTokenExpired and
include the error message from the exception (it.message) to populate the
user-visible message.
- Around line 46-50: The code uses a forced non-null assertion on
getRefreshToken() inside the branch guarded by
dataStoreRepository.isSavedJwtTokens(), which can still produce an NPE; change
the logic in LoginViewModel so you safely retrieve and validate the refresh
token before calling authRepository.updateAccessToken: call
dataStoreRepository.getRefreshToken().first(), check for null (or empty) and
handle it (e.g., log/warn and skip/update token flow or clear saved tokens and
return) instead of using !!, then pass the non-null value to
authRepository.updateAccessToken(refreshToken = ...) or bail out gracefully.
- Around line 77-79: The onFailure block currently only logs errors with
Timber.d and fails to reset isLoading or inform the UI; update the onFailure
handler in LoginViewModel (the lambda passed to .onFailure) to set isLoading =
false, emit a SideEffect of type ShowLoginError (add ShowLoginError to
LoginContract if missing) carrying a user-friendly message or the error reason,
and replace or complement Timber.d(it.message.toString()) with Timber.e(it) for
proper error logging.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt (1)
core/ui/src/main/java/com/neki/android/core/ui/MviIntentStore.kt (2)
mviIntentStore(53-60)postSideEffect(39-41)
⏰ 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: ci-build
🔇 Additional comments (1)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt (1)
16-26: LGTM! 의존성 주입과 MVI 스토어 초기화가 올바릅니다.HiltViewModel 설정과 MviIntentStore 초기화가 적절하게 구현되어 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
@core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt:
- Around line 82-90: The loadTokens block creates a BearerTokens with empty
strings when individual token flows are null, causing invalid Authorization
headers; change loadTokens to read access and refresh tokens using firstOrNull()
without falling back to "" and only construct and return
BearerTokens(accessToken, refreshToken) if both tokens are non-null (and
optionally non-blank); otherwise return null so no Authorization header is sent.
Ensure you update the loadTokens lambda that references
dataStoreRepository.getAccessToken(), dataStoreRepository.getRefreshToken(), and
BearerTokens accordingly.
- Around line 96-102: The code currently calls client.post("/api/auth/refresh")
with RefreshTokenRequest(refreshToken =
dataStoreRepository.getRefreshToken().firstOrNull() ?: ""), which sends an empty
token; instead read val refreshToken =
dataStoreRepository.getRefreshToken().firstOrNull() and if refreshToken is null
or blank perform an early return (or propagate an appropriate error) before
calling client.post, then use refreshToken in RefreshTokenRequest; update the
logic around client.post("/api/auth/refresh"), RefreshTokenRequest and
dataStoreRepository.getRefreshToken() accordingly.
In
@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt:
- Around line 47-56: You call authRepository.updateAccessToken(...) even when
refresh token may be absent; first retrieve the refresh token from
dataStoreRepository.getRefreshToken().firstOrNull(), check if it is null or
blank, and only call authRepository.updateAccessToken(refreshToken) when
non-empty; if it is missing, skip the API call and handle the case (e.g., call
authEventManager.emitTokenExpired() or postSideEffect as appropriate) instead of
passing an empty string to updateAccessToken.
- Around line 77-79: In loginWithKakao's onFailure block replace the silent
Timber.d(...) with code that surfaces the error to the UI—emit the existing
FailLogin intent (or invoke the view event/toast helper used elsewhere)
including a user-friendly message and the error detail; locate the onFailure in
LoginViewModel.loginWithKakao and dispatch FailLogin (or call the same
toast/showError method used by the FailLogin intent) so users see feedback
instead of just a log.
🧹 Nitpick comments (3)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt (1)
43-60: 토큰 검증 중 로딩 상태 누락앱 진입 시 저장된 토큰을 확인하고 갱신하는 동안
isLoading상태가 설정되지 않아, 사용자에게 로그인 화면이 잠깐 보였다가 홈으로 이동하는 깜빡임 현상이 발생할 수 있습니다.♻️ 로딩 상태 추가 제안
private fun checkLoginState( reduce: (LoginState.() -> LoginState) -> Unit, postSideEffect: (LoginSideEffect) -> Unit, ) = viewModelScope.launch { + reduce { copy(isLoading = true) } if (dataStoreRepository.isSavedJwtTokens().first()) { Timber.d("JWT 토큰 O") authRepository.updateAccessToken( refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "", ).onSuccess { postSideEffect(LoginSideEffect.NavigateToHome) }.onFailure { Timber.d(it.message.toString()) authEventManager.emitTokenExpired() + reduce { copy(isLoading = false) } } } else { Timber.d("JWT 토큰 X") + reduce { copy(isLoading = false) } } }core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt (2)
122-129: 이중 부정 로직을 단순화하여 가독성 개선 권장.
shouldNotJwt변수명과!shouldNotJwt반환 로직이 이중 부정을 형성하여 가독성을 떨어뜨립니다.♻️ 긍정형 변수명으로 리팩토링
sendWithoutRequest { request -> - val shouldNotJwt = sendWithoutJwtUrls.any { + val shouldSendJwt = sendWithoutJwtUrls.none { request.url.encodedPath == it } - Timber.d("Bearer 인증 필요 API 여부 : $shouldNotJwt") - !shouldNotJwt + Timber.d("Bearer 인증 필요 API 여부 : $shouldSendJwt") + shouldSendJwt }또는:
sendWithoutRequest { request -> - val shouldNotJwt = sendWithoutJwtUrls.any { + val isPublicEndpoint = sendWithoutJwtUrls.any { request.url.encodedPath == it } - Timber.d("Bearer 인증 필요 API 여부 : $shouldNotJwt") - !shouldNotJwt + Timber.d("Bearer 인증 필요 API 여부 : ${!isPublicEndpoint}") + !isPublicEndpoint }
113-119: 토큰 갱신 실패 시 오류 유형별 처리 권장.현재 모든 예외를 동일하게 처리하여 토큰을 삭제하고 만료 이벤트를 발생시킵니다. 네트워크 오류와 인증 오류를 구분하면 더 나은 사용자 경험을 제공할 수 있습니다:
- 네트워크 오류: 재시도 로직 또는 사용자에게 연결 오류 안내
- 401/403 인증 오류: 토큰 만료로 간주하여 재로그인 유도
PR 노트에서
TooGenericExceptionCaught를 의도적으로 비활성화했음을 확인했지만, 향후 개선을 위해 제안합니다.♻️ 오류 유형별 처리 예시
+import io.ktor.client.plugins.ClientRequestException +import io.ktor.http.HttpStatusCode } catch (e: Exception) { - Timber.e(e) - dataStoreRepository.clearTokens() - authEventManager.emitTokenExpired() - null + when (e) { + is ClientRequestException -> { + when (e.response.status) { + HttpStatusCode.Unauthorized, HttpStatusCode.Forbidden -> { + Timber.e("Refresh Token 만료 또는 유효하지 않음") + dataStoreRepository.clearTokens() + authEventManager.emitTokenExpired() + } + else -> { + Timber.e("토큰 갱신 실패 (HTTP ${e.response.status}): ${e.message}") + dataStoreRepository.clearTokens() + authEventManager.emitTokenExpired() + } + } + } + else -> { + // 네트워크 오류 등: 일시적 문제일 수 있으므로 토큰은 유지하되 재시도 실패 시 처리 + Timber.e("토큰 갱신 중 예외 발생: ${e.message}", e) + // 향후 재시도 로직 추가 고려 + dataStoreRepository.clearTokens() + authEventManager.emitTokenExpired() + } + } + null }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.ktfeature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt (1)
core/ui/src/main/java/com/neki/android/core/ui/MviIntentStore.kt (2)
mviIntentStore(53-60)postSideEffect(39-41)
⏰ 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: ci-build
🔇 Additional comments (4)
feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt (2)
17-27: LGTM!Hilt DI와 MVI 패턴 설정이 적절합니다.
internal가시성으로 feature 모듈 내부에서만 접근 가능하도록 한 점도 좋습니다.
29-41: LGTM!Intent 핸들링 구조가 깔끔하고 MVI 패턴을 잘 따르고 있습니다.
core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt (2)
4-4: 동적 BASE_URL 구성이 적절합니다.BuildConfig에서 BASE_URL을 가져오도록 변경한 것은 환경별(dev/staging/prod) URL 관리에 유용합니다.
Also applies to: 44-44
47-50: JWT 미포함 엔드포인트 설정이 정확합니다.Android 클라이언트 측 구현이 올바르게 되어 있습니다:
/api/auth/kakao/login: 새로운 로그인 요청이므로 JWT 제외 필요 (✓)/api/auth/refresh: Refresh Token을 사용한 갱신이므로 JWT 제외 필요 (✓)
sendWithoutRequest블록에서 요청 경로를sendWithoutJwtUrls목록과 매칭하여 올바르게 필터링하고 있습니다. Refresh Token 검증, 자동 토큰 갱신, 만료 시 이벤트 발생 등 토큰 관리 로직도 완벽하게 구현되어 있습니다.
Comment on lines
+82
to
+90
| loadTokens { | ||
| Timber.d("BearerAuth - loadTokens") | ||
| if (dataStoreRepository.isSavedJwtTokens().first()) { | ||
| BearerTokens( | ||
| accessToken = dataStoreRepository.getAccessToken().firstOrNull() ?: "", | ||
| refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "", | ||
| ) | ||
| } else null | ||
| } |
There was a problem hiding this comment.
토큰이 없을 때 빈 문자열 대신 null 반환 필요.
isSavedJwtTokens()가 true를 반환하더라도 개별 토큰 flow가 null을 반환하면 빈 문자열로 BearerTokens를 생성합니다. 이는 잘못된 Authorization 헤더를 전송하여 인증 실패를 유발할 수 있습니다.
🔒 토큰 null 처리 개선안
loadTokens {
Timber.d("BearerAuth - loadTokens")
if (dataStoreRepository.isSavedJwtTokens().first()) {
- BearerTokens(
- accessToken = dataStoreRepository.getAccessToken().firstOrNull() ?: "",
- refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "",
- )
+ val accessToken = dataStoreRepository.getAccessToken().firstOrNull()
+ val refreshToken = dataStoreRepository.getRefreshToken().firstOrNull()
+ if (accessToken != null && refreshToken != null) {
+ BearerTokens(
+ accessToken = accessToken,
+ refreshToken = refreshToken,
+ )
+ } else {
+ Timber.w("토큰이 저장되어 있다고 표시되었으나 실제 값이 없음")
+ dataStoreRepository.clearTokens()
+ null
+ }
} else null
}📝 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
| loadTokens { | |
| Timber.d("BearerAuth - loadTokens") | |
| if (dataStoreRepository.isSavedJwtTokens().first()) { | |
| BearerTokens( | |
| accessToken = dataStoreRepository.getAccessToken().firstOrNull() ?: "", | |
| refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "", | |
| ) | |
| } else null | |
| } | |
| loadTokens { | |
| Timber.d("BearerAuth - loadTokens") | |
| if (dataStoreRepository.isSavedJwtTokens().first()) { | |
| val accessToken = dataStoreRepository.getAccessToken().firstOrNull() | |
| val refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() | |
| if (accessToken != null && refreshToken != null) { | |
| BearerTokens( | |
| accessToken = accessToken, | |
| refreshToken = refreshToken, | |
| ) | |
| } else { | |
| Timber.w("토큰이 저장되어 있다고 표시되었으나 실제 값이 없음") | |
| dataStoreRepository.clearTokens() | |
| null | |
| } | |
| } else null | |
| } |
🤖 Prompt for AI Agents
In
@core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt
around lines 82 - 90, The loadTokens block creates a BearerTokens with empty
strings when individual token flows are null, causing invalid Authorization
headers; change loadTokens to read access and refresh tokens using firstOrNull()
without falling back to "" and only construct and return
BearerTokens(accessToken, refreshToken) if both tokens are non-null (and
optionally non-blank); otherwise return null so no Authorization header is sent.
Ensure you update the loadTokens lambda that references
dataStoreRepository.getAccessToken(), dataStoreRepository.getRefreshToken(), and
BearerTokens accordingly.
Comment on lines
+92
to
+120
| refreshTokens { | ||
| Timber.d("BearerAuth - AccessToken 갱신 시도") | ||
| if (oldTokens != null) { | ||
| return@refreshTokens try { | ||
| val response = client.post("/api/auth/refresh") { | ||
| setBody( | ||
| RefreshTokenRequest( | ||
| refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "", | ||
| ), | ||
| ) | ||
| }.body<BasicResponse<AuthResponse>>() | ||
|
|
||
| dataStoreRepository.saveJwtTokens( | ||
| accessToken = response.data.accessToken, | ||
| refreshToken = response.data.refreshToken, | ||
| ) | ||
|
|
||
| BearerTokens( | ||
| accessToken = response.data.accessToken, | ||
| refreshToken = response.data.refreshToken, | ||
| ) | ||
| } catch (e: Exception) { | ||
| Timber.e(e) | ||
| dataStoreRepository.clearTokens() | ||
| authEventManager.emitTokenExpired() | ||
| null | ||
| } | ||
| } else null | ||
| } |
There was a problem hiding this comment.
동시 토큰 갱신 요청에 대한 경쟁 조건(race condition) 발생 가능.
여러 API 요청이 동시에 만료된 토큰을 감지하면 각각 refreshTokens를 호출하여 중복된 갱신 요청이 발생할 수 있습니다. 이는 다음 문제를 야기합니다:
- 불필요한 중복 API 호출
- 서버가 갱신 시도 횟수를 제한하는 경우 계정 잠금 위험
- 토큰 저장 시 race condition
Ktor의 bearer 인증은 기본적으로 동기화를 제공하지 않으므로 명시적인 처리가 필요합니다.
🔒 Mutex를 사용한 토큰 갱신 동기화
NetworkModule 상단에 Mutex 추가:
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
@Module
@InstallIn(SingletonComponent::class)
internal object NetworkModule {
private val tokenRefreshMutex = Mutex()
// ... rest of code
}refreshTokens 블록에 Mutex 적용:
refreshTokens {
Timber.d("BearerAuth - AccessToken 갱신 시도")
- if (oldTokens != null) {
- return@refreshTokens try {
- val response = client.post("/api/auth/refresh") {
- setBody(
- RefreshTokenRequest(
- refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "",
- ),
- )
- }.body<BasicResponse<AuthResponse>>()
-
- dataStoreRepository.saveJwtTokens(
- accessToken = response.data.accessToken,
- refreshToken = response.data.refreshToken,
- )
-
- BearerTokens(
- accessToken = response.data.accessToken,
- refreshToken = response.data.refreshToken,
- )
- } catch (e: Exception) {
- Timber.e(e)
- dataStoreRepository.clearTokens()
- authEventManager.emitTokenExpired()
- null
- }
- } else null
+ tokenRefreshMutex.withLock {
+ // 갱신 중 다른 요청이 이미 토큰을 갱신했을 수 있으므로 재확인
+ val currentAccessToken = dataStoreRepository.getAccessToken().firstOrNull()
+ if (currentAccessToken != null && currentAccessToken != oldTokens?.accessToken) {
+ Timber.d("다른 요청이 이미 토큰을 갱신함")
+ return@withLock BearerTokens(
+ accessToken = currentAccessToken,
+ refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "",
+ )
+ }
+
+ if (oldTokens != null) {
+ return@withLock try {
+ val response = client.post("/api/auth/refresh") {
+ setBody(
+ RefreshTokenRequest(
+ refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "",
+ ),
+ )
+ }.body<BasicResponse<AuthResponse>>()
+
+ dataStoreRepository.saveJwtTokens(
+ accessToken = response.data.accessToken,
+ refreshToken = response.data.refreshToken,
+ )
+
+ BearerTokens(
+ accessToken = response.data.accessToken,
+ refreshToken = response.data.refreshToken,
+ )
+ } catch (e: Exception) {
+ Timber.e(e)
+ dataStoreRepository.clearTokens()
+ authEventManager.emitTokenExpired()
+ null
+ }
+ } else null
+ }
}
Comment on lines
+96
to
+102
| val response = client.post("/api/auth/refresh") { | ||
| setBody( | ||
| RefreshTokenRequest( | ||
| refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "", | ||
| ), | ||
| ) | ||
| }.body<BasicResponse<AuthResponse>>() |
There was a problem hiding this comment.
Refresh Token이 없을 때 빈 문자열 대신 조기 반환 필요.
Line 99에서 firstOrNull() ?: ""를 사용하면 Refresh Token이 없을 때도 빈 문자열로 API 요청을 시도합니다. 이는 불필요한 API 호출과 서버 측 오류를 발생시킵니다.
♻️ Refresh Token null 체크 추가
refreshTokens {
Timber.d("BearerAuth - AccessToken 갱신 시도")
if (oldTokens != null) {
+ val refreshToken = dataStoreRepository.getRefreshToken().firstOrNull()
+ if (refreshToken.isNullOrEmpty()) {
+ Timber.w("Refresh Token이 없어 갱신 불가")
+ dataStoreRepository.clearTokens()
+ authEventManager.emitTokenExpired()
+ return@refreshTokens null
+ }
return@refreshTokens try {
val response = client.post("/api/auth/refresh") {
setBody(
RefreshTokenRequest(
- refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "",
+ refreshToken = refreshToken,
),
)
}.body<BasicResponse<AuthResponse>>()🤖 Prompt for AI Agents
In
@core/data/src/main/java/com/neki/android/core/data/remote/di/NetworkModule.kt
around lines 96 - 102, The code currently calls client.post("/api/auth/refresh")
with RefreshTokenRequest(refreshToken =
dataStoreRepository.getRefreshToken().firstOrNull() ?: ""), which sends an empty
token; instead read val refreshToken =
dataStoreRepository.getRefreshToken().firstOrNull() and if refreshToken is null
or blank perform an early return (or propagate an appropriate error) before
calling client.post, then use refreshToken in RefreshTokenRequest; update the
logic around client.post("/api/auth/refresh"), RefreshTokenRequest and
dataStoreRepository.getRefreshToken() accordingly.
Comment on lines
+47
to
+56
| if (dataStoreRepository.isSavedJwtTokens().first()) { | ||
| Timber.d("JWT 토큰 O") | ||
| authRepository.updateAccessToken( | ||
| refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "", | ||
| ).onSuccess { | ||
| postSideEffect(LoginSideEffect.NavigateToHome) | ||
| }.onFailure { | ||
| Timber.d(it.message.toString()) | ||
| authEventManager.emitTokenExpired() | ||
| } |
There was a problem hiding this comment.
Refresh Token이 없는 경우 불필요한 API 호출 발생 가능
firstOrNull() ?: ""로 빈 문자열을 전달하면 실패가 예상되는 API 호출이 발생합니다. Refresh Token이 없는 경우를 먼저 확인하는 것이 좋습니다.
🔧 제안된 수정 사항
if (dataStoreRepository.isSavedJwtTokens().first()) {
Timber.d("JWT 토큰 O")
+ val refreshToken = dataStoreRepository.getRefreshToken().firstOrNull()
+ if (refreshToken.isNullOrEmpty()) {
+ Timber.d("Refresh Token 없음")
+ authEventManager.emitTokenExpired()
+ return@launch
+ }
authRepository.updateAccessToken(
- refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "",
+ refreshToken = refreshToken,
).onSuccess {
postSideEffect(LoginSideEffect.NavigateToHome)
}.onFailure {
Timber.d(it.message.toString())
authEventManager.emitTokenExpired()
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (dataStoreRepository.isSavedJwtTokens().first()) { | |
| Timber.d("JWT 토큰 O") | |
| authRepository.updateAccessToken( | |
| refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() ?: "", | |
| ).onSuccess { | |
| postSideEffect(LoginSideEffect.NavigateToHome) | |
| }.onFailure { | |
| Timber.d(it.message.toString()) | |
| authEventManager.emitTokenExpired() | |
| } | |
| if (dataStoreRepository.isSavedJwtTokens().first()) { | |
| Timber.d("JWT 토큰 O") | |
| val refreshToken = dataStoreRepository.getRefreshToken().firstOrNull() | |
| if (refreshToken.isNullOrEmpty()) { | |
| Timber.d("Refresh Token 없음") | |
| authEventManager.emitTokenExpired() | |
| return@launch | |
| } | |
| authRepository.updateAccessToken( | |
| refreshToken = refreshToken, | |
| ).onSuccess { | |
| postSideEffect(LoginSideEffect.NavigateToHome) | |
| }.onFailure { | |
| Timber.d(it.message.toString()) | |
| authEventManager.emitTokenExpired() | |
| } | |
| } |
🤖 Prompt for AI Agents
In
@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt
around lines 47 - 56, You call authRepository.updateAccessToken(...) even when
refresh token may be absent; first retrieve the refresh token from
dataStoreRepository.getRefreshToken().firstOrNull(), check if it is null or
blank, and only call authRepository.updateAccessToken(refreshToken) when
non-empty; if it is missing, skip the API call and handle the case (e.g., call
authEventManager.emitTokenExpired() or postSideEffect as appropriate) instead of
passing an empty string to updateAccessToken.
Comment on lines
+77
to
+79
| .onFailure { | ||
| Timber.d(it.message.toString()) | ||
| } |
There was a problem hiding this comment.
서버 로그인 실패 시 사용자 피드백 누락
loginWithKakao API 호출 실패 시 로그만 출력되고 사용자에게 피드백이 없습니다. FailLogin intent에서는 토스트를 보여주지만, 서버 로그인 실패 시에는 사용자가 로딩 후 아무 변화 없이 대기하게 됩니다.
🐛 사용자 피드백 추가 제안
.onFailure {
Timber.d(it.message.toString())
+ postSideEffect(LoginSideEffect.ShowToastMessage("로그인에 실패했습니다. 다시 시도해주세요."))
}📝 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
| .onFailure { | |
| Timber.d(it.message.toString()) | |
| } | |
| .onFailure { | |
| Timber.d(it.message.toString()) | |
| postSideEffect(LoginSideEffect.ShowToastMessage("로그인에 실패했습니다. 다시 시도해주세요.")) | |
| } |
🤖 Prompt for AI Agents
In
@feature/auth/impl/src/main/kotlin/com/neki/android/feature/auth/impl/LoginViewModel.kt
around lines 77 - 79, In loginWithKakao's onFailure block replace the silent
Timber.d(...) with code that surfaces the error to the UI—emit the existing
FailLogin intent (or invoke the view event/toast helper used elsewhere)
including a user-friendly message and the error detail; locate the onFailure in
LoginViewModel.loginWithKakao and dispatch FailLogin (or call the same
toast/showError method used by the FailLogin intent) so users see feedback
instead of just a log.
ikseong00
approved these changes
Jan 13, 2026
ikseong00
left a comment
ikseong00
left a comment
Contributor
There was a problem hiding this comment.
Q. 앱 진입 시 AccessToken, RefreshToken 확인 로직
이해했습니다!
Q. LoginScreen에서 카카오 로그인 성공/실패 여부를 viewModel.store.onIntent(LoginIntent.SuccessLogin(idToken))처럼 Intent를 정의해서onIntent{} 를 이용하는게 맞겠죠..? 성공/실패 여부에 따라 서버에도 전송 후 postSideEffect(LoginSideEffect.NavigateToHome) 홈화면으로 이동시킬 SideEffect를 발생시켜야 하니까요?
네네 이후 후처리를 고려하거나, 로직적 수정이 발생했을 때 대비해서 MVI 측면에서 좋은 것 같습니다!
Q. 403(RefreshToken까지 만료) 처리를 AuthEventManager/AuthEventManagerImpl을 통해 방출하고, MainActivity에서 옵저빙 중 RefreshTokenExpired 만료 이벤트가 발생한다면 로그인 화면으로 이동시키는 방법 밖에 떠오르지 않는데 좀 더 좋은 방법이 있는지 모르겠네요,, (-> 근데 RefreshToken의 주기는 2주~한달 정도로 길기 때문에 앱이 2주 넘게 죽지 않는 이상 앱을 사용하는 도중 RefreshToken이 만료되는 일은 없기는 합니다.)
카톡에서 얘기 나눈 것처럼 추후 서버측 토큰 주기가 길어졌을 때 수정하면 좋을 것 같습니다!
Ojongseok
added a commit
that referenced
this pull request
May 25, 2026
[feat] #12 카카오 로그인 구현 및 JWT 토큰 갱신 로직 구성
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
2 participants
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.
🔗 관련 이슈
📙 작업 설명
🧪 테스트 내역 (선택)
💬 추가 설명 or 리뷰 포인트 (선택)
Q. 앱 진입 시 AccessToken, RefreshToken 확인 로직
이해가 되실지 모르겠네요. 예전에는 이렇게 했었는데 일반적인 방법인지 흠
Q. LoginScreen에서 카카오 로그인 성공/실패 여부를
viewModel.store.onIntent(LoginIntent.SuccessLogin(idToken))처럼 Intent를 정의해서onIntent{}를 이용하는게 맞겠죠..? 성공/실패 여부에 따라 서버에도 전송 후postSideEffect(LoginSideEffect.NavigateToHome)홈화면으로 이동시킬 SideEffect를 발생시켜야 하니까요?Q. 403(RefreshToken까지 만료) 처리를 AuthEventManager/AuthEventManagerImpl을 통해 방출하고, MainActivity에서 옵저빙 중 RefreshTokenExpired 만료 이벤트가 발생한다면 로그인 화면으로 이동시키는 방법 밖에 떠오르지 않는데 좀 더 좋은 방법이 있는지 모르겠네요,, (-> 근데 RefreshToken의 주기는 2주~한달 정도로 길기 때문에 앱이 2주 넘게 죽지 않는 이상 앱을 사용하는 도중 RefreshToken이 만료되는 일은 없기는 합니다.)
Summary by CodeRabbit
새로운 기능
인증
기타
✏️ Tip: You can customize this high-level summary in your review settings.