diff --git a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs index 317a711..632fa9e 100644 --- a/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs +++ b/src/ModelContextProtocol.Core/Authentication/ClientOAuthProvider.cs @@ -417,13 +417,24 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient // The existing access token must be invalid to have resulted in a 401 response, but refresh might still work. var resourceUri = GetResourceUri(protectedResourceMetadata); + // Restore any client registration persisted alongside the tokens. On a cold start a durable + // token cache may hold a refresh token together with the client ID it was issued to, while this + // provider has not assigned a client ID yet. Restoring it here makes the refresh below possible + // and avoids a redundant dynamic client registration in the assignment block. + var cachedTokens = await _tokenCache.GetTokensAsync(cancellationToken).ConfigureAwait(false); + RestoreCachedClientCredentials(cachedTokens); + // Only attempt a token refresh if we haven't attempted to already for this request. // Also only attempt a token refresh for a 401 Unauthorized responses. Other response status codes - // should not be used for expired access tokens. This is important because 403 forbiden responses can - // be used for incremental consent which cannot be acheived with a simple refresh. + // should not be used for expired access tokens. This is important because 403 forbidden responses can + // be used for incremental consent which cannot be achieved with a simple refresh. + // A refresh also requires a client ID. On a cold start one may not be available yet (and could not + // be restored from the cache), in which case we fall through to the client-ID assignment block and + // the authorization-code flow below rather than throwing. if (!attemptedRefresh && response.StatusCode == System.Net.HttpStatusCode.Unauthorized && - await _tokenCache.GetTokensAsync(cancellationToken).ConfigureAwait(false) is { RefreshToken: { Length: > 0 } refreshToken }) + !string.IsNullOrEmpty(_clientId) && + cachedTokens is { RefreshToken: { Length: > 0 } refreshToken }) { var accessToken = await RefreshTokensAsync(refreshToken, resourceUri, authServerMetadata, cancellationToken).ConfigureAwait(false); if (accessToken is not null) @@ -841,6 +852,11 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient TokenType = tokenResponse.TokenType, Scope = tokenResponse.Scope, ObtainedAt = DateTimeOffset.UtcNow, + // Persist the client registration alongside the tokens so a durable cache can use the + // refresh token after a process restart without re-running dynamic client registration. + ClientId = _clientId, + ClientSecret = _clientSecret, + TokenEndpointAuthMethod = _tokenEndpointAuthMethod, }; await _tokenCache.StoreTokensAsync(tokens, cancellationToken).ConfigureAwait(false); @@ -1401,6 +1417,38 @@ internal sealed partial class ClientOAuthProvider : McpHttpClient private string GetClientIdOrThrow() => _clientId ?? throw new InvalidOperationException("Client ID is not available. This may indicate an issue with dynamic client registration."); + /// + /// Restores the client registration persisted alongside cached tokens when this provider has not been + /// assigned a client ID yet. This allows a durable to use a refresh token that + /// survived a process restart without re-running dynamic client registration. An explicitly configured + /// client ID always takes precedence, so nothing is restored when one is already available. + /// + /// + /// Callers must hold _tokenAcquisitionLock: this writes the shared _clientId, + /// _clientSecret, and _tokenEndpointAuthMethod fields, which the lock serializes. + /// Like the persisted refresh token, the restored client ID assumes the durable cache belongs to the + /// current authorization server; a single cache shared across authorization servers is not supported + /// (an inherent property of the single-container design). + /// + private void RestoreCachedClientCredentials(TokenContainer? tokens) + { + if (!string.IsNullOrEmpty(_clientId) || string.IsNullOrEmpty(tokens?.ClientId)) + { + return; + } + + // The guard above guarantees a non-null container, but the older nullable flow analysis on + // netstandard2.0/net472 doesn't infer that from the null-conditional check, so capture a + // non-null local and use it for every assignment. + var cached = tokens!; + + // Assign _clientId last. Callers treat a non-empty _clientId as "registration complete", so the + // secret and auth method must already be in place before _clientId becomes observable. + _clientSecret ??= cached.ClientSecret; + _tokenEndpointAuthMethod ??= cached.TokenEndpointAuthMethod; + _clientId = cached.ClientId; + } + [DoesNotReturn] private static void ThrowFailedToHandleUnauthorizedResponse(string message) => throw new McpException($"Failed to handle unauthorized response with 'Bearer' scheme. {message}"); diff --git a/src/ModelContextProtocol.Core/Authentication/TokenContainer.cs b/src/ModelContextProtocol.Core/Authentication/TokenContainer.cs index 8126137..6f6a542 100644 --- a/src/ModelContextProtocol.Core/Authentication/TokenContainer.cs +++ b/src/ModelContextProtocol.Core/Authentication/TokenContainer.cs @@ -35,5 +35,46 @@ public sealed class TokenContainer /// public required DateTimeOffset ObtainedAt { get; set; } + /// + /// Gets or sets the OAuth client ID that these tokens were issued to. + /// + /// + /// This is persisted alongside the tokens so that a durable can survive a + /// process restart: on a cold start the client ID is restored from the cache, allowing a persisted + /// to be used without re-running dynamic client registration or prompting + /// the user to re-authorize. It reflects the client ID currently in use, whether that was obtained via + /// dynamic client registration, a client-id metadata document, or configured explicitly. On a cold + /// start it is only restored when no client ID has been configured, so an explicitly configured client + /// ID always takes precedence. + /// + public string? ClientId { get; set; } + + /// + /// Gets or sets the OAuth client secret that these tokens were issued to, if any. + /// + /// + /// This is persisted alongside so a durable can use a + /// persisted after a restart. It is only populated when a client secret was + /// issued (for example via dynamic client registration). + /// + /// Security: persisting this means a durable stores a confidential client + /// credential, not just the refresh token (which for a confidential client is not usable on its own). + /// Cache implementations that persist to durable storage must protect these values at rest (for + /// example with OS-level encryption or a dedicated secret store); otherwise a cache compromise would + /// expose a complete, usable credential set. + /// + /// + public string? ClientSecret { get; set; } + + /// + /// Gets or sets the token endpoint authentication method associated with . + /// + /// + /// This is persisted alongside so a refresh performed on a cold start uses the + /// same authentication method that was negotiated when the client was registered (for example + /// none for a public client rather than the default client_secret_post). + /// + public string? TokenEndpointAuthMethod { get; set; } + internal bool IsExpired => ExpiresIn is not null && DateTimeOffset.UtcNow >= ObtainedAt.AddSeconds(ExpiresIn.Value); } diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/TokenCacheTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/TokenCacheTests.cs index 09aa165..cdd7475 100644 --- a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/TokenCacheTests.cs +++ b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/TokenCacheTests.cs @@ -187,6 +187,152 @@ public class TokenCacheTests : OAuthTestBase Assert.NotEqual("invalid-token", tokenCache.LastStoredToken.AccessToken); } + [Fact] + public async Task GetTokenAsync_ColdStartWithDynamicRegistration_RefreshesUsingPersistedCredentials() + { + await using var app = await StartMcpServerAsync(); + + var tokenCache = new TestTokenCache(); + var authDelegateCalledInitially = false; + + // First "process": no ClientId is configured, so the client registers dynamically. + await using var setupTransport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + RedirectUri = new Uri("http://localhost:1179/callback"), + DynamicClientRegistration = new() + { + ClientName = "Test MCP Client", + ClientUri = new Uri("https://example.com"), + }, + AuthorizationCallbackHandler = (context, ct) => + { + authDelegateCalledInitially = true; + return HandleAuthorizationUrlAsync(context, ct); + }, + TokenCache = tokenCache, + }, + }, HttpClient, LoggerFactory); + + await using (var setupClient = await McpClient.CreateAsync(setupTransport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)) + { + // Just connecting should trigger dynamic registration, authorization, and storage. + } + + Assert.True(authDelegateCalledInitially, "Authorization callback should be called for the initial authorization"); + Assert.False(TestOAuthServer.HasRefreshedToken, "Token should not have been refreshed yet"); + Assert.NotNull(tokenCache.LastStoredToken); + Assert.False( + string.IsNullOrEmpty(tokenCache.LastStoredToken.ClientId), + "The dynamically registered client ID should be persisted alongside the tokens"); + + // Simulate a cold start: the access token is no longer valid, but the refresh token persists. + // The new provider has no client ID configured and must restore it from the cache to refresh + // instead of throwing "Client ID is not available". + tokenCache.LastStoredToken.AccessToken = "invalid-token"; + var authDelegateCalledAgain = false; + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + RedirectUri = new Uri("http://localhost:1179/callback"), + DynamicClientRegistration = new() + { + ClientName = "Test MCP Client", + ClientUri = new Uri("https://example.com"), + }, + AuthorizationCallbackHandler = (context, ct) => + { + authDelegateCalledAgain = true; + return HandleAuthorizationUrlAsync(context, ct); + }, + TokenCache = tokenCache, + }, + }, HttpClient, LoggerFactory); + + await using var client = await McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.False(authDelegateCalledAgain, "Authorization callback should not be called when the persisted refresh token can be used"); + Assert.True(TestOAuthServer.HasRefreshedToken, "Token should have been refreshed using the persisted client credentials"); + Assert.NotEqual("invalid-token", tokenCache.LastStoredToken.AccessToken); + } + + [Fact] + public async Task GetTokenAsync_ColdStartWithoutPersistedClientId_FallsBackToReauthorization() + { + await using var app = await StartMcpServerAsync(); + + var tokenCache = new TestTokenCache(); + var authDelegateCalledInitially = false; + + await using var setupTransport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + RedirectUri = new Uri("http://localhost:1179/callback"), + DynamicClientRegistration = new() + { + ClientName = "Test MCP Client", + ClientUri = new Uri("https://example.com"), + }, + AuthorizationCallbackHandler = (context, ct) => + { + authDelegateCalledInitially = true; + return HandleAuthorizationUrlAsync(context, ct); + }, + TokenCache = tokenCache, + }, + }, HttpClient, LoggerFactory); + + await using (var setupClient = await McpClient.CreateAsync(setupTransport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken)) + { + } + + Assert.True(authDelegateCalledInitially, "Authorization callback should be called for the initial authorization"); + Assert.NotNull(tokenCache.LastStoredToken); + + // Simulate a durable cache that persisted tokens but not the client registration (for example + // an entry written by an older version). On a cold start the provider cannot refresh without a + // client ID and must fall back to a fresh authorization rather than throwing. + tokenCache.LastStoredToken.AccessToken = "invalid-token"; + tokenCache.LastStoredToken.ClientId = null; + tokenCache.LastStoredToken.ClientSecret = null; + tokenCache.LastStoredToken.TokenEndpointAuthMethod = null; + var authDelegateCalledAgain = false; + + await using var transport = new HttpClientTransport(new() + { + Endpoint = new(McpServerUrl), + OAuth = new() + { + RedirectUri = new Uri("http://localhost:1179/callback"), + DynamicClientRegistration = new() + { + ClientName = "Test MCP Client", + ClientUri = new Uri("https://example.com"), + }, + AuthorizationCallbackHandler = (context, ct) => + { + authDelegateCalledAgain = true; + return HandleAuthorizationUrlAsync(context, ct); + }, + TokenCache = tokenCache, + }, + }, HttpClient, LoggerFactory); + + // Should not throw "Client ID is not available"; instead re-authorizes via dynamic + // registration and the authorization-code flow. + await using var client = await McpClient.CreateAsync(transport, loggerFactory: LoggerFactory, cancellationToken: TestContext.Current.CancellationToken); + + Assert.True(authDelegateCalledAgain, "Authorization callback should be called to re-authorize when no client ID is available to refresh"); + Assert.False(TestOAuthServer.HasRefreshedToken, "A refresh should not be attempted without a client ID"); + } + private TokenContainer CreateInvalidToken() { return new TokenContainer