Skip to content

Commit 359ab60

Browse files
committed
Keep re-authorization on its own request's protocol version
Releasing context.lock before the protected request is yielded let a second request restamp the shared context.protocol_version while the first was in flight. The first request's 401 or 403 re-authorization then ran on the other request's MCP-Protocol-Version, which decides whether the resource parameter is sent. Holding the lock across the yield used to hide this. Read the header into a local once, and restamp context.protocol_version from it inside the 401 and 403 blocks after the lock is re-acquired. Every read of context.protocol_version happens under the lock in one of those regions, so this needs no new parameter on the token-request helpers. Also wrap the concurrency test's call_done.wait() in anyio.fail_after(5) so a failure in the sibling task fails fast instead of hanging.
1 parent c3c6e59 commit 359ab60

2 files changed

Lines changed: 75 additions & 3 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,12 +579,14 @@ async def _validate_resource_match(self, prm: ProtectedResourceMetadata) -> None
579579

580580
async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
581581
"""httpx2 auth flow integration."""
582+
# Capture protocol version from request headers
583+
protocol_version = request.headers.get(MCP_PROTOCOL_VERSION_HEADER)
584+
582585
async with self.context.lock:
583586
if not self._initialized:
584587
await self._initialize()
585588

586-
# Capture protocol version from request headers
587-
self.context.protocol_version = request.headers.get(MCP_PROTOCOL_VERSION_HEADER)
589+
self.context.protocol_version = protocol_version
588590

589591
if not self.context.is_token_valid() and self.context.can_refresh_token():
590592
# Try to refresh token
@@ -605,6 +607,10 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
605607

606608
if response.status_code == 401:
607609
async with self.context.lock:
610+
# Another request may have stamped its own version while this one was in
611+
# flight; re-authorization has to use the version this request carried.
612+
self.context.protocol_version = protocol_version
613+
608614
# Perform full OAuth flow
609615
try:
610616
# OAuth flow must be inline due to generator constraints
@@ -759,6 +765,8 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
759765
yield request
760766
elif response.status_code == 403:
761767
async with self.context.lock:
768+
self.context.protocol_version = protocol_version
769+
762770
# Step 1: Extract error field from WWW-Authenticate header
763771
error = extract_field_from_www_auth(response, "error")
764772

tests/client/test_auth.py

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3278,7 +3278,8 @@ async def get_sse_stream() -> None:
32783278
request = await flow.__anext__()
32793279
sse_sent.set()
32803280
# The server holds the stream open, so the response lands after the call is answered.
3281-
await call_done.wait()
3281+
with anyio.fail_after(5):
3282+
await call_done.wait()
32823283
with pytest.raises(StopAsyncIteration):
32833284
await flow.asend(httpx2.Response(200, request=request))
32843285

@@ -3295,3 +3296,66 @@ async def call_tool() -> None:
32953296
async with anyio.create_task_group() as tg:
32963297
tg.start_soon(get_sse_stream)
32973298
tg.start_soon(call_tool)
3299+
3300+
3301+
@pytest.mark.anyio
3302+
async def test_step_up_uses_the_protocol_version_of_its_own_request(
3303+
oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
3304+
):
3305+
"""Re-authorization must use the protocol version of the request that was challenged.
3306+
3307+
``context.protocol_version`` is shared and the lock is no longer held across the
3308+
protected request, so a second request can stamp its own version in between and
3309+
otherwise flip the ``resource`` parameter for the first one.
3310+
"""
3311+
oauth_provider.context.current_tokens = valid_tokens
3312+
oauth_provider.context.token_expiry_time = time.time() + 1800
3313+
oauth_provider.context.client_info = OAuthClientInformationFull(
3314+
client_id="test_client_id",
3315+
client_secret="test_client_secret",
3316+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
3317+
)
3318+
oauth_provider._initialized = True
3319+
3320+
captured_state: str | None = None
3321+
3322+
async def capture_redirect(url: str) -> None:
3323+
nonlocal captured_state
3324+
captured_state = parse_qs(urlparse(url).query).get("state", [None])[0]
3325+
3326+
async def mock_callback() -> AuthorizationCodeResult:
3327+
return AuthorizationCodeResult(code="auth_code", state=captured_state)
3328+
3329+
oauth_provider.context.redirect_handler = capture_redirect
3330+
oauth_provider.context.callback_handler = mock_callback
3331+
3332+
flow = oauth_provider.async_auth_flow(
3333+
httpx2.Request("GET", "https://api.example.com/v1/mcp", headers={"mcp-protocol-version": "2025-06-18"})
3334+
)
3335+
request = await flow.__anext__()
3336+
3337+
# A request on an older protocol version goes out while the first one is in flight.
3338+
other = oauth_provider.async_auth_flow(
3339+
httpx2.Request("POST", "https://api.example.com/v1/mcp", headers={"mcp-protocol-version": "2025-03-26"})
3340+
)
3341+
await other.__anext__()
3342+
await other.aclose()
3343+
3344+
response_403 = httpx2.Response(
3345+
403,
3346+
headers={"WWW-Authenticate": 'Bearer error="insufficient_scope", scope="admin:write"'},
3347+
request=request,
3348+
)
3349+
token_exchange_request = await flow.asend(response_403)
3350+
3351+
assert "resource=" in token_exchange_request.content.decode()
3352+
3353+
# Drive the flow to completion so the context lock is released cleanly
3354+
token_response = httpx2.Response(
3355+
200,
3356+
json={"access_token": "new", "token_type": "Bearer", "expires_in": 3600, "scope": "admin:write"},
3357+
request=token_exchange_request,
3358+
)
3359+
final_request = await flow.asend(token_response)
3360+
with pytest.raises(StopAsyncIteration):
3361+
await flow.asend(httpx2.Response(200, request=final_request))

0 commit comments

Comments
 (0)