Conversation
There was a problem hiding this comment.
Code Review
This pull request removes the dependency on the twitter_login package, transitioning the Twitter sign-in flow to use Firebase's native signInWithProvider on Android, iOS, and macOS. Consequently, apiKey and apiSecretKey are now optional and only required on Windows. The integration tests have been updated to mock the Firebase Auth provider instead of the third-party Twitter login client. Regarding the feedback, a critical issue was identified where a null auth.currentUser during a link or upgrade action would silently fail due to a null-shorting operator, leaving the UI hanging; adding an explicit null check and error notification is recommended.
| if (action == AuthAction.link || shouldUpgradeAnonymous) { | ||
| auth.currentUser | ||
| ?.linkWithProvider(firebaseAuthProvider) | ||
| .then(_onLinked) | ||
| .catchError(authListener.onError); | ||
| return; | ||
| } |
There was a problem hiding this comment.
If auth.currentUser is null when attempting to link or upgrade an anonymous user, the null-shorting operator (?.) will cause the entire chain to evaluate to null and do nothing. This leaves the UI hanging in a loading state indefinitely because authListener.onError is never called.
We should explicitly check if auth.currentUser is null and notify the listener of the error.
if (action == AuthAction.link || shouldUpgradeAnonymous) {
final currentUser = auth.currentUser;
if (currentUser == null) {
authListener.onError(StateError('No user is currently signed in to link with.'));
return;
}
currentUser
.linkWithProvider(firebaseAuthProvider)
.then(_onLinked)
.catchError(authListener.onError);
return;
}d240bed to
5b5edd6
Compare
twitter_login has not published since July 2023 and ships an Android build.gradle with no namespace that pins AGP 4.1.0, which capped this repo at AGP 8.7.3 and blocked #693 from using flutter_web_auth_2. TwitterProvider now signs in through auth.signInWithProvider on Android and iOS, mirroring AppleProvider, so Firebase performs the OAuth dance and the Twitter API key and secret move out of the app binary into the Firebase console. macOS and Windows keep the vendored OAuth 1.0a flow, which is why apiKey and apiSecretKey survive as optional parameters rather than being removed. macOS stays on the desktop flow because signInWithProvider is not available to it: FLTFirebaseAuthPlugin.swift carves out Apple and Game Center, then fails every other provider under `#if os(macOS)` with unsupported-platform. Android has no equivalent restriction. - AuthAction.none throws UnsupportedError on Android and iOS, since signInWithProvider cannot return a credential without also creating a session. - Anonymous users are upgraded with linkWithProvider so the anonymous uid survives sign in. - A debug-only diagnostic warns once when apiKey or apiSecretKey are passed on a platform that now ignores them. - Restores compileSdk to flutter.compileSdkVersion and bumps AGP to 8.9.1, now that nothing pins it. BREAKING CHANGE: consumers must set the Twitter app callback URL to the Firebase auth handler, add the Encoded App ID URL scheme on iOS, and register their SHA-1 on Android. AuthAction.none now throws on Android and iOS, and the credential passed to onCredentialLinked is a plain AuthCredential rather than an OAuthCredential.
5b5edd6 to
8d34ab7
Compare
Closes #696.
twitter_loginhas not published a release since July 2023, and its Androidbuild.gradledeclares nonamespacewhile pinning AGP 4.1.0. The namespace fix merged upstream in 2024 but was never released, which is whytests/androidwas held at AGP 8.7.3 with a hardcodedcompileSdk, and why #693 could not adoptflutter_web_auth_2.TwitterProvidernow signs in throughauth.signInWithProvideron Android and iOS, mirroringAppleProvider. Firebase performs the OAuth dance, so the Twitter API key and secret move out of the app binary and into the Firebase console. macOS and Windows keep the vendored OAuth 1.0a flow, which is whyapiKeyandapiSecretKeyremain, now as optional parameters. Withtwitter_logingone,compileSdkreturns toflutter.compileSdkVersionand AGP moves to 8.9.1, which unblocks #693.Neither desktop platform can use
signInWithProvider. On Windows the C++ SDK rejects it outright (firebase/flutterfire#13231). On macOS the generic OAuth flow does not exist in the Firebase Apple SDK at all:FIROAuthProvider.getCredentialWith(_:)is declared inside#if os(iOS)in firebase-ios-sdk 12.18.0, as is theAuthUIDelegatepresentation layer behind it, and that has been true continuously since 9.5.0.FLTFirebaseAuthPlugin.swift's#if os(macOS)branch returningunsupported-platformis the visible symptom rather than the cause, and it cannot simply be removed, since there is no symbol for the plugin to call. Apple sign-in works on macOS only because it takes a separatelaunchAppleSignInRequestpath built onASAuthorization. What does work on macOS is the credential path,signInWithCredentialand friends, which carry no such restriction, and that is exactly what the vendored OAuth 1.0a flow uses.https://<project>.firebaseapp.com/__/auth/handler, add the Encoded App ID URL scheme on iOS, and register their SHA-1 on Android. Nothing stops compiling, so a debug-only diagnostic warns once whenapiKeyorapiSecretKeyare passed on a platform that now ignores them.AuthAction.nonethrowsUnsupportedErroron Android and iOS, because a credential cannot be obtained without also creating a session, and the credential handed toonCredentialLinkedis now a plainAuthCredentialrather than anOAuthCredential. macOS and Windows behaviour is unchanged.Still to do
Draft until these land:
docs/firebase-ui-auth/providers/oauth.md, covering the callback URL, URL scheme and SHA-1, and dropping thetwitter_logininstall step.firebase_ui_authexample's iOSInfo.plist.