Skip to content

Commit f8feb1c

Browse files
authored
Revert "feat: unify OAuth scope discovery between automatic and manual flows"
1 parent 24ef319 commit f8feb1c

File tree

6 files changed

+20
-469
lines changed

6 files changed

+20
-469
lines changed

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

Lines changed: 4 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ const mockOAuthMetadata = {
2525
token_endpoint: "https://oauth.example.com/token",
2626
response_types_supported: ["code"],
2727
grant_types_supported: ["authorization_code"],
28-
scopes_supported: ["read", "write"],
2928
};
3029

3130
const mockOAuthClientInfo = {
@@ -57,57 +56,6 @@ import {
5756
import { OAuthMetadata } from "@modelcontextprotocol/sdk/shared/auth.js";
5857
import { EMPTY_DEBUGGER_STATE } from "@/lib/auth-types";
5958

60-
// Mock local auth module
61-
jest.mock("@/lib/auth", () => ({
62-
DebugInspectorOAuthClientProvider: jest.fn().mockImplementation(() => ({
63-
tokens: jest.fn().mockImplementation(() => Promise.resolve(undefined)),
64-
clear: jest.fn().mockImplementation(() => {
65-
// Mock the real clear() behavior which removes items from sessionStorage
66-
sessionStorage.removeItem("[https://example.com/mcp] mcp_tokens");
67-
sessionStorage.removeItem("[https://example.com/mcp] mcp_client_info");
68-
sessionStorage.removeItem(
69-
"[https://example.com/mcp] mcp_server_metadata",
70-
);
71-
}),
72-
redirectUrl: "http://localhost:3000/oauth/callback/debug",
73-
clientMetadata: {
74-
redirect_uris: ["http://localhost:3000/oauth/callback/debug"],
75-
token_endpoint_auth_method: "none",
76-
grant_types: ["authorization_code", "refresh_token"],
77-
response_types: ["code"],
78-
client_name: "MCP Inspector",
79-
},
80-
clientInformation: jest.fn().mockImplementation(async () => {
81-
const serverUrl = "https://example.com/mcp";
82-
const preregisteredKey = `[${serverUrl}] ${SESSION_KEYS.PREREGISTERED_CLIENT_INFORMATION}`;
83-
const preregisteredData = sessionStorage.getItem(preregisteredKey);
84-
if (preregisteredData) {
85-
return JSON.parse(preregisteredData);
86-
}
87-
const dynamicKey = `[${serverUrl}] ${SESSION_KEYS.CLIENT_INFORMATION}`;
88-
const dynamicData = sessionStorage.getItem(dynamicKey);
89-
if (dynamicData) {
90-
return JSON.parse(dynamicData);
91-
}
92-
return undefined;
93-
}),
94-
saveClientInformation: jest.fn().mockImplementation((clientInfo) => {
95-
const serverUrl = "https://example.com/mcp";
96-
const key = `[${serverUrl}] ${SESSION_KEYS.CLIENT_INFORMATION}`;
97-
sessionStorage.setItem(key, JSON.stringify(clientInfo));
98-
}),
99-
saveTokens: jest.fn(),
100-
redirectToAuthorization: jest.fn(),
101-
saveCodeVerifier: jest.fn(),
102-
codeVerifier: jest.fn(),
103-
saveServerMetadata: jest.fn(),
104-
getServerMetadata: jest.fn(),
105-
})),
106-
discoverScopes: jest.fn().mockResolvedValue("read write" as never),
107-
}));
108-
109-
import { discoverScopes } from "@/lib/auth";
110-
11159
// Type the mocked functions properly
11260
const mockDiscoverAuthorizationServerMetadata =
11361
discoverAuthorizationServerMetadata as jest.MockedFunction<
@@ -127,9 +75,6 @@ const mockDiscoverOAuthProtectedResourceMetadata =
12775
discoverOAuthProtectedResourceMetadata as jest.MockedFunction<
12876
typeof discoverOAuthProtectedResourceMetadata
12977
>;
130-
const mockDiscoverScopes = discoverScopes as jest.MockedFunction<
131-
typeof discoverScopes
132-
>;
13378

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

161-
// Set default mock behaviors with complete OAuth metadata
162-
mockDiscoverAuthorizationServerMetadata.mockResolvedValue({
163-
issuer: "https://oauth.example.com",
164-
authorization_endpoint: "https://oauth.example.com/authorize",
165-
token_endpoint: "https://oauth.example.com/token",
166-
response_types_supported: ["code"],
167-
grant_types_supported: ["authorization_code"],
168-
scopes_supported: ["read", "write"],
169-
});
106+
mockDiscoverAuthorizationServerMetadata.mockResolvedValue(
107+
mockOAuthMetadata,
108+
);
170109
mockRegisterClient.mockResolvedValue(mockOAuthClientInfo);
171110
mockDiscoverOAuthProtectedResourceMetadata.mockRejectedValue(
172111
new Error("No protected resource metadata found"),
@@ -488,24 +427,7 @@ describe("AuthDebugger", () => {
488427
});
489428
});
490429

491-
it("should include scope in authorization URL when scopes_supported is not present", async () => {
492-
const updateAuthState =
493-
await setupAuthorizationUrlTest(mockOAuthMetadata);
494-
495-
// Wait for the updateAuthState to be called
496-
await waitFor(() => {
497-
expect(updateAuthState).toHaveBeenCalledWith(
498-
expect.objectContaining({
499-
authorizationUrl: expect.stringContaining("scope="),
500-
}),
501-
);
502-
});
503-
});
504-
505-
it("should omit scope from authorization URL when discoverScopes returns undefined", async () => {
506-
// Mock discoverScopes to return undefined (no scopes available)
507-
mockDiscoverScopes.mockResolvedValueOnce(undefined);
508-
430+
it("should not include scope in authorization URL when scopes_supported is not present", async () => {
509431
const updateAuthState =
510432
await setupAuthorizationUrlTest(mockOAuthMetadata);
511433

client/src/lib/__tests__/auth.test.ts

Lines changed: 0 additions & 155 deletions
This file was deleted.

client/src/lib/auth.ts

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,45 +6,10 @@ import {
66
OAuthTokensSchema,
77
OAuthClientMetadata,
88
OAuthMetadata,
9-
OAuthProtectedResourceMetadata,
109
} from "@modelcontextprotocol/sdk/shared/auth.js";
11-
import { discoverAuthorizationServerMetadata } from "@modelcontextprotocol/sdk/client/auth.js";
1210
import { SESSION_KEYS, getServerSpecificKey } from "./constants";
1311
import { generateOAuthState } from "@/utils/oauthUtils";
1412

15-
/**
16-
* Discovers OAuth scopes from server metadata, with preference for resource metadata scopes
17-
* @param serverUrl - The MCP server URL
18-
* @param resourceMetadata - Optional resource metadata containing preferred scopes
19-
* @returns Promise resolving to space-separated scope string or undefined
20-
*/
21-
export const discoverScopes = async (
22-
serverUrl: string,
23-
resourceMetadata?: OAuthProtectedResourceMetadata,
24-
): Promise<string | undefined> => {
25-
try {
26-
const metadata = await discoverAuthorizationServerMetadata(
27-
new URL("/", serverUrl),
28-
);
29-
30-
// Prefer resource metadata scopes, but fall back to OAuth metadata if empty
31-
const resourceScopes = resourceMetadata?.scopes_supported;
32-
const oauthScopes = metadata?.scopes_supported;
33-
34-
const scopesSupported =
35-
resourceScopes && resourceScopes.length > 0
36-
? resourceScopes
37-
: oauthScopes;
38-
39-
return scopesSupported && scopesSupported.length > 0
40-
? scopesSupported.join(" ")
41-
: undefined;
42-
} catch (error) {
43-
console.debug("OAuth scope discovery failed:", error);
44-
return undefined;
45-
}
46-
};
47-
4813
export const getClientInformationFromSessionStorage = async ({
4914
serverUrl,
5015
isPreregistered,

0 commit comments

Comments
 (0)