Skip to content

Commit 9c635ff

Browse files
halter73Copilot
andauthored
Validate OAuth authorization state (#1726)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 950ac7c commit 9c635ff

9 files changed

Lines changed: 170 additions & 26 deletions

File tree

docs/list-of-diagnostics.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,4 @@ When APIs are marked as obsolete, a diagnostic is emitted to warn users that the
4545
| `MCP9004` | In place | <xref:ModelContextProtocol.AspNetCore.HttpServerTransportOptions.EnableLegacySse> opts into the legacy SSE transport which has no built-in HTTP-level backpressure. Use Streamable HTTP instead. See [Stateless — Legacy SSE transport](xref:stateless#legacy-sse-transport) for details. |
4646
| `MCP9005` | In place | The Roots, Sampling, and Logging features are deprecated as of specification version 2026-07-28 and may be removed in a future version. See SEP-2577 for more information. |
4747
| `MCP9006` | In place | The stateful Streamable HTTP configuration knobs on <xref:ModelContextProtocol.AspNetCore.HttpServerTransportOptions>`EventStreamStore`, `SessionMigrationHandler`, `PerSessionExecutionContext`, `IdleTimeout`, and `MaxIdleSessionCount` — only apply when `Stateless = false`. Starting with the `2026-07-28` protocol revision, Streamable HTTP no longer supports sessions, and the SDK now defaults `Stateless` to `true`. These knobs remain available for back-compat with the legacy stateful Streamable HTTP transport but new code should target the stateless path. |
48-
| `MCP9007` | In place | `AuthorizationRedirectDelegate` and `ClientOAuthOptions.AuthorizationRedirectDelegate` are retained for source and binary compatibility but cannot provide the RFC 9207 authorization-response issuer. Use `ClientOAuthOptions.AuthorizationCallbackHandler` for issuer-aware authorization flows. |
48+
| `MCP9007` | In place | `AuthorizationRedirectDelegate` and `ClientOAuthOptions.AuthorizationRedirectDelegate` are retained for source and binary compatibility but cannot provide the authorization-response state or RFC 9207 issuer. State and issuer validation are skipped when these APIs are used. Use `ClientOAuthOptions.AuthorizationCallbackHandler` for response-bound, issuer-aware authorization flows. |

samples/ProtectedMcpClient/Program.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@
9898
var context = await listener.GetContextAsync();
9999
var query = HttpUtility.ParseQueryString(context.Request.Url?.Query ?? string.Empty);
100100
var code = query["code"];
101+
var state = query["state"];
101102
var iss = query["iss"];
102103
var error = query["error"];
103104

@@ -121,7 +122,7 @@
121122
}
122123

123124
Console.WriteLine("Authorization code received successfully.");
124-
return new AuthorizationResult { Code = code, Iss = iss };
125+
return new AuthorizationResult { Code = code, State = state, Iss = iss };
125126
}
126127
catch (Exception ex)
127128
{

src/ModelContextProtocol.Core/Authentication/AuthorizationResult.cs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
namespace ModelContextProtocol.Authentication;
22

33
/// <summary>
4-
/// Represents the result of an OAuth authorization redirect, containing the authorization code
5-
/// and optionally the issuer identifier from the authorization response.
4+
/// Represents the result of an OAuth authorization redirect, containing the authorization code,
5+
/// state, and optionally the issuer identifier from the authorization response.
66
/// </summary>
77
/// <remarks>
88
/// <para>
9+
/// The <see cref="State"/> property must be populated from the <c>state</c> query parameter in the
10+
/// redirect URI. The SDK validates it against the value sent in the authorization request to bind
11+
/// the response to the initiating transaction and mitigate cross-site request forgery attacks.
12+
/// </para>
13+
/// <para>
914
/// The <see cref="Iss"/> property should be populated from the <c>iss</c> query parameter in the
1015
/// redirect URI when present, as specified by
1116
/// <see href="https://datatracker.ietf.org/doc/html/rfc9207">RFC 9207</see>.
@@ -20,6 +25,16 @@ public sealed class AuthorizationResult
2025
/// </summary>
2126
public string? Code { get; init; }
2227

28+
/// <summary>
29+
/// Gets the state value returned in the authorization response.
30+
/// </summary>
31+
/// <remarks>
32+
/// Implementations of <see cref="ClientOAuthOptions.AuthorizationCallbackHandler"/> must populate this
33+
/// property from the <c>state</c> query parameter of the redirect URI callback. The SDK requires an
34+
/// exact match with the state sent in the authorization request before exchanging the authorization code.
35+
/// </remarks>
36+
public string? State { get; init; }
37+
2338
/// <summary>
2439
/// Gets the issuer identifier returned in the authorization response per
2540
/// <see href="https://datatracker.ietf.org/doc/html/rfc9207">RFC 9207</see>.

src/ModelContextProtocol.Core/Authentication/ClientOAuthOptions.cs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,10 @@ public sealed class ClientOAuthOptions
8080
/// </para>
8181
/// <para>
8282
/// Custom implementations might open a browser, start an HTTP listener, or use other mechanisms to capture
83-
/// the authorization response. They should return both the <c>code</c> and <c>iss</c> query parameters
84-
/// from the redirect URI callback. This enables the SDK to validate the <c>iss</c> parameter per
83+
/// the authorization response. They must return the <c>code</c> and <c>state</c> query parameters,
84+
/// and should return the <c>iss</c> query parameter when present, from the redirect URI callback.
85+
/// The SDK requires an exact state match before exchanging the code. Returning <c>iss</c> enables
86+
/// the SDK to validate the parameter per
8587
/// <see href="https://datatracker.ietf.org/doc/html/rfc9207">RFC 9207</see>, which mitigates
8688
/// mix-up attacks.
8789
/// </para>
@@ -96,9 +98,10 @@ public sealed class ClientOAuthOptions
9698
/// </summary>
9799
/// <remarks>
98100
/// <para>
99-
/// This delegate returns only the authorization code and cannot provide the <c>iss</c> parameter from
100-
/// the authorization response. Consequently, RFC 9207 issuer validation is skipped when this delegate
101-
/// is used. Use <see cref="AuthorizationCallbackHandler"/> for issuer-aware authorization flows.
101+
/// This delegate returns only the authorization code and cannot provide the <c>state</c> or <c>iss</c>
102+
/// parameter from the authorization response. Consequently, state and RFC 9207 issuer validation are
103+
/// skipped when this delegate is used. Use <see cref="AuthorizationCallbackHandler"/> for response-bound,
104+
/// issuer-aware authorization flows.
102105
/// </para>
103106
/// <para>
104107
/// This property cannot be configured together with <see cref="AuthorizationCallbackHandler"/>.
@@ -139,8 +142,9 @@ public sealed class ClientOAuthOptions
139142
/// </summary>
140143
/// <remarks>
141144
/// <para>
142-
/// Parameters specified cannot override or append to any automatically set parameters like the "redirect_uri",
143-
/// which should instead be configured via <see cref="RedirectUri"/>.
145+
/// Parameters specified cannot override or append to any automatically set parameters like
146+
/// <c>redirect_uri</c> or <c>state</c>. The redirect URI should instead be configured via
147+
/// <see cref="RedirectUri"/>, while state is generated uniquely for each authorization transaction.
144148
/// </para>
145149
/// </remarks>
146150
public IDictionary<string, string> AdditionalAuthorizationParameters { get; set; } = new Dictionary<string, string>();

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
3232
private readonly IDictionary<string, string> _additionalAuthorizationParameters;
3333
private readonly Func<IReadOnlyList<Uri>, Uri?> _authServerSelector;
3434
private readonly Func<AuthorizationCallbackContext, CancellationToken, Task<AuthorizationResult?>> _authorizationCallbackHandler;
35+
private readonly bool _validateAuthorizationResponseState;
3536
private readonly bool _validateAuthorizationResponseIssuer;
3637
private readonly Uri? _clientMetadataDocumentUri;
3738

@@ -120,6 +121,7 @@ public ClientOAuthProvider(
120121
if (options.AuthorizationCallbackHandler is not null)
121122
{
122123
_authorizationCallbackHandler = options.AuthorizationCallbackHandler;
124+
_validateAuthorizationResponseState = true;
123125
_validateAuthorizationResponseIssuer = true;
124126
}
125127
else if (authorizationRedirectDelegate is not null)
@@ -131,11 +133,13 @@ public ClientOAuthProvider(
131133
context.RedirectUri,
132134
cancellationToken).ConfigureAwait(false),
133135
};
136+
_validateAuthorizationResponseState = false;
134137
_validateAuthorizationResponseIssuer = false;
135138
}
136139
else
137140
{
138141
_authorizationCallbackHandler = DefaultAuthorizationUrlHandler;
142+
_validateAuthorizationResponseState = true;
139143
_validateAuthorizationResponseIssuer = true;
140144
}
141145

