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
2 changes: 1 addition & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
that trips NU1902 under our TreatWarningsAsErrors policy. -->
<AspireHostingVersion>13.4.6</AspireHostingVersion>
<CommunityToolkitAspireVersion>13.4.0</CommunityToolkitAspireVersion>
<ModelContextProtocolVersion>2.0.0</ModelContextProtocolVersion>
<ModelContextProtocolVersion>2.1.0</ModelContextProtocolVersion>
</PropertyGroup>
<!-- App dependencies -->
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// -----------------------------------------------------------------------
// <copyright file="OAuthClientRejectionHandlerTests.cs" company="Petabridge, LLC">
// Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com>
// </copyright>
// -----------------------------------------------------------------------
using System.Net;
using Netclaw.Configuration.Http;
using Xunit;

namespace Netclaw.Configuration.Tests.Http;

public sealed class OAuthClientRejectionHandlerTests
{
[Fact]
public async Task Authorization_code_exchange_rejected_with_invalid_client_throws()
{
var error = await Assert.ThrowsAsync<McpOAuthClientRejectedException>(() =>
ExchangeAsync(
grant: "authorization_code",
status: HttpStatusCode.BadRequest,
body: """{"error":"invalid_client"}"""));

// The message carries the OAuth error code so the manager's message-based checks match.
Assert.Contains("invalid_client", error.Message, StringComparison.Ordinal);
}

[Fact]
public async Task Refresh_grant_rejected_with_invalid_client_passes_through()
{
// A refresh failure must keep the SDK's graceful null path, so the handler stays out
// of the way even when the error code matches.
var response = await ExchangeAsync(
grant: "refresh_token",
status: HttpStatusCode.BadRequest,
body: """{"error":"invalid_client"}""");

Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
Assert.Contains(
"invalid_client",
await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken),
StringComparison.Ordinal);
}

[Fact]
public async Task Authorization_code_exchange_with_other_error_passes_through()
{
var response = await ExchangeAsync(
grant: "authorization_code",
status: HttpStatusCode.BadRequest,
body: """{"error":"invalid_grant"}""");

// The body must survive the handler's inspection so the SDK can still read it.
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
Assert.Contains(
"invalid_grant",
await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken),
StringComparison.Ordinal);
}

[Fact]
public async Task Successful_authorization_code_exchange_passes_through()
{
var response = await ExchangeAsync(
grant: "authorization_code",
status: HttpStatusCode.OK,
body: """{"access_token":"token"}""");

Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Contains(
"access_token",
await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken),
StringComparison.Ordinal);
}

[Fact]
public async Task Non_form_request_passes_through_even_on_bad_request()
{
// MCP JSON-RPC traffic never carries a grant_type; a 400 with the literal text must
// not be mistaken for a token rejection.
using var handler = new OAuthClientRejectionHandler { InnerHandler = new StubHandler(HttpStatusCode.BadRequest, """{"error":"invalid_client"}""") };
using var client = new HttpClient(handler);
using var request = new HttpRequestMessage(HttpMethod.Post, "https://example.invalid/mcp")
{
Content = new StringContent("""{"jsonrpc":"2.0"}""", System.Text.Encoding.UTF8, "application/json"),
};

using var response = await client.SendAsync(request, TestContext.Current.CancellationToken);

Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
}

private static async Task<HttpResponseMessage> ExchangeAsync(string grant, HttpStatusCode status, string body)
{
var handler = new OAuthClientRejectionHandler { InnerHandler = new StubHandler(status, body) };
var client = new HttpClient(handler);
using var request = new HttpRequestMessage(HttpMethod.Post, "https://example.invalid/oauth/token")
{
Content = new FormUrlEncodedContent(new Dictionary<string, string>
{
["grant_type"] = grant,
["client_id"] = "client-1",
}),
};

return await client.SendAsync(request, TestContext.Current.CancellationToken);
}

private sealed class StubHandler(HttpStatusCode status, string body) : HttpMessageHandler
{
protected override Task<HttpResponseMessage> SendAsync(
HttpRequestMessage request,
CancellationToken cancellationToken)
=> Task.FromResult(new HttpResponseMessage(status)
{
Content = new StringContent(body, System.Text.Encoding.UTF8, "application/json"),
});
}
}
82 changes: 80 additions & 2 deletions src/Netclaw.Configuration/Http/McpHttpClientFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
// Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com>
// </copyright>
// -----------------------------------------------------------------------
using System.Net;

namespace Netclaw.Configuration.Http;

/// <summary>
Expand All @@ -29,9 +31,15 @@ internal static class McpHttpClientFactory
internal static HttpClient Create(HttpMessageHandler innerHandler)
{
ArgumentNullException.ThrowIfNull(innerHandler);
return new HttpClient(new ProtocolVersionHandler

// Order matters: the rejection handler must see the token endpoint's final
// response, so it wraps the protocol-version handler rather than the reverse.
return new HttpClient(new OAuthClientRejectionHandler
{
InnerHandler = innerHandler,
InnerHandler = new ProtocolVersionHandler
{
InnerHandler = innerHandler,
},
});
}

Expand Down Expand Up @@ -61,3 +69,73 @@ protected override Task<HttpResponseMessage> SendAsync(
}
}
}

/// <summary>
/// Surfaces an OAuth <c>invalid_client</c> rejection from the token endpoint as a distinct
/// exception the MCP SDK does not recognize.
/// </summary>
/// <remarks>
/// MCP SDK 2.1 probes the server with <c>server/discover</c> before the initialize
/// handshake. When the authorization-code exchange for that probe fails with
/// <c>400 invalid_client</c>, the SDK reads the 400 as an unsupported-protocol signal,
/// falls back to the initialize handshake, and calls the one-shot authorization callback a
/// second time. That second call fails as "authorization already in progress" and hides the
/// real reason the exchange failed, so the manager can no longer tell a dead dynamic client
/// registration from any other connection failure. This handler reads the token error before
/// the SDK sees the 400 and throws a type the discover fallback does not catch, so the true
/// cause reaches the manager. The exchange is doomed either way; the throw only replaces the
/// misleading failure with an accurate one.
/// </remarks>
internal sealed class OAuthClientRejectionHandler : DelegatingHandler
{
private const string FormContentType = "application/x-www-form-urlencoded";
private const string AuthorizationCodeGrant = "grant_type=authorization_code";
private const string InvalidClientError = "invalid_client";

protected override async Task<HttpResponseMessage> SendAsync(
HttpRequestMessage request,
CancellationToken cancellationToken)
{
// Only the OAuth token endpoint sends form-urlencoded bodies; MCP JSON-RPC traffic is
// JSON. Skip the body read for everything else so tool-call payloads stay untouched.
if (request.Method != HttpMethod.Post
|| request.Content?.Headers.ContentType?.MediaType != FormContentType)
{
return await base.SendAsync(request, cancellationToken);
}

var requestBody = await request.Content.ReadAsStringAsync(cancellationToken);
var response = await base.SendAsync(request, cancellationToken);

// Only the authorization-code exchange drives client-identity discard. A refresh
// failure keeps the SDK's graceful null path, so it is deliberately left untouched.
if (response.StatusCode is HttpStatusCode.BadRequest
&& requestBody.Contains(AuthorizationCodeGrant, StringComparison.Ordinal)
&& await IsInvalidClientErrorAsync(response))
{
response.Dispose();
throw new McpOAuthClientRejectedException();
}

return response;
}

private static async Task<bool> IsInvalidClientErrorAsync(HttpResponseMessage response)
{
// Buffer the small error body so the check does not consume the stream the caller
// would read on the pass-through path.
await response.Content.LoadIntoBufferAsync();
var body = await response.Content.ReadAsStringAsync();
return body.Contains(InvalidClientError, StringComparison.Ordinal);
}
}

/// <summary>
/// Signals that the OAuth token endpoint rejected the client registration with
/// <c>invalid_client</c>. The message carries the OAuth error code so the manager's
/// message-based auth-failure checks recognize it.
/// </summary>
internal sealed class McpOAuthClientRejectedException()
: Exception(
"The OAuth token endpoint rejected the client registration (invalid_client). " +
"The dynamic client identity is no longer valid and must be discarded.");
22 changes: 21 additions & 1 deletion src/Netclaw.Daemon.Tests/Mcp/McpSdkOAuthFlowIntegrationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
using ModelContextProtocol.Server;
using Netclaw.Actors.Tools;
using Netclaw.Configuration;
using Netclaw.Configuration.Http;
using Netclaw.Configuration.Secrets;
using Netclaw.Daemon.Mcp;
using Netclaw.Tests.Utilities;
Expand Down Expand Up @@ -812,7 +813,10 @@ private sealed class FakeServerMcpRuntime(
public IClientTransport CreateHttpTransport(HttpClientTransportOptions options)
{
LastHttpOptions = options;
return new HttpClientTransport(options, server.CreateHttpClient(), ownsHttpClient: true);
// Mirror the production runtime: the SDK's OAuth token exchange runs through the
// rejection handler, so a 400 invalid_client surfaces instead of the SDK's
// protocol fallback masking it.
return new HttpClientTransport(options, server.CreateTransportHttpClient(), ownsHttpClient: true);
}

public Task<McpClient> CreateAsync(
Expand Down Expand Up @@ -1036,6 +1040,22 @@ public HttpClient CreateHttpClient()
return client;
}

/// <summary>
/// Builds the client the SDK transport uses, wrapping the in-memory test server with
/// the same OAuth rejection handler the production runtime installs.
/// </summary>
public HttpClient CreateTransportHttpClient()
{
var client = new HttpClient(new OAuthClientRejectionHandler
{
InnerHandler = _app.GetTestServer().CreateHandler(),
})
{
BaseAddress = _state.Origin,
};
return client;
}

public async Task<BrowserAuthorizationResult> AuthorizeAsync(
Uri authorizationUri,
Uri redirectUri,
Expand Down
6 changes: 5 additions & 1 deletion src/Netclaw.Daemon/Mcp/McpClientManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1525,7 +1525,11 @@ private static bool IsAuthFailureMessage(string message)
/// so an operator-pinned OAuthClientId is never discarded behind their back.
/// </summary>
private static bool IsInvalidClientFailure(Exception ex)
=> ex.Message.Contains("invalid_client", StringComparison.OrdinalIgnoreCase)
// SDK 2.1's discover probe swallows the token endpoint's 400 invalid_client as a
// protocol-fallback signal, so OAuthClientRejectionHandler re-throws it as this type
// before the SDK can hide it. The message check below still covers a raw SDK failure.
=> ex is McpOAuthClientRejectedException
|| ex.Message.Contains("invalid_client", StringComparison.OrdinalIgnoreCase)
// A registration is bound to the issuer that granted it. When the resource server
// moves to a new issuer, SDK 2.0 refuses to reuse the old one and offers no remedy
// of its own, so the stale identity has to go or every retry repeats the failure.
Expand Down
Loading