Skip to content

Commit ddd06c2

Browse files
authored
Merge branch 'main' into dcr-fallback-fix
2 parents e9f1da3 + 2feac76 commit ddd06c2

File tree

8 files changed

+75
-27
lines changed

8 files changed

+75
-27
lines changed

client/src/components/__tests__/AuthDebugger.test.tsx

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const mockOAuthClientInfo = {
3636
// Mock MCP SDK functions - must be before imports
3737
jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
3838
auth: jest.fn(),
39-
discoverOAuthMetadata: jest.fn(),
39+
discoverAuthorizationServerMetadata: jest.fn(),
4040
registerClient: jest.fn(),
4141
startAuthorization: jest.fn(),
4242
exchangeAuthorization: jest.fn(),
@@ -46,7 +46,7 @@ jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
4646

4747
// Import the functions to get their types
4848
import {
49-
discoverOAuthMetadata,
49+
discoverAuthorizationServerMetadata,
5050
registerClient,
5151
startAuthorization,
5252
exchangeAuthorization,
@@ -57,9 +57,10 @@ import { OAuthMetadata } from "@modelcontextprotocol/sdk/shared/auth.js";
5757
import { EMPTY_DEBUGGER_STATE } from "@/lib/auth-types";
5858

5959
// Type the mocked functions properly
60-
const mockDiscoverOAuthMetadata = discoverOAuthMetadata as jest.MockedFunction<
61-
typeof discoverOAuthMetadata
62-
>;
60+
const mockDiscoverAuthorizationServerMetadata =
61+
discoverAuthorizationServerMetadata as jest.MockedFunction<
62+
typeof discoverAuthorizationServerMetadata
63+
>;
6364
const mockRegisterClient = registerClient as jest.MockedFunction<
6465
typeof registerClient
6566
>;
@@ -102,7 +103,9 @@ describe("AuthDebugger", () => {
102103
// Suppress console errors in tests to avoid JSDOM navigation noise
103104
jest.spyOn(console, "error").mockImplementation(() => {});
104105

105-
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata);
106+
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
107+
mockOAuthMetadata,
108+
);
106109
mockRegisterClient.mockResolvedValue(mockOAuthClientInfo);
107110
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(
108111
new Error("No protected resource metadata found"),
@@ -203,7 +206,7 @@ describe("AuthDebugger", () => {
203206
});
204207

205208
// Should first discover and save OAuth metadata
206-
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith(
209+
expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
207210
new URL("https://example.com/"),
208211
);
209212

@@ -216,7 +219,7 @@ describe("AuthDebugger", () => {
216219
});
217220

218221
it("should show error when quick OAuth flow fails to discover metadata", async () => {
219-
mockDiscoverOAuthMetadata.mockRejectedValue(
222+
mockDiscoverAuthorizationServerMetadata.mockRejectedValue(
220223
new Error("Metadata discovery failed"),
221224
);
222225

@@ -362,7 +365,7 @@ describe("AuthDebugger", () => {
362365
fireEvent.click(screen.getByText("Continue"));
363366
});
364367

365-
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith(
368+
expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
366369
new URL("https://example.com/"),
367370
);
368371
});
@@ -606,7 +609,9 @@ describe("AuthDebugger", () => {
606609
mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValue(
607610
mockResourceMetadata,
608611
);
609-
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata);
612+
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
613+
mockOAuthMetadata,
614+
);
610615

611616
await act(async () => {
612617
renderAuthDebugger({
@@ -660,7 +665,9 @@ describe("AuthDebugger", () => {
660665
// Mock failed metadata discovery
661666
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(mockError);
662667
// But OAuth metadata should still work with the original URL
663-
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata);
668+
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
669+
mockOAuthMetadata,
670+
);
664671

665672
await act(async () => {
666673
renderAuthDebugger({
@@ -700,7 +707,7 @@ describe("AuthDebugger", () => {
700707
});
701708

702709
// Verify that regular OAuth metadata discovery was still called
703-
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith(
710+
expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
704711
new URL("https://example.com/"),
705712
);
706713
});

client/src/lib/auth.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
OAuthMetadata,
99
} from "@modelcontextprotocol/sdk/shared/auth.js";
1010
import { SESSION_KEYS, getServerSpecificKey } from "./constants";
11+
import { generateOAuthState } from "@/utils/oauthUtils";
1112

1213
export const getClientInformationFromSessionStorage = async ({
1314
serverUrl,
@@ -86,6 +87,10 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
8687
};
8788
}
8889

90+
state(): string | Promise<string> {
91+
return generateOAuthState();
92+
}
93+
8994
async clientInformation() {
9095
// Try to get the preregistered client information from session storage first
9196
const preregisteredClientInformation =
@@ -129,6 +134,12 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
129134
}
130135

131136
redirectToAuthorization(authorizationUrl: URL) {
137+
if (
138+
authorizationUrl.protocol !== "http:" &&
139+
authorizationUrl.protocol !== "https:"
140+
) {
141+
throw new Error("Authorization URL must be HTTP or HTTPS");
142+
}
132143
window.location.href = authorizationUrl.href;
133144
}
134145

client/src/lib/hooks/useConnection.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ export function useConnection({
351351
return;
352352
}
353353

354+
let lastRequest = "";
354355
try {
355356
// Inject auth manually instead of using SSEClientTransport, because we're
356357
// proxying through the inspector server first.
@@ -564,7 +565,9 @@ export function useConnection({
564565
}
565566

566567
if (capabilities?.logging && defaultLoggingLevel) {
568+
lastRequest = "logging/setLevel";
567569
await client.setLoggingLevel(defaultLoggingLevel);
570+
lastRequest = "";
568571
}
569572

570573
if (onElicitationRequest) {
@@ -578,6 +581,17 @@ export function useConnection({
578581
setMcpClient(client);
579582
setConnectionStatus("connected");
580583
} catch (e) {
584+
if (
585+
lastRequest === "logging/setLevel" &&
586+
e instanceof McpError &&
587+
e.code === ErrorCode.MethodNotFound
588+
) {
589+
toast({
590+
title: "Error",
591+
description: `Server declares logging capability but doesn't implement method: "${lastRequest}"`,
592+
variant: "destructive",
593+
});
594+
}
581595
console.error(e);
582596
setConnectionStatus("error");
583597
}

client/src/lib/oauth-state-machine.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { OAuthStep, AuthDebuggerState } from "./auth-types";
22
import { DebugInspectorOAuthClientProvider } from "./auth";
33
import {
4-
discoverOAuthMetadata,
4+
discoverAuthorizationServerMetadata,
55
registerClient,
66
startAuthorization,
77
exchangeAuthorization,
@@ -12,6 +12,7 @@ import {
1212
OAuthMetadataSchema,
1313
OAuthProtectedResourceMetadata,
1414
} from "@modelcontextprotocol/sdk/shared/auth.js";
15+
import { generateOAuthState } from "@/utils/oauthUtils";
1516

1617
export interface StateMachineContext {
1718
state: AuthDebuggerState;
@@ -56,7 +57,7 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
5657
resourceMetadata ?? undefined,
5758
);
5859

59-
const metadata = await discoverOAuthMetadata(authServerUrl);
60+
const metadata = await discoverAuthorizationServerMetadata(authServerUrl);
6061
if (!metadata) {
6162
throw new Error("Failed to discover OAuth metadata");
6263
}
@@ -117,21 +118,14 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
117118
scope = metadata.scopes_supported.join(" ");
118119
}
119120

120-
// Generate a random state
121-
const array = new Uint8Array(32);
122-
crypto.getRandomValues(array);
123-
const state = Array.from(array, (byte) =>
124-
byte.toString(16).padStart(2, "0"),
125-
).join("");
126-
127121
const { authorizationUrl, codeVerifier } = await startAuthorization(
128122
context.serverUrl,
129123
{
130124
metadata,
131125
clientInformation,
132126
redirectUrl: context.provider.redirectUrl,
133127
scope,
134-
state: state,
128+
state: generateOAuthState(),
135129
resource: context.state.resource ?? undefined,
136130
},
137131
);

client/src/utils/__tests__/oauthUtils.ts renamed to client/src/utils/__tests__/oauthUtils.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {
22
generateOAuthErrorDescription,
33
parseOAuthCallbackParams,
4+
generateOAuthState,
45
} from "@/utils/oauthUtils.ts";
56

67
describe("parseOAuthCallbackParams", () => {
@@ -75,4 +76,11 @@ describe("generateOAuthErrorDescription", () => {
7576
"Error: invalid_request.\nDetails: The request could not be completed as dialed.\nMore info: https://example.com/error-docs.",
7677
);
7778
});
79+
80+
describe("generateOAuthState", () => {
81+
it("Returns a string", () => {
82+
expect(generateOAuthState()).toBeDefined();
83+
expect(generateOAuthState()).toHaveLength(64);
84+
});
85+
});
7886
});

client/src/utils/oauthUtils.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,20 @@ export const parseOAuthCallbackParams = (location: string): CallbackParams => {
4848
};
4949
};
5050

51+
/**
52+
* Generate a random state for the OAuth 2.0 flow.
53+
*
54+
* @returns A random state for the OAuth 2.0 flow.
55+
*/
56+
export const generateOAuthState = () => {
57+
// Generate a random state
58+
const array = new Uint8Array(32);
59+
crypto.getRandomValues(array);
60+
return Array.from(array, (byte) => byte.toString(16).padStart(2, "0")).join(
61+
"",
62+
);
63+
};
64+
5165
export const generateOAuthErrorDescription = (
5266
params: Extract<CallbackParams, { successful: false }>,
5367
): string => {

package-lock.json

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
"@modelcontextprotocol/inspector-cli": "^0.16.3",
5151
"@modelcontextprotocol/inspector-client": "^0.16.3",
5252
"@modelcontextprotocol/inspector-server": "^0.16.3",
53-
"@modelcontextprotocol/sdk": "^1.17.0",
53+
"@modelcontextprotocol/sdk": "^1.17.2",
5454
"concurrently": "^9.2.0",
5555
"open": "^10.2.0",
5656
"shell-quote": "^1.8.3",

0 commit comments

Comments
 (0)