[PM-13074] Explicitly sync FIDO2 credentials (#4012)

app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/sdk/model/Fido2CredentialStoreImpl.kt

@@ -9,6 +9,7 @@ import com.x8bit.bitwarden.data.autofill.util.isActiveWithFido2Credentials
 import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage
 import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
 import com.x8bit.bitwarden.data.vault.repository.VaultRepository
+import com.x8bit.bitwarden.data.vault.repository.model.SyncVaultDataResult
 
 /**
  * Primary implementation of [Fido2CredentialStore].
@@ -24,7 +25,12 @@ class Fido2CredentialStoreImpl(
      * Return all active ciphers that contain FIDO 2 credentials.
      */
     override suspend fun allCredentials(): List<CipherView> {
-        vaultRepository.sync()
+        val syncResult = vaultRepository.syncFido2Credentials()
+        if (syncResult is SyncVaultDataResult.Error) {
+            syncResult.throwable
+                ?.let { throw it }
+                ?: throw IllegalStateException("Sync failed.")
+        }
         return vaultRepository.ciphersStateFlow.value.data
             ?.filter { it.isActiveWithFido2Credentials }
             ?: emptyList()
@@ -40,7 +46,12 @@ class Fido2CredentialStoreImpl(
     override suspend fun findCredentials(ids: List<ByteArray>?, ripId: String): List<CipherView> {
         val userId = getActiveUserIdOrThrow()
 
-        vaultRepository.sync()
+        val syncResult = vaultRepository.syncFido2Credentials()
+        if (syncResult is SyncVaultDataResult.Error) {
+            syncResult.throwable
+                ?.let { throw it }
+                ?: throw IllegalStateException("Sync failed.")
+        }
 
         val ciphersWithFido2Credentials = vaultRepository.ciphersStateFlow.value.data
             ?.filter { it.isActiveWithFido2Credentials }

app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepository.kt

@@ -24,6 +24,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.ExportVaultDataResult
 import com.x8bit.bitwarden.data.vault.repository.model.GenerateTotpResult
 import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult
 import com.x8bit.bitwarden.data.vault.repository.model.SendData
+import com.x8bit.bitwarden.data.vault.repository.model.SyncVaultDataResult
 import com.x8bit.bitwarden.data.vault.repository.model.TotpCodeResult
 import com.x8bit.bitwarden.data.vault.repository.model.UpdateFolderResult
 import com.x8bit.bitwarden.data.vault.repository.model.UpdateSendResult
@@ -116,6 +117,12 @@ interface VaultRepository : CipherManager, VaultLockManager {
      */
     fun syncIfNecessary()
 
+    /**
+     * Syncs the user's FIDO 2 credentials. This is an explicit request to sync and is not dependent
+     * on whether the last sync time was sufficiently in the past.
+     */
+    suspend fun syncFido2Credentials(): SyncVaultDataResult
+
     /**
      * Flow that represents the data for a specific vault item as found by ID. This may emit `null`
      * if the item cannot be found.

app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt

@@ -65,6 +65,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.ExportVaultDataResult
 import com.x8bit.bitwarden.data.vault.repository.model.GenerateTotpResult
 import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult
 import com.x8bit.bitwarden.data.vault.repository.model.SendData
+import com.x8bit.bitwarden.data.vault.repository.model.SyncVaultDataResult
 import com.x8bit.bitwarden.data.vault.repository.model.TotpCodeResult
 import com.x8bit.bitwarden.data.vault.repository.model.UpdateFolderResult
 import com.x8bit.bitwarden.data.vault.repository.model.UpdateSendResult
@@ -83,9 +84,11 @@ import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedSdkSend
 import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedSdkSendList
 import com.x8bit.bitwarden.ui.vault.feature.vault.model.VaultFilterType
 import com.x8bit.bitwarden.ui.vault.feature.vault.util.toFilteredList
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.Job
+import kotlinx.coroutines.async
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharingStarted
@@ -312,7 +315,6 @@ class VaultRepositoryImpl(
         }
     }
 
-    @Suppress("LongMethod")
     override fun sync() {
         val userId = activeUserId ?: return
         if (!syncJob.isCompleted) return
@@ -321,74 +323,7 @@ class VaultRepositoryImpl(
         mutableFoldersStateFlow.updateToPendingOrLoading()
         mutableCollectionsStateFlow.updateToPendingOrLoading()
         mutableSendDataStateFlow.updateToPendingOrLoading()
-        syncJob = ioScope.launch {
-            val lastSyncInstant = settingsDiskSource
-                .getLastSyncTime(userId = userId)
-                ?.toEpochMilli()
-                ?: 0
-
-            syncService
-                .getAccountRevisionDateMillis()
-                .fold(
-                    onSuccess = { serverRevisionDate ->
-                        if (serverRevisionDate < lastSyncInstant) {
-                            // We can skip the actual sync call if there is no new data
-                            vaultDiskSource.resyncVaultData(userId)
-                            settingsDiskSource.storeLastSyncTime(
-                                userId = userId,
-                                lastSyncTime = clock.instant(),
-                            )
-                            return@launch
-                        }
-                    },
-                    onFailure = {
-                        updateVaultStateFlowsToError(it)
-                        return@launch
-                    },
-                )
-
-            syncService
-                .sync()
-                .fold(
-                    onSuccess = { syncResponse ->
-                        val localSecurityStamp =
-                            authDiskSource.userState?.activeAccount?.profile?.stamp
-                        val serverSecurityStamp = syncResponse.profile.securityStamp
-
-                        // Log the user out if the stamps do not match
-                        localSecurityStamp?.let {
-                            if (serverSecurityStamp != localSecurityStamp) {
-                                userLogoutManager.softLogout(userId = userId, isExpired = true)
-                                return@launch
-                            }
-                        }
-
-                        // Update user information with additional information from sync response
-                        authDiskSource.userState = authDiskSource
-                            .userState
-                            ?.toUpdatedUserStateJson(
-                                syncResponse = syncResponse,
-                            )
-
-                        unlockVaultForOrganizationsIfNecessary(syncResponse = syncResponse)
-                        storeProfileData(syncResponse = syncResponse)
-                        // Treat absent network policies as known empty data to
-                        // distinguish between unknown null data.
-                        authDiskSource.storePolicies(
-                            userId = userId,
-                            policies = syncResponse.policies.orEmpty(),
-                        )
-                        settingsDiskSource.storeLastSyncTime(
-                            userId = userId,
-                            lastSyncTime = clock.instant(),
-                        )
-                        vaultDiskSource.replaceVaultData(userId = userId, vault = syncResponse)
-                    },
-                    onFailure = { throwable ->
-                        updateVaultStateFlowsToError(throwable)
-                    },
-                )
-        }
+        syncJob = ioScope.launch { syncInternal(userId) }
     }
 
     @Suppress("MagicNumber")
@@ -405,6 +340,20 @@ class VaultRepositoryImpl(
         }
     }
 
+    override suspend fun syncFido2Credentials(): SyncVaultDataResult {
+        val userId = activeUserId
+            ?: return SyncVaultDataResult.Error(throwable = null)
+        syncJob = ioScope
+            .async { syncInternal(userId) }
+            .also {
+                return try {
+                    it.await()
+                } catch (e: CancellationException) {
+                    SyncVaultDataResult.Error(throwable = e)
+                }
+            }
+    }
+
     override fun getVaultItemStateFlow(itemId: String): StateFlow<DataState<CipherView?>> =
         vaultDataStateFlow
             .map { dataState ->
@@ -1355,6 +1304,78 @@ class VaultRepositoryImpl(
             .onSuccess { vaultDiskSource.saveFolder(userId, it) }
     }
     //endregion Push Notification helpers
+
+    @Suppress("LongMethod")
+    private suspend fun syncInternal(userId: String): SyncVaultDataResult {
+        val lastSyncInstant = settingsDiskSource
+            .getLastSyncTime(userId = userId)
+            ?.toEpochMilli()
+            ?: 0
+
+        syncService
+            .getAccountRevisionDateMillis()
+            .fold(
+                onSuccess = { serverRevisionDate ->
+                    if (serverRevisionDate < lastSyncInstant) {
+                        // We can skip the actual sync call if there is no new data
+                        vaultDiskSource.resyncVaultData(userId)
+                        settingsDiskSource.storeLastSyncTime(
+                            userId = userId,
+                            lastSyncTime = clock.instant(),
+                        )
+                        return SyncVaultDataResult.Success
+                    }
+                },
+                onFailure = {
+                    updateVaultStateFlowsToError(it)
+                    return SyncVaultDataResult.Error(it)
+                },
+            )
+
+        syncService
+            .sync()
+            .fold(
+                onSuccess = { syncResponse ->
+                    val localSecurityStamp =
+                        authDiskSource.userState?.activeAccount?.profile?.stamp
+                    val serverSecurityStamp = syncResponse.profile.securityStamp
+
+                    // Log the user out if the stamps do not match
+                    localSecurityStamp?.let {
+                        if (serverSecurityStamp != localSecurityStamp) {
+                            userLogoutManager.softLogout(userId = userId, isExpired = true)
+                            return SyncVaultDataResult.Error(throwable = null)
+                        }
+                    }
+
+                    // Update user information with additional information from sync response
+                    authDiskSource.userState = authDiskSource
+                        .userState
+                        ?.toUpdatedUserStateJson(
+                            syncResponse = syncResponse,
+                        )
+
+                    unlockVaultForOrganizationsIfNecessary(syncResponse = syncResponse)
+                    storeProfileData(syncResponse = syncResponse)
+                    // Treat absent network policies as known empty data to
+                    // distinguish between unknown null data.
+                    authDiskSource.storePolicies(
+                        userId = userId,
+                        policies = syncResponse.policies.orEmpty(),
+                    )
+                    settingsDiskSource.storeLastSyncTime(
+                        userId = userId,
+                        lastSyncTime = clock.instant(),
+                    )
+                    vaultDiskSource.replaceVaultData(userId = userId, vault = syncResponse)
+                    return SyncVaultDataResult.Success
+                },
+                onFailure = { throwable ->
+                    updateVaultStateFlowsToError(throwable)
+                    return SyncVaultDataResult.Error(throwable)
+                },
+            )
+    }
 }
 
 private fun <T> Throwable.toNetworkOrErrorState(data: T?): DataState<T> =

app/src/main/java/com/x8bit/bitwarden/data/vault/repository/model/SyncVaultDataResult.kt

@@ -0,0 +1,18 @@
+package com.x8bit.bitwarden.data.vault.repository.model
+
+/**
+ * Represents the result of a sync operation.
+ */
+sealed class SyncVaultDataResult {
+    /**
+     * Indicates a successful sync operation.
+     */
+    data object Success : SyncVaultDataResult()
+
+    /**
+     * Indicates a failed sync operation.
+     *
+     * @property throwable The exception that caused the failure, if any.
+     */
+    data class Error(val throwable: Throwable?) : SyncVaultDataResult()
+}

app/src/test/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryTest.kt

@@ -86,6 +86,7 @@ import com.x8bit.bitwarden.data.vault.repository.model.ExportVaultDataResult
 import com.x8bit.bitwarden.data.vault.repository.model.GenerateTotpResult
 import com.x8bit.bitwarden.data.vault.repository.model.RemovePasswordSendResult
 import com.x8bit.bitwarden.data.vault.repository.model.SendData
+import com.x8bit.bitwarden.data.vault.repository.model.SyncVaultDataResult
 import com.x8bit.bitwarden.data.vault.repository.model.UpdateFolderResult
 import com.x8bit.bitwarden.data.vault.repository.model.UpdateSendResult
 import com.x8bit.bitwarden.data.vault.repository.model.VaultData
@@ -1072,20 +1073,24 @@ class VaultRepositoryTest {
     }
 
     @Test
-    fun `sync when the last sync time is more recent than the revision date should not sync `() {
-        val userId = "mockId-1"
-        fakeAuthDiskSource.userState = MOCK_USER_STATE
-        every {
-            settingsDiskSource.getLastSyncTime(userId = userId)
-        } returns clock.instant().plus(2, ChronoUnit.MINUTES)
+    fun `sync when the last sync time is more recent than the revision date should not sync `() =
+        runTest {
+            val userId = "mockId-1"
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            every {
+                settingsDiskSource.getLastSyncTime(userId = userId)
+            } returns clock.instant().plus(2, ChronoUnit.MINUTES)
 
-        vaultRepository.sync()
+            vaultRepository.sync()
 
-        verify(exactly = 1) {
-            settingsDiskSource.storeLastSyncTime(userId = userId, lastSyncTime = clock.instant())
+            verify(exactly = 1) {
+                settingsDiskSource.storeLastSyncTime(
+                    userId = userId,
+                    lastSyncTime = clock.instant(),
+                )
+            }
+            coVerify(exactly = 0) { syncService.sync() }
         }
-        coVerify(exactly = 0) { syncService.sync() }
-    }
 
     @Test
     fun `lockVaultForCurrentUser should delegate to the VaultLockManager`() {
@@ -4329,6 +4334,69 @@ class VaultRepositoryTest {
         }
     }
 
+    @Test
+    fun `syncFido2Credentials should return result`() = runTest {
+        fakeAuthDiskSource.userState = MOCK_USER_STATE
+        val userId = "mockId-1"
+        val mockSyncResponse = createMockSyncResponse(number = 1)
+        coEvery {
+            syncService.sync()
+        } returns mockSyncResponse.asSuccess()
+        coEvery {
+            vaultSdkSource.initializeOrganizationCrypto(
+                userId = userId,
+                request = InitOrgCryptoRequest(
+                    organizationKeys = createMockOrganizationKeys(1),
+                ),
+            )
+        } returns InitializeCryptoResult.Success.asSuccess()
+        coEvery {
+            vaultDiskSource.replaceVaultData(
+                userId = MOCK_USER_STATE.activeUserId,
+                vault = mockSyncResponse,
+            )
+        } just runs
+
+        every {
+            settingsDiskSource.storeLastSyncTime(
+                MOCK_USER_STATE.activeUserId,
+                clock.instant(),
+            )
+        } just runs
+
+        val syncResult = vaultRepository.syncFido2Credentials()
+        assertEquals(SyncVaultDataResult.Success, syncResult)
+    }
+
+    @Test
+    fun `syncFido2Credentials should return error when getAccountRevisionDateMillis fails`() =
+        runTest {
+            fakeAuthDiskSource.userState = MOCK_USER_STATE
+            val throwable = Throwable()
+            coEvery {
+                syncService.getAccountRevisionDateMillis()
+            } returns throwable.asFailure()
+            val syncResult = vaultRepository.syncFido2Credentials()
+            assertEquals(
+                SyncVaultDataResult.Error(throwable = throwable),
+                syncResult,
+            )
+        }
+
+    @Test
+    fun `syncFido2Credentials should return error when sync fails`() = runTest {
+        fakeAuthDiskSource.userState = MOCK_USER_STATE
+        val throwable = Throwable()
+        coEvery {
+            syncService.sync()
+        } returns throwable.asFailure()
+        val syncResult = vaultRepository.syncFido2Credentials()
+        assertEquals(
+            SyncVaultDataResult.Error(throwable = throwable),
+            syncResult,
+        )
+    }
+
     //region Helper functions
 
     /**