MCP server separation into Authorization Server (AS) and Resource Server (RS) roles per spec PR #338 #982
Conversation
| "type": "http.response.start", | ||
| "status": status_code, | ||
| "headers": [ | ||
| (b"content-type", b"application/json"), |
There was a problem hiding this comment.
I don't think it makes sense for MCP to be using the low level ASGI API. I know it's not introduced in this PR, but it makes me sad. 😞
This is missing content-length btw.
| @click.command() | ||
| @click.option("--port", default=9000, help="Port to listen on") | ||
| @click.option("--host", default="localhost", help="Host to bind to") | ||
| def main(port: int, host: str) -> int: |
There was a problem hiding this comment.
This will give a lot of maintenance burden. People will ask a lot of uvicorn's parameters to be added here.
There was a problem hiding this comment.
The purpose of this file is demonstrate mcp oauth concepts, not to be a production server. Removed the host as it's not needed, but port is useful
examples/servers/simple-auth/mcp_simple_auth/github_oauth_provider.py
Outdated
Show resolved
Hide resolved
| # Server settings | ||
| host: str = "localhost" | ||
| port: int = 8000 | ||
| server_url: AnyHttpUrl = AnyHttpUrl("http://localhost:8000") |
There was a problem hiding this comment.
Pydantic is appending a trailing slash on AnyHttpUrl - I think we should stop using AnyHttpUrl and use plain str in the whole code source.
Even if you are setting up http://localhost:8000, the real value will be http://localhost:8000/.
There was a problem hiding this comment.
noted, this should be done outside of this PR, added to follow ups
| server_url: AnyHttpUrl = AnyHttpUrl("http://localhost:8000") | ||
| github_callback_path: str = "http://localhost:8000/github/callback" | ||
|
|
||
| def __init__(self, **data): |
There was a problem hiding this comment.
What is this for? Why is there no type hints?
There was a problem hiding this comment.
Without the __init__, you get the error "Arguments missing for parameters 'github_client_id', 'github_client_secret'" because Pydantic expects these to be explicitly passed, not understanding they should come from environment variables. One option would be to just make then optional
There was a problem hiding this comment.
That's the nature of BaseSettings. Is the idea that they can either be set programmatically or via env var?
There was a problem hiding this comment.
yes, moved to optional. Using github in the example servers brings too much complexity, like having secrets etc, will remove it in the follow up PR.
|
@ihrpr Hi! Thanks so much for driving this change, do you have any sense of when this PR might be merged? 🙏 (ballpark estimate) |
Background: modelcontextprotocol/modelcontextprotocol#338
Closes: #921
Implementation:
TokenVerifierprotocol to decouple token validation from OAuth provider logicExample:
Follow up: