Skip to content
This repository was archived by the owner on Oct 17, 2026. It is now read-only.

fix(mcp): OAuth token in connection tests, 502 for OAuth errors, optional session id - #1

Open
kelvinymoura wants to merge 3 commits into
mariomosca:feat/mcp-oauth-dcr-headersfrom
kelvinymoura:fix/mcp-oauth-test-connection-errors
Open

kelvinymoura wants to merge 3 commits into
mariomosca:feat/mcp-oauth-dcr-headersfrom
kelvinymoura:fix/mcp-oauth-test-connection-errors

Conversation

@kelvinymoura

@kelvinymoura kelvinymoura commented Oct 15, 2026 •

Copy link
Copy Markdown

Follow-up to iOfficeAI#1029, fixing three gaps found while testing the OAuth login end to end against https://mcp.cloudflare.com/mcp.

  • Connection test ignored the OAuth token. POST /api/mcp/test-connection now authenticates HTTP/SSE tests with the stored token via IMcpAccessTokenProvider + apply_bearer_header, like session builds, without overriding a user-configured Authorization header. The Tools page no longer reports "authentication required" after a successful login.
  • OAuth failures surfaced as a generic 500. Discovery and dynamic client registration failures now return 502 MCP_OAUTH_DISCOVERY_FAILED / MCP_OAUTH_REGISTRATION_FAILED with the real message; registration passes through the server's RFC 7591 error body. The discovery variant follows fix(mcp): discover OAuth endpoints per spec and support dynamic client registration iOfficeAI/AionCore#988.
  • Live test required Mcp-Session-Id. oauth_live_manual echoes it only when the server returns one; stateless streamable HTTP servers such as Cloudflare's send none.

Testing: new connection_test_oauth_integration.rs (bearer-protected fake MCP server: no token → needs auth, stored token → lists tools, user header wins, no cross-user token, SSE presents the token), new mcp_e2e cases (connection test before/after storing a token; discovery failure → 502), and error-mapping unit tests. On Windows: cargo fmt --check, clippy -D warnings for aionui-mcp and aionui-app (lib + mcp_e2e), cargo test -p aionui-mcp -p aionui-ai-agent and cargo test -p aionui-app --test mcp_e2e. Pre-existing Unix-only tests and lints in aionui-ai-agent and the stdio cases in connection_test_integration fail on Windows independently of this change.

Live, against https://mcp.cloudflare.com/mcp: oauth_live_manual passed (discovery, dynamic client registration, authenticated initialize → 200, tools/list → 4 tools; the server returned no Mcp-Session-Id), and in AionUi's Tools page, running on this branch's backend, the connection test succeeded after the OAuth login and listed the tools, with no "authentication required" state left.

POST /api/mcp/test-connection connected without the token stored by the
OAuth login, so after a successful login the Tools page kept reporting
"authentication required" even though agent sessions already sent the
bearer.

The connection test service now takes the same IMcpAccessTokenProvider
the session build uses and adds `Authorization: Bearer <token>` to HTTP
and SSE tests through apply_bearer_header, leaving an Authorization
header configured by the user untouched. The app wires the shared
McpOAuthService into it.
McpError::OAuth maps to ApiError::Internal, so a server that publishes
no OAuth metadata, or a rejected dynamic client registration, reached
the user as a generic "Internal server error.".

Discovery failures now raise McpError::OAuthDiscovery and RFC 7591
registration failures McpError::OAuthRegistration. Both map to 502 with
the codes MCP_OAUTH_DISCOVERY_FAILED and MCP_OAUTH_REGISTRATION_FAILED
and keep their message; a rejected registration carries the
authorization server's own error body. The discovery variant follows
the mapping introduced in iOfficeAI#988.
The live OAuth test panicked against stateless streamable HTTP servers,
such as https://mcp.cloudflare.com/mcp, that never return an
Mcp-Session-Id. The header is optional in the streamable HTTP
transport, so the test now echoes it only when initialize returned one.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant