From 49d4e340ded597e4757f4f66ebbf84761231413b Mon Sep 17 00:00:00 2001 From: Pranav Senthilnathan Date: Wed, 21 Oct 2026 18:48:25 -0700 Subject: [PATCH 1/6] Enforce PKCE metadata compliance in OAuth client (#730) (#1700) (cherry picked from commit 7369a265a89d748b86f028fb132f58ba4d22b961) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Authentication/ClientOAuthProvider.cs | 68 +++++++- .../OAuth/AuthTests.cs | 154 ++++++++++++++++++ .../Program.cs | 18 +- 3 files changed, 230 insertions(+), 10 deletions(-) diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index ecef8e15e..7b26eab6a 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -334,8 +334,15 @@ static bool IsValidClientMetadataDocumentUri(Uri uri) private async Task GetAuthServerMetadataAsync(Uri authServerUri, string? resourceUri, CancellationToken cancellationToken) { + // Tracks whether at least one well-known endpoint returned a structurally valid metadata document. + // This distinguishes "no metadata document was found" (eligible for the 2025-03-26 legacy fallback) + // from "a metadata document exists but failed PKCE validation" (must not fall back to synthesized defaults). + var metadataDocumentFound = false; + McpException? pkceValidationFailure = null; + foreach (var wellKnownEndpoint in GetWellKnownAuthorizationServerMetadataUris(authServerUri)) { + AuthorizationServerMetadata? metadata; try { var response = await _httpClient.GetAsync(wellKnownEndpoint, cancellationToken).ConfigureAwait(false); @@ -345,7 +352,7 @@ private async Task GetAuthServerMetadataAsync(Uri a } using var stream = await response.Content.ReadAsStreamAsync(cancellationToken).ConfigureAwait(false); - var metadata = await JsonSerializer.DeserializeAsync(stream, McpJsonUtilities.JsonContext.Default.AuthorizationServerMetadata, cancellationToken).ConfigureAwait(false); + metadata = await JsonSerializer.DeserializeAsync(stream, McpJsonUtilities.JsonContext.Default.AuthorizationServerMetadata, cancellationToken).ConfigureAwait(false); if (metadata is null) { @@ -363,23 +370,63 @@ private async Task GetAuthServerMetadataAsync(Uri a ThrowFailedToHandleUnauthorizedResponse($"AuthorizationEndpoint must use HTTP or HTTPS. '{metadata.AuthorizationEndpoint}' does not meet this requirement."); } - metadata.ResponseTypesSupported ??= ["code"]; - metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"]; - metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"]; - metadata.CodeChallengeMethodsSupported ??= ["S256"]; - - return metadata; + // A structurally valid metadata document was discovered. Even if it fails PKCE validation + // below, its existence disqualifies the legacy fallback that would otherwise synthesize S256. + metadataDocumentFound = true; } catch (Exception ex) { LogErrorFetchingAuthServerMetadata(ex, wellKnownEndpoint); + continue; + } + + try + { + // Per the MCP spec, clients MUST verify PKCE support via authorization server metadata and refuse + // to proceed without it. Rather than failing on the first document that lacks it, skip to the next + // discovery endpoint: different well-known endpoints (OAuth 2.0 vs OpenID Connect) can return + // different documents for the same server, and OpenID Connect metadata commonly includes the field. + var codeChallengeMethods = metadata.CodeChallengeMethodsSupported; + if (codeChallengeMethods is null) + { + ThrowFailedToHandleUnauthorizedResponse( + $"Authorization server metadata from '{wellKnownEndpoint}' does not include 'code_challenge_methods_supported'. MCP clients require PKCE support and must refuse to proceed."); + } + + if (!codeChallengeMethods.Contains("S256")) + { + var advertisedMethods = codeChallengeMethods.Count > 0 + ? string.Join(", ", codeChallengeMethods) + : ""; + ThrowFailedToHandleUnauthorizedResponse( + $"Authorization server metadata from '{wellKnownEndpoint}' does not advertise required PKCE method 'S256' in 'code_challenge_methods_supported'. Advertised methods: {advertisedMethods}."); + } + } + catch (Exception ex) + { + LogAuthServerMetadataMissingPkceSupport(ex, wellKnownEndpoint); + pkceValidationFailure = ex as McpException ?? new McpException(ex.Message, ex); + continue; } + + metadata.ResponseTypesSupported ??= ["code"]; + metadata.GrantTypesSupported ??= ["authorization_code", "refresh_token"]; + metadata.TokenEndpointAuthMethodsSupported ??= ["client_secret_post"]; + + return metadata; + } + + // A discovered metadata document that failed PKCE validation must not be replaced with synthesized + // defaults. Surface the specific PKCE failure regardless of whether PRM was available. + if (metadataDocumentFound && pkceValidationFailure is not null) + { + throw pkceValidationFailure; } if (resourceUri is null) { - // 2025-03-26 backcompat: when PRM is unavailable and auth server metadata discovery - // also fails, fall back to default endpoint paths per the 2025-03-26 spec. + // 2025-03-26 backcompat: when PRM is unavailable and no auth server metadata document was + // discovered, fall back to default endpoint paths per the 2025-03-26 spec. return BuildDefaultAuthServerMetadata(authServerUri); } @@ -989,6 +1036,9 @@ private static void ThrowFailedToHandleUnauthorizedResponse(string message) => [LoggerMessage(Level = LogLevel.Error, Message = "Error fetching auth server metadata from {Endpoint}")] partial void LogErrorFetchingAuthServerMetadata(Exception ex, Uri endpoint); + [LoggerMessage(Level = LogLevel.Warning, Message = "Authorization server metadata from {Endpoint} does not satisfy PKCE requirements; skipping it and trying the next discovery endpoint.")] + partial void LogAuthServerMetadataMissingPkceSupport(Exception ex, Uri endpoint); + [LoggerMessage(Level = LogLevel.Information, Message = "Performing dynamic client registration with {RegistrationEndpoint}")] partial void LogPerformingDynamicClientRegistration(Uri registrationEndpoint); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index c4979fb10..ee20e5908 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -131,6 +131,85 @@ public async Task CanAuthenticate_WithClientMetadataDocument() transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); } + [Fact] + public async Task CannotAuthenticate_WhenMetadataOmitsPkceMethods() + { + TestOAuthServer.CodeChallengeMethodsSupported = null; + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + // No discovery endpoint advertises PKCE, so metadata discovery is exhausted. The precise PKCE reason + // is logged as each endpoint is skipped. + Assert.Contains( + MockLoggerProvider.LogMessages, + m => m.Exception?.Message.Contains("code_challenge_methods_supported") == true); + } + + [Fact] + public async Task CannotAuthenticate_WhenMetadataLacksS256PkceMethod() + { + TestOAuthServer.CodeChallengeMethodsSupported = ["plain"]; + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains( + MockLoggerProvider.LogMessages, + m => m.Exception?.Message.Contains("required PKCE method 'S256'") == true); + } + + [Fact] + public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherAdvertisesIt() + { + // The OAuth 2.0 authorization server metadata endpoint is tried before the OpenID Connect one. + // Simulate a server where only the OpenID Connect document advertises PKCE support, and verify the + // client falls through to it rather than failing on the first PKCE-less document. + TestOAuthServer.MetadataPathsWithoutPkceSupport.Add("/.well-known/oauth-authorization-server"); + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + } + [Fact] public async Task UsesDynamicClientRegistration_WhenCimdNotSupported() { @@ -1261,4 +1340,79 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() await using var client = await McpClient.CreateAsync( transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); } + + [Fact] + public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataOmitsPkceMethods() + { + // 2025-03-26 backcompat regression guard: PRM is unavailable (resourceUri is null), but the server + // DOES serve an auth server metadata document that omits 'code_challenge_methods_supported'. + // The client must refuse to proceed rather than falling back to synthesized S256 defaults, since a + // discovered metadata document that fails PKCE validation disqualifies the legacy default fallback. + TestOAuthServer.ExpectResource = false; + + // Use JwtBearer as the challenge scheme so the 401 response does NOT include resource_metadata. + Builder.Services.Configure(options => options.DefaultChallengeScheme = JwtBearerDefaults.AuthenticationScheme); + + Builder.Services.Configure(JwtBearerDefaults.AuthenticationScheme, options => + { + options.TokenValidationParameters.ValidateAudience = false; + }); + + await using var app = Builder.Build(); + + app.Use(async (context, next) => + { + // Return 404 for PRM to simulate a legacy server that doesn't support RFC 9728. + if (context.Request.Path.StartsWithSegments("/.well-known/oauth-protected-resource")) + { + context.Response.StatusCode = StatusCodes.Status404NotFound; + return; + } + + // Serve auth server metadata that omits 'code_challenge_methods_supported' entirely. + if (context.Request.Path.StartsWithSegments("/.well-known/oauth-authorization-server") || + context.Request.Path.StartsWithSegments("/.well-known/openid-configuration")) + { + context.Response.ContentType = "application/json"; + await context.Response.WriteAsync($$""" + { + "issuer": "{{OAuthServerUrl}}", + "authorization_endpoint": "{{McpServerUrl}}/authorize", + "token_endpoint": "{{McpServerUrl}}/token", + "registration_endpoint": "{{McpServerUrl}}/register", + "response_types_supported": ["code"], + "grant_types_supported": ["authorization_code", "refresh_token"], + "token_endpoint_auth_methods_supported": ["client_secret_post"] + } + """); + return; + } + + await next(); + }); + + app.UseAuthentication(); + app.UseAuthorization(); + app.MapMcp().RequireAuthorization(); + await app.StartAsync(TestContext.Current.CancellationToken); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + // The specific PKCE failure reason is surfaced rather than a generic discovery failure or a + // silently-synthesized S256 fallback. + Assert.Contains("code_challenge_methods_supported", ex.Message); + } } diff --git a/tests/ModelContextProtocol.TestOAuthServer/Program.cs b/tests/ModelContextProtocol.TestOAuthServer/Program.cs index 1da81c2a7..3995b0e0e 100644 --- a/tests/ModelContextProtocol.TestOAuthServer/Program.cs +++ b/tests/ModelContextProtocol.TestOAuthServer/Program.cs @@ -90,6 +90,20 @@ public Program(ILoggerProvider? loggerProvider = null, IConnectionListenerFactor /// public bool ExpectResource { get; set; } = true; + /// + /// Gets or sets the code challenge methods advertised by metadata endpoints. + /// + /// + /// The default value is ["S256"]. + /// + public List? CodeChallengeMethodsSupported { get; set; } = ["S256"]; + + /// + /// Gets the set of metadata paths that should omit code_challenge_methods_supported from their + /// response, simulating a server whose discovery endpoints advertise differing PKCE support. + /// + public HashSet MetadataPathsWithoutPkceSupport { get; } = new(StringComparer.OrdinalIgnoreCase); + public HashSet DisabledMetadataPaths { get; } = new(StringComparer.OrdinalIgnoreCase); public IReadOnlyCollection MetadataRequests => _metadataRequests.ToArray(); @@ -222,7 +236,9 @@ IResult HandleMetadataRequest(HttpContext context, string? issuerPath = null) ScopesSupported = ["openid", "profile", "email", "mcp:tools"], TokenEndpointAuthMethodsSupported = ["client_secret_post"], ClaimsSupported = ["sub", "iss", "name", "email", "aud"], - CodeChallengeMethodsSupported = ["S256"], + CodeChallengeMethodsSupported = MetadataPathsWithoutPkceSupport.Contains(context.Request.Path) + ? null + : CodeChallengeMethodsSupported, GrantTypesSupported = ["authorization_code", "refresh_token"], IntrospectionEndpoint = $"{_url}/introspect", RegistrationEndpoint = $"{_url}/register", From 193ad4cd4258075fb41468b54ec728f794c2f97a Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 21 Oct 2026 19:15:16 -0700 Subject: [PATCH 2/6] Require explicit opt-in for legacy OAuth metadata fallback Keep the release/1.x OAuth client fail-closed when PKCE support cannot be verified from authorization server metadata. Retain the historical no-metadata fallback only behind an explicit process-wide compatibility switch, with a warning and deployment requirements. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/concepts/transports/transports.md | 29 ++++ samples/ProtectedMcpClient/README.md | 2 + .../Authentication/ClientOAuthOptions.cs | 9 ++ .../Authentication/ClientOAuthProvider.cs | 13 +- .../OAuth/AuthTests.cs | 152 ++++++++++++++++-- .../TestAttributes.cs | 4 + .../Program.cs | 6 + .../Program.cs | 12 ++ 8 files changed, 208 insertions(+), 19 deletions(-) create mode 100644 tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs diff --git a/docs/concepts/transports/transports.md b/docs/concepts/transports/transports.md index eaacda709..f000b9df9 100644 --- a/docs/concepts/transports/transports.md +++ b/docs/concepts/transports/transports.md @@ -145,6 +145,35 @@ var transport = new HttpClientTransport(new HttpClientTransportOptions }); ``` +#### OAuth PKCE metadata validation (1.x) + +When using with an HTTP transport, authorization server metadata must advertise `S256` in `code_challenge_methods_supported`. A document that omits that field or advertises only other methods is skipped while the client tries alternative well-known endpoints. If none supplies valid metadata with `S256`, authentication fails before requesting an authorization code or exchanging tokens. This is a compatibility tightening: servers that implement PKCE but do not advertise it must update their metadata. + +The 1.x SDK's authorization callback returns only a code, so the SDK cannot validate response `state`. [RFC 9700, Section 4.7.1](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.7.1) permits relying on PKCE for CSRF protection only when authorization server support has been ensured. Metadata verification checks **advertised support**, not the server's actual implementation. The authorization server must correctly enforce PKCE, including the [downgrade defenses in Section 4.8.2](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.8.2). + +**Legacy compatibility escape hatch.** By default, the 1.x client also rejects authentication when authorization server metadata cannot be discovered; it no longer assumes `S256` support. For a trusted legacy deployment whose PKCE support you have independently established, you can explicitly restore the MCP 2025-03-26 default endpoint fallback at application startup: + +```csharp +AppContext.SetSwitch( + "ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback", + true); +``` + +Alternatively, configure the same switch in the application's project file: + +```xml + + + +``` + +The switch is **disabled when absent or `false`**. When enabled, it restores default `/authorize`, `/token`, and `/register` paths on the MCP server's origin only when protected resource metadata is unavailable and no usable authorization server metadata document was discovered. It never bypasses PKCE validation of a discovered document. The client still sends an `S256` challenge and a code verifier, and logs a warning whenever it uses the unverified fallback. + +> [!WARNING] +> Enabling this switch does not verify PKCE support or add state validation. Use it only after independently establishing that your trusted authorization server correctly enforces PKCE, including downgrade defenses. The setting is **process-wide**, not a per-server allowlist: it affects every OAuth client in the process. Do not enable it in a process that also connects to untrusted authorization servers, or toggle it around individual requests. Prefer fixing metadata discovery and advertising `S256` so the switch can remain disabled. + #### Resuming sessions Streamable HTTP supports session resumption. Save the session ID, server capabilities, and server info from the original session, then use to reconnect: diff --git a/samples/ProtectedMcpClient/README.md b/samples/ProtectedMcpClient/README.md index 81ae67cee..194b4f57a 100644 --- a/samples/ProtectedMcpClient/README.md +++ b/samples/ProtectedMcpClient/README.md @@ -69,6 +69,8 @@ The client is configured with: - **OAuth Server**: `https://localhost:7029` - **Protected Resource**: `http://localhost:7071` +The authorization server must advertise `S256` PKCE support in its metadata. The test server does this by default. The 1.x SDK rejects missing PKCE advertisement and, by default, unavailable authorization server metadata; see [OAuth PKCE metadata validation](../../docs/concepts/transports/transports.md#oauth-pkce-metadata-validation-1x) for the narrowly scoped, process-wide legacy compatibility switch and its security requirements. + ## Available Tools Once authenticated, the client can access weather tools including: diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs index 483e3643e..c0037f4bd 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs @@ -3,6 +3,15 @@ namespace ModelContextProtocol.Authentication; /// /// Provides configuration options for the . /// +/// +/// Authorization server metadata must advertise the S256 PKCE method. If metadata discovery fails, +/// authorization fails before requesting an authorization code or exchanging tokens. For trusted legacy +/// deployments without protected resource metadata and without usable authorization server metadata, the process-wide +/// ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback switch +/// can explicitly enable the 2025-03-26 default endpoint fallback. It is disabled by default and does not +/// bypass PKCE validation of discovered metadata. Hosts enabling it must independently establish that +/// their authorization servers correctly enforce PKCE, including downgrade defenses. +/// public sealed class ClientOAuthOptions { /// diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 7b26eab6a..33d72d6af 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -335,7 +335,7 @@ static bool IsValidClientMetadataDocumentUri(Uri uri) private async Task GetAuthServerMetadataAsync(Uri authServerUri, string? resourceUri, CancellationToken cancellationToken) { // Tracks whether at least one well-known endpoint returned a structurally valid metadata document. - // This distinguishes "no metadata document was found" (eligible for the 2025-03-26 legacy fallback) + // This distinguishes "no metadata document was found" (eligible for the opt-in 2025-03-26 legacy fallback) // from "a metadata document exists but failed PKCE validation" (must not fall back to synthesized defaults). var metadataDocumentFound = false; McpException? pkceValidationFailure = null; @@ -425,8 +425,14 @@ private async Task GetAuthServerMetadataAsync(Uri a if (resourceUri is null) { + if (!AppContext.TryGetSwitch("ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback", out var enabled) || !enabled) + { + ThrowFailedToHandleUnauthorizedResponse("Authorization server metadata could not be discovered, so PKCE support could not be verified."); + } + // 2025-03-26 backcompat: when PRM is unavailable and no auth server metadata document was - // discovered, fall back to default endpoint paths per the 2025-03-26 spec. + // discovered, allow explicitly opted-in hosts to use the default endpoint paths. + LogUsingLegacyOAuthMetadataFallback(authServerUri); return BuildDefaultAuthServerMetadata(authServerUri); } @@ -1039,6 +1045,9 @@ private static void ThrowFailedToHandleUnauthorizedResponse(string message) => [LoggerMessage(Level = LogLevel.Warning, Message = "Authorization server metadata from {Endpoint} does not satisfy PKCE requirements; skipping it and trying the next discovery endpoint.")] partial void LogAuthServerMetadataMissingPkceSupport(Exception ex, Uri endpoint); + [LoggerMessage(Level = LogLevel.Warning, Message = "Using legacy OAuth metadata fallback for {AuthorizationServer} because ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback is enabled. PKCE support could not be verified. Use this switch only for trusted deployments whose PKCE support has been independently established.")] + partial void LogUsingLegacyOAuthMetadataFallback(Uri authorizationServer); + [LoggerMessage(Level = LogLevel.Information, Message = "Performing dynamic client registration with {RegistrationEndpoint}")] partial void LogPerformingDynamicClientRegistration(Uri registrationEndpoint); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index ee20e5908..338aeab5c 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using ModelContextProtocol; using ModelContextProtocol.AspNetCore.Authentication; using ModelContextProtocol.Authentication; @@ -19,9 +20,11 @@ namespace ModelContextProtocol.AspNetCore.Tests.OAuth; +[Collection(nameof(DisableParallelization))] public class AuthTests : OAuthTestBase { private const string ClientMetadataDocumentUrl = $"{OAuthServerUrl}/client-metadata/cimd-client.json"; + private const string LegacyOAuthMetadataFallbackSwitch = "ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback"; public AuthTests(ITestOutputHelper outputHelper) : base(outputHelper) @@ -131,9 +134,13 @@ public async Task CanAuthenticate_WithClientMetadataDocument() transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); } - [Fact] - public async Task CannotAuthenticate_WhenMetadataOmitsPkceMethods() + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task CannotAuthenticate_WhenMetadataOmitsPkceMethods(bool? allowLegacyFallback) { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); TestOAuthServer.CodeChallengeMethodsSupported = null; await using var app = await StartMcpServerAsync(); @@ -152,6 +159,10 @@ public async Task CannotAuthenticate_WhenMetadataOmitsPkceMethods() await Assert.ThrowsAsync(() => McpClient.CreateAsync( transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + // No discovery endpoint advertises PKCE, so metadata discovery is exhausted. The precise PKCE reason // is logged as each endpoint is skipped. Assert.Contains( @@ -159,9 +170,13 @@ await Assert.ThrowsAsync(() => McpClient.CreateAsync( m => m.Exception?.Message.Contains("code_challenge_methods_supported") == true); } - [Fact] - public async Task CannotAuthenticate_WhenMetadataLacksS256PkceMethod() + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task CannotAuthenticate_WhenMetadataLacksS256PkceMethod(bool? allowLegacyFallback) { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); TestOAuthServer.CodeChallengeMethodsSupported = ["plain"]; await using var app = await StartMcpServerAsync(); @@ -180,14 +195,22 @@ public async Task CannotAuthenticate_WhenMetadataLacksS256PkceMethod() await Assert.ThrowsAsync(() => McpClient.CreateAsync( transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + Assert.Contains( MockLoggerProvider.LogMessages, m => m.Exception?.Message.Contains("required PKCE method 'S256'") == true); } - [Fact] - public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherAdvertisesIt() + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherAdvertisesIt(bool? allowLegacyFallback) { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); // The OAuth 2.0 authorization server metadata endpoint is tried before the OpenID Connect one. // Simulate a server where only the OpenID Connect document advertises PKCE support, and verify the // client falls through to it rather than failing on the first PKCE-less document. @@ -208,6 +231,43 @@ public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherA await using var client = await McpClient.CreateAsync( transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Contains("/.well-known/oauth-authorization-server", TestOAuthServer.MetadataRequests); + Assert.Contains("/.well-known/openid-configuration", TestOAuthServer.MetadataRequests); + Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(1, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIsUnavailable_WithProtectedResourceMetadata(bool? allowLegacyFallback) + { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); + TestOAuthServer.DisabledMetadataPaths.Add("/.well-known/oauth-authorization-server"); + TestOAuthServer.DisabledMetadataPaths.Add("/.well-known/openid-configuration"); + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); } [Fact] @@ -1239,12 +1299,18 @@ await context.Response.WriteAsync($$""" transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); } - [Fact] - public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() + [Theory] + [InlineData(null)] + [InlineData(false)] + [InlineData(true)] + public async Task LegacyServerUsingDefaultEndpointFallback_RequiresExplicitOptIn(bool? allowLegacyFallback) { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); + Assert.Equal(allowLegacyFallback.HasValue, AppContext.TryGetSwitch(LegacyOAuthMetadataFallbackSwitch, out var enabled)); + Assert.Equal(allowLegacyFallback.GetValueOrDefault(), enabled); + // 2025-03-26 backcompat: server does NOT serve PRM AND does NOT serve auth server metadata. - // The client should fall back to default endpoint paths (/authorize, /token, /register) - // on the MCP server's origin. + // Only explicitly opted-in clients can use default endpoint paths on the MCP server's origin. TestOAuthServer.ExpectResource = false; // Use JwtBearer as the challenge scheme so the 401 response does NOT include resource_metadata. @@ -1258,6 +1324,9 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() await using var app = Builder.Build(); + var oauthRequests = 0; + var authorizationRedirectCalled = false; + // Capture HttpClient for use in the proxy middleware. var httpClient = HttpClient; @@ -1283,6 +1352,7 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() var path = context.Request.Path.Value; if (path is "/authorize" or "/token" or "/register") { + Interlocked.Increment(ref oauthRequests); var targetUrl = $"{OAuthServerUrl}{path}{context.Request.QueryString}"; using var proxyRequest = new HttpRequestMessage(new HttpMethod(context.Request.Method), targetUrl); @@ -1333,19 +1403,50 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() ClientId = "demo-client", ClientSecret = "demo-secret", RedirectUri = new Uri("http://localhost:1179/callback"), - AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + AuthorizationRedirectDelegate = (uri, redirect, ct) => + { + authorizationRedirectCalled = true; + return HandleAuthorizationUrlAsync(uri, redirect, ct); + }, }, }, HttpClient, LoggerFactory); - await using var client = await McpClient.CreateAsync( - transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + if (allowLegacyFallback == true) + { + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.True(authorizationRedirectCalled); + Assert.Equal(2, oauthRequests); + Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(1, TestOAuthServer.TokenRequestCount); + Assert.Contains(MockLoggerProvider.LogMessages, + m => m.LogLevel == LogLevel.Warning && m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + else + { + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("PKCE support could not be verified", ex.Message); + Assert.False(authorizationRedirectCalled); + Assert.Equal(0, oauthRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } } - [Fact] - public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataOmitsPkceMethods() + [Theory] + [InlineData(null, false)] + [InlineData(false, false)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataLacksPkceSupport(bool? allowLegacyFallback, bool advertisePlain) { + using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); // 2025-03-26 backcompat regression guard: PRM is unavailable (resourceUri is null), but the server - // DOES serve an auth server metadata document that omits 'code_challenge_methods_supported'. + // DOES serve an auth server metadata document that omits PKCE support or advertises only plain. // The client must refuse to proceed rather than falling back to synthesized S256 defaults, since a // discovered metadata document that fails PKCE validation disqualifies the legacy default fallback. TestOAuthServer.ExpectResource = false; @@ -1369,7 +1470,7 @@ public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataOmitsPkceMetho return; } - // Serve auth server metadata that omits 'code_challenge_methods_supported' entirely. + // Serve metadata that omits PKCE support or advertises only plain. if (context.Request.Path.StartsWithSegments("/.well-known/oauth-authorization-server") || context.Request.Path.StartsWithSegments("/.well-known/openid-configuration")) { @@ -1377,6 +1478,7 @@ public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataOmitsPkceMetho await context.Response.WriteAsync($$""" { "issuer": "{{OAuthServerUrl}}", + {{(advertisePlain ? "\"code_challenge_methods_supported\": [\"plain\"]," : "")}} "authorization_endpoint": "{{McpServerUrl}}/authorize", "token_endpoint": "{{McpServerUrl}}/token", "registration_endpoint": "{{McpServerUrl}}/register", @@ -1414,5 +1516,21 @@ await context.Response.WriteAsync($$""" // The specific PKCE failure reason is surfaced rather than a generic discovery failure or a // silently-synthesized S256 fallback. Assert.Contains("code_challenge_methods_supported", ex.Message); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + + private sealed class LegacyFallbackSwitchScope : IDisposable + { + private readonly object? _previousValue = AppContext.GetData(LegacyOAuthMetadataFallbackSwitch); + + public LegacyFallbackSwitchScope(bool? enabled) + { + // Switch data can be removed again, unlike a value installed with AppContext.SetSwitch. + AppContext.SetData(LegacyOAuthMetadataFallbackSwitch, enabled?.ToString()); + } + + public void Dispose() => AppContext.SetData(LegacyOAuthMetadataFallbackSwitch, _previousValue); } } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs b/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs new file mode 100644 index 000000000..83a5192d3 --- /dev/null +++ b/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs @@ -0,0 +1,4 @@ +namespace ModelContextProtocol.AspNetCore.Tests; + +[CollectionDefinition(nameof(DisableParallelization), DisableParallelization = true)] +public sealed class DisableParallelization; diff --git a/tests/ModelContextProtocol.ConformanceClient/Program.cs b/tests/ModelContextProtocol.ConformanceClient/Program.cs index b5e048dd0..19d954a18 100644 --- a/tests/ModelContextProtocol.ConformanceClient/Program.cs +++ b/tests/ModelContextProtocol.ConformanceClient/Program.cs @@ -19,6 +19,12 @@ var scenario = args[0]; var endpoint = args[1]; +if (scenario == "auth/2025-03-26-oauth-endpoint-fallback") +{ + // Only the trusted legacy conformance fixture opts into metadata-free endpoint discovery. + AppContext.SetSwitch("ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback", true); +} + McpClientOptions options = new() { ClientInfo = new() diff --git a/tests/ModelContextProtocol.TestOAuthServer/Program.cs b/tests/ModelContextProtocol.TestOAuthServer/Program.cs index 3995b0e0e..0ec6dd335 100644 --- a/tests/ModelContextProtocol.TestOAuthServer/Program.cs +++ b/tests/ModelContextProtocol.TestOAuthServer/Program.cs @@ -28,6 +28,8 @@ public sealed class Program private readonly ConcurrentDictionary _clients = new(); private readonly ConcurrentQueue _metadataRequests = new(); + private int _authorizationRequestCount; + private int _tokenRequestCount; private readonly RSA _rsa; private readonly string _keyId; @@ -107,6 +109,12 @@ public Program(ILoggerProvider? loggerProvider = null, IConnectionListenerFactor public HashSet DisabledMetadataPaths { get; } = new(StringComparer.OrdinalIgnoreCase); public IReadOnlyCollection MetadataRequests => _metadataRequests.ToArray(); + /// Gets the number of requests received by the authorization endpoint. + public int AuthorizationRequestCount => Volatile.Read(ref _authorizationRequestCount); + + /// Gets the number of requests received by the token endpoint. + public int TokenRequestCount => Volatile.Read(ref _tokenRequestCount); + /// /// Entry point for the application. /// @@ -300,6 +308,8 @@ IResult HandleMetadataRequest(HttpContext context, string? issuerPath = null) [FromQuery] string? state, [FromQuery] string? resource) => { + Interlocked.Increment(ref _authorizationRequestCount); + // Validate client if (!_clients.TryGetValue(client_id, out var client)) { @@ -381,6 +391,8 @@ IResult HandleMetadataRequest(HttpContext context, string? issuerPath = null) // Token endpoint app.MapPost("/token", async (HttpContext context) => { + Interlocked.Increment(ref _tokenRequestCount); + var form = await context.Request.ReadFormAsync(); // Authenticate client From 37dbe3fc49389e85f20c59ea3207619fe9eafcf7 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 22 Oct 2026 12:25:21 -0700 Subject: [PATCH 3/6] Backport RFC 8414 OAuth metadata issuer validation Adapt the metadata-only checks and fail-fast McpException handling from #1605 (4aade885583e90eed8192721cc35f7b49411464c), preserving its 2025-03-26 legacy exemption. Add exact-string matching and pre-authorization rejection coverage without introducing response-aware callbacks or RFC 9207 response validation. Update the test-only conformance pin from 0.1.13 to 0.1.15 for corrected tenant issuer fixtures (modelcontextprotocol/conformance#152; 50c3cf1a430274ebdaaddeb95ef2c786a6bca290). Keep the PKCE requirements and fallback switch unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/concepts/transports/transports.md | 4 + package-lock.json | 227 +++++++++++++++++- package.json | 2 +- samples/ProtectedMcpClient/README.md | 2 +- .../Authentication/ClientOAuthOptions.cs | 6 +- .../Authentication/ClientOAuthProvider.cs | 31 +++ .../OAuth/AuthTests.cs | 140 ++++++++++- .../OAuthServerMetadata.cs | 3 +- .../Program.cs | 12 +- 9 files changed, 414 insertions(+), 13 deletions(-) diff --git a/docs/concepts/transports/transports.md b/docs/concepts/transports/transports.md index f000b9df9..e631ebd87 100644 --- a/docs/concepts/transports/transports.md +++ b/docs/concepts/transports/transports.md @@ -149,6 +149,10 @@ var transport = new HttpClientTransport(new HttpClientTransportOptions When using with an HTTP transport, authorization server metadata must advertise `S256` in `code_challenge_methods_supported`. A document that omits that field or advertises only other methods is skipped while the client tries alternative well-known endpoints. If none supplies valid metadata with `S256`, authentication fails before requesting an authorization code or exchanging tokens. This is a compatibility tightening: servers that implement PKCE but do not advertise it must update their metadata. +When protected resource metadata is available, authorization server metadata must also include an `issuer` that exactly matches the selected authorization server identifier, as required by [RFC 8414, Section 3.3](https://www.rfc-editor.org/rfc/rfc8414.html#section-3.3). Comparison is ordinal and uses the original URI strings: differences in case, trailing slashes, or percent-encoding are rejected even when URI normalization would make them equivalent. A missing or mismatched issuer immediately stops discovery before authorization or token exchange; the client does not try another metadata endpoint or use synthetic defaults. For the MCP 2025-03-26 legacy path without protected resource metadata, these issuer checks remain exempt because discovery uses the MCP server's origin, which need not equal the canonical issuer. PKCE requirements still apply on that path. + +These are **metadata issuer** checks, not RFC 9207 authorization-response `iss` validation. The 1.x code-only callback cannot provide a response issuer for SDK validation; that separate response check is not included. PKCE is not a substitute for issuer validation. + The 1.x SDK's authorization callback returns only a code, so the SDK cannot validate response `state`. [RFC 9700, Section 4.7.1](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.7.1) permits relying on PKCE for CSRF protection only when authorization server support has been ensured. Metadata verification checks **advertised support**, not the server's actual implementation. The authorization server must correctly enforce PKCE, including the [downgrade defenses in Section 4.8.2](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.8.2). **Legacy compatibility escape hatch.** By default, the 1.x client also rejects authentication when authorization server metadata cannot be discovered; it no longer assumes `S256` support. For a trusted legacy deployment whose PKCE support you have independently established, you can explicitly restore the MCP 2025-03-26 default endpoint fallback at application startup: diff --git a/package-lock.json b/package-lock.json index 34d53809d..638bb697a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,11 +1,11 @@ { - "name": "csharp-sdk", + "name": "halter73-refactored-garbanzo", "lockfileVersion": 3, "requires": true, "packages": { "": { "dependencies": { - "@modelcontextprotocol/conformance": "0.1.13", + "@modelcontextprotocol/conformance": "0.1.15", "@modelcontextprotocol/server-everything": "2026.1.26", "@modelcontextprotocol/server-memory": "2026.1.26" } @@ -23,12 +23,13 @@ } }, "node_modules/@modelcontextprotocol/conformance": { - "version": "0.1.13", - "resolved": "https://registry.npmjs.org/@modelcontextprotocol/conformance/-/conformance-0.1.13.tgz", - "integrity": "sha512-J9cC7Dsi7hTn7sxorvxDZ+wC8OlHAkOmVxVm1/IBQ8HEpYJu8r0eAUEw68vTmjBPP5WcU8Jld4RZ3cOFp/Ih1g==", + "version": "0.1.15", + "resolved": "https://registry.npmjs.org/@modelcontextprotocol/conformance/-/conformance-0.1.15.tgz", + "integrity": "sha512-B1eNYpv5kas9YFC40su7SWhKHR2DwYzJRpiX6dfWzDWMcDr71myKVj//PwyqUHC7oucs3EJqcqnwvSuUwvenoQ==", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.26.0", + "@octokit/rest": "^22.0.0", "commander": "^14.0.2", "eventsource-parser": "^3.0.6", "express": "^5.1.0", @@ -110,6 +111,204 @@ "mcp-server-memory": "dist/index.js" } }, + "node_modules/@octokit/auth-token": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@octokit/auth-token/-/auth-token-6.0.0.tgz", + "integrity": "sha512-P4YJBPdPSpWTQ1NU4XYdvHvXJJDxM6YwpS0FZHRgP7YFkdVxsWcpWGy/NVqlAA7PcPCnMacXlRm1y2PFZRWL/w==", + "license": "MIT", + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/core": { + "version": "7.0.8", + "resolved": "https://registry.npmjs.org/@octokit/core/-/core-7.0.8.tgz", + "integrity": "sha512-L7y8eYc+AwxGr2PWI4WFt1VG4TiJ66c26BD16mXpYIlXxG0SMigM1+m4aTSlYyBr5BlQsGAlz8uDCoZN4SEMcg==", + "license": "MIT", + "dependencies": { + "@octokit/auth-token": "^6.0.0", + "@octokit/graphql": "^9.0.5", + "@octokit/request": "^10.0.16", + "@octokit/request-error": "^7.1.2", + "@octokit/types": "^18.0.0", + "before-after-hook": "^4.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/endpoint": { + "version": "11.0.5", + "resolved": "https://registry.npmjs.org/@octokit/endpoint/-/endpoint-11.0.5.tgz", + "integrity": "sha512-iXa654H3yFafF/ieHkukfbgWo2rmXD2ceD0ZOtrPhw1bc3FDch1d9N/TNs0FQ1/cIbwb7kspUX8jzIs8nzb9DQ==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^18.0.0", + "universal-user-agent": "^7.0.2" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/graphql": { + "version": "9.0.5", + "resolved": "https://registry.npmjs.org/@octokit/graphql/-/graphql-9.0.5.tgz", + "integrity": "sha512-bt/hm03LeU6Vy7FwTrkkC9p3XGT/lBwClglMqxBSe5/q0E5CdJTXeAqEI0vlw89/LF/G6tryTIH8HirZ3prMVg==", + "license": "MIT", + "dependencies": { + "@octokit/request": "^10.0.16", + "@octokit/types": "^18.0.0", + "universal-user-agent": "^7.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/openapi-types": { + "version": "29.0.1", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-29.0.1.tgz", + "integrity": "sha512-9qWOMFNxxLokERcms42rU0PTLqQmVs7g5E41TI4mCOxmpFayD1rfC7XxOL55cG9MBZLFlC31BrR37myMKardwg==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-paginate-rest": { + "version": "14.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-paginate-rest/-/plugin-paginate-rest-14.0.0.tgz", + "integrity": "sha512-fNVRE7ufJiAA3XUrha2omTA39M6IXIc6GIZLvlbsm8QOQCYvpq/LkMNGyFlB1d8hTDzsAXa3OKtybdMAYsV/fw==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^16.0.0" + }, + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-paginate-rest/node_modules/@octokit/openapi-types": { + "version": "27.0.0", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-27.0.0.tgz", + "integrity": "sha512-whrdktVs1h6gtR+09+QsNk2+FO+49j6ga1c55YZudfEG+oKJVvJLQi3zkOm5JjiUXAagWK2tI2kTGKJ2Ys7MGA==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-paginate-rest/node_modules/@octokit/types": { + "version": "16.0.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-16.0.0.tgz", + "integrity": "sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^27.0.0" + } + }, + "node_modules/@octokit/plugin-request-log": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-request-log/-/plugin-request-log-6.0.0.tgz", + "integrity": "sha512-UkOzeEN3W91/eBq9sPZNQ7sUBvYCqYbrrD8gTbBuGtHEuycE4/awMXcYvx6sVYo7LypPhmQwwpUe4Yyu4QZN5Q==", + "license": "MIT", + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-rest-endpoint-methods": { + "version": "17.0.0", + "resolved": "https://registry.npmjs.org/@octokit/plugin-rest-endpoint-methods/-/plugin-rest-endpoint-methods-17.0.0.tgz", + "integrity": "sha512-B5yCyIlOJFPqUUeiD0cnBJwWJO8lkJs5d8+ze9QDP6SvfiXSz1BF+91+0MeI1d2yxgOhU/O+CvtiZ9jSkHhFAw==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^16.0.0" + }, + "engines": { + "node": ">= 20" + }, + "peerDependencies": { + "@octokit/core": ">=6" + } + }, + "node_modules/@octokit/plugin-rest-endpoint-methods/node_modules/@octokit/openapi-types": { + "version": "27.0.0", + "resolved": "https://registry.npmjs.org/@octokit/openapi-types/-/openapi-types-27.0.0.tgz", + "integrity": "sha512-whrdktVs1h6gtR+09+QsNk2+FO+49j6ga1c55YZudfEG+oKJVvJLQi3zkOm5JjiUXAagWK2tI2kTGKJ2Ys7MGA==", + "license": "MIT" + }, + "node_modules/@octokit/plugin-rest-endpoint-methods/node_modules/@octokit/types": { + "version": "16.0.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-16.0.0.tgz", + "integrity": "sha512-sKq+9r1Mm4efXW1FCk7hFSeJo4QKreL/tTbR0rz/qx/r1Oa2VV83LTA/H/MuCOX7uCIJmQVRKBcbmWoySjAnSg==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^27.0.0" + } + }, + "node_modules/@octokit/request": { + "version": "10.0.16", + "resolved": "https://registry.npmjs.org/@octokit/request/-/request-10.0.16.tgz", + "integrity": "sha512-A0zWGjHzISIb+9ccG8s0dq7LKO5zVpJLRICjgUb+sJxEWqn8RUHB1rD3AE51+PECvXHIxqZ1VVvs4fHTSD9nUQ==", + "license": "MIT", + "dependencies": { + "@octokit/endpoint": "^11.0.5", + "@octokit/request-error": "^7.1.2", + "@octokit/types": "^18.0.0", + "content-type": "^3.0.0", + "json-with-bigint": "^3.5.12", + "universal-user-agent": "^7.0.2" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/request-error": { + "version": "7.1.2", + "resolved": "https://registry.npmjs.org/@octokit/request-error/-/request-error-7.1.2.tgz", + "integrity": "sha512-XZRuT3xZ84D3gYErI1DZvhJ33dCWVV6uzBtWkaBB4TvA/L6eOeTZodxLFVB44bBEEo3vEx7y00UfX1tBLrtLRg==", + "license": "MIT", + "dependencies": { + "@octokit/types": "^18.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/request/node_modules/content-type": { + "version": "3.1.1", + "resolved": "https://registry.npmjs.org/content-type/-/content-type-3.1.1.tgz", + "integrity": "sha512-GW4qUsfFo59d0HbUibDlWv5wPz+vAAcaTWbKIuKCf0JkC7wWkSyf8f13IpXn5JkeMlB2P8iTSSIjwXljorg2vA==", + "license": "MIT", + "engines": { + "node": ">=22" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/express" + } + }, + "node_modules/@octokit/rest": { + "version": "22.0.1", + "resolved": "https://registry.npmjs.org/@octokit/rest/-/rest-22.0.1.tgz", + "integrity": "sha512-Jzbhzl3CEexhnivb1iQ0KJ7s5vvjMWcmRtq5aUsKmKDrRW6z3r84ngmiFKFvpZjpiU/9/S6ITPFRpn5s/3uQJw==", + "license": "MIT", + "dependencies": { + "@octokit/core": "^7.0.6", + "@octokit/plugin-paginate-rest": "^14.0.0", + "@octokit/plugin-request-log": "^6.0.0", + "@octokit/plugin-rest-endpoint-methods": "^17.0.0" + }, + "engines": { + "node": ">= 20" + } + }, + "node_modules/@octokit/types": { + "version": "18.0.0", + "resolved": "https://registry.npmjs.org/@octokit/types/-/types-18.0.0.tgz", + "integrity": "sha512-l6bAF43PNxkJp6g+W4PjoUSSkxHomXw2nOum5CTftJz1NlV3vu93NImgOYtLf6CbBUb5j+fiuzW0PPQ5JTSvZA==", + "license": "MIT", + "dependencies": { + "@octokit/openapi-types": "^29.0.1" + } + }, "node_modules/accepts": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/accepts/-/accepts-2.0.0.tgz", @@ -156,6 +355,12 @@ } } }, + "node_modules/before-after-hook": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-4.0.0.tgz", + "integrity": "sha512-q6tR3RPqIB1pMiTRMFcZwuG5T8vwp+vUvEG0vuI6B+Rikh5BfPp2fQ82c925FOs+b0lcFQ8CFrL+KbilfZFhOQ==", + "license": "Apache-2.0" + }, "node_modules/body-parser": { "version": "2.2.2", "resolved": "https://registry.npmjs.org/body-parser/-/body-parser-2.2.2.tgz", @@ -743,6 +948,12 @@ "integrity": "sha512-fQhoXdcvc3V28x7C7BMs4P5+kNlgUURe2jmUT1T//oBRMDrqy1QPelJimwZGo7Hg9VPV3EQV5Bnq4hbFy2vetA==", "license": "BSD-2-Clause" }, + "node_modules/json-with-bigint": { + "version": "3.5.12", + "resolved": "https://registry.npmjs.org/json-with-bigint/-/json-with-bigint-3.5.12.tgz", + "integrity": "sha512-uwbF/wSSuOgC7qqlq27Xp5B6a2MHVug3t0idZdTqu0JnlFvgJuH7ju+KAk/J06C7GfhoYy2gnb9wz2INqcne7w==", + "license": "MIT" + }, "node_modules/jszip": { "version": "3.10.1", "resolved": "https://registry.npmjs.org/jszip/-/jszip-3.10.1.tgz", @@ -1229,6 +1440,12 @@ "node": ">=20.18.1" } }, + "node_modules/universal-user-agent": { + "version": "7.0.3", + "resolved": "https://registry.npmjs.org/universal-user-agent/-/universal-user-agent-7.0.3.tgz", + "integrity": "sha512-TmnEAEAsBJVZM/AADELsK76llnwcf9vMKuPz8JflO1frO8Lchitr0fNaN9d+Ap0BjKtqWqd/J17qeDnXh8CL2A==", + "license": "ISC" + }, "node_modules/unpipe": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/unpipe/-/unpipe-1.0.0.tgz", diff --git a/package.json b/package.json index b05f143aa..a324ac45a 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "description": "Pinned npm dependencies for MCP C# SDK integration and conformance tests", "dependencies": { - "@modelcontextprotocol/conformance": "0.1.13", + "@modelcontextprotocol/conformance": "0.1.15", "@modelcontextprotocol/server-everything": "2026.1.26", "@modelcontextprotocol/server-memory": "2026.1.26" } diff --git a/samples/ProtectedMcpClient/README.md b/samples/ProtectedMcpClient/README.md index 194b4f57a..7930f244c 100644 --- a/samples/ProtectedMcpClient/README.md +++ b/samples/ProtectedMcpClient/README.md @@ -69,7 +69,7 @@ The client is configured with: - **OAuth Server**: `https://localhost:7029` - **Protected Resource**: `http://localhost:7071` -The authorization server must advertise `S256` PKCE support in its metadata. The test server does this by default. The 1.x SDK rejects missing PKCE advertisement and, by default, unavailable authorization server metadata; see [OAuth PKCE metadata validation](../../docs/concepts/transports/transports.md#oauth-pkce-metadata-validation-1x) for the narrowly scoped, process-wide legacy compatibility switch and its security requirements. +The authorization server must advertise `S256` PKCE support in its metadata. When protected resource metadata is available, its metadata `issuer` must also exactly match the selected authorization server identifier. The test server satisfies both requirements by default. The 1.x SDK rejects missing PKCE advertisement and, by default, unavailable authorization server metadata; see [OAuth PKCE metadata validation](../../docs/concepts/transports/transports.md#oauth-pkce-metadata-validation-1x) for the issuer matching rules, narrowly scoped process-wide legacy compatibility switch, and security requirements. ## Available Tools diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs index c0037f4bd..3134d9668 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs @@ -5,7 +5,11 @@ namespace ModelContextProtocol.Authentication; /// /// /// Authorization server metadata must advertise the S256 PKCE method. If metadata discovery fails, -/// authorization fails before requesting an authorization code or exchanging tokens. For trusted legacy +/// authorization fails before requesting an authorization code or exchanging tokens. When protected resource +/// metadata is available, the authorization server metadata issuer must exactly match the selected authorization +/// server identifier, without normalizing URI case, trailing slashes, or percent-encoding. Missing or mismatched +/// issuers immediately stop discovery. These issuer checks are skipped for the 2025-03-26 legacy path without +/// protected resource metadata, where discovery uses the server origin rather than the canonical issuer. For trusted legacy /// deployments without protected resource metadata and without usable authorization server metadata, the process-wide /// ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback switch /// can explicitly enable the 2025-03-26 default endpoint fallback. It is disabled by default and does not diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 33d72d6af..e0ca6775e 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -370,10 +370,41 @@ private async Task GetAuthServerMetadataAsync(Uri a ThrowFailedToHandleUnauthorizedResponse($"AuthorizationEndpoint must use HTTP or HTTPS. '{metadata.AuthorizationEndpoint}' does not meet this requirement."); } + // Validate the issuer in the metadata document per RFC 8414 Section 3.3: + // the issuer value MUST be identical to the issuer identifier used to construct + // the well-known URL. + // Skip validation in legacy backcompat mode (resourceUri is null) because the + // authServerUri was derived from the server origin rather than from Protected + // Resource Metadata, so it may not match the server's canonical issuer. + // Note: resourceUri is null exclusively in the 2025-03-26 legacy path. For newer + // protocol versions, ExtractProtectedResourceMetadata throws if the PRM document + // omits the resource field (VerifyResourceMatch returns false for null Resource), + // so we never reach this point with resourceUri == null in non-legacy flows. + if (resourceUri is not null && metadata.Issuer is null) + { + ThrowFailedToHandleUnauthorizedResponse( + $"Authorization server metadata from '{wellKnownEndpoint}' did not provide the required issuer (RFC 8414 Section 2)."); + } + + // RFC 8414 requires an identical issuer value, so do not normalize URI case, + // trailing slashes, or percent-encoding before comparison. + if (resourceUri is not null && + !string.Equals(metadata.Issuer!.OriginalString, authServerUri.OriginalString, StringComparison.Ordinal)) + { + ThrowFailedToHandleUnauthorizedResponse( + $"Authorization server metadata issuer '{metadata.Issuer}' does not match the expected issuer '{authServerUri}' (RFC 8414 Section 3.3)."); + } + // A structurally valid metadata document was discovered. Even if it fails PKCE validation // below, its existence disqualifies the legacy fallback that would otherwise synthesize S256. metadataDocumentFound = true; } + catch (McpException) + { + // Metadata validation failures are security signals and must not fall back to + // another well-known endpoint. + throw; + } catch (Exception ex) { LogErrorFetchingAuthServerMetadata(ex, wellKnownEndpoint); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index 338aeab5c..25ef4d25c 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -831,6 +831,135 @@ public async Task CanAuthenticate_WithAuthorizationServerPathFallbacks() TestOAuthServer.MetadataRequests); } + [Theory] + [InlineData(OAuthServerUrl)] + [InlineData(OAuthServerUrl + "/tenant")] + public async Task CanAuthenticate_WhenAuthorizationServerMetadataIssuerMatches(string expectedIssuer) + { + Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => + { + options.ResourceMetadata!.AuthorizationServers = [expectedIssuer]; + }); + + await using var app = await StartMcpServerAsync(); + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(1, TestOAuthServer.TokenRequestCount); + } + + [Theory] + [InlineData(OAuthServerUrl, "https://attacker.example")] + [InlineData(OAuthServerUrl, OAuthServerUrl + "/")] + [InlineData(OAuthServerUrl, "HTTPS://LOCALHOST:7029")] + [InlineData(OAuthServerUrl + "/tenant", OAuthServerUrl + "/%74enant")] + public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIssuerMismatches(string expectedIssuer, string metadataIssuer) + { + using var switchScope = new LegacyFallbackSwitchScope(true); + TestOAuthServer.MetadataIssuerOverride = metadataIssuer; + Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => + { + options.ResourceMetadata!.AuthorizationServers = [expectedIssuer]; + }); + + await using var app = await StartMcpServerAsync(); + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("does not match the expected issuer", ex.Message); + Assert.Single(TestOAuthServer.MetadataRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + + [Fact] + public async Task CannotAuthenticate_WhenAuthorizationServerMetadataOmitsIssuer() + { + using var switchScope = new LegacyFallbackSwitchScope(true); + TestOAuthServer.IncludeIssuerInMetadata = false; + + await using var app = await StartMcpServerAsync(); + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("did not provide the required issuer", ex.Message); + Assert.Single(TestOAuthServer.MetadataRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + } + + [Fact] + public async Task AuthorizationRedirectDelegate_DoesNotSkipMetadataIssuerValidation() + { + TestOAuthServer.MetadataIssuerOverride = "https://attacker.example"; + var authorizationRedirectCalled = false; + await using var app = await StartMcpServerAsync(); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = (uri, redirect, ct) => + { + authorizationRedirectCalled = true; + return HandleAuthorizationUrlAsync(uri, redirect, ct); + }, + }, + }, HttpClient, LoggerFactory); + + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("does not match the expected issuer", ex.Message); + Assert.False(authorizationRedirectCalled); + Assert.Single(TestOAuthServer.MetadataRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + } + [Fact] public async Task CanAuthenticate_WithResourceMetadataPathFallbacks() { @@ -1184,8 +1313,10 @@ public async Task ResourceMetadata_PreservesExplicitTrailingSlash() transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); } - [Fact] - public async Task CanAuthenticate_WithLegacyServerWithoutProtectedResourceMetadata() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task CanAuthenticate_WithLegacyServerWithoutProtectedResourceMetadata(bool includeIssuer) { // 2025-03-26 backcompat: server does NOT serve PRM, but DOES serve auth server metadata. // The client should fall back to using the MCP server's origin as the auth server @@ -1223,7 +1354,7 @@ public async Task CanAuthenticate_WithLegacyServerWithoutProtectedResourceMetada context.Response.ContentType = "application/json"; await context.Response.WriteAsync($$""" { - "issuer": "{{OAuthServerUrl}}", + {{(includeIssuer ? $"\"issuer\": \"{OAuthServerUrl}\"," : "")}} "authorization_endpoint": "{{McpServerUrl}}/authorize", "token_endpoint": "{{McpServerUrl}}/token", "registration_endpoint": "{{McpServerUrl}}/register", @@ -1297,6 +1428,9 @@ await context.Response.WriteAsync($$""" await using var client = await McpClient.CreateAsync( transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(1, TestOAuthServer.TokenRequestCount); } [Theory] diff --git a/tests/ModelContextProtocol.TestOAuthServer/OAuthServerMetadata.cs b/tests/ModelContextProtocol.TestOAuthServer/OAuthServerMetadata.cs index c05a45ba2..c1f834fb7 100644 --- a/tests/ModelContextProtocol.TestOAuthServer/OAuthServerMetadata.cs +++ b/tests/ModelContextProtocol.TestOAuthServer/OAuthServerMetadata.cs @@ -12,7 +12,8 @@ internal sealed class OAuthServerMetadata /// REQUIRED. The authorization server's issuer identifier, which is a URL that uses the "https" scheme and has no query or fragment components. /// [JsonPropertyName("issuer")] - public required string Issuer { get; init; } + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string? Issuer { get; init; } /// /// Gets or sets the authorization endpoint URL. diff --git a/tests/ModelContextProtocol.TestOAuthServer/Program.cs b/tests/ModelContextProtocol.TestOAuthServer/Program.cs index 0ec6dd335..3ba6e9e92 100644 --- a/tests/ModelContextProtocol.TestOAuthServer/Program.cs +++ b/tests/ModelContextProtocol.TestOAuthServer/Program.cs @@ -92,6 +92,16 @@ public Program(ILoggerProvider? loggerProvider = null, IConnectionListenerFactor /// public bool ExpectResource { get; set; } = true; + /// + /// Gets or sets a value indicating whether authorization server metadata includes an issuer. + /// + public bool IncludeIssuerInMetadata { get; set; } = true; + + /// + /// Gets or sets an issuer value that overrides the authorization server's metadata issuer. + /// + public string? MetadataIssuerOverride { get; set; } + /// /// Gets or sets the code challenge methods advertised by metadata endpoints. /// @@ -234,7 +244,7 @@ IResult HandleMetadataRequest(HttpContext context, string? issuerPath = null) var metadata = new OAuthServerMetadata { - Issuer = $"{_url}{issuerPath}", + Issuer = IncludeIssuerInMetadata ? MetadataIssuerOverride ?? $"{_url}{issuerPath}" : null, AuthorizationEndpoint = $"{_url}/authorize", TokenEndpoint = $"{_url}/token", JwksUri = $"{_url}/.well-known/jwks.json", From 1f6bc470fb11d7e575583b47a7b73cdf2f4af9a9 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 22 Oct 2026 12:52:42 -0700 Subject: [PATCH 4/6] Use Node 22 for the conformance test runtime The pinned conformance 0.1.15 bundle imports fs.globSync, which is unavailable in Node 20. Align the build/test workflow with the Node 22 runtime on main without changing the setup-node action, dependency pins, SDK behavior, or test expectations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/ci-build-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-build-test.yml b/.github/workflows/ci-build-test.yml index dc788d08e..faab32ff8 100644 --- a/.github/workflows/ci-build-test.yml +++ b/.github/workflows/ci-build-test.yml @@ -52,7 +52,7 @@ jobs: - name: 🔧 Set up Node.js uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: - node-version: '20' + node-version: '22' - name: 📦 Install pinned npm dependencies for tests run: npm ci From 7905336f1a9e3dfc40c48d60a1a8fc72dd31c9c4 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Sat, 24 Oct 2026 14:50:54 -0700 Subject: [PATCH 5/6] Restore the original npm lockfile name Undo the incidental worktree-directory name captured by npm during the conformance dependency update. Preserve every dependency entry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- package-lock.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index 638bb697a..8926ee88e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "halter73-refactored-garbanzo", + "name": "csharp-sdk", "lockfileVersion": 3, "requires": true, "packages": { From 279b264ea31c404fa9a9ef0b69f69dae03dcb2e5 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Sat, 24 Oct 2026 16:26:44 -0700 Subject: [PATCH 6/6] Keep OAuth tests parallel without testhost switch mutations Use ordinary default-state tests instead of in-process AppContext permutations and remove the class-wide nonparallel collection and helper. Retain secure-default PKCE/issuer checks and no-authorization assertions, with legacy fallback opt-in covered by the existing conformance subprocess. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../OAuth/AuthTests.cs | 115 +++++------------- .../TestAttributes.cs | 4 - 2 files changed, 28 insertions(+), 91 deletions(-) delete mode 100644 tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index 25ef4d25c..db89df18e 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -5,7 +5,6 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; using ModelContextProtocol; using ModelContextProtocol.AspNetCore.Authentication; using ModelContextProtocol.Authentication; @@ -20,11 +19,9 @@ namespace ModelContextProtocol.AspNetCore.Tests.OAuth; -[Collection(nameof(DisableParallelization))] public class AuthTests : OAuthTestBase { private const string ClientMetadataDocumentUrl = $"{OAuthServerUrl}/client-metadata/cimd-client.json"; - private const string LegacyOAuthMetadataFallbackSwitch = "ModelContextProtocol.Authentication.AllowLegacyOAuthMetadataFallback"; public AuthTests(ITestOutputHelper outputHelper) : base(outputHelper) @@ -134,13 +131,9 @@ public async Task CanAuthenticate_WithClientMetadataDocument() transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); } - [Theory] - [InlineData(null)] - [InlineData(false)] - [InlineData(true)] - public async Task CannotAuthenticate_WhenMetadataOmitsPkceMethods(bool? allowLegacyFallback) + [Fact] + public async Task CannotAuthenticate_WhenMetadataOmitsPkceMethods() { - using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); TestOAuthServer.CodeChallengeMethodsSupported = null; await using var app = await StartMcpServerAsync(); @@ -161,7 +154,7 @@ await Assert.ThrowsAsync(() => McpClient.CreateAsync( Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); Assert.Equal(0, TestOAuthServer.TokenRequestCount); - Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); // No discovery endpoint advertises PKCE, so metadata discovery is exhausted. The precise PKCE reason // is logged as each endpoint is skipped. @@ -170,13 +163,9 @@ await Assert.ThrowsAsync(() => McpClient.CreateAsync( m => m.Exception?.Message.Contains("code_challenge_methods_supported") == true); } - [Theory] - [InlineData(null)] - [InlineData(false)] - [InlineData(true)] - public async Task CannotAuthenticate_WhenMetadataLacksS256PkceMethod(bool? allowLegacyFallback) + [Fact] + public async Task CannotAuthenticate_WhenMetadataLacksS256PkceMethod() { - using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); TestOAuthServer.CodeChallengeMethodsSupported = ["plain"]; await using var app = await StartMcpServerAsync(); @@ -197,20 +186,16 @@ await Assert.ThrowsAsync(() => McpClient.CreateAsync( Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); Assert.Equal(0, TestOAuthServer.TokenRequestCount); - Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); Assert.Contains( MockLoggerProvider.LogMessages, m => m.Exception?.Message.Contains("required PKCE method 'S256'") == true); } - [Theory] - [InlineData(null)] - [InlineData(false)] - [InlineData(true)] - public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherAdvertisesIt(bool? allowLegacyFallback) + [Fact] + public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherAdvertisesIt() { - using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); // The OAuth 2.0 authorization server metadata endpoint is tried before the OpenID Connect one. // Simulate a server where only the OpenID Connect document advertises PKCE support, and verify the // client falls through to it rather than failing on the first PKCE-less document. @@ -236,16 +221,12 @@ public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherA Assert.Contains("/.well-known/openid-configuration", TestOAuthServer.MetadataRequests); Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); Assert.Equal(1, TestOAuthServer.TokenRequestCount); - Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); } - [Theory] - [InlineData(null)] - [InlineData(false)] - [InlineData(true)] - public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIsUnavailable_WithProtectedResourceMetadata(bool? allowLegacyFallback) + [Fact] + public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIsUnavailable_WithProtectedResourceMetadata() { - using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); TestOAuthServer.DisabledMetadataPaths.Add("/.well-known/oauth-authorization-server"); TestOAuthServer.DisabledMetadataPaths.Add("/.well-known/openid-configuration"); await using var app = await StartMcpServerAsync(); @@ -267,7 +248,7 @@ await Assert.ThrowsAsync(() => McpClient.CreateAsync( Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); Assert.Equal(0, TestOAuthServer.TokenRequestCount); - Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); } [Fact] @@ -868,7 +849,6 @@ public async Task CanAuthenticate_WhenAuthorizationServerMetadataIssuerMatches(s [InlineData(OAuthServerUrl + "/tenant", OAuthServerUrl + "/%74enant")] public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIssuerMismatches(string expectedIssuer, string metadataIssuer) { - using var switchScope = new LegacyFallbackSwitchScope(true); TestOAuthServer.MetadataIssuerOverride = metadataIssuer; Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => { @@ -895,13 +875,12 @@ public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIssuerMismat Assert.Single(TestOAuthServer.MetadataRequests); Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); Assert.Equal(0, TestOAuthServer.TokenRequestCount); - Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); } [Fact] public async Task CannotAuthenticate_WhenAuthorizationServerMetadataOmitsIssuer() { - using var switchScope = new LegacyFallbackSwitchScope(true); TestOAuthServer.IncludeIssuerInMetadata = false; await using var app = await StartMcpServerAsync(); @@ -924,7 +903,7 @@ public async Task CannotAuthenticate_WhenAuthorizationServerMetadataOmitsIssuer( Assert.Single(TestOAuthServer.MetadataRequests); Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); Assert.Equal(0, TestOAuthServer.TokenRequestCount); - Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); } [Fact] @@ -1433,16 +1412,9 @@ await context.Response.WriteAsync($$""" Assert.Equal(1, TestOAuthServer.TokenRequestCount); } - [Theory] - [InlineData(null)] - [InlineData(false)] - [InlineData(true)] - public async Task LegacyServerUsingDefaultEndpointFallback_RequiresExplicitOptIn(bool? allowLegacyFallback) + [Fact] + public async Task CannotAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() { - using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); - Assert.Equal(allowLegacyFallback.HasValue, AppContext.TryGetSwitch(LegacyOAuthMetadataFallbackSwitch, out var enabled)); - Assert.Equal(allowLegacyFallback.GetValueOrDefault(), enabled); - // 2025-03-26 backcompat: server does NOT serve PRM AND does NOT serve auth server metadata. // Only explicitly opted-in clients can use default endpoint paths on the MCP server's origin. TestOAuthServer.ExpectResource = false; @@ -1545,40 +1517,22 @@ public async Task LegacyServerUsingDefaultEndpointFallback_RequiresExplicitOptIn }, }, HttpClient, LoggerFactory); - if (allowLegacyFallback == true) - { - await using var client = await McpClient.CreateAsync( - transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); - Assert.True(authorizationRedirectCalled); - Assert.Equal(2, oauthRequests); - Assert.Equal(1, TestOAuthServer.AuthorizationRequestCount); - Assert.Equal(1, TestOAuthServer.TokenRequestCount); - Assert.Contains(MockLoggerProvider.LogMessages, - m => m.LogLevel == LogLevel.Warning && m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); - } - else - { - var ex = await Assert.ThrowsAsync(() => McpClient.CreateAsync( - transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); - - Assert.Contains("PKCE support could not be verified", ex.Message); - Assert.False(authorizationRedirectCalled); - Assert.Equal(0, oauthRequests); - Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); - Assert.Equal(0, TestOAuthServer.TokenRequestCount); - Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); - } + Assert.Contains("PKCE support could not be verified", ex.Message); + Assert.False(authorizationRedirectCalled); + Assert.Equal(0, oauthRequests); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); } [Theory] - [InlineData(null, false)] - [InlineData(false, false)] - [InlineData(true, false)] - [InlineData(true, true)] - public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataLacksPkceSupport(bool? allowLegacyFallback, bool advertisePlain) + [InlineData(false)] + [InlineData(true)] + public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataLacksPkceSupport(bool advertisePlain) { - using var switchScope = new LegacyFallbackSwitchScope(allowLegacyFallback); // 2025-03-26 backcompat regression guard: PRM is unavailable (resourceUri is null), but the server // DOES serve an auth server metadata document that omits PKCE support or advertises only plain. // The client must refuse to proceed rather than falling back to synthesized S256 defaults, since a @@ -1652,19 +1606,6 @@ await context.Response.WriteAsync($$""" Assert.Contains("code_challenge_methods_supported", ex.Message); Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); Assert.Equal(0, TestOAuthServer.TokenRequestCount); - Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains(LegacyOAuthMetadataFallbackSwitch)); - } - - private sealed class LegacyFallbackSwitchScope : IDisposable - { - private readonly object? _previousValue = AppContext.GetData(LegacyOAuthMetadataFallbackSwitch); - - public LegacyFallbackSwitchScope(bool? enabled) - { - // Switch data can be removed again, unlike a value installed with AppContext.SetSwitch. - AppContext.SetData(LegacyOAuthMetadataFallbackSwitch, enabled?.ToString()); - } - - public void Dispose() => AppContext.SetData(LegacyOAuthMetadataFallbackSwitch, _previousValue); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); } } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs b/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs deleted file mode 100644 index 83a5192d3..000000000 --- a/tests/ModelContextProtocol.AspNetCore.Tests/TestAttributes.cs +++ /dev/null @@ -1,4 +0,0 @@ -namespace ModelContextProtocol.AspNetCore.Tests; - -[CollectionDefinition(nameof(DisableParallelization), DisableParallelization = true)] -public sealed class DisableParallelization;