Remove private reflection from tests (#1107)

Copilot · stephentoub · halter73 · web-flow · commit 8b73a42c9d53 · 2025-12-16T21:20:42.000+02:00
Co-authored-by: copilot-swe-agent[bot] &lt;198982749+Copilot@users.noreply.github.com&gt;
Co-authored-by: stephentoub &lt;2642209+stephentoub@users.noreply.github.com&gt;
Co-authored-by: halter73 &lt;54385+halter73@users.noreply.github.com&gt;
diff --git a/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs b/src/ModelContextProtocol.AspNetCore/Authentication/McpAuthenticationHandler.cs
@@ -130,7 +130,7 @@ private static string GetConfiguredResourceMetadataPath(Uri resourceMetadataUri)
 
     private async Task<bool> HandleResourceMetadataRequestAsync(Uri? derivedResourceUri = null)
     {
-        var resourceMetadata = CloneResourceMetadata(Options.ResourceMetadata, derivedResourceUri);
+        var resourceMetadata = Options.ResourceMetadata?.Clone(derivedResourceUri);
 
         if (Options.Events.OnResourceMetadataRequest is not null)
         {
@@ -192,32 +192,6 @@ protected override Task HandleChallengeAsync(AuthenticationProperties properties
         return base.HandleChallengeAsync(properties);
     }
 
-    internal static ProtectedResourceMetadata? CloneResourceMetadata(ProtectedResourceMetadata? resourceMetadata, Uri? derivedResourceUri = null)
-    {
-        if (resourceMetadata is null)
-        {
-            return null;
-        }
-
-        return new ProtectedResourceMetadata
-        {
-            Resource = resourceMetadata.Resource ?? derivedResourceUri,
-            AuthorizationServers = [.. resourceMetadata.AuthorizationServers],
-            BearerMethodsSupported = [.. resourceMetadata.BearerMethodsSupported],
-            ScopesSupported = [.. resourceMetadata.ScopesSupported],
-            JwksUri = resourceMetadata.JwksUri,
-            ResourceSigningAlgValuesSupported = resourceMetadata.ResourceSigningAlgValuesSupported is not null ? [.. resourceMetadata.ResourceSigningAlgValuesSupported] : null,
-            ResourceName = resourceMetadata.ResourceName,
-            ResourceDocumentation = resourceMetadata.ResourceDocumentation,
-            ResourcePolicyUri = resourceMetadata.ResourcePolicyUri,
-            ResourceTosUri = resourceMetadata.ResourceTosUri,
-            TlsClientCertificateBoundAccessTokens = resourceMetadata.TlsClientCertificateBoundAccessTokens,
-            AuthorizationDetailsTypesSupported = resourceMetadata.AuthorizationDetailsTypesSupported is not null ? [.. resourceMetadata.AuthorizationDetailsTypesSupported] : null,
-            DpopSigningAlgValuesSupported = resourceMetadata.DpopSigningAlgValuesSupported is not null ? [.. resourceMetadata.DpopSigningAlgValuesSupported] : null,
-            DpopBoundAccessTokensRequired = resourceMetadata.DpopBoundAccessTokensRequired
-        };
-    }
-
     [LoggerMessage(Level = LogLevel.Warning, Message = "Resource metadata request host did not match configured host '{ConfiguredHost}'.")]
     private static partial void LogResourceMetadataHostMismatch(ILogger logger, string configuredHost);
 
diff --git a/src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs b/src/ModelContextProtocol.Core/Authentication/ProtectedResourceMetadata.cs
@@ -191,4 +191,30 @@ public sealed class ProtectedResourceMetadata
     /// </summary>
     [JsonIgnore]
     internal string? WwwAuthenticateScope { get; set; }
+
+    /// <summary>
+    /// Creates a deep copy of this <see cref="ProtectedResourceMetadata"/> instance, optionally overriding the Resource property.
+    /// </summary>
+    /// <param name="derivedResourceUri">Optional URI to use for the Resource property if the original Resource is null.</param>
+    /// <returns>A new instance of <see cref="ProtectedResourceMetadata"/> with cloned values.</returns>
+    public ProtectedResourceMetadata Clone(Uri? derivedResourceUri = null)
+    {
+        return new ProtectedResourceMetadata
+        {
+            Resource = Resource ?? derivedResourceUri,
+            AuthorizationServers = [.. AuthorizationServers],
+            BearerMethodsSupported = [.. BearerMethodsSupported],
+            ScopesSupported = [.. ScopesSupported],
+            JwksUri = JwksUri,
+            ResourceSigningAlgValuesSupported = ResourceSigningAlgValuesSupported is not null ? [.. ResourceSigningAlgValuesSupported] : null,
+            ResourceName = ResourceName,
+            ResourceDocumentation = ResourceDocumentation,
+            ResourcePolicyUri = ResourcePolicyUri,
+            ResourceTosUri = ResourceTosUri,
+            TlsClientCertificateBoundAccessTokens = TlsClientCertificateBoundAccessTokens,
+            AuthorizationDetailsTypesSupported = AuthorizationDetailsTypesSupported is not null ? [.. AuthorizationDetailsTypesSupported] : null,
+            DpopSigningAlgValuesSupported = DpopSigningAlgValuesSupported is not null ? [.. DpopSigningAlgValuesSupported] : null,
+            DpopBoundAccessTokensRequired = DpopBoundAccessTokensRequired
+        };
+    }
 }
diff --git a/tests/Common/Utils/TestServerTransport.cs b/tests/Common/Utils/TestServerTransport.cs
@@ -91,4 +91,12 @@ private async Task WriteMessageAsync(JsonRpcMessage message, CancellationToken c
     {
         await _messageChannel.Writer.WriteAsync(message, cancellationToken);
     }
+
+    /// <summary>
+    /// Sends a message from the client to the server (simulating client-to-server communication).
+    /// </summary>
+    public async Task SendClientMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default)
+    {
+        await _messageChannel.Writer.WriteAsync(message, cancellationToken);
+    }
 }
diff --git a/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs b/tests/ModelContextProtocol.AspNetCore.Tests/OAuth/AuthTests.cs
@@ -9,7 +9,6 @@
 using ModelContextProtocol.Client;
 using ModelContextProtocol.Server;
 using System.Net;
-using System.Reflection;
 using System.Security.Claims;
 using Xunit.Sdk;
 
@@ -751,105 +750,4 @@ await McpClient.CreateAsync(
 
         Assert.Contains("does not match", ex.Message);
     }
-
-    [Fact]
-    public void CloneResourceMetadataClonesAllProperties()
-    {
-        var propertyNames = typeof(ProtectedResourceMetadata).GetProperties().Select(property => property.Name).ToList();
-
-        // Set metadata properties to non-default values to verify they're copied.
-        var metadata = new ProtectedResourceMetadata
-        {
-            Resource = new Uri("https://example.com/resource"),
-            AuthorizationServers = [new Uri("https://auth1.example.com"), new Uri("https://auth2.example.com")],
-            BearerMethodsSupported = ["header", "body", "query"],
-            ScopesSupported = ["read", "write", "admin"],
-            JwksUri = new Uri("https://example.com/.well-known/jwks.json"),
-            ResourceSigningAlgValuesSupported = ["RS256", "ES256"],
-            ResourceName = "Test Resource",
-            ResourceDocumentation = new Uri("https://docs.example.com"),
-            ResourcePolicyUri = new Uri("https://example.com/policy"),
-            ResourceTosUri = new Uri("https://example.com/terms"),
-            TlsClientCertificateBoundAccessTokens = true,
-            AuthorizationDetailsTypesSupported = ["payment_initiation", "account_information"],
-            DpopSigningAlgValuesSupported = ["RS256", "PS256"],
-            DpopBoundAccessTokensRequired = true
-        };
-
-        // Use reflection to call the internal CloneResourceMetadata method
-        var handlerType = typeof(McpAuthenticationHandler);
-        var cloneMethod = handlerType.GetMethod("CloneResourceMetadata", BindingFlags.Static | BindingFlags.NonPublic);
-        Assert.NotNull(cloneMethod);
-
-        var clonedMetadata = (ProtectedResourceMetadata?)cloneMethod.Invoke(null, [metadata, null]);
-        Assert.NotNull(clonedMetadata);
-
-        // Ensure the cloned metadata is not the same instance
-        Assert.NotSame(metadata, clonedMetadata);
-
-        // Verify Resource property
-        Assert.Equal(metadata.Resource, clonedMetadata.Resource);
-        Assert.True(propertyNames.Remove(nameof(metadata.Resource)));
-
-        // Verify AuthorizationServers list is cloned and contains the same values
-        Assert.NotSame(metadata.AuthorizationServers, clonedMetadata.AuthorizationServers);
-        Assert.Equal(metadata.AuthorizationServers, clonedMetadata.AuthorizationServers);
-        Assert.True(propertyNames.Remove(nameof(metadata.AuthorizationServers)));
-
-        // Verify BearerMethodsSupported list is cloned and contains the same values
-        Assert.NotSame(metadata.BearerMethodsSupported, clonedMetadata.BearerMethodsSupported);
-        Assert.Equal(metadata.BearerMethodsSupported, clonedMetadata.BearerMethodsSupported);
-        Assert.True(propertyNames.Remove(nameof(metadata.BearerMethodsSupported)));
-
-        // Verify ScopesSupported list is cloned and contains the same values
-        Assert.NotSame(metadata.ScopesSupported, clonedMetadata.ScopesSupported);
-        Assert.Equal(metadata.ScopesSupported, clonedMetadata.ScopesSupported);
-        Assert.True(propertyNames.Remove(nameof(metadata.ScopesSupported)));
-
-        // Verify JwksUri property
-        Assert.Equal(metadata.JwksUri, clonedMetadata.JwksUri);
-        Assert.True(propertyNames.Remove(nameof(metadata.JwksUri)));
-
-        // Verify ResourceSigningAlgValuesSupported list is cloned (nullable list)
-        Assert.NotSame(metadata.ResourceSigningAlgValuesSupported, clonedMetadata.ResourceSigningAlgValuesSupported);
-        Assert.Equal(metadata.ResourceSigningAlgValuesSupported, clonedMetadata.ResourceSigningAlgValuesSupported);
-        Assert.True(propertyNames.Remove(nameof(metadata.ResourceSigningAlgValuesSupported)));
-
-        // Verify ResourceName property
-        Assert.Equal(metadata.ResourceName, clonedMetadata.ResourceName);
-        Assert.True(propertyNames.Remove(nameof(metadata.ResourceName)));
-
-        // Verify ResourceDocumentation property
-        Assert.Equal(metadata.ResourceDocumentation, clonedMetadata.ResourceDocumentation);
-        Assert.True(propertyNames.Remove(nameof(metadata.ResourceDocumentation)));
-
-        // Verify ResourcePolicyUri property
-        Assert.Equal(metadata.ResourcePolicyUri, clonedMetadata.ResourcePolicyUri);
-        Assert.True(propertyNames.Remove(nameof(metadata.ResourcePolicyUri)));
-
-        // Verify ResourceTosUri property
-        Assert.Equal(metadata.ResourceTosUri, clonedMetadata.ResourceTosUri);
-        Assert.True(propertyNames.Remove(nameof(metadata.ResourceTosUri)));
-
-        // Verify TlsClientCertificateBoundAccessTokens property
-        Assert.Equal(metadata.TlsClientCertificateBoundAccessTokens, clonedMetadata.TlsClientCertificateBoundAccessTokens);
-        Assert.True(propertyNames.Remove(nameof(metadata.TlsClientCertificateBoundAccessTokens)));
-
-        // Verify AuthorizationDetailsTypesSupported list is cloned (nullable list)
-        Assert.NotSame(metadata.AuthorizationDetailsTypesSupported, clonedMetadata.AuthorizationDetailsTypesSupported);
-        Assert.Equal(metadata.AuthorizationDetailsTypesSupported, clonedMetadata.AuthorizationDetailsTypesSupported);
-        Assert.True(propertyNames.Remove(nameof(metadata.AuthorizationDetailsTypesSupported)));
-
-        // Verify DpopSigningAlgValuesSupported list is cloned (nullable list)
-        Assert.NotSame(metadata.DpopSigningAlgValuesSupported, clonedMetadata.DpopSigningAlgValuesSupported);
-        Assert.Equal(metadata.DpopSigningAlgValuesSupported, clonedMetadata.DpopSigningAlgValuesSupported);
-        Assert.True(propertyNames.Remove(nameof(metadata.DpopSigningAlgValuesSupported)));
-
-        // Verify DpopBoundAccessTokensRequired property
-        Assert.Equal(metadata.DpopBoundAccessTokensRequired, clonedMetadata.DpopBoundAccessTokensRequired);
-        Assert.True(propertyNames.Remove(nameof(metadata.DpopBoundAccessTokensRequired)));
-
-        // Ensure we've checked every property. When new properties get added, we'll have to update this test along with the CloneResourceMetadata implementation.
-        Assert.Empty(propertyNames);
-    }
 }
