diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index ecef8e15e..56c01aaae 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -41,12 +41,15 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient private readonly HttpClient _httpClient; private readonly ILogger _logger; + private readonly bool _credentialsArePreRegistered; private string? _clientId; private string? _clientSecret; private string? _tokenEndpointAuthMethod; - private ITokenCache _tokenCache; + private readonly InvalidatableTokenCache _tokenCache; private AuthorizationServerMetadata? _authServerMetadata; + private Uri? _boundAuthServerIssuer; + private bool _isCimdClientId; /// /// Initializes a new instance of the class using the specified options. @@ -74,6 +77,7 @@ public ClientOAuthProvider( _clientId = options.ClientId; _clientSecret = options.ClientSecret; + _credentialsArePreRegistered = options.ClientId is not null; _redirectUri = options.RedirectUri ?? throw new ArgumentException("ClientOAuthOptions.RedirectUri must configured.", nameof(options)); _configuredScopes = options.Scopes is null ? null : string.Join(" ", options.Scopes); _additionalAuthorizationParameters = options.AdditionalAuthorizationParameters; @@ -89,7 +93,7 @@ public ClientOAuthProvider( _dcrClientUri = options.DynamicClientRegistration?.ClientUri; _dcrInitialAccessToken = options.DynamicClientRegistration?.InitialAccessToken; _dcrResponseDelegate = options.DynamicClientRegistration?.ResponseDelegate; - _tokenCache = options.TokenCache ?? new InMemoryTokenCache(); + _tokenCache = new InvalidatableTokenCache(options.TokenCache ?? new InMemoryTokenCache()); } /// @@ -272,6 +276,11 @@ private async Task GetAccessTokenAsync(HttpResponseMessage response, boo // Get auth server metadata var authServerMetadata = await GetAuthServerMetadataAsync(selectedAuthServer, protectedResourceMetadata.Resource, cancellationToken).ConfigureAwait(false); + // Check for authorization server change per MCP SEP-2352 and update the bound issuer. + // This must happen before attempting token refresh to prevent stale tokens/credentials from being reused. + var currentIssuer = authServerMetadata.Issuer ?? selectedAuthServer; + HandleAuthorizationServerChange(currentIssuer); + // The existing access token must be invalid to have resulted in a 401 response, but refresh might still work. var resourceUri = GetResourceUri(protectedResourceMetadata); @@ -287,6 +296,7 @@ await _tokenCache.GetTokensAsync(cancellationToken).ConfigureAwait(false) is { R if (accessToken is not null) { // A non-null result indicates the refresh succeeded and the new tokens have been stored. + _boundAuthServerIssuer = currentIssuer; return accessToken; } } @@ -308,8 +318,9 @@ await _tokenCache.GetTokensAsync(cancellationToken).ConfigureAwait(false) is { R // Determine the token endpoint auth method from server metadata if not already set by DCR. _tokenEndpointAuthMethod ??= authServerMetadata.TokenEndpointAuthMethodsSupported?.FirstOrDefault(); - // Store auth server metadata for future refresh operations + // Store auth server metadata and bound issuer for future refresh operations _authServerMetadata = authServerMetadata; + _boundAuthServerIssuer = currentIssuer; // Perform the OAuth flow return await InitiateAuthorizationCodeFlowAsync(protectedResourceMetadata, authServerMetadata, cancellationToken).ConfigureAwait(false); @@ -324,6 +335,7 @@ private void ApplyClientIdMetadataDocument(Uri metadataUri) } _clientId = metadataUri.AbsoluteUri; + _isCimdClientId = true; // See: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-client-id-metadata-document-00#section-3 static bool IsValidClientMetadataDocumentUri(Uri uri) @@ -973,6 +985,58 @@ private static string ToBase64UrlString(byte[] bytes) private string GetClientIdOrThrow() => _clientId ?? throw new InvalidOperationException("Client ID is not available. This may indicate an issue with dynamic client registration."); + /// + /// Detects if the authorization server has changed and handles the change according to the credential type. + /// Per MCP SEP-2352: clients MUST maintain separate state per AS and MUST NOT reuse credentials across ASes. + /// + private void HandleAuthorizationServerChange(Uri currentIssuer) + { + if (_boundAuthServerIssuer is null || UrisAreEquivalent(_boundAuthServerIssuer, currentIssuer)) + { + // First time binding, or same AS - no change to handle. + return; + } + + // The authorization server has changed. + if (_credentialsArePreRegistered) + { + // Pre-registered credentials are AS-specific. Per SEP-2352, clients SHOULD surface an error + // rather than silently attempting to use mismatched credentials. + throw new McpException( + $"The authorization server has changed from '{_boundAuthServerIssuer}' to '{currentIssuer}'. " + + "Pre-registered credentials are bound to a specific authorization server and cannot be reused with a different one."); + } + + // Invalidate cached tokens from the previous AS to prevent reuse. + _tokenCache.Invalidate(); + + // Clear stale AS metadata and token endpoint auth method so they are refreshed for the new AS. + _authServerMetadata = null; + _tokenEndpointAuthMethod = null; + + if (_isCimdClientId) + { + // CIMD-based client IDs are portable (self-hosted HTTPS URLs resolved by the AS on demand). + // Per SEP-2352: "No re-registration is needed when the authorization server changes." + // Keep the client ID but clear tokens (already done above). + LogAuthorizationServerChangedCimd(_boundAuthServerIssuer, currentIssuer); + } + else + { + // DCR-obtained credentials are AS-specific. Clear them so re-registration occurs with the new AS. + _clientId = null; + _clientSecret = null; + _isCimdClientId = false; + LogAuthorizationServerChangedDcr(_boundAuthServerIssuer, currentIssuer); + } + } + + private static bool UrisAreEquivalent(Uri a, Uri b) => + Uri.Compare(a, b, + UriComponents.Scheme | UriComponents.Host | UriComponents.Port | UriComponents.Path, + UriFormat.SafeUnescaped, + StringComparison.OrdinalIgnoreCase) == 0; + [DoesNotReturn] private static void ThrowFailedToHandleUnauthorizedResponse(string message) => throw new McpException($"Failed to handle unauthorized response with 'Bearer' scheme. {message}"); @@ -1003,4 +1067,41 @@ private static void ThrowFailedToHandleUnauthorizedResponse(string message) => [LoggerMessage(Level = LogLevel.Debug, Message = "Missing resource_metadata parameter from WWW-Authenticate header. Falling back to {MetadataUri}")] partial void LogMissingResourceMetadataParameter(Uri metadataUri); + + [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.")] + partial void LogAuthorizationServerChangedCimd(Uri oldIssuer, Uri newIssuer); + + [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.")] + partial void LogAuthorizationServerChangedDcr(Uri oldIssuer, Uri newIssuer); + + /// + /// Wraps an and allows the cached tokens to be invalidated + /// without changing the public interface. When invalidated, + /// returns until new tokens are stored via . + /// + private sealed class InvalidatableTokenCache(ITokenCache inner) : ITokenCache + { + private bool _isInvalidated; + + /// Marks the cached tokens as stale so they will not be returned by . + public void Invalidate() => _isInvalidated = true; + + /// + public async ValueTask GetTokensAsync(CancellationToken cancellationToken) + { + if (_isInvalidated) + { + return null; + } + + return await inner.GetTokensAsync(cancellationToken).ConfigureAwait(false); + } + + /// + public async ValueTask StoreTokensAsync(TokenContainer tokens, CancellationToken cancellationToken) + { + _isInvalidated = false; + await inner.StoreTokensAsync(tokens, cancellationToken).ConfigureAwait(false); + } + } } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthServerBindingTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthServerBindingTests.cs new file mode 100644 index 000000000..3f95f41b2 --- /dev/null +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthServerBindingTests.cs @@ -0,0 +1,272 @@ +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.JwtBearer; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.AspNetCore.Authentication; +using ModelContextProtocol.Authentication; +using ModelContextProtocol.Client; +using ModelContextProtocol.Server; + +namespace ModelContextProtocol.AspNetCore.Tests.OAuth; + +/// +/// Tests that verify the client enforces authorization server binding per MCP SEP-2352. +/// Clients MUST maintain separate credentials per authorization server and MUST NOT +/// reuse credentials from a different authorization server when the AS changes. +/// +public class AuthServerBindingTests : OAuthTestBase +{ + private const string ClientMetadataDocumentUrl = $"{OAuthServerUrl}/client-metadata/cimd-client.json"; + private const string AlternateAuthServerIssuer = $"{OAuthServerUrl}/v2"; + + public AuthServerBindingTests(ITestOutputHelper outputHelper) + : base(outputHelper) + { + } + + [Fact] + public async Task AuthServerChange_WithPreRegisteredCredentials_ThrowsMcpException() + { + // Arrange: configure the MCP server to dynamically switch its authorization server + var hasChangedAs = false; + + Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => + { + options.Events.OnResourceMetadataRequest = ctx => + { + ctx.ResourceMetadata = new ProtectedResourceMetadata + { + AuthorizationServers = { hasChangedAs ? AlternateAuthServerIssuer : OAuthServerUrl }, + ScopesSupported = ["mcp:tools"], + }; + return Task.CompletedTask; + }; + }); + + Builder.Services.AddMcpServer(options => options.ToolCollection = new()); + + await using var app = await StartMcpServerAsync(configureMiddleware: app => + { + // After the AS changes, return 401 for authenticated MCP requests to trigger re-authentication. + app.Use(async (context, next) => + { + if (hasChangedAs && context.Request.Method == HttpMethods.Post && context.Request.Path == "/" && + context.Request.Headers.Authorization.Count > 0) + { + await context.ChallengeAsync(JwtBearerDefaults.AuthenticationScheme); + await context.Response.StartAsync(context.RequestAborted); + await context.Response.Body.FlushAsync(context.RequestAborted); + return; + } + + await next(context); + }); + }); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + // Pre-registered credentials: ClientId is provided + ClientId = "demo-client", + ClientSecret = "demo-secret", + RedirectUri = new Uri("http://localhost:1179/callback"), + AuthorizationRedirectDelegate = HandleAuthorizationUrlAsync, + }, + }, HttpClient, LoggerFactory); + + // Act: connect initially with AS1 - should succeed + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + // Simulate the authorization server changing + hasChangedAs = true; + + // Assert: a subsequent request must throw because pre-registered credentials are AS-specific + var ex = await Assert.ThrowsAsync(async () => + await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken)); + + Assert.Contains("authorization server has changed", ex.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains(OAuthServerUrl, ex.Message, StringComparison.OrdinalIgnoreCase); + Assert.Contains(AlternateAuthServerIssuer, ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task AuthServerChange_WithDcrCredentials_TriggersReregistration() + { + // Arrange: configure the MCP server to dynamically switch its authorization server + var hasChangedAs = false; + var dcrCallCount = 0; + var authDelegateCallCount = 0; + + Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => + { + options.Events.OnResourceMetadataRequest = ctx => + { + ctx.ResourceMetadata = new ProtectedResourceMetadata + { + AuthorizationServers = { hasChangedAs ? AlternateAuthServerIssuer : OAuthServerUrl }, + ScopesSupported = ["mcp:tools"], + }; + return Task.CompletedTask; + }; + }); + + Builder.Services.AddMcpServer(options => options.ToolCollection = new()); + + await using var app = await StartMcpServerAsync(configureMiddleware: app => + { + app.Use(async (context, next) => + { + if (hasChangedAs && context.Request.Method == HttpMethods.Post && context.Request.Path == "/" && + context.Request.Headers.Authorization.Count > 0) + { + await context.ChallengeAsync(JwtBearerDefaults.AuthenticationScheme); + await context.Response.StartAsync(context.RequestAborted); + await context.Response.Body.FlushAsync(context.RequestAborted); + return; + } + + await next(context); + }); + }); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + // No ClientId: DCR will be used + RedirectUri = new Uri("http://localhost:1179/callback"), + DynamicClientRegistration = new() + { + ClientName = "Test MCP Client", + ResponseDelegate = (_, _) => + { + dcrCallCount++; + return Task.CompletedTask; + }, + }, + AuthorizationRedirectDelegate = (uri, redirect, ct) => + { + authDelegateCallCount++; + // On the second call (after AS change), return null to stop the flow. + // This lets us verify DCR happened without needing the full token exchange to succeed. + if (authDelegateCallCount > 1) + { + return Task.FromResult(null); + } + + return HandleAuthorizationUrlAsync(uri, redirect, ct); + }, + }, + }, HttpClient, LoggerFactory); + + // Act: connect initially with AS1 using DCR - should succeed + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, dcrCallCount); // DCR happened once for AS1 + + // Simulate the authorization server changing + hasChangedAs = true; + + // This will fail because the auth delegate returns null (by design to observe the re-registration), + // but DCR with the new AS must have occurred first. + var ex = await Assert.ThrowsAsync(async () => + await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken)); + + // Assert: DCR was triggered a second time for the new AS + Assert.Equal(2, dcrCallCount); + Assert.Contains("authorization code", ex.Message, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task AuthServerChange_WithCimdCredentials_ClearsTokensButKeepsPortableClientId() + { + // Arrange: configure the MCP server to dynamically switch its authorization server + var hasChangedAs = false; + var authDelegateCallCount = 0; + Uri? secondAuthUri = null; + + Builder.Services.Configure(McpAuthenticationDefaults.AuthenticationScheme, options => + { + options.Events.OnResourceMetadataRequest = ctx => + { + ctx.ResourceMetadata = new ProtectedResourceMetadata + { + AuthorizationServers = { hasChangedAs ? AlternateAuthServerIssuer : OAuthServerUrl }, + ScopesSupported = ["mcp:tools"], + }; + return Task.CompletedTask; + }; + }); + + Builder.Services.AddMcpServer(options => options.ToolCollection = new()); + + await using var app = await StartMcpServerAsync(configureMiddleware: app => + { + app.Use(async (context, next) => + { + if (hasChangedAs && context.Request.Method == HttpMethods.Post && context.Request.Path == "/" && + context.Request.Headers.Authorization.Count > 0) + { + await context.ChallengeAsync(JwtBearerDefaults.AuthenticationScheme); + await context.Response.StartAsync(context.RequestAborted); + await context.Response.Body.FlushAsync(context.RequestAborted); + return; + } + + await next(context); + }); + }); + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + // CIMD: client ID is a portable HTTPS URL resolved by the AS + RedirectUri = new Uri("http://localhost:1179/callback"), + ClientMetadataDocumentUri = new Uri(ClientMetadataDocumentUrl), + AuthorizationRedirectDelegate = (uri, redirect, ct) => + { + authDelegateCallCount++; + if (authDelegateCallCount == 1) + { + return HandleAuthorizationUrlAsync(uri, redirect, ct); + } + + // Capture the second auth URI to verify the CIMD client ID is preserved + secondAuthUri = uri; + // Return null to stop the flow - we've captured what we need + return Task.FromResult(null); + }, + }, + }, HttpClient, LoggerFactory); + + // Act: connect initially with AS1 using CIMD - should succeed + await using var client = await McpClient.CreateAsync( + transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.Equal(1, authDelegateCallCount); // One auth code flow for AS1 + + // Simulate the authorization server changing + hasChangedAs = true; + + // This will fail because we return null from the auth delegate, + // but the important thing is to verify the CIMD client ID is still used. + var ex = await Assert.ThrowsAsync(async () => + await client.ListToolsAsync(cancellationToken: TestContext.Current.CancellationToken)); + + // Assert: the auth code flow was triggered again (tokens were cleared) + Assert.Equal(2, authDelegateCallCount); + + // Assert: the CIMD client ID (portable URL) was used in the second auth request, + // not a new DCR-registered ID + Assert.NotNull(secondAuthUri); + Assert.Contains(Uri.EscapeDataString(ClientMetadataDocumentUrl), secondAuthUri.Query, StringComparison.OrdinalIgnoreCase); + } +}