Skip to content

Commit e4b0350

Browse files
Varun SharmaCopilot
andcommitted
fix: only retry 403 responses for insufficient_scope error
Move retry logic inside the insufficient_scope branch so that 403 responses with other errors (e.g. invalid_token, access_denied) raise OAuthFlowError immediately instead of making a doomed retry with the same token. Fixes #1602 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 62575ed commit e4b0350

File tree

2 files changed

+53
-5
lines changed

2 files changed

+53
-5
lines changed

src/mcp/client/auth/oauth2.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -612,12 +612,12 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
612612
# Retry with new tokens
613613
self._add_auth_header(request)
614614
yield request
615-
elif response.status_code == 403:
615+
elif response.status_code == 403: # pragma: no branch
616616
# Step 1: Extract error field from WWW-Authenticate header
617617
error = extract_field_from_www_auth(response, "error")
618618

619619
# Step 2: Check if we need to step-up authorization
620-
if error == "insufficient_scope": # pragma: no branch
620+
if error == "insufficient_scope":
621621
try:
622622
# Step 2a: Update the required scopes
623623
self.context.client_metadata.scope = get_client_metadata_scopes(
@@ -631,6 +631,8 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
631631
logger.exception("OAuth flow error")
632632
raise
633633

634-
# Retry with new tokens
635-
self._add_auth_header(request)
636-
yield request
634+
# Retry with new tokens
635+
self._add_auth_header(request)
636+
yield request
637+
else:
638+
raise OAuthFlowError(f"Access forbidden: {error or 'insufficient permissions'}")

tests/client/test_auth.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1410,6 +1410,52 @@ async def mock_callback() -> tuple[str, str | None]:
14101410
except StopAsyncIteration:
14111411
pass # Expected
14121412

1413+
@pytest.mark.anyio
1414+
@pytest.mark.parametrize(
1415+
("www_authenticate", "expected_error_substring"),
1416+
[
1417+
('Bearer error="invalid_token"', "invalid_token"),
1418+
("Bearer", "insufficient permissions"),
1419+
('Bearer error="access_denied"', "access_denied"),
1420+
],
1421+
)
1422+
async def test_403_non_insufficient_scope_raises_error(
1423+
self,
1424+
oauth_provider: OAuthClientProvider,
1425+
mock_storage: MockTokenStorage,
1426+
valid_tokens: OAuthToken,
1427+
www_authenticate: str,
1428+
expected_error_substring: str,
1429+
):
1430+
"""Test that 403 without insufficient_scope raises OAuthFlowError instead of retrying."""
1431+
client_info = OAuthClientInformationFull(
1432+
client_id="test_client_id",
1433+
client_secret="test_client_secret",
1434+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
1435+
)
1436+
await mock_storage.set_tokens(valid_tokens)
1437+
await mock_storage.set_client_info(client_info)
1438+
oauth_provider.context.current_tokens = valid_tokens
1439+
oauth_provider.context.token_expiry_time = time.time() + 1800
1440+
oauth_provider.context.client_info = client_info
1441+
oauth_provider._initialized = True
1442+
1443+
test_request = httpx.Request("GET", "https://api.example.com/mcp")
1444+
auth_flow = oauth_provider.async_auth_flow(test_request)
1445+
1446+
# First request with auth header
1447+
request = await auth_flow.__anext__()
1448+
1449+
# Send 403 with non-insufficient_scope error
1450+
response_403 = httpx.Response(
1451+
403,
1452+
headers={"WWW-Authenticate": www_authenticate},
1453+
request=request,
1454+
)
1455+
1456+
with pytest.raises(OAuthFlowError, match=expected_error_substring):
1457+
await auth_flow.asend(response_403)
1458+
14131459

14141460
@pytest.mark.parametrize(
14151461
(

0 commit comments

Comments
 (0)