Merge branch 'main' into request-propagation-in-tools
This commit is contained in:
commit
96ff8573cf
4 changed files with 2180 additions and 134 deletions
61
src/shared/auth-utils.test.ts
Normal file
61
src/shared/auth-utils.test.ts
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
import { resourceUrlFromServerUrl, checkResourceAllowed } from './auth-utils.js';
|
||||
|
||||
describe('auth-utils', () => {
|
||||
describe('resourceUrlFromServerUrl', () => {
|
||||
it('should remove fragments', () => {
|
||||
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(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(new URL('https://EXAMPLE.COM/PATH')).href).toBe('https://example.com/PATH');
|
||||
// Ports preserved
|
||||
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(new URL('https://example.com?foo=bar&baz=qux')).href).toBe('https://example.com/?foo=bar&baz=qux');
|
||||
// Trailing slashes preserved
|
||||
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/');
|
||||
});
|
||||
});
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
});
|
||||
54
src/shared/auth-utils.ts
Normal file
54
src/shared/auth-utils.ts
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
/**
|
||||
* Utilities for handling OAuth resource URIs.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Converts a server URL to a resource URL by removing the fragment.
|
||||
* 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 | 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);
|
||||
}
|
||||
|
|
@ -15,7 +15,7 @@ export function getDisplayName(metadata: BaseMetadata): string {
|
|||
if (metadata.title !== undefined && metadata.title !== '') {
|
||||
return metadata.title;
|
||||
}
|
||||
|
||||
|
||||
// Then check for annotations.title (only present in Tool objects)
|
||||
if ('annotations' in metadata) {
|
||||
const metadataWithAnnotations = metadata as BaseMetadata & { annotations?: { title?: string } };
|
||||
|
|
@ -23,14 +23,7 @@ export function getDisplayName(metadata: BaseMetadata): string {
|
|||
return metadataWithAnnotations.annotations.title;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
// Finally fall back to name
|
||||
return metadata.name;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if an object has a custom title different from its name.
|
||||
*/
|
||||
export function hasCustomTitle(metadata: BaseMetadata): boolean {
|
||||
return metadata.title !== undefined && metadata.title !== metadata.name;
|
||||
}
|
||||
|
|
@ -465,6 +465,7 @@ describe("mergeCapabilities", () => {
|
|||
experimental: {
|
||||
feature: true,
|
||||
},
|
||||
elicitation: {},
|
||||
roots: {
|
||||
newProp: true,
|
||||
},
|
||||
|
|
@ -473,6 +474,7 @@ describe("mergeCapabilities", () => {
|
|||
const merged = mergeCapabilities(base, additional);
|
||||
expect(merged).toEqual({
|
||||
sampling: {},
|
||||
elicitation: {},
|
||||
roots: {
|
||||
listChanged: true,
|
||||
newProp: true,
|
||||
|
|
|
|||
Loading…
Reference in a new issue