Skip to content

fix(local-dev): complete Google sign-in against the Auth emulator on LAN - #12453

Open
formed2forge wants to merge 6 commits into
BasedHardware:mainfrom
formed2forge:fix/local-dev-lan-emulator-auth
Open

fix(local-dev): complete Google sign-in against the Auth emulator on LAN#12453
formed2forge wants to merge 6 commits into
BasedHardware:mainfrom
formed2forge:fix/local-dev-lan-emulator-auth

Conversation

@formed2forge

@formed2forge formed2forge commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • local_dev Google sign-in on a physical iPhone no longer opens Safari against plain-HTTP /v1/auth/authorize (blank page; offline harness has no Google client). The app asks for response_mode=json, exchanges the PKCE code, and signs in with an Auth-emulator custom token.
  • Document the physical-device path as LAN (RFC 1918) OMI_DEV_HOST for both setup.sh and make dev-up. iOS now prompts for local-network access (NSLocalNetworkUsageDescription). Two Home Screen icons can both be named Omi Dev.
  • Related, competing design: feat(app+backend): local-development sign-in for contributors without OAuth #11784 (feat/local-dev-emulator-auth) adds a separate "Sign in (local dev)" button and out-of-band remint. This PR keeps the Google button, which is what a newcomer taps. Do not land both blindly.

Failure-Class: new

The new class is: when FIREBASE_AUTH_EMULATOR_HOST is set, /v1/auth/authorize must complete PKCE against the emulator (JSON or app-scheme redirect) instead of redirecting to Google/Apple. A production deployment without the emulator must reject response_mode=json.

Test plan

  • backend/.venv pytest tests/unit/test_auth_redirect_uri.py — 65 passed (JSON mode without emulator fails closed; emulator mints a code and custom token; Google redirect is not called)
  • flutter test test/unit/emulator_authorize_callback_test.dart — 3 passed
  • app/test/shell/ios_dev_ats_config_test.sh — ArbitraryLoads kept, LocalNetworking not combined, usage description present
  • Live on Revtim's iPhone (iPhone 17 Pro, iOS 27, UDID 00008150-001C3DAC0A38401C): OMI_DEV_HOST=192.168.2.200, harness bound, onboarding GET 200, Google sign-in completed without Safari. Tailscale 100.81.134.49 was ENETUNREACH (phone not on the tailnet).
  • Docs from scratch (isolated /tmp tree): npm install mintlify; docs/ npm ci succeeds. mintlify broken-links does not report AppSetup.mdx. npm run build still fails on missing rollout/ (pre-existing on main). mintlify broken-links still hits a pre-existing parse error in doc/hardware/omiglass/monitoring.mdx.

This commit was not re-run on the phone against current main; the live path was exercised on the same authorize-JSON + LAN host combination before the slice was ported onto upstream/main.

Review in cubic

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @formed2forge — this is a genuinely well-built piece of local-dev tooling, and the failure-class doc plus tests on both sides make it easy to review. The design (skip Safari, complete PKCE against the Auth emulator with response_mode=json) is fail-closed where it matters. I'm requesting changes for three concrete items, none of which are structural.

What I verified as solid

  • backend/routers/auth.pyresponse_mode=json is rejected with a 400 when FIREBASE_AUTH_EMULATOR_HOST is unset, the redirect-uri allowlist (_validate_redirect_uri) still runs before any code is minted, and _auth_code_data_from_session keeps the emulator code PKCE-bound. _generate_emulator_custom_token is only reachable when the stored credentials carry "emulator": true, which only the env-gated path sets — a real Google ID-token exchange can't accidentally route to it.
  • app/lib/services/auth_service.dart — the extract of _launchAuthorizationAndWaitForCallback is an improvement on the old inline flow (the finally now always cancels the app_links subscription and clears the method-channel handler), and the external-browser launch for plain-HTTP URLs is correctly commented. _completeEmulatorAuthorization fails closed on a missing code.
  • backend/tests/unit/test_auth_redirect_uri.py — the new TestEmulatorAuthorize tests cover the fail-closed json-mode rejection, Google redirect not called, and the emulator custom-token mint; the autouse _clear_auth_emulator_host fixture keeps the env var from leaking into the PKCE suite. app/test/unit/emulator_authorize_callback_test.dart covers URI building, callback rebuild, and the no-code failure.

