-
Notifications
You must be signed in to change notification settings - Fork 18
(fix): serve legacy OAuth discovery endpoints for 2025-03-26 MCP clients #77
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,14 @@ | |
| * ``/mcp`` — the MCP Streamable-HTTP endpoint, gated by a bearer-token check that | ||
| returns an RFC 9728 ``WWW-Authenticate`` challenge when unauthenticated. | ||
| * ``/.well-known/oauth-protected-resource/mcp`` — RFC 9728 protected resource | ||
| metadata pointing at the project's Appwrite OAuth authorization server. | ||
| metadata pointing at the project's Appwrite OAuth authorization server. Also | ||
| served at the root form (no ``/mcp`` suffix) for clients that don't apply | ||
| path insertion. | ||
| * ``/.well-known/oauth-authorization-server`` — backwards-compatibility shim for | ||
| clients on the 2025-03-26 MCP authorization spec (e.g. Raycast), which expect | ||
| authorization-server metadata at the MCP server's own origin instead of | ||
| following ``resource_metadata`` from the 401 challenge. Mirrors the Appwrite | ||
| authorization server's discovery document verbatim. | ||
| * ``/healthz`` — liveness probe. | ||
|
|
||
| Auth uses the SDK primitives (``BearerAuthBackend`` + ``AuthContextMiddleware``) so the | ||
|
|
@@ -42,6 +49,7 @@ | |
| from . import error_monitoring, telemetry | ||
| from .auth import ( | ||
| AppwriteTokenVerifier, | ||
| authorization_server_metadata, | ||
| protected_resource_metadata, | ||
| public_base_url, | ||
| resource_metadata_url, | ||
|
|
@@ -291,6 +299,19 @@ async def protected_resource_metadata_endpoint(request: Request) -> JSONResponse | |
| return JSONResponse(metadata, headers=headers) | ||
|
|
||
|
|
||
| async def authorization_server_metadata_endpoint(request: Request) -> JSONResponse: | ||
| """Legacy discovery shim (MCP authorization spec 2025-03-26). | ||
|
|
||
| Older clients fetch authorization-server metadata from the MCP server's own | ||
| origin rather than the ``authorization_servers`` entry in the protected | ||
| resource metadata. Serve the Appwrite authorization server's discovery | ||
| document verbatim so those clients find the real authorize/token/register | ||
| endpoints (the ``issuer`` inside points at the Appwrite endpoint, not here). | ||
| """ | ||
| metadata = await authorization_server_metadata() | ||
| return JSONResponse(metadata, headers=dict(CORS_HEADERS)) | ||
|
Comment on lines
+302
to
+312
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Prompt To Fix With AIThis is a comment left during a code review.
Path: src/mcp_server_appwrite/http_app.py
Line: 302-312
Comment:
**Unhandled discovery failure returns 500 with no client-facing error**
`authorization_server_metadata()` raises when the upstream discovery fetch fails and there is no stale cached document (e.g. first request after a cold start during an Appwrite outage). The exception propagates unhandled and Starlette returns a 500 response. The existing `protected_resource_metadata_endpoint` has the same gap, but this new endpoint is on the unauthenticated discovery path — a 500 here prevents any client from completing OAuth at all. A `try/except` that returns a `503 Service Unavailable` with a `Retry-After` header would give clients a clearer signal.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
|
|
||
| async def health_endpoint(request: Request) -> PlainTextResponse: | ||
| return PlainTextResponse(f"appwrite-mcp {SERVER_VERSION} ok") | ||
|
|
||
|
|
@@ -339,6 +360,19 @@ async def lifespan(app: Starlette): | |
| endpoint=protected_resource_metadata_endpoint, | ||
| methods=["GET", "OPTIONS"], | ||
| ), | ||
| # Root form, for clients that don't apply RFC 9728 path insertion. | ||
| Route( | ||
| "/.well-known/oauth-protected-resource", | ||
| endpoint=protected_resource_metadata_endpoint, | ||
| methods=["GET", "OPTIONS"], | ||
| ), | ||
| # Legacy 2025-03-26 MCP clients (e.g. Raycast) discover the | ||
| # authorization server at the MCP origin itself. | ||
| Route( | ||
| "/.well-known/oauth-authorization-server", | ||
| endpoint=authorization_server_metadata_endpoint, | ||
| methods=["GET", "OPTIONS"], | ||
| ), | ||
| Route("/favicon.svg", endpoint=favicon_svg_endpoint, methods=["GET"]), | ||
| Route("/favicon.ico", endpoint=favicon_ico_endpoint, methods=["GET"]), | ||
| Route("/healthz", endpoint=health_endpoint, methods=["GET"]), | ||
|
|
||
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.
RFC 8414 §3 requires that the
issuervalue in an/.well-known/oauth-authorization-serverresponse equals the URL prefix used to construct the discovery request (i.e.https://mcp.appwrite.io). This endpoint returns the raw Appwrite upstream document, whoseissuerishttps://cloud.appwrite.io/v1/oauth2/console. Strictly conformant clients (and any client that adds RFC 8414 validation in a future update) will verify this and reject the document. The current fix relies on Raycast not enforcing this check. Worth noting explicitly in the docstring that this is a deliberate deviation, and consider whether anissuer-patched copy could work (though it would break any client that validates token issuers against this document).Prompt To Fix With AI