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

Author: ihrpr

src/client/auth.test.ts

@@ -954,10 +954,19 @@ describe("OAuth Authorization", () => {
     });
 
     it("passes resource parameter through authorization flow", async () => {
-      // Mock successful metadata discovery
+      // Mock successful metadata discovery - need to include protected resource metadata
       mockFetch.mockImplementation((url) => {
         const urlString = url.toString();
-        if (urlString.includes("/.well-known/oauth-authorization-server")) {
+        if (urlString.includes("/.well-known/oauth-protected-resource")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              resource: "https://api.example.com/mcp-server",
+              authorization_servers: ["https://auth.example.com"],
+            }),
+          });
+        } else if (urlString.includes("/.well-known/oauth-authorization-server")) {
           return Promise.resolve({
             ok: true,
             status: 200,
@@ -1002,11 +1011,20 @@ describe("OAuth Authorization", () => {
     });
 
     it("includes resource in token exchange when authorization code is provided", async () => {
-      // Mock successful metadata discovery and token exchange
+      // Mock successful metadata discovery and token exchange - need protected resource metadata
       mockFetch.mockImplementation((url) => {
         const urlString = url.toString();
 
-        if (urlString.includes("/.well-known/oauth-authorization-server")) {
+        if (urlString.includes("/.well-known/oauth-protected-resource")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              resource: "https://api.example.com/mcp-server",
+              authorization_servers: ["https://auth.example.com"],
+            }),
+          });
+        } else if (urlString.includes("/.well-known/oauth-authorization-server")) {
           return Promise.resolve({
             ok: true,
             status: 200,
@@ -1062,11 +1080,20 @@ describe("OAuth Authorization", () => {
     });
 
     it("includes resource in token refresh", async () => {
-      // Mock successful metadata discovery and token refresh
+      // Mock successful metadata discovery and token refresh - need protected resource metadata
       mockFetch.mockImplementation((url) => {
         const urlString = url.toString();
 
-        if (urlString.includes("/.well-known/oauth-authorization-server")) {
+        if (urlString.includes("/.well-known/oauth-protected-resource")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              resource: "https://api.example.com/mcp-server",
+              authorization_servers: ["https://auth.example.com"],
+            }),
+          });
+        } else if (urlString.includes("/.well-known/oauth-authorization-server")) {
           return Promise.resolve({
             ok: true,
             status: 200,
@@ -1244,5 +1271,197 @@ describe("OAuth Authorization", () => {
       // Should use the PRM's resource value, not the full requested URL
       expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/");
     });
+
+    it("excludes resource parameter when Protected Resource Metadata is not present", async () => {
+      // Mock metadata discovery where protected resource metadata is not available (404)
+      // but authorization server metadata is available
+      mockFetch.mockImplementation((url) => {
+        const urlString = url.toString();
+
+        if (urlString.includes("/.well-known/oauth-protected-resource")) {
+          // Protected resource metadata not available
+          return Promise.resolve({
+            ok: false,
+            status: 404,
+          });
+        } else if (urlString.includes("/.well-known/oauth-authorization-server")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              issuer: "https://auth.example.com",
+              authorization_endpoint: "https://auth.example.com/authorize",
+              token_endpoint: "https://auth.example.com/token",
+              response_types_supported: ["code"],
+              code_challenge_methods_supported: ["S256"],
+            }),
+          });
+        }
+
+        return Promise.resolve({ ok: false, status: 404 });
+      });
+
+      // Mock provider methods
+      (mockProvider.clientInformation as jest.Mock).mockResolvedValue({
+        client_id: "test-client",
+        client_secret: "test-secret",
+      });
+      (mockProvider.tokens as jest.Mock).mockResolvedValue(undefined);
+      (mockProvider.saveCodeVerifier as jest.Mock).mockResolvedValue(undefined);
+      (mockProvider.redirectToAuthorization as jest.Mock).mockResolvedValue(undefined);
+
+      // Call auth - should not include resource parameter
+      const result = await auth(mockProvider, {
+        serverUrl: "https://api.example.com/mcp-server",
+      });
+
+      expect(result).toBe("REDIRECT");
+
+      // Verify the authorization URL does NOT include the resource parameter
+      expect(mockProvider.redirectToAuthorization).toHaveBeenCalledWith(
+        expect.objectContaining({
+          searchParams: expect.any(URLSearchParams),
+        })
+      );
+
+      const redirectCall = (mockProvider.redirectToAuthorization as jest.Mock).mock.calls[0];
+      const authUrl: URL = redirectCall[0];
+      // Resource parameter should not be present when PRM is not available
+      expect(authUrl.searchParams.has("resource")).toBe(false);
+    });
+
+    it("excludes resource parameter in token exchange when Protected Resource Metadata is not present", async () => {
+      // Mock metadata discovery - no protected resource metadata, but auth server metadata available
+      mockFetch.mockImplementation((url) => {
+        const urlString = url.toString();
+
+        if (urlString.includes("/.well-known/oauth-protected-resource")) {
+          return Promise.resolve({
+            ok: false,
+            status: 404,
+          });
+        } else if (urlString.includes("/.well-known/oauth-authorization-server")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              issuer: "https://auth.example.com",
+              authorization_endpoint: "https://auth.example.com/authorize",
+              token_endpoint: "https://auth.example.com/token",
+              response_types_supported: ["code"],
+              code_challenge_methods_supported: ["S256"],
+            }),
+          });
+        } else if (urlString.includes("/token")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              access_token: "access123",
+              token_type: "Bearer",
+              expires_in: 3600,
+              refresh_token: "refresh123",
+            }),
+          });
+        }
+
+        return Promise.resolve({ ok: false, status: 404 });
+      });
+
+      // Mock provider methods for token exchange
+      (mockProvider.clientInformation as jest.Mock).mockResolvedValue({
+        client_id: "test-client",
+        client_secret: "test-secret",
+      });
+      (mockProvider.codeVerifier as jest.Mock).mockResolvedValue("test-verifier");
+      (mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);
+
+      // Call auth with authorization code
+      const result = await auth(mockProvider, {
+        serverUrl: "https://api.example.com/mcp-server",
+        authorizationCode: "auth-code-123",
+      });
+
+      expect(result).toBe("AUTHORIZED");
+
+      // Find the token exchange call
+      const tokenCall = mockFetch.mock.calls.find(call =>
+        call[0].toString().includes("/token")
+      );
+      expect(tokenCall).toBeDefined();
+
+      const body = tokenCall![1].body as URLSearchParams;
+      // Resource parameter should not be present when PRM is not available
+      expect(body.has("resource")).toBe(false);
+      expect(body.get("code")).toBe("auth-code-123");
+    });
+
+    it("excludes resource parameter in token refresh when Protected Resource Metadata is not present", async () => {
+      // Mock metadata discovery - no protected resource metadata, but auth server metadata available
+      mockFetch.mockImplementation((url) => {
+        const urlString = url.toString();
+
+        if (urlString.includes("/.well-known/oauth-protected-resource")) {
+          return Promise.resolve({
+            ok: false,
+            status: 404,
+          });
+        } else if (urlString.includes("/.well-known/oauth-authorization-server")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              issuer: "https://auth.example.com",
+              authorization_endpoint: "https://auth.example.com/authorize",
+              token_endpoint: "https://auth.example.com/token",
+              response_types_supported: ["code"],
+              code_challenge_methods_supported: ["S256"],
+            }),
+          });
+        } else if (urlString.includes("/token")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              access_token: "new-access123",
+              token_type: "Bearer",
+              expires_in: 3600,
+            }),
+          });
+        }
+
+        return Promise.resolve({ ok: false, status: 404 });
+      });
+
+      // Mock provider methods for token refresh
+      (mockProvider.clientInformation as jest.Mock).mockResolvedValue({
+        client_id: "test-client",
+        client_secret: "test-secret",
+      });
+      (mockProvider.tokens as jest.Mock).mockResolvedValue({
+        access_token: "old-access",
+        refresh_token: "refresh123",
+      });
+      (mockProvider.saveTokens as jest.Mock).mockResolvedValue(undefined);
+
+      // Call auth with existing tokens (should trigger refresh)
+      const result = await auth(mockProvider, {
+        serverUrl: "https://api.example.com/mcp-server",
+      });
+
+      expect(result).toBe("AUTHORIZED");
+
+      // Find the token refresh call
+      const tokenCall = mockFetch.mock.calls.find(call =>
+        call[0].toString().includes("/token")
+      );
+      expect(tokenCall).toBeDefined();
+
+      const body = tokenCall![1].body as URLSearchParams;
+      // Resource parameter should not be present when PRM is not available
+      expect(body.has("resource")).toBe(false);
+      expect(body.get("grant_type")).toBe("refresh_token");
+      expect(body.get("refresh_token")).toBe("refresh123");
+    });
   });
 });

