From aecf460c96b3a56603c6410ccd6f522c521ae998 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Wed, 10 Aug 2022 14:00:36 +0200 Subject: [PATCH] Improve tests for lockscreen (#6796) * Improve tests * Address review comments. * Refactor pin code tests and code to improve testability. * Fix lint issues --- .../pin/lockscreen/ui/LockScreenViewModel.kt | 19 +- .../pin/lockscreen/ui/LockScreenViewState.kt | 4 +- .../fragment/LockScreenViewModelTests.kt | 218 +++++++++++------- .../java/im/vector/app/test/Extensions.kt | 5 + .../im/vector/app/test/FlowTestObserver.kt | 10 +- 5 files changed, 155 insertions(+), 101 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewModel.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewModel.kt index d40f67ea35..33ea590f1d 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewModel.kt @@ -64,8 +64,6 @@ class LockScreenViewModel @AssistedInject constructor( private val biometricHelper = biometricHelperFactory.create(initialState.lockScreenConfiguration) - private var firstEnteredCode: String? = null - // BiometricPrompt will automatically disable system auth after too many failed auth attempts private var isSystemAuthTemporarilyDisabledByBiometricPrompt = false @@ -108,18 +106,17 @@ class LockScreenViewModel @AssistedInject constructor( val state = awaitState() when (state.lockScreenConfiguration.mode) { LockScreenMode.CREATE -> { - if (firstEnteredCode == null && state.lockScreenConfiguration.needsNewCodeValidation) { - firstEnteredCode = code - _viewEvents.post(LockScreenViewEvent.ClearPinCode(false)) - emit(PinCodeState.FirstCodeEntered) + val enteredPinCode = (state.pinCodeState as? PinCodeState.FirstCodeEntered)?.pinCode + if (enteredPinCode == null && state.lockScreenConfiguration.needsNewCodeValidation) { + _viewEvents.post(LockScreenViewEvent.ClearPinCode(confirmationFailed = false)) + emit(PinCodeState.FirstCodeEntered(code)) } else { - if (!state.lockScreenConfiguration.needsNewCodeValidation || code == firstEnteredCode) { + if (!state.lockScreenConfiguration.needsNewCodeValidation || code == enteredPinCode) { pinCodeHelper.createPinCode(code) _viewEvents.post(LockScreenViewEvent.CodeCreationComplete) emit(null) } else { - firstEnteredCode = null - _viewEvents.post(LockScreenViewEvent.ClearPinCode(true)) + _viewEvents.post(LockScreenViewEvent.ClearPinCode(confirmationFailed = true)) emit(PinCodeState.Idle) } } @@ -137,7 +134,9 @@ class LockScreenViewModel @AssistedInject constructor( }.catch { error -> _viewEvents.post(LockScreenViewEvent.AuthError(AuthMethod.PIN_CODE, error)) }.onEach { newPinState -> - newPinState?.let { setState { copy(pinCodeState = it) } } + if (newPinState != null) { + setState { copy(pinCodeState = newPinState) } + } }.launchIn(viewModelScope) @SuppressLint("NewApi") diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewState.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewState.kt index f689e1faf1..c6c6359f4f 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewState.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewState.kt @@ -27,11 +27,11 @@ data class LockScreenViewState( val isBiometricKeyInvalidated: Boolean, ) : MavericksState { constructor(lockScreenConfiguration: LockScreenConfiguration) : this( - lockScreenConfiguration, false, false, PinCodeState.Idle, false + lockScreenConfiguration, false, false, PinCodeState.Idle, false, ) } sealed class PinCodeState { object Idle : PinCodeState() - object FirstCodeEntered : PinCodeState() + data class FirstCodeEntered(val pinCode: String) : PinCodeState() } diff --git a/vector/src/test/java/im/vector/app/features/pin/lockscreen/fragment/LockScreenViewModelTests.kt b/vector/src/test/java/im/vector/app/features/pin/lockscreen/fragment/LockScreenViewModelTests.kt index 18dfdf9145..6037d9933e 100644 --- a/vector/src/test/java/im/vector/app/features/pin/lockscreen/fragment/LockScreenViewModelTests.kt +++ b/vector/src/test/java/im/vector/app/features/pin/lockscreen/fragment/LockScreenViewModelTests.kt @@ -19,9 +19,11 @@ package im.vector.app.features.pin.lockscreen.fragment import android.app.KeyguardManager import android.os.Build import android.security.keystore.KeyPermanentlyInvalidatedException +import androidx.biometric.BiometricPrompt import androidx.fragment.app.FragmentActivity import com.airbnb.mvrx.test.MvRxTestRule import com.airbnb.mvrx.withState +import im.vector.app.features.pin.lockscreen.biometrics.BiometricAuthError import im.vector.app.features.pin.lockscreen.biometrics.BiometricHelper import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode @@ -42,10 +44,8 @@ import io.mockk.every import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runTest -import org.amshove.kluent.shouldBeEqualTo -import org.amshove.kluent.shouldBeFalse +import org.amshove.kluent.shouldBeTrue import org.amshove.kluent.shouldNotBeEqualTo import org.junit.Before import org.junit.Rule @@ -76,138 +76,141 @@ class LockScreenViewModelTests { @Test fun `init migrates old keys to new ones if needed`() { + // given val initialState = createViewState() + // when LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) - + // then coVerify { keysMigrator.migrateIfNeeded() } } @Test fun `init updates the initial state with biometric info`() = runTest { + // given every { biometricHelper.isSystemAuthEnabledAndValid } returns true val initialState = createViewState() + // when val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) - advanceUntilIdle() - val newState = viewModel.awaitState() + // then + val newState = viewModel.awaitState() // Can't use viewModel.test() here since we want to record events emitted on init newState shouldNotBeEqualTo initialState } @Test fun `Updating the initial state with biometric info waits until device is unlocked on Android 12+`() = runTest { + // given val initialState = createViewState() versionProvider.value = Build.VERSION_CODES.S + // when LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) - advanceUntilIdle() + // then verify { keyguardManager.isDeviceLocked } } @Test fun `when ViewModel is instantiated initialState is updated with biometric info`() { + // given + givenShowBiometricPromptAutomatically() val initialState = createViewState() - // This should set canUseBiometricAuth to true - every { biometricHelper.isSystemAuthEnabledAndValid } returns true + // when val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) - val newState = withState(viewModel) { it } - initialState shouldNotBeEqualTo newState + // then + withState(viewModel) { newState -> + initialState shouldNotBeEqualTo newState + } } @Test - fun `when onPinCodeEntered is called in VERIFY mode, the code is verified and the result is emitted as a ViewEvent`() = runTest { + fun `when onPinCodeEntered is called in VERIFY mode and verification is successful, code is verified and result is emitted as a ViewEvent`() = runTest { + // given val initialState = createViewState() val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) coEvery { pinCodeHelper.verifyPinCode(any()) } returns true - - val events = viewModel.test().viewEvents - events.assertNoValues() - - val stateBefore = viewModel.awaitState() - + val test = viewModel.test() + // when viewModel.handle(LockScreenAction.PinCodeEntered("1234")) + // then coVerify { pinCodeHelper.verifyPinCode(any()) } - events.assertValues(LockScreenViewEvent.AuthSuccessful(AuthMethod.PIN_CODE)) + test.assertEvents(LockScreenViewEvent.AuthSuccessful(AuthMethod.PIN_CODE)) + test.assertStates(initialState) + } + @Test + fun `when onPinCodeEntered is called in VERIFY mode and verification fails, the error result is emitted as a ViewEvent`() = runTest { + // given + val initialState = createViewState() + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) coEvery { pinCodeHelper.verifyPinCode(any()) } returns false + val test = viewModel.test() + // when viewModel.handle(LockScreenAction.PinCodeEntered("1234")) - events.assertValues(LockScreenViewEvent.AuthSuccessful(AuthMethod.PIN_CODE), LockScreenViewEvent.AuthFailure(AuthMethod.PIN_CODE)) - - val stateAfter = viewModel.awaitState() - stateBefore shouldBeEqualTo stateAfter + // then + coVerify { pinCodeHelper.verifyPinCode(any()) } + test.assertEvents(LockScreenViewEvent.AuthFailure(AuthMethod.PIN_CODE)) + test.assertStates(initialState) } @Test fun `when onPinCodeEntered is called in CREATE mode with no confirmation needed it creates the pin code`() = runTest { + // given val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = false) val initialState = createViewState(lockScreenConfiguration = configuration) val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) - - val events = viewModel.test().viewEvents - events.assertNoValues() - + val test = viewModel.test() + // when viewModel.handle(LockScreenAction.PinCodeEntered("1234")) + // then coVerify { pinCodeHelper.createPinCode(any()) } - - events.assertValues(LockScreenViewEvent.CodeCreationComplete) + test.assertEvents(LockScreenViewEvent.CodeCreationComplete) } @Test fun `when onPinCodeEntered is called twice in CREATE mode with confirmation needed it verifies and creates the pin code`() = runTest { + // given + val pinCode = "1234" val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = true) - val initialState = createViewState(lockScreenConfiguration = configuration) + val initialState = createViewState(lockScreenConfiguration = configuration, pinCodeState = PinCodeState.FirstCodeEntered(pinCode)) val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) - - val events = viewModel.test().viewEvents - events.assertNoValues() - - viewModel.handle(LockScreenAction.PinCodeEntered("1234")) - - events.assertValues(LockScreenViewEvent.ClearPinCode(false)) - val pinCodeState = viewModel.awaitState().pinCodeState - pinCodeState shouldBeEqualTo PinCodeState.FirstCodeEntered - - viewModel.handle(LockScreenAction.PinCodeEntered("1234")) - events.assertValues(LockScreenViewEvent.ClearPinCode(false), LockScreenViewEvent.CodeCreationComplete) + val test = viewModel.test() + // when + viewModel.handle(LockScreenAction.PinCodeEntered(pinCode)) + // then + test.assertEvents(LockScreenViewEvent.CodeCreationComplete) + .assertLatestState { (it.pinCodeState as? PinCodeState.FirstCodeEntered)?.pinCode == pinCode } } @Test fun `when onPinCodeEntered is called in CREATE mode with incorrect confirmation it clears the pin code`() = runTest { + // given val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = true) - val initialState = createViewState(lockScreenConfiguration = configuration) + val initialState = createViewState(lockScreenConfiguration = configuration, pinCodeState = PinCodeState.FirstCodeEntered("1234")) val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) - - val events = viewModel.test().viewEvents - events.assertNoValues() - - viewModel.handle(LockScreenAction.PinCodeEntered("1234")) - - events.assertValues(LockScreenViewEvent.ClearPinCode(false)) - val pinCodeState = viewModel.awaitState().pinCodeState - pinCodeState shouldBeEqualTo PinCodeState.FirstCodeEntered - + val test = viewModel.test() + // when viewModel.handle(LockScreenAction.PinCodeEntered("4321")) - events.assertValues(LockScreenViewEvent.ClearPinCode(false), LockScreenViewEvent.ClearPinCode(true)) - val newPinCodeState = viewModel.awaitState().pinCodeState - newPinCodeState shouldBeEqualTo PinCodeState.Idle + // then + test.assertEvents(LockScreenViewEvent.ClearPinCode(true)) + .assertLatestState(initialState.copy(pinCodeState = PinCodeState.Idle)) } @Test fun `onPinCodeEntered handles exceptions`() = runTest { + // given val initialState = createViewState() val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val exception = IllegalStateException("Something went wrong") coEvery { pinCodeHelper.verifyPinCode(any()) } throws exception - - val events = viewModel.test().viewEvents - events.assertNoValues() - + val test = viewModel.test() + // when viewModel.handle(LockScreenAction.PinCodeEntered("1234")) - - events.assertValues(LockScreenViewEvent.AuthError(AuthMethod.PIN_CODE, exception)) + // then + test.assertEvents(LockScreenViewEvent.AuthError(AuthMethod.PIN_CODE, exception)) } @Test fun `when showBiometricPrompt catches a KeyPermanentlyInvalidatedException it disables biometric authentication`() = runTest { + // given versionProvider.value = Build.VERSION_CODES.M - every { biometricHelper.isSystemKeyValid } returns false val exception = KeyPermanentlyInvalidatedException() coEvery { biometricHelper.authenticate(any()) } throws exception @@ -218,49 +221,81 @@ class LockScreenViewModelTests { lockScreenConfiguration = configuration ) val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) - - val events = viewModel.test().viewEvents - events.assertNoValues() - + val test = viewModel.test() + // when viewModel.handle(LockScreenAction.ShowBiometricPrompt(mockk())) - - events.assertValues(LockScreenViewEvent.ShowBiometricKeyInvalidatedMessage) + // then + test.assertEvents(LockScreenViewEvent.ShowBiometricKeyInvalidatedMessage) + // Biometric key is invalidated so biometric auth is disabled + .assertLatestState { !it.canUseBiometricAuth } verify { biometricHelper.disableAuthentication() } - - // System key was deleted, biometric auth should be disabled - every { biometricHelper.isSystemAuthEnabledAndValid } returns false - val newState = viewModel.awaitState() - newState.canUseBiometricAuth.shouldBeFalse() } @Test fun `when showBiometricPrompt receives an event it propagates it as a ViewEvent`() = runTest { + // given val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) coEvery { biometricHelper.authenticate(any()) } returns flowOf(false, true) - - val events = viewModel.test().viewEvents - events.assertNoValues() - + val test = viewModel.test() + // when viewModel.handle(LockScreenAction.ShowBiometricPrompt(mockk())) - - events.assertValues(LockScreenViewEvent.AuthFailure(AuthMethod.BIOMETRICS), LockScreenViewEvent.AuthSuccessful(AuthMethod.BIOMETRICS)) + // then + test.assertEvents(LockScreenViewEvent.AuthFailure(AuthMethod.BIOMETRICS), LockScreenViewEvent.AuthSuccessful(AuthMethod.BIOMETRICS)) } @Test fun `showBiometricPrompt handles exceptions`() = runTest { + // given val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val exception = IllegalStateException("Something went wrong") coEvery { biometricHelper.authenticate(any()) } throws exception - - val events = viewModel.test().viewEvents - events.assertNoValues() - + val test = viewModel.test() + // when viewModel.handle(LockScreenAction.ShowBiometricPrompt(mockk())) - - events.assertValues(LockScreenViewEvent.AuthError(AuthMethod.BIOMETRICS, exception)) + // then + test.assertEvents(LockScreenViewEvent.AuthError(AuthMethod.BIOMETRICS, exception)) } - private fun createViewState( + @Test + fun `when showBiometricPrompt handles isAuthDisabledError, canUseBiometricAuth becomes false`() = runTest { + // given + val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) + val exception = BiometricAuthError(BiometricPrompt.ERROR_LOCKOUT_PERMANENT, "Permanent lockout") + coEvery { biometricHelper.authenticate(any()) } throws exception + val test = viewModel.test() + // when + viewModel.handle(LockScreenAction.ShowBiometricPrompt(mockk())) + // then + exception.isAuthDisabledError.shouldBeTrue() + test.assertEvents(LockScreenViewEvent.AuthError(AuthMethod.BIOMETRICS, exception)) + .assertLatestState { !it.canUseBiometricAuth } + } + + @Test + fun `when OnUIReady action is received and showBiometricPromptAutomatically is true it shows prompt`() = runTest { + // given + givenShowBiometricPromptAutomatically() + val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) + val test = viewModel.test() + // when + viewModel.handle(LockScreenAction.OnUIReady) + // then + test.assertEvents(LockScreenViewEvent.ShowBiometricPromptAutomatically) + } + + @Test + fun `when OnUIReady action is received and isBiometricKeyInvalidated is true it shows prompt`() = runTest { + // given + givenBiometricKeyIsInvalidated() + val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) + val test = viewModel.test() + // when + viewModel.handle(LockScreenAction.OnUIReady) + // then + test.assertEvents(LockScreenViewEvent.ShowBiometricKeyInvalidatedMessage) + } + + private fun createViewState( lockScreenConfiguration: LockScreenConfiguration = createDefaultConfiguration(), canUseBiometricAuth: Boolean = false, showBiometricPromptAutomatically: Boolean = false, @@ -286,4 +321,13 @@ class LockScreenViewModelTests { isDeviceCredentialUnlockEnabled, needsNewCodeValidation ).let(otherChanges) + + private fun givenBiometricKeyIsInvalidated() { + every { biometricHelper.hasSystemKey } returns true + every { biometricHelper.isSystemKeyValid } returns false + } + + private fun givenShowBiometricPromptAutomatically() { + every { biometricHelper.isSystemAuthEnabledAndValid } returns true + } } diff --git a/vector/src/test/java/im/vector/app/test/Extensions.kt b/vector/src/test/java/im/vector/app/test/Extensions.kt index 5ac17cc5ff..2fbab3b71b 100644 --- a/vector/src/test/java/im/vector/app/test/Extensions.kt +++ b/vector/src/test/java/im/vector/app/test/Extensions.kt @@ -91,6 +91,11 @@ class ViewModelTest( return this } + fun assertLatestState(predicate: (S) -> Boolean): ViewModelTest { + states.assertLatestValue(predicate) + return this + } + fun finish() { states.finish() viewEvents.finish() diff --git a/vector/src/test/java/im/vector/app/test/FlowTestObserver.kt b/vector/src/test/java/im/vector/app/test/FlowTestObserver.kt index db828be232..ce8d27cd2a 100644 --- a/vector/src/test/java/im/vector/app/test/FlowTestObserver.kt +++ b/vector/src/test/java/im/vector/app/test/FlowTestObserver.kt @@ -47,8 +47,14 @@ class FlowTestObserver( return this } - fun assertLatestValue(value: T) { - assertTrue(values.last() == value) + fun assertLatestValue(predicate: (T) -> Boolean): FlowTestObserver { + assertTrue(predicate(values.last())) + return this + } + + fun assertLatestValue(value: T): FlowTestObserver { + assertEquals(value, values.last()) + return this } fun assertValues(values: List): FlowTestObserver {