From 7b5f7cd808ed277146e1b60aaf569ba4a7cba254 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 17 Jun 2026 14:03:29 -0400 Subject: [PATCH] Add pin opt-out support to regV5. --- .../v2/AppRegistrationStorageController.kt | 3 + .../registration/RegistrationNavigation.kt | 17 +++- .../registration/RegistrationRepository.kt | 20 ++++ .../screens/pincreation/PinCreationScreen.kt | 96 ++++++++++++++++++- .../pincreation/PinCreationScreenEvents.kt | 1 + .../pincreation/PinCreationViewModel.kt | 13 ++- .../PinEntryForRegistrationLockViewModel.kt | 5 +- .../PinEntryForSvrRestoreViewModel.kt | 7 +- .../screens/pinentry/PinEntryScreen.kt | 28 +++--- .../src/main/protowire/Registration.proto | 4 + .../src/main/res/values/strings.xml | 10 ++ .../pincreation/PinCreationViewModelTest.kt | 13 +++ ...inEntryForRegistrationLockViewModelTest.kt | 12 +++ .../PinEntryForSvrRestoreViewModelTest.kt | 13 +++ 14 files changed, 221 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt index 49daec4019..9d572df105 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/registration/v2/AppRegistrationStorageController.kt @@ -218,6 +218,9 @@ class AppRegistrationStorageController(private val context: Context) : StorageCo data.registrationLockEnabled, data.accountEntropyPool.isNotEmpty() ) + } else if (data.pinOptedOut) { + Log.i(TAG, "[commitRegistrationData] User opted out of creating a PIN. Applying opt-out.") + SvrRepository.optOutOfPin(rotateAep = false) } Unit diff --git a/feature/registration/src/main/java/org/signal/registration/RegistrationNavigation.kt b/feature/registration/src/main/java/org/signal/registration/RegistrationNavigation.kt index 9c6331466a..2db746ced7 100644 --- a/feature/registration/src/main/java/org/signal/registration/RegistrationNavigation.kt +++ b/feature/registration/src/main/java/org/signal/registration/RegistrationNavigation.kt @@ -87,6 +87,7 @@ import org.signal.registration.screens.phonenumber.PhoneNumberEntryScreenEvents import org.signal.registration.screens.phonenumber.PhoneNumberEntryViewModel import org.signal.registration.screens.phonenumber.PhoneNumberScreen import org.signal.registration.screens.pincreation.PinCreationScreen +import org.signal.registration.screens.pincreation.PinCreationScreenEvents import org.signal.registration.screens.pincreation.PinCreationViewModel import org.signal.registration.screens.pinentry.PinEntryForRegistrationLockViewModel import org.signal.registration.screens.pinentry.PinEntryForSmsBypassViewModel @@ -250,6 +251,7 @@ private const val COUNTRY_CODE_RESULT = "country_code_result" private const val BACKUP_CREDENTIAL_RESULT = "backup_credential_result" private const val LOCAL_BACKUP_RESTORE_RESULT = "local_backup_restore_result" private const val PHONE_NUMBER_DISCOVERABILITY_RESULT = "phone_number_discoverability_result" +private const val PIN_LEARN_MORE_URL = "https://support.signal.org/hc/articles/360007059792" /** * Sets up the navigation graph for the registration flow using Navigation 3. @@ -584,10 +586,23 @@ private fun EntryProviderScope.navigationEntries( ) ) val state by viewModel.state.collectAsStateWithLifecycle() + val context = LocalContext.current PinCreationScreen( state = state, - onEvent = { viewModel.onEvent(it) } + onEvent = { event -> + when (event) { + PinCreationScreenEvents.LearnMore -> { + LinkActions.openUrl(context, PIN_LEARN_MORE_URL) { error -> + when (error) { + OpenUrlError.NoBrowserFound -> Toast.makeText(context, R.string.LinkActions_error_no_browser_found, Toast.LENGTH_SHORT).show() + } + } + } + + else -> viewModel.onEvent(event) + } + } ) } diff --git a/feature/registration/src/main/java/org/signal/registration/RegistrationRepository.kt b/feature/registration/src/main/java/org/signal/registration/RegistrationRepository.kt index 19f2659161..2f5123cf97 100644 --- a/feature/registration/src/main/java/org/signal/registration/RegistrationRepository.kt +++ b/feature/registration/src/main/java/org/signal/registration/RegistrationRepository.kt @@ -513,6 +513,26 @@ class RegistrationRepository(val context: Context, val networkController: Networ result } + /** + * Records that the user has chosen not to create a PIN. + * + * This does not perform the opt-out itself -- it simply notes the user's choice in the in-progress + * registration data and commits it. The app applies the actual opt-out (clearing PIN/registration lock + * state, refreshing attributes, etc.) when it persists the committed [org.signal.registration.proto.RegistrationData]. + * + * Any previously-recorded PIN state is cleared so the persisted blob stays internally consistent. + */ + suspend fun setPinOptedOut(): Unit = withContext(Dispatchers.IO) { + Log.i(TAG, "[setPinOptedOut] Recording PIN opt-out in registration data.") + storageController.updateInProgressRegistrationData { + this.pinOptedOut = true + this.pin = "" + this.pinIsAlphanumeric = false + this.registrationLockEnabled = false + } + storageController.commitRegistrationData() + } + suspend fun getPreExistingRegistrationData(): PreExistingRegistrationData? { return storageController.getPreExistingRegistrationData() } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt index 1f076d662a..b136129db9 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreen.kt @@ -20,11 +20,13 @@ import androidx.compose.foundation.text.ClickableText import androidx.compose.foundation.text.KeyboardActions import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.material3.TextButton import androidx.compose.material3.TextField +import androidx.compose.material3.TopAppBarScrollBehavior import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue @@ -35,7 +37,10 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.focus.FocusRequester import androidx.compose.ui.focus.focusRequester +import androidx.compose.ui.graphics.vector.ImageVector +import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.res.stringResource +import androidx.compose.ui.res.vectorResource import androidx.compose.ui.text.SpanStyle import androidx.compose.ui.text.buildAnnotatedString import androidx.compose.ui.text.input.ImeAction @@ -47,12 +52,17 @@ import androidx.compose.ui.text.withStyle import androidx.compose.ui.unit.dp import org.signal.core.ui.compose.AllDevicePreviews import org.signal.core.ui.compose.Buttons +import org.signal.core.ui.compose.Dialogs +import org.signal.core.ui.compose.DropdownMenus +import org.signal.core.ui.compose.IconButtons.IconButton import org.signal.core.ui.compose.Previews +import org.signal.core.ui.compose.Scaffolds import org.signal.core.ui.compose.SignalIcons import org.signal.registration.R import org.signal.registration.screens.RegistrationScaffold import org.signal.registration.screens.TwoPaneRegistrationScaffold import org.signal.registration.screens.attachDebugLogHelper +import org.signal.core.ui.R as CoreR /** * PIN creation screen for the registration flow. @@ -98,6 +108,7 @@ fun PinCreationScreen( } } +@OptIn(ExperimentalMaterial3Api::class) @Composable private fun OnePaneLayout( params: RegistrationScaffold.Params.OnePane, @@ -110,15 +121,23 @@ private fun OnePaneLayout( modifier: Modifier = Modifier ) { val scrollState = rememberScrollState() + val topBarScrollBehavior = RegistrationScaffold.rememberTopBarScrollBehavior() RegistrationScaffold( modifier = modifier.fillMaxSize(), + topBar = { + PinCreationTopBar( + scrollBehavior = topBarScrollBehavior, + onEvent = onEvent + ) + }, content = { Column( modifier = Modifier .fillMaxSize() + .nestedScroll(topBarScrollBehavior.nestedScrollConnection) .verticalScroll(scrollState) - .padding(params.panePadding(hasHeader = false)) + .padding(params.panePadding(hasHeader = true)) ) { PinDescription( isConfirmEnabled = state.isConfirmEnabled, @@ -158,6 +177,7 @@ private fun OnePaneLayout( ) } +@OptIn(ExperimentalMaterial3Api::class) @Composable private fun TwoPaneLayout( params: RegistrationScaffold.Params.TwoPane, @@ -171,15 +191,23 @@ private fun TwoPaneLayout( ) { val firstPaneScrollState = rememberScrollState() val secondPaneScrollState = rememberScrollState() + val topBarScrollBehavior = RegistrationScaffold.rememberTopBarScrollBehavior() TwoPaneRegistrationScaffold( modifier = modifier.fillMaxSize(), params = params, + topBar = { + PinCreationTopBar( + scrollBehavior = topBarScrollBehavior, + onEvent = onEvent + ) + }, firstPane = { paddingValues -> Column( modifier = Modifier .weight(1f) .fillMaxHeight() + .nestedScroll(topBarScrollBehavior.nestedScrollConnection) .verticalScroll(firstPaneScrollState) .padding(paddingValues) ) { @@ -194,6 +222,7 @@ private fun TwoPaneLayout( modifier = Modifier .weight(1f) .fillMaxHeight() + .nestedScroll(topBarScrollBehavior.nestedScrollConnection) .verticalScroll(secondPaneScrollState) .padding(paddingValues) ) { @@ -352,6 +381,71 @@ private fun KeyboardToggleButton( } } +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun PinCreationTopBar( + scrollBehavior: TopAppBarScrollBehavior, + onEvent: (PinCreationScreenEvents) -> Unit +) { + var showOptOutDialog by rememberSaveable { mutableStateOf(false) } + + if (showOptOutDialog) { + Dialogs.SimpleAlertDialog( + title = stringResource(R.string.PinCreationScreen__warning), + body = stringResource(R.string.PinCreationScreen__disable_pin_warning), + confirm = stringResource(R.string.PinCreationScreen__disable_pin), + dismiss = stringResource(R.string.PinCreationScreen__cancel), + onConfirm = { + showOptOutDialog = false + onEvent(PinCreationScreenEvents.OptOut) + }, + onDismiss = { showOptOutDialog = false } + ) + } + + Scaffolds.DefaultTopAppBar( + title = "", + titleContent = { _, _ -> }, + onNavigationClick = { }, + navigationIcon = null, + scrollBehavior = scrollBehavior, + actions = { + val menuController = remember { DropdownMenus.MenuController() } + + IconButton( + onClick = { menuController.show() }, + modifier = Modifier.padding(horizontal = 8.dp) + ) { + Icon( + imageVector = ImageVector.vectorResource(CoreR.drawable.symbol_more_vertical_24), + contentDescription = stringResource(R.string.RegistrationActivity_open_menu) + ) + } + + DropdownMenus.Menu( + controller = menuController, + offsetX = 24.dp, + offsetY = 0.dp + ) { + DropdownMenus.Item( + text = { Text(text = stringResource(R.string.PinCreationScreen__learn_more_about_pins)) }, + onClick = { + menuController.hide() + onEvent(PinCreationScreenEvents.LearnMore) + } + ) + DropdownMenus.Item( + text = { Text(text = stringResource(R.string.PinCreationScreen__disable_pin)) }, + onClick = { + menuController.hide() + showOptOutDialog = true + } + ) + } + } + ) +} + @Composable private fun NextButton( params: RegistrationScaffold.Params, diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreenEvents.kt b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreenEvents.kt index 057537d8ea..c8667e40e8 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreenEvents.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationScreenEvents.kt @@ -11,4 +11,5 @@ sealed class PinCreationScreenEvents : DebugLoggableModel() { data class PinSubmitted(val pin: String) : PinCreationScreenEvents() data object ToggleKeyboard : PinCreationScreenEvents() data object LearnMore : PinCreationScreenEvents() + data object OptOut : PinCreationScreenEvents() } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt index 928112e6c4..3a6f60c46a 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pincreation/PinCreationViewModel.kt @@ -65,12 +65,21 @@ class PinCreationViewModel( } is PinCreationScreenEvents.LearnMore -> { - // TODO [registration] - Show learn more dialog or navigate to help screen - throw NotImplementedError("Show learn more dialog or navigate to help screen") + // Handled by the navigation layer, which opens the help URL directly. + } + is PinCreationScreenEvents.OptOut -> { + _state.value = state.copy(isConfirmEnabled = false) + applyOptOut() } } } + private suspend fun applyOptOut() { + Log.i(TAG, "[OptOut] User opted out of creating a PIN. Recording choice and completing registration.") + repository.setPinOptedOut() + parentEventEmitter(RegistrationFlowEvent.RegistrationComplete) + } + @VisibleForTesting fun applyParentState(state: PinCreationState, parentState: RegistrationFlowState): PinCreationState { return state.copy(accountEntropyPool = parentState.accountEntropyPool) diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModel.kt index 5648d01e9e..150ff4b7d5 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModel.kt @@ -184,8 +184,9 @@ class PinEntryForRegistrationLockViewModel( } private fun handleSkip() { - Log.d(TAG, "Skip requested - this will result in account data loss after timeRemaining: $timeRemaining ms") - // TODO [registration] - Show confirmation dialog warning about data loss, then proceed without PIN + // Registration lock is enforced server-side, so there's no way to register without the PIN. The skip option is + // never shown in this mode, so reaching here indicates a bug. + throw NotImplementedError("Skip is not a valid action during registration lock PIN entry") } class Factory( diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModel.kt index a73a66c0b4..275aae8eb0 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModel.kt @@ -143,9 +143,10 @@ class PinEntryForSvrRestoreViewModel( } } - private fun handleSkip() { - // TODO [registration] - Handle skip - throw NotImplementedError("Handle skip") + private suspend fun handleSkip() { + Log.i(TAG, "[Skip] User opted out of restoring data and creating a PIN. Recording choice and completing registration.") + repository.setPinOptedOut() + parentEventEmitter(RegistrationFlowEvent.RegistrationComplete) } class Factory( diff --git a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryScreen.kt b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryScreen.kt index b5337116fb..a37f760885 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryScreen.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/pinentry/PinEntryScreen.kt @@ -145,12 +145,14 @@ private fun OnePaneLayout( ) } - SkipButton( - onSkip = { onEvent(PinEntryScreenEvents.Skip) }, - modifier = Modifier - .align(Alignment.TopEnd) - .padding(params.edgeInset) - ) + if (state.mode != PinEntryState.Mode.RegistrationLock) { + SkipButton( + onSkip = { onEvent(PinEntryScreenEvents.Skip) }, + modifier = Modifier + .align(Alignment.TopEnd) + .padding(params.edgeInset) + ) + } } }, footer = { @@ -222,12 +224,14 @@ private fun TwoPaneLayout( ) } - SkipButton( - onSkip = { onEvent(PinEntryScreenEvents.Skip) }, - modifier = Modifier - .align(Alignment.TopEnd) - .padding(params.edgeInset) - ) + if (state.mode != PinEntryState.Mode.RegistrationLock) { + SkipButton( + onSkip = { onEvent(PinEntryScreenEvents.Skip) }, + modifier = Modifier + .align(Alignment.TopEnd) + .padding(params.edgeInset) + ) + } } }, footer = { diff --git a/feature/registration/src/main/protowire/Registration.proto b/feature/registration/src/main/protowire/Registration.proto index a65348c895..c3db3ef149 100644 --- a/feature/registration/src/main/protowire/Registration.proto +++ b/feature/registration/src/main/protowire/Registration.proto @@ -34,6 +34,10 @@ message RegistrationData { bytes temporaryMasterKey = 17; bool registrationLockEnabled = 18; + // Whether the user chose not to create a PIN during registration. The app is responsible for applying the + // actual opt-out (clearing PIN/registration lock state, refreshing attributes, etc.) when it commits this data. + bool pinOptedOut = 24; + // SVR credentials (from appendSvrCredentials / getRestoredSvrCredentials) repeated SvrCredential svrCredentials = 19; diff --git a/feature/registration/src/main/res/values/strings.xml b/feature/registration/src/main/res/values/strings.xml index 41b2dbf5a3..834f09581f 100644 --- a/feature/registration/src/main/res/values/strings.xml +++ b/feature/registration/src/main/res/values/strings.xml @@ -333,6 +333,16 @@ PIN must be at least 4 characters Re-enter PIN + + Learn more about PINs + + Warning + + If you disable the PIN, you will lose all data when you re-register Signal unless you manually back up and restore. You cannot turn on Registration Lock while the PIN is disabled. + + Disable PIN + + Cancel Registration Lock diff --git a/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt index 166f5c827b..48fff9a9ae 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/pincreation/PinCreationViewModelTest.kt @@ -103,6 +103,19 @@ class PinCreationViewModelTest { assertThat(emittedParentEvents.first()).isEqualTo(RegistrationFlowEvent.ResetState) } + // ==================== OptOut Tests ==================== + + @Test + fun `OptOut records opt-out and completes registration`() = runTest(testDispatcher) { + val initialState = PinCreationState(accountEntropyPool = AccountEntropyPool.generate()) + + viewModel.applyEvent(initialState, PinCreationScreenEvents.OptOut) + + coVerify { mockRepository.setPinOptedOut() } + assertThat(emittedParentEvents).hasSize(1) + assertThat(emittedParentEvents.first()).isEqualTo(RegistrationFlowEvent.RegistrationComplete) + } + // ==================== applyParentState Tests ==================== @Test diff --git a/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModelTest.kt index a1fc4d389f..2584cf2c57 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForRegistrationLockViewModelTest.kt @@ -5,6 +5,7 @@ package org.signal.registration.screens.pinentry +import assertk.assertFailure import assertk.assertThat import assertk.assertions.hasSize import assertk.assertions.isEqualTo @@ -306,6 +307,17 @@ class PinEntryForRegistrationLockViewModelTest { assertThat(emittedStates.last().oneTimeEvent).isEqualTo(PinEntryState.OneTimeEvent.UnknownError) } + // ==================== Skip Tests ==================== + + @Test + fun `Skip throws because skipping is not valid during registration lock`() = runTest { + val initialState = PinEntryState(mode = PinEntryState.Mode.RegistrationLock) + + assertFailure { + viewModel.applyEvent(initialState, PinEntryScreenEvents.Skip, parentEventEmitter, stateEmitter) + }.isInstanceOf() + } + // ==================== ToggleKeyboard Tests ==================== @Test diff --git a/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModelTest.kt index 3efc688685..54c6abe65a 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/pinentry/PinEntryForSvrRestoreViewModelTest.kt @@ -222,6 +222,19 @@ class PinEntryForSvrRestoreViewModelTest { assertThat(emittedStates.last().oneTimeEvent).isEqualTo(PinEntryState.OneTimeEvent.UnknownError) } + // ==================== Skip Tests ==================== + + @Test + fun `Skip records PIN opt-out and completes registration`() = runTest { + val initialState = PinEntryState(mode = PinEntryState.Mode.SvrRestore) + + viewModel.applyEvent(initialState, PinEntryScreenEvents.Skip, parentEventEmitter, stateEmitter) + + coVerify { mockRepository.setPinOptedOut() } + assertThat(emittedParentEvents).hasSize(1) + assertThat(emittedParentEvents.first()).isEqualTo(RegistrationFlowEvent.RegistrationComplete) + } + // ==================== ToggleKeyboard Tests ==================== @Test