diff --git a/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs b/tests/ModelContextProtocol.Tests/Server/McpServerTests.cs
@@ -128,14 +128,18 @@ public async Task SampleAsync_Should_Throw_Exception_If_Client_Does_Not_Support_
         // Arrange
         await using var transport = new TestServerTransport();
         await using var server = McpServer.Create(transport, _options, LoggerFactory);
-        SetClientCapabilities(server, new ClientCapabilities());
+        var runTask = server.RunAsync(TestContext.Current.CancellationToken);
+        await InitializeServerAsync(transport, new ClientCapabilities(), TestContext.Current.CancellationToken);
 
         var action = async () => await server.SampleAsync(
             new CreateMessageRequestParams { Messages = [], MaxTokens = 1000 }, 
             CancellationToken.None);
 
         // Act & Assert
         await Assert.ThrowsAsync<InvalidOperationException>(action);
+
+        await transport.DisposeAsync();
+        await runTask;
     }
 
     [Fact]
@@ -144,9 +148,8 @@ public async Task SampleAsync_Should_SendRequest()
         // Arrange
         await using var transport = new TestServerTransport();
         await using var server = McpServer.Create(transport, _options, LoggerFactory);
-        SetClientCapabilities(server, new ClientCapabilities { Sampling = new SamplingCapability() });
-
         var runTask = server.RunAsync(TestContext.Current.CancellationToken);
