Skip to content

Conversation

@maxisbey
Copy link
Contributor

Motivation and Context

This PR addresses two related OAuth discovery issues that affect backward compatibility and correctness:

Issue #1495: Legacy Server Compatibility

Version 1.18.0 broke compatibility with legacy MCP servers (like Linear and Atlassian) that don't yet support Protected Resource Metadata (PRM) endpoints. The implementation raised an exception when PRM discovery failed, preventing fallback to the March 2025 spec behavior.

Issue #1623: Incorrect OAuth Discovery URL Priority

The OAuth discovery URL ordering was incorrect, causing the system to try root-level URLs before path-based OIDC URLs. This broke authentication with services like Jira MCP where tenant-specific authorization servers should take precedence over root-level servers.

Changes

1. Enable Fallback to March 2025 Spec (Issue #1495)

  • Removed exception when PRM discovery fails completely
  • Added fallback logic to use root-based OAuth discovery for backward compatibility
  • When PRM unavailable, only check /.well-known/oauth-authorization-server at the root
  • Maintains compatibility with legacy servers like Linear (https://mcp.linear.app/sse) and Atlassian (https://mcp.atlassian.com/v1/sse)

2. Fix OAuth Discovery URL Ordering (Issue #1623)

  • Path-based discovery only when needed: Only check path-based URLs when the authorization server URL (from PRM) contains a path
  • Prevents incorrect root discovery: Avoids mistakenly discovering root AS when tenant-specific AS exists (e.g., Okta multi-tenant setups)
  • Follows RFC 8414 priority: Path-aware OAuth → Path-aware OIDC → OIDC appended format → Root (only when no path)
  • Fixes TypeScript SDK parity issue: Aligns with the fix made in TypeScript SDK v1.21.1

Implementation Details

Function Renaming:

  • build_protected_resource_discovery_urlsbuild_protected_resource_metadata_discovery_urls
  • get_discovery_urlsbuild_oauth_authorization_server_metadata_discovery_urls

New Behavior:

  • build_oauth_authorization_server_metadata_discovery_urls(auth_server_url: str | None, server_url: str)
    • Legacy path (auth_server_url is None): Only try {root}/.well-known/oauth-authorization-server
    • Path-aware (auth_server_url has path like /tenant1): Only try path-based URLs in priority order
    • Root-only (auth_server_url is root like https://auth.example.com): Only try root URLs

Examples

Example 1: Legacy Server (No PRM)

Server: https://mcp.linear.app/sse
1. Try PRM URLs (all fail):
   - /.well-known/oauth-protected-resource/sse
   - /.well-known/oauth-protected-resource
2. Fall back to legacy OAuth discovery:
   - https://mcp.linear.app/.well-known/oauth-authorization-server ✓

Example 2: Path-Aware Discovery

Server: https://api.example.com/v1/mcp
PRM returns: https://auth.example.com/tenant1
1. Try path-aware URLs only:
   - https://auth.example.com/.well-known/oauth-authorization-server/tenant1
   - https://auth.example.com/.well-known/openid-configuration/tenant1
   - https://auth.example.com/tenant1/.well-known/openid-configuration
2. Does NOT try root (prevents wrong AS discovery)

Example 3: Root Authorization Server

Server: https://api.example.com/mcp
PRM returns: https://auth.example.com
1. Try root URLs only:
   - https://auth.example.com/.well-known/oauth-authorization-server
   - https://auth.example.com/.well-known/openid-configuration

Breaking Changes

Some invalid server configurations will no longer work (these violate RFC specifications):

  1. No PRM available AND OASM at a path other than root
  2. PRM returns auth server URL with path, but OASM only exists at root

These configurations are not expected to exist in practice.

Testing

  • Updated existing tests to use new function signatures
  • Added test for path-aware discovery behavior
  • All pre-commit hooks pass (formatting, linting, type checking)

Additional Context

  • Aligns with SEP-985 (Align OAuth 2.0 Protected Resource Metadata with RFC 9728)
  • Follows RFC 8414 discovery priorities
  • Matches TypeScript SDK behavior after their similar fix
  • Community consensus from issue discussions supports this approach

Related Issues

Fixes #1495
Fixes #1623

This commit addresses two related OAuth discovery issues:

1. Enable fallback to March 2025 spec for legacy servers (issue #1495)
   - Remove exception when PRM discovery fails completely
   - Fall back to root-level OAuth discovery for backward compatibility
   - When PRM unavailable, only check /.well-known/oauth-authorization-server
   - Maintains compatibility with legacy servers like Linear and Atlassian

2. Fix OAuth discovery URL ordering (issue #1623)
   - Only check path-based URLs when auth server URL contains a path
   - Prevents incorrectly discovering root AS when tenant-specific AS exists
   - Follows RFC 8414 priority: path-aware OAuth, then path-aware OIDC, then root
   - Fixes issue where root URLs were tried before path-based OIDC URLs

Changes:
- Renamed build_protected_resource_discovery_urls to build_protected_resource_metadata_discovery_urls
- Renamed get_discovery_urls to build_oauth_authorization_server_metadata_discovery_urls
- New function signature accepts optional auth_server_url and required server_url
- Legacy behavior: when auth_server_url is None, only try root URL
- Path-aware behavior: when auth_server_url has path, only try path-based URLs
- Root behavior: when auth_server_url has no path, only try root URLs

Breaking changes:
- Some invalid server configurations will no longer work:
  - No PRM available and OASM at a path other than root
  - PRM returns auth server URL with path, but OASM only at root

These configurations violate RFC specifications and are not expected to exist.

Github-Issue: #1495
Github-Issue: #1623
This commit adds and updates tests to properly cover the new OAuth discovery
logic for legacy server compatibility and path-aware discovery.

New tests:
- test_oauth_discovery_legacy_fallback_when_no_prm: Verifies that when
  PRM discovery fails, only root OAuth URL is tried (March 2025 spec)
- test_oauth_discovery_path_aware_when_auth_server_has_path: Ensures
  path-based URLs are tried when auth server URL has a path
- test_oauth_discovery_root_when_auth_server_has_no_path: Ensures
  root URLs are tried when auth server URL has no path
- test_oauth_discovery_root_when_auth_server_has_only_slash: Handles
  trailing slash edge case
- test_legacy_server_no_prm_falls_back_to_root_oauth_discovery: End-to-end
  test simulating Linear-style legacy servers
- test_legacy_server_with_different_prm_and_root_urls: Tests fallback
  with custom WWW-Authenticate PRM URLs

Updated tests:
- test_oauth_discovery_fallback_conditions: Updated to reflect new
  path-aware discovery behavior (no root URLs when auth server has path)
- test_oauth_discovery_fallback_order: Simplified to focus on path-aware case

All tests pass with proper linting and type checking.

Github-Issue: #1495
Github-Issue: #1623
@maxisbey maxisbey requested a review from pcarleton November 13, 2025 18:09
Copy link
Member

@pcarleton pcarleton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, LGTM

@pcarleton pcarleton merged commit 91ccdb3 into main Nov 13, 2025
21 checks passed
@pcarleton pcarleton deleted the fix-oauth-discovery-fallback branch November 13, 2025 19:37
LucaButBoring pushed a commit to LucaButBoring/mcp-python-sdk that referenced this pull request Nov 17, 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.

Review OAuth discovery URL priority order for path-based vs root-based OIDC Protected Resource Metadata request failed: 404

3 participants