Skip to content

fix(auth): clear MFA enrollment flow state on every entry - #2473

Merged
russellwheatley merged 1 commit into
version-10.0.0-beta05from
fix/mfa-enrollment-clear-flow-state-on-entry
Sep 2, 2026
Merged

fix(auth): clear MFA enrollment flow state on every entry#2473
russellwheatley merged 1 commit into
version-10.0.0-beta05from
fix/mfa-enrollment-clear-flow-state-on-entry

Conversation

@demolaf

@demolaf demolaf commented Sep 2, 2026

Copy link
Copy Markdown
Member

MfaEnrollmentFlowState is remembered once above the back stack and handed to every enrollment step, and six of its nine fields are rememberSaveable. Nothing ever cleared it, so re-entering the flow landed the user on a form pre-filled from the previous attempt — phone number, verification code, and a consumed smsSession still live. Across a sign-out that is the previous user's data, which is the leak flagged as security-high on #2467.

MfaEnrollmentFlowState.reset() returns all nine fields to the values rememberMfaEnrollmentFlowState() starts them at, and both host entry points now go through a NavBackStack<NavKey>.enterMfaEnrollment that resets before pushing — so there is no spelling of "enter the flow" that skips the clear. The clear happens on entry rather than on completion because completion is not the only exit: skip, back off the lowest step, the signed-out redirect and process death mid-flow all leave state behind, and entry is the point where a stale value is actually harmful.

Entry by named step is included, and lands on the step the host asked for rather than being redirected through the resolved start step — a host deliberately showing the picker under a single-factor configuration keeps that behaviour. The funnel is a single predicate, toKey() is AuthRoute.MfaEnrollment.Step, which the FlowEntry/Destination split from #2469 makes exact: a FlowEntry resolves to its own startKey() and a Destination to itself, so MfaEnrollment and its four steps match and nothing else does — including AuthRoute.Reauth(step = ...), whose key is the Reauth wrapper.

Adds 5 tests driving the real FirebaseAuthScreen through sign-in, both factor branches, a successful enrollment and re-entry. Both entry sites are pinned independently rather than cross-covered, and all 12 mutation controls were verified to fail the test that names the behaviour they break — including each of reset()'s nine fields individually. resendTimer and smsSession are pinned by the unit-level reset() test only: driving a real SMS send from a host test would require naming a Firebase-internal class whose obfuscated name changes between releases (zzam in 24.0.1, zzao in 24.2.0), which would break on the next dependency bump. That limitation is documented at the assertion.

This re-lands the fix from #2463, which merged into feat/mfa-enrollment-per-step-nav an hour after that branch had already reached version-10.0.0-beta05, so it never arrived. It is a reimplementation rather than a cherry-pick: #2469 replaced Navigation 2 in the meantime, and the original was written against NavHostController and route strings that no longer exist.


Maintainer note: Fixes internal CPRN-384

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to reset the Multi-Factor Authentication (MFA) enrollment flow state upon entry. It adds a reset() function to MfaEnrollmentFlowState to clear all fields, and helper extensions (entersMfaEnrollment and enterMfaEnrollment) to ensure the state is cleared whenever the flow is entered. Additionally, a comprehensive test suite MfaEnrollmentEntryResetTest has been added to verify this behavior across different entry points. I have no feedback to provide.

@russellwheatley
russellwheatley merged commit 9f7aad8 into version-10.0.0-beta05 Sep 2, 2026
14 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in studio-2394994192-60a69 Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants