Skip to content

Conversation

@i-meant-to-be
Copy link
Contributor

@i-meant-to-be i-meant-to-be commented Aug 2, 2025

🚩 연관 이슈

closed #12

📝 작업 내용

문제 상황

  • Google OAuth 도입을 위해 액세스 토큰과 리프레시 토큰을 저장할 수 있는 저장소 필요
  • 위 토큰들을 암호화할 수 있는 도구 필요

해결 방법

의사 결정 과정

블로그에 열심히 썼다. 링크에서 확인 가능.

Proto DataStore 도입

암호화된 토큰들을 기기에 저장하기 위해, 로컬 저장소로 사용 가능한 Proto DataStore 도입

토큰 저장소 TokenRepo 도입

  • Proto DataStore을 사용하는 토큰 저장소 도입
  • 이 토큰 저장소는 토큰 번들(액세스 토큰 + 리프레시 토큰)과 토큰 생성에 사용된 IV(initialization vector)를 저장함
  • 아래의 암호화 도구를 사용하여 토큰을 암호화/복호화한 후 사용할 수 있게 준비함

암호화 도구 CryptoManager 도입

  • 토큰 번들 R/W 시 암호화를 지원하는 암호화 도구 도입
  • 암호화에 사용하는 키가 유출될 위험이 없도록 방지하기 위해, 키를 대신 관리해주는 Android Keystore 도입
  • 암호화 시 AES-256, GCM 적용하여 무결성 및 인증도 지원

테스트 코드 추가

다음의 테스트 2종 추가:

  • Android 환경에서 TokenRepo가 Hilt로 DI를 제대로 받고 있는지 확인하는 테스트
  • CryptoManager의 암호화/복호화가 잘 이루어지는지 확인하는 테스트

그 외

향후 프로젝트 개발 청사진으로 사용할 구조를 담은 structure.txt 추가

🏞️ 스크린샷 (선택)

없음

🗣️ 리뷰 요구사항 (선택)

없음

Summary by CodeRabbit

Summary by CodeRabbit

  • 신규 기능

    • 암호화된 토큰 번들을 안전하게 저장 및 불러오는 기능이 추가되었습니다.
    • Jetpack DataStore 및 Google Protocol Buffers 기반의 데이터 저장소가 도입되었습니다.
    • 앱 내에 안전한 토큰 암호화 및 복호화 기능이 적용되었습니다.
    • 토큰 저장 및 조회를 위한 신규 데이터 모델과 저장소 인터페이스가 추가되었습니다.
  • 테스트

    • 토큰 저장소와 암호화 매니저에 대한 Hilt 기반의 계측 테스트가 추가되었습니다.
    • 테스트 환경에서 사용할 가짜 암호화 매니저 및 데이터 모듈이 도입되었습니다.
    • Hilt를 활용한 사용자 정의 테스트 러너가 도입되어 의존성 주입 환경이 개선되었습니다.
  • 문서화

    • 프로젝트 구조와 아키텍처를 설명하는 구조 파일이 추가되었습니다.
  • 작업 및 라이브러리

    • 프로토콜 버퍼, DataStore, Dagger Hilt 테스트 등 관련 라이브러리 및 플러그인이 추가되었습니다.

@i-meant-to-be i-meant-to-be self-assigned this Aug 2, 2025
@i-meant-to-be i-meant-to-be added feat 새로운 기능 추가 test 테스트 관련 labels Aug 2, 2025
@coderabbitai
Copy link

coderabbitai bot commented Aug 2, 2025

Walkthrough

이 변경사항은 Android 앱에 토큰 저장소와 암호화 도구를 도입합니다. DataStore, Protocol Buffers, Hilt DI, Dagger, 그리고 AES 기반의 암호화 관리 기능이 추가되었으며, 저장소 및 암호화 테스트 코드와 관련된 Hilt 테스트 환경도 구성되었습니다. 빌드 설정 및 프로젝트 구조도 이에 맞게 확장되었습니다.

Changes