+        await InitializeServerAsync(transport, new ClientCapabilities { Sampling = new SamplingCapability() }, TestContext.Current.CancellationToken);
 
         // Act
         var result = await server.SampleAsync(
@@ -155,8 +158,10 @@ public async Task SampleAsync_Should_SendRequest()
 
         Assert.NotNull(result);
         Assert.NotEmpty(transport.SentMessages);
-        Assert.IsType<JsonRpcRequest>(transport.SentMessages[0]);
-        Assert.Equal(RequestMethods.SamplingCreateMessage, ((JsonRpcRequest)transport.SentMessages[0]).Method);
+        // First message is the initialize response, second is the sampling request
+        Assert.True(transport.SentMessages.Count >= 2, "Expected at least 2 messages (initialize response and sampling request)");
+        var samplingRequest = Assert.IsType<JsonRpcRequest>(transport.SentMessages[1]);
+        Assert.Equal(RequestMethods.SamplingCreateMessage, samplingRequest.Method);
 
         await transport.DisposeAsync();
         await runTask;
@@ -168,12 +173,16 @@ public async Task RequestRootsAsync_Should_Throw_Exception_If_Client_Does_Not_Su
         // Arrange
         await using var transport = new TestServerTransport();
         await using var server = McpServer.Create(transport, _options, LoggerFactory);
-        SetClientCapabilities(server, new ClientCapabilities());
+        var runTask = server.RunAsync(TestContext.Current.CancellationToken);
+        await InitializeServerAsync(transport, new ClientCapabilities(), TestContext.Current.CancellationToken);
 
         // Act & Assert
         await Assert.ThrowsAsync<InvalidOperationException>(async () => await server.RequestRootsAsync(
             new ListRootsRequestParams(), 
             CancellationToken.None));
+
+        await transport.DisposeAsync();
+        await runTask;
     }
 
     [Fact]
