mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-28 05:12:26 +00:00
f9d1bc8c27
Pasting a Google Client ID / Secret never completed OAuth: Drive (and its Google siblings) accepted the credentials, showed no error, opened no browser, and never appeared in Data Sources. Root cause is three coupled defects, all reproduced at the unit level against main with a FastAPI TestClient (no Google creds, network-free): (A/B) POST /connect routed a `client_id:client_secret` pair into the connector's handle_callback, which spawned a daemon thread that popped a browser and ran its own localhost:8789 callback server. That thread fails silently in the bundled desktop context (`except Exception: pass`), so the connector never gained an access_token; /connect returned status "pending" and the UI's 20x2s poll timed out with no error. Fix: in POST /connect, an OAuth `client_id:client_secret` pair now persists the client credentials to every Google credential file and returns an `oauth_required` directive pointing at the in-process server flow, instead of the silent background thread. The Google connectors' handle_callback no longer spawns the browser thread for the pair case — it only persists the creds; the server's /oauth/start -> /oauth/callback owns the consent round-trip. (C) The would-be-correct server flow was itself broken: under `from __future__ import annotations` plus a `Request` import local to the router factory, FastAPI could not resolve the stringized `request: Request` annotation. /oauth/start returned HTTP 422 (request mis-bound as a query param) and /oauth/callback injected None -> AttributeError on `request.base_url`. Fix: import `Request` at module scope and make the callback's `request` a required injected dependency. A malformed/blank client pair now raises HTTP 400 with the provider setup URL instead of a perpetual silent "pending" (REVIEW.md silent-failure discipline). Frontend: DataSourcesPage now opens the server OAuth window when /connect returns `oauth_required`, then polls until connected; connect errors surface the backend detail; the Drive setup steps document the "Web application" OAuth client + server-callback redirect URI the in-process flow requires. Tests (run on the main venv, hermetic — no ~/.openjarvis pollution): - test_oauth_flow.py: the three handle_callback tests now assert NO browser is opened and only client creds are persisted (was: assert background flow ran). - test_connectors_router_oauth.py (new): reproduces + fixes all three defects via TestClient with mocked token exchange; parametrized over gdrive/gcalendar/ gcontacts/gmail/google_tasks to prove the shared OAuth path is fixed for every sibling and that a single consent writes the access_token to all six Google credential files and flips is_connected() to True. Full tests/connectors suite: 355 passed. Relationship to PR #510: #510 rewrites all of these files (account-scoped retrieval) but still carries all three defects. This fix is intentionally scoped to the OAuth path and does not modify oauth.py, to minimize collision. A maintainer can either merge this and rebase #510 on top, or port these changes into #510. See PR body for details. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>