Merge pull request #3930 from element-hq/feature/fga/check_fingerprin…

…t_before_enabling change : confirm biometric before allowing biometric unlock.
element-hq · Nov 25, 2024 · 9ad8c35 · 9ad8c35
2 parents a080444 + 2b75fff
commit 9ad8c35
Show file tree

Hide file tree

Showing 18 changed files with 320 additions and 117 deletions.
diff --git a/...l/src/main/kotlin/io/element/android/features/lockscreen/impl/DefaultLockScreenService.kt b/...l/src/main/kotlin/io/element/android/features/lockscreen/impl/DefaultLockScreenService.kt
@@ -10,7 +10,7 @@ package io.element.android.features.lockscreen.impl
 import com.squareup.anvil.annotations.ContributesBinding
 import io.element.android.features.lockscreen.api.LockScreenLockState
 import io.element.android.features.lockscreen.api.LockScreenService
-import io.element.android.features.lockscreen.impl.biometric.BiometricUnlockManager
+import io.element.android.features.lockscreen.impl.biometric.BiometricAuthenticatorManager
 import io.element.android.features.lockscreen.impl.biometric.DefaultBiometricUnlockCallback
 import io.element.android.features.lockscreen.impl.pin.DefaultPinCodeManagerCallback
 import io.element.android.features.lockscreen.impl.pin.PinCodeManager
@@ -45,7 +45,7 @@ class DefaultLockScreenService @Inject constructor(
     private val coroutineScope: CoroutineScope,
     private val sessionObserver: SessionObserver,
     private val appForegroundStateService: AppForegroundStateService,
-    biometricUnlockManager: BiometricUnlockManager,
+    biometricAuthenticatorManager: BiometricAuthenticatorManager,
 ) : LockScreenService {
     private val _lockState = MutableStateFlow<LockScreenLockState>(LockScreenLockState.Unlocked)
     override val lockState: StateFlow<LockScreenLockState> = _lockState
@@ -62,8 +62,8 @@ class DefaultLockScreenService @Inject constructor(
                 _lockState.value = LockScreenLockState.Unlocked
             }
         })
-        biometricUnlockManager.addCallback(object : DefaultBiometricUnlockCallback() {
-            override fun onBiometricUnlockSuccess() {
+        biometricAuthenticatorManager.addCallback(object : DefaultBiometricUnlockCallback() {
+            override fun onBiometricAuthenticationSuccess() {
                 _lockState.value = LockScreenLockState.Unlocked
                 coroutineScope.launch {
                     lockScreenStore.resetCounter()

diff --git a/...kscreen/impl/biometric/BiometricUnlock.kt → .../impl/biometric/BiometricAuthenticator.kt b/...kscreen/impl/biometric/BiometricUnlock.kt → .../impl/biometric/BiometricAuthenticator.kt
@@ -21,11 +21,11 @@ import timber.log.Timber
 import java.security.InvalidKeyException
 import javax.crypto.Cipher
 
-interface BiometricUnlock {
+interface BiometricAuthenticator {
     interface Callback {
         fun onBiometricSetupError()
-        fun onBiometricUnlockSuccess()
-        fun onBiometricUnlockFailed(error: Exception?)
+        fun onBiometricAuthenticationSuccess()
+        fun onBiometricAuthenticationFailed(error: Exception?)
     }
 
     sealed interface AuthenticationResult {
@@ -38,23 +38,23 @@ interface BiometricUnlock {
     suspend fun authenticate(): AuthenticationResult
 }
 
-class NoopBiometricUnlock : BiometricUnlock {
+class NoopBiometricAuthentication : BiometricAuthenticator {
     override val isActive: Boolean = false
     override fun setup() = Unit
-    override suspend fun authenticate() = BiometricUnlock.AuthenticationResult.Failure()
+    override suspend fun authenticate() = BiometricAuthenticator.AuthenticationResult.Failure()
 }
 
-class DefaultBiometricUnlock(
+class DefaultBiometricAuthentication(
     private val activity: FragmentActivity,
     private val promptInfo: PromptInfo,
     private val secretKeyRepository: SecretKeyRepository,
     private val encryptionDecryptionService: EncryptionDecryptionService,
     private val keyAlias: String,
-    private val callbacks: List<BiometricUnlock.Callback>
-) : BiometricUnlock {
+    private val callbacks: List<BiometricAuthenticator.Callback>
+) : BiometricAuthenticator {
     override val isActive: Boolean = true
 
-    private lateinit var cryptoObject: CryptoObject
+    private var cryptoObject: CryptoObject? = null
 
     override fun setup() {
         try {
@@ -67,11 +67,10 @@ class DefaultBiometricUnlock(
         }
     }
 
-    override suspend fun authenticate(): BiometricUnlock.AuthenticationResult {
-        if (!this::cryptoObject.isInitialized) {
-            return BiometricUnlock.AuthenticationResult.Failure()
-        }
-        val deferredAuthenticationResult = CompletableDeferred<BiometricUnlock.AuthenticationResult>()
+    override suspend fun authenticate(): BiometricAuthenticator.AuthenticationResult {
+        val cryptoObject = cryptoObject ?: return BiometricAuthenticator.AuthenticationResult.Failure()
+
+        val deferredAuthenticationResult = CompletableDeferred<BiometricAuthenticator.AuthenticationResult>()
         val executor = ContextCompat.getMainExecutor(activity.baseContext)
         val callback = AuthenticationCallback(callbacks, deferredAuthenticationResult)
         val prompt = BiometricPrompt(activity, executor, callback)
@@ -80,7 +79,7 @@ class DefaultBiometricUnlock(
             deferredAuthenticationResult.await()
         } catch (cancellation: CancellationException) {
             prompt.cancelAuthentication()
-            BiometricUnlock.AuthenticationResult.Failure(cancellation)
+            BiometricAuthenticator.AuthenticationResult.Failure(cancellation)
         }
     }
 
@@ -91,30 +90,30 @@ class DefaultBiometricUnlock(
 }
 
 private class AuthenticationCallback(
-    private val callbacks: List<BiometricUnlock.Callback>,
-    private val deferredAuthenticationResult: CompletableDeferred<BiometricUnlock.AuthenticationResult>,
+    private val callbacks: List<BiometricAuthenticator.Callback>,
+    private val deferredAuthenticationResult: CompletableDeferred<BiometricAuthenticator.AuthenticationResult>,
 ) : BiometricPrompt.AuthenticationCallback() {
     override fun onAuthenticationError(errorCode: Int, errString: CharSequence) {
         super.onAuthenticationError(errorCode, errString)
         val biometricUnlockError = BiometricUnlockError(errorCode, errString.toString())
-        callbacks.forEach { it.onBiometricUnlockFailed(biometricUnlockError) }
-        deferredAuthenticationResult.complete(BiometricUnlock.AuthenticationResult.Failure(biometricUnlockError))
+        callbacks.forEach { it.onBiometricAuthenticationFailed(biometricUnlockError) }
+        deferredAuthenticationResult.complete(BiometricAuthenticator.AuthenticationResult.Failure(biometricUnlockError))
     }
 
     override fun onAuthenticationFailed() {
         super.onAuthenticationFailed()
-        callbacks.forEach { it.onBiometricUnlockFailed(null) }
+        callbacks.forEach { it.onBiometricAuthenticationFailed(null) }
     }
 
     override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) {
         super.onAuthenticationSucceeded(result)
         if (result.cryptoObject?.cipher.isValid()) {
-            callbacks.forEach { it.onBiometricUnlockSuccess() }
-            deferredAuthenticationResult.complete(BiometricUnlock.AuthenticationResult.Success)
+            callbacks.forEach { it.onBiometricAuthenticationSuccess() }
+            deferredAuthenticationResult.complete(BiometricAuthenticator.AuthenticationResult.Success)
         } else {
             val error = IllegalStateException("Invalid cipher")
-            callbacks.forEach { it.onBiometricUnlockFailed(error) }
-            deferredAuthenticationResult.complete(BiometricUnlock.AuthenticationResult.Failure())
+            callbacks.forEach { it.onBiometricAuthenticationFailed(error) }
+            deferredAuthenticationResult.complete(BiometricAuthenticator.AuthenticationResult.Failure())
         }
     }
 

diff --git a/.../impl/biometric/BiometricUnlockManager.kt → ...iometric/BiometricAuthenticatorManager.kt b/.../impl/biometric/BiometricUnlockManager.kt → ...iometric/BiometricAuthenticatorManager.kt
@@ -9,7 +9,7 @@ package io.element.android.features.lockscreen.impl.biometric
 
 import androidx.compose.runtime.Composable
 
-interface BiometricUnlockManager {
+interface BiometricAuthenticatorManager {
     /**
      * If the device is secured for example with a pin, pattern or password.
      */
@@ -20,9 +20,18 @@ interface BiometricUnlockManager {
      */
     val hasAvailableAuthenticator: Boolean
 
-    fun addCallback(callback: BiometricUnlock.Callback)
-    fun removeCallback(callback: BiometricUnlock.Callback)
+    fun addCallback(callback: BiometricAuthenticator.Callback)
+    fun removeCallback(callback: BiometricAuthenticator.Callback)
 
+    /**
+     * Remember a biometric authenticator ready for unlocking the app.
+     */
+    @Composable
+    fun rememberUnlockBiometricAuthenticator(): BiometricAuthenticator
+
+    /**
+     * Remember a biometric authenticator ready for confirmation.
+     */
     @Composable
-    fun rememberBiometricUnlock(): BiometricUnlock
+    fun rememberConfirmBiometricAuthenticator(): BiometricAuthenticator
 }
diff --git a/...iometric/DefaultBiometricUnlockManager.kt → ...c/DefaultBiometricAuthenticatorManager.kt b/...iometric/DefaultBiometricUnlockManager.kt → ...c/DefaultBiometricAuthenticatorManager.kt
@@ -31,6 +31,7 @@ import io.element.android.libraries.cryptography.api.SecretKeyRepository
 import io.element.android.libraries.di.AppScope
 import io.element.android.libraries.di.ApplicationContext
 import io.element.android.libraries.di.SingleIn
+import io.element.android.libraries.ui.strings.CommonStrings
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import java.util.concurrent.CopyOnWriteArrayList
@@ -40,15 +41,15 @@ private const val SECRET_KEY_ALIAS = "elementx.SECRET_KEY_ALIAS_BIOMETRIC"
 
 @ContributesBinding(AppScope::class)
 @SingleIn(AppScope::class)
-class DefaultBiometricUnlockManager @Inject constructor(
+class DefaultBiometricAuthenticatorManager @Inject constructor(
     @ApplicationContext private val context: Context,
     private val lockScreenStore: LockScreenStore,
     private val lockScreenConfig: LockScreenConfig,
     private val encryptionDecryptionService: EncryptionDecryptionService,
     private val secretKeyRepository: SecretKeyRepository,
     private val coroutineScope: CoroutineScope,
-) : BiometricUnlockManager {
-    private val callbacks = CopyOnWriteArrayList<BiometricUnlock.Callback>()
+) : BiometricAuthenticatorManager {
+    private val callbacks = CopyOnWriteArrayList<BiometricAuthenticator.Callback>()
     private val biometricManager = BiometricManager.from(context)
     private val keyguardManager: KeyguardManager = context.getSystemService()!!
 
@@ -85,16 +86,42 @@ class DefaultBiometricUnlockManager @Inject constructor(
     }
 
     @Composable
-    override fun rememberBiometricUnlock(): BiometricUnlock {
+    override fun rememberUnlockBiometricAuthenticator(): BiometricAuthenticator {
         val isBiometricAllowed by lockScreenStore.isBiometricUnlockAllowed().collectAsState(initial = false)
         val lifecycleState by LocalLifecycleOwner.current.lifecycle.currentStateFlow.collectAsState()
         val isAvailable by remember(lifecycleState) {
-            derivedStateOf {
-                isBiometricAllowed && hasAvailableAuthenticator
-            }
+            derivedStateOf { isBiometricAllowed && hasAvailableAuthenticator }
         }
         val promptTitle = stringResource(id = R.string.screen_app_lock_biometric_unlock_title_android)
         val promptNegative = stringResource(id = R.string.screen_app_lock_use_pin_android)
+        return rememberBiometricAuthenticator(
+            isAvailable = isAvailable,
+            promptTitle = promptTitle,
+            promptNegative = promptNegative,
+        )
+    }
+
+    @Composable
+    override fun rememberConfirmBiometricAuthenticator(): BiometricAuthenticator {
+        val lifecycleState by LocalLifecycleOwner.current.lifecycle.currentStateFlow.collectAsState()
+        val isAvailable by remember(lifecycleState) {
+            derivedStateOf { hasAvailableAuthenticator }
+        }
+        val promptTitle = stringResource(id = R.string.screen_app_lock_confirm_biometric_authentication_android)
+        val promptNegative = stringResource(id = CommonStrings.action_cancel)
+        return rememberBiometricAuthenticator(
+            isAvailable = isAvailable,
+            promptTitle = promptTitle,
+            promptNegative = promptNegative,
+        )
+    }
+
+    @Composable
+    private fun rememberBiometricAuthenticator(
+        isAvailable: Boolean,
+        promptTitle: String,
+        promptNegative: String,
+    ): BiometricAuthenticator {
         val activity = LocalContext.current.findFragmentActivity()
         return remember(isAvailable) {
             if (isAvailable && activity != null) {
@@ -108,7 +135,7 @@ class DefaultBiometricUnlockManager @Inject constructor(
                     setNegativeButtonText(promptNegative)
                     setAllowedAuthenticators(authenticators)
                 }.build()
-                DefaultBiometricUnlock(
+                DefaultBiometricAuthentication(
                     activity = activity,
                     promptInfo = promptInfo,
                     secretKeyRepository = secretKeyRepository,
@@ -117,16 +144,16 @@ class DefaultBiometricUnlockManager @Inject constructor(
                     callbacks = callbacks + internalCallback
                 )
             } else {
-                NoopBiometricUnlock()
+                NoopBiometricAuthentication()
             }
         }
     }
 
-    override fun addCallback(callback: BiometricUnlock.Callback) {
+    override fun addCallback(callback: BiometricAuthenticator.Callback) {
         callbacks.add(callback)
     }
 
-    override fun removeCallback(callback: BiometricUnlock.Callback) {
+    override fun removeCallback(callback: BiometricAuthenticator.Callback) {
         callbacks.remove(callback)
     }
 

diff --git a/...n/io/element/android/features/lockscreen/impl/biometric/DefaultBiometricUnlockCallback.kt b/...n/io/element/android/features/lockscreen/impl/biometric/DefaultBiometricUnlockCallback.kt
@@ -7,8 +7,8 @@
 
 package io.element.android.features.lockscreen.impl.biometric
 
-open class DefaultBiometricUnlockCallback : BiometricUnlock.Callback {
+open class DefaultBiometricUnlockCallback : BiometricAuthenticator.Callback {
     override fun onBiometricSetupError() = Unit
-    override fun onBiometricUnlockSuccess() = Unit
-    override fun onBiometricUnlockFailed(error: Exception?) = Unit
+    override fun onBiometricAuthenticationSuccess() = Unit
+    override fun onBiometricAuthenticationFailed(error: Exception?) = Unit
 }
diff --git a/...otlin/io/element/android/features/lockscreen/impl/settings/LockScreenSettingsPresenter.kt b/...otlin/io/element/android/features/lockscreen/impl/settings/LockScreenSettingsPresenter.kt
@@ -15,7 +15,8 @@ import androidx.compose.runtime.produceState
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.setValue
 import io.element.android.features.lockscreen.impl.LockScreenConfig
-import io.element.android.features.lockscreen.impl.biometric.BiometricUnlockManager
+import io.element.android.features.lockscreen.impl.biometric.BiometricAuthenticator
+import io.element.android.features.lockscreen.impl.biometric.BiometricAuthenticatorManager
 import io.element.android.features.lockscreen.impl.pin.PinCodeManager
 import io.element.android.features.lockscreen.impl.storage.LockScreenStore
 import io.element.android.libraries.architecture.Presenter
@@ -27,7 +28,7 @@ class LockScreenSettingsPresenter @Inject constructor(
     private val lockScreenConfig: LockScreenConfig,
     private val pinCodeManager: PinCodeManager,
     private val lockScreenStore: LockScreenStore,
-    private val biometricUnlockManager: BiometricUnlockManager,
+    private val biometricAuthenticatorManager: BiometricAuthenticatorManager,
     private val coroutineScope: CoroutineScope,
 ) : Presenter<LockScreenSettingsState> {
     @Composable
@@ -42,6 +43,8 @@ class LockScreenSettingsPresenter @Inject constructor(
             mutableStateOf(false)
         }
 
+        val biometricUnlock = biometricAuthenticatorManager.rememberConfirmBiometricAuthenticator()
+
         fun handleEvents(event: LockScreenSettingsEvents) {
             when (event) {
                 LockScreenSettingsEvents.CancelRemovePin -> showRemovePinConfirmation = false
@@ -56,7 +59,14 @@ class LockScreenSettingsPresenter @Inject constructor(
                 LockScreenSettingsEvents.OnRemovePin -> showRemovePinConfirmation = true
                 LockScreenSettingsEvents.ToggleBiometricAllowed -> {
                     coroutineScope.launch {
-                        lockScreenStore.setIsBiometricUnlockAllowed(!isBiometricEnabled)
+                        if (!isBiometricEnabled) {
+                            biometricUnlock.setup()
+                            if (biometricUnlock.authenticate() == BiometricAuthenticator.AuthenticationResult.Success) {
+                                lockScreenStore.setIsBiometricUnlockAllowed(true)
+                            }
+                        } else {
+                            lockScreenStore.setIsBiometricUnlockAllowed(false)
+                        }
                     }
                 }
             }
@@ -66,7 +76,7 @@ class LockScreenSettingsPresenter @Inject constructor(
             showRemovePinOption = showRemovePinOption,
             isBiometricEnabled = isBiometricEnabled,
             showRemovePinConfirmation = showRemovePinConfirmation,
-            showToggleBiometric = biometricUnlockManager.isDeviceSecured,
+            showToggleBiometric = biometricAuthenticatorManager.isDeviceSecured,
             eventSink = ::handleEvents
         )
     }

diff --git a/.../main/kotlin/io/element/android/features/lockscreen/impl/setup/LockScreenSetupFlowNode.kt b/.../main/kotlin/io/element/android/features/lockscreen/impl/setup/LockScreenSetupFlowNode.kt
@@ -20,6 +20,7 @@ import com.bumble.appyx.navmodel.backstack.operation.newRoot
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.anvilannotations.ContributesNode
+import io.element.android.features.lockscreen.impl.biometric.BiometricAuthenticatorManager
 import io.element.android.features.lockscreen.impl.pin.DefaultPinCodeManagerCallback
 import io.element.android.features.lockscreen.impl.pin.PinCodeManager
 import io.element.android.features.lockscreen.impl.setup.biometric.SetupBiometricNode
@@ -35,6 +36,7 @@ class LockScreenSetupFlowNode @AssistedInject constructor(
     @Assisted buildContext: BuildContext,
     @Assisted plugins: List<Plugin>,
     private val pinCodeManager: PinCodeManager,
+    val biometricAuthenticatorManager: BiometricAuthenticatorManager,
 ) : BaseFlowNode<LockScreenSetupFlowNode.NavTarget>(
     backstack = BackStack(
         initialElement = NavTarget.Pin,
@@ -61,7 +63,11 @@ class LockScreenSetupFlowNode @AssistedInject constructor(
 
     private val pinCodeManagerCallback = object : DefaultPinCodeManagerCallback() {
         override fun onPinCodeCreated() {
-            backstack.newRoot(NavTarget.Biometric)
+            if (biometricAuthenticatorManager.hasAvailableAuthenticator) {
+                backstack.newRoot(NavTarget.Biometric)
+            } else {
+                onSetupDone()
+            }
         }
     }