@@ -182,17 +191,19 @@ public async Task RequestRootsAsync_Should_SendRequest()
         // Arrange
         await using var transport = new TestServerTransport();
         await using var server = McpServer.Create(transport, _options, LoggerFactory);
-        SetClientCapabilities(server, new ClientCapabilities { Roots = new RootsCapability() });
         var runTask = server.RunAsync(TestContext.Current.CancellationToken);
+        await InitializeServerAsync(transport, new ClientCapabilities { Roots = new RootsCapability() }, TestContext.Current.CancellationToken);
 
         // Act
         var result = await server.RequestRootsAsync(new ListRootsRequestParams(), CancellationToken.None);
 
         // Assert
         Assert.NotNull(result);
         Assert.NotEmpty(transport.SentMessages);
-        Assert.IsType<JsonRpcRequest>(transport.SentMessages[0]);
-        Assert.Equal(RequestMethods.RootsList, ((JsonRpcRequest)transport.SentMessages[0]).Method);
+        // First message is the initialize response, second is the roots request
+        Assert.True(transport.SentMessages.Count >= 2, "Expected at least 2 messages (initialize response and roots request)");
+        var rootsRequest = Assert.IsType<JsonRpcRequest>(transport.SentMessages[1]);
+        Assert.Equal(RequestMethods.RootsList, rootsRequest.Method);
 
         await transport.DisposeAsync();
         await runTask;
@@ -204,12 +215,16 @@ public async Task ElicitAsync_Should_Throw_Exception_If_Client_Does_Not_Support_
         // Arrange
         await using var transport = new TestServerTransport();
         await using var server = McpServer.Create(transport, _options, LoggerFactory);
