Fix ClientOAuthProvider "Client ID is not available" on cold start (#1705)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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
+        // should not be used for expired access tokens. This is important because 403 forbidden responses can
-        // be used for incremental consent which cannot be acheived with a simple refresh.
+        // 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.");
 
+    /// <summary>
+    /// Restores the client registration persisted alongside cached tokens when this provider has not been
+    /// assigned a client ID yet. This allows a durable <see cref="ITokenCache"/> 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.
+    /// </summary>
+    /// <remarks>
+    /// Callers must hold <c>_tokenAcquisitionLock</c>: this writes the shared <c>_clientId</c>,
+    /// <c>_clientSecret</c>, and <c>_tokenEndpointAuthMethod</c> 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 <see cref="ITokenCache"/> design).
+    /// </remarks>
+    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}");

src/ModelContextProtocol.Core/Authentication/TokenContainer.cs

@@ -35,5 +35,46 @@ public sealed class TokenContainer
     /// </summary>
     public required DateTimeOffset ObtainedAt { get; set; }
 
+    /// <summary>
+    /// Gets or sets the OAuth client ID that these tokens were issued to.
+    /// </summary>
+    /// <remarks>
+    /// This is persisted alongside the tokens so that a durable <see cref="ITokenCache"/> can survive a
+    /// process restart: on a cold start the client ID is restored from the cache, allowing a persisted
+    /// <see cref="RefreshToken"/> 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.
+    /// </remarks>
+    public string? ClientId { get; set; }
+
+    /// <summary>
+    /// Gets or sets the OAuth client secret that these tokens were issued to, if any.
+    /// </summary>
+    /// <remarks>
+    /// This is persisted alongside <see cref="ClientId"/> so a durable <see cref="ITokenCache"/> can use a
+    /// persisted <see cref="RefreshToken"/> after a restart. It is only populated when a client secret was
+    /// issued (for example via dynamic client registration).
+    /// <para>
+    /// Security: persisting this means a durable <see cref="ITokenCache"/> 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.
+    /// </para>
+    /// </remarks>
+    public string? ClientSecret { get; set; }
+
+    /// <summary>
+    /// Gets or sets the token endpoint authentication method associated with <see cref="ClientId"/>.
+    /// </summary>
+    /// <remarks>
+    /// This is persisted alongside <see cref="ClientId"/> so a refresh performed on a cold start uses the
+    /// same authentication method that was negotiated when the client was registered (for example
+    /// <c>none</c> for a public client rather than the default <c>client_secret_post</c>).
+    /// </remarks>
+    public string? TokenEndpointAuthMethod { get; set; }
+
     internal bool IsExpired => ExpiresIn is not null && DateTimeOffset.UtcNow >= ObtainedAt.AddSeconds(ExpiresIn.Value);
 }

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