Skip to content

Commit af22e21

Browse files
Copilotstephentoub
andauthored
Implement AS binding and change detection per MCP SEP-2352
Agent-Logs-Url: https://github.com/modelcontextprotocol/csharp-sdk/sessions/101fdc47-ecb5-4163-829b-0b320592a4be Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
1 parent 2392567 commit af22e21

2 files changed

Lines changed: 373 additions & 3 deletions

File tree

src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs

Lines changed: 101 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,15 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient
4141

4242
private readonly HttpClient _httpClient;
4343
private readonly ILogger _logger;
44+
private readonly bool _credentialsArePreRegistered;
4445

4546
private string? _clientId;
4647
private string? _clientSecret;
4748
private string? _tokenEndpointAuthMethod;
48-
private ITokenCache _tokenCache;
49+
private readonly InvalidatableTokenCache _tokenCache;
4950
private AuthorizationServerMetadata? _authServerMetadata;
51+
private Uri? _boundAuthServerIssuer;
52+
private bool _isCimdClientId;
5053

5154
/// <summary>
5255
/// Initializes a new instance of the <see cref="ClientOAuthProvider"/> class using the specified options.
@@ -74,6 +77,7 @@ public ClientOAuthProvider(
7477

7578
_clientId = options.ClientId;
7679
_clientSecret = options.ClientSecret;
80+
_credentialsArePreRegistered = options.ClientId is not null;
7781
_redirectUri = options.RedirectUri ?? throw new ArgumentException("ClientOAuthOptions.RedirectUri must configured.", nameof(options));
7882
_configuredScopes = options.Scopes is null ? null : string.Join(" ", options.Scopes);
7983
_additionalAuthorizationParameters = options.AdditionalAuthorizationParameters;
@@ -89,7 +93,7 @@ public ClientOAuthProvider(
8993
_dcrClientUri = options.DynamicClientRegistration?.ClientUri;
9094
_dcrInitialAccessToken = options.DynamicClientRegistration?.InitialAccessToken;
9195
_dcrResponseDelegate = options.DynamicClientRegistration?.ResponseDelegate;
92-
_tokenCache = options.TokenCache ?? new InMemoryTokenCache();
96+
_tokenCache = new InvalidatableTokenCache(options.TokenCache ?? new InMemoryTokenCache());
9397
}
9498

9599
/// <summary>
@@ -272,6 +276,11 @@ private async Task<string> GetAccessTokenAsync(HttpResponseMessage response, boo
272276
// Get auth server metadata
273277
var authServerMetadata = await GetAuthServerMetadataAsync(selectedAuthServer, protectedResourceMetadata.Resource, cancellationToken).ConfigureAwait(false);
274278

279+
// Check for authorization server change per MCP SEP-2352 and update the bound issuer.
280+
// This must happen before attempting token refresh to prevent stale tokens/credentials from being reused.
281+
var currentIssuer = authServerMetadata.Issuer ?? selectedAuthServer;
282+
HandleAuthorizationServerChange(currentIssuer);
283+
275284
// The existing access token must be invalid to have resulted in a 401 response, but refresh might still work.
276285
var resourceUri = GetResourceUri(protectedResourceMetadata);
277286

@@ -287,6 +296,7 @@ await _tokenCache.GetTokensAsync(cancellationToken).ConfigureAwait(false) is { R
287296
if (accessToken is not null)
288297
{
289298
// A non-null result indicates the refresh succeeded and the new tokens have been stored.
299+
_boundAuthServerIssuer = currentIssuer;
290300
return accessToken;
291301
}
292302
}
@@ -308,8 +318,9 @@ await _tokenCache.GetTokensAsync(cancellationToken).ConfigureAwait(false) is { R
308318
// Determine the token endpoint auth method from server metadata if not already set by DCR.
309319
_tokenEndpointAuthMethod ??= authServerMetadata.TokenEndpointAuthMethodsSupported?.FirstOrDefault();
310320

311-
// Store auth server metadata for future refresh operations
321+
// Store auth server metadata and bound issuer for future refresh operations
312322
_authServerMetadata = authServerMetadata;
323+
_boundAuthServerIssuer = currentIssuer;
313324

314325
// Perform the OAuth flow
315326
return await InitiateAuthorizationCodeFlowAsync(protectedResourceMetadata, authServerMetadata, cancellationToken).ConfigureAwait(false);
@@ -324,6 +335,7 @@ private void ApplyClientIdMetadataDocument(Uri metadataUri)
324335
}
325336

326337
_clientId = metadataUri.AbsoluteUri;
338+
_isCimdClientId = true;
327339

328340
// See: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-client-id-metadata-document-00#section-3
329341
static bool IsValidClientMetadataDocumentUri(Uri uri)
@@ -973,6 +985,55 @@ private static string ToBase64UrlString(byte[] bytes)
973985

974986
private string GetClientIdOrThrow() => _clientId ?? throw new InvalidOperationException("Client ID is not available. This may indicate an issue with dynamic client registration.");
975987

988+
/// <summary>
989+
/// Detects if the authorization server has changed and handles the change according to the credential type.
990+
/// Per MCP SEP-2352: clients MUST maintain separate state per AS and MUST NOT reuse credentials across ASes.
991+
/// </summary>
992+
private void HandleAuthorizationServerChange(Uri currentIssuer)
993+
{
994+
if (_boundAuthServerIssuer is null || UrisAreEquivalent(_boundAuthServerIssuer, currentIssuer))
995+
{
996+
// First time binding, or same AS - no change to handle.
997+
return;
998+
}
999+
1000+
// The authorization server has changed.
1001+
if (_credentialsArePreRegistered)
1002+
{
1003+
// Pre-registered credentials are AS-specific. Per SEP-2352, clients SHOULD surface an error
1004+
// rather than silently attempting to use mismatched credentials.
1005+
throw new McpException(
1006+
$"The authorization server has changed from '{_boundAuthServerIssuer}' to '{currentIssuer}'. " +
1007+
"Pre-registered credentials are bound to a specific authorization server and cannot be reused with a different one.");
1008+
}
1009+
1010+
// Invalidate cached tokens from the previous AS to prevent reuse.
1011+
_tokenCache.Invalidate();
1012+
1013+
// Clear stale AS metadata and token endpoint auth method so they are refreshed for the new AS.
1014+
_authServerMetadata = null;
1015+
_tokenEndpointAuthMethod = null;
1016+
1017+
if (_isCimdClientId)
1018+
{
1019+
// CIMD-based client IDs are portable (self-hosted HTTPS URLs resolved by the AS on demand).
1020+
// Per SEP-2352: "No re-registration is needed when the authorization server changes."
1021+
// Keep the client ID but clear tokens (already done above).
1022+
LogAuthorizationServerChangedCimd(_boundAuthServerIssuer, currentIssuer);
1023+
}
1024+
else
1025+
{
1026+
// DCR-obtained credentials are AS-specific. Clear them so re-registration occurs with the new AS.
1027+
_clientId = null;
1028+
_clientSecret = null;
1029+
_isCimdClientId = false;
1030+
LogAuthorizationServerChangedDcr(_boundAuthServerIssuer, currentIssuer);
1031+
}
1032+
}
1033+
1034+
private static bool UrisAreEquivalent(Uri a, Uri b) =>
1035+
string.Equals(a.AbsoluteUri.TrimEnd('/'), b.AbsoluteUri.TrimEnd('/'), StringComparison.OrdinalIgnoreCase);
1036+
9761037
[DoesNotReturn]
9771038
private static void ThrowFailedToHandleUnauthorizedResponse(string message) =>
9781039
throw new McpException($"Failed to handle unauthorized response with 'Bearer' scheme. {message}");
@@ -1003,4 +1064,41 @@ private static void ThrowFailedToHandleUnauthorizedResponse(string message) =>
10031064

10041065
[LoggerMessage(Level = LogLevel.Debug, Message = "Missing resource_metadata parameter from WWW-Authenticate header. Falling back to {MetadataUri}")]
10051066
partial void LogMissingResourceMetadataParameter(Uri metadataUri);
1067+
1068+
[LoggerMessage(Level = LogLevel.Warning, Message = "Authorization server changed from '{OldIssuer}' to '{NewIssuer}'. The CIMD-based client ID is portable and will be reused, but cached tokens have been invalidated.")]
1069+
partial void LogAuthorizationServerChangedCimd(Uri oldIssuer, Uri newIssuer);
1070+
1071+
[LoggerMessage(Level = LogLevel.Warning, Message = "Authorization server changed from '{OldIssuer}' to '{NewIssuer}'. DCR credentials have been cleared and will be re-registered with the new authorization server.")]
1072+
partial void LogAuthorizationServerChangedDcr(Uri oldIssuer, Uri newIssuer);
1073+
1074+
/// <summary>
1075+
/// Wraps an <see cref="ITokenCache"/> and allows the cached tokens to be invalidated
1076+
/// without changing the public interface. When invalidated, <see cref="GetTokensAsync"/>
1077+
/// returns <see langword="null"/> until new tokens are stored via <see cref="StoreTokensAsync"/>.
1078+
/// </summary>
1079+
private sealed class InvalidatableTokenCache(ITokenCache inner) : ITokenCache
1080+
{
1081+
private bool _isInvalidated;
1082+
1083+
/// <summary>Marks the cached tokens as stale so they will not be returned by <see cref="GetTokensAsync"/>.</summary>
1084+
public void Invalidate() => _isInvalidated = true;
1085+
1086+
/// <inheritdoc/>
1087+
public async ValueTask<TokenContainer?> GetTokensAsync(CancellationToken cancellationToken)
1088+
{
1089+
if (_isInvalidated)
1090+
{
1091+
return null;
1092+
}
1093+
1094+
return await inner.GetTokensAsync(cancellationToken).ConfigureAwait(false);
1095+
}
1096+
1097+
/// <inheritdoc/>
1098+
public async ValueTask StoreTokensAsync(TokenContainer tokens, CancellationToken cancellationToken)
1099+
{
1100+
_isInvalidated = false;
1101+
await inner.StoreTokensAsync(tokens, cancellationToken).ConfigureAwait(false);
1102+
}
1103+
}
10061104
}

0 commit comments

Comments
 (0)