Skip to content

Revert "feat: unify OAuth scope discovery between automatic and manual flows" #717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Revert "feat: unify OAuth scope discovery between automatic and manual flows" #717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 4 additions & 82 deletions client/src/components/__tests__/AuthDebugger.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ 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 @@ -57,57 +56,6 @@ 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));
}),
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 @@ -127,9 +75,6 @@ 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 @@ -158,15 +103,9 @@ describe("AuthDebugger", () => {
// Suppress console errors in tests to avoid JSDOM navigation noise
jest.spyOn(console, "error").mockImplementation(() => {});

// 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"],
});
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
mockOAuthMetadata,
);
mockRegisterClient.mockResolvedValue(mockOAuthClientInfo);
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(
new Error("No protected resource metadata found"),
Expand Down Expand Up @@ -488,24 +427,7 @@ describe("AuthDebugger", () => {
});
});

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);

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

Expand Down
155 changes: 0 additions & 155 deletions client/src/lib/__tests__/auth.test.ts
View file
Open in desktop

This file was deleted.

35 changes: 0 additions & 35 deletions client/src/lib/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,45 +6,10 @@ 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