[PM-21385] Defer feature flag check for Bitwarden account sync (#5222)

This commit is contained in:
Patrick Honkonen 2025-05-20 14:09:15 -04:00 committed by GitHub
parent 34aed2ac65
commit 54efc74907
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 79 additions and 60 deletions

View File

@ -40,6 +40,7 @@ import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.firstOrNull
import kotlinx.coroutines.flow.flatMapConcat
import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.launchIn
@ -155,44 +156,22 @@ class AuthenticatorRepositoryImpl @Inject constructor(
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
override val sharedCodesStateFlow: StateFlow<SharedVerificationCodesState> by lazy { override val sharedCodesStateFlow: StateFlow<SharedVerificationCodesState> by lazy {
if (!featureFlagManager.getFeatureFlag(FlagKey.PasswordManagerSync)) { featureFlagManager
MutableStateFlow(SharedVerificationCodesState.FeatureNotEnabled) .getFeatureFlagFlow(FlagKey.PasswordManagerSync)
} else { .flatMapLatest { isFeatureEnabled ->
authenticatorBridgeManager if (isFeatureEnabled) {
.accountSyncStateFlow authenticatorBridgeManager
.flatMapLatest { accountSyncState -> .accountSyncStateFlow
when (accountSyncState) { .flatMapConcat { it.toSharedVerificationCodesStateFlow() }
AccountSyncState.AppNotInstalled -> } else {
MutableStateFlow(SharedVerificationCodesState.AppNotInstalled) flowOf(SharedVerificationCodesState.FeatureNotEnabled)
AccountSyncState.SyncNotEnabled ->
MutableStateFlow(SharedVerificationCodesState.SyncNotEnabled)
AccountSyncState.Error ->
MutableStateFlow(SharedVerificationCodesState.Error)
AccountSyncState.Loading ->
MutableStateFlow(SharedVerificationCodesState.Loading)
AccountSyncState.OsVersionNotSupported -> MutableStateFlow(
SharedVerificationCodesState.OsVersionNotSupported,
)
is AccountSyncState.Success -> {
val verificationCodesList =
accountSyncState.accounts.toAuthenticatorItems()
totpCodeManager
.getTotpCodesFlow(verificationCodesList)
.map { SharedVerificationCodesState.Success(it) }
}
}
} }
.stateIn( }
scope = unconfinedScope, .stateIn(
started = SharingStarted.WhileSubscribed(), scope = unconfinedScope,
initialValue = SharedVerificationCodesState.Loading, started = SharingStarted.WhileSubscribed(STOP_TIMEOUT_DELAY_MS),
) initialValue = SharedVerificationCodesState.Loading,
} )
} }
@OptIn(ExperimentalCoroutinesApi::class) @OptIn(ExperimentalCoroutinesApi::class)
@ -298,6 +277,33 @@ class AuthenticatorRepositoryImpl @Inject constructor(
override val firstTimeAccountSyncFlow: Flow<Unit> override val firstTimeAccountSyncFlow: Flow<Unit>
get() = firstTimeAccountSyncChannel.receiveAsFlow() get() = firstTimeAccountSyncChannel.receiveAsFlow()
@Suppress("MaxLineLength")
private fun AccountSyncState.toSharedVerificationCodesStateFlow(): Flow<SharedVerificationCodesState> =
when (this) {
AccountSyncState.AppNotInstalled ->
flowOf(SharedVerificationCodesState.AppNotInstalled)
AccountSyncState.SyncNotEnabled ->
flowOf(SharedVerificationCodesState.SyncNotEnabled)
AccountSyncState.Error ->
flowOf(SharedVerificationCodesState.Error)
AccountSyncState.Loading ->
flowOf(SharedVerificationCodesState.Loading)
AccountSyncState.OsVersionNotSupported -> flowOf(
SharedVerificationCodesState.OsVersionNotSupported,
)
is AccountSyncState.Success -> {
val verificationCodesList = accounts.toAuthenticatorItems()
totpCodeManager
.getTotpCodesFlow(verificationCodesList)
.map { SharedVerificationCodesState.Success(it) }
}
}
private suspend fun encodeVaultDataToCsv(fileUri: Uri): ExportDataResult { private suspend fun encodeVaultDataToCsv(fileUri: Uri): ExportDataResult {
val headerLine = val headerLine =
"folder,favorite,type,name,login_uri,login_totp" "folder,favorite,type,name,login_uri,login_totp"

View File

@ -45,8 +45,14 @@ class AuthenticatorRepositoryTest {
private val mockFileManager = mockk<FileManager>() private val mockFileManager = mockk<FileManager>()
private val mockImportManager = mockk<ImportManager>() private val mockImportManager = mockk<ImportManager>()
private val mockDispatcherManager = FakeDispatcherManager() private val mockDispatcherManager = FakeDispatcherManager()
private val mutablePasswordSyncFlagStateFlow = MutableStateFlow(true)
private val mockFeatureFlagManager = mockk<FeatureFlagManager> { private val mockFeatureFlagManager = mockk<FeatureFlagManager> {
every { getFeatureFlag(FlagKey.PasswordManagerSync) } returns true every {
getFeatureFlagFlow(FlagKey.PasswordManagerSync)
} returns mutablePasswordSyncFlagStateFlow
every {
getFeatureFlag(FlagKey.PasswordManagerSync)
} returns mutablePasswordSyncFlagStateFlow.value
} }
private val settingsRepository: SettingsRepository = mockk { private val settingsRepository: SettingsRepository = mockk {
every { previouslySyncedBitwardenAccountIds } returns emptySet() every { previouslySyncedBitwardenAccountIds } returns emptySet()
@ -84,25 +90,27 @@ class AuthenticatorRepositoryTest {
} }
@Test @Test
fun `sharedCodesStateFlow value should be FeatureNotEnabled when feature flag is off`() { fun `sharedCodesStateFlow value should be FeatureNotEnabled when feature flag is off`() =
every { runTest {
mockFeatureFlagManager.getFeatureFlag(FlagKey.PasswordManagerSync) val repository = AuthenticatorRepositoryImpl(
} returns false authenticatorDiskSource = fakeAuthenticatorDiskSource,
val repository = AuthenticatorRepositoryImpl( authenticatorBridgeManager = mockAuthenticatorBridgeManager,
authenticatorDiskSource = fakeAuthenticatorDiskSource, featureFlagManager = mockFeatureFlagManager,
authenticatorBridgeManager = mockAuthenticatorBridgeManager, totpCodeManager = mockTotpCodeManager,
featureFlagManager = mockFeatureFlagManager, fileManager = mockFileManager,
totpCodeManager = mockTotpCodeManager, importManager = mockImportManager,
fileManager = mockFileManager, dispatcherManager = mockDispatcherManager,
importManager = mockImportManager, settingRepository = settingsRepository,
dispatcherManager = mockDispatcherManager, )
settingRepository = settingsRepository, mutablePasswordSyncFlagStateFlow.value = false
) mutableAccountSyncStateFlow.value = AccountSyncState.Success(emptyList())
assertEquals( repository.sharedCodesStateFlow.test {
SharedVerificationCodesState.FeatureNotEnabled, assertEquals(
repository.sharedCodesStateFlow.value, SharedVerificationCodesState.FeatureNotEnabled,
) awaitItem(),
} )
}
}
@Test @Test
fun `ciphersStateFlow should emit sorted authenticator items when disk source changes`() = fun `ciphersStateFlow should emit sorted authenticator items when disk source changes`() =
@ -117,9 +125,6 @@ class AuthenticatorRepositoryTest {
@Test @Test
fun `sharedCodesStateFlow should emit FeatureNotEnabled when feature flag is off`() = runTest { fun `sharedCodesStateFlow should emit FeatureNotEnabled when feature flag is off`() = runTest {
every {
mockFeatureFlagManager.getFeatureFlag(FlagKey.PasswordManagerSync)
} returns false
val repository = AuthenticatorRepositoryImpl( val repository = AuthenticatorRepositoryImpl(
authenticatorDiskSource = fakeAuthenticatorDiskSource, authenticatorDiskSource = fakeAuthenticatorDiskSource,
authenticatorBridgeManager = mockAuthenticatorBridgeManager, authenticatorBridgeManager = mockAuthenticatorBridgeManager,
@ -130,6 +135,7 @@ class AuthenticatorRepositoryTest {
dispatcherManager = mockDispatcherManager, dispatcherManager = mockDispatcherManager,
settingRepository = settingsRepository, settingRepository = settingsRepository,
) )
mutablePasswordSyncFlagStateFlow.value = false
repository.sharedCodesStateFlow.test { repository.sharedCodesStateFlow.test {
assertEquals( assertEquals(
SharedVerificationCodesState.FeatureNotEnabled, SharedVerificationCodesState.FeatureNotEnabled,

View File

@ -156,6 +156,13 @@ internal class AuthenticatorBridgeManagerImpl(
if (!isBound) { if (!isBound) {
mutableSharedAccountsStateFlow.value = AccountSyncState.Error mutableSharedAccountsStateFlow.value = AccountSyncState.Error
} else if (mutableSharedAccountsStateFlow.value == AccountSyncState.AppNotInstalled) {
// This scenario occurs when the Authenticator is installed before Bitwarden, because
// `AppNotInstalled` is the initial state. Binding to the service simply means Bitwarden
// is installed, but does not indicate whether syncing is enabled. When/if syncing is
// toggled in Bitwarden, `onServiceConnected` will be invoked and the state
// will be updated.
mutableSharedAccountsStateFlow.value = AccountSyncState.SyncNotEnabled
} }
} }