diff --git a/client/src/lib/__tests__/auth.test.ts b/client/src/lib/__tests__/auth.test.ts index 329b7f027..639d9d829 100644 --- a/client/src/lib/__tests__/auth.test.ts +++ b/client/src/lib/__tests__/auth.test.ts @@ -1,4 +1,4 @@ -import { discoverScopes } from "../auth"; +import { discoverScopes, InspectorOAuthClientProvider } from "../auth"; import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js"; jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ @@ -153,3 +153,107 @@ describe("discoverScopes", () => { }, ); }); + +describe("InspectorOAuthClientProvider", () => { + let provider: InspectorOAuthClientProvider; + + beforeEach(() => { + // Clear sessionStorage before each test + sessionStorage.clear(); + provider = new InspectorOAuthClientProvider("https://example.com"); + }); + + afterEach(() => { + sessionStorage.clear(); + }); + + describe("client_secret handling", () => { + it("should temporarily store client_secret for OAuth flow", async () => { + const clientInfo = { + client_id: "test_client_id", + client_secret: "test_client_secret", + redirect_uris: ["http://localhost:3000/oauth/callback"], + }; + + // Save client information (this should store the secret temporarily) + provider.saveClientInformation(clientInfo); + + // Retrieve client information (should include the temporary secret) + const retrievedInfo = await provider.clientInformation(); + expect(retrievedInfo?.client_id).toBe("test_client_id"); + expect(retrievedInfo?.client_secret).toBe("test_client_secret"); + + // Verify that the secret is not stored in sessionStorage + const storedValue = sessionStorage.getItem( + "[https://example.com] mcp_client_information", + ); + expect(storedValue).toBeTruthy(); + const storedInfo = JSON.parse(storedValue!); + expect(storedInfo.client_secret).toBeUndefined(); + expect(storedInfo.client_id).toBe("test_client_id"); + }); + + it("should clear temporary client_secret after saving tokens", async () => { + const clientInfo = { + client_id: "test_client_id", + client_secret: "test_client_secret", + redirect_uris: ["http://localhost:3000/oauth/callback"], + }; + + // Save client information + provider.saveClientInformation(clientInfo); + + // Verify secret is available + let retrievedInfo = await provider.clientInformation(); + expect(retrievedInfo?.client_secret).toBe("test_client_secret"); + + // Save tokens (this should clear the temporary secret) + provider.saveTokens({ + access_token: "test_access_token", + token_type: "Bearer", + expires_in: 3600, + }); + + // Verify secret is no longer available + retrievedInfo = await provider.clientInformation(); + expect(retrievedInfo?.client_secret).toBeUndefined(); + }); + + it("should clear temporary client_secret when clear is called", async () => { + const clientInfo = { + client_id: "test_client_id", + client_secret: "test_client_secret", + redirect_uris: ["http://localhost:3000/oauth/callback"], + }; + + // Save client information + provider.saveClientInformation(clientInfo); + + // Verify secret is available + let retrievedInfo = await provider.clientInformation(); + expect(retrievedInfo?.client_secret).toBe("test_client_secret"); + + // Clear OAuth state + provider.clear(); + + // Verify secret is no longer available + retrievedInfo = await provider.clientInformation(); + expect(retrievedInfo).toBeUndefined(); + }); + + it("should handle client information without client_secret", async () => { + const clientInfo = { + client_id: "test_client_id", + redirect_uris: ["http://localhost:3000/oauth/callback"], + }; + + // Save client information without secret + provider.saveClientInformation(clientInfo); + + // Retrieve client information + const retrievedInfo = await provider.clientInformation(); + expect(retrievedInfo?.client_id).toBe("test_client_id"); + expect(retrievedInfo?.client_secret).toBeUndefined(); + }); + }); +}); diff --git a/client/src/lib/auth.ts b/client/src/lib/auth.ts index 78d52a59e..b60c06742 100644 --- a/client/src/lib/auth.ts +++ b/client/src/lib/auth.ts @@ -111,6 +111,7 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { sessionStorage.setItem(SESSION_KEYS.SERVER_URL, serverUrl); } scope: string | undefined; + private temporaryClientSecret?: string; get redirectUrl() { return window.location.origin + "/oauth/callback"; @@ -141,16 +142,30 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { }); // If no preregistered client information is found, get the dynamically registered client information - return ( + let clientInfo = preregisteredClientInformation ?? (await getClientInformationFromSessionStorage({ serverUrl: this.serverUrl, isPreregistered: false, - })) - ); + })); + + // If we have a temporary client secret, add it back for this request + if (clientInfo && this.temporaryClientSecret) { + clientInfo = { + ...clientInfo, + client_secret: this.temporaryClientSecret, + }; + } + + return clientInfo; } saveClientInformation(clientInformation: OAuthClientInformation) { + // Store client_secret temporarily for OAuth flow + if (clientInformation.client_secret) { + this.temporaryClientSecret = clientInformation.client_secret; + } + // Remove client_secret before storing (not needed after initial OAuth flow) const safeInfo = Object.fromEntries( Object.entries(clientInformation).filter( @@ -179,6 +194,9 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { saveTokens(tokens: OAuthTokens) { const key = getServerSpecificKey(SESSION_KEYS.TOKENS, this.serverUrl); sessionStorage.setItem(key, JSON.stringify(tokens)); + + // Clear temporary client secret after successful token exchange + this.temporaryClientSecret = undefined; } redirectToAuthorization(authorizationUrl: URL) { @@ -223,6 +241,9 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider { sessionStorage.removeItem( getServerSpecificKey(SESSION_KEYS.CODE_VERIFIER, this.serverUrl), ); + + // Clear temporary client secret + this.temporaryClientSecret = undefined; } }