Conversation
📝 Walkthrough작업 요약이 변경사항은 두 개의 새로운 코어 모듈(token, datastore)을 프로젝트에 추가합니다. core:token 모듈은 TokenProvider 인터페이스를 정의하고, core:datastore 모듈은 Android DataStore를 활용하여 인증 토큰을 직렬화 및 관리하는 AuthTokenProvider 구현을 제공합니다. 이를 통해 토큰 저장소 추상화 계층을 도입하고, Koin DI를 통해 의존성을 해결합니다. 코드 리뷰 예상 소요 시간🎯 3 (Moderate) | ⏱️ ~35 minutes 상세 검토 의견✅ 긍정적인 측면
개선 제안 및 검토 사항🔍 AuthConfigure.kt 관련문제점: 기본 JWT 값이 하드코딩되어 있는 부분이 있습니다. val accessToken: String = "eyJhbGci..." // 실제 값이 길어 보임개선 방향:
🔍 AuthTokenProvider.kt 관련문제점: private val dataStore: DataStore<AuthConfigure> = context.authDataStore
suspend fun accessToken(): String {
return dataStore.data.first().accessToken
}검토 사항:
🔍 DataStore.kt 관련개선 제안:
🔍 DataStoreModule.kt 관련검토 사항: single<TokenProvider> { AuthTokenProvider(get()) }
🔍 gradle/libs.versions.toml 관련개선 사항: androidx-datastore = "1.2.0"
🔍 설정 파일들 관련긍정적: ProGuard 규칙 파일과 AndroidManifest.xml을 템플릿 형태로 포함한 것은 앞으로의 확장성을 고려한 좋은 결정입니다. 최종 체크리스트
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@core/datastore/src/main/java/com/twix/datastore/AuthConfigure.kt`:
- Around line 13-21: AuthConfigure currently hardcodes real JWTs in the
accessToken and refreshToken defaults; replace those defaults with safe values
(empty string or null) and remove any embedded tokens, update any call sites
that construct AuthConfigure (or read AuthConfigure.accessToken/refreshToken) to
accept and properly handle empty/null tokens, and ensure tests or local/debug
builds inject test tokens via configuration or environment rather than relying
on AuthConfigure's defaults; reference the AuthConfigure data class and its
accessToken and refreshToken properties when making the change.
- Around line 28-36: The readFrom implementation in AuthConfigure.readFrom
currently swallows SerializationException and returns defaultValue, which
prevents DataStore from detecting corrupted files; instead, catch
SerializationException and rethrow an
androidx.datastore.core.CorruptionException (including the original
SerializationException as the cause and a helpful message) from the
readFrom(InputStream) method so DataStore can delete the corrupted file and
recover using defaultValue; update the catch block in the readFrom function
(where AuthConfigure.serializer(), input.readBytes().decodeToString(), and
defaultValue are referenced) to throw CorruptionException rather than returning
defaultValue.
In `@core/datastore/src/main/java/com/twix/datastore/AuthTokenProvider.kt`:
- Around line 11-30: Auth tokens are being saved in plaintext via the
DataStore/Serializer pipeline (see AuthTokenProvider -> dataStore and
AuthConfigureSerializer readFrom/writeTo writing "auth-configure.json"), so
replace plaintext storage with encryption: either migrate to Jetpack Security's
encrypted solution (create a MasterKey and use
EncryptedDataStore/EncryptedFile-backed DataStore for AuthConfigure so
AuthTokenProvider continues to read/write encrypted data), or implement
encryption inside AuthConfigureSerializer's readFrom/writeTo using a
MasterKey/Cipher to encrypt bytes before writing and decrypt after reading (use
AndroidX Security Crypto APIs or a vetted library like Tink), and update
DataStore usage accordingly so accessToken()/saveToken()/clear() operate on the
encrypted store.
In `@gradle/libs.versions.toml`:
- Line 18: The project declares the wrong DataStore artifact: replace the
preferences-only artifact with the typed DataStore artifact so the
Serializer-based API used in code works; update the dependency entry so the
project depends on androidx.datastore:datastore (which provides Serializer<T>,
DataStore<T>, and the dataStore delegate) instead of the datastore-preferences
artifact referenced for androidx-datastore, so classes/interfaces like
AuthConfigure, Serializer<T>, DataStore<AuthConfigure>, and the dataStore
delegate resolve correctly.
🧹 Nitpick comments (2)
core/datastore/build.gradle.kts (1)
11-15: TokenProvider가 공개 API에 노출될 수 있어 api/가시성 조정이 필요합니다.
AuthTokenProvider가 public이며TokenProvider를 구현하므로, 소비 모듈에서TokenProvider타입이 필요해질 수 있습니다.
api(projects.core.token)으로 전환하거나,AuthTokenProvider를internal로 제한해 구현체를 숨기는 방향을 검토해 주세요.♻️ 제안 변경
- implementation(projects.core.token) + api(projects.core.token)As per coding guidelines "implementation / api 구분이 명확한가?".
app/build.gradle.kts (1)
33-34: app에서 core.token 직접 사용 여부를 확인해 주세요.
앱 코드에서TokenProvider등을 직접 참조하지 않는다면 의존성이 불필요해 빌드 그래프가 커질 수 있습니다.
직접 사용 여부를 확인한 뒤, 불필요하면 제거하거나 필요 시 core.datastore가api로 노출하도록 조정하는 방향을 검토해 주세요. 앱에서 직접 사용하는 클래스가 있나요?As per coding guidelines "불필요한 의존성이 없는가?".
| private val dataStore: DataStore<AuthConfigure> | ||
| get() = context.authDataStore | ||
|
|
||
| override suspend fun accessToken(): String = dataStore.data.first().accessToken | ||
|
|
||
| override suspend fun refreshToken(): String = dataStore.data.first().refreshToken | ||
|
|
||
| override suspend fun saveToken( | ||
| accessToken: String, | ||
| refreshToken: String, | ||
| ) { | ||
| dataStore.updateData { | ||
| it.copy(accessToken = accessToken, refreshToken = refreshToken) | ||
| } | ||
| } | ||
|
|
||
| override suspend fun clear() { | ||
| dataStore.updateData { | ||
| it.copy(accessToken = "", refreshToken = "") | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f -e kt | head -20Repository: YAPP-Github/Twix-Android
Length of output: 1630
🏁 Script executed:
rg "authDataStore" --type kt -A 3 -B 3Repository: YAPP-Github/Twix-Android
Length of output: 94
🏁 Script executed:
rg "context\.authDataStore\|authDataStore\s*=" --type kt -A 5Repository: YAPP-Github/Twix-Android
Length of output: 94
🏁 Script executed:
find . -type f -name "*.kt" | xargs grep -l "DataStore\|authDataStore" | head -10Repository: YAPP-Github/Twix-Android
Length of output: 201
🏁 Script executed:
cat -n core/datastore/src/main/java/com/twix/datastore/DataStore.ktRepository: YAPP-Github/Twix-Android
Length of output: 431
🏁 Script executed:
cat -n core/datastore/src/main/java/com/twix/datastore/AuthTokenProvider.ktRepository: YAPP-Github/Twix-Android
Length of output: 1175
🏁 Script executed:
find . -type f -name "*Serializer*" | grep -i authRepository: YAPP-Github/Twix-Android
Length of output: 50
🏁 Script executed:
rg "Serializer|serializer" --type-listRepository: YAPP-Github/Twix-Android
Length of output: 457
🏁 Script executed:
find . -type f \( -name "*.kt" -o -name "*.java" \) | xargs grep -l "AuthConfigureSerializer" | head -5Repository: YAPP-Github/Twix-Android
Length of output: 197
🏁 Script executed:
rg "object AuthConfigureSerializer|class AuthConfigureSerializer" -A 20Repository: YAPP-Github/Twix-Android
Length of output: 2099
🏁 Script executed:
find . -type f -name "*.kt" | xargs grep -l "Serializer" | grep -i authRepository: YAPP-Github/Twix-Android
Length of output: 134
🏁 Script executed:
cat -n core/datastore/src/main/java/com/twix/datastore/AuthConfigure.ktRepository: YAPP-Github/Twix-Android
Length of output: 2187
토큰이 평문으로 저장되어 보안 위험이 있습니다.
AuthConfigureSerializer의 readFrom/writeTo에서 Json.decodeFromString/encodeToString를 사용하고 있어, 토큰이 보호 없이 "auth-configure.json" 파일에 평문으로 저장됩니다. 루팅된 기기나 백업 환경에서 토큰이 노출될 수 있습니다.
개선 방안:
- 권장: Jetpack Security의 EncryptedDataStore + MasterKey를 사용하는 것이 가장 표준적입니다
- 대안: 현재 구조를 유지하면서 Serializer에서 암복호화를 적용할 수도 있습니다 (예: Tink나 BouncyCastle 라이브러리 활용)
EncryptedDataStore 적용이 어렵다면, Serializer에 암호화 로직을 추가하는 예시를 제공할 수 있습니다. 보안 요구사항상 암호화 적용이 필수인지 확인 부탁드립니다.
🤖 Prompt for AI Agents
In `@core/datastore/src/main/java/com/twix/datastore/AuthTokenProvider.kt` around
lines 11 - 30, Auth tokens are being saved in plaintext via the
DataStore/Serializer pipeline (see AuthTokenProvider -> dataStore and
AuthConfigureSerializer readFrom/writeTo writing "auth-configure.json"), so
replace plaintext storage with encryption: either migrate to Jetpack Security's
encrypted solution (create a MasterKey and use
EncryptedDataStore/EncryptedFile-backed DataStore for AuthConfigure so
AuthTokenProvider continues to read/write encrypted data), or implement
encryption inside AuthConfigureSerializer's readFrom/writeTo using a
MasterKey/Cipher to encrypt bytes before writing and decrypt after reading (use
AndroidX Security Crypto APIs or a vetted library like Tink), and update
DataStore usage accordingly so accessToken()/saveToken()/clear() operate on the
encrypted store.
| # AndroidX | ||
| androidx-core-ktx = "1.17.0" | ||
| androidx-lifecycle-runtime-ktx = "2.10.0" | ||
| androidx-datastore = "1.2.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find gradle configuration files
git ls-files | grep -E "\.toml$|build\.gradle|settings\.gradle"Repository: YAPP-Github/Twix-Android
Length of output: 646
🏁 Script executed:
# Check the actual artifact configuration in libs.versions.toml
cat gradle/libs.versions.tomlRepository: YAPP-Github/Twix-Android
Length of output: 9311
🏁 Script executed:
# Search for DataStore imports to understand which library is being used
rg "import.*datastore" --type kotlin -iRepository: YAPP-Github/Twix-Android
Length of output: 597
🏁 Script executed:
# Look for DataStore API usage
rg "dataStore|DataStore|Preferences\|DataStoreFactory" --type kotlin -B 2 -A 3Repository: YAPP-Github/Twix-Android
Length of output: 4271
🏁 Script executed:
cat core/datastore/build.gradle.ktsRepository: YAPP-Github/Twix-Android
Length of output: 401
🏁 Script executed:
# Check the full AuthConfigure and AuthConfigureSerializer implementation
cat core/datastore/src/main/java/com/twix/datastore/AuthConfigure.ktRepository: YAPP-Github/Twix-Android
Length of output: 1823
🏁 Script executed:
cat core/datastore/src/main/java/com/twix/datastore/DataStore.ktRepository: YAPP-Github/Twix-Android
Length of output: 361
DataStore 의존성을 수정해야 합니다
현재 datastore-preferences artifact를 선언하고 있지만, 코드에서는 typed DataStore API를 사용하고 있습니다. 코드 분석 결과:
AuthConfigure를Serializer<T>로 직렬화하고 있음 (typed DataStore)DataStore<AuthConfigure>와dataStore위임 함수를 사용 중- 이 모든 API는
androidx.datastore:datastore라이브러리에만 포함됨
문제: datastore-preferences는 key-value 저장소용이며, 커스텀 Serializer<T> 인터페이스와 typed DataStore<T> API를 제공하지 않습니다. 현재 선언된 artifact로는 코드가 정상 동작할 수 없습니다.
🔧 수정 사항
-androidx-datastore = { module = "androidx.datastore:datastore-preferences", version.ref = "androidx-datastore" }
+androidx-datastore = { module = "androidx.datastore:datastore", version.ref = "androidx-datastore" }🤖 Prompt for AI Agents
In `@gradle/libs.versions.toml` at line 18, The project declares the wrong
DataStore artifact: replace the preferences-only artifact with the typed
DataStore artifact so the Serializer-based API used in code works; update the
dependency entry so the project depends on androidx.datastore:datastore (which
provides Serializer<T>, DataStore<T>, and the dataStore delegate) instead of the
datastore-preferences artifact referenced for androidx-datastore, so
classes/interfaces like AuthConfigure, Serializer<T>, DataStore<AuthConfigure>,
and the dataStore delegate resolve correctly.
1 participant
이슈 번호
#57
리뷰/머지 희망 기한 (선택)
작업내용
결과물
리뷰어에게 추가로 요구하는 사항 (선택)
같은 core 레벨에 있는 network 모듈과 datastore이 서로 직접 의존하지 않도록 core:token 모듈을 추가하고
인터페이스를 구현해 의존성을 역전시켰어 ! network --> token(interface) <-- datastore (concrete)