[BIT-2361] Display account deletion error message provided by network response (#1389)

This commit is contained in:
Patrick Honkonen 2024-05-29 08:51:16 -04:00 committed by Álison Fernandes
parent 5d5c5809d1
commit 6ff39e486d
11 changed files with 105 additions and 39 deletions

View file

@ -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<String, List<String?>>?,
) : DeleteAccountResponseJson()
}

View file

@ -1,5 +1,6 @@
package com.x8bit.bitwarden.data.auth.datasource.network.service 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.PasswordHintResponseJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.ResendEmailRequestJson import com.x8bit.bitwarden.data.auth.datasource.network.model.ResendEmailRequestJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.ResetPasswordRequestJson import com.x8bit.bitwarden.data.auth.datasource.network.model.ResetPasswordRequestJson
@ -18,7 +19,10 @@ interface AccountsService {
/** /**
* Make delete account request. * Make delete account request.
*/ */
suspend fun deleteAccount(masterPasswordHash: String?, oneTimePassword: String?): Result<Unit> suspend fun deleteAccount(
masterPasswordHash: String?,
oneTimePassword: String?,
): Result<DeleteAccountResponseJson>
/** /**
* Request a one-time passcode that is sent to the user's email. * Request a one-time passcode that is sent to the user's email.

View file

@ -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.api.AuthenticatedAccountsApi
import com.x8bit.bitwarden.data.auth.datasource.network.model.CreateAccountKeysRequest 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.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.PasswordHintRequestJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.PasswordHintResponseJson 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.ResendEmailRequestJson
@ -34,13 +35,26 @@ class AccountsServiceImpl(
override suspend fun deleteAccount( override suspend fun deleteAccount(
masterPasswordHash: String?, masterPasswordHash: String?,
oneTimePassword: String?, oneTimePassword: String?,
): Result<Unit> = ): Result<DeleteAccountResponseJson> =
authenticatedAccountsApi.deleteAccount( authenticatedAccountsApi
DeleteAccountRequestJson( .deleteAccount(
masterPasswordHash = masterPasswordHash, DeleteAccountRequestJson(
oneTimePassword = oneTimePassword, masterPasswordHash = masterPasswordHash,
), oneTimePassword = oneTimePassword,
) ),
)
.map {
DeleteAccountResponseJson.Success
}
.recoverCatching { throwable ->
throwable
.toBitwardenError()
.parseErrorBodyOrNull<DeleteAccountResponseJson.Invalid>(
code = 400,
json = json,
)
?: throw throwable
}
override suspend fun requestOneTimePasscode(): Result<Unit> = override suspend fun requestOneTimePasscode(): Result<Unit> =
authenticatedAccountsApi.requestOtp() authenticatedAccountsApi.requestOtp()

View file

@ -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.AccountTokensJson
import com.x8bit.bitwarden.data.auth.datasource.disk.model.ForcePasswordResetReason 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.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.DeviceDataModel
import com.x8bit.bitwarden.data.auth.datasource.network.model.GetTokenResponseJson 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.IdentityTokenAuthModel
@ -383,7 +384,7 @@ class AuthRepositoryImpl(
masterPassword: String, masterPassword: String,
): DeleteAccountResult { ): DeleteAccountResult {
val profile = authDiskSource.userState?.activeAccount?.profile val profile = authDiskSource.userState?.activeAccount?.profile
?: return DeleteAccountResult.Error ?: return DeleteAccountResult.Error(message = null)
mutableHasPendingAccountDeletionStateFlow.value = true mutableHasPendingAccountDeletionStateFlow.value = true
return authSdkSource return authSdkSource
.hashPassword( .hashPassword(
@ -398,12 +399,7 @@ class AuthRepositoryImpl(
oneTimePassword = null, oneTimePassword = null,
) )
} }
.onSuccess { logout() } .finalizeDeleteAccount()
.onFailure { clearPendingAccountDeletion() }
.fold(
onFailure = { DeleteAccountResult.Error },
onSuccess = { DeleteAccountResult.Success },
)
} }
override suspend fun deleteAccountWithOneTimePassword( override suspend fun deleteAccountWithOneTimePassword(
@ -415,14 +411,35 @@ class AuthRepositoryImpl(
masterPasswordHash = null, masterPasswordHash = null,
oneTimePassword = oneTimePassword, oneTimePassword = oneTimePassword,
) )
.onSuccess { logout() } .finalizeDeleteAccount()
.onFailure { clearPendingAccountDeletion() }
.fold(
onFailure = { DeleteAccountResult.Error },
onSuccess = { DeleteAccountResult.Success },
)
} }
private fun Result<DeleteAccountResponseJson>.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") @Suppress("ReturnCount")
override suspend fun createNewSsoUser(): NewSsoUserResult { override suspend fun createNewSsoUser(): NewSsoUserResult {
val account = authDiskSource.userState?.activeAccount ?: return NewSsoUserResult.Failure val account = authDiskSource.userState?.activeAccount ?: return NewSsoUserResult.Failure

View file

@ -12,5 +12,5 @@ sealed class DeleteAccountResult {
/** /**
* There was an error deleting the account. * There was an error deleting the account.
*/ */
data object Error : DeleteAccountResult() data class Error(val message: String?) : DeleteAccountResult()
} }

