Skip to content

Commit 600c623

Browse files
ihrprihrpr
authored
Merge pull request #693 from modelcontextprotocol/pcarleton/resource-only-when-prm-present
[auth] refactor resource selection to not include resource if PRM is not present
2 parents ebf8e2d + 15a2277 commit 600c623

File tree

2 files changed

+241
-17
lines changed

2 files changed

+241
-17
lines changed

‎src/client/auth.test.ts

Lines changed: 225 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -954,10 +954,19 @@ describe("OAuth Authorization", () => {
954954
});
955955

956956
it("passes resource parameter through authorization flow", async () => {
957-
// Mock successful metadata discovery
957+
// Mock successful metadata discovery - need to include protected resource metadata
958958
mockFetch.mockImplementation((url) => {
959959
const urlString = url.toString();
960-
if (urlString.includes("/.well-known/oauth-authorization-server")) {
960+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
961+
return Promise.resolve({
962+
ok: true,
963+
status: 200,
964+
json: async () => ({
965+
resource: "https://api.example.com/mcp-server",
966+
authorization_servers: ["https://auth.example.com"],
967+
}),
968+
});
969+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
961970
return Promise.resolve({
962971
ok: true,
963972
status: 200,
@@ -1002,11 +1011,20 @@ describe("OAuth Authorization", () => {
10021011
});
10031012

10041013
it("includes resource in token exchange when authorization code is provided", async () => {
1005-
// Mock successful metadata discovery and token exchange
1014+
// Mock successful metadata discovery and token exchange - need protected resource metadata
10061015
mockFetch.mockImplementation((url) => {
10071016
const urlString = url.toString();
10081017

1009-
if (urlString.includes("/.well-known/oauth-authorization-server")) {
1018+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1019+
return Promise.resolve({
1020+
ok: true,
1021+
status: 200,
1022+
json: async () => ({
1023+
resource: "https://api.example.com/mcp-server",
1024+
authorization_servers: ["https://auth.example.com"],
1025+
}),
1026+
});
1027+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
10101028
return Promise.resolve({
10111029
ok: true,
10121030
status: 200,
@@ -1062,11 +1080,20 @@ describe("OAuth Authorization", () => {
10621080
});
10631081

10641082
it("includes resource in token refresh", async () => {
1065-
// Mock successful metadata discovery and token refresh
1083+
// Mock successful metadata discovery and token refresh - need protected resource metadata
10661084
mockFetch.mockImplementation((url) => {
10671085
const urlString = url.toString();
10681086

1069-
if (urlString.includes("/.well-known/oauth-authorization-server")) {
1087+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1088+
return Promise.resolve({
1089+
ok: true,
1090+
status: 200,
1091+
json: async () => ({
1092+
resource: "https://api.example.com/mcp-server",
1093+
authorization_servers: ["https://auth.example.com"],
1094+
}),
1095+
});
1096+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
10701097
return Promise.resolve({
10711098
ok: true,
10721099
status: 200,
@@ -1244,5 +1271,197 @@ describe("OAuth Authorization", () => {
12441271
// Should use the PRM's resource value, not the full requested URL
12451272
expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/");
12461273
});
1274+
1275+
it("excludes resource parameter when Protected Resource Metadata is not present", async () => {
1276+
// Mock metadata discovery where protected resource metadata is not available (404)
1277+
// but authorization server metadata is available
1278+
mockFetch.mockImplementation((url) => {
1279+
const urlString = url.toString();
1280+
1281+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1282+
// Protected resource metadata not available
1283+
return Promise.resolve({
1284+
ok: false,
1285+
status: 404,
1286+
});
1287+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
1288+
return Promise.resolve({
1289+
ok: true,
1290+
status: 200,
1291+
json: async () => ({
1292+
issuer: "https://auth.example.com",
1293+
authorization_endpoint: "https://auth.example.com/authorize",
1294+
token_endpoint: "https://auth.example.com/token",
1295+
response_types_supported: ["code"],
1296+
code_challenge_methods_supported: ["S256"],
1297+
}),
1298+
});
1299+
}
1300+
1301+
return Promise.resolve({ ok: false, status: 404 });
1302+
});
1303+
1304+
// Mock provider methods
1305+
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
1306+
client_id: "test-client",
1307+
client_secret: "test-secret",
1308+
});
1309+
(mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
1310+
(mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
1311+
(mockProvider.redirectToAuthorization as jest.Mock).mockResolvedValue(undefined);
1312+
1313+
// Call auth - should not include resource parameter
1314+
const result = await auth(mockProvider, {
1315+
serverUrl: "https://api.example.com/mcp-server",
1316+
});
1317+
1318+
expect(result).toBe("REDIRECT");
1319+
1320+
// Verify the authorization URL does NOT include the resource parameter
1321+
expect(mockProvider.redirectToAuthorization).toHaveBeenCalledWith(
1322+
expect.objectContaining({
1323+
searchParams: expect.any(URLSearchParams),
1324+
})
1325+
);
1326+
1327+
const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
1328+
const authUrl: URL = redirectCall[0];
1329+
// Resource parameter should not be present when PRM is not available
1330+
expect(authUrl.searchParams.has("resource")).toBe(false);
1331+
});
1332+
1333+
it("excludes resource parameter in token exchange when Protected Resource Metadata is not present", async () => {
1334+
// Mock metadata discovery - no protected resource metadata, but auth server metadata available
1335+
mockFetch.mockImplementation((url) => {
1336+
const urlString = url.toString();
1337+
1338+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1339+
return Promise.resolve({
1340+
ok: false,
1341+
status: 404,
1342+
});
1343+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
1344+
return Promise.resolve({
1345+
ok: true,
1346+
status: 200,
1347+
json: async () => ({
1348+
issuer: "https://auth.example.com",
1349+
authorization_endpoint: "https://auth.example.com/authorize",
1350+
token_endpoint: "https://auth.example.com/token",
1351+
response_types_supported: ["code"],
1352+
code_challenge_methods_supported: ["S256"],
1353+
}),
1354+
});
1355+
} else if (urlString.includes("/token")) {
1356+
return Promise.resolve({
1357+
ok: true,
1358+
status: 200,
1359+
json: async () => ({
1360+
access_token: "access123",
1361+
token_type: "Bearer",
1362+
expires_in: 3600,
1363+
refresh_token: "refresh123",
1364+
}),
1365+
});
1366+
}
1367+
1368+
return Promise.resolve({ ok: false, status: 404 });
1369+
});
1370+
1371+
// Mock provider methods for token exchange
1372+
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
1373+
client_id: "test-client",
1374+
client_secret: "test-secret",
1375+
});
1376+
(mockProvider.codeVerifier as jest.Mock).mockResolvedValue("test-verifier");
1377+
(mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);
1378+
1379+
// Call auth with authorization code
1380+
const result = await auth(mockProvider, {
1381+
serverUrl: "https://api.example.com/mcp-server",
1382+
authorizationCode: "auth-code-123",
1383+
});
1384+
1385+
expect(result).toBe("AUTHORIZED");
1386+
1387+
// Find the token exchange call
1388+
const tokenCall = mockFetch.mock.calls.find(call =>
1389+
call[0].toString().includes("/token")
1390+
);
1391+
expect(tokenCall).toBeDefined();
1392+
1393+
const body = tokenCall![1].body as URLSearchParams;
1394+
// Resource parameter should not be present when PRM is not available
1395+
expect(body.has("resource")).toBe(false);
1396+
expect(body.get("code")).toBe("auth-code-123");
1397+
});
1398+
1399+
it("excludes resource parameter in token refresh when Protected Resource Metadata is not present", async () => {
1400+
// Mock metadata discovery - no protected resource metadata, but auth server metadata available
1401+
mockFetch.mockImplementation((url) => {
1402+
const urlString = url.toString();
1403+
1404+
if (urlString.includes("/.well-known/oauth-protected-resource")) {
1405+
return Promise.resolve({
1406+
ok: false,
1407+
status: 404,
1408+
});
1409+
} else if (urlString.includes("/.well-known/oauth-authorization-server")) {
1410+
return Promise.resolve({
1411+
ok: true,
1412+
status: 200,
1413+
json: async () => ({
1414+
issuer: "https://auth.example.com",
1415+
authorization_endpoint: "https://auth.example.com/authorize",
1416+
token_endpoint: "https://auth.example.com/token",
1417+
response_types_supported: ["code"],
1418+
code_challenge_methods_supported: ["S256"],
1419+
}),
1420+
});
1421+
} else if (urlString.includes("/token")) {
1422+
return Promise.resolve({
1423+
ok: true,
1424+
status: 200,
1425+
json: async () => ({
1426+
access_token: "new-access123",
1427+
token_type: "Bearer",
1428+
expires_in: 3600,
1429+
}),
1430+
});
1431+
}
1432+
1433+
return Promise.resolve({ ok: false, status: 404 });
1434+
});
1435+
1436+
// Mock provider methods for token refresh
1437+
(mockProvider.clientInformation as jest.Mock).mockResolvedValue({
1438+
client_id: "test-client",
1439+
client_secret: "test-secret",
1440+
});
1441+
(mockProvider.tokens as jest.Mock).mockResolvedValue({
1442+
access_token: "old-access",
1443+
refresh_token: "refresh123",
1444+
});
1445+
(mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);
1446+
1447+
// Call auth with existing tokens (should trigger refresh)
1448+
const result = await auth(mockProvider, {
1449+
serverUrl: "https://api.example.com/mcp-server",
1450+
});
1451+
1452+
expect(result).toBe("AUTHORIZED");
1453+
1454+
// Find the token refresh call
1455+
const tokenCall = mockFetch.mock.calls.find(call =>
1456+
call[0].toString().includes("/token")
1457+
);
1458+
expect(tokenCall).toBeDefined();
1459+
1460+
const body = tokenCall![1].body as URLSearchParams;
1461+
// Resource parameter should not be present when PRM is not available
1462+
expect(body.has("resource")).toBe(false);
1463+
expect(body.get("grant_type")).toBe("refresh_token");
1464+
expect(body.get("refresh_token")).toBe("refresh123");
1465+
});
12471466
});
12481467
});

‎src/client/auth.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -198,19 +198,24 @@ export async function auth(
198198
}
199199

200200
export async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
201-
let resource = resourceUrlFromServerUrl(serverUrl);
201+
const defaultResource = resourceUrlFromServerUrl(serverUrl);
202+
203+
// If provider has custom validation, delegate to it
202204
if (provider.validateResourceURL) {
203-
return await provider.validateResourceURL(resource, resourceMetadata?.resource);
204-
} else if (resourceMetadata) {
205-
if (checkResourceAllowed({ requestedResource: resource, configuredResource: resourceMetadata.resource })) {
206-
// If the resource mentioned in metadata is valid, prefer it since it is what the server is telling us to request.
207-
resource = new URL(resourceMetadata.resource);
208-
} else {
209-
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource} (or origin)`);
210-
}
205+
return await provider.validateResourceURL(defaultResource, resourceMetadata?.resource);
206+
}
207+
208+
// Only include resource parameter when Protected Resource Metadata is present
209+
if (!resourceMetadata) {
210+
return undefined;
211211
}
212212

213-
return resource;
213+
// Validate that the metadata's resource is compatible with our request
214+
if (!checkResourceAllowed({ requestedResource: defaultResource, configuredResource: resourceMetadata.resource })) {
215+
throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)`);
216+
}
217+
// Prefer the resource from metadata since it's what the server is telling us to request
218+
return new URL(resourceMetadata.resource);
214219
}
215220

216221
/**
@@ -360,7 +365,7 @@ export async function discoverOAuthMetadata(
360365
try {
361366
const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer);
362367
response = await tryMetadataDiscovery(rootUrl, protocolVersion);
363-
368+
364369
if (response.status === 404) {
365370
return undefined;
366371
}

0 commit comments

Comments
 (0)