Decouple unlocking and syncing and add syncIfNecessary (#733)

This commit is contained in:
Brian Yencho
2024-06-20 17:08:07 +01:00
committed by Álison Fernandes
parent 91df6b5e25
commit e1a078e511
9 changed files with 77 additions and 70 deletions
@@ -227,7 +227,7 @@ class AuthRepositoryImpl(
settingsRepository.setDefaultsIfNecessary(
userId = userStateJson.activeUserId,
)
vaultRepository.sync()
vaultRepository.syncIfNecessary()
hasPendingAccountAddition = false
LoginResult.Success
}
@@ -95,10 +95,19 @@ interface VaultRepository : VaultLockManager {
fun deleteVaultData(userId: String)
/**
* Attempt to sync the vault data.
* Sync the vault data for the current user.
*
* Unlike [syncIfNecessary], this will always perform the requested sync and should only be
* utilized in cases where the user specifically requested the action.
*/
fun sync()
/**
* Checks if conditions have been met to perform a sync request and, if so, syncs the vault
* data for the current user.
*/
fun syncIfNecessary()
/**
* Flow that represents the data for a specific vault item as found by ID. This may emit `null`
* if the item cannot be found.
@@ -129,18 +138,17 @@ interface VaultRepository : VaultLockManager {
fun emitTotpCodeResult(totpCodeResult: TotpCodeResult)
/**
* Attempt to unlock the vault with the given [masterPassword] and syncs the vault data for the
* currently active user.
* Attempt to unlock the vault with the given [masterPassword] and for the currently active
* user.
*/
suspend fun unlockVaultWithMasterPasswordAndSync(
suspend fun unlockVaultWithMasterPassword(
masterPassword: String,
): VaultUnlockResult
/**
* Attempt to unlock the vault with the given [pin] and syncs the vault data for the currently
* active user.
* Attempt to unlock the vault with the given [pin] for the currently active user.
*/
suspend fun unlockVaultWithPinAndSync(
suspend fun unlockVaultWithPin(
pin: String,
): VaultUnlockResult
@@ -257,6 +257,11 @@ class VaultRepositoryImpl(
}
}
override fun syncIfNecessary() {
// TODO: Add conditions for when a sync may be performed and test accordingly (BIT-1454)
sync()
}
override fun getVaultItemStateFlow(itemId: String): StateFlow<DataState<CipherView?>> =
vaultDataStateFlow
.map { dataState ->
@@ -346,7 +351,7 @@ class VaultRepositoryImpl(
}
@Suppress("ReturnCount")
override suspend fun unlockVaultWithMasterPasswordAndSync(
override suspend fun unlockVaultWithMasterPassword(
masterPassword: String,
): VaultUnlockResult {
val userId = activeUserId ?: return VaultUnlockResult.InvalidStateError
@@ -361,14 +366,13 @@ class VaultRepositoryImpl(
)
.also {
if (it is VaultUnlockResult.Success) {
sync()
deriveTemporaryPinProtectedUserKeyIfNecessary(userId = userId)
}
}
}
@Suppress("ReturnCount")
override suspend fun unlockVaultWithPinAndSync(
override suspend fun unlockVaultWithPin(
pin: String,
): VaultUnlockResult {
val userId = activeUserId ?: return VaultUnlockResult.InvalidStateError
@@ -381,11 +385,6 @@ class VaultRepositoryImpl(
pinProtectedUserKey = pinProtectedUserKey,
),
)
.also {
if (it is VaultUnlockResult.Success) {
sync()
}
}
}
override suspend fun unlockVault(
@@ -133,13 +133,13 @@ class VaultUnlockViewModel @Inject constructor(
viewModelScope.launch {
val vaultUnlockResult = when (state.vaultUnlockType) {
VaultUnlockType.MASTER_PASSWORD -> {
vaultRepo.unlockVaultWithMasterPasswordAndSync(
vaultRepo.unlockVaultWithMasterPassword(
mutableStateFlow.value.input,
)
}
VaultUnlockType.PIN -> {
vaultRepo.unlockVaultWithPinAndSync(
vaultRepo.unlockVaultWithPin(
mutableStateFlow.value.input,
)
}
@@ -74,6 +74,9 @@ class VaultViewModel @Inject constructor(
get() = state.vaultFilterData?.selectedVaultFilterType ?: VaultFilterType.AllVaults
init {
// Attempt a sync each time we are on a fresh Vault Screen.
vaultRepository.syncIfNecessary()
// Reset the current vault filter type for the current user
vaultRepository.vaultFilterType = vaultFilterTypeOrDefault
settingsRepository
@@ -517,7 +517,7 @@ class AuthRepositoryTest {
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.Success
coEvery { vaultRepository.sync() } just runs
coEvery { vaultRepository.syncIfNecessary() } just runs
every {
GET_TOKEN_RESPONSE_SUCCESS.toUserState(
previousUserState = null,
@@ -552,7 +552,7 @@ class AuthRepositoryTest {
organizationKeys = null,
masterPassword = PASSWORD,
)
vaultRepository.sync()
vaultRepository.syncIfNecessary()
}
assertEquals(
SINGLE_USER_STATE_1,
@@ -595,7 +595,7 @@ class AuthRepositoryTest {
masterPassword = PASSWORD,
)
} returns VaultUnlockResult.Success
coEvery { vaultRepository.sync() } just runs
coEvery { vaultRepository.syncIfNecessary() } just runs
every {
GET_TOKEN_RESPONSE_SUCCESS.toUserState(
previousUserState = SINGLE_USER_STATE_2,
@@ -632,7 +632,7 @@ class AuthRepositoryTest {
organizationKeys = null,
masterPassword = PASSWORD,
)
vaultRepository.sync()
vaultRepository.syncIfNecessary()
}
assertEquals(
MULTI_USER_STATE,
@@ -587,7 +587,7 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithMasterPasswordAndSync with missing user state should return InvalidStateError `() =
fun `unlockVaultWithMasterPassword with missing user state should return InvalidStateError `() =
runTest {
fakeAuthDiskSource.userState = null
assertEquals(
@@ -598,7 +598,7 @@ class VaultRepositoryTest {
vaultRepository.vaultStateFlow.value,
)
val result = vaultRepository.unlockVaultWithMasterPasswordAndSync(masterPassword = "")
val result = vaultRepository.unlockVaultWithMasterPassword(masterPassword = "")
assertEquals(
VaultUnlockResult.InvalidStateError,
@@ -615,7 +615,7 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithMasterPasswordAndSync with missing user key should return InvalidStateError `() =
fun `unlockVaultWithMasterPassword with missing user key should return InvalidStateError `() =
runTest {
assertEquals(
VaultState(
@@ -625,7 +625,7 @@ class VaultRepositoryTest {
vaultRepository.vaultStateFlow.value,
)
val result = vaultRepository.unlockVaultWithMasterPasswordAndSync(masterPassword = "")
val result = vaultRepository.unlockVaultWithMasterPassword(masterPassword = "")
fakeAuthDiskSource.storeUserKey(
userId = "mockId-1",
userKey = null,
@@ -650,7 +650,7 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithMasterPasswordAndSync with missing private key should return InvalidStateError `() =
fun `unlockVaultWithMasterPassword with missing private key should return InvalidStateError `() =
runTest {
assertEquals(
VaultState(
@@ -659,7 +659,7 @@ class VaultRepositoryTest {
),
vaultRepository.vaultStateFlow.value,
)
val result = vaultRepository.unlockVaultWithMasterPasswordAndSync(masterPassword = "")
val result = vaultRepository.unlockVaultWithMasterPassword(masterPassword = "")
fakeAuthDiskSource.storeUserKey(
userId = "mockId-1",
userKey = "mockKey-1",
@@ -685,7 +685,7 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithMasterPasswordAndSync with VaultLockManager Success and no encrypted PIN should unlock for the current user, sync, and return Success`() =
fun `unlockVaultWithMasterPassword with VaultLockManager Success and no encrypted PIN should unlock for the current user and return Success`() =
runTest {
val userId = "mockId-1"
val mockVaultUnlockResult = VaultUnlockResult.Success
@@ -705,7 +705,7 @@ class VaultRepositoryTest {
)
}
val result = vaultRepository.unlockVaultWithMasterPasswordAndSync(
val result = vaultRepository.unlockVaultWithMasterPassword(
masterPassword = "mockPassword-1",
)
@@ -713,7 +713,6 @@ class VaultRepositoryTest {
mockVaultUnlockResult,
result,
)
coVerify { syncService.sync() }
coVerify {
vaultLockManager.unlockVault(
userId = userId,
@@ -733,7 +732,7 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithMasterPasswordAndSync with VaultLockManager Success and a stored encrypted pin should unlock for the current user, sync, derive a new pin-protected key, and return Success`() =
fun `unlockVaultWithMasterPassword with VaultLockManager Success and a stored encrypted pin should unlock for the current user, derive a new pin-protected key, and return Success`() =
runTest {
val userId = "mockId-1"
val encryptedPin = "encryptedPin"
@@ -758,7 +757,7 @@ class VaultRepositoryTest {
)
}
val result = vaultRepository.unlockVaultWithMasterPasswordAndSync(
val result = vaultRepository.unlockVaultWithMasterPassword(
masterPassword = "mockPassword-1",
)
@@ -771,7 +770,6 @@ class VaultRepositoryTest {
pinProtectedUserKey = pinProtectedUserKey,
inMemoryOnly = true,
)
coVerify { syncService.sync() }
coVerify {
vaultLockManager.unlockVault(
userId = userId,
@@ -795,13 +793,13 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithMasterPasswordAndSync with VaultLockManager non-Success should unlock for the current user and return the error`() =
fun `unlockVaultWithMasterPassword with VaultLockManager non-Success should unlock for the current user and return the error`() =
runTest {
val userId = "mockId-1"
val mockVaultUnlockResult = VaultUnlockResult.InvalidStateError
prepareStateForUnlocking(unlockResult = mockVaultUnlockResult)
val result = vaultRepository.unlockVaultWithMasterPasswordAndSync(
val result = vaultRepository.unlockVaultWithMasterPassword(
masterPassword = "mockPassword-1",
)
@@ -809,7 +807,6 @@ class VaultRepositoryTest {
mockVaultUnlockResult,
result,
)
coVerify(exactly = 0) { syncService.sync() }
coVerify {
vaultLockManager.unlockVault(
userId = userId,
@@ -828,7 +825,7 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithPinAndSync with missing user state should return InvalidStateError `() =
fun `unlockVaultWithPin with missing user state should return InvalidStateError `() =
runTest {
fakeAuthDiskSource.userState = null
assertEquals(
@@ -839,7 +836,7 @@ class VaultRepositoryTest {
vaultRepository.vaultStateFlow.value,
)
val result = vaultRepository.unlockVaultWithPinAndSync(pin = "1234")
val result = vaultRepository.unlockVaultWithPin(pin = "1234")
assertEquals(
VaultUnlockResult.InvalidStateError,
@@ -856,7 +853,7 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithPinAndSync with missing pin-protected user key should return InvalidStateError `() =
fun `unlockVaultWithPin with missing pin-protected user key should return InvalidStateError `() =
runTest {
assertEquals(
VaultState(
@@ -866,7 +863,7 @@ class VaultRepositoryTest {
vaultRepository.vaultStateFlow.value,
)
val result = vaultRepository.unlockVaultWithPinAndSync(pin = "1234")
val result = vaultRepository.unlockVaultWithPin(pin = "1234")
fakeAuthDiskSource.storePinProtectedUserKey(
userId = "mockId-1",
pinProtectedUserKey = null,
@@ -891,7 +888,7 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithPinAndSync with missing private key should return InvalidStateError `() =
fun `unlockVaultWithPin with missing private key should return InvalidStateError `() =
runTest {
assertEquals(
VaultState(
@@ -900,7 +897,7 @@ class VaultRepositoryTest {
),
vaultRepository.vaultStateFlow.value,
)
val result = vaultRepository.unlockVaultWithPinAndSync(pin = "1234")
val result = vaultRepository.unlockVaultWithPin(pin = "1234")
fakeAuthDiskSource.storePinProtectedUserKey(
userId = "mockId-1",
pinProtectedUserKey = "mockKey-1",
@@ -925,19 +922,18 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithPinAndSync with VaultLockManager Success should unlock for the current user, sync, and return Success`() =
fun `unlockVaultWithPin with VaultLockManager Success should unlock for the current user and return Success`() =
runTest {
val userId = "mockId-1"
val mockVaultUnlockResult = VaultUnlockResult.Success
prepareStateForUnlocking(unlockResult = mockVaultUnlockResult)
val result = vaultRepository.unlockVaultWithPinAndSync(pin = "1234")
val result = vaultRepository.unlockVaultWithPin(pin = "1234")
assertEquals(
mockVaultUnlockResult,
result,
)
coVerify { syncService.sync() }
coVerify {
vaultLockManager.unlockVault(
userId = userId,
@@ -955,19 +951,18 @@ class VaultRepositoryTest {
@Suppress("MaxLineLength")
@Test
fun `unlockVaultWithPinAndSync with VaultLockManager non-Success should unlock for the current user and return the error`() =
fun `unlockVaultWithPin with VaultLockManager non-Success should unlock for the current user and return the error`() =
runTest {
val userId = "mockId-1"
val mockVaultUnlockResult = VaultUnlockResult.InvalidStateError
prepareStateForUnlocking(unlockResult = mockVaultUnlockResult)
val result = vaultRepository.unlockVaultWithPinAndSync(pin = "1234")
val result = vaultRepository.unlockVaultWithPin(pin = "1234")
assertEquals(
mockVaultUnlockResult,
result,
)
coVerify(exactly = 0) { syncService.sync() }
coVerify {
vaultLockManager.unlockVault(
userId = userId,
@@ -259,7 +259,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
} returns VaultUnlockResult.AuthenticationError
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -272,7 +272,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
}
}
@@ -285,7 +285,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
} returns VaultUnlockResult.GenericError
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -298,7 +298,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
}
}
@@ -311,7 +311,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
} returns VaultUnlockResult.InvalidStateError
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -324,7 +324,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
}
}
@@ -337,7 +337,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
} returns VaultUnlockResult.Success
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -346,7 +346,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
}
}
@@ -360,7 +360,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
val resultFlow = bufferedMutableSharedFlow<VaultUnlockResult>()
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
} coAnswers { resultFlow.first() }
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -383,7 +383,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithMasterPasswordAndSync(password)
vaultRepository.unlockVaultWithMasterPassword(password)
}
}
@@ -396,7 +396,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
} returns VaultUnlockResult.AuthenticationError
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -409,7 +409,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
}
}
@@ -422,7 +422,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
} returns VaultUnlockResult.GenericError
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -435,7 +435,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
}
}
@@ -448,7 +448,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
} returns VaultUnlockResult.InvalidStateError
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -461,7 +461,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
}
}
@@ -474,7 +474,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
)
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
} returns VaultUnlockResult.Success
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -483,7 +483,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
}
}
@@ -497,7 +497,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
val resultFlow = bufferedMutableSharedFlow<VaultUnlockResult>()
val viewModel = createViewModel(state = initialState)
coEvery {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
} coAnswers { resultFlow.first() }
viewModel.trySendAction(VaultUnlockAction.UnlockClick)
@@ -520,7 +520,7 @@ class VaultUnlockViewModelTest : BaseViewModelTest() {
viewModel.stateFlow.value,
)
coVerify {
vaultRepository.unlockVaultWithPinAndSync(pin)
vaultRepository.unlockVaultWithPin(pin)
}
}
@@ -69,14 +69,16 @@ class VaultViewModelTest : BaseViewModelTest() {
every { vaultFilterType = any() } returns Unit
every { vaultDataStateFlow } returns mutableVaultDataStateFlow
every { sync() } just runs
every { syncIfNecessary() } just runs
every { lockVaultForCurrentUser() } just runs
every { lockVault(any()) } just runs
}
@Test
fun `initial state should be correct`() {
fun `initial state should be correct and should trigger a syncIfNecessary call`() {
val viewModel = createViewModel()
assertEquals(DEFAULT_STATE, viewModel.stateFlow.value)
verify { vaultRepository.syncIfNecessary() }
}
@Test