fix(ui_oauth): replace desktop_webview_auth with flutter_web_auth_2 - #693
fix(ui_oauth): replace desktop_webview_auth with flutter_web_auth_2#693demolaf wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the dependency on desktop_webview_auth with flutter_web_auth_2 and introduces custom OAuth implementations for Google, Facebook, and Twitter sign-in flows on desktop. The review feedback highlights several important issues: a potential CSRF vulnerability in the Facebook sign-in flow due to unvalidated state parameters, a missing 'W' in the nonce generation character set, unsafe type casting of query parameters in the Twitter sign-in flow, and incorrect configuration of FlutterWebAuth2Options for non-HTTPS redirect URIs.
desktop_webview_auth is unmaintained and its archived repo cannot ship a Package.swift for iOS, which breaks Swift Package Manager builds for any app depending on firebase_ui_auth or firebase_ui_oauth. Rewires the desktop OAuth sign-in flow in firebase_ui_oauth to use flutter_web_auth_2 instead, vendoring the provider URL-building and callback-parsing logic that desktop_webview_auth previously supplied so the google/facebook/twitter provider packages need no changes.
A fresh review of the flutter_web_auth_2 migration found two bugs. flutter_web_auth_2 only matches an https callback URL by host/path from macOS 14.4 onward; below that it completes on the first https navigation it sees, well before the OAuth provider's real redirect, so Facebook/Twitter desktop sign-in would silently break on older macOS. desktopSignIn now checks the OS version and fails loudly instead. The vendored Twitter OAuth1.0a access-token exchange also signed requests with an empty token secret, since the request token's secret was discarded instead of threaded through to the signing step, and a denied-consent callback crashed on an unsafe cast instead of cancelling cleanly. Both are fixed.
packages/firebase_ui_oauth/example/macos still targeted macOS 10.12, stale against the installed Flutter SDK's own CocoaPods requirements and below the minimum current Xcode toolchains support, so flutter build macos failed at pod install regardless of any other dependency. Bumped the Podfile and Xcode project deployment target to 12.0 to match.
Addresses gemini-code-assist findings on #693: FacebookSignInArgs generated a state nonce but never checked it against the callback, leaving the flow open to CSRF; authorizeFromCallback now rejects a mismatched or missing state before accepting the result. Also fixes the nonce character set (was missing the letter W), and stops passing FlutterWebAuth2Options.httpsHost/httpsPath for a non-https redirectUri (e.g. Twitter's custom-scheme callback), which would otherwise pass HTTPS-only options for a URL that has neither.
6f164cc to
9cda555
Compare
russellwheatley
left a comment
There was a problem hiding this comment.
Logic looks solid, the state validation, nonce charset, and Twitter token-secret threading all check out against your fix commits. Requesting changes mainly for the android e2e failure: flutter_web_auth_2 pulls in androidx.browser:browser:1.9.0 transitively, which needs AGP 8.9.1+, but tests/android/settings.gradle is still pinned to 8.7.3, so checkDebugAarMetadata fails on this branch. Left two inline notes too.
… message flutter_web_auth_2 pulls in androidx.browser:browser:1.9.0 on Android, which requires Android Gradle Plugin 8.9.1+. tests/android is intentionally pinned to AGP 8.7.3 pending a twitter_login release (see the compileSdk comment in tests/android/app/build.gradle), so bumping AGP isn't safe here. Force androidx.browser down to 1.8.0 instead, which doesn't carry that requirement. Also stops swallowing the underlying HTTP failure when Twitter's request-token exchange fails, per review feedback.
Forcing androidx.browser alone wasn't enough: flutter_web_auth_2 also directly depends on androidx.activity:activity-ktx 1.10.1, which pulls in an androidx.core version that also requires AGP 8.9.1+. Confirmed twitter_login still has no pub.dev release with the AGP-compatibility fix (namespace declaration merged upstream in 2024, never published; pub.dev still serves 4.4.2), so AGP stays at 8.7.3. Force the whole androidx.browser/activity/core set down to older, mutually-compatible versions instead.
Fixes #454.
firebase_ui_oauth depended on desktop_webview_auth for desktop (macOS/Linux/Windows) OAuth sign-in. That package lives in the archived invertase/flutter_desktop_webview_auth repo and ships an iOS platform entry with no Package.swift, which breaks Swift Package Manager builds for any app that pulls in firebase_ui_auth or firebase_ui_oauth. A fix exists upstream (invertase/flutter_desktop_webview_auth#73) but can't be merged since the repo is archived.
This replaces desktop_webview_auth with flutter_web_auth_2, which is actively maintained and already ships Package.swift for both iOS and macOS. The desktop OAuth URL-building and callback-parsing logic that desktop_webview_auth previously supplied (including Twitter's OAuth 1.0a request-token signing) is vendored locally in firebase_ui_oauth, so the google/facebook/twitter provider packages needed no changes.
Preview