refactor: migrate to RemoteOAuthStorage for web OAuth handling
This commit is contained in:
parent
d6fe2688e2
commit
7bd26c938e
9 changed files with 317 additions and 44 deletions
|
|
@ -86,7 +86,7 @@ import { useServers } from "@inspector/core/react/useServers.js";
|
|||
import { useSettingsDraft } from "@inspector/core/react/useSettingsDraft.js";
|
||||
import { useClientSettingsDraft } from "@inspector/core/react/useClientSettingsDraft.js";
|
||||
import { useEmaIdpLoginState } from "@inspector/core/react/useEmaIdpLoginState.js";
|
||||
import { getBrowserOAuthStorage } from "@inspector/core/auth/browser/index.js";
|
||||
import { getWebRemoteOAuthStorage } from "./lib/remoteOAuthStorage";
|
||||
import { useManagedTools } from "@inspector/core/react/useManagedTools.js";
|
||||
import { useManagedPrompts } from "@inspector/core/react/useManagedPrompts.js";
|
||||
import { useManagedResources } from "@inspector/core/react/useManagedResources.js";
|
||||
|
|
@ -1428,6 +1428,13 @@ function App() {
|
|||
[messages],
|
||||
);
|
||||
|
||||
// Shared OAuth runtime store (oauth.json via /api/storage/oauth). Memoized so
|
||||
// connect, EMA IdP session, and per-server clear share one in-memory view.
|
||||
const webOAuthStorage = useMemo(
|
||||
() => getWebRemoteOAuthStorage(getAuthToken()),
|
||||
[],
|
||||
);
|
||||
|
||||
// Backend-backed session storage used to carry the fetch (Network) log
|
||||
// across the OAuth full-page redirect. The auth handshake's first half —
|
||||
// protected-resource + auth-server discovery and Dynamic Client
|
||||
|
|
@ -2098,9 +2105,10 @@ function App() {
|
|||
// `onToggleConnection` unloaded the previous one), so all React state is
|
||||
// reset and we recover the initiating server from sessionStorage. We wait for
|
||||
// `servers` to hydrate before acting; the ref guard keeps the exchange to a
|
||||
// single run. The persisted PKCE verifier + DCR client info live in
|
||||
// `BrowserOAuthStorage` and survive the redirect, so `completeOAuthFlow`
|
||||
// exchanges the code without needing the original in-memory state machine.
|
||||
// single run. The persisted PKCE verifier + DCR client info live in shared
|
||||
// `RemoteOAuthStorage` (`oauth.json`) and survive the redirect, so
|
||||
// `completeOAuthFlow` exchanges the code without needing the original
|
||||
// in-memory state machine.
|
||||
useEffect(() => {
|
||||
if (typeof window === "undefined") return;
|
||||
if (window.location.pathname !== OAUTH_CALLBACK_PATH) return;
|
||||
|
|
@ -3239,10 +3247,9 @@ function App() {
|
|||
|
||||
const clientSettingsModalValue = clientSettingsDraft ?? EMPTY_CLIENT_SETTINGS;
|
||||
|
||||
const emaOAuthStorage = useMemo(() => getBrowserOAuthStorage(), []);
|
||||
const { loginState: emaIdpLoginState, logout: logoutEmaIdp } =
|
||||
useEmaIdpLoginState(
|
||||
emaOAuthStorage,
|
||||
webOAuthStorage,
|
||||
clientSettingsModalValue.emaEnabled
|
||||
? clientSettingsModalValue.issuer
|
||||
: undefined,
|
||||
|
|
@ -3270,6 +3277,7 @@ function App() {
|
|||
config: server.config,
|
||||
inspectorClient: isActive ? inspectorClient : null,
|
||||
isActiveConnection: isActive,
|
||||
oauthStorage: webOAuthStorage,
|
||||
});
|
||||
if (!cleared) return;
|
||||
|
||||
|
|
@ -3295,6 +3303,7 @@ function App() {
|
|||
[
|
||||
activeServerId,
|
||||
inspectorClient,
|
||||
webOAuthStorage,
|
||||
finalizeExplicitDisconnect,
|
||||
clearOAuthResumeOnExplicitDisconnect,
|
||||
],
|
||||
|
|
|
|||
124
clients/web/src/lib/environmentFactory.test.ts
Normal file
124
clients/web/src/lib/environmentFactory.test.ts
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { BrowserNavigation } from "@inspector/core/auth/browser/index.js";
|
||||
import { MutableRedirectUrlProvider } from "@inspector/core/auth/providers.js";
|
||||
import { createWebEnvironment } from "./environmentFactory";
|
||||
import {
|
||||
getWebRemoteOAuthStorage,
|
||||
resetWebRemoteOAuthStorageCacheForTests,
|
||||
} from "./remoteOAuthStorage";
|
||||
|
||||
describe("createWebEnvironment", () => {
|
||||
beforeEach(() => {
|
||||
vi.stubGlobal("window", {
|
||||
location: {
|
||||
protocol: "http:",
|
||||
host: "127.0.0.1:6299",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
resetWebRemoteOAuthStorageCacheForTests();
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("wires RemoteOAuthStorage shared with getWebRemoteOAuthStorage", () => {
|
||||
const redirectUrlProvider = new MutableRedirectUrlProvider();
|
||||
const first = createWebEnvironment("unit-test-token", redirectUrlProvider);
|
||||
const second = createWebEnvironment("unit-test-token", redirectUrlProvider);
|
||||
|
||||
expect(first.environment.oauth).toBeDefined();
|
||||
expect(second.environment.oauth).toBeDefined();
|
||||
expect(second.environment.oauth!.storage).toBe(
|
||||
first.environment.oauth!.storage,
|
||||
);
|
||||
expect(first.environment.oauth!.storage).toBe(
|
||||
getWebRemoteOAuthStorage("unit-test-token"),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses BrowserNavigation for oauth.navigation", () => {
|
||||
const { environment } = createWebEnvironment(
|
||||
undefined,
|
||||
new MutableRedirectUrlProvider(),
|
||||
);
|
||||
expect(environment.oauth).toBeDefined();
|
||||
expect(environment.oauth!.navigation).toBeInstanceOf(BrowserNavigation);
|
||||
});
|
||||
|
||||
it("returns the same logger instance as environment.logger", () => {
|
||||
const { environment, logger } = createWebEnvironment(
|
||||
"tok",
|
||||
new MutableRedirectUrlProvider(),
|
||||
);
|
||||
expect(logger).toBe(environment.logger);
|
||||
});
|
||||
|
||||
it("passes redirectUrlProvider into oauth config", () => {
|
||||
const redirectUrlProvider = new MutableRedirectUrlProvider();
|
||||
redirectUrlProvider.redirectUrl = "http://127.0.0.1:6299/oauth/callback";
|
||||
const { environment } = createWebEnvironment("tok", redirectUrlProvider);
|
||||
if (!environment.oauth) {
|
||||
throw new Error("expected oauth config");
|
||||
}
|
||||
const { redirectUrlProvider: oauthRedirect } = environment.oauth;
|
||||
if (!oauthRedirect) {
|
||||
throw new Error("expected redirectUrlProvider");
|
||||
}
|
||||
expect(oauthRedirect.getRedirectUrl()).toBe(
|
||||
"http://127.0.0.1:6299/oauth/callback",
|
||||
);
|
||||
});
|
||||
|
||||
it("forwards onBeforeOAuthRedirect to BrowserNavigation", () => {
|
||||
const onBeforeOAuthRedirect = vi.fn<(authorizationUrl: URL) => void>();
|
||||
const { environment } = createWebEnvironment(
|
||||
"tok",
|
||||
new MutableRedirectUrlProvider(),
|
||||
onBeforeOAuthRedirect,
|
||||
);
|
||||
if (!environment.oauth) {
|
||||
throw new Error("expected oauth config");
|
||||
}
|
||||
const { navigation } = environment.oauth;
|
||||
if (!navigation) {
|
||||
throw new Error("expected navigation");
|
||||
}
|
||||
const authUrl = new URL("https://idp.example/authorize?state=abc");
|
||||
navigation.navigateToAuthorization(authUrl);
|
||||
expect(onBeforeOAuthRedirect).toHaveBeenCalledWith(authUrl);
|
||||
});
|
||||
|
||||
it("routes fetch through the wrapped global fetch at the window origin", async () => {
|
||||
const remoteBody = {
|
||||
ok: true,
|
||||
status: 200,
|
||||
statusText: "OK",
|
||||
headers: { "content-type": "text/plain" },
|
||||
body: "echoed",
|
||||
};
|
||||
const fetchMock = vi.fn<typeof fetch>().mockImplementation(
|
||||
async () =>
|
||||
new Response(JSON.stringify(remoteBody), {
|
||||
status: 200,
|
||||
headers: { "content-type": "application/json" },
|
||||
}),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const { environment } = createWebEnvironment(
|
||||
"api-tok",
|
||||
new MutableRedirectUrlProvider(),
|
||||
);
|
||||
fetchMock.mockClear();
|
||||
|
||||
const res = await environment.fetch!("http://upstream.test/mcp");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(await res.text()).toBe("echoed");
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
expect(String(fetchMock.mock.calls[0]?.[0])).toBe(
|
||||
"http://127.0.0.1:6299/api/fetch",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -4,11 +4,9 @@ import {
|
|||
createRemoteFetch,
|
||||
createRemoteLogger,
|
||||
} from "@inspector/core/mcp/remote/index.js";
|
||||
import {
|
||||
getBrowserOAuthStorage,
|
||||
BrowserNavigation,
|
||||
} from "@inspector/core/auth/browser/index.js";
|
||||
import { BrowserNavigation } from "@inspector/core/auth/browser/index.js";
|
||||
import type { RedirectUrlProvider } from "@inspector/core/auth/index.js";
|
||||
import { getWebRemoteOAuthStorage } from "./remoteOAuthStorage.js";
|
||||
|
||||
export interface WebEnvironmentResult {
|
||||
environment: InspectorClientEnvironment;
|
||||
|
|
@ -20,16 +18,17 @@ export interface WebEnvironmentResult {
|
|||
* - transport / fetch / logger all routed through the in-process Hono
|
||||
* backend at `window.location.origin` (the `clients/web/server`
|
||||
* dev-backend wires this in `/api/*`).
|
||||
* - OAuth storage + navigation use the `BrowserOAuthStorage` (sessionStorage)
|
||||
* and `BrowserNavigation` (full-page redirect) adapters.
|
||||
* - OAuth storage uses `RemoteOAuthStorage` (shared `oauth.json` via
|
||||
* `/api/storage/oauth`); navigation uses `BrowserNavigation`.
|
||||
*
|
||||
* Returns both the assembled environment and the logger so callers can share
|
||||
* the same pino instance for any direct logging they need to do, instead of
|
||||
* reaching back through the client.
|
||||
*
|
||||
* `authToken` is read from a higher level (currently unused in this app since
|
||||
* v2 has no auth-token UI yet, but kept in the signature so the wiring is
|
||||
* ready when token plumbing lands).
|
||||
* `authToken` is supplied by `App.tsx` (`getAuthToken()`) and forwarded to
|
||||
* remote transport/fetch/logger and `getWebRemoteOAuthStorage` so every
|
||||
* `/api/*` call (including `/api/storage/oauth`) carries `x-mcp-remote-auth`
|
||||
* when the backend requires it.
|
||||
*
|
||||
* `onBeforeOAuthRedirect` runs synchronously immediately before the OAuth
|
||||
* full-page redirect (see `BrowserNavigation`). The app uses it to flush the
|
||||
|
|
@ -67,7 +66,7 @@ export function createWebEnvironment(
|
|||
}),
|
||||
logger,
|
||||
oauth: {
|
||||
storage: getBrowserOAuthStorage(),
|
||||
storage: getWebRemoteOAuthStorage(authToken),
|
||||
navigation: new BrowserNavigation(undefined, onBeforeOAuthRedirect),
|
||||
redirectUrlProvider,
|
||||
},
|
||||
|
|
|
|||
66
clients/web/src/lib/remoteOAuthStorage.test.ts
Normal file
66
clients/web/src/lib/remoteOAuthStorage.test.ts
Normal file
|
|
@ -0,0 +1,66 @@
|
|||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
getRemoteOAuthStorage,
|
||||
getWebRemoteOAuthStorage,
|
||||
resetWebRemoteOAuthStorageCacheForTests,
|
||||
} from "./remoteOAuthStorage";
|
||||
|
||||
describe("remoteOAuthStorage", () => {
|
||||
afterEach(() => {
|
||||
resetWebRemoteOAuthStorageCacheForTests();
|
||||
});
|
||||
|
||||
it("returns one RemoteOAuthStorage instance per cache key", () => {
|
||||
const a = getRemoteOAuthStorage({
|
||||
baseUrl: "http://127.0.0.1:6277",
|
||||
authToken: "tok-a",
|
||||
});
|
||||
const aAgain = getRemoteOAuthStorage({
|
||||
baseUrl: "http://127.0.0.1:6277",
|
||||
authToken: "tok-a",
|
||||
});
|
||||
const b = getRemoteOAuthStorage({
|
||||
baseUrl: "http://127.0.0.1:6277",
|
||||
authToken: "tok-b",
|
||||
});
|
||||
|
||||
expect(aAgain).toBe(a);
|
||||
expect(b).not.toBe(a);
|
||||
});
|
||||
|
||||
it("getWebRemoteOAuthStorage uses window.location origin", () => {
|
||||
vi.stubGlobal("window", {
|
||||
location: {
|
||||
protocol: "http:",
|
||||
host: "127.0.0.1:6299",
|
||||
},
|
||||
});
|
||||
|
||||
const storage = getWebRemoteOAuthStorage("smoke-web-token");
|
||||
const again = getWebRemoteOAuthStorage("smoke-web-token");
|
||||
|
||||
expect(again).toBe(storage);
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("throws when window is unavailable", () => {
|
||||
vi.stubGlobal("window", undefined);
|
||||
expect(() => getWebRemoteOAuthStorage()).toThrow(
|
||||
"getWebRemoteOAuthStorage requires a browser environment",
|
||||
);
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("creates a new instance after the test cache reset", () => {
|
||||
const first = getRemoteOAuthStorage({
|
||||
baseUrl: "http://127.0.0.1:6277",
|
||||
authToken: "reset-me",
|
||||
});
|
||||
resetWebRemoteOAuthStorageCacheForTests();
|
||||
const second = getRemoteOAuthStorage({
|
||||
baseUrl: "http://127.0.0.1:6277",
|
||||
authToken: "reset-me",
|
||||
});
|
||||
expect(second).not.toBe(first);
|
||||
});
|
||||
});
|
||||
55
clients/web/src/lib/remoteOAuthStorage.ts
Normal file
55
clients/web/src/lib/remoteOAuthStorage.ts
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
import { RemoteOAuthStorage } from "@inspector/core/auth/remote/storage-remote.js";
|
||||
|
||||
export interface WebRemoteOAuthStorageOptions {
|
||||
baseUrl: string;
|
||||
authToken?: string;
|
||||
}
|
||||
|
||||
let cached: { cacheKey: string; storage: RemoteOAuthStorage } | undefined;
|
||||
|
||||
function buildCacheKey(options: WebRemoteOAuthStorageOptions): string {
|
||||
return `${options.baseUrl}\0${options.authToken ?? ""}`;
|
||||
}
|
||||
|
||||
const defaultFetch: typeof fetch = (...args) => globalThis.fetch(...args);
|
||||
|
||||
/**
|
||||
* Shared web OAuth store: `RemoteOAuthStorage` → `GET/POST /api/storage/oauth`
|
||||
* → `~/.mcp-inspector/storage/oauth.json` (same file as CLI/TUI).
|
||||
*
|
||||
* Memoized by `{ baseUrl, authToken }` so connect, EMA IdP session, and
|
||||
* per-server clear all mutate the same in-memory view.
|
||||
*/
|
||||
export function getRemoteOAuthStorage(
|
||||
options: WebRemoteOAuthStorageOptions,
|
||||
): RemoteOAuthStorage {
|
||||
const cacheKey = buildCacheKey(options);
|
||||
if (cached?.cacheKey === cacheKey) {
|
||||
return cached.storage;
|
||||
}
|
||||
cached = {
|
||||
cacheKey,
|
||||
storage: new RemoteOAuthStorage({
|
||||
baseUrl: options.baseUrl,
|
||||
authToken: options.authToken,
|
||||
fetchFn: defaultFetch,
|
||||
}),
|
||||
};
|
||||
return cached.storage;
|
||||
}
|
||||
|
||||
/** Current origin + optional API token (see `getAuthToken()` in App.tsx). */
|
||||
export function getWebRemoteOAuthStorage(
|
||||
authToken?: string,
|
||||
): RemoteOAuthStorage {
|
||||
if (typeof window === "undefined") {
|
||||
throw new Error("getWebRemoteOAuthStorage requires a browser environment");
|
||||
}
|
||||
const baseUrl = `${window.location.protocol}//${window.location.host}`;
|
||||
return getRemoteOAuthStorage({ baseUrl, authToken });
|
||||
}
|
||||
|
||||
/** @internal Vitest isolation */
|
||||
export function resetWebRemoteOAuthStorageCacheForTests(): void {
|
||||
cached = undefined;
|
||||
}
|
||||
|
|
@ -1,5 +1,8 @@
|
|||
import { describe, it, expect, beforeEach, afterEach } from "vitest";
|
||||
import { BrowserOAuthStorage } from "@inspector/core/auth/browser/storage.js";
|
||||
import {
|
||||
BrowserOAuthStorage,
|
||||
getBrowserOAuthStorage,
|
||||
} from "@inspector/core/auth/browser/storage.js";
|
||||
import type {
|
||||
OAuthClientInformation,
|
||||
OAuthTokens,
|
||||
|
|
@ -41,6 +44,14 @@ const mockSessionStorage = new MockSessionStorage();
|
|||
(global as typeof globalThis & { sessionStorage?: Storage }).sessionStorage =
|
||||
mockSessionStorage;
|
||||
|
||||
describe("getBrowserOAuthStorage", () => {
|
||||
it("returns the same singleton on repeated calls", () => {
|
||||
const first = getBrowserOAuthStorage();
|
||||
const second = getBrowserOAuthStorage();
|
||||
expect(second).toBe(first);
|
||||
});
|
||||
});
|
||||
|
||||
describe("BrowserOAuthStorage", () => {
|
||||
let storage: BrowserOAuthStorage;
|
||||
const testServerUrl = "http://localhost:3000";
|
||||
|
|
|
|||
|
|
@ -1,14 +1,16 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { getBrowserOAuthStorage } from "@inspector/core/auth/browser/index.js";
|
||||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import { BrowserOAuthStorage } from "@inspector/core/auth/browser/storage.js";
|
||||
import { clearServerOAuthState } from "./clearServerOAuthState";
|
||||
|
||||
describe("clearServerOAuthState", () => {
|
||||
let storage: BrowserOAuthStorage;
|
||||
|
||||
beforeEach(() => {
|
||||
getBrowserOAuthStorage().clear("https://mcp.example.com/mcp");
|
||||
storage = new BrowserOAuthStorage();
|
||||
storage.clear("https://mcp.example.com/mcp");
|
||||
});
|
||||
|
||||
it("clears storage by server URL when not the active connection", async () => {
|
||||
const storage = getBrowserOAuthStorage();
|
||||
await storage.saveTokens("https://mcp.example.com/mcp", {
|
||||
access_token: "tok",
|
||||
token_type: "Bearer",
|
||||
|
|
@ -17,6 +19,7 @@ describe("clearServerOAuthState", () => {
|
|||
const cleared = clearServerOAuthState({
|
||||
config: { type: "streamable-http", url: "https://mcp.example.com/mcp" },
|
||||
isActiveConnection: false,
|
||||
oauthStorage: storage,
|
||||
});
|
||||
|
||||
expect(cleared).toBe(true);
|
||||
|
|
@ -26,18 +29,18 @@ describe("clearServerOAuthState", () => {
|
|||
});
|
||||
|
||||
it("uses the live client when clearing the active connection", () => {
|
||||
const inspectorClient = {
|
||||
clearOAuthTokens: vi.fn(),
|
||||
};
|
||||
const clearOAuthTokens = vi.fn<() => void>();
|
||||
const inspectorClient = { clearOAuthTokens };
|
||||
|
||||
const cleared = clearServerOAuthState({
|
||||
config: { type: "streamable-http", url: "https://mcp.example.com/mcp" },
|
||||
inspectorClient: inspectorClient as never,
|
||||
inspectorClient,
|
||||
isActiveConnection: true,
|
||||
oauthStorage: storage,
|
||||
});
|
||||
|
||||
expect(cleared).toBe(true);
|
||||
expect(inspectorClient.clearOAuthTokens).toHaveBeenCalledTimes(1);
|
||||
expect(clearOAuthTokens).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns false for stdio servers", () => {
|
||||
|
|
@ -45,6 +48,7 @@ describe("clearServerOAuthState", () => {
|
|||
clearServerOAuthState({
|
||||
config: { type: "stdio", command: "node", args: [] },
|
||||
isActiveConnection: false,
|
||||
oauthStorage: storage,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { getBrowserOAuthStorage } from "@inspector/core/auth/browser/index.js";
|
||||
import type { OAuthStorage } from "@inspector/core/auth/storage.js";
|
||||
import { getOAuthServerUrl } from "@inspector/core/mcp/config.js";
|
||||
import type { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
|
||||
import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
|
||||
|
|
@ -6,8 +6,10 @@ import type { MCPServerConfig } from "@inspector/core/mcp/types.js";
|
|||
export interface ClearServerOAuthStateParams {
|
||||
config: MCPServerConfig;
|
||||
/** When set and this server is the active connection, clear via the live client. */
|
||||
inspectorClient?: InspectorClient | null;
|
||||
inspectorClient?: Pick<InspectorClient, "clearOAuthTokens"> | null;
|
||||
isActiveConnection: boolean;
|
||||
/** Shared web OAuth store; required so clear hits the same blob as connect. */
|
||||
oauthStorage: OAuthStorage;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -26,7 +28,7 @@ export function clearServerOAuthState(
|
|||
if (params.isActiveConnection && params.inspectorClient) {
|
||||
params.inspectorClient.clearOAuthTokens();
|
||||
} else {
|
||||
getBrowserOAuthStorage().clear(serverUrl);
|
||||
params.oauthStorage.clear(serverUrl);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ export default defineConfig(({ command }) => {
|
|||
reporter: ['text', 'html', 'json-summary'],
|
||||
include: [
|
||||
'src/components/**/*.{ts,tsx}',
|
||||
'src/lib/**/*.{ts,tsx}',
|
||||
'src/utils/**/*.{ts,tsx}',
|
||||
'clients/web/server/**/*.{ts,tsx}',
|
||||
path.join(repoRoot, 'core/mcp/**/*.{ts,tsx}'),
|
||||
|
|
|
|||
Loading…
Reference in a new issue