Cohort / File(s) Change Summary
빌드 및 의존성 설정
build.gradle.kts, app/build.gradle.kts, gradle/libs.versions.toml
프로토콜 버퍼, DataStore, Dagger Hilt 테스트, protobuf 및 관련 의존성 추가 및 설정, protobuf Gradle 플러그인 통합
프로토콜 버퍼 정의 및 직렬화
app/src/main/proto/encrypted_tokens.proto, app/src/main/java/com/debatetimer/app/data/serializer/TokenSerializer.kt
EncryptedTokens 프로토콜 버퍼 메시지 정의, DataStore용 protobuf 직렬화기 및 Context 확장 프로퍼티 추가
토큰 저장소 및 암호화 도구 구현
app/src/main/java/com/debatetimer/app/data/model/TokenBundle.kt, app/src/main/java/com/debatetimer/app/data/repo/TokenRepo.kt, app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt, app/src/main/java/com/debatetimer/app/util/crypto/CryptoManager.kt, app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt
TokenBundle 데이터 클래스, TokenRepo 인터페이스 및 구현체, CryptoManager 인터페이스 및 AES 기반 구현체 추가
DI 모듈 및 테스트 설정
app/src/main/java/com/debatetimer/app/di/DataModule.kt, app/src/androidTest/java/com/debatetimer/app/di/FakeDataModule.kt
Hilt DI 모듈 및 테스트용 페이크 DI 모듈 추가, 실제/가짜 구현체 바인딩
테스트 러너 및 테스트 코드
app/src/androidTest/java/com/debatetimer/app/HiltTestRunner.kt, app/src/androidTest/java/com/debatetimer/app/data/repo/TokenRepoImplHiltTest.kt, app/src/androidTest/java/com/debatetimer/app/util/crypto/CryptoManagerImplTest.kt, app/src/androidTest/java/com/debatetimer/app/util/crypto/FakeCryptoManagerImpl.kt
Hilt 테스트 러너, TokenRepo 및 CryptoManager 테스트, 페이크 CryptoManager 구현체 추가
프로젝트 구조 문서
app/src/main/java/com/debatetimer/app/structure.txt
계층별 프로젝트 디렉토리 구조 설명 파일 추가

Sequence Diagram(s)

