diff --git a/docs/concepts/transports/transports.md b/docs/concepts/transports/transports.md index a62ba15d5..dc59f7c81 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 + +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. + +**Compatibility tightening: metadata-free legacy servers.** By default, the 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 or issuer validation of discovered metadata. The client still sends an `S256` challenge and a code verifier, and logs a warning whenever it uses the unverified fallback. + +[RFC 9700, Sections 2.1.1](https://www.rfc-editor.org/rfc/rfc9700.html#section-2.1.1) and [4.7.1](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.7.1) permit deployment-specific assurance of PKCE support, but synthesizing metadata is not assurance. Response `state` validation protects response correlation and CSRF; it does not prevent an intercepted authorization code from being redeemed directly. PKCE remains necessary. + +> [!WARNING] +> Enabling this switch does not verify PKCE support or change state validation. Use it only after independently establishing that your trusted authorization server correctly enforces PKCE, including the [downgrade defenses in RFC 9700, Section 4.8.2](https://www.rfc-editor.org/rfc/rfc9700.html#section-4.8.2). 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. + #### Controlling client timeout timers in tests The short budget excludes time spent acquiring an OAuth token. After authorization, the discovery probe receives a fresh budget. The overall and caller cancellation still apply. diff --git a/samples/ProtectedMcpClient/README.md b/samples/ProtectedMcpClient/README.md index 81ae67cee..af294d19b 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 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) 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 df1c44c1f..9cd109187 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 872d0572b..fb63c1ebf 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -513,7 +513,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; @@ -634,8 +634,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); } @@ -1587,6 +1593,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 2da187d6f..8df96696e 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -383,6 +383,10 @@ public async Task CannotAuthenticate_WhenMetadataOmitsPkceMethods() await Assert.ThrowsAsync(() => TestMcpClient.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("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. Assert.Contains( @@ -411,6 +415,10 @@ public async Task CannotAuthenticate_WhenMetadataLacksS256PkceMethod() await Assert.ThrowsAsync(() => TestMcpClient.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("Using legacy OAuth metadata fallback")); + Assert.Contains( MockLoggerProvider.LogMessages, m => m.Exception?.Message.Contains("required PKCE method 'S256'") == true); @@ -439,6 +447,35 @@ public async Task CanAuthenticate_WhenFirstMetadataEndpointOmitsPkce_ButAnotherA await using var client = await TestMcpClient.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("Using legacy OAuth metadata fallback")); + } + + [Fact] + public async Task CannotAuthenticate_WhenAuthorizationServerMetadataIsUnavailable_WithProtectedResourceMetadata() + { + TestOAuthServer.DisabledMetadataPaths.Add("/.well-known/oauth-authorization-server"); + TestOAuthServer.DisabledMetadataPaths.Add("/.well-known/openid-configuration"); + await using var app = await StartMcpServerAsync(); + var authorizationCallbackCalled = false; + + await using var transport = CreateOAuthTransport((context, cancellationToken) => + { + authorizationCallbackCalled = true; + return HandleAuthorizationUrlAsync(context, cancellationToken); + }); + + await Assert.ThrowsAsync(() => TestMcpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.False(authorizationCallbackCalled); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); } [Fact] @@ -2077,11 +2114,10 @@ await context.Response.WriteAsync($$""" } [Fact] - public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() + public async Task CannotAuthenticate_WithLegacyServerWithoutMetadata_ByDefault() { // 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. + // Default clients must reject authentication before using the conventional OAuth endpoints. TestOAuthServer.ExpectResource = false; // Use JwtBearer as the challenge scheme so the 401 response does NOT include resource_metadata. @@ -2095,8 +2131,8 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() await using var app = Builder.Build(); - // Capture HttpClient for use in the proxy middleware. - var httpClient = HttpClient; + var oauthRequests = 0; + var authorizationCallbackCalled = false; app.Use(async (context, next) => { @@ -2107,7 +2143,7 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() return; } - // Return 404 for auth server metadata to force fallback to default endpoints. + // Return 404 for auth server metadata to exercise the metadata-free legacy path. if (context.Request.Path.StartsWithSegments("/.well-known/oauth-authorization-server") || context.Request.Path.StartsWithSegments("/.well-known/openid-configuration")) { @@ -2115,42 +2151,11 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() return; } - // Proxy default OAuth endpoints to the real OAuth server. - // In a real 2025-03-26 deployment, the MCP server itself would host these endpoints. var path = context.Request.Path.Value; if (path is "/authorize" or "/token" or "/register") { - var targetUrl = $"{OAuthServerUrl}{path}{context.Request.QueryString}"; - using var proxyRequest = new HttpRequestMessage(new HttpMethod(context.Request.Method), targetUrl); - - if (context.Request.ContentLength > 0 || context.Request.ContentType is not null) - { - proxyRequest.Content = new StreamContent(context.Request.Body); - if (context.Request.ContentType is not null) - { - proxyRequest.Content.Headers.ContentType = System.Net.Http.Headers.MediaTypeHeaderValue.Parse(context.Request.ContentType); - } - } - - if (context.Request.Headers.Authorization.Count > 0) - { - proxyRequest.Headers.TryAddWithoutValidation("Authorization", context.Request.Headers.Authorization.ToString()); - } - - using var response = await httpClient.SendAsync(proxyRequest); - context.Response.StatusCode = (int)response.StatusCode; - - if (response.Headers.Location is not null) - { - context.Response.Headers.Location = response.Headers.Location.ToString(); - } - - if (response.Content.Headers.ContentType is not null) - { - context.Response.ContentType = response.Content.Headers.ContentType.ToString(); - } - - await response.Content.CopyToAsync(context.Response.Body); + Interlocked.Increment(ref oauthRequests); + context.Response.StatusCode = StatusCodes.Status500InternalServerError; return; } @@ -2170,19 +2175,32 @@ public async Task CanAuthenticate_WithLegacyServerUsingDefaultEndpointFallback() ClientId = "demo-client", ClientSecret = "demo-secret", RedirectUri = new Uri("http://localhost:1179/callback"), - AuthorizationCallbackHandler = HandleAuthorizationUrlAsync, + AuthorizationCallbackHandler = (context, cancellationToken) => + { + authorizationCallbackCalled = true; + return HandleAuthorizationUrlAsync(context, cancellationToken); + }, }, }, HttpClient, LoggerFactory); - await using var client = await TestMcpClient.CreateAsync( - transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + var ex = await Assert.ThrowsAsync(() => TestMcpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("PKCE support could not be verified", ex.Message); + Assert.False(authorizationCallbackCalled); + 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")); } - [Fact] - public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataOmitsPkceMethods() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task CannotAuthenticate_WithLegacyServerWhoseMetadataLacksPkceSupport(bool advertisePlain) { // 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; @@ -2206,7 +2224,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")) { @@ -2214,6 +2232,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", @@ -2233,17 +2252,12 @@ await context.Response.WriteAsync($$""" app.MapMcp().RequireAuthorization(); await app.StartAsync(TestContext.Current.CancellationToken); - await using var transport = new HttpClientTransport(new() + var authorizationCallbackCalled = false; + await using var transport = CreateOAuthTransport((context, cancellationToken) => { - Endpoint = new(McpServerUrl), - OAuth = new() - { - ClientId = "demo-client", - ClientSecret = "demo-secret", - RedirectUri = new Uri("http://localhost:1179/callback"), - AuthorizationCallbackHandler = HandleAuthorizationUrlAsync, - }, - }, HttpClient, LoggerFactory); + authorizationCallbackCalled = true; + return HandleAuthorizationUrlAsync(context, cancellationToken); + }); var ex = await Assert.ThrowsAsync(() => TestMcpClient.CreateAsync( transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)); @@ -2251,6 +2265,10 @@ 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.False(authorizationCallbackCalled); + Assert.Equal(0, TestOAuthServer.AuthorizationRequestCount); + Assert.Equal(0, TestOAuthServer.TokenRequestCount); + Assert.DoesNotContain(MockLoggerProvider.LogMessages, m => m.Message.Contains("Using legacy OAuth metadata fallback")); } [Fact] diff --git a/tests/ModelContextProtocol.ConformanceClient/Program.cs b/tests/ModelContextProtocol.ConformanceClient/Program.cs index 718c782f9..7dfbd283e 100644 --- a/tests/ModelContextProtocol.ConformanceClient/Program.cs +++ b/tests/ModelContextProtocol.ConformanceClient/Program.cs @@ -20,6 +20,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 73663dc94..1fa369d77 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 int _authorizationCodeTokenRequestCount; private readonly RSA _rsa; @@ -138,6 +140,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); + /// Gets the number of authorization-code token exchange requests received. public int AuthorizationCodeTokenRequestCount => Volatile.Read(ref _authorizationCodeTokenRequestCount); @@ -343,6 +351,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)) { @@ -428,6 +438,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