Skip to content

Commit de38133

Browse files
committed
Fix OAuth discovery fallback and URL ordering
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
1 parent 7d12e83 commit de38133

File tree

3 files changed

+56
-35
lines changed

3 files changed

+56
-35
lines changed

src/mcp/client/auth/oauth2.py

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@
2121

2222
from mcp.client.auth import OAuthFlowError, OAuthTokenError
2323
from mcp.client.auth.utils import (
24-
build_protected_resource_discovery_urls,
24+
build_oauth_authorization_server_metadata_discovery_urls,
25+
build_protected_resource_metadata_discovery_urls,
2526
create_client_registration_request,
2627
create_oauth_metadata_request,
2728
extract_field_from_www_auth,
2829
extract_resource_metadata_from_www_auth,
2930
extract_scope_from_www_auth,
3031
get_client_metadata_scopes,
31-
get_discovery_urls,
3232
handle_auth_metadata_response,
3333
handle_protected_resource_response,
3434
handle_registration_response,
@@ -463,34 +463,34 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
463463
www_auth_resource_metadata_url = extract_resource_metadata_from_www_auth(response)
464464

465465
# Step 1: Discover protected resource metadata (SEP-985 with fallback support)
466-
prm_discovery_urls = build_protected_resource_discovery_urls(
466+
prm_discovery_urls = build_protected_resource_metadata_discovery_urls(
467467
www_auth_resource_metadata_url, self.context.server_url
468468
)
469-
prm_discovery_success = False
469+
470470
for url in prm_discovery_urls: # pragma: no branch
471471
discovery_request = create_oauth_metadata_request(url)
472472

473473
discovery_response = yield discovery_request # sending request
474474

475475
prm = await handle_protected_resource_response(discovery_response)
476476
if prm:
477-
prm_discovery_success = True
478-
479-
# saving the response metadata
480477
self.context.protected_resource_metadata = prm
481-
if prm.authorization_servers: # pragma: no branch
482-
self.context.auth_server_url = str(prm.authorization_servers[0])
483478

479+
# todo: try all authorization_servers to find the OASM
480+
assert (
481+
len(prm.authorization_servers) > 0
482+
) # this is always true as authorization_servers has a min length of 1
483+
484+
self.context.auth_server_url = str(prm.authorization_servers[0])
484485
break
485486
else:
486487
logger.debug(f"Protected resource metadata discovery failed: {url}")
487-
if not prm_discovery_success:
488-
raise OAuthFlowError(
489-
"Protected resource metadata discovery failed: no valid metadata found"
490-
) # pragma: no cover
491488

492-
# Step 2: Discover OAuth metadata (with fallback for legacy servers)
493-
asm_discovery_urls = get_discovery_urls(self.context.auth_server_url or self.context.server_url)
489+
asm_discovery_urls = build_oauth_authorization_server_metadata_discovery_urls(
490+
self.context.auth_server_url, self.context.server_url
491+
)
492+
493+
# Step 2: Discover OAuth Authorization Server Metadata (OASM) (with fallback for legacy servers)
494494
for url in asm_discovery_urls: # pragma: no cover
495495
oauth_metadata_request = create_oauth_metadata_request(url)
496496
oauth_metadata_response = yield oauth_metadata_request

src/mcp/client/auth/utils.py

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def extract_resource_metadata_from_www_auth(response: Response) -> str | None:
6464
return extract_field_from_www_auth(response, "resource_metadata")
6565

6666

67-
def build_protected_resource_discovery_urls(www_auth_url: str | None, server_url: str) -> list[str]:
67+
def build_protected_resource_metadata_discovery_urls(www_auth_url: str | None, server_url: str) -> list[str]:
6868
"""
6969
Build ordered list of URLs to try for protected resource metadata discovery.
7070
@@ -126,8 +126,21 @@ def get_client_metadata_scopes(
126126
return None
127127

128128

129-
def get_discovery_urls(auth_server_url: str) -> list[str]:
130-
"""Generate ordered list of (url, type) tuples for discovery attempts."""
129+
def build_oauth_authorization_server_metadata_discovery_urls(auth_server_url: str | None, server_url: str) -> list[str]:
130+
"""
131+
Generate ordered list of (url, type) tuples for discovery attempts.
132+
133+
Args:
134+
auth_server_url: URL for the OAuth Authorization Metadata URL if found, otherwise None
135+
server_url: URL for the MCP server, used as a fallback if auth_server_url is None
136+
"""
137+
138+
if not auth_server_url:
139+
# Legacy path using the 2025-03-26 spec:
140+
# link: https://modelcontextprotocol.io/specification/2025-03-26/basic/authorization
141+
parsed = urlparse(server_url)
142+
return [f"{parsed.scheme}://{parsed.netloc}/.well-known/oauth-authorization-server"]
143+
131144
urls: list[str] = []
132145
parsed = urlparse(auth_server_url)
133146
base_url = f"{parsed.scheme}://{parsed.netloc}"
@@ -137,18 +150,22 @@ def get_discovery_urls(auth_server_url: str) -> list[str]:
137150
oauth_path = f"/.well-known/oauth-authorization-server{parsed.path.rstrip('/')}"
138151
urls.append(urljoin(base_url, oauth_path))
139152

140-
# OAuth root fallback
141-
urls.append(urljoin(base_url, "/.well-known/oauth-authorization-server"))
142-
143-
# RFC 8414 section 5: Path-aware OIDC discovery
144-
# See https://www.rfc-editor.org/rfc/rfc8414.html#section-5
145-
if parsed.path and parsed.path != "/":
153+
# RFC 8414 section 5: Path-aware OIDC discovery
154+
# See https://www.rfc-editor.org/rfc/rfc8414.html#section-5
146155
oidc_path = f"/.well-known/openid-configuration{parsed.path.rstrip('/')}"
147156
urls.append(urljoin(base_url, oidc_path))
148157

158+
# https://openid.net/specs/openid-connect-discovery-1_0.html
159+
oidc_path = f"{parsed.path.rstrip('/')}/.well-known/openid-configuration"
160+
urls.append(urljoin(base_url, oidc_path))
161+
return urls
162+
163+
# OAuth root
164+
urls.append(urljoin(base_url, "/.well-known/oauth-authorization-server"))
165+
149166
# OIDC 1.0 fallback (appends to full URL per OIDC spec)
150-
oidc_fallback = f"{auth_server_url.rstrip('/')}/.well-known/openid-configuration"
151-
urls.append(oidc_fallback)
167+
# https://openid.net/specs/openid-connect-discovery-1_0.html
168+
urls.append(urljoin(base_url, "/.well-known/openid-configuration"))
152169

153170
return urls
154171

tests/client/test_auth.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,13 @@
1212

1313
from mcp.client.auth import OAuthClientProvider, PKCEParameters
1414
from mcp.client.auth.utils import (
15-
build_protected_resource_discovery_urls,
15+
build_oauth_authorization_server_metadata_discovery_urls,
16+
build_protected_resource_metadata_discovery_urls,
1617
create_oauth_metadata_request,
1718
extract_field_from_www_auth,
1819
extract_resource_metadata_from_www_auth,
1920
extract_scope_from_www_auth,
2021
get_client_metadata_scopes,
21-
get_discovery_urls,
2222
handle_registration_response,
2323
)
2424
from mcp.shared.auth import OAuthClientInformationFull, OAuthClientMetadata, OAuthToken, ProtectedResourceMetadata
@@ -275,7 +275,7 @@ async def callback_handler() -> tuple[str, str | None]:
275275
status_code=401, headers={}, request=httpx.Request("GET", "https://request-api.example.com")
276276
)
277277

278-
urls = build_protected_resource_discovery_urls(
278+
urls = build_protected_resource_metadata_discovery_urls(
279279
extract_resource_metadata_from_www_auth(init_response), provider.context.server_url
280280
)
281281
assert len(urls) == 1
@@ -286,7 +286,7 @@ async def callback_handler() -> tuple[str, str | None]:
286286
'Bearer resource_metadata="https://prm.example.com/.well-known/oauth-protected-resource/path"'
287287
)
288288

289-
urls = build_protected_resource_discovery_urls(
289+
urls = build_protected_resource_metadata_discovery_urls(
290290
extract_resource_metadata_from_www_auth(init_response), provider.context.server_url
291291
)
292292
assert len(urls) == 2
@@ -309,12 +309,16 @@ class TestOAuthFallback:
309309

310310
@pytest.mark.anyio
311311
async def test_oauth_discovery_fallback_order(self, oauth_provider: OAuthClientProvider):
312-
"""Test fallback URL construction order."""
313-
discovery_urls = get_discovery_urls(oauth_provider.context.auth_server_url or oauth_provider.context.server_url)
312+
"""Test fallback URL construction order when auth server URL has a path."""
313+
# Simulate PRM discovery returning an auth server URL with a path
314+
oauth_provider.context.auth_server_url = oauth_provider.context.server_url
315+
316+
discovery_urls = build_oauth_authorization_server_metadata_discovery_urls(
317+
oauth_provider.context.auth_server_url, oauth_provider.context.server_url
318+
)
314319

315320
assert discovery_urls == [
316321
"https://api.example.com/.well-known/oauth-authorization-server/v1/mcp",
317-
"https://api.example.com/.well-known/oauth-authorization-server",
318322
"https://api.example.com/.well-known/openid-configuration/v1/mcp",
319323
"https://api.example.com/v1/mcp/.well-known/openid-configuration",
320324
]
@@ -1084,7 +1088,7 @@ async def callback_handler() -> tuple[str, str | None]:
10841088
)
10851089

10861090
# Build discovery URLs
1087-
discovery_urls = build_protected_resource_discovery_urls(
1091+
discovery_urls = build_protected_resource_metadata_discovery_urls(
10881092
extract_resource_metadata_from_www_auth(init_response), provider.context.server_url
10891093
)
10901094

@@ -1224,7 +1228,7 @@ async def callback_handler() -> tuple[str, str | None]:
12241228
)
12251229

12261230
# Build discovery URLs
1227-
discovery_urls = build_protected_resource_discovery_urls(
1231+
discovery_urls = build_protected_resource_metadata_discovery_urls(
12281232
extract_resource_metadata_from_www_auth(init_response), provider.context.server_url
12291233
)
12301234

0 commit comments

Comments
 (0)