fix(auth): don't reset MFA enrollment state when already in the flow - #2477
Open
demolaf wants to merge 1 commit into
Open
fix(auth): don't reset MFA enrollment state when already in the flow#2477demolaf wants to merge 1 commit into
demolaf wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request prevents state resets and duplicate entries when navigating within the MFA enrollment flow, particularly addressing issues with second taps and un-hosted screens. It introduces a guard to prevent re-entry when a flow step is already on the stack, resets the flow state on completion for un-hosted screens, and adds comprehensive tests covering these scenarios. The reviewer suggests reusing the existing mfaEnrollmentStartStep function in unhostedStartStep to reduce code duplication and improve maintainability.
demolaf
marked this pull request as ready for review
September 4, 2026 12:36
demolaf
force-pushed
the
version-10.0.0-beta05
branch
from
September 6, 2026 10:55
dfab195 to
0a1fbd7
Compare
demolaf
force-pushed
the
fix/mfa-enrollment-leftover-state
branch
from
September 6, 2026 13:36
a7342f1 to
3128546
Compare
demolaf
force-pushed
the
version-10.0.0-beta05
branch
from
September 7, 2026 12:25
5d227bf to
2d959d6
Compare
demolaf
force-pushed
the
fix/mfa-enrollment-leftover-state
branch
from
September 8, 2026 10:36
3128546 to
7a9f66d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#2473 clears
MfaEnrollmentFlowStateon entry to the flow, funnelling both host entry pointsthrough
enterMfaEnrollmentso no spelling of "enter the flow" skips the clear. The reset isunconditional, and entry is reachable from a destination that stays composed while the push onto
it runs — so a second tap can arrive after the flow is already entered, and the reset that exists
to clear the previous enrolment clears the one in progress instead.
onNavigatewidens it: ittakes any
AuthRoute, so a host can name a step of the flow while the flow is already on thestack.
enterMfaEnrollmentis now a no-op when a step of the flow is already on the stack — the flow isalready entered, so there is nothing left to do. That also enforces the
pushUniqueburied-caseinvariant that was previously only asserted in a comment. The pop-fade case is deliberately not
guarded: the stack is already truncated by then, so entry there is a legitimate fresh one.
Adds 3 tests driving the real
FirebaseAuthScreen: re-entry from inside the flow through bothhost entry sites, including
onNavigatenaming a different step, and sign-out → second user,which is the case the clear exists for and was pinned only at the unit level before.
This PR also reset the un-hosted
MfaEnrollmentScreenafter a completed enrolment. #2480 removedthe un-hosted mode outright —
step,onNavigateToStep,onNavigateBackandflowStateare nowrequired — so that defect went with the code path and those commits were dropped rather than
rebased.
Maintainer note: Fixes internal CPRN-405