Skip to content

fix(auth): offer no email recovery for account linking without an Email provider - #2551

Open
Mastersam07 wants to merge 1 commit into
masterfrom
fix/account-linking-recover-without-email
Open

Mastersam07 wants to merge 1 commit into
masterfrom
fix/account-linking-recover-without-email

Conversation

@Mastersam07

Copy link
Copy Markdown
Contributor

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, EmailAuthScreen called .first() on an empty provider list and crashed with NoSuchElementException.

Change

  • FirebaseAuthScreen: the AccountLinkingRequiredException recover action is offered only when an Email provider is configured. Without one, the dialog shows the error with no action. The check is the new isEmailProviderConfigured(), not isEmailSignUpOffered(): linking signs in to an account that already exists, so apps that turn off sign-up still get the recovery.
  • EmailAuthScreen: .first() is now firstOrNull() 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 WelcomeBackIdpPrompt behaviour) 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:

  • no recover action for an account that needs linking when no Email provider is configured
  • the recover action is still offered when sign-up is turned off
  • EmailAuthScreen renders nothing without an Email provider instead of throwing

With 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 failures

Maintainer note: Fixes internal CPRN-634

…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.

@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 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.

@Mastersam07

Copy link
Copy Markdown
Contributor Author

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.mp4

After (this PR)

The dialog offers only "Dismiss". Tapping it closes the dialog, and the user stays on the sign-in screen.

issue2550-after.mp4

Side by side

issue2550-side-by-side.mp4

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.

1 participant