[Fix] 로그인 / 로그아웃 토큰 및 네비게이션 수정 #14#21
Conversation
- refreshToken이 비어 있을 때 require 예외 대신 null 반환처리 - refresh API 호출 실패 시 runCatching으로 예외를 흡수하고 refresh 실패로 정리 - 재발급 성공 후 DataStore 재조회 없이 응답 토큰으로 BearerTokens 구성 - 사용하지 않는 accessToken 변수 제거
- 로그아웃 후 초기화 화면 - 로그아웃 시 서버 응답과 무관하게 로컬 세션 정리되도록 수정
Walkthrough인증 관련 DI 한정자 추가, 토큰 리프레시 실패 처리 강화, 로그아웃 시 elderID 초기화 및 네비게이션 popUpTo 변경, 로그인 상태 확인 전에 토큰 존재를 검사하도록 UseCase에 DataStore 의존성 추가가 적용되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI
participant UseCase as CheckLoginStatusUseCase
participant DS as DataStoreRepository
participant Elders as EldersInfoRepository
participant EldId as ElderIdRepository
participant Nav as Navigator
UI->>UseCase: invoke()
UseCase->>DS: read accessToken, refreshToken
alt token missing
UseCase->>Nav: navigateToLoginAfterLogout()
else token present
UseCase->>Elders: getElders()
Elders-->>UseCase: elders list
UseCase->>EldId: read selected elder id
UseCase->>Nav: navigate based on elder/time/subscription
end
sequenceDiagram
participant Client as NetworkLayer
participant DS as DataStoreRepository
participant AuthAPI as AuthService
participant DS2 as DataStoreRepository
Client->>DS: read refreshToken
alt refreshToken blank
Client->>DS: clearTokens()
Client-->>Client: return null
else
Client->>AuthAPI: refreshToken()
AuthAPI-->>Client: success/failure
alt success with body
Client->>DS2: save accessToken, refreshToken (from response)
Client-->>Client: return BearerTokens(new)
else failure
Client->>DS2: clearTokens()
Client-->>Client: return null
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can disable poems in the walkthrough.Disable the |
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/konkuk/medicarecall/data/di/NetworkModule.kt`:
- Around line 83-92: The current withLock block clears only the refresh token
(dataStoreRepository.saveRefreshToken("")) on empty/exception/failure cases
which leaves the access token behind and causes inconsistent state; update the
logic in the withLock that calls refreshService.refreshToken(refreshToken) so
that: 1) if refreshToken.isNullOrBlank() then call clearTokens() to remove both
tokens and return null; 2) when runCatching { refreshService.refreshToken(...) }
fails or the refresh response indicates unrecoverable error, call clearTokens()
and return null; 3) for transient failures (e.g., network exception or non-fatal
error) avoid mutating stored tokens — do not call saveRefreshToken("") — and
simply return null so the existing tokens remain; apply the same adjustment to
the analogous block referenced at lines 104-107. Ensure you use
dataStoreRepository.clearTokens(), dataStoreRepository.saveRefreshToken(...),
refreshService.refreshToken(...), and the enclosing withLock to locate and
modify the code.
In
`@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/data/repositoryimpl/UserRepositoryImpl.kt`:
- Around line 33-44: logout() currently wraps only the network call in
runCatching so tokenStore.clearTokens() and elderIdRepository.clearElderIds()
can throw and escape the Result contract; wrap the local cleanup inside the same
exception-safe flow (or perform each cleanup in its own runCatching and
aggregate errors) so that failures from tokenStore.clearTokens() or
elderIdRepository.clearElderIds() are captured and logout() returns a
corresponding Result.failure, while still attempting both cleanups (i.e., call
authService.logout(...).handleNullableResponse() and then run
tokenStore.clearTokens() and elderIdRepository.clearElderIds() under runCatching
blocks and convert any thrown exceptions into a single Result).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94c71e21-2d1a-446a-b78e-b1ce88187418
📒 Files selected for processing (4)
composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/data/di/ApiModule.ktcomposeApp/src/commonMain/kotlin/com/konkuk/medicarecall/data/di/NetworkModule.ktcomposeApp/src/commonMain/kotlin/com/konkuk/medicarecall/data/repositoryimpl/UserRepositoryImpl.ktcomposeApp/src/commonMain/kotlin/com/konkuk/medicarecall/ui/navigation/MainNavigator.kt
| if (refreshToken.isNullOrBlank()) { | ||
| dataStoreRepository.saveRefreshToken("") | ||
| return@withLock null | ||
| } | ||
|
|
||
| require(!refreshToken.isNullOrEmpty()) | ||
| val refreshResponse = refreshService.refreshToken(refreshToken) | ||
| val refreshResponse = runCatching { | ||
| refreshService.refreshToken(refreshToken) | ||
| }.getOrElse { | ||
| dataStoreRepository.saveRefreshToken("") | ||
| return@withLock null |
There was a problem hiding this comment.
refresh 실패 시 토큰을 부분적으로만 지우면 상태가 꼬입니다.
여기서는 빈 refresh token, 예외, 실패 응답을 전부 saveRefreshToken("")로 처리하고 있어서 access token은 그대로 남습니다. 그러면 토큰 저장소가 반쯤만 비워진 상태가 되고, 일시적인 네트워크/서버 장애까지 세션 만료로 오인하게 돼요. 재발급이 확실히 불가능한 경우에만 clearTokens()로 둘을 함께 정리하고, 그 외 실패는 저장값을 유지한 채 null만 반환하는 쪽이 더 안전합니다.
Also applies to: 104-107
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/data/di/NetworkModule.kt`
around lines 83 - 92, The current withLock block clears only the refresh token
(dataStoreRepository.saveRefreshToken("")) on empty/exception/failure cases
which leaves the access token behind and causes inconsistent state; update the
logic in the withLock that calls refreshService.refreshToken(refreshToken) so
that: 1) if refreshToken.isNullOrBlank() then call clearTokens() to remove both
tokens and return null; 2) when runCatching { refreshService.refreshToken(...) }
fails or the refresh response indicates unrecoverable error, call clearTokens()
and return null; 3) for transient failures (e.g., network exception or non-fatal
error) avoid mutating stored tokens — do not call saveRefreshToken("") — and
simply return null so the existing tokens remain; apply the same adjustment to
the analogous block referenced at lines 104-107. Ensure you use
dataStoreRepository.clearTokens(), dataStoreRepository.saveRefreshToken(...),
refreshService.refreshToken(...), and the enclosing withLock to locate and
modify the code.
| override suspend fun logout(): Result<Unit> { | ||
| runCatching { | ||
| val refresh = tokenStore.getRefreshToken() | ||
| if (!refresh.isNullOrBlank()) { | ||
| authService.logout("Bearer $refresh").handleNullableResponse() | ||
| } | ||
| } | ||
|
|
||
| tokenStore.clearTokens() | ||
| elderIdRepository.clearElderIds() | ||
|
|
||
| return Result.success(Unit) |
There was a problem hiding this comment.
로컬 cleanup도 Result 계약 안으로 넣어 주세요.
지금은 서버 로그아웃만 runCatching 안에 있고 clearTokens() / clearElderIds()는 밖에 있어서, 저장소 I/O가 실패하면 logout()이 Result.failure가 아니라 예외를 그대로 던집니다. 게다가 첫 cleanup이 실패하면 다음 cleanup도 건너뛰게 돼요. 로컬 정리까지 개별적으로 시도한 뒤 최종 Result를 만드는 편이 더 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/data/repositoryimpl/UserRepositoryImpl.kt`
around lines 33 - 44, logout() currently wraps only the network call in
runCatching so tokenStore.clearTokens() and elderIdRepository.clearElderIds()
can throw and escape the Result contract; wrap the local cleanup inside the same
exception-safe flow (or perform each cleanup in its own runCatching and
aggregate errors) so that failures from tokenStore.clearTokens() or
elderIdRepository.clearElderIds() are captured and logout() returns a
corresponding Result.failure, while still attempting both cleanups (i.e., call
authService.logout(...).handleNullableResponse() and then run
tokenStore.clearTokens() and elderIdRepository.clearElderIds() under runCatching
blocks and convert any thrown exceptions into a single Result).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/domain/usecase/CheckLoginStatusUseCase.kt (1)
22-27: 토큰 선검사 자체는 좋고, 조회는 한 번으로 묶는 편이 더 안전해요.현재
DataStoreRepositoryImpl기준으로getAccessToken()/getRefreshToken()가 각각 별도data.first()를 타서, 로그아웃이나 토큰 갱신과 타이밍이 겹치면 서로 다른 시점의 값을 조합할 수 있습니다. 세션 유무 판정은getTokens()같은 단일 스냅샷 조회로 묶어두면 더 안정적입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/domain/usecase/CheckLoginStatusUseCase.kt` around lines 22 - 27, The current pre-check in CheckLoginStatusUseCase.kt calls dataStoreRepository.getAccessToken() and getRefreshToken() separately, which can observe inconsistent snapshots; change the use logic to fetch both tokens in a single atomic snapshot (e.g., add or use a dataStoreRepository.getTokens() or getSessionSnapshot() that returns both access and refresh together) and replace the two separate calls in the runCatching block with a single call that checks the returned pair/object for null/blank before returning NavigationDestination.GoToLogin; update the DataStoreRepositoryImpl to expose that single-snapshot accessor if it doesn't exist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/domain/usecase/CheckLoginStatusUseCase.kt`:
- Around line 22-27: The current pre-check in CheckLoginStatusUseCase.kt calls
dataStoreRepository.getAccessToken() and getRefreshToken() separately, which can
observe inconsistent snapshots; change the use logic to fetch both tokens in a
single atomic snapshot (e.g., add or use a dataStoreRepository.getTokens() or
getSessionSnapshot() that returns both access and refresh together) and replace
the two separate calls in the runCatching block with a single call that checks
the returned pair/object for null/blank before returning
NavigationDestination.GoToLogin; update the DataStoreRepositoryImpl to expose
that single-snapshot accessor if it doesn't exist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc03b02b-29e5-4a03-9dc3-a7dc8929e884
📒 Files selected for processing (1)
composeApp/src/commonMain/kotlin/com/konkuk/medicarecall/domain/usecase/CheckLoginStatusUseCase.kt
|
@alswlekk 리뷰부탁드립니다! |
🔗 관련 이슈
📙 작업 설명
로그인
로그인 전 요청에서 auth refresh 로직이 개입하며 illegal requirement 예외가 발생
로그아웃
로그아웃 이후에도 로컬 세션 상태가 남아 있어 시작하기 -> Home으로 바로 이동하는 문제
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
버그 수정
개선