Skip to content

Commit aedc88c

Browse files
committed
fix(client/auth): finalize the discovery sub-generator when the flow is closed mid-discovery
httpx2's _send_handling_auth acloses the auth flow on any transport error or cancellation, throwing GeneratorExit at the relay's yield — caught by neither except StopAsyncIteration nor except Exception — so both hand-relay sites (the 401 branch and the 403 step-up) abandoned the suspended _discover_authorization_server_metadata sub-generator to GC (a ResourceWarning on trio, which filterwarnings=["error"] turns into a test failure for any future abort-mid-discovery test). Both relays now drive the sub-generator under contextlib.aclosing, matching the eager-refresh relay in #3263, so closing the outer flow finalizes it in the same unwind. Covered by two regression tests that close the flow mid-discovery and assert the captured sub-generator reports exhaustion instead of a live suspended frame.
1 parent 43d9599 commit aedc88c

2 files changed

Lines changed: 105 additions & 17 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import string
1111
import time
1212
from collections.abc import AsyncGenerator, Awaitable, Callable
13+
from contextlib import aclosing
1314
from dataclasses import dataclass, field
1415
from typing import Any, Protocol, get_args
1516
from urllib.parse import quote, urlencode, urljoin, urlparse
@@ -812,15 +813,17 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
812813
# metadata, applying the SEP-2352 issuer checks along the way. The
813814
# sequence lives in a sub-generator shared with the 403 step-up;
814815
# its requests are relayed by hand (`yield from` cannot cross an
815-
# async generator).
816-
discovery = self._discover_authorization_server_metadata(response)
817-
try:
818-
discovery_request = await anext(discovery)
819-
while True:
820-
discovery_response = yield discovery_request
821-
discovery_request = await discovery.asend(discovery_response)
822-
except StopAsyncIteration:
823-
pass
816+
# async generator). `aclosing` finalizes the sub-generator when
817+
# httpx2 closes this flow mid-discovery (transport error or
818+
# cancellation throws `GeneratorExit` at the relay's `yield`).
819+
async with aclosing(self._discover_authorization_server_metadata(response)) as discovery:
820+
try:
821+
discovery_request = await anext(discovery)
822+
while True:
823+
discovery_response = yield discovery_request
824+
discovery_request = await discovery.asend(discovery_response)
825+
except StopAsyncIteration:
826+
pass
824827

