Skip to content

Commit c4f4314

Browse files
committed
fix(auth): route Google sign-in cancellation to AuthState.Cancelled instead of Error
1 parent 8b18a4f commit c4f4314

4 files changed

Lines changed: 65 additions & 82 deletions

File tree

auth/src/main/java/com/firebase/ui/auth/configuration/auth_provider/GoogleAuthProvider+FirebaseAuthUI.kt

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import androidx.compose.runtime.Composable
66
import androidx.compose.runtime.remember
77
import androidx.compose.runtime.rememberCoroutineScope
88
import androidx.credentials.CredentialManager
9+
import androidx.credentials.exceptions.GetCredentialCancellationException
910
import androidx.credentials.exceptions.GetCredentialException
1011
import androidx.credentials.exceptions.NoCredentialException
1112
import com.firebase.ui.auth.AuthException
@@ -94,7 +95,9 @@ internal fun FirebaseAuthUI.rememberGoogleSignInHandler(
9495
* **Error Handling:**
9596
* - [GoogleIdTokenParsingException]: Library version mismatch
9697
* - [NoCredentialException]: No Google accounts on device
97-
* - [GetCredentialException]: User cancellation, configuration errors, or no credentials
98+
* - [GetCredentialCancellationException]: User dismissed the Credential Manager sheet -
99+
* updates [AuthState.Cancelled] and does not throw
100+
* - [GetCredentialException]: Configuration errors or no credentials
98101
* - Configuration errors trigger detailed developer guidance logs
99102
*
100103
* @param context Android context for Credential Manager
@@ -214,6 +217,13 @@ internal suspend fun FirebaseAuthUI.signInWithGoogle(
214217
// Re-throw to let UI handle the account linking flow
215218
updateAuthState(AuthState.Error(e))
216219
throw e
220+
} catch (e: GetCredentialCancellationException) {
221+
// User dismissed the Credential Manager sheet - this is a normal user action,
222+
// not an error, so it goes to AuthState.Cancelled instead of AuthState.Error.
223+
// Swallow (don't rethrow) so rememberGoogleSignInHandler's catch block doesn't
224+
// overwrite this state with AuthState.Error.
225+
updateAuthState(AuthState.Cancelled)
226+
217227
} catch (e: CancellationException) {
218228
val cancelledException = AuthException.AuthCancelledException(
219229
message = "Sign in with google was cancelled",

auth/src/main/java/com/firebase/ui/auth/ui/components/ErrorRecoveryDialog.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ fun ErrorRecoveryDialog(
130130
* @param stringProvider The [AuthUIStringProvider] for localized strings
131131
* @return The localized recovery message
132132
*/
133-
private fun getRecoveryMessage(
133+
internal fun getRecoveryMessage(
134134
error: AuthException,
135135
stringProvider: AuthUIStringProvider
136136
): String {
@@ -202,12 +202,12 @@ private fun getRecoveryMessage(
202202
* @param stringProvider The [AuthUIStringProvider] for localized strings
203203
* @return The localized action text
204204
*/
205-
private fun getRecoveryActionText(
205+
internal fun getRecoveryActionText(
206206
error: AuthException,
207207
stringProvider: AuthUIStringProvider
208208
): String {
209209
return when (error) {
210-
is AuthException.AuthCancelledException -> error.message ?: stringProvider.continueText
210+
is AuthException.AuthCancelledException -> stringProvider.continueText
211211
is AuthException.EmailAlreadyInUseException -> stringProvider.signInDefault // Use existing "Sign in" text
212212
is AuthException.AccountLinkingRequiredException -> stringProvider.signInDefault // User needs to sign in to link accounts
213213
is AuthException.DifferentSignInMethodRequiredException ->
@@ -236,7 +236,7 @@ private fun getRecoveryActionText(
236236
* @param error The [AuthException] to check
237237
* @return `true` if the error is recoverable, `false` otherwise
238238
*/
239-
private fun isRecoverable(error: AuthException): Boolean {
239+
internal fun isRecoverable(error: AuthException): Boolean {
240240
return when (error) {
241241
is AuthException.NetworkException -> true
242242
is AuthException.InvalidCredentialsException -> true

auth/src/test/java/com/firebase/ui/auth/configuration/auth_provider/GoogleAuthProviderFirebaseAuthUITest.kt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package com.firebase.ui.auth.configuration.auth_provider
1717
import android.content.Context
1818
import androidx.core.net.toUri
1919
import androidx.credentials.CredentialManager
20+
import androidx.credentials.exceptions.GetCredentialCancellationException
2021
import androidx.test.core.app.ApplicationProvider
2122
import com.firebase.ui.auth.AuthException
2223
import com.firebase.ui.auth.AuthState
@@ -47,7 +48,9 @@ import org.mockito.Mockito.`when`
4748
import org.mockito.MockitoAnnotations
4849
import org.mockito.kotlin.any
4950
import org.mockito.kotlin.argumentCaptor
51+
import org.mockito.kotlin.doAnswer
5052
import org.mockito.kotlin.eq
53+
import org.mockito.kotlin.whenever
5154
import org.robolectric.RobolectricTestRunner
5255
import org.robolectric.annotation.Config
5356

@@ -539,6 +542,46 @@ class GoogleAuthProviderFirebaseAuthUITest {
539542
assertThat(errorState.exception).isInstanceOf(AuthException.AuthCancelledException::class.java)
540543
}
541544

545+
@Test
546+
fun `Sign in with Google when Credential Manager sheet is dismissed should update state to Cancelled without throwing`() = runTest {
547+
// GetCredentialCancellationException is a checked exception, so it must be stubbed via
548+
// doAnswer rather than thenThrow (which validates against the method's declared throws).
549+
doAnswer { throw GetCredentialCancellationException("User cancelled the selector") }
550+
.whenever(mockCredentialManagerProvider)
551+
.getGoogleCredential(
552+
context = eq(applicationContext),
553+
credentialManager = any<CredentialManager>(),
554+
serverClientId = eq("test-client-id"),
555+
filterByAuthorizedAccounts = eq(true),
556+
autoSelectEnabled = eq(false)
557+
)
558+
559+
val instance = FirebaseAuthUI.create(firebaseApp, mockFirebaseAuth)
560+
val googleProvider = AuthProvider.Google(
561+
serverClientId = "test-client-id",
562+
scopes = emptyList()
563+
)
564+
val config = authUIConfiguration {
565+
context = applicationContext
566+
providers {
567+
provider(googleProvider)
568+
}
569+
}
570+
571+
// Should not throw - user cancellation is not an error
572+
instance.signInWithGoogle(
573+
context = applicationContext,
574+
config = config,
575+
provider = googleProvider,
576+
authorizationProvider = mockAuthorizationProvider,
577+
credentialManagerProvider = mockCredentialManagerProvider
578+
)
579+
580+
// Verify state is Cancelled, not Error
581+
val finalState = instance.authStateFlow().first()
582+
assertThat(finalState).isEqualTo(AuthState.Cancelled)
583+
}
584+
542585
// =============================================================================================
543586
// signInWithGoogle - Anonymous Upgrade
544587
// =============================================================================================

auth/src/test/java/com/firebase/ui/auth/ui/components/ErrorRecoveryDialogLogicTest.kt

Lines changed: 7 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -185,9 +185,10 @@ class ErrorRecoveryDialogLogicTest {
185185
}
186186

187187
@Test
188-
fun `getRecoveryActionText returns continue for AuthCancelledException`() {
189-
// Arrange
190-
val error = AuthException.AuthCancelledException("Auth cancelled")
188+
fun `getRecoveryActionText returns continue for AuthCancelledException, ignoring the raw exception message`() {
189+
// Arrange - message mirrors the raw GetCredentialCancellationException message from GH #2422;
190+
// the action label must never surface this raw string to the user
191+
val error = AuthException.AuthCancelledException("User cancelled the selector")
191192

192193
// Act
193194
val actionText = getRecoveryActionText(error, mockStringProvider)
@@ -209,15 +210,15 @@ class ErrorRecoveryDialogLogicTest {
209210
}
210211

211212
@Test
212-
fun `getRecoveryActionText returns continue for AccountLinkingRequiredException`() {
213-
// Arrange
213+
fun `getRecoveryActionText returns sign in for AccountLinkingRequiredException`() {
214+
// Arrange - user needs to sign in with the existing method to link accounts
214215
val error = AuthException.AccountLinkingRequiredException("Account linking required")
215216

216217
// Act
217218
val actionText = getRecoveryActionText(error, mockStringProvider)
218219

219220
// Assert
220-
Truth.assertThat(actionText).isEqualTo("Continue")
221+
Truth.assertThat(actionText).isEqualTo("Sign in")
221222
}
222223

223224
@Test
@@ -308,75 +309,4 @@ class ErrorRecoveryDialogLogicTest {
308309
// Act & Assert
309310
Truth.assertThat(isRecoverable(error)).isTrue()
310311
}
311-
312-
// Helper functions to test the private functions - we need to make them internal for testing
313-
private fun getRecoveryMessage(error: AuthException, stringProvider: AuthUIStringProvider): String {
314-
return when (error) {
315-
is AuthException.NetworkException -> stringProvider.networkErrorRecoveryMessage
316-
is AuthException.InvalidCredentialsException -> {
317-
// Use the actual error message from Firebase if available, otherwise fallback to generic message
318-
error.message?.takeIf { it.isNotBlank() && it != "Invalid credentials provided" }
319-
?: stringProvider.invalidCredentialsRecoveryMessage
320-
}
321-
is AuthException.UserNotFoundException -> stringProvider.userNotFoundRecoveryMessage
322-
is AuthException.WeakPasswordException -> {
323-
val baseMessage = stringProvider.weakPasswordRecoveryMessage
324-
error.reason?.let { reason ->
325-
"$baseMessage\n\nReason: $reason"
326-
} ?: baseMessage
327-
}
328-
is AuthException.EmailAlreadyInUseException -> {
329-
val baseMessage = stringProvider.emailAlreadyInUseRecoveryMessage
330-
error.email?.let { email ->
331-
"$baseMessage ($email)"
332-
} ?: baseMessage
333-
}
334-
is AuthException.TooManyRequestsException -> stringProvider.tooManyRequestsRecoveryMessage
335-
is AuthException.MfaRequiredException -> stringProvider.mfaRequiredRecoveryMessage
336-
is AuthException.AccountLinkingRequiredException -> stringProvider.accountLinkingRequiredRecoveryMessage
337-
is AuthException.DifferentSignInMethodRequiredException ->
338-
error.message ?: stringProvider.accountLinkingRequiredRecoveryMessage
339-
is AuthException.AuthCancelledException -> stringProvider.authCancelledRecoveryMessage
340-
is AuthException.UnknownException -> stringProvider.unknownErrorRecoveryMessage
341-
else -> stringProvider.unknownErrorRecoveryMessage
342-
}
343-
}
344-
345-
private fun getRecoveryActionText(error: AuthException, stringProvider: AuthUIStringProvider): String {
346-
return when (error) {
347-
is AuthException.AuthCancelledException -> stringProvider.continueText
348-
is AuthException.EmailAlreadyInUseException -> stringProvider.signInDefault
349-
is AuthException.AccountLinkingRequiredException -> stringProvider.continueText
350-
is AuthException.DifferentSignInMethodRequiredException -> when (error.suggestedSignInMethod) {
351-
GoogleAuthProvider.PROVIDER_ID -> stringProvider.continueWithGoogle
352-
EmailAuthProvider.EMAIL_LINK_SIGN_IN_METHOD -> stringProvider.signInWithEmailLink
353-
else -> stringProvider.continueText
354-
}
355-
is AuthException.MfaRequiredException -> stringProvider.continueText
356-
is AuthException.NetworkException,
357-
is AuthException.InvalidCredentialsException,
358-
is AuthException.UserNotFoundException,
359-
is AuthException.WeakPasswordException,
360-
is AuthException.TooManyRequestsException,
361-
is AuthException.UnknownException -> stringProvider.retryAction
362-
else -> stringProvider.retryAction
363-
}
364-
}
365-
366-
private fun isRecoverable(error: AuthException): Boolean {
367-
return when (error) {
368-
is AuthException.NetworkException -> true
369-
is AuthException.InvalidCredentialsException -> true
370-
is AuthException.UserNotFoundException -> true
371-
is AuthException.WeakPasswordException -> true
372-
is AuthException.EmailAlreadyInUseException -> true
373-
is AuthException.TooManyRequestsException -> false
374-
is AuthException.MfaRequiredException -> true
375-
is AuthException.AccountLinkingRequiredException -> true
376-
is AuthException.DifferentSignInMethodRequiredException -> true
377-
is AuthException.AuthCancelledException -> true
378-
is AuthException.UnknownException -> true
379-
else -> true
380-
}
381-
}
382312
}

0 commit comments

Comments
 (0)