@@ -155,11 +159,11 @@ public ClientOAuthProvider(
155159
private static Uri? DefaultAuthServerSelector(IReadOnlyList<Uri> availableServers) => availableServers.FirstOrDefault();
156160

157161
/// <summary>
158-
/// Default authorization URL handler that displays the URL to the user for manual input.
162+
/// Default authorization URL handler that displays the URL to the user and parses the resulting redirect URL.
159163
/// </summary>
160164
/// <param name="context">The context containing the authorization and redirect URIs.</param>
161165
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests.</param>
162-
/// <returns>The authorization result entered by the user, or null if none was provided.</returns>
166+
/// <returns>The authorization result parsed from the redirect URL.</returns>
163167
private static Task<AuthorizationResult?> DefaultAuthorizationUrlHandler(
164168
AuthorizationCallbackContext context,
165169
CancellationToken cancellationToken)
@@ -179,6 +183,7 @@ public ClientOAuthProvider(
179183
return Task.FromResult<AuthorizationResult?>(new()
180184
{
181185
Code = queryParams["code"],
186+
State = queryParams["state"],
182187
Iss = queryParams["iss"],
183188
});
184189
}
@@ -687,10 +692,11 @@ private async Task<string> InitiateAuthorizationCodeFlowAsync(
687692
AuthorizationServerMetadata authServerMetadata,
688693
CancellationToken cancellationToken)
689694
{
690-
var codeVerifier = GenerateCodeVerifier();
695+
var codeVerifier = GenerateRandomBase64UrlValue();
691696
var codeChallenge = GenerateCodeChallenge(codeVerifier);
697+
var state = GenerateRandomBase64UrlValue();
692698

693-
var authUrl = BuildAuthorizationUrl(protectedResourceMetadata, authServerMetadata, codeChallenge);
699+
var authUrl = BuildAuthorizationUrl(protectedResourceMetadata, authServerMetadata, codeChallenge, state);
694700

695701
var authResult = await _authorizationCallbackHandler(
696702
new AuthorizationCallbackContext
@@ -700,9 +706,19 @@ private async Task<string> InitiateAuthorizationCodeFlowAsync(
700706
},
701707
cancellationToken).ConfigureAwait(false);
702708

703-
if (authResult is null || string.IsNullOrEmpty(authResult.Code))
709+
if (authResult is null)
704710
{
705-
ThrowFailedToHandleUnauthorizedResponse($"The {nameof(ClientOAuthOptions.AuthorizationCallbackHandler)} returned a null or empty authorization code.");
711+
ThrowFailedToHandleUnauthorizedResponse($"The {nameof(ClientOAuthOptions.AuthorizationCallbackHandler)} returned a null authorization result.");
712+
}
713+
714+
if (_validateAuthorizationResponseState)
715+
{
716+
ValidateStateResponse(authResult!.State, state);
717+
}
718+
719+
if (string.IsNullOrEmpty(authResult.Code))
720+
{
721+
ThrowFailedToHandleUnauthorizedResponse("The authorization callback returned a null or empty authorization code.");
706722
}
707723

708724
if (_validateAuthorizationResponseIssuer)
@@ -721,7 +737,8 @@ private async Task<string> InitiateAuthorizationCodeFlowAsync(
721737
private Uri BuildAuthorizationUrl(
722738
ProtectedResourceMetadata protectedResourceMetadata,
723739
AuthorizationServerMetadata authServerMetadata,
724-
string codeChallenge)
740+
string codeChallenge,
741+
string state)
725742
{
726743
var resourceUri = GetResourceUri(protectedResourceMetadata);
727744

@@ -732,6 +749,7 @@ private Uri BuildAuthorizationUrl(
732749
["response_type"] = "code",
733750
["code_challenge"] = codeChallenge,
734751
["code_challenge_method"] = "S256",
752+
["state"] = state,
735753
};
736754

737755
if (resourceUri is not null)
@@ -1107,6 +1125,26 @@ private bool ChallengeIntroducesNewScopes(ProtectedResourceMetadata protectedRes
11071125
return scope + " " + OfflineAccess;
11081126
}
11091127

1128+
/// <summary>
1129+
/// Validates that an authorization response is bound to the transaction that initiated it.
1130+
/// </summary>
1131+
/// <param name="state">The state returned in the authorization response.</param>
1132+
/// <param name="expectedState">The state sent in the authorization request.</param>
1133+
private static void ValidateStateResponse(string? state, string expectedState)
1134+
{
1135+
if (string.IsNullOrEmpty(state))
1136+
{
1137+
ThrowFailedToHandleUnauthorizedResponse(
1138+
"The authorization response did not include the required state parameter.");
1139+
}
1140+
1141+
if (!string.Equals(state, expectedState, StringComparison.Ordinal))
1142+
{
1143+
ThrowFailedToHandleUnauthorizedResponse(
1144+
"The authorization response state did not match the state sent in the authorization request.");
1145+
}
1146+
}
1147+
11101148
/// <summary>
11111149
/// Validates the <c>iss</c> parameter from an authorization response per
11121150
/// <see href="https://datatracker.ietf.org/doc/html/rfc9207">RFC 9207</see>.
@@ -1378,7 +1416,7 @@ private async Task<ProtectedResourceMetadata> ExtractProtectedResourceMetadata(H
13781416
yield return (new Uri($"{hostBase}{ProtectedResourceMetadataWellKnownPath}"), new Uri(hostBase));
13791417
}
13801418

1381-
private static string GenerateCodeVerifier()
1419+
private static string GenerateRandomBase64UrlValue()
13821420
{
13831421
#if NET9_0_OR_GREATER
13841422
Span<byte> bytes = stackalloc byte[32];

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

Lines changed: 80 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,77 @@ public void HttpClientTransport_RejectsBothAuthorizationCallbacks()
176176
#pragma warning restore MCP9007
177177
}
178178

179+
[Fact]
180+
public async Task CanAuthenticate_WhenAuthorizationResponseStateMatches()
181+
{
182+
await using var app = await StartMcpServerAsync();
183+
184+
string? requestedState = null;
185+
await using var transport = CreateOAuthTransport((context, cancellationToken) =>
186+
{
187+
requestedState = QueryHelpers.ParseQuery(context.AuthorizationUri.Query)["state"];
188+
return HandleAuthorizationUrlAsync(context, cancellationToken);
189+
});
190+
191+
await using var client = await McpClient.CreateAsync(
192+
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
193+
194+
Assert.NotNull(requestedState);
195+
Assert.True(requestedState.Length >= 43);
196+
Assert.Equal(1, TestOAuthServer.AuthorizationCodeTokenRequestCount);
197+
}
198+
199+
[Fact]
200+
public async Task CannotAuthenticate_WhenAuthorizationResponseStateIsMissing()
201+
{
202+
await using var app = await StartMcpServerAsync();
203+
await using var transport = CreateOAuthTransport(
204+
(_, _) => Task.FromResult<ModelContextProtocol.Authentication.AuthorizationResult?>(new() { Code = "unused-code" }));
205+
206+
var ex = await Assert.ThrowsAsync<McpException>(() => McpClient.CreateAsync(
207+
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));
208+
209+
Assert.Contains("did not include the required state parameter", ex.Message);
210+
Assert.Equal(0, TestOAuthServer.AuthorizationCodeTokenRequestCount);
211+
}
212+
213+
[Fact]
214+
public async Task CannotAuthenticate_WhenAuthorizationResponseStateMismatches()
215+
{
216+
await using var app = await StartMcpServerAsync();
217+
await using var transport = CreateOAuthTransport(
218+
(_, _) => Task.FromResult<ModelContextProtocol.Authentication.AuthorizationResult?>(
219+
new() { Code = "unused-code", State = "unexpected-state" }));
220+
221+
var ex = await Assert.ThrowsAsync<McpException>(() => McpClient.CreateAsync(
222+
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken));
223+
224+
Assert.Contains("state did not match", ex.Message);
225+
Assert.Equal(0, TestOAuthServer.AuthorizationCodeTokenRequestCount);
226+
}
227+
228+
[Fact]
229+
public async Task AuthorizationRequests_UseUniqueStateValues()
230+
{
231+
await using var app = await StartMcpServerAsync();
232+
List<string> requestedStates = [];
233+
234+
for (var i = 0; i < 2; i++)
235+
{
236+
await using var transport = CreateOAuthTransport((context, cancellationToken) =>
237+
{
238+
requestedStates.Add(QueryHelpers.ParseQuery(context.AuthorizationUri.Query)["state"].ToString());
239+
return HandleAuthorizationUrlAsync(context, cancellationToken);
240+
});
241+
242+
await using var client = await McpClient.CreateAsync(
243+
transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken);
244+
}
245+
246+
Assert.Equal(2, requestedStates.Count);
247+
Assert.Equal(2, requestedStates.Distinct(StringComparer.Ordinal).Count());
248+
}
249+
179250
[Fact]
180251
public async Task CannotAuthenticate_WithoutOAuthConfiguration()
181252
{
@@ -550,8 +621,10 @@ public async Task CanAuthenticate_WithExtraParams()
550621
Assert.Contains("custom_param=custom_value", lastAuthorizationUri?.Query);
551622
}
552623

553-
[Fact]
554-
public async Task CannotOverrideExistingParameters_WithExtraParams()
624+
[Theory]
625+
[InlineData("redirect_uri")]
626+
[InlineData("state")]
627+
public async Task CannotOverrideExistingParameters_WithExtraParams(string parameterName)
555628
{
556629
await using var app = await StartMcpServerAsync();
557630

@@ -566,7 +639,7 @@ public async Task CannotOverrideExistingParameters_WithExtraParams()
566639
AuthorizationCallbackHandler = HandleAuthorizationUrlAsync,
567640
AdditionalAuthorizationParameters = new Dictionary<string, string>
568641
{
569-
["redirect_uri"] = "custom_value",
642+
[parameterName] = "custom_value",
570643
}
571644
},
572645
}, HttpClient, LoggerFactory);
@@ -2434,7 +2507,9 @@ public async Task AuthorizationFlow_ScopeSelector_ReturningEmpty_OmitsScopeParam
24342507
Assert.False(scopePresent);
24352508
}
24362509

2437-
private HttpClientTransport CreateOAuthTransport() =>
2510+
private HttpClientTransport CreateOAuthTransport(
2511+
Func<AuthorizationCallbackContext, CancellationToken, Task<ModelContextProtocol.Authentication.AuthorizationResult?>>?
2512+
authorizationCallbackHandler = null) =>
24382513
new(new()
24392514
{
24402515
Endpoint = new(McpServerUrl),
@@ -2443,7 +2518,7 @@ private HttpClientTransport CreateOAuthTransport() =>
24432518
ClientId = "demo-client",
24442519
ClientSecret = "demo-secret",
24452520
RedirectUri = new Uri("http://localhost:1179/callback"),
2446-
AuthorizationCallbackHandler = HandleAuthorizationUrlAsync,
2521+
AuthorizationCallbackHandler = authorizationCallbackHandler ?? HandleAuthorizationUrlAsync,
24472522
},
24482523
}, HttpClient, LoggerFactory);
24492524

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ protected async Task<WebApplication> StartMcpServerAsync(string path = "", strin
120120
return new ModelContextProtocol.Authentication.AuthorizationResult
121121
{
122122
Code = queryParams["code"],
123+
State = queryParams["state"],
123124
Iss = queryParams.TryGetValue("iss", out var iss) ? (string?)iss : null,
124125
};
125126
}

0 commit comments

Comments
 (0)