From 06c49208d7b59e139ce1a031073cdd7daa3c8d9e Mon Sep 17 00:00:00 2001 From: Michal Tajchert Date: Sun, 7 Dec 2025 10:28:19 +0100 Subject: [PATCH] Fix OTP code freeze when returning to Authenticator app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor TotpCodeManager to use per-item StateFlow caching, matching the Password Manager's pattern. This prevents flow recreation on resubscribe and removes the 5-second stop timeout that caused stale state when returning from background. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../authenticator/manager/TotpCodeManager.kt | 7 +- .../manager/TotpCodeManagerImpl.kt | 153 ++++++++++++------ .../manager/di/AuthenticatorManagerModule.kt | 3 + .../repository/AuthenticatorRepositoryImpl.kt | 6 +- .../repository/model/AuthenticatorItem.kt | 22 +-- .../util/SharedAccountDataExtensions.kt | 1 + .../manager/util/TotpCodeManagerTest.kt | 54 ++++++- .../manager/util/VerificationCodeItemUtil.kt | 28 +++- .../repository/AuthenticatorRepositoryTest.kt | 3 +- .../itemlisting/ItemListingViewModelTest.kt | 4 +- .../VerificationCodeItemExtensionsTest.kt | 2 +- 11 files changed, 205 insertions(+), 78 deletions(-) diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/TotpCodeManager.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/TotpCodeManager.kt index 1d7dfa23891..d53c1cfae0f 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/TotpCodeManager.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/TotpCodeManager.kt @@ -3,7 +3,7 @@ package com.bitwarden.authenticator.data.authenticator.manager import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemAlgorithm import com.bitwarden.authenticator.data.authenticator.manager.model.VerificationCodeItem import com.bitwarden.authenticator.data.authenticator.repository.model.AuthenticatorItem -import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.StateFlow /** * Manages the flows for getting verification codes. @@ -11,11 +11,12 @@ import kotlinx.coroutines.flow.Flow interface TotpCodeManager { /** - * Flow for getting a DataState with multiple verification code items. + * StateFlow for getting multiple verification code items. Returns a StateFlow that emits + * updated verification codes every second. */ fun getTotpCodesFlow( itemList: List, - ): Flow> + ): StateFlow> @Suppress("UndocumentedPublicClass") companion object { diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/TotpCodeManagerImpl.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/TotpCodeManagerImpl.kt index f38bba3e0eb..3bf3e3b3272 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/TotpCodeManagerImpl.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/TotpCodeManagerImpl.kt @@ -3,84 +3,137 @@ package com.bitwarden.authenticator.data.authenticator.manager import com.bitwarden.authenticator.data.authenticator.datasource.sdk.AuthenticatorSdkSource import com.bitwarden.authenticator.data.authenticator.manager.model.VerificationCodeItem import com.bitwarden.authenticator.data.authenticator.repository.model.AuthenticatorItem +import com.bitwarden.core.data.manager.dispatcher.DispatcherManager +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.cancel import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.onCompletion +import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.isActive import java.time.Clock -import java.util.UUID import javax.inject.Inject private const val ONE_SECOND_MILLISECOND = 1000L /** * Primary implementation of [TotpCodeManager]. + * + * This implementation uses per-item [StateFlow] caching to prevent flow recreation on each + * subscribe, ensuring smooth UI updates when returning from background. The pattern mirrors + * the Password Manager's [TotpCodeManagerImpl]. */ class TotpCodeManagerImpl @Inject constructor( private val authenticatorSdkSource: AuthenticatorSdkSource, private val clock: Clock, + private val dispatcherManager: DispatcherManager, ) : TotpCodeManager { + private val unconfinedScope = CoroutineScope(dispatcherManager.unconfined) + + /** + * Cache of per-item StateFlows to prevent recreation on each subscribe. + * Key is the [AuthenticatorItem], value is the cached [StateFlow] for that item. + */ + private val mutableItemVerificationCodeStateFlowMap = + mutableMapOf>() + override fun getTotpCodesFlow( itemList: List, - ): Flow> { + ): StateFlow> { if (itemList.isEmpty()) { - return flowOf(emptyList()) + return MutableStateFlow(emptyList()) + } + + val stateFlows = itemList.map { getOrCreateItemStateFlow(it) } + + return combine(stateFlows) { results -> + results.filterNotNull() } - val flows = itemList.map { it.toFlowOfVerificationCodes() } - return combine(flows) { it.toList() } + .stateIn( + scope = unconfinedScope, + started = SharingStarted.WhileSubscribed(), + initialValue = emptyList(), + ) } - private fun AuthenticatorItem.toFlowOfVerificationCodes(): Flow { - val otpUri = this.otpUri - return flow { - var item: VerificationCodeItem? = null - while (currentCoroutineContext().isActive) { - val time = (clock.millis() / ONE_SECOND_MILLISECOND).toInt() - if (item == null || item.isExpired(clock)) { - // If the item is expired or we haven't generated our first item, - // generate a new code using the SDK: - item = authenticatorSdkSource - .generateTotp(otpUri, clock.instant()) - .getOrNull() - ?.let { response -> - VerificationCodeItem( - code = response.code, - periodSeconds = response.period.toInt(), - timeLeftSeconds = response.period.toInt() - - time % response.period.toInt(), - issueTime = clock.millis(), - id = when (source) { - is AuthenticatorItem.Source.Local -> source.cipherId - is AuthenticatorItem.Source.Shared -> UUID.randomUUID() - .toString() - }, - issuer = issuer, - label = label, - source = source, - ) - } - ?: run { - // We are assuming that our otp URIs can generate a valid code. - // If they can't, we'll just silently omit that code from the list. - currentCoroutineContext().cancel() - return@flow - } - } else { - // Item is not expired, just update time left: - item = item.copy( - timeLeftSeconds = item.periodSeconds - (time % item.periodSeconds), - ) + /** + * Gets an existing [StateFlow] for the given [item] or creates a new one if it doesn't exist. + * Each item gets its own [CoroutineScope] to manage its lifecycle independently. + */ + private fun getOrCreateItemStateFlow( + item: AuthenticatorItem, + ): StateFlow { + return mutableItemVerificationCodeStateFlowMap.getOrPut(item) { + // Define a per-item scope so that we can clear the Flow from the map when it is + // no longer needed. + val itemScope = CoroutineScope(dispatcherManager.unconfined) + + createVerificationCodeFlow(item) + .onCompletion { + mutableItemVerificationCodeStateFlowMap.remove(item) + itemScope.cancel() } - // Emit item - emit(item) - // Wait one second before heading to the top of the loop: - delay(ONE_SECOND_MILLISECOND) + .stateIn( + scope = itemScope, + started = SharingStarted.WhileSubscribed(), + initialValue = null, + ) + } + } + + /** + * Creates a flow that emits [VerificationCodeItem] updates every second for the given [item]. + */ + private fun createVerificationCodeFlow( + item: AuthenticatorItem, + ): Flow = flow { + var verificationCodeItem: VerificationCodeItem? = null + + while (currentCoroutineContext().isActive) { + val dateTime = clock.instant() + val time = dateTime.epochSecond.toInt() + + if (verificationCodeItem == null || verificationCodeItem.isExpired(clock)) { + // If the item is expired, or we haven't generated our first item, + // generate a new code using the SDK: + authenticatorSdkSource + .generateTotp(item.otpUri, dateTime) + .onSuccess { response -> + verificationCodeItem = VerificationCodeItem( + code = response.code, + periodSeconds = response.period.toInt(), + timeLeftSeconds = response.period.toInt() - + (time % response.period.toInt()), + issueTime = clock.millis(), + id = item.cipherId, + issuer = item.issuer, + label = item.label, + source = item.source, + ) + } + .onFailure { + // We are assuming that our otp URIs can generate a valid code. + // If they can't, we'll just silently omit that code from the list. + emit(null) + return@flow + } + } else { + // Item is not expired, just update time left: + verificationCodeItem = verificationCodeItem.copy( + timeLeftSeconds = verificationCodeItem.periodSeconds - + (time % verificationCodeItem.periodSeconds), + ) } + + emit(verificationCodeItem) + delay(ONE_SECOND_MILLISECOND) } } } diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/di/AuthenticatorManagerModule.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/di/AuthenticatorManagerModule.kt index 7a45a4d7f11..4e18c63e582 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/di/AuthenticatorManagerModule.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/manager/di/AuthenticatorManagerModule.kt @@ -3,6 +3,7 @@ package com.bitwarden.authenticator.data.authenticator.manager.di import com.bitwarden.authenticator.data.authenticator.datasource.sdk.AuthenticatorSdkSource import com.bitwarden.authenticator.data.authenticator.manager.TotpCodeManager import com.bitwarden.authenticator.data.authenticator.manager.TotpCodeManagerImpl +import com.bitwarden.core.data.manager.dispatcher.DispatcherManager import dagger.Module import dagger.Provides import dagger.hilt.InstallIn @@ -22,8 +23,10 @@ object AuthenticatorManagerModule { fun provideTotpCodeManager( authenticatorSdkSource: AuthenticatorSdkSource, clock: Clock, + dispatcherManager: DispatcherManager, ): TotpCodeManager = TotpCodeManagerImpl( authenticatorSdkSource = authenticatorSdkSource, clock = clock, + dispatcherManager = dispatcherManager, ) } diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryImpl.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryImpl.kt index d52c05aec18..c08dc252067 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryImpl.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryImpl.kt @@ -157,7 +157,7 @@ class AuthenticatorRepositoryImpl @Inject constructor( .flatMapLatest { it.toSharedVerificationCodesStateFlow() } .stateIn( scope = unconfinedScope, - started = SharingStarted.WhileSubscribed(STOP_TIMEOUT_DELAY_MS), + started = SharingStarted.WhileSubscribed(), initialValue = SharedVerificationCodesState.Loading, ) } @@ -171,8 +171,8 @@ class AuthenticatorRepositoryImpl @Inject constructor( authenticatorData.items .map { entity -> AuthenticatorItem( + cipherId = entity.id, source = AuthenticatorItem.Source.Local( - cipherId = entity.id, isFavorite = entity.favorite, ), otpUri = entity.toOtpAuthUriString(), @@ -197,7 +197,7 @@ class AuthenticatorRepositoryImpl @Inject constructor( } .stateIn( scope = unconfinedScope, - started = SharingStarted.WhileSubscribed(STOP_TIMEOUT_DELAY_MS), + started = SharingStarted.WhileSubscribed(), initialValue = DataState.Loading, ) } diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/model/AuthenticatorItem.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/model/AuthenticatorItem.kt index 3910ee996d0..892c4a17935 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/model/AuthenticatorItem.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/model/AuthenticatorItem.kt @@ -4,12 +4,14 @@ package com.bitwarden.authenticator.data.authenticator.repository.model * Represents all the information required to generate TOTP verification codes, including both * local codes and codes shared from the main Bitwarden app. * - * @param source Distinguishes between local and shared items. - * @param otpUri OTP URI. - * @param issuer The issuer of the codes. - * @param label The label of the item. + * @property cipherId The cipher ID. + * @property source Distinguishes between local and shared items. + * @property otpUri OTP URI. + * @property issuer The issuer of the codes. + * @property label The label of the item. */ data class AuthenticatorItem( + val cipherId: String, val source: Source, val otpUri: String, val issuer: String?, @@ -24,22 +26,20 @@ data class AuthenticatorItem( /** * The item is from the local Authenticator app database. * - * @param cipherId Local cipher ID. - * @param isFavorite Whether the user has marked the item as a favorite. + * @property isFavorite Whether the user has marked the item as a favorite. */ data class Local( - val cipherId: String, val isFavorite: Boolean, ) : Source() /** * The item is shared from the main Bitwarden app. * - * @param userId User ID from the main Bitwarden app. Used to group authenticator items + * @property userId User ID from the main Bitwarden app. Used to group authenticator items * by account. - * @param nameOfUser Username from the main Bitwarden app. - * @param email Email of the user. - * @param environmentLabel Label for the Bitwarden environment, like "bitwaren.com" + * @property nameOfUser Username from the main Bitwarden app. + * @property email Email of the user. + * @property environmentLabel Label for the Bitwarden environment, like "bitwaren.com" */ data class Shared( val userId: String, diff --git a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/util/SharedAccountDataExtensions.kt b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/util/SharedAccountDataExtensions.kt index 7420fe11fbf..fe3bfcd38ec 100644 --- a/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/util/SharedAccountDataExtensions.kt +++ b/authenticator/src/main/kotlin/com/bitwarden/authenticator/data/authenticator/repository/util/SharedAccountDataExtensions.kt @@ -29,6 +29,7 @@ fun List.toAuthenticatorItems(): List(), awaitItem()) - awaitComplete() + } + } + + @Test + fun `getTotpCodesFlow should emit data if a valid value is passed in`() = + runTest { + val totp = "otpUri" + val authenticatorItems = listOf( + createMockAuthenticatorItem(number = 1, otpUri = totp), + ) + val code = "123456" + val totpResponse = TotpResponse(code = code, period = 30u) + coEvery { + authenticatorSdkSource.generateTotp(totp = totp, time = clock.instant()) + } returns totpResponse.asSuccess() + + val expected = createMockVerificationCodeItem( + number = 1, + code = code, + issueTime = clock.instant().toEpochMilli(), + timeLeftSeconds = 30, + ) + + manager.getTotpCodesFlow(authenticatorItems).test { + assertEquals(listOf(expected), awaitItem()) + } + } + + @Test + fun `getTotpCodesFlow should emit empty list if unable to generate auth code`() = + runTest { + val totp = "otpUri" + val authenticatorItems = listOf( + createMockAuthenticatorItem(number = 1, otpUri = totp), + ) + coEvery { + authenticatorSdkSource.generateTotp(totp = totp, time = clock.instant()) + } returns Exception().asFailure() + + manager.getTotpCodesFlow(authenticatorItems).test { + assertEquals( + emptyList(), + awaitItem(), + ) } } } diff --git a/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/manager/util/VerificationCodeItemUtil.kt b/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/manager/util/VerificationCodeItemUtil.kt index df3a3ced912..e04b701d13f 100644 --- a/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/manager/util/VerificationCodeItemUtil.kt +++ b/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/manager/util/VerificationCodeItemUtil.kt @@ -3,6 +3,29 @@ package com.bitwarden.authenticator.data.authenticator.manager.util import com.bitwarden.authenticator.data.authenticator.manager.model.VerificationCodeItem import com.bitwarden.authenticator.data.authenticator.repository.model.AuthenticatorItem +/** + * Creates a mock [AuthenticatorItem] for testing purposes. + * + * @param number A number used to generate unique values for the mock item. + * @return A [AuthenticatorItem] with mock data based on the provided number. + */ +@Suppress("LongParameterList") +fun createMockAuthenticatorItem( + number: Int, + cipherId: String = "mockId-$number", + source: AuthenticatorItem.Source = createMockLocalAuthenticatorItemSource(), + otpUri: String = "otpUri-$number", + label: String? = "mockLabel-$number", + issuer: String? = "mockIssuer-$number", +): AuthenticatorItem = + AuthenticatorItem( + cipherId = cipherId, + source = source, + otpUri = otpUri, + issuer = issuer, + label = label, + ) + /** * Creates a mock [VerificationCodeItem] for testing purposes. * @@ -19,7 +42,7 @@ fun createMockVerificationCodeItem( issueTime: Long = 0, label: String = "mockLabel-$number", issuer: String = "mockIssuer-$number", - source: AuthenticatorItem.Source = createMockLocalAuthenticatorItemSource(number = number), + source: AuthenticatorItem.Source = createMockLocalAuthenticatorItemSource(), ): VerificationCodeItem = VerificationCodeItem( code = code, @@ -36,12 +59,9 @@ fun createMockVerificationCodeItem( * Creates a mock [AuthenticatorItem.Source.Local] for testing purposes. */ fun createMockLocalAuthenticatorItemSource( - number: Int, - cipherId: String = "mockId-$number", isFavorite: Boolean = false, ): AuthenticatorItem.Source.Local = AuthenticatorItem.Source.Local( - cipherId = cipherId, isFavorite = isFavorite, ) diff --git a/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryTest.kt b/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryTest.kt index c9ff924d331..8c77cc119c1 100644 --- a/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryTest.kt +++ b/authenticator/src/test/kotlin/com/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryTest.kt @@ -38,7 +38,6 @@ import io.mockk.unmockkConstructor import io.mockk.unmockkStatic import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals @@ -163,7 +162,7 @@ class AuthenticatorRepositoryTest { every { sharedAccounts.toAuthenticatorItems() } returns authenticatorItems every { mockTotpCodeManager.getTotpCodesFlow(authenticatorItems) - } returns flowOf(verificationCodes) + } returns MutableStateFlow(verificationCodes) authenticatorRepository.sharedCodesStateFlow.test { assertEquals(SharedVerificationCodesState.Loading, awaitItem()) mutableAccountSyncStateFlow.value = AccountSyncState.Success(sharedAccounts) diff --git a/authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt b/authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt index 9e9c9e3c97b..42442ec17ef 100644 --- a/authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt +++ b/authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/itemlisting/ItemListingViewModelTest.kt @@ -604,7 +604,7 @@ private val LOCAL_VERIFICATION_ITEMS = listOf( id = "1", issuer = "issuer", label = "accountName", - source = AuthenticatorItem.Source.Local("1", isFavorite = false), + source = AuthenticatorItem.Source.Local(isFavorite = false), ), VerificationCodeItem( code = "123456", @@ -614,7 +614,7 @@ private val LOCAL_VERIFICATION_ITEMS = listOf( id = "1", issuer = "issuer", label = "accountName", - source = AuthenticatorItem.Source.Local("1", isFavorite = true), + source = AuthenticatorItem.Source.Local(isFavorite = true), ), ) diff --git a/authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/util/VerificationCodeItemExtensionsTest.kt b/authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/util/VerificationCodeItemExtensionsTest.kt index afd7bb48d3c..cfdf37f0dde 100644 --- a/authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/util/VerificationCodeItemExtensionsTest.kt +++ b/authenticator/src/test/kotlin/com/bitwarden/authenticator/ui/authenticator/feature/util/VerificationCodeItemExtensionsTest.kt @@ -16,7 +16,7 @@ class VerificationCodeItemExtensionsTest { val alertThresholdSeconds = 7 val favoriteItem = createMockVerificationCodeItem( number = 1, - source = createMockLocalAuthenticatorItemSource(number = 1, isFavorite = true), + source = createMockLocalAuthenticatorItemSource(isFavorite = true), ) val nonFavoriteItem = createMockVerificationCodeItem(number = 2)