mirror of
https://github.com/bitwarden/android.git
synced 2026-08-26 22:04:07 -05:00
[PM-23560] bug: Added guard to ensure duplicate scan events are not fired (#6687)
This commit is contained in:
+27
-3
@@ -1,20 +1,29 @@
|
||||
package com.x8bit.bitwarden.ui.vault.feature.qrcodescan
|
||||
|
||||
import android.os.Parcelable
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import com.bitwarden.ui.platform.base.BaseViewModel
|
||||
import com.bitwarden.ui.platform.base.DeferredBackgroundEvent
|
||||
import com.bitwarden.ui.platform.util.getTotpDataOrNull
|
||||
import com.x8bit.bitwarden.data.vault.repository.VaultRepository
|
||||
import com.x8bit.bitwarden.data.vault.repository.model.TotpCodeResult
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.parcelize.Parcelize
|
||||
import javax.inject.Inject
|
||||
|
||||
private const val KEY_STATE = "state"
|
||||
|
||||
/**
|
||||
* Handles [QrCodeScanAction] and launches [QrCodeScanEvent] for the [QrCodeScanScreen].
|
||||
*/
|
||||
@HiltViewModel
|
||||
class QrCodeScanViewModel @Inject constructor(
|
||||
private val vaultRepository: VaultRepository,
|
||||
) : BaseViewModel<Unit, QrCodeScanEvent, QrCodeScanAction>(
|
||||
initialState = Unit,
|
||||
savedStateHandle: SavedStateHandle,
|
||||
) : BaseViewModel<QrCodeScanState, QrCodeScanEvent, QrCodeScanAction>(
|
||||
initialState = savedStateHandle[KEY_STATE]
|
||||
?: QrCodeScanState(hasHandledScan = false),
|
||||
) {
|
||||
override fun handleAction(action: QrCodeScanAction) {
|
||||
when (action) {
|
||||
@@ -35,6 +44,12 @@ class QrCodeScanViewModel @Inject constructor(
|
||||
|
||||
// For more information: https://bitwarden.com/help/authenticator-keys/#support-for-more-parameters
|
||||
private fun handleQrCodeScanReceive(action: QrCodeScanAction.QrCodeScanReceive) {
|
||||
if (state.hasHandledScan) {
|
||||
return
|
||||
}
|
||||
mutableStateFlow.update {
|
||||
it.copy(hasHandledScan = true)
|
||||
}
|
||||
val qrCode = action.qrCode
|
||||
qrCode
|
||||
.getTotpDataOrNull()
|
||||
@@ -55,8 +70,9 @@ sealed class QrCodeScanEvent {
|
||||
|
||||
/**
|
||||
* Navigate back.
|
||||
* Added DeferredBackgroundEvent as QrCodeScan might be fired before events are consumed
|
||||
*/
|
||||
data object NavigateBack : QrCodeScanEvent()
|
||||
data object NavigateBack : QrCodeScanEvent(), DeferredBackgroundEvent
|
||||
|
||||
/**
|
||||
* Navigate to manual code entry screen.
|
||||
@@ -89,3 +105,11 @@ sealed class QrCodeScanAction {
|
||||
*/
|
||||
data object CameraSetupErrorReceive : QrCodeScanAction()
|
||||
}
|
||||
|
||||
/**
|
||||
* Represents the state of the QrCodeScan screen.
|
||||
*/
|
||||
@Parcelize
|
||||
data class QrCodeScanState(
|
||||
val hasHandledScan: Boolean,
|
||||
) : Parcelable
|
||||
|
||||
+30
-1
@@ -1,5 +1,6 @@
|
||||
package com.x8bit.bitwarden.ui.vault.feature.qrcodescan
|
||||
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import app.cash.turbine.test
|
||||
import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow
|
||||
import com.bitwarden.ui.platform.base.BaseViewModelTest
|
||||
@@ -104,8 +105,36 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
}
|
||||
}
|
||||
|
||||
private fun createViewModel(): QrCodeScanViewModel =
|
||||
@Test
|
||||
fun `QrCodeScanReceive scan already handled should emit NavigateBack without emitting code`() =
|
||||
runTest {
|
||||
val viewModel = createViewModel()
|
||||
val validCode = "otpauth://totp/Test:me?secret=JBSWY3dpeHPK3PXP"
|
||||
val result = TotpCodeResult.Success(validCode)
|
||||
every { validCode.getTotpDataOrNull() } returns mockk()
|
||||
|
||||
viewModel.eventFlow.test {
|
||||
viewModel.trySendAction(QrCodeScanAction.QrCodeScanReceive(validCode))
|
||||
assertEquals(QrCodeScanEvent.NavigateBack, awaitItem())
|
||||
viewModel.trySendAction(QrCodeScanAction.QrCodeScanReceive(validCode))
|
||||
expectNoEvents()
|
||||
}
|
||||
|
||||
// emitTotpCodeResult called exactly once across both actions
|
||||
verify(exactly = 1) { vaultRepository.emitTotpCodeResult(result) }
|
||||
val expectedState = DEFAULT_STATE.copy(hasHandledScan = true)
|
||||
assertEquals(expectedState, viewModel.stateFlow.value)
|
||||
}
|
||||
|
||||
private fun createViewModel(
|
||||
initialState: QrCodeScanState? = null,
|
||||
): QrCodeScanViewModel =
|
||||
QrCodeScanViewModel(
|
||||
savedStateHandle = SavedStateHandle().apply { set("state", initialState) },
|
||||
vaultRepository = vaultRepository,
|
||||
)
|
||||
|
||||
private val DEFAULT_STATE = QrCodeScanState(
|
||||
hasHandledScan = false,
|
||||
)
|
||||
}
|
||||
|
||||
+23
-4
@@ -2,6 +2,7 @@ package com.bitwarden.authenticator.ui.authenticator.feature.qrcodescan
|
||||
|
||||
import android.os.Parcelable
|
||||
import androidx.core.net.toUri
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import com.bitwarden.authenticator.data.authenticator.manager.TotpCodeManager
|
||||
import com.bitwarden.authenticator.data.authenticator.repository.AuthenticatorRepository
|
||||
import com.bitwarden.authenticator.data.authenticator.repository.model.TotpCodeResult
|
||||
@@ -10,12 +11,15 @@ import com.bitwarden.authenticator.data.platform.repository.SettingsRepository
|
||||
import com.bitwarden.authenticator.ui.platform.feature.settings.data.model.DefaultSaveOption
|
||||
import com.bitwarden.authenticatorbridge.manager.AuthenticatorBridgeManager
|
||||
import com.bitwarden.ui.platform.base.BaseViewModel
|
||||
import com.bitwarden.ui.platform.base.DeferredBackgroundEvent
|
||||
import com.bitwarden.ui.platform.util.getTotpDataOrNull
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.parcelize.Parcelize
|
||||
import javax.inject.Inject
|
||||
|
||||
private const val KEY_STATE = "state"
|
||||
|
||||
/**
|
||||
* Handles [QrCodeScanAction] and launches [QrCodeScanEvent] for the [QrCodeScanScreen].
|
||||
*/
|
||||
@@ -25,8 +29,13 @@ class QrCodeScanViewModel @Inject constructor(
|
||||
private val authenticatorBridgeManager: AuthenticatorBridgeManager,
|
||||
private val authenticatorRepository: AuthenticatorRepository,
|
||||
private val settingsRepository: SettingsRepository,
|
||||
savedStateHandle: SavedStateHandle,
|
||||
) : BaseViewModel<QrCodeScanState, QrCodeScanEvent, QrCodeScanAction>(
|
||||
initialState = QrCodeScanState(dialog = null),
|
||||
initialState = savedStateHandle[KEY_STATE]
|
||||
?: QrCodeScanState(
|
||||
hasHandledScan = false,
|
||||
dialog = null,
|
||||
),
|
||||
) {
|
||||
|
||||
/**
|
||||
@@ -69,7 +78,10 @@ class QrCodeScanViewModel @Inject constructor(
|
||||
|
||||
private fun handleSaveToBitwardenDismiss() {
|
||||
mutableStateFlow.update {
|
||||
it.copy(dialog = null)
|
||||
it.copy(
|
||||
hasHandledScan = false,
|
||||
dialog = null,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,6 +94,12 @@ class QrCodeScanViewModel @Inject constructor(
|
||||
}
|
||||
|
||||
private fun handleQrCodeScanReceive(action: QrCodeScanAction.QrCodeScanReceive) {
|
||||
if (state.hasHandledScan) {
|
||||
return
|
||||
}
|
||||
mutableStateFlow.update {
|
||||
it.copy(hasHandledScan = true)
|
||||
}
|
||||
val scannedCode = action.qrCode
|
||||
if (scannedCode.startsWith(TotpCodeManager.TOTP_CODE_PREFIX)) {
|
||||
handleTotpUriReceive(scannedCode)
|
||||
@@ -90,7 +108,6 @@ class QrCodeScanViewModel @Inject constructor(
|
||||
} else {
|
||||
authenticatorRepository.emitTotpCodeResult(TotpCodeResult.CodeScanningError)
|
||||
sendEvent(QrCodeScanEvent.NavigateBack)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
@@ -166,6 +183,7 @@ class QrCodeScanViewModel @Inject constructor(
|
||||
@Parcelize
|
||||
data class QrCodeScanState(
|
||||
val dialog: DialogState?,
|
||||
val hasHandledScan: Boolean,
|
||||
) : Parcelable {
|
||||
|
||||
/**
|
||||
@@ -193,8 +211,9 @@ sealed class QrCodeScanEvent {
|
||||
|
||||
/**
|
||||
* Navigate back.
|
||||
* Added DeferredBackgroundEvent as QrCodeScan might be fired before events are consumed
|
||||
*/
|
||||
data object NavigateBack : QrCodeScanEvent()
|
||||
data object NavigateBack : QrCodeScanEvent(), DeferredBackgroundEvent
|
||||
|
||||
/**
|
||||
* Navigate to manual code entry screen.
|
||||
|
||||
+1
@@ -142,5 +142,6 @@ class QrCodeScanScreenTest : AuthenticatorComposeTest() {
|
||||
}
|
||||
|
||||
private val DEFAULT_STATE = QrCodeScanState(
|
||||
hasHandledScan = false,
|
||||
dialog = null,
|
||||
)
|
||||
|
||||
+75
-10
@@ -1,6 +1,7 @@
|
||||
package com.bitwarden.authenticator.ui.authenticator.feature.qrcodescan
|
||||
|
||||
import android.net.Uri
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import app.cash.turbine.test
|
||||
import com.bitwarden.authenticator.data.authenticator.repository.AuthenticatorRepository
|
||||
import com.bitwarden.authenticator.data.authenticator.repository.model.SharedVerificationCodesState
|
||||
@@ -91,7 +92,10 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
viewModel.trySendAction(QrCodeScanAction.SaveToBitwardenClick(false))
|
||||
}
|
||||
val expectedState =
|
||||
DEFAULT_STATE.copy(dialog = QrCodeScanState.DialogState.SaveToBitwardenError)
|
||||
DEFAULT_STATE.copy(
|
||||
hasHandledScan = true,
|
||||
dialog = QrCodeScanState.DialogState.SaveToBitwardenError,
|
||||
)
|
||||
assertEquals(expectedState, viewModel.stateFlow.value)
|
||||
verify { authenticatorBridgeManager.startAddTotpLoginItemFlow(VALID_TOTP_CODE) }
|
||||
}
|
||||
@@ -169,8 +173,7 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
}
|
||||
|
||||
@Test
|
||||
@Suppress("MaxLineLength")
|
||||
fun `on SaveToBitwardenErrorDismiss recieve should clear dialog state`() {
|
||||
fun `on SaveToBitwardenErrorDismiss receive should clear dialog state`() {
|
||||
val viewModel = createViewModel()
|
||||
every {
|
||||
authenticatorBridgeManager.startAddTotpLoginItemFlow(VALID_TOTP_CODE)
|
||||
@@ -179,13 +182,19 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
viewModel.trySendAction(QrCodeScanAction.QrCodeScanReceive(VALID_TOTP_CODE))
|
||||
viewModel.trySendAction(QrCodeScanAction.SaveToBitwardenClick(false))
|
||||
val expectedState =
|
||||
DEFAULT_STATE.copy(dialog = QrCodeScanState.DialogState.SaveToBitwardenError)
|
||||
DEFAULT_STATE.copy(
|
||||
hasHandledScan = true,
|
||||
dialog = QrCodeScanState.DialogState.SaveToBitwardenError,
|
||||
)
|
||||
assertEquals(expectedState, viewModel.stateFlow.value)
|
||||
verify { authenticatorBridgeManager.startAddTotpLoginItemFlow(VALID_TOTP_CODE) }
|
||||
|
||||
// Clear dialog:
|
||||
viewModel.trySendAction(QrCodeScanAction.SaveToBitwardenErrorDismiss)
|
||||
assertEquals(DEFAULT_STATE, viewModel.stateFlow.value)
|
||||
assertEquals(
|
||||
DEFAULT_STATE,
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -199,7 +208,10 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
} just runs
|
||||
viewModel.trySendAction(QrCodeScanAction.QrCodeScanReceive(VALID_TOTP_CODE))
|
||||
verify { authenticatorRepository.emitTotpCodeResult(VALID_TOTP_RESULT) }
|
||||
assertEquals(DEFAULT_STATE, viewModel.stateFlow.value)
|
||||
assertEquals(
|
||||
DEFAULT_STATE.copy(hasHandledScan = true),
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -219,7 +231,7 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
assertEquals(QrCodeScanEvent.NavigateBack, awaitItem())
|
||||
}
|
||||
verify { authenticatorRepository.emitTotpCodeResult(VALID_TOTP_RESULT) }
|
||||
assertEquals(DEFAULT_STATE, viewModel.stateFlow.value)
|
||||
assertEquals(DEFAULT_STATE.copy(hasHandledScan = true), viewModel.stateFlow.value)
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -239,11 +251,13 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
assertEquals(QrCodeScanEvent.NavigateBack, awaitItem())
|
||||
}
|
||||
verify { authenticatorBridgeManager.startAddTotpLoginItemFlow(VALID_TOTP_CODE) }
|
||||
assertEquals(DEFAULT_STATE, viewModel.stateFlow.value)
|
||||
assertEquals(
|
||||
DEFAULT_STATE.copy(hasHandledScan = true),
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
@Suppress("MaxLineLength")
|
||||
fun `on QrCodeScanReceive when code is invalid should emit result and navigate back`() =
|
||||
runTest {
|
||||
val viewModel = createViewModel()
|
||||
@@ -259,7 +273,57 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
verify { authenticatorRepository.emitTotpCodeResult(TotpCodeResult.CodeScanningError) }
|
||||
}
|
||||
|
||||
private fun createViewModel() = QrCodeScanViewModel(
|
||||
@Test
|
||||
fun `on QrCodeScanReceive second scan after valid TOTP scan is ignored`() = runTest {
|
||||
val viewModel = createViewModel()
|
||||
every { settingsRepository.defaultSaveOption } returns DefaultSaveOption.LOCAL
|
||||
every {
|
||||
authenticatorRepository.sharedCodesStateFlow.value
|
||||
} returns SharedVerificationCodesState.Success(emptyList())
|
||||
every {
|
||||
authenticatorRepository.emitTotpCodeResult(VALID_TOTP_RESULT)
|
||||
} just runs
|
||||
viewModel.eventFlow.test {
|
||||
viewModel.trySendAction(QrCodeScanAction.QrCodeScanReceive(VALID_TOTP_CODE))
|
||||
viewModel.trySendAction(QrCodeScanAction.QrCodeScanReceive(VALID_TOTP_CODE))
|
||||
assertEquals(QrCodeScanEvent.NavigateBack, awaitItem())
|
||||
expectNoEvents()
|
||||
}
|
||||
verify(exactly = 1) { authenticatorRepository.emitTotpCodeResult(VALID_TOTP_RESULT) }
|
||||
assertEquals(
|
||||
DEFAULT_STATE.copy(hasHandledScan = true),
|
||||
viewModel.stateFlow.value,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `on QrCodeScanReceive second scan after error scan is ignored`() = runTest {
|
||||
val viewModel = createViewModel()
|
||||
val invalidQrCode = "otpauth://totp/secret=SECRET"
|
||||
every { invalidQrCode.getTotpDataOrNull() } returns null
|
||||
every {
|
||||
authenticatorRepository.emitTotpCodeResult(TotpCodeResult.CodeScanningError)
|
||||
} just runs
|
||||
every { settingsRepository.defaultSaveOption } returns DefaultSaveOption.LOCAL
|
||||
every {
|
||||
authenticatorRepository.emitTotpCodeResult(VALID_TOTP_RESULT)
|
||||
} just runs
|
||||
viewModel.eventFlow.test {
|
||||
viewModel.trySendAction(QrCodeScanAction.QrCodeScanReceive(invalidQrCode))
|
||||
viewModel.trySendAction(QrCodeScanAction.QrCodeScanReceive(VALID_TOTP_CODE))
|
||||
assertEquals(QrCodeScanEvent.NavigateBack, awaitItem())
|
||||
expectNoEvents()
|
||||
}
|
||||
verify(exactly = 1) {
|
||||
authenticatorRepository.emitTotpCodeResult(TotpCodeResult.CodeScanningError)
|
||||
}
|
||||
verify(exactly = 0) { authenticatorRepository.emitTotpCodeResult(VALID_TOTP_RESULT) }
|
||||
}
|
||||
|
||||
private fun createViewModel(
|
||||
initialState: QrCodeScanState? = null,
|
||||
) = QrCodeScanViewModel(
|
||||
savedStateHandle = SavedStateHandle().apply { set("state", initialState) },
|
||||
authenticatorBridgeManager = authenticatorBridgeManager,
|
||||
authenticatorRepository = authenticatorRepository,
|
||||
settingsRepository = settingsRepository,
|
||||
@@ -267,6 +331,7 @@ class QrCodeScanViewModelTest : BaseViewModelTest() {
|
||||
}
|
||||
|
||||
private val DEFAULT_STATE = QrCodeScanState(
|
||||
hasHandledScan = false,
|
||||
dialog = null,
|
||||
)
|
||||
private const val VALID_TOTP_CODE = "otpauth://totp/Label?secret=SECRET&issuer=Issuer"
|
||||
|
||||
Reference in New Issue
Block a user