sequenceDiagram
    participant App
    participant TokenRepoImpl
    participant DataStore
    participant CryptoManager

    App->>TokenRepoImpl: saveTokens(TokenBundle)
    TokenRepoImpl->>CryptoManager: encrypt(json(TokenBundle))
    CryptoManager-->>TokenRepoImpl: (encryptedBytes, iv)
    TokenRepoImpl->>DataStore: update(EncryptedTokens)
    DataStore-->>TokenRepoImpl: 저장 완료

    App->>TokenRepoImpl: getTokens()
    TokenRepoImpl->>DataStore: read()
    DataStore-->>TokenRepoImpl: EncryptedTokens
    TokenRepoImpl->>CryptoManager: decrypt(encryptedBytes, iv)
    CryptoManager-->>TokenRepoImpl: json(TokenBundle)
    TokenRepoImpl-->>App: Result<TokenBundle>
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective Addressed Explanation
토큰 저장소 구현 (#12)
암호화 도구 구현 (#12)

Assessment against linked issues: Out-of-scope changes

해당 변경사항 중 명확하게 범위를 벗어난 코드 변경은 발견되지 않았습니다.

Poem

토큰을 안전하게, 암호화로 꽁꽁!
데이터는 DataStore에, 비밀은 AES로 봉!
Hilt와 프로토콜 버퍼, 토끼도 감탄해요,
테스트도 빵빵, 구조도 반듯—
오늘도 코드는 튼튼하게,
🐰 "보안은 내 친구!"
토끼가 박수 짝짝짝!

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0c8e5c7 and 3c10768.

📒 Files selected for processing (2)
  • app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt
  • app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.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: build_and_test
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/#12

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🧹 Nitpick comments (18)
app/src/main/java/com/debatetimer/app/data/model/TokenBundle.kt (1)

5-9: 보안을 위한 개선 사항을 고려해보세요.

데이터 클래스 구조는 올바르게 구현되었지만, 보안 측면에서 몇 가지 개선점이 있습니다:

  1. toString() 메서드가 토큰을 로그에 노출시킬 수 있습니다
  2. 메모리에서 토큰이 평문으로 저장됩니다

다음과 같은 개선을 고려해보세요:

 @Serializable
 data class TokenBundle(
     val accessToken: String,
     val refreshToken: String,
-)
+) {
+    override fun toString(): String = "TokenBundle(accessToken=***, refreshToken=***)"
+}
app/src/main/java/com/debatetimer/app/data/repo/TokenRepo.kt (1)

5-8: 리포지토리 인터페이스 설계가 잘 구현되었습니다.

비동기 처리와 타입 안전한 에러 처리를 위한 suspendResult<T> 사용이 적절합니다.

API 설계 개선을 위한 제안사항:

 interface TokenRepo {
+    /**
+     * 저장된 토큰을 조회합니다.
+     * @return 성공 시 TokenBundle, 실패 시 에러
+     */
     suspend fun getTokens(): Result<TokenBundle>
+    
+    /**
+     * 토큰을 안전하게 저장합니다.
+     * @param bundle 저장할 토큰 번들
+     * @return 성공 시 Unit, 실패 시 에러
+     */
-    suspend fun saveTokens(bundle: TokenBundle): Result<Boolean>
+    suspend fun saveTokens(bundle: TokenBundle): Result<Unit>
+    
+    /**
+     * 저장된 토큰을 삭제합니다.
+     */
+    suspend fun clearTokens(): Result<Unit>
 }
app/src/main/java/com/debatetimer/app/util/crypto/CryptoManager.kt (1)

3-6: 잘 설계된 암호화 인터페이스입니다.

인터페이스 설계가 우수합니다:

  • 암호화 시 IV(Initialization Vector)를 함께 반환하는 것은 보안 모범 사례입니다
  • 명확한 메서드 시그니처로 AES-GCM 암호화 패턴에 적합합니다
  • 인터페이스 추상화를 통해 실제 구현체와 테스트용 가짜 구현체를 분리할 수 있습니다

더 나은 문서화를 위해 KDoc 주석 추가를 고려해보세요:

+/**
+ * 토큰 암호화 및 복호화를 담당하는 인터페이스
+ */
 interface CryptoManager {
+    /**
+     * 평문을 암호화합니다.
+     * @param plainText 암호화할 평문
+     * @return 암호화된 데이터와 IV의 쌍
+     */
     fun encrypt(plainText: String): Pair<ByteArray, ByteArray>
+    /**
+     * 암호화된 데이터를 복호화합니다.
+     * @param encryptedText 암호화된 데이터
+     * @param iv 초기화 벡터
+     * @return 복호화된 평문
+     */
     fun decrypt(encryptedText: ByteArray, iv: ByteArray): String
 }
app/src/main/java/com/debatetimer/app/structure.txt (3)

5-5: 패키지 명을 실제 프로젝트에 맞게 수정하세요.

플레이스홀더 패키지명 com/yourcompany/yourapp/이 사용되었습니다. 실제 프로젝트 패키지명인 com/debatetimer/app/로 변경해야 합니다.

-                └── com/yourcompany/yourapp/
+                └── com/debatetimer/app/

9-16: API 관련 파일명을 프로젝트에 맞게 구체화하세요.

플레이스홀더 파일명들(YourApiService.kt, YourApiClient.kt, YourRepository.kt 등)이 사용되었습니다. Google OAuth 통합을 위한 구체적인 파일명으로 변경하는 것을 고려해보세요.

예시:

-                    │   │   │   └── YourApiService.kt             // Retrofit interface for API endpoints
+                    │   │   │   └── GoogleOAuthService.kt         // Retrofit interface for Google OAuth endpoints
-                    │   │   └── LoginResponse.kt              // API-specific data classes (e.g., JSON response)
+                    │   │   └── OAuthTokenResponse.kt         // OAuth token response data class

17-18: 토큰 관련 모델 추가를 고려하세요.

PR에서 구현된 TokenBundle 클래스와 암호화 관련 모델들이 구조도에 누락되었습니다.

                    │   └── model/
                    │   │   └── User.kt                         // Domain-specific data classes for the app
+                    │   │   └── TokenBundle.kt                  // OAuth token bundle data class
app/src/androidTest/java/com/debatetimer/app/util/crypto/FakeCryptoManagerImpl.kt (1)

14-17: 복호화 메서드에 입력 검증 추가를 고려하세요.

현재 구현은 입력 데이터가 올바른 형식인지 검증하지 않습니다. 테스트 환경에서도 예외 상황을 시뮬레이션할 수 있도록 개선할 수 있습니다.

 override fun decrypt(encryptedText: ByteArray, iv: ByteArray): String {
     val fakePlaintext = String(encryptedText, Charsets.UTF_8)
+    if (!fakePlaintext.endsWith(suffix)) {
+        throw IllegalArgumentException("Invalid encrypted text format")
+    }
     return fakePlaintext.removeSuffix(suffix)
 }
app/src/androidTest/java/com/debatetimer/app/util/crypto/CryptoManagerImplTest.kt (1)

23-34: 추가 테스트 케이스 추가를 고려하세요.

현재 기본적인 라운드트립 테스트만 있습니다. 암호화 컴포넌트의 견고성을 위해 추가 테스트를 고려해보세요.

다음과 같은 테스트 케이스들을 추가할 수 있습니다:

@Test
fun encrypt_emptyString_handlesCorrectly() {
    val originalText = ""
    val (ciphertext, iv) = cryptoManager.encrypt(originalText)
    val decryptedText = cryptoManager.decrypt(ciphertext, iv)
    assertThat(decryptedText, equalTo(originalText))
}

@Test
fun encrypt_longString_handlesCorrectly() {
    val originalText = "A".repeat(1000)
    val (ciphertext, iv) = cryptoManager.encrypt(originalText)
    val decryptedText = cryptoManager.decrypt(ciphertext, iv)
    assertThat(decryptedText, equalTo(originalText))
}

@Test
fun encrypt_specialCharacters_handlesCorrectly() {
    val originalText = "특수문자!@#$%^&*()[]{}|\\:;\"'<>,.?/~`"
    val (ciphertext, iv) = cryptoManager.encrypt(originalText)
    val decryptedText = cryptoManager.decrypt(ciphertext, iv)
    assertThat(decryptedText, equalTo(originalText))
}
app/src/androidTest/java/com/debatetimer/app/data/repo/TokenRepoImplHiltTest.kt (1)

30-46: 추가 테스트 시나리오를 고려하세요.

기본적인 저장/조회 테스트는 잘 구현되었습니다. 하지만 더 포괄적인 테스트 커버리지를 위해 추가 시나리오를 고려해보세요.

다음과 같은 테스트들을 추가할 수 있습니다:

@Test
fun getTokens_whenNoTokensSaved_returnsFailure() = runBlocking {
    val result = tokenRepo.getTokens()
    assertThat(result.isFailure, equalTo(true))
}

@Test
fun saveTokens_overwriteExisting_returnsNewTokens() = runBlocking {
    val firstBundle = TokenBundle("token1", "refresh1")
    val secondBundle = TokenBundle("token2", "refresh2")
    
    tokenRepo.saveTokens(firstBundle)
    tokenRepo.saveTokens(secondBundle)
    val result = tokenRepo.getTokens()
    
    assertThat(result.getOrNull(), equalTo(secondBundle))
}

@Test
fun saveTokens_emptyTokens_handlesCorrectly() = runBlocking {
    val emptyBundle = TokenBundle("", "")
    tokenRepo.saveTokens(emptyBundle)
    val result = tokenRepo.getTokens()
    
    assertThat(result.getOrNull(), equalTo(emptyBundle))
}
app/src/main/java/com/debatetimer/app/data/serializer/TokenSerializer.kt (2)

13-13: 파일명 상수 정의가 적절합니다.

하드코딩된 파일명 대신 상수로 정의한 것이 좋습니다. 하지만 향후 다중 사용자 지원이나 파일명 충돌을 고려한다면 더 유연한 접근을 고려해볼 수 있습니다.

필요시 사용자별 파일명을 지원할 수 있도록 개선할 수 있습니다:

fun getTokenFileName(userId: String? = null): String {
    return if (userId != null) {
        "encrypted_tokens_${userId}.pb"
    } else {
        FILE_NAME
    }
}

18-24: 에러 처리 개선을 고려하세요.

현재 InvalidProtocolBufferException만 처리하고 있습니다. 다른 IO 예외나 스키마 버전 불일치 등의 상황도 고려해볼 수 있습니다.

 override suspend fun readFrom(input: InputStream): EncryptedTokens {
     try {
         return EncryptedTokens.parseFrom(input)
-    } catch (exception: InvalidProtocolBufferException) {
-        throw CorruptionException("Cannot read proto.", exception)
+    } catch (exception: InvalidProtocolBufferException) {
+        throw CorruptionException("Cannot read encrypted tokens proto: ${exception.message}", exception)
+    } catch (exception: Exception) {
+        throw CorruptionException("Unexpected error reading encrypted tokens", exception)
     }
 }
app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt (4)

37-49: Result 패턴 사용이 적절하지만 중복 코드가 있습니다.

getTokens 메서드에서 이미 getBundle()이 예외를 던지므로 추가적인 try-catch가 불필요할 수 있습니다.

다음과 같이 코드를 간소화할 수 있습니다:

 override suspend fun getTokens(): Result<TokenBundle> {
-    try {
+    return try {
         val bundle = getBundle()
-
-        return if (bundle != null) {
+        if (bundle != null) {
             Result.success(bundle)
         } else {
             Result.failure(NoSuchElementException())
         }
-    } catch (e: Exception) {
-        return Result.failure(e)
+    } catch (e: Exception) {
+        Result.failure(e)
     }
 }

9-9: Gson 사용 시 보안 고려사항을 검토하세요.

Gson 인스턴스를 매번 새로 생성하는 것보다 싱글톤으로 사용하는 것이 성능상 유리하며, 토큰과 같은 민감한 데이터 직렬화 시 추가 보안 설정을 고려해야 합니다.

다음과 같이 Gson 인스턴스를 개선하는 것을 권장합니다:

클래스 상단에 companion object 추가:

+    companion object {
+        private val gson = Gson().newBuilder()
+            .disableHtmlEscaping()
+            .create()
+    }

그리고 Gson() 호출을 gson으로 교체:

-            return Gson().fromJson(decryptedTokens, TokenBundle::class.java)
+            return gson.fromJson(decryptedTokens, TokenBundle::class.java)

-                val serializedBundle = Gson().toJson(bundle)
+                val serializedBundle = gson.toJson(bundle)

37-49: Result 패턴 사용이 우수합니다.

적절한 Result 패턴을 사용하여 성공/실패를 명확하게 처리하고 있습니다. 하지만 NoSuchElementException을 실패로 반환하는 것보다는 별도의 예외 타입을 고려해보세요.

토큰이 없는 경우를 위한 커스텀 예외를 생성하는 것이 어떨까요?

class TokenNotFoundException : Exception("No tokens found in storage")

53-61: Gson 인스턴스 재사용 고려

Gson() 을 호출할 때마다 새 인스턴스를 만들고 있습니다. DI 로 주입하거나 lazy 싱글톤으로 재사용하면 객체 생성 비용을 줄일 수 있습니다.

app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt (3)

58-67: 입력 검증 추가를 고려해보세요.

암호화 메서드가 올바르게 구현되었지만, 빈 문자열이나 null 입력에 대한 검증을 추가하는 것이 좋겠습니다.

 override fun encrypt(plainText: String): Pair<ByteArray, ByteArray> {
+    require(plainText.isNotEmpty()) { "PlainText cannot be empty" }
+    
     // 1. Load cipher instance
     val cipher = Cipher.getInstance(TRANSFORMATION)

69-79: 복호화 구현이 안전합니다.

GCM 모드의 적절한 태그 길이(128비트)와 IV 처리가 올바르게 구현되어 있습니다. 하지만 입력 검증을 추가하면 더 견고해질 것입니다.

 override fun decrypt(encryptedText: ByteArray, iv: ByteArray): String {
+    require(encryptedText.isNotEmpty()) { "Encrypted text cannot be empty" }
+    require(iv.size == 12) { "IV must be 12 bytes for GCM mode" }
+    
     // 1. Load cipher instance and prepare variables
     val cipher = Cipher.getInstance(TRANSFORMATION)

69-79: IV 길이 검증이 필요합니다

AES/GCM 은 일반적으로 12-byte IV 를 기대합니다.
빈 배열이나 예상보다 짧은 IV 가 전달되면 IllegalArgumentException 이 발생합니다.
복호화 전에 iv.size != 12 와 같은 검사를 추가해 조기에 예외를 명확히 처리해 주세요.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 714da27 and 11e2636.

📒 Files selected for processing (17)
  • app/build.gradle.kts (4 hunks)
  • app/src/androidTest/java/com/debatetimer/app/HiltTestRunner.kt (1 hunks)
  • app/src/androidTest/java/com/debatetimer/app/data/repo/TokenRepoImplHiltTest.kt (1 hunks)
  • app/src/androidTest/java/com/debatetimer/app/di/FakeDataModule.kt (1 hunks)
  • app/src/androidTest/java/com/debatetimer/app/util/crypto/CryptoManagerImplTest.kt (1 hunks)
  • app/src/androidTest/java/com/debatetimer/app/util/crypto/FakeCryptoManagerImpl.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/data/model/TokenBundle.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/data/repo/TokenRepo.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/data/serializer/TokenSerializer.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/di/DataModule.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/structure.txt (1 hunks)
  • app/src/main/java/com/debatetimer/app/util/crypto/CryptoManager.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt (1 hunks)
  • app/src/main/proto/encrypted_tokens.proto (1 hunks)
  • build.gradle.kts (1 hunks)
  • gradle/libs.versions.toml (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: in the debate-timer android project, navigation destinations in mainnavhost.kt are being added incre...
Learnt from: i-meant-to-be
PR: debate-timer/debate-timer-fe-android#11
File: app/src/main/java/com/debatetimer/app/navigation/MainNavHost.kt:10-14
Timestamp: 2025-08-02T07:20:07.945Z
Learning: In the debate-timer Android project, navigation destinations in MainNavHost.kt are being added incrementally during development, so an empty NavHost block in early stages is intentional and acceptable.

Applied to files:

  • app/src/main/java/com/debatetimer/app/structure.txt
🪛 Buf (1.55.1)
app/src/main/proto/encrypted_tokens.proto

1-1: Files must have a package defined.

(PACKAGE_DEFINED)

⏰ 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: build_and_test
🔇 Additional comments (24)
build.gradle.kts (1)

11-11: 프로토콜 버퍼 플러그인 추가가 올바르게 구현되었습니다.

다른 플러그인들과 동일한 패턴을 따라 apply false로 선언되어 있어, 하위 모듈에서 필요에 따라 적용할 수 있도록 설정되었습니다.

gradle/libs.versions.toml (3)

48-48: 테스트 및 데이터 저장소 종속성이 올바르게 추가되었습니다.

Hilt 테스트 지원과 DataStore 종속성들이 적절하게 구성되어 있습니다. 프로토콜 버퍼와의 통합도 올바르게 설정되었습니다.

Also applies to: 53-55


65-65: 프로토콜 버퍼 플러그인 별칭이 올바르게 정의되었습니다.

플러그인 버전이 프로토콜 버퍼 Gradle 플러그인 버전과 일치하며, 명명 규칙을 잘 따르고 있습니다.


22-24: Protobuf 및 DataStore 버전 재검증 필요

  • GitHub API로 조회된 최신 Protobuf 릴리스 태그(v31.1)와 설정된 protobufKotlin = "4.31.1"가 정확히 일치하는지 확인해주세요.
  • DataStore 버전 조회 스크립트가 결과를 반환하지 않아, Maven Central API(예: https://search.maven.org/solrsearch/select?q=g:%22androidx.datastore%22+AND+a:%22datastore-core%22&rows=1&wt=json)나 공식 AndroidX 릴리스 노트를 통해 datastore = "1.1.7"이 최신인지 검증해주세요.
  • 보안 취약점(DoS)은 >= 4.0.0.rc.1, < 4.28.2 범위에서 보고되었으며, 현재 버전 4.31.1은 해당 범위를 벗어나므로 별도 조치는 필요하지 않습니다.

위 사항을 검토하여 문제가 없다면 그대로 병합하고, 버전이 불일치하거나 더 최신 릴리스가 있다면 적절히 업데이트해주세요.

app/src/main/proto/encrypted_tokens.proto (1)

6-9: 암호화된 토큰 메시지 구조가 적절하게 설계되었습니다.

  • bytes 타입 사용으로 바이너리 데이터 저장에 적합
  • 암호화된 토큰 번들과 초기화 벡터의 분리가 보안상 올바름
  • 필드 번호 할당이 적절함
app/src/androidTest/java/com/debatetimer/app/HiltTestRunner.kt (1)

8-20: LGTM! 표준적인 Hilt 테스트 러너 구현입니다.

HiltTestApplication을 사용하여 테스트 환경에서 Hilt DI를 활성화하는 표준적인 패턴을 올바르게 구현하였습니다. 한글 주석도 명확하게 작성되어 있어 코드의 목적을 잘 설명하고 있습니다.

app/build.gradle.kts (3)

12-12: Protobuf 플러그인 추가가 적절합니다.

Google Protocol Buffers 플러그인이 올바르게 추가되었습니다. 토큰 저장을 위한 직렬화에 필요한 설정입니다.


26-26: 커스텀 테스트 러너 설정이 올바릅니다.

HiltTestRunner로 변경하여 Hilt DI를 테스트 환경에서 사용할 수 있도록 적절하게 구성되었습니다. 구현된 HiltTestRunner 클래스와 일치합니다.


104-107: 새로운 종속성들이 적절하게 추가되었습니다.

DataStore와 protobuf 관련 종속성들이 토큰 저장 기능에 필요한 것들로 적절하게 선택되었습니다:

  • protobuf-kotlin-lite: Android 최적화를 위한 lite 버전
  • datastore-core/preferences: 암호화된 토큰 저장소 구현에 필요
  • hilt-android-testing: 테스트 환경에서 Hilt DI 지원
app/src/main/java/com/debatetimer/app/di/DataModule.kt (1)

13-27: 표준적이고 잘 구성된 Hilt 모듈입니다.

Dagger Hilt 모듈이 모범 사례를 따라 올바르게 구현되었습니다:

  • SingletonComponent에 설치하여 앱 전체에서 싱글톤으로 사용
  • @BINDS를 사용하여 효율적인 인터페이스-구현체 바인딩
  • CryptoManager와 TokenRepo 모두 싱글톤 스코프로 적절하게 설정
  • 추상 클래스와 추상 메서드를 사용한 깔끔한 구조
app/src/androidTest/java/com/debatetimer/app/di/FakeDataModule.kt (1)

13-33: 테스트용 모듈이 잘 설계되었습니다.

@TestInstallIn을 사용하여 프로덕션 모듈을 테스트 환경에서 적절하게 대체하는 우수한 설계입니다:

  • FakeCryptoManagerImpl로 암호화 동작을 제어 가능하게 만듦
  • TokenRepoImpl은 실제 구현체를 사용하여 토큰 저장소 로직을 실제로 테스트
  • 이 접근 방식으로 토큰 저장소의 동작을 격리된 환경에서 안정적으로 테스트 가능
  • 한글 주석이 모듈의 목적을 명확하게 설명함
app/src/androidTest/java/com/debatetimer/app/util/crypto/FakeCryptoManagerImpl.kt (1)

5-12: 테스트용 가짜 구현이 적절합니다.

간단한 접미사 추가/제거 방식으로 암호화/복호화를 시뮬레이션하는 것은 테스트 목적에 적합합니다. 결정적(deterministic) 동작으로 테스트 결과를 예측 가능하게 만듭니다.

app/src/androidTest/java/com/debatetimer/app/util/crypto/CryptoManagerImplTest.kt (1)

11-20: 테스트 설정이 적절합니다.

AndroidJUnit4 러너를 사용하고 각 테스트 전에 CryptoManagerImpl을 직접 생성하는 방식이 단위 테스트에 적합합니다. Hilt를 사용하지 않고 직접 인스턴스를 생성하는 것이 이 경우에는 올바른 접근입니다.

app/src/androidTest/java/com/debatetimer/app/data/repo/TokenRepoImplHiltTest.kt (1)

14-28: Hilt 통합 테스트 설정이 올바릅니다.

HiltAndroidRule을 사용하고 @before에서 의존성 주입을 수행하는 방식이 적절합니다. FakeDataModule을 통해 가짜 암호화 매니저를 주입받아 테스트하는 구조가 좋습니다.

app/src/main/java/com/debatetimer/app/data/serializer/TokenSerializer.kt (2)

15-29: 프로토버프 직렬화 구현이 올바릅니다.

DataStore Serializer를 올바르게 구현했습니다. InvalidProtocolBufferException을 CorruptionException으로 변환하는 에러 처리가 적절하고, 프로토버프의 기본 파싱/쓰기 메서드를 올바르게 사용했습니다.


31-34: DataStore 확장 프로퍼티가 편리합니다.

Context 확장 프로퍼티를 통해 DataStore 인스턴스에 쉽게 접근할 수 있도록 한 것이 좋습니다. 이는 Android DataStore의 권장 패턴을 따릅니다.

app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt (5)

12-22: 암호화 상수 정의가 적절합니다.

AES-256-GCM 알고리즘 사용과 관련 상수들이 보안 모범 사례에 따라 올바르게 정의되어 있습니다. GCM 모드는 인증과 암호화를 동시에 제공하여 데이터 무결성을 보장합니다.


32-56: 키 생성 로직이 잘 구현되어 있습니다.

기존 키 확인 후 없을 경우에만 새로 생성하는 로직이 적절하며, KeyGenParameterSpec 설정도 보안 요구사항에 맞게 구성되어 있습니다.


58-67: 암호화 메서드 구현을 확인했습니다.

암호화 로직이 올바르게 구현되어 있고 IV와 암호문을 함께 반환하는 것이 적절합니다.


13-22: 암호화 상수 설정이 우수합니다.

AES-256-GCM 알고리즘 선택과 상수 정의가 보안 모범 사례를 잘 따르고 있습니다. GCM 모드는 인증된 암호화를 제공하여 데이터 무결성과 기밀성을 모두 보장합니다.


32-56: 키 관리 로직이 안전하게 구현되었습니다.

Android Keystore를 사용한 키 생성 및 검색 로직이 올바르게 구현되어 있습니다. 하드웨어 지원 보안과 적절한 키 용도 설정이 잘 되어 있습니다.

app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt (3)

15-20: 클래스 구조가 적절합니다.

의존성 주입과 DataStore 초기화가 올바르게 구현되어 있습니다.


51-67: saveTokens 메서드 구현이 견고합니다.

토큰 저장 로직이 올바르게 구현되어 있고, 암호화와 DataStore 업데이트가 적절히 처리되고 있습니다.


53-61: DataStore 업데이트 로직이 적절합니다.

DataStore의 updateData 사용과 protobuf builder 패턴이 올바르게 구현되어 있습니다. 원자적 업데이트를 보장하여 데이터 일관성을 유지합니다.

Copy link

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (1)
app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt (1)

48-49: NoSuchElementException 처리가 불필요합니다.

DataStore.data.first()는 빈 데이터의 경우에도 기본값을 반환하므로 이 예외는 발생하지 않습니다. 이미 33-35번 줄에서 빈 데이터를 처리하고 있으므로 이 catch 블록은 제거해도 됩니다.

-        } catch (_: NoSuchElementException) {
-            return Result.success(null)
🧹 Nitpick comments (2)
app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt (1)

59-60: 토큰 유효성 검증에 에러 메시지를 추가하세요.

현재 require 문에 에러 메시지가 없어 디버깅 시 어떤 토큰이 문제인지 파악하기 어렵습니다.

-            require(bundle.accessToken.isNotBlank())
-            require(bundle.refreshToken.isNotBlank())
+            require(bundle.accessToken.isNotBlank()) { "Access token cannot be blank" }
+            require(bundle.refreshToken.isNotBlank()) { "Refresh token cannot be blank" }
app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt (1)

31-31: 더 구체적인 예외 타입을 사용하는 것을 고려하세요.

RuntimeException은 너무 일반적입니다. 암호화 관련 작업임을 명시하는 커스텀 예외나 IllegalStateException을 사용하는 것이 좋습니다.

-            throw RuntimeException("Failed to load Android KeyStore", e)
+            throw IllegalStateException("Failed to load Android KeyStore", e)
-            throw RuntimeException("Failed to decrypt data", e)
+            throw IllegalStateException("Failed to decrypt data", e)

Also applies to: 94-94

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11e2636 and 0c8e5c7.

📒 Files selected for processing (6)
  • app/src/main/java/com/debatetimer/app/data/model/TokenBundle.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/data/repo/TokenRepo.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/util/crypto/CryptoManager.kt (1 hunks)
  • app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt (1 hunks)
  • app/src/main/proto/encrypted_tokens.proto (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • app/src/main/java/com/debatetimer/app/data/repo/TokenRepo.kt
  • app/src/main/java/com/debatetimer/app/data/model/TokenBundle.kt
  • app/src/main/java/com/debatetimer/app/util/crypto/CryptoManager.kt
🧰 Additional context used
🪛 Buf (1.55.1)
app/src/main/proto/encrypted_tokens.proto

3-3: Files with package "com.debatetimer.app.data" must be within a directory "com/debatetimer/app/data" relative to root but were in directory "app/src/main/proto".

(PACKAGE_DIRECTORY_MATCH)

🔇 Additional comments (4)
app/src/main/proto/encrypted_tokens.proto (1)

1-11: 프로토콜 버퍼 정의가 적절하게 구성되었습니다.

패키지 선언과 Java 옵션이 올바르게 설정되었고, 암호화된 토큰과 초기화 벡터를 저장하기 위한 메시지 구조가 간결하고 명확합니다.

app/src/main/java/com/debatetimer/app/data/repo/TokenRepoImpl.kt (1)

21-25: Gson 인스턴스 구성이 적절합니다.

disableHtmlEscaping()을 사용하여 불필요한 HTML 이스케이핑을 방지하고, companion object로 재사용 가능하게 구현한 것이 좋습니다.

app/src/main/java/com/debatetimer/app/util/crypto/CryptoManagerImpl.kt (2)

25-33: KeyStore 지연 초기화가 잘 구현되었습니다.

lazy 델리게이트를 사용하여 실제 사용 시점에 초기화되도록 했고, 예외 처리도 적절히 추가되었습니다.


78-96: 복호화 메서드의 유효성 검증과 예외 처리가 우수합니다.

입력 데이터와 IV에 대한 검증이 명확하고, GCM 모드에 맞는 12바이트 IV 길이 검증도 포함되어 있습니다. 예외 처리도 적절히 구현되었습니다.

@i-meant-to-be i-meant-to-be merged commit c35452d into develop Aug 4, 2025
2 checks passed
@i-meant-to-be i-meant-to-be deleted the feat/#12 branch August 4, 2025 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feat 새로운 기능 추가 test 테스트 관련

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] 토큰 저장소 및 암호화 도구 구현

2 participants