Skip to content

fix: preserve canonical URL format in OAuth resource parameter per MCP auth spec #829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xiaoyijun
Copy link
Contributor

Fix OAuth resource parameter URL handling to preserve canonical URL format and prevent automatic trailing slash addition, and also ensuring compliance with MCP auth specification requirements.

Motivation and Context

When using the MCP Client SDK, I discovered that resource indicators from MCP server backend's resource metadata were automatically getting trailing slashes appended, causing authentication server validation failures when checking the resource indicator.

This issue occurred because the previous implementation used URL.href which automatically normalizes URLs by:

  1. Adding trailing slashes to domain-only URLs (e.g., https://example.com becomes https://example.com/)
  2. Converting schemes and host components to lowercase (e.g., HTTPS://EXAMPLE.COM becomes https://example.com/)

Both behaviors violate the MCP auth specification requirements. According to the MCP auth specification:

While both https://mcp.example.com/ (with trailing slash) and https://mcp.example.com (without trailing slash) are technically valid absolute URIs according to RFC 3986, implementations SHOULD consistently use the form without the trailing slash for better interoperability unless the trailing slash is semantically significant for the specific resource.
While the canonical form uses lowercase scheme and host components, implementations SHOULD accept uppercase scheme and host components for robustness and interoperability.

How Has This Been Tested?

  • Unit tests: Added comprehensive test coverage for resourceUrlFromServerUrl function including:

    • URL format preservation (with/without trailing slashes)
    • Fragment removal with various URL structures
    • Case preservation for uppercase schemes and host components
    • Edge cases (empty fragments, IPv6 addresses, complex URLs)
    • Comparison tests demonstrating differences from URL.href behavior
  • OAuth flow integration tests: Added test suite "resource URL handling (trailing slash preservation)" in src/client/auth.test.ts:

    • preserves server URLs without trailing slash in resource parameter - verifies authorization flow
    • preserves server URLs with trailing slash in resource parameter - verifies authorization flow
    • handles token exchange with preserved resource URL format - verifies token exchange flow
  • Existing tests: All 667 existing tests pass, ensuring no regressions in:

    • OAuth authorization flows
    • Client transport functionality (SSE and StreamableHTTP)
    • Auth utilities and helper functions
  • Build verification: TypeScript compilation and ESLint checks pass

Breaking Changes

Minor breaking change: The exported function signatures in the auth module have changed from accepting URL objects to string parameters to ensure MCP spec compliance:

  • validateResourceURL?(serverUrl: string, resource?: string) (was URL objects)
  • selectResourceURL(serverUrl: string, ...) (was URL object)
  • resourceUrlFromServerUrl(url: string) (was URL object)

Impact: Most users are unlikely to be affected as these functions are primarily used internally by the SDK. The change was necessary because the previous implementation with URL objects automatically normalized URLs in ways that violated the MCP auth specification requirements.

Migration: If you were directly using these functions, update calls to pass string URLs instead of URL objects. Use .href property if converting from URL objects.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@xiaoyijun
Copy link
Contributor Author

It's strange, the CI failed. All tests passed in my local machine.
image

@pcarleton pcarleton requested review from pcarleton and removed request for ochafik and ihrpr July 31, 2025 14:50
@pcarleton pcarleton self-assigned this Jul 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants