-
Notifications
You must be signed in to change notification settings - Fork 4
[CHORE,FEAT] 플랫폼별 TokenManager 구현, Ktor client에서의 토큰 처리 구현 #21
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플랫폼별(안드로이드/iOS/JVM/wasmJs) DataStore와 공통 TokenManager 인터페이스·구현을 추가하고, DI에 바인딩했으며 HttpClientFactory 시그니처를 변경해 TokenManager 기반의 Bearer 인증 및 토큰 재발급 흐름을 도입했다. 빌드 의존성에 DataStore와 ktor-client-auth가 추가(중복 항목 포함)됐다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as TokenTestScreen
participant VM as TokenTestViewModel
participant TM as TokenManager
participant HC as HttpClientFactory/HttpClient
participant API as Server
rect rgba(220,240,255,0.6)
note over UI,VM: UI 초기 로드/상태 표시
User->>UI: 화면 진입
UI->>VM: observe tokens
VM->>TM: getAccessToken()/getRefreshToken()
TM-->>VM: tokens
VM-->>UI: update state
end
rect rgba(235,255,220,0.6)
note over HC,API: 요청 시 인증 + 재발급 흐름
HC->>TM: loadTokens (access/refresh)
TM-->>HC: tokens
HC->>API: Request with Authorization: Bearer <access>
alt 401 or expired
HC->>API: POST /member/reissue { refreshToken }
API-->>HC: 200 { accessToken, refreshToken }
HC->>TM: saveTokens(newAccess, newRefresh)
HC->>API: Retry original request with new access
else success
API-->>HC: 2xx Response
end
HC-->>UI: Response/result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 6
🧹 Nitpick comments (2)
composeApp/build.gradle.kts (1)
90-91: Authorization 플러그인 의존성이 중복으로 선언되어 있어요두 줄이 동일한
ktor.client.auth의존성이라 최종 그래들 설정에 중복으로 들어갑니다. 한 줄만 남겨서 깔끔하게 정리해 주세요.- implementation(libs.ktor.client.auth) - implementation(libs.ktor.client.auth) + implementation(libs.ktor.client.auth)composeApp/src/commonMain/kotlin/org/whosin/client/presentation/dummy/TokenTestViewModel.kt (1)
25-54: 불필요한 재조회 줄이기 권장토큰 저장/삭제 직후에 다시
loadTokens()를 호출하면서 DataStore에서 재조회가 한 번 더 발생하고 있습니다. 동일한 코루틴 안에서 이미 저장한 값(또는 삭제 결과)을 알고 있으므로, 추가 I/O 없이_accessToken/_refreshToken을 직접 갱신하면 비용을 줄이고 순서를 확실하게 만들 수 있어요. 아래처럼loadTokens내부 로직을suspend함수로 분리하고, 저장/삭제 후에는 그 함수를 직접 호출하도록 리팩터링해보는 건 어떨까요?- fun loadTokens() { - viewModelScope.launch { - _accessToken.value = tokenManager.getAccessToken() - _refreshToken.value = tokenManager.getRefreshToken() - } - } + private suspend fun syncTokens() { + _accessToken.value = tokenManager.getAccessToken() + _refreshToken.value = tokenManager.getRefreshToken() + } + + fun loadTokens() { + viewModelScope.launch { + syncTokens() + } + } fun saveTestTokens() { viewModelScope.launch { tokenManager.saveTokens( accessToken = "test_access_token", refreshToken = "test_refresh_token" ) - loadTokens() + syncTokens() } } fun saveCustomTokens(accessToken: String, refreshToken: String) { viewModelScope.launch { tokenManager.saveTokens(accessToken, refreshToken) - loadTokens() + syncTokens() } } fun clearTokens() { viewModelScope.launch { tokenManager.clearToken() - loadTokens() + syncTokens() } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
composeApp/build.gradle.kts(3 hunks)composeApp/src/androidMain/kotlin/org/whosin/client/datastore/createDataStore.android.kt(1 hunks)composeApp/src/androidMain/kotlin/org/whosin/client/di/DIModules.android.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/App.kt(2 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/TokenManager.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/TokenManagerImpl.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/createDataStore.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/core/network/HttpClientFactory.kt(2 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/data/dto/request/ReissueTokenRequestDto.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/di/DIModules.kt(3 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/presentation/dummy/TokenTestScreen.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/presentation/dummy/TokenTestViewModel.kt(1 hunks)composeApp/src/iosMain/kotlin/org/whosin/client/datastore/createDataStore.ios.kt(1 hunks)composeApp/src/iosMain/kotlin/org/whosin/client/di/DIModules.ios.kt(1 hunks)composeApp/src/jvmMain/kotlin/org/whosin/client/datastore/createDataStore.jvm.kt(1 hunks)composeApp/src/jvmMain/kotlin/org/whosin/client/di/DIModules.jvm.kt(1 hunks)composeApp/src/wasmJsMain/kotlin/org/whosin/client/datastore/JsTokenManagerImpl.kt(1 hunks)composeApp/src/wasmJsMain/kotlin/org/whosin/client/di/DIModules.wasmJs.kt(1 hunks)gradle/libs.versions.toml(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/TokenManager.kt (1)
composeApp/src/commonMain/kotlin/org/whosin/client/presentation/auth/login/viewmodel/LoginViewModel.kt (2)
token(12-16)token(14-14)
composeApp/src/commonMain/kotlin/org/whosin/client/App.kt (2)
composeApp/src/commonMain/kotlin/org/whosin/client/presentation/dummy/DummyScreen.kt (1)
DummyScreen(20-47)composeApp/src/commonMain/kotlin/org/whosin/client/presentation/dummy/DummyViewModel.kt (1)
setup(18-21)
composeApp/src/commonMain/kotlin/org/whosin/client/data/dto/request/ReissueTokenRequestDto.kt (2)
composeApp/src/commonMain/kotlin/org/whosin/client/data/dto/response/LoginResponseDto.kt (2)
accessToken(18-24)success(6-16)composeApp/src/commonMain/kotlin/org/whosin/client/data/dto/request/LoginRequestDto.kt (1)
composeApp/src/wasmJsMain/kotlin/org/whosin/client/datastore/JsTokenManagerImpl.kt (2)
composeApp/src/commonMain/kotlin/org/whosin/client/presentation/auth/login/viewmodel/LoginViewModel.kt (2)
token(14-14)token(12-16)composeApp/src/commonMain/kotlin/org/whosin/client/data/dto/response/LoginResponseDto.kt (1)
accessToken(18-24)
composeApp/src/jvmMain/kotlin/org/whosin/client/datastore/createDataStore.jvm.kt (3)
composeApp/src/androidMain/kotlin/org/whosin/client/datastore/createDataStore.android.kt (1)
createDataStore(9-13)composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/createDataStore.kt (1)
createDataStore(8-12)composeApp/src/iosMain/kotlin/org/whosin/client/datastore/createDataStore.ios.kt (1)
createDataStore(12-24)
composeApp/src/androidMain/kotlin/org/whosin/client/datastore/createDataStore.android.kt (3)
composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/createDataStore.kt (1)
createDataStore(8-12)composeApp/src/iosMain/kotlin/org/whosin/client/datastore/createDataStore.ios.kt (1)
createDataStore(12-24)composeApp/src/jvmMain/kotlin/org/whosin/client/datastore/createDataStore.jvm.kt (1)
createDataStore(8-12)
composeApp/src/commonMain/kotlin/org/whosin/client/core/network/HttpClientFactory.kt (1)
composeApp/src/commonMain/kotlin/org/whosin/client/presentation/dummy/TokenTestViewModel.kt (1)
loadTokens(25-30)
composeApp/src/iosMain/kotlin/org/whosin/client/datastore/createDataStore.ios.kt (3)
composeApp/src/androidMain/kotlin/org/whosin/client/datastore/createDataStore.android.kt (1)
createDataStore(9-13)composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/createDataStore.kt (1)
createDataStore(8-12)composeApp/src/jvmMain/kotlin/org/whosin/client/datastore/createDataStore.jvm.kt (1)
createDataStore(8-12)
🔇 Additional comments (4)
gradle/libs.versions.toml (1)
55-56: 선행 버전으로 맞추면 의존성 해석이 정상화됩니다위에서 버전을
1.1.1로 내리면 여기 선언된datastore-preferences,datastore두 개 모듈도 문제없이 받아집니다. 버전 교체 후 한 번 동기화 확인 부탁드려요.composeApp/src/androidMain/kotlin/org/whosin/client/datastore/createDataStore.android.kt (1)
9-12: Android DataStore 경로 구성 OK.
내부 filesDir 아래에 공용 DataStore 파일을 매핑한 점이 깔끔합니다.composeApp/src/iosMain/kotlin/org/whosin/client/datastore/createDataStore.ios.kt (1)
13-23: iOS 문서 디렉터리 활용 잘 되어 있습니다.
Documents 경로를 보장한 뒤 공용 팩토리에 넘겨서 플랫폼별 구현을 맞춘 부분 좋아요.composeApp/src/androidMain/kotlin/org/whosin/client/di/DIModules.android.kt (1)
17-18: DI 바인딩 연결 상태 양호합니다.
DataStore 싱글톤과 TokenManager 구현체를 플랫폼 모듈에 잘 묶어두신 덕분에 주입 흐름이 명확합니다.
| override suspend fun getAccessToken(): String? = dataStore.data.first()[accessKey] | ||
| override suspend fun getRefreshToken(): String? = dataStore.data.first()[refreshKey] | ||
|
|
||
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | ||
| dataStore.edit { | ||
| it[accessKey] = accessToken | ||
| it[refreshKey] = refreshToken | ||
| } | ||
| } | ||
|
|
||
| override suspend fun clearToken() { | ||
| dataStore.edit { it.clear() } | ||
| } |
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.
DataStore 읽기 실패 시 앱이 크래시됩니다
현재 dataStore.data.first() 호출에서 발생하는 IOException을 잡아주지 않아 복구 불가능한 크래시로 이어집니다. 공식 가이드처럼 emptyPreferences()를 emit하도록 잡아주세요.
import androidx.datastore.preferences.core.edit
+import androidx.datastore.preferences.core.emptyPreferences
import androidx.datastore.preferences.core.stringPreferencesKey
import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.catch
+import java.io.IOException
@@
- override suspend fun getAccessToken(): String? = dataStore.data.first()[accessKey]
- override suspend fun getRefreshToken(): String? = dataStore.data.first()[refreshKey]
+ private val dataFlow = dataStore.data.catch { error ->
+ if (error is IOException) {
+ emit(emptyPreferences())
+ } else {
+ throw error
+ }
+ }
+
+ override suspend fun getAccessToken(): String? = dataFlow.first()[accessKey]
+ override suspend fun getRefreshToken(): String? = dataFlow.first()[refreshKey]📝 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.
| override suspend fun getAccessToken(): String? = dataStore.data.first()[accessKey] | |
| override suspend fun getRefreshToken(): String? = dataStore.data.first()[refreshKey] | |
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | |
| dataStore.edit { | |
| it[accessKey] = accessToken | |
| it[refreshKey] = refreshToken | |
| } | |
| } | |
| override suspend fun clearToken() { | |
| dataStore.edit { it.clear() } | |
| } | |
| private val dataFlow = dataStore.data.catch { error -> | |
| if (error is IOException) { | |
| emit(emptyPreferences()) | |
| } else { | |
| throw error | |
| } | |
| } | |
| override suspend fun getAccessToken(): String? = dataFlow.first()[accessKey] | |
| override suspend fun getRefreshToken(): String? = dataFlow.first()[refreshKey] | |
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | |
| dataStore.edit { | |
| it[accessKey] = accessToken | |
| it[refreshKey] = refreshToken | |
| } | |
| } | |
| override suspend fun clearToken() { | |
| dataStore.edit { it.clear() } | |
| } |
🤖 Prompt for AI Agents
In
composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/TokenManagerImpl.kt
around lines 15 to 27, the direct calls to dataStore.data.first() can throw
IOException and crash the app; change the reads to use dataStore.data.catch { if
(it is IOException) emit(emptyPreferences()) else throw it } .first() and then
access the keys (e.g., dataStore.data.catch { ... }.first()[accessKey]) so that
on IOException emptyPreferences() is emitted instead of propagating the
exception; keep saveTokens and clearToken as-is.
| sendWithoutRequest { request -> | ||
| val host = "https://"+request.url.host+"/" | ||
| val path = request.url.encodedPath | ||
| val pathWithNoAuth = listOf( | ||
| "jokes", | ||
| "users/signup", | ||
| "users/find-password", | ||
| "auth/login", | ||
| "auth/email", | ||
| "auth/email/validation" | ||
| ) | ||
| // 결과가 true면 Authorization 헤더 추가, false면 제거 | ||
| if(host != BASE_URL){ | ||
| println("External API - No Auth") | ||
| false | ||
| }else{ | ||
| // pathWithNoAuth에 있는 경로에는 Authorization 헤더 제외 | ||
| val isNoAuthPath = pathWithNoAuth.any { noAuthPath -> | ||
| path.startsWith(noAuthPath) || path.contains(noAuthPath) | ||
| } | ||
| println("isNoAuthPath: $isNoAuthPath") | ||
| !isNoAuthPath | ||
| } | ||
| } |
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.
sendWithoutRequest 반환값이 뒤집혀 Authorization 헤더가 외부로 노출되고 있습니다.
Ktor Bearer 플러그인에서는 true를 반환할 때 인증 헤더를 생략하는데, 현재 구현은 외부 도메인이나 인증이 필요 없는 경로에서 false를 돌려주고 있어 토큰이 그대로 붙습니다. 그 결과 로그인 API 같은 비보호 엔드포인트에도 Authorization 헤더가 섞이고, 다른 호스트로 나가는 요청에까지 토큰이 실릴 위험이 있습니다. 아울러 호스트 비교 시 무조건 https://로 보정해 BASE_URL에 포트/프로토콜이 다를 경우 매칭이 틀어지는 문제도 겹쳐 있습니다. 아래처럼 Url(BASE_URL)로 기원 호스트를 정확히 비교하고, true를 반환해 인증을 건너뛰도록 뒤집어 주세요.
@@
-import io.ktor.http.ContentType
-import io.ktor.http.contentType
-import io.ktor.http.encodedPath
+import io.ktor.http.ContentType
+import io.ktor.http.Url
+import io.ktor.http.contentType
+import io.ktor.http.encodedPath
@@
- install(Auth){
+ val baseUrl = Url(BASE_URL)
+ install(Auth){
bearer {
@@
- sendWithoutRequest { request ->
- val host = "https://"+request.url.host+"/"
- val path = request.url.encodedPath
- val pathWithNoAuth = listOf(
+ sendWithoutRequest { request ->
+ val isSameOrigin = request.url.protocol == baseUrl.protocol &&
+ request.url.host == baseUrl.host &&
+ request.url.port == baseUrl.port
+ if (!isSameOrigin) {
+ println("External API - No Auth")
+ return@sendWithoutRequest true
+ }
+ val path = request.url.encodedPath.trimStart('/')
+ val pathWithNoAuth = listOf(
"jokes",
@@
- // 결과가 true면 Authorization 헤더 추가, false면 제거
- if(host != BASE_URL){
- println("External API - No Auth")
- false
- }else{
- // pathWithNoAuth에 있는 경로에는 Authorization 헤더 제외
- val isNoAuthPath = pathWithNoAuth.any { noAuthPath ->
- path.startsWith(noAuthPath) || path.contains(noAuthPath)
- }
- println("isNoAuthPath: $isNoAuthPath")
- !isNoAuthPath
- }
+ val isNoAuthPath = pathWithNoAuth.any { noAuthPath ->
+ path.startsWith(noAuthPath)
+ }
+ println("isNoAuthPath: $isNoAuthPath")
+ isNoAuthPath
}📝 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.
| sendWithoutRequest { request -> | |
| val host = "https://"+request.url.host+"/" | |
| val path = request.url.encodedPath | |
| val pathWithNoAuth = listOf( | |
| "jokes", | |
| "users/signup", | |
| "users/find-password", | |
| "auth/login", | |
| "auth/email", | |
| "auth/email/validation" | |
| ) | |
| // 결과가 true면 Authorization 헤더 추가, false면 제거 | |
| if(host != BASE_URL){ | |
| println("External API - No Auth") | |
| false | |
| }else{ | |
| // pathWithNoAuth에 있는 경로에는 Authorization 헤더 제외 | |
| val isNoAuthPath = pathWithNoAuth.any { noAuthPath -> | |
| path.startsWith(noAuthPath) || path.contains(noAuthPath) | |
| } | |
| println("isNoAuthPath: $isNoAuthPath") | |
| !isNoAuthPath | |
| } | |
| } | |
| // === Imports === | |
| import io.ktor.http.ContentType | |
| import io.ktor.http.Url | |
| import io.ktor.http.contentType | |
| import io.ktor.http.encodedPath | |
| // === Inside your HttpClient builder === | |
| val baseUrl = Url(BASE_URL) | |
| install(Auth) { | |
| bearer { | |
| sendWithoutRequest { request -> | |
| // 1) Skip auth for any non-same-origin request | |
| val isSameOrigin = request.url.protocol == baseUrl.protocol && | |
| request.url.host == baseUrl.host && | |
| request.url.port == baseUrl.port | |
| if (!isSameOrigin) { | |
| println("External API - No Auth") | |
| return@sendWithoutRequest true | |
| } | |
| // 2) Trim leading '/' and check against no‐auth paths | |
| val path = request.url.encodedPath.trimStart('/') | |
| val pathWithNoAuth = listOf( | |
| "jokes", | |
| "users/signup", | |
| "users/find-password", | |
| "auth/login", | |
| "auth/email", | |
| "auth/email/validation" | |
| ) | |
| val isNoAuthPath = pathWithNoAuth.any { noAuthPath -> | |
| path.startsWith(noAuthPath) | |
| } | |
| println("isNoAuthPath: $isNoAuthPath") | |
| // true → skip Authorization header, false → include it | |
| isNoAuthPath | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
composeApp/src/commonMain/kotlin/org/whosin/client/core/network/HttpClientFactory.kt
around lines 54 to 77, the sendWithoutRequest logic is inverted and the host
comparison is fragile: Ktor expects sendWithoutRequest to return true to SKIP
adding Authorization, but currently it returns false for external hosts and
no-auth paths so the token is sent; also rebuilding host with "https://" loses
port/protocol info. Fix by constructing a Url(BASE_URL) and compare full origin
(protocol/host/port) against request.url's origin, return true when the request
is to a different origin OR when the request path matches one of the no-auth
paths (i.e., skip auth), otherwise return false so the Authorization header is
applied only for matching protected requests.
| val rt = tokenManager.getRefreshToken() ?: "no_token" | ||
| val response = client.post("member/reissue"){ | ||
| setBody { | ||
| ReissueTokenRequestDto( | ||
| refreshToken = rt | ||
| ) | ||
| } | ||
| markAsRefreshTokenRequest() | ||
| }.body<TokenDto>() | ||
| tokenManager.saveTokens( | ||
| accessToken = response.accessToken, | ||
| refreshToken = response.refreshToken | ||
| ) | ||
| val accessToken = response.accessToken | ||
| val refreshToken = response.refreshToken | ||
| BearerTokens(accessToken,refreshToken) | ||
| } |
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.
refreshTokens 요청 본문이 잘못 직렬화되어 재발급이 실패합니다.
setBody { ... }는 람다를 그대로 인자로 넘기기 때문에 실제로는 Function0 객체가 전송되어 직렬화 에러가 납니다. 게다가 refresh 토큰이 없을 때도 "no_token"으로 API를 두드리고 있어 불필요한 호출이 발생합니다. 토큰이 없으면 바로 null을 반환해 재시도를 중단하고, DTO 인스턴스를 직접 넘기도록 수정해 주세요.
- val rt = tokenManager.getRefreshToken() ?: "no_token"
- val response = client.post("member/reissue"){
- setBody {
- ReissueTokenRequestDto(
- refreshToken = rt
- )
- }
+ val currentRefreshToken = tokenManager.getRefreshToken() ?: return@refreshTokens null
+ val response = client.post("member/reissue") {
+ setBody(
+ ReissueTokenRequestDto(
+ refreshToken = currentRefreshToken
+ )
+ )
@@
- val accessToken = response.accessToken
- val refreshToken = response.refreshToken
- BearerTokens(accessToken,refreshToken)
+ BearerTokens(
+ accessToken = response.accessToken,
+ refreshToken = response.refreshToken
+ )📝 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.
| val rt = tokenManager.getRefreshToken() ?: "no_token" | |
| val response = client.post("member/reissue"){ | |
| setBody { | |
| ReissueTokenRequestDto( | |
| refreshToken = rt | |
| ) | |
| } | |
| markAsRefreshTokenRequest() | |
| }.body<TokenDto>() | |
| tokenManager.saveTokens( | |
| accessToken = response.accessToken, | |
| refreshToken = response.refreshToken | |
| ) | |
| val accessToken = response.accessToken | |
| val refreshToken = response.refreshToken | |
| BearerTokens(accessToken,refreshToken) | |
| } | |
| // if there's no refresh token, abort the retry | |
| val currentRefreshToken = tokenManager.getRefreshToken() ?: return@refreshTokens null | |
| val response = client.post("member/reissue") { | |
| setBody( | |
| ReissueTokenRequestDto( | |
| refreshToken = currentRefreshToken | |
| ) | |
| ) | |
| markAsRefreshTokenRequest() | |
| }.body<TokenDto>() | |
| tokenManager.saveTokens( | |
| accessToken = response.accessToken, | |
| refreshToken = response.refreshToken | |
| ) | |
| BearerTokens( | |
| accessToken = response.accessToken, | |
| refreshToken = response.refreshToken | |
| ) | |
| } |
🤖 Prompt for AI Agents
In
composeApp/src/commonMain/kotlin/org/whosin/client/core/network/HttpClientFactory.kt
around lines 79 to 95, the refresh token request body is being passed as a
lambda (setBody { ... }) which sends a Function0 and fails serialization, and
the code also calls the API with a placeholder "no_token" when no refresh token
exists. Fix by first checking tokenManager.getRefreshToken(): if null, return
null to stop retry; otherwise create a ReissueTokenRequestDto(refreshToken = rt)
instance and pass that directly to setBody(requestDto) (not as a lambda), then
proceed to parse the response and save tokens as before.
| fun createDataStore(): DataStore<Preferences> { | ||
| return createDataStore { | ||
| DATA_STORE_FILE_NAME | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
데스크톱에서 DataStore 경로를 절대 경로로 보장해주세요
현재 문자열로 파일명만 넘기면 JVM 데스크톱 패키징 시(예: DMG/EXE) 기본 작업 디렉터리가 읽기 전용일 수 있어서 PreferenceDataStoreFactory.createWithPath가 즉시 실패하거나, 실행 위치에 따라 토큰 파일이 흩어지는 문제가 생깁니다. 사용자 홈 등 쓰기 가능한 고정 경로를 만들어 준 뒤 그 아래에 파일을 두도록 조정해주세요.
+import java.nio.file.Files
+import java.nio.file.Paths
+
fun createDataStore(): DataStore<Preferences> {
- return createDataStore {
- DATA_STORE_FILE_NAME
- }
+ return createDataStore {
+ val directory = Paths.get(System.getProperty("user.home"), ".whosin")
+ Files.createDirectories(directory)
+ directory.resolve(DATA_STORE_FILE_NAME).toString()
+ }
}🤖 Prompt for AI Agents
In
composeApp/src/jvmMain/kotlin/org/whosin/client/datastore/createDataStore.jvm.kt
around lines 8-11, the DataStore is being created with only a filename which can
resolve to a read-only or unpredictable working directory on packaged desktop
apps; change this to build an absolute path under a writable fixed location
(e.g., userHome/.yourAppName/datastore or similar), ensure the parent directory
exists (createDirectories if missing) before calling the DataStore factory that
accepts a Path, and pass that absolute Path (not a raw filename) to
PreferenceDataStoreFactory.createWithPath/createWithFile so the store is always
written to a writable, deterministic location.
| override suspend fun getAccessToken(): String? { | ||
| TODO("Not yet implemented") | ||
| } | ||
|
|
||
| override suspend fun getRefreshToken(): String? { | ||
| TODO("Not yet implemented") | ||
| } | ||
|
|
||
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | ||
| TODO("Not yet implemented") | ||
| } | ||
|
|
||
| override suspend fun clearToken() { | ||
| TODO("Not yet implemented") |
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.
TODO 메서드는 JS 환경에서 바로 크래시 납니다
TODO()는 호출 즉시 NotImplementedError를 던져 wasm/js 타겟에서 토큰을 읽거나 저장하려는 시점마다 앱이 죽습니다. 최소한 메모리에 보관하는 기본 구현을 넣어주세요.
-import org.whosin.client.core.datastore.TokenManager
+import kotlinx.coroutines.sync.Mutex
+import kotlinx.coroutines.sync.withLock
+import org.whosin.client.core.datastore.TokenManager
@@
-class JsTokenManagerImpl : TokenManager{
- override suspend fun getAccessToken(): String? {
- TODO("Not yet implemented")
- }
-
- override suspend fun getRefreshToken(): String? {
- TODO("Not yet implemented")
- }
-
- override suspend fun saveTokens(accessToken: String, refreshToken: String) {
- TODO("Not yet implemented")
- }
-
- override suspend fun clearToken() {
- TODO("Not yet implemented")
- }
+class JsTokenManagerImpl : TokenManager {
+ private val mutex = Mutex()
+ private var accessToken: String? = null
+ private var refreshToken: String? = null
+
+ override suspend fun getAccessToken(): String? = mutex.withLock { accessToken }
+
+ override suspend fun getRefreshToken(): String? = mutex.withLock { refreshToken }
+
+ override suspend fun saveTokens(accessToken: String, refreshToken: String) {
+ mutex.withLock {
+ this.accessToken = accessToken
+ this.refreshToken = refreshToken
+ }
+ }
+
+ override suspend fun clearToken() {
+ mutex.withLock {
+ accessToken = null
+ refreshToken = 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.
| override suspend fun getAccessToken(): String? { | |
| TODO("Not yet implemented") | |
| } | |
| override suspend fun getRefreshToken(): String? { | |
| TODO("Not yet implemented") | |
| } | |
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | |
| TODO("Not yet implemented") | |
| } | |
| override suspend fun clearToken() { | |
| TODO("Not yet implemented") | |
| import kotlinx.coroutines.sync.Mutex | |
| import kotlinx.coroutines.sync.withLock | |
| import org.whosin.client.core.datastore.TokenManager | |
| class JsTokenManagerImpl : TokenManager { | |
| private val mutex = Mutex() | |
| private var accessToken: String? = null | |
| private var refreshToken: String? = null | |
| override suspend fun getAccessToken(): String? = mutex.withLock { accessToken } | |
| override suspend fun getRefreshToken(): String? = mutex.withLock { refreshToken } | |
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | |
| mutex.withLock { | |
| this.accessToken = accessToken | |
| this.refreshToken = refreshToken | |
| } | |
| } | |
| override suspend fun clearToken() { | |
| mutex.withLock { | |
| accessToken = null | |
| refreshToken = null | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
composeApp/src/wasmJsMain/kotlin/org/whosin/client/datastore/JsTokenManagerImpl.kt
around lines 6 to 19, the methods currently call TODO() which throws
NotImplementedError immediately in JS/WASM and crashes the app; replace those
TODOs with a simple in-memory token store: add private mutable properties for
accessToken and refreshToken, implement getAccessToken/getRefreshToken to return
those values, implement saveTokens to set them, and implement clearToken to null
them; keep the implementation minimal and synchronous (no platform-specific
storage) so JS/WASM code will not crash when tokens are accessed.
| kotlinx-serialization = "1.9.0" | ||
| koin = "4.1.0" | ||
| buildkonfig = "0.17.1" | ||
| datastore = "1.1.7" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DataStore 버전이 리포지터리에 존재하지 않아요
1.1.7은 Google Maven에 아직 올라오지 않은 버전이라 의존성 해석 단계에서 바로 실패합니다. 지금 시점에 퍼블리시된 최신 안정판은 1.1.1이니 그걸로 맞춰 주세요.
-datastore = "1.1.7"
+datastore = "1.1.1"📝 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.
| datastore = "1.1.7" | |
| datastore = "1.1.1" |
🤖 Prompt for AI Agents
In gradle/libs.versions.toml around line 22, the datastore version is set to
1.1.7 which is not published to Google Maven and breaks dependency resolution;
change the version string to the latest published stable 1.1.1 (replace "1.1.7"
with "1.1.1") and then refresh/resolve Gradle dependencies (e.g., run a Gradle
sync/refresh) to confirm the project builds.
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)
composeApp/build.gradle.kts (1)
94-95: 중복된 auth 의존성 제거해요
ktor.client.auth가 두 번 추가되어 있어요. 그대로 두면 불필요한 중복이라 IDE 경고만 늘어납니다. 하나만 남겨주세요.- implementation(libs.ktor.client.auth) - implementation(libs.ktor.client.auth) + implementation(libs.ktor.client.auth)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
composeApp/build.gradle.kts(3 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/App.kt(2 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/di/DIModules.kt(3 hunks)gradle/libs.versions.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- composeApp/src/commonMain/kotlin/org/whosin/client/App.kt
- gradle/libs.versions.toml
⏰ 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). (2)
- GitHub Check: ios-build
- GitHub Check: android-build
🔇 Additional comments (3)
composeApp/src/commonMain/kotlin/org/whosin/client/di/DIModules.kt (3)
14-14: TokenTestViewModel import 반영 굿입니다.
테스트 플로우 확인하려는 의도가 코드에 잘 드러나서 이해가 쉬워졌어요.
30-30: TokenManager 주입 포함 확인했습니다.
HttpClientFactory.create단계에서 엔진과 토큰 매니저가 함께 묶여서, 토큰 리프레시 흐름이 자연스럽게 돌 것 같아요.
51-51: TokenTestViewModel DI 등록 OK.
테스트 스크린을 켰을 때 바로 뷰모델이 붙도록 준비된 점이 사용 편의성에 도움이 되네요.
🚀 이슈번호
✏️ 변경사항
📷 스크린샷
✍️ 사용법
🎸 기타
Summary by CodeRabbit
신기능
개선/리팩터
작업/기타