hub-mcp: add OAuth 2.0 device-flow auth (RFC 8628)#234
Merged
Conversation
Implementation plan for Design C' — Google as device-flow AS for the hub-mcp ↔ quarto-hub WebSocket. 10 phases, TDD-first; Phase 1 records empirical-verification lock-ins (2026-05-19).
…(Phase 2) Land Phase 2 of the hub-mcp Google device-flow plan. The hub now accepts two credential kinds (HttpOnly cookie from the SPA, `Authorization: Bearer <jwt>` from quarto-hub-mcp), gates CSRF + WS-Origin on the cookie kind only, rejects dual-credential requests with HTTP 400, and emits per-call audit events via `tracing::event!`. Key surface: - `AuthConfig.additional_audiences: Vec<String>` + `audiences()`. - `OidcClaims` carries `aud` (string-or-array deserialized), `azp`, `iat`. - `validate_azp_and_iat` implements the OIDC §3.1.3.7 azp rule and the future-`iat` rejection the `jsonwebtoken` validator does not cover. - `build_auth_state_from_parts` lets tests inject the decoder without OIDC discovery (production path still does discovery). - `CredentialKind`, `Credential`, `extract_credential`; `Authenticated` now carries the kind; `update_document`, `auth_logout`, `auth_refresh` gate CSRF on it; the WS handler gates Origin on it. - `--additional-audiences` / `QUARTO_HUB_ADDITIONAL_AUDIENCES` plumbed through both `hub` and `quarto hub`. 33 new integration tests in `crates/quarto-hub/tests/auth_bearer.rs` (mock OIDC provider, in-process axum hub on random port, RS256 JWT minting, captured tracing events) plus 7 lib tests. `cargo xtask verify --skip-hub-build` clean. Closes bd-qabnx (design lock-in), bd-0ufq5 (Phase 2 task), and bd-wzhsf (dual-credential CVE-prevention). Plan: claude-notes/plans/2026-05-05-hub-mcp-device-flow-implementation.md §Phase 2.
New src/auth/device-flow.ts on oauth4webapi: initiateDeviceFlow uses
None() clientAuth so client_secret never travels with the device-auth
request; pollDeviceFlowOnce performs a single token-endpoint poll and
maps oauth errors to typed DeviceFlowDeniedError / DeviceFlowExpiredError
or to {kind:'pending'|'slow_down'} for the caller to schedule. Env
sourcing via loadDeviceFlowConfigFromEnv refuses to start without both
QUARTO_HUB_MCP_CLIENT_ID and QUARTO_HUB_MCP_CLIENT_SECRET. redactTokens
strips ya29.*, 1//*, and JWT-shaped substrings; every log call site
funnels through it. 23 Vitest specs cover the surface; npm run typecheck
clean. Phase 3 (audit logging) was already landed inside Phase 2 — plan
updated to document.
Adds `CredentialStore` backed by `@napi-rs/keyring` (DPAPI / Keychain / Secret Service). Schema_version-1 JSON blob keyed by `dev.quarto.hub-mcp:<issuer>:<client_id>`; injectable `KeyringBackend` for unit tests; in-process tail-promise mutex; asymmetric error handling (read folds to null with redacted warning, write/clear re-wrap as `KeyringUnavailableError` with `redactTokens` over the backend message to prevent token-byte leakage). Widens the Phase-4 `no_baked_default_client_id_or_secret` walker to skip every `*.test.ts` so fixture client_id strings are allowed in tests.
RefreshManager wraps the credential store with Google's /token refresh-token grant. `getValidIdToken` returns the cached id_token or proactively refreshes within a 60s skew of expiry; `forceRefresh` is the 401-retry primitive for Phase 8's connection-manager. An in-flight Promise mutex coalesces concurrent callers onto a single /token POST. `invalid_grant` clears the store and throws a typed ReauthRequired; all other failures leave the store byte-identical.
…ase 7) New auth-tools module exposes the two MCP tools the agent drives to complete a Google device flow. AuthToolsState owns the closure-scoped device_code cache and enforces RFC 8628 §3.5 rate limiting; the canonical verification URL is a hard-coded constant in this module so an attacker who controls Google's response cannot phish via the verification_uri. ConnectionManager gains a stub lastObservedAuthMode() that Phase 8 will wire up.
Ships the connection-manager side of the Google device-flow auth chain.
quarto-sync-client gains a Node-only `NodeWebSocketClientAdapter` that
constructs `new WebSocket(url, [], { headers })` so the WS upgrade carries
`Authorization: Bearer <token>`; browser bundles never pull it in.
ConnectionManager runs a `/health` probe + try-then-fallback policy
(401 → forceRefresh + retry → ReauthRequired; no-creds + 401 →
AuthRequiredError), threads `lastObservedAuthMode` for Phase 7's
short-circuit, and gates Bearer-over-plain-HTTP to non-loopback hosts
behind `QUARTO_HUB_MCP_ALLOW_INSECURE_AUTH=1`.
Record cargo xtask verify, dual-credential CVE (400 + conflicting_credentials body), Bearer-skips/Cookie-blocks asymmetric Origin gating, audit-log shape, hub-mcp env smokes, and plaintext-leak grep. bd-cxara in_progress; browser-driven sub-items deferred to user verification.
Adds an operator-facing setup doc and the end-user package README;
ticks the Phase 10 checklist and records the completion notes.
Includes a regression test (tracing_redacts_google_token_shapes)
that drives ya29.* / 1//* token shapes through the rejected-token
audit path to confirm the jwt_decode:{err} detail field never
carries token bytes.
…ation_uri Google's /device/code endpoint returns verification_url; oauth4webapi strictly asserts the RFC 8628 verification_uri field and throws OperationProcessingError otherwise, so every live authenticate_start failed before the device flow could begin. initiateDeviceFlow now rewrites the response body to add the canonical field when only the Google-shape one is present. Phase 4 fixtures used the RFC field and hid the gap; the Phase 0 verification log's "oauth4webapi normalises" claim was wrong and is corrected.
When the hub rejected a freshly-refreshed id_token, ConnectionManager threw ReauthRequired but left the new token in the keyring. The next authenticate_start short-circuited on "Already authenticated as …", trapping the agent in a silent state mismatch between local view (token valid) and hub view (token rejected). Clearing the store before throwing forces getValidIdToken to raise ReauthRequired, so the next authenticate_start falls through to a fresh device flow.
Adds a third auth tool that removes the persisted credential bundle from the OS keyring and discards any cached device_code. Idempotent and destructive (annotated accordingly). Use cases: explicit escape hatch when the agent gets stuck against a hub that no longer accepts the cached identity, switching Google accounts mid-session, operator debugging. Does not touch Google-side grants — revocation still happens at myaccount.google.com.
quarto-sync-client unconditionally constructed `new IndexedDBStorageAdapter()` in `connect` / `createNewProject`. The adapter reads the `indexedDB` global eagerly, so Node throws `indexedDB is not defined` at construction. New buildStorageAdapter selector picks an in-memory MemoryStorageAdapter when the global is absent and IndexedDB when it is present; both client.ts sites route through it. The 6 "pre-existing" failures in hub-mcp.test.ts labelled as baseline in Phase 5-8 completion notes were this same bug — they are the only specs that exercise the real SyncClient and now pass against wss://sync.automerge.org.
Was pointing at wss://sync.automerge.org (Automerge's public demo server, useful as a try-it-out config when hub-mcp first landed). Switching to our production hub now that the auth surface is in place; users who want a different endpoint should override via a user-scope `claude mcp add -s user` entry.
Covers the two paths Phase 9 leaves to user-driven verification: SPA Google sign-in (cookie) and hub-mcp device flow (Bearer). Splits one-time setup (Google Cloud Console origins/redirect URIs, claude mcp add) from per-session setup (env vars, hub launch). Six items (3, 4, 5, 6, 4b, 4a) ordered to minimise hub restarts; optional keyring helpers in their own section.
shikokuchuo
changed the title
`Basic dXNlcjpwYXNz` triggered GitHub's hardcoded-HTTP-basic-credential detector even though `user:pass` is the textbook placeholder. Replace with `Basic placeholder` / `Token placeholder` — the hub's extractor rejects unknown schemes on the prefix without parsing the rest, so the value's shape doesn't matter for the assertion.
Collapses three sequential String.replace passes into one alternation regex so we scan the input once per call instead of three times.
Pass cbor's Uint8Array straight to socket.send instead of slicing into a fresh ArrayBuffer per message. The Node `ws` package and browser WebSocket both accept Uint8Array directly; WebSocketLike's `send` parameter widens to match.
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.
Closes #51.
Adds Google OAuth 2.0 device-authorization grant (RFC 8628) so
quarto-hub-mcpcan authenticate toquarto-hub. Hub-mcp runs the device flow in the user's browser, persists the resulting ID + refresh tokens in the OS keyring, and attachesAuthorization: Bearer …on the WS upgrade. The hub keeps its existing JWKS validator; SPA cookie path unchanged. Phase log:claude-notes/plans/2026-05-05-hub-mcp-device-flow-implementation.md.Hub (
crates/quarto-hub)--additional-audiencesfor the SPA + hub-mcp client pair; OIDC §3.1.3.7azpenforcement; future-iatrejection.400 {"error":"conflicting_credentials"}when bothCookieandAuthorizationare presented — closes the CVE-shaped gap that drove this audit.credential_kind: Bearer skips both, cookie still enforces.tracing::event!(target: "quarto_hub::audit", action, outcome, credential_kind, sub, detail?)per auth decision;Authorization/Cookiescrubbed from request spans.hub-mcp (
ts-packages/quarto-hub-mcp)authenticate_start,authenticate_finish,authenticate_clear(destructive/idempotent escape hatch).@napi-rs/keyring(Keychain / Secret Service / DPAPI). No plaintext on disk; no silent fallback.invalid_grantclears store and throwsReauthRequired.ConnectionManagerprobes/health, refreshes on 401, retries once, clears store + throwsReauthRequiredon persistent 401.lastObservedAuthModeletsauthenticate_startshort-circuit against no-auth hubs.QUARTO_HUB_MCP_ALLOW_INSECURE_AUTH=1.redactTokensscrubsya29.*,1//*, and JWT-shaped substrings from every log call site;uncaughtException/unhandledRejectionhandlers scrub stack traces.quarto-sync-clientNodeWebSocketClientAdapterthreads Bearer through the WS upgrade viaws. Browser path unchanged.MemoryStorageAdapter(Node) orIndexedDBStorageAdapter(browser) — fixes theindexedDB is not definedruntime error that had been suppressed as "baseline" since Phase 5.Bug fixes found during Phase 9
verification_url→verification_urinormalisation ininitiateDeviceFlow;oauth4webapirejected every live Google response without it.ConnectionManagerclears the credential store on persistent 401 so the nextauthenticate_startinitiates a fresh flow instead of replying "Already authenticated".Operator + docs
.mcp.jsonnow points atwss://quarto-hub.com/ws. Operator registers a "TV and Limited Input devices" OAuth client alongside the SPA's and publishesQUARTO_HUB_MCP_CLIENT_ID/QUARTO_HUB_MCP_CLIENT_SECRETto end users.claude-notes/instructions/hub-mcp-operator-runbook.md,ts-packages/quarto-hub-mcp/README.md,claude-notes/instructions/auth-verification.md.Verification
cargo xtask verifygreen. 33 new Bearer-path integration specs inquarto-hub; hub-mcp 144/144; sync-client 91/91. Autonomous half of Phase 9 recorded in the plan; user-driven half walked through inauth-verification.md.