diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsFragment.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsFragment.kt
index 3747956125..da3699c2dd 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsFragment.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsFragment.kt
@@ -14,6 +14,7 @@ import androidx.activity.result.contract.ActivityResultContracts.StartActivityFo
import androidx.annotation.StringRes
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
+import androidx.compose.ui.res.stringResource
import androidx.fragment.app.viewModels
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.navigation.fragment.findNavController
@@ -26,6 +27,8 @@ import org.signal.core.ui.compose.ComposeFragment
import org.signal.core.util.ServiceUtil
import org.signal.core.util.logging.Log
import org.thoughtcrime.securesms.R
+import org.thoughtcrime.securesms.components.compose.BiometricsAuthentication
+import org.thoughtcrime.securesms.components.compose.rememberBiometricsAuthentication
import org.thoughtcrime.securesms.components.settings.app.account.authenticator.TotpNavArgs
import org.thoughtcrime.securesms.dependencies.AppDependencies
import org.thoughtcrime.securesms.lock.v2.CreateSvrPinActivity
@@ -67,7 +70,13 @@ class AccountSettingsFragment : ComposeFragment() {
override fun FragmentContent() {
val state by viewModel.state.collectAsStateWithLifecycle()
- CollectActions(viewModel.actions) { action -> handleAction(action) }
+ val removalBiometrics = rememberBiometricsAuthentication(
+ promptTitle = stringResource(AppSettingsR.string.AccountSettingsFragment__unlock_to_remove_two_factor_method),
+ educationSheetMessage = stringResource(AppSettingsR.string.AccountSettingsFragment__to_remove_this_method_confirm_its_you),
+ onAuthenticationFailed = { viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticationFailed) }
+ )
+
+ CollectActions(viewModel.actions) { action -> handleAction(action, removalBiometrics) }
AccountSettingsScreen(
state = state,
@@ -75,7 +84,7 @@ class AccountSettingsFragment : ComposeFragment() {
)
}
- private fun handleAction(action: AccountSettingsAction) {
+ private fun handleAction(action: AccountSettingsAction, removalBiometrics: BiometricsAuthentication) {
when (action) {
AccountSettingsAction.NavigateBack -> requireActivity().onBackPressedDispatcher.onBackPressed()
AccountSettingsAction.LaunchCreatePinFlow -> pinFlowLauncher.launch(CreateSvrPinActivity.getIntentForPinCreate(requireContext()))
@@ -89,6 +98,12 @@ class AccountSettingsFragment : ComposeFragment() {
Bundle().apply { TotpNavArgs.putRenamedApp(this, action.app) }
)
}
+ is AccountSettingsAction.AuthenticateToRemoveMethod -> {
+ removalBiometrics.withBiometricsAuthentication {
+ viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(action.method))
+ }
+ }
+ AccountSettingsAction.ShowRemovalAuthenticationFailed -> toast(AppSettingsR.string.AccountSettingsFragment__authentication_required)
AccountSettingsAction.ShowTotpAppRemoved -> toast(AppSettingsR.string.AccountSettingsFragment__authenticator_app_removed)
AccountSettingsAction.ShowTotpAppRemovalFailed -> toast(AppSettingsR.string.AccountSettingsFragment__couldnt_remove_authenticator_app)
// TODO Open the two-factor authentication support article once one exists.
diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt
index eab4a73807..810b995b16 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt
@@ -95,7 +95,13 @@ class AccountSettingsViewModel(
applyRenameMethodClicked(event.method)
}
is AccountSettingsEvent.RemoveMethodClicked -> {
- applyRemoveMethodClicked(event.method)
+ _actions.send(AccountSettingsAction.AuthenticateToRemoveMethod(event.method))
+ }
+ is AccountSettingsEvent.MethodRemovalAuthenticated -> {
+ applyMethodRemovalAuthenticated(event.method)
+ }
+ AccountSettingsEvent.MethodRemovalAuthenticationFailed -> {
+ _actions.send(AccountSettingsAction.ShowRemovalAuthenticationFailed)
}
AccountSettingsEvent.RemoveTotpAppConfirmed -> {
applyRemoveTotpAppConfirmed()
@@ -194,7 +200,7 @@ class AccountSettingsViewModel(
}
}
- private fun applyRemoveMethodClicked(method: TwoFactorMethod) {
+ private fun applyMethodRemovalAuthenticated(method: TwoFactorMethod) {
when (method.kind) {
TwoFactorMethod.Kind.AUTHENTICATOR_APP -> {
_state.update { it.copy(dialog = Dialog.ConfirmRemoveTotpApp(method.id)) }
diff --git a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt
index 244cd2cf31..1dc8751d25 100644
--- a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt
+++ b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt
@@ -416,8 +416,9 @@ class AccountSettingsViewModelTest {
assertThat(actions).isEmpty()
}
+ /** Removing a second factor is guarded by the screen lock, so nothing happens until the user gets past it. */
@Test
- fun `RemoveMethodClicked asks the user to confirm first`() = runTest(testDispatcher) {
+ fun `RemoveMethodClicked asks for the screen lock first`() = runTest(testDispatcher) {
every { repository.isPhoneNumberless() } returns true
coEvery { repository.getTwoFactorMethods() } returns methods(TOTP_APP)
@@ -426,18 +427,46 @@ class AccountSettingsViewModelTest {
viewModel.onEvent(AccountSettingsEvent.RemoveMethodClicked(TOTP_APP))
- assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.ConfirmRemoveTotpApp(TOTP_APP.id))
- assertThat(actions).isEmpty()
+ assertThat(actions.last()).isEqualTo(AccountSettingsAction.AuthenticateToRemoveMethod(TOTP_APP))
+ assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.None)
}
@Test
- fun `RemoveMethodClicked for an unsupported passkey does nothing`() = runTest(testDispatcher) {
+ fun `MethodRemovalAuthenticated asks the user to confirm before removing`() = runTest(testDispatcher) {
+ every { repository.isPhoneNumberless() } returns true
+ coEvery { repository.getTwoFactorMethods() } returns methods(TOTP_APP)
+
+ val viewModel = createViewModel()
+
+ viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP))
+
+ assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.ConfirmRemoveTotpApp(TOTP_APP.id))
+ }
+
+ @Test
+ fun `MethodRemovalAuthenticationFailed says so and removes nothing`() = runTest(testDispatcher) {
+ every { repository.isPhoneNumberless() } returns true
+ coEvery { repository.getTwoFactorMethods() } returns methods(TOTP_APP)
+
+ val viewModel = createViewModel()
+ val actions = collectActions(viewModel.actions)
+
+ viewModel.onEvent(AccountSettingsEvent.RemoveMethodClicked(TOTP_APP))
+ viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticationFailed)
+
+ assertThat(actions.last()).isEqualTo(AccountSettingsAction.ShowRemovalAuthenticationFailed)
+ assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.None)
+ coVerify(exactly = 0) { repository.removeTotpApp(any()) }
+ }
+
+ @Test
+ fun `MethodRemovalAuthenticated for an unsupported passkey does nothing`() = runTest(testDispatcher) {
every { repository.isPhoneNumberless() } returns true
coEvery { repository.getTwoFactorMethods() } returns methods(PASSKEY)
val viewModel = createViewModel()
- viewModel.onEvent(AccountSettingsEvent.RemoveMethodClicked(PASSKEY))
+ viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(PASSKEY))
assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.None)
}
@@ -450,7 +479,7 @@ class AccountSettingsViewModelTest {
val viewModel = createViewModel()
val actions = collectActions(viewModel.actions)
- viewModel.onEvent(AccountSettingsEvent.RemoveMethodClicked(TOTP_APP))
+ viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP))
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed)
coVerify { repository.removeTotpApp(TOTP_APP.id) }
@@ -480,7 +509,7 @@ class AccountSettingsViewModelTest {
val viewModel = createViewModel()
- viewModel.onEvent(AccountSettingsEvent.RemoveMethodClicked(TOTP_APP))
+ viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP))
coEvery { repository.getTwoFactorMethods() } returns methods()
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed)
@@ -497,7 +526,7 @@ class AccountSettingsViewModelTest {
val viewModel = createViewModel()
val actions = collectActions(viewModel.actions)
- viewModel.onEvent(AccountSettingsEvent.RemoveMethodClicked(TOTP_APP))
+ viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP))
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed)
assertThat(actions.last()).isEqualTo(AccountSettingsAction.ShowTotpAppRemovalFailed)
diff --git a/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsAction.kt b/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsAction.kt
index b0cb9e7e5f..071870966b 100644
--- a/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsAction.kt
+++ b/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsAction.kt
@@ -36,6 +36,12 @@ sealed interface AccountSettingsAction {
/** Open the screen that renames [app]. */
data class NavigateToRenameTotpApp(val app: TotpApp) : AccountSettingsAction
+ /** Ask the user to get past their screen lock before we remove [method] from the account. */
+ data class AuthenticateToRemoveMethod(val method: TwoFactorMethod) : AccountSettingsAction
+
+ /** Tell the user we couldn't confirm it was them, so nothing was removed. */
+ data object ShowRemovalAuthenticationFailed : AccountSettingsAction
+
/** Tell the user their authenticator app was removed. */
data object ShowTotpAppRemoved : AccountSettingsAction
diff --git a/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt b/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt
index 78e8c54961..9c3ef68841 100644
--- a/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt
+++ b/feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt
@@ -54,9 +54,15 @@ sealed interface AccountSettingsEvent {
/** The user tapped the rename option in [method]'s overflow menu. */
data class RenameMethodClicked(val method: TwoFactorMethod) : AccountSettingsEvent
- /** The user tapped the remove option in [method]'s overflow menu, which asks them to confirm first. */
+ /** The user tapped the remove option in [method]'s overflow menu, which asks for the screen lock first. */
data class RemoveMethodClicked(val method: TwoFactorMethod) : AccountSettingsEvent
+ /** The user got past their screen lock, so we can go on asking them to confirm removing [method]. */
+ data class MethodRemovalAuthenticated(val method: TwoFactorMethod) : AccountSettingsEvent
+
+ /** The screen lock turned the user away, so [RemoveMethodClicked] goes no further. */
+ data object MethodRemovalAuthenticationFailed : AccountSettingsEvent
+
/** The user confirmed removing the authenticator app named by the open dialog, which removes it. */
data object RemoveTotpAppConfirmed : AccountSettingsEvent
diff --git a/feature/app-settings/src/main/res/values/strings.xml b/feature/app-settings/src/main/res/values/strings.xml
index 83b0184d19..6314e00dab 100644
--- a/feature/app-settings/src/main/res/values/strings.xml
+++ b/feature/app-settings/src/main/res/values/strings.xml
@@ -72,6 +72,12 @@
Remove authenticator app?
You won\'t be able to use this app for one-time verification codes when you sign in.
+
+ Unlock to remove two-factor method
+
+ To remove this method, confirm it\'s you
+
+ Authentication required
Can\'t add authenticator app