From ab7613e0f0a0534eeb67d2668e5a2e256932dbdf Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Fri, 30 May 2025 23:58:22 -0700 Subject: [PATCH 1/8] Add `state` param to OAuth /authorize request When doing an OAuth `/authorize` request to an OAuth authorization server, include a `state` parameter, as described at https://support.okta.com/help/s/article/the-authentication-request-has-an-invalid-state-parameter?language=en_US This enables OAuth to work with Okta. Fixes: https://github.com/orgs/mcp-auth/discussions/44, modelcontextprotocol/inspector#442 --- client/src/components/OAuthCallback.tsx | 17 ++++++++++++++ client/src/lib/auth.ts | 30 +++++++++++++++++++++++++ client/src/utils/oauthUtils.ts | 11 +++++++++ 3 files changed, 58 insertions(+) diff --git a/client/src/components/OAuthCallback.tsx b/client/src/components/OAuthCallback.tsx index 80ed62bb7..8f8fa7e8a 100644 --- a/client/src/components/OAuthCallback.tsx +++ b/client/src/components/OAuthCallback.tsx @@ -36,6 +36,9 @@ const OAuthCallback = ({ onConnect }: OAuthCallbackProps) => { }); const params = parseOAuthCallbackParams(window.location.search); + // Extract state from query params + const urlParams = new URLSearchParams(window.location.search); + const returnedState = urlParams.get("state"); if (!params.successful) { return notifyError(generateOAuthErrorDescription(params)); } @@ -45,6 +48,20 @@ const OAuthCallback = ({ onConnect }: OAuthCallbackProps) => { return notifyError("Missing Server URL"); } + // Validate state parameter + const serverAuthProvider = new InspectorOAuthClientProvider( + serverUrl, + undefined, + undefined, + ); + const expectedState = serverAuthProvider.getState(); + serverAuthProvider.clearState(); // Always clear after checking + if (!returnedState || !expectedState || returnedState !== expectedState) { + return notifyError( + "Invalid or missing OAuth state parameter. Please try logging in again." + ); + } + const clientInformation = await getClientInformationFromSessionStorage(serverUrl); diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 8d034296a..a3f693a7d 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -6,6 +6,7 @@ import { OAuthTokensSchema, } from "@modelcontextprotocol/sdk/shared/auth.js"; import { SESSION_KEYS, getServerSpecificKey } from "./constants"; +import { generateRandomState } from "@/utils/oauthUtils"; export const getClientInformationFromSessionStorage = async ( serverUrl: string, @@ -98,7 +99,36 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { sessionStorage.setItem(key, JSON.stringify(tokens)); } + /** + * Generate, store, and return a new state parameter for OAuth. + */ + generateAndStoreState(): string { + const state = generateRandomState(32); + const key = getServerSpecificKey("oauth_state", this.serverUrl); + sessionStorage.setItem(key, state); + return state; + } + + /** + * Retrieve the stored state parameter for this serverUrl. + */ + getState(): string | null { + const key = getServerSpecificKey("oauth_state", this.serverUrl); + return sessionStorage.getItem(key); + } + + /** + * Remove the stored state parameter for this serverUrl. + */ + clearState() { + const key = getServerSpecificKey("oauth_state", this.serverUrl); + sessionStorage.removeItem(key); + } + redirectToAuthorization(authorizationUrl: URL) { + // Generate and store a new state parameter + const state = this.generateAndStoreState(); + authorizationUrl.searchParams.set("state", state); const authParams = this.authParams(); console.log("authParams", authParams); if (authParams) { diff --git a/client/src/utils/oauthUtils.ts b/client/src/utils/oauthUtils.ts index c971271e3..79e753ced 100644 --- a/client/src/utils/oauthUtils.ts +++ b/client/src/utils/oauthUtils.ts @@ -63,3 +63,14 @@ export const generateOAuthErrorDescription = ( .filter(Boolean) .join("\n"); }; + +/** + * Generates a cryptographically secure random string for use as OAuth state or PKCE code_verifier. + * @param length Number of characters in the generated string (default: 32) + */ +export function generateRandomState(length = 32): string { + const charset = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + const array = new Uint8Array(length); + window.crypto.getRandomValues(array); + return Array.from(array, (byte) => charset[byte % charset.length]).join(''); +} From a93fd44ea48268a3487c7d22f46b370191b30096 Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Sat, 31 May 2025 00:30:40 -0700 Subject: [PATCH 2/8] npm run prettier-fix --- client/src/components/OAuthCallback.tsx | 2 +- client/src/utils/oauthUtils.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/src/components/OAuthCallback.tsx b/client/src/components/OAuthCallback.tsx index 8f8fa7e8a..9c19e6957 100644 --- a/client/src/components/OAuthCallback.tsx +++ b/client/src/components/OAuthCallback.tsx @@ -58,7 +58,7 @@ const OAuthCallback = ({ onConnect }: OAuthCallbackProps) => { serverAuthProvider.clearState(); // Always clear after checking if (!returnedState || !expectedState || returnedState !== expectedState) { return notifyError( - "Invalid or missing OAuth state parameter. Please try logging in again." + "Invalid or missing OAuth state parameter. Please try logging in again.", ); } diff --git a/client/src/utils/oauthUtils.ts b/client/src/utils/oauthUtils.ts index 79e753ced..db9ba22ff 100644 --- a/client/src/utils/oauthUtils.ts +++ b/client/src/utils/oauthUtils.ts @@ -69,8 +69,9 @@ export const generateOAuthErrorDescription = ( * @param length Number of characters in the generated string (default: 32) */ export function generateRandomState(length = 32): string { - const charset = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'; + const charset = + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; const array = new Uint8Array(length); window.crypto.getRandomValues(array); - return Array.from(array, (byte) => charset[byte % charset.length]).join(''); + return Array.from(array, (byte) => charset[byte % charset.length]).join(""); } From 9b34f4fa90b687ee399fa3bcec59ebb45211e04a Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Sat, 31 May 2025 00:34:33 -0700 Subject: [PATCH 3/8] Add `generateRandomState` test to `oauthUtils.ts` --- client/src/utils/__tests__/oauthUtils.ts | 27 +++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/client/src/utils/__tests__/oauthUtils.ts b/client/src/utils/__tests__/oauthUtils.ts index cc9674cb2..22e3757fe 100644 --- a/client/src/utils/__tests__/oauthUtils.ts +++ b/client/src/utils/__tests__/oauthUtils.ts @@ -1,7 +1,8 @@ import { generateOAuthErrorDescription, parseOAuthCallbackParams, -} from "@/utils/oauthUtils.ts"; + generateRandomState, +} from "@/utils/oauthUtils"; describe("parseOAuthCallbackParams", () => { it("Returns successful: true and code when present", () => { @@ -76,3 +77,27 @@ describe("generateOAuthErrorDescription", () => { ); }); }); + +describe("generateRandomState", () => { + it("generates a string of the correct length", () => { + const state = generateRandomState(32); + expect(state).toHaveLength(32); + const state16 = generateRandomState(16); + expect(state16).toHaveLength(16); + }); + + it("generates a string with only allowed characters", () => { + const charset = + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; + const state = generateRandomState(64); + for (const char of state) { + expect(charset.includes(char)).toBe(true); + } + }); + + it("generates different values on subsequent calls (randomness)", () => { + const state1 = generateRandomState(32); + const state2 = generateRandomState(32); + expect(state1).not.toEqual(state2); + }); +}); From 04e3d72188e0e45215bdd6e9a74b6a3ceef637ae Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Sat, 31 May 2025 00:39:42 -0700 Subject: [PATCH 4/8] Add client/src/lib/auth.state.test.ts --- client/src/lib/auth.state.test.ts | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 client/src/lib/auth.state.test.ts diff --git a/client/src/lib/auth.state.test.ts b/client/src/lib/auth.state.test.ts new file mode 100644 index 000000000..6aa3fd3ae --- /dev/null +++ b/client/src/lib/auth.state.test.ts @@ -0,0 +1,31 @@ +import { InspectorOAuthClientProvider } from "./auth"; + +describe("InspectorOAuthClientProvider state parameter", () => { + const serverUrl = "https://example.com"; + let provider: InspectorOAuthClientProvider; + + beforeEach(() => { + provider = new InspectorOAuthClientProvider(serverUrl); + sessionStorage.clear(); + }); + + it("generates, stores, and retrieves state", () => { + const state = provider.generateAndStoreState(); + expect(state).toBeDefined(); + expect(state).toEqual(provider.getState()); + expect(state).toHaveLength(32); + }); + + it("clears state from sessionStorage", () => { + provider.generateAndStoreState(); + provider.clearState(); + expect(provider.getState()).toBeNull(); + }); + + it("generates a new state each time", () => { + const state1 = provider.generateAndStoreState(); + provider.clearState(); + const state2 = provider.generateAndStoreState(); + expect(state1).not.toEqual(state2); + }); +}); From 091f68e7673ddf2eb1927a6d2fa603d5e91a44d0 Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Sat, 31 May 2025 00:46:43 -0700 Subject: [PATCH 5/8] Add client/src/lib/auth.authorize-url.test.ts --- client/src/lib/auth.authorize-url.test.ts | 39 +++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 client/src/lib/auth.authorize-url.test.ts diff --git a/client/src/lib/auth.authorize-url.test.ts b/client/src/lib/auth.authorize-url.test.ts new file mode 100644 index 000000000..1cb1e1621 --- /dev/null +++ b/client/src/lib/auth.authorize-url.test.ts @@ -0,0 +1,39 @@ +import { InspectorOAuthClientProvider } from "./auth"; + +describe("OAuth /authorize URL includes state parameter", () => { + const serverUrl = "https://example.com"; + let provider: InspectorOAuthClientProvider; + + // Suppress console.log for this test suite + beforeAll(() => { + jest.spyOn(console, "log").mockImplementation(() => {}); + }); + + beforeEach(() => { + provider = new InspectorOAuthClientProvider(serverUrl); + sessionStorage.clear(); + }); + + it("includes state parameter in the authorization URL", () => { + // Mock window.location.href + const originalLocation = window.location; + delete (window as any).location; + (window as any).location = { href: "" }; + + const url = new URL("https://authserver.com/authorize"); + provider.redirectToAuthorization(url); + + // Check that the URL contains the state parameter + expect(window.location.href).toContain("state="); + const stateInUrl = new URL(window.location.href).searchParams.get("state"); + expect(stateInUrl).toBeDefined(); + expect(stateInUrl!.length).toBeGreaterThan(0); + + // Restore window.location + window.location = originalLocation; + }); + + afterAll(() => { + (console.log as jest.Mock).mockRestore(); + }); +}); From 0de8a3634a4eb1bbaa7f99af408d8d12ecb48375 Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Wed, 4 Jun 2025 06:46:07 -0700 Subject: [PATCH 6/8] Replace "oauth_state" with SESSION_KEYS.OAUTH_STATE --- client/src/lib/auth.ts | 6 +++--- client/src/lib/constants.ts | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index a3f693a7d..a41340c28 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -104,7 +104,7 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { */ generateAndStoreState(): string { const state = generateRandomState(32); - const key = getServerSpecificKey("oauth_state", this.serverUrl); + const key = getServerSpecificKey(SESSION_KEYS.OAUTH_STATE, this.serverUrl); sessionStorage.setItem(key, state); return state; } @@ -113,7 +113,7 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { * Retrieve the stored state parameter for this serverUrl. */ getState(): string | null { - const key = getServerSpecificKey("oauth_state", this.serverUrl); + const key = getServerSpecificKey(SESSION_KEYS.OAUTH_STATE, this.serverUrl); return sessionStorage.getItem(key); } @@ -121,7 +121,7 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { * Remove the stored state parameter for this serverUrl. */ clearState() { - const key = getServerSpecificKey("oauth_state", this.serverUrl); + const key = getServerSpecificKey(SESSION_KEYS.OAUTH_STATE, this.serverUrl); sessionStorage.removeItem(key); } diff --git a/client/src/lib/constants.ts b/client/src/lib/constants.ts index d1d3e0787..b574250ae 100644 --- a/client/src/lib/constants.ts +++ b/client/src/lib/constants.ts @@ -7,6 +7,7 @@ export const SESSION_KEYS = { TOKENS: "mcp_tokens", CLIENT_INFORMATION: "mcp_client_information", OAUTH_PARAMS: "mcp_oauth_params", + OAUTH_STATE: "oauth_state", } as const; // Generate server-specific session storage keys From 53495df96eea4ce75734360176b9abcb268568bc Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Wed, 4 Jun 2025 09:12:42 -0700 Subject: [PATCH 7/8] Fix a bunch of TypeScript build errors from apparently forgetting to add some new defaultProps ``` > @modelcontextprotocol/inspector-client@0.10.2 build > tsc -b && vite build src/components/__tests__/Sidebar.test.tsx:44:10 - error TS2739: Type '{ connectionStatus: "disconnected"; transportType: "stdio"; setTransportType: Mock; command: string; setCommand: Mock; ... 16 more ...; setConfig: Mock<...>; }' is missing the following properties from type 'SidebarProps': oauthClientId, setOauthClientId, oauthParams, setOauthParams 44 ~~~~~~~ src/components/__tests__/Sidebar.test.tsx:185:12 - error TS2739: Type '{ bearerToken: string; transportType: "sse"; connectionStatus: "disconnected"; setTransportType: Mock; command: string; setCommand: Mock; ... 15 more ...; setConfig: Mock<...>; }' is missing the following properties from type 'SidebarProps': oauthClientId, setOauthClientId, oauthParams, setOauthParams 185 ; command: string; setCommand: Mock; ... 15 more ...; setConfig: Mock<...>; }' is missing the following properties from type 'SidebarProps': oauthClientId, setOauthClientId, oauthParams, setOauthParams 215 ; setEnv: Mock; connectionStatus: "disconnected"; transportType: "stdio"; setTransportType: Mock; ... 16 more ...; setConfig: Mock<...>; }' is missing the following properties from type 'SidebarProps': oauthClientId, setOauthClientId, oauthParams, setOauthParams 379 ~~~~~~~ src/components/__tests__/Sidebar.test.tsx:414:12 - error TS2739: Type '{ env: { NEW_KEY: string; }; connectionStatus: "disconnected"; transportType: "stdio"; setTransportType: Mock; command: string; setCommand: Mock; ... 15 more ...; setConfig: Mock<...>; }' is missing the following properties from type 'SidebarProps': oauthClientId, setOauthClientId, oauthParams, setOauthParams 414 ~~~~~~~ src/components/__tests__/Sidebar.test.tsx:599:12 - error TS2739: Type '{ config: InspectorConfig; setConfig: Mock; connectionStatus: "disconnected"; transportType: "stdio"; setTransportType: Mock<...>; ... 16 more ...; loggingSupported: boolean; }' is missing the following properties from type 'SidebarProps': oauthClientId, setOauthClientId, oauthParams, setOauthParams 599 { setEnv: jest.fn(), bearerToken: "", setBearerToken: jest.fn(), + oauthClientId: "", + setOauthClientId: jest.fn(), + oauthParams: "", + setOauthParams: jest.fn(), onConnect: jest.fn(), onDisconnect: jest.fn(), stdErrNotifications: [], From 978f3d366b4e4ef152a81d4e611461741af3e69c Mon Sep 17 00:00:00 2001 From: Marc Abramowitz Date: Wed, 4 Jun 2025 09:19:37 -0700 Subject: [PATCH 8/8] Fix type error on window.location in auth.authorize-url.test.ts --- client/src/lib/auth.authorize-url.test.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/client/src/lib/auth.authorize-url.test.ts b/client/src/lib/auth.authorize-url.test.ts index 1cb1e1621..53c6404db 100644 --- a/client/src/lib/auth.authorize-url.test.ts +++ b/client/src/lib/auth.authorize-url.test.ts @@ -15,10 +15,13 @@ describe("OAuth /authorize URL includes state parameter", () => { }); it("includes state parameter in the authorization URL", () => { - // Mock window.location.href + // Mock window.location.href using Object.defineProperty const originalLocation = window.location; - delete (window as any).location; - (window as any).location = { href: "" }; + + Object.defineProperty(window, "location", { + value: { href: "" }, + writable: true, + }); const url = new URL("https://authserver.com/authorize"); provider.redirectToAuthorization(url); @@ -30,7 +33,10 @@ describe("OAuth /authorize URL includes state parameter", () => { expect(stateInUrl!.length).toBeGreaterThan(0); // Restore window.location - window.location = originalLocation; + Object.defineProperty(window, "location", { + value: originalLocation, + writable: true, + }); }); afterAll(() => {