bitwarden/android
1
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2025-12-11 04:39:19 -06:00
Code Issues Packages Projects Releases 169 Wiki Activity

[PM-21782] Improve create cipher error handling (#5362)

Browse Source
This commit is contained in:
Patrick Honkonen 2025-06-16 10:23:30 -04:00 committed by GitHub
parent 469df4495a
commit 95f146fb3e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 320 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/CipherManagerImpl.kt
View File

@ -7,6 +7,7 @@ import com.bitwarden.core.data.util.asSuccess
import com.bitwarden.core.data.util.flatMap
import com.bitwarden.network.model.AttachmentJsonResponse
import com.bitwarden.network.model.CreateCipherInOrganizationJsonRequest
import com.bitwarden.network.model.CreateCipherResponseJson
import com.bitwarden.network.model.ShareCipherJsonRequest
import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
import com.bitwarden.network.model.UpdateCipherResponseJson
@ -52,19 +53,33 @@ class CipherManagerImpl(
override suspend fun createCipher(cipherView: CipherView): CreateCipherResult {
val userId = activeUserId
?: return CreateCipherResult.Error(error = NoActiveUserException())
?: return CreateCipherResult.Error(
error = NoActiveUserException(),
errorMessage = null,
)
return vaultSdkSource
.encryptCipher(
userId = userId,
cipherView = cipherView,
)
.flatMap { ciphersService.createCipher(body = it.toEncryptedNetworkCipher()) }
.onSuccess { vaultDiskSource.saveCipher(userId = userId, cipher = it) }
.map { response ->
when (response) {
is CreateCipherResponseJson.Invalid -> {
CreateCipherResult.Error(errorMessage = response.message, error = null)
}
is CreateCipherResponseJson.Success -> {
vaultDiskSource.saveCipher(userId = userId, cipher = response.cipher)
CreateCipherResult.Success
}
}
}
.fold(
onFailure = { CreateCipherResult.Error(error = it) },
onFailure = { CreateCipherResult.Error(errorMessage = null, error = it) },
onSuccess = {
reviewPromptManager.registerAddCipherAction()
CreateCipherResult.Success
it
},
)
}
@ -74,7 +89,7 @@ class CipherManagerImpl(
collectionIds: List<String>,
): CreateCipherResult {
val userId = activeUserId
?: return CreateCipherResult.Error(error = NoActiveUserException())
?: return CreateCipherResult.Error(errorMessage = null, error = NoActiveUserException())
return vaultSdkSource
.encryptCipher(
userId = userId,
@ -88,17 +103,26 @@ class CipherManagerImpl(
),
)
}
.onSuccess {
vaultDiskSource.saveCipher(
userId = userId,
cipher = it.copy(collectionIds = collectionIds),
)
.map { response ->
when (response) {
is CreateCipherResponseJson.Invalid -> {
CreateCipherResult.Error(errorMessage = response.message, error = null)
}
is CreateCipherResponseJson.Success -> {
vaultDiskSource.saveCipher(
userId = userId,
cipher = response.cipher.copy(collectionIds = collectionIds),
)
CreateCipherResult.Success
}
}
}
.fold(
onFailure = { CreateCipherResult.Error(error = it) },
onFailure = { CreateCipherResult.Error(errorMessage = null, error = it) },
onSuccess = {
reviewPromptManager.registerAddCipherAction()
CreateCipherResult.Success
it
},
)
}

5
app/src/main/kotlin/com/x8bit/bitwarden/data/vault/repository/model/CreateCipherResult.kt
View File

@ -11,7 +11,8 @@ sealed class CreateCipherResult {
data object Success : CreateCipherResult()
/**
* Generic error while creating cipher.
* Generic error while creating cipher. The optional [errorMessage] may be displayed directly in
* the UI when present.
*/
data class Error(val error: Throwable) : CreateCipherResult()
data class Error(val errorMessage: String?, val error: Throwable?) : CreateCipherResult()
}

6
app/src/main/kotlin/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModel.kt
View File

@ -1598,7 +1598,11 @@ class VaultAddEditViewModel @Inject constructor(
is CreateCipherResult.Error -> {
showDialog(
dialogState = VaultAddEditState.DialogState.Generic(
message = R.string.generic_error_message.asText(),
title = R.string.an_error_has_occurred.asText(),
message = result
.errorMessage
?.asText()
?: R.string.generic_error_message.asText(),
error = result.error,
),
)

128
app/src/test/kotlin/com/x8bit/bitwarden/data/vault/manager/CipherManagerTest.kt
View File

@ -6,6 +6,7 @@ import com.bitwarden.core.data.util.asFailure
import com.bitwarden.core.data.util.asSuccess
import com.bitwarden.network.model.AttachmentJsonRequest
import com.bitwarden.network.model.CreateCipherInOrganizationJsonRequest
import com.bitwarden.network.model.CreateCipherResponseJson
import com.bitwarden.network.model.ShareCipherJsonRequest
import com.bitwarden.network.model.SyncResponseJson
import com.bitwarden.network.model.UpdateCipherCollectionsJsonRequest
@ -46,6 +47,7 @@ import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedNetworkCipher
import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedNetworkCipherResponse
import com.x8bit.bitwarden.data.vault.repository.util.toEncryptedSdkCipher
import com.x8bit.bitwarden.data.vault.repository.util.toNetworkAttachmentRequest
import io.mockk.Ordering
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
@ -56,7 +58,6 @@ import io.mockk.mockkStatic
import io.mockk.runs
import io.mockk.unmockkConstructor
import io.mockk.unmockkStatic
import io.mockk.verify
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals
@ -123,7 +124,13 @@ class CipherManagerTest {
val result = cipherManager.createCipher(cipherView = mockk())
assertEquals(CreateCipherResult.Error(error = NoActiveUserException()), result)
assertEquals(
CreateCipherResult.Error(
errorMessage = null,
error = NoActiveUserException(),
),
result,
)
}
@Test
@ -142,7 +149,7 @@ class CipherManagerTest {
val result = cipherManager.createCipher(cipherView = mockCipherView)
assertEquals(CreateCipherResult.Error(error = error), result)
assertEquals(CreateCipherResult.Error(errorMessage = null, error = error), result)
}
@Test
@ -173,7 +180,46 @@ class CipherManagerTest {
val result = cipherManager.createCipher(cipherView = mockCipherView)
assertEquals(CreateCipherResult.Error(error = error), result)
assertEquals(CreateCipherResult.Error(errorMessage = null, error = error), result)
}
@Suppress("MaxLineLength")
@Test
fun `createCipher with ciphersService createCipher Invalid response should return CreateCipherResult failure`() =
runTest {
fakeAuthDiskSource.userState = MOCK_USER_STATE
val userId = "mockId-1"
val mockCipherView = createMockCipherView(number = 1)
coEvery {
vaultSdkSource.encryptCipher(
userId = userId,
cipherView = mockCipherView,
)
} returns createMockEncryptionContext(
number = 1,
cipher = createMockSdkCipher(number = 1, clock = clock),
).asSuccess()
coEvery {
ciphersService.createCipher(
body = createMockCipherJsonRequest(
number = 1,
login = createMockLogin(number = 1, uri = null),
),
)
} returns CreateCipherResponseJson.Invalid(
message = "You do not have permission to edit this.",
validationErrors = null,
).asSuccess()
val result = cipherManager.createCipher(cipherView = mockCipherView)
assertEquals(
CreateCipherResult.Error(
errorMessage = "You do not have permission to edit this.",
error = null,
),
result,
)
}
@Test
@ -200,13 +246,16 @@ class CipherManagerTest {
login = createMockLogin(number = 1, uri = null),
),
)
} returns mockCipher.asSuccess()
} returns CreateCipherResponseJson.Success(mockCipher).asSuccess()
coEvery { vaultDiskSource.saveCipher(userId, mockCipher) } just runs
val result = cipherManager.createCipher(cipherView = mockCipherView)
assertEquals(CreateCipherResult.Success, result)
verify(exactly = 1) { reviewPromptManager.registerAddCipherAction() }
coVerify(ordering = Ordering.ORDERED) {
vaultDiskSource.saveCipher(userId, mockCipher)
reviewPromptManager.registerAddCipherAction()
}
}
@Test
@ -219,7 +268,13 @@ class CipherManagerTest {
collectionIds = mockk(),
)
assertEquals(CreateCipherResult.Error(error = NoActiveUserException()), result)
assertEquals(
CreateCipherResult.Error(
errorMessage = null,
error = NoActiveUserException(),
),
result,
)
}
@Test
@ -242,7 +297,7 @@ class CipherManagerTest {
collectionIds = mockk(),
)
assertEquals(CreateCipherResult.Error(error = error), result)
assertEquals(CreateCipherResult.Error(errorMessage = null, error = error), result)
}
@Test
@ -279,7 +334,52 @@ class CipherManagerTest {
collectionIds = listOf("mockId-1"),
)
assertEquals(CreateCipherResult.Error(error = error), result)
assertEquals(CreateCipherResult.Error(errorMessage = null, error = error), result)
}
@Suppress("MaxLineLength")
@Test
fun `createCipherInOrganization with ciphersService createCipher Invalid response should return CreateCipherResult failure`() =
runTest {
fakeAuthDiskSource.userState = MOCK_USER_STATE
val userId = "mockId-1"
val mockCipherView = createMockCipherView(number = 1)
coEvery {
vaultSdkSource.encryptCipher(
userId = userId,
cipherView = mockCipherView,
)
} returns createMockEncryptionContext(
number = 1,
cipher = createMockSdkCipher(number = 1, clock = clock),
).asSuccess()
coEvery {
ciphersService.createCipherInOrganization(
body = CreateCipherInOrganizationJsonRequest(
cipher = createMockCipherJsonRequest(
number = 1,
login = createMockLogin(number = 1, uri = null),
),
collectionIds = listOf("mockId-1"),
),
)
} returns CreateCipherResponseJson.Invalid(
message = "You do not have permission to edit this.",
validationErrors = null,
).asSuccess()
val result = cipherManager.createCipherInOrganization(
cipherView = mockCipherView,
collectionIds = listOf("mockId-1"),
)
assertEquals(
CreateCipherResult.Error(
errorMessage = "You do not have permission to edit this.",
error = null,
),
result,
)
}
@Test
@ -309,7 +409,7 @@ class CipherManagerTest {
collectionIds = listOf("mockId-1"),
),
)
} returns mockCipher.asSuccess()
} returns CreateCipherResponseJson.Success(mockCipher).asSuccess()
coEvery {
vaultDiskSource.saveCipher(
userId,
@ -323,7 +423,13 @@ class CipherManagerTest {
)
assertEquals(CreateCipherResult.Success, result)
verify(exactly = 1) { reviewPromptManager.registerAddCipherAction() }
coVerify(ordering = Ordering.ORDERED) {
vaultDiskSource.saveCipher(
userId,
mockCipher.copy(collectionIds = listOf("mockId-1")),
)
reviewPromptManager.registerAddCipherAction()
}
}
@Test

57
app/src/test/kotlin/com/x8bit/bitwarden/ui/vault/feature/addedit/VaultAddEditViewModelTest.kt
View File

@ -1876,7 +1876,7 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
coEvery {
vaultRepository.createCipherInOrganization(any(), any())
} returns CreateCipherResult.Error(error = Throwable("Oh dang"))
} returns CreateCipherResult.Error(errorMessage = null, error = Throwable("Oh dang"))
viewModel.trySendAction(VaultAddEditAction.Common.SaveClick)
assertEquals(
@ -1890,6 +1890,61 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
)
}
@Test
fun `in edit mode, SaveClick should show network error message in dialog when not null`() =
runTest {
val stateWithName = createVaultAddItemState(
commonContentViewState = createCommonContentViewState(
name = "mockName-1",
),
)
mutableVaultDataFlow.value = DataState.Loaded(createVaultData())
val viewModel = createAddVaultItemViewModel(
createSavedStateHandleWithState(
state = stateWithName,
vaultAddEditType = VaultAddEditType.AddItem,
vaultItemCipherType = VaultItemCipherType.LOGIN,
),
)
coEvery {
vaultRepository.createCipherInOrganization(any(), any())
} returns CreateCipherResult.Error(
errorMessage = "Network error message",
error = null,
)
viewModel.trySendAction(VaultAddEditAction.Common.SaveClick)
assertEquals(
stateWithName.copy(
dialog = VaultAddEditState.DialogState.Generic(
title = R.string.an_error_has_occurred.asText(),
message = "Network error message".asText(),
),
),
viewModel.stateFlow.value,
)
// Verify default error message is shown when errorMessage is null
coEvery {
vaultRepository.createCipherInOrganization(any(), any())
} returns CreateCipherResult.Error(
errorMessage = null,
error = null,
)
viewModel.trySendAction(VaultAddEditAction.Common.SaveClick)
assertEquals(
stateWithName.copy(
dialog = VaultAddEditState.DialogState.Generic(
title = R.string.an_error_has_occurred.asText(),
message = R.string.generic_error_message.asText(),
),
),
viewModel.stateFlow.value,
)
}
@Test
fun `in edit mode, SaveClick should show dialog, and remove it once an item is saved`() =
runTest {

32
network/src/main/kotlin/com/bitwarden/network/model/CreateCipherResponseJson.kt Normal file
View File

@ -0,0 +1,32 @@
package com.bitwarden.network.model
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
/**
* Models the response from the create cipher request.
*/
sealed class CreateCipherResponseJson {
/**
* The request completed successfully and returned the created [cipher].
*/
data class Success(val cipher: SyncResponseJson.Cipher) : CreateCipherResponseJson()
/**
* Represents the json body of an invalid create request.
*
* @param message A general, user-displayable error message.
* @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("message")
val message: String?,
@SerialName("validationErrors")
val validationErrors: Map<String, List<String>>?,
) : CreateCipherResponseJson()
}

5
network/src/main/kotlin/com/bitwarden/network/service/CiphersService.kt
View File

@ -5,6 +5,7 @@ import com.bitwarden.network.model.AttachmentJsonRequest
import com.bitwarden.network.model.AttachmentJsonResponse
import com.bitwarden.network.model.CipherJsonRequest
import com.bitwarden.network.model.CreateCipherInOrganizationJsonRequest
import com.bitwarden.network.model.CreateCipherResponseJson
import com.bitwarden.network.model.ImportCiphersJsonRequest
import com.bitwarden.network.model.ImportCiphersResponseJson
import com.bitwarden.network.model.ShareCipherJsonRequest
@ -21,14 +22,14 @@ interface CiphersService {
/**
* Attempt to create a cipher.
*/
suspend fun createCipher(body: CipherJsonRequest): Result<SyncResponseJson.Cipher>
suspend fun createCipher(body: CipherJsonRequest): Result<CreateCipherResponseJson>
/**
* Attempt to create a cipher that belongs to an organization.
*/
suspend fun createCipherInOrganization(
body: CreateCipherInOrganizationJsonRequest,
): Result<SyncResponseJson.Cipher>
): Result<CreateCipherResponseJson>
/**
* Attempt to upload an attachment file.

27
network/src/main/kotlin/com/bitwarden/network/service/CiphersServiceImpl.kt
View File

@ -8,6 +8,7 @@ import com.bitwarden.network.model.AttachmentJsonRequest
import com.bitwarden.network.model.AttachmentJsonResponse
import com.bitwarden.network.model.CipherJsonRequest
import com.bitwarden.network.model.CreateCipherInOrganizationJsonRequest
import com.bitwarden.network.model.CreateCipherResponseJson
import com.bitwarden.network.model.FileUploadType
import com.bitwarden.network.model.ImportCiphersJsonRequest
import com.bitwarden.network.model.ImportCiphersResponseJson
@ -36,16 +37,38 @@ internal class CiphersServiceImpl(
private val json: Json,
private val clock: Clock,
) : CiphersService {
override suspend fun createCipher(body: CipherJsonRequest): Result<SyncResponseJson.Cipher> =
override suspend fun createCipher(
body: CipherJsonRequest,
): Result<CreateCipherResponseJson> =
ciphersApi
.createCipher(body = body)
.toResult()
.map { CreateCipherResponseJson.Success(it) }
.recoverCatching { throwable ->
throwable
.toBitwardenError()
.parseErrorBodyOrNull<CreateCipherResponseJson.Invalid>(
code = NetworkErrorCode.BAD_REQUEST,
json = json,
)
?: throw throwable
}
override suspend fun createCipherInOrganization(
body: CreateCipherInOrganizationJsonRequest,
): Result<SyncResponseJson.Cipher> = ciphersApi
): Result<CreateCipherResponseJson> = ciphersApi
.createCipherInOrganization(body = body)
.toResult()
.map { CreateCipherResponseJson.Success(it) }
.recoverCatching { throwable ->
throwable
.toBitwardenError()
.parseErrorBodyOrNull<CreateCipherResponseJson.Invalid>(
code = NetworkErrorCode.BAD_REQUEST,
json = json,
)
?: throw throwable
}
override suspend fun createAttachment(
cipherId: String,

45
network/src/test/kotlin/com/bitwarden/network/service/CiphersServiceTest.kt
View File

@ -6,6 +6,7 @@ import com.bitwarden.network.api.CiphersApi
import com.bitwarden.network.base.BaseServiceTest
import com.bitwarden.network.model.AttachmentJsonResponse
import com.bitwarden.network.model.CreateCipherInOrganizationJsonRequest
import com.bitwarden.network.model.CreateCipherResponseJson
import com.bitwarden.network.model.FileUploadType
import com.bitwarden.network.model.ImportCiphersJsonRequest
import com.bitwarden.network.model.ImportCiphersResponseJson
@ -67,7 +68,22 @@ class CiphersServiceTest : BaseServiceTest() {
body = createMockCipherJsonRequest(number = 1),
)
assertEquals(
createMockCipher(number = 1),
CreateCipherResponseJson.Success(createMockCipher(number = 1)),
result.getOrThrow(),
)
}
@Test
fun `createCipher should return Invalid with correct data`() = runTest {
server.enqueue(MockResponse().setResponseCode(400).setBody(CREATE_CIPHER_INVALID_JSON))
val result = ciphersService.createCipher(
body = createMockCipherJsonRequest(number = 1),
)
assertEquals(
CreateCipherResponseJson.Invalid(
message = "Cipher was not encrypted for the current user. Please try again.",
validationErrors = null,
),
result.getOrThrow(),
)
}
@ -82,11 +98,30 @@ class CiphersServiceTest : BaseServiceTest() {
),
)
assertEquals(
createMockCipher(number = 1),
CreateCipherResponseJson.Success(createMockCipher(number = 1)),
result.getOrThrow(),
)
}
@Test
fun `createCipherInOrganization should return Invalid with correct data`() =
runTest {
server.enqueue(MockResponse().setResponseCode(400).setBody(CREATE_CIPHER_INVALID_JSON))
val result = ciphersService.createCipherInOrganization(
body = CreateCipherInOrganizationJsonRequest(
cipher = createMockCipherJsonRequest(number = 1),
collectionIds = listOf("12345"),
),
)
assertEquals(
CreateCipherResponseJson.Invalid(
message = "Cipher was not encrypted for the current user. Please try again.",
validationErrors = null,
),
result.getOrThrow(),
)
}
@Test
fun `createAttachment should return the correct response`() = runTest {
server.enqueue(MockResponse().setBody(CREATE_ATTACHMENT_SUCCESS_JSON))
@ -606,6 +641,12 @@ private const val UPDATE_CIPHER_INVALID_JSON = """
"validationErrors": null
}
"""
private const val CREATE_CIPHER_INVALID_JSON = """
{
"message": "Cipher was not encrypted for the current user. Please try again.",
"validationErrors": null
}
"""
private const val GET_CIPHER_ATTACHMENT_SUCCESS_JSON = """
{