feat: JWT 토큰 암호화 저장 및 검증 로그 추가 - #75
Merged
Merged
Conversation
KSafe 라이브러리를 활용한 암호화 토큰 저장소 구현 및 카카오 로그인 시 토큰 저장/조회 검증 로그 추가 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughJWT 토큰의 암호화된 로컬 저장을 위해 KSafe 라이브러리를 추가하고, LocalAuthDataSource 인터페이스와 구현체를 도입하여 토큰 관리 기능을 확장했습니다. Kakao 로그인 후 토큰을 자동으로 저장하는 흐름이 추가되었습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 2
🤖 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/data/auth/repository/AuthRepositoryImpl.kt`:
- Around line 17-20: The saveTokens function can leave the repository in an
inconsistent state if saving the refresh token throws after the access token was
already saved; update AuthRepositoryImpl.saveTokens to perform the two saves
atomically by wrapping localAuthDataSource.saveAccessToken and saveRefreshToken
in a try/catch and, on any exception, roll back to a consistent state (e.g.,
call localAuthDataSource.clearTokens or revert the access token) and rethrow or
propagate the error; alternatively, use a transaction/atomic API if
localAuthDataSource supports it—refer to saveTokens,
localAuthDataSource.saveAccessToken, localAuthDataSource.saveRefreshToken, and
localAuthDataSource.clearTokens when making the change.
In
`@composeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/local/datasource/auth/datasource/LocalAuthDataSourceImpl.kt`:
- Around line 10-20: saveAccessToken and saveRefreshToken currently allow blank
strings which can later be read as null by getAccessToken and obscure root
causes; add validation to reject blank tokens before persisting. In the
implementations of saveAccessToken(token: String) and saveRefreshToken(token:
String) perform a check like token.isNotBlank() and throw an
IllegalArgumentException (or return a failure) when blank, otherwise call
ksafe.put(KEY_ACCESS_TOKEN, token, encrypted = true) /
ksafe.put(KEY_REFRESH_TOKEN, token, encrypted = true); keep getAccessToken()
behavior unchanged but mention KEY_ACCESS_TOKEN and KEY_REFRESH_TOKEN to locate
the usages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 74d1ee23-e093-44d3-8338-bbd7ac39e19b
📒 Files selected for processing (13)
composeApp/build.gradle.ktscomposeApp/src/androidMain/kotlin/com/chukchukhaksa/mobile/di/AndroidModule.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/data/auth/datasource/LocalAuthDataSource.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/InitKoin.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/local/datasource/auth/datasource/LocalAuthDataSourceImpl.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/local/datasource/auth/di/LocalAuthDataSourceModule.ktcomposeApp/src/commonMain/kotlin/com/chukchukhaksa/mobile/presentation/timetable/timetable/component/WebViewGuideScreen.ktcomposeApp/src/iosMain/kotlin/com/chukchukhaksa/mobile/di/IosModule.ktgradle/libs.versions.toml
Comment on lines
+17
to
+20
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | ||
| localAuthDataSource.saveAccessToken(accessToken) | ||
| localAuthDataSource.saveRefreshToken(refreshToken) | ||
| } |
There was a problem hiding this comment.
토큰 저장이 원자적이지 않아 부분 저장 상태가 생길 수 있습니다.
Line 18 저장 후 Line 19에서 예외가 나면 access token만 갱신되고 refresh token은 이전 상태로 남을 수 있습니다. 저장 실패 시 롤백(예: clearTokens)을 보장해 일관성을 맞춰주세요.
수정 예시
override suspend fun saveTokens(accessToken: String, refreshToken: String) {
- localAuthDataSource.saveAccessToken(accessToken)
- localAuthDataSource.saveRefreshToken(refreshToken)
+ runCatching {
+ localAuthDataSource.saveAccessToken(accessToken)
+ localAuthDataSource.saveRefreshToken(refreshToken)
+ }.onFailure { error ->
+ localAuthDataSource.clearTokens()
+ throw error
+ }
}📝 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
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | |
| localAuthDataSource.saveAccessToken(accessToken) | |
| localAuthDataSource.saveRefreshToken(refreshToken) | |
| } | |
| override suspend fun saveTokens(accessToken: String, refreshToken: String) { | |
| runCatching { | |
| localAuthDataSource.saveAccessToken(accessToken) | |
| localAuthDataSource.saveRefreshToken(refreshToken) | |
| }.onFailure { error -> | |
| localAuthDataSource.clearTokens() | |
| throw error | |
| } | |
| } |
🤖 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/data/auth/repository/AuthRepositoryImpl.kt`
around lines 17 - 20, The saveTokens function can leave the repository in an
inconsistent state if saving the refresh token throws after the access token was
already saved; update AuthRepositoryImpl.saveTokens to perform the two saves
atomically by wrapping localAuthDataSource.saveAccessToken and saveRefreshToken
in a try/catch and, on any exception, roll back to a consistent state (e.g.,
call localAuthDataSource.clearTokens or revert the access token) and rethrow or
propagate the error; alternatively, use a transaction/atomic API if
localAuthDataSource supports it—refer to saveTokens,
localAuthDataSource.saveAccessToken, localAuthDataSource.saveRefreshToken, and
localAuthDataSource.clearTokens when making the change.
Comment on lines
+10
to
+20
| override suspend fun saveAccessToken(token: String) { | ||
| ksafe.put(KEY_ACCESS_TOKEN, token, encrypted = true) | ||
| } | ||
|
|
||
| override suspend fun getAccessToken(): String? { | ||
| return ksafe.get(KEY_ACCESS_TOKEN, "", encrypted = true).ifEmpty { null } | ||
| } | ||
|
|
||
| override suspend fun saveRefreshToken(token: String) { | ||
| ksafe.put(KEY_REFRESH_TOKEN, token, encrypted = true) | ||
| } |
There was a problem hiding this comment.
빈 토큰 저장을 허용하면 인증 상태가 즉시 깨질 수 있습니다.
saveAccessToken/saveRefreshToken에서 빈 문자열을 허용하면, 조회 시 null로 바뀌어 런타임에서 원인 추적이 어려워집니다. 저장 전에 isNotBlank() 검증으로 빠르게 실패시키는 편이 안전합니다.
수정 예시
override suspend fun saveAccessToken(token: String) {
+ require(token.isNotBlank()) { "access token must not be blank" }
ksafe.put(KEY_ACCESS_TOKEN, token, encrypted = true)
}
override suspend fun saveRefreshToken(token: String) {
+ require(token.isNotBlank()) { "refresh token must not be blank" }
ksafe.put(KEY_REFRESH_TOKEN, token, encrypted = true)
}📝 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
| override suspend fun saveAccessToken(token: String) { | |
| ksafe.put(KEY_ACCESS_TOKEN, token, encrypted = true) | |
| } | |
| override suspend fun getAccessToken(): String? { | |
| return ksafe.get(KEY_ACCESS_TOKEN, "", encrypted = true).ifEmpty { null } | |
| } | |
| override suspend fun saveRefreshToken(token: String) { | |
| ksafe.put(KEY_REFRESH_TOKEN, token, encrypted = true) | |
| } | |
| override suspend fun saveAccessToken(token: String) { | |
| require(token.isNotBlank()) { "access token must not be blank" } | |
| ksafe.put(KEY_ACCESS_TOKEN, token, encrypted = true) | |
| } | |
| override suspend fun getAccessToken(): String? { | |
| return ksafe.get(KEY_ACCESS_TOKEN, "", encrypted = true).ifEmpty { null } | |
| } | |
| override suspend fun saveRefreshToken(token: String) { | |
| require(token.isNotBlank()) { "refresh token must not be blank" } | |
| ksafe.put(KEY_REFRESH_TOKEN, token, encrypted = true) | |
| } |
🤖 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/local/datasource/auth/datasource/LocalAuthDataSourceImpl.kt`
around lines 10 - 20, saveAccessToken and saveRefreshToken currently allow blank
strings which can later be read as null by getAccessToken and obscure root
causes; add validation to reject blank tokens before persisting. In the
implementations of saveAccessToken(token: String) and saveRefreshToken(token:
String) perform a check like token.isNotBlank() and throw an
IllegalArgumentException (or return a failure) when blank, otherwise call
ksafe.put(KEY_ACCESS_TOKEN, token, encrypted = true) /
ksafe.put(KEY_REFRESH_TOKEN, token, encrypted = true); keep getAccessToken()
behavior unchanged but mention KEY_ACCESS_TOKEN and KEY_REFRESH_TOKEN to locate
the usages.
BEEEAM-J
approved these changes
Mar 4, 2026
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.
KSafe 라이브러리를 활용한 암호화 토큰 저장소 구현 및
카카오 로그인 시 토큰 저장/조회 검증 로그 추가
📌 PR 요약
🌱 작업한 내용
🌱 PR 포인트
📸 스크린샷
📮 관련 이슈
RCA 룰을 사용하여 코드 리뷰를 해주세요
R (Request Changes): 적극적으로 반영을 고려해주세요C (Comment): 웬만하면 반영해주세요A (Approve): 반영해도 좋고, 넘어가도 좋습니다. 사소한 의견입니다.Summary by CodeRabbit
변경사항
새로운 기능
개선사항