Skip to content

Commit c0779fc

Browse files
JamesNKJamesNK
authored
Validate AddCallCredentials are used by client (#1756)
1 parent fedf5ab commit c0779fc

File tree

8 files changed

+192
-6
lines changed

8 files changed

+192
-6
lines changed

‎src/Grpc.Net.Client/GrpcChannel.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,8 @@ private void ResolveCredentials(GrpcChannelOptions channelOptions, out bool isSe
205205
{
206206
throw new InvalidOperationException($"Unable to determine the TLS configuration of the channel from address '{Address}'. " +
207207
$"{nameof(GrpcChannelOptions)}.{nameof(GrpcChannelOptions.Credentials)} must be specified when the address doesn't have a 'http' or 'https' scheme. " +
208-
"To call TLS endpoints, set credentials to 'new SslCredentials()'. " +
209-
"To call non-TLS endpoints, set credentials to 'ChannelCredentials.Insecure'.");
208+
$"To call TLS endpoints, set credentials to '{nameof(ChannelCredentials)}.{nameof(ChannelCredentials.SecureSsl)}'. " +
209+
$"To call non-TLS endpoints, set credentials to '{nameof(ChannelCredentials)}.{nameof(ChannelCredentials.Insecure)}'.");
210210
}
211211
callCredentials = null;
212212
}

‎src/Grpc.Net.Client/Internal/DefaultChannelCredentialsConfigurator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
namespace Grpc.Net.Client.Internal
2222
{
23-
internal class DefaultChannelCredentialsConfigurator : ChannelCredentialsConfiguratorBase
23+
internal sealed class DefaultChannelCredentialsConfigurator : ChannelCredentialsConfiguratorBase
2424
{
2525
public bool? IsSecure { get; private set; }
2626
public List<CallCredentials>? CallCredentials { get; private set; }

‎src/Grpc.Net.ClientFactory/GrpcClientFactoryOptions.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ public class GrpcClientFactoryOptions
5858
/// </summary>
5959
public Func<CallInvoker, object>? Creator { get; set; }
6060

61+
internal bool HasCallCredentials { get; set; }
62+
6163
internal static CallInvoker BuildInterceptors(
6264
CallInvoker callInvoker,
6365
IServiceProvider serviceProvider,

‎src/Grpc.Net.ClientFactory/GrpcClientServiceExtensions.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,6 @@ private static IHttpClientBuilder AddGrpcHttpClient<TClient>(this IServiceCollec
341341
});
342342
});
343343

344-
345344
var builder = new DefaultHttpClientBuilder(services, name);
346345