Requested changes

  1. Formatting check is failingdart format --line-length 120 wants to reformat 2 of the 3 changed .dart files (CI pins Flutter 3.44.5; a local formatter version may produce different output). Please run dart format on the changed files and push.
  2. Docs now contradict the harness and setup script on Tailscaledocs/doc/developer/AppSetup.mdx and app/README.md narrow OMI_DEV_HOST to "LAN (RFC 1918)" and drop Tailscale, but scripts/dev-harness/dev_harness/safety.py still deliberately accepts CGNAT 100.64.0.0/10 (its comment cites #11730/#11652 as the documented device path), and app/setup.sh still warns to set "LAN or Tailscale address". Your ENETUNREACH was the phone not being on the tailnet, not a harness restriction. Please either keep Tailscale documented as supported or narrow all three places consistently.
  3. app/ios/Runner/Info-Dev.plist — stray AppGroupIdentifier addition$(APP_GROUP_IDENTIFIER) isn't defined anywhere in project.pbxproj, no code reads this plist key (BatteryWidget hardcodes the group id in SharedDefaults.swift), and generate_ios_dev_info_plist.sh regenerates the file from Info.plist and would silently drop the key. It looks like it drifted in from another branch. Please remove it or explain what consumes it. (The NSLocalNetworkUsageDescription addition is correct and needed for the iOS local-network prompt — good catch adding it to both the plist and the generator so it survives regeneration, with coverage in ios_dev_ats_config_test.sh.)

Notes for maintainers

  • The emulator branch in auth_authorize triggers for every authorize client when FIREBASE_AUTH_EMULATOR_HOST is set — an operator who sets that var in a real deployment silently converts all Google/Apple sign-ins into unauthenticated emulator logins (local-google uid). That matches how the rest of the Firebase emulator harness behaves, but it's worth a maintainer's explicit sign-off on an auth-surface change of this shape. There's also a duplicated _log_auth_event pair (auth_code_created + authorize_redirect_created) in the emulator branch — harmless, but intentional?
  • The one-line app/AGENTS.md addition under Auth Methods documents this flow for coding agents; it's accurate against this diff, but it encodes this PR's design — if #11784's separate-button design lands instead, that line needs rewording. The PR description itself flags the #11784 overlap; the two shouldn't both land, so that's a direction call for a maintainer.
  • .github/failure-classes/FC-emulator-authorize-opens-browser.json matches the established failure-class schema nicely.

Human maintainer sign-off requested on the auth-surface gating and the choice between this and #11784; the three requested changes above are mechanical and don't need that discussion to resolve.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added security-review Touches auth, provider routing, secrets, or security-sensitive surfaces docs-accuracy Documentation or committed reports need accuracy fixes labels Aug 31, 2026
@Git-on-my-level

Copy link
Copy Markdown
Collaborator

CI note on the red Backend unit suite run for this head (run 33350310129): it is not caused by this PR. The only failing file is tests/unit/test_chat_file_upload_unsupported.py, which errors in _make_chat_client with ImportError: cannot import name 'is_gateway_model_not_found' from 'utils.llm.gateway_client' — the test-harness stub in tests/unit/_chat_router_test_harness.py predated that import, which landed on main in e6b545c ("degrade file chat on missing gateway lane") shortly before this branch's run. Main already fixed the stub in 34f245f ("test(backend): centralize chat gateway stubs"), which landed ~30 min after this run started, so the failure clears on rebase. This PR's own backend tests (tests/unit/test_auth_redirect_uri.py) passed in that same run, and none of the PR's 12 files touch the chat/gateway surface.

None of the three items from the earlier review are affected by this — they stand as listed (formatting, Tailscale docs consistency, the stray AppGroupIdentifier key).


by AI on behalf of David.

@Git-on-my-level Git-on-my-level added needs-maintainer-review Needs a human maintainer to sign off before merge flutter flutter work labels Sep 1, 2026
@cursor
cursor Bot force-pushed the fix/local-dev-lan-emulator-auth branch 2 times, most recently from fbd9bf1 to 43aebbc Compare September 2, 2026 15:31

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @formed2forge — re-checked after the rebase onto newer main (43aebbc). One thing improved on its own: the red Backend unit suite run cleared exactly as predicted once the chat-gateway stub fix from main was picked up by the rebase, and the hermetic/Dart gates are green on this head. Your own auth tests were green throughout.

The three items from the earlier review are still open on this head, though — none of them structural:

  1. Formatting still red on this head — the Formatting check (run 33649065643) fails with dart format --line-length 120 --set-exit-if-changed naming app/lib/providers/auth_provider.dart and app/lib/services/auth_service.dart — the same two files as before (likely a local formatter version differing from the pinned CI toolchain). app/test/unit/emulator_authorize_callback_test.dart passes formatting fine.

  2. Docs still contradict the harness and setup script on Tailscaledocs/doc/developer/AppSetup.mdx and app/README.md narrow OMI_DEV_HOST to "LAN (RFC 1918)", but on this base scripts/dev-harness/dev_harness/safety.py still deliberately accepts CGNAT 100.64.0.0/10 (its comment cites #11730/#11652/#11782 as the documented device path) and app/setup.sh still tells developers to set the "LAN or Tailscale address". Either keep Tailscale documented as supported or narrow all three places together.

  3. AppGroupIdentifier still drifts in app/ios/Runner/Info-Dev.plist$(APP_GROUP_IDENTIFIER) is defined nowhere: zero occurrences in project.pbxproj and none repo-wide (code search), and generate_ios_dev_info_plist.sh regenerates this file from Info.plist, which doesn't carry the key — so it would silently disappear on the next regeneration anyway. Please drop it or point at what consumes it. (NSLocalNetworkUsageDescription is the opposite and done right: added in the plist, the generator, and asserted by ios_dev_ats_config_test.sh.)

What I re-verified as solid on this head, for the record:

  • backend/routers/auth.pyresponse_mode=json still 400s when FIREBASE_AUTH_EMULATOR_HOST is unset; _validate_redirect_uri still runs before any code is minted; emulator codes stay PKCE-bound via _auth_code_data_from_session; _generate_emulator_custom_token requires both the stored "emulator": true credentials and the env flag, and those credentials are server-stored with the auth code, not client-submitted. (Tiny nit, still present: the emulator branch logs auth_code_created and authorize_redirect_created back-to-back.)
  • app/lib/services/auth_service.dart_completeEmulatorAuthorization fails closed on non-200 and on a missing code, and _signInWithOAuthCredentials now throws rather than silently returning when the emulator response lacks a custom token. The _launchAuthorizationAndWaitForCallback extract still cleans up the app_links subscription and method-channel handler in finally.
  • backend/tests/unit/test_auth_redirect_uri.pyTestEmulatorAuthorize still covers json-mode rejection, app-scheme redirect (Google not called), and the custom-token mint; the autouse _clear_auth_emulator_host fixture keeps the env var out of the PKCE suite.
  • app/AGENTS.md — the new Web OAuth/USE_WEB_AUTH line matches this diff exactly (local_dev completes against the emulator, no Safari), so agents reading it get an accurate picture.
  • .github/failure-classes/FC-emulator-authorize-opens-browser.json — the stated contract matches the implemented behavior on both ends.

For maintainers: #11784 (separate "Sign in (local dev)" button + out-of-band remint) is still open from the same author, and the emulator branch in auth_authorize still applies to every authorize client whenever FIREBASE_AUTH_EMULATOR_HOST is set — a deployment that sets that var silently converts all Google/Apple sign-ins into emulator logins. The choice between the two designs, and explicit sign-off on that auth-surface shape, needs a human maintainer before merge.


by AI on behalf of David.

@formed2forge

Copy link
Copy Markdown
Contributor Author

All three review items addressed: Dart/Python formatting (reverted to Flutter 3.44.5 pinned toolchain style), stray AppGroupIdentifier key removed from Info-Dev.plist, and Tailscale docs restored to match the harness's actual accepted CIDR ranges. Ready for re-review.

@formed2forge

Copy link
Copy Markdown
Contributor Author

Formatting is now clean — the local dart 3.13.2 and CI's Flutter 3.44.5 bundled dart disagree on the indentation of a multi-line ?? expression, so restructured to extract the l10n lookup into a local variable so the ?? and its fallback fit on one line. dart format is now a no-op on the file. Ready for re-review.

@Git-on-my-level
Git-on-my-level dismissed stale reviews from themself September 4, 2026 07:28

Dismissed: all three blocking items (Formatting check red, Tailscale docs contradiction, stray AppGroupIdentifier in Info-Dev.plist) are resolved on head 8ced740 — Formatting check green, docs consistent with the harness CGNAT acceptance again, and the key removed.

@Git-on-my-level Git-on-my-level removed the docs-accuracy Documentation or committed reports need accuracy fixes label Sep 4, 2026

@Git-on-my-level Git-on-my-level left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @formed2forge — re-reviewed on 8ced740. All three items from the last review are resolved, and I've dismissed the outstanding change requests:

  1. Formatting — green on this head; extracting the l10n lookup into authFailedMsg in app/lib/providers/auth_provider.dart was the right fix for the dart 3.13.2 / CI 3.44.5 ??-indentation divergence, and it reads better anyway.
  2. Tailscale docsdocs/doc/developer/AppSetup.mdx and app/README.md now say "LAN or Tailscale (RFC 1918 / CGNAT)", consistent again with scripts/dev-harness/dev_harness/safety.py (which deliberately accepts 100.64.0.0/10) and app/setup.sh's "LAN or Tailscale address" guidance.
  3. AppGroupIdentifier — gone; zero occurrences in app/ios/Runner/Info-Dev.plist on this head. NSLocalNetworkUsageDescription remains done right: declared in the plist, re-added by generate_ios_dev_info_plist.sh, and asserted by ios_dev_ats_config_test.sh.

Re-verified the rest on this head, for the record:

  • backend/routers/auth.pyresponse_mode=json still 400s when FIREBASE_AUTH_EMULATOR_HOST is unset; _validate_redirect_uri still runs before any code is minted; emulator codes stay PKCE-bound via _auth_code_data_from_session; the emulator custom-token mint requires both the server-stored "emulator": true credentials and the env flag, so a real Google exchange can't route to it.
  • app/lib/services/auth_service.dart_completeEmulatorAuthorization fails closed on non-200 and on a missing code; the extracted _launchAuthorizationAndWaitForCallback now cancels the app_links subscription and clears the method-channel handler in finally (an improvement over the old inline flow); only plain-http:// authorize URLs switch to LaunchMode.externalApplication, with a comment explaining the SFSafariViewController/ATS reason.
  • Tests on both sides hold up: TestEmulatorAuthorize in backend/tests/unit/test_auth_redirect_uri.py covers the fail-closed json rejection, Google-redirect-not-called, and the emulator token mint, with the autouse _clear_auth_emulator_host fixture keeping the env var out of the PKCE suite; app/test/unit/emulator_authorize_callback_test.dart covers URI building, callback rebuild, and the no-code failure.
  • .github/failure-classes/FC-emulator-authorize-opens-browser.json matches the registry schema and the behavior actually implemented on both ends.
  • app/AGENTS.md — the new Web OAuth/USE_WEB_AUTH line is accurate (the env flag is real, and only local_dev sets the emulator profile flag), so agents working in app/ get a correct picture of the local-dev auth flow.

Two things still need a human maintainer before merge, same as before — neither is a code defect:

  • #11784 (separate "Sign in (local dev)" button + out-of-band remint) is still open from the same author and covers the same user outcome; a maintainer needs to pick the design so both don't land.
  • The emulator branch in auth_authorize applies to every authorize client whenever FIREBASE_AUTH_EMULATOR_HOST is set — sign-off on that auth-surface shape (vs. a narrower opt-in) is a security/product decision.

That's why security-review and needs-maintainer-review stay; I've dropped docs-accuracy since the doc contradiction is fixed. Thanks for the quick, complete turnaround on all three items.


by AI on behalf of David.

formed2forge and others added 6 commits September 4, 2026 13:33
The offline harness has no Google client, and iOS will not render the
plain-HTTP authorize page. local_dev now finishes PKCE over JSON and
signs in with an emulator custom token, so a physical phone on the Mac's
LAN can tap Google without Safari.

Failure-Class: new
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Use the already-validated redirect_uri and state instead of the mixed
session dict so the pre-push typecheck can pass.

Failure-Class: new
Co-authored-by: Cursor <cursoragent@cursor.com>
…, Tailscale docs

- Dart formatting: revert constructor-initialiser and map-literal indentation in
  auth_service.dart and auth_provider.dart to Flutter 3.44.5 (Dart) style; the
  PR was authored with a newer formatter that produces a different shape for `:` lists
  and `=>` map literals, and for the long ternary in auth_provider.dart.
- Remove stray `AppGroupIdentifier`/`$(APP_GROUP_IDENTIFIER)` key-value pair from
  app/ios/Runner/Info-Dev.plist; the variable is undefined in project.pbxproj and
  would be silently dropped on the next generate_ios_dev_info_plist.sh regeneration.
- Restore Tailscale as a documented OMI_DEV_HOST option in docs/doc/developer/AppSetup.mdx
  and app/README.md to match scripts/dev-harness/dev_harness/safety.py (which accepts
  CGNAT 100.64.0.0/10) and app/setup.sh (which still says "LAN or Tailscale address");
  the original narrowing to RFC 1918-only was inconsistent across the three places.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P5TqoWB9unzNKthnD1pJUP
@cursor
cursor Bot force-pushed the fix/local-dev-lan-emulator-auth branch from 8ced740 to 12bb5ef Compare September 4, 2026 13:35
@formed2forge

Copy link
Copy Markdown
Contributor Author

Rebased onto BasedHardware/omi main 176a02fe0d.

  • old: 8ced740b38
  • new: 12bb5ef469
  • now 0 behind / 6 ahead, MERGEABLE

Was DIRTY; now MERGEABLE. Conflict in auth_service.dart kept the LAN Google-button emulator path. Independent of #11784.
No other changes in this push.

@Git-on-my-level

Copy link
Copy Markdown
Collaborator

Re-reviewed on the rebased head 12bb5ef — the conflict resolution in app/lib/services/auth_service.dart kept the LAN Google-button emulator path intact, and the full head-vs-main diff holds together. File-by-file notes:

  • backend/routers/auth.py — the emulator completion is gated on FIREBASE_AUTH_EMULATOR_HOST in both directions: response_mode=json is rejected with a 400 when the emulator is unset (logged as emulator_response_mode_without_emulator), and the emulator branch is what skips the Google/Apple redirect. The minted code still goes through _auth_code_data_from_session, so PKCE binding survives, and _generate_emulator_custom_token re-checks the env before minting. Fail-closed shape is right.
  • app/lib/services/auth_service.dart — extracting _launchAuthorizationAndWaitForCallback quietly fixes a latent leak: the app_links subscription is now cancelled in a finally instead of only inside the timeout callback. _completeEmulatorAuthorization fails closed on non-200 and on a body without a code, and the emulator sign-in throws if the custom token is missing rather than falling through.
  • backend/tests/unit/test_auth_redirect_uri.py + app/test/unit/emulator_authorize_callback_test.dart — good coverage of both directions (json rejected without emulator; emulator path never calls _google_auth_redirect), and the autouse _clear_auth_emulator_host fixture keeps the env out of the older PKCE tests. Dart side green in CI.
  • app/ios/Runner/Info-Dev.plist, app/scripts/generate_ios_dev_info_plist.sh, app/test/shell/ios_dev_ats_config_test.shNSLocalNetworkUsageDescription lands in the plist, the generator, and the generator's shell test, so a setup run can't silently revert this one the way it did the ATS keys.
  • app/AGENTS.md, app/README.md, docs/doc/developer/AppSetup.mdx, .github/failure-classes/FC-emulator-authorize-opens-browser.json — docs match the code (RFC 1918/CGNAT host rules, emulator completion, rebuild-after-OMI_DEV_HOST, the blank-Safari accordion). The AGENTS.md line is accurate — USE_WEB_AUTH is real and local_dev is the only profile with usesFirebaseAuthEmulator: true; it's descriptive with no new mandates, so nothing extra to review there beyond the auth sign-off below.
  • app/lib/providers/auth_provider.dart — emulator-profile failures now surface the raw error; scoped to usesFirebaseAuthEmulator, so production keeps the localized generic copy.

The red Backend unit suite on this head is not from this PR: run-slow-guardrails-ci.sh fails because backend/tests/slow_guardrail_manifest.txt (line 47 at this head, inherited from main at rebase time) still lists tests/unit/test_ws_g_module_aliases.py, which the WS-G cleanup deleted. Main fixed that in #12740 about an hour after this run started — a rerun or a fresh rebase onto main should clear it. None of this PR's 12 files touch that lane.

What's left is genuinely human: sign-off on the emulator completion semantics (a stable local-{provider} Firebase identity stands in for real provider identity in local_dev — intended, but it's an auth surface), and the design overlap with the still-open #11784, which takes the separate-button approach to the same problem. Both shouldn't land blind. Keeping security-review and needs-maintainer-review for those two decisions; adding positive-signal — three review rounds of formatting/docs/fix iterations, each addressed cleanly.


by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

@Git-on-my-level Git-on-my-level added the positive-signal Good PR — positive signal, not a formal approval label Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flutter flutter work needs-maintainer-review Needs a human maintainer to sign off before merge positive-signal Good PR — positive signal, not a formal approval security-review Touches auth, provider routing, secrets, or security-sensitive surfaces

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants