mirror of
https://github.com/bitwarden/android.git
synced 2024-11-22 09:25:58 +03:00
BIT-1246, BIT-1250: Show correct permission-related errors when editing (#482)
This commit is contained in:
parent
8476e55b5a
commit
1e996fcbbe
11 changed files with 244 additions and 58 deletions
|
@ -9,6 +9,7 @@ import dagger.Module
|
|||
import dagger.Provides
|
||||
import dagger.hilt.InstallIn
|
||||
import dagger.hilt.components.SingletonComponent
|
||||
import kotlinx.serialization.json.Json
|
||||
import retrofit2.create
|
||||
import javax.inject.Singleton
|
||||
|
||||
|
@ -23,8 +24,10 @@ object VaultNetworkModule {
|
|||
@Singleton
|
||||
fun provideCiphersService(
|
||||
retrofits: Retrofits,
|
||||
json: Json,
|
||||
): CiphersService = CiphersServiceImpl(
|
||||
ciphersApi = retrofits.authenticatedApiRetrofit.create(),
|
||||
json = json,
|
||||
)
|
||||
|
||||
@Provides
|
||||
|
|
|
@ -0,0 +1,33 @@
|
|||
package com.x8bit.bitwarden.data.vault.datasource.network.model
|
||||
|
||||
import kotlinx.serialization.SerialName
|
||||
import kotlinx.serialization.Serializable
|
||||
|
||||
/**
|
||||
* Models the response from the update cipher request.
|
||||
*/
|
||||
sealed class UpdateCipherResponseJson {
|
||||
/**
|
||||
* The request completed successfully and returned the updated [cipher].
|
||||
*/
|
||||
data class Success(
|
||||
val cipher: SyncResponseJson.Cipher,
|
||||
) : UpdateCipherResponseJson()
|
||||
|
||||
/**
|
||||
* Represents the json body of an invalid update 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>>?,
|
||||
) : UpdateCipherResponseJson()
|
||||
}
|
|
@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.vault.datasource.network.service
|
|||
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.CipherJsonRequest
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
|
||||
|
||||
/**
|
||||
* Provides an API for querying ciphers endpoints.
|
||||
|
@ -18,5 +19,5 @@ interface CiphersService {
|
|||
suspend fun updateCipher(
|
||||
cipherId: String,
|
||||
body: CipherJsonRequest,
|
||||
): Result<SyncResponseJson.Cipher>
|
||||
): Result<UpdateCipherResponseJson>
|
||||
}
|
||||
|
|
|
@ -1,11 +1,16 @@
|
|||
package com.x8bit.bitwarden.data.vault.datasource.network.service
|
||||
|
||||
import com.x8bit.bitwarden.data.platform.datasource.network.model.toBitwardenError
|
||||
import com.x8bit.bitwarden.data.platform.datasource.network.util.parseErrorBodyOrNull
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.api.CiphersApi
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.CipherJsonRequest
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
|
||||
import kotlinx.serialization.json.Json
|
||||
|
||||
class CiphersServiceImpl constructor(
|
||||
private val ciphersApi: CiphersApi,
|
||||
private val json: Json,
|
||||
) : CiphersService {
|
||||
override suspend fun createCipher(body: CipherJsonRequest): Result<SyncResponseJson.Cipher> =
|
||||
ciphersApi.createCipher(body = body)
|
||||
|
@ -13,9 +18,20 @@ class CiphersServiceImpl constructor(
|
|||
override suspend fun updateCipher(
|
||||
cipherId: String,
|
||||
body: CipherJsonRequest,
|
||||
): Result<SyncResponseJson.Cipher> =
|
||||
ciphersApi.updateCipher(
|
||||
cipherId = cipherId,
|
||||
body = body,
|
||||
)
|
||||
): Result<UpdateCipherResponseJson> =
|
||||
ciphersApi
|
||||
.updateCipher(
|
||||
cipherId = cipherId,
|
||||
body = body,
|
||||
)
|
||||
.map { UpdateCipherResponseJson.Success(cipher = it) }
|
||||
.recoverCatching { throwable ->
|
||||
throwable
|
||||
.toBitwardenError()
|
||||
.parseErrorBodyOrNull<UpdateCipherResponseJson.Invalid>(
|
||||
code = 400,
|
||||
json = json,
|
||||
)
|
||||
?: throw throwable
|
||||
}
|
||||
}
|
||||
|
|
|
@ -22,6 +22,7 @@ import com.x8bit.bitwarden.data.platform.util.asSuccess
|
|||
import com.x8bit.bitwarden.data.platform.util.flatMap
|
||||
import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.service.CiphersService
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.service.SyncService
|
||||
import com.x8bit.bitwarden.data.vault.datasource.sdk.VaultSdkSource
|
||||
|
@ -387,10 +388,18 @@ class VaultRepositoryImpl(
|
|||
)
|
||||
}
|
||||
.fold(
|
||||
onFailure = { UpdateCipherResult.Error },
|
||||
onSuccess = {
|
||||
sync()
|
||||
UpdateCipherResult.Success
|
||||
onFailure = { UpdateCipherResult.Error(errorMessage = null) },
|
||||
onSuccess = { response ->
|
||||
when (response) {
|
||||
is UpdateCipherResponseJson.Invalid -> {
|
||||
UpdateCipherResult.Error(errorMessage = response.message)
|
||||
}
|
||||
|
||||
is UpdateCipherResponseJson.Success -> {
|
||||
sync()
|
||||
UpdateCipherResult.Success
|
||||
}
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
|
|
|
@ -11,7 +11,8 @@ sealed class UpdateCipherResult {
|
|||
data object Success : UpdateCipherResult()
|
||||
|
||||
/**
|
||||
* Generic error while updating cipher.
|
||||
* Generic error while updating cipher. The optional [errorMessage] may be displayed directly in
|
||||
* the UI when present.
|
||||
*/
|
||||
data object Error : UpdateCipherResult()
|
||||
data class Error(val errorMessage: String?) : UpdateCipherResult()
|
||||
}
|
||||
|
|
|
@ -690,10 +690,18 @@ class VaultAddEditViewModel @Inject constructor(
|
|||
action: VaultAddEditAction.Internal.UpdateCipherResultReceive,
|
||||
) {
|
||||
mutableStateFlow.update { it.copy(dialog = null) }
|
||||
when (action.updateCipherResult) {
|
||||
when (val result = action.updateCipherResult) {
|
||||
is UpdateCipherResult.Error -> {
|
||||
// TODO Display error dialog BIT-501
|
||||
sendEvent(VaultAddEditEvent.ShowToast(message = "Save Item Failure".asText()))
|
||||
mutableStateFlow.update {
|
||||
it.copy(
|
||||
dialog = VaultAddEditState.DialogState.Error(
|
||||
message = result
|
||||
.errorMessage
|
||||
?.asText()
|
||||
?: R.string.generic_error_message.asText(),
|
||||
),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
is UpdateCipherResult.Success -> {
|
||||
|
|
|
@ -13,7 +13,7 @@ import retrofit2.Retrofit
|
|||
*/
|
||||
abstract class BaseServiceTest {
|
||||
|
||||
private val json = PlatformNetworkModule.providesJson()
|
||||
protected val json = PlatformNetworkModule.providesJson()
|
||||
|
||||
protected val server = MockWebServer().apply { start() }
|
||||
|
||||
|
|
|
@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.vault.datasource.network.service
|
|||
|
||||
import com.x8bit.bitwarden.data.platform.base.BaseServiceTest
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.api.CiphersApi
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockCipher
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockCipherJsonRequest
|
||||
import kotlinx.coroutines.test.runTest
|
||||
|
@ -15,6 +16,7 @@ class CiphersServiceTest : BaseServiceTest() {
|
|||
|
||||
private val ciphersService: CiphersService = CiphersServiceImpl(
|
||||
ciphersApi = ciphersApi,
|
||||
json = json,
|
||||
)
|
||||
|
||||
@Test
|
||||
|
@ -30,17 +32,37 @@ class CiphersServiceTest : BaseServiceTest() {
|
|||
}
|
||||
|
||||
@Test
|
||||
fun `updateCipher should return the correct response`() = runTest {
|
||||
server.enqueue(MockResponse().setBody(CREATE_UPDATE_CIPHER_SUCCESS_JSON))
|
||||
val result = ciphersService.updateCipher(
|
||||
cipherId = "cipher-id-1",
|
||||
body = createMockCipherJsonRequest(number = 1),
|
||||
)
|
||||
assertEquals(
|
||||
createMockCipher(number = 1),
|
||||
result.getOrThrow(),
|
||||
)
|
||||
}
|
||||
fun `updateCipher with success response should return a Success with the correct cipher`() =
|
||||
runTest {
|
||||
server.enqueue(MockResponse().setBody(CREATE_UPDATE_CIPHER_SUCCESS_JSON))
|
||||
val result = ciphersService.updateCipher(
|
||||
cipherId = "cipher-id-1",
|
||||
body = createMockCipherJsonRequest(number = 1),
|
||||
)
|
||||
assertEquals(
|
||||
UpdateCipherResponseJson.Success(
|
||||
cipher = createMockCipher(number = 1),
|
||||
),
|
||||
result.getOrThrow(),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `updateCipher with an invalid response should return an Invalid with the correct data`() =
|
||||
runTest {
|
||||
server.enqueue(MockResponse().setResponseCode(400).setBody(UPDATE_CIPHER_INVALID_JSON))
|
||||
val result = ciphersService.updateCipher(
|
||||
cipherId = "cipher-id-1",
|
||||
body = createMockCipherJsonRequest(number = 1),
|
||||
)
|
||||
assertEquals(
|
||||
UpdateCipherResponseJson.Invalid(
|
||||
message = "You do not have permission to edit this.",
|
||||
validationErrors = null,
|
||||
),
|
||||
result.getOrThrow(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private const val CREATE_UPDATE_CIPHER_SUCCESS_JSON = """
|
||||
|
@ -134,3 +156,10 @@ private const val CREATE_UPDATE_CIPHER_SUCCESS_JSON = """
|
|||
"key": "mockKey-1"
|
||||
}
|
||||
"""
|
||||
|
||||
private const val UPDATE_CIPHER_INVALID_JSON = """
|
||||
{
|
||||
"message": "You do not have permission to edit this.",
|
||||
"validationErrors": null
|
||||
}
|
||||
"""
|
||||
|
|
|
@ -21,6 +21,7 @@ import com.x8bit.bitwarden.data.platform.util.asFailure
|
|||
import com.x8bit.bitwarden.data.platform.util.asSuccess
|
||||
import com.x8bit.bitwarden.data.vault.datasource.disk.VaultDiskSource
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.UpdateCipherResponseJson
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockCipher
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockCipherJsonRequest
|
||||
import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockCollection
|
||||
|
@ -1746,12 +1747,12 @@ class VaultRepositoryTest {
|
|||
cipherView = mockCipherView,
|
||||
)
|
||||
|
||||
assertEquals(UpdateCipherResult.Error, result)
|
||||
assertEquals(UpdateCipherResult.Error(errorMessage = null), result)
|
||||
}
|
||||
|
||||
@Test
|
||||
@Suppress("MaxLineLength")
|
||||
fun `updateCipher with ciphersService updateCipher failure should return UpdateCipherResult failure`() =
|
||||
fun `updateCipher with ciphersService updateCipher failure should return UpdateCipherResult Error with a null message`() =
|
||||
runTest {
|
||||
val cipherId = "cipherId1234"
|
||||
val mockCipherView = createMockCipherView(number = 1)
|
||||
|
@ -1770,12 +1771,12 @@ class VaultRepositoryTest {
|
|||
cipherView = mockCipherView,
|
||||
)
|
||||
|
||||
assertEquals(UpdateCipherResult.Error, result)
|
||||
assertEquals(UpdateCipherResult.Error(errorMessage = null), result)
|
||||
}
|
||||
|
||||
@Test
|
||||
@Suppress("MaxLineLength")
|
||||
fun `updateCipher with ciphersService updateCipher success should return UpdateCipherResult success`() =
|
||||
fun `updateCipher with ciphersService updateCipher Invalid response should return UpdateCipherResult Error with a non-null message`() =
|
||||
runTest {
|
||||
val cipherId = "cipherId1234"
|
||||
val mockCipherView = createMockCipherView(number = 1)
|
||||
|
@ -1787,7 +1788,43 @@ class VaultRepositoryTest {
|
|||
cipherId = cipherId,
|
||||
body = createMockCipherJsonRequest(number = 1),
|
||||
)
|
||||
} returns createMockCipher(number = 1).asSuccess()
|
||||
} returns UpdateCipherResponseJson
|
||||
.Invalid(
|
||||
message = "You do not have permission to edit this.",
|
||||
validationErrors = null,
|
||||
)
|
||||
.asSuccess()
|
||||
|
||||
val result = vaultRepository.updateCipher(
|
||||
cipherId = cipherId,
|
||||
cipherView = mockCipherView,
|
||||
)
|
||||
|
||||
assertEquals(
|
||||
UpdateCipherResult.Error(
|
||||
errorMessage = "You do not have permission to edit this.",
|
||||
),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
@Suppress("MaxLineLength")
|
||||
fun `updateCipher with ciphersService updateCipher Success response should return UpdateCipherResult success`() =
|
||||
runTest {
|
||||
val cipherId = "cipherId1234"
|
||||
val mockCipherView = createMockCipherView(number = 1)
|
||||
coEvery {
|
||||
vaultSdkSource.encryptCipher(cipherView = mockCipherView)
|
||||
} returns createMockSdkCipher(number = 1).asSuccess()
|
||||
coEvery {
|
||||
ciphersService.updateCipher(
|
||||
cipherId = cipherId,
|
||||
body = createMockCipherJsonRequest(number = 1),
|
||||
)
|
||||
} returns UpdateCipherResponseJson
|
||||
.Success(cipher = createMockCipher(number = 1))
|
||||
.asSuccess()
|
||||
coEvery {
|
||||
syncService.sync()
|
||||
} returns Result.success(createMockSyncResponse(1))
|
||||
|
|
|
@ -266,39 +266,88 @@ class VaultAddEditViewModelTest : BaseViewModelTest() {
|
|||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `in edit mode, SaveClick createCipher error should emit ShowToast`() = runTest {
|
||||
val cipherView = mockk<CipherView>()
|
||||
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
|
||||
val stateWithName = createVaultAddItemState(
|
||||
vaultAddEditType = vaultAddEditType,
|
||||
commonContentViewState = createCommonContentViewState(
|
||||
name = "mockName",
|
||||
),
|
||||
)
|
||||
|
||||
every { cipherView.toViewState() } returns stateWithName.viewState
|
||||
coEvery {
|
||||
vaultRepository.updateCipher(DEFAULT_EDIT_ITEM_ID, any())
|
||||
} returns UpdateCipherResult.Error
|
||||
mutableVaultItemFlow.value = DataState.Loaded(cipherView)
|
||||
|
||||
val viewModel = createAddVaultItemViewModel(
|
||||
createSavedStateHandleWithState(
|
||||
state = stateWithName,
|
||||
fun `in edit mode, SaveClick updateCipher error with a null message should show an error dialog with a generic message`() =
|
||||
runTest {
|
||||
val cipherView = mockk<CipherView>()
|
||||
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
|
||||
val stateWithName = createVaultAddItemState(
|
||||
vaultAddEditType = vaultAddEditType,
|
||||
),
|
||||
)
|
||||
commonContentViewState = createCommonContentViewState(
|
||||
name = "mockName",
|
||||
),
|
||||
)
|
||||
|
||||
every { cipherView.toViewState() } returns stateWithName.viewState
|
||||
coEvery {
|
||||
vaultRepository.updateCipher(DEFAULT_EDIT_ITEM_ID, any())
|
||||
} returns UpdateCipherResult.Error(errorMessage = null)
|
||||
mutableVaultItemFlow.value = DataState.Loaded(cipherView)
|
||||
|
||||
val viewModel = createAddVaultItemViewModel(
|
||||
createSavedStateHandleWithState(
|
||||
state = stateWithName,
|
||||
vaultAddEditType = vaultAddEditType,
|
||||
),
|
||||
)
|
||||
|
||||
viewModel.eventFlow.test {
|
||||
viewModel.actionChannel.trySend(VaultAddEditAction.Common.SaveClick)
|
||||
assertEquals(VaultAddEditEvent.ShowToast("Save Item Failure".asText()), awaitItem())
|
||||
|
||||
assertEquals(
|
||||
stateWithName.copy(
|
||||
dialog = VaultAddEditState.DialogState.Error(
|
||||
message = R.string.generic_error_message.asText(),
|
||||
),
|
||||
),
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
coVerify(exactly = 1) {
|
||||
vaultRepository.updateCipher(DEFAULT_EDIT_ITEM_ID, any())
|
||||
}
|
||||
}
|
||||
|
||||
coVerify(exactly = 1) {
|
||||
vaultRepository.updateCipher(DEFAULT_EDIT_ITEM_ID, any())
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `in edit mode, SaveClick updateCipher error with a non-null message should show an error dialog with that message`() =
|
||||
runTest {
|
||||
val cipherView = mockk<CipherView>()
|
||||
val vaultAddEditType = VaultAddEditType.EditItem(DEFAULT_EDIT_ITEM_ID)
|
||||
val stateWithName = createVaultAddItemState(
|
||||
vaultAddEditType = vaultAddEditType,
|
||||
commonContentViewState = createCommonContentViewState(
|
||||
name = "mockName",
|
||||
),
|
||||
)
|
||||
val errorMessage = "You do not have permission to edit this."
|
||||
|
||||
every { cipherView.toViewState() } returns stateWithName.viewState
|
||||
coEvery {
|
||||
vaultRepository.updateCipher(DEFAULT_EDIT_ITEM_ID, any())
|
||||
} returns UpdateCipherResult.Error(errorMessage = errorMessage)
|
||||
mutableVaultItemFlow.value = DataState.Loaded(cipherView)
|
||||
|
||||
val viewModel = createAddVaultItemViewModel(
|
||||
createSavedStateHandleWithState(
|
||||
state = stateWithName,
|
||||
vaultAddEditType = vaultAddEditType,
|
||||
),
|
||||
)
|
||||
|
||||
viewModel.actionChannel.trySend(VaultAddEditAction.Common.SaveClick)
|
||||
|
||||
assertEquals(
|
||||
stateWithName.copy(
|
||||
dialog = VaultAddEditState.DialogState.Error(
|
||||
message = errorMessage.asText(),
|
||||
),
|
||||
),
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
coVerify(exactly = 1) {
|
||||
vaultRepository.updateCipher(DEFAULT_EDIT_ITEM_ID, any())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `Saving item with an empty name field will cause a dialog to show up`() = runTest {
|
||||
|
|
Loading…
Reference in a new issue