feat(auth): reshape reauthContent into a ReauthContentState content slot - #2452
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a robust reauthentication flow in FirebaseUI Auth for Android, separating operation-level cancellations (AuthState.Cancelled) from flow-level aborts (AuthState.Aborted). It adds support for a custom, stateless reauthContent slot in FirebaseAuthScreen while keeping credential exchanges owned by the library, locks the email field to read-only during reauthentication, and resolves several state-resetting edge cases. The review feedback suggests making the OAuth reauthentication path more robust and fail-fast by explicitly throwing an exception if auth.currentUser is unexpectedly null, rather than silently failing with a safe call.
5dfbe74 to
81b3b32
Compare
1e1858f to
82c68c0
Compare
692a7a5 to
f05a83c
Compare
russellwheatley
left a comment
There was a problem hiding this comment.
Two suggestions on the reauth config copy and state survival across rotation, nothing blocking otherwise. The uid-matching consumption logic, the internal AuthState.Success constructor, and the inert-while-armed gating all check out.
4ac3329 to
0469224
Compare
russellwheatley
left a comment
There was a problem hiding this comment.
Re-reviewed after the state-machine refactor and the two follow-up fixes. Both issues from the last round are fixed properly, not just carried over (isCredentialLinkingEnabled/isAnonymousUpgradeEnabled reset now applies at both reauth-config construction sites, and the rememberSaveable gap turned into a proper ReauthPresentationState + Saver that reconciles correctly on both rotation and process death). Two minor items left, neither blocking on their own, flagging so they don't get lost:
- inline comment below on AuthState.Success's constructor going internal
- e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/ReauthFlowTest.kt has no dedicated case for sign-out-during-retry or phone-verification-teardown. Both are already covered at the unit/screen level (FirebaseAuthScreenReauthIdleResetTest.kt, PhoneAuthScreenVerificationLifecycleTest.kt), so this is more of a nice-to-have than something I'd hold the PR on.
…/reauth-content-state-slot # Conflicts: # auth/src/main/java/com/firebase/ui/auth/ui/components/AuthTextField.kt # auth/src/main/java/com/firebase/ui/auth/ui/components/ErrorRecoveryDialog.kt # auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt # auth/src/main/java/com/firebase/ui/auth/ui/screens/email/SignInUI.kt # auth/src/test/java/com/firebase/ui/auth/ui/screens/email/SignInUITest.kt
…lot (#2452) * feat(auth): reshape reauthContent into a ReauthContentState content slot * fix(auth): address reauth review findings and retain state across recreation * refactor(auth): make reauthentication a request-scoped state machine * fix(auth): keep a proved reauthentication alive when its operation signs out * fix(auth): tear down phone verification when a reauthentication attempt fails * test(auth): cover sign-out-during-retry and phone reauth failure end to end * test(auth): drop the flaky phone reauth e2e case
reauthContentwas documented and shaped as a content slot alongsideemailContent,phoneContentand the MFA slots, but it received only(AuthState.ReauthenticationRequired, onDismiss)and every API needed to build a reauthentication UI —filterToLinkedProviders,isReauthenticationMode, the federated provider driver — wasinternalorprivate. A custom slot could therefore only perform email/password reauthentication and had to dead-end for a Google- or OAuth-only account. The success handoff was also easy to get wrong:onDismissreset auth state toIdlewhileretryOperationemittedAuthState.Success, so both orderings a caller would naturally reach either clobbered the success or cancelled the scope the retry ran in, silently dropping the sensitive operation.reauthContentnow receives a singleReauthContentStatecarrying the user, the reason, the providers already filtered to those linked to that user, and callbacks to select a provider or dismiss. The caller renders a provider chooser; the library owns credential exchange and dismiss/retry sequencing. SelectingAuthProvider.EmailorAuthProvider.Phonehands off to the library's own sub-flow, honouring the caller'semailContent/phoneContent, and an MFA-enrolled user now completes the second factor inside the reauth surface rather than having the challenge render beneath it.Reauthentication is a request-scoped state machine rather than seven independently mutable Compose holders.
AuthState.Reauthenticationcarries arequestIdand the pending operation; proof of reauthentication is areauthenticatedUidstamped onAuthState.Successat the three credential-exchange sites, and the pending operation is consumed only for a library-published success on that same uid, exactly once. Activity recreation resumes the same request; process death reports an interruption rather than dropping it.reauthContenttakes a singleReauthContentStateinstead of(state, onDismiss).AuthState.Successcan no longer be constructed outside the library. It records which uid a reauthentication re-proved, and that proof must not be forgeable by app code.AuthState.ReauthenticationRequiredis nowAuthState.Reauthentication.Required, nested with the other reauthentication phases under a new publicAuthState.Reauthenticationsealed class.ReauthContentStatemoves tocom.firebase.ui.auth.ui.screens.reauth.MfaChallengeScreenandMfaEnrollmentScreenmove tocom.firebase.ui.auth.ui.screens.mfa.While a reauthentication is in progress,
authStateFlow()andAuthFlowController.state()emitAuthState.Reauthenticationphases, sois AuthState.Error,is AuthState.Loadingandis AuthState.Cancelleddo not match for that window. The outcome is published as an ordinary state once the request completes. This is documented inauth/README.md.ReauthContentState.kt: new public state holder, following theMfaEnrollmentContentStateconventions.AuthState.kt:SuccessgainsreauthenticatedUidand aninternalconstructor; the reauthentication phases become a nested sealed hierarchy keyed byrequestId.FirebaseAuthUI.kt: one guarded transition entry point plus session start/finish; ordinary states are folded into reauthentication phases only while a screen is registered to drain them, so an arming created by public API with no screen composed stays inert.FirebaseAuthScreen.kt: the linked-provider list reaches the slot instead of being discarded; provider selection, error-dialog recovery, deep links and the non-terminal navigation branches are inert while a reauthentication is armed.EmailAuthProvider+FirebaseAuthUI.kt,OAuthProvider+FirebaseAuthUI.kt: stampreauthenticatedUidwhere the reauthenticated identity is known; account creation and credential linking are rejected in reauthentication mode.SignInUI.kt: sign-up, password recovery and email-link sign-in are not offered while reauthenticating, and Credential Manager autofill is skipped so a saved password for another account cannot be auto-submitted.ui/screens/reauth/,ui/screens/mfa/: reauthentication and MFA UI extracted into their own packages, mirroring the existingui/screens/email/andui/screens/phone/.Added
FirebaseAuthScreenReauthContentStateTest,EmailAuthScreenReauthEmailLockTestand coverage acrossFirebaseAuthUIAuthStateTest, plus e2e coverage of reauthentication through the slot — every new test verified to be load-bearing by temporarily reverting the fix and confirming it fails.Usage