Skip to content
Open
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
1 change: 1 addition & 0 deletions agent_memory_server/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ class Settings(BaseSettings):
oauth2_audience: str | None = None
oauth2_jwks_url: str | None = None
oauth2_algorithms: list[str] = ["RS256"]
oauth2_resource_host: str | None = None

# Token Authentication settings
token_auth_enabled: bool = False
Expand Down
11 changes: 11 additions & 0 deletions agent_memory_server/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,17 @@ async def lifespan(app: FastAPI):
app.include_router(memory_router)


@app.get("/.well-known/oauth-authorization-server")
async def oauth_authorization_server_metadata():
"""RFC 9728 Protected Resource Metadata for MCP OIDC discovery."""
if not settings.oauth2_issuer_url:
return {"error": "OIDC not configured"}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Error response returns HTTP 200 instead of error status

Medium Severity

When oauth2_issuer_url is not configured, oauth_authorization_server_metadata returns {"error": "OIDC not configured"} with an implicit HTTP 200 status. Clients will interpret this as valid metadata and attempt to parse the resource and authorization_servers fields, leading to confusing failures downstream. An appropriate HTTP error status (e.g., 404 or 501) would correctly signal that the metadata is unavailable.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2e0b5c7. Configure here.

return {
"resource": f"https://{settings.oauth2_resource_host or 'localhost'}",
"authorization_servers": [settings.oauth2_issuer_url],
}
Comment on lines +136 to +141
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

When OIDC isn't configured (or oauth2_resource_host is unset), this endpoint returns a 200 with either an error object or resource defaulting to https://localhost. That can cause clients to cache/act on invalid metadata. Consider returning an appropriate non-2xx status (e.g., 404/503 via HTTPException) and requiring settings.oauth2_resource_host to be set before serving metadata (instead of defaulting).

Copilot uses AI. Check for mistakes.
Comment on lines +133 to +141
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

This route is registered at /.well-known/oauth-authorization-server, but the response body and docstring are for RFC 9728 Protected Resource Metadata (resource + authorization_servers). Clients following RFC 9728 will look for /.well-known/oauth-protected-resource (and RFC 8414 uses different fields for oauth-authorization-server). As written, discovery is likely to fail because the well-known path doesn't match the payload type.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Wrong well-known path for RFC 9728 metadata

High Severity

The endpoint path /.well-known/oauth-authorization-server is the RFC 8414 Authorization Server Metadata path, but the response body contains RFC 9728 Protected Resource Metadata fields (resource, authorization_servers). The PR description and docstring both say this is an "RFC 9728 Protected Resource Metadata" endpoint, so the correct path is /.well-known/oauth-protected-resource. MCP clients performing RFC 9728 discovery will look for the resource metadata at the correct path and won't find it here.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2e0b5c7. Configure here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Missing guard allows incorrect localhost resource metadata

Medium Severity

The guard condition in oauth_authorization_server_metadata only checks oauth2_issuer_url, while _build_mcp_auth_kwargs requires all three: auth_mode == "oauth2", oauth2_issuer_url, and oauth2_resource_host. This inconsistency means the endpoint can serve metadata with resource: "https://localhost" when oauth2_resource_host is unset, producing incorrect metadata that would misdirect clients. It also serves OAuth metadata even when auth_mode is "disabled" or "token".

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1f5dc32. Configure here.



def on_start_logger(port: int):
"""Log startup information"""
print("\n-----------------------------------")
Expand Down
72 changes: 51 additions & 21 deletions agent_memory_server/mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from typing import Any

import ulid
from mcp.server.auth.provider import AccessToken, TokenVerifier
from mcp.server.auth.settings import AuthSettings
from mcp.server.fastmcp import FastMCP as _FastMCPBase

from agent_memory_server import working_memory as working_memory_core
Expand Down Expand Up @@ -179,22 +181,16 @@ async def run_sse_async(self):
).serve()

def streamable_http_app(self):
"""Return a Starlette app for streamable-http with namespace routing."""
from mcp.server.streamable_http_manager import StreamableHTTPSessionManager
from starlette.applications import Starlette
"""Return a Starlette app for streamable-http with namespace routing.

Extends the parent's streamable_http_app() by adding a namespace-prefixed
route while preserving auth middleware and protected resource routes.
"""
from starlette.requests import Request
from starlette.routing import Route

