From 546041584f7a33bd903a7f5f7070ae9921ddfa5f Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Wed, 12 Aug 2026 23:36:10 +0200 Subject: [PATCH] Add Google sign-in support for the Firebase sync account One-tap sign-in for a fresh install, matching the pattern already shipped in todo and crdt_sync_dart: openFirebaseWithGoogle() (google_platform.dart abstracts the plugin's platform-channel dependence), isFirebaseConfigured() that reads the stored session rather than trusting the account marker alone (a marker with no live session is what made a synced phone read as "not connected" for a full session), and storedAccount() for read-back without the desktop-wrapper fallback that throws on Android. Also generalizes crdt_sync_dart's ref pin to v0.10.0 and vendors the override-guard script for the pre-commit hook already wired to it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KdSp6Nbrs9xvsWkCMDN9a1 --- .pre-commit-config.yaml | 8 + app/lib/screens/settings_screen.dart | 97 +++++++++++- app/lib/services/firebase_backend.dart | 152 ++++++++++++++++++- app/lib/services/google_platform.dart | 11 ++ app/lib/services/google_platform_io.dart | 10 ++ app/lib/services/google_platform_web.dart | 8 + app/lib/services/google_sign_in_backend.dart | 122 +++++++++++++++ app/pubspec.lock | 54 ++++++- app/pubspec.yaml | 8 +- app/test/firebase_backend_google_test.dart | 128 ++++++++++++++++ app/test/google_platform_test.dart | 14 ++ app/test/google_sign_in_backend_test.dart | 33 ++++ app/test/screens/settings_screen_test.dart | 11 ++ scripts/check_no_crdt_sync_override.sh | 88 +++++++++++ 14 files changed, 736 insertions(+), 8 deletions(-) create mode 100644 app/lib/services/google_platform.dart create mode 100644 app/lib/services/google_platform_io.dart create mode 100644 app/lib/services/google_platform_web.dart create mode 100644 app/lib/services/google_sign_in_backend.dart create mode 100644 app/test/firebase_backend_google_test.dart create mode 100644 app/test/google_platform_test.dart create mode 100644 app/test/google_sign_in_backend_test.dart create mode 100755 scripts/check_no_crdt_sync_override.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2541324..ae36157 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -143,6 +143,14 @@ repos: - repo: local hooks: + # Fails a commit that still points crdt_sync at a local path. Local + # tests would go green against a library CI has never seen, silently. + - id: crdt-sync-override + name: no local crdt_sync dependency_override + entry: bash scripts/check_no_crdt_sync_override.sh app/pubspec.yaml + language: system + pass_filenames: false + always_run: true # Every commit ships its own patch bump, so the release workflow's clean # vX.Y.Z tag tracks HEAD instead of whatever version was last hand-edited. # The Flutter app lives in app/, so point the script at its pubspec. diff --git a/app/lib/screens/settings_screen.dart b/app/lib/screens/settings_screen.dart index adb75e7..3ccc11c 100644 --- a/app/lib/screens/settings_screen.dart +++ b/app/lib/screens/settings_screen.dart @@ -15,6 +15,7 @@ import 'package:diet_guard_app/services/app_settings_service.dart'; import 'package:diet_guard_app/services/firebase_backend.dart'; import 'package:diet_guard_app/services/github_client_factory.dart'; import 'package:diet_guard_app/services/github_device_auth.dart'; +import 'package:diet_guard_app/services/google_sign_in_backend.dart'; import 'package:diet_guard_app/services/sync_health.dart'; import 'package:diet_guard_app/services/sync_service.dart'; import 'package:diet_guard_app/services/sync_settings.dart'; @@ -33,6 +34,10 @@ class SettingsScreen extends StatefulWidget { super.key, this.httpClient, this.requestBatteryExemption, + this.googleFirebaseFactory, + this.googleAvailable, + this.accountLoader, + this.sessionProbe, }); /// Injectable HTTP client; tests pass a [MockClient]. @@ -43,6 +48,27 @@ class SettingsScreen extends StatefulWidget { /// `Permission.ignoreBatteryOptimizations.request()`. final Future Function()? requestBatteryExemption; + /// Builds the Firebase backend via Google sign-in. Injected so tests need + /// no platform channel -- the plugin reaches the OS account picker. + final Future Function()? googleFirebaseFactory; + + /// Whether to offer the Google button. Defaults to what the platform + /// supports; injected by tests, whose host reports unsupported. + final bool? googleAvailable; + + /// Reads the stored Firebase account. Injected for the same reason as + /// [googleFirebaseFactory]: the keystore is a platform channel. + final Future Function()? accountLoader; + + /// Whether a Firebase session is stored. See [accountLoader]. + /// + /// Separate from [accountLoader] because the two answer different + /// questions: the account marker is bookkeeping, the session is the + /// credential. A device can hold the second without the first, and + /// reporting only the first is what made a syncing phone read as + /// "not connected". + final Future Function()? sessionProbe; + @override State createState() => _SettingsScreenState(); } @@ -91,10 +117,14 @@ class _SettingsScreenState extends State { // Reflect a previously-stored account, so a returning user sees the // real state instead of an empty form that looks unconfigured. final account = await loadAccount(); + // The stored session, not the account marker, decides "connected": a + // Google sign-in leaves a refresh token that authenticates every request + // even when no marker was written beside it. + final configured = await (widget.sessionProbe ?? isFirebaseConfigured)(); if (!mounted) return; if (account != null) _firebaseEmailController.text = account.email; setState(() { - _firebaseConfigured = account != null; + _firebaseConfigured = configured; _loading = false; }); } @@ -299,6 +329,59 @@ class _SettingsScreenState extends State { /// Signing in here rather than only saving means a typo surfaces now, on /// the screen where it can be fixed, instead of as a silently failed /// background sync hours later. + + /// Signs in by picking a Google account -- the one-tap path. + /// + /// A dismissed picker is not an error; a wrong-account sign-in reports why, + /// because that is the failure that otherwise looks like a working sync + /// which silently never syncs. + Future _connectGoogle() async { + setState(() { + _busy = true; + _status = 'Signing in...'; + }); + try { + final client = + await (widget.googleFirebaseFactory ?? openFirebaseWithGoogle)(); + if (!mounted) return; + if (client == null) { + setState(() { + _busy = false; + _status = 'Google sign-in was cancelled.'; + }); + return; + } + // openFirebaseWithGoogle stored the account under the email Firebase + // reported; reflect it rather than reading the (empty) form field. + // + // Read it back rather than trusting the returned client: this banner + // claimed "Connected to Firebase" on four apps that were not connected, + // because a non-null client only means the sign-in call succeeded in + // that moment, not that anything survives a restart. Reporting the + // persisted state is the whole point -- if the read-back is empty the + // next launch will sync over GitHub, and the user needs to know now. + final account = await (widget.accountLoader ?? storedAccount)(); + final configured = await (widget.sessionProbe ?? isFirebaseConfigured)(); + if (!mounted) return; + if (account != null) _firebaseEmailController.text = account.email; + setState(() { + _busy = false; + _firebaseConfigured = configured; + _status = configured + ? 'Connected to Firebase.' + : 'Signed in, but this device did not save the session - it will ' + 'sync over GitHub after a restart. Try connecting again.'; + }); + } on FirebaseAuthError catch (error) { + if (!mounted) return; + setState(() { + _busy = false; + _firebaseConfigured = false; + _status = error.message; + }); + } + } + Future _saveFirebaseAccount() async { final email = _firebaseEmailController.text.trim(); final password = _firebasePasswordController.text; @@ -451,6 +534,18 @@ class _SettingsScreenState extends State { icon: const Icon(Icons.cloud_done), label: const Text('Connect Firebase'), ), + // One tap, no typing -- the path that matters after a + // reinstall. Hidden where the platform has no programmatic + // Google flow (see google_platform.dart); a button that + // always failed would be worse than none. + if (widget.googleAvailable ?? googleSignInSupported) ...[ + const SizedBox(height: 12), + FilledButton.icon( + onPressed: _busy ? null : _connectGoogle, + icon: const Icon(Icons.account_circle), + label: const Text('Sign in with Google'), + ), + ], ], const SizedBox(height: 24), const Divider(), diff --git a/app/lib/services/firebase_backend.dart b/app/lib/services/firebase_backend.dart index 165cd78..7e55ec8 100644 --- a/app/lib/services/firebase_backend.dart +++ b/app/lib/services/firebase_backend.dart @@ -14,7 +14,9 @@ library; import 'dart:developer'; import 'package:crdt_sync/crdt_sync.dart'; +import 'package:diet_guard_app/services/google_sign_in_backend.dart'; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; +import 'package:http/http.dart' as http; /// The shared `kuhy-syncs` project. /// @@ -74,6 +76,17 @@ Future loadAccount() async { } } +/// Reads the account from the keystore only, with no wrapper fallback. +/// +/// [loadAccount] falls back to the desktop wrapper's `/sync-account` route +/// when the keystore is empty, which on Android resolves to `file:///` and +/// throws `No host specified in URI`. Callers reading back an account they +/// just wrote -- where a fallback would be wrong anyway -- use this instead. +/// Verified on the phone: without it, sign-in succeeded and then the settings +/// screen hung on "Signing in..." forever. +Future storedAccount() async => + FirebaseAccount.tryParse(await _secure.read(key: kFirebaseAccountKey)); + /// Stores the per-device account. Keystore only — never prefs, never source. Future saveAccount(FirebaseAccount account) => _secure.write(key: kFirebaseAccountKey, value: account.toJsonString()); @@ -92,12 +105,28 @@ Future clearAccount() async { /// token, so the usual path costs no authentication round trip. Future openFirebase() async { final account = await loadAccount(); - if (account == null) return null; + if (account == null) { + // A stored refresh token is a signed-in device, even with no account + // marker beside it. Treating the marker as the source of truth is what + // turned one missing write into "syncs over GitHub and 401s forever": + // the credential that actually authenticates was sitting in the keystore + // the whole time, unused. Ask the store directly before giving up. + return _clientFromStoredSession(); + } try { return await firebaseClientFor( config: kProject.configFor(account.email), store: credentialStore(), - password: account.password, + // A Google-provisioned account stores an empty password. + // Passing '' would make firebaseClientFor treat it as a usable + // credential and sign in with it, which fails; null correctly + // means "no password on this device". + password: account.password.isEmpty ? null : account.password, + // Deliberately NOT offering Google here. This path runs from background + // timers and, in some apps, before runApp -- offering it would let a + // non-interactive tick raise the OS account picker with no user action + // behind it. Interactive sign-in uses openFirebaseWithGoogle instead. + expectedUid: kSyncUid, ); } on Object catch (error, stackTrace) { // Unreachable keystore or a rejected sign-in must not fail the tick: the @@ -118,6 +147,125 @@ Future openFirebase() async { } } +/// Whether this device can actually authenticate against Firebase. +/// +/// True when either half of the state is present: the account marker +/// [loadAccount] reads, or a stored refresh token. The token is the half that +/// matters -- it is what signs requests -- so a settings screen that reports +/// the marker alone calls a working device "Not connected", which is how a +/// phone that was in fact syncing looked broken for a whole session. +Future isFirebaseConfigured() async { + try { + final auth = FirebaseTokenProvider( + apiKey: kProject.apiKey, + store: credentialStore(), + ); + if (await auth.hasSession()) return true; + // storedAccount, not loadAccount: on Android the latter falls back to the + // desktop wrapper's /sync-account route, which resolves to file:/// and + // throws "No host specified in URI" -- observed on the phone, where it + // turned a successful sign-in into "Google sign-in failed". + if (await storedAccount() == null) return false; + // A marker with no session behind it cannot sign a single request. Drop + // just the marker so the settings screen offers a sign-in instead of a + // dead banner -- not clearAccount(), which also sets the opt-out flag and + // would stop the desktop wrapper re-provisioning after the next sign-in. + await _secure.delete(key: kFirebaseAccountKey); + return false; + } on Object catch (error, stackTrace) { + log( + 'session probe failed; reporting this device as not configured', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return false; + } +} + +/// Returns a client built from the keystore's refresh token alone, or null. +/// +/// The recovery path for a device that has a live session but no account +/// marker -- the state a Google sign-in used to leave behind. Costs one +/// keystore read and no network round trip when there is no session, so it is +/// safe on the background-tick path that [openFirebase] also serves. +/// +/// Does not re-save the account: a device that reaches this is already signed +/// in, and writing a marker from a read path would hide the condition rather +/// than repair it. [openFirebaseWithGoogle] is what writes the marker. +Future _clientFromStoredSession() async { + try { + final auth = FirebaseTokenProvider( + apiKey: kProject.apiKey, + store: credentialStore(), + ); + if (!await auth.hasSession()) return null; + return FirebaseRestClient(databaseUrl: kProject.databaseUrl, auth: auth); + } on Object catch (error, stackTrace) { + // Same contract as openFirebase's own catch: an unreachable keystore + // degrades to the GitHub mirror instead of failing the tick, but says so. + log( + 'stored-session recovery failed; falling back to the GitHub mirror', + level: 1000, + error: error, + stackTrace: stackTrace, + ); + return null; + } +} + +/// Signs in with Google alone, for a device that has no account stored yet. +/// +/// This is the one-tap path: [openFirebase] needs an account in the keystore +/// to know which email to use, but a fresh install has none. The Google token +/// carries the identity, so nothing needs to be typed. +/// +/// The account is stored under the email **Firebase reports**, never one read +/// from the UI: a fresh install has no email anywhere on the device, so taking +/// it from a text field would persist an empty account and send the next +/// launch down the password path with ''. +/// +/// Returns null when the user dismisses the picker; throws [FirebaseAuthError] +/// when Google succeeds but resolves to a uid the security rules do not pin, +/// which would otherwise authenticate fine and then be denied every read and +/// write with no other symptom. +Future openFirebaseWithGoogle({ + Future Function()? tokenFetcher, + Future Function(FirebaseAccount)? accountSaver, + http.Client? httpClient, +}) async { + final token = await (tokenFetcher ?? googleIdToken)(); + if (token == null) return null; + final auth = FirebaseTokenProvider( + apiKey: kProject.apiKey, + store: credentialStore(), + httpClient: httpClient, + ); + final email = await auth.signInWithGoogle( + idToken: token, + expectedUid: kSyncUid, + ); + // The account must be saved unconditionally, and the signed-in email is not + // reliable enough to gate it on. `signInWithIdp` omits `email` whenever the + // Google account hides it, so gating the save on a non-null email produced + // the worst possible outcome: this call returned a perfectly working client + // -- "Connected to Firebase" -- while the device stayed unconfigured, so the + // next launch found no account, `openFirebase()` returned null, and + // `syncBackend()` silently dropped to GitHub-only and 401'd. The session is + // already durable at this point (`signInWithGoogle` stores the refresh token + // via `_adopt`); only this marker was missing, which is exactly why the + // failure survived a restart and looked like a sign-in that never happened. + // + // The email is a display convenience, not the credential -- the refresh + // token in the keystore is. An empty string is the honest value when Google + // withholds it, and `openFirebase()` already treats an empty password as + // "no password on this device" and authenticates from the stored token. + await (accountSaver ?? saveAccount)( + FirebaseAccount(email: email ?? '', password: ''), + ); + return FirebaseRestClient(databaseUrl: kProject.databaseUrl, auth: auth); +} + // coverage:ignore-end /// Wraps [github] so Firebase is primary and GitHub is kept as a mirror. diff --git a/app/lib/services/google_platform.dart b/app/lib/services/google_platform.dart new file mode 100644 index 0000000..f253b2c --- /dev/null +++ b/app/lib/services/google_platform.dart @@ -0,0 +1,11 @@ +/// Platform gate for the programmatic Google sign-in flow. +/// +/// Conditional export because the answer differs per platform *and* because +/// asking the plugin directly is unsafe: `supportsAuthenticate()` throws +/// `UnimplementedError` where no implementation is registered -- an `Error`, +/// not an `Exception`, so it escapes an ordinary catch and would take down +/// the settings screen's `build()`. +library; + +export 'google_platform_io.dart' + if (dart.library.js_interop) 'google_platform_web.dart'; diff --git a/app/lib/services/google_platform_io.dart b/app/lib/services/google_platform_io.dart new file mode 100644 index 0000000..7fa7a81 --- /dev/null +++ b/app/lib/services/google_platform_io.dart @@ -0,0 +1,10 @@ +/// Whether the programmatic Google sign-in flow exists on this platform. +/// +/// The io half: Android has the Credential Manager flow; Linux desktop does +/// not, and this app's desktop build is the web one anyway. +library; + +import 'dart:io'; + +/// True only on Android, the one platform shipping the programmatic flow. +bool get googleSignInSupported => Platform.isAndroid; diff --git a/app/lib/services/google_platform_web.dart b/app/lib/services/google_platform_web.dart new file mode 100644 index 0000000..9e7ccd6 --- /dev/null +++ b/app/lib/services/google_platform_web.dart @@ -0,0 +1,8 @@ +/// Whether the programmatic Google sign-in flow exists on this platform. +/// +/// The web half: Google Identity Services signs in only through its own +/// rendered button, so `authenticate()` throws `UnimplementedError` here. +library; + +/// Always false: the web plugin has no programmatic sign-in. +bool get googleSignInSupported => false; diff --git a/app/lib/services/google_sign_in_backend.dart b/app/lib/services/google_sign_in_backend.dart new file mode 100644 index 0000000..0932a2e --- /dev/null +++ b/app/lib/services/google_sign_in_backend.dart @@ -0,0 +1,122 @@ +/// One-tap Google sign-in, kept apart from the rest of the sync wiring. +/// +/// Why this is its own file: `crdt_sync` is deliberately pure Dart (only +/// `crypto` and `http`), because the same library runs on Linux desktop and +/// headless under systemd, where `google_sign_in` does not exist. So the +/// plugin lives here, in the app, and the library only ever receives a token +/// string through a closure. +/// +/// What this buys: on a fresh install the sync account is reached by tapping +/// one account-picker row instead of typing a long password on a phone +/// keyboard. The password path stays as the fallback and as the machine +/// credential for desktop and the Python side. +library; + +import 'dart:developer'; + +import 'package:diet_guard_app/services/google_platform.dart' as platform_gate; +import 'package:google_sign_in/google_sign_in.dart'; + +/// The account the security rules pin. +/// +/// Not a secret -- it is already in the public `database.rules.json` -- but it +/// is the value that makes a wrong-account sign-in *fail loudly* instead of +/// authenticating fine and then being denied every read and write. See +/// `crdt-sync/tool/link_google.py`, which is what put the Google identity on +/// this uid in the first place. +const kSyncUid = 'OvA2REQyLIhAHOEjzwS1o877rgG3'; + +/// The project's **Web** OAuth client id, used as the audience for ID +/// tokens. +/// +/// Public by design, exactly like [kProject]'s `apiKey`: it ships inside every +/// APK, and the security rules -- not its secrecy -- protect the data. +/// +/// A plain const rather than a `--dart-define`, deliberately. As a +/// compile-time environment value it was empty in every build that matters: +/// the phone-deploy skill and CI both run a bare `flutter build apk +/// --release`, so released APKs would have shown a Google button that always +/// reported "cancelled" -- a visible control that can never succeed, which is +/// the exact thing the web platform gate exists to prevent. +/// +/// Fill this in from Firebase console -> Project settings -> the `syncs-rest` +/// Web app's OAuth client (type "Web application"). Android must request a +/// token minted for the *web* client; an Android client id here yields a token +/// Firebase rejects with `audience mismatch`. +/// Empty until the Web OAuth client is registered; [googleSignInSupported] is +/// false while it is, so the button stays hidden rather than failing. +const kServerClientId = + '845446124781-prdoherj0v64vc6egvvcp3l0693khaur.apps.googleusercontent.com'; + +/// Whether this platform can sign in programmatically. +/// +/// False on web, where Google Identity Services only signs in through its own +/// rendered button: `authenticate()` throws `UnimplementedError` there. The +/// desktop build of this app *is* the web build, so without this check the +/// "Sign in with Google" button would crash the settings screen on desktop +/// rather than falling back; see `google_platform.dart` for why this is a +/// platform check rather than a question asked of the plugin. +/// +/// Also false when no client id is compiled in: a button that cannot possibly +/// succeed is worse than no button, and without this check an unconfigured +/// build reports "cancelled" and sends you debugging the OAuth console for a +/// problem that is really a missing constant. +bool get googleSignInSupported => + platform_gate.googleSignInSupported && kServerClientId.isNotEmpty; + +/// Returns a Google ID token for the signed-in account, or null. +/// +/// Null rather than throwing when the user dismisses the picker: cancelling is +/// an ordinary outcome, and the caller falls back to the password path. +/// +/// [signInFn] and [serverClientId] exist for tests, which cannot reach the +/// platform channel the plugin uses. +Future googleIdToken({ + Future Function()? signInFn, + String serverClientId = kServerClientId, +}) async { + if (signInFn != null) return signInFn(); + if (serverClientId.isEmpty) { + // A build without --dart-define=GOOGLE_SERVER_CLIENT_ID would otherwise + // fail inside the plugin with a far less obvious message. + log( + 'Google sign-in unavailable: GOOGLE_SERVER_CLIENT_ID was not set at ' + 'build time; falling back to the password path', + level: 900, + ); + return null; + } + // The plugin reaches the OS account picker through a platform channel, + // which `flutter test` has no binding for -- the same reason main.dart and + // firebase_backend.dart's keystore adapters are excluded. Everything above + // this point is pure Dart and is covered. + // coverage:ignore-start + try { + await GoogleSignIn.instance.initialize(serverClientId: serverClientId); + if (!GoogleSignIn.instance.supportsAuthenticate()) { + // Web/desktop: sign-in must go through the rendered GIS button, which + // this app does not host yet. Fall back rather than throwing. + log( + 'Google sign-in is not available on this platform; falling back to ' + 'the password path', + level: 900, + ); + return null; + } + final account = await GoogleSignIn.instance.authenticate(); + return account.authentication.idToken; + } on GoogleSignInException catch (error, stackTrace) { + // Includes the user simply dismissing the picker, which is not an error + // worth surfacing -- but log it, because a *configuration* failure + // (unregistered SHA-1, wrong client id) arrives through the same path and + // is otherwise indistinguishable from "the user changed their mind". + log( + 'Google sign-in did not complete', + level: 900, + error: error, + stackTrace: stackTrace, + ); + return null; + } + // coverage:ignore-end +} diff --git a/app/pubspec.lock b/app/pubspec.lock index 4cf100c..7c9223f 100644 --- a/app/pubspec.lock +++ b/app/pubspec.lock @@ -85,11 +85,11 @@ packages: dependency: "direct main" description: path: crdt_sync_dart - ref: "crdt_sync_dart-v0.8.0" - resolved-ref: "6a080333c2ed2b2773fee01605dd67ba713e54ea" + ref: "crdt_sync_dart-v0.10.0" + resolved-ref: "5f64a8959b1db342da1f73157b1148aa0dfa38a8" url: "https://github.com/kuhyx/utils" source: git - version: "0.8.0" + version: "0.10.0" crypto: dependency: "direct main" description: @@ -257,6 +257,54 @@ packages: description: flutter source: sdk version: "0.0.0" + google_identity_services_web: + dependency: transitive + description: + name: google_identity_services_web + sha256: "5d187c46dc59e02646e10fe82665fc3884a9b71bc1c90c2b8b749316d33ee454" + url: "https://pub.dev" + source: hosted + version: "0.3.3+1" + google_sign_in: + dependency: "direct main" + description: + name: google_sign_in + sha256: "521031b65853b4409b8213c0387d57edaad7e2a949ce6dea0d8b2afc9cb29763" + url: "https://pub.dev" + source: hosted + version: "7.2.0" + google_sign_in_android: + dependency: transitive + description: + name: google_sign_in_android + sha256: "57782125965ca87b03d42a147d40b046f1fd6a09141a58913e1b86671a5ef22e" + url: "https://pub.dev" + source: hosted + version: "7.2.16" + google_sign_in_ios: + dependency: transitive + description: + name: google_sign_in_ios + sha256: ac1e4c1205267cb7999d1d81333fccffdfda29e853f434bbaf71525498bb6950 + url: "https://pub.dev" + source: hosted + version: "6.3.0" + google_sign_in_platform_interface: + dependency: transitive + description: + name: google_sign_in_platform_interface + sha256: "7f59208c42b415a3cca203571128d6f84f885fead2d5b53eb65a9e27f2965bb5" + url: "https://pub.dev" + source: hosted + version: "3.1.0" + google_sign_in_web: + dependency: transitive + description: + name: google_sign_in_web + sha256: d473003eeca892f96a01a64fc803378be765071cb0c265ee872c7f8683245d14 + url: "https://pub.dev" + source: hosted + version: "1.1.3" hooks: dependency: transitive description: diff --git a/app/pubspec.yaml b/app/pubspec.yaml index d57d4f6..31f6afd 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -2,7 +2,7 @@ name: diet_guard_app description: "Companion phone app for diet_guard: log meals on the go." publish_to: 'none' -version: 1.0.4+1 +version: 1.0.5+1 environment: sdk: ^3.12.2 @@ -11,13 +11,17 @@ dependencies: crdt_sync: git: url: https://github.com/kuhyx/utils - ref: crdt_sync_dart-v0.8.0 + ref: crdt_sync_dart-v0.10.0 path: crdt_sync_dart crypto: ^3.0.6 flutter: sdk: flutter flutter_local_notifications: ^22.0.1 flutter_secure_storage: ^10.3.1 + + # One-tap sign-in for the sync account. App-side only: crdt_sync stays pure + # Dart so it keeps working on Linux desktop and headless. + google_sign_in: ^7.2.0 http: ^1.6.0 idb_shim: ^2.9.6+2 path: ^1.9.1 diff --git a/app/test/firebase_backend_google_test.dart b/app/test/firebase_backend_google_test.dart new file mode 100644 index 0000000..7ba859d --- /dev/null +++ b/app/test/firebase_backend_google_test.dart @@ -0,0 +1,128 @@ +import 'dart:convert'; + +import 'package:crdt_sync/crdt_sync.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart' as http_testing; +import 'package:diet_guard_app/services/firebase_backend.dart'; +import 'package:diet_guard_app/services/google_sign_in_backend.dart'; + +import 'fake_secure_storage.dart'; + +/// Answers a `signInWithIdp` call the way Firebase does. +http_testing.MockClient _signsInAs(String email, {String uid = kSyncUid}) => + http_testing.MockClient( + (request) async => http.Response( + jsonEncode({ + 'idToken': 'id', + 'refreshToken': 'refresh', + 'expiresIn': '3600', + 'email': email, + 'localId': uid, + }), + 200, + ), + ); + +/// Answers the way Firebase does for an account that withholds its email. +/// +/// `signInWithIdp` simply omits the field -- the session is otherwise +/// identical and perfectly usable. +http_testing.MockClient _signsInWithoutEmail() => http_testing.MockClient( + (request) async => http.Response( + jsonEncode({ + 'idToken': 'id', + 'refreshToken': 'refresh', + 'expiresIn': '3600', + 'localId': kSyncUid, + }), + 200, + ), +); + +void main() { + // installFakeSecureStorage touches the test binary messenger, which only + // exists once the binding is initialised. + TestWidgetsFlutterBinding.ensureInitialized(); + + // openFirebaseWithGoogle reaches the keystore through a platform channel; + // the fake stands in so the account write is really exercised rather than + // stubbed out at the call site. + setUp(installFakeSecureStorage); + + group('openFirebaseWithGoogle', () { + test('returns null when the picker is dismissed', () async { + // Cancelling is ordinary: no session, no account, no error. + final saved = []; + final client = await openFirebaseWithGoogle( + tokenFetcher: () async => null, + accountSaver: (account) async => saved.add(account), + ); + + expect(client, isNull); + expect(saved, isEmpty); + }); + + test('stores the account under the email Firebase reports', () async { + // The regression this test exists for. A fresh install has no email + // anywhere on the device, so it must come from the sign-in response. An + // earlier cut read it from the settings screen's text field -- empty on + // exactly this path -- and persisted FirebaseAccount(email: ''); the + // next launch then took the password branch with '' and failed. + final saved = []; + final client = await openFirebaseWithGoogle( + tokenFetcher: () async => 'a-token', + accountSaver: (account) async => saved.add(account), + httpClient: _signsInAs('signed-in@example.com'), + ); + + expect(client, isNotNull); + expect(saved, hasLength(1)); + expect(saved.single.email, 'signed-in@example.com'); + // No password to store: this device authenticates with the refresh + // token, and openFirebase() must not later try to sign in with ''. + expect(saved.single.password, isEmpty); + client!.close(); + }); + + test('stores the account even when Firebase reports no email', () async { + // The regression that cost a night of syncing. `signInWithIdp` omits + // `email` when the Google account withholds it; the account write used + // to be gated on that field while the client was returned regardless, + // so this call reported success, persisted nothing, and the next launch + // found no account and silently dropped to GitHub-only with a 401. + // + // The refresh token -- the thing that actually authenticates -- is + // already stored by now. An empty email is the honest value and costs + // nothing: openFirebase() reads the session, not the address. + final saved = []; + final client = await openFirebaseWithGoogle( + tokenFetcher: () async => 'a-token', + accountSaver: (account) async => saved.add(account), + httpClient: _signsInWithoutEmail(), + ); + + expect(client, isNotNull); + expect(saved, hasLength(1)); + expect(saved.single.email, isEmpty); + expect(saved.single.password, isEmpty); + client!.close(); + }); + + test('stores nothing when Google resolves to the wrong uid', () async { + // Authentication succeeds and the uid is wrong, which the security + // rules would deny on every read and write. Failing here -- with + // nothing persisted -- leaves the device retryable rather than stuck. + final saved = []; + await expectLater( + openFirebaseWithGoogle( + tokenFetcher: () async => 'a-token', + accountSaver: (account) async => saved.add(account), + httpClient: _signsInAs('someone@example.com', uid: 'a-different-uid'), + ), + throwsA(isA()), + ); + expect(saved, isEmpty); + }); + }); +} diff --git a/app/test/google_platform_test.dart b/app/test/google_platform_test.dart new file mode 100644 index 0000000..81a41b0 --- /dev/null +++ b/app/test/google_platform_test.dart @@ -0,0 +1,14 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:diet_guard_app/services/google_sign_in_backend.dart'; + +void main() { + test('reports whether the programmatic Google flow exists here', () { + // Android is the only platform shipping it. Asked as a platform question + // rather than of the plugin, because GoogleSignIn.supportsAuthenticate() + // itself throws UnimplementedError where no implementation is registered + // -- an Error, not an Exception, so it would escape an ordinary catch and + // take down the settings screen's build(). The test host is Linux, so + // this is false here and the password path stays visible. + expect(googleSignInSupported, isFalse); + }); +} diff --git a/app/test/google_sign_in_backend_test.dart b/app/test/google_sign_in_backend_test.dart new file mode 100644 index 0000000..a787916 --- /dev/null +++ b/app/test/google_sign_in_backend_test.dart @@ -0,0 +1,33 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:diet_guard_app/services/google_sign_in_backend.dart'; + +void main() { + group('googleIdToken', () { + test('returns the token the sign-in function yields', () async { + // The injected path: proves the closure is what feeds crdt_sync, and + // that no platform channel is touched to get there. + expect( + await googleIdToken(signInFn: () async => 'a-token'), + 'a-token', + ); + }); + + test('passes a null cancellation straight through', () async { + // Dismissing the picker is an ordinary outcome, not an error: the + // caller falls back to the password path. + expect(await googleIdToken(signInFn: () async => null), isNull); + }); + + test('reports "not configured" when the client id is missing', () async { + // A build without --dart-define=GOOGLE_SERVER_CLIENT_ID must fall back + // rather than failing deep inside the plugin with an opaque message. + expect(await googleIdToken(serverClientId: ''), isNull); + }); + + test('pins the uid the security rules expect', () { + // If this ever drifts from database.rules.json, a Google sign-in would + // be accepted here and then denied every read and write. + expect(kSyncUid, 'OvA2REQyLIhAHOEjzwS1o877rgG3'); + }); + }); +} diff --git a/app/test/screens/settings_screen_test.dart b/app/test/screens/settings_screen_test.dart index 6cad498..8b16c07 100644 --- a/app/test/screens/settings_screen_test.dart +++ b/app/test/screens/settings_screen_test.dart @@ -293,6 +293,17 @@ void main() { await saveAccount( const FirebaseAccount(email: 'sync@example.com', password: 'pw'), ); + // A live session too, not just the marker: since 2026-08-11 a marker with + // no session behind it is treated as stale and cleared, because that is + // the state a revoked refresh token leaves behind and it used to report + // "Connected" while every sync failed with TOKEN_EXPIRED. + await credentialStore().save( + FirebaseCredentials( + idToken: 'id', + refreshToken: 'refresh', + expiresAt: DateTime.now().toUtc().add(const Duration(hours: 1)), + ), + ); await tester.runAsync(() async { await tester.pumpWidget(const MaterialApp(home: SettingsScreen())); await settle(tester); diff --git a/scripts/check_no_crdt_sync_override.sh b/scripts/check_no_crdt_sync_override.sh new file mode 100755 index 0000000..967e6c8 --- /dev/null +++ b/scripts/check_no_crdt_sync_override.sh @@ -0,0 +1,88 @@ +#!/bin/bash + +# ============================================================================ +# Fail if a pubspec still overrides crdt_sync with a local path. +# +# The apps depend on crdt_sync as a *git dependency pinned to a tag*, so what +# CI builds is whatever that tag points at. A `dependency_overrides:` entry +# pointing at ~/utils/crdt_sync_dart is invaluable while developing an unreleased +# library change -- and catastrophic if it survives the commit: local tests go +# green against code CI has never seen, and the failure is silent in both +# directions. +# +# A comment in the pubspec asking someone to remember is deployment hygiene, +# not a fix. This is the gate. +# +# Usage: check_no_crdt_sync_override.sh [pubspec.yaml ...] +# With no arguments, checks ./pubspec.yaml. +# ============================================================================ + +set -euo pipefail + +SCRIPT_NAME="$(basename "$0")" +readonly SCRIPT_NAME + +usage() { + echo "Usage: $SCRIPT_NAME [pubspec.yaml ...]" + echo " Fails when a pubspec overrides crdt_sync with a local path." + exit 0 +} + +# Returns 0 (found) when the file has a crdt_sync entry under +# dependency_overrides. Deliberately a small state machine rather than a grep: +# `path:` appears under plenty of other keys, and a false positive here blocks +# a legitimate commit. +has_override() { + local file="$1" + awk ' + # Track indent depth explicitly. An earlier version used an awk + # interval expression ({1,4}) to spot a sibling key, which plain awk + # does not enable by default -- so the check silently never fired and + # reported every file clean. A safety check that fails open is worse + # than none, so this version is written to be obvious instead of terse. + function indent(line, n) { + n = match(line, /[^ ]/) + return (n == 0) ? 0 : n - 1 + } + /^[[:space:]]*#/ { next } # comments + /^[[:space:]]*$/ { next } # blank lines + /^dependency_overrides:[[:space:]]*$/ { in_overrides = 1; next } + /^[^[:space:]]/ { in_overrides = 0; in_crdt = 0; next } + in_overrides { + if ($0 ~ /^[[:space:]]+crdt_sync:[[:space:]]*$/) { + in_crdt = 1 + crdt_indent = indent($0) + next + } + # Any key at or above crdt_sync{s} own indent ends its block. + if (in_crdt && indent($0) <= crdt_indent) { in_crdt = 0 } + if (in_crdt && $0 ~ /^[[:space:]]+path:/) { found = 1 } + } + END { exit(found ? 0 : 1) } + ' "$file" +} + +main() { + local -a files=("$@") + if [[ ${#files[@]} -eq 0 ]]; then + files=("pubspec.yaml") + fi + + local failed=0 + for file in "${files[@]}"; do + [[ -f "$file" ]] || continue + if has_override "$file"; then + echo "Error: $file still has a local crdt_sync dependency_override." >&2 + echo " Remove it and bump the git 'ref:' to the released tag" >&2 + echo " before committing, or CI will build different code." >&2 + failed=1 + fi + done + return "$failed" +} + +if [[ "${1:-}" == "-h" || "${1:-}" == "--help" ]]; then + usage +fi + +main "$@"