-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(auth): report Google/Facebook/OAuth/Anonymous sign-in failures via onSignInFailure #2433
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.
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
base: version-10.0.0-beta04
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ import kotlinx.coroutines.launch | |
| * @param config The [AuthUIConfiguration] containing authentication settings | ||
| * @param provider The [AuthProvider.Facebook] configuration with scopes and credential provider | ||
| * @param loginManagerProvider Provides logout operations to clear stale Facebook sessions | ||
| * @param onSignInFailure Callback invoked with the resulting [AuthException] on failure | ||
| * | ||
| * @return A launcher function that starts the Facebook sign-in flow when invoked | ||
| * | ||
|
|
@@ -58,6 +59,7 @@ internal fun FirebaseAuthUI.rememberSignInWithFacebookLauncher( | |
| config: AuthUIConfiguration, | ||
| provider: AuthProvider.Facebook, | ||
| loginManagerProvider: AuthProvider.Facebook.LoginManagerProvider = AuthProvider.Facebook.DefaultLoginManagerProvider(), | ||
| onSignInFailure: (AuthException) -> Unit = {}, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| ): () -> Unit { | ||
| val coroutineScope = rememberCoroutineScope() | ||
| val callbackManager = remember { CallbackManager.Factory.create() } | ||
|
|
@@ -87,9 +89,11 @@ internal fun FirebaseAuthUI.rememberSignInWithFacebookLauncher( | |
| } catch (e: AuthException) { | ||
| // Already an AuthException, don't re-wrap it | ||
| updateAuthState(AuthState.Error(e)) | ||
| onSignInFailure(e) | ||
| } catch (e: Exception) { | ||
| val authException = AuthException.from(e, context) | ||
| updateAuthState(AuthState.Error(authException)) | ||
| onSignInFailure(authException) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -106,6 +110,7 @@ internal fun FirebaseAuthUI.rememberSignInWithFacebookLauncher( | |
| authException | ||
| ) | ||
| ) | ||
| onSignInFailure(authException) | ||
| } | ||
| }) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ import kotlinx.coroutines.launch | |
| * @param context Android context for Credential Manager | ||
| * @param config Authentication UI configuration | ||
| * @param provider Google provider configuration with server client ID and optional scopes | ||
| * @param onSignInFailure Callback invoked with the resulting [AuthException] on failure | ||
| * @return A callback function that initiates Google Sign-In when invoked | ||
| * | ||
| * @see signInWithGoogle | ||
|
|
@@ -57,6 +58,7 @@ internal fun FirebaseAuthUI.rememberGoogleSignInHandler( | |
| context: Context, | ||
| config: AuthUIConfiguration, | ||
| provider: AuthProvider.Google, | ||
| onSignInFailure: (AuthException) -> Unit = {}, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| ): () -> Unit { | ||
| val coroutineScope = rememberCoroutineScope() | ||
| return remember(this, config) { | ||
|
|
@@ -66,9 +68,11 @@ internal fun FirebaseAuthUI.rememberGoogleSignInHandler( | |
| signInWithGoogle(context, config, provider) | ||
| } catch (e: AuthException) { | ||
| updateAuthState(AuthState.Error(e)) | ||
| onSignInFailure(e) | ||
| } catch (e: Exception) { | ||
| val authException = AuthException.from(e, context) | ||
| updateAuthState(AuthState.Error(authException)) | ||
| onSignInFailure(authException) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ import kotlinx.coroutines.tasks.await | |
| * | ||
| * @param config Authentication UI configuration | ||
| * @param provider OAuth provider configuration | ||
| * @param onSignInFailure Callback invoked with the resulting [AuthException] on failure | ||
| * | ||
| * @return Lambda that triggers OAuth sign-in when invoked | ||
| * | ||
|
|
@@ -54,6 +55,7 @@ internal fun FirebaseAuthUI.rememberOAuthSignInHandler( | |
| activity: Activity?, | ||
| config: AuthUIConfiguration, | ||
| provider: AuthProvider.OAuth, | ||
| onSignInFailure: (AuthException) -> Unit = {}, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| ): () -> Unit { | ||
| val coroutineScope = rememberCoroutineScope() | ||
| activity ?: throw IllegalStateException( | ||
|
|
@@ -73,9 +75,11 @@ internal fun FirebaseAuthUI.rememberOAuthSignInHandler( | |
| ) | ||
| } catch (e: AuthException) { | ||
| updateAuthState(AuthState.Error(e)) | ||
| onSignInFailure(e) | ||
| } catch (e: Exception) { | ||
| val authException = AuthException.from(e, context) | ||
| updateAuthState(AuthState.Error(authException)) | ||
| onSignInFailure(authException) | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
rememberblock on line 31 currently only keys onthis(remember(this)). Sinceconfig,onSignInFailure, andcontextare captured inside the lambda but not included in therememberkeys, any changes to these parameters (such as a new callback instance or updated configuration) will not be reflected in the returned launcher. This can lead to stale captures and memory leaks.\n\nPlease update therememberkeys to include these dependencies:\nkotlin\nreturn remember(this, context, config, onSignInFailure) {\n