Merge branch 'main' into update-dependencies
This commit is contained in:
commit
b7b62af282
8 changed files with 75 additions and 27 deletions
|
|
@ -36,7 +36,7 @@ const mockOAuthClientInfo = {
|
|||
// Mock MCP SDK functions - must be before imports
|
||||
jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
|
||||
auth: jest.fn(),
|
||||
discoverOAuthMetadata: jest.fn(),
|
||||
discoverAuthorizationServerMetadata: jest.fn(),
|
||||
registerClient: jest.fn(),
|
||||
startAuthorization: jest.fn(),
|
||||
exchangeAuthorization: jest.fn(),
|
||||
|
|
@ -46,7 +46,7 @@ jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
|
|||
|
||||
// Import the functions to get their types
|
||||
import {
|
||||
discoverOAuthMetadata,
|
||||
discoverAuthorizationServerMetadata,
|
||||
registerClient,
|
||||
startAuthorization,
|
||||
exchangeAuthorization,
|
||||
|
|
@ -57,9 +57,10 @@ import { OAuthMetadata } from "@modelcontextprotocol/sdk/shared/auth.js";
|
|||
import { EMPTY_DEBUGGER_STATE } from "@/lib/auth-types";
|
||||
|
||||
// Type the mocked functions properly
|
||||
const mockDiscoverOAuthMetadata = discoverOAuthMetadata as jest.MockedFunction<
|
||||
typeof discoverOAuthMetadata
|
||||
>;
|
||||
const mockDiscoverAuthorizationServerMetadata =
|
||||
discoverAuthorizationServerMetadata as jest.MockedFunction<
|
||||
typeof discoverAuthorizationServerMetadata
|
||||
>;
|
||||
const mockRegisterClient = registerClient as jest.MockedFunction<
|
||||
typeof registerClient
|
||||
>;
|
||||
|
|
@ -102,7 +103,9 @@ describe("AuthDebugger", () => {
|
|||
// Suppress console errors in tests to avoid JSDOM navigation noise
|
||||
jest.spyOn(console, "error").mockImplementation(() => {});
|
||||
|
||||
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata);
|
||||
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
|
||||
mockOAuthMetadata,
|
||||
);
|
||||
mockRegisterClient.mockResolvedValue(mockOAuthClientInfo);
|
||||
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(
|
||||
new Error("No protected resource metadata found"),
|
||||
|
|
@ -203,7 +206,7 @@ describe("AuthDebugger", () => {
|
|||
});
|
||||
|
||||
// Should first discover and save OAuth metadata
|
||||
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith(
|
||||
expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
|
||||
new URL("https://example.com/"),
|
||||
);
|
||||
|
||||
|
|
@ -216,7 +219,7 @@ describe("AuthDebugger", () => {
|
|||
});
|
||||
|
||||
it("should show error when quick OAuth flow fails to discover metadata", async () => {
|
||||
mockDiscoverOAuthMetadata.mockRejectedValue(
|
||||
mockDiscoverAuthorizationServerMetadata.mockRejectedValue(
|
||||
new Error("Metadata discovery failed"),
|
||||
);
|
||||
|
||||
|
|
@ -362,7 +365,7 @@ describe("AuthDebugger", () => {
|
|||
fireEvent.click(screen.getByText("Continue"));
|
||||
});
|
||||
|
||||
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith(
|
||||
expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
|
||||
new URL("https://example.com/"),
|
||||
);
|
||||
});
|
||||
|
|
@ -509,7 +512,9 @@ describe("AuthDebugger", () => {
|
|||
mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValue(
|
||||
mockResourceMetadata,
|
||||
);
|
||||
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata);
|
||||
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
|
||||
mockOAuthMetadata,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
renderAuthDebugger({
|
||||
|
|
@ -563,7 +568,9 @@ describe("AuthDebugger", () => {
|
|||
// Mock failed metadata discovery
|
||||
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(mockError);
|
||||
// But OAuth metadata should still work with the original URL
|
||||
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata);
|
||||
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
|
||||
mockOAuthMetadata,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
renderAuthDebugger({
|
||||
|
|
@ -603,7 +610,7 @@ describe("AuthDebugger", () => {
|
|||
});
|
||||
|
||||
// Verify that regular OAuth metadata discovery was still called
|
||||
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith(
|
||||
expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
|
||||
new URL("https://example.com/"),
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import {
|
|||
OAuthMetadata,
|
||||
} from "@modelcontextprotocol/sdk/shared/auth.js";
|
||||
import { SESSION_KEYS, getServerSpecificKey } from "./constants";
|
||||
import { generateOAuthState } from "@/utils/oauthUtils";
|
||||
|
||||
export const getClientInformationFromSessionStorage = async ({
|
||||
serverUrl,
|
||||
|
|
@ -86,6 +87,10 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
|
|||
};
|
||||
}
|
||||
|
||||
state(): string | Promise<string> {
|
||||
return generateOAuthState();
|
||||
}
|
||||
|
||||
async clientInformation() {
|
||||
// Try to get the preregistered client information from session storage first
|
||||
const preregisteredClientInformation =
|
||||
|
|
@ -129,6 +134,12 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
|
|||
}
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -351,6 +351,7 @@ export function useConnection({
|
|||
return;
|
||||
}
|
||||
|
||||
let lastRequest = "";
|
||||
try {
|
||||
// Inject auth manually instead of using SSEClientTransport, because we're
|
||||
// proxying through the inspector server first.
|
||||
|
|
@ -564,7 +565,9 @@ export function useConnection({
|
|||
}
|
||||
|
||||
if (capabilities?.logging && defaultLoggingLevel) {
|
||||
lastRequest = "logging/setLevel";
|
||||
await client.setLoggingLevel(defaultLoggingLevel);
|
||||
lastRequest = "";
|
||||
}
|
||||
|
||||
if (onElicitationRequest) {
|
||||
|
|
@ -578,6 +581,17 @@ export function useConnection({
|
|||
setMcpClient(client);
|
||||
setConnectionStatus("connected");
|
||||
} 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);
|
||||
setConnectionStatus("error");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { OAuthStep, AuthDebuggerState } from "./auth-types";
|
||||
import { DebugInspectorOAuthClientProvider } from "./auth";
|
||||
import {
|
||||
discoverOAuthMetadata,
|
||||
discoverAuthorizationServerMetadata,
|
||||
registerClient,
|
||||
startAuthorization,
|
||||
exchangeAuthorization,
|
||||
|
|
@ -12,6 +12,7 @@ import {
|
|||
OAuthMetadataSchema,
|
||||
OAuthProtectedResourceMetadata,
|
||||
} from "@modelcontextprotocol/sdk/shared/auth.js";
|
||||
import { generateOAuthState } from "@/utils/oauthUtils";
|
||||
|
||||
export interface StateMachineContext {
|
||||
state: AuthDebuggerState;
|
||||
|
|
@ -56,7 +57,7 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
|
|||
resourceMetadata ?? undefined,
|
||||
);
|
||||
|
||||
const metadata = await discoverOAuthMetadata(authServerUrl);
|
||||
const metadata = await discoverAuthorizationServerMetadata(authServerUrl);
|
||||
if (!metadata) {
|
||||
throw new Error("Failed to discover OAuth metadata");
|
||||
}
|
||||
|
|
@ -113,13 +114,6 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
|
|||
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(
|
||||
context.serverUrl,
|
||||
{
|
||||
|
|
@ -127,7 +121,7 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
|
|||
clientInformation,
|
||||
redirectUrl: context.provider.redirectUrl,
|
||||
scope,
|
||||
state: state,
|
||||
state: generateOAuthState(),
|
||||
resource: context.state.resource ?? undefined,
|
||||
},
|
||||
);
|
||||
|
|
|
|||
86
client/src/utils/__tests__/oauthUtils.test.ts
Normal file
86
client/src/utils/__tests__/oauthUtils.test.ts
Normal 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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -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 = (
|
||||
params: Extract<CallbackParams, { successful: false }>,
|
||||
): string => {
|
||||
|
|
|
|||
8
package-lock.json
generated
8
package-lock.json
generated
|
|
@ -17,7 +17,7 @@
|
|||
"@modelcontextprotocol/inspector-cli": "^0.16.3",
|
||||
"@modelcontextprotocol/inspector-client": "^0.16.3",
|
||||
"@modelcontextprotocol/inspector-server": "^0.16.3",
|
||||
"@modelcontextprotocol/sdk": "^1.17.0",
|
||||
"@modelcontextprotocol/sdk": "^1.17.2",
|
||||
"concurrently": "^9.2.0",
|
||||
"open": "^10.2.0",
|
||||
"shell-quote": "^1.8.3",
|
||||
|
|
@ -2009,9 +2009,9 @@
|
|||
"link": true
|
||||
},
|
||||
"node_modules/@modelcontextprotocol/sdk": {
|
||||
"version": "1.17.0",
|
||||
"resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.17.0.tgz",
|
||||
"integrity": "sha512-qFfbWFA7r1Sd8D697L7GkTd36yqDuTkvz0KfOGkgXR8EUhQn3/EDNIR/qUdQNMT8IjmasBvHWuXeisxtXTQT2g==",
|
||||
"version": "1.17.2",
|
||||
"resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.17.2.tgz",
|
||||
"integrity": "sha512-EFLRNXR/ixpXQWu6/3Cu30ndDFIFNaqUXcTqsGebujeMan9FzhAaFFswLRiFj61rgygDRr8WO1N+UijjgRxX9g==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"ajv": "^6.12.6",
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@
|
|||
"@modelcontextprotocol/inspector-cli": "^0.16.3",
|
||||
"@modelcontextprotocol/inspector-client": "^0.16.3",
|
||||
"@modelcontextprotocol/inspector-server": "^0.16.3",
|
||||
"@modelcontextprotocol/sdk": "^1.17.0",
|
||||
"@modelcontextprotocol/sdk": "^1.17.2",
|
||||
"concurrently": "^9.2.0",
|
||||
"open": "^10.2.0",
|
||||
"shell-quote": "^1.8.3",
|
||||
|
|
|
|||
Loading…
Reference in a new issue