-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(auth): get_access_token reflects current request in stateful sessions #2675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Epochex
wants to merge
8
commits into
modelcontextprotocol:main
Choose a base branch
from
Epochex:fix/streamable-http-auth-context-current-request
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+181
−1
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
630c683
fix(auth): make get_access_token per-request in stateful sessions
Epochex 3e898a1
fix(auth): avoid Request.user assertion without auth middleware
Epochex 9f1f361
chore(auth): type-safe auth context push
Epochex e3088cc
fix auth context reset for streamable HTTP
Epochex 29e499d
test(auth): use httpx2 in streamable HTTP auth test
Epochex 3b172f4
test(auth): reuse streamable HTTP session for token refresh
Epochex e60e70a
test(auth): cover bearer auth helper branch
Epochex 562731c
test(auth): mark async timeout coverage arc
Epochex File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
148 changes: 148 additions & 0 deletions
148
tests/server/auth/test_get_access_token_streamable_http.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| import time | ||
|
|
||
| import anyio | ||
| import httpx2 | ||
| import pytest | ||
| from mcp_types import ( | ||
| CallToolRequestParams, | ||
| CallToolResult, | ||
| ListToolsResult, | ||
| PaginatedRequestParams, | ||
| ProgressNotificationParams, | ||
| TextContent, | ||
| Tool, | ||
| ) | ||
| from starlette.applications import Starlette | ||
| from starlette.middleware import Middleware | ||
| from starlette.middleware.authentication import AuthenticationMiddleware | ||
| from starlette.routing import Mount | ||
|
|
||
| from mcp import Client | ||
| from mcp.client.streamable_http import streamable_http_client | ||
| from mcp.server import Server, ServerRequestContext | ||
| from mcp.server.auth.middleware.auth_context import AuthContextMiddleware, get_access_token | ||
| from mcp.server.auth.middleware.bearer_auth import BearerAuthBackend | ||
| from mcp.server.auth.provider import AccessToken | ||
| from mcp.server.streamable_http_manager import StreamableHTTPSessionManager | ||
|
|
||
|
|
||
| class _EchoTokenVerifier: | ||
| """Accepts any bearer token and echoes it back as the verified AccessToken.""" | ||
|
|
||
| async def verify_token(self, token: str) -> AccessToken | None: | ||
| return AccessToken(token=token, client_id="test-client", scopes=[], expires_at=int(time.time()) + 3600) | ||
|
|
||
|
|
||
| async def _handle_whoami(ctx: ServerRequestContext, params: CallToolRequestParams) -> CallToolResult: | ||
| access = get_access_token() | ||
| text = access.token if access else "<none>" | ||
| return CallToolResult(content=[TextContent(type="text", text=text)]) | ||
|
|
||
|
|
||
| async def _handle_list_tools(ctx: ServerRequestContext, params: PaginatedRequestParams | None) -> ListToolsResult: | ||
| return ListToolsResult(tools=[Tool(name="whoami", input_schema={"type": "object", "properties": {}})]) | ||
|
|
||
|
|
||
| class _MutableBearerAuth(httpx2.Auth): | ||
| def __init__(self, token: str) -> None: | ||
| self.token = token | ||
|
|
||
| def auth_flow(self, request: httpx2.Request): | ||
| request.headers["Authorization"] = f"Bearer {self.token}" | ||
| yield request | ||
|
|
||
|
|
||
| async def _call_whoami(client: Client) -> str: | ||
| result = await client.call_tool("whoami", {}) | ||
| assert isinstance(result.content[0], TextContent) | ||
| return result.content[0].text | ||
|
|
||
|
|
||
| @pytest.mark.anyio | ||
| async def test_get_access_token_reflects_current_request_in_stateful_session() -> None: | ||
| host = "testserver" | ||
|
|
||
| server = Server( | ||
| "auth-test-server", | ||
| on_call_tool=_handle_whoami, | ||
| on_list_tools=_handle_list_tools, | ||
| ) | ||
|
|
||
| session_manager = StreamableHTTPSessionManager(app=server, stateless=False) | ||
|
|
||
| asgi_app = Starlette( | ||
| routes=[Mount("/mcp", app=session_manager.handle_request)], | ||
| middleware=[ | ||
| Middleware(AuthenticationMiddleware, backend=BearerAuthBackend(_EchoTokenVerifier())), | ||
| Middleware(AuthContextMiddleware), | ||
| ], | ||
| lifespan=lambda app: session_manager.run(), | ||
| ) | ||
|
|
||
| async with asgi_app.router.lifespan_context(asgi_app): | ||
| auth = _MutableBearerAuth("token-A") | ||
| async with ( | ||
| httpx2.ASGITransport(asgi_app) as transport, | ||
| httpx2.AsyncClient( | ||
| transport=transport, | ||
| base_url=f"http://{host}", | ||
| auth=auth, | ||
| timeout=httpx2.Timeout(30, read=30), | ||
| follow_redirects=True, | ||
| ) as http_client, | ||
| Client(streamable_http_client(f"http://{host}/mcp", http_client=http_client), mode="legacy") as client, | ||
| ): | ||
| assert await _call_whoami(client) == "token-A" | ||
|
|
||
| auth.token = "token-B" | ||
| assert await _call_whoami(client) == "token-B" | ||
|
|
||
|
|
||
| @pytest.mark.anyio | ||
| async def test_notification_handler_get_access_token_reflects_current_request_in_stateful_session() -> None: | ||
| host = "testserver" | ||
| send_token, receive_token = anyio.create_memory_object_stream[str](10) | ||
|
|
||
| async def handle_progress(ctx: ServerRequestContext, params: ProgressNotificationParams) -> None: | ||
| access = get_access_token() | ||
| await send_token.send(access.token if access else "<none>") | ||
|
|
||
| server = Server( | ||
| "auth-test-server", | ||
| on_call_tool=_handle_whoami, | ||
| on_list_tools=_handle_list_tools, | ||
| ) | ||
| server.add_notification_handler("notifications/progress", ProgressNotificationParams, handle_progress) | ||
|
|
||
| session_manager = StreamableHTTPSessionManager(app=server, stateless=False) | ||
|
|
||
| asgi_app = Starlette( | ||
| routes=[Mount("/mcp", app=session_manager.handle_request)], | ||
| middleware=[ | ||
| Middleware(AuthenticationMiddleware, backend=BearerAuthBackend(_EchoTokenVerifier())), | ||
| Middleware(AuthContextMiddleware), | ||
| ], | ||
| lifespan=lambda app: session_manager.run(), | ||
| ) | ||
|
|
||
| async with send_token, receive_token, asgi_app.router.lifespan_context(asgi_app): | ||
| auth = _MutableBearerAuth("token-A") | ||
| async with ( | ||
| httpx2.ASGITransport(asgi_app) as transport, | ||
| httpx2.AsyncClient( | ||
| transport=transport, | ||
| base_url=f"http://{host}", | ||
| auth=auth, | ||
| timeout=httpx2.Timeout(30, read=30), | ||
| follow_redirects=True, | ||
| ) as http_client, | ||
| Client(streamable_http_client(f"http://{host}/mcp", http_client=http_client), mode="legacy") as client, | ||
| ): | ||
| await client.send_progress_notification("token-A", 0.1) # pyright: ignore[reportDeprecated] | ||
| with anyio.fail_after(5): | ||
| assert await receive_token.receive() == "token-A" | ||
|
|
||
| auth.token = "token-B" | ||
| await client.send_progress_notification("token-B", 0.2) # pyright: ignore[reportDeprecated] | ||
| with anyio.fail_after(5): # pragma: no branch - coverage misreports the normal context exit arc | ||
| assert await receive_token.receive() == "token-B" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Notification handlers in a stateful Streamable HTTP session can still read the session-creating request's token after a later authenticated notification. Scope
push_auth_context_from_request(ctx.request)/pop_auth_context()around_on_notify's middleware call too, soget_access_token()matches that notification's HTTP request.Prompt for AI agents