feat(auth): reshape reauthContent into a ReauthContentState content slot - #2452
feat(auth): reshape reauthContent into a ReauthContentState content slot#2452demolaf wants to merge 5 commits into
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.
| reauthSubRoute.value = null | ||
| reauthError.value = null | ||
| pendingReauthState.value = state | ||
| pendingReauthConfig.value = configuration.copy( |
There was a problem hiding this comment.
This copy() only overrides providers/isNewEmailAccountsAllowed/isReauthenticationMode, so isCredentialLinkingEnabled stays whatever the app set at the top level. If it's true, signInAndLinkWithCredential takes the linkWithCredential branch instead of reauthenticate for a credential that's already linked (filterToLinkedProviders only offers linked ones), which either gets rejected by Firebase or succeeds without actually satisfying the recent-login guarantee this flow exists for. Not covered by a test that combines isCredentialLinkingEnabled with isReauthenticationMode. Might be worth forcing isCredentialLinkingEnabled = false (and isAnonymousUpgradeEnabled = false for symmetry) here.
There was a problem hiding this comment.
Fixed, canLinkCredential now requires !config.isReauthenticationMode, so a reauthentication can never use linkWithCredential. Left isAnonymousUpgradeEnabled alone — anonymous users have no linked providers, so it's unreachable.
4ac3329 to
0469224
Compare
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