Merge branch 'main' into dcr-fallback-fix

This commit is contained in:
Ola Hungerford 2025-08-13 05:34:14 -07:00 • committed by GitHub
commit ddd06c207c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 75 additions and 27 deletions

View file

@ -36,7 +36,7 @@ const mockOAuthClientInfo = {
// Mock MCP SDK functions - must be before imports // Mock MCP SDK functions - must be before imports
jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({ jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
auth: jest.fn(), auth: jest.fn(),
discoverOAuthMetadata: jest.fn(), discoverAuthorizationServerMetadata: jest.fn(),
registerClient: jest.fn(), registerClient: jest.fn(),
startAuthorization: jest.fn(), startAuthorization: jest.fn(),
exchangeAuthorization: jest.fn(), exchangeAuthorization: jest.fn(),
@ -46,7 +46,7 @@ jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
// Import the functions to get their types // Import the functions to get their types
import { import {
discoverOAuthMetadata, discoverAuthorizationServerMetadata,
registerClient, registerClient,
startAuthorization, startAuthorization,
exchangeAuthorization, exchangeAuthorization,
@ -57,9 +57,10 @@ import { OAuthMetadata } from "@modelcontextprotocol/sdk/shared/auth.js";
import { EMPTY_DEBUGGER_STATE } from "@/lib/auth-types"; import { EMPTY_DEBUGGER_STATE } from "@/lib/auth-types";
// Type the mocked functions properly // Type the mocked functions properly
const mockDiscoverOAuthMetadata = discoverOAuthMetadata as jest.MockedFunction< const mockDiscoverAuthorizationServerMetadata =
typeof discoverOAuthMetadata discoverAuthorizationServerMetadata as jest.MockedFunction<
>; typeof discoverAuthorizationServerMetadata
>;
const mockRegisterClient = registerClient as jest.MockedFunction< const mockRegisterClient = registerClient as jest.MockedFunction<
typeof registerClient typeof registerClient
>; >;
@ -102,7 +103,9 @@ describe("AuthDebugger", () => {
// Suppress console errors in tests to avoid JSDOM navigation noise // Suppress console errors in tests to avoid JSDOM navigation noise
jest.spyOn(console, "error").mockImplementation(() => {}); jest.spyOn(console, "error").mockImplementation(() => {});
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata); mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
mockOAuthMetadata,
);
mockRegisterClient.mockResolvedValue(mockOAuthClientInfo); mockRegisterClient.mockResolvedValue(mockOAuthClientInfo);
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue( mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(
new Error("No protected resource metadata found"), new Error("No protected resource metadata found"),
@ -203,7 +206,7 @@ describe("AuthDebugger", () => {
}); });
// Should first discover and save OAuth metadata // Should first discover and save OAuth metadata
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith( expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
new URL("https://example.com/"), new URL("https://example.com/"),
); );
@ -216,7 +219,7 @@ describe("AuthDebugger", () => {
}); });
it("should show error when quick OAuth flow fails to discover metadata", async () => { it("should show error when quick OAuth flow fails to discover metadata", async () => {
mockDiscoverOAuthMetadata.mockRejectedValue( mockDiscoverAuthorizationServerMetadata.mockRejectedValue(
new Error("Metadata discovery failed"), new Error("Metadata discovery failed"),
); );
@ -362,7 +365,7 @@ describe("AuthDebugger", () => {
fireEvent.click(screen.getByText("Continue")); fireEvent.click(screen.getByText("Continue"));
}); });
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith( expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
new URL("https://example.com/"), new URL("https://example.com/"),
); );
}); });
@ -606,7 +609,9 @@ describe("AuthDebugger", () => {
mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValue( mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValue(
mockResourceMetadata, mockResourceMetadata,
); );
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata); mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
mockOAuthMetadata,
);
await act(async () => { await act(async () => {
renderAuthDebugger({ renderAuthDebugger({
@ -660,7 +665,9 @@ describe("AuthDebugger", () => {
// Mock failed metadata discovery // Mock failed metadata discovery
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(mockError); mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(mockError);
// But OAuth metadata should still work with the original URL // But OAuth metadata should still work with the original URL
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata); mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
mockOAuthMetadata,
);
await act(async () => { await act(async () => {
renderAuthDebugger({ renderAuthDebugger({
@ -700,7 +707,7 @@ describe("AuthDebugger", () => {
}); });
// Verify that regular OAuth metadata discovery was still called // Verify that regular OAuth metadata discovery was still called
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith( expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
new URL("https://example.com/"), new URL("https://example.com/"),
); );
}); });

View file

@ -8,6 +8,7 @@ import {
OAuthMetadata, OAuthMetadata,
} from "@modelcontextprotocol/sdk/shared/auth.js"; } from "@modelcontextprotocol/sdk/shared/auth.js";
import { SESSION_KEYS, getServerSpecificKey } from "./constants"; import { SESSION_KEYS, getServerSpecificKey } from "./constants";
import { generateOAuthState } from "@/utils/oauthUtils";
export const getClientInformationFromSessionStorage = async ({ export const getClientInformationFromSessionStorage = async ({
serverUrl, serverUrl,
@ -86,6 +87,10 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
}; };
} }
state(): string | Promise<string> {
return generateOAuthState();
}
async clientInformation() { async clientInformation() {
// Try to get the preregistered client information from session storage first // Try to get the preregistered client information from session storage first
const preregisteredClientInformation = const preregisteredClientInformation =
@ -129,6 +134,12 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
} }
redirectToAuthorization(authorizationUrl: URL) { redirectToAuthorization(authorizationUrl: URL) {
if (
authorizationUrl.protocol !== "http:" &&
authorizationUrl.protocol !== "https:"
) {
throw new Error("Authorization URL must be HTTP or HTTPS");
}
window.location.href = authorizationUrl.href; window.location.href = authorizationUrl.href;
} }

View file

@ -351,6 +351,7 @@ export function useConnection({
return; return;
} }
let lastRequest = "";
try { try {
// Inject auth manually instead of using SSEClientTransport, because we're // Inject auth manually instead of using SSEClientTransport, because we're
// proxying through the inspector server first. // proxying through the inspector server first.
@ -564,7 +565,9 @@ export function useConnection({
} }
if (capabilities?.logging && defaultLoggingLevel) { if (capabilities?.logging && defaultLoggingLevel) {
lastRequest = "logging/setLevel";
await client.setLoggingLevel(defaultLoggingLevel); await client.setLoggingLevel(defaultLoggingLevel);
lastRequest = "";
} }
if (onElicitationRequest) { if (onElicitationRequest) {
@ -578,6 +581,17 @@ export function useConnection({
setMcpClient(client); setMcpClient(client);
setConnectionStatus("connected"); setConnectionStatus("connected");
} catch (e) { } catch (e) {
if (
lastRequest === "logging/setLevel" &&
e instanceof McpError &&
e.code === ErrorCode.MethodNotFound
) {
toast({
title: "Error",
description: `Server declares logging capability but doesn't implement method: "${lastRequest}"`,
variant: "destructive",
});
}
console.error(e); console.error(e);
setConnectionStatus("error"); setConnectionStatus("error");
} }

View file

@ -1,7 +1,7 @@
import { OAuthStep, AuthDebuggerState } from "./auth-types"; import { OAuthStep, AuthDebuggerState } from "./auth-types";
import { DebugInspectorOAuthClientProvider } from "./auth"; import { DebugInspectorOAuthClientProvider } from "./auth";
import { import {
discoverOAuthMetadata, discoverAuthorizationServerMetadata,
registerClient, registerClient,
startAuthorization, startAuthorization,
exchangeAuthorization, exchangeAuthorization,
@ -12,6 +12,7 @@ import {
OAuthMetadataSchema, OAuthMetadataSchema,
OAuthProtectedResourceMetadata, OAuthProtectedResourceMetadata,
} from "@modelcontextprotocol/sdk/shared/auth.js"; } from "@modelcontextprotocol/sdk/shared/auth.js";
import { generateOAuthState } from "@/utils/oauthUtils";
export interface StateMachineContext { export interface StateMachineContext {
state: AuthDebuggerState; state: AuthDebuggerState;
@ -56,7 +57,7 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
resourceMetadata ?? undefined, resourceMetadata ?? undefined,
); );
const metadata = await discoverOAuthMetadata(authServerUrl); const metadata = await discoverAuthorizationServerMetadata(authServerUrl);
if (!metadata) { if (!metadata) {
throw new Error("Failed to discover OAuth metadata"); throw new Error("Failed to discover OAuth metadata");
} }
@ -117,13 +118,6 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
scope = metadata.scopes_supported.join(" "); scope = metadata.scopes_supported.join(" ");
} }
// Generate a random state
const array = new Uint8Array(32);
crypto.getRandomValues(array);
const state = Array.from(array, (byte) =>
byte.toString(16).padStart(2, "0"),
).join("");
const { authorizationUrl, codeVerifier } = await startAuthorization( const { authorizationUrl, codeVerifier } = await startAuthorization(
context.serverUrl, context.serverUrl,
{ {
@ -131,7 +125,7 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
clientInformation, clientInformation,
redirectUrl: context.provider.redirectUrl, redirectUrl: context.provider.redirectUrl,
scope, scope,
state: state, state: generateOAuthState(),
resource: context.state.resource ?? undefined, resource: context.state.resource ?? undefined,
}, },
); );

View file

@ -0,0 +1,86 @@
import {
generateOAuthErrorDescription,
parseOAuthCallbackParams,
generateOAuthState,
} from "@/utils/oauthUtils.ts";
describe("parseOAuthCallbackParams", () => {
it("Returns successful: true and code when present", () => {
expect(parseOAuthCallbackParams("?code=fake-code")).toEqual({
successful: true,
code: "fake-code",
});
});
it("Returns successful: false and error when error is present", () => {
expect(parseOAuthCallbackParams("?error=access_denied")).toEqual({
successful: false,
error: "access_denied",
error_description: null,
error_uri: null,
});
});
it("Returns optional error metadata fields when present", () => {
const search =
"?error=access_denied&" +
"error_description=User%20Denied%20Request&" +
"error_uri=https%3A%2F%2Fexample.com%2Ferror-docs";
expect(parseOAuthCallbackParams(search)).toEqual({
successful: false,
error: "access_denied",
error_description: "User Denied Request",
error_uri: "https://example.com/error-docs",
});
});
it("Returns error when nothing present", () => {
expect(parseOAuthCallbackParams("?")).toEqual({
successful: false,
error: "invalid_request",
error_description: "Missing code or error in response",
error_uri: null,
});
});
});
describe("generateOAuthErrorDescription", () => {
it("When only error is present", () => {
expect(
generateOAuthErrorDescription({
successful: false,
error: "invalid_request",
error_description: null,
error_uri: null,
}),
).toBe("Error: invalid_request.");
});
it("When error description is present", () => {
expect(
generateOAuthErrorDescription({
successful: false,
error: "invalid_request",
error_description: "The request could not be completed as dialed",
error_uri: null,
}),
).toEqual(
"Error: invalid_request.\nDetails: The request could not be completed as dialed.",
);
});
it("When all fields present", () => {
expect(
generateOAuthErrorDescription({
successful: false,
error: "invalid_request",
error_description: "The request could not be completed as dialed",
error_uri: "https://example.com/error-docs",
}),
).toEqual(
"Error: invalid_request.\nDetails: The request could not be completed as dialed.\nMore info: https://example.com/error-docs.",
);
});
describe("generateOAuthState", () => {
it("Returns a string", () => {
expect(generateOAuthState()).toBeDefined();
expect(generateOAuthState()).toHaveLength(64);
});
});
});

View file

@ -48,6 +48,20 @@ export const parseOAuthCallbackParams = (location: string): CallbackParams => {
}; };
}; };
/**
* Generate a random state for the OAuth 2.0 flow.
*
* @returns A random state for the OAuth 2.0 flow.
*/
export const generateOAuthState = () => {
// Generate a random state
const array = new Uint8Array(32);
crypto.getRandomValues(array);
return Array.from(array, (byte) => byte.toString(16).padStart(2, "0")).join(
"",
);
};
export const generateOAuthErrorDescription = ( export const generateOAuthErrorDescription = (
params: Extract<CallbackParams, { successful: false }>, params: Extract<CallbackParams, { successful: false }>,
): string => { ): string => {

8
package-lock.json generated
View file

@ -17,7 +17,7 @@
"@modelcontextprotocol/inspector-cli": "^0.16.3", "@modelcontextprotocol/inspector-cli": "^0.16.3",
"@modelcontextprotocol/inspector-client": "^0.16.3", "@modelcontextprotocol/inspector-client": "^0.16.3",
"@modelcontextprotocol/inspector-server": "^0.16.3", "@modelcontextprotocol/inspector-server": "^0.16.3",
"@modelcontextprotocol/sdk": "^1.17.0", "@modelcontextprotocol/sdk": "^1.17.2",
"concurrently": "^9.2.0", "concurrently": "^9.2.0",
"open": "^10.2.0", "open": "^10.2.0",
"shell-quote": "^1.8.3", "shell-quote": "^1.8.3",
@ -1999,9 +1999,9 @@
"link": true "link": true
}, },
"node_modules/@modelcontextprotocol/sdk": { "node_modules/@modelcontextprotocol/sdk": {
"version": "1.17.0", "version": "1.17.2",
"resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.17.0.tgz", "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.17.2.tgz",
"integrity": "sha512-qFfbWFA7r1Sd8D697L7GkTd36yqDuTkvz0KfOGkgXR8EUhQn3/EDNIR/qUdQNMT8IjmasBvHWuXeisxtXTQT2g==", "integrity": "sha512-EFLRNXR/ixpXQWu6/3Cu30ndDFIFNaqUXcTqsGebujeMan9FzhAaFFswLRiFj61rgygDRr8WO1N+UijjgRxX9g==",
"license": "MIT", "license": "MIT",
"dependencies": { "dependencies": {
"ajv": "^6.12.6", "ajv": "^6.12.6",

View file

@ -50,7 +50,7 @@
"@modelcontextprotocol/inspector-cli": "^0.16.3", "@modelcontextprotocol/inspector-cli": "^0.16.3",
"@modelcontextprotocol/inspector-client": "^0.16.3", "@modelcontextprotocol/inspector-client": "^0.16.3",
"@modelcontextprotocol/inspector-server": "^0.16.3", "@modelcontextprotocol/inspector-server": "^0.16.3",
"@modelcontextprotocol/sdk": "^1.17.0", "@modelcontextprotocol/sdk": "^1.17.2",
"concurrently": "^9.2.0", "concurrently": "^9.2.0",
"open": "^10.2.0", "open": "^10.2.0",
"shell-quote": "^1.8.3", "shell-quote": "^1.8.3",