Skip to content

fix(auth): don't reset MFA enrollment state when already in the flow - #2477

Open
demolaf wants to merge 1 commit into
version-10.0.0-beta05from
fix/mfa-enrollment-leftover-state
Open

fix(auth): don't reset MFA enrollment state when already in the flow#2477
demolaf wants to merge 1 commit into
version-10.0.0-beta05from
fix/mfa-enrollment-leftover-state

Conversation

@demolaf

@demolaf demolaf commented Sep 3, 2026

Copy link
Copy Markdown
Member

#2473 clears MfaEnrollmentFlowState on entry to the flow, funnelling both host entry points
through enterMfaEnrollment so no spelling of "enter the flow" skips the clear. The reset is
unconditional, 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. onNavigate widens it: it
takes any AuthRoute, so a host can name a step of the flow while the flow is already on the
stack.

enterMfaEnrollment is now a no-op when a step of the flow is already on the stack — the flow is
already entered, so there is nothing left to do. That also enforces the pushUnique buried-case
invariant 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 both
host entry sites, including onNavigate naming 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 MfaEnrollmentScreen after a completed enrolment. #2480 removed
the un-hosted mode outright — step, onNavigateToStep, onNavigateBack and flowState are now
required — so that defect went with the code path and those commits were dropped rather than
rebased.


Maintainer note: Fixes internal CPRN-405

@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 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.

Comment thread auth/src/main/java/com/firebase/ui/auth/ui/screens/mfa/MfaEnrollmentScreen.kt Outdated
@demolaf
demolaf marked this pull request as ready for review September 4, 2026 12:36
@demolaf
demolaf force-pushed the version-10.0.0-beta05 branch from dfab195 to 0a1fbd7 Compare September 6, 2026 10:55
@demolaf
demolaf force-pushed the fix/mfa-enrollment-leftover-state branch from a7342f1 to 3128546 Compare September 6, 2026 13:36
@demolaf demolaf changed the title fix(auth): clear leftover MFA enrollment state after #2473's reset-on-entry fix(auth): reset MFA enrollment state after an enrolment, never during one Sep 6, 2026
@demolaf demolaf changed the title fix(auth): reset MFA enrollment state after an enrolment, never during one fix(auth): reset MFA enrollment state after completion Sep 6, 2026
@demolaf
demolaf force-pushed the version-10.0.0-beta05 branch from 5d227bf to 2d959d6 Compare September 7, 2026 12:25
@demolaf
demolaf force-pushed the fix/mfa-enrollment-leftover-state branch from 3128546 to 7a9f66d Compare September 8, 2026 10:36
@demolaf demolaf changed the title fix(auth): reset MFA enrollment state after completion fix(auth): don't reset MFA enrollment state when already in the flow Sep 8, 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.

1 participant