Signal/Android
1
0
Fork 0
mirror of https://github.com/signalapp/Signal-Android.git synced 2026-09-20 00:35:47 +01:00
Code Issues Packages Projects Releases Wiki Activity

Add screenlock guard to removing two-factor method.

Browse Source
This commit is contained in:
Greyson Parrelli
2026-09-09 16:37:45 -04:00
committed by Cody Henthorne
parent 1872894698
commit 818f22576f
6 changed files with 81 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsFragment.kt
+17 -2
View File
@@ -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.
app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModel.kt
+8 -2
View File
@@ -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)) }
app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/AccountSettingsViewModelTest.kt
+37 -8
View File
@@ -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)
feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsAction.kt
+6
View File
@@ -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
feature/app-settings/src/main/java/org/signal/appsettings/account/AccountSettingsEvent.kt
+7 -1
View File
@@ -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
feature/app-settings/src/main/res/values/strings.xml
+6
View File
@@ -72,6 +72,12 @@
<string name="AccountSettingsFragment__remove_authenticator_app">Remove authenticator app?</string>
<!-- Body of the dialog confirming removal of an authenticator app -->
<string name="AccountSettingsFragment__you_wont_be_able_to_use_this_app">You won\'t be able to use this app for one-time verification codes when you sign in.</string>
<!-- Title of the device screen lock prompt shown before a two-factor authentication method can be removed -->
<string name="AccountSettingsFragment__unlock_to_remove_two_factor_method">Unlock to remove two-factor method</string>
<!-- Title of the sheet explaining that the device screen lock is needed before a two-factor authentication method can be removed -->
<string name="AccountSettingsFragment__to_remove_this_method_confirm_its_you">To remove this method, confirm it\'s you</string>
<!-- Toast shown when the device screen lock wasn\'t confirmed, so nothing was removed -->
<string name="AccountSettingsFragment__authentication_required">Authentication required</string>
<!-- Title of the dialog shown when the account already has as many authenticator apps as it\'s allowed -->
<string name="AccountSettingsFragment__cant_add_authenticator_app">Can\'t add authenticator app</string>
<!-- Body of the dialog shown when the account already has as many authenticator apps as it\'s allowed -->