diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt index 082fafdfcd..bea12001da 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepository.kt @@ -69,6 +69,11 @@ interface AuthRepository : AuthenticatorProvider { */ var hasPendingAccountAddition: Boolean + /** + * Clears the pending deletion state that occurs when the an account is successfully deleted. + */ + fun clearPendingAccountDeletion() + /** * Attempt to delete the current account and logout them out upon success. */ diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index d28c24a85b..58519570f0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -6,8 +6,8 @@ import com.bitwarden.crypto.Kdf import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson.CaptchaRequired -import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson.TwoFactorRequired import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson.Success +import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson.TwoFactorRequired import com.x8bit.bitwarden.data.auth.datasource.network.model.IdentityTokenAuthModel import com.x8bit.bitwarden.data.auth.datasource.network.model.PasswordHintResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.RefreshTokenResponseJson @@ -62,6 +62,7 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import javax.inject.Singleton @@ -86,7 +87,8 @@ class AuthRepositoryImpl( dispatcherManager: DispatcherManager, private val elapsedRealtimeMillisProvider: () -> Long = { SystemClock.elapsedRealtime() }, ) : AuthRepository { - private val mutableHasPendingAccountAdditionStateFlow = MutableStateFlow(false) + private val mutableHasPendingAccountAdditionStateFlow = MutableStateFlow(false) + private val mutableHasPendingAccountDeletionStateFlow = MutableStateFlow(false) /** * The auth information to make the identity token request will need to be @@ -128,11 +130,13 @@ class AuthRepositoryImpl( authDiskSource.userOrganizationsListFlow, vaultRepository.vaultStateFlow, mutableHasPendingAccountAdditionStateFlow, + mutableHasPendingAccountDeletionStateFlow, ) { userStateJson, userOrganizationsList, vaultState, hasPendingAccountAddition, + _, -> userStateJson ?.toUserState( @@ -142,6 +146,11 @@ class AuthRepositoryImpl( vaultUnlockTypeProvider = ::getVaultUnlockType, ) } + .filter { + // If there is a pending account deletion, continue showing + // the original UserState until it is confirmed. + !mutableHasPendingAccountDeletionStateFlow.value + } .stateIn( scope = collectionScope, started = SharingStarted.Eagerly, @@ -170,9 +179,14 @@ class AuthRepositoryImpl( override var hasPendingAccountAddition: Boolean by mutableHasPendingAccountAdditionStateFlow::value + override fun clearPendingAccountDeletion() { + mutableHasPendingAccountDeletionStateFlow.value = false + } + override suspend fun deleteAccount(password: String): DeleteAccountResult { val profile = authDiskSource.userState?.activeAccount?.profile ?: return DeleteAccountResult.Error + mutableHasPendingAccountDeletionStateFlow.value = true return authSdkSource .hashPassword( email = profile.email, @@ -182,6 +196,7 @@ class AuthRepositoryImpl( ) .flatMap { hashedPassword -> accountsService.deleteAccount(hashedPassword) } .onSuccess { logout() } + .onFailure { clearPendingAccountDeletion() } .fold( onFailure = { DeleteAccountResult.Error }, onSuccess = { DeleteAccountResult.Success }, diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreen.kt index 0445e6722f..5c802b9753 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreen.kt @@ -67,6 +67,16 @@ fun DeleteAccountScreen( } when (val dialog = state.dialog) { + DeleteAccountState.DeleteAccountDialog.DeleteSuccess -> BitwardenBasicDialog( + visibilityState = BasicDialogState.Shown( + title = null, + message = R.string.your_account_has_been_permanently_deleted.asText(), + ), + onDismissRequest = remember(viewModel) { + { viewModel.trySendAction(DeleteAccountAction.AccountDeletionConfirm) } + }, + ) + is DeleteAccountState.DeleteAccountDialog.Error -> BitwardenBasicDialog( visibilityState = BasicDialogState.Shown( title = R.string.an_error_has_occurred.asText(), diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountViewModel.kt index 4d41dff977..691a1483b0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountViewModel.kt @@ -43,6 +43,7 @@ class DeleteAccountViewModel @Inject constructor( DeleteAccountAction.CancelClick -> handleCancelClick() DeleteAccountAction.CloseClick -> handleCloseClick() is DeleteAccountAction.DeleteAccountClick -> handleDeleteAccountClick(action) + DeleteAccountAction.AccountDeletionConfirm -> handleAccountDeletionConfirm() DeleteAccountAction.DismissDialog -> handleDismissDialog() is DeleteAccountAction.Internal.DeleteAccountComplete -> { handleDeleteAccountComplete(action) @@ -68,6 +69,11 @@ class DeleteAccountViewModel @Inject constructor( } } + private fun handleAccountDeletionConfirm() { + authRepository.clearPendingAccountDeletion() + mutableStateFlow.update { it.copy(dialog = null) } + } + private fun handleDismissDialog() { mutableStateFlow.update { it.copy(dialog = null) } } @@ -77,8 +83,9 @@ class DeleteAccountViewModel @Inject constructor( ) { when (action.result) { DeleteAccountResult.Success -> { - mutableStateFlow.update { it.copy(dialog = null) } - // TODO: Display a dialog confirming account deletion (BIT-1184) + mutableStateFlow.update { + it.copy(dialog = DeleteAccountState.DeleteAccountDialog.DeleteSuccess) + } } DeleteAccountResult.Error -> { @@ -106,6 +113,12 @@ data class DeleteAccountState( * Displays a dialog. */ sealed class DeleteAccountDialog : Parcelable { + /** + * Dialog to confirm to the user that the account has been deleted. + */ + @Parcelize + data object DeleteSuccess : DeleteAccountDialog() + /** * Displays the error dialog when deleting an account fails. */ @@ -160,6 +173,11 @@ sealed class DeleteAccountAction { val masterPassword: String, ) : DeleteAccountAction() + /** + * The user has confirmed that their account has been deleted. + */ + data object AccountDeletionConfirm : DeleteAccountAction() + /** * The user has clicked to dismiss the dialog. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index f6b1ad64ea..d997f58eb4 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -314,6 +314,48 @@ class AuthRepositoryTest { assertNull(repository.rememberedEmailAddress) } + @Test + fun `clear Pending Account Deletion should unblock userState updates`() = runTest { + val masterPassword = "hello world" + val hashedMasterPassword = "dlrow olleh" + val originalUserState = SINGLE_USER_STATE_1.toUserState( + vaultState = VAULT_STATE, + userOrganizationsList = emptyList(), + hasPendingAccountAddition = false, + vaultUnlockTypeProvider = { VaultUnlockType.MASTER_PASSWORD }, + ) + val finalUserState = SINGLE_USER_STATE_2.toUserState( + vaultState = VAULT_STATE, + userOrganizationsList = emptyList(), + hasPendingAccountAddition = false, + vaultUnlockTypeProvider = { VaultUnlockType.MASTER_PASSWORD }, + ) + val kdf = SINGLE_USER_STATE_1.activeAccount.profile.toSdkParams() + coEvery { + authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION) + } returns hashedMasterPassword.asSuccess() + coEvery { + accountsService.deleteAccount(hashedMasterPassword) + } returns Unit.asSuccess() + fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 + + repository.userStateFlow.test { + assertEquals(originalUserState, awaitItem()) + + // Deleting the account sets the pending deletion flag + repository.deleteAccount(password = masterPassword) + + // Update the account. No changes are emitted because + // the pending deletion blocks the update. + fakeAuthDiskSource.userState = SINGLE_USER_STATE_2 + expectNoEvents() + + // Clearing the pending deletion allows the change to go through + repository.clearPendingAccountDeletion() + assertEquals(finalUserState, awaitItem()) + } + } + @Test fun `delete account fails if not logged in`() = runTest { val masterPassword = "hello world" diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreenTest.kt index 9be4976e8e..24410652b2 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountScreenTest.kt @@ -91,6 +91,40 @@ class DeleteAccountScreenTest : BaseComposeTest() { .assertExists() } + @Test + fun `delete success dialog presence should update with dialog state`() { + val message = "Your account has been permanently deleted" + composeTestRule + .onAllNodesWithText(message) + .filterToOne(hasAnyAncestor(isDialog())) + .assertDoesNotExist() + + mutableStateFlow.update { + it.copy(dialog = DeleteAccountState.DeleteAccountDialog.DeleteSuccess) + } + + composeTestRule + .onAllNodesWithText(message) + .filterToOne(hasAnyAncestor(isDialog())) + .assertExists() + } + + @Test + fun `delete success dialog dismiss should emit DeleteAccountAction`() { + mutableStateFlow.update { + it.copy(dialog = DeleteAccountState.DeleteAccountDialog.DeleteSuccess) + } + + composeTestRule + .onAllNodesWithText("Ok") + .filterToOne(hasAnyAncestor(isDialog())) + .performClick() + + verify { + viewModel.trySendAction(DeleteAccountAction.AccountDeletionConfirm) + } + } + @Test fun `delete account dialog should dismiss on cancel click`() { composeTestRule diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountViewModelTest.kt index 89d56b2ad4..6a4ad9a2e2 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccount/DeleteAccountViewModelTest.kt @@ -9,7 +9,11 @@ import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest import com.x8bit.bitwarden.ui.platform.base.util.asText import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.every +import io.mockk.just import io.mockk.mockk +import io.mockk.runs +import io.mockk.verify import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test @@ -52,21 +56,26 @@ class DeleteAccountViewModelTest : BaseViewModelTest() { } @Test - fun `on DeleteAccountClick should make the delete call`() = runTest { - val viewModel = createViewModel() - val masterPassword = "ckasb kcs ja" - coEvery { authRepo.deleteAccount(masterPassword) } returns DeleteAccountResult.Success + fun `on DeleteAccountClick should update dialog state when delete account succeeds`() = + runTest { + val viewModel = createViewModel() + val masterPassword = "ckasb kcs ja" + coEvery { authRepo.deleteAccount(masterPassword) } returns DeleteAccountResult.Success - viewModel.trySendAction(DeleteAccountAction.DeleteAccountClick(masterPassword)) + viewModel.trySendAction(DeleteAccountAction.DeleteAccountClick(masterPassword)) - assertEquals(DEFAULT_STATE, viewModel.stateFlow.value) - coVerify { - authRepo.deleteAccount(masterPassword) + assertEquals( + DEFAULT_STATE.copy(dialog = DeleteAccountState.DeleteAccountDialog.DeleteSuccess), + viewModel.stateFlow.value, + ) + + coVerify { + authRepo.deleteAccount(masterPassword) + } } - } @Test - fun `on DeleteAccountClick should update dialog state`() = runTest { + fun `on DeleteAccountClick should update dialog state when deleteAccount fails`() = runTest { val viewModel = createViewModel() val masterPassword = "ckasb kcs ja" coEvery { authRepo.deleteAccount(masterPassword) } returns DeleteAccountResult.Error @@ -87,6 +96,25 @@ class DeleteAccountViewModelTest : BaseViewModelTest() { } } + @Test + fun `AccountDeletionConfirm should clear dialog state and call clearPendingAccountDeletion`() = + runTest { + every { authRepo.clearPendingAccountDeletion() } just runs + val state = DEFAULT_STATE.copy( + dialog = DeleteAccountState.DeleteAccountDialog.DeleteSuccess, + ) + val viewModel = createViewModel(state = state) + + viewModel.trySendAction(DeleteAccountAction.AccountDeletionConfirm) + assertEquals( + DEFAULT_STATE.copy(dialog = null), + viewModel.stateFlow.value, + ) + verify { + authRepo.clearPendingAccountDeletion() + } + } + @Test fun `on DismissDialog should clear dialog state`() = runTest { val state = DEFAULT_STATE.copy(