Address PR feedback: improve auth failure tracking, update DNS rebinding protection, add client credentials tests

- Replace _scopeStepUpCount with _repeatedAuthFailureCount to track all auth failures
- Reset counter only on successful requests to prevent infinite loops
- Replace custom DNS rebinding middleware with AllowedHosts configuration
- Add client_credentials grant support to TestOAuthServer with basic auth and post methods
- Add ClientCredentialsTests with infinite loop prevention test
- Fix off-by-one error in MaxReconnectionAttempts

Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>

Author: Copilot

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

@@ -50,12 +50,13 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
     private string? _clientSecret;
     private ITokenCache _tokenCache;
     private AuthorizationServerMetadata? _authServerMetadata;
-    private int _scopeStepUpCount;
+    private int _repeatedAuthFailureCount;
 
     /// <summary>
-    /// Maximum number of scope step-up retries before failing.
+    /// Maximum number of repeated auth failure retries before failing.
+    /// This prevents infinite loops when tokens are never accepted by the server.
     /// </summary>
-    private const int MaxScopeStepUpRetries = 3;
+    private const int MaxRepeatedAuthFailures = 3;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="ClientOAuthProvider"/> class using the specified options.
@@ -166,6 +167,8 @@ internal override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage r
             return await HandleUnauthorizedResponseAsync(request, message, response, attemptedRefresh, cancellationToken).ConfigureAwait(false);
         }
 
+        // Reset the auth failure counter on successful request
+        _repeatedAuthFailureCount = 0;
         return response;
     }
 
