Skip to content

Commit 22d3735

Browse files
committed
Apply the redirect rule to OAuth's own requests; clearer message for an https-to-http redirect
Second round of review follow-ups for the origin-scoped redirect handling: - The rule "follow a redirect only within the request's origin, only when it keeps the method" now lives in one predicate, next_request_within_origin, which also declines a Location that carries userinfo (httpx2 would send it as Basic auth). - OAuthClientProvider and IdentityAssertionOAuthProvider apply that rule to the requests their flows make (metadata discovery, registration, token, refresh) through a small RedirectAwareAuth base, instead of those requests following nothing. A redirect elsewhere is still handed to the flow as a non-success, and the registration/token/refresh errors now name it. - When the redirect budget (the client's max_redirects) is spent, the last redirect is handed back unfollowed like any other, so a redirect loop fails the one call with the usual error instead of raising TooManyRedirects out of the transport. - The "not followed" error for an https endpoint redirected to plain http on the same host explains the likely cause (a TLS-terminating proxy the server does not trust, often plus a trailing slash) and suggests the https form of the location rather than the http one; locations are printed without query or userinfo. - docs: the OAuth and transports pages describe the shared rule; the ASGI mounting example points clients at /notes/ (the path that does not redirect).
1 parent 4a4b8fb commit 22d3735

12 files changed

Lines changed: 300 additions & 84 deletions

File tree

‎docs/client/oauth-clients.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ The first time `Client` sends a request, the server answers `401`. The provider
8383

8484
After that it is quiet. Tokens come out of storage, an expired access token is refreshed with the refresh token, and only when none of that works does it run the flow again.
8585

86-
One transport rule applies to all of these requests: they are made while an MCP request is in flight, and like it they do not follow redirects to other addresses (they follow none at all), so the metadata, registration and token URLs must answer directly.
86+
One transport rule applies to all of these requests: like the MCP request they run inside, they follow a redirect only when it stays on the same origin and keeps the method (a trailing-slash 307/308, say), and treat any other redirect as that URL not answering.
8787

8888
You wrote none of it. Two keyword arguments remain (`client_metadata_url` and `validate_resource_url`), and this file needs neither. `client_metadata_url` is the one worth knowing about; it gets its own section below.
8989

‎docs/client/transports.md‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,8 @@ environment variables or pass an explicit `verify=ssl_context` to your `httpx2.A
7676
`httpx2` keeps the familiar `httpx` API, so if you know `httpx` you already know how to do auth,
7777
proxies, event hooks, retries and connection limits here. The SDK adds nothing on top and takes
7878
nothing away, with one exception: redirects. MCP requests follow the same-origin rule above rather
79-
than the client's `follow_redirects`, and requests an `auth=` handler makes while one is in flight
80-
(OAuth discovery, registration, token) do not follow redirects, so those URLs must answer directly.
81-
It is also where OAuth plugs in:
79+
than the client's `follow_redirects`, and the requests the SDK's OAuth providers make while one is
80+
in flight (discovery, registration, token) follow that rule too. It is also where OAuth plugs in:
8281
`httpx2.AsyncClient(auth=OAuthClientProvider(...))`. That whole flow is **[OAuth clients](oauth-clients.md)**.
8382

8483
## stdio

‎docs/run/asgi.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ That trailing `/mcp` is `streamable_http_path`. Set it to `"/"` and the mount pr
9494
--8<-- "docs_src/asgi/tutorial004.py"
9595
```
9696

97-
Now clients connect to `/notes`, not `/notes/mcp`.
97+
Now clients connect to `/notes/`, not `/notes/mcp`.
9898

9999
## CORS for browser clients
100100

‎src/mcp/client/auth/extensions/identity_assertion.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
union_scopes,
4040
validate_metadata_issuer,
4141
)
42+
from mcp.shared._httpx_utils import RedirectAwareAuth, redirect_note
4243
from mcp.shared.auth import JWT_BEARER_GRANT_TYPE, OAuthClientInformationFull, OAuthToken
4344
from mcp.shared.auth_utils import calculate_token_expiry, resource_url_from_server_url
4445

@@ -56,7 +57,7 @@ def _origin(url: str) -> tuple[str, str, int | None]:
5657
return (parsed.scheme, parsed.hostname or "", port)
5758

5859