if self._session_manager is None:
self._session_manager = StreamableHTTPSessionManager(
app=self._mcp_server,
event_store=self._event_store,
retry_interval=self._retry_interval,
json_response=self.settings.json_response,
stateless=self.settings.stateless_http,
security_settings=self.settings.transport_security,
)

# Get the parent's app which includes auth middleware and well-known routes
app = super().streamable_http_app()
session_manager = self._session_manager
mcp_instance = self

Expand All @@ -212,14 +208,9 @@ async def __call__(self, scope, receive, send):
handler = _NamespaceAwareHandler()
path = self.settings.streamable_http_path

return Starlette(
debug=self.settings.debug,
routes=[
Route(path, endpoint=handler),
Route(f"/{{namespace}}{path}", endpoint=handler),
],
lifespan=lambda app: session_manager.run(),
)
# Add namespace-prefixed route to the existing app
app.routes.append(Route(f"/{{namespace}}{path}", endpoint=handler))
return app

async def run_streamable_http_async(self):
"""Start streamable HTTP MCP server."""
Expand Down Expand Up @@ -255,12 +246,51 @@ async def run_stdio_async(self):
"""


class JWTTokenVerifier(TokenVerifier):
"""Verify JWT tokens from Hydra using the existing auth module."""

async def verify_token(self, token: str) -> AccessToken | None:
from agent_memory_server.auth import verify_jwt

try:
user_info = verify_jwt(token)
Comment on lines +253 to +256
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

JWTTokenVerifier.verify_token() is async but calls the synchronous verify_jwt(), which performs blocking I/O (httpx.Client().get(...) inside JWKSCache.get_jwks). In the MCP streamable-http transport this can block the event loop under load. Consider running verify_jwt() in a thread (e.g., anyio.to_thread.run_sync) or refactoring JWKS fetching/verification to an async path for MCP.

Suggested change
from agent_memory_server.auth import verify_jwt
try:
user_info = verify_jwt(token)
from anyio import to_thread
from agent_memory_server.auth import verify_jwt
try:
user_info = await to_thread.run_sync(verify_jwt, token)

Copilot uses AI. Check for mistakes.
scopes = user_info.scope.split() if user_info.scope else []
return AccessToken(
token=token,
client_id=user_info.sub,
scopes=scopes,
expires_at=user_info.exp,
)
except Exception:
return None
Comment on lines +255 to +265
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

verify_token() catches Exception and returns None, which will silently convert internal errors (e.g., misconfiguration, JWKS fetch failures) into an auth failure and make diagnosis difficult. Consider catching only expected auth-related exceptions (e.g., FastAPI HTTPException/JWT validation errors) and logging them; let unexpected exceptions propagate so they surface as 5xx with logs.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sync HTTP call blocks async event loop

Low Severity

JWTTokenVerifier.verify_token is an async method that calls the synchronous verify_jwt() directly. Internally, verify_jwt uses httpx.Client (synchronous) to fetch JWKS keys, which can block the event loop for up to 10 seconds during cache misses. In the REST API, FastAPI runs sync dependencies in a thread pool automatically, but here the sync call runs directly on the event loop. Using asyncio.to_thread or similar would avoid the blocking.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2e0b5c7. Configure here.



def _build_mcp_auth_kwargs() -> dict[str, Any]:
"""Build auth kwargs for FastMCP if OAuth2 is configured."""
if (
settings.auth_mode != "oauth2"
or not settings.oauth2_issuer_url
or not settings.oauth2_resource_host
):
return {}
Comment on lines +270 to +275
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

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

_build_mcp_auth_kwargs() gates MCP auth on auth_mode and OAuth2 settings, but it ignores settings.disable_auth. That can lead to the REST API running with auth disabled while MCP enforces OAuth (or vice versa), which is surprising given disable_auth is treated as the global kill switch in auth.get_current_user(). Consider including settings.disable_auth in this guard (or aligning on a single source of truth for enabling auth).

Copilot uses AI. Check for mistakes.

resource_url = f"https://{settings.oauth2_resource_host}"
return {
"auth": AuthSettings(
issuer_url=settings.oauth2_issuer_url,
resource_server_url=resource_url,
),
"token_verifier": JWTTokenVerifier(),
}


mcp_app = FastMCP(
"Redis Agent Memory Server",
host=settings.mcp_host,
port=settings.mcp_port,
instructions=INSTRUCTIONS,
default_namespace=settings.default_mcp_namespace,
**_build_mcp_auth_kwargs(),
)


Expand Down
Loading