feat: first-party Google Drive integration — search and read Drive files from chat (stacked on #285) - #290
Closed
amal66 wants to merge 6 commits into
Closed
Conversation
amal66
marked this pull request as draft
August 4, 2026 14:47
…les from chat
WHY THIS MATTERS
"Search my Google Drive for X and summarize it" should be one Connect
click away. The MCP route to that experience is blocked for most
deployments: Google's hosted Drive MCP server (drivemcp.googleapis.com)
sits behind the Workspace Developer Preview Program, so OAuth succeeds,
tools list — and every tools/call returns a bare PERMISSION_DENIED
(verified live: same token, same scopes, REST works, MCP refuses). The
plain Drive REST API is GA and ungated. So the integration goes first
party: run OAuth ourselves, call REST directly.
HOW IT WORKS
- lib/integrations/googleDrive.ts owns the whole surface:
* OAuth: PKCE (S256) against accounts.google.com with
access_type=offline & prompt=consent — the two parameters without
which Google never issues a refresh token (the same lesson the MCP
connector flow learned in Open-Legal-Products#185). Tokens AES-GCM-encrypted at rest
(one row per user), state rows hashed + TTL'd, refresh handled
transparently with a 60s expiry leeway; a revoked grant
(invalid_grant) deletes the row so the UI honestly shows
disconnected instead of failing every call.
* Tools: google_drive_search (name + fullText, q-escaping for quotes
and backslashes so user input cannot break out of the query
literal), google_drive_list_recent, google_drive_read_file
(Docs/Sheets/Slides exported as text/CSV; PDF via the existing
pdfjs extractor; .docx via mammoth; 60k-char cap). Read-only scope
(drive.readonly) — write tools would need broader consent and
confirmation policy, deliberately out of scope.
* The tools reuse the MCP event shape (connector_name "Google
Drive"), so the chat UI renders them with the existing connector
treatment — zero changes to the event pipeline or frontend chat.
- Tool registry offers the tools only when the user has a token row;
the dispatcher routes the google_drive_ prefix ahead of mcp_.
- Routes: /user/integrations/google-drive (status), /oauth/start,
/oauth/callback (reuses the MCP popup renderer), DELETE disconnect
(best-effort Google-side revocation, then row deletion).
- Frontend: a dedicated card on Account → Connectors — no server URL to
type, just Connect → Google consent popup → status-polling (COOP
severs window.opener on Google's consent page, so polling our own
status endpoint is the source of truth) → tools active. Disconnect
next to it.
- Env: GOOGLE_DRIVE_OAUTH_CLIENT_ID/_SECRET, falling back to
GOOGLE_MCP_OAUTH_* so one Cloud Console client serves both features.
README gains a self-hosting section with the exact Console steps
(enable drive.googleapis.com, the redirect URI to add, test-user vs
verification implications of the restricted scope).
TESTING
- 8 new unit tests: PKCE/offline URL construction, fail-fast setup
error with copy-pasteable redirect URI, tool gating on connection,
q-escaping, Google Doc export path, invalid_grant row cleanup.
- Full backend suite: 460 passing. Frontend tsc + suite clean.
…ge's MFA machinery
WHY THIS MATTERS
The backend gates POST /user/integrations/google-drive/oauth/start and
DELETE /user/integrations/google-drive behind requireMfaIfEnrolled: a
user who has enrolled a second factor but whose current session has not
verified it gets a 403 with code `mfa_verification_required`. Every
other sensitive action on the Connectors page (create/save/delete a
connector, toggle tools, refresh) already handles that answer by
opening the MFA verification popup and retrying once the user has
verified. The new Google Drive card did not: its connect() and
disconnect() called the API directly, so an MFA-enrolled user clicking
"Connect" saw a dead-end red error string quoting the backend's 403 —
with no way to proceed short of re-logging-in.
WHAT IS THE MFA "STEP-UP" PATTERN HERE
This page implements step-up authentication: routine reads work with a
normal session, but state-changing actions on credential-bearing
resources demand a fresh second-factor proof. The client half of the
pattern is `runSensitiveAction(action, fn)`:
1. pre-check: `needsMfaVerification()` — if the session is known to
need verification, remember `action` and open the popup instead
of even attempting `fn`;
2. catch: if `fn` still hits the backend gate, `isMfaRequiredError`
recognises the 403 and the same popup opens;
3. resume: once the popup reports "verified", `handleMfaVerified`
re-dispatches the remembered `action` descriptor.
Step 3 is why the wrapper works on *descriptors* (`{ type: "delete",
connectorId }`) rather than closures: the retry must run after an
arbitrary delay, from the popup's callback, against fresh state.
HOW THE FIX WORKS
- `PendingMfaAction` gains two descriptors: `{ type: "drive-connect" }`
and `{ type: "drive-disconnect" }`.
- The card's connect()/disconnect() bodies now run inside
`runSensitiveAction`, passed down as a prop. Their internal
try/catch keeps genuine failures local to the card (the card owns its
own error line), but rethrows MFA challenges so the wrapper can see
them:
} catch (e) {
if (isMfaRequiredError(e)) throw e; // page machinery's job
setError(...); // card-local failure
}
- Resumption needs the page to call *into* the card (the actions close
over card-local state like `setStatus`), so the card registers a
`GoogleDriveCardHandle` — `{ connect, disconnect }` — into a ref the
parent owns, via an every-render effect so the handle never captures
stale state. `handleMfaVerified` re-invokes the interrupted action
through that ref, exactly as it re-dispatches every other descriptor.
- One browser subtlety: the OAuth popup must be opened synchronously in
the click handler (an `await` before `window.open` loses the user-
activation token and the popup gets blocked), so the card still opens
it before entering the wrapper. On the resumed, post-verification run
there is no click; if the browser then refuses the popup, the
existing `window.location.assign(authorizationUrl)` fallback engages.
TESTING
frontend: npx tsc --noEmit clean; connectors page vitest suite (OAuth
poll cancellation) passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…just success
WHY THIS MATTERS
The Google Drive card's connect() opened its OAuth popup and closed it
on exactly one path: successful authorization. Every other way out of
the flow — startGoogleDriveOAuth() failing (backend down, OAuth client
unconfigured, MFA challenge), the five-minute authorization timeout,
and the user pressing the card's Cancel affordance — returned with the
window still open. The user is left staring at an orphaned blank
"about:blank" popup they have to hunt down and close by hand, and
repeated attempts stack them up. The pre-existing MCP flow in this same
file (connectConnectorOAuth) already solved this; the Drive card just
didn't follow it.
WHAT IS THE PATTERN: RESOURCE CLEANUP IN try/finally
A popup window is a resource exactly like a file handle or a lock: once
acquired, it must be released on *every* control-flow path, and the
number of exit paths only grows as code evolves (this function has at
least five). Enumerating them by hand — a close() sprinkled on each
early return — is how leaks happen; the next contributor adds a sixth
path and forgets. The robust shape is the one the language gives you:
const popup = window.open(...); // acquire
try {
...any number of returns/throws...
} finally {
try { popup?.close(); } catch {} // release, unconditionally
}
The inner try/catch around close() matters too: Google serves its
consent page with Cross-Origin-Opener-Policy: same-origin, which severs
the opener relationship; some browsers then throw on cross-origin
window operations. A failed close must not mask the real error that got
us into the finally.
HOW THE FIX WORKS
The success-path popup.close() moves into a finally block wrapping the
whole connect() body — mirroring connectConnectorOAuth's cleanup a few
hundred lines down. The finally also covers the MFA detour introduced
by the previous commit (runSensitiveAction may park the action and open
the verification popup before the OAuth flow even starts; the blank
popup opened with the click still gets closed). `popup?.close()` keeps
the popup-blocked fallback path (popup === null, full-page redirect)
safe.
TESTING
frontend: npx tsc --noEmit clean; connectors page vitest suite passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WHY THIS MATTERS
backend/schema.sql has a "Direct client grant hardening" section that
does two things, in order: (1) `revoke all ... from anon,
authenticated` per table, because the browser talks to Supabase
directly with those roles and backend-owned tables must not be readable
or writable from the client; (2) one bulk
`grant select, insert, update, delete on all tables in schema public to
service_role`, because the backend connects as service_role and the
tables are owned by the bootstrap role.
The new user_google_drive_tokens and google_drive_oauth_states tables
were appended AFTER that section, so on a fresh schema.sql bootstrap
both halves miss them:
- the bulk service_role grant executed before the tables existed
("on all tables" is expanded at execution time, not tracked
afterwards), so the backend has no privileges on its own token
store — every Drive status/connect call fails with a permission
error on a clean deployment;
- they are absent from the per-table revoke list, so whatever default
privileges the environment applies to new public tables (hosted
Supabase grants anon/authenticated on everything by default) stay in
place on the tables holding encrypted OAuth tokens.
WHAT IS DEFENSE IN DEPTH HERE
RLS is already enabled on both tables with no user policies, which
blocks anon/authenticated row access even where grants slip through —
that is why this ships as a hardening fix rather than a live token
leak. But the file's own convention is belt and braces: RLS guards
rows, revoked grants guard the table (including metadata like row
counts via ANALYZE-visible stats, and any future ALTER that disables
RLS), and the two fail independently.
HOW THE FIX WORKS
Both files gain the exact statements the sibling MCP OAuth tables use
(user_mcp_oauth_tokens / user_mcp_oauth_states in the hardening list):
revoke all on public.user_google_drive_tokens from anon, authenticated;
revoke all on public.google_drive_oauth_states from anon, authenticated;
grant select, insert, update, delete on ... to service_role;
- backend/schema.sql: appended inside the Drive section with a comment
explaining why the statements are repeated after the hardening
section (fresh-bootstrap correctness).
- backend/migrations/20260804_01_google_drive_integration.sql: the same
statements, because on an existing hosted database the migration —
not schema.sql — creates the tables, and Supabase default privileges
would otherwise grant the browser roles access (the same reason
20260724_02_tabular_folder_rows.sql carries its own revoke/grant
pair). No sequences are involved (both PKs are uuid), so no sequence
grants are needed.
TESTING
Statements mirror the existing, exercised pattern verbatim. The
Supabase stack suite (test:stack) that applies schema.sql needs a
running local Supabase Docker stack, which is not available in this
environment; backend unit suite passes (it does not exercise SQL
bootstrap).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… that cannot work
WHY THIS MATTERS
The Drive status endpoint already reports whether the deployment can do
this at all: `getGoogleDriveStatus()` returns `configured: false` when
the server has no GOOGLE_DRIVE_OAUTH_CLIENT_ID/_SECRET (and no
GOOGLE_MCP_OAUTH_* fallback). But no UI read the flag. On an
unconfigured self-hosted deployment the card rendered a perfectly
ordinary, enabled Connect button; clicking it opened a blank popup
(spawned optimistically, before the backend is asked for the
authorization URL), then the /oauth/start call failed and the user was
left with an error string and an empty window — for a condition that is
not an error at all, just a server that hasn't been set up.
WHAT IS THE PRINCIPLE: DON'T OFFER ACTIONS THAT CANNOT SUCCEED
A control's enabled state is a promise that the action can work. When
the client already knows an action is impossible — the capability flag
is sitting in state it fetched — showing a live button converts a
static configuration fact into a runtime failure the user has to
diagnose. The kind fix is to (a) disable the control and (b) say what
would make it available, so a self-hoster reads the next step off the
card instead of off a stack trace.
HOW THE FIX WORKS
- The Connect button's `disabled` becomes `busy || !status.configured`.
The shared account button style already renders a proper disabled
look (`disabled:cursor-not-allowed disabled:opacity-45`), so no new
styling is needed.
- When the card is neither loading nor connected and `configured` is
false, a one-line hint appears under the row pointing the
administrator at the "Google Drive Integration" section of the
README, which walks through the Cloud Console setup and the env vars.
- The status-fetch error fallback already fabricates
`{ connected: false, configured: false }`, so a backend that cannot
even answer the status probe now degrades to the same honest disabled
state instead of an enabled button that would fail identically.
TESTING
frontend: npx tsc --noEmit clean; connectors page vitest suite passes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WHY THIS MATTERS This branch was originally stacked on the Google MCP connector work (PR Open-Legal-Products#285), which introduced GOOGLE_MCP_OAUTH_CLIENT_ID/_SECRET as the OAuth client for Google-hosted MCP servers. The Drive integration politely fell back to those variables so one Cloud Console client could serve both features. PR Open-Legal-Products#285 was closed unmerged, so on main those variables are not read by anything else, are not documented anywhere, and are not part of any setup flow. Keeping the fallback would ship configuration surface that references a feature this codebase does not have: an operator grepping for GOOGLE_MCP_OAUTH_CLIENT_ID would find a consumer but no producer, and the error message would advertise "MCP equivalents" that no README explains. WHAT IS THE FALLBACK BEING REMOVED googleDriveOAuthEnv() resolved the client as GOOGLE_DRIVE_OAUTH_CLIENT_ID || GOOGLE_MCP_OAUTH_CLIENT_ID (and the same for the secret), and the fail-fast setup error suggested setting either pair. The test suite's env hygiene saved, deleted, and restored the MCP pair around every test so the fallback could not leak in. HOW THE FIX WORKS googleDriveOAuthEnv() now reads only GOOGLE_DRIVE_OAUTH_CLIENT_ID and GOOGLE_DRIVE_OAUTH_CLIENT_SECRET, the setup error names only that pair, and the test env bookkeeping for the MCP variables is deleted. This is config-surface-only: no OAuth, token, or tool behavior changes. If a Google MCP connector feature lands later, reintroducing a shared-client fallback is a one-line change made with both features in view. TESTING backend: npx tsc --noEmit clean; vitest suite passes including the googleDrive unit tests that assert the fail-fast setup error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
amal66
force-pushed
the
olp-pr/google-drive-native
branch
from
August 6, 2026 03:20
a061949 to
b2b85cc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #285 (shares its branch history — review only the last commit here until #285 lands).
What this adds
"Search my Google Drive for X and summarize it" as a one-click-connect experience: a Google Drive card on Account → Connectors runs a first-party OAuth flow, and three read-only chat tools (
google_drive_search,google_drive_read_file,google_drive_list_recent) call the GA Drive REST API directly. Google Docs/Sheets/Slides export as text; PDF and Word convert via the existing extractors; results carry the same untrusted-context framing as MCP tool output, and the chat UI renders the calls with the existing connector treatment (no event-pipeline changes).Why not the Drive MCP server
Google's hosted Drive MCP (
drivemcp.googleapis.com) is gated behind the Workspace Developer Preview Program. Verified live during #285's testing: with both APIs enabled and the full advertised scope set, OAuth succeeds andtools/listworks, but everytools/callreturns a barePERMISSION_DENIEDfor non-enrolled accounts — while the same token lists files over plain Drive REST without complaint. The REST API has no such gate, so the reliable path for stock deployments is first-party. (#285's MCP connector support still works for enrolled accounts.)Design notes
access_type=offline&prompt=consent(Google issues refresh tokens only with these — same lesson as fix: Google Workspace MCP connectors — durable refresh tokens + reliable OAuth popup #185). Tokens AES-GCM-encrypted at rest, hashed + TTL'd state rows, transparent refresh with 60s leeway;invalid_grantdeletes the row so status stays honest.drive.readonlyscope; Driveqvalues escape quotes/backslashes so user input can't break out of the query literal; 60k-char cap per file read.GOOGLE_MCP_OAUTH_*so one Cloud client serves both features. README gains a full self-hosting section (APIs to enable, redirect URI, test-mode vs restricted-scope verification).Testing
🤖 Generated with Claude Code