Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 105 additions & 1 deletion client/src/lib/__tests__/auth.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => ({
Expand Down Expand Up @@ -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();
});
});
});
27 changes: 24 additions & 3 deletions client/src/lib/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
}

Expand Down