From 805fea630cade0fbbac82d0bdb0d5704a80c25ac Mon Sep 17 00:00:00 2001 From: David Perez Date: Fri, 9 Aug 2024 09:09:41 -0500 Subject: [PATCH] Add logic for biometric unlock to SetupUnlockScreen (#3702) --- .../feature/accountsetup/SetupUnlockScreen.kt | 35 ++++- .../accountsetup/SetupUnlockViewModel.kt | 100 +++++++++++++- .../handlers/SetupUnlockHandler.kt | 6 + .../accountsetup/SetupUnlockScreenTest.kt | 122 ++++++++++++++++++ .../accountsetup/SetupUnlockViewModelTest.kt | 100 +++++++++++++- 5 files changed, 359 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreen.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreen.kt index c3bbba7ee..69d22b1b5 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreen.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreen.kt @@ -42,7 +42,9 @@ import com.x8bit.bitwarden.ui.platform.base.util.EventsEffect import com.x8bit.bitwarden.ui.platform.components.appbar.BitwardenTopAppBar import com.x8bit.bitwarden.ui.platform.components.button.BitwardenFilledButton import com.x8bit.bitwarden.ui.platform.components.button.BitwardenTextButton +import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenLoadingDialog import com.x8bit.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog +import com.x8bit.bitwarden.ui.platform.components.dialog.LoadingDialogState import com.x8bit.bitwarden.ui.platform.components.scaffold.BitwardenScaffold import com.x8bit.bitwarden.ui.platform.components.toggle.BitwardenUnlockWithBiometricsSwitch import com.x8bit.bitwarden.ui.platform.components.toggle.BitwardenUnlockWithPinSwitch @@ -63,12 +65,28 @@ fun SetupUnlockScreen( ) { val state by viewModel.stateFlow.collectAsStateWithLifecycle() val handler = remember(viewModel) { SetupUnlockHandler.create(viewModel = viewModel) } + var showBiometricsPrompt by rememberSaveable { mutableStateOf(value = false) } EventsEffect(viewModel = viewModel) { event -> when (event) { SetupUnlockEvent.NavigateToSetupAutofill -> onNavigateToSetupAutofill() + is SetupUnlockEvent.ShowBiometricsPrompt -> { + showBiometricsPrompt = true + biometricsManager.promptBiometrics( + onSuccess = { + handler.unlockWithBiometricToggle() + showBiometricsPrompt = false + }, + onCancel = { showBiometricsPrompt = false }, + onLockOut = { showBiometricsPrompt = false }, + onError = { showBiometricsPrompt = false }, + cipher = event.cipher, + ) + } } } + SetupUnlockScreenDialogs(dialogState = state.dialogState) + val scrollBehavior = TopAppBarDefaults.pinnedScrollBehavior(rememberTopAppBarState()) BitwardenScaffold( modifier = Modifier @@ -84,6 +102,7 @@ fun SetupUnlockScreen( ) { innerPadding -> SetupUnlockScreenContent( state = state, + showBiometricsPrompt = showBiometricsPrompt, handler = handler, biometricsManager = biometricsManager, modifier = Modifier @@ -96,6 +115,7 @@ fun SetupUnlockScreen( @Composable private fun SetupUnlockScreenContent( state: SetupUnlockState, + showBiometricsPrompt: Boolean, handler: SetupUnlockHandler, modifier: Modifier = Modifier, biometricsManager: BiometricsManager, @@ -116,7 +136,7 @@ private fun SetupUnlockScreenContent( Spacer(modifier = Modifier.height(height = 24.dp)) BitwardenUnlockWithBiometricsSwitch( isBiometricsSupported = biometricsManager.isBiometricsSupported, - isChecked = state.isUnlockWithBiometricsEnabled, + isChecked = state.isUnlockWithBiometricsEnabled || showBiometricsPrompt, onDisableBiometrics = handler.onDisableBiometrics, onEnableBiometrics = handler.onEnableBiometrics, modifier = Modifier @@ -270,3 +290,16 @@ private fun SetupUnlockHeaderLandscape( } } } + +@Composable +private fun SetupUnlockScreenDialogs( + dialogState: SetupUnlockState.DialogState?, +) { + when (dialogState) { + is SetupUnlockState.DialogState.Loading -> BitwardenLoadingDialog( + visibilityState = LoadingDialogState.Shown(text = dialogState.title), + ) + + null -> Unit + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt index 6efb6f790..0dda1033d 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModel.kt @@ -2,13 +2,21 @@ package com.x8bit.bitwarden.ui.auth.feature.accountsetup import android.os.Parcelable import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.viewModelScope +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository +import com.x8bit.bitwarden.data.platform.repository.model.BiometricsKeyResult import com.x8bit.bitwarden.ui.platform.base.BaseViewModel +import com.x8bit.bitwarden.ui.platform.base.util.Text +import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.components.toggle.UnlockWithPinState import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize +import javax.crypto.Cipher import javax.inject.Inject private const val KEY_STATE = "state" @@ -30,6 +38,7 @@ class SetupUnlockViewModel @Inject constructor( cipher = biometricsEncryptionManager.getOrCreateCipher(userId = userId), ) SetupUnlockState( + userId = userId, isUnlockWithPasswordEnabled = authRepository .userStateFlow .value @@ -38,6 +47,7 @@ class SetupUnlockViewModel @Inject constructor( isUnlockWithPinEnabled = settingsRepository.isUnlockWithPinEnabled, isUnlockWithBiometricsEnabled = settingsRepository.isUnlockWithBiometricsEnabled && isBiometricsValid, + dialogState = null, ) }, ) { @@ -51,6 +61,7 @@ class SetupUnlockViewModel @Inject constructor( } is SetupUnlockAction.UnlockWithPinToggle -> handleUnlockWithPinToggle(action) + is SetupUnlockAction.Internal -> handleInternalActions(action) } } @@ -59,7 +70,12 @@ class SetupUnlockViewModel @Inject constructor( } private fun handleEnableBiometricsClick() { - // TODO: Handle biometric unlocking logic PM-10624 + sendEvent( + SetupUnlockEvent.ShowBiometricsPrompt( + // Generate a new key in case the previous one was invalidated + cipher = biometricsEncryptionManager.createCipher(userId = state.userId), + ), + ) } private fun handleSetUpLaterClick() { @@ -69,12 +85,58 @@ class SetupUnlockViewModel @Inject constructor( private fun handleUnlockWithBiometricToggle( action: SetupUnlockAction.UnlockWithBiometricToggle, ) { - // TODO: Handle biometric unlocking logic PM-10624 + if (action.isEnabled) { + mutableStateFlow.update { + it.copy( + dialogState = SetupUnlockState.DialogState.Loading(R.string.saving.asText()), + isUnlockWithBiometricsEnabled = true, + ) + } + viewModelScope.launch { + val result = settingsRepository.setupBiometricsKey() + sendAction(SetupUnlockAction.Internal.BiometricsKeyResultReceive(result)) + } + } else { + settingsRepository.clearBiometricsKey() + mutableStateFlow.update { it.copy(isUnlockWithBiometricsEnabled = false) } + } } private fun handleUnlockWithPinToggle(action: SetupUnlockAction.UnlockWithPinToggle) { // TODO: Handle pin unlocking logic PM-10628 } + + private fun handleInternalActions(action: SetupUnlockAction.Internal) { + when (action) { + is SetupUnlockAction.Internal.BiometricsKeyResultReceive -> { + handleBiometricsKeyResultReceive(action) + } + } + } + + private fun handleBiometricsKeyResultReceive( + action: SetupUnlockAction.Internal.BiometricsKeyResultReceive, + ) { + when (action.result) { + BiometricsKeyResult.Error -> { + mutableStateFlow.update { + it.copy( + dialogState = null, + isUnlockWithBiometricsEnabled = false, + ) + } + } + + BiometricsKeyResult.Success -> { + mutableStateFlow.update { + it.copy( + dialogState = null, + isUnlockWithBiometricsEnabled = true, + ) + } + } + } + } } /** @@ -82,15 +144,30 @@ class SetupUnlockViewModel @Inject constructor( */ @Parcelize data class SetupUnlockState( + val userId: String, val isUnlockWithPasswordEnabled: Boolean, val isUnlockWithPinEnabled: Boolean, val isUnlockWithBiometricsEnabled: Boolean, + val dialogState: DialogState?, ) : Parcelable { /** * Indicates whether the continue button should be enabled or disabled. */ val isContinueButtonEnabled: Boolean get() = isUnlockWithBiometricsEnabled || isUnlockWithPinEnabled + + /** + * Represents the dialog UI state for the setup unlock screen. + */ + sealed class DialogState : Parcelable { + /** + * Displays a loading dialog with a title. + */ + @Parcelize + data class Loading( + val title: Text, + ) : DialogState() + } } /** @@ -101,6 +178,13 @@ sealed class SetupUnlockEvent { * Navigate to autofill setup. */ data object NavigateToSetupAutofill : SetupUnlockEvent() + + /** + * Shows the prompt for biometrics using with the given [cipher]. + */ + data class ShowBiometricsPrompt( + val cipher: Cipher, + ) : SetupUnlockEvent() } /** @@ -135,4 +219,16 @@ sealed class SetupUnlockAction { * The user clicked the set up later button. */ data object SetUpLaterClick : SetupUnlockAction() + + /** + * Models actions that can be sent by the view model itself. + */ + sealed class Internal : SetupUnlockAction() { + /** + * A biometrics key result has been received. + */ + data class BiometricsKeyResultReceive( + val result: BiometricsKeyResult, + ) : Internal() + } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/handlers/SetupUnlockHandler.kt b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/handlers/SetupUnlockHandler.kt index 601f53799..4086ca650 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/handlers/SetupUnlockHandler.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/handlers/SetupUnlockHandler.kt @@ -14,6 +14,7 @@ data class SetupUnlockHandler( val onUnlockWithPinToggle: (UnlockWithPinState) -> Unit, val onContinueClick: () -> Unit, val onSetUpLaterClick: () -> Unit, + val unlockWithBiometricToggle: () -> Unit, ) { companion object { /** @@ -35,6 +36,11 @@ data class SetupUnlockHandler( }, onContinueClick = { viewModel.trySendAction(SetupUnlockAction.ContinueClick) }, onSetUpLaterClick = { viewModel.trySendAction(SetupUnlockAction.SetUpLaterClick) }, + unlockWithBiometricToggle = { + viewModel.trySendAction( + SetupUnlockAction.UnlockWithBiometricToggle(isEnabled = true), + ) + }, ) } } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreenTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreenTest.kt index f327be8ba..4476930b4 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreenTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockScreenTest.kt @@ -14,6 +14,7 @@ import androidx.compose.ui.test.performScrollTo import androidx.compose.ui.test.performTextInput import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest +import com.x8bit.bitwarden.ui.platform.base.util.asText import com.x8bit.bitwarden.ui.platform.components.toggle.UnlockWithPinState import com.x8bit.bitwarden.ui.platform.manager.biometrics.BiometricsManager import com.x8bit.bitwarden.ui.util.assertNoDialogExists @@ -21,6 +22,7 @@ import io.mockk.every import io.mockk.just import io.mockk.mockk import io.mockk.runs +import io.mockk.slot import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.update @@ -28,13 +30,27 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.robolectric.annotation.Config +import javax.crypto.Cipher class SetupUnlockScreenTest : BaseComposeTest() { private var onNavigateToSetupAutofillCalled = false + private val captureBiometricsSuccess = slot<(cipher: Cipher?) -> Unit>() + private val captureBiometricsCancel = slot<() -> Unit>() + private val captureBiometricsLockOut = slot<() -> Unit>() + private val captureBiometricsError = slot<() -> Unit>() private val biometricsManager: BiometricsManager = mockk { every { isBiometricsSupported } returns true + every { + promptBiometrics( + onSuccess = capture(captureBiometricsSuccess), + onCancel = capture(captureBiometricsCancel), + onLockOut = capture(captureBiometricsLockOut), + onError = capture(captureBiometricsError), + cipher = CIPHER, + ) + } just runs } private val mutableStateFlow = MutableStateFlow(DEFAULT_STATE) @@ -114,6 +130,90 @@ class SetupUnlockScreenTest : BaseComposeTest() { } } + @Test + fun `on unlock with biometrics toggle should un-toggle on cancel`() { + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOff() + mutableEventFlow.tryEmit(SetupUnlockEvent.ShowBiometricsPrompt(cipher = CIPHER)) + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOn() + captureBiometricsCancel.captured() + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOff() + verify(exactly = 0) { + viewModel.trySendAction(any()) + } + } + + @Test + fun `on unlock with biometrics toggle should un-toggle on error`() { + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOff() + mutableEventFlow.tryEmit(SetupUnlockEvent.ShowBiometricsPrompt(cipher = CIPHER)) + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOn() + captureBiometricsError.captured() + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOff() + verify(exactly = 0) { + viewModel.trySendAction(any()) + } + } + + @Test + fun `on unlock with biometrics toggle should un-toggle on lock out`() { + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOff() + mutableEventFlow.tryEmit(SetupUnlockEvent.ShowBiometricsPrompt(cipher = CIPHER)) + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOn() + captureBiometricsLockOut.captured() + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOff() + verify(exactly = 0) { + viewModel.trySendAction(any()) + } + } + + @Test + fun `on unlock with biometrics toggle should send UnlockWithBiometricToggle on success`() { + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOff() + mutableEventFlow.tryEmit(SetupUnlockEvent.ShowBiometricsPrompt(cipher = CIPHER)) + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOn() + captureBiometricsSuccess.captured(CIPHER) + composeTestRule + .onNodeWithText(text = "Unlock with Biometrics") + .performScrollTo() + .assertIsOff() + verify(exactly = 1) { + viewModel.trySendAction(SetupUnlockAction.UnlockWithBiometricToggle(isEnabled = true)) + } + } + @Test fun `on unlock with pin code should be toggled on or off according to state`() { composeTestRule.onNodeWithText(text = "Unlock with PIN code").assertIsOff() @@ -469,10 +569,32 @@ class SetupUnlockScreenTest : BaseComposeTest() { viewModel.trySendAction(SetupUnlockAction.SetUpLaterClick) } } + + @Test + fun `Loading Dialog should be displayed according to state`() { + val title = "title" + composeTestRule.assertNoDialogExists() + + mutableStateFlow.update { + it.copy(dialogState = SetupUnlockState.DialogState.Loading(title = title.asText())) + } + composeTestRule + .onAllNodesWithText(text = title) + .filterToOne(hasAnyAncestor(isDialog())) + .assertIsDisplayed() + + mutableStateFlow.update { it.copy(dialogState = null) } + composeTestRule.assertNoDialogExists() + } } +private const val DEFAULT_USER_ID: String = "user_id" private val DEFAULT_STATE: SetupUnlockState = SetupUnlockState( + userId = DEFAULT_USER_ID, isUnlockWithPinEnabled = false, isUnlockWithPasswordEnabled = true, isUnlockWithBiometricsEnabled = false, + dialogState = null, ) + +private val CIPHER = mockk() diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt index a9dfd10b3..fbab1a0eb 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/auth/feature/accountsetup/SetupUnlockViewModelTest.kt @@ -2,14 +2,22 @@ package com.x8bit.bitwarden.ui.auth.feature.accountsetup import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test +import com.x8bit.bitwarden.R import com.x8bit.bitwarden.data.auth.repository.AuthRepository import com.x8bit.bitwarden.data.auth.repository.model.UserState import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository +import com.x8bit.bitwarden.data.platform.repository.model.BiometricsKeyResult import com.x8bit.bitwarden.data.platform.repository.model.Environment import com.x8bit.bitwarden.ui.platform.base.BaseViewModelTest +import com.x8bit.bitwarden.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.runs +import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions.assertEquals @@ -57,6 +65,94 @@ class SetupUnlockViewModelTest : BaseViewModelTest() { } } + @Test + fun `on UnlockWithBiometricToggle false should call clearBiometricsKey and update the state`() = + runTest { + val initialState = DEFAULT_STATE.copy(isUnlockWithBiometricsEnabled = true) + every { settingsRepository.isUnlockWithBiometricsEnabled } returns true + every { settingsRepository.clearBiometricsKey() } just runs + val viewModel = createViewModel(initialState) + assertEquals(initialState, viewModel.stateFlow.value) + + viewModel.trySendAction(SetupUnlockAction.UnlockWithBiometricToggle(isEnabled = false)) + + assertEquals( + initialState.copy(isUnlockWithBiometricsEnabled = false), + viewModel.stateFlow.value, + ) + verify(exactly = 1) { + settingsRepository.clearBiometricsKey() + } + } + + @Suppress("MaxLineLength") + @Test + fun `on UnlockWithBiometricToggle true and setupBiometricsKey error should update the state accordingly`() = + runTest { + coEvery { settingsRepository.setupBiometricsKey() } returns BiometricsKeyResult.Error + val viewModel = createViewModel() + + viewModel.stateFlow.test { + assertEquals(DEFAULT_STATE, awaitItem()) + viewModel.trySendAction( + SetupUnlockAction.UnlockWithBiometricToggle(isEnabled = true), + ) + assertEquals( + DEFAULT_STATE.copy( + dialogState = SetupUnlockState.DialogState.Loading( + title = R.string.saving.asText(), + ), + isUnlockWithBiometricsEnabled = true, + ), + awaitItem(), + ) + assertEquals( + DEFAULT_STATE.copy( + dialogState = null, + isUnlockWithBiometricsEnabled = false, + ), + awaitItem(), + ) + } + coVerify(exactly = 1) { + settingsRepository.setupBiometricsKey() + } + } + + @Suppress("MaxLineLength") + @Test + fun `on UnlockWithBiometricToggle true and setupBiometricsKey success should call update the state accordingly`() = + runTest { + coEvery { settingsRepository.setupBiometricsKey() } returns BiometricsKeyResult.Success + val viewModel = createViewModel() + + viewModel.stateFlow.test { + assertEquals(DEFAULT_STATE, awaitItem()) + viewModel.trySendAction( + SetupUnlockAction.UnlockWithBiometricToggle(isEnabled = true), + ) + assertEquals( + DEFAULT_STATE.copy( + dialogState = SetupUnlockState.DialogState.Loading( + title = R.string.saving.asText(), + ), + isUnlockWithBiometricsEnabled = true, + ), + awaitItem(), + ) + assertEquals( + DEFAULT_STATE.copy( + dialogState = null, + isUnlockWithBiometricsEnabled = true, + ), + awaitItem(), + ) + } + coVerify(exactly = 1) { + settingsRepository.setupBiometricsKey() + } + } + private fun createViewModel( state: SetupUnlockState? = null, ): SetupUnlockViewModel = @@ -68,14 +164,16 @@ class SetupUnlockViewModelTest : BaseViewModelTest() { ) } +private const val DEFAULT_USER_ID: String = "activeUserId" private val DEFAULT_STATE: SetupUnlockState = SetupUnlockState( + userId = DEFAULT_USER_ID, isUnlockWithPinEnabled = false, isUnlockWithPasswordEnabled = true, isUnlockWithBiometricsEnabled = false, + dialogState = null, ) private val CIPHER = mockk() -private const val DEFAULT_USER_ID: String = "activeUserId" private val DEFAULT_USER_STATE: UserState = UserState( activeUserId = DEFAULT_USER_ID, accounts = listOf(