Skip to content

Commit b16a415

Browse files
ochafikochafik
committed
cleanups
1 parent ec0c504 commit b16a415

File tree

8 files changed

+25
-52
lines changed

8 files changed

+25
-52
lines changed

‎src/client/auth.test.ts‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -938,8 +938,7 @@ describe("OAuth Authorization", () => {
938938

939939
// Call the auth function with a resource that has a fragment
940940
const result = await auth(mockProvider, {
941-
serverUrl: "https://resource.example.com",
942-
resource: new URL("https://api.example.com/mcp-server#fragment"),
941+
serverUrl: "https://api.example.com/mcp-server#fragment",
943942
});
944943

945944
expect(result).toBe("REDIRECT");
@@ -987,8 +986,7 @@ describe("OAuth Authorization", () => {
987986

988987
// Call auth without authorization code (should trigger redirect)
989988
const result = await auth(mockProvider, {
990-
serverUrl: "https://resource.example.com",
991-
resource: new URL("https://api.example.com/mcp-server"),
989+
serverUrl: "https://api.example.com/mcp-server",
992990
});
993991

994992
expect(result).toBe("REDIRECT");
@@ -1048,9 +1046,8 @@ describe("OAuth Authorization", () => {
10481046

10491047
// Call auth with authorization code
10501048
const result = await auth(mockProvider, {
1051-
serverUrl: "https://resource.example.com",
1049+
serverUrl: "https://api.example.com/mcp-server",
10521050
authorizationCode: "auth-code-123",
1053-
resource: new URL("https://api.example.com/mcp-server"),
10541051
});
10551052

10561053
expect(result).toBe("AUTHORIZED");
@@ -1111,8 +1108,7 @@ describe("OAuth Authorization", () => {
11111108

11121109
// Call auth with existing tokens (should trigger refresh)
11131110
const result = await auth(mockProvider, {
1114-
serverUrl: "https://resource.example.com",
1115-
resource: new URL("https://api.example.com/mcp-server"),
1111+
serverUrl: "https://api.example.com/mcp-server",
11161112
});
11171113

11181114
expect(result).toBe("AUTHORIZED");
@@ -1160,8 +1156,7 @@ describe("OAuth Authorization", () => {
11601156

11611157
// Call auth with empty resource parameter
11621158
const result = await auth(mockProvider, {
1163-
serverUrl: "https://resource.example.com",
1164-
resource: undefined,
1159+
serverUrl: "https://api.example.com/mcp-server",
11651160
});
11661161

11671162
expect(result).toBe("REDIRECT");
@@ -1203,8 +1198,7 @@ describe("OAuth Authorization", () => {
12031198

12041199
// Call auth with resource containing multiple # symbols
12051200
const result = await auth(mockProvider, {
1206-
serverUrl: "https://resource.example.com",
1207-
resource: new URL("https://api.example.com/mcp-server#fragment#another"),
1201+
serverUrl: "https://api.example.com/mcp-server#fragment#another",
12081202
});
12091203

12101204
expect(result).toBe("REDIRECT");
@@ -1248,8 +1242,7 @@ describe("OAuth Authorization", () => {
12481242
// This tests the security fix that prevents token confusion between
12491243
// multiple MCP servers on the same domain
12501244
const result1 = await auth(mockProvider, {
1251-
serverUrl: "https://api.example.com",
1252-
resource: new URL("https://api.example.com/mcp-server-1/v1"),
1245+
serverUrl: "https://api.example.com/mcp-server-1/v1",
12531246
});
12541247

12551248
expect(result1).toBe("REDIRECT");
@@ -1263,8 +1256,7 @@ describe("OAuth Authorization", () => {
12631256

12641257
// Test with different path on same domain
12651258
const result2 = await auth(mockProvider, {
1266-
serverUrl: "https://api.example.com",
1267-
resource: new URL("https://api.example.com/mcp-server-2/v1"),
1259+
serverUrl: "https://api.example.com/mcp-server-2/v1",
12681260
});
12691261

12701262
expect(result2).toBe("REDIRECT");
@@ -1308,8 +1300,7 @@ describe("OAuth Authorization", () => {
13081300

13091301
// Call auth with resource containing query parameters
13101302
const result = await auth(mockProvider, {
1311-
serverUrl: "https://resource.example.com",
1312-
resource: new URL("https://api.example.com/mcp-server?param=value&another=test"),
1303+
serverUrl: "https://api.example.com/mcp-server?param=value&another=test",
13131304
});
13141305

13151306
expect(result).toBe("REDIRECT");

‎src/client/auth.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,13 @@ export async function auth(
9494
authorizationCode,
9595
scope,
9696
resourceMetadataUrl,
97-
resource
9897
}: {
9998
serverUrl: string | URL;
10099
authorizationCode?: string;
101100
scope?: string;
102-
resourceMetadataUrl?: URL;
103-
resource?: URL }): Promise<AuthResult> {
101+
resourceMetadataUrl?: URL }): Promise<AuthResult> {
104102

105-
// Remove fragment from resource parameter if provided
106-
let canonicalResource: URL | undefined;
107-
if (resource) {
108-
canonicalResource = resourceUrlFromServerUrl(new URL(resource));
109-
}
103+
const resource = resourceUrlFromServerUrl(typeof serverUrl === "string" ? new URL(serverUrl) : serverUrl);
110104

111105
let authorizationServerUrl = serverUrl;
112106
try {
@@ -151,7 +145,7 @@ export async function auth(
151145
authorizationCode,
152146
codeVerifier,
153147
redirectUri: provider.redirectUrl,
154-
resource: canonicalResource,
148+
resource,
155149
});
156150

157151
await provider.saveTokens(tokens);
@@ -168,7 +162,7 @@ export async function auth(
168162
metadata,
169163
clientInformation,
170164
refreshToken: tokens.refresh_token,
171-
resource: canonicalResource,
165+
resource,
172166
});
173167

174168
await provider.saveTokens(newTokens);
@@ -187,7 +181,7 @@ export async function auth(
187181
state,
188182
redirectUrl: provider.redirectUrl,
189183
scope: scope || provider.clientMetadata.scope,
190-
resource: canonicalResource,
184+
resource,
191185
});
192186

193187
await provider.saveCodeVerifier(codeVerifier);

‎src/client/sse.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ export class SSEClientTransport implements Transport {
9090
result = await auth(this._authProvider, {
9191
serverUrl: this._url,
9292
resourceMetadataUrl: this._resourceMetadataUrl,
93-
resource: resourceUrlFromServerUrl(new URL(this._url))
9493
});
9594
} catch (error) {
9695
this.onerror?.(error as Error);
@@ -210,7 +209,6 @@ export class SSEClientTransport implements Transport {
210209
serverUrl: this._url,
211210
authorizationCode,
212211
resourceMetadataUrl: this._resourceMetadataUrl,
213-
resource: resourceUrlFromServerUrl(new URL(this._url))
214212
});
215213
if (result !== "AUTHORIZED") {
216214
throw new UnauthorizedError("Failed to authorize");
@@ -249,7 +247,6 @@ export class SSEClientTransport implements Transport {
249247
const result = await auth(this._authProvider, {
250248
serverUrl: this._url,
251249
resourceMetadataUrl: this._resourceMetadataUrl,
252-
resource: resourceUrlFromServerUrl(new URL(this._url))
253250
});
254251
if (result !== "AUTHORIZED") {
255252
throw new UnauthorizedError();

‎src/client/streamableHttp.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ export class StreamableHTTPClientTransport implements Transport {
153153
result = await auth(this._authProvider, {
154154
serverUrl: this._url,
155155
resourceMetadataUrl: this._resourceMetadataUrl,
156-
resource: resourceUrlFromServerUrl(new URL(this._url))
157156
});
158157
} catch (error) {
159158
this.onerror?.(error as Error);
@@ -371,7 +370,6 @@ export class StreamableHTTPClientTransport implements Transport {
371370
serverUrl: this._url,
372371
authorizationCode,
373372
resourceMetadataUrl: this._resourceMetadataUrl,
374-
resource: resourceUrlFromServerUrl(new URL(this._url))
375373
});
376374
if (result !== "AUTHORIZED") {
377375
throw new UnauthorizedError("Failed to authorize");
@@ -423,7 +421,6 @@ export class StreamableHTTPClientTransport implements Transport {
423421
const result = await auth(this._authProvider, {
424422
serverUrl: this._url,
425423
resourceMetadataUrl: this._resourceMetadataUrl,
426-
resource: resourceUrlFromServerUrl(new URL(this._url))
427424
});
428425
if (result !== "AUTHORIZED") {
429426
throw new UnauthorizedError();

‎src/examples/server/demoInMemoryOAuthProvider.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,14 @@ export class DemoInMemoryClientsStore implements OAuthRegisteredClientsStore {
2121
}
2222
}
2323

24+
/**
25+
* 🚨 DEMO ONLY - NOT FOR PRODUCTION
26+
*
27+
* This example demonstrates MCP OAuth flow but lacks some of the features required for production use,
28+
* for example:
29+
* - Persistent token storage
30+
* - Rate limiting
31+
*/
2432
export class DemoInMemoryAuthProvider implements OAuthServerProvider {
2533
clientsStore = new DemoInMemoryClientsStore();
2634
private codes = new Map<string, {
@@ -119,12 +127,12 @@ export class DemoInMemoryAuthProvider implements OAuthServerProvider {
119127
}
120128

121129
async exchangeRefreshToken(
122-
client: OAuthClientInformationFull,
130+
_client: OAuthClientInformationFull,
123131
_refreshToken: string,
124132
_scopes?: string[],
125133
resource?: URL
126134
): Promise<OAuthTokens> {
127-
throw new Error('Refresh tokens not implemented for example demo');
135+
throw new Error('Not implemented for example demo');
128136
}
129137

130138
async verifyAccessToken(token: string): Promise<AuthInfo> {

‎src/examples/server/simpleStreamableHttp.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -282,12 +282,7 @@ if (useOAuth) {
282282
const mcpServerUrl = new URL(`http://localhost:${MCP_PORT}`);
283283
const authServerUrl = new URL(`http://localhost:${AUTH_PORT}`);
284284

285-
// Configure the demo auth provider to validate resources match this server
286-
const demoProviderConfig = {
287-
serverUrl: mcpServerUrl.href,
288-
validateResourceMatchesServer: false // Set to true to enable strict validation
289-
};
290-
const oauthMetadata: OAuthMetadata = setupAuthServer(authServerUrl, demoProviderConfig);
285+
const oauthMetadata: OAuthMetadata = setupAuthServer(authServerUrl, mcpServerUrl);
291286

292287
const tokenVerifier = {
293288
verifyAccessToken: async (token: string) => {

‎src/server/auth/handlers/authorize.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,6 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
119119
const { scope, code_challenge, resource } = parseResult.data;
120120
state = parseResult.data.state;
121121

122-
// Pass through the resource parameter to the provider
123-
// The provider can decide how to validate it
124-
125122
// Validate scopes
126123
let requestedScopes: string[] = [];
127124
if (scope !== undefined) {

‎src/server/auth/handlers/token.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,6 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand
9393

9494
const { code, code_verifier, redirect_uri, resource } = parseResult.data;
9595

96-
// Pass through the resource parameter to the provider
97-
// The provider can decide how to validate it
98-
9996
const skipLocalPkceValidation = provider.skipLocalPkceValidation;
10097

10198
// Perform local PKCE validation unless explicitly skipped
@@ -127,9 +124,6 @@ export function tokenHandler({ provider, rateLimit: rateLimitConfig }: TokenHand
127124

128125
const { refresh_token, scope, resource } = parseResult.data;
129126

130-
// Pass through the resource parameter to the provider
131-
// The provider can decide how to validate it
132-
133127
const scopes = scope?.split(" ");
134128
const tokens = await provider.exchangeRefreshToken(client, refresh_token, scopes, resource ? new URL(resource) : undefined);
135129
res.status(200).json(tokens);

0 commit comments

Comments
 (0)