Skip to content

Commit a9d322e

Browse files
committed
fix: skip eager token refresh when OAuth metadata is unavailable
When the auth server lives under a non-root path (e.g. /oauth2/api/v1/token), the eager refresh at the top of async_auth_flow used the fallback urljoin(get_authorization_base_url(server_url), '/token') which strips the path, hitting the wrong endpoint. Fix: only attempt the eager refresh when oauth_metadata is already populated (i.e. we know the real token_endpoint). Without metadata, let the request proceed with the stale token, receive a 401, and run full PRM/ASM discovery before retrying — which resolves the correct token endpoint. Includes regression test: test_auth_flow_skips_eager_refresh_when_metadata_missing Fixes #3240
1 parent a4f4ccd commit a9d322e

2 files changed

Lines changed: 58 additions & 2 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,8 +586,17 @@ async def async_auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx
586586
# Capture protocol version from request headers
587587
self.context.protocol_version = request.headers.get(MCP_PROTOCOL_VERSION_HEADER)
588588

589-
if not self.context.is_token_valid() and self.context.can_refresh_token():
590-
# Try to refresh token
589+
if (
590+
not self.context.is_token_valid()
591+
and self.context.can_refresh_token()
592+
and self.context.oauth_metadata is not None
593+
):
594+
# Try to refresh token — only when we already have OAuth metadata.
595+
# Without metadata the token endpoint is unknown; the fallback
596+
# urljoin(base_url, "/token") strips the path when the AS lives
597+
# under a non-root path (e.g. /oauth2/api/v1/token). Skipping
598+
# the refresh here lets the request proceed with the stale token,
599+
# receive a 401, and run full metadata discovery before retrying.
591600
refresh_request = await self._refresh_token()
592601
refresh_response = yield refresh_request
593602

tests/client/test_auth.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,7 +1290,54 @@ async def test_auth_flow_with_no_tokens(self, oauth_provider: OAuthClientProvide
12901290
assert oauth_provider.context.current_tokens.access_token == "new_access_token"
12911291
assert oauth_provider.context.token_expiry_time is not None
12921292

1293+
12931294
@pytest.mark.anyio
1295+
async def test_auth_flow_skips_eager_refresh_when_metadata_missing(
1296+
self, oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage
1297+
):
1298+
"""When oauth_metadata is None the eager refresh must be skipped.
1299+
1300+
Without metadata the token endpoint is unknown; the fallback
1301+
urljoin(base_url, '/token') strips the path when the AS lives under a
1302+
non-root path (e.g. /oauth2/api/v1/token). The fix guards the eager
1303+
refresh on ``oauth_metadata is not None`` so the stale-token request
1304+
proceeds, gets a 401, and runs full metadata discovery instead.
1305+
"""
1306+
# Set up expired tokens with a refresh token but NO oauth_metadata.
1307+
expired_tokens = OAuthToken(
1308+
access_token="expired_access_token",
1309+
token_type="Bearer",
1310+
expires_in=0,
1311+
refresh_token="test_refresh_token",
1312+
scope="read write",
1313+
)
1314+
await mock_storage.set_tokens(expired_tokens)
1315+
oauth_provider.context.current_tokens = expired_tokens
1316+
oauth_provider.context.token_expiry_time = time.time() - 100 # Expired
1317+
oauth_provider._initialized = True
1318+
oauth_provider.context.client_info = OAuthClientInformationFull(
1319+
client_id="test_client",
1320+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
1321+
)
1322+
# oauth_metadata is None (default) — this is the key condition.
1323+
1324+
test_request = httpx2.Request("GET", "https://api.example.com/v1/mcp")
1325+
auth_flow = oauth_provider.async_auth_flow(test_request)
1326+
1327+
# The first yield should be the original request WITHOUT an auth header,
1328+
# NOT a refresh request to the wrong endpoint.
1329+
request = await auth_flow.__anext__()
1330+
assert "Authorization" not in request.headers
1331+
assert str(request.url) == "https://api.example.com/v1/mcp"
1332+
assert request.method == "GET"
1333+
1334+
# The token was not consumed by a failed refresh.
1335+
assert oauth_provider.context.current_tokens is not None
1336+
assert oauth_provider.context.current_tokens.refresh_token == "test_refresh_token"
1337+
1338+
# Close the generator to avoid warnings.
1339+
await auth_flow.aclose()
1340+
12941341
async def test_auth_flow_no_unnecessary_retry_after_oauth(
12951342
self, oauth_provider: OAuthClientProvider, mock_storage: MockTokenStorage, valid_tokens: OAuthToken
12961343
):

0 commit comments

Comments
 (0)