Skip to content
Merged
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
86 changes: 82 additions & 4 deletions client/src/components/__tests__/AuthDebugger.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const mockOAuthMetadata = {
token_endpoint: "https://oauth.example.com/token",
response_types_supported: ["code"],
grant_types_supported: ["authorization_code"],
scopes_supported: ["read", "write"],
};

const mockOAuthClientInfo = {
Expand Down Expand Up @@ -56,6 +57,57 @@ import {
import { OAuthMetadata } from "@modelcontextprotocol/sdk/shared/auth.js";
import { EMPTY_DEBUGGER_STATE } from "@/lib/auth-types";

// Mock local auth module
jest.mock("@/lib/auth", () => ({
DebugInspectorOAuthClientProvider: jest.fn().mockImplementation(() => ({
tokens: jest.fn().mockImplementation(() => Promise.resolve(undefined)),
clear: jest.fn().mockImplementation(() => {
// Mock the real clear() behavior which removes items from sessionStorage
sessionStorage.removeItem("[https://example.com/mcp] mcp_tokens");
sessionStorage.removeItem("[https://example.com/mcp] mcp_client_info");
sessionStorage.removeItem(
"[https://example.com/mcp] mcp_server_metadata",
);
}),
redirectUrl: "http://localhost:3000/oauth/callback/debug",
clientMetadata: {
redirect_uris: ["http://localhost:3000/oauth/callback/debug"],
token_endpoint_auth_method: "none",
grant_types: ["authorization_code", "refresh_token"],
response_types: ["code"],
client_name: "MCP Inspector",
},
clientInformation: jest.fn().mockImplementation(async () => {
const serverUrl = "https://example.com/mcp";
const preregisteredKey = `[${serverUrl}] ${SESSION_KEYS.PREREGISTERED_CLIENT_INFORMATION}`;
const preregisteredData = sessionStorage.getItem(preregisteredKey);
if (preregisteredData) {
return JSON.parse(preregisteredData);
}
const dynamicKey = `[${serverUrl}] ${SESSION_KEYS.CLIENT_INFORMATION}`;
const dynamicData = sessionStorage.getItem(dynamicKey);
if (dynamicData) {
return JSON.parse(dynamicData);
}
return undefined;
}),
saveClientInformation: jest.fn().mockImplementation((clientInfo) => {
const serverUrl = "https://example.com/mcp";
const key = `[${serverUrl}] ${SESSION_KEYS.CLIENT_INFORMATION}`;
sessionStorage.setItem(key, JSON.stringify(clientInfo));
}),
Comment on lines +80 to +98
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the OAuth scope discovery commits, 2 AuthDebugger tests were failing because the test mocks were too simple. The clientInformation mock always returned undefined instead of checking sessionStorage for preregistered client info, and the saveClientInformation mock did nothing instead of actually saving data. The OAuth flow expected these mocks to behave like the real OAuth client provider by checking sessionStorage for existing client information and saving dynamic registration results when needed. Once we fixed the mocks to simulate this real behavior, the tests started passing again.

saveTokens: jest.fn(),
redirectToAuthorization: jest.fn(),
saveCodeVerifier: jest.fn(),
codeVerifier: jest.fn(),
saveServerMetadata: jest.fn(),
getServerMetadata: jest.fn(),
})),
discoverScopes: jest.fn().mockResolvedValue("read write" as never),
}));

import { discoverScopes } from "@/lib/auth";

// Type the mocked functions properly
const mockDiscoverAuthorizationServerMetadata =
discoverAuthorizationServerMetadata as jest.MockedFunction<
Expand All @@ -75,6 +127,9 @@ const mockDiscoverOAuthProtectedResourceMetadata =
discoverOAuthProtectedResourceMetadata as jest.MockedFunction<
typeof discoverOAuthProtectedResourceMetadata
>;
const mockDiscoverScopes = discoverScopes as jest.MockedFunction<
typeof discoverScopes
>;

const sessionStorageMock = {
getItem: jest.fn(),
Expand Down Expand Up @@ -103,9 +158,15 @@ describe("AuthDebugger", () => {
// Suppress console errors in tests to avoid JSDOM navigation noise
jest.spyOn(console, "error").mockImplementation(() => {});

mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
mockOAuthMetadata,
);
// Set default mock behaviors with complete OAuth metadata
mockDiscoverAuthorizationServerMetadata.mockResolvedValue({
issuer: "https://oauth.example.com",
authorization_endpoint: "https://oauth.example.com/authorize",
token_endpoint: "https://oauth.example.com/token",
response_types_supported: ["code"],
grant_types_supported: ["authorization_code"],
scopes_supported: ["read", "write"],
});
mockRegisterClient.mockResolvedValue(mockOAuthClientInfo);
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(
new Error("No protected resource metadata found"),
Expand Down Expand Up @@ -427,7 +488,24 @@ describe("AuthDebugger", () => {
});
});

it("should not include scope in authorization URL when scopes_supported is not present", async () => {
it("should include scope in authorization URL when scopes_supported is not present", async () => {
const updateAuthState =
await setupAuthorizationUrlTest(mockOAuthMetadata);

// Wait for the updateAuthState to be called
await waitFor(() => {
expect(updateAuthState).toHaveBeenCalledWith(
expect.objectContaining({
authorizationUrl: expect.stringContaining("scope="),
}),
);
});
});

it("should omit scope from authorization URL when discoverScopes returns undefined", async () => {
// Mock discoverScopes to return undefined (no scopes available)
mockDiscoverScopes.mockResolvedValueOnce(undefined);

const updateAuthState =
await setupAuthorizationUrlTest(mockOAuthMetadata);

Expand Down
155 changes: 155 additions & 0 deletions client/src/lib/__tests__/auth.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
import { discoverScopes } from "../auth";
import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js";

jest.mock("@modelcontextprotocol/sdk/client/auth.js", () => ({
discoverAuthorizationServerMetadata: jest.fn(),
}));

const mockDiscoverAuth =
discoverAuthorizationServerMetadata as jest.MockedFunction<
typeof discoverAuthorizationServerMetadata
>;

const baseMetadata = {
issuer: "https://test.com",
authorization_endpoint: "https://test.com/authorize",
token_endpoint: "https://test.com/token",
response_types_supported: ["code"],
grant_types_supported: ["authorization_code"],
scopes_supported: ["read", "write"],
};

describe("discoverScopes", () => {
beforeEach(() => {
jest.clearAllMocks();
});

const testCases = [
{
name: "returns joined scopes from OAuth metadata",
mockResolves: baseMetadata,
serverUrl: "https://example.com",
expected: "read write",
expectedCallUrl: "https://example.com/",
},
{
name: "prefers resource metadata over OAuth metadata",
mockResolves: baseMetadata,
serverUrl: "https://example.com",
resourceMetadata: {
resource: "https://example.com",
scopes_supported: ["admin", "full"],
},
expected: "admin full",
},
{
name: "falls back to OAuth when resource has empty scopes",
mockResolves: baseMetadata,
serverUrl: "https://example.com",
resourceMetadata: {
resource: "https://example.com",
scopes_supported: [],
},
expected: "read write",
},
{
name: "normalizes URL with port and path",
mockResolves: baseMetadata,
serverUrl: "https://example.com:8080/some/path",
expected: "read write",
expectedCallUrl: "https://example.com:8080/",
},
{
name: "normalizes URL with trailing slash",
mockResolves: baseMetadata,
serverUrl: "https://example.com/",
expected: "read write",
expectedCallUrl: "https://example.com/",
},
{
name: "handles single scope",
mockResolves: { ...baseMetadata, scopes_supported: ["admin"] },
serverUrl: "https://example.com",
expected: "admin",
},
{
name: "prefers resource metadata even with fewer scopes",
mockResolves: {
...baseMetadata,
scopes_supported: ["read", "write", "admin", "full"],
},
serverUrl: "https://example.com",
resourceMetadata: {
resource: "https://example.com",
scopes_supported: ["read"],
},
expected: "read",
},
];

const undefinedCases = [
{
name: "returns undefined when OAuth discovery fails",
mockRejects: new Error("Discovery failed"),
serverUrl: "https://example.com",
},
{
name: "returns undefined when OAuth has no scopes",
mockResolves: { ...baseMetadata, scopes_supported: [] },
serverUrl: "https://example.com",
},
{
name: "returns undefined when scopes_supported missing",
mockResolves: (() => {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { scopes_supported, ...rest } = baseMetadata;
return rest;
})(),
serverUrl: "https://example.com",
},
{
name: "returns undefined with resource metadata but OAuth fails",
mockRejects: new Error("No OAuth metadata"),
serverUrl: "https://example.com",
resourceMetadata: {
resource: "https://example.com",
scopes_supported: ["read", "write"],
},
},
];

test.each(testCases)(
"$name",
async ({
mockResolves,
serverUrl,
resourceMetadata,
expected,
expectedCallUrl,
}) => {
mockDiscoverAuth.mockResolvedValue(mockResolves);

const result = await discoverScopes(serverUrl, resourceMetadata);

expect(result).toBe(expected);
if (expectedCallUrl) {
expect(mockDiscoverAuth).toHaveBeenCalledWith(new URL(expectedCallUrl));
}
},
);

test.each(undefinedCases)(
"$name",
async ({ mockResolves, mockRejects, serverUrl, resourceMetadata }) => {
if (mockRejects) {
mockDiscoverAuth.mockRejectedValue(mockRejects);
} else {
mockDiscoverAuth.mockResolvedValue(mockResolves);
}

const result = await discoverScopes(serverUrl, resourceMetadata);

expect(result).toBeUndefined();
},
);
});
35 changes: 35 additions & 0 deletions client/src/lib/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,45 @@ import {
OAuthTokensSchema,
OAuthClientMetadata,
OAuthMetadata,
OAuthProtectedResourceMetadata,
} from "@modelcontextprotocol/sdk/shared/auth.js";
import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js";
import { SESSION_KEYS, getServerSpecificKey } from "./constants";
import { generateOAuthState } from "@/utils/oauthUtils";

/**
* Discovers OAuth scopes from server metadata, with preference for resource metadata scopes
* @param serverUrl - The MCP server URL
* @param resourceMetadata - Optional resource metadata containing preferred scopes
* @returns Promise resolving to space-separated scope string or undefined
*/
export const discoverScopes = async (
serverUrl: string,
resourceMetadata?: OAuthProtectedResourceMetadata,
): Promise<string | undefined> => {
try {
const metadata = await discoverAuthorizationServerMetadata(
new URL("/", serverUrl),
);

// Prefer resource metadata scopes, but fall back to OAuth metadata if empty
const resourceScopes = resourceMetadata?.scopes_supported;
const oauthScopes = metadata?.scopes_supported;

const scopesSupported =
resourceScopes && resourceScopes.length > 0
? resourceScopes
: oauthScopes;

return scopesSupported && scopesSupported.length > 0
? scopesSupported.join(" ")
: undefined;
} catch (error) {
console.debug("OAuth scope discovery failed:", error);
return undefined;
}
};

export const getClientInformationFromSessionStorage = async ({
serverUrl,
isPreregistered,
Expand Down
Loading