From 13a634c3b9d43f7517d6fc109bcd29e5087efc76 Mon Sep 17 00:00:00 2001 From: Xue Cai Date: Mon, 16 Mar 2026 11:55:13 -0700 Subject: [PATCH] fix: propagate auth errors (401/403) immediately instead of falling back to SSE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When AutoDetectingClientSessionTransport receives a 401 or 403 from the initial Streamable HTTP POST, it should NOT fall back to SSE transport. Auth errors are not transport-related — the server understood the request but rejected the credentials. Falling back to SSE would: 1. Fail with the same credentials (or a different transport error) 2. Mask the real authentication error from the caller This was discovered investigating Azure AI Search MCP endpoints returning 403 on Streamable HTTP POST, which caused the SDK to fall back to SSE GET, resulting in a confusing 405 error that hid the real auth failure. The fix adds an explicit check for 401/403 before the SSE fallback path, and throws HttpRequestException with the auth status code immediately. Code reference: AutoDetectingClientSessionTransport.InitializeAsync https://github.com/modelcontextprotocol/csharp-sdk/blob/v0.9.0-preview.2/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs#L61 Testing: # Build all frameworks (only netstandard2.0 fails — pre-existing on main): dotnet build tests/ModelContextProtocol.Tests/ '/p:NoWarn=NU1903%3BMCPEXP001' # Run auto-detect tests (4 tests × 3 frameworks = 12 total, 0 failures): dotnet test tests/ModelContextProtocol.Tests/ '/p:NoWarn=NU1903%3BMCPEXP001' --no-build --filter "FullyQualifiedName~HttpClientTransportAutoDetectTests" --framework net10.0 dotnet test tests/ModelContextProtocol.Tests/ '/p:NoWarn=NU1903%3BMCPEXP001' --no-build --filter "FullyQualifiedName~HttpClientTransportAutoDetectTests" --framework net9.0 dotnet test tests/ModelContextProtocol.Tests/ '/p:NoWarn=NU1903%3BMCPEXP001' --no-build --filter "FullyQualifiedName~HttpClientTransportAutoDetectTests" --framework net8.0 # Run full transport suite (75 tests × 3 frameworks = 225 total, 0 failures): dotnet test tests/ModelContextProtocol.Tests/ '/p:NoWarn=NU1903%3BMCPEXP001' --no-build --filter "FullyQualifiedName~Transport" --framework net10.0 dotnet test tests/ModelContextProtocol.Tests/ '/p:NoWarn=NU1903%3BMCPEXP001' --no-build --filter "FullyQualifiedName~Transport" --framework net9.0 dotnet test tests/ModelContextProtocol.Tests/ '/p:NoWarn=NU1903%3BMCPEXP001' --no-build --filter "FullyQualifiedName~Transport" --framework net8.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../AutoDetectingClientSessionTransport.cs | 20 ++++++- .../HttpClientTransportAutoDetectTests.cs | 53 +++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs b/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs index 209d644d2..43f2dd009 100644 --- a/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs +++ b/src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs @@ -73,9 +73,21 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can LogUsingStreamableHttp(_name); ActiveTransport = streamableHttpTransport; } + else if (IsAuthError(response.StatusCode)) + { + // Authentication/authorization errors (401, 403) are not transport-related — + // the server understood the request but rejected the credentials. Falling back + // to SSE would fail with the same credentials and mask the real error. + await streamableHttpTransport.DisposeAsync().ConfigureAwait(false); + + LogStreamableHttpAuthError(_name, response.StatusCode); + + await response.EnsureSuccessStatusCodeWithResponseBodyAsync(cancellationToken).ConfigureAwait(false); + } else { - // If the status code is not success, fall back to SSE + // Non-auth, non-success status codes (404, 405, 501, etc.) suggest the server + // may not support Streamable HTTP — fall back to SSE. LogStreamableHttpFailed(_name, response.StatusCode); await streamableHttpTransport.DisposeAsync().ConfigureAwait(false); @@ -91,6 +103,9 @@ private async Task InitializeAsync(JsonRpcMessage message, CancellationToken can } } + private static bool IsAuthError(HttpStatusCode statusCode) => + statusCode is HttpStatusCode.Unauthorized or HttpStatusCode.Forbidden; + private async Task InitializeSseTransportAsync(JsonRpcMessage message, CancellationToken cancellationToken) { if (_options.KnownSessionId is not null) @@ -139,6 +154,9 @@ public async ValueTask DisposeAsync() [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} streamable HTTP transport failed with status code {StatusCode}, falling back to SSE transport.")] private partial void LogStreamableHttpFailed(string endpointName, HttpStatusCode statusCode); + [LoggerMessage(Level = LogLevel.Warning, Message = "{EndpointName} streamable HTTP transport received authentication error {StatusCode}. Not falling back to SSE.")] + private partial void LogStreamableHttpAuthError(string endpointName, HttpStatusCode statusCode); + [LoggerMessage(Level = LogLevel.Information, Message = "{EndpointName} using Streamable HTTP transport.")] private partial void LogUsingStreamableHttp(string endpointName); diff --git a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs index 768ebf7ea..09c553c26 100644 --- a/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs +++ b/tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs @@ -47,6 +47,59 @@ public async Task AutoDetectMode_UsesStreamableHttp_WhenServerSupportsIt() Assert.NotNull(session); } + [Theory] + [InlineData(HttpStatusCode.Unauthorized)] + [InlineData(HttpStatusCode.Forbidden)] + public async Task AutoDetectMode_DoesNotFallBackToSse_OnAuthError(HttpStatusCode authStatusCode) + { + // Auth errors (401, 403) are not transport-related — the server understood the + // request but rejected the credentials. The SDK should propagate the error + // immediately instead of falling back to SSE, which would mask the real cause. + var options = new HttpClientTransportOptions + { + Endpoint = new Uri("http://localhost"), + TransportMode = HttpTransportMode.AutoDetect, + Name = "AutoDetect test client" + }; + + using var mockHttpHandler = new MockHttpHandler(); + using var httpClient = new HttpClient(mockHttpHandler); + await using var transport = new HttpClientTransport(options, httpClient, LoggerFactory); + + var requestMethods = new List(); + + mockHttpHandler.RequestHandler = (request) => + { + requestMethods.Add(request.Method); + + if (request.Method == HttpMethod.Post) + { + // Streamable HTTP POST returns auth error + return Task.FromResult(new HttpResponseMessage + { + StatusCode = authStatusCode, + Content = new StringContent($"{{\"error\": \"{authStatusCode}\"}}") + }); + } + + // SSE GET should never be reached + throw new InvalidOperationException("Should not fall back to SSE on auth error"); + }; + + // ConnectAsync for AutoDetect mode just creates the transport without sending + // any HTTP request. The auto-detection is triggered lazily by the first + // SendMessageAsync call, which happens inside McpClient.CreateAsync when it + // sends the JSON-RPC "initialize" message. + var ex = await Assert.ThrowsAsync( + () => McpClient.CreateAsync(transport, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal(authStatusCode, ex.StatusCode); + + // Verify only POST was sent — no GET fallback + Assert.Single(requestMethods); + Assert.Equal(HttpMethod.Post, requestMethods[0]); + } + [Fact] public async Task AutoDetectMode_FallsBackToSse_WhenStreamableHttpFails() {