Skip to content

Commit 9871f6d

Browse files
olaservoolaservo
authored
Merge branch 'main' into 7-20-losing-resource-bug
2 parents 05f212a + 16edf53 commit 9871f6d

File tree

9 files changed

+173
-45
lines changed

9 files changed

+173
-45
lines changed

‎cli/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@modelcontextprotocol/inspector-cli",
3-
"version": "0.16.3",
3+
"version": "0.16.4",
44
"description": "CLI for the Model Context Protocol inspector",
55
"license": "MIT",
66
"author": "Anthropic, PBC (https://anthropic.com)",
@@ -21,7 +21,7 @@
2121
},
2222
"devDependencies": {},
2323
"dependencies": {
24-
"@modelcontextprotocol/sdk": "^1.17.0",
24+
"@modelcontextprotocol/sdk": "^1.17.2",
2525
"commander": "^13.1.0",
2626
"spawn-rx": "^5.1.2"
2727
}

‎client/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@modelcontextprotocol/inspector-client",
3-
"version": "0.16.3",
3+
"version": "0.16.4",
44
"description": "Client-side application for the Model Context Protocol inspector",
55
"license": "MIT",
66
"author": "Anthropic, PBC (https://anthropic.com)",
@@ -25,7 +25,7 @@
2525
"cleanup:e2e": "node e2e/global-teardown.js"
2626
},
2727
"dependencies": {
28-
"@modelcontextprotocol/sdk": "^1.17.0",
28+
"@modelcontextprotocol/sdk": "^1.17.2",
2929
"@radix-ui/react-checkbox": "^1.1.4",
3030
"@radix-ui/react-dialog": "^1.1.3",
3131
"@radix-ui/react-icons": "^1.3.0",

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

Lines changed: 116 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
});
@@ -439,6 +442,103 @@ describe("AuthDebugger", () => {
439442
});
440443
});
441444