347346
builder.Services.AddTransient<TClient>(s =>

‎src/Grpc.Net.ClientFactory/GrpcHttpClientBuilderExtensions.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ public static IHttpClientBuilder AddCallCredentials(this IHttpClientBuilder buil
151151

152152
builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
153153
{
154+
options.HasCallCredentials = true;
154155
options.CallOptionsActions.Add((callOptionsContext) =>
155156
{
156157
var credentials = CallCredentials.FromInterceptor((context, metadata) => authInterceptor(context, metadata));
@@ -184,6 +185,7 @@ public static IHttpClientBuilder AddCallCredentials(this IHttpClientBuilder buil
184185

185186
builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
186187
{
188+
options.HasCallCredentials = true;
187189
options.CallOptionsActions.Add((callOptionsContext) =>
188190
{
189191
var credentials = CallCredentials.FromInterceptor((context, metadata) => authInterceptor(context, metadata, callOptionsContext.ServiceProvider));
@@ -217,6 +219,7 @@ public static IHttpClientBuilder AddCallCredentials(this IHttpClientBuilder buil
217219

218220
builder.Services.Configure<GrpcClientFactoryOptions>(builder.Name, options =>
219221
{
222+
options.HasCallCredentials = true;
220223
options.CallOptionsActions.Add((callOptionsContext) =>
221224
{
222225
callOptionsContext.CallOptions = ResolveCallOptionsCredentials(callOptionsContext.CallOptions, credentials);

‎src/Grpc.Net.ClientFactory/Internal/GrpcCallInvokerFactory.cs‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,15 @@ private CallInvoker CreateInvoker(EntryKey key)
106106
throw new InvalidOperationException($@"Could not resolve the address for gRPC client '{name}'. Set an address when registering the client: services.AddGrpcClient<{type.Name}>(o => o.Address = new Uri(""https://localhost:5001""))");
107107
}
108108

109+
if (clientFactoryOptions.HasCallCredentials && !AreCallCredentialsSupported(channelOptions, address))
110+
{
111+
// Throw error to tell dev that call credentials will never be used.
112+
throw new InvalidOperationException(
113+
$"Call credential configured for gRPC client '{name}' requires TLS, and the client isn't configured to use TLS. " +
114+
$"Either configure a TLS address, or use the call credential without TLS by setting {nameof(GrpcChannelOptions)}.{nameof(GrpcChannelOptions.UnsafeUseInsecureChannelCallCredentials)} to true: " +
115+
@"client.AddCallCredentials((context, metadata) => {}).ConfigureChannel(o => o.UnsafeUseInsecureChannelCallCredentials = true)");
116+
}
117+
109118
var channel = GrpcChannel.ForAddress(address, channelOptions);
110119

111120
var httpClientCallInvoker = channel.CreateCallInvoker();
@@ -125,5 +134,63 @@ private CallInvoker CreateInvoker(EntryKey key)
125134
throw;
126135
}
127136
}
137+
138+
private static bool AreCallCredentialsSupported(GrpcChannelOptions channelOptions, Uri address)
139+
{
140+
bool isSecure;
141+
142+
if (address.Scheme == Uri.UriSchemeHttps)
143+
{
144+
isSecure = true;
145+
}
146+
else if (address.Scheme == Uri.UriSchemeHttp)
147+
{
148+
isSecure = false;
149+
}
150+
else
151+
{
152+
// Load balancing means the address won't have a standard scheme, e.g. dns:///
153+
// Use call credentials to figure out whether the channel is secure.
154+
isSecure = HasSecureCredentials(channelOptions.Credentials);
155+
}
156+
157+
return isSecure || channelOptions.UnsafeUseInsecureChannelCallCredentials;
158+
159+
static bool HasSecureCredentials(ChannelCredentials? channelCredentials)
160+
{
161+
if (channelCredentials == null)
162+
{
163+
return false;
164+
}
165+
if (channelCredentials is SslCredentials)
166+
{
167+
return true;
168+
}
169+
170+
var configurator = new ClientFactoryCredentialsConfigurator();
171+
channelCredentials.InternalPopulateConfiguration(configurator, channelCredentials);
172+
173+
return configurator.IsSecure ?? false;
174+
}
175+
}
176+
177+
private sealed class ClientFactoryCredentialsConfigurator : ChannelCredentialsConfiguratorBase
178+
{
179+
public bool? IsSecure { get; private set; }
180+
181+
public override void SetInsecureCredentials(object state)
182+
{
183+
IsSecure = false;
184+
}
185+
186+
public override void SetSslCredentials(object state, string? rootCertificates, KeyCertificatePair? keyCertificatePair, VerifyPeerCallback? verifyPeerCallback)
187+
{
188+
IsSecure = true;
189+
}
190+
191+
public override void SetCompositeCredentials(object state, ChannelCredentials channelCredentials, CallCredentials callCredentials)
192+
{
193+
}
194+
}
128195
}
129196
}

‎test/Grpc.Net.Client.Tests/GrpcChannelTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ public void Resolver_NoChannelCredentials_Error()
475475
// Assert
476476
Assert.AreEqual("Unable to determine the TLS configuration of the channel from address 'test:///localhost'. " +
477477
"GrpcChannelOptions.Credentials must be specified when the address doesn't have a 'http' or 'https' scheme. " +
478-
"To call TLS endpoints, set credentials to 'new SslCredentials()'. " +
478+
"To call TLS endpoints, set credentials to 'ChannelCredentials.SecureSsl'. " +
479479
"To call non-TLS endpoints, set credentials to 'ChannelCredentials.Insecure'.", ex.Message);
480480
}
481481

‎test/Grpc.Net.ClientFactory.Tests/GrpcHttpClientBuilderExtensionsTests.cs‎

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
using System.Net;
2020
using Greet;
2121
using Grpc.Core;
22+
#if NET5_0_OR_GREATER
23+
using Grpc.Net.Client.Balancer;
24+
#endif
2225
using Grpc.Net.ClientFactory;
2326
using Grpc.Tests.Shared;
2427
using Microsoft.Extensions.DependencyInjection;
@@ -593,6 +596,118 @@ public async Task AddCallCredentials_CallCredentials_HeaderAdded()
593596
Assert.AreEqual("auth!", sentRequest!.Headers.GetValues("factory-authorize").Single());
594597
}
595598

599+
[Test]
600+
public void AddCallCredentials_InsecureChannel_Error()
601+
{
602+
// Arrange
603+
var services = new ServiceCollection();
604+
services
605+
.AddGrpcClient<Greeter.GreeterClient>(o =>
606+
{
607+
o.Address = new Uri("http://localhost");
608+
})
609+
.AddCallCredentials(CallCredentials.FromInterceptor((context, metadata) =>
610+
{
611+
metadata.Add("factory-authorize", "auth!");
612+
return Task.CompletedTask;
613+
}))
614+
.ConfigurePrimaryHttpMessageHandler(() => new TestHttpMessageHandler());
615+
616+
var serviceProvider = services.BuildServiceProvider(validateScopes: true);
617+
618+
// Act
619+
var clientFactory = serviceProvider.GetRequiredService<GrpcClientFactory>();
620+
621+
var ex = Assert.Throws<InvalidOperationException>(() => clientFactory.CreateClient<Greeter.GreeterClient>(nameof(Greeter.GreeterClient)))!;
622+
623+
// Assert
624+
Assert.AreEqual("Call credential configured for gRPC client 'GreeterClient' requires TLS, and the client isn't configured to use TLS. " +
625+
"Either configure a TLS address, or use the call credential without TLS by setting GrpcChannelOptions.UnsafeUseInsecureChannelCallCredentials to true: " +
626+
"client.AddCallCredentials((context, metadata) => {}).ConfigureChannel(o => o.UnsafeUseInsecureChannelCallCredentials = true)", ex.Message);
627+
}
628+
629+
#if NET5_0_OR_GREATER
630+
[Test]
631+
public void AddCallCredentials_StaticLoadBalancingSecureChannel_Success()
632+
{
633+
// Arrange
634+
HttpRequestMessage? sentRequest = null;
635+
636+
var services = new ServiceCollection();
637+
services.AddSingleton<ResolverFactory>(new StaticResolverFactory(_ => new[]
638+
{
639+
new BalancerAddress("localhost", 80)
640+
}));
641+
642+
// Can't use ConfigurePrimaryHttpMessageHandler with load balancing because underlying
643+
services
644+
.AddGrpcClient<Greeter.GreeterClient>(o =>
645+
{
646+
o.Address = new Uri("static:///localhost");
647+
})
648+
.ConfigureChannel(o =>
649+
{
650+
o.Credentials = ChannelCredentials.SecureSsl;
651+
})
652+
.AddCallCredentials(CallCredentials.FromInterceptor((context, metadata) =>
653+
{
654+
metadata.Add("factory-authorize", "auth!");
655+
return Task.CompletedTask;
656+
}))
657+
.AddHttpMessageHandler(() => new TestHttpMessageHandler(request =>
658+
{
659+
sentRequest = request;
660+
}));
661+
662+
var serviceProvider = services.BuildServiceProvider(validateScopes: true);
663+
664+
// Act & Assert
665+
var clientFactory = serviceProvider.GetRequiredService<GrpcClientFactory>();
666+
_ = clientFactory.CreateClient<Greeter.GreeterClient>(nameof(Greeter.GreeterClient));
667+
668+
// No call because there isn't an endpoint at localhost:80
669+
}
670+
#endif
671+
672+
[Test]
673+
public async Task AddCallCredentials_InsecureChannel_UnsafeUseInsecureChannelCallCredentials_Success()
674+
{
675+
// Arrange
676+
HttpRequestMessage? sentRequest = null;
677+
678+
var services = new ServiceCollection();
679+
services
680+
.AddGrpcClient<Greeter.GreeterClient>(o =>
681+
{
682+
o.Address = new Uri("http://localhost");
683+
})
684+
.AddCallCredentials(CallCredentials.FromInterceptor((context, metadata) =>
685+
{
686+
metadata.Add("factory-authorize", "auth!");
687+
return Task.CompletedTask;
688+
}))
689+
.ConfigureChannel(o => o.UnsafeUseInsecureChannelCallCredentials = true)
690+
.ConfigurePrimaryHttpMessageHandler(() => new TestHttpMessageHandler(request =>
691+
{
692+
sentRequest = request;
693+
}));
694+
695+
var serviceProvider = services.BuildServiceProvider(validateScopes: true);
696+
697+
// Act
698+
var clientFactory = serviceProvider.GetRequiredService<GrpcClientFactory>();
699+
var client = clientFactory.CreateClient<Greeter.GreeterClient>(nameof(Greeter.GreeterClient));
700+
701+
var response = await client.SayHelloAsync(
702+
new HelloRequest(),
703+
new CallOptions()).ResponseAsync.DefaultTimeout();
704+
705+
// Assert
706+
Assert.NotNull(response);
707+
708+
Assert.AreEqual("auth!", sentRequest!.Headers.GetValues("factory-authorize").Single());
709+
}
710+
596711
[Test]
597712
public async Task AddCallCredentials_PassedInCallCredentials_Combine()
598713
{
@@ -664,7 +779,7 @@ public DerivedGreeterClient(CallInvoker callInvoker) : base(callInvoker)
664779
}
665780
}
666781

667-
private class TestHttpMessageHandler : HttpMessageHandler
782+
private class TestHttpMessageHandler : DelegatingHandler
668783
{
669784
public bool Invoked { get; private set; }
670785

0 commit comments

Comments
 (0)