View file

@ -95,18 +95,19 @@ class DeleteAccountViewModel @Inject constructor(
private fun handleDeleteAccountComplete( private fun handleDeleteAccountComplete(
action: DeleteAccountAction.Internal.DeleteAccountComplete, action: DeleteAccountAction.Internal.DeleteAccountComplete,
) { ) {
when (action.result) { when (val result = action.result) {
DeleteAccountResult.Success -> { DeleteAccountResult.Success -> {
mutableStateFlow.update { mutableStateFlow.update {
it.copy(dialog = DeleteAccountState.DeleteAccountDialog.DeleteSuccess) it.copy(dialog = DeleteAccountState.DeleteAccountDialog.DeleteSuccess)
} }
} }
DeleteAccountResult.Error -> { is DeleteAccountResult.Error -> {
mutableStateFlow.update { mutableStateFlow.update {
it.copy( it.copy(
dialog = DeleteAccountState.DeleteAccountDialog.Error( dialog = DeleteAccountState.DeleteAccountDialog.Error(
message = R.string.generic_error_message.asText(), message = result.message?.asText()
?: R.string.generic_error_message.asText(),
), ),
) )
} }

View file

@ -151,10 +151,11 @@ class DeleteAccountConfirmationViewModel @Inject constructor(
) { ) {
mutableStateFlow.update { currentState -> mutableStateFlow.update { currentState ->
currentState.copy( currentState.copy(
dialog = when (action.deleteAccountResult) { dialog = when (val result = action.deleteAccountResult) {
DeleteAccountResult.Error -> { is DeleteAccountResult.Error -> {
DeleteAccountConfirmationState.DeleteAccountConfirmationDialog.Error( DeleteAccountConfirmationState.DeleteAccountConfirmationDialog.Error(
message = R.string.generic_error_message.asText(), message = result.message?.asText()
?: R.string.generic_error_message.asText(),
) )
} }

View file

@ -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.PendingAuthRequestJson
import com.x8bit.bitwarden.data.auth.datasource.disk.model.UserStateJson 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.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.GetTokenResponseJson
import com.x8bit.bitwarden.data.auth.datasource.network.model.IdentityTokenAuthModel import com.x8bit.bitwarden.data.auth.datasource.network.model.IdentityTokenAuthModel
import com.x8bit.bitwarden.data.auth.datasource.network.model.KdfTypeJson import com.x8bit.bitwarden.data.auth.datasource.network.model.KdfTypeJson
@ -601,7 +602,7 @@ class AuthRepositoryTest {
masterPasswordHash = hashedMasterPassword, masterPasswordHash = hashedMasterPassword,
oneTimePassword = null, oneTimePassword = null,
) )
} returns Unit.asSuccess() } returns DeleteAccountResponseJson.Success.asSuccess()
fakeAuthDiskSource.userState = SINGLE_USER_STATE_1 fakeAuthDiskSource.userState = SINGLE_USER_STATE_1
repository.userStateFlow.test { repository.userStateFlow.test {
@ -625,7 +626,7 @@ class AuthRepositoryTest {
fun `delete account fails if not logged in`() = runTest { fun `delete account fails if not logged in`() = runTest {
val masterPassword = "hello world" val masterPassword = "hello world"
val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword) val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword)
assertEquals(DeleteAccountResult.Error, result) assertEquals(DeleteAccountResult.Error(message = null), result)
} }
@Test @Test
@ -639,7 +640,7 @@ class AuthRepositoryTest {
val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword) val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword)
assertEquals(DeleteAccountResult.Error, result) assertEquals(DeleteAccountResult.Error(message = null), result)
coVerify { coVerify {
authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION) authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION)
} }
@ -663,7 +664,7 @@ class AuthRepositoryTest {
val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword) val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword)
assertEquals(DeleteAccountResult.Error, result) assertEquals(DeleteAccountResult.Error(message = null), result)
coVerify { coVerify {
authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION) authSdkSource.hashPassword(EMAIL, masterPassword, kdf, HashPurpose.SERVER_AUTHORIZATION)
accountsService.deleteAccount( accountsService.deleteAccount(
@ -687,7 +688,7 @@ class AuthRepositoryTest {
masterPasswordHash = hashedMasterPassword, masterPasswordHash = hashedMasterPassword,
oneTimePassword = null, oneTimePassword = null,
) )
} returns Unit.asSuccess() } returns DeleteAccountResponseJson.Success.asSuccess()
val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword) val result = repository.deleteAccountWithMasterPassword(masterPassword = masterPassword)
@ -710,7 +711,7 @@ class AuthRepositoryTest {
masterPasswordHash = null, masterPasswordHash = null,
oneTimePassword = oneTimePassword, oneTimePassword = oneTimePassword,
) )
} returns Unit.asSuccess() } returns DeleteAccountResponseJson.Success.asSuccess()
val result = repository.deleteAccountWithOneTimePassword( val result = repository.deleteAccountWithOneTimePassword(
oneTimePassword = oneTimePassword, oneTimePassword = oneTimePassword,

View file

@ -451,7 +451,7 @@ class LoginViewModelTest : BaseViewModelTest() {
savedStateHandle = SavedStateHandle().also { savedStateHandle = SavedStateHandle().also {
it["email_address"] = EMAIL it["email_address"] = EMAIL
it["state"] = state it["state"] = state
} },
) )
companion object { companion object {

View file

@ -126,7 +126,7 @@ class DeleteAccountViewModelTest : BaseViewModelTest() {
val masterPassword = "ckasb kcs ja" val masterPassword = "ckasb kcs ja"
coEvery { coEvery {
authRepo.deleteAccountWithMasterPassword(masterPassword) authRepo.deleteAccountWithMasterPassword(masterPassword)
} returns DeleteAccountResult.Error } returns DeleteAccountResult.Error(message = null)
viewModel.trySendAction(DeleteAccountAction.DeleteAccountConfirmDialogClick(masterPassword)) viewModel.trySendAction(DeleteAccountAction.DeleteAccountConfirmDialogClick(masterPassword))

View file

@ -119,11 +119,11 @@ class DeleteAccountConfirmationViewModelTest : BaseViewModelTest() {
@Test @Test
@Suppress("MaxLineLength") @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 { runTest {
coEvery { coEvery {
authRepo.deleteAccountWithOneTimePassword("123456") authRepo.deleteAccountWithOneTimePassword("123456")
} returns DeleteAccountResult.Error } returns DeleteAccountResult.Error(message = "Delete account error")
val initialState = DEFAULT_STATE.copy( val initialState = DEFAULT_STATE.copy(
verificationCode = "123456", verificationCode = "123456",
) )
@ -144,7 +144,7 @@ class DeleteAccountConfirmationViewModelTest : BaseViewModelTest() {
assertEquals( assertEquals(
initialState.copy( initialState.copy(
dialog = DeleteAccountConfirmationState.DeleteAccountConfirmationDialog.Error( dialog = DeleteAccountConfirmationState.DeleteAccountConfirmationDialog.Error(
message = R.string.generic_error_message.asText(), message = "Delete account error".asText(),
), ),
), ),
awaitItem(), awaitItem(),