445+
describe("Client Registration behavior", () => {
446+
it("uses preregistered (static) client information without calling DCR", async () => {
447+
const preregClientInfo = {
448+
client_id: "static_client_id",
449+
client_secret: "static_client_secret",
450+
redirect_uris: ["http://localhost:3000/oauth/callback/debug"],
451+
};
452+
453+
// Return preregistered client info for the server-specific key
454+
sessionStorageMock.getItem.mockImplementation((key) => {
455+
if (
456+
key ===
457+
`[${defaultProps.serverUrl}] ${SESSION_KEYS.PREREGISTERED_CLIENT_INFORMATION}`
458+
) {
459+
return JSON.stringify(preregClientInfo);
460+
}
461+
return null;
462+
});
463+
464+
const updateAuthState = jest.fn();
465+
466+
await act(async () => {
467+
renderAuthDebugger({
468+
updateAuthState,
469+
authState: {
470+
...defaultAuthState,
471+
isInitiatingAuth: false,
472+
oauthStep: "client_registration",
473+
oauthMetadata: mockOAuthMetadata as unknown as OAuthMetadata,
474+
},
475+
});
476+
});
477+
478+
// Proceed from client_registration → authorization_redirect
479+
await act(async () => {
480+
fireEvent.click(screen.getByText("Continue"));
481+
});
482+
483+
// Should NOT attempt dynamic client registration
484+
expect(mockRegisterClient).not.toHaveBeenCalled();
485+
486+
// Should advance with the preregistered client info
487+
expect(updateAuthState).toHaveBeenCalledWith(
488+
expect.objectContaining({
489+
oauthClientInfo: expect.objectContaining({
490+
client_id: "static_client_id",
491+
}),
492+
oauthStep: "authorization_redirect",
493+
}),
494+
);
495+
});
496+
497+
it("falls back to DCR when no static client information is available", async () => {
498+
// No preregistered or dynamic client info present in session storage
499+
sessionStorageMock.getItem.mockImplementation(() => null);
500+
501+
// DCR returns a new client
502+
mockRegisterClient.mockResolvedValueOnce(mockOAuthClientInfo);
503+
504+
const updateAuthState = jest.fn();
505+
506+
await act(async () => {
507+
renderAuthDebugger({
508+
updateAuthState,
509+
authState: {
510+
...defaultAuthState,
511+
isInitiatingAuth: false,
512+
oauthStep: "client_registration",
513+
oauthMetadata: mockOAuthMetadata as unknown as OAuthMetadata,
514+
},
515+
});
516+
});
517+
518+
await act(async () => {
519+
fireEvent.click(screen.getByText("Continue"));
520+
});
521+
522+
expect(mockRegisterClient).toHaveBeenCalledTimes(1);
523+
524+
// Should save and advance with the DCR client info
525+
expect(updateAuthState).toHaveBeenCalledWith(
526+
expect.objectContaining({
527+
oauthClientInfo: expect.objectContaining({
528+
client_id: "test_client_id",
529+
}),
530+
oauthStep: "authorization_redirect",
531+
}),
532+
);
533+
534+
// Verify the dynamically registered client info was persisted
535+
expect(sessionStorage.setItem).toHaveBeenCalledWith(
536+
`[${defaultProps.serverUrl}] ${SESSION_KEYS.CLIENT_INFORMATION}`,
537+
expect.any(String),
538+
);
539+
});
540+
});
541+
442542
describe("OAuth State Persistence", () => {
443543
it("should store auth state to sessionStorage before redirect in Quick OAuth Flow", async () => {
444544
const updateAuthState = jest.fn();
@@ -509,7 +609,9 @@ describe("AuthDebugger", () => {
509609
mockDiscoverOAuthProtectedResourceMetadata.mockResolvedValue(
510610
mockResourceMetadata,
511611
);
512-
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata);
612+
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
613+
mockOAuthMetadata,
614+
);
513615

514616
await act(async () => {
515617
renderAuthDebugger({
@@ -563,7 +665,9 @@ describe("AuthDebugger", () => {
563665
// Mock failed metadata discovery
564666
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(mockError);
565667
// But OAuth metadata should still work with the original URL
566-
mockDiscoverOAuthMetadata.mockResolvedValue(mockOAuthMetadata);
668+
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
669+
mockOAuthMetadata,
670+
);
567671

568672
await act(async () => {
569673
renderAuthDebugger({
@@ -603,7 +707,7 @@ describe("AuthDebugger", () => {
603707
});
604708

605709
// Verify that regular OAuth metadata discovery was still called
606-
expect(mockDiscoverOAuthMetadata).toHaveBeenCalledWith(
710+
expect(mockDiscoverAuthorizationServerMetadata).toHaveBeenCalledWith(
607711
new URL("https://example.com/"),
608712
);
609713
});

‎client/src/lib/auth.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ export class InspectorOAuthClientProvider implements OAuthClientProvider {
134134
}
135135

136136
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+
}
137143
window.location.href = authorizationUrl.href;
138144
}
139145

‎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: 11 additions & 7 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,
@@ -62,7 +62,7 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
6262
context.provider.saveResource(resource);
6363
}
6464

65-
const metadata = await discoverOAuthMetadata(authServerUrl);
65+
const metadata = await discoverAuthorizationServerMetadata(authServerUrl);
6666
if (!metadata) {
6767
throw new Error("Failed to discover OAuth metadata");
6868
}
@@ -94,12 +94,16 @@ export const oauthTransitions: Record<OAuthStep, StateTransition> = {
9494
clientMetadata.scope = scopesSupported.join(" ");
9595
}
9696

97-
const fullInformation = await registerClient(context.serverUrl, {
98-
metadata,
99-
clientMetadata,
100-
});
97+
// Try Static client first, with DCR as fallback
98+
let fullInformation = await context.provider.clientInformation();
99+
if (!fullInformation) {
100+
fullInformation = await registerClient(context.serverUrl, {
101+
metadata,
102+
clientMetadata,
103+
});
104+
context.provider.saveClientInformation(fullInformation);
105+
}
101106

102-
context.provider.saveClientInformation(fullInformation);
103107
context.updateState({
104108
oauthClientInfo: fullInformation,
105109
oauthStep: "authorization_redirect",

0 commit comments

Comments
 (0)