825828
# Step 3: Apply scope selection strategy
826829
self.context.client_metadata.scope = get_client_metadata_scopes(
@@ -884,14 +887,16 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
884887
if self.context.oauth_metadata is None and (
885888
self.context.registration_secret_expired() or self.context.client_info is None
886889
):
887-
discovery = self._discover_authorization_server_metadata(response)
888-
try:
889-
discovery_request = await anext(discovery)
890-
while True:
891-
discovery_response = yield discovery_request
892-
discovery_request = await discovery.asend(discovery_response)
893-
except StopAsyncIteration:
894-
pass
890+
# `aclosing` mirrors the 401 relay above: it finalizes the
891+
# sub-generator when httpx2 closes this flow mid-discovery.
892+
async with aclosing(self._discover_authorization_server_metadata(response)) as discovery:
893+
try:
894+
discovery_request = await anext(discovery)
895+
while True:
896+
discovery_response = yield discovery_request
897+
discovery_request = await discovery.asend(discovery_response)
898+
except StopAsyncIteration:
899+
pass
895900

896901
# Step 2a: Union previously requested scopes with the newly challenged
897902
# scopes (SEP-2350) so escalating one operation keeps the others' grants.

tests/client/test_auth.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import base64
44
import json
55
import time
6+
from collections.abc import AsyncGenerator
67
from unittest import mock
78
from urllib.parse import parse_qs, quote, unquote, urlparse
89

@@ -4031,3 +4032,85 @@ async def mock_callback() -> AuthorizationCodeResult:
40314032
await auth_flow.asend(httpx2.Response(200, request=final_request))
40324033
except StopAsyncIteration:
40334034
pass
4035+
4036+
4037+
@pytest.mark.anyio
4038+
async def test_closing_the_flow_mid_discovery_finalizes_the_401_discovery_sub_generator(
4039+
oauth_provider: OAuthClientProvider,
4040+
):
4041+
"""httpx2's `_send_handling_auth` acloses the auth flow when a discovery request
4042+
fails at the transport level (or the request is cancelled), throwing `GeneratorExit`
4043+
at the 401 relay's `yield`; the relay must finalize the discovery sub-generator with
4044+
the flow rather than abandon it suspended to GC (a `ResourceWarning` under trio).
4045+
The sub-generator is captured via a wrapper because its finalization is not
4046+
observable through the auth-flow protocol itself.
4047+
"""
4048+
captured: list[AsyncGenerator[httpx2.Request, httpx2.Response]] = []
4049+
original = oauth_provider._discover_authorization_server_metadata
4050+
4051+
def capturing(response: httpx2.Response) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
4052+
discovery = original(response)
4053+
captured.append(discovery)
4054+
return discovery
4055+
4056+
oauth_provider._discover_authorization_server_metadata = capturing
4057+
oauth_provider._initialized = True
4058+
4059+
auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
4060+
request = await auth_flow.__anext__()
4061+
4062+
# 401 → the flow yields the first discovery request, suspending both generators mid-relay.
4063+
prm_req = await auth_flow.asend(httpx2.Response(401, request=request))
4064+
assert "oauth-protected-resource" in str(prm_req.url)
4065+
assert len(captured) == 1
4066+
4067+
# Closing the flow here mirrors httpx2 aborting it on a transport error.
4068+
await auth_flow.aclose()
4069+
4070+
# The sub-generator was closed with the flow: probing it reports exhaustion instead
4071+
# of resuming a suspended frame.
4072+
with pytest.raises(StopAsyncIteration):
4073+
await captured[0].__anext__()
4074+
4075+
4076+
@pytest.mark.anyio
4077+
async def test_closing_the_flow_mid_discovery_finalizes_the_403_step_up_discovery_sub_generator(
4078+
oauth_provider: OAuthClientProvider,
4079+
):
4080+
"""The 403 step-up's discovery relay must finalize its sub-generator when httpx2
4081+
acloses the flow mid-discovery, exactly like the 401 relay (same abandonment
4082+
hazard, same fix); the sub-generator is captured via a wrapper because its
4083+
finalization is not observable through the auth-flow protocol itself.
4084+
"""
4085+
captured: list[AsyncGenerator[httpx2.Request, httpx2.Response]] = []
4086+
original = oauth_provider._discover_authorization_server_metadata
4087+
4088+
def capturing(response: httpx2.Response) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
4089+
discovery = original(response)
4090+
captured.append(discovery)
4091+
return discovery
4092+
4093+
oauth_provider._discover_authorization_server_metadata = capturing
4094+
# Restart shape: a live token and no stored registration, so the 403 step-up must
4095+
# discover before registering (`oauth_metadata` is never restored by `_initialize`).
4096+
oauth_provider.context.current_tokens = OAuthToken(access_token="live-token")
4097+
oauth_provider._initialized = True
4098+
4099+
auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
4100+
request = await auth_flow.__anext__()
4101+
4102+
response_403 = httpx2.Response(
4103+
403,
4104+
headers={"WWW-Authenticate": 'Bearer error="insufficient_scope", scope="write"'},
4105+
request=request,
4106+
)
4107+
4108+
# 403 → the step-up yields the first discovery request, suspending both generators mid-relay.
4109+
prm_req = await auth_flow.asend(response_403)
4110+
assert "oauth-protected-resource" in str(prm_req.url)
4111+
assert len(captured) == 1
4112+
4113+
await auth_flow.aclose()
4114+
4115+
with pytest.raises(StopAsyncIteration):
4116+
await captured[0].__anext__()

0 commit comments

Comments
 (0)