Skip to content

refactor(auth)!: decouple reauthentication from the FirebaseAuthUI shared state flow - #2478

Merged
demolaf merged 20 commits into
version-10.0.0-beta05from
refactor/reauth-remove-shared-flow-coupling
Sep 7, 2026
Merged

refactor(auth)!: decouple reauthentication from the FirebaseAuthUI shared state flow#2478
demolaf merged 20 commits into
version-10.0.0-beta05from
refactor/reauth-remove-shared-flow-coupling

Conversation

@demolaf

@demolaf demolaf commented Sep 4, 2026

Copy link
Copy Markdown
Member

FirebaseAuthUI was the SDK entry point, the process-wide AuthState channel and the reauthentication session store all at once. Reauthentication requests now travel on their own channel, provider code emits to an AuthFlowScope sink instead of the singleton's flow, and the phase machine lives in a composition-scoped holder — nine members deleted from FirebaseAuthUI, none added.

withReauth no longer retains the caller's lambda on a process-scoped singleton: it raises a request, suspends on a resolver, and retries on the caller's own coroutine. delete() goes through the same path instead of publishing Required and throwing.

  • AuthFlowScope.kt: provider extensions move off FirebaseAuthUI, so provider code can no longer reach the public state flow at all — the isolation is compiler-held rather than review-held.
  • ReauthFlowState.kt: the phase machine, scoped to one FirebaseAuthScreen's composition. Aborted now clears the phase and resolves the caller under both hosts.
  • Added SignInStateSequenceTest and ReauthFlowStateTest, plus emulator-backed e2e coverage for a password change and an account deletion through reauthentication.

⚠️ Breaking Changes

  • AuthState.Reauthentication.Required.retryOperation is removed outright — pre-GA, zero external usage, no shim.
  • AuthState.Reauthentication.Required's (user, reason) constructor is now internal. Raising a request is withReauth/delete()'s job; constructing one yourself no longer did anything.
  • A declined reauthentication throws AuthException.AuthCancelledException instead of returning quietly, and delete() reports that rather than InvalidCredentialsException.
  • Publishing AuthState.Reauthentication.Required to authStateFlow() yourself no longer raises a request; only withReauth and delete() do.
  • Rotation during the reauth sheet abandons the pending operation unless the caller launched from a scope that survives it.

Maintainer note: Fixes internal CPRN-413

@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 refactors the reauthentication flow in FirebaseUI Auth to use a composition-scoped AuthFlowScope and coroutine-based suspension (CompletableDeferred) instead of process-local callbacks on FirebaseAuthUI. This prevents sensitive operations from being lost or executed multiple times during Activity recreation. Various auth providers have been updated to emit states through this new scope. Feedback is provided to improve code safety in authUserState by avoiding the !! operator on user.email through smart casting.

Comment thread auth/src/main/java/com/firebase/ui/auth/AuthFlowScope.kt Outdated
@demolaf
demolaf marked this pull request as ready for review September 4, 2026 11:37
@demolaf
demolaf force-pushed the refactor/reauth-remove-shared-flow-coupling branch from bf937e9 to c3c7802 Compare September 6, 2026 10:29
demolaf and others added 15 commits September 6, 2026 11:55
@demolaf
demolaf force-pushed the refactor/reauth-remove-shared-flow-coupling branch from c3c7802 to d918b91 Compare September 6, 2026 10:55

@russellwheatley russellwheatley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Went through this end to end (checked out the branch, read the actual files rather than just the diff, since a phase-machine/coroutine refactor like this is easy to get wrong on diff alone).

The decoupling itself looks solid: AuthFlowScope genuinely walls provider code off from the old singleton flow, all six provider files migrated consistently, and the nine deleted FirebaseAuthUI members have no leftover references anywhere.

Found one hang I think is real (see inline) plus a smaller state-coherence issue on failed retries, and a couple of stale doc comments left over from the refactor. Details inline.

Comment thread auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt Outdated
Comment thread auth/src/main/java/com/firebase/ui/auth/FirebaseAuthUI.kt Outdated

@russellwheatley russellwheatley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@demolaf
demolaf disabled the stack merge September 7, 2026 10:40
@demolaf
demolaf merged commit 5fc7ec4 into master Sep 7, 2026
18 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in studio-2394994192-60a69 Sep 7, 2026
@demolaf

demolaf commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

This was not merged into master deliberately. GitHub retargeted it from version-10.0.0-beta05 to master when #2459 was merged by mistake, so it landed on top of that merge. Reverted from master in #2487 — these commits now sit on version-10.0.0-beta05 and ship as part of #2486.

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