Skip to content

♻️ [KMP] Refactor and simplify refresh token login in AuthProviderImpl #47

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.

Sign up for GitHub

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

Merged
JuliaJakubcova merged 1 commit into from
Mar 5, 2026
Merged

♻️ [KMP] Refactor and simplify refresh token login in AuthProviderImpl #47

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 9 additions & 22 deletions shared/auth/src/commonMain/kotlin/kmp/shared/auth/data/provider/AuthProviderImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ import com.russhwolf.settings.Settings
import com.russhwolf.settings.set
import kmp.shared.auth.data.remote.TokenRefresher
import kmp.shared.base.data.provider.AuthProvider
import kotlinx.coroutines.Deferred
import kotlinx.coroutines.async
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.NonCancellable
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.withContext

internal class AuthProviderImpl(
private val settings: Settings,
Expand All @@ -20,29 +19,17 @@ internal class AuthProviderImpl(
set(value) = settings.set(TOKEN_KEY, value)

private val mutex = Mutex()
private var inFlight: Deferred<String?>? = null

override suspend fun refreshToken(): String? = coroutineScope {
mutex.withLock {
inFlight?.let { return@coroutineScope it.await() }
override suspend fun refreshToken(): String? {
val oldToken = token

val task = async {
val fresh = tokenRefresher.refresh()
token = fresh
fresh
}
inFlight = task
task
}.let { task ->
try {
task.await()
} finally {
mutex.withLock {
if (inFlight === task) {
inFlight = null
}
mutex.withLock {
if (oldToken == token) {
withContext(NonCancellable) {
token = tokenRefresher.refresh()
Comment on lines +27 to +29

@coderabbitai coderabbitai Bot Feb 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider the null refresh result scenario.

If tokenRefresher.refresh() returns null (e.g. refresh token expired), token is set to null. Every subsequent caller will snapshot oldToken = null, enter the lock, see null == null → true, and re-attempt the refresh — potentially hammering the auth server.

If this is a realistic failure mode, consider short-circuiting when the refresh yields null (e.g. clear credentials and force re-login rather than retrying indefinitely).

🤖 Prompt for AI Agents 
In
`@shared/auth/src/commonMain/kotlin/kmp/shared/auth/data/provider/AuthProviderImpl.kt`
around lines 27 - 29, The refresh result from tokenRefresher.refresh() may be
null, causing token to be set to null and every subsequent caller to repeatedly
attempt refresh; change the logic inside AuthProviderImpl where token is
assigned (the withContext(NonCancellable) block around tokenRefresher.refresh())
to handle a null return: if refresh() returns null, do not assign null to token
— instead clear stored credentials / set a "needsLogin" flag or throw a specific
AuthenticationRequiredException so callers will stop retrying, persist or
broadcast the logged-out state as appropriate, and ensure the lock-release path
communicates that no further automatic retries should occur until explicit
re-login.

}
}
return token
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down