diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/DeleteAccountResponseJson.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/DeleteAccountResponseJson.kt new file mode 100644 index 0000000000..577f38883a --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/model/DeleteAccountResponseJson.kt @@ -0,0 +1,28 @@ +package com.x8bit.bitwarden.data.auth.datasource.network.model + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** + * Models response bodies from the delete account request. + */ +sealed class DeleteAccountResponseJson { + + /** + * Models a successful deletion response. + */ + data object Success : DeleteAccountResponseJson() + + /** + * Models the json body of a deletion error. + * + * @param validationErrors a map where each value is a list of error messages for each key. + * The values in the array should be used for display to the user, since the keys tend to come + * back as nonsense. (eg: empty string key) + */ + @Serializable + data class Invalid( + @SerialName("validationErrors") + val validationErrors: Map>?, + ) : DeleteAccountResponseJson() +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/AccountsService.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/AccountsService.kt index c489fec79a..7ac269a3d6 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/AccountsService.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/AccountsService.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.data.auth.datasource.network.service +import com.x8bit.bitwarden.data.auth.datasource.network.model.DeleteAccountResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.PasswordHintResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.ResendEmailRequestJson import com.x8bit.bitwarden.data.auth.datasource.network.model.ResetPasswordRequestJson @@ -18,7 +19,10 @@ interface AccountsService { /** * Make delete account request. */ - suspend fun deleteAccount(masterPasswordHash: String?, oneTimePassword: String?): Result + suspend fun deleteAccount( + masterPasswordHash: String?, + oneTimePassword: String?, + ): Result /** * Request a one-time passcode that is sent to the user's email. diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/AccountsServiceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/AccountsServiceImpl.kt index 22580720b7..8686fe877c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/AccountsServiceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/datasource/network/service/AccountsServiceImpl.kt @@ -4,6 +4,7 @@ import com.x8bit.bitwarden.data.auth.datasource.network.api.AccountsApi import com.x8bit.bitwarden.data.auth.datasource.network.api.AuthenticatedAccountsApi import com.x8bit.bitwarden.data.auth.datasource.network.model.CreateAccountKeysRequest import com.x8bit.bitwarden.data.auth.datasource.network.model.DeleteAccountRequestJson +import com.x8bit.bitwarden.data.auth.datasource.network.model.DeleteAccountResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.PasswordHintRequestJson import com.x8bit.bitwarden.data.auth.datasource.network.model.PasswordHintResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.ResendEmailRequestJson @@ -34,13 +35,26 @@ class AccountsServiceImpl( override suspend fun deleteAccount( masterPasswordHash: String?, oneTimePassword: String?, - ): Result = - authenticatedAccountsApi.deleteAccount( - DeleteAccountRequestJson( - masterPasswordHash = masterPasswordHash, - oneTimePassword = oneTimePassword, - ), - ) + ): Result = + authenticatedAccountsApi + .deleteAccount( + DeleteAccountRequestJson( + masterPasswordHash = masterPasswordHash, + oneTimePassword = oneTimePassword, + ), + ) + .map { + DeleteAccountResponseJson.Success + } + .recoverCatching { throwable -> + throwable + .toBitwardenError() + .parseErrorBodyOrNull( + code = 400, + json = json, + ) + ?: throw throwable + } override suspend fun requestOneTimePasscode(): Result = authenticatedAccountsApi.requestOtp() 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 98eca01555..7153fbf062 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 @@ -9,6 +9,7 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.AuthDiskSource import com.x8bit.bitwarden.data.auth.datasource.disk.model.AccountTokensJson import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson +import com.x8bit.bitwarden.data.auth.datasource.network.model.DeleteAccountResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.DeviceDataModel import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.IdentityTokenAuthModel @@ -383,7 +384,7 @@ class AuthRepositoryImpl( masterPassword: String, ): DeleteAccountResult { val profile = authDiskSource.userState?.activeAccount?.profile - ?: return DeleteAccountResult.Error + ?: return DeleteAccountResult.Error(message = null) mutableHasPendingAccountDeletionStateFlow.value = true return authSdkSource .hashPassword( @@ -398,12 +399,7 @@ class AuthRepositoryImpl( oneTimePassword = null, ) } - .onSuccess { logout() } - .onFailure { clearPendingAccountDeletion() } - .fold( - onFailure = { DeleteAccountResult.Error }, - onSuccess = { DeleteAccountResult.Success }, - ) + .finalizeDeleteAccount() } override suspend fun deleteAccountWithOneTimePassword( @@ -415,14 +411,35 @@ class AuthRepositoryImpl( masterPasswordHash = null, oneTimePassword = oneTimePassword, ) - .onSuccess { logout() } - .onFailure { clearPendingAccountDeletion() } - .fold( - onFailure = { DeleteAccountResult.Error }, - onSuccess = { DeleteAccountResult.Success }, - ) + .finalizeDeleteAccount() } + private fun Result.finalizeDeleteAccount(): DeleteAccountResult = + fold( + onFailure = { + clearPendingAccountDeletion() + DeleteAccountResult.Error(message = null) + }, + onSuccess = { response -> + when (response) { + is DeleteAccountResponseJson.Invalid -> { + clearPendingAccountDeletion() + DeleteAccountResult.Error( + message = response.validationErrors + ?.values + ?.firstOrNull() + ?.firstOrNull(), + ) + } + + DeleteAccountResponseJson.Success -> { + logout() + DeleteAccountResult.Success + } + } + }, + ) + @Suppress("ReturnCount") override suspend fun createNewSsoUser(): NewSsoUserResult { val account = authDiskSource.userState?.activeAccount ?: return NewSsoUserResult.Failure diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/DeleteAccountResult.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/DeleteAccountResult.kt index d885fc825f..f4b79119be 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/DeleteAccountResult.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/model/DeleteAccountResult.kt @@ -12,5 +12,5 @@ sealed class DeleteAccountResult { /** * There was an error deleting the account. */ - data object Error : DeleteAccountResult() + data class Error(val message: String?) : DeleteAccountResult() } 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 7060000223..7cb9f34e3b 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 @@ -95,18 +95,19 @@ class DeleteAccountViewModel @Inject constructor( private fun handleDeleteAccountComplete( action: DeleteAccountAction.Internal.DeleteAccountComplete, ) { - when (action.result) { + when (val result = action.result) { DeleteAccountResult.Success -> { mutableStateFlow.update { it.copy(dialog = DeleteAccountState.DeleteAccountDialog.DeleteSuccess) } } - DeleteAccountResult.Error -> { + is DeleteAccountResult.Error -> { mutableStateFlow.update { it.copy( dialog = DeleteAccountState.DeleteAccountDialog.Error( - message = R.string.generic_error_message.asText(), + message = result.message?.asText() + ?: R.string.generic_error_message.asText(), ), ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationViewModel.kt index 2b665ab06d..04ea4e6676 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationViewModel.kt @@ -151,10 +151,11 @@ class DeleteAccountConfirmationViewModel @Inject constructor( ) { mutableStateFlow.update { currentState -> currentState.copy( - dialog = when (action.deleteAccountResult) { - DeleteAccountResult.Error -> { + dialog = when (val result = action.deleteAccountResult) { + is DeleteAccountResult.Error -> { DeleteAccountConfirmationState.DeleteAccountConfirmationDialog.Error( - message = R.string.generic_error_message.asText(), + message = result.message?.asText() + ?: R.string.generic_error_message.asText(), ) } 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 1ebe033e4e..4b15c7a4ea 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 @@ -18,6 +18,7 @@ import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetRea import com.x8bit.bitwarden.data.auth.datasource.disk.model.PendingAuthRequestJson import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson import com.x8bit.bitwarden.data.auth.datasource.disk.util.FakeAuthDiskSource +import com.x8bit.bitwarden.data.auth.datasource.network.model.DeleteAccountResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson import com.x8bit.bitwarden.data.auth.datasource.network.model.IdentityTokenAuthModel import com.x8bit.bitwarden.data.auth.datasource.network.model.KdfTypeJson @@ -601,7 +602,7 @@ class AuthRepositoryTest { masterPasswordHash = hashedMasterPassword, oneTimePassword = null, ) - } returns Unit.asSuccess() + } returns DeleteAccountResponseJson.Success.asSuccess() fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 repository.userStateFlow.test { @@ -625,7 +626,7 @@ class AuthRepositoryTest { fun `delete account fails if not logged in`() = runTest { val masterPassword = "hello world" val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword) - assertEquals(DeleteAccountResult.Error, result) + assertEquals(DeleteAccountResult.Error(message = null), result) } @Test @@ -639,7 +640,7 @@ class AuthRepositoryTest { val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword) - assertEquals(DeleteAccountResult.Error, result) + assertEquals(DeleteAccountResult.Error(message = null), result) coVerify { authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION) } @@ -663,7 +664,7 @@ class AuthRepositoryTest { val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword) - assertEquals(DeleteAccountResult.Error, result) + assertEquals(DeleteAccountResult.Error(message = null), result) coVerify { authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION) accountsService.deleteAccount( @@ -687,7 +688,7 @@ class AuthRepositoryTest { masterPasswordHash = hashedMasterPassword, oneTimePassword = null, ) - } returns Unit.asSuccess() + } returns DeleteAccountResponseJson.Success.asSuccess() val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword) @@ -710,7 +711,7 @@ class AuthRepositoryTest { masterPasswordHash = null, oneTimePassword = oneTimePassword, ) - } returns Unit.asSuccess() + } returns DeleteAccountResponseJson.Success.asSuccess() val result = repository.deleteAccountWithOneTimePassword( oneTimePassword = oneTimePassword, diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt index df143122e7..052722e94e 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/login/LoginViewModelTest.kt @@ -451,7 +451,7 @@ class LoginViewModelTest : BaseViewModelTest() { savedStateHandle = SavedStateHandle().also { it["email_address"] = EMAIL it["state"] = state - } + }, ) companion object { 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 15b1f3e612..5868a0e407 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 @@ -126,7 +126,7 @@ class DeleteAccountViewModelTest : BaseViewModelTest() { val masterPassword = "ckasb kcs ja" coEvery { authRepo.deleteAccountWithMasterPassword(masterPassword) - } returns DeleteAccountResult.Error + } returns DeleteAccountResult.Error(message = null) viewModel.trySendAction(DeleteAccountAction.DeleteAccountConfirmDialogClick(masterPassword)) diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationViewModelTest.kt index 5343619bcf..11d4db6294 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/accountsecurity/deleteaccountconfirmation/DeleteAccountConfirmationViewModelTest.kt @@ -119,11 +119,11 @@ class DeleteAccountConfirmationViewModelTest : BaseViewModelTest() { @Test @Suppress("MaxLineLength") - fun `on DeleteAccountClick with DeleteAccountResult Error should set dialog to Error`() = + fun `on DeleteAccountClick with DeleteAccountResult Error should set dialog to Error with message`() = runTest { coEvery { authRepo.deleteAccountWithOneTimePassword("123456") - } returns DeleteAccountResult.Error + } returns DeleteAccountResult.Error(message = "Delete account error") val initialState = DEFAULT_STATE.copy( verificationCode = "123456", ) @@ -144,7 +144,7 @@ class DeleteAccountConfirmationViewModelTest : BaseViewModelTest() { assertEquals( initialState.copy( dialog = DeleteAccountConfirmationState.DeleteAccountConfirmationDialog.Error( - message = R.string.generic_error_message.asText(), + message = "Delete account error".asText(), ), ), awaitItem(),