BWA-106 - Import process failures with limited error feedback (#313)

This commit is contained in:
Phil Cappelli
2025-01-10 10:32:34 -05:00
committed by GitHub
parent 43844739a2
commit f595cee7f6
12 changed files with 237 additions and 126 deletions

View File

@@ -21,23 +21,33 @@ class ImportManagerImpl(
importFileFormat: ImportFileFormat,
byteArray: ByteArray,
): ImportDataResult {
val parser = createParser(importFileFormat)
return processParseResult(parser.parseForResult(byteArray))
}
val parser: ExportParser = when (importFileFormat) {
ImportFileFormat.BITWARDEN_JSON -> BitwardenExportParser(importFileFormat)
ImportFileFormat.TWO_FAS_JSON -> TwoFasExportParser()
ImportFileFormat.LAST_PASS_JSON -> LastPassExportParser()
ImportFileFormat.AEGIS -> AegisExportParser()
private fun createParser(
importFileFormat: ImportFileFormat,
): ExportParser = when (importFileFormat) {
ImportFileFormat.BITWARDEN_JSON -> BitwardenExportParser(importFileFormat)
ImportFileFormat.TWO_FAS_JSON -> TwoFasExportParser()
ImportFileFormat.LAST_PASS_JSON -> LastPassExportParser()
ImportFileFormat.AEGIS -> AegisExportParser()
}
private suspend fun processParseResult(
parseResult: ExportParseResult,
): ImportDataResult = when (parseResult) {
is ExportParseResult.Error -> {
ImportDataResult.Error(
title = parseResult.title,
message = parseResult.message,
)
}
return when (val parseResult = parser.parse(byteArray)) {
is ExportParseResult.Error -> {
ImportDataResult.Error(parseResult.message)
}
is ExportParseResult.Success -> {
authenticatorDiskSource.saveItem(*parseResult.items.toTypedArray())
ImportDataResult.Success
}
is ExportParseResult.Success -> {
val items = parseResult.items.toTypedArray()
authenticatorDiskSource.saveItem(*items)
ImportDataResult.Success
}
}
}

View File

@@ -14,9 +14,14 @@ sealed class ExportParseResult {
data class Success(val items: List<AuthenticatorItemEntity>) : ExportParseResult()
/**
* Indicates there was an error while parsing the selected file.
* Represents an error that occurred while parsing the selected file.
* Provides user-friendly messages to display to the user.
*
* @property message User friendly message displayed to the user, if provided.
* @property title A user-friendly title summarizing the error.
* @property message A detailed message describing the error.
*/
data class Error(val message: Text? = null) : ExportParseResult()
data class Error(
val title: Text? = null,
val message: Text? = null,
) : ExportParseResult()
}

View File

@@ -14,7 +14,11 @@ sealed class ImportDataResult {
/**
* Indicates import was not successful.
*
* @property message Optional [Text] indicating why the import failed.
* @property title An optional [Text] providing a brief title of the reason the import failed.
* @property message An optional [Text] containing an explanation of why the import failed.
*/
data class Error(val message: Text? = null) : ImportDataResult()
data class Error(
val title: Text? = null,
val message: Text? = null,
) : ImportDataResult()
}

View File

@@ -6,17 +6,15 @@ import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.Aut
import com.bitwarden.authenticator.data.platform.manager.imports.model.AegisJsonExport
import com.bitwarden.authenticator.data.platform.manager.imports.model.ExportParseResult
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.decodeFromStream
import java.io.ByteArrayInputStream
import java.io.IOException
import java.util.UUID
/**
* Implementation of [ExportParser] responsible for parsing exports from the Aegis application.
*/
class AegisExportParser : ExportParser {
class AegisExportParser : ExportParser() {
@OptIn(ExperimentalSerializationApi::class)
override fun parse(byteArray: ByteArray): ExportParseResult {
val importJson = Json {
@@ -25,22 +23,14 @@ class AegisExportParser : ExportParser {
explicitNulls = false
}
return try {
val exportData = importJson
.decodeFromStream<AegisJsonExport>(ByteArrayInputStream(byteArray))
ExportParseResult.Success(
items = exportData
.db
.entries
.toAuthenticatorItemEntities(),
)
} catch (e: SerializationException) {
ExportParseResult.Error()
} catch (e: IllegalArgumentException) {
ExportParseResult.Error()
} catch (e: IOException) {
ExportParseResult.Error()
}
val exportData = importJson.decodeFromStream<AegisJsonExport>(
ByteArrayInputStream(byteArray),
)
return ExportParseResult.Success(
items = exportData.db.entries
.toAuthenticatorItemEntities(),
)
}
private fun List<AegisJsonExport.Database.Entry>.toAuthenticatorItemEntities() =

View File

@@ -11,18 +11,16 @@ import com.bitwarden.authenticator.data.platform.manager.imports.model.ExportPar
import com.bitwarden.authenticator.data.platform.manager.imports.model.ImportFileFormat
import com.bitwarden.authenticator.ui.platform.base.util.asText
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.decodeFromStream
import java.io.ByteArrayInputStream
import java.io.IOException
/**
* Implementation of [ExportParser] responsible for parsing exports from the Bitwarden application.
*/
class BitwardenExportParser(
private val fileFormat: ImportFileFormat,
) : ExportParser {
) : ExportParser() {
override fun parse(byteArray: ByteArray): ExportParseResult {
return when (fileFormat) {
ImportFileFormat.BITWARDEN_JSON -> importJsonFile(byteArray)
@@ -38,26 +36,20 @@ class BitwardenExportParser(
explicitNulls = false
}
return try {
val exportData = importJson
.decodeFromStream<ExportJsonData>(ByteArrayInputStream(byteArray))
ExportParseResult.Success(
items = exportData
.items
.filter { it.login?.totp != null }
.toAuthenticatorItemEntities(),
)
} catch (e: SerializationException) {
ExportParseResult.Error()
} catch (e: IllegalArgumentException) {
ExportParseResult.Error()
} catch (e: IOException) {
ExportParseResult.Error()
}
val exportData = importJson.decodeFromStream<ExportJsonData>(
ByteArrayInputStream(byteArray),
)
return ExportParseResult.Success(
items = exportData.items
.filter { it.login?.totp != null }
.toAuthenticatorItemEntities(),
)
}
private fun List<ExportJsonData.ExportItem>.toAuthenticatorItemEntities() =
map { it.toAuthenticatorItemEntity() }
private fun List<ExportJsonData.ExportItem>.toAuthenticatorItemEntities() = map {
it.toAuthenticatorItemEntity()
}
@Suppress("MaxLineLength", "CyclomaticComplexMethod", "LongMethod")
private fun ExportJsonData.ExportItem.toAuthenticatorItemEntity(): AuthenticatorItemEntity {
@@ -73,8 +65,7 @@ class BitwardenExportParser(
}
else -> {
val uriString =
"${TotpCodeManager.TOTP_CODE_PREFIX}/$name?${TotpCodeManager.SECRET_PARAM}=$otpString"
val uriString = "${TotpCodeManager.TOTP_CODE_PREFIX}/$name?${TotpCodeManager.SECRET_PARAM}=$otpString"
Uri.parse(uriString)
}
}
@@ -115,8 +106,7 @@ class BitwardenExportParser(
TotpCodeManager.STEAM_DIGITS_DEFAULT
}
}
val issuer = otpUri.getQueryParameter(TotpCodeManager.ISSUER_PARAM)
?: name
val issuer = otpUri.getQueryParameter(TotpCodeManager.ISSUER_PARAM) ?: name
val label = when (type) {
AuthenticatorItemType.TOTP -> {

View File

@@ -1,17 +1,57 @@
package com.bitwarden.authenticator.data.platform.manager.imports.parsers
import com.bitwarden.authenticator.R
import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemEntity
import com.bitwarden.authenticator.data.platform.manager.imports.model.ExportParseResult
import com.bitwarden.authenticator.ui.platform.base.util.asText
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.MissingFieldException
import kotlinx.serialization.SerializationException
import java.io.IOException
/**
* Responsible for transforming exported authenticator data to a format consumable by this
* application.
*/
interface ExportParser {
abstract class ExportParser {
/**
* Converts the given [byteArray] content of a file to a collection of
* [AuthenticatorItemEntity].
*/
fun parse(byteArray: ByteArray): ExportParseResult
protected abstract fun parse(byteArray: ByteArray): ExportParseResult
/**
* Parses the given byte array into an [ExportParseResult].
*
* This method attempts to deserialize the input data and return a successful result.
* If deserialization fails due to various exceptions, an appropriate error result
* is returned instead.
*
* Exceptions handled include:
* - [MissingFieldException]: If required fields are missing in the input data.
* - [SerializationException]: If the input data cannot be processed due to invalid format.
* - [IllegalArgumentException]: If an argument provided to a method is invalid.
* - [IOException]: If an I/O error occurs during processing.
*
* @param byteArray The input data to be parsed as a [ByteArray].
* @return [ExportParseResult] indicating success or a specific error result.
*/
@OptIn(ExperimentalSerializationApi::class)
fun parseForResult(byteArray: ByteArray): ExportParseResult = try {
parse(byteArray = byteArray)
} catch (error: MissingFieldException) {
ExportParseResult.Error(
title = R.string.required_information_missing.asText(),
message = R.string.required_information_missing_message.asText(),
)
} catch (error: SerializationException) {
ExportParseResult.Error(
title = R.string.file_could_not_be_processed.asText(),
message = R.string.file_could_not_be_processed_message.asText(),
)
} catch (error: IllegalArgumentException) {
ExportParseResult.Error(message = error.message?.asText())
} catch (error: IOException) {
ExportParseResult.Error(message = error.message?.asText())
}
}

View File

@@ -6,18 +6,16 @@ import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.Aut
import com.bitwarden.authenticator.data.platform.manager.imports.model.ExportParseResult
import com.bitwarden.authenticator.data.platform.manager.imports.model.LastPassJsonExport
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.decodeFromStream
import java.io.ByteArrayInputStream
import java.io.IOException
import java.util.UUID
/**
* An [ExportParser] responsible for transforming LastPass export files into Bitwarden Authenticator
* items.
*/
class LastPassExportParser : ExportParser {
class LastPassExportParser : ExportParser() {
@OptIn(ExperimentalSerializationApi::class)
override fun parse(byteArray: ByteArray): ExportParseResult {
@@ -27,25 +25,19 @@ class LastPassExportParser : ExportParser {
explicitNulls = false
}
return try {
val exportData =
importJson.decodeFromStream<LastPassJsonExport>(ByteArrayInputStream(byteArray))
ExportParseResult.Success(
items = exportData
.accounts
.toAuthenticatorItemEntities(),
)
} catch (e: SerializationException) {
ExportParseResult.Error()
} catch (e: IllegalArgumentException) {
ExportParseResult.Error()
} catch (e: IOException) {
ExportParseResult.Error()
}
val exportData = importJson.decodeFromStream<LastPassJsonExport>(
ByteArrayInputStream(byteArray),
)
return ExportParseResult.Success(
items = exportData.accounts
.toAuthenticatorItemEntities(),
)
}
private fun List<LastPassJsonExport.Account>.toAuthenticatorItemEntities() =
map { it.toAuthenticatorItemEntity() }
private fun List<LastPassJsonExport.Account>.toAuthenticatorItemEntities() = map {
it.toAuthenticatorItemEntity()
}
private fun LastPassJsonExport.Account.toAuthenticatorItemEntity(): AuthenticatorItemEntity {

View File

@@ -8,11 +8,9 @@ import com.bitwarden.authenticator.data.platform.manager.imports.model.ExportPar
import com.bitwarden.authenticator.data.platform.manager.imports.model.TwoFasJsonExport
import com.bitwarden.authenticator.ui.platform.base.util.asText
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.SerializationException
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.decodeFromStream
import java.io.ByteArrayInputStream
import java.io.IOException
import java.util.UUID
private const val TOKEN_TYPE_HOTP = "HOTP"
@@ -21,7 +19,7 @@ private const val TOKEN_TYPE_HOTP = "HOTP"
* An [ExportParser] responsible for transforming 2FAS export files into Bitwarden Authenticator
* items.
*/
class TwoFasExportParser : ExportParser {
class TwoFasExportParser : ExportParser() {
override fun parse(byteArray: ByteArray): ExportParseResult {
return import2fasJson(byteArray)
}
@@ -35,30 +33,24 @@ class TwoFasExportParser : ExportParser {
encodeDefaults = true
}
return try {
val exportData = importJson
.decodeFromStream<TwoFasJsonExport>(ByteArrayInputStream(byteArray))
val exportData = importJson.decodeFromStream<TwoFasJsonExport>(
ByteArrayInputStream(byteArray),
)
if (!exportData.servicesEncrypted.isNullOrEmpty()) {
ExportParseResult.Error(
message = R.string.import_2fas_password_protected_not_supported.asText(),
)
} else {
ExportParseResult.Success(
items = exportData.services.toAuthenticatorItemEntities(),
)
}
} catch (e: SerializationException) {
ExportParseResult.Error()
} catch (e: IllegalArgumentException) {
ExportParseResult.Error()
} catch (e: IOException) {
ExportParseResult.Error()
return if (!exportData.servicesEncrypted.isNullOrEmpty()) {
ExportParseResult.Error(
message = R.string.import_2fas_password_protected_not_supported.asText(),
)
} else {
ExportParseResult.Success(
items = exportData.services.toAuthenticatorItemEntities(),
)
}
}
private fun List<TwoFasJsonExport.Service>.toAuthenticatorItemEntities() =
mapNotNull { it.toAuthenticatorItemEntityOrNull() }
private fun List<TwoFasJsonExport.Service>.toAuthenticatorItemEntities() = mapNotNull {
it.toAuthenticatorItemEntityOrNull()
}
@Suppress("MaxLineLength")
private fun TwoFasJsonExport.Service.toAuthenticatorItemEntityOrNull(): AuthenticatorItemEntity {

View File

@@ -24,6 +24,7 @@ import androidx.compose.ui.res.stringResource
import androidx.compose.ui.semantics.semantics
import androidx.compose.ui.semantics.testTag
import androidx.compose.ui.unit.dp
import androidx.core.net.toUri
import androidx.hilt.navigation.compose.hiltViewModel
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import com.bitwarden.authenticator.R
@@ -31,9 +32,8 @@ import com.bitwarden.authenticator.data.platform.manager.imports.model.ImportFil
import com.bitwarden.authenticator.ui.platform.base.util.EventsEffect
import com.bitwarden.authenticator.ui.platform.components.appbar.BitwardenTopAppBar
import com.bitwarden.authenticator.ui.platform.components.button.BitwardenFilledTonalButton
import com.bitwarden.authenticator.ui.platform.components.dialog.BasicDialogState
import com.bitwarden.authenticator.ui.platform.components.dialog.BitwardenBasicDialog
import com.bitwarden.authenticator.ui.platform.components.dialog.BitwardenLoadingDialog
import com.bitwarden.authenticator.ui.platform.components.dialog.BitwardenTwoButtonDialog
import com.bitwarden.authenticator.ui.platform.components.dialog.LoadingDialogState
import com.bitwarden.authenticator.ui.platform.components.dropdown.BitwardenMultiSelectButton
import com.bitwarden.authenticator.ui.platform.components.scaffold.BitwardenScaffold
@@ -85,15 +85,19 @@ fun ImportingScreen(
when (val dialog = state.dialogState) {
is ImportState.DialogState.Error -> {
BitwardenBasicDialog(
visibilityState = BasicDialogState.Shown(
title = dialog.title,
message = dialog.message,
),
onDismissRequest = remember(viewModel) {
{
viewModel.trySendAction(ImportAction.DialogDismiss)
}
BitwardenTwoButtonDialog(
title = dialog.title?.invoke(),
message = dialog.message.invoke(),
confirmButtonText = stringResource(id = R.string.get_help),
onConfirmClick = {
intentManager.launchUri("https://bitwarden.com/help".toUri())
},
dismissButtonText = stringResource(id = R.string.cancel),
onDismissClick = {
viewModel.trySendAction(ImportAction.DialogDismiss)
},
onDismissRequest = {
viewModel.trySendAction(ImportAction.DialogDismiss)
},
)
}

View File

@@ -73,10 +73,16 @@ class ImportingViewModel @Inject constructor(
private fun handleImportLocationReceive(action: ImportAction.ImportLocationReceive) {
mutableStateFlow.update { it.copy(dialogState = ImportState.DialogState.Loading()) }
viewModelScope.launch {
val result =
authenticatorRepository.importVaultData(state.importFileFormat, action.fileUri)
sendAction(ImportAction.Internal.SaveImportDataToUriResultReceive(result))
val result = authenticatorRepository.importVaultData(
format = state.importFileFormat,
fileData = action.fileUri,
)
sendAction(
ImportAction.Internal.SaveImportDataToUriResultReceive(result),
)
}
}
@@ -94,9 +100,8 @@ class ImportingViewModel @Inject constructor(
mutableStateFlow.update {
it.copy(
dialogState = ImportState.DialogState.Error(
title = R.string.an_error_has_occurred.asText(),
message = result.message
?: R.string.import_vault_failure.asText(),
title = result.title ?: R.string.an_error_has_occurred.asText(),
message = result.message ?: R.string.import_vault_failure.asText(),
),
)
}

View File

@@ -146,4 +146,9 @@
<string name="add_code_to_bitwarden">Add code to Bitwarden</string>
<string name="add_code_locally">Add code locally</string>
<string name="local_codes">Local codes</string>
<string name="required_information_missing">Required Information Missing</string>
<string name="required_information_missing_message">"Required info is missing (e.g., services or secret). Check your file and try again. Visit bitwarden.com/help for support"</string>
<string name="file_could_not_be_processed">"File Could Not Be Processed"</string>
<string name="file_could_not_be_processed_message">"File could not be processed. Ensure its valid JSON and try again. Need help? Visit bitwarden.com/help"</string>
<string name="get_help">Get Help</string>
</resources>

View File

@@ -0,0 +1,74 @@
package com.bitwarden.authenticator.data.platform.manager.imports
import com.bitwarden.authenticator.data.authenticator.datasource.disk.AuthenticatorDiskSource
import com.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemEntity
import com.bitwarden.authenticator.data.platform.manager.imports.model.ExportParseResult
import com.bitwarden.authenticator.data.platform.manager.imports.model.ImportDataResult
import com.bitwarden.authenticator.data.platform.manager.imports.model.ImportFileFormat
import com.bitwarden.authenticator.data.platform.manager.imports.parsers.BitwardenExportParser
import com.bitwarden.authenticator.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.mockkConstructor
import io.mockk.runs
import io.mockk.unmockkConstructor
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
class ImportManagerTest {
private val mockAuthenticatorDiskSource = mockk<AuthenticatorDiskSource>()
private val manager = ImportManagerImpl(
authenticatorDiskSource = mockAuthenticatorDiskSource,
)
@BeforeEach
fun setup() {
mockkConstructor(BitwardenExportParser::class)
}
@AfterEach
fun tearDown() {
unmockkConstructor(BitwardenExportParser::class)
}
@Test
fun `ImportManager returns success result from ExportParser and saves items to disk`() =
runTest {
val listOfItems = emptyList<AuthenticatorItemEntity>()
coEvery {
mockAuthenticatorDiskSource.saveItem(*listOfItems.toTypedArray())
} just runs
every {
anyConstructed<BitwardenExportParser>().parseForResult(any())
} returns ExportParseResult.Success(listOfItems)
val result = manager.import(ImportFileFormat.BITWARDEN_JSON, DEFAULT_BYTE_ARRAY)
assertEquals(ImportDataResult.Success, result)
coVerify(exactly = 1) {
mockAuthenticatorDiskSource.saveItem(*listOfItems.toTypedArray())
}
}
@Test
fun `ImportManager returns correct error result from ExportParser`() = runTest {
val errorMessage = "borked".asText()
every {
anyConstructed<BitwardenExportParser>().parseForResult(any())
} returns ExportParseResult.Error(message = errorMessage)
val result = manager.import(ImportFileFormat.BITWARDEN_JSON, DEFAULT_BYTE_ARRAY)
assertEquals(ImportDataResult.Error(message = errorMessage), result)
}
}
private val DEFAULT_BYTE_ARRAY = "".toByteArray()