diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt index 304ffb075..17d07f099 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt @@ -98,9 +98,9 @@ import com.firebase.ui.auth.ui.screens.email.isEmailLinkSignInOffered import com.firebase.ui.auth.ui.screens.email.isEmailSignUpOffered import com.firebase.ui.auth.ui.screens.email.navigateToEmailStep import com.firebase.ui.auth.ui.screens.mfa.MfaChallengeScreen +import com.firebase.ui.auth.ui.screens.mfa.enterMfaEnrollment import com.firebase.ui.auth.ui.screens.mfa.exitMfaEnrollment import com.firebase.ui.auth.ui.screens.mfa.mfaEnrollmentDestinations -import com.firebase.ui.auth.ui.screens.mfa.mfaEnrollmentStartStep import com.firebase.ui.auth.ui.screens.mfa.rememberMfaEnrollmentFlowState import com.firebase.ui.auth.ui.screens.phone.PhoneAuthContentState import com.firebase.ui.auth.ui.screens.phone.PhoneAuthScreen @@ -410,8 +410,9 @@ fun FirebaseAuthScreen( // Inert while armed: this content stays composed beneath the slot. if (reauthState == null) { if (configuration.isMfaEnabled) { - navController.navigate( - mfaEnrollmentStartStep(mfaConfiguration).route + navController.enterMfaEnrollment( + mfaConfiguration, + mfaEnrollmentFlowState, ) } else { val exception = AuthException.AuthCancelledException( @@ -453,10 +454,16 @@ fun FirebaseAuthScreen( onNavigate = { route -> // Inert while armed: this content stays composed beneath the slot. if (reauthState == null) { - // MfaEnrollment.route names SelectFactor; one factor skips it. - if (route == AuthRoute.MfaEnrollment) { - navController.navigate( - mfaEnrollmentStartStep(mfaConfiguration).route + // Naming the flow resolves a start step; naming one of its + // steps lands there. Either way the flow state is cleared, + // and MfaEnrollment.route alone cannot tell the two apart. + if (route == AuthRoute.MfaEnrollment || + route is AuthRoute.MfaEnrollment.Step + ) { + navController.enterMfaEnrollment( + mfaConfiguration, + mfaEnrollmentFlowState, + step = route as? AuthRoute.MfaEnrollment.Step, ) } else { navController.navigate(route.route) diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentDestinations.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentDestinations.kt index db7fe7be7..d7dfc5b9b 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentDestinations.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentDestinations.kt @@ -72,7 +72,28 @@ class MfaEnrollmentFlowState internal constructor( val totpQrCodeUrl: MutableState, val selectedCountry: MutableState, val totpSecretExpiredMessage: MutableState, -) +) { + /** + * Returns every field to the value [rememberMfaEnrollmentFlowState] starts it at. + * + * Called on flow *entry* — [enterMfaEnrollment] — not on completion: a flow that has been + * left is still composed for the length of the exit transition, and clearing underneath it + * leaves a rendered step whose controls act on state that is already gone. + * + * @since 10.0.0 + */ + fun reset() { + selectedFactor.value = null + phoneNumber.value = "" + verificationCode.value = "" + resendTimerSeconds.intValue = 0 + smsSession.value = null + totpSecret.value = null + totpQrCodeUrl.value = null + selectedCountry.value = CountryUtils.getDefaultCountry() + totpSecretExpiredMessage.value = null + } +} /** * Creates and remembers the [MfaEnrollmentFlowState] a host installs [mfaEnrollmentDestinations] @@ -167,6 +188,36 @@ internal fun NavHostController.navigateToMfaStep(step: AuthRoute.MfaEnrollment.S navigate(step.route) } +/** + * Enters the enrolment flow on a cleared [flowState], landing on [step] or, without one, on + * [mfaEnrollmentStartStep]. + * + * The clear happens here rather than when the flow completes because [flowState] outlives the + * flow — six of its fields are `rememberSaveable`, so an enrolment's phone number and verification + * code otherwise survive both the exit and process death, and the next entry lands on a filled-in + * form whose Verify button re-submits a code Firebase has already consumed. Clearing on the way + * out instead would mutate state a leaving step still reads while it is composed for the exit + * transition; on the way in there is no such step. + * + * Both of the host's flow entry points go through here: `AuthSuccessUiContext.onManageMfa`, and + * `onNavigate` given [AuthRoute.MfaEnrollment] or any one of its steps. + * + * @param step Where a caller that named a step rather than the whole flow asked to land; null + * means it named the flow, which resolves through [mfaEnrollmentStartStep]. Entry takes the step + * as a parameter rather than leaving that caller to navigate for itself, because + * [AuthRoute.MfaEnrollment] and [AuthRoute.MfaEnrollment.SelectFactor] report the same + * [AuthRoute.route]: a caller doing its own `navigate` cannot honour a named step without also + * skipping the clear. + */ +internal fun NavHostController.enterMfaEnrollment( + configuration: MfaConfiguration, + flowState: MfaEnrollmentFlowState, + step: AuthRoute.MfaEnrollment.Step? = null, +) { + flowState.reset() + navigate((step ?: mfaEnrollmentStartStep(configuration)).route) +} + /** * Leaves the MFA enrolment flow, popping a step at a time until the top of the back stack is not * one. diff --git a/auth/src/test/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreenMfaEnrollmentEntryTest.kt b/auth/src/test/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreenMfaEnrollmentEntryTest.kt new file mode 100644 index 000000000..bfe14fe1e --- /dev/null +++ b/auth/src/test/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreenMfaEnrollmentEntryTest.kt @@ -0,0 +1,329 @@ +/* + * Copyright 2025 Google Inc. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the + * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.firebase.ui.auth.ui.screens + +import androidx.compose.material3.Text +import androidx.compose.ui.Modifier +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.lifecycle.viewmodel.compose.LocalViewModelStoreOwner +import androidx.navigation.NavBackStackEntry +import androidx.test.core.app.ApplicationProvider +import com.firebase.ui.auth.AuthState +import com.firebase.ui.auth.FirebaseAuthUI +import com.firebase.ui.auth.configuration.AuthUIConfiguration +import com.firebase.ui.auth.configuration.MfaConfiguration +import com.firebase.ui.auth.configuration.MfaFactor +import com.firebase.ui.auth.configuration.auth_provider.AuthProvider +import com.firebase.ui.auth.configuration.authUIConfiguration +import com.firebase.ui.auth.data.ALL_COUNTRIES +import com.firebase.ui.auth.data.CountryData +import com.firebase.ui.auth.mfa.MfaEnrollmentContentState +import com.firebase.ui.auth.mfa.MfaEnrollmentStep +import com.firebase.ui.auth.util.CountryUtils +import com.google.common.truth.Truth.assertThat +import com.google.firebase.FirebaseApp +import com.google.firebase.FirebaseOptions +import com.google.firebase.auth.FirebaseAuth +import com.google.firebase.auth.FirebaseUser +import com.google.firebase.auth.MultiFactor +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito.`when` +import org.mockito.MockitoAnnotations +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Covers how [FirebaseAuthScreen] enters the MFA enrolment flow from its authenticated destination + * — the wiring that [com.firebase.ui.auth.ui.screens.mfa.enterMfaEnrollment] exists for. + * + * Hosts the real screen rather than a bare `NavHost`, because which of the host's two entry points + * clears the flow's state is a property of `FirebaseAuthScreen` itself, not of the destinations: + * `onManageMfa` and `onNavigate` each choose their own way in. + * + * @suppress Internal test class + */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, sdk = [34]) +class FirebaseAuthScreenMfaEnrollmentEntryTest { + + @get:Rule + val composeTestRule = createComposeRule() + + @Mock + private lateinit var mockAuth: FirebaseAuth + + @Mock + private lateinit var mockUser: FirebaseUser + + @Mock + private lateinit var mockMultiFactor: MultiFactor + + private lateinit var authUI: FirebaseAuthUI + + private var enrollmentState: MfaEnrollmentContentState? = null + private var uiContext: AuthSuccessUiContext? = null + + /** + * The back-stack entry the authenticated destination is currently composed in, or null while + * it is not composed — see [leaveFlow] for what its identity is worth. + */ + private var hostEntry: NavBackStackEntry? = null + + /** [hostEntry] as first composed, which every later one is compared against. */ + private var firstHostEntry: NavBackStackEntry? = null + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + FirebaseAuthUI.clearInstanceCache() + val context = ApplicationProvider.getApplicationContext() + FirebaseApp.getApps(context).forEach { it.delete() } + val app = FirebaseApp.initializeApp( + context, + FirebaseOptions.Builder() + .setApiKey("fake-api-key") + .setApplicationId("fake-app-id") + .setProjectId("fake-project-id") + .build() + ) + `when`(mockAuth.app).thenReturn(app) + `when`(mockAuth.currentUser).thenReturn(mockUser) + `when`(mockUser.uid).thenReturn("mfa-entry-user") + `when`(mockUser.email).thenReturn("user@example.com") + `when`(mockUser.isEmailVerified).thenReturn(true) + `when`(mockUser.multiFactor).thenReturn(mockMultiFactor) + `when`(mockMultiFactor.enrolledFactors).thenReturn(emptyList()) + authUI = FirebaseAuthUI.create(app, mockAuth) + } + + @After + fun tearDown() { + enrollmentState = null + uiContext = null + hostEntry = null + firstHostEntry = null + FirebaseAuthUI.clearInstanceCache() + val context = ApplicationProvider.getApplicationContext() + FirebaseApp.getApps(context).forEach { + try { + it.delete() + } catch (_: Exception) { + } + } + } + + /** The CPRN-384 regression, through the public entry point a host actually calls. */ + @Test + fun `re-entering from the authenticated destination starts on an empty form`() { + startAuthenticated() + manageMfa() + selectFactor(MfaFactor.Sms) + typePhoneNumber(TYPED_PHONE_NUMBER) + val pickedCountry = nonDefaultCountry() + selectCountry(pickedCountry) + assertThat(requireNotNull(enrollmentState).phoneNumber).isEqualTo(TYPED_PHONE_NUMBER) + assertThat(requireNotNull(enrollmentState).selectedCountry).isEqualTo(pickedCountry) + leaveFlow() + + manageMfa() + selectFactor(MfaFactor.Sms) + + assertThat(requireNotNull(enrollmentState).step).isEqualTo(MfaEnrollmentStep.ConfigureSms) + assertThat(requireNotNull(enrollmentState).phoneNumber).isEmpty() + assertThat(requireNotNull(enrollmentState).selectedCountry) + .isEqualTo(CountryUtils.getDefaultCountry()) + } + + /** + * [AuthSuccessUiContext.onNavigate] is the other way into the flow, and it names + * [AuthRoute.MfaEnrollment] rather than a step — so it has to clear the same state + * [AuthSuccessUiContext.onManageMfa] does. + */ + @Test + fun `entering through onNavigate also starts on an empty form`() { + startAuthenticated() + manageMfa() + selectFactor(MfaFactor.Sms) + typePhoneNumber(TYPED_PHONE_NUMBER) + leaveFlow() + + navigateTo(AuthRoute.MfaEnrollment) + selectFactor(MfaFactor.Sms) + + assertThat(requireNotNull(enrollmentState).phoneNumber).isEmpty() + } + + /** + * The step-level entry, which used to slip past the clear entirely: naming + * [AuthRoute.MfaEnrollment.SelectFactor] is indistinguishable to a host from naming + * [AuthRoute.MfaEnrollment], because the two report the same [AuthRoute.route] — so it has to + * clear the same state. Across a sign-out the state it used to keep is another user's. + */ + @Test + fun `entering through onNavigate at a named step also starts on an empty form`() { + startAuthenticated() + manageMfa() + selectFactor(MfaFactor.Sms) + typePhoneNumber(TYPED_PHONE_NUMBER) + leaveFlow() + + navigateTo(AuthRoute.MfaEnrollment.SelectFactor) + + assertThat(requireNotNull(enrollmentState).step).isEqualTo(MfaEnrollmentStep.SelectFactor) + assertThat(requireNotNull(enrollmentState).phoneNumber).isEmpty() + assertThat(requireNotNull(enrollmentState).selectedFactor).isNull() + } + + /** + * The clear must not cost the host the destination it asked for. Under an SMS-only + * configuration [com.firebase.ui.auth.ui.screens.mfa.mfaEnrollmentStartStep] resolves to + * [MfaEnrollmentStep.ConfigureSms], so routing a step-level entry through it would take a + * host that deliberately named [AuthRoute.MfaEnrollment.SelectFactor] — to show the factor + * picker, or to let the user unenroll an existing factor — somewhere else. + */ + @Test + fun `a named step is honoured over the configuration's resolved start step`() { + startAuthenticated(MfaConfiguration(allowedFactors = listOf(MfaFactor.Sms))) + manageMfa() + assertThat(requireNotNull(enrollmentState).step).isEqualTo(MfaEnrollmentStep.ConfigureSms) + typePhoneNumber(TYPED_PHONE_NUMBER) + leaveFlow() + + navigateTo(AuthRoute.MfaEnrollment.SelectFactor) + + assertThat(requireNotNull(enrollmentState).step).isEqualTo(MfaEnrollmentStep.SelectFactor) + assertThat(requireNotNull(enrollmentState).phoneNumber).isEmpty() + } + + // Harness + + private fun startAuthenticated( + mfaConfiguration: MfaConfiguration = MfaConfiguration(), + ) { + composeTestRule.setContent { + FirebaseAuthScreen( + configuration = emailConfiguration(), + authUI = authUI, + onSignInSuccess = {}, + onSignInFailure = {}, + onSignInCancelled = {}, + mfaConfiguration = mfaConfiguration, + mfaEnrollmentContent = { state -> enrollmentState = state }, + authenticatedContent = { _, context -> + uiContext = context + // navigation-compose composes a destination inside its own entry's + // LocalOwnersProvider, so this is that destination's NavBackStackEntry. + val entry = LocalViewModelStoreOwner.current as NavBackStackEntry + hostEntry = entry + if (firstHostEntry == null) firstHostEntry = entry + Text( + text = "authenticated", + modifier = Modifier.testTag(AUTHENTICATED_TAG), + ) + }, + ) + } + composeTestRule.runOnIdle { + authUI.updateAuthState( + AuthState.Success(result = null, user = mockUser, isNewUser = false) + ) + } + composeTestRule.waitForIdle() + composeTestRule.onNodeWithTag(AUTHENTICATED_TAG).assertIsDisplayed() + } + + private fun manageMfa() { + composeTestRule.runOnIdle { requireNotNull(uiContext).onManageMfa() } + composeTestRule.waitForIdle() + } + + private fun selectFactor(factor: MfaFactor) { + composeTestRule.runOnIdle { requireNotNull(enrollmentState).onFactorSelected(factor) } + composeTestRule.waitForIdle() + } + + private fun typePhoneNumber(value: String) { + composeTestRule.runOnIdle { requireNotNull(enrollmentState).onPhoneNumberChange(value) } + composeTestRule.waitForIdle() + } + + private fun selectCountry(country: CountryData) { + composeTestRule.runOnIdle { requireNotNull(enrollmentState).onCountrySelected(country) } + composeTestRule.waitForIdle() + } + + /** + * A country the device locale cannot already have selected — [CountryUtils.getDefaultCountry] + * reads `Locale.getDefault().country`, so hardcoding one would let a differently-localed run + * turn the "country is back to the default" assertion vacuous. + */ + private fun nonDefaultCountry(): CountryData { + val default = CountryUtils.getDefaultCountry() + return ALL_COUNTRIES.first { it.countryCode != default.countryCode } + } + + /** Enters [route] the way a host does, through [AuthSuccessUiContext.onNavigate]. */ + private fun navigateTo(route: AuthRoute) { + composeTestRule.runOnIdle { requireNotNull(uiContext).onNavigate(route) } + composeTestRule.waitForIdle() + } + + /** + * Skips out of the flow, from however deep in it the caller got, and asserts the + * authenticated destination is showing again — so the entry these tests then make really is a + * re-entry from outside the flow. + * + * The identity check is what makes the tag assertion mean something: a host rebuilt from + * scratch would display the tag just as happily, and rebuilding it destroys whatever + * `rememberSaveable` an `authenticatedContent` holds. Only reference equality on the + * `NavBackStackEntry` separates "popped back to the host" from "built a new host". [hostEntry] + * is cleared first so a destination that never recomposes fails rather than reporting the + * instance it last saw. (Assertion borrowed from the PR #2467 review.) + */ + private fun leaveFlow() { + hostEntry = null + composeTestRule.runOnIdle { + requireNotNull(requireNotNull(enrollmentState).onSkipClick).invoke() + } + composeTestRule.waitForIdle() + composeTestRule.onNodeWithTag(AUTHENTICATED_TAG).assertIsDisplayed() + assertThat(hostEntry).isSameInstanceAs(requireNotNull(firstHostEntry)) + } + + private fun emailConfiguration(): AuthUIConfiguration = authUIConfiguration { + context = ApplicationProvider.getApplicationContext() + providers { + provider( + AuthProvider.Email( + emailLinkActionCodeSettings = null, + passwordValidationRules = emptyList(), + ) + ) + } + } + + private companion object { + const val AUTHENTICATED_TAG = "authenticated-destination" + const val TYPED_PHONE_NUMBER = "5551234567" + } +} diff --git a/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentFlowStateRestorationTest.kt b/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentFlowStateRestorationTest.kt index b30340f9b..3d09127c3 100644 --- a/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentFlowStateRestorationTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentFlowStateRestorationTest.kt @@ -32,6 +32,7 @@ import com.firebase.ui.auth.mfa.MfaEnrollmentContentState import com.firebase.ui.auth.mfa.SmsEnrollmentSession import com.firebase.ui.auth.mfa.TotpSecret import com.firebase.ui.auth.ui.screens.AuthRoute +import com.firebase.ui.auth.util.CountryUtils import com.google.common.truth.Truth.assertThat import com.google.firebase.FirebaseApp import com.google.firebase.FirebaseOptions @@ -64,6 +65,9 @@ import org.robolectric.annotation.Config * Guards the regression where a lost `smsSession` left * [MfaEnrollmentContentState.onResendCodeClick] a silent no-op. * + * Also covers the same fields from the other side: [MfaEnrollmentFlowState.reset], which has to + * name every one of them to put a re-entered flow back at its initial values. + * * @suppress Internal test class */ @RunWith(RobolectricTestRunner::class) @@ -198,6 +202,56 @@ class MfaEnrollmentFlowStateRestorationTest { assertThat(requireNotNull(flowState).totpQrCodeUrl.value).isNull() } + /** + * A field left out of [MfaEnrollmentFlowState.reset] is a field that leaks an old enrolment + * into the next one, so this asserts on all nine rather than on the ones a UI happens to show. + */ + @Test + fun `reset returns every field to the value rememberMfaEnrollmentFlowState starts it at`() { + composeTestRule.setContent { MfaFlowHost() } + composeTestRule.waitForIdle() + + // SelectFactor is the one step whose LaunchedEffect writes nothing back. + assertThat(currentRoute()).isEqualTo(AuthRoute.MfaEnrollment.SelectFactor.routePattern) + composeTestRule.runOnIdle { + val state = requireNotNull(flowState) + state.selectedFactor.value = MfaFactor.Sms + state.phoneNumber.value = TYPED_PHONE_NUMBER + state.verificationCode.value = TYPED_VERIFICATION_CODE + state.resendTimerSeconds.intValue = 30 + state.smsSession.value = SmsEnrollmentSession( + verificationId = "verification-id", + phoneNumber = "+1$TYPED_PHONE_NUMBER", + forceResendingToken = null, + sentAt = System.currentTimeMillis(), + ) + state.totpSecret.value = TotpSecret.from(mock(FirebaseTotpSecret::class.java)) + state.totpQrCodeUrl.value = FAKE_QR_URL + state.selectedCountry.value = CountryData( + name = "United Kingdom", + dialCode = "+44", + countryCode = "GB", + flagEmoji = "🇬🇧", + ) + state.totpSecretExpiredMessage.value = TOTP_SECRET_EXPIRED_MESSAGE + } + composeTestRule.waitForIdle() + + composeTestRule.runOnIdle { requireNotNull(flowState).reset() } + composeTestRule.waitForIdle() + + val state = requireNotNull(flowState) + assertThat(state.selectedFactor.value).isNull() + assertThat(state.phoneNumber.value).isEmpty() + assertThat(state.verificationCode.value).isEmpty() + assertThat(state.resendTimerSeconds.intValue).isEqualTo(0) + assertThat(state.smsSession.value).isNull() + assertThat(state.totpSecret.value).isNull() + assertThat(state.totpQrCodeUrl.value).isNull() + assertThat(state.selectedCountry.value).isEqualTo(CountryUtils.getDefaultCountry()) + assertThat(state.totpSecretExpiredMessage.value).isNull() + } + @Composable private fun MfaFlowHost() { val controller = rememberNavController() diff --git a/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentRouteNavigationTest.kt b/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentRouteNavigationTest.kt index 8cc4065d1..e8b1dd81a 100644 --- a/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentRouteNavigationTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentRouteNavigationTest.kt @@ -32,6 +32,7 @@ import com.firebase.ui.auth.FirebaseAuthUI import com.firebase.ui.auth.configuration.MfaConfiguration import com.firebase.ui.auth.configuration.MfaFactor import com.firebase.ui.auth.mfa.MfaEnrollmentContentState +import com.firebase.ui.auth.mfa.SmsEnrollmentSession import com.firebase.ui.auth.ui.screens.AuthRoute import com.google.android.gms.tasks.Task import com.google.android.gms.tasks.Tasks @@ -42,6 +43,9 @@ import com.google.firebase.auth.FirebaseAuth import com.google.firebase.auth.FirebaseUser import com.google.firebase.auth.MultiFactor import com.google.firebase.auth.MultiFactorSession +import com.google.firebase.auth.PhoneAuthCredential +import com.google.firebase.auth.PhoneMultiFactorAssertion +import com.google.firebase.auth.PhoneMultiFactorGenerator import com.google.firebase.auth.TotpMultiFactorGenerator import com.google.firebase.auth.TotpSecret as FirebaseTotpSecret import org.junit.After @@ -92,6 +96,9 @@ class MfaEnrollmentRouteNavigationTest { private var lastState: MfaEnrollmentContentState? = null private var pressBack: (() -> Unit)? = null private var reportComplete: (() -> Unit)? = null + private var flowState: MfaEnrollmentFlowState? = null + private var completions = 0 + private val errors = mutableListOf() @Before fun setUp() { @@ -122,6 +129,9 @@ class MfaEnrollmentRouteNavigationTest { lastState = null pressBack = null reportComplete = null + flowState = null + completions = 0 + errors.clear() FirebaseAuthUI.clearInstanceCache() val context = ApplicationProvider.getApplicationContext() FirebaseApp.getApps(context).forEach { @@ -418,6 +428,74 @@ class MfaEnrollmentRouteNavigationTest { assertThat(backStackRoutes()).containsExactly(AuthRoute.Success.routePattern) } + // Entering the flow clears whatever the last enrolment left behind + + @Test + fun `re-entering after a successful enrolment starts at the start step with an empty form`() { + startOutsideFlow() + selectFactor(MfaFactor.Sms) + typePhoneNumber(TYPED_PHONE_NUMBER) + pushVerifyFactorDirectly() + enrollSuccessfullyWithSms() + + reenterFlow() + + assertThat(currentRoute()) + .isEqualTo(AuthRoute.MfaEnrollment.SelectFactor.routePattern) + val state = requireNotNull(flowState) + assertThat(state.phoneNumber.value).isEmpty() + assertThat(state.verificationCode.value).isEmpty() + assertThat(state.selectedFactor.value).isNull() + assertThat(state.smsSession.value).isNull() + } + + /** An SMS-only configuration has no `SelectFactor`, so re-entry must land on `ConfigureSms`. */ + @Test + fun `re-entering an SMS-only flow starts at ConfigureSms with an empty form`() { + val configuration = smsOnlyConfiguration() + startOutsideFlow(configuration) + typePhoneNumber(TYPED_PHONE_NUMBER) + pushVerifyFactorDirectly() + enrollSuccessfullyWithSms() + + reenterFlow(configuration) + + assertThat(currentRoute()) + .isEqualTo(AuthRoute.MfaEnrollment.ConfigureSms.routePattern) + assertThat(requireNotNull(lastState).phoneNumber).isEmpty() + assertThat(requireNotNull(lastState).verificationCode).isEmpty() + } + + /** + * Pins the clear to flow *entry*, not completion. Clearing on completion is what sank the + * previous fix: it pulls state out from under a step that is still composed for the length of + * the exit transition, whose Verify button then throws `"No factor selected"` through + * `onError` *after* enrolment has already succeeded. + */ + @Test + fun `a successful enrolment reports no error and clears nothing until the flow is re-entered`() { + startOutsideFlow() + selectFactor(MfaFactor.Sms) + typePhoneNumber(TYPED_PHONE_NUMBER) + pushVerifyFactorDirectly() + + enrollSuccessfullyWithSms() + + assertThat(completions).isEqualTo(1) + assertThat(errors).isEmpty() + val state = requireNotNull(flowState) + assertThat(state.phoneNumber.value).isEqualTo(TYPED_PHONE_NUMBER) + assertThat(state.selectedFactor.value).isEqualTo(MfaFactor.Sms) + assertThat(state.verificationCode.value).isEqualTo(SMS_CODE) + + // Only now, with no step of the flow composed, is anything cleared. + reenterFlow() + + assertThat(errors).isEmpty() + assertThat(state.phoneNumber.value).isEmpty() + assertThat(state.verificationCode.value).isEmpty() + } + // Every public AuthRoute.MfaEnrollment value is a registered destination /** @@ -516,11 +594,24 @@ class MfaEnrollmentRouteNavigationTest { /** Enters the flow the way both of `FirebaseAuthScreen`'s entry points do. */ private fun enterFlow(configuration: MfaConfiguration) { composeTestRule.runOnIdle { - requireNotNull(navController).navigate(mfaEnrollmentStartStep(configuration).route) + requireNotNull(navController) + .enterMfaEnrollment(configuration, requireNotNull(flowState)) } composeTestRule.waitForIdle() } + /** + * Enters the flow again, after an enrolment has already left it. + * + * The route assertion is the point of the helper: entry is only meaningful from outside, so + * an exit that stranded the stack on a step — or popped [HOST_ROUTE] itself — would otherwise + * leave the re-entry tests silently measuring wherever the exit stopped. + */ + private fun reenterFlow(configuration: MfaConfiguration = twoFactorConfiguration()) { + assertThat(currentRoute()).isEqualTo(HOST_ROUTE) + enterFlow(configuration) + } + /** * The live [HOST_ROUTE] entry, or null once it is gone. Compared by reference: a pop leaves * the same instance, a reset builds a new one and destroys whatever was scoped to the old. @@ -559,6 +650,44 @@ class MfaEnrollmentRouteNavigationTest { composeTestRule.waitForIdle() } + private fun typeVerificationCode(value: String) { + composeTestRule.runOnIdle { requireNotNull(lastState).onVerificationCodeChange(value) } + composeTestRule.waitForIdle() + } + + /** + * Drives `onVerifyClick` down the SMS branch to a successful `multiFactor.enroll`. + * + * Seeds the auto-verified [SmsEnrollmentSession] that `onSendSmsCodeClick` would have stored, + * which is the one shape of session that needs no `PhoneAuthProvider.getCredential` call, so + * only the assertion factory has to be stubbed. + */ + private fun enrollSuccessfullyWithSms() { + val credential = mock(PhoneAuthCredential::class.java) + val assertion = mock(PhoneMultiFactorAssertion::class.java) + `when`(mockMultiFactor.enroll(assertion, SMS_DISPLAY_NAME)) + .thenReturn(Tasks.forResult(null)) + composeTestRule.runOnIdle { + requireNotNull(flowState).smsSession.value = SmsEnrollmentSession( + verificationId = "", + phoneNumber = "+1$TYPED_PHONE_NUMBER", + forceResendingToken = null, + sentAt = System.currentTimeMillis(), + autoVerifiedCredential = credential, + ) + } + typeVerificationCode(SMS_CODE) + + mockStatic(PhoneMultiFactorGenerator::class.java).use { generatorStatic -> + generatorStatic.`when` { + PhoneMultiFactorGenerator.getAssertion(credential) + }.thenReturn(assertion) + + composeTestRule.runOnIdle { requireNotNull(lastState).onVerifyClick() } + composeTestRule.waitForIdle() + } + } + /** Enters [AuthRoute.MfaEnrollment.VerifyFactor] as `onSendSmsCodeClick` does, minus the * real SMS network call. */ private fun pushVerifyFactorDirectly() { @@ -601,12 +730,13 @@ class MfaEnrollmentRouteNavigationTest { ) { val controller = rememberNavController() val dispatcher = LocalOnBackPressedDispatcherOwner.current?.onBackPressedDispatcher - val flowState = rememberMfaEnrollmentFlowState() + val enrollmentFlowState = rememberMfaEnrollmentFlowState() val exit: () -> Unit = { controller.exitMfaEnrollment() } SideEffect { navController = controller pressBack = dispatcher?.let { { it.onBackPressed() } } reportComplete = exit + flowState = enrollmentFlowState } NavHost( @@ -626,11 +756,16 @@ class MfaEnrollmentRouteNavigationTest { configuration = configuration, authConfiguration = null, authUI = authUI, - flowState = flowState, + flowState = enrollmentFlowState, content = { state -> lastState = state }, - onComplete = exit, + // Counted as well as forwarded, to tell a completion that also left the flow + // apart from one that never fired. + onComplete = { + completions++ + exit() + }, onSkip = exit, - onError = {}, + onError = { errors += it }, ) } } @@ -644,6 +779,10 @@ class MfaEnrollmentRouteNavigationTest { const val HOST_ROUTE = "host_outside_flow" const val TYPED_PHONE_NUMBER = "5551234567" + const val SMS_CODE = "123456" + + /** `MfaEnrollmentScreen` hard-codes this display name on the SMS enrolment call. */ + const val SMS_DISPLAY_NAME = "SMS" const val FAKE_SHARED_SECRET = "JBSWY3DPEHPK3PXP" const val FAKE_QR_URL = "otpauth://totp/test-issuer:user%40example.com?secret=JBSWY3DPEHPK3PXP" }