modelcontextprotocol/inspector
Watch
1
0
Fork
You've already forked inspector
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(auth): await OAuth storage load() to fix remote OAuth races

Browse source
This commit is contained in:
Bob Dickinson 2026-07-03 14:18:00 -07:00
commit a24091484c
9 changed files with 245 additions and 133 deletions
Download patch file Download diff file Expand all files Collapse all files

1
clients/web/src/test/core/auth/connection-state.test.ts
View file

@ -22,6 +22,7 @@ function createStorage(
}> = {},
): OAuthStorage {
return {
load: vi.fn().mockResolvedValue(undefined),
getTokens: vi.fn().mockResolvedValue(overrides.tokens),
getClientInformation: vi.fn(async (_url, isPreregistered) =>
isPreregistered ? overrides.preregistered : overrides.dynamic,

1
clients/web/src/test/core/auth/ema/emaFlow.test.ts
View file

@ -43,6 +43,7 @@ function createMemoryStorage(
const clientInfoByKey: Record<string, unknown> = {};
const codeVerifierByKey: Record<string, string> = {};
return {
load: vi.fn().mockResolvedValue(undefined),
getIdpSession: vi.fn(async (issuer: string) => idpSessions[issuer]),
saveIdpSession: vi.fn(async (issuer: string, updates) => {
idpSessions[issuer] = { ...idpSessions[issuer], ...updates };

4
clients/web/src/test/core/auth/ema/idpOidc.test.ts
View file

@ -29,6 +29,7 @@ describe("idpOidc refresh", () => {
beforeEach(() => {
Object.keys(idpSessions).forEach((k) => delete idpSessions[k]);
storage = {
load: vi.fn().mockResolvedValue(undefined),
getIdpSession: vi.fn(async (issuer: string) => idpSessions[issuer]),
saveIdpSession: vi.fn(async (issuer: string, updates) => {
idpSessions[issuer] = { ...idpSessions[issuer], ...updates };
@ -264,6 +265,8 @@ describe("startIdpOidcAuthorization", () => {
beforeEach(() => {
Object.keys(saved).forEach((k) => delete saved[k]);
storage = {
load: vi.fn().mockResolvedValue(undefined),
getServerMetadata: vi.fn(() => null),
saveCodeVerifier: vi.fn(async (key: string, value: string) => {
saved[`cv:${key}`] = value;
}),
@ -331,6 +334,7 @@ describe("completeIdpOidcAuthorization", () => {
function buildStorage(overrides: Partial<OAuthStorage> = {}): OAuthStorage {
return {
load: vi.fn().mockResolvedValue(undefined),
getServerMetadata: vi.fn(() => minimalOAuthAsMetadata(IDP_ISSUER)),
getClientInformation: vi.fn(async () => ({
client_id: "idp-client",

9
clients/web/src/test/core/auth/ema/idpSession.test.ts
View file

@ -19,6 +19,7 @@ describe("idpSession", () => {
beforeEach(() => {
storage = {
load: vi.fn().mockResolvedValue(undefined),
getIdpSession: vi.fn(),
saveIdpSession: vi.fn(),
clearIdpSession: vi.fn(),
@ -73,15 +74,15 @@ describe("idpSession", () => {
);
});
it("clearEmaIdpSession clears idp session, leg-1 key, and tagged resource servers", () => {
clearEmaIdpSession(storage, "https://idp.test/");
it("clearEmaIdpSession clears idp session, leg-1 key, and tagged resource servers", async () => {
await clearEmaIdpSession(storage, "https://idp.test/");
expect(storage.clearIdpSession).toHaveBeenCalledWith("https://idp.test");
expect(storage.clear).toHaveBeenCalledWith("ema-idp:https://idp.test");
expect(storage.clearEnterpriseManagedResourceServers).toHaveBeenCalled();
});
it("clearEmaIdpSession no-ops when issuer normalizes to empty", () => {
clearEmaIdpSession(storage, "");
it("clearEmaIdpSession no-ops when issuer normalizes to empty", async () => {
await clearEmaIdpSession(storage, "");
expect(storage.clearIdpSession).not.toHaveBeenCalled();
expect(storage.clear).not.toHaveBeenCalled();
expect(

5
clients/web/src/test/core/auth/providers.test.ts
View file

@ -213,6 +213,7 @@ describe("OAuthNavigation", () => {
function makeStorage(): OAuthStorage {
return {
load: vi.fn().mockResolvedValue(undefined),
getScope: vi.fn(() => undefined),
getClientInformation: vi.fn(async () => undefined),
saveClientInformation: vi.fn(async () => undefined),
@ -240,11 +241,11 @@ describe("OAuthNavigation", () => {
return new BaseOAuthClientProvider(SERVER, config);
}
it("clear() delegates to storage.clear with the server url", () => {
it("clear() delegates to storage.clear with the server url", async () => {
const storage = makeStorage();
const provider = makeProvider(storage);
provider.clear();
await provider.clear();
expect(storage.clear).toHaveBeenCalledWith(SERVER);
});

68
clients/web/src/test/core/auth/storage-browser.test.ts
View file

@ -77,7 +77,7 @@ describe("BrowserOAuthStorage", () => {
client_secret: "test-secret",
};
storage.saveClientInformation(testServerUrl, clientInfo, {
await storage.saveClientInformation(testServerUrl, clientInfo, {
registrationKind: "dcr",
});
const result = await storage.getClientInformation(testServerUrl);
@ -93,7 +93,7 @@ describe("BrowserOAuthStorage", () => {
// Use the storage API instead of manually setting sessionStorage
// since BrowserOAuthStorage now uses Zustand with a different storage format
storage.savePreregisteredClientInformation(
await storage.savePreregisteredClientInformation(
testServerUrl,
preregisteredInfo,
);
@ -113,7 +113,7 @@ describe("BrowserOAuthStorage", () => {
client_id: "test-client-id",
};
storage.saveClientInformation(testServerUrl, clientInfo, {
await storage.saveClientInformation(testServerUrl, clientInfo, {
registrationKind: "dcr",
});
const result = await storage.getClientInformation(testServerUrl);
@ -130,10 +130,10 @@ describe("BrowserOAuthStorage", () => {
client_id: "second-id",
};
storage.saveClientInformation(testServerUrl, firstInfo, {
await storage.saveClientInformation(testServerUrl, firstInfo, {
registrationKind: "dcr",
});
storage.saveClientInformation(testServerUrl, secondInfo, {
await storage.saveClientInformation(testServerUrl, secondInfo, {
registrationKind: "cimd",
});
const result = await storage.getClientInformation(testServerUrl);
@ -163,7 +163,7 @@ describe("BrowserOAuthStorage", () => {
expires_in: 3600,
};
storage.saveTokens(testServerUrl, tokens);
await storage.saveTokens(testServerUrl, tokens);
const result = await storage.getTokens(testServerUrl);
expect(result).toEqual(tokens);
@ -177,7 +177,7 @@ describe("BrowserOAuthStorage", () => {
token_type: "Bearer",
};
storage.saveTokens(testServerUrl, tokens);
await storage.saveTokens(testServerUrl, tokens);
const result = await storage.getTokens(testServerUrl);
expect(result).toEqual(tokens);
@ -194,7 +194,7 @@ describe("BrowserOAuthStorage", () => {
await storage.saveTokens(emaUrl, tokens, { enterpriseManaged: true });
await storage.saveTokens(standardUrl, tokens);
storage.clearEnterpriseManagedResourceServers();
await storage.clearEnterpriseManagedResourceServers();
expect(await storage.getTokens(emaUrl)).toBeUndefined();
expect(await storage.getTokens(standardUrl)).toEqual(tokens);
@ -210,7 +210,7 @@ describe("BrowserOAuthStorage", () => {
it("should return stored code verifier", async () => {
const codeVerifier = "test-code-verifier";
storage.saveCodeVerifier(testServerUrl, codeVerifier);
await storage.saveCodeVerifier(testServerUrl, codeVerifier);
const result = await storage.getCodeVerifier(testServerUrl);
expect(result).toBe(codeVerifier);
@ -221,7 +221,7 @@ describe("BrowserOAuthStorage", () => {
it("should save code verifier", async () => {
const codeVerifier = "test-code-verifier";
storage.saveCodeVerifier(testServerUrl, codeVerifier);
await storage.saveCodeVerifier(testServerUrl, codeVerifier);
const result = await storage.getCodeVerifier(testServerUrl);
expect(result).toBe(codeVerifier);
@ -237,7 +237,7 @@ describe("BrowserOAuthStorage", () => {
it("should return stored scope", async () => {
const scope = "read write";
storage.saveScope(testServerUrl, scope);
await storage.saveScope(testServerUrl, scope);
const result = await storage.getScope(testServerUrl);
expect(result).toBe(scope);
@ -248,7 +248,7 @@ describe("BrowserOAuthStorage", () => {
it("should save scope", async () => {
const scope = "read write";
storage.saveScope(testServerUrl, scope);
await storage.saveScope(testServerUrl, scope);
const result = await storage.getScope(testServerUrl);
expect(result).toBe(scope);
@ -269,7 +269,7 @@ describe("BrowserOAuthStorage", () => {
response_types_supported: ["code"],
};
storage.saveServerMetadata(testServerUrl, metadata);
await storage.saveServerMetadata(testServerUrl, metadata);
const result = await storage.getServerMetadata(testServerUrl);
expect(result).toEqual(metadata);
@ -285,7 +285,7 @@ describe("BrowserOAuthStorage", () => {
response_types_supported: ["code"],
};
storage.saveServerMetadata(testServerUrl, metadata);
await storage.saveServerMetadata(testServerUrl, metadata);
const result = await storage.getServerMetadata(testServerUrl);
expect(result).toEqual(metadata);
@ -294,7 +294,7 @@ describe("BrowserOAuthStorage", () => {
describe("clearClientInformation", () => {
it("removes the dynamically-registered client info by default", async () => {
storage.saveClientInformation(
await storage.saveClientInformation(
testServerUrl,
{ client_id: "dyn" },
{ registrationKind: "dcr" },
@ -302,18 +302,18 @@ describe("BrowserOAuthStorage", () => {
expect(await storage.getClientInformation(testServerUrl)).toEqual({
client_id: "dyn",
});
storage.clearClientInformation(testServerUrl);
await storage.clearClientInformation(testServerUrl);
expect(await storage.getClientInformation(testServerUrl)).toBeUndefined();
});
it("removes the preregistered client info when isPreregistered=true", async () => {
storage.savePreregisteredClientInformation(testServerUrl, {
await storage.savePreregisteredClientInformation(testServerUrl, {
client_id: "pre",
});
expect(await storage.getClientInformation(testServerUrl, true)).toEqual({
client_id: "pre",
});
storage.clearClientInformation(testServerUrl, true);
await storage.clearClientInformation(testServerUrl, true);
expect(
await storage.getClientInformation(testServerUrl, true),
).toBeUndefined();
@ -322,26 +322,26 @@ describe("BrowserOAuthStorage", () => {
describe("individual clear methods", () => {
it("clearTokens removes only tokens", async () => {
storage.saveTokens(testServerUrl, {
await storage.saveTokens(testServerUrl, {
access_token: "t",
token_type: "Bearer",
});
expect(await storage.getTokens(testServerUrl)).toBeDefined();
storage.clearTokens(testServerUrl);
await storage.clearTokens(testServerUrl);
expect(await storage.getTokens(testServerUrl)).toBeUndefined();
});
it("clearCodeVerifier removes only the PKCE verifier", async () => {
storage.saveCodeVerifier(testServerUrl, "verifier");
await storage.saveCodeVerifier(testServerUrl, "verifier");
expect(storage.getCodeVerifier(testServerUrl)).toBe("verifier");
storage.clearCodeVerifier(testServerUrl);
await storage.clearCodeVerifier(testServerUrl);
expect(storage.getCodeVerifier(testServerUrl)).toBeUndefined();
});
it("clearScope removes only the scope", async () => {
storage.saveScope(testServerUrl, "read");
await storage.saveScope(testServerUrl, "read");
expect(storage.getScope(testServerUrl)).toBe("read");
storage.clearScope(testServerUrl);
await storage.clearScope(testServerUrl);
expect(storage.getScope(testServerUrl)).toBeUndefined();
});
@ -352,9 +352,9 @@ describe("BrowserOAuthStorage", () => {
token_endpoint: "http://localhost:3000/token",
response_types_supported: ["code"],
};
storage.saveServerMetadata(testServerUrl, metadata);
await storage.saveServerMetadata(testServerUrl, metadata);
expect(storage.getServerMetadata(testServerUrl)).toEqual(metadata);
storage.clearServerMetadata(testServerUrl);
await storage.clearServerMetadata(testServerUrl);
expect(storage.getServerMetadata(testServerUrl)).toBeNull();
});
});
@ -369,12 +369,12 @@ describe("BrowserOAuthStorage", () => {
token_type: "Bearer",
};
storage.saveClientInformation(testServerUrl, clientInfo, {
await storage.saveClientInformation(testServerUrl, clientInfo, {
registrationKind: "dcr",
});
storage.saveTokens(testServerUrl, tokens);
await storage.saveTokens(testServerUrl, tokens);
storage.clear(testServerUrl);
await storage.clear(testServerUrl);
expect(await storage.getClientInformation(testServerUrl)).toBeUndefined();
expect(await storage.getTokens(testServerUrl)).toBeUndefined();
@ -386,14 +386,14 @@ describe("BrowserOAuthStorage", () => {
client_id: "test-client-id",
};
storage.saveClientInformation(testServerUrl, clientInfo, {
await storage.saveClientInformation(testServerUrl, clientInfo, {
registrationKind: "dcr",
});
storage.saveClientInformation(otherServerUrl, clientInfo, {
await storage.saveClientInformation(otherServerUrl, clientInfo, {
registrationKind: "dcr",
});
storage.clear(testServerUrl);
await storage.clear(testServerUrl);
expect(await storage.getClientInformation(testServerUrl)).toBeUndefined();
expect(await storage.getClientInformation(otherServerUrl)).toEqual(
@ -415,10 +415,10 @@ describe("BrowserOAuthStorage", () => {
client_id: "client-2",
};
storage.saveClientInformation(server1Url, clientInfo1, {
await storage.saveClientInformation(server1Url, clientInfo1, {
registrationKind: "dcr",
});
storage.saveClientInformation(server2Url, clientInfo2, {
await storage.saveClientInformation(server2Url, clientInfo2, {
registrationKind: "dcr",
});

49
clients/web/src/test/core/auth/storage-remote.test.ts
View file

@ -50,7 +50,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
{ client_id: "dyn" },
{ registrationKind: "dcr" },
);
storage.clearClientInformation(serverUrl);
await storage.clearClientInformation(serverUrl);
expect(await storage.getClientInformation(serverUrl)).toBeUndefined();
});
@ -58,7 +58,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
await storage.savePreregisteredClientInformation(serverUrl, {
client_id: "pre",
});
storage.clearClientInformation(serverUrl, true);
await storage.clearClientInformation(serverUrl, true);
expect(await storage.getClientInformation(serverUrl, true)).toBeUndefined();
});
@ -66,21 +66,21 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
const tokens = { access_token: "t", token_type: "Bearer" };
await storage.saveTokens(serverUrl, tokens);
expect(await storage.getTokens(serverUrl)).toEqual(tokens);
storage.clearTokens(serverUrl);
await storage.clearTokens(serverUrl);
expect(await storage.getTokens(serverUrl)).toBeUndefined();
});
it("codeVerifier round-trip and clearCodeVerifier", async () => {
await storage.saveCodeVerifier(serverUrl, "verifier");
expect(storage.getCodeVerifier(serverUrl)).toBe("verifier");
storage.clearCodeVerifier(serverUrl);
await storage.clearCodeVerifier(serverUrl);
expect(storage.getCodeVerifier(serverUrl)).toBeUndefined();
});
it("scope round-trip and clearScope", async () => {
await storage.saveScope(serverUrl, "read write");
expect(storage.getScope(serverUrl)).toBe("read write");
storage.clearScope(serverUrl);
await storage.clearScope(serverUrl);
expect(storage.getScope(serverUrl)).toBeUndefined();
});
@ -93,7 +93,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
};
await storage.saveServerMetadata(serverUrl, md);
expect(storage.getServerMetadata(serverUrl)).toEqual(md);
storage.clearServerMetadata(serverUrl);
await storage.clearServerMetadata(serverUrl);
expect(storage.getServerMetadata(serverUrl)).toBeNull();
});
@ -107,7 +107,7 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
access_token: "t",
token_type: "Bearer",
});
storage.clear(serverUrl);
await storage.clear(serverUrl);
expect(await storage.getClientInformation(serverUrl)).toBeUndefined();
expect(await storage.getTokens(serverUrl)).toBeUndefined();
});
@ -120,4 +120,39 @@ describe("RemoteOAuthStorage (unit, mocked fetch)", () => {
// No public accessor; constructing without throwing covers the default-branch.
expect(s).toBeInstanceOf(RemoteOAuthStorage);
});
it("load() waits for remote GET before sync reads see persisted state", async () => {
const persisted = {
state: {
servers: {
[serverUrl]: { codeVerifier: "persisted-verifier" },
},
idpSessions: {},
},
version: 0,
};
let resolveFetch!: (value: Response) => void;
const delayedFetch = vi.fn(
() =>
new Promise<Response>((resolve) => {
resolveFetch = resolve;
}),
) as unknown as typeof fetch;
const delayedStorage = new RemoteOAuthStorage({
baseUrl: "http://remote.example",
storeId: `delayed-${Math.random().toString(36).slice(2)}`,
fetchFn: delayedFetch,
});
const loadPromise = delayedStorage.load();
expect(delayedStorage.getCodeVerifier(serverUrl)).toBeUndefined();
resolveFetch(new Response(JSON.stringify(persisted), { status: 200 }));
await loadPromise;
expect(delayedStorage.getCodeVerifier(serverUrl)).toBe(
"persisted-verifier",
);
});
});

11
clients/web/src/test/core/mcp/oauthManager.test.ts
View file

@ -35,6 +35,7 @@ function createMockParams(
const dispatchOAuthError = vi.fn();
const storage = {
load: vi.fn().mockResolvedValue(undefined),
getScope: vi.fn().mockReturnValue(undefined),
getClientInformation: vi.fn().mockResolvedValue(undefined),
getClientRegistrationKind: vi.fn().mockReturnValue(undefined),
@ -45,7 +46,7 @@ function createMockParams(
saveTokens: vi.fn().mockResolvedValue(undefined),
getCodeVerifier: vi.fn().mockReturnValue("verifier"),
saveCodeVerifier: vi.fn().mockResolvedValue(undefined),
clear: vi.fn(),
clear: vi.fn().mockResolvedValue(undefined),
clearClientInformation: vi.fn(),
clearTokens: vi.fn(),
clearCodeVerifier: vi.fn(),
@ -126,10 +127,10 @@ describe("OAuthManager", () => {
});
describe("clearOAuthTokens", () => {
it("calls storage.clear(serverUrl) when storage is configured", () => {
it("calls storage.clear(serverUrl) when storage is configured", async () => {
const params = createMockParams();
const manager = new OAuthManager(params);
manager.clearOAuthTokens();
await manager.clearOAuthTokens();
expect(params.initialConfig.storage!.clear).toHaveBeenCalledWith(
SERVER_URL,
);
@ -137,7 +138,7 @@ describe("OAuthManager", () => {
expect(manager.getOAuthFlowStep()).toBeUndefined();
});
it("no-ops when storage is not configured", () => {
it("no-ops when storage is not configured", async () => {
const params = createMockParams({
initialConfig: {
redirectUrlProvider: {
@ -147,7 +148,7 @@ describe("OAuthManager", () => {
} as OAuthManagerConfig,
});
const manager = new OAuthManager(params);
manager.clearOAuthTokens();
await manager.clearOAuthTokens();
expect(params.getServerUrl).not.toHaveBeenCalled();
});
});

19
clients/web/src/test/core/react/useEmaIdpLoginState.test.tsx
View file

@ -8,10 +8,13 @@ describe("useEmaIdpLoginState", () => {
beforeEach(() => {
storage = {
load: vi.fn().mockResolvedValue(undefined),
getIdpSession: vi.fn().mockResolvedValue(undefined),
clearIdpSession: vi.fn(),
clear: vi.fn(),
clearEnterpriseManagedResourceServers: vi.fn(),
clearIdpSession: vi.fn().mockResolvedValue(undefined),
clear: vi.fn().mockResolvedValue(undefined),
clearEnterpriseManagedResourceServers: vi
.fn()
.mockResolvedValue(undefined),
} as unknown as OAuthStorage;
});
@ -56,10 +59,12 @@ describe("useEmaIdpLoginState", () => {
result.current.logout();
});
expect(storage.clearIdpSession).toHaveBeenCalledWith("https://idp.test");
expect(storage.clear).toHaveBeenCalledWith("ema-idp:https://idp.test");
expect(storage.clearEnterpriseManagedResourceServers).toHaveBeenCalled();
expect(result.current.loginState).toBe("none");
await waitFor(() => {
expect(storage.clearIdpSession).toHaveBeenCalledWith("https://idp.test");
expect(storage.clear).toHaveBeenCalledWith("ema-idp:https://idp.test");
expect(storage.clearEnterpriseManagedResourceServers).toHaveBeenCalled();
expect(result.current.loginState).toBe("none");
});
});
it("reports 'expired' for an expired token with no refresh token", async () => {