From 00640cb3d8122ae50401fb3507397aac3698537c Mon Sep 17 00:00:00 2001 From: demolaf Date: Thu, 23 Jul 2026 02:06:59 +0100 Subject: [PATCH 1/7] fix(auth): keep TopLevelDialogController alive across authState changes --- .../ui/components/TopLevelDialogController.kt | 22 ++-- .../ui/auth/ui/screens/FirebaseAuthScreen.kt | 2 +- .../TopLevelDialogControllerTest.kt | 102 ++++++++++++++++++ 3 files changed, 116 insertions(+), 10 deletions(-) create mode 100644 auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt b/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt index 4cd0aadd8..bb13019b0 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt @@ -40,18 +40,18 @@ val LocalTopLevelDialogController = compositionLocalOf * dialogController?.showErrorDialog( @@ -68,7 +68,7 @@ val LocalTopLevelDialogController = compositionLocalOf AuthState ) { private var dialogState by mutableStateOf(null) private val shownErrorStates = mutableSetOf() @@ -89,7 +89,7 @@ class TopLevelDialogController( onDismiss: () -> Unit = {} ) { // Get current error state - val currentErrorState = authState as? AuthState.Error + val currentErrorState = currentAuthState() as? AuthState.Error // If this exact error state has already been shown, skip if (currentErrorState != null && currentErrorState in shownErrorStates) { @@ -162,13 +162,17 @@ class TopLevelDialogController( /** * Creates and remembers a [TopLevelDialogController]. + * + * [authState] is a lambda rather than a snapshot value so the controller can read the + * live auth state on every [TopLevelDialogController.showErrorDialog] call without being + * recreated (and losing its de-duplication history) whenever the auth state changes. */ @Composable fun rememberTopLevelDialogController( stringProvider: AuthUIStringProvider, - authState: AuthState + authState: () -> AuthState ): TopLevelDialogController { - return remember(stringProvider, authState) { + return remember(stringProvider) { TopLevelDialogController(stringProvider, authState) } } diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt index fffcf5905..2c39ab21a 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt @@ -139,7 +139,7 @@ fun FirebaseAuthScreen( val navController = rememberNavController() val authState by remember(authUI) { authUI.authStateFlow() }.collectAsState(AuthState.Idle) - val dialogController = rememberTopLevelDialogController(stringProvider, authState) + val dialogController = rememberTopLevelDialogController(stringProvider) { authState } val lastSuccessfulUserId = remember { mutableStateOf(null) } val pendingLinkingCredential = remember { mutableStateOf(null) } val pendingResolver = remember { mutableStateOf(null) } diff --git a/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt b/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt new file mode 100644 index 000000000..219da7061 --- /dev/null +++ b/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt @@ -0,0 +1,102 @@ +package com.firebase.ui.auth.ui.components + +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.test.core.app.ApplicationProvider +import com.firebase.ui.auth.AuthException +import com.firebase.ui.auth.AuthState +import com.firebase.ui.auth.configuration.string_provider.DefaultAuthUIStringProvider +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Unit tests for [TopLevelDialogController] and [rememberTopLevelDialogController]. + * + * These cover the fix for a bug where keying `remember` on the live `authState` value recreated + * the controller (and wiped its `shownErrorStates` de-duplication set) on every state change — + * which, combined with screens resetting `AuthState` back to `Idle` immediately after consuming + * an `Error`, would tear down and discard the just-shown dialog on the very next recomposition. + * + * @suppress Internal test class + */ +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, sdk = [34]) +class TopLevelDialogControllerTest { + + @get:Rule + val composeTestRule = createComposeRule() + + private lateinit var stringProvider: DefaultAuthUIStringProvider + + @Test + fun `controller survives an authState change instead of being recreated`() { + stringProvider = DefaultAuthUIStringProvider(ApplicationProvider.getApplicationContext()) + var state: AuthState = AuthState.Idle + lateinit var controller: TopLevelDialogController + + composeTestRule.setContent { + controller = rememberTopLevelDialogController(stringProvider) { state } + controller.CurrentDialog() + } + + val error = AuthState.Error(Exception("boom")) + composeTestRule.runOnIdle { + state = error + controller.showErrorDialog( + exception = AuthException.from(error.exception, stringProvider) + ) + } + composeTestRule.waitForIdle() + composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertExists() + + // Simulate what the fixed screens now do immediately after showing the dialog: reset + // the shared auth state back to Idle. If the controller were still keyed on authState, + // this recomposition would tear it down (and the dialog with it). + composeTestRule.runOnIdle { + state = AuthState.Idle + } + composeTestRule.waitForIdle() + + composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertExists() + } + + @Test + fun `showErrorDialog does not re-show the same error state twice`() { + stringProvider = DefaultAuthUIStringProvider(ApplicationProvider.getApplicationContext()) + var state: AuthState = AuthState.Idle + lateinit var controller: TopLevelDialogController + + composeTestRule.setContent { + controller = rememberTopLevelDialogController(stringProvider) { state } + controller.CurrentDialog() + } + + val error = AuthState.Error(Exception("boom")) + val exception = AuthException.from(error.exception, stringProvider) + + composeTestRule.runOnIdle { + state = error + controller.showErrorDialog(exception = exception) + } + composeTestRule.waitForIdle() + composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertExists() + + composeTestRule.runOnIdle { + controller.dismissDialog() + } + composeTestRule.waitForIdle() + composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertDoesNotExist() + + // Calling showErrorDialog again for the *same* Error instance (state hasn't changed) + // must be a no-op — the de-dup set persists across calls since the controller is not + // recreated between them. + composeTestRule.runOnIdle { + controller.showErrorDialog(exception = exception) + } + composeTestRule.waitForIdle() + composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertDoesNotExist() + } +} From 89bf3190ac653d33cb8a5eb39a8ce2468bbe9164 Mon Sep 17 00:00:00 2001 From: demolaf Date: Thu, 23 Jul 2026 02:09:52 +0100 Subject: [PATCH 2/7] fix(auth): reset transient AuthState back to Idle after screens consume it --- .../ui/auth/ui/screens/FirebaseAuthScreen.kt | 6 ++++ .../auth/ui/screens/email/EmailAuthScreen.kt | 36 ++++++++++++++++--- .../auth/ui/screens/phone/PhoneAuthScreen.kt | 11 ++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt index 2c39ab21a..0a783607a 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt @@ -542,6 +542,9 @@ fun FirebaseAuthScreen( // Keep external cancellation reporting centralized here so child screens // can handle local navigation without triggering duplicate callbacks. onSignInCancelled() + // Consume the one-off Cancelled notification so it doesn't leak to a + // freshly created FirebaseAuthScreen instance (e.g. a new Activity). + authUI.updateAuthState(AuthState.Idle) } is AuthState.Idle -> { @@ -632,6 +635,9 @@ fun FirebaseAuthScreen( // Dialog dismissed } ) + // Consume the one-off Error notification immediately after handling it so + // it doesn't leak to a freshly created FirebaseAuthScreen instance. + authUI.updateAuthState(AuthState.Idle) } } diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt index 667fc364b..8693b86ec 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt @@ -24,6 +24,7 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue import com.firebase.ui.auth.AuthException import com.firebase.ui.auth.AuthState import com.firebase.ui.auth.FirebaseAuthUI @@ -167,8 +168,14 @@ fun EmailAuthScreen( val authCredentialForLinking = remember { credentialForLinking } val errorMessage = if (authState is AuthState.Error) (authState as AuthState.Error).exception.message else null - val resetLinkSent = authState is AuthState.PasswordResetLinkSent - val emailSignInLinkSent = authState is AuthState.EmailSignInLinkSent + + // PasswordResetLinkSent/EmailSignInLinkSent are one-off notifications consumed (and reset to + // Idle) in the LaunchedEffect below, so they can't be derived from authState directly the way + // errorMessage is above — the confirmation dialogs in ResetPasswordUI/SignInEmailLinkUI latch + // their visibility via remember(resetLinkSent), which would immediately re-close as soon as + // authState flips back to Idle. Latch them locally instead. + var resetLinkSentLocal by rememberSaveable { mutableStateOf(false) } + var emailSignInLinkSentLocal by rememberSaveable { mutableStateOf(false) } // Track if credentials were retrieved from Credential Manager val retrievedCredential = remember { mutableStateOf?>(null) } @@ -229,10 +236,26 @@ fun EmailAuthScreen( // Dialog dismissed } ) + // Consume the one-off Error notification immediately after handling it so + // it doesn't leak to a freshly created EmailAuthScreen instance. + authUI.updateAuthState(AuthState.Idle) } is AuthState.Cancelled -> { onCancel() + // Consume the one-off Cancelled notification so it doesn't leak to a + // freshly created EmailAuthScreen instance. + authUI.updateAuthState(AuthState.Idle) + } + + is AuthState.PasswordResetLinkSent -> { + resetLinkSentLocal = true + authUI.updateAuthState(AuthState.Idle) + } + + is AuthState.EmailSignInLinkSent -> { + emailSignInLinkSentLocal = true + authUI.updateAuthState(AuthState.Idle) } else -> Unit @@ -247,8 +270,8 @@ fun EmailAuthScreen( confirmPassword = confirmPasswordTextValue.value, isLoading = isLoading, error = errorMessage, - resetLinkSent = resetLinkSent, - emailSignInLinkSent = emailSignInLinkSent, + resetLinkSent = resetLinkSentLocal, + emailSignInLinkSent = emailSignInLinkSentLocal, onEmailChange = { email -> emailTextValue.value = email }, @@ -286,6 +309,7 @@ fun EmailAuthScreen( } }, onSignInEmailLinkClick = { + emailSignInLinkSentLocal = false coroutineScope.launch { try { if (emailLinkFromDifferentDevice != null) { @@ -327,6 +351,7 @@ fun EmailAuthScreen( } }, onSendResetLinkClick = { + resetLinkSentLocal = false coroutineScope.launch { try { authUI.sendPasswordResetEmail( @@ -346,14 +371,17 @@ fun EmailAuthScreen( onGoToSignIn = { textValues.forEach { it.value = "" } mode.value = EmailAuthMode.SignIn + emailSignInLinkSentLocal = false }, onGoToResetPassword = { textValues.forEach { it.value = "" } mode.value = EmailAuthMode.ResetPassword + resetLinkSentLocal = false }, onGoToEmailLinkSignIn = { textValues.forEach { it.value = "" } mode.value = EmailAuthMode.EmailLinkSignIn + emailSignInLinkSentLocal = false }, ) diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt index e317c639d..91fa0a34e 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt @@ -196,6 +196,11 @@ fun PhoneAuthScreen( pendingVerificationPhoneNumber.value = null verificationStartTime.value = null + // Consume the one-off SMSAutoVerified notification before starting the async + // sign-in call below, so it doesn't leak to a freshly created PhoneAuthScreen + // instance and isn't clobbered by whatever state that call itself produces. + authUI.updateAuthState(AuthState.Idle) + coroutineScope.launch { try { authUI.signInWithPhoneAuthCredential( @@ -228,10 +233,16 @@ fun PhoneAuthScreen( // Dialog dismissed } ) + // Consume the one-off Error notification immediately after handling it so + // it doesn't leak to a freshly created PhoneAuthScreen instance. + authUI.updateAuthState(AuthState.Idle) } is AuthState.Cancelled -> { onCancel() + // Consume the one-off Cancelled notification so it doesn't leak to a + // freshly created PhoneAuthScreen instance. + authUI.updateAuthState(AuthState.Idle) } else -> Unit From cb6aa6751791cddd5eac0b74f4063597bc2d4be9 Mon Sep 17 00:00:00 2001 From: demolaf Date: Thu, 23 Jul 2026 02:10:03 +0100 Subject: [PATCH 3/7] test(auth): prove one-off AuthState no longer leaks across screen instances --- .../ui/auth/FirebaseAuthUIAuthStateTest.kt | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt b/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt index 8a4715c97..a251ea1e8 100644 --- a/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt @@ -318,6 +318,61 @@ class FirebaseAuthUIAuthStateTest { assertThat(states[2]).isEqualTo(AuthState.Cancelled) // After second update } + // ============================================================================================= + // Stale one-off AuthState regression tests + // + // Screens consume Error/Cancelled/SMSAutoVerified by immediately resetting the shared + // singleton back to Idle in the same effect that reacts to them (see FirebaseAuthScreen, + // EmailAuthScreen, PhoneAuthScreen). These tests prove a fresh collector attaching after that + // reset — simulating a newly created screen/Activity instance — never sees the stale value. + // ============================================================================================= + + @Test + fun `Error does not leak to a fresh collector after being consumed`() = runBlocking { + `when`(mockFirebaseAuth.currentUser).thenReturn(null) + + // Simulate a screen observing Error and then consuming it, as the fixed screens now do. + authUI.updateAuthState(AuthState.Error(Exception("boom"))) + authUI.updateAuthState(AuthState.Idle) + + // A brand-new collector (simulating a freshly created Activity) must see Idle. + assertThat(authUI.authStateFlow().first()).isEqualTo(AuthState.Idle) + } + + @Test + fun `Cancelled does not leak to a fresh collector after being consumed`() = runBlocking { + `when`(mockFirebaseAuth.currentUser).thenReturn(null) + + authUI.updateAuthState(AuthState.Cancelled) + authUI.updateAuthState(AuthState.Idle) + + assertThat(authUI.authStateFlow().first()).isEqualTo(AuthState.Idle) + } + + @Test + fun `SMSAutoVerified does not leak to a fresh collector after being consumed`() = runBlocking { + `when`(mockFirebaseAuth.currentUser).thenReturn(null) + val credential = mock(com.google.firebase.auth.PhoneAuthCredential::class.java) + + authUI.updateAuthState(AuthState.SMSAutoVerified(credential)) + authUI.updateAuthState(AuthState.Idle) + + assertThat(authUI.authStateFlow().first()).isEqualTo(AuthState.Idle) + } + + @Test + fun `Error left uncleared still leaks to a fresh collector (pins down the bug being fixed)`() = + runBlocking { + `when`(mockFirebaseAuth.currentUser).thenReturn(null) + + // No consuming reset here — this documents the pre-fix behavior so the underlying + // combine()-prefers-non-Idle mechanism (FirebaseAuthUI.authStateFlow()) doesn't + // silently change without a corresponding consume step in the screens. + authUI.updateAuthState(AuthState.Error(Exception("boom"))) + + assertThat(authUI.authStateFlow().first()).isInstanceOf(AuthState.Error::class.java) + } + // ============================================================================================= // AuthState Class Tests // ============================================================================================= From 55a574465c98f7dd8180278f585ae6ef164f4aab Mon Sep 17 00:00:00 2001 From: demolaf Date: Thu, 23 Jul 2026 02:37:48 +0100 Subject: [PATCH 4/7] fix(auth): don't key TopLevelDialogController's remember on unmemoized stringProvider --- .../firebase/ui/auth/ui/components/TopLevelDialogController.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt b/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt index bb13019b0..cf41a3c5c 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt @@ -172,7 +172,7 @@ fun rememberTopLevelDialogController( stringProvider: AuthUIStringProvider, authState: () -> AuthState ): TopLevelDialogController { - return remember(stringProvider) { + return remember { TopLevelDialogController(stringProvider, authState) } } From 4b6658aac7f73d1c5065ccf69c34c4818cadc08c Mon Sep 17 00:00:00 2001 From: demolaf Date: Thu, 23 Jul 2026 09:24:40 +0100 Subject: [PATCH 5/7] refactor(auth): remove explanatory comments from the auth-state-reset fix --- .../ui/components/TopLevelDialogController.kt | 4 ---- .../ui/auth/ui/screens/FirebaseAuthScreen.kt | 4 ---- .../ui/auth/ui/screens/email/EmailAuthScreen.kt | 9 --------- .../ui/auth/ui/screens/phone/PhoneAuthScreen.kt | 7 ------- .../ui/auth/FirebaseAuthUIAuthStateTest.kt | 14 -------------- .../components/TopLevelDialogControllerTest.kt | 16 ---------------- 6 files changed, 54 deletions(-) diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt b/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt index cf41a3c5c..9c5dfe329 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt @@ -162,10 +162,6 @@ class TopLevelDialogController( /** * Creates and remembers a [TopLevelDialogController]. - * - * [authState] is a lambda rather than a snapshot value so the controller can read the - * live auth state on every [TopLevelDialogController.showErrorDialog] call without being - * recreated (and losing its de-duplication history) whenever the auth state changes. */ @Composable fun rememberTopLevelDialogController( diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt index 0a783607a..58d6d93e6 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt @@ -542,8 +542,6 @@ fun FirebaseAuthScreen( // Keep external cancellation reporting centralized here so child screens // can handle local navigation without triggering duplicate callbacks. onSignInCancelled() - // Consume the one-off Cancelled notification so it doesn't leak to a - // freshly created FirebaseAuthScreen instance (e.g. a new Activity). authUI.updateAuthState(AuthState.Idle) } @@ -635,8 +633,6 @@ fun FirebaseAuthScreen( // Dialog dismissed } ) - // Consume the one-off Error notification immediately after handling it so - // it doesn't leak to a freshly created FirebaseAuthScreen instance. authUI.updateAuthState(AuthState.Idle) } } diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt index 8693b86ec..3146a0ecf 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt @@ -169,11 +169,6 @@ fun EmailAuthScreen( val errorMessage = if (authState is AuthState.Error) (authState as AuthState.Error).exception.message else null - // PasswordResetLinkSent/EmailSignInLinkSent are one-off notifications consumed (and reset to - // Idle) in the LaunchedEffect below, so they can't be derived from authState directly the way - // errorMessage is above — the confirmation dialogs in ResetPasswordUI/SignInEmailLinkUI latch - // their visibility via remember(resetLinkSent), which would immediately re-close as soon as - // authState flips back to Idle. Latch them locally instead. var resetLinkSentLocal by rememberSaveable { mutableStateOf(false) } var emailSignInLinkSentLocal by rememberSaveable { mutableStateOf(false) } @@ -236,15 +231,11 @@ fun EmailAuthScreen( // Dialog dismissed } ) - // Consume the one-off Error notification immediately after handling it so - // it doesn't leak to a freshly created EmailAuthScreen instance. authUI.updateAuthState(AuthState.Idle) } is AuthState.Cancelled -> { onCancel() - // Consume the one-off Cancelled notification so it doesn't leak to a - // freshly created EmailAuthScreen instance. authUI.updateAuthState(AuthState.Idle) } diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt index 91fa0a34e..fa6617a82 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt @@ -196,9 +196,6 @@ fun PhoneAuthScreen( pendingVerificationPhoneNumber.value = null verificationStartTime.value = null - // Consume the one-off SMSAutoVerified notification before starting the async - // sign-in call below, so it doesn't leak to a freshly created PhoneAuthScreen - // instance and isn't clobbered by whatever state that call itself produces. authUI.updateAuthState(AuthState.Idle) coroutineScope.launch { @@ -233,15 +230,11 @@ fun PhoneAuthScreen( // Dialog dismissed } ) - // Consume the one-off Error notification immediately after handling it so - // it doesn't leak to a freshly created PhoneAuthScreen instance. authUI.updateAuthState(AuthState.Idle) } is AuthState.Cancelled -> { onCancel() - // Consume the one-off Cancelled notification so it doesn't leak to a - // freshly created PhoneAuthScreen instance. authUI.updateAuthState(AuthState.Idle) } diff --git a/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt b/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt index a251ea1e8..469c012a3 100644 --- a/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt @@ -318,24 +318,13 @@ class FirebaseAuthUIAuthStateTest { assertThat(states[2]).isEqualTo(AuthState.Cancelled) // After second update } - // ============================================================================================= - // Stale one-off AuthState regression tests - // - // Screens consume Error/Cancelled/SMSAutoVerified by immediately resetting the shared - // singleton back to Idle in the same effect that reacts to them (see FirebaseAuthScreen, - // EmailAuthScreen, PhoneAuthScreen). These tests prove a fresh collector attaching after that - // reset — simulating a newly created screen/Activity instance — never sees the stale value. - // ============================================================================================= - @Test fun `Error does not leak to a fresh collector after being consumed`() = runBlocking { `when`(mockFirebaseAuth.currentUser).thenReturn(null) - // Simulate a screen observing Error and then consuming it, as the fixed screens now do. authUI.updateAuthState(AuthState.Error(Exception("boom"))) authUI.updateAuthState(AuthState.Idle) - // A brand-new collector (simulating a freshly created Activity) must see Idle. assertThat(authUI.authStateFlow().first()).isEqualTo(AuthState.Idle) } @@ -365,9 +354,6 @@ class FirebaseAuthUIAuthStateTest { runBlocking { `when`(mockFirebaseAuth.currentUser).thenReturn(null) - // No consuming reset here — this documents the pre-fix behavior so the underlying - // combine()-prefers-non-Idle mechanism (FirebaseAuthUI.authStateFlow()) doesn't - // silently change without a corresponding consume step in the screens. authUI.updateAuthState(AuthState.Error(Exception("boom"))) assertThat(authUI.authStateFlow().first()).isInstanceOf(AuthState.Error::class.java) diff --git a/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt b/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt index 219da7061..2d7c8c724 100644 --- a/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt @@ -12,16 +12,6 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config -/** - * Unit tests for [TopLevelDialogController] and [rememberTopLevelDialogController]. - * - * These cover the fix for a bug where keying `remember` on the live `authState` value recreated - * the controller (and wiped its `shownErrorStates` de-duplication set) on every state change — - * which, combined with screens resetting `AuthState` back to `Idle` immediately after consuming - * an `Error`, would tear down and discard the just-shown dialog on the very next recomposition. - * - * @suppress Internal test class - */ @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.NONE, sdk = [34]) class TopLevelDialogControllerTest { @@ -52,9 +42,6 @@ class TopLevelDialogControllerTest { composeTestRule.waitForIdle() composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertExists() - // Simulate what the fixed screens now do immediately after showing the dialog: reset - // the shared auth state back to Idle. If the controller were still keyed on authState, - // this recomposition would tear it down (and the dialog with it). composeTestRule.runOnIdle { state = AuthState.Idle } @@ -90,9 +77,6 @@ class TopLevelDialogControllerTest { composeTestRule.waitForIdle() composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertDoesNotExist() - // Calling showErrorDialog again for the *same* Error instance (state hasn't changed) - // must be a no-op — the de-dup set persists across calls since the controller is not - // recreated between them. composeTestRule.runOnIdle { controller.showErrorDialog(exception = exception) } From a6848c219da0e5b94aa10500bdc610530575e83b Mon Sep 17 00:00:00 2001 From: demolaf Date: Thu, 23 Jul 2026 10:04:29 +0100 Subject: [PATCH 6/7] updates --- .../auth/ui/components/TopLevelDialogController.kt | 4 ++++ .../ui/auth/ui/screens/FirebaseAuthScreen.kt | 2 ++ .../ui/auth/ui/screens/email/EmailAuthScreen.kt | 4 ++++ .../ui/auth/ui/screens/phone/PhoneAuthScreen.kt | 3 +++ .../firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt | 6 ++++++ .../ui/components/TopLevelDialogControllerTest.kt | 12 ++++++++++++ 6 files changed, 31 insertions(+) diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt b/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt index 9c5dfe329..cf41a3c5c 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/components/TopLevelDialogController.kt @@ -162,6 +162,10 @@ class TopLevelDialogController( /** * Creates and remembers a [TopLevelDialogController]. + * + * [authState] is a lambda rather than a snapshot value so the controller can read the + * live auth state on every [TopLevelDialogController.showErrorDialog] call without being + * recreated (and losing its de-duplication history) whenever the auth state changes. */ @Composable fun rememberTopLevelDialogController( diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt index 58d6d93e6..c213b56eb 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/FirebaseAuthScreen.kt @@ -542,6 +542,7 @@ fun FirebaseAuthScreen( // Keep external cancellation reporting centralized here so child screens // can handle local navigation without triggering duplicate callbacks. onSignInCancelled() + // Consumed so this doesn't leak to a freshly created screen. authUI.updateAuthState(AuthState.Idle) } @@ -633,6 +634,7 @@ fun FirebaseAuthScreen( // Dialog dismissed } ) + // Consumed immediately so this doesn't leak to a freshly created screen. authUI.updateAuthState(AuthState.Idle) } } diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt index 3146a0ecf..3de1876c6 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/email/EmailAuthScreen.kt @@ -169,6 +169,8 @@ fun EmailAuthScreen( val errorMessage = if (authState is AuthState.Error) (authState as AuthState.Error).exception.message else null + // Latched locally since these get consumed (reset to Idle) below — deriving directly from + // authState would close ResetPasswordUI/SignInEmailLinkUI's dialogs as soon as it resets. var resetLinkSentLocal by rememberSaveable { mutableStateOf(false) } var emailSignInLinkSentLocal by rememberSaveable { mutableStateOf(false) } @@ -231,11 +233,13 @@ fun EmailAuthScreen( // Dialog dismissed } ) + // Consumed immediately so this doesn't leak to a freshly created screen. authUI.updateAuthState(AuthState.Idle) } is AuthState.Cancelled -> { onCancel() + // Consumed so this doesn't leak to a freshly created screen. authUI.updateAuthState(AuthState.Idle) } diff --git a/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt b/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt index fa6617a82..6da6e5465 100644 --- a/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt +++ b/auth/src/main/java/com/firebase/ui/auth/ui/screens/phone/PhoneAuthScreen.kt @@ -196,6 +196,7 @@ fun PhoneAuthScreen( pendingVerificationPhoneNumber.value = null verificationStartTime.value = null + // Consumed before the async sign-in call so it can't be clobbered by that call's own state. authUI.updateAuthState(AuthState.Idle) coroutineScope.launch { @@ -230,11 +231,13 @@ fun PhoneAuthScreen( // Dialog dismissed } ) + // Consumed immediately so this doesn't leak to a freshly created screen. authUI.updateAuthState(AuthState.Idle) } is AuthState.Cancelled -> { onCancel() + // Consumed so this doesn't leak to a freshly created screen. authUI.updateAuthState(AuthState.Idle) } diff --git a/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt b/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt index 469c012a3..78e7f0dd3 100644 --- a/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/FirebaseAuthUIAuthStateTest.kt @@ -318,6 +318,10 @@ class FirebaseAuthUIAuthStateTest { assertThat(states[2]).isEqualTo(AuthState.Cancelled) // After second update } + // ============================================================================================= + // Stale one-off AuthState regression tests + // ============================================================================================= + @Test fun `Error does not leak to a fresh collector after being consumed`() = runBlocking { `when`(mockFirebaseAuth.currentUser).thenReturn(null) @@ -325,6 +329,7 @@ class FirebaseAuthUIAuthStateTest { authUI.updateAuthState(AuthState.Error(Exception("boom"))) authUI.updateAuthState(AuthState.Idle) + // A brand-new collector (simulating a freshly created Activity) must see Idle. assertThat(authUI.authStateFlow().first()).isEqualTo(AuthState.Idle) } @@ -354,6 +359,7 @@ class FirebaseAuthUIAuthStateTest { runBlocking { `when`(mockFirebaseAuth.currentUser).thenReturn(null) + // No consuming reset here — documents the pre-fix leaking behavior. authUI.updateAuthState(AuthState.Error(Exception("boom"))) assertThat(authUI.authStateFlow().first()).isInstanceOf(AuthState.Error::class.java) diff --git a/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt b/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt index 2d7c8c724..0d43e9750 100644 --- a/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt +++ b/auth/src/test/java/com/firebase/ui/auth/ui/components/TopLevelDialogControllerTest.kt @@ -12,6 +12,16 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config +/** + * Unit tests for [TopLevelDialogController] and [rememberTopLevelDialogController]. + * + * These cover the fix for a bug where keying `remember` on the live `authState` value recreated + * the controller (and wiped its `shownErrorStates` de-duplication set) on every state change — + * which, combined with screens resetting `AuthState` back to `Idle` immediately after consuming + * an `Error`, would tear down and discard the just-shown dialog on the very next recomposition. + * + * @suppress Internal test class + */ @RunWith(RobolectricTestRunner::class) @Config(manifest = Config.NONE, sdk = [34]) class TopLevelDialogControllerTest { @@ -42,6 +52,7 @@ class TopLevelDialogControllerTest { composeTestRule.waitForIdle() composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertExists() + // Mirrors the fixed screens resetting authState right after showing the dialog. composeTestRule.runOnIdle { state = AuthState.Idle } @@ -77,6 +88,7 @@ class TopLevelDialogControllerTest { composeTestRule.waitForIdle() composeTestRule.onNodeWithText(stringProvider.errorDialogTitle).assertDoesNotExist() + // Same Error instance again — must be a no-op, the de-dup set persists across calls. composeTestRule.runOnIdle { controller.showErrorDialog(exception = exception) } From 07af5ec988e453d6580bd0173ec230066e6988ea Mon Sep 17 00:00:00 2001 From: demolaf Date: Thu, 23 Jul 2026 17:57:17 +0100 Subject: [PATCH 7/7] test(auth): fix e2e tests exposed by the auth-state reset now working correctly --- .../ui/screens/AnonymousAuthScreenTest.kt | 34 ++++++++++------ .../ui/auth/ui/screens/EmailAuthScreenTest.kt | 40 ++++++++++++++----- 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/AnonymousAuthScreenTest.kt b/e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/AnonymousAuthScreenTest.kt index 743a26db4..6ef77c0b5 100644 --- a/e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/AnonymousAuthScreenTest.kt +++ b/e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/AnonymousAuthScreenTest.kt @@ -310,9 +310,13 @@ class AnonymousAuthScreenTest { } var currentAuthState: AuthState = AuthState.Idle + var capturedFailure: AuthException? = null composeTestRule.setContent { - TestAuthScreen(configuration = configuration) + TestAuthScreen( + configuration = configuration, + onSignInFailure = { capturedFailure = it }, + ) val authState by authUI.authStateFlow().collectAsState(AuthState.Idle) currentAuthState = authState } @@ -382,12 +386,18 @@ class AnonymousAuthScreenTest { composeTestRule.waitForIdle() shadowOf(Looper.getMainLooper()).idle() - // Step 5: Wait for error state (AccountLinkingRequiredException) + // Step 5: Wait for onSignInFailure to fire with AccountLinkingRequiredException. + // + // This is captured via the onSignInFailure callback rather than polling authStateFlow(): + // the screen resets AuthState back to Idle immediately after consuming the Error (so a + // second, independent authStateFlow() collector — like polling currentAuthState here — + // can miss the transient value entirely per StateFlow's conflation contract), whereas + // onSignInFailure is a direct, synchronous call from the same effect, so it can't race. println("TEST: Waiting for AccountLinkingRequiredException...") composeTestRule.waitUntil(timeoutMillis = AUTH_STATE_WAIT_TIMEOUT_MS) { shadowOf(Looper.getMainLooper()).idle() - println("TEST: Auth state: $currentAuthState") - currentAuthState is AuthState.Error + println("TEST: Captured failure: $capturedFailure") + capturedFailure != null } // Step 6: Verify ErrorRecoveryDialog is displayed @@ -396,24 +406,22 @@ class AnonymousAuthScreenTest { .assertIsDisplayed() // Verify exception - assertThat(currentAuthState).isInstanceOf(AuthState.Error::class.java) - val errorState = currentAuthState as AuthState.Error - assertThat(errorState.exception).isInstanceOf(AuthException.AccountLinkingRequiredException::class.java) + assertThat(capturedFailure).isInstanceOf(AuthException.AccountLinkingRequiredException::class.java) - val linkingException = errorState.exception as AuthException.AccountLinkingRequiredException + val linkingException = capturedFailure as AuthException.AccountLinkingRequiredException assertThat(linkingException.email).isEqualTo(email) } @Composable - private fun TestAuthScreen(configuration: AuthUIConfiguration) { - composeTestRule.waitForIdle() - shadowOf(Looper.getMainLooper()).idle() - + private fun TestAuthScreen( + configuration: AuthUIConfiguration, + onSignInFailure: (AuthException) -> Unit = {}, + ) { FirebaseAuthScreen( configuration = configuration, authUI = authUI, onSignInSuccess = { result -> }, - onSignInFailure = { exception: AuthException -> }, + onSignInFailure = onSignInFailure, onSignInCancelled = {}, authenticatedContent = { state, uiContext -> when (state) { diff --git a/e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/EmailAuthScreenTest.kt b/e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/EmailAuthScreenTest.kt index d438eb45b..7ca09692c 100644 --- a/e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/EmailAuthScreenTest.kt +++ b/e2eTest/src/test/java/com/firebase/ui/auth/ui/screens/EmailAuthScreenTest.kt @@ -20,6 +20,7 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsNotDisplayed import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollTo @@ -28,6 +29,7 @@ import androidx.credentials.CreatePasswordRequest import androidx.credentials.CredentialManager import androidx.credentials.GetCredentialRequest import androidx.credentials.GetCredentialResponse +import androidx.credentials.exceptions.NoCredentialException import androidx.test.core.app.ActivityScenario import androidx.test.core.app.ApplicationProvider import com.firebase.ui.auth.AuthState @@ -138,6 +140,11 @@ class EmailAuthScreenTest { fun tearDown() { closeable.close() + // Sign out first: the FirebaseAuth SDK instance backing authUI isn't recreated until the + // next test's setUp() deletes/reinitializes the FirebaseApp, so a session left signed in + // here would otherwise still be live when the next test's composition starts. + authUI.auth.signOut() + // Clean up after each test to prevent test pollution FirebaseAuthUI.clearInstanceCache() @@ -585,22 +592,24 @@ class EmailAuthScreenTest { shadowOf(Looper.getMainLooper()).idle() composeAndroidTestRule.waitForIdle() - // Wait for auth state to transition to EmailSignInLinkSent - println("TEST: Waiting for auth state change... Current state: $currentAuthState") + // Wait for the "email link sent" dialog to appear, rather than polling currentAuthState: + // the screen resets AuthState back to Idle immediately after consuming + // EmailSignInLinkSent (so a second, independent authStateFlow() collector — like + // currentAuthState here — can miss the transient value entirely per StateFlow's + // conflation contract), whereas the dialog's visibility is latched in local Compose + // state that isn't reset the same way, so it's a reliable, non-racy signal. + println("TEST: Waiting for email link sent dialog...") composeAndroidTestRule.waitUntil(timeoutMillis = AUTH_STATE_WAIT_TIMEOUT_MS) { shadowOf(Looper.getMainLooper()).idle() - println("TEST: Auth state during wait: $currentAuthState") - currentAuthState is AuthState.EmailSignInLinkSent + composeAndroidTestRule.onAllNodesWithText(stringProvider.emailSignInLinkSentDialogTitle) + .fetchSemanticsNodes().isNotEmpty() } // Ensure final recomposition is complete before assertions shadowOf(Looper.getMainLooper()).idle() composeAndroidTestRule.waitForIdle() - // Verify the auth state and user properties - println("TEST: Verifying auth state: $currentAuthState") - assertThat(currentAuthState) - .isInstanceOf(AuthState.EmailSignInLinkSent::class.java) + // Verify the dialog and user properties assertThat(authUI.auth.currentUser).isNull() composeAndroidTestRule.onNodeWithText(stringProvider.emailSignInLinkSentDialogTitle) .assertIsDisplayed() @@ -842,9 +851,15 @@ class EmailAuthScreenTest { whenever(mockCredentialManager.createCredential(any(), any())) .thenReturn(mock()) - // Mock successful credential retrieval + // No credential exists yet for this account (sign-up hasn't happened), so the very first + // mount's auto-retrieval attempt must find nothing rather than "succeed" with a credential + // for an account that doesn't exist — otherwise it triggers a real sign-in failure (and, + // now that dialogs correctly persist, a real error dialog) before step 1 even runs. + // thenAnswer (not thenThrow) since getCredential's suspend-compiled signature doesn't + // declare GetCredentialException, which Mockito otherwise rejects as an invalid checked + // exception for the method. whenever(mockCredentialManager.getCredential(any(), any())) - .thenReturn(mockCredentialResponse) + .thenAnswer { throw NoCredentialException() } val configuration = authUIConfiguration { context = applicationContext @@ -906,6 +921,11 @@ class EmailAuthScreenTest { verify(mockCredentialManager, times(1)).createCredential(any(), any()) println("TEST: Sign-up complete, credentials saved (createCredential called once)") + // Now that the account actually exists, retrieval can start "succeeding" — matching the + // real scenario this test verifies (auto-sign-in via a previously-saved credential). + whenever(mockCredentialManager.getCredential(any(), any())) + .thenReturn(mockCredentialResponse) + // STEP 2: Sign out println("TEST: Signing out...") authUI.auth.signOut()