Feature/#70 kakao login server - #71
Merged
Merged
Conversation
서버 통신을 위한 Ktor 3.3.1 HTTP 클라이언트를 KMP 방식으로 구성하고, JSON pretty print 로깅 기능을 포함한 Koin 싱글턴 모듈을 등록 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 서버 로그인 API 연동 레이어 추가 (AuthRepository, RemoteAuthDataSource, SignInRequest/Response) - KakaoLoginUseCase에서 카카오 로그인 후 서버 signIn API 호출하도록 변경 - 카카오 로그인 시 nonce를 SHA-256 해싱하여 전달하도록 수정 (Android/iOS) - 공통 ApiResponse/ApiException 모델 및 SHA-256 유틸리티 추가 - HTTP Client 로깅 개선 (ResponseObserver로 응답 바디 별도 출력) - CheckNeedForceUpdateUseCaseTest MockRepository에 스토어 URL 필드 추가 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughKtor 멀티플랫폼 HTTP 클라이언트 의존성 및 플랫폼별 엔진(OkHttp/Darwin), SHA-256 해시 구현, 카카오 로그인 흐름 변경(원본 nonce + 해시된 nonce), 원격 인증 데이터 소스/리포지토리, API 모델과 DI 모듈을 추가합니다. (50단어 이내) Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Mobile Client
participant KLC as KakaoLoginUseCase
participant KSC as KakaoSignInClient
participant KakaoSDK as Kakao SDK
participant AuthRepo as AuthRepository
participant Backend as Backend API
Client->>KLC: invoke(context)
KLC->>KSC: signIn(context)
KSC->>KSC: rawNonce 생성
KSC->>KSC: hashedNonce = sha256Hex(rawNonce)
KSC->>KakaoSDK: loginWithKakaoTalk(hashedNonce)
KakaoSDK-->>KSC: idToken
KSC-->>KLC: KakaoSignInResult(idToken, nonce = rawNonce)
KLC->>AuthRepo: signIn(idToken, nonce = rawNonce)
AuthRepo->>Backend: POST /users/signin { id_token, nonce }
Backend-->>AuthRepo: SignInResponse(accessToken, refreshToken, isPortalLinked)
AuthRepo-->>KLC: SignInResult
KLC-->>Client: Result<SignInResult>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composeApp/src/androidMain/kotlin/com/chukchukhaksa/mobile/common/kmp/KakaoSignInClient.android.kt (1)
20-35:⚠️ Potential issue | 🔴 Criticalnonce를 SHA-256으로 해시하면 안 됩니다. 서버 검증에 실패합니다.
Line 20에서 nonce를 SHA-256으로 해시하여 SDK에 전달하고 있으나, OpenID Connect 명세와 Kakao 공식 문서에 따르면:
- Kakao Android SDK는 nonce를 그대로(평문) 전달해야 함
- ID Token의
nonceclaim은 원문 그대로 포함되어야 함 (수정 없음)- 서버는 ID Token의 nonce claim이 원래 저장한 평문 nonce와 정확히 일치하는지 검증
현재 코드는 해시된 nonce를 ID Token에 포함시키지만, 서버는 평문 nonce로 검증하므로 검증이 실패합니다.
sha256Hex(nonce)호출을 제거하고 평문 nonce를 그대로 사용하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/androidMain/kotlin/com/chukchukhaksa/mobile/common/kmp/KakaoSignInClient.android.kt` around lines 20 - 35, Remove the SHA-256 hashing step and pass the original plaintext nonce through to the Kakao SDK and result; specifically delete the sha256Hex(nonce) usage (the val hashedNonce = sha256Hex(nonce) and any place hashedNonce is supplied to the SDK) and ensure the code uses the original nonce variable when initiating the sign-in flow and when creating KakaoSignInResult (keep the callback and continuation logic intact, but stop calling sha256Hex and stop supplying hashedNonce to the SDK so the ID Token’s nonce claim matches the original plaintext nonce).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/auth/RemoteAuthDataSourceImpl.kt`:
- Around line 18-23: RemoteAuthDataSourceImpl.signIn currently triggers full
response logging via HttpClientModule.ResponseObserver when isDebug=true, which
causes sensitive fields (SignInResponse.accessToken, refreshToken) to be written
to logs after getDataOrThrow(); update ResponseObserver to detect authentication
responses (e.g., request.url.path contains "users/signin" or response body type
SignInResponse) and redact or filter sensitive fields before logging (replace
tokens with masked values), or skip logging for auth endpoints entirely; ensure
masking logic targets the SignInResponse fields accessToken and refreshToken and
is applied before any call to getDataOrThrow().
In
`@composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/common/ApiResponse.kt`:
- Around line 18-23: Update ApiResponse<T>.getDataOrThrow to validate the
response success flag as well as data: inside getDataOrThrow (function
ApiResponse.getDataOrThrow) check if success == true before returning data, and
if success is false (even when data != null) throw ApiException using
error?.code and error?.message (fallback to the existing default message) so
responses with success=false never return data; keep the existing error message
fallbacks.
In
`@composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/di/HttpClientModule.kt`:
- Around line 32-40: The HttpClient configuration in the
HttpClient(httpClientEngineFactory) block is missing the HttpTimeout plugin, so
add install(HttpTimeout) inside that client builder (near
install(ContentNegotiation)) and set appropriate timeouts (e.g.,
requestTimeoutMillis, connectTimeoutMillis, socketTimeoutMillis) to prevent
indefinite hangs; update the HttpClientModule where HttpClient(...) is defined
to include install(HttpTimeout) with sensible values or configurable constants.
- Around line 42-63: The HTTP debug logging currently exposes sensitive headers
and full response bodies; update the Logging installation in HttpClientModule to
call the Ktor sanitizeHeader API to mask Authorization, Cookie, Set-Cookie (and
any other sensitive header keys) before logging and change the log level
handling so headers are sanitized even in debug; also modify the
ResponseObserver onResponse flow (which currently calls response.bodyAsText()
and prettyPrintJson()) to first check Content-Type (only log JSON/text), enforce
a maximum size (e.g., skip or truncate bodies above a safe threshold), and avoid
logging bodies when they contain potential PII or binary data—apply these checks
and truncation before calling prettyPrintJson() or Napier.d to ensure sensitive
data is never emitted.
In
`@composeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/common/kmp/Sha256Test.kt`:
- Around line 50-57: The test uuidFormat in Sha256Test.kt is inconsistent: its
name and comment say it should use a UUID-like nonce but it calls
sha256Hex("foo"); either rename the test (e.g., sha256OfFoo) to match the
current input or replace the input with a UUID-form string (or generate one) to
match the UUID intent; update the assertion expected hash to the correct SHA-256
hex for the chosen input and keep the reference to sha256Hex so the test target
remains clear.
In
`@composeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/domain/config/usecase/CheckNeedForceUpdateUseCaseTest.kt`:
- Line 33: The tests in CheckNeedForceUpdateUseCaseTest only assert
result.needForceUpdate which can miss regressions in the platform-specific store
URL mapping; update each assertion block (the tests that currently call
assertTrue(result.needForceUpdate)) to also assert the URL field on the same
result object (e.g., assertEquals(expectedUrl, result.storeUrl) or the actual
URL property name used in the response) for Android and iOS force-update cases
so both needForceUpdate and the corresponding platform store URL are verified
together.
In
`@composeApp/src/iosMain/kotlin/com/chukchukhaksa/mobile/common/kmp/Sha256.ios.kt`:
- Around line 29-95: Current iOS file implements SHA-256 manually in the
function sha256; replace it with a call to the platform crypto API to match
Android's MessageDigest. Create/keep a common expect fun sha256(message:
ByteArray): ByteArray and implement the ios actual to call CommonCrypto's
CC_SHA256 (or CryptoKit/SHA256) via Kotlin/Native interop, returning a 32-byte
result; remove the manual block processing in sha256 (including the w/K usage)
and ensure the new implementation produces identical byte-ordering as the
current code (big-endian) and is covered by existing tests.
---
Outside diff comments:
In
`@composeApp/src/androidMain/kotlin/com/chukchukhaksa/mobile/common/kmp/KakaoSignInClient.android.kt`:
- Around line 20-35: Remove the SHA-256 hashing step and pass the original
plaintext nonce through to the Kakao SDK and result; specifically delete the
sha256Hex(nonce) usage (the val hashedNonce = sha256Hex(nonce) and any place
hashedNonce is supplied to the SDK) and ensure the code uses the original nonce
variable when initiating the sign-in flow and when creating KakaoSignInResult
(keep the callback and continuation logic intact, but stop calling sha256Hex and
stop supplying hashedNonce to the SDK so the ID Token’s nonce claim matches the
original plaintext nonce).
ℹ️ Review info
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (26)
composeApp/build.gradle.ktscomposeApp/src/androidMain/kotlin/com/chukchukhaksa/mobile/common/kmp/HttpClientEngine.android.ktcomposeApp/src/androidMain/kotlin/com/chukchukhaksa/mobile/common/kmp/KakaoSignInClient.android.ktcomposeApp/src/androidMain/kotlin/com/chukchukhaksa/mobile/common/kmp/Sha256.android.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/kmp/HttpClientEngine.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/common/kmp/Sha256.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/data/auth/datasource/RemoteAuthDataSource.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/data/auth/di/AuthRepositoryModule.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/data/auth/repository/AuthRepositoryImpl.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/di/DomainModules.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/di/InitKoin.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/domain/auth/model/SignInResult.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/domain/auth/repository/AuthRepository.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/domain/auth/usecase/KakaoLoginUseCase.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/auth/RemoteAuthDataSourceImpl.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/auth/model/SignInRequest.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/auth/model/SignInResponse.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/common/ApiException.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/common/ApiResponse.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/di/HttpClientModule.ktcomposeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/common/kmp/Sha256Test.ktcomposeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/domain/config/usecase/CheckNeedForceUpdateUseCaseTest.ktcomposeApp/src/iosMain/kotlin/com/chukchukhaksa/mobile/common/kmp/HttpClientEngine.ios.ktcomposeApp/src/iosMain/kotlin/com/chukchukhaksa/mobile/common/kmp/KakaoSignInClient.ios.ktcomposeApp/src/iosMain/kotlin/com/chukchukhaksa/mobile/common/kmp/Sha256.ios.ktgradle/libs.versions.toml
- HttpClient에 타임아웃 설정 추가 (요청 15초, 연결 10초, 소켓 15초) - 로깅 시 민감한 헤더(Authorization, Cookie) 마스킹 처리 - ResponseObserver에서 JSON 응답만 로깅하도록 제한 및 본문 미리보기 4096자 제한 - Sha256Test 메서드명을 실제 테스트 내용에 맞게 수정 - CheckNeedForceUpdateUseCaseTest에 storeUrl 검증 assertion 추가 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
composeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/domain/config/usecase/CheckNeedForceUpdateUseCaseTest.kt (1)
55-56:⚠️ Potential issue | 🟡 Minor비강제 업데이트 케이스 2곳에서
storeUrl단언이 빠져 있습니다.Line 55, Line 94는
needForceUpdate만 검증합니다. 이 상태면 비강제 업데이트인데 URL이 잘못 채워져도 테스트가 통과할 수 있습니다. 두 케이스 모두assertNull(result.storeUrl)를 추가해 주세요.수정 제안 diff
@@ fun shouldReturnFalseWhenCurrentVersionEqualsMinimumVersionForAndroid() = runTest { mockRepository.androidMinVersion = "1.5.0" @@ val result = useCase(Platform.Android, "1.5.0").getOrThrow() @@ assertFalse(result.needForceUpdate) + assertNull(result.storeUrl) } @@ fun shouldHandleVersionWithMissingParts() = runTest { mockRepository.androidMinVersion = "1.0" @@ val result = useCase(Platform.Android, "1.0.0").getOrThrow() @@ assertFalse(result.needForceUpdate) + assertNull(result.storeUrl) }Also applies to: 94-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/domain/config/usecase/CheckNeedForceUpdateUseCaseTest.kt` around lines 55 - 56, In CheckNeedForceUpdateUseCaseTest locate the two non-forced update assertions that currently only check assertFalse(result.needForceUpdate) (the occurrences around the two test cases where result is asserted non-forced) and add assertNull(result.storeUrl) immediately after each assertFalse to ensure storeUrl is not populated for non-force-update cases; update the test methods referencing the variable result in those cases so they assert both needForceUpdate is false and storeUrl is null.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/di/HttpClientModule.kt`:
- Around line 73-76: The log currently always prints "RESPONSE
BODY:\n${preview.prettyPrintJson()}" via Napier.d in HttpClientModule.kt; change
it so you compute the JSON payload string from preview.prettyPrintJson() and if
its length exceeds 4096 (or the chosen max) append or replace with a clear
truncated indicator (e.g., "...[TRUNCATED, length=NNNN]") and include the
original length in the message, then pass that final string to Napier.d so logs
show whether the response was cut off; update the Napier.d call site (the code
using preview.prettyPrintJson()) to use this truncated-aware string.
---
Duplicate comments:
In
`@composeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/domain/config/usecase/CheckNeedForceUpdateUseCaseTest.kt`:
- Around line 55-56: In CheckNeedForceUpdateUseCaseTest locate the two
non-forced update assertions that currently only check
assertFalse(result.needForceUpdate) (the occurrences around the two test cases
where result is asserted non-forced) and add assertNull(result.storeUrl)
immediately after each assertFalse to ensure storeUrl is not populated for
non-force-update cases; update the test methods referencing the variable result
in those cases so they assert both needForceUpdate is false and storeUrl is
null.
ℹ️ Review info
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/di/HttpClientModule.ktcomposeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/common/kmp/Sha256Test.ktcomposeApp/src/commonTest/kotlin/com/chukchukhaksa/mobile/domain/config/usecase/CheckNeedForceUpdateUseCaseTest.kt
Comment on lines
+73
to
+76
| Napier.d( | ||
| "RESPONSE BODY:\n${preview.prettyPrintJson()}", | ||
| tag = "HttpClient", | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
응답 본문 잘림 여부를 로그 메시지에 표시하면 좋겠습니다.
현재 로그 메시지가 "RESPONSE BODY"로 고정되어 있어 4096자 초과 시 잘렸는지 알기 어렵습니다. 디버깅 시 혼동을 줄이기 위해 잘림 여부를 표시하는 것을 고려해 주세요.
✨ 제안 수정안
val body = response.bodyAsText()
if (body.isNotBlank()) {
+ val isTruncated = body.length > 4096
val preview = body.take(4096)
+ val label = if (isTruncated) "RESPONSE BODY (truncated ${body.length} -> 4096)" else "RESPONSE BODY"
Napier.d(
- "RESPONSE BODY:\n${preview.prettyPrintJson()}",
+ "$label:\n${preview.prettyPrintJson()}",
tag = "HttpClient",
)
}📝 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
| Napier.d( | |
| "RESPONSE BODY:\n${preview.prettyPrintJson()}", | |
| tag = "HttpClient", | |
| ) | |
| if (isDebug) { | |
| install(ResponseObserver) { | |
| onResponse { response -> | |
| val contentType = response.headers[io.ktor.http.HttpHeaders.ContentType] | |
| if (contentType?.contains("application/json", ignoreCase = true) != true) return@onResponse | |
| val body = response.bodyAsText() | |
| if (body.isNotBlank()) { | |
| val isTruncated = body.length > 4096 | |
| val preview = body.take(4096) | |
| val label = if (isTruncated) "RESPONSE BODY (truncated ${body.length} -> 4096)" else "RESPONSE BODY" | |
| Napier.d( | |
| "$label:\n${preview.prettyPrintJson()}", | |
| tag = "HttpClient", | |
| ) | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/remote/di/HttpClientModule.kt`
around lines 73 - 76, The log currently always prints "RESPONSE
BODY:\n${preview.prettyPrintJson()}" via Napier.d in HttpClientModule.kt; change
it so you compute the JSON payload string from preview.prettyPrintJson() and if
its length exceeds 4096 (or the chosen max) append or replace with a clear
truncated indicator (e.g., "...[TRUNCATED, length=NNNN]") and include the
original length in the message, then pass that final string to Napier.d so logs
show whether the response was cut off; update the Napier.d call site (the code
using preview.prettyPrintJson()) to use this truncated-aware string.
Merged
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.
📌 PR 요약
🌱 작업한 내용
🌱 PR 포인트
📸 스크린샷
📮 관련 이슈
RCA 룰을 사용하여 코드 리뷰를 해주세요
R (Request Changes): 적극적으로 반영을 고려해주세요C (Comment): 웬만하면 반영해주세요A (Approve): 반영해도 좋고, 넘어가도 좋습니다. 사소한 의견입니다.Summary by CodeRabbit
릴리스 노트
새로운 기능
테스트