src/client/auth.ts

@@ -198,19 +198,24 @@ export async function auth(
 }
 
 export async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
-  let resource = resourceUrlFromServerUrl(serverUrl);
+  const defaultResource = resourceUrlFromServerUrl(serverUrl);
+
+  // If provider has custom validation, delegate to it
   if (provider.validateResourceURL) {
-    return await provider.validateResourceURL(resource, resourceMetadata?.resource);
-  } else if (resourceMetadata) {
-    if (checkResourceAllowed({ requestedResource: resource, configuredResource: resourceMetadata.resource })) {
-      // If the resource mentioned in metadata is valid, prefer it since it is what the server is telling us to request.
-      resource = new URL(resourceMetadata.resource);
-    } else {
-      throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource} (or origin)`);
-    }
+    return await provider.validateResourceURL(defaultResource, resourceMetadata?.resource);
+  }
+
+  // Only include resource parameter when Protected Resource Metadata is present
+  if (!resourceMetadata) {
+    return undefined;
   }
 
-  return resource;
+  // Validate that the metadata's resource is compatible with our request
+  if (!checkResourceAllowed({ requestedResource: defaultResource, configuredResource: resourceMetadata.resource })) {
+    throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${defaultResource} (or origin)`);
+  }
+  // Prefer the resource from metadata since it's what the server is telling us to request
+  return new URL(resourceMetadata.resource);
 }
 
 /**
@@ -360,7 +365,7 @@ export async function discoverOAuthMetadata(
     try {
       const rootUrl = new URL("/.well-known/oauth-authorization-server", issuer);
       response = await tryMetadataDiscovery(rootUrl, protocolVersion);
-      
+
       if (response.status === 404) {
         return undefined;
       }