[Feature/#8] AuthToken 저장을 위한 DataStore 구현 #15
Conversation
- datastore-preferences 라이브러리 alias 수정 - dataStore 모듈 kotlinx.serialization.json 의존성 추가
- Crypto 클래스를 Crypto 인터페이스와 SecureCrypto 구현체로 분리 - 패키지 구조 변경 (security -> security.crypto) - CryptoTest 클래스명을 SecureCryptoTest로 변경하고, 리팩토링된 클래스에 맞게 수정
- AuthTokenSerializer 클래스 추가 - TokenSerializer 인터페이스 추가 - AuthTokenSerializer 테스트 코드 작성
- AuthTokenDataStore 인터페이스 추가 - AuthTokenDataStoreImpl 구현체 추가
|
""" Walkthrough이 변경사항은 인증 토큰(AuthToken)을 안전하게 저장하고 관리하기 위한 DataStore 모듈을 새롭게 도입합니다. 암호화 기능(Crypto)과 Hilt 기반 DI 모듈, 직렬화/역직렬화 로직, 테스트 코드, 그리고 관련 의존성 및 빌드 설정이 포함되어 있습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant DataStoreModule
participant SecurityModule
participant DataStore
participant AuthTokenSerializer
participant Crypto
App->>DataStoreModule: 의존성 주입 요청 (DataStore<AuthToken>)
DataStoreModule->>AuthTokenSerializer: TokenSerializer 생성 (Crypto 주입)
DataStoreModule->>DataStore: DataStore<AuthToken> 생성 (AuthTokenSerializer, 파일)
App->>DataStore: 인증 토큰 저장/조회 요청
DataStore->>AuthTokenSerializer: 직렬화/역직렬화 요청
AuthTokenSerializer->>Crypto: 암호화/복호화 수행
AuthTokenSerializer-->>DataStore: 직렬화/역직렬화 결과 반환
DataStore-->>App: 인증 토큰 데이터 반환
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
core/security/src/main/java/com/threegap/bitnagil/security/keystore/AndroidKeyProvider.kt (2)
21-33: 키 생성 로직의 Keystore provider 누락 및 CBC 사용에 따른 보안 위험
KeyGenerator.getInstance(ALGORITHM)호출 시 provider 를 지정하지 않으면 기본 Provider 가 선택돼 키가 Keystore 에 저장되지 않을 수 있습니다."AndroidKeyStore"provider 명시가 권장됩니다.- AES/CBC + PKCS7 조합은 무결성 검증이 없어 토큰 위변조 위험이 있습니다. 가능한 경우 인증·무결성을 제공하는 AES/GCM 으로 전환해 주세요.
- return KeyGenerator.getInstance(ALGORITHM).apply { + return KeyGenerator.getInstance(ALGORITHM, "AndroidKeyStore").apply { @@ - .setBlockModes(BLOCK_MODE) - .setEncryptionPaddings(PADDING) + .setBlockModes(KeyProperties.BLOCK_MODE_GCM) + .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE)변경 시
BLOCK_MODE,PADDING상수 및 복호화 로직도 함께 업데이트해야 합니다.
9-18: 동시성 고려: KeyStore 접근은 synchronized 블록으로 감싸는 것이 안전
KeyStore#getEntry와generateKey()가 멀티스레드 환경에서 동시에 호출될 경우 race condition 이 발생할 수 있습니다. 간단히synchronized(this)로 보호하거나 싱글톤 스코프에서 초기화하도록 고려해 주세요.
🧹 Nitpick comments (10)
core/security/build.gradle.kts (1)
2-4: Hilt 플러그인 추가 후 의존성 누락 가능성 확인 필요
bitnagil.android.hilt플러그인을 추가했지만, 현재dependencies {}블록에는hilt-android,hilt-compiler(ksp/kapt)등 컴파일·런타임 의존성이 보이지 않습니다. 커스텀 플러그인이 내부적으로 의존성을 주입하지 않는다면 컴파일 오류가 발생합니다.
플러그인 구현을 확인하거나 아래 의존성을 명시적으로 추가해 주세요.dependencies { implementation(libs.hilt.android) ksp(libs.hilt.compiler) // kapt 사용 시 kapt(...) }core/security/src/main/java/com/threegap/bitnagil/security/keystore/KeyProvider.kt (1)
1-7: 인터페이스 외부 노출 최소화 고려
KeyProvider가 보안 모듈 내부에서만 사용된다면internal로 가시성을 제한해 불필요한 API surface 를 줄이는 편이 좋습니다.
또한 KDoc 를 추가해 키 관리 방식(AES, 키스토어 alias 등)을 명시하면 유지보수성이 향상됩니다.core/datastore/build.gradle.kts (1)
17-18: 테스트 의존성: JUnit 버전 명확화 권장
androidx.junit만으론 JUnit4 러너만 포함될 수 있습니다.
다른 모듈 테스트에서kotlin.coroutines.test를 사용한다면org.junit.jupiter:junit-jupiter(JUnit5) 필요성도 검토해 주세요.core/datastore/src/main/java/com/threegap/bitnagil/datastore/serializer/TokenSerializer.kt (1)
3-6: 인터페이스 분리 의도 명시 필요
Serializer<AuthToken>를 그대로 구현해도 충분한데 래퍼 인터페이스를 둔 이유(가시성 제한, 테스트 스텁 분리 등)를 KDoc 로 설명하면 유지보수 가독성이 올라갑니다.core/security/src/main/java/com/threegap/bitnagil/security/di/SecurityModule.kt (1)
20-22: Cipher Transformation 하드코딩 제거 고려
SecureCrypto(keyProvider)가 기본 변환 문자열을 내부에 갖고 있을 가능성이 큽니다. 암호화 정책 변경이 잦다면@Named("cipherTransformation")로 주입받아 모듈 밖에서 관리하도록 하면 유연성이 향상됩니다.core/security/src/test/java/com/threegap/bitnagil/security/crypto/SecureCryptoTest.kt (1)
56-64: 짧은 입력 복호화 테스트의 기대 결과 불명확현재 IllegalArgumentException 만 검증하고 있지만, 구현체가
BadPaddingException을 던질 가능성도 있습니다. 구현 변경 시 테스트가 취약해질 수 있으므로assertThrowsAnyOf(IllegalArgumentException, BadPaddingException)등으로 범위를 넓혀두는 것을 권장합니다.core/datastore/src/main/java/com/threegap/bitnagil/datastore/serializer/AuthTokenSerializer.kt (1)
31-33: 예외 처리 시 로깅 추가를 고려해보세요.현재 모든 예외를 묵시적으로 처리하고 있는데, 디버깅을 위해 로깅을 추가하는 것을 권장합니다.
} catch (e: Exception) { + // TODO: 로깅 추가 고려 AuthToken() }core/datastore/src/main/java/com/threegap/bitnagil/datastore/di/DataStoreModule.kt (1)
34-34: 파일명 상수화를 고려해보세요.하드코딩된 파일명을 상수로 추출하면 유지보수성이 향상됩니다.
+ private const val AUTH_TOKEN_FILE_NAME = "auth-token.enc" + @Provides @Singleton fun provideAuthTokenDataStore( @ApplicationContext context: Context, tokenSerializer: TokenSerializer, ): DataStore<AuthToken> = DataStoreFactory.create( serializer = tokenSerializer, - produceFile = { context.dataStoreFile("auth-token.enc") }, + produceFile = { context.dataStoreFile(AUTH_TOKEN_FILE_NAME) }, corruptionHandler = ReplaceFileCorruptionHandler { AuthToken() }, )core/datastore/src/test/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImplTest.kt (2)
32-56: FakeAuthTokenSerializer의 예외 처리를 개선하세요.
readFrom메서드에서 모든 예외를 삼키고 기본값을 반환하는 것은 테스트에서 실제 오류를 숨길 수 있습니다. 적어도 로깅을 추가하여 디버깅을 도와주세요.override suspend fun readFrom(input: InputStream): AuthToken { return try { input.bufferedReader().use { Json.decodeFromString(AuthToken.serializer(), it.readText()) } } catch (e: Exception) { + println("FakeAuthTokenSerializer.readFrom failed: ${e.message}") AuthToken() } }
160-176: 테스트 예외 처리 패턴을 현대적인 방식으로 개선할 수 있습니다.
@Test(expected = ...)대신assertThrows를 사용하면 더 명확하고 유연한 테스트를 작성할 수 있습니다.-@Test(expected = RuntimeException::class) -fun `updateAuthToken에서 예외 발생시 예외가 전파되어야 한다`() = +@Test +fun `updateAuthToken에서 예외 발생시 예외가 전파되어야 한다`() = runTest { // given val brokenStore = // ... 기존 코드 val failingDataStore = AuthTokenDataStoreImpl(brokenStore) - // when & then - failingDataStore.updateAuthToken(AuthToken("access", "refresh")) + // when & then + val exception = assertThrows<RuntimeException> { + failingDataStore.updateAuthToken(AuthToken("access", "refresh")) + } + assertEquals("updateAuthToken failed", exception.message) }같은 패턴을 다른 예외 테스트들에도 적용할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
core/datastore/build.gradle.kts(1 hunks)core/datastore/src/main/java/com/threegap/bitnagil/datastore/di/DataStoreModule.kt(1 hunks)core/datastore/src/main/java/com/threegap/bitnagil/datastore/model/AuthToken.kt(1 hunks)core/datastore/src/main/java/com/threegap/bitnagil/datastore/serializer/AuthTokenSerializer.kt(1 hunks)core/datastore/src/main/java/com/threegap/bitnagil/datastore/serializer/TokenSerializer.kt(1 hunks)core/datastore/src/main/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStore.kt(1 hunks)core/datastore/src/main/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImpl.kt(1 hunks)core/datastore/src/test/java/com/threegap/bitnagil/datastore/serializer/AuthTokenSerializerTest.kt(1 hunks)core/datastore/src/test/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImplTest.kt(1 hunks)core/security/build.gradle.kts(1 hunks)core/security/src/main/java/com/threegap/bitnagil/security/crypto/Crypto.kt(1 hunks)core/security/src/main/java/com/threegap/bitnagil/security/crypto/SecureCrypto.kt(1 hunks)core/security/src/main/java/com/threegap/bitnagil/security/di/SecurityModule.kt(1 hunks)core/security/src/main/java/com/threegap/bitnagil/security/keystore/AndroidKeyProvider.kt(1 hunks)core/security/src/main/java/com/threegap/bitnagil/security/keystore/KeyProvider.kt(1 hunks)core/security/src/test/java/com/threegap/bitnagil/security/crypto/SecureCryptoTest.kt(8 hunks)gradle/libs.versions.toml(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.26.0)
core/security/src/test/java/com/threegap/bitnagil/security/crypto/SecureCryptoTest.kt
116-116: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 detekt (1.23.8)
core/datastore/src/test/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImplTest.kt
[warning] 41-41: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (21)
gradle/libs.versions.toml (1)
64-65: alias 변경에 따른 전역 참조 업데이트 필요
androidx-datastore→androidx-datastore-preferences로 alias가 변경되었습니다. 기존 모듈에서libs.androidx.datastore를 참조 중이라면 컴파일 오류가 발생하니 일괄 치환 여부를 확인해 주세요.#!/bin/bash # 변경되지 않은 참조가 남아있는지 검색 rg --no-heading "libs\.androidx\.datastore\b" -ncore/datastore/src/main/java/com/threegap/bitnagil/datastore/model/AuthToken.kt (1)
5-9: 데이터 클래스 설계 적합토큰 모델 정의 및 기본값 처리 모두 적절합니다. 직렬화를 위한
@Serializable도 포함돼 있어 DataStore 에 사용할 준비가 되어 있습니다.core/security/src/main/java/com/threegap/bitnagil/security/di/SecurityModule.kt (1)
18-22: AndroidKeyProvider 생성 시 Context 주입 여부 확인
AndroidKeyProvider()가 매개변수 없이 생성되고 있습니다. 내부에서ApplicationContext가 필요하다면 Hilt 의@ApplicationContext를 주입받도록 변경하지 않으면 런타임 NPE 가 발생합니다.- fun provideKeyProvider(): KeyProvider = AndroidKeyProvider() + fun provideKeyProvider( + @ApplicationContext context: Context, + ): KeyProvider = AndroidKeyProvider(context)필요 시
Context가 필요 없음을 명시하는 주석을 추가하세요.core/security/src/test/java/com/threegap/bitnagil/security/crypto/SecureCryptoTest.kt (1)
116-121: Gitleaks false positive 검토Static-analysis 에서 “Generic API Key” 로 표시했으나, 실제로는 테스트용
AES키를 동적으로 생성합니다. CI 규칙에 따라 예외 목록에 추가하거나 주석(// no-secret)을 달아 오탐을 방지하세요.core/datastore/src/test/java/com/threegap/bitnagil/datastore/serializer/AuthTokenSerializerTest.kt (3)
14-32: 테스트 구조가 잘 설계되었습니다.테스트 클래스의 구조와 FakeCrypto 구현이 매우 깔끔합니다. 실제 암호화 로직과 분리하여 직렬화 로직만 집중적으로 테스트할 수 있도록 설계되었습니다.
49-61: 암호화 및 Base64 인코딩 로직이 올바르게 테스트되었습니다.writeTo 메서드의 암호화와 Base64 인코딩 플로우가 정확히 검증되고 있습니다. 예상되는 결과와 실제 결과를 올바르게 비교하고 있습니다.
77-95: 예외 처리 시나리오가 잘 커버되었습니다.복호화 실패 시 기본값을 반환하는 동작이 올바르게 테스트되었습니다. 이는 실제 운영 환경에서 발생할 수 있는 예외 상황에 대한 안전장치를 검증하는 중요한 테스트입니다.
core/security/src/main/java/com/threegap/bitnagil/security/crypto/SecureCrypto.kt (3)
7-10: 인터페이스 구현과 internal 가시성 적용이 적절합니다.클래스가 Crypto 인터페이스를 구현하고 internal로 가시성을 제한한 것은 모듈간 의존성을 인터페이스로 추상화하고 구현 세부사항을 은닉하는 좋은 설계입니다.
11-17: 암호화 구현이 보안 모범사례를 따릅니다.AES/CBC/PKCS7Padding을 사용하고 IV를 암호문과 함께 저장하는 방식은 표준적인 접근법입니다. 매번 새로운 IV를 생성하여 동일한 평문도 다른 암호문으로 변환됩니다.
19-28: 복호화 구현에서 입력 검증이 적절합니다.복호화 전에 입력 데이터의 최소 길이를 검증하여 잘못된 데이터로 인한 예외를 방지하는 것이 좋습니다. IV와 데이터를 분리하는 로직도 올바릅니다.
core/datastore/src/main/java/com/threegap/bitnagil/datastore/serializer/AuthTokenSerializer.kt (3)
13-20: 클래스 설계가 깔끔합니다.internal 가시성과 @Inject 생성자를 사용한 의존성 주입, 그리고 인터페이스 구현이 적절하게 설계되었습니다. 기본값 제공도 안전한 접근법입니다.
21-34: 입력 스트림 읽기 로직이 안전하고 효율적입니다.IO 작업에 적절한 코루틴 컨텍스트를 사용하고, use 블록으로 리소스를 안전하게 관리하며, 예외 발생시 기본값을 반환하는 방어적 프로그래밍이 잘 적용되었습니다.
36-49: 출력 스트림 쓰기 로직이 체계적입니다.JSON 직렬화 → 암호화 → Base64 인코딩 → IO 쓰기의 단계별 처리가 명확하고, 코루틴 컨텍스트 전환과 리소스 관리도 적절합니다.
core/datastore/src/main/java/com/threegap/bitnagil/datastore/di/DataStoreModule.kt (3)
19-21: Hilt 모듈 구성이 적절합니다.SingletonComponent에 설치하여 앱 전체에서 동일한 인스턴스를 사용하도록 한 것이 AuthToken 관리에 적합합니다.
22-24: 의존성 제공 설정이 올바릅니다.Crypto 인터페이스를 주입받아 AuthTokenSerializer를 생성하는 방식으로 의존성 역전 원칙을 잘 따르고 있습니다.
26-36: DataStore 설정이 보안과 안정성을 고려하여 잘 구성되었습니다.파일명에 ".enc" 확장자를 사용하여 암호화됨을 명시하고, 손상된 데이터에 대한 복구 전략을 제공한 것이 훌륭합니다.
core/datastore/src/main/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStore.kt (1)
6-16: 인터페이스 설계가 우수합니다.Flow를 통한 반응형 데이터 제공, suspend 함수를 통한 비동기 지원, 그리고 전체/부분 업데이트 메서드를 제공하여 유연하고 사용하기 쉬운 API를 제공합니다. 모든 업데이트 메서드가 결과를 반환하는 것도 좋은 설계입니다.
core/datastore/src/test/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImplTest.kt (1)
21-231: 포괄적인 테스트 커버리지가 잘 구현되었습니다.모든 주요 시나리오(전체 업데이트, 부분 업데이트, 클리어, Flow 동작, 예외 전파)를 다루는 훌륭한 테스트 구성입니다. 특히 부분 업데이트 시 기존 값이 유지되는지 확인하는 테스트가 매우 중요합니다.
core/datastore/src/main/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImpl.kt (3)
16-62: 일관된 오류 처리 패턴이 잘 구현되었습니다.모든 메서드에서
runCatching을 사용한 일관된 오류 처리와 로깅이 잘 구현되어 있습니다. 이는 디버깅과 모니터링에 도움이 됩니다.
9-13: internal 가시성과 Hilt DI 구성이 적절합니다.클래스를
internal로 선언하여 모듈 간 결합도를 낮추고, 인터페이스를 통한 의존성 주입이 잘 구현되었습니다.
14-14: Flow 노출이 간결하고 효율적입니다.DataStore의 데이터를 직접 노출하는 방식이 간결하고 효율적입니다.
core/security/src/main/java/com/threegap/bitnagil/security/crypto/Crypto.kt
Show resolved
Hide resolved
core/datastore/src/main/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImpl.kt
Outdated
Show resolved
Hide resolved
core/datastore/src/main/java/com/threegap/bitnagil/datastore/model/AuthToken.kt
Show resolved
Hide resolved
- fake주입 클래스 접근 제어자 변경 - 테스트명을 직렬화/역직렬화에 맞게 수정
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/datastore/src/test/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImplTest.kt (1)
160-231: 예외 처리 테스트에서 코드 중복 제거 고려모든 예외 테스트 메서드들이 매우 유사한 구조를 가지고 있어 코드 중복이 발생하고 있습니다.
다음과 같이 헬퍼 메서드를 사용하여 중복을 줄일 수 있습니다:
private fun createBrokenDataStore(errorMessage: String): DataStore<AuthToken> { return object : DataStore<AuthToken> { override val data = flowOf(AuthToken()) override suspend fun updateData(transform: suspend (AuthToken) -> AuthToken): AuthToken { throw RuntimeException(errorMessage) } } } @Test(expected = RuntimeException::class) fun `updateAuthToken에서 예외 발생시 예외가 전파되어야 한다`() = runTest { val failingDataStore = AuthTokenDataStoreImpl(createBrokenDataStore("updateAuthToken failed")) failingDataStore.updateAuthToken(AuthToken("access", "refresh")) }이렇게 하면 유지보수성이 향상되고 테스트 의도가 더 명확해집니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/datastore/src/test/java/com/threegap/bitnagil/datastore/serializer/AuthTokenSerializerTest.kt(1 hunks)core/datastore/src/test/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImplTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/datastore/src/test/java/com/threegap/bitnagil/datastore/serializer/AuthTokenSerializerTest.kt
🧰 Additional context used
🪛 detekt (1.23.8)
core/datastore/src/test/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImplTest.kt
[warning] 41-41: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (6)
core/datastore/src/test/java/com/threegap/bitnagil/datastore/storage/AuthTokenDataStoreImplTest.kt (6)
21-31: 테스트 클래스 설정이 적절합니다TemporaryFolder 규칙을 사용하여 테스트 간 격리를 보장하고, 필요한 의존성들이 올바르게 선언되어 있습니다.
58-67: Setup 메서드가 올바르게 구현되었습니다DataStore 팩토리를 사용하여 임시 파일 기반 DataStore를 생성하고, 테스트 대상인 AuthTokenDataStoreImpl을 적절히 초기화했습니다.
69-84: 전체 토큰 업데이트 테스트가 잘 작성되었습니다Given-When-Then 패턴을 사용하여 명확한 테스트 구조를 만들었고, 업데이트된 토큰이 올바르게 반환되는지 검증합니다.
86-103: 부분 업데이트 테스트들이 훌륭합니다AccessToken과 RefreshToken을 개별적으로 업데이트할 때 다른 토큰이 유지되는지 확인하는 중요한 테스트 케이스들입니다. 실제 사용 시나리오를 잘 반영하고 있습니다.
Also applies to: 105-122
124-140: 토큰 클리어 테스트가 적절합니다클리어 후 기본값으로 리셋되는지 확인하는 중요한 테스트입니다.
142-158: Flow 테스트가 반응형 특성을 잘 검증합니다DataStore의 반응형 특성을 테스트하여 저장된 토큰이 Flow를 통해 올바르게 방출되는지 확인합니다.
[ PR Content ]
AuthToken 저장을 위한 DataStore를 구현합니다.
Related issue
Screenshot 📸
Work Description
To Reviewers 📢
추가로 구현간 참고한 아티클 공유해봅니다~!
How to Unit Test Jetpack DataStore
Summary by CodeRabbit
신규 기능
버그 수정
테스트
문서화
리팩터링
기타