update resource to be a url

This commit is contained in:
Olivier Chafik 2025-06-17 00:18:33 +01:00
commit 224a2e242d
No known key found for this signature in database
8 changed files with 33 additions and 92 deletions

View file

@ -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, {

View file

@ -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');
}

View file

@ -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)
);

View file

@ -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

View file

@ -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.

View file

@ -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
);

View file

@ -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({

View file

@ -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/');
});
});
});