-        SetClientCapabilities(server, new ClientCapabilities());
+        var runTask = server.RunAsync(TestContext.Current.CancellationToken);
+        await InitializeServerAsync(transport, new ClientCapabilities(), TestContext.Current.CancellationToken);
 
         // Act & Assert
         await Assert.ThrowsAsync<InvalidOperationException>(async () => await server.ElicitAsync(
             new ElicitRequestParams { Message = "" }, 
             CancellationToken.None));
+
+        await transport.DisposeAsync();
+        await runTask;
     }
 
     [Fact]
@@ -218,23 +233,25 @@ public async Task ElicitAsync_Should_SendRequest()
         // Arrange
         await using var transport = new TestServerTransport();
         await using var server = McpServer.Create(transport, _options, LoggerFactory);
-        SetClientCapabilities(server, new ClientCapabilities
+        var runTask = server.RunAsync(TestContext.Current.CancellationToken);
+        await InitializeServerAsync(transport, new ClientCapabilities
         {
             Elicitation = new()
             {
                 Form = new(),
             },
-        });
-        var runTask = server.RunAsync(TestContext.Current.CancellationToken);
+        }, TestContext.Current.CancellationToken);
 
         // Act
         var result = await server.ElicitAsync(new ElicitRequestParams { Message = "", RequestedSchema = new() }, CancellationToken.None);
 
         // Assert
         Assert.NotNull(result);
         Assert.NotEmpty(transport.SentMessages);
-        Assert.IsType<JsonRpcRequest>(transport.SentMessages[0]);
-        Assert.Equal(RequestMethods.ElicitationCreate, ((JsonRpcRequest)transport.SentMessages[0]).Method);
+        // First message is the initialize response, second is the elicit request
+        Assert.True(transport.SentMessages.Count >= 2, "Expected at least 2 messages (initialize response and elicit request)");
+        var elicitRequest = Assert.IsType<JsonRpcRequest>(transport.SentMessages[1]);
+        Assert.Equal(RequestMethods.ElicitationCreate, elicitRequest.Method);
 
         await transport.DisposeAsync();
         await runTask;
@@ -844,11 +861,33 @@ public async Task Can_SendMessage_Before_RunAsync()
         Assert.Same(logNotification, transport.SentMessages[0]);
     }
 
-    private static void SetClientCapabilities(McpServer server, ClientCapabilities capabilities)
+    private static async Task InitializeServerAsync(TestServerTransport transport, ClientCapabilities capabilities, CancellationToken cancellationToken = default)
     {
-        FieldInfo? field = server.GetType().GetField("_clientCapabilities", BindingFlags.NonPublic | BindingFlags.Instance);
-        Assert.NotNull(field);
-        field.SetValue(server, capabilities);
+        var initializeRequest = new JsonRpcRequest
+        {
+            Id = new RequestId("init-1"),
+            Method = RequestMethods.Initialize,
+            Params = JsonSerializer.SerializeToNode(new InitializeRequestParams
+            {
+                ProtocolVersion = "2024-11-05",
+                Capabilities = capabilities,
+                ClientInfo = new Implementation { Name = "test-client", Version = "1.0.0" }
+            }, McpJsonUtilities.DefaultOptions)
+        };
+
+        var tcs = new TaskCompletionSource<bool>();
+        transport.OnMessageSent = (message) =>
+        {
+            if (message is JsonRpcResponse response && response.Id == initializeRequest.Id)
+            {
+                tcs.TrySetResult(true);
+            }
+        };
+
+        await transport.SendClientMessageAsync(initializeRequest, cancellationToken);
+
+        // Wait for the initialize response to be sent
+        await tcs.Task.WaitAsync(TimeSpan.FromSeconds(5), cancellationToken);
     }
 
     private sealed class TestServerForIChatClient(bool supportsSampling) : McpServer

Original file line number	Diff line number	Diff line change
`@@ -91,4 +91,12 @@ private async Task WriteMessageAsync(JsonRpcMessage message, CancellationToken c`
`91`	`91`	`{`
`92`	`92`	`await _messageChannel.Writer.WriteAsync(message, cancellationToken);`
`93`	`93`	`}`
	`94`	`+`
	`95`	`+ /// <summary>`
	`96`	`+ /// Sends a message from the client to the server (simulating client-to-server communication).`
	`97`	`+ /// </summary>`
	`98`	`+ public async Task SendClientMessageAsync(JsonRpcMessage message, CancellationToken cancellationToken = default)`
	`99`	`+ {`
	`100`	`+ await _messageChannel.Writer.WriteAsync(message, cancellationToken);`
	`101`	`+ }`
`94`	`102`	`}`