From dfc8526b474c979fc8a2d5b128f9fe88551e7d6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Tue, 9 Aug 2022 09:54:05 +0200 Subject: [PATCH] Refactor lockscreen implementation. Try to fix issues and simplify flow. --- .../biometrics/BiometricHelperTests.kt | 18 +-- .../crypto/LockScreenKeyRepositoryTests.kt | 4 - .../im/vector/app/features/pin/PinFragment.kt | 39 +++--- .../lockscreen/biometrics/BiometricHelper.kt | 44 +++---- .../configuration/LockScreenConfiguration.kt | 6 +- .../LockScreenConfiguratorProvider.kt | 58 --------- .../pin/lockscreen/di/LockScreenModule.kt | 5 + .../pin/lockscreen/ui/LockScreenFragment.kt | 24 ++-- .../pin/lockscreen/ui/LockScreenViewEvent.kt | 1 + .../pin/lockscreen/ui/LockScreenViewModel.kt | 114 +++++++++--------- .../pin/lockscreen/ui/LockScreenViewState.kt | 6 +- .../settings/VectorSettingsPinFragment.kt | 10 +- .../fragment/LockScreenViewModelTests.kt | 69 ++++++----- 13 files changed, 183 insertions(+), 215 deletions(-) delete mode 100644 vector/src/main/java/im/vector/app/features/pin/lockscreen/configuration/LockScreenConfiguratorProvider.kt diff --git a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelperTests.kt b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelperTests.kt index 53c154ae30..2ec69cf0b1 100644 --- a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelperTests.kt +++ b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelperTests.kt @@ -31,7 +31,6 @@ import androidx.test.filters.SdkSuppress import androidx.test.platform.app.InstrumentationRegistry import im.vector.app.TestBuildVersionSdkIntProvider import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration -import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguratorProvider import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode import im.vector.app.features.pin.lockscreen.crypto.LockScreenCryptoConstants import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeyRepository @@ -54,8 +53,10 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import kotlinx.coroutines.test.runTest +import org.amshove.kluent.coInvoking import org.amshove.kluent.shouldBeFalse import org.amshove.kluent.shouldBeTrue +import org.amshove.kluent.shouldThrow import org.junit.Before import org.junit.Ignore import org.junit.Test @@ -239,36 +240,35 @@ class BiometricHelperTests { @Test @SdkSuppress(minSdkVersion = Build.VERSION_CODES.R) // Due to some issues with mockk and CryptoObject initialization - fun authenticateCreatesSystemKeyIfNeededOnSuccessOnAndroidM() = runTest { + fun enableAuthenticationDeletesSystemKeyOnFailure() = runTest { buildVersionSdkIntProvider.value = Build.VERSION_CODES.M - every { lockScreenKeyRepository.isSystemKeyValid() } returns true val mockAuthChannel = Channel(capacity = 1) val biometricUtils = spyk(createBiometricHelper(createDefaultConfiguration(isBiometricsEnabled = true))) { every { createAuthChannel() } returns mockAuthChannel every { authenticateWithPromptInternal(any(), any(), any()) } returns mockk() } + every { lockScreenKeyRepository.deleteSystemKey() } returns Unit val latch = CountDownLatch(1) val intent = Intent(InstrumentationRegistry.getInstrumentation().targetContext, LockScreenTestActivity::class.java) ActivityScenario.launch(intent).onActivity { activity -> activity.lifecycleScope.launch { + val exception = IllegalStateException("Some error") launch { - mockAuthChannel.send(true) - mockAuthChannel.close() + mockAuthChannel.close(exception) } - biometricUtils.authenticate(activity).collect() + coInvoking { biometricUtils.enableAuthentication(activity).collect() } shouldThrow exception latch.countDown() } } latch.await(1, TimeUnit.SECONDS) - verify { lockScreenKeyRepository.ensureSystemKey() } + verify { lockScreenKeyRepository.deleteSystemKey() } } private fun createBiometricHelper(configuration: LockScreenConfiguration): BiometricHelper { val context = InstrumentationRegistry.getInstrumentation().targetContext - val configProvider = LockScreenConfiguratorProvider(configuration) - return BiometricHelper(context, lockScreenKeyRepository, configProvider, biometricManager, buildVersionSdkIntProvider) + return BiometricHelper(configuration, context, lockScreenKeyRepository, biometricManager, buildVersionSdkIntProvider) } private fun createDefaultConfiguration( diff --git a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepositoryTests.kt b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepositoryTests.kt index 924dbfee9e..8d14ca9153 100644 --- a/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepositoryTests.kt +++ b/vector/src/androidTest/java/im/vector/app/features/pin/lockscreen/crypto/LockScreenKeyRepositoryTests.kt @@ -17,8 +17,6 @@ package im.vector.app.features.pin.lockscreen.crypto import androidx.test.platform.app.InstrumentationRegistry -import im.vector.app.features.pin.lockscreen.crypto.migrations.LegacyPinCodeMigrator -import im.vector.app.features.settings.VectorPreferences import io.mockk.clearAllMocks import io.mockk.every import io.mockk.mockk @@ -44,8 +42,6 @@ class LockScreenKeyRepositoryTests { } private lateinit var lockScreenKeyRepository: LockScreenKeyRepository - private val legacyPinCodeMigrator: LegacyPinCodeMigrator = mockk(relaxed = true) - private val vectorPreferences: VectorPreferences = mockk(relaxed = true) private val keyStore: KeyStore by lazy { KeyStore.getInstance(LockScreenCryptoConstants.ANDROID_KEY_STORE).also { it.load(null) } diff --git a/vector/src/main/java/im/vector/app/features/pin/PinFragment.kt b/vector/src/main/java/im/vector/app/features/pin/PinFragment.kt index 69548f24f0..1688452167 100644 --- a/vector/src/main/java/im/vector/app/features/pin/PinFragment.kt +++ b/vector/src/main/java/im/vector/app/features/pin/PinFragment.kt @@ -24,6 +24,7 @@ import android.view.View import android.view.ViewGroup import android.widget.Toast import com.airbnb.mvrx.args +import com.airbnb.mvrx.asMavericksArgs import com.google.android.material.dialog.MaterialAlertDialogBuilder import im.vector.app.R import im.vector.app.core.extensions.replaceFragment @@ -33,7 +34,7 @@ import im.vector.app.databinding.FragmentPinBinding import im.vector.app.features.MainActivity import im.vector.app.features.MainActivityArgs import im.vector.app.features.pin.lockscreen.biometrics.BiometricAuthError -import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguratorProvider +import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode import im.vector.app.features.pin.lockscreen.ui.AuthMethod import im.vector.app.features.pin.lockscreen.ui.LockScreenFragment @@ -51,7 +52,7 @@ data class PinArgs( class PinFragment @Inject constructor( private val pinCodeStore: PinCodeStore, private val vectorPreferences: VectorPreferences, - private val configuratorProvider: LockScreenConfiguratorProvider, + private val defaultConfiguration: LockScreenConfiguration, ) : VectorBaseFragment() { private val fragmentArgs: PinArgs by args() @@ -81,21 +82,17 @@ class PinFragment @Inject constructor( vectorBaseActivity.finish() } } - - configuratorProvider.updateDefaultConfiguration { - copy( - mode = LockScreenMode.CREATE, - title = getString(R.string.create_pin_title), - needsNewCodeValidation = true, - newCodeConfirmationTitle = getString(R.string.create_pin_confirm_title), - ) - } + createFragment.arguments = defaultConfiguration.copy( + mode = LockScreenMode.CREATE, + title = getString(R.string.create_pin_title), + needsNewCodeValidation = true, + newCodeConfirmationTitle = getString(R.string.create_pin_confirm_title), + ).asMavericksArgs() replaceFragment(R.id.pinFragmentContainer, createFragment) } private fun showAuthFragment() { val authFragment = LockScreenFragment() - val canUseBiometrics = vectorPreferences.useBiometricsToUnlock() authFragment.onLeftButtonClickedListener = View.OnClickListener { displayForgotPinWarningDialog() } authFragment.lockScreenListener = object : LockScreenListener { override fun onAuthenticationFailure(authMethod: AuthMethod) { @@ -133,18 +130,12 @@ class PinFragment @Inject constructor( .show() } } - configuratorProvider.updateDefaultConfiguration { - copy( - mode = LockScreenMode.VERIFY, - title = getString(R.string.auth_pin_title), - isStrongBiometricsEnabled = isStrongBiometricsEnabled && canUseBiometrics, - isWeakBiometricsEnabled = isWeakBiometricsEnabled && canUseBiometrics, - isDeviceCredentialUnlockEnabled = isDeviceCredentialUnlockEnabled && canUseBiometrics, - autoStartBiometric = canUseBiometrics, - leftButtonTitle = getString(R.string.auth_pin_forgot), - clearCodeOnError = true, - ) - } + authFragment.arguments = defaultConfiguration.copy( + mode = LockScreenMode.VERIFY, + title = getString(R.string.auth_pin_title), + leftButtonTitle = getString(R.string.auth_pin_forgot), + clearCodeOnError = true, + ).asMavericksArgs() replaceFragment(R.id.pinFragmentContainer, authFragment) } diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelper.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelper.kt index ae4fa637b4..9bcf6e4264 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelper.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/biometrics/BiometricHelper.kt @@ -31,10 +31,12 @@ import androidx.biometric.BiometricPrompt import androidx.core.content.ContextCompat import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentActivity +import dagger.assisted.Assisted +import dagger.assisted.AssistedFactory +import dagger.assisted.AssistedInject import dagger.hilt.android.qualifiers.ApplicationContext import im.vector.app.R import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration -import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguratorProvider import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeyRepository import im.vector.app.features.pin.lockscreen.ui.fallbackprompt.FallbackBiometricDialogFragment import im.vector.app.features.pin.lockscreen.utils.DevicePromptCheck @@ -54,22 +56,24 @@ import kotlinx.coroutines.launch import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider import java.security.KeyStore import javax.crypto.Cipher -import javax.inject.Inject import kotlin.coroutines.CoroutineContext /** * This is a helper to manage system authentication (biometric and other types) and the system key. */ -class BiometricHelper @Inject constructor( +class BiometricHelper @AssistedInject constructor( + @Assisted private val configuration: LockScreenConfiguration, @ApplicationContext private val context: Context, private val lockScreenKeyRepository: LockScreenKeyRepository, - private val configurationProvider: LockScreenConfiguratorProvider, private val biometricManager: BiometricManager, private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, ) { private var prompt: BiometricPrompt? = null - private val configuration: LockScreenConfiguration get() = configurationProvider.currentConfiguration + @AssistedFactory + interface BiometricHelperFactory { + fun create(configuration: LockScreenConfiguration): BiometricHelper + } /** * Returns true if a weak biometric method (i.e.: some face or iris unlock implementations) can be used. @@ -174,16 +178,18 @@ class BiometricHelper @Inject constructor( when (val exception = result.exceptionOrNull()) { null -> result.getOrNull()?.let { emit(it) } else -> { - // Exception found, stop collecting, throw it and remove the prompt reference + // Exception found: + // 1. Stop collecting. + // 2. Remove the system key if we were creating it. + // 3. Throw the exception and remove the prompt reference + if (!checkSystemKeyExists) { + lockScreenKeyRepository.deleteSystemKey() + } prompt = null throw exception } } } - // Generates the system key on successful authentication - if (buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M) { - lockScreenKeyRepository.ensureSystemKey() - } // Channel is closed, remove prompt reference prompt = null } @@ -213,11 +219,11 @@ class BiometricHelper @Inject constructor( .setAllowedAuthenticators(authenticators) .build() - return BiometricPrompt(activity, executor, callback).also { + return BiometricPrompt(activity, executor, callback).also { prompt -> showFallbackFragmentIfNeeded(activity, channel.receiveAsFlow(), executor.asCoroutineDispatcher()) { // For some reason this seems to be needed unless we want to receive a fragment transaction exception delay(1L) - it.authenticate(promptInfo, cryptoObject) + prompt.authenticate(promptInfo, cryptoObject) } } } @@ -253,11 +259,9 @@ class BiometricHelper @Inject constructor( ): BiometricPrompt.AuthenticationCallback = object : BiometricPrompt.AuthenticationCallback() { private val scope = CoroutineScope(coroutineContext) override fun onAuthenticationError(errorCode: Int, errString: CharSequence) { - scope.launch { - // Error is a terminal event, should close both the Channel and the CoroutineScope to free resources. - channel.close(BiometricAuthError(errorCode, errString.toString())) - scope.cancel() - } + // Error is a terminal event, should close both the Channel and the CoroutineScope to free resources. + channel.close(BiometricAuthError(errorCode, errString.toString())) + scope.cancel() } override fun onAuthenticationFailed() { @@ -274,10 +278,8 @@ class BiometricHelper @Inject constructor( scope.cancel() } } else { - scope.launch { - channel.close(IllegalStateException("System key was not valid after authentication.")) - scope.cancel() - } + channel.close(IllegalStateException("System key was not valid after authentication.")) + scope.cancel() } } diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/configuration/LockScreenConfiguration.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/configuration/LockScreenConfiguration.kt index 8f3e67dfe5..12846c254c 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/configuration/LockScreenConfiguration.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/configuration/LockScreenConfiguration.kt @@ -16,9 +16,13 @@ package im.vector.app.features.pin.lockscreen.configuration +import android.os.Parcelable +import kotlinx.parcelize.Parcelize + /** * Configuration to be used by the lockscreen feature. */ +@Parcelize data class LockScreenConfiguration( /** Which mode should the UI display, [LockScreenMode.VERIFY] or [LockScreenMode.CREATE]. */ val mode: LockScreenMode, @@ -56,4 +60,4 @@ data class LockScreenConfiguration( val biometricSubtitle: String? = null, /** Text for the cancel button of the Biometric prompt dialog. Optional. */ val biometricCancelButtonTitle: String? = null, -) +) : Parcelable diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/configuration/LockScreenConfiguratorProvider.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/configuration/LockScreenConfiguratorProvider.kt deleted file mode 100644 index 338ac66125..0000000000 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/configuration/LockScreenConfiguratorProvider.kt +++ /dev/null @@ -1,58 +0,0 @@ -/* - * Copyright (c) 2022 New Vector Ltd - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package im.vector.app.features.pin.lockscreen.configuration - -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.MutableStateFlow -import javax.inject.Inject -import javax.inject.Singleton - -/** - * Class used to hold both the [defaultConfiguration] and an updated version in [currentConfiguration]. - */ -@Singleton -class LockScreenConfiguratorProvider @Inject constructor( - /** Default [LockScreenConfiguration], any derived configuration created using [updateDefaultConfiguration] will use this as a base. */ - val defaultConfiguration: LockScreenConfiguration, -) { - - private val mutableConfigurationFlow = MutableStateFlow(defaultConfiguration) - - /** - * A [Flow] that emits any changes in configuration. - */ - val configurationFlow: Flow = mutableConfigurationFlow - - /** - * The current configuration to be read and used. - */ - val currentConfiguration get() = mutableConfigurationFlow.value - - /** - * Applies the changes in [block] to the [defaultConfiguration] to generate a new [currentConfiguration]. - */ - fun updateDefaultConfiguration(block: LockScreenConfiguration.() -> LockScreenConfiguration) { - mutableConfigurationFlow.value = defaultConfiguration.block() - } - - /** - * Resets the [currentConfiguration] to the [defaultConfiguration]. - */ - fun reset() { - mutableConfigurationFlow.value = defaultConfiguration - } -} diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenModule.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenModule.kt index fb333b96bb..811a66f3af 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenModule.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/di/LockScreenModule.kt @@ -16,8 +16,10 @@ package im.vector.app.features.pin.lockscreen.di +import android.app.KeyguardManager import android.content.Context import androidx.biometric.BiometricManager +import androidx.core.content.getSystemService import dagger.Binds import dagger.Module import dagger.Provides @@ -83,6 +85,9 @@ object LockScreenModule { SecretStoringUtils(context, keyStore, buildVersionSdkIntProvider), buildVersionSdkIntProvider, ) + + @Provides + fun provideKeyguardManager(context: Context): KeyguardManager = context.getSystemService()!! } @Module diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenFragment.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenFragment.kt index 9eea61ac82..9a618ce939 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenFragment.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenFragment.kt @@ -34,6 +34,10 @@ import im.vector.app.databinding.FragmentLockScreenBinding import im.vector.app.features.pin.lockscreen.configuration.LockScreenConfiguration import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode import im.vector.app.features.pin.lockscreen.views.LockScreenCodeView +import kotlinx.coroutines.flow.distinctUntilChangedBy +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach @AndroidEntryPoint class LockScreenFragment : VectorBaseFragment() { @@ -55,22 +59,12 @@ class LockScreenFragment : VectorBaseFragment() { handleEvent(it) } - withState(viewModel) { state -> - if (state.lockScreenConfiguration.mode == LockScreenMode.CREATE) return@withState - - viewLifecycleOwner.lifecycleScope.launchWhenResumed { - if (state.canUseBiometricAuth && state.isBiometricKeyInvalidated) { - lockScreenListener?.onBiometricKeyInvalidated() - } else if (state.showBiometricPromptAutomatically) { + viewModel.stateFlow.distinctUntilChangedBy { it.showBiometricPromptAutomatically } + .filter { it.showBiometricPromptAutomatically } + .onEach { showBiometricPrompt() } - } - } - } - - override fun onDestroy() { - super.onDestroy() - viewModel.reset() + .launchIn(viewLifecycleOwner.lifecycleScope) } override fun invalidate() = withState(viewModel) { state -> @@ -83,6 +77,7 @@ class LockScreenFragment : VectorBaseFragment() { setupTitleView(views.titleTextView, false, state.lockScreenConfiguration) } } + renderDeleteOrFingerprintButtons(views, views.codeView.enteredDigits) } @@ -123,6 +118,7 @@ class LockScreenFragment : VectorBaseFragment() { is LockScreenViewEvent.AuthSuccessful -> lockScreenListener?.onAuthenticationSuccess(viewEvent.method) is LockScreenViewEvent.AuthFailure -> onAuthFailure(viewEvent.method) is LockScreenViewEvent.AuthError -> onAuthError(viewEvent.method, viewEvent.throwable) + is LockScreenViewEvent.ShowBiometricKeyInvalidatedMessage -> lockScreenListener?.onBiometricKeyInvalidated() } } diff --git a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewEvent.kt b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewEvent.kt index cbde16876c..e340486bc0 100644 --- a/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewEvent.kt +++ b/vector/src/main/java/im/vector/app/features/pin/lockscreen/ui/LockScreenViewEvent.kt @@ -24,4 +24,5 @@ sealed class LockScreenViewEvent : VectorViewEvents { data class AuthSuccessful(val method: AuthMethod) : LockScreenViewEvent() data class AuthFailure(val method: AuthMethod) : LockScreenViewEvent() data class AuthError(val method: AuthMethod, val throwable: Throwable) : LockScreenViewEvent() + object ShowBiometricKeyInvalidatedMessage : LockScreenViewEvent() } 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 79c1967670..417ebdbd93 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 @@ -17,12 +17,12 @@ package im.vector.app.features.pin.lockscreen.ui import android.annotation.SuppressLint +import android.app.KeyguardManager import android.os.Build import android.security.keystore.KeyPermanentlyInvalidatedException +import androidx.annotation.VisibleForTesting import androidx.fragment.app.FragmentActivity import com.airbnb.mvrx.MavericksViewModelFactory -import com.airbnb.mvrx.ViewModelContext -import com.airbnb.mvrx.withState import dagger.assisted.Assisted import dagger.assisted.AssistedFactory import dagger.assisted.AssistedInject @@ -31,26 +31,28 @@ import im.vector.app.core.di.hiltMavericksViewModelFactory import im.vector.app.core.platform.VectorViewModel 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.LockScreenConfiguratorProvider import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeysMigrator import im.vector.app.features.pin.lockscreen.pincode.PinCodeHelper +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.emitAll import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeoutOrNull import org.matrix.android.sdk.api.util.BuildVersionSdkIntProvider +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds class LockScreenViewModel @AssistedInject constructor( @Assisted val initialState: LockScreenViewState, private val pinCodeHelper: PinCodeHelper, - private val biometricHelper: BiometricHelper, + biometricHelperFactory: BiometricHelper.BiometricHelperFactory, private val lockScreenKeysMigrator: LockScreenKeysMigrator, - private val configuratorProvider: LockScreenConfiguratorProvider, - private val buildVersionSdkIntProvider: BuildVersionSdkIntProvider, + private val versionProvider: BuildVersionSdkIntProvider, + private val keyguardManager: KeyguardManager, ) : VectorViewModel(initialState) { @AssistedFactory @@ -58,27 +60,9 @@ class LockScreenViewModel @AssistedInject constructor( override fun create(initialState: LockScreenViewState): LockScreenViewModel } - companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() { + companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() - override fun initialState(viewModelContext: ViewModelContext): LockScreenViewState { - return LockScreenViewState( - lockScreenConfiguration = DUMMY_CONFIGURATION, - canUseBiometricAuth = false, - showBiometricPromptAutomatically = false, - pinCodeState = PinCodeState.Idle, - isBiometricKeyInvalidated = false, - ) - } - - private val DUMMY_CONFIGURATION = LockScreenConfiguration( - mode = LockScreenMode.VERIFY, - pinCodeLength = 4, - isStrongBiometricsEnabled = false, - isDeviceCredentialUnlockEnabled = false, - isWeakBiometricsEnabled = false, - needsNewCodeValidation = false, - ) - } + private val biometricHelper = biometricHelperFactory.create(initialState.lockScreenConfiguration) private var firstEnteredCode: String? = null @@ -86,12 +70,20 @@ class LockScreenViewModel @AssistedInject constructor( private var isSystemAuthTemporarilyDisabledByBiometricPrompt = false init { - // We need this to run synchronously before we start reading the configurations - runBlocking { lockScreenKeysMigrator.migrateIfNeeded() } + viewModelScope.launch { + // Wait until the keyguard is unlocked before performing migrations, it might cause crashes otherwise on Android 12 and 12L + waitUntilKeyguardIsUnlocked() + // Migrate pin code / system keys if needed + lockScreenKeysMigrator.migrateIfNeeded() + // Update initial state with biometric info + updateStateWithBiometricInfo() - configuratorProvider.configurationFlow - .onEach { updateConfiguration(it) } - .launchIn(viewModelScope) + val state = awaitState() + // If when initialized we detect a key invalidation, we should show an error message. + if (state.isBiometricKeyInvalidated) { + onBiometricKeyInvalidated() + } + } } override fun handle(action: LockScreenAction) { @@ -141,13 +133,18 @@ class LockScreenViewModel @AssistedInject constructor( private fun showBiometricPrompt(activity: FragmentActivity) = flow { emitAll(biometricHelper.authenticate(activity)) }.catch { error -> - if (buildVersionSdkIntProvider.get() >= Build.VERSION_CODES.M && error is KeyPermanentlyInvalidatedException) { - removeBiometricAuthentication() - } else if (error is BiometricAuthError && error.isAuthDisabledError) { - isSystemAuthTemporarilyDisabledByBiometricPrompt = true - updateStateWithBiometricInfo() + when { + versionProvider.get() >= Build.VERSION_CODES.M && error is KeyPermanentlyInvalidatedException -> { + onBiometricKeyInvalidated() + } + else -> { + if (error is BiometricAuthError && error.isAuthDisabledError) { + isSystemAuthTemporarilyDisabledByBiometricPrompt = true + updateStateWithBiometricInfo() + } + _viewEvents.post(LockScreenViewEvent.AuthError(AuthMethod.BIOMETRICS, error)) + } } - _viewEvents.post(LockScreenViewEvent.AuthError(AuthMethod.BIOMETRICS, error)) }.onEach { success -> _viewEvents.post( if (success) LockScreenViewEvent.AuthSuccessful(AuthMethod.BIOMETRICS) @@ -155,24 +152,22 @@ class LockScreenViewModel @AssistedInject constructor( ) }.launchIn(viewModelScope) - fun reset() { - configuratorProvider.reset() - } - - private fun removeBiometricAuthentication() { + private suspend fun onBiometricKeyInvalidated() { biometricHelper.disableAuthentication() updateStateWithBiometricInfo() + _viewEvents.post(LockScreenViewEvent.ShowBiometricKeyInvalidatedMessage) } - private fun updateStateWithBiometricInfo() { - val configuration = withState(this) { it.lockScreenConfiguration } - val canUseBiometricAuth = configuration.mode == LockScreenMode.VERIFY && + @SuppressLint("NewApi") + private suspend fun updateStateWithBiometricInfo() { + // This is a terrible hack, but I found no other way to ensure this would be called only after the device is considered unlocked on Android 12+ + waitUntilKeyguardIsUnlocked() + setState { + val isBiometricKeyInvalidated = biometricHelper.hasSystemKey && !biometricHelper.isSystemKeyValid + val canUseBiometricAuth = lockScreenConfiguration.mode == LockScreenMode.VERIFY && !isSystemAuthTemporarilyDisabledByBiometricPrompt && biometricHelper.isSystemAuthEnabledAndValid - val isBiometricKeyInvalidated = biometricHelper.hasSystemKey && !biometricHelper.isSystemKeyValid - val showBiometricPromptAutomatically = canUseBiometricAuth && - configuration.autoStartBiometric - setState { + val showBiometricPromptAutomatically = canUseBiometricAuth && lockScreenConfiguration.autoStartBiometric copy( canUseBiometricAuth = canUseBiometricAuth, showBiometricPromptAutomatically = showBiometricPromptAutomatically, @@ -181,8 +176,19 @@ class LockScreenViewModel @AssistedInject constructor( } } - private fun updateConfiguration(configuration: LockScreenConfiguration) { - setState { copy(lockScreenConfiguration = configuration) } - updateStateWithBiometricInfo() + /** + * Wait until the device is unlocked. There seems to be a behavior change on Android 12 that makes [KeyguardManager.isDeviceLocked] return `false` even + * after an Activity's `onResume` method. If we mix that with the system keys needing the device to be unlocked before they're used, we get crashes. + * See issue [#6768](https://github.com/vector-im/element-android/issues/6768). + */ + @SuppressLint("NewApi") + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal suspend fun waitUntilKeyguardIsUnlocked() { + if (versionProvider.get() < Build.VERSION_CODES.S) return + withTimeoutOrNull(5.seconds) { + while (keyguardManager.isDeviceLocked) { + delay(50.milliseconds) + } + } } } 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 8d2037953b..f689e1faf1 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 @@ -25,7 +25,11 @@ data class LockScreenViewState( val showBiometricPromptAutomatically: Boolean, val pinCodeState: PinCodeState, val isBiometricKeyInvalidated: Boolean, -) : MavericksState +) : MavericksState { + constructor(lockScreenConfiguration: LockScreenConfiguration) : this( + lockScreenConfiguration, false, false, PinCodeState.Idle, false + ) +} sealed class PinCodeState { object Idle : PinCodeState() diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt index 4d95fc362b..db402758f1 100644 --- a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsPinFragment.kt @@ -28,6 +28,8 @@ import im.vector.app.features.notifications.NotificationDrawerManager import im.vector.app.features.pin.PinCodeStore import im.vector.app.features.pin.PinMode 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 import kotlinx.coroutines.flow.collect import kotlinx.coroutines.launch import org.matrix.android.sdk.api.extensions.orFalse @@ -38,12 +40,15 @@ class VectorSettingsPinFragment @Inject constructor( private val pinCodeStore: PinCodeStore, private val navigator: Navigator, private val notificationDrawerManager: NotificationDrawerManager, - private val biometricHelper: BiometricHelper, + biometricHelperFactory: BiometricHelper.BiometricHelperFactory, + defaultLockScreenConfiguration: LockScreenConfiguration, ) : VectorSettingsBaseFragment() { override var titleRes = R.string.settings_security_application_protection_screen_title override val preferenceXmlRes = R.xml.vector_settings_pin + private val biometricHelper = biometricHelperFactory.create(defaultLockScreenConfiguration.copy(mode = LockScreenMode.CREATE)) + private val usePinCodePref by lazy { findPreference(VectorPreferences.SETTINGS_SECURITY_USE_PIN_CODE_FLAG)!! } @@ -102,9 +107,10 @@ class VectorSettingsPinFragment @Inject constructor( }.onFailure { showEnableBiometricErrorMessage() } + updateBiometricPrefState(isPinCodeChecked = usePinCodePref.isChecked) } - false + true } else { disableBiometricAuthentication() true 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 9e80bb490b..18dfdf9145 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 @@ -16,6 +16,7 @@ package im.vector.app.features.pin.lockscreen.fragment +import android.app.KeyguardManager import android.os.Build import android.security.keystore.KeyPermanentlyInvalidatedException import androidx.fragment.app.FragmentActivity @@ -23,7 +24,6 @@ import com.airbnb.mvrx.test.MvRxTestRule import com.airbnb.mvrx.withState 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.LockScreenConfiguratorProvider import im.vector.app.features.pin.lockscreen.configuration.LockScreenMode import im.vector.app.features.pin.lockscreen.crypto.LockScreenKeysMigrator import im.vector.app.features.pin.lockscreen.pincode.PinCodeHelper @@ -42,6 +42,7 @@ 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 @@ -57,7 +58,15 @@ class LockScreenViewModelTests { private val pinCodeHelper = mockk(relaxed = true) private val biometricHelper = mockk(relaxed = true) + private val biometricHelperFactory = object : BiometricHelper.BiometricHelperFactory { + override fun create(configuration: LockScreenConfiguration): BiometricHelper { + return biometricHelper + } + } private val keysMigrator = mockk(relaxed = true) + private val keyguardManager = mockk(relaxed = true) { + every { isDeviceLocked } returns false + } private val versionProvider = TestBuildVersionSdkIntProvider() @Before @@ -68,19 +77,36 @@ class LockScreenViewModelTests { @Test fun `init migrates old keys to new ones if needed`() { val initialState = createViewState() - val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) coVerify { keysMigrator.migrateIfNeeded() } } + @Test + fun `init updates the initial state with biometric info`() = runTest { + every { biometricHelper.isSystemAuthEnabledAndValid } returns true + val initialState = createViewState() + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) + advanceUntilIdle() + val newState = viewModel.awaitState() + newState shouldNotBeEqualTo initialState + } + + @Test + fun `Updating the initial state with biometric info waits until device is unlocked on Android 12+`() = runTest { + val initialState = createViewState() + versionProvider.value = Build.VERSION_CODES.S + LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) + advanceUntilIdle() + verify { keyguardManager.isDeviceLocked } + } + @Test fun `when ViewModel is instantiated initialState is updated with biometric info`() { val initialState = createViewState() - val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) // This should set canUseBiometricAuth to true every { biometricHelper.isSystemAuthEnabledAndValid } returns true - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val newState = withState(viewModel) { it } initialState shouldNotBeEqualTo newState } @@ -88,8 +114,7 @@ class LockScreenViewModelTests { @Test fun `when onPinCodeEntered is called in VERIFY mode, the code is verified and the result is emitted as a ViewEvent`() = runTest { val initialState = createViewState() - val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) coEvery { pinCodeHelper.verifyPinCode(any()) } returns true val events = viewModel.test().viewEvents @@ -113,8 +138,7 @@ class LockScreenViewModelTests { fun `when onPinCodeEntered is called in CREATE mode with no confirmation needed it creates the pin code`() = runTest { val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = false) val initialState = createViewState(lockScreenConfiguration = configuration) - val configProvider = LockScreenConfiguratorProvider(configuration) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val events = viewModel.test().viewEvents events.assertNoValues() @@ -128,9 +152,8 @@ class LockScreenViewModelTests { @Test fun `when onPinCodeEntered is called twice in CREATE mode with confirmation needed it verifies and creates the pin code`() = runTest { val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = true) - val configProvider = LockScreenConfiguratorProvider(configuration) val initialState = createViewState(lockScreenConfiguration = configuration) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val events = viewModel.test().viewEvents events.assertNoValues() @@ -149,8 +172,7 @@ class LockScreenViewModelTests { fun `when onPinCodeEntered is called in CREATE mode with incorrect confirmation it clears the pin code`() = runTest { val configuration = createDefaultConfiguration(mode = LockScreenMode.CREATE, needsNewCodeValidation = true) val initialState = createViewState(lockScreenConfiguration = configuration) - val configProvider = LockScreenConfiguratorProvider(configuration) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val events = viewModel.test().viewEvents events.assertNoValues() @@ -170,8 +192,7 @@ class LockScreenViewModelTests { @Test fun `onPinCodeEntered handles exceptions`() = runTest { val initialState = createViewState() - val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val exception = IllegalStateException("Something went wrong") coEvery { pinCodeHelper.verifyPinCode(any()) } throws exception @@ -187,39 +208,34 @@ class LockScreenViewModelTests { fun `when showBiometricPrompt catches a KeyPermanentlyInvalidatedException it disables biometric authentication`() = runTest { versionProvider.value = Build.VERSION_CODES.M - every { biometricHelper.isSystemAuthEnabledAndValid } returns true - every { biometricHelper.isSystemKeyValid } returns true + every { biometricHelper.isSystemKeyValid } returns false val exception = KeyPermanentlyInvalidatedException() coEvery { biometricHelper.authenticate(any()) } throws exception - coEvery { biometricHelper.disableAuthentication() } coAnswers { - every { biometricHelper.isSystemAuthEnabledAndValid } returns false - } val configuration = createDefaultConfiguration(mode = LockScreenMode.VERIFY, needsNewCodeValidation = true, isBiometricsEnabled = true) - val configProvider = LockScreenConfiguratorProvider(configuration) val initialState = createViewState( canUseBiometricAuth = true, isBiometricKeyInvalidated = false, lockScreenConfiguration = configuration ) - val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(initialState, pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val events = viewModel.test().viewEvents events.assertNoValues() viewModel.handle(LockScreenAction.ShowBiometricPrompt(mockk())) - events.assertValues(LockScreenViewEvent.AuthError(AuthMethod.BIOMETRICS, exception)) + events.assertValues(LockScreenViewEvent.ShowBiometricKeyInvalidatedMessage) 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 { - val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) coEvery { biometricHelper.authenticate(any()) } returns flowOf(false, true) val events = viewModel.test().viewEvents @@ -232,8 +248,7 @@ class LockScreenViewModelTests { @Test fun `showBiometricPrompt handles exceptions`() = runTest { - val configProvider = LockScreenConfiguratorProvider(createDefaultConfiguration()) - val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelper, keysMigrator, configProvider, versionProvider) + val viewModel = LockScreenViewModel(createViewState(), pinCodeHelper, biometricHelperFactory, keysMigrator, versionProvider, keyguardManager) val exception = IllegalStateException("Something went wrong") coEvery { biometricHelper.authenticate(any()) } throws exception