Repository navigation
fix(auth): offer no email recovery for account linking without an Email provider - #2551
Mastersam07 wants to merge 1 commit into
Conversation
…il provider The account-linking dialog's recover action always opened the email sign-in step. With no AuthProvider.Email configured, EmailAuthScreen called .first() on an empty provider list and crashed. Gate the recover action on an Email provider being configured (not on sign-up being offered, since linking signs in to an existing account), and make EmailAuthScreen render nothing instead of throwing when a host routes to it without one.
There was a problem hiding this comment.
Code Review
This pull request prevents crashes during account linking recovery when an Email provider is not configured. It introduces a check to verify if an Email provider is configured before attempting to navigate to the email sign-in step, and updates EmailAuthScreen to return early instead of crashing if no Email provider is found. Additionally, comprehensive unit tests have been added to verify these behaviors. I have no further feedback to provide.
|
These were recorded on an Android emulator (Pixel 6, API 33) using the configuration from #2550: Google and Facebook providers, no AuthProvider.Email. The account collision is simulated. A real Facebook login can't be scripted on an emulator, so a temporary demo screen has a button that sends the same AuthState.Error(AccountLinkingRequiredException) that the Facebook sign-in path sends when the email is already in use. Everything after that is the real library: the error dialog, its recover action, and the crash or the fix. The yellow banner and the button belong to that local demo screen and aren't part of this PR. The red circles mark taps. Before (master, 10.0.0) The dialog offers "Sign in". Tapping it opens the Email sign-in step, which crashes with NoSuchElementException: List is empty at EmailAuthScreen.kt:212, the same trace as the report. issue2550-before.mp4After (this PR) The dialog offers only "Dismiss". Tapping it closes the dialog, and the user stays on the sign-in screen. issue2550-after.mp4Side by side issue2550-side-by-side.mp4 |
Refs #2550
Problem
When a provider sign-in (for example Facebook) hits an existing account with the same email, the error dialog's recover action always opened the email sign-in step. If the app configures no
AuthProvider.Email,EmailAuthScreencalled.first()on an empty provider list and crashed withNoSuchElementException.Change
FirebaseAuthScreen: theAccountLinkingRequiredExceptionrecover action is offered only when an Email provider is configured. Without one, the dialog shows the error with no action. The check is the newisEmailProviderConfigured(), notisEmailSignUpOffered(): linking signs in to an account that already exists, so apps that turn off sign-up still get the recovery.EmailAuthScreen:.first()is nowfirstOrNull()with an early return, so a host that routes to this public composable without an Email provider gets an empty step rather than a crash.This is the crash guard only. Linking through the provider that owns the account (the 8.x
WelcomeBackIdpPromptbehaviour) and localizing the dialog message are tracked separately, which is why this PR refers to #2550 rather than closing it.Testing
New tests in
FirebaseAuthScreenEmailRecoveryTest:EmailAuthScreenrenders nothing without an Email provider instead of throwingWith the source change reverted, the two no-Email tests fail (one with the reported
NoSuchElementException)../scripts/build.sh(JDK 21): pass./gradlew lintAll: pass./gradlew :auth:testDebugUnitTest: 1104 tests, 0 failuresMaintainer note: Fixes internal CPRN-634