update resource to be a url

ochafik · ochafik · commit 224a2e242d95 · 2025-06-17T00:18:33.000+01:00
diff --git a/src/client/auth.ts b/src/client/auth.ts
@@ -100,12 +100,12 @@ export async function auth(
     authorizationCode?: string;
     scope?: string;
     resourceMetadataUrl?: URL;
-    resource?: string }): Promise<AuthResult> {
+    resource?: URL }): Promise<AuthResult> {
 
   // Remove fragment from resource parameter if provided
-  let canonicalResource: string | undefined;
+  let canonicalResource: URL | undefined;
   if (resource) {
-    canonicalResource = resourceUrlFromServerUrl(resource);
+    canonicalResource = resourceUrlFromServerUrl(new URL(resource));
   }
 
   let authorizationServerUrl = serverUrl;
@@ -329,7 +329,7 @@ export async function startAuthorization(
     redirectUrl: string | URL;
     scope?: string;
     state?: string;
-    resource?: string;
+    resource?: URL;
   },
 ): Promise<{ authorizationUrl: URL; codeVerifier: string }> {
   const responseType = "code";
@@ -380,7 +380,7 @@ export async function startAuthorization(
   }
 
   if (resource) {
-    authorizationUrl.searchParams.set("resource", resource);
+    authorizationUrl.searchParams.set("resource", resource.href);
   }
 
   return { authorizationUrl, codeVerifier };
@@ -404,7 +404,7 @@ export async function exchangeAuthorization(
     authorizationCode: string;
     codeVerifier: string;
     redirectUri: string | URL;
-    resource?: string;
+    resource?: URL;
   },
 ): Promise<OAuthTokens> {
   const grantType = "authorization_code";
@@ -439,7 +439,7 @@ export async function exchangeAuthorization(
   }
 
   if (resource) {
-    params.set("resource", resource);
+    params.set("resource", resource.href);
   }
 
   const response = await fetch(tokenUrl, {
@@ -471,7 +471,7 @@ export async function refreshAuthorization(
     metadata?: OAuthMetadata;
     clientInformation: OAuthClientInformation;
     refreshToken: string;
-    resource?: string;
+    resource?: URL;
   },
 ): Promise<OAuthTokens> {
   const grantType = "refresh_token";
@@ -504,7 +504,7 @@ export async function refreshAuthorization(
   }
 
   if (resource) {
-    params.set("resource", resource);
+    params.set("resource", resource.href);
   }
 
   const response = await fetch(tokenUrl, {
diff --git a/src/examples/server/demoInMemoryOAuthProvider.ts b/src/examples/server/demoInMemoryOAuthProvider.ts
@@ -122,7 +122,7 @@ export class DemoInMemoryAuthProvider implements OAuthServerProvider {
     client: OAuthClientInformationFull,
     _refreshToken: string,
     _scopes?: string[],
-    resource?: string
+    resource?: URL
   ): Promise<OAuthTokens> {
     throw new Error('Refresh tokens not implemented for example demo');
   }
diff --git a/src/server/auth/handlers/authorize.test.ts b/src/server/auth/handlers/authorize.test.ts
@@ -295,7 +295,7 @@ describe('Authorization Handler', () => {
       expect(mockProviderWithResource).toHaveBeenCalledWith(
         validClient,
         expect.objectContaining({
-          resource: 'https://api.example.com/resource',
+          resource: new URL('https://api.example.com/resource'),
           redirectUri: 'https://example.com/callback',
           codeChallenge: 'challenge123'
         }),
@@ -365,7 +365,7 @@ describe('Authorization Handler', () => {
       expect(mockProviderWithResources).toHaveBeenCalledWith(
         validClient,
         expect.objectContaining({
-          resource: 'https://api1.example.com/resource',
+          resource: new URL('https://api1.example.com/resource'),
           state: 'test-state'
         }),
         expect.any(Object)
@@ -391,7 +391,7 @@ describe('Authorization Handler', () => {
       expect(mockProviderPost).toHaveBeenCalledWith(
         validClient,
         expect.objectContaining({
-          resource: 'https://api.example.com/resource'
+          resource: new URL('https://api.example.com/resource')
         }),
         expect.any(Object)
       );
diff --git a/src/server/auth/handlers/authorize.ts b/src/server/auth/handlers/authorize.ts
@@ -142,7 +142,7 @@ export function authorizationHandler({ provider, rateLimit: rateLimitConfig }: A
         scopes: requestedScopes,
         redirectUri: redirect_uri,
         codeChallenge: code_challenge,
-        resource,
+        resource: resource ? new URL(resource) : undefined,
       }, res);
     } catch (error) {
       // Post-redirect errors - redirect with error parameters
diff --git a/src/server/auth/provider.ts b/src/server/auth/provider.ts
@@ -42,13 +42,13 @@ export interface OAuthServerProvider {
     authorizationCode: string, 
     codeVerifier?: string,
     redirectUri?: string,
-    resource?: string
+    resource?: URL
   ): Promise<OAuthTokens>;
 
   /**
    * Exchanges a refresh token for an access token.
    */
-  exchangeRefreshToken(client: OAuthClientInformationFull, refreshToken: string, scopes?: string[], resource?: string): Promise<OAuthTokens>;
+  exchangeRefreshToken(client: OAuthClientInformationFull, refreshToken: string, scopes?: string[], resource?: URL): Promise<OAuthTokens>;
 
   /**
    * Verifies an access token and returns information about it.
diff --git a/src/server/auth/providers/proxyProvider.test.ts b/src/server/auth/providers/proxyProvider.test.ts
@@ -112,7 +112,7 @@ describe("Proxy OAuth Server Provider", () => {
           codeChallenge: 'test-challenge',
           state: 'test-state',
           scopes: ['read', 'write'],
-          resource: 'https://api.example.com/resource'
+          resource: new URL('https://api.example.com/resource')
         },
         mockResponse
       );
diff --git a/src/server/auth/providers/proxyProvider.ts b/src/server/auth/providers/proxyProvider.ts
@@ -134,7 +134,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
     // Add optional standard OAuth parameters
     if (params.state) searchParams.set("state", params.state);
     if (params.scopes?.length) searchParams.set("scope", params.scopes.join(" "));
-    if (params.resource) searchParams.set("resource", params.resource);
+    if (params.resource) searchParams.set("resource", params.resource.href);
 
     targetUrl.search = searchParams.toString();
     res.redirect(targetUrl.toString());
@@ -154,7 +154,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
     authorizationCode: string,
     codeVerifier?: string,
     redirectUri?: string,
-    resource?: string
+    resource?: URL
   ): Promise<OAuthTokens> {
     const params = new URLSearchParams({
       grant_type: "authorization_code",
@@ -199,7 +199,7 @@ export class ProxyOAuthServerProvider implements OAuthServerProvider {
     client: OAuthClientInformationFull,
     refreshToken: string,
     scopes?: string[],
-    resource?: string
+    resource?: URL
   ): Promise<OAuthTokens> {
 
     const params = new URLSearchParams({
diff --git a/src/shared/auth-utils.test.ts b/src/shared/auth-utils.test.ts
@@ -1,89 +1,30 @@
-import { validateResourceUri, extractResourceUri, resourceUrlFromServerUrl } from './auth-utils.js';
+import { resourceUrlFromServerUrl } from './auth-utils.js';
 
 describe('auth-utils', () => {
   describe('resourceUrlFromServerUrl', () => {
     it('should remove fragments', () => {
-      expect(resourceUrlFromServerUrl('https://example.com/path#fragment')).toBe('https://example.com/path');
-      expect(resourceUrlFromServerUrl('https://example.com#fragment')).toBe('https://example.com');
-      expect(resourceUrlFromServerUrl('https://example.com/path?query=1#fragment')).toBe('https://example.com/path?query=1');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com/path#fragment')).href).toBe('https://example.com/path');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com#fragment')).href).toBe('https://example.com/');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com/path?query=1#fragment')).href).toBe('https://example.com/path?query=1');
     });
 
     it('should return URL unchanged if no fragment', () => {
-      expect(resourceUrlFromServerUrl('https://example.com')).toBe('https://example.com');
-      expect(resourceUrlFromServerUrl('https://example.com/path')).toBe('https://example.com/path');
-      expect(resourceUrlFromServerUrl('https://example.com/path?query=1')).toBe('https://example.com/path?query=1');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com')).href).toBe('https://example.com/');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com/path')).href).toBe('https://example.com/path');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com/path?query=1')).href).toBe('https://example.com/path?query=1');
     });
 
     it('should keep everything else unchanged', () => {
       // Case sensitivity preserved
-      expect(resourceUrlFromServerUrl('HTTPS://EXAMPLE.COM/PATH')).toBe('HTTPS://EXAMPLE.COM/PATH');
+      expect(resourceUrlFromServerUrl(new URL('https://EXAMPLE.COM/PATH')).href).toBe('https://example.com/PATH');
       // Ports preserved
-      expect(resourceUrlFromServerUrl('https://example.com:443/path')).toBe('https://example.com:443/path');
-      expect(resourceUrlFromServerUrl('https://example.com:8080/path')).toBe('https://example.com:8080/path');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com:443/path')).href).toBe('https://example.com/path');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com:8080/path')).href).toBe('https://example.com:8080/path');
       // Query parameters preserved
-      expect(resourceUrlFromServerUrl('https://example.com?foo=bar&baz=qux')).toBe('https://example.com?foo=bar&baz=qux');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com?foo=bar&baz=qux')).href).toBe('https://example.com/?foo=bar&baz=qux');
       // Trailing slashes preserved
-      expect(resourceUrlFromServerUrl('https://example.com/')).toBe('https://example.com/');
-      expect(resourceUrlFromServerUrl('https://example.com/path/')).toBe('https://example.com/path/');
-    });
-  });
-
-
-  describe('validateResourceUri', () => {
-    it('should accept valid resource URIs without fragments', () => {
-      expect(() => validateResourceUri('https://example.com')).not.toThrow();
-      expect(() => validateResourceUri('https://example.com/path')).not.toThrow();
-      expect(() => validateResourceUri('http://example.com:8080')).not.toThrow();
-      expect(() => validateResourceUri('https://example.com?query=1')).not.toThrow();
-      expect(() => validateResourceUri('ftp://example.com')).not.toThrow(); // Only fragment check now
-    });
-
-    it('should reject URIs with fragments', () => {
-      expect(() => validateResourceUri('https://example.com#fragment')).toThrow('must not contain a fragment');
-      expect(() => validateResourceUri('https://example.com/path#section')).toThrow('must not contain a fragment');
-      expect(() => validateResourceUri('https://example.com?query=1#anchor')).toThrow('must not contain a fragment');
-    });
-
-    it('should accept any URI without fragment', () => {
-      // These are all valid now since we only check for fragments
-      expect(() => validateResourceUri('//example.com')).not.toThrow();
-      expect(() => validateResourceUri('https://user:pass@example.com')).not.toThrow();
-      expect(() => validateResourceUri('/path')).not.toThrow();
-      expect(() => validateResourceUri('path')).not.toThrow();
-    });
-  });
-
-  describe('extractResourceUri', () => {
-    it('should remove fragments from URLs', () => {
-      expect(extractResourceUri('https://example.com/path#fragment')).toBe('https://example.com/path');
-      expect(extractResourceUri('https://example.com/path?query=1#fragment')).toBe('https://example.com/path?query=1');
-    });
-
-    it('should handle URL object', () => {
-      const url = new URL('https://example.com:8443/path?query=1#fragment');
-      expect(extractResourceUri(url)).toBe('https://example.com:8443/path?query=1');
-    });
-
-    it('should keep everything else unchanged', () => {
-      // Preserves case
-      expect(extractResourceUri('HTTPS://EXAMPLE.COM/path')).toBe('HTTPS://EXAMPLE.COM/path');
-      // Preserves all ports
-      expect(extractResourceUri('https://example.com:443/path')).toBe('https://example.com:443/path');
-      expect(extractResourceUri('http://example.com:80/path')).toBe('http://example.com:80/path');
-      // Preserves query parameters
-      expect(extractResourceUri('https://example.com/path?query=1')).toBe('https://example.com/path?query=1');
-      // Preserves trailing slashes
-      expect(extractResourceUri('https://example.com/')).toBe('https://example.com/');
-      expect(extractResourceUri('https://example.com/app1/')).toBe('https://example.com/app1/');
-    });
-
-    it('should distinguish between different paths on same domain', () => {
-      // This is the key test for the security concern mentioned
-      const app1 = extractResourceUri('https://api.example.com/mcp-server-1');
-      const app2 = extractResourceUri('https://api.example.com/mcp-server-2');
-      expect(app1).not.toBe(app2);
-      expect(app1).toBe('https://api.example.com/mcp-server-1');
-      expect(app2).toBe('https://api.example.com/mcp-server-2');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com/')).href).toBe('https://example.com/');
+      expect(resourceUrlFromServerUrl(new URL('https://example.com/path/')).href).toBe('https://example.com/path/');
     });
   });
 });

Original file line number	Diff line number	Diff line change
`@@ -122,7 +122,7 @@ export class DemoInMemoryAuthProvider implements OAuthServerProvider {`
`122`	`122`	`client: OAuthClientInformationFull,`
`123`	`123`	`_refreshToken: string,`
`124`	`124`	`_scopes?: string[],`
`125`		`- resource?: string`
	`125`	`+ resource?: URL`
`126`	`126`	`): Promise<OAuthTokens> {`
`127`	`127`	`throw new Error('Refresh tokens not implemented for example demo');`
`128`	`128`	`}`