Merge pull request modelcontextprotocol#664 from modelcontextprotocol/pcarleton/auth-resource-validation2

[auth] adjust default validation for resource parameter in client flow, and server example

Author: ihrpr

src/client/auth.test.ts

@@ -1041,9 +1041,70 @@ describe("OAuth Authorization", () => {
 
       // Verify custom validation method was called
       expect(mockValidateResourceURL).toHaveBeenCalledWith(
-        "https://api.example.com/mcp-server",
+        new URL("https://api.example.com/mcp-server"),
         "https://different-resource.example.com/mcp-server"
       );
     });
+
+    it("uses prefix of server URL from PRM resource as resource parameter", async () => {
+      // Mock successful metadata discovery with resource URL that is a prefix of requested URL
+      mockFetch.mockImplementation((url) => {
+        const urlString = url.toString();
+
+        if (urlString.includes("/.well-known/oauth-protected-resource")) {
+          return Promise.resolve({
+            ok: true,
+            status: 200,
+            json: async () => ({
+              // Resource is a prefix of the requested server URL
+              resource: "https://api.example.com/",
+              authorization_servers: ["https://auth.example.com"],
+            }),
+          });
+        } 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 with a URL that has the resource as prefix
+      const result = await auth(mockProvider, {
+        serverUrl: "https://api.example.com/mcp-server/endpoint",
+      });
+
+      expect(result).toBe("REDIRECT");
+
+      // Verify the authorization URL includes the resource parameter from PRM
+      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];
+      // Should use the PRM's resource value, not the full requested URL
+      expect(authUrl.searchParams.get("resource")).toBe("https://api.example.com/");
+    });
   });
 });

src/client/auth.ts

@@ -2,7 +2,7 @@ import pkceChallenge from "pkce-challenge";
 import { LATEST_PROTOCOL_VERSION } from "../types.js";
 import type { OAuthClientMetadata, OAuthClientInformation, OAuthTokens, OAuthMetadata, OAuthClientInformationFull, OAuthProtectedResourceMetadata } from "../shared/auth.js";
 import { OAuthClientInformationFullSchema, OAuthMetadataSchema, OAuthProtectedResourceMetadataSchema, OAuthTokensSchema } from "../shared/auth.js";
