-
Notifications
You must be signed in to change notification settings - Fork 4
[FIX, FEAT] http client에서의 토큰 새로고침 수정, 회원 탈퇴 로직 추가 #37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough토큰 관리에 HttpClient 주입·무효화 기능이 추가되었고, 계정 삭제 API가 DTO→DataSource→Repository→ViewModel→UI까지 연동되었습니다. 로그아웃 흐름이 조정되었으며, HomeScreen의 뷰모델 파라미터가 제거되고 내부에서 주입하도록 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant MP as MyPageScreen
participant VM as MyPageViewModel
participant REPO as AuthRepository
participant DS as RemoteAuthDataSource
participant API as Server
U->>MP: 탈퇴 확인 클릭
MP->>VM: deleteAccount()
VM->>REPO: deleteAccount()
REPO->>DS: deleteAccount()
DS->>API: POST users/delete/account
API-->>DS: DeleteAccountResponseDto / Error
DS-->>REPO: ApiResult
REPO-->>VM: ApiResult
alt Success
VM->>VM: 토큰 정리 및 만료 처리
VM-->>MP: 완료 상태 반영
else Error
VM-->>MP: 에러 메시지 갱신
end
sequenceDiagram
autonumber
participant TM as TokenManagerImpl
participant HCF as HttpClientFactory
participant HC as HttpClient
participant BA as BearerAuthProvider
HCF->>HC: HttpClient 생성
HCF->>TM: setHttpClient(HC)
note over TM,HC: HttpClient 참조 저장
TM->>TM: saveTokens(...) 또는 clearToken()
TM->>HC: invalidateAuthTokens()
HC->>BA: clear cached bearer tokens
BA-->>HC: 다음 요청 시 loadTokens 재호출
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this 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
🧹 Nitpick comments (4)
composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/TokenManagerImpl.kt (2)
26-36: 토큰 검증 로직에 대한 제안토큰 저장 후 즉시 검증하는 건 좋은데,
IllegalStateException을 던지는 게 좀 과할 수 있어요. DataStore는 일반적으로 신뢰할 수 있는 저장소이고, 저장 직후 읽기에서 불일치가 발생할 확률은 매우 낮습니다.만약 이게 실제로 발생했던 문제라면 괜찮지만, 그게 아니라면 로그만 남기고 넘어가는 게 나을 수도 있습니다. 예외를 던지면 정상적인 인증 플로우가 중단될 수 있거든요.
다음과 같이 경고 로그로 바꾸는 건 어떨까요?
- if (savedAccessToken != accessToken || savedRefreshToken != refreshToken) { - throw IllegalStateException("토큰 저장이 완료되지 않았습니다.") - } + if (savedAccessToken != accessToken || savedRefreshToken != refreshToken) { + println("경고: 토큰 저장 검증 실패. Expected: $accessToken, Got: $savedAccessToken") + }
7-49: HttpClient 통합이 잘 되었습니다!토큰 저장/삭제 후 캐시 무효화 로직이 잘 구현되어 있고, setHttpClient으로 의존성 주입도 깔끔하네요. PR 목표였던 "로그아웃 후 재로그인 시 loadToken 블록이 실행되지 않던 문제"가 이걸로 해결될 것 같습니다.
참고로, 현재
println을 사용하고 있는데, 나중에 시간 되면 proper logging framework (예: Kermit, Napier 등)로 교체하면 더 좋을 것 같아요. 하지만 지금은 이 정도로도 충분합니다.composeApp/src/commonMain/kotlin/org/whosin/client/presentation/mypage/MyPageViewModel.kt (2)
141-146: 성공 시 로딩 상태를 리셋하는 것을 고려해보세요.회원 탈퇴 성공 후 토큰이 클리어되고 화면 전환이 일어나겠지만, 혹시 모를 지연 상황을 대비해
isLoading = false를 추가하는 것이 좋을 것 같습니다.is ApiResult.Success -> { + _uiState.update{ it.copy(isLoading = false) } println("MyPageViewModel: 회원 탈퇴 성공") // 토큰 삭제 및 로그인 화면으로 이동 tokenManager.clearToken() TokenExpiredManager.setTokenExpired() }
132-133: 토큰 정리 로직의 중복을 제거하는 것을 고려해보세요.
logout()과deleteAccount()모두 동일한 토큰 정리 로직을 가지고 있습니다. 헬퍼 함수로 추출하면 유지보수가 더 쉬워질 것 같습니다.private fun clearAuthenticationState() { tokenManager.clearToken() TokenExpiredManager.setTokenExpired() }그리고 두 함수에서 다음과 같이 사용:
// logout() 함수에서 clearAuthenticationState() // deleteAccount() 함수에서 clearAuthenticationState()Also applies to: 144-145
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/TokenManager.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/TokenManagerImpl.kt(2 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/core/network/HttpClientFactory.kt(6 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/data/dto/response/DeleteAccountResponseDto.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/data/remote/RemoteAuthDataSource.kt(2 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/data/repository/AuthRepository.kt(2 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/presentation/home/HomeScreen.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/presentation/mypage/MyPageScreen.kt(1 hunks)composeApp/src/commonMain/kotlin/org/whosin/client/presentation/mypage/MyPageViewModel.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
composeApp/src/commonMain/kotlin/org/whosin/client/data/remote/RemoteAuthDataSource.kt
[warning] 221-221: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
⏰ 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). (2)
- GitHub Check: android-build
- GitHub Check: ios-build
🔇 Additional comments (10)
composeApp/src/commonMain/kotlin/org/whosin/client/data/remote/RemoteAuthDataSource.kt (1)
204-232: 구현이 깔끔하네요!기존 패턴과 일관성 있게 잘 작성하셨습니다. 에러 핸들링도
login(),logout()과 동일한 방식으로 처리되어 있어서 좋습니다.참고: Static analysis가 line 221에서 SwallowedException 경고를 내고 있는데, 이건 false positive입니다. Line 230에서
cause = t로 원본 예외를 제대로 전달하고 있습니다.composeApp/src/commonMain/kotlin/org/whosin/client/data/dto/response/DeleteAccountResponseDto.kt (1)
1-16: 잘 만들어진 DTO입니다!다른 Response DTO들과 일관된 구조로 깔끔하게 작성되었습니다. Serialization 설정도 적절하고, nullable
data필드도 합리적이네요.composeApp/src/commonMain/kotlin/org/whosin/client/presentation/home/HomeScreen.kt (1)
65-65: 좋은 리팩터링이에요!ViewModel을 파라미터로 받는 대신 내부에서 주입하도록 변경한 건 좋은 선택입니다. 호출하는 쪽의 코드가 간결해지고, Koin의 DI 패턴과도 잘 맞네요.
composeApp/src/commonMain/kotlin/org/whosin/client/data/repository/AuthRepository.kt (1)
45-46: 깔끔한 구현입니다!다른 Repository 메서드들과 동일한 패턴으로 잘 작성되었습니다. 단순 위임 구조가 명확하네요.
composeApp/src/commonMain/kotlin/org/whosin/client/presentation/mypage/MyPageScreen.kt (1)
224-224: TODO 구현 완료!회원 탈퇴 API 연동이 잘 되었네요. 확인 다이얼로그와 연결도 적절합니다.
composeApp/src/commonMain/kotlin/org/whosin/client/core/datastore/TokenManager.kt (1)
10-12: 토큰 리프레시 문제를 깔끔하게 해결했네요!HttpClient 참조를 주입할 수 있게 해서 토큰 변경 시 캐시를 무효화하는 건 좋은 접근입니다. PR 설명에 언급된 "로그아웃 후 재로그인 시 loadToken이 실행되지 않던 문제"를 해결하는 핵심 부분이네요.
composeApp/src/commonMain/kotlin/org/whosin/client/core/network/HttpClientFactory.kt (3)
56-65: loadTokens 개선이 좋습니다!빈 토큰을 null로 반환하도록 개선한 건 좋은 방어 로직이네요. 로깅도 추가되어서 디버깅이 쉬워질 것 같습니다.
164-177: 캐시 무효화 메커니즘이 잘 구현되었습니다!
invalidateAuthTokens()확장 함수가 깔끔하게 작성되었네요. BearerAuthProvider를 통해 캐시된 토큰을 클리어하는 방식이 적절하고, 에러 핸들링도 있어서 안전합니다.이 함수가 TokenManagerImpl에서 호출되어 토큰 변경 시 HttpClient가 새로 토큰을 로드하도록 만드는 핵심 메커니즘이네요. PR 목표였던 문제를 제대로 해결한 것 같습니다!
111-118: Refresh 토큰 요청에 AccessToken 헤더 포함 여부 확인
RFC 6749(및 일반 OAuth2 흐름)에서는 grant_type=refresh_token 요청에 access token을 Authorization 헤더에 포함하지 않습니다.
서버 API 사양상 이 헤더가 필수인지 재확인하고, 불필요하다면 해당 수동 헤더 추가 로직을 제거해주세요.composeApp/src/commonMain/kotlin/org/whosin/client/presentation/mypage/MyPageViewModel.kt (1)
122-128: try-catch가 ApiResult를 제대로 처리하지 못할 수 있습니다.
authRepository.logout가ApiResult를 반환한다면, try-catch로는 API 실패를 감지할 수 없습니다.ApiResult.Error는 예외가 아니라 정상적인 반환 값이기 때문입니다.다음과 같이 수정하는 것을 고려해보세요:
- if (!refreshToken.isNullOrEmpty()) { - try { - authRepository.logout(refreshToken) - println("MyPageViewModel: 로그아웃 API 호출 완료") - } catch (e: Exception) { - println("MyPageViewModel: 로그아웃 API 호출 실패 - ${e.message}") - } + if (!refreshToken.isNullOrEmpty()) { + when (val result = authRepository.logout(refreshToken)) { + is ApiResult.Success -> { + println("MyPageViewModel: 로그아웃 API 호출 완료") + } + is ApiResult.Error -> { + println("MyPageViewModel: 로그아웃 API 호출 실패 - ${result.message}") + } + } }
authRepository.logout의 반환 타입을 확인하기 위해 다음 스크립트를 실행해주세요:
🚀 이슈번호
✏️ 변경사항
📷 스크린샷
✍️ 사용법
🎸 기타
Summary by CodeRabbit