Skip to content

Commit aabc009

Browse files
committed
test(oauth): cover refresh success, failure, and a skipped re-check
The refresh re-check and the failure flag now sit outside the lock's exit so branch coverage can see them. A peer that stores a valid token while this caller waits does not start a second refresh. A non-200 refresh clears the token and sends the original request unauthenticated.
1 parent fc0517f commit aabc009

2 files changed

Lines changed: 154 additions & 3 deletions

File tree

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -640,14 +640,23 @@ async def _auth_flow(self, request: httpx2.Request) -> AsyncGenerator[httpx2.Req
640640

641641
if needs_refresh:
642642
async with self.context.refresh_lock:
643-
refresh_request = None
643+
# Re-check outside the inner `async with` so a peer refresh that
644+
# landed while this caller waited is a normal branch, not one
645+
# hidden in the lock's exit.
646+
still_needs_refresh = False
644647
async with self.context.lock:
645-
if self._needs_token_refresh():
648+
still_needs_refresh = self._needs_token_refresh()
649+
refresh_request = None
650+
if still_needs_refresh:
651+
async with self.context.lock:
646652
refresh_request = await self._refresh_token()
647653
if refresh_request is not None:
648654
refresh_response = yield refresh_request
655+
refresh_ok = False
649656
async with self.context.lock:
650-
if not await self._handle_refresh_response(refresh_response):
657+
refresh_ok = await self._handle_refresh_response(refresh_response)
658+
if not refresh_ok:
659+
async with self.context.lock:
651660
self._initialized = False
652661

653662
async with self.context.lock:

‎tests/client/test_auth.py‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from unittest import mock
88
from urllib.parse import parse_qs, quote, unquote, urlparse
99

10+
import anyio
1011
import httpx2
1112
import pytest
1213
from inline_snapshot import Is, snapshot
@@ -3667,3 +3668,144 @@ async def echo_callback() -> AuthorizationCodeResult:
36673668
await auth_flow.asend(httpx2.Response(200, request=final_req))
36683669
except StopAsyncIteration:
36693670
pass
3671+
3672+
3673+
def _expired_refreshable(oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken) -> None:
3674+
"""Token is present and refreshable, but past its expiry. Discovery is already done."""
3675+
oauth_provider._initialized = True
3676+
oauth_provider.context.current_tokens = valid_tokens
3677+
oauth_provider.context.token_expiry_time = time.time() - 30
3678+
oauth_provider.context.client_info = OAuthClientInformationFull(
3679+
client_id="test_client",
3680+
client_secret="test_secret",
3681+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
3682+
token_endpoint_auth_method="client_secret_post",
3683+
)
3684+
3685+
3686+
@pytest.mark.anyio
3687+
async def test_expired_token_is_refreshed_then_the_original_request_is_sent(
3688+
oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken, mock_storage: MockTokenStorage
3689+
):
3690+
"""A successful refresh stores the new token and the original request goes out with it.
3691+
3692+
SDK behaviour: refresh runs outside ``context.lock``, and a 200 refresh response
3693+
leaves the provider initialized.
3694+
"""
3695+
_expired_refreshable(oauth_provider, valid_tokens)
3696+
3697+
auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
3698+
refresh_request = await auth_flow.__anext__()
3699+
3700+
assert refresh_request.method == "POST"
3701+
assert "grant_type=refresh_token" in refresh_request.content.decode()
3702+
refresh_response = httpx2.Response(
3703+
200,
3704+
json={"access_token": "refreshed-access", "token_type": "Bearer", "expires_in": 3600, "refresh_token": "kept"},
3705+
request=refresh_request,
3706+
)
3707+
primary = await auth_flow.asend(refresh_response)
3708+
3709+
assert primary.method == "GET"
3710+
assert primary.headers["Authorization"] == "Bearer refreshed-access"
3711+
assert oauth_provider._initialized is True
3712+
stored = await mock_storage.get_tokens()
3713+
assert stored is not None
3714+
assert stored.access_token == "refreshed-access"
3715+
try:
3716+
await auth_flow.asend(httpx2.Response(200, request=primary))
3717+
except StopAsyncIteration:
3718+
pass
3719+
3720+
3721+
@pytest.mark.anyio
3722+
async def test_failed_refresh_clears_the_token_and_sends_the_request_unauthenticated(
3723+
oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
3724+
):
3725+
"""A non-200 refresh drops the stored token and the original request goes out without it.
3726+
3727+
SDK behaviour: refresh failure marks the provider uninitialized so the next call rediscovers.
3728+
"""
3729+
_expired_refreshable(oauth_provider, valid_tokens)
3730+
3731+
auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
3732+
refresh_request = await auth_flow.__anext__()
3733+
assert refresh_request.method == "POST"
3734+
primary = await auth_flow.asend(httpx2.Response(400, json={"error": "invalid_grant"}, request=refresh_request))
3735+
3736+
assert primary.method == "GET"
3737+
assert "Authorization" not in primary.headers
3738+
assert oauth_provider._initialized is False
3739+
assert oauth_provider.context.current_tokens is None
3740+
try:
3741+
await auth_flow.asend(httpx2.Response(200, request=primary))
3742+
except StopAsyncIteration:
3743+
pass
3744+
3745+
3746+
@pytest.mark.anyio
3747+
async def test_refresh_waiter_sends_the_token_stored_while_it_waited(
3748+
oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
3749+
):
3750+
"""A caller that finds an expired token does not refresh if a peer stored a valid one while it waited.
3751+
3752+
The re-check is under ``refresh_lock``. This drives ``async_auth_flow`` and holds that lock
3753+
so the wait is real; the public client has no other way to observe the single-flight.
3754+
"""
3755+
_expired_refreshable(oauth_provider, valid_tokens)
3756+
3757+
class HoldableLock:
3758+
"""Stand-in for ``refresh_lock`` that signals when a waiter arrives."""
3759+
3760+
def __init__(self) -> None:
3761+
self._inner = anyio.Lock()
3762+
self.waiting = anyio.Event()
3763+
self.arm = False
3764+
3765+
async def __aenter__(self):
3766+
if self.arm:
3767+
self.waiting.set()
3768+
await self._inner.acquire()
3769+
return self
3770+
3771+
async def __aexit__(self, *_exc: object) -> None:
3772+
self._inner.release()
3773+
3774+
async def acquire(self) -> None:
3775+
await self._inner.acquire()
3776+
3777+
def release(self) -> None:
3778+
self._inner.release()
3779+
3780+
def locked(self) -> bool:
3781+
return self._inner.locked()
3782+
3783+
lock = HoldableLock()
3784+
oauth_provider.context.refresh_lock = lock
3785+
await lock.acquire()
3786+
lock.arm = True
3787+
sent: list[httpx2.Request] = []
3788+
3789+
async def drive() -> None:
3790+
auth_flow = oauth_provider.async_auth_flow(httpx2.Request("GET", "https://api.example.com/v1/mcp"))
3791+
request = await auth_flow.__anext__()
3792+
sent.append(request)
3793+
try:
3794+
await auth_flow.asend(httpx2.Response(200, request=request))
3795+
except StopAsyncIteration:
3796+
pass
3797+
3798+
try:
3799+
async with anyio.create_task_group() as task_group:
3800+
task_group.start_soon(drive)
3801+
with anyio.fail_after(5):
3802+
await lock.waiting.wait()
3803+
oauth_provider.context.token_expiry_time = time.time() + 3600
3804+
lock.release()
3805+
finally:
3806+
if lock.locked():
3807+
lock.release()
3808+
3809+
assert len(sent) == 1
3810+
assert sent[0].method == "GET"
3811+
assert sent[0].headers["Authorization"] == "Bearer test_access_token"

0 commit comments

Comments
 (0)