-import { resourceUrlFromServerUrl } from "../shared/auth-utils.js";
+import { checkResourceAllowed, resourceUrlFromServerUrl } from "../shared/auth-utils.js";
 
 /**
  * Implements an end-to-end OAuth client to be used with one MCP server.
@@ -197,14 +197,17 @@ export async function auth(
   return "REDIRECT";
 }
 
-async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
+export async function selectResourceURL(serverUrl: string| URL, provider: OAuthClientProvider, resourceMetadata?: OAuthProtectedResourceMetadata): Promise<URL | undefined> {
+  let resource = resourceUrlFromServerUrl(serverUrl);
   if (provider.validateResourceURL) {
-    return await provider.validateResourceURL(serverUrl, resourceMetadata?.resource);
-  }
-
-  const resource = resourceUrlFromServerUrl(typeof serverUrl === "string" ? new URL(serverUrl) : serverUrl);
-  if (resourceMetadata && resourceMetadata.resource !== resource.href) {
-    throw new Error(`Protected resource ${resourceMetadata.resource} does not match expected ${resource}`);
+    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 resource;

src/examples/server/simpleStreamableHttp.ts

@@ -9,6 +9,7 @@ import { CallToolResult, GetPromptResult, isInitializeRequest, PrimitiveSchemaDe
 import { InMemoryEventStore } from '../shared/inMemoryEventStore.js';
 import { setupAuthServer } from './demoInMemoryOAuthProvider.js';
 import { OAuthMetadata } from 'src/shared/auth.js';
+import { checkResourceAllowed } from 'src/shared/auth-utils.js';
 
 // Check for OAuth flag
 const useOAuth = process.argv.includes('--oauth');
@@ -463,7 +464,7 @@ if (useOAuth) {
         if (!data.aud) {
           throw new Error(`Resource Indicator (RFC8707) missing`);
         }
-        if (data.aud !== mcpServerUrl.href) {
+        if (!checkResourceAllowed({ requestedResource: data.aud, configuredResource: mcpServerUrl })) {
           throw new Error(`Expected resource indicator ${mcpServerUrl}, got: ${data.aud}`);
         }
       }

src/shared/auth-utils.test.ts

@@ -1,4 +1,4 @@
-import { resourceUrlFromServerUrl } from './auth-utils.js';
+import { resourceUrlFromServerUrl, checkResourceAllowed } from './auth-utils.js';
 
 describe('auth-utils', () => {
   describe('resourceUrlFromServerUrl', () => {
@@ -27,4 +27,35 @@ describe('auth-utils', () => {
       expect(resourceUrlFromServerUrl(new URL('https://example.com/path/')).href).toBe('https://example.com/path/');
     });
   });
-});
+
+  describe('resourceMatches', () => {
+    it('should match identical URLs', () => {
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/path', configuredResource: 'https://example.com/path' })).toBe(true);
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/', configuredResource: 'https://example.com/' })).toBe(true);
+    });
+
+    it('should not match URLs with different paths', () => {
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/path1', configuredResource: 'https://example.com/path2' })).toBe(false);
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/', configuredResource: 'https://example.com/path' })).toBe(false);
+    });
+
+    it('should not match URLs with different domains', () => {
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/path', configuredResource: 'https://example.org/path' })).toBe(false);
+    });
+
+    it('should not match URLs with different ports', () => {
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com:8080/path', configuredResource: 'https://example.com/path' })).toBe(false);
+    });
+
+    it('should not match URLs where one path is a sub-path of another', () => {
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/mcpxxxx', configuredResource: 'https://example.com/mcp' })).toBe(false);
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/folder', configuredResource: 'https://example.com/folder/subfolder' })).toBe(false);
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/api/v1', configuredResource: 'https://example.com/api' })).toBe(true);
+    });
+
+    it('should handle trailing slashes vs no trailing slashes', () => {
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/mcp/', configuredResource: 'https://example.com/mcp' })).toBe(true);
+      expect(checkResourceAllowed({ requestedResource: 'https://example.com/folder', configuredResource: 'https://example.com/folder/' })).toBe(false);
+    });
+  });
+});

src/shared/auth-utils.ts

@@ -7,8 +7,48 @@
  * RFC 8707 section 2 states that resource URIs "MUST NOT include a fragment component".
  * Keeps everything else unchanged (scheme, domain, port, path, query).
  */
-export function resourceUrlFromServerUrl(url: URL): URL {
-  const resourceURL = new URL(url.href);
+export function resourceUrlFromServerUrl(url: URL | string ): URL {
+  const resourceURL = typeof url === "string" ? new URL(url) : new URL(url.href);
   resourceURL.hash = ''; // Remove fragment
   return resourceURL;
 }
+
+/**
+ * Checks if a requested resource URL matches a configured resource URL.
+ * A requested resource matches if it has the same scheme, domain, port,
+ * and its path starts with the configured resource's path.
+ *
+ * @param requestedResource The resource URL being requested
+ * @param configuredResource The resource URL that has been configured
+ * @returns true if the requested resource matches the configured resource, false otherwise
+ */
+ export function checkResourceAllowed(
+   { requestedResource, configuredResource }: {
+     requestedResource: URL | string;
+     configuredResource: URL | string
+   }
+ ): boolean {
+   const requested = typeof requestedResource === "string" ? new URL(requestedResource) : new URL(requestedResource.href);
+   const configured = typeof configuredResource === "string" ? new URL(configuredResource) : new URL(configuredResource.href);
+
+   // Compare the origin (scheme, domain, and port)
+   if (requested.origin !== configured.origin) {
+     return false;
+   }
+
+   // Handle cases like requested=/foo and configured=/foo/
+   if (requested.pathname.length < configured.pathname.length) {
+     return false
+   }
+
+   // Check if the requested path starts with the configured path
+   // Ensure both paths end with / for proper comparison
+   // This ensures that if we have paths like "/api" and "/api/users",
+   // we properly detect that "/api/users" is a subpath of "/api"
+   // By adding a trailing slash if missing, we avoid false positives
+   // where paths like "/api123" would incorrectly match "/api"
+   const requestedPath = requested.pathname.endsWith('/') ? requested.pathname : requested.pathname + '/';
+   const configuredPath = configured.pathname.endsWith('/') ? configured.pathname : configured.pathname + '/';
+
+   return requestedPath.startsWith(configuredPath);
+ }