Skip to content

Conversation

ihrpr
Copy link
Contributor

@ihrpr ihrpr commented Jun 24, 2025

As identified in modelcontextprotocol/typescript-sdk#687 we need fix it in Python SDK as well

Fixed OAuth discovery URL construction to comply with RFC 8414 by preserving path components. Previously, /.well-known/oauth-authorization-server discovery was dropping the path portion of authorization server URLs (e.g.,
https://example.com/path/mcp became https://example.com/.well-known/... instead of https://example.com/.well-known/.../path/mcp).

This fix enables operation with auth servers hosted at subpaths, matching the same issue recently fixed in the TypeScript SDK.

Fallback for backwards compatibility:

@ihrpr ihrpr requested a review from pcarleton June 24, 2025 10:05
pcarleton
pcarleton previously approved these changes Jun 24, 2025
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.

LGTM!

def _build_well_known_path(self, pathname: str) -> str:
"""Construct well-known path for OAuth metadata discovery."""
well_known_path = f"/.well-known/oauth-authorization-server{pathname}"
if pathname.endswith("/"):
Copy link
Member

Choose a reason for hiding this comment

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

This tripped me up for a minute, b/c i thought it was referring to // at the end, but I see you have a test for it, and it follow this from the RFC:

If the issuer identifier value contains a path component, any
terminating "/" MUST be removed before inserting "/.well-known/" and
the well-known URI suffix between the host component and the path
component. The client would make the following request when the
issuer identifier is "https://example.com/issuer1" and the well-known
URI suffix is "oauth-authorization-server" to obtain the metadata,
since the issuer identifier contains a path component:

GET /.well-known/oauth-authorization-server/issuer1 HTTP/1.1 Host: example.com

Base automatically changed from ihrpr/auth-cleanup to main June 24, 2025 14:43
@ihrpr ihrpr dismissed pcarleton’s stale review June 24, 2025 14:43

The base branch was changed.

@ihrpr ihrpr merged commit 6747688 into main Jun 24, 2025
8 of 9 checks passed
@ihrpr ihrpr deleted the ihrpr/fix-dropping-path branch June 24, 2025 14:43
saqadri pushed a commit to saqadri/stdio-fixes that referenced this pull request Aug 6, 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