diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index a20ef9f51..d29dfcf26 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -1368,6 +1368,10 @@ private async Task ExtractProtectedResourceMetadata(H /// The parameter string from the WWW-Authenticate header. /// The name of the parameter to extract. /// The value of the parameter, or null if not found. + /// + /// Follows the auth-param and quoted-string grammar from RFC 9110 section 11.6.1 / section 5.6.4, + /// so commas and escaped quotes (\") inside a quoted value don't get mistaken for parameter boundaries. + /// private static string? ParseWwwAuthenticateParameters(string parameters, string parameterName) { if (parameters.IndexOf(parameterName, StringComparison.OrdinalIgnoreCase) == -1) @@ -1375,33 +1379,103 @@ private async Task ExtractProtectedResourceMetadata(H return null; } - foreach (var part in parameters.Split(',')) + int i = 0; + int length = parameters.Length; + + while (i < length) { - var trimmedPart = part.AsSpan().Trim(); - int equalsIndex = trimmedPart.IndexOf('='); + while (i < length && (parameters[i] == ',' || IsOptionalWhitespace(parameters[i]))) + { + i++; + } - if (equalsIndex <= 0) + if (i >= length) { + break; + } + + int nameStart = i; + while (i < length && parameters[i] != '=' && parameters[i] != ',' && !IsOptionalWhitespace(parameters[i])) + { + i++; + } + + var name = parameters.AsSpan(nameStart, i - nameStart); + + while (i < length && IsOptionalWhitespace(parameters[i])) + { + i++; + } + + if (i >= length || parameters[i] != '=') + { + // Not a well-formed "token = value" auth-param; skip past it to the next one. + while (i < length && parameters[i] != ',') + { + i++; + } continue; } - var key = trimmedPart[..equalsIndex].Trim(); + i++; // Consume '='. - if (key.Equals(parameterName, StringComparison.OrdinalIgnoreCase)) + while (i < length && IsOptionalWhitespace(parameters[i])) { - var value = trimmedPart[(equalsIndex + 1)..].Trim(); - if (value.Length > 0 && value[0] == '"' && value[^1] == '"') + i++; + } + + string value; + if (i < length && parameters[i] == '"') + { + i++; // Consume the opening quote. + var sb = new StringBuilder(); + + while (i < length && parameters[i] != '"') + { + if (parameters[i] == '\\' && i + 1 < length) + { + // quoted-pair: the backslash is dropped and the following octet is literal. + i++; + } + + sb.Append(parameters[i]); + i++; + } + + if (i < length) + { + i++; // Consume the closing quote. + } + + value = sb.ToString(); + } + else + { + int valueStart = i; + while (i < length && parameters[i] != ',') { - value = value[1..^1]; + i++; } - return value.ToString(); + value = parameters.AsSpan(valueStart, i - valueStart).TrimEnd().ToString(); + } + + if (name.Equals(parameterName, StringComparison.OrdinalIgnoreCase)) + { + return value; + } + + while (i < length && parameters[i] != ',') + { + i++; } } return null; } + private static bool IsOptionalWhitespace(char c) => c is ' ' or '\t'; + private static IEnumerable<(Uri WellKnownUri, Uri ExpectedResourceUri)> GetWellKnownResourceMetadataUris(Uri resourceUri) { var builder = new UriBuilder(resourceUri); diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs index 693c77943..ce8f4a909 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs @@ -779,6 +779,114 @@ public async Task AuthorizationFlow_UsesScopeFromChallengeHeader() Assert.Equal(challengeScopes, requestedScope); } + [Fact] + public async Task AuthorizationFlow_UsesScopeFromChallengeHeader_WhenScopeIsAQuotedComma() + { + // Regression test for https://github.com/modelcontextprotocol/csharp-sdk/issues/1088. + // ParseWwwAuthenticateParameters used to split the header on every comma, including ones + // inside a quoted value. When a quoted value was exactly a comma, that produced a bare `"` + // fragment that the old code then tried to unquote, throwing ArgumentOutOfRangeException + // instead of authenticating. + await using var app = Builder.Build(); + app.Use(next => + { + return async context => + { + await next(context); + + if (context.Response.StatusCode != 401) + { + return; + } + + context.Response.Headers.WWWAuthenticate = $"Bearer resource_metadata=\"{McpServerUrl}/.well-known/oauth-protected-resource\", scope=\",\""; + }; + }); + app.UseAuthentication(); + app.UseAuthorization(); + + app.MapMcp().RequireAuthorization(); + await app.StartAsync(TestContext.Current.CancellationToken); + + string? requestedScope = null; + + 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"), + AuthorizationCallbackHandler = (context, ct) => + { + var query = QueryHelpers.ParseQuery(context.AuthorizationUri.Query); + requestedScope = query["scope"].ToString(); + return HandleAuthorizationUrlAsync(context, ct); + }, + }, + }, HttpClient, LoggerFactory); + + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(",", requestedScope); + } + + [Fact] + public async Task AuthorizationFlow_UsesScopeFromChallengeHeader_WithCommaAndEscapedQuoteInValue() + { + // Regression test for https://github.com/modelcontextprotocol/csharp-sdk/issues/1088: a comma + // or an escaped double-quote inside a quoted auth-param value must not be mistaken for the end + // of the value or a new parameter boundary. + const string expectedScope = "read,write\"admin\""; + + await using var app = Builder.Build(); + app.Use(next => + { + return async context => + { + await next(context); + + if (context.Response.StatusCode != 401) + { + return; + } + + context.Response.Headers.WWWAuthenticate = $"Bearer resource_metadata=\"{McpServerUrl}/.well-known/oauth-protected-resource\", scope=\"read,write\\\"admin\\\"\""; + }; + }); + app.UseAuthentication(); + app.UseAuthorization(); + + app.MapMcp().RequireAuthorization(); + await app.StartAsync(TestContext.Current.CancellationToken); + + string? requestedScope = null; + + 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"), + AuthorizationCallbackHandler = (context, ct) => + { + var query = QueryHelpers.ParseQuery(context.AuthorizationUri.Query); + requestedScope = query["scope"].ToString(); + return HandleAuthorizationUrlAsync(context, ct); + }, + }, + }, HttpClient, LoggerFactory); + + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(expectedScope, requestedScope); + } + [Fact] public async Task AuthorizationFlow_UsesScopeFromForbiddenHeader() {