Merge pull request #1612 from modelcontextprotocol/v2/1549-oauth-storage-no-zustand
refactor(auth): remove Zustand from OAuth persistence, clean up storage IO (#1549)
This commit is contained in:
Cliff Hall
GitHub
commit
4ee9bbce0f
62 changed files with 1249 additions and 2018 deletions
|
|
@ -22,7 +22,7 @@ inspector/
|
|||
│ ├── tui/ # TUI client (Ink + React, tsup bundle)
|
||||
│ ├── launcher/ # Shared launcher (relative imports into sibling build/ outputs)
|
||||
├── core/ # Shared core code (no package.json — consumed via the `@inspector/core` vite alias)
|
||||
│ ├── auth/ # OAuth: state machine, providers, discovery, storage;
|
||||
│ ├── auth/ # OAuth: providers, discovery, OAuthStorage + persist backends;
|
||||
│ │ # mid-session recovery (challenge.ts WWW-Authenticate
|
||||
│ │ # parsing, scopes.ts SEP-2350 scope union, oauthUx.ts
|
||||
│ │ # shared copy, mcpAuth.ts force-reauthorization)
|
||||
|
|
@ -41,9 +41,9 @@ inspector/
|
|||
│ │ ├── node/ # Node stdio transport factory
|
||||
│ │ ├── remote/ # Browser HTTP/SSE transport + remote logger/fetch
|
||||
│ │ │ └── node/ # Hono-based remote server backend (used by remote/ above)
|
||||
│ │ └── state/ # Zustand-style state stores consumed by core/react/
|
||||
│ │ └── state/ # InspectorClient state stores consumed by core/react/
|
||||
│ ├── react/ # React hooks over the state stores
|
||||
│ └── storage/ # File and remote storage adapters (Zustand middleware)
|
||||
│ └── storage/ # File I/O helpers (store-io.ts) used by OAuth persist backends
|
||||
├── test-servers/ # Composable MCP test servers + fixtures used by integration tests.
|
||||
│ ├── src/ # TypeScript sources.
|
||||
│ ├── build/ # Built JS (gitignored). Produced by `npm run test-servers:build`
|
||||
|
|
|
|||
32
clients/cli/package-lock.json
generated
32
clients/cli/package-lock.json
generated
|
|
@ -15,8 +15,7 @@
|
|||
"atomically": "^2.1.1",
|
||||
"commander": "^13.1.0",
|
||||
"pino": "^9.14.0",
|
||||
"zod": "^4.3.6",
|
||||
"zustand": "^5.0.13"
|
||||
"zod": "^4.3.6"
|
||||
},
|
||||
"bin": {
|
||||
"mcp-inspector-cli": "build/index.js"
|
||||
|
|
@ -5675,35 +5674,6 @@
|
|||
"peerDependencies": {
|
||||
"zod": "^3.25.28 || ^4"
|
||||
}
|
||||
},
|
||||
"node_modules/zustand": {
|
||||
"version": "5.0.14",
|
||||
"resolved": "https://registry.npmjs.org/zustand/-/zustand-5.0.14.tgz",
|
||||
"integrity": "sha512-/8tAspM5LMPr28b3fwLYrtdj77ECpfZviaP75CMTnwO8ISyaE4GDIG/9rDDYq/cH9D2Xw2A2RXglLInmVBQB/g==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=12.20.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@types/react": ">=18.0.0",
|
||||
"immer": ">=9.0.6",
|
||||
"react": ">=18.0.0",
|
||||
"use-sync-external-store": ">=1.2.0"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"@types/react": {
|
||||
"optional": true
|
||||
},
|
||||
"immer": {
|
||||
"optional": true
|
||||
},
|
||||
"react": {
|
||||
"optional": true
|
||||
},
|
||||
"use-sync-external-store": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,8 +38,7 @@
|
|||
"atomically": "^2.1.1",
|
||||
"commander": "^13.1.0",
|
||||
"pino": "^9.14.0",
|
||||
"zod": "^4.3.6",
|
||||
"zustand": "^5.0.13"
|
||||
"zod": "^4.3.6"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@eslint/js": "^9.39.4",
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ export default defineConfig({
|
|||
platform: 'node',
|
||||
// Bundle core source; leave npm deps external.
|
||||
noExternal: [/^@inspector\/core/],
|
||||
external: ['@napi-rs/keyring', '@modelcontextprotocol/sdk', 'commander', 'pino', 'zustand'],
|
||||
external: ['@napi-rs/keyring', '@modelcontextprotocol/sdk', 'commander', 'pino'],
|
||||
esbuildOptions(options) {
|
||||
options.alias = {
|
||||
'@inspector/core': path.join(repoRoot, 'core'),
|
||||
|
|
|
|||
988
clients/launcher/package-lock.json
generated
988
clients/launcher/package-lock.json
generated
File diff suppressed because it is too large
Load diff
32
clients/tui/package-lock.json
generated
32
clients/tui/package-lock.json
generated
|
|
@ -20,8 +20,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"react": "^19.2.4",
|
||||
"zod": "^4.3.6",
|
||||
"zustand": "^5.0.13"
|
||||
"zod": "^4.3.6"
|
||||
},
|
||||
"bin": {
|
||||
"mcp-inspector-tui": "build/index.js"
|
||||
|
|
@ -7510,35 +7509,6 @@
|
|||
"peerDependencies": {
|
||||
"zod": "^3.25.0 || ^4.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/zustand": {
|
||||
"version": "5.0.14",
|
||||
"resolved": "https://registry.npmjs.org/zustand/-/zustand-5.0.14.tgz",
|
||||
"integrity": "sha512-/8tAspM5LMPr28b3fwLYrtdj77ECpfZviaP75CMTnwO8ISyaE4GDIG/9rDDYq/cH9D2Xw2A2RXglLInmVBQB/g==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=12.20.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@types/react": ">=18.0.0",
|
||||
"immer": ">=9.0.6",
|
||||
"react": ">=18.0.0",
|
||||
"use-sync-external-store": ">=1.2.0"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"@types/react": {
|
||||
"optional": true
|
||||
},
|
||||
"immer": {
|
||||
"optional": true
|
||||
},
|
||||
"react": {
|
||||
"optional": true
|
||||
},
|
||||
"use-sync-external-store": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,8 +38,7 @@
|
|||
"open": "^10.2.0",
|
||||
"pino": "^9.14.0",
|
||||
"react": "^19.2.4",
|
||||
"zod": "^4.3.6",
|
||||
"zustand": "^5.0.13"
|
||||
"zod": "^4.3.6"
|
||||
},
|
||||
"overrides": {
|
||||
"ink-select-input": "^6.2.0"
|
||||
|
|
|
|||
|
|
@ -22,7 +22,6 @@ export default defineConfig({
|
|||
'open',
|
||||
'commander',
|
||||
'pino',
|
||||
'zustand',
|
||||
'@modelcontextprotocol/sdk',
|
||||
'@napi-rs/keyring',
|
||||
],
|
||||
|
|
|
|||
34
clients/web/package-lock.json
generated
34
clients/web/package-lock.json
generated
|
|
@ -34,8 +34,7 @@
|
|||
"react-markdown": "^10.1.0",
|
||||
"react-syntax-highlighter": "^16.1.1",
|
||||
"remark-gfm": "^4.0.1",
|
||||
"zod": "^4.3.6",
|
||||
"zustand": "^5.0.13"
|
||||
"zod": "^4.3.6"
|
||||
},
|
||||
"bin": {
|
||||
"mcp-inspector-web": "build/index.js"
|
||||
|
|
@ -9952,7 +9951,7 @@
|
|||
"version": "1.6.0",
|
||||
"resolved": "https://registry.npmjs.org/use-sync-external-store/-/use-sync-external-store-1.6.0.tgz",
|
||||
"integrity": "sha512-Pp6GSwGP/NrPIrxVFAIkOQeyw8lFenOHijQWkUTrDvrF4ALqylP2C/KCkeS9dpUM3KvYRQhna5vt7IL95+ZQ9w==",
|
||||
"devOptional": true,
|
||||
"dev": true,
|
||||
"license": "MIT",
|
||||
"peerDependencies": {
|
||||
"react": "^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0"
|
||||
|
|
@ -10359,35 +10358,6 @@
|
|||
"zod": "^3.25.0 || ^4.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/zustand": {
|
||||
"version": "5.0.13",
|
||||
"resolved": "https://registry.npmjs.org/zustand/-/zustand-5.0.13.tgz",
|
||||
"integrity": "sha512-efI2tVaVQPqtOh114loML/Z80Y4NP3yc+Ff0fYiZJPauNeWZeIp/bRFD7I9bfmCOYBh/PHxlglQ9+wvlwnPikQ==",
|
||||
"license": "MIT",
|
||||
"engines": {
|
||||
"node": ">=12.20.0"
|
||||
},
|
||||
"peerDependencies": {
|
||||
"@types/react": ">=18.0.0",
|
||||
"immer": ">=9.0.6",
|
||||
"react": ">=18.0.0",
|
||||
"use-sync-external-store": ">=1.2.0"
|
||||
},
|
||||
"peerDependenciesMeta": {
|
||||
"@types/react": {
|
||||
"optional": true
|
||||
},
|
||||
"immer": {
|
||||
"optional": true
|
||||
},
|
||||
"react": {
|
||||
"optional": true
|
||||
},
|
||||
"use-sync-external-store": {
|
||||
"optional": true
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/zwitch": {
|
||||
"version": "2.0.4",
|
||||
"resolved": "https://registry.npmjs.org/zwitch/-/zwitch-2.0.4.tgz",
|
||||
|
|
|
|||
|
|
@ -59,8 +59,7 @@
|
|||
"react-markdown": "^10.1.0",
|
||||
"react-syntax-highlighter": "^16.1.1",
|
||||
"remark-gfm": "^4.0.1",
|
||||
"zod": "^4.3.6",
|
||||
"zustand": "^5.0.13"
|
||||
"zod": "^4.3.6"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@chromatic-com/storybook": "^5.0.1",
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ describe("ensureCimdClientRegistration", () => {
|
|||
storage = {
|
||||
getClientInformation: vi.fn(async () => undefined),
|
||||
saveClientInformation: vi.fn(async () => {}),
|
||||
getScope: vi.fn(() => undefined),
|
||||
getScope: vi.fn().mockResolvedValue(undefined),
|
||||
getTokens: vi.fn(async () => undefined),
|
||||
saveTokens: vi.fn(async () => {}),
|
||||
clear: vi.fn(),
|
||||
|
|
|
|||
|
|
@ -14,11 +14,13 @@ function createStorage(
|
|||
tokens: Awaited<ReturnType<OAuthStorage["getTokens"]>>;
|
||||
preregistered: Awaited<ReturnType<OAuthStorage["getClientInformation"]>>;
|
||||
dynamic: Awaited<ReturnType<OAuthStorage["getClientInformation"]>>;
|
||||
registrationKind: ReturnType<OAuthStorage["getClientRegistrationKind"]>;
|
||||
registrationKind: Awaited<
|
||||
ReturnType<OAuthStorage["getClientRegistrationKind"]>
|
||||
>;
|
||||
scope: string | undefined;
|
||||
serverMetadata: ReturnType<OAuthStorage["getServerMetadata"]>;
|
||||
serverMetadata: Awaited<ReturnType<OAuthStorage["getServerMetadata"]>>;
|
||||
idpSession: Awaited<ReturnType<OAuthStorage["getIdpSession"]>>;
|
||||
idpMetadata: ReturnType<OAuthStorage["getServerMetadata"]>;
|
||||
idpMetadata: Awaited<ReturnType<OAuthStorage["getServerMetadata"]>>;
|
||||
}> = {},
|
||||
): OAuthStorage {
|
||||
return {
|
||||
|
|
@ -27,15 +29,17 @@ function createStorage(
|
|||
getClientInformation: vi.fn(async (_url, isPreregistered) =>
|
||||
isPreregistered ? overrides.preregistered : overrides.dynamic,
|
||||
),
|
||||
getScope: vi.fn().mockReturnValue(overrides.scope),
|
||||
getServerMetadata: vi.fn((url: string) => {
|
||||
getScope: vi.fn().mockResolvedValue(overrides.scope),
|
||||
getServerMetadata: vi.fn(async (url: string) => {
|
||||
if (url.startsWith("ema-idp:")) {
|
||||
return overrides.idpMetadata ?? null;
|
||||
}
|
||||
return overrides.serverMetadata ?? null;
|
||||
}),
|
||||
getIdpSession: vi.fn().mockResolvedValue(overrides.idpSession),
|
||||
getClientRegistrationKind: vi.fn(() => overrides.registrationKind),
|
||||
getClientRegistrationKind: vi
|
||||
.fn()
|
||||
.mockResolvedValue(overrides.registrationKind),
|
||||
saveClientInformation: vi.fn(),
|
||||
savePreregisteredClientInformation: vi.fn(),
|
||||
saveTokens: vi.fn(),
|
||||
|
|
|
|||
|
|
@ -49,19 +49,21 @@ function createMemoryStorage(
|
|||
idpSessions[issuer] = { ...idpSessions[issuer], ...updates };
|
||||
}),
|
||||
clearIdpSession: vi.fn(),
|
||||
getServerMetadata: vi.fn((key: string) => metadataByKey[key]),
|
||||
saveServerMetadata: vi.fn((key: string, metadata: unknown) => {
|
||||
getServerMetadata: vi.fn(async (key: string) => metadataByKey[key] ?? null),
|
||||
saveServerMetadata: vi.fn(async (key: string, metadata: unknown) => {
|
||||
metadataByKey[key] = metadata;
|
||||
}),
|
||||
getClientInformation: vi.fn((key: string) => clientInfoByKey[key]),
|
||||
savePreregisteredClientInformation: vi.fn((key: string, info: unknown) => {
|
||||
clientInfoByKey[key] = info;
|
||||
}),
|
||||
getCodeVerifier: vi.fn((key: string) => codeVerifierByKey[key]),
|
||||
saveCodeVerifier: vi.fn((key: string, verifier: string) => {
|
||||
getClientInformation: vi.fn(async (key: string) => clientInfoByKey[key]),
|
||||
savePreregisteredClientInformation: vi.fn(
|
||||
async (key: string, info: unknown) => {
|
||||
clientInfoByKey[key] = info;
|
||||
},
|
||||
),
|
||||
getCodeVerifier: vi.fn(async (key: string) => codeVerifierByKey[key]),
|
||||
saveCodeVerifier: vi.fn(async (key: string, verifier: string) => {
|
||||
codeVerifierByKey[key] = verifier;
|
||||
}),
|
||||
clearCodeVerifier: vi.fn((key: string) => {
|
||||
clearCodeVerifier: vi.fn(async (key: string) => {
|
||||
delete codeVerifierByKey[key];
|
||||
}),
|
||||
getTokens: vi.fn(async (url: string) => savedTokens[url] as never),
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ describe("idpOidc refresh", () => {
|
|||
saveIdpSession: vi.fn(async (issuer: string, updates) => {
|
||||
idpSessions[issuer] = { ...idpSessions[issuer], ...updates };
|
||||
}),
|
||||
getServerMetadata: vi.fn(() => minimalOAuthAsMetadata(IDP_ISSUER)),
|
||||
getServerMetadata: vi.fn(async () => minimalOAuthAsMetadata(IDP_ISSUER)),
|
||||
clearIdpSession: vi.fn(),
|
||||
} as unknown as OAuthStorage;
|
||||
});
|
||||
|
|
@ -266,7 +266,7 @@ describe("startIdpOidcAuthorization", () => {
|
|||
Object.keys(saved).forEach((k) => delete saved[k]);
|
||||
storage = {
|
||||
load: vi.fn().mockResolvedValue(undefined),
|
||||
getServerMetadata: vi.fn(() => null),
|
||||
getServerMetadata: vi.fn(async () => null),
|
||||
saveCodeVerifier: vi.fn(async (key: string, value: string) => {
|
||||
saved[`cv:${key}`] = value;
|
||||
}),
|
||||
|
|
@ -335,13 +335,13 @@ describe("completeIdpOidcAuthorization", () => {
|
|||
function buildStorage(overrides: Partial<OAuthStorage> = {}): OAuthStorage {
|
||||
return {
|
||||
load: vi.fn().mockResolvedValue(undefined),
|
||||
getServerMetadata: vi.fn(() => minimalOAuthAsMetadata(IDP_ISSUER)),
|
||||
getServerMetadata: vi.fn(async () => minimalOAuthAsMetadata(IDP_ISSUER)),
|
||||
getClientInformation: vi.fn(async () => ({
|
||||
client_id: "idp-client",
|
||||
client_secret: "idp-secret",
|
||||
token_endpoint_auth_method: "client_secret_post",
|
||||
})),
|
||||
getCodeVerifier: vi.fn(() => "verifier-123"),
|
||||
getCodeVerifier: vi.fn(async () => "verifier-123"),
|
||||
clearCodeVerifier: vi.fn((key: string) => {
|
||||
clearedVerifiers.push(key);
|
||||
}),
|
||||
|
|
@ -474,7 +474,7 @@ describe("completeIdpOidcAuthorization", () => {
|
|||
});
|
||||
|
||||
it("throws when the PKCE verifier is missing", async () => {
|
||||
storage = buildStorage({ getCodeVerifier: vi.fn(() => undefined) });
|
||||
storage = buildStorage({ getCodeVerifier: vi.fn(async () => undefined) });
|
||||
await expect(
|
||||
completeIdpOidcAuthorization({
|
||||
idp,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,66 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { dirname, resolve } from "node:path";
|
||||
|
||||
/**
|
||||
* Regression guard for the browser bundle. The browser's `RemoteOAuthStorage`
|
||||
* (App.tsx -> lib/remoteOAuthStorage.ts -> remote/storage-remote.ts) chains
|
||||
* through a handful of `core/auth` + `core/storage` modules, so ALL of them
|
||||
* must stay Node-free. `oauth-persist.ts` previously value-imported
|
||||
* `store-io.js`, which pulls `node:fs` + `atomically` (-> `stubborn-fs` ->
|
||||
* `node:process.getuid`) into the browser bundle and blanked the app at
|
||||
* runtime. The Node-only file backend now lives in
|
||||
* `core/auth/node/oauth-persist-file.ts`.
|
||||
*
|
||||
* This guard is transitive: it scans every module reachable from
|
||||
* `storage-remote.ts` (short of the `node/` boundary), so a forbidden import
|
||||
* added one hop away — e.g. through `store-serialize.ts` or `store.ts` — is
|
||||
* caught too, not just a direct import in `oauth-persist.ts`.
|
||||
*/
|
||||
|
||||
const here = dirname(fileURLToPath(import.meta.url));
|
||||
const coreDir = resolve(here, "../../../../../../core");
|
||||
|
||||
// Browser-reachable modules (value-import graph from storage-remote.ts,
|
||||
// excluding the intentional `node/` backend which is never bundled).
|
||||
const BROWSER_REACHABLE = [
|
||||
"auth/remote/storage-remote.ts",
|
||||
"auth/oauth-storage.ts",
|
||||
"auth/store.ts",
|
||||
"auth/oauth-persist.ts",
|
||||
"storage/store-serialize.ts",
|
||||
];
|
||||
|
||||
const FORBIDDEN = [/(^|\/)store-io(\.js)?$/, /^node:/, /^atomically$/];
|
||||
|
||||
function importSpecifiers(source: string): string[] {
|
||||
const specs: string[] = [];
|
||||
const re = /(?:import|export)[^;]*?from\s*["']([^"']+)["']/g;
|
||||
let m: RegExpExecArray | null;
|
||||
while ((m = re.exec(source)) !== null) {
|
||||
specs.push(m[1]);
|
||||
}
|
||||
return specs;
|
||||
}
|
||||
|
||||
describe("browser-reachable OAuth storage modules stay Node-free", () => {
|
||||
it.each(BROWSER_REACHABLE)(
|
||||
"%s imports no Node built-ins, atomically, or store-io",
|
||||
(relPath) => {
|
||||
const source = readFileSync(resolve(coreDir, relPath), "utf-8");
|
||||
const offenders = importSpecifiers(source).filter((spec) =>
|
||||
FORBIDDEN.some((re) => re.test(spec)),
|
||||
);
|
||||
expect(offenders).toEqual([]);
|
||||
},
|
||||
);
|
||||
|
||||
it("oauth-persist.ts sources its (de)serializers from store-serialize", () => {
|
||||
const source = readFileSync(
|
||||
resolve(coreDir, "auth/oauth-persist.ts"),
|
||||
"utf-8",
|
||||
);
|
||||
expect(importSpecifiers(source)).toContain("../storage/store-serialize.js");
|
||||
});
|
||||
});
|
||||
225
clients/web/src/test/core/auth/oauth-persist.test.ts
Normal file
225
clients/web/src/test/core/auth/oauth-persist.test.ts
Normal file
|
|
@ -0,0 +1,225 @@
|
|||
import { describe, it, expect, vi } from "vitest";
|
||||
import {
|
||||
parseOAuthPersistBlob,
|
||||
serializeOAuthPersistBlob,
|
||||
createRemoteOAuthPersistBackend,
|
||||
createSessionOAuthPersistBackend,
|
||||
OAUTH_PERSIST_STORAGE_KEY,
|
||||
} from "@inspector/core/auth/oauth-persist.js";
|
||||
import type { OAuthPersistSnapshot } from "@inspector/core/auth/oauth-persist.js";
|
||||
|
||||
const SNAPSHOT: OAuthPersistSnapshot = {
|
||||
servers: { "http://s": { scope: "read" } },
|
||||
idpSessions: {},
|
||||
};
|
||||
|
||||
function jsonResponse(body: unknown, status = 200): Response {
|
||||
return new Response(JSON.stringify(body), { status });
|
||||
}
|
||||
|
||||
describe("parseOAuthPersistBlob", () => {
|
||||
it("returns null for empty input", () => {
|
||||
expect(parseOAuthPersistBlob(null)).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for an empty object (server missing-file response)", () => {
|
||||
// The remote read() path relies on this: the server answers a missing
|
||||
// store with `c.json({}, 200)`, and the backend passes that straight
|
||||
// through instead of special-casing an empty object.
|
||||
expect(parseOAuthPersistBlob({})).toBeNull();
|
||||
});
|
||||
|
||||
it("reads plain JSON with servers and idpSessions", () => {
|
||||
const snapshot = {
|
||||
servers: {
|
||||
"http://example.com": { codeVerifier: "v1" },
|
||||
},
|
||||
idpSessions: {
|
||||
"https://idp.example": { idToken: "token" },
|
||||
},
|
||||
};
|
||||
expect(parseOAuthPersistBlob(JSON.stringify(snapshot))).toEqual(snapshot);
|
||||
});
|
||||
|
||||
it("accepts an already-parsed object without re-serializing", () => {
|
||||
const snapshot = {
|
||||
servers: {
|
||||
"http://example.com": { codeVerifier: "v1" },
|
||||
},
|
||||
idpSessions: {},
|
||||
};
|
||||
expect(parseOAuthPersistBlob(snapshot)).toEqual(snapshot);
|
||||
});
|
||||
|
||||
it("promotes legacy persist envelope state to the top level", () => {
|
||||
const legacy = {
|
||||
state: {
|
||||
servers: {
|
||||
"http://example.com": {
|
||||
tokens: { access_token: "t", token_type: "Bearer" },
|
||||
},
|
||||
},
|
||||
idpSessions: {},
|
||||
},
|
||||
version: 0,
|
||||
};
|
||||
expect(parseOAuthPersistBlob(JSON.stringify(legacy))).toEqual({
|
||||
servers: legacy.state.servers,
|
||||
idpSessions: {},
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("serializeOAuthPersistBlob", () => {
|
||||
it("writes plain JSON without a state/version envelope", () => {
|
||||
const snapshot = {
|
||||
servers: { "http://example.com": { scope: "read" } },
|
||||
idpSessions: {},
|
||||
};
|
||||
const raw = serializeOAuthPersistBlob(snapshot);
|
||||
expect(JSON.parse(raw)).toEqual(snapshot);
|
||||
expect(raw).not.toContain('"version"');
|
||||
expect(raw).not.toMatch(/"state"\s*:/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createRemoteOAuthPersistBackend", () => {
|
||||
const baseUrl = "http://remote.example/";
|
||||
const storeId = "oauth";
|
||||
const url = "http://remote.example/api/storage/oauth";
|
||||
|
||||
it("read() returns the parsed snapshot and sends the auth header", async () => {
|
||||
const fetchFn = vi.fn(async () => jsonResponse(SNAPSHOT));
|
||||
const backend = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
authToken: "tok",
|
||||
fetchFn: fetchFn as unknown as typeof fetch,
|
||||
});
|
||||
expect(await backend.read()).toEqual(SNAPSHOT);
|
||||
expect(fetchFn).toHaveBeenCalledWith(url, {
|
||||
method: "GET",
|
||||
headers: { "x-mcp-remote-auth": "Bearer tok" },
|
||||
});
|
||||
});
|
||||
|
||||
it("read() returns null for the empty-object missing-file response", async () => {
|
||||
const backend = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
fetchFn: (async () => jsonResponse({})) as unknown as typeof fetch,
|
||||
});
|
||||
expect(await backend.read()).toBeNull();
|
||||
});
|
||||
|
||||
it("read() returns null on 404 and throws on other errors", async () => {
|
||||
const notFound = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
fetchFn: (async () =>
|
||||
new Response("", { status: 404 })) as unknown as typeof fetch,
|
||||
});
|
||||
expect(await notFound.read()).toBeNull();
|
||||
|
||||
const failing = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
fetchFn: (async () =>
|
||||
new Response("", { status: 500 })) as unknown as typeof fetch,
|
||||
});
|
||||
await expect(failing.read()).rejects.toThrow(/Failed to read store: 500/);
|
||||
});
|
||||
|
||||
it("write() POSTs the serialized snapshot and throws on failure", async () => {
|
||||
let capturedBody: string | undefined;
|
||||
const ok = vi.fn<typeof fetch>(async (_input, init) => {
|
||||
capturedBody = init?.body as string | undefined;
|
||||
return new Response("", { status: 200 });
|
||||
});
|
||||
const backend = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
fetchFn: ok,
|
||||
});
|
||||
await backend.write(SNAPSHOT);
|
||||
expect(ok).toHaveBeenCalledWith(
|
||||
url,
|
||||
expect.objectContaining({ method: "POST" }),
|
||||
);
|
||||
expect(JSON.parse(capturedBody ?? "")).toEqual(SNAPSHOT);
|
||||
|
||||
const failing = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
fetchFn: (async () =>
|
||||
new Response("", { status: 500 })) as unknown as typeof fetch,
|
||||
});
|
||||
await expect(failing.write(SNAPSHOT)).rejects.toThrow(
|
||||
/Failed to write store: 500/,
|
||||
);
|
||||
});
|
||||
|
||||
it("remove() DELETEs, tolerates 404, and throws on other errors", async () => {
|
||||
const ok = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
authToken: "tok",
|
||||
fetchFn: (async () =>
|
||||
new Response("", { status: 200 })) as unknown as typeof fetch,
|
||||
});
|
||||
await expect(ok.remove!()).resolves.toBeUndefined();
|
||||
|
||||
const gone = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
fetchFn: (async () =>
|
||||
new Response("", { status: 404 })) as unknown as typeof fetch,
|
||||
});
|
||||
await expect(gone.remove!()).resolves.toBeUndefined();
|
||||
|
||||
const failing = createRemoteOAuthPersistBackend({
|
||||
baseUrl,
|
||||
storeId,
|
||||
fetchFn: (async () =>
|
||||
new Response("", { status: 500 })) as unknown as typeof fetch,
|
||||
});
|
||||
await expect(failing.remove!()).rejects.toThrow(
|
||||
/Failed to delete store: 500/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createSessionOAuthPersistBackend", () => {
|
||||
function fakeStorage() {
|
||||
const map = new Map<string, string>();
|
||||
return {
|
||||
getItem: (k: string) => map.get(k) ?? null,
|
||||
setItem: (k: string, v: string) => void map.set(k, v),
|
||||
removeItem: (k: string) => void map.delete(k),
|
||||
map,
|
||||
} as unknown as Storage & { map: Map<string, string> };
|
||||
}
|
||||
|
||||
it("round-trips a snapshot through the default storage key", async () => {
|
||||
const storage = fakeStorage();
|
||||
const backend = createSessionOAuthPersistBackend({
|
||||
getStorage: () => storage,
|
||||
});
|
||||
expect(await backend.read()).toBeNull();
|
||||
await backend.write(SNAPSHOT);
|
||||
expect(storage.map.has(OAUTH_PERSIST_STORAGE_KEY)).toBe(true);
|
||||
expect(await backend.read()).toEqual(SNAPSHOT);
|
||||
await backend.remove!();
|
||||
expect(await backend.read()).toBeNull();
|
||||
});
|
||||
|
||||
it("honors a custom storage key", async () => {
|
||||
const storage = fakeStorage();
|
||||
const backend = createSessionOAuthPersistBackend({
|
||||
storageKey: "custom-key",
|
||||
getStorage: () => storage,
|
||||
});
|
||||
await backend.write(SNAPSHOT);
|
||||
expect(storage.map.has("custom-key")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
@ -214,7 +214,7 @@ describe("OAuthNavigation", () => {
|
|||
function makeStorage(): OAuthStorage {
|
||||
return {
|
||||
load: vi.fn().mockResolvedValue(undefined),
|
||||
getScope: vi.fn(() => undefined),
|
||||
getScope: vi.fn().mockResolvedValue(undefined),
|
||||
getClientInformation: vi.fn(async () => undefined),
|
||||
saveClientInformation: vi.fn(async () => undefined),
|
||||
savePreregisteredClientInformation: vi.fn(async () => undefined),
|
||||
|
|
@ -222,9 +222,9 @@ describe("OAuthNavigation", () => {
|
|||
getTokens: vi.fn(async () => undefined),
|
||||
saveTokens: vi.fn(async () => undefined),
|
||||
saveCodeVerifier: vi.fn(async () => undefined),
|
||||
getCodeVerifier: vi.fn(() => undefined),
|
||||
getCodeVerifier: vi.fn().mockResolvedValue(undefined),
|
||||
clear: vi.fn(),
|
||||
getServerMetadata: vi.fn(() => null),
|
||||
getServerMetadata: vi.fn().mockResolvedValue(null),
|
||||
saveServerMetadata: vi.fn(async () => undefined),
|
||||
} as unknown as OAuthStorage;
|
||||
}
|
||||
|
|
@ -272,27 +272,28 @@ describe("OAuthNavigation", () => {
|
|||
expect(await provider.clientInformation()).toEqual({ client_id: "dyn" });
|
||||
});
|
||||
|
||||
it("codeVerifier() throws when none is saved", () => {
|
||||
it("codeVerifier() throws when none is saved", async () => {
|
||||
const storage = makeStorage();
|
||||
const provider = makeProvider(storage);
|
||||
|
||||
expect(() => provider.codeVerifier()).toThrow(
|
||||
await expect(provider.codeVerifier()).rejects.toThrow(
|
||||
/No code verifier saved for session/,
|
||||
);
|
||||
});
|
||||
|
||||
it("codeVerifier() returns the saved verifier", () => {
|
||||
it("codeVerifier() returns the saved verifier", async () => {
|
||||
const storage = makeStorage();
|
||||
vi.mocked(storage.getCodeVerifier).mockReturnValue("cv-1");
|
||||
vi.mocked(storage.getCodeVerifier).mockResolvedValue("cv-1");
|
||||
const provider = makeProvider(storage);
|
||||
|
||||
expect(provider.codeVerifier()).toBe("cv-1");
|
||||
expect(await provider.codeVerifier()).toBe("cv-1");
|
||||
});
|
||||
|
||||
it("clientMetadata reflects the stored scope when present", () => {
|
||||
it("clientMetadata reflects the stored scope when present", async () => {
|
||||
const storage = makeStorage();
|
||||
vi.mocked(storage.getScope).mockReturnValue("read write");
|
||||
vi.mocked(storage.getScope).mockResolvedValue("read write");
|
||||
const provider = makeProvider(storage);
|
||||
await provider.prepareForAuth();
|
||||
|
||||
expect(provider.clientMetadata.scope).toBe("read write");
|
||||
});
|
||||
|
|
@ -380,7 +381,7 @@ describe("OAuthNavigation", () => {
|
|||
expect(storage.saveCodeVerifier).toHaveBeenCalledWith(SERVER, "cv");
|
||||
expect(storage.saveServerMetadata).toHaveBeenCalled();
|
||||
expect(await provider.tokens()).toBeUndefined();
|
||||
expect(provider.getServerMetadata()).toBeNull();
|
||||
expect(await provider.getServerMetadata()).toBeNull();
|
||||
const state = await provider.state();
|
||||
expect(typeof state).toBe("string");
|
||||
expect(state.length).toBeGreaterThan(0);
|
||||
|
|
|
|||
|
|
@ -91,16 +91,11 @@ describe("BrowserOAuthStorage", () => {
|
|||
client_secret: "preregistered-secret",
|
||||
};
|
||||
|
||||
// Use the storage API instead of manually setting sessionStorage
|
||||
// since BrowserOAuthStorage now uses Zustand with a different storage format
|
||||
await storage.savePreregisteredClientInformation(
|
||||
testServerUrl,
|
||||
preregisteredInfo,
|
||||
);
|
||||
|
||||
// Wait for Zustand to persist
|
||||
await new Promise((resolve) => setTimeout(resolve, 100));
|
||||
|
||||
const result = await storage.getClientInformation(testServerUrl, true);
|
||||
|
||||
expect(result).toEqual(preregisteredInfo);
|
||||
|
|
@ -139,14 +134,18 @@ describe("BrowserOAuthStorage", () => {
|
|||
const result = await storage.getClientInformation(testServerUrl);
|
||||
|
||||
expect(result).toEqual(secondInfo);
|
||||
expect(storage.getClientRegistrationKind(testServerUrl)).toBe("cimd");
|
||||
expect(await storage.getClientRegistrationKind(testServerUrl)).toBe(
|
||||
"cimd",
|
||||
);
|
||||
});
|
||||
|
||||
it("savePreregisteredClientInformation sets static registration kind", async () => {
|
||||
await storage.savePreregisteredClientInformation(testServerUrl, {
|
||||
client_id: "static-id",
|
||||
});
|
||||
expect(storage.getClientRegistrationKind(testServerUrl)).toBe("static");
|
||||
expect(await storage.getClientRegistrationKind(testServerUrl)).toBe(
|
||||
"static",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -333,16 +332,16 @@ describe("BrowserOAuthStorage", () => {
|
|||
|
||||
it("clearCodeVerifier removes only the PKCE verifier", async () => {
|
||||
await storage.saveCodeVerifier(testServerUrl, "verifier");
|
||||
expect(storage.getCodeVerifier(testServerUrl)).toBe("verifier");
|
||||
expect(await storage.getCodeVerifier(testServerUrl)).toBe("verifier");
|
||||
await storage.clearCodeVerifier(testServerUrl);
|
||||
expect(storage.getCodeVerifier(testServerUrl)).toBeUndefined();
|
||||
expect(await storage.getCodeVerifier(testServerUrl)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("clearScope removes only the scope", async () => {
|
||||
await storage.saveScope(testServerUrl, "read");
|
||||
expect(storage.getScope(testServerUrl)).toBe("read");
|
||||
expect(await storage.getScope(testServerUrl)).toBe("read");
|
||||
await storage.clearScope(testServerUrl);
|
||||
expect(storage.getScope(testServerUrl)).toBeUndefined();
|
||||
expect(await storage.getScope(testServerUrl)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("clearServerMetadata removes only the cached metadata", async () => {
|
||||
|
|
@ -353,9 +352,9 @@ describe("BrowserOAuthStorage", () => {
|
|||
response_types_supported: ["code"],
|
||||
};
|
||||
await storage.saveServerMetadata(testServerUrl, metadata);
|
||||
expect(storage.getServerMetadata(testServerUrl)).toEqual(metadata);
|
||||
expect(await storage.getServerMetadata(testServerUrl)).toEqual(metadata);
|
||||
await storage.clearServerMetadata(testServerUrl);
|
||||
expect(storage.getServerMetadata(testServerUrl)).toBeNull();
|
||||
expect(await storage.getServerMetadata(testServerUrl)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -1,9 +1,5 @@
|
|||
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||
import { RemoteOAuthStorage } from "@inspector/core/auth/remote/storage-remote.js";
|
||||
import {
|
||||
type OAuthStore,
|
||||
waitForOAuthStorePersistLoad,
|
||||
} from "@inspector/core/auth/store.js";
|
||||
|
||||
const NOOP_FETCH = vi.fn(
|
||||
async () =>
|
||||
|
|
@ -76,16 +72,16 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
|
|||
|
||||
it("codeVerifier round-trip and clearCodeVerifier", async () => {
|
||||
await storage.saveCodeVerifier(serverUrl, "verifier");
|
||||
expect(storage.getCodeVerifier(serverUrl)).toBe("verifier");
|
||||
expect(await storage.getCodeVerifier(serverUrl)).toBe("verifier");
|
||||
await storage.clearCodeVerifier(serverUrl);
|
||||
expect(storage.getCodeVerifier(serverUrl)).toBeUndefined();
|
||||
expect(await storage.getCodeVerifier(serverUrl)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("scope round-trip and clearScope", async () => {
|
||||
await storage.saveScope(serverUrl, "read write");
|
||||
expect(storage.getScope(serverUrl)).toBe("read write");
|
||||
expect(await storage.getScope(serverUrl)).toBe("read write");
|
||||
await storage.clearScope(serverUrl);
|
||||
expect(storage.getScope(serverUrl)).toBeUndefined();
|
||||
expect(await storage.getScope(serverUrl)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("serverMetadata round-trip and clearServerMetadata", async () => {
|
||||
|
|
@ -96,9 +92,9 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
|
|||
response_types_supported: ["code"],
|
||||
};
|
||||
await storage.saveServerMetadata(serverUrl, md);
|
||||
expect(storage.getServerMetadata(serverUrl)).toEqual(md);
|
||||
expect(await storage.getServerMetadata(serverUrl)).toEqual(md);
|
||||
await storage.clearServerMetadata(serverUrl);
|
||||
expect(storage.getServerMetadata(serverUrl)).toBeNull();
|
||||
expect(await storage.getServerMetadata(serverUrl)).toBeNull();
|
||||
});
|
||||
|
||||
it("clear() wipes all state for a server", async () => {
|
||||
|
|
@ -125,13 +121,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
|
|||
expect(s).toBeInstanceOf(RemoteOAuthStorage);
|
||||
});
|
||||
|
||||
it("waitForOAuthStorePersistLoad resolves for an unregistered store handle", async () => {
|
||||
await expect(
|
||||
waitForOAuthStorePersistLoad({} as OAuthStore),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("load() waits for remote GET before sync reads see persisted state", async () => {
|
||||
it("getCodeVerifier loads remote state automatically when not preloaded", async () => {
|
||||
const persisted = {
|
||||
state: {
|
||||
servers: {
|
||||
|
|
@ -155,15 +145,10 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
|
|||
fetchFn: delayedFetch,
|
||||
});
|
||||
|
||||
const loadPromise = delayedStorage.load();
|
||||
expect(delayedStorage.getCodeVerifier(serverUrl)).toBeUndefined();
|
||||
|
||||
const getPromise = delayedStorage.getCodeVerifier(serverUrl);
|
||||
resolveFetch(new Response(JSON.stringify(persisted), { status: 200 }));
|
||||
await loadPromise;
|
||||
|
||||
expect(delayedStorage.getCodeVerifier(serverUrl)).toBe(
|
||||
"persisted-verifier",
|
||||
);
|
||||
expect(await getPromise).toBe("persisted-verifier");
|
||||
});
|
||||
|
||||
it("load() rejects when remote GET fails instead of hanging", async () => {
|
||||
|
|
|
|||
84
clients/web/src/test/core/auth/store.test.ts
Normal file
84
clients/web/src/test/core/auth/store.test.ts
Normal file
|
|
@ -0,0 +1,84 @@
|
|||
import { describe, it, expect } from "vitest";
|
||||
import { OAuthMemoryStore } from "@inspector/core/auth/store.js";
|
||||
|
||||
describe("OAuthMemoryStore", () => {
|
||||
it("returns empty defaults for unknown server / issuer", () => {
|
||||
const store = new OAuthMemoryStore();
|
||||
expect(store.getState().getServerState("http://x")).toEqual({});
|
||||
expect(store.getState().getIdpSession("http://idp")).toEqual({});
|
||||
});
|
||||
|
||||
it("merges partial updates immutably via setServerState / setIdpSession", () => {
|
||||
const store = new OAuthMemoryStore();
|
||||
const s = store.getState();
|
||||
s.setServerState("http://x", { scope: "read" });
|
||||
s.setServerState("http://x", { codeVerifier: "v" });
|
||||
expect(store.getState().getServerState("http://x")).toEqual({
|
||||
scope: "read",
|
||||
codeVerifier: "v",
|
||||
});
|
||||
s.setIdpSession("http://idp", { idToken: "t" });
|
||||
expect(store.getState().getIdpSession("http://idp")).toEqual({
|
||||
idToken: "t",
|
||||
});
|
||||
});
|
||||
|
||||
it("clears a single server and a single idp session", () => {
|
||||
const store = new OAuthMemoryStore({
|
||||
servers: { "http://x": { scope: "read" } },
|
||||
idpSessions: { "http://idp": { idToken: "t" } },
|
||||
});
|
||||
store.getState().clearServerState("http://x");
|
||||
store.getState().clearIdpSession("http://idp");
|
||||
expect(store.getState().getServerState("http://x")).toEqual({});
|
||||
expect(store.getState().getIdpSession("http://idp")).toEqual({});
|
||||
});
|
||||
|
||||
it("clears only enterprise-managed servers", () => {
|
||||
const store = new OAuthMemoryStore({
|
||||
servers: {
|
||||
"http://managed": { enterpriseManaged: true },
|
||||
"http://plain": { scope: "read" },
|
||||
},
|
||||
idpSessions: {},
|
||||
});
|
||||
store.getState().clearEnterpriseManagedResourceServers();
|
||||
expect(store.getState().getServerState("http://managed")).toEqual({});
|
||||
expect(store.getState().getServerState("http://plain")).toEqual({
|
||||
scope: "read",
|
||||
});
|
||||
});
|
||||
|
||||
it("snapshot() returns a shallow copy of current state", () => {
|
||||
const store = new OAuthMemoryStore({
|
||||
servers: { "http://x": { scope: "read" } },
|
||||
idpSessions: {},
|
||||
});
|
||||
const snap = store.snapshot();
|
||||
expect(snap).toEqual({
|
||||
servers: { "http://x": { scope: "read" } },
|
||||
idpSessions: {},
|
||||
});
|
||||
// Mutating the store afterward must not change the earlier snapshot.
|
||||
store.getState().setServerState("http://y", { scope: "write" });
|
||||
expect(snap.servers["http://y"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("replace() tolerates a snapshot missing servers or idpSessions", () => {
|
||||
// Exercises both `?? {}` fallbacks in replace().
|
||||
const noServers = new OAuthMemoryStore({
|
||||
idpSessions: { "http://idp": { idToken: "t" } },
|
||||
} as never);
|
||||
expect(noServers.snapshot()).toEqual({
|
||||
servers: {},
|
||||
idpSessions: { "http://idp": { idToken: "t" } },
|
||||
});
|
||||
const noSessions = new OAuthMemoryStore({
|
||||
servers: { "http://x": { scope: "read" } },
|
||||
} as never);
|
||||
expect(noSessions.snapshot()).toEqual({
|
||||
servers: { "http://x": { scope: "read" } },
|
||||
idpSessions: {},
|
||||
});
|
||||
});
|
||||
});
|
||||
Some files were not shown because too many files have changed in this diff Show more
Loading…
Reference in a new issue