Add support for remote-oauth-support Fix #686 #764
Conversation
| requires-python = ">=3.10" | ||
| authors = [{ name = "Anthropic, PBC." }] | ||
| license = { text = "MIT" } | ||
| dependencies = [ |
|
|
||
|
|
||
| class TokenValidatorJWT(TokenValidator[AccessToken]): | ||
| def __init__(self, resource_metadata: ProtectedResourceMetadata): |
There was a problem hiding this comment.
Probably needs super init:
super().__init__()| # Set environment variables first (see above) | ||
|
|
||
| # Run the server | ||
| uv run mcp-simple-auth |
| uv run mcp-simple-auth | ||
| ``` | ||
|
|
||
| The server will start on `http://localhost:8000`. |
There was a problem hiding this comment.
I'd make it more explicit that it starts on http://localhost:8000/sse
|
|
||
| This server supports multiple transport protocols that can run on the same port: | ||
|
|
||
| #### SSE (Server-Sent Events) - Default |
There was a problem hiding this comment.
Can we switch the default to streamable http / move sse down? SSE is now deprecated
Also, seems that connecting to the SSE server doesn't trigger authentication the way it does to the Streamable HTTP server.
| 3. No other service is using port 8000 | ||
| 4. The transport specified is valid (`sse` or `streamable-http`) | ||
|
|
||
| You can use [Inspector](https://github.com/modelcontextprotocol/inspector) to test Auth |
There was a problem hiding this comment.
Have you found an Inspector workflow that works for you? Even after applying the couple of fixes I suggested, I hit a wall w/ the AS not setting CORS headers that would allow localhost to proceed.
| @@ -138,14 +140,23 @@ def __init__( | |||
| self, | |||
| name: str | None = None, | |||
| instructions: str | None = None, | |||
| auth_server_provider: OAuthAuthorizationServerProvider[Any, Any, Any] | |||
| | None = None, | |||
| auth_server_details: dict[str, Any] | None = None, | |||
There was a problem hiding this comment.
While these changes make sense, an important consideration is this class comes from FastMCP 1.0.
I reckon we'll probably want to update the SDK to FastMCP 2.0 at some point, so might be better not to diverge our version of FastMCP, if possible.
I'd suggest maybe to evaluate if the same change is needed / applicable to FastMCP 2.0 and send a PR to them, while we figure out if / how we can update to 2.0? (if they accept the patch, we can even backport it to our 1.0 fork if the overall upgrade to 2.0 isn't straightforward, without fearing that said upgrade would be harder down the line).
Does this make sense?
| status_code=404, detail="Protected resource metadata not configured" | ||
| ) | ||
| return Response( | ||
| self._protected_resource_metadata.model_dump_json(), |
There was a problem hiding this comment.
self._protected_resource_metadata.model_dump_json(exclude_none=True),(otherwise fields that are optional but not-nullable, like jwks_uri, fail to be validated if set to None, e.g. by the TS SDK (used by the Inspector)
| routes.append( | ||
| Route( | ||
| "/.well-known/oauth-protected-resource", | ||
| self._serve_protected_resource_metadata, |
There was a problem hiding this comment.
We might need to wrap this one with cors_middleware as done elsewhere in the sdk
This contains fix for the issue: #686
As per the new authorization spec for MCP Servers as outlined here: https://modelcontextprotocol.io/specification/draft/basic/authorization, implementing the remote oauth support for the FastMCP Servers.
As part of the spec, the only responsibility of the MCP Server should be to indicate to the client it's oauth protected resource and indicate to the client where to find the authorization server.
This is just the initial version and based on the feedback from @localden, @ihrpr , I intend to keep making changes to complete the test cases and full feature documentation.
Motivation and Context
Implement supoprt for RFC9728, along with other requirements outlined in the spec.
How Has This Been Tested?
Breaking Changes
May have breaking changes.
Types of changes
Checklist
Additional context