Skip to content

fix(auth): fix "Already resumed" crash in phone auth SMS auto-verification #2447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
demolaf wants to merge 1 commit into
base: version-10.0.0-beta04
Choose a base branch
from
Open

fix(auth): fix "Already resumed" crash in phone auth SMS auto-verification #2447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions auth/src/main/java/com/firebase/ui/auth/FirebaseAuthUI.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.callbackFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.tasks.await
import java.util.concurrent.ConcurrentHashMap

Expand Down Expand Up @@ -366,6 +367,16 @@ class FirebaseAuthUI private constructor(
_authStateFlow.value = state
}

/**
* Clears a pending [AuthState.Loading] by resetting to [AuthState.Idle]. States written by
* another operation in the meantime are left untouched.
*/
internal fun clearLoadingState() {

@russellwheatley russellwheatley Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only skips non-Loading states, so a newer Loading from a resend or another in-flight call would get reset to Idle. The new test covers PasswordResetLinkSent, not a second Loading. Can you key this on the Loading this call actually wrote, or drop the "left untouched" claim in the doc comment if that's not actually guaranteed?

_authStateFlow.update { current ->
if (current is AuthState.Loading) AuthState.Idle else current
}
}

internal fun updateAuthStateWithResult(result: AuthResult?, defaultIsNewUser: Boolean = false) {
val user = result?.user
if (user != null) {
Expand Down
35 changes: 33 additions & 2 deletions auth/src/main/java/com/firebase/ui/auth/configuration/auth_provider/AuthProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ import com.google.firebase.auth.actionCodeSettings
import kotlinx.coroutines.suspendCancellableCoroutine
import kotlinx.coroutines.tasks.await
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicBoolean
import kotlin.coroutines.resume
import kotlin.coroutines.resumeWithException
import kotlin.coroutines.suspendCoroutine

@AuthUIConfigurationDsl
class AuthProvidersBuilder {
Expand Down Expand Up @@ -407,25 +407,56 @@ abstract class AuthProvider(open val providerId: String, open val providerName:
multiFactorSession: MultiFactorSession?,
isInstantVerificationEnabled: Boolean,
): VerifyPhoneNumberResult {
return suspendCoroutine { continuation ->
// Firebase can invoke more than one callback per request; only the first may
// resolve the continuation, so guard with a latch.
val isResolved = AtomicBoolean(false)
return suspendCancellableCoroutine { continuation ->
Comment thread
demolaf marked this conversation as resolved.
// Firebase can't unregister callbacks, so trip the latch on cancellation to
// keep any later callback off the continuation entirely.
continuation.invokeOnCancellation { isResolved.set(true) }
val options = PhoneAuthOptions.newBuilder(auth)
.setPhoneNumber(phoneNumber)
.requireSmsValidation(!isInstantVerificationEnabled)
.setTimeout(timeout, TimeUnit.SECONDS)
.setCallbacks(object :
PhoneAuthProvider.OnVerificationStateChangedCallbacks() {
override fun onVerificationCompleted(credential: PhoneAuthCredential) {
if (!isResolved.compareAndSet(false, true)) {

@russellwheatley russellwheatley Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This latch stops the crash, but it also drops the auto-retrieved credential on the exact path this PR is fixing. Firebase's SMS auto-retrieval order is onCodeSent then onVerificationCompleted, so the user lands on the code-entry screen after Firebase already verified them. The Verifier interface is single-shot, so you'll need a side channel (PhoneAuthScreen already signs in on SMSAutoVerified). Can you keep the latch for the crash, but still emit SMSAutoVerified when a late onVerificationCompleted arrives after onCodeSent?

Log.d(
"FirebaseAuthUI.verifyPhoneNumber",
"Dropping late onVerificationCompleted callback; " +
"verification was already resolved"
)
return
}
continuation.resume(VerifyPhoneNumberResult.AutoVerified(credential))
}

override fun onVerificationFailed(e: FirebaseException) {
if (!isResolved.compareAndSet(false, true)) {
Log.w(
"FirebaseAuthUI.verifyPhoneNumber",
"Dropping late onVerificationFailed callback; " +
"verification was already resolved",
e
)
return
}
continuation.resumeWithException(e)
}

override fun onCodeSent(
verificationId: String,
token: PhoneAuthProvider.ForceResendingToken,
) {
if (!isResolved.compareAndSet(false, true)) {
Log.d(
"FirebaseAuthUI.verifyPhoneNumber",
"Dropping late onCodeSent callback; verification was " +
"already resolved"
)
return
}
continuation.resume(
VerifyPhoneNumberResult.NeedsManualVerification(
verificationId,
Expand Down
12 changes: 5 additions & 7 deletions ...java/com/firebase/ui/auth/configuration/auth_provider/PhoneAuthProvider+FirebaseAuthUI.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ import kotlinx.coroutines.CancellationException
*
* @throws AuthException.InvalidCredentialsException if the phone number is invalid
* @throws AuthException.TooManyRequestsException if SMS quota is exceeded
* @throws AuthException.AuthCancelledException if the operation is cancelled
* @throws AuthException.NetworkException if a network error occurs
* @throws kotlinx.coroutines.CancellationException if the caller's coroutine is cancelled
*/
internal suspend fun FirebaseAuthUI.verifyPhoneNumber(
provider: AuthProvider.Phone,
Expand Down Expand Up @@ -136,12 +136,10 @@ internal suspend fun FirebaseAuthUI.verifyPhoneNumber(
}
}
} catch (e: CancellationException) {

@russellwheatley russellwheatley Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now the only provider that rethrows a raw CancellationException and resets to Idle. Everything else wraps it as AuthCancelledException and emits AuthState.Error. Rethrowing is closer to structured concurrency, but it's a one-off. Can you either match the other providers, or call out why phone verify is special?

val cancelledException = AuthException.AuthCancelledException(
message = "Verify phone number was cancelled",
cause = e
)
updateAuthState(AuthState.Error(cancelledException))
throw cancelledException
// User-initiated cancellation isn't an error: clear our own Loading, then rethrow so
// structured concurrency holds and no spurious Error reaches authStateFlow.
clearLoadingState()
throw e
} catch (e: AuthException) {
updateAuthState(AuthState.Error(e))
throw e
Expand Down
Loading