Fix authenticator app removal.

This commit is contained in:
Greyson Parrelli
2026-09-10 17:10:14 -04:00
committed by Cody Henthorne
parent 78d6f51742
commit fd01a99f16
5 changed files with 23 additions and 20 deletions
@@ -106,8 +106,8 @@ class AccountSettingsViewModel(
AccountSettingsEvent.AuthenticationFailed -> {
_actions.send(AccountSettingsAction.ShowAuthenticationFailed)
}
AccountSettingsEvent.RemoveTotpAppConfirmed -> {
applyRemoveTotpAppConfirmed()
is AccountSettingsEvent.RemoveTotpAppConfirmed -> {
applyRemoveTotpAppConfirmed(event.appId)
}
AccountSettingsEvent.AdvancedPinSettingsClicked -> {
_actions.send(AccountSettingsAction.NavigateToAdvancedPinSettings)
@@ -214,11 +214,9 @@ class AccountSettingsViewModel(
}
}
private suspend fun applyRemoveTotpAppConfirmed() {
val dialog = _state.value.dialog as? Dialog.ConfirmRemoveTotpApp ?: return
private suspend fun applyRemoveTotpAppConfirmed(appId: Long) {
_state.update { it.copy(dialog = Dialog.None) }
removeTotpApp(dialog.appId)
removeTotpApp(appId)
}
private suspend fun removeTotpApp(appId: Long) {
@@ -480,25 +480,29 @@ class AccountSettingsViewModelTest {
val actions = collectActions(viewModel.actions)
viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP))
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed)
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(TOTP_APP.id))
coVerify { repository.removeTotpApp(TOTP_APP.id) }
assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.None)
assertThat(actions.last()).isEqualTo(AccountSettingsAction.ShowTotpAppRemoved)
}
/** The open dialog is what says which app is being removed, so a confirmation without one has no app to act on. */
/** The dialog dismisses itself before it confirms, so the removal has to survive the dismissal that lands first. */
@Test
fun `RemoveTotpAppConfirmed without the confirmation dialog removes nothing`() = runTest(testDispatcher) {
fun `RemoveTotpAppConfirmed removes the app even though the dialog dismissed itself first`() = 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.RemoveTotpAppConfirmed)
viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP))
viewModel.onEvent(AccountSettingsEvent.DialogDismissed)
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(TOTP_APP.id))
coVerify(exactly = 0) { repository.removeTotpApp(any()) }
assertThat(viewModel.state.value.signalLogin!!.twoFactorMethods).containsExactly(TOTP_APP)
coVerify { repository.removeTotpApp(TOTP_APP.id) }
assertThat(viewModel.state.value.dialog).isEqualTo(Dialog.None)
assertThat(actions.last()).isEqualTo(AccountSettingsAction.ShowTotpAppRemoved)
}
/** The list is what tells the user the app is gone, so it has to be read again rather than assumed. */
@@ -512,7 +516,7 @@ class AccountSettingsViewModelTest {
viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP))
coEvery { repository.getTwoFactorMethods() } returns methods()
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed)
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(TOTP_APP.id))
assertThat(viewModel.state.value.signalLogin!!.twoFactorMethods).isEmpty()
}
@@ -527,7 +531,7 @@ class AccountSettingsViewModelTest {
val actions = collectActions(viewModel.actions)
viewModel.onEvent(AccountSettingsEvent.MethodRemovalAuthenticated(TOTP_APP))
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed)
viewModel.onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(TOTP_APP.id))
assertThat(actions.last()).isEqualTo(AccountSettingsAction.ShowTotpAppRemovalFailed)
assertThat(viewModel.state.value.signalLogin!!.twoFactorMethods).containsExactly(TOTP_APP)
@@ -66,8 +66,8 @@ sealed interface AccountSettingsEvent {
/** The screen lock turned the user away, so whatever asked for it goes no further. */
data object AuthenticationFailed : AccountSettingsEvent
/** The user confirmed removing the authenticator app named by the open dialog, which removes it. */
data object RemoveTotpAppConfirmed : AccountSettingsEvent
/** The user confirmed removing the authenticator app with [appId], which removes it. */
data class RemoveTotpAppConfirmed(val appId: Long) : AccountSettingsEvent
/** The user tapped the advanced PIN settings row. */
data object AdvancedPinSettingsClicked : AccountSettingsEvent
@@ -375,7 +375,7 @@ fun AccountSettingsScreen(
RegistrationLockConfirmationDialog(dialog, onEvent)
}
}
is Dialog.ConfirmRemoveTotpApp -> ConfirmRemoveTotpAppDialog(onEvent = onEvent)
is Dialog.ConfirmRemoveTotpApp -> ConfirmRemoveTotpAppDialog(appId = dialog.appId, onEvent = onEvent)
Dialog.MaxTotpAppsReached -> MaxTotpAppsReachedDialog(maxApps = state.signalLogin?.maxTotpApps ?: 0, onEvent = onEvent)
}
}
@@ -617,13 +617,14 @@ private fun DeleteAllDataConfirmationDialog(
@Composable
private fun ConfirmRemoveTotpAppDialog(
appId: Long,
onEvent: (AccountSettingsEvent) -> Unit
) {
Dialogs.SimpleAlertDialog(
title = stringResource(R.string.AccountSettingsFragment__remove_authenticator_app),
body = stringResource(R.string.AccountSettingsFragment__you_wont_be_able_to_use_this_app),
confirm = stringResource(R.string.AccountSettingsFragment__remove),
onConfirm = { onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed) },
onConfirm = { onEvent(AccountSettingsEvent.RemoveTotpAppConfirmed(appId)) },
onDismiss = { onEvent(AccountSettingsEvent.DialogDismissed) },
dismiss = stringResource(android.R.string.cancel),
onDismissRequest = { onEvent(AccountSettingsEvent.DialogDismissed) },
@@ -885,7 +886,7 @@ private fun ConfirmPinToDisableRemindersDialogPreview() {
@Composable
private fun ConfirmRemoveTotpAppDialogPreview() {
Previews.Preview {
ConfirmRemoveTotpAppDialog(onEvent = {})
ConfirmRemoveTotpAppDialog(appId = 1, onEvent = {})
}
}
@@ -416,7 +416,7 @@ class AccountSettingsScreenTest {
composeTestRule.onNodeWithTag(AccountSettingsTestTags.DIALOG_CONFIRM_REMOVE_TOTP_APP).assertIsDisplayed()
composeTestRule.onNodeWithTag(Dialogs.TEST_TAG_ALERT_DIALOG_CONFIRM_BUTTON).performClick()
assertThat(events).contains(AccountSettingsEvent.RemoveTotpAppConfirmed)
assertThat(events).contains(AccountSettingsEvent.RemoveTotpAppConfirmed(METHODS[0].id))
}
@Test