Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion src/mcp_server_appwrite/http_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 RFC 8414 issuer mismatch may cause strict clients to reject the document

RFC 8414 §3 requires that the issuer value in an /.well-known/oauth-authorization-server response equals the URL prefix used to construct the discovery request (i.e. https://mcp.appwrite.io). This endpoint returns the raw Appwrite upstream document, whose issuer is https://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 an issuer-patched copy could work (though it would break any client that validates token issuers against this document).

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mcp_server_appwrite/http_app.py
Line: 302-312

Comment:
**RFC 8414 issuer mismatch may cause strict clients to reject the document**

RFC 8414 §3 requires that the `issuer` value in an `/.well-known/oauth-authorization-server` response equals the URL prefix used to construct the discovery request (i.e. `https://mcp.appwrite.io`). This endpoint returns the raw Appwrite upstream document, whose `issuer` is `https://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 an `issuer`-patched copy could work (though it would break any client that validates token issuers against this document).

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

Comment on lines +302 to +312

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

Prompt To Fix With AI
This 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.

Fix in Claude Code Fix in Codex



async def health_endpoint(request: Request) -> PlainTextResponse:
return PlainTextResponse(f"appwrite-mcp {SERVER_VERSION} ok")

Expand Down Expand Up @@ -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"]),
Expand Down
59 changes: 59 additions & 0 deletions tests/unit/test_http_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
_client_from_user_agent,
_find_initialize_params,
_send_401,
authorization_server_metadata_endpoint,
build_app,
protected_resource_metadata_endpoint,
)


Expand Down Expand Up @@ -170,6 +173,62 @@ def test_401_omits_scope_hint_when_discovery_unavailable(self):
self.assertNotIn("scope=", challenge)


class WellKnownMetadataEndpointTests(unittest.TestCase):
"""Discovery endpoints, including the legacy shims for clients on the
2025-03-26 MCP authorization spec (e.g. Raycast) that fetch
``/.well-known/oauth-authorization-server`` from the MCP origin instead of
following ``resource_metadata`` from the 401 challenge."""

ENV = {
"APPWRITE_ENDPOINT": "https://cloud.appwrite.io/v1",
"MCP_PUBLIC_URL": "https://mcp.appwrite.io",
"APPWRITE_PROJECT_ID": "console",
}
DISCOVERY = {
"issuer": "https://cloud.appwrite.io/v1/oauth2/console",
"authorization_endpoint": "https://cloud.appwrite.io/v1/oauth2/console/authorize",
"token_endpoint": "https://cloud.appwrite.io/v1/oauth2/console/token",
"registration_endpoint": "https://cloud.appwrite.io/v1/oauth2/console/register",
"jwks_uri": "https://cloud.appwrite.io/v1/oauth2/console/.well-known/jwks.json",
"scopes_supported": ["openid", "all"],
}

def setUp(self):
patcher = mock.patch.dict(os.environ, self.ENV, clear=False)
patcher.start()
self.addCleanup(patcher.stop)
auth._store_discovery("console", dict(self.DISCOVERY))
self.addCleanup(lambda: auth._discovery_cache.pop("console", None))

def _body(self, response) -> dict:
return json.loads(response.body)

def test_authorization_server_metadata_mirrors_discovery(self):
response = asyncio.run(authorization_server_metadata_endpoint(mock.Mock()))
self.assertEqual(response.status_code, 200)
self.assertEqual(self._body(response), self.DISCOVERY)
self.assertIn("access-control-allow-origin", response.headers)

def test_protected_resource_metadata_names_canonical_resource(self):
response = asyncio.run(protected_resource_metadata_endpoint(mock.Mock()))
self.assertEqual(response.status_code, 200)
body = self._body(response)
self.assertEqual(body["resource"], "https://mcp.appwrite.io/mcp")
self.assertEqual(body["authorization_servers"], [self.DISCOVERY["issuer"]])

def test_app_routes_include_legacy_discovery_paths(self):
# build_mcp_server flips the module-global upload transport to "http";
# restore it so stdio-transport tests elsewhere keep seeing the default.
from mcp_server_appwrite import server as server_module

original_transport = server_module._UPLOAD_TRANSPORT
self.addCleanup(setattr, server_module, "_UPLOAD_TRANSPORT", original_transport)
paths = {getattr(route, "path", None) for route in build_app().routes}
self.assertIn("/.well-known/oauth-protected-resource/mcp", paths)
self.assertIn("/.well-known/oauth-protected-resource", paths)
self.assertIn("/.well-known/oauth-authorization-server", paths)


class ClientFromUserAgentTests(unittest.TestCase):
def test_product_token(self):
self.assertEqual(
Expand Down
Loading