@@ -263,18 +266,11 @@ private async Task<HttpResponseMessage> HandleUnauthorizedResponseAsync(
     /// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests.</param>
     private async Task<string> GetAccessTokenAsync(HttpResponseMessage response, bool attemptedRefresh, CancellationToken cancellationToken)
     {
-        // Check if this is a scope step-up retry (403 with insufficient_scope)
-        if (HasInsufficientScopeError(response))
+        // Track all auth failures to prevent infinite redirect loops.
+        // This counter is only reset when a request succeeds (in SendAsync).
+        if (++_repeatedAuthFailureCount > MaxRepeatedAuthFailures)
         {
-            if (++_scopeStepUpCount >= MaxScopeStepUpRetries)
-            {
-                ThrowFailedToHandleUnauthorizedResponse($"Maximum scope step-up retry limit ({MaxScopeStepUpRetries}) exceeded.");
-            }
-        }
-        else
-        {
-            // Reset the counter for non-scope-step-up requests (e.g., initial auth or token expiry)
-            _scopeStepUpCount = 0;
+            ThrowFailedToHandleUnauthorizedResponse($"Maximum repeated authentication failure limit ({MaxRepeatedAuthFailures}) exceeded. The server may be rejecting all tokens.");
         }
 
         // Get available authorization servers from the 401 or 403 response

src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs

@@ -145,7 +145,7 @@ private async Task ReceiveMessagesAsync(CancellationToken cancellationToken)
     {
         int attempt = 0;
 
-        while (attempt < _options.MaxReconnectionAttempts && !cancellationToken.IsCancellationRequested)
+        while (attempt <= _options.MaxReconnectionAttempts && !cancellationToken.IsCancellationRequested)
         {
             try
             {

tests/ModelContextProtocol.AspNetCore.Tests/OAuth/ClientCredentialsTests.cs

@@ -0,0 +1,109 @@
+using Microsoft.AspNetCore.Authentication.JwtBearer;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.IdentityModel.Tokens;
+using ModelContextProtocol.AspNetCore;
+using ModelContextProtocol.AspNetCore.Authentication;
+using ModelContextProtocol.Authentication;
+using ModelContextProtocol.Client;
+
+namespace ModelContextProtocol.AspNetCore.Tests.OAuth;
+
+/// <summary>
+/// Tests for client_credentials OAuth flow with various authentication methods.
+/// </summary>
+public class ClientCredentialsTests : OAuthTestBase
+{
+    public ClientCredentialsTests(ITestOutputHelper outputHelper)
+        : base(outputHelper)
+    {
+    }
+
+    [Fact]
+    public async Task CanAuthenticate_WithClientCredentials_ClientSecretPost()
+    {
+        await using var app = await StartMcpServerAsync();
+
+        // Use client_credentials flow with client_secret_post authentication
+        // Note: No AuthorizationRedirectDelegate means machine-to-machine flow will be attempted
+        await using var transport = new HttpClientTransport(new()
+        {
+            Endpoint = new(McpServerUrl),
+            OAuth = new ClientOAuthOptions
+            {
+                ClientId = "client-credentials-post",
+                ClientSecret = "cc-secret-post",
+                RedirectUri = new Uri("http://localhost:1179/callback"),
+                // No AuthorizationRedirectDelegate - triggers client_credentials flow
+            },
+        }, HttpClient, LoggerFactory);
+
+        await using var client = await McpClient.CreateAsync(
+            transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
+
+        Assert.NotNull(client);
+    }
+
+    [Fact]
+    public async Task CanAuthenticate_WithClientCredentials_ClientSecretBasic()
+    {
+        await using var app = await StartMcpServerAsync();
+
+        // Use client_credentials flow with client_secret_basic authentication
+        await using var transport = new HttpClientTransport(new()
+        {
+            Endpoint = new(McpServerUrl),
+            OAuth = new ClientOAuthOptions
+            {
+                ClientId = "client-credentials-basic",
+                ClientSecret = "cc-secret-basic",
+                RedirectUri = new Uri("http://localhost:1179/callback"),
+                // No AuthorizationRedirectDelegate - triggers client_credentials flow
+            },
+        }, HttpClient, LoggerFactory);
+
+        await using var client = await McpClient.CreateAsync(
+            transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
+
+        Assert.NotNull(client);
+    }
+
+    [Fact]
+    public async Task DoesNotLoopIndefinitely_WhenTokensAlwaysRejected()
+    {
+        // Set up a server that always returns 401 even after authentication
+        // This simulates a buggy MCP server that never accepts tokens
+        var app = Builder.Build();
+        
+        // Add middleware that always returns 401 with the MCP auth challenge
+        app.Use((HttpContext context, RequestDelegate next) =>
+        {
+            context.Response.StatusCode = StatusCodes.Status401Unauthorized;
+            context.Response.Headers.WWWAuthenticate = $"Bearer realm=\"{OAuthServerUrl}\" resource_metadata=\"{McpServerUrl}/.well-known/oauth-protected-resource\"";
+            return context.Response.WriteAsync("Unauthorized");
+        });
+
+        await app.StartAsync(TestContext.Current.CancellationToken);
+        await using var _ = app;
+
+        await using var transport = new HttpClientTransport(new()
+        {
+            Endpoint = new(McpServerUrl),
+            OAuth = new ClientOAuthOptions
+            {
+                ClientId = "client-credentials-post",
+                ClientSecret = "cc-secret-post",
+                RedirectUri = new Uri("http://localhost:1179/callback"),
+                // No AuthorizationRedirectDelegate - triggers client_credentials flow
+            },
+        }, HttpClient, LoggerFactory);
+
+        // Should throw McpException after max retries, not loop indefinitely
+        var ex = await Assert.ThrowsAsync<McpException>(async () =>
+            await McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));
+
+        Assert.Contains("Maximum repeated authentication failure limit", ex.Message);
+    }
+}

tests/ModelContextProtocol.ConformanceServer/Program.cs

@@ -92,8 +92,6 @@ public static async Task MainAsync(string[] args, ILoggerProvider? loggerProvide
 
         var app = builder.Build();
 
-        app.UseMcpDnsRebindingProtection();
-
         app.MapMcp();
 
         app.MapGet("/health", () => "Healthy");

tests/ModelContextProtocol.ConformanceServer/appsettings.json

@@ -5,5 +5,5 @@
       "Microsoft.AspNetCore": "Warning"
     }
   },
-  "AllowedHosts": "*"
+  "AllowedHosts": "localhost;127.0.0.1;[::1]"
 }

tests/ModelContextProtocol.TestOAuthServer/ClientInfo.cs

@@ -26,4 +26,25 @@ internal sealed class ClientInfo
     /// Gets or sets the list of redirect URIs allowed for this client.
     /// </summary>
     public List<string> RedirectUris { get; init; } = [];
+
+    /// <summary>
+    /// Gets or sets the token endpoint auth method for this client.
+    /// Supported values: "client_secret_post", "client_secret_basic", "private_key_jwt", "none"
+    /// </summary>
+    public string TokenEndpointAuthMethod { get; init; } = "client_secret_post";
+
+    /// <summary>
+    /// Gets or sets the allowed grant types for this client.
+    /// </summary>
+    public List<string> AllowedGrantTypes { get; init; } = ["authorization_code", "refresh_token"];
+
+    /// <summary>
+    /// Gets or sets the client's JWKS URI for JWT client assertion verification.
+    /// </summary>
+    public string? JwksUri { get; init; }
+
+    /// <summary>
+    /// Gets or sets the client's public key PEM for JWT client assertion verification (inline, no JWKS fetch).
+    /// </summary>
+    public string? PublicKeyPem { get; init; }
 }