From e08c2966c3f246bc26645fbc68717c8c87e0e12d Mon Sep 17 00:00:00 2001 From: Michelle Tang Date: Mon, 24 Jun 2024 10:46:29 -0700 Subject: [PATCH] Move biometrics check when linking a device. --- .../linkdevice/AddLinkDeviceFragment.kt | 70 +------------------ .../linkdevice/LinkDeviceFragment.kt | 58 ++++++++++++++- .../linkdevice/LinkDeviceQrScanScreen.kt | 11 +-- .../linkdevice/LinkDeviceSettingsState.kt | 1 - .../linkdevice/LinkDeviceViewModel.kt | 24 +------ app/src/main/res/values/strings.xml | 4 +- .../main/java/org/signal/qr/QrScannerView.kt | 11 --- .../main/java/org/signal/qr/ScannerView.kt | 2 - .../main/java/org/signal/qr/ScannerView19.kt | 12 ---- .../main/java/org/signal/qr/ScannerView21.kt | 9 --- 10 files changed, 67 insertions(+), 135 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/AddLinkDeviceFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/AddLinkDeviceFragment.kt index f7a4c29f66..e0ef86cc8f 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/AddLinkDeviceFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/AddLinkDeviceFragment.kt @@ -2,12 +2,7 @@ package org.thoughtcrime.securesms.linkdevice import android.Manifest import android.annotation.SuppressLint -import android.os.Bundle -import android.view.View import android.widget.Toast -import androidx.activity.result.ActivityResultLauncher -import androidx.biometric.BiometricManager -import androidx.biometric.BiometricPrompt import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.padding import androidx.compose.material3.Icon @@ -30,9 +25,6 @@ import com.google.accompanist.permissions.rememberPermissionState import org.signal.core.ui.Previews import org.signal.core.ui.Scaffolds import org.signal.core.ui.SignalPreview -import org.signal.core.util.logging.Log -import org.thoughtcrime.securesms.BiometricDeviceAuthentication -import org.thoughtcrime.securesms.BiometricDeviceLockContract import org.thoughtcrime.securesms.R import org.thoughtcrime.securesms.compose.ComposeFragment import org.thoughtcrime.securesms.permissions.Permissions @@ -43,42 +35,7 @@ import org.thoughtcrime.securesms.util.navigation.safeNavigate */ class AddLinkDeviceFragment : ComposeFragment() { - companion object { - private val TAG = Log.tag(AddLinkDeviceFragment::class) - } - private val viewModel: LinkDeviceViewModel by activityViewModels() - private lateinit var biometricAuth: BiometricDeviceAuthentication - private lateinit var biometricDeviceLockLauncher: ActivityResultLauncher - - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - super.onViewCreated(view, savedInstanceState) - - biometricDeviceLockLauncher = registerForActivityResult(BiometricDeviceLockContract()) { result: Int -> - if (result == BiometricDeviceAuthentication.AUTHENTICATED) { - viewModel.addDevice() - } else { - viewModel.clearBiometrics() - } - } - - val promptInfo = BiometricPrompt.PromptInfo.Builder() - .setAllowedAuthenticators(BiometricDeviceAuthentication.ALLOWED_AUTHENTICATORS) - .setTitle(requireContext().getString(R.string.BiometricDeviceAuthentication__signal)) - .setConfirmationRequired(true) - .build() - biometricAuth = BiometricDeviceAuthentication( - BiometricManager.from(requireActivity()), - BiometricPrompt(requireActivity(), BiometricAuthenticationListener()), - promptInfo - ) - } - - override fun onPause() { - super.onPause() - viewModel.clearBiometrics() - biometricAuth.cancelAuthentication() - } @OptIn(ExperimentalPermissionsApi::class) @Composable @@ -103,14 +60,7 @@ class AddLinkDeviceFragment : ComposeFragment() { onRequestPermissions = { askPermissions() }, onShowFrontCamera = { viewModel.showFrontCamera() }, onQrCodeScanned = { data -> viewModel.onQrCodeScanned(data) }, - onQrCodeApproved = { - viewModel.onQrCodeApproved() - if (biometricAuth.canAuthenticate()) { - biometricAuth.authenticate(requireContext(), true) { biometricDeviceLockLauncher.launch(getString(R.string.BiometricDeviceAuthentication__signal)) } - } else { - viewModel.addDevice() - } - }, + onQrCodeApproved = { viewModel.addDevice() }, onQrCodeDismissed = { viewModel.onQrCodeDismissed() }, onQrCodeRetry = { viewModel.onQrCodeScanned(state.url) }, onLinkDeviceSuccess = { @@ -134,23 +84,6 @@ class AddLinkDeviceFragment : ComposeFragment() { override fun onRequestPermissionsResult(requestCode: Int, permissions: Array, grantResults: IntArray) { Permissions.onRequestPermissionsResult(this, requestCode, permissions, grantResults) } - - private inner class BiometricAuthenticationListener : BiometricPrompt.AuthenticationCallback() { - override fun onAuthenticationError(errorCode: Int, errorString: CharSequence) { - Log.w(TAG, "Linked device authentication error: $errorCode") - viewModel.clearBiometrics() - onAuthenticationFailed() - } - - override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) { - Log.i(TAG, "Linked device authentication succeeded") - viewModel.addDevice() - } - - override fun onAuthenticationFailed() { - Log.w(TAG, "Linked device unable to authenticate") - } - } } @Composable @@ -188,7 +121,6 @@ private fun MainScreen( onQrCodeAccepted = onQrCodeApproved, onQrCodeDismissed = onQrCodeDismissed, onQrCodeRetry = onQrCodeRetry, - pendingBiometrics = state.pendingBiometrics, linkDeviceResult = state.linkDeviceResult, onLinkDeviceSuccess = onLinkDeviceSuccess, onLinkDeviceFailure = onLinkDeviceFailure, diff --git a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceFragment.kt index 2946bd0f04..27769e2af6 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceFragment.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceFragment.kt @@ -3,6 +3,9 @@ package org.thoughtcrime.securesms.linkdevice import android.os.Bundle import android.view.View import android.widget.Toast +import androidx.activity.result.ActivityResultLauncher +import androidx.biometric.BiometricManager +import androidx.biometric.BiometricPrompt import androidx.compose.foundation.Image import androidx.compose.foundation.background import androidx.compose.foundation.clickable @@ -59,6 +62,9 @@ import org.signal.core.ui.Dividers import org.signal.core.ui.Previews import org.signal.core.ui.Scaffolds import org.signal.core.ui.SignalPreview +import org.signal.core.util.logging.Log +import org.thoughtcrime.securesms.BiometricDeviceAuthentication +import org.thoughtcrime.securesms.BiometricDeviceLockContract import org.thoughtcrime.securesms.R import org.thoughtcrime.securesms.compose.ComposeFragment import org.thoughtcrime.securesms.util.DateUtils @@ -72,12 +78,40 @@ private const val PLACEHOLDER = "__ICON_PLACEHOLDER__" */ class LinkDeviceFragment : ComposeFragment() { + companion object { + private val TAG = Log.tag(LinkDeviceFragment::class) + } + private val viewModel: LinkDeviceViewModel by activityViewModels() + private lateinit var biometricAuth: BiometricDeviceAuthentication + private lateinit var biometricDeviceLockLauncher: ActivityResultLauncher override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) viewModel.initialize(requireContext()) + + biometricDeviceLockLauncher = registerForActivityResult(BiometricDeviceLockContract()) { result: Int -> + if (result == BiometricDeviceAuthentication.AUTHENTICATED) { + findNavController().safeNavigate(R.id.action_linkDeviceFragment_to_addLinkDeviceFragment) + } + } + + val promptInfo = BiometricPrompt.PromptInfo.Builder() + .setAllowedAuthenticators(BiometricDeviceAuthentication.ALLOWED_AUTHENTICATORS) + .setTitle(requireContext().getString(R.string.LinkDeviceFragment__unlock_to_link)) + .setConfirmationRequired(true) + .build() + biometricAuth = BiometricDeviceAuthentication( + BiometricManager.from(requireActivity()), + BiometricPrompt(requireActivity(), BiometricAuthenticationListener()), + promptInfo + ) + } + + override fun onPause() { + super.onPause() + biometricAuth.cancelAuthentication() } @Composable @@ -110,7 +144,13 @@ class LinkDeviceFragment : ComposeFragment() { navController = navController, modifier = Modifier.padding(contentPadding), onLearnMore = { navController.safeNavigate(R.id.action_linkDeviceFragment_to_linkDeviceLearnMoreBottomSheet) }, - onLinkDevice = { navController.safeNavigate(R.id.action_linkDeviceFragment_to_addLinkDeviceFragment) }, + onLinkDevice = { + if (biometricAuth.canAuthenticate()) { + biometricAuth.authenticate(requireContext(), true) { biometricDeviceLockLauncher.launch(getString(R.string.LinkDeviceFragment__unlock_to_link)) } + } else { + navController.safeNavigate(R.id.action_linkDeviceFragment_to_addLinkDeviceFragment) + } + }, setDeviceToRemove = { device -> viewModel.setDeviceToRemove(device) }, onRemoveDevice = { device -> viewModel.removeDevice(requireContext(), device) } ) @@ -122,6 +162,22 @@ class LinkDeviceFragment : ComposeFragment() { requireActivity().finishAfterTransition() } } + + private inner class BiometricAuthenticationListener : BiometricPrompt.AuthenticationCallback() { + override fun onAuthenticationError(errorCode: Int, errorString: CharSequence) { + Log.w(TAG, "Authentication error: $errorCode") + onAuthenticationFailed() + } + + override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) { + Log.i(TAG, "Authentication succeeded") + findNavController().safeNavigate(R.id.action_linkDeviceFragment_to_addLinkDeviceFragment) + } + + override fun onAuthenticationFailed() { + Log.w(TAG, "Unable to authenticate") + } + } } @Composable diff --git a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceQrScanScreen.kt b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceQrScanScreen.kt index 33e6b9a4fc..f9418c36e8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceQrScanScreen.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceQrScanScreen.kt @@ -33,7 +33,6 @@ fun LinkDeviceQrScanScreen( onQrCodeAccepted: () -> Unit, onQrCodeDismissed: () -> Unit, onQrCodeRetry: () -> Unit, - pendingBiometrics: Boolean, linkDeviceResult: LinkDeviceRepository.LinkDeviceResult, onLinkDeviceSuccess: () -> Unit, onLinkDeviceFailure: () -> Unit, @@ -95,13 +94,9 @@ fun LinkDeviceQrScanScreen( view }, update = { view: QrScannerView -> - if (pendingBiometrics) { - view.destroy() - } else { - view.start(lifecycleOwner = lifecycleOwner, forceLegacy = CameraXModelBlocklist.isBlocklisted()) - if (showFrontCamera != null) { - view.toggleCamera() - } + view.start(lifecycleOwner = lifecycleOwner, forceLegacy = CameraXModelBlocklist.isBlocklisted()) + if (showFrontCamera != null) { + view.toggleCamera() } }, hasPermission = hasPermission, diff --git a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceSettingsState.kt b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceSettingsState.kt index e554b37fd8..a6168fe67b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceSettingsState.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceSettingsState.kt @@ -17,6 +17,5 @@ data class LinkDeviceSettingsState( val linkDeviceResult: LinkDeviceRepository.LinkDeviceResult = LinkDeviceRepository.LinkDeviceResult.UNKNOWN, val showFinishedSheet: Boolean = false, val seenIntroSheet: Boolean = false, - val pendingBiometrics: Boolean = false, val pendingNewDevice: Boolean = false ) diff --git a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceViewModel.kt index fcadcf7720..a74baf2ca0 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/linkdevice/LinkDeviceViewModel.kt @@ -98,8 +98,7 @@ class LinkDeviceViewModel : ViewModel() { _state.update { val frontCamera = it.showFrontCamera it.copy( - showFrontCamera = if (frontCamera == null) true else !frontCamera, - pendingBiometrics = false + showFrontCamera = if (frontCamera == null) true else !frontCamera ) } } @@ -140,16 +139,6 @@ class LinkDeviceViewModel : ViewModel() { } } - fun onQrCodeApproved() { - _state.update { - it.copy( - qrCodeFound = false, - qrCodeInvalid = false, - pendingBiometrics = true - ) - } - } - fun onQrCodeDismissed() { _state.update { it.copy( @@ -159,21 +148,14 @@ class LinkDeviceViewModel : ViewModel() { } } - fun clearBiometrics() { - _state.update { - it.copy( - pendingBiometrics = false - ) - } - } - fun addDevice() { val uri = Uri.parse(_state.value.url) viewModelScope.launch(Dispatchers.IO) { val result = LinkDeviceRepository.addDevice(uri) _state.update { it.copy( - pendingBiometrics = false, + qrCodeFound = false, + qrCodeInvalid = false, linkDeviceResult = result, url = "" ) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 87f877702b..a786c79b9e 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -869,7 +869,7 @@ %1$s Messages and chat info are protected by end-to-end encryption on all devices - Signal on Desktop or iPad + Signal on desktop or iPad All messaging on linked devices is private @@ -891,6 +891,8 @@ Loading… No linked devices + + Unlock to link a device diff --git a/qr/lib/src/main/java/org/signal/qr/QrScannerView.kt b/qr/lib/src/main/java/org/signal/qr/QrScannerView.kt index b77cafab24..831ba57fcc 100644 --- a/qr/lib/src/main/java/org/signal/qr/QrScannerView.kt +++ b/qr/lib/src/main/java/org/signal/qr/QrScannerView.kt @@ -23,7 +23,6 @@ class QrScannerView @JvmOverloads constructor( private var scannerView: ScannerView? = null private val qrDataPublish: PublishSubject = PublishSubject.create() - private var forceLegacy: Boolean = false val qrData: Observable = qrDataPublish @@ -38,14 +37,12 @@ class QrScannerView @JvmOverloads constructor( addView(scannerView) this.scannerView = (scannerView as ScannerView) - this.forceLegacy = forceLegacy } @JvmOverloads fun start(lifecycleOwner: LifecycleOwner, forceLegacy: Boolean = false) { if (scannerView != null) { Log.w(TAG, "Attempt to start scanning that has already started") - scannerView?.resume() return } @@ -64,14 +61,6 @@ class QrScannerView @JvmOverloads constructor( scannerView?.toggleCamera() } - // Biometrics require use of camera so we disable when needed - fun destroy() { - scannerView?.destroy() - if (!forceLegacy) { - scannerView = null - } - } - companion object { private val TAG = Log.tag(QrScannerView::class.java) } diff --git a/qr/lib/src/main/java/org/signal/qr/ScannerView.kt b/qr/lib/src/main/java/org/signal/qr/ScannerView.kt index 4a41e98f03..8a4774c695 100644 --- a/qr/lib/src/main/java/org/signal/qr/ScannerView.kt +++ b/qr/lib/src/main/java/org/signal/qr/ScannerView.kt @@ -8,6 +8,4 @@ import androidx.lifecycle.LifecycleOwner interface ScannerView { fun start(lifecycleOwner: LifecycleOwner) fun toggleCamera() - fun resume() - fun destroy() } diff --git a/qr/lib/src/main/java/org/signal/qr/ScannerView19.kt b/qr/lib/src/main/java/org/signal/qr/ScannerView19.kt index c5a1e0a70a..617f03c110 100644 --- a/qr/lib/src/main/java/org/signal/qr/ScannerView19.kt +++ b/qr/lib/src/main/java/org/signal/qr/ScannerView19.kt @@ -59,16 +59,4 @@ internal class ScannerView19 constructor( lifecycleObserver.onResume(it) } } - - override fun resume() { - lifecycleOwner?.let { - lifecycleObserver.onResume(it) - } - } - - override fun destroy() { - lifecycleOwner?.let { - lifecycleObserver.onPause(it) - } - } } diff --git a/qr/lib/src/main/java/org/signal/qr/ScannerView21.kt b/qr/lib/src/main/java/org/signal/qr/ScannerView21.kt index 9cadcaaf99..d876de2d8e 100644 --- a/qr/lib/src/main/java/org/signal/qr/ScannerView21.kt +++ b/qr/lib/src/main/java/org/signal/qr/ScannerView21.kt @@ -36,7 +36,6 @@ internal class ScannerView21 constructor( private val lifecycleObserver: DefaultLifecycleObserver = object : DefaultLifecycleObserver { override fun onDestroy(owner: LifecycleOwner) { - cameraProvider?.unbindAll() cameraProvider = null camera = null analyzerExecutor.shutdown() @@ -79,14 +78,6 @@ internal class ScannerView21 constructor( lifecycleOwner.lifecycle.addObserver(lifecycleObserver) } - override fun resume() = Unit - - override fun destroy() { - lifecyleOwner?.let { - lifecycleObserver.onDestroy(it) - } - } - private fun onCameraProvider(lifecycle: LifecycleOwner, cameraProvider: ProcessCameraProvider?) { if (cameraProvider == null) { Log.w(TAG, "Camera provider is null")