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 e610fbca5..6a1968ce6 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 @@ -487,7 +487,8 @@ fun FirebaseAuthScreen( onManageMfa = { if (reauthState == null) { if (configuration.isMfaEnabled) { - // pushUnique invariant: Success is one entry — nothing to bury. + // A second tap while the flow is already entered is a + // no-op: enterMfaEnrollment guards its own reset. backStack.enterMfaEnrollment( route = AuthRoute.MfaEnrollment, configuration = mfaConfiguration, 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 d2d62ed52..44a04eea1 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 @@ -265,14 +265,20 @@ internal val AuthRoute.entersMfaEnrollment: Boolean * for the picker, and redirecting it to that factor's configuration step would be a behavior * change. Only [AuthRoute.MfaEnrollment], which names the flow rather than a step, is resolved. * - * `pushUnique` invariant: both host call sites enter from a stack that cannot already hold a step - * of this flow, so the buried-case trim never fires. + * A no-op when a step of this flow is already on the stack. Entry is reachable from a destination + * that stays composed while the push runs, so a second tap can arrive mid-flow — and the reset, + * which exists to clear the *previous* enrolment, would there clear the one in progress. Nothing + * is left to do in that case: the flow is already entered. + * + * `pushUnique` invariant: that guard is also what keeps the buried-case trim from firing, since + * the only key this pushes is a step of a flow the stack has just been shown not to hold. */ internal fun NavBackStack.enterMfaEnrollment( route: AuthRoute, configuration: MfaConfiguration, flowState: MfaEnrollmentFlowState, ) { + if (any { it is AuthRoute.MfaEnrollment.Step }) return flowState.reset() pushUnique( when (route) { diff --git a/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentEntryResetTest.kt b/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentEntryResetTest.kt index a338d04f4..b23dfdc43 100644 --- a/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentEntryResetTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentEntryResetTest.kt @@ -82,7 +82,10 @@ import org.robolectric.annotation.Config * * an entry naming a specific step clearing **and still landing on that step**, including the * single-factor case where the named step is not the one [mfaEnrollmentStartStep] resolves to; * * every one of the nine fields going back to what [rememberMfaEnrollmentFlowState] created it - * with, asserted against the factory's own values rather than against restated literals. + * with, asserted against the factory's own values rather than against restated literals; + * * a sign-out and a second user in between, the case the clear is actually there for; + * * entry arriving *from inside* the flow doing nothing, since the clear would there take the + * enrolment in progress rather than the previous one. * * The emulator cannot perform a real TOTP enrolment, so the secret and the assertion are mocked — * the same approach [MfaEnrollmentHostDestinationsTest] takes. @@ -105,6 +108,13 @@ class MfaEnrollmentEntryResetTest { @Mock private lateinit var mockMultiFactor: MultiFactor + /** The user the flow state must not carry anything of the first user's into. */ + @Mock + private lateinit var secondUser: FirebaseUser + + @Mock + private lateinit var secondUserMultiFactor: MultiFactor + private lateinit var applicationContext: Context private lateinit var authUI: FirebaseAuthUI @@ -133,6 +143,11 @@ class MfaEnrollmentEntryResetTest { `when`(mockUser.isEmailVerified).thenReturn(true) `when`(mockUser.multiFactor).thenReturn(mockMultiFactor) `when`(mockMultiFactor.enrolledFactors).thenReturn(emptyList()) + `when`(secondUser.uid).thenReturn("mfa-entry-reset-second-user") + `when`(secondUser.email).thenReturn("second@example.com") + `when`(secondUser.isEmailVerified).thenReturn(true) + `when`(secondUser.multiFactor).thenReturn(secondUserMultiFactor) + `when`(secondUserMultiFactor.enrolledFactors).thenReturn(emptyList()) authUI = FirebaseAuthUI.create(app, mockAuth) } @@ -189,6 +204,69 @@ class MfaEnrollmentEntryResetTest { } } + /** + * The case the reset exists for. Six of the nine fields are `rememberSaveable` and the + * instance holding them is remembered by [FirebaseAuthScreen] above the `NavDisplay`, so it + * outlives the signed-in session that filled it — the second user would otherwise open the + * form on the first user's phone number and verification code. + */ + @Test + fun `a second user after a sign-out enters on a blank form`() { + withMockedTotpEnrollment { + renderSignedIn() + enterThroughManageMfa() + dirtyEveryReachableFieldAndEnroll() + + signOutAndSignIn(secondUser) + enterThroughManageMfa() + + assertEnteredBlankOn(MfaEnrollmentStep.SelectFactor) + } + } + + // ============================================================================================= + // Entering while already in the flow + // ============================================================================================= + + /** + * The success destination stays composed while the push onto it runs, so a second + * `onManageMfa` can arrive after the flow is already entered — and the reset, which is there + * to clear the *previous* enrolment, would clear the one in progress. Entry while a step of + * the flow is on the stack must do nothing at all. + */ + @Test + fun `entering again from inside the flow keeps what is already typed`() { + renderSignedIn() + enterThroughManageMfa() + selectFactor(MfaFactor.Sms) + typePhoneNumber() + + composeTestRule.runOnIdle { requireNotNull(uiContext).onManageMfa() } + composeTestRule.waitForIdle() + + assertStep(MfaEnrollmentStep.ConfigureSms) + assertThat(requireNotNull(mfaState).phoneNumber).isEqualTo(TYPED_PHONE_NUMBER) + assertThat(requireNotNull(mfaState).selectedFactor).isEqualTo(MfaFactor.Sms) + } + + /** + * The same guard through the other entry site, and naming a *different* step: `onNavigate` + * takes any [AuthRoute], so a host can name a step of the flow while the flow is already on + * the stack. That must not move the user off the step they are filling in either. + */ + @Test + fun `naming a step from inside the flow does not move or clear it`() { + renderSignedIn() + enterThroughManageMfa() + selectFactor(MfaFactor.Sms) + typePhoneNumber() + + enterThroughNavigate(AuthRoute.MfaEnrollment.SelectFactor) + + assertStep(MfaEnrollmentStep.ConfigureSms) + assertThat(requireNotNull(mfaState).phoneNumber).isEqualTo(TYPED_PHONE_NUMBER) + } + // ============================================================================================= // Entry naming a step: clear it, and still land where the caller asked // ============================================================================================= @@ -346,6 +424,23 @@ class MfaEnrollmentEntryResetTest { composeTestRule.onNodeWithTag(AUTHENTICATED_TAG).assertIsDisplayed() } + /** + * Signs the current user out through the success destination's own control, then signs [user] + * in — the whole point being that [FirebaseAuthScreen] is never left, so the flow state it + * remembers survives the change of user. + */ + private fun signOutAndSignIn(user: FirebaseUser) { + composeTestRule.runOnIdle { requireNotNull(uiContext).onSignOut() } + composeTestRule.waitForIdle() + + `when`(mockAuth.currentUser).thenReturn(user) + composeTestRule.runOnIdle { + authUI.updateAuthState(AuthState.Success(result = null, user = user, isNewUser = false)) + } + composeTestRule.waitForIdle() + composeTestRule.onNodeWithTag(AUTHENTICATED_TAG).assertIsDisplayed() + } + /** Host entry site A. */ private fun enterThroughManageMfa( startStep: MfaEnrollmentStep = MfaEnrollmentStep.SelectFactor,