59-
class IdentityAssertionOAuthProvider(httpx2.Auth):
60+
class IdentityAssertionOAuthProvider(RedirectAwareAuth):
6061
"""`httpx2.Auth` for the SEP-990 ID-JAG flow (RFC 7523 jwt-bearer grant) against a configured AS.
6162
6263
The authorization server `issuer` is fixed at construction; metadata is fetched from its
@@ -159,7 +160,7 @@ def _build_token_request(self, scope: str | None, assertion: str) -> httpx2.Requ
159160
data["client_secret"] = self._client.client_secret
160161
return httpx2.Request("POST", self._token_endpoint, data=data, headers=headers)
161162

162-
async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
163+
async def _auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
163164
async with self._lock:
164165
if not self._initialized:
165166
self._tokens = await self._storage.get_tokens()
@@ -201,7 +202,9 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
201202
token_response = yield self._build_token_request(scope_to_request, assertion)
202203
if token_response.status_code != 200:
203204
body = (await token_response.aread()).decode(errors="replace")
204-
raise OAuthTokenError(f"Token exchange failed ({token_response.status_code}): {body}")
205+
raise OAuthTokenError(
206+
f"Token exchange failed ({token_response.status_code}){redirect_note(token_response)}: {body}"
207+
)
205208
tokens = await handle_token_response_scopes(token_response)
206209
if tokens.scope is None:
207210
tokens.scope = scope_to_request

‎src/mcp/client/auth/oauth2.py‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
validate_authorization_response_iss,
4242
validate_metadata_issuer,
4343
)
44+
from mcp.shared._httpx_utils import RedirectAwareAuth, redirect_note
4445
from mcp.shared.auth import (
4546
AuthorizationCodeResult,
4647
OAuthClientInformationFull,
@@ -276,7 +277,7 @@ def prepare_token_auth(
276277
return data, headers
277278

278279

279-
class OAuthClientProvider(httpx2.Auth):
280+
class OAuthClientProvider(RedirectAwareAuth):
280281
"""OAuth2 authentication for httpx2.
281282
282283
Handles OAuth flow with automatic client registration and token storage.
@@ -469,7 +470,9 @@ async def _handle_token_response(self, response: httpx2.Response) -> None:
469470
if response.status_code not in {200, 201}:
470471
body = await response.aread()
471472
body_text = body.decode("utf-8")
472-
raise OAuthTokenError(f"Token exchange failed ({response.status_code}): {body_text}")
473+
raise OAuthTokenError(
474+
f"Token exchange failed ({response.status_code}){redirect_note(response)}: {body_text}"
475+
)
473476

474477
# Parse and validate response with scope validation
475478
token_response = await handle_token_response_scopes(response)
@@ -519,7 +522,7 @@ async def _refresh_token(self) -> httpx2.Request:
519522
async def _handle_refresh_response(self, response: httpx2.Response) -> bool:
520523
"""Handle token refresh response. Returns True if successful."""
521524
if response.status_code != 200:
522-
logger.warning(f"Token refresh failed: {response.status_code}")
525+
logger.warning(f"Token refresh failed: {response.status_code}{redirect_note(response)}")
523526
self.context.clear_tokens()
524527
return False
525528

@@ -577,8 +580,8 @@ async def _validate_resource_match(self, prm: ProtectedResourceMetadata) -> None
577580
if not check_resource_allowed(requested_resource=default_resource, configured_resource=prm_resource):
578581
raise OAuthFlowError(f"Protected resource {prm_resource} does not match expected {default_resource}")
579582

580-
async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
581-
"""httpx2 auth flow integration."""
583+
async def _auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
584+
"""The OAuth flow proper; `async_auth_flow` drives it (see `RedirectAwareAuth`)."""
582585
async with self.context.lock:
583586
if not self._initialized:
584587
await self._initialize()

‎src/mcp/client/auth/utils.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from pydantic_core import from_json
99

1010
from mcp.client.auth import OAuthFlowError, OAuthRegistrationError, OAuthTokenError
11+
from mcp.shared._httpx_utils import redirect_note
1112
from mcp.shared.auth import (
1213
OAuthClientInformationFull,
1314
OAuthClientMetadata,
@@ -297,7 +298,9 @@ async def handle_registration_response(response: Response) -> OAuthClientInforma
297298
"""Handle registration response."""
298299
if response.status_code not in (200, 201):
299300
await response.aread()
300-
raise OAuthRegistrationError(f"Registration failed: {response.status_code} {response.text}")
301+
raise OAuthRegistrationError(
302+
f"Registration failed: {response.status_code}{redirect_note(response)} {response.text}"
303+
)
301304

302305
try:
303306
content = await response.aread()

‎src/mcp/client/sse.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ async def sse_client(
5757
(same scheme, host and port, or http to https on the same host with default ports) and
5858
keeps the request method; any other redirect is not followed, so connecting fails with
5959
`httpx2.HTTPStatusError` for the redirect response. The client's `follow_redirects`
60-
setting is not consulted, and requests `auth` makes during an MCP request do not follow
61-
redirects.
60+
setting is not consulted; the SDK's OAuth providers apply the same rule to the requests
61+
they make.
6262
auth: Optional httpx2 authentication handler.
6363
on_session_created: Optional callback invoked with the session ID when received.
6464
"""

‎src/mcp/client/streamable_http.py‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,16 @@ def _unfollowed_redirect(response: httpx2.Response) -> str | None:
7171
"""Describe a redirect `stream_within_origin` left unfollowed, or None if `response` is not one."""
7272
if response.next_request is None:
7373
return None
74-
location = response.next_request.url
74+
sent = response.request.url
75+
# Query and userinfo are left out: they can carry state that does not belong in logs.
76+
location = response.next_request.url.copy_with(userinfo=b"", query=None, fragment=None)
77+
if sent.scheme == "https" and location.scheme == "http" and location.host == sent.host:
78+
return (
79+
f"Redirect to {location} not followed: it would downgrade this HTTPS endpoint to plain HTTP.\n"
80+
"The server is likely behind a TLS-terminating proxy whose forwarded headers it does not trust,\n"
81+
f"often combined with a trailing-slash difference. Try {location.copy_with(scheme='https')} instead, "
82+
"or fix the proxy settings."
83+
)
7584
return f"Redirect to {location} not followed; use that URL as the endpoint if it is the intended server"
7685

7786

@@ -687,8 +696,8 @@ async def streamable_http_client(
687696
endpoint's origin (same scheme, host and port, or http to https on the same host with
688697
default ports) and keeps the request method (307/308); any other redirect is not
689698
followed and the message it answered fails with an error naming the location. The
690-
client's `follow_redirects` setting is not consulted, and requests its `auth` handler
691-
makes during an MCP request do not follow redirects.
699+
client's `follow_redirects` setting is not consulted; the SDK's OAuth providers apply the
700+
same rule to the requests they make.
692701
terminate_on_close: If True, send a DELETE request to terminate the session when the context exits.
693702
694703
Yields:

‎src/mcp/shared/_httpx_utils.py‎

Lines changed: 94 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Utilities for creating and using httpx2 AsyncClient instances in the MCP transports."""
22

3-
from collections.abc import AsyncIterator
3+
from abc import ABC, abstractmethod
4+
from collections.abc import AsyncGenerator
45
from contextlib import asynccontextmanager
56
from typing import Any, Protocol
67

@@ -15,6 +16,9 @@
1516
# The headers httpx2.AsyncClient.sse() adds to an event-stream request.
1617
_SSE_HEADERS = {"Accept": "text/event-stream", "Cache-Control": "no-store"}
1718

19+
# How many redirects one auth-flow request may follow within its origin (see RedirectAwareAuth).
20+
_AUTH_REDIRECT_LIMIT = 5
21+
1822

1923
class McpHttpClientFactory(Protocol): # pragma: no branch
2024
def __call__( # pragma: no branch
@@ -78,51 +82,65 @@ def _within_origin(url: httpx2.URL, location: httpx2.URL) -> bool:
7882
)
7983

8084

85+
def next_request_within_origin(response: httpx2.Response) -> httpx2.Request | None:
86+
"""The request that follows `response`'s redirect, if it is one the MCP transports follow.
87+
88+
That is when httpx2 built a next request for it (a redirect status with a
89+
Location), the next request keeps the method (307/308, or any redirect of a
90+
GET: httpx2 turns a POST into a body-less GET for 301/302/303, which would
91+
drop the message), its URL stays within the origin of the request just sent
92+
(same scheme, host and port, or http to https on the same host with default
93+
ports), and the Location carries no userinfo (which httpx2 would otherwise
94+
send as Basic auth). None for anything else, including a non-redirect.
95+
"""
96+
next_request = response.next_request
97+
if next_request is None:
98+
return None
99+
sent = response.request
100+
if (
101+
next_request.method != sent.method
102+
or next_request.url.userinfo
103+
or not _within_origin(sent.url, next_request.url)
104+
):
105+
return None
106+
return next_request
107+
108+
81109
@asynccontextmanager
82110
async def stream_within_origin(
83111
client: httpx2.AsyncClient, method: str, url: httpx2.URL | str, **kwargs: Any
84-
) -> AsyncIterator[httpx2.Response]:
112+
) -> AsyncGenerator[httpx2.Response]:
85113
"""`client.stream(...)`, following redirects only while they stay within the request's origin.
86114
87115
An MCP transport talks to one configured endpoint, and everything on a request
88-
(headers, auth, body) was configured for that endpoint. A redirect that stays
89-
on the origin of the request just sent (same scheme, host and port, or http to
90-
https on the same host with default ports) and keeps the request's method,
91-
such as a 307/308 trailing-slash normalisation, is followed using httpx2's
92-
own next-request rules. Any other redirect is not followed: the redirect
93-
response itself is yielded, the way httpx2 hands one back when
94-
`follow_redirects` is off, and the caller treats it as the non-success it
95-
is. (httpx2 rewrites a POST into a body-less GET for 301/302/303, which
96-
would drop the message, so those count as not followed for anything but a
97-
GET.) The client's own `follow_redirects` setting is not consulted, and
98-
requests an `httpx2.Auth` flow makes during the call are sent the same way,
99-
so they do not follow redirects either.
100-
101-
Raises:
102-
httpx2.TooManyRedirects: More than `client.max_redirects` redirects were followed.
116+
(headers, auth, body) was configured for that endpoint. A redirect that
117+
`next_request_within_origin` accepts, such as a 307/308 trailing-slash
118+
normalisation, is followed, at most `client.max_redirects` times. Any other
119+
redirect (or one past that budget) is not followed: the redirect response
120+
itself is yielded, the way httpx2 hands one back when `follow_redirects` is
121+
off, and the caller treats it as the non-success it is. The client's own
122+
`follow_redirects` setting is not consulted. Requests an `httpx2.Auth` flow
123+
makes during the call are sent without following either; the SDK's OAuth
124+
providers apply the same rule to their own requests.
103125
"""
104126
request = client.build_request(method, url, **kwargs)
105-
for _ in range(client.max_redirects + 1):
127+
followed = 0
128+
while True:
106129
response = await client.send(request, stream=True, follow_redirects=False)
107-
# Set by httpx2, with its own method/body/header rules, only when the response is a redirect.
108-
next_request = response.next_request
109-
if (
110-
next_request is None
111-
or next_request.method != response.request.method
112-
or not _within_origin(response.request.url, next_request.url)
113-
):
114-
try:
115-
yield response
116-
finally:
117-
await response.aclose()
118-
return
130+
next_request = next_request_within_origin(response)
131+
if next_request is None or followed == client.max_redirects:
132+
break
119133
try:
120134
# Drain the redirect body so the connection returns to the pool, as httpx2 does when it follows.
121135
await response.aread()
122136
finally:
123137
await response.aclose()
124138
request = next_request
125-
raise httpx2.TooManyRedirects("Exceeded maximum allowed redirects.", request=request)
139+
followed += 1
140+
try:
141+
yield response
142+
finally:
143+
await response.aclose()
126144

127145

128146
async def request_within_origin(
@@ -137,9 +155,53 @@ async def request_within_origin(
137155
@asynccontextmanager
138156
async def sse_within_origin(
139157
client: httpx2.AsyncClient, url: httpx2.URL | str, *, headers: dict[str, str] | None = None
140-
) -> AsyncIterator[httpx2.EventSource]:
158+
) -> AsyncGenerator[httpx2.EventSource]:
141159
"""`client.sse(url)` with the redirect handling of `stream_within_origin`."""
142160
merged = httpx2.Headers(_SSE_HEADERS)
143161
merged.update(headers or {})
144162
async with stream_within_origin(client, "GET", url, headers=merged) as response:
145163
yield httpx2.EventSource(response)
164+
165+
166+
def redirect_note(response: httpx2.Response) -> str:
167+
"""A suffix naming the location of a redirect response that was not followed, else empty."""
168+
if response.next_request is None:
169+
return ""
170+
return f" (redirected to {response.next_request.url}; not followed)"
171+
172+
173+
class RedirectAwareAuth(ABC, httpx2.Auth):
174+
"""An `httpx2.Auth` whose own requests follow redirects the way MCP transport requests do.
175+
176+
The transports send every request with redirect following off and follow a
177+
redirect themselves only within the endpoint's origin (`stream_within_origin`).
178+
httpx2 applies that per-request setting to the requests an auth flow makes
179+
too (metadata discovery, registration, token), so on their own those would
180+
follow nothing. Subclasses write their flow as `_auth_flow`; this class
181+
drives it and, for each request the flow makes other than the one being
182+
authenticated, follows a redirect that `next_request_within_origin` accepts,
183+
up to `_AUTH_REDIRECT_LIMIT` times. Any other redirect response is handed
184+
to the flow as it is.
185+
"""
186+
187+
@abstractmethod
188+
def _auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
189+
"""The subclass's flow, written as `httpx2.Auth.async_auth_flow` otherwise would be."""
190+
191+
async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Request, httpx2.Response]:
192+
flow = self._auth_flow(request)
193+
try:
194+
outgoing = await flow.__anext__()
195+
while True:
196+
response = yield outgoing
197+
if outgoing is not request:
198+
for _ in range(_AUTH_REDIRECT_LIMIT):
199+
follow = next_request_within_origin(response)
200+
if follow is None:
201+
break
202+
response = yield follow
203+
outgoing = await flow.asend(response)
204+
except StopAsyncIteration:
205+
return
206+
finally:
207+
await flow.aclose()

0 commit comments

Comments
 (0)