inspector/core/mcp/state/fetchRequestLogState.ts
Cliff Hall 98192fd86e
fix(auth): persist OAuth pre-redirect Network log across the redirect (#1384) (#1385)
* feat(auth): inject MCP_INSPECTOR_API_TOKEN into served index.html (#1378)

Reloading the web client at the bare URL (no `?MCP_INSPECTOR_API_TOKEN=…`
query string) with empty sessionStorage made every `/api/*` request 401 —
the browser had no way to recover the backend's auth token.

Embed the token into `index.html` on every page load so the browser no
longer depends on the query string surviving navigation:

- New shared helper `clients/web/server/inject-auth-token.ts` embeds
  `<script>window.__INSPECTOR_API_TOKEN__ = "…"</script>` (escaped against
  `</script>` injection; no-op when auth is dangerously omitted).
- Dev: the Vite plugin injects via `transformIndexHtml`.
- Prod: the Hono server injects on the `/` route.
- `App.tsx` `getAuthToken()` now reads the injected global first, then the
  query string, then sessionStorage (both fallbacks preserved).
- Shared global name lives in `INSPECTOR_API_TOKEN_GLOBAL`
  (`core/mcp/remote/constants.ts`).

Tests: helper unit coverage + an integration test exercising the real
prod server's `/` → `/api/*` flow (injected token authenticates; missing
token 401s). AGENTS.md documents the token-recovery order.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* feat(auth): wire OAuth authorization-code flow into App.tsx (#1379)

OAuth-protected MCP servers could not be connected to from the v2 web
client: the core OAuth pipeline exists, but App.tsx never invoked it, so a
connect attempt 401'd and surfaced "Remote send failed (401): … Missing
Authorization header" as a toast.

Wire the two missing entry points (all core primitives already in place):

- Auto-trigger on 401: in onToggleConnection's catch, detect an upstream
  401 (isUnauthorizedError) and call client.authenticate(), which runs
  discovery + DCR (backend-proxied) and redirects the page to the auth
  server via BrowserNavigation. The initiating server id is persisted to
  sessionStorage first, since the OAuth `state` carries only mode+authId
  and the full-page redirect wipes React state.
- /oauth/callback handler: a mount effect that, once `servers` hydrate,
  parses the callback params, recovers the pending server, rebuilds its
  InspectorClient, runs completeOAuthFlow(code) (PKCE verifier + DCR client
  info survive in BrowserOAuthStorage), replaceState("/") so a reload can't
  replay the single-use code, then connect(). An `error=` callback toasts
  instead of retrying.

connect() already attaches the OAuth provider to the transport
(inspectorClient.ts), so once tokens land in BrowserOAuthStorage the
outbound request carries the bearer token.

Extracted the pure pieces (constants + isUnauthorizedError) to
src/utils/oauthFlow.ts with unit tests. Verified end-to-end in a real
browser against the MCP SDK demo OAuth server: Connect -> redirect ->
auto-approve -> callback -> Connected, with the access token shown in the
Connection Info modal (#1377).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): persist OAuth pre-redirect Network log across the redirect (#1384)

After #1379, the Network tab showed only the post-redirect auth HTTP
(discovery re-run + POST /token); the pre-redirect discovery and the DCR
POST /register that run during authenticate() were lost when the page
navigated to the auth server.

Root causes:

1. Ordering — BrowserNavigation set `window.location.href` before the
   client's `saveSession` event fired (OAuthManager calls onBeforeOAuthRedirect
   *after* auth() already navigated), so the save raced the unload and was
   dropped. Fix: BrowserNavigation now runs a synchronous `beforeNavigate`
   hook immediately before assigning location.href; App wires it through
   createWebEnvironment to flush the active fetch log to RemoteInspectorClient
   Storage (keyed by the authId parsed from the auth URL) via a keepalive POST
   that outlives the unload.

2. Illegal invocation — RemoteInspectorClientStorage defaulted to
   `this.fetchFn = globalThis.fetch` and called `this.fetchFn(...)`, which
   re-binds `this` and makes native fetch throw "Illegal invocation"
   (swallowed by the catch). This silently broke *all* session save/load.
   Fix: default to a wrapper that preserves the global receiver.

3. Restore race — hydrateFetchRequests replaced the list, so a load that
   resolved after the resuming connect appended live entries would clobber
   them. Fix: merge restored (older) entries ahead of live ones, dedupe by id.

saveSession also now uses keepalive: true.

Verified end-to-end against the MCP SDK demo OAuth server: the connected
page's Network tab shows the full handshake — pre-redirect discovery + DCR
/register plus post-redirect discovery + /token as `auth`, alongside
`transport`. Added unit tests for the beforeNavigate ordering and the
hydrate merge/dedupe.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): inject token into prod SPA fallback + prime sessionStorage + no-store (#1378)

Addresses code-review feedback on the token-injection PR:

- Prod `server.ts` SPA deep-link fallback (e.g. `/oauth/callback`) previously
  served the raw index.html off disk via serveStatic, bypassing injection — so
  a bookmark/reload at a non-`/` route with empty sessionStorage would 401.
  Route the SPA fallback through the same `serveIndexHtml` (inject) helper;
  real static assets (paths with a dot) still serve verbatim. Dev already
  injected on every HTML serve via Vite `transformIndexHtml`.
- `getAuthToken()` now persists the injected `window.__INSPECTOR_API_TOKEN__`
  to sessionStorage (not just the URL-param branch), priming the backstop for
  any later navigation that loses the global.
- Injected HTML responses now send `Cache-Control: no-store`, so a page
  carrying a token isn't cached and served stale after a restart regenerates
  the token.

Integration tests added: SPA fallback (`/oauth/callback`) carries the token,
`Cache-Control: no-store` on injected HTML, real assets served verbatim, and
unknown `/api` routes 404 rather than falling through to the HTML shell.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(auth): address #1383 review — split OAuth/connect toasts, tighten 401 match (#1379)

Code-review feedback on the OAuth-wiring PR:

- Callback effect: split completeOAuthFlow vs connect() into separate
  try/catch blocks. A token-exchange failure now reads "OAuth token exchange
  failed … Please try connecting again." (the single-use code is spent and the
  URL was cleared, so a reload can't retry); a post-OAuth connect failure reads
  "Failed to connect" since OAuth actually succeeded and re-clicking Connect
  reuses the persisted tokens.
- isUnauthorizedError: anchor the message fallback on the transport's
  `failed …(401)` wording instead of a bare `(401)`, so an unrelated `(401)`
  spliced into an error message can't trip the OAuth flow. Added a test.
- Documented that clearing the pending id + URL before the server lookup is
  intentional (deleted/renamed server mid-flow → require a fresh Connect).

Also merges the squash-merged #1382 base from v2/main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(auth): address #1385 review — clarify double-save + keepalive cap + test fetch default (#1384)

Code-review feedback on the OAuth Network-log persistence PR:

- Documented the double-save: `FetchRequestLogState`'s `saveSession` listener
  is the backstop; `BrowserNavigation`'s `beforeNavigate` hook is the primary
  flush for the redirect case. Notes the listener may lose the navigation race
  and is harmless when it duplicates (last-writer-wins, identical payload).
- Reworded the keepalive comment in `RemoteInspectorClientStorage.saveSession`:
  the 64KB cap is general (the method is also reachable from the listener with
  the full session log), so a long session could exceed it and drop silently —
  acceptable since the persisted log is best-effort, not load-bearing.
- Added a regression test that constructs `RemoteInspectorClientStorage`
  without a `fetchFn`, stubs `globalThis.fetch`, and asserts the default
  wrapper calls it (locks in the "Illegal invocation" fix, which callers
  otherwise swallow).

Optional items (logger.warn on swallowed save errors; setupClientForServer dep
churn) acknowledged on the PR, not changed.

Also merges the squash-merged #1383 base from v2/main.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-05-30 20:39:25 -04:00

206 lines
7.5 KiB
TypeScript

/**
* FetchRequestLogState: holds the fetch request log, subscribes to the protocol
* "fetchRequest" event. Protocol emits per-entry; this manager owns the list and
* emits both `fetchRequest` (single entry) and `fetchRequestsChange` (full list)
* on append, and `fetchRequestsChange` on clear. Does not clear on connect or
* disconnect.
*
* When `sessionStorage` and `sessionId` are provided, restores fetch requests
* from storage on creation and listens for the client's `saveSession` event to
* persist before OAuth redirect.
*
* Ported from v1.5/main. v2 substitutes `InspectorClientProtocol` for the
* concrete `InspectorClient` since the runtime class is not yet ported.
*/
import type { InspectorClientProtocol } from "../inspectorClientProtocol.js";
import type { FetchRequestEntry } from "../types.js";
import type {
InspectorClientStorage,
InspectorClientSessionState,
} from "../sessionStorage.js";
import type { InspectorClientEventMap } from "../inspectorClientEventTarget.js";
import {
TypedEventTarget,
type TypedEventGeneric,
} from "../typedEventTarget.js";
export interface FetchRequestLogStateEventMap {
fetchRequest: FetchRequestEntry;
fetchRequestsChange: FetchRequestEntry[];
}
export interface FetchRequestLogStateOptions {
/**
* Maximum number of fetch requests to store (0 = unlimited, not recommended).
* When exceeded, oldest entries are dropped. Default 1000.
*/
maxFetchRequests?: number;
/**
* When provided with sessionId, fetch requests are restored from storage on
* creation and saved when the client dispatches `saveSession` (e.g. before
* OAuth redirect).
*/
sessionStorage?: InspectorClientStorage;
/** Session ID for load/save; required for sessionStorage to have effect. */
sessionId?: string;
}
export class FetchRequestLogState extends TypedEventTarget<FetchRequestLogStateEventMap> {
private fetchRequests: FetchRequestEntry[] = [];
private client: InspectorClientProtocol | null = null;
private unsubscribe: (() => void) | null = null;
private readonly maxFetchRequests: number;
constructor(
client: InspectorClientProtocol,
options: FetchRequestLogStateOptions = {},
) {
super();
this.maxFetchRequests = options.maxFetchRequests ?? 1000;
this.client = client;
const onFetchRequest = (
event: TypedEventGeneric<InspectorClientEventMap, "fetchRequest">,
): void => {
const entry = event.detail;
if (
this.maxFetchRequests > 0 &&
this.fetchRequests.length >= this.maxFetchRequests
) {
this.fetchRequests.shift();
}
this.fetchRequests.push(entry);
this.dispatchTypedEvent("fetchRequest", entry);
this.dispatchTypedEvent("fetchRequestsChange", this.getFetchRequests());
};
this.client.addEventListener("fetchRequest", onFetchRequest);
// Body is read asynchronously by fetchTracking and arrives via a
// separate event; patch the matching entry in place and re-emit so
// React subscribers re-render with the body filled in.
const onFetchRequestBodyUpdate = (
event: TypedEventGeneric<
InspectorClientEventMap,
"fetchRequestBodyUpdate"
>,
): void => {
const { id, responseBody } = event.detail;
const idx = this.fetchRequests.findIndex((e) => e.id === id);
if (idx === -1) return;
this.fetchRequests[idx] = {
...this.fetchRequests[idx]!,
responseBody,
};
this.dispatchTypedEvent("fetchRequestsChange", this.getFetchRequests());
};
this.client.addEventListener(
"fetchRequestBodyUpdate",
onFetchRequestBodyUpdate,
);
const sessionStorage = options.sessionStorage;
const sessionId = options.sessionId;
if (sessionStorage) {
// Backstop persistence on the client's `saveSession` event. For the
// OAuth full-page-redirect case the web client's `BrowserNavigation`
// `beforeNavigate` hook is the *primary* flush (it runs synchronously
// before navigation, so a keepalive request survives the unload); this
// listener fires from the same event but can lose the race with an
// already-scheduled navigation. It remains the save path for any
// non-redirect `saveSession` caller (e.g. a future token-refresh save
// point) and is harmless when it duplicates the primary flush —
// last-writer-wins on an identical payload under the same id.
const onSaveSession = (
event: TypedEventGeneric<InspectorClientEventMap, "saveSession">,
): void => {
const { sessionId: id } = event.detail;
const state: InspectorClientSessionState = {
fetchRequests: this.getFetchRequests(),
createdAt: Date.now(),
updatedAt: Date.now(),
};
sessionStorage.saveSession(id, state).catch(() => {
// fire-and-forget; storage may log internally
});
};
this.client.addEventListener("saveSession", onSaveSession);
this.unsubscribe = () => {
if (this.client) {
this.client.removeEventListener("fetchRequest", onFetchRequest);
this.client.removeEventListener(
"fetchRequestBodyUpdate",
onFetchRequestBodyUpdate,
);
this.client.removeEventListener("saveSession", onSaveSession);
}
this.client = null;
};
if (sessionId) {
sessionStorage
.loadSession(sessionId)
.then((state) => {
if (this.client && state?.fetchRequests?.length) {
this.hydrateFetchRequests(state.fetchRequests);
}
})
.catch(() => {
// fire-and-forget; storage may log internally. Matches the
// saveSession swallow above so a corrupt or unreadable session
// doesn't surface as an unhandled rejection.
});
}
} else {
this.unsubscribe = () => {
if (this.client) {
this.client.removeEventListener("fetchRequest", onFetchRequest);
this.client.removeEventListener(
"fetchRequestBodyUpdate",
onFetchRequestBodyUpdate,
);
}
this.client = null;
};
}
}
// Restore persisted entries (e.g. the pre-redirect OAuth Network log loaded
// on the `/oauth/callback` page). Merges rather than replaces: the async
// load races against entries appended live by the resuming connect
// (`completeOAuthFlow` + transport handshake), so we must not clobber
// whichever arrived first. Restored entries are older, so they go in front;
// duplicates (by id) already present from a live append are skipped.
private hydrateFetchRequests(entries: FetchRequestEntry[]): void {
if (entries.length === 0) return;
const existingIds = new Set(this.fetchRequests.map((e) => e.id));
const restored = entries.filter((e) => !existingIds.has(e.id));
if (restored.length === 0) return;
const merged = [...restored, ...this.fetchRequests];
this.fetchRequests =
this.maxFetchRequests > 0
? merged.slice(-this.maxFetchRequests)
: merged;
this.dispatchTypedEvent("fetchRequestsChange", this.getFetchRequests());
}
getFetchRequests(): FetchRequestEntry[] {
return [...this.fetchRequests];
}
/**
* Clear all fetch requests. Dispatches fetchRequestsChange only if the list
* was non-empty.
*/
clearFetchRequests(): void {
if (this.fetchRequests.length === 0) return;
this.fetchRequests = [];
this.dispatchTypedEvent("fetchRequestsChange", []);
}
destroy(): void {
this.unsubscribe?.();
this.unsubscribe = null;
this.fetchRequests = [];
}
}