From ef8b4142a83ef671382ae0b8ea24e215be57a14b Mon Sep 17 00:00:00 2001 From: Mrxx99 Date: Sat, 2 Nov 2024 16:44:16 +0100 Subject: [PATCH 01/14] Add support for auth in URL --- src/NATS.Client.Core/NatsConnection.cs | 58 ++++++++++++++++++- tests/NATS.Client.Core.Tests/ClusterTests.cs | 32 ++++++++-- .../NatsConnectionTest.Auth.cs | 44 +++++++++++--- tests/NATS.Client.TestUtilities/NatsServer.cs | 52 +++++++++++++---- .../NatsServerOpts.cs | 23 ++++++++ 5 files changed, 183 insertions(+), 26 deletions(-) diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index 85862e7b7..834446c0c 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -76,7 +76,7 @@ public NatsConnection() public NatsConnection(NatsOpts opts) { _logger = opts.LoggerFactory.CreateLogger(); - Opts = opts; + Opts = ReadUserInfoFromConnectionString(opts); ConnectionState = NatsConnectionState.Closed; _waitForOpenConnection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _disposedCancellationTokenSource = new CancellationTokenSource(); @@ -289,6 +289,62 @@ internal ValueTask UnsubscribeAsync(int sid) return default; } + + private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) + { + var first = true; + + var natsUris = opts.GetSeedUris(); + var maskedUris = new List(natsUris.Length); + + foreach (var natsUri in natsUris) + { + var uriBuilder = new UriBuilder(natsUri.Uri); + + if (uriBuilder.UserName is { Length: > 0 } username) + { + if (first) + { + first = false; + + if (uriBuilder.Password is { Length: > 0 } password) + { + opts = opts with + { + AuthOpts = opts.AuthOpts with + { + Username = uriBuilder.UserName, + Password = uriBuilder.Password, + }, + }; + + uriBuilder.Password = "***"; // to redact the password from logs + } + else + { + opts = opts with + { + AuthOpts = opts.AuthOpts with + { + Token = uriBuilder.UserName, + }, + }; + + uriBuilder.UserName = "***"; // to redact the token from logs + } + } + + } + + maskedUris.Add(uriBuilder.ToString().TrimEnd('/')); + } + + var combinedUri = string.Join(",", maskedUris); + opts = opts with { Url = combinedUri }; + + return opts; + } + private async ValueTask InitialConnectAsync() { Debug.Assert(ConnectionState == NatsConnectionState.Connecting, "Connection state"); diff --git a/tests/NATS.Client.Core.Tests/ClusterTests.cs b/tests/NATS.Client.Core.Tests/ClusterTests.cs index 0ef8be7fc..05b3f2cec 100644 --- a/tests/NATS.Client.Core.Tests/ClusterTests.cs +++ b/tests/NATS.Client.Core.Tests/ClusterTests.cs @@ -2,23 +2,43 @@ namespace NATS.Client.Core.Tests; public class ClusterTests(ITestOutputHelper output) { - [Fact] - public async Task Seed_urls_on_retry() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Seed_urls_on_retry(bool userAuthInUrl) { await using var cluster1 = new NatsCluster( new NullOutputHelper(), TransportType.Tcp, - (i, b) => b.WithServerName($"c1n{i}")); + (i, b) => + { + b.WithServerName($"c1n{i}"); + if (userAuthInUrl) + { + b.AddServerConfig("resources/configs/auth/password.conf"); + b.WithClientUrlAuthentication("a", "b"); + } + }, + userAuthInUrl); await using var cluster2 = new NatsCluster( new NullOutputHelper(), TransportType.Tcp, - (i, b) => b.WithServerName($"c2n{i}")); + (i, b) => + { + b.WithServerName($"c2n{i}"); + if (userAuthInUrl) + { + b.AddServerConfig("resources/configs/auth/password.conf"); + b.WithClientUrlAuthentication("a", "b"); + } + }, + userAuthInUrl); // Use the first node from each cluster as the seed // so that we can confirm seeds are used on retry - var url1 = cluster1.Server1.ClientUrl; - var url2 = cluster2.Server1.ClientUrl; + var url1 = userAuthInUrl ? cluster1.Server1.ClientUrlWithAuth : cluster1.Server1.ClientUrl; + var url2 = userAuthInUrl ? cluster2.Server1.ClientUrlWithAuth : cluster2.Server1.ClientUrl; await using var nats = new NatsConnection(new NatsOpts { diff --git a/tests/NATS.Client.Core.Tests/NatsConnectionTest.Auth.cs b/tests/NATS.Client.Core.Tests/NatsConnectionTest.Auth.cs index 16aad262b..7e2baa12e 100644 --- a/tests/NATS.Client.Core.Tests/NatsConnectionTest.Auth.cs +++ b/tests/NATS.Client.Core.Tests/NatsConnectionTest.Auth.cs @@ -12,6 +12,15 @@ public static IEnumerable GetAuthConfigs() NatsOpts.Default with { AuthOpts = NatsAuthOpts.Default with { Token = "s3cr3t", }, }), }; + yield return new object[] + { + new Auth( + "TOKEN_IN_CONNECTIONSTRING", + "resources/configs/auth/token.conf", + NatsOpts.Default, + urlAuth: "s3cr3t"), + }; + yield return new object[] { new Auth( @@ -23,6 +32,15 @@ NatsOpts.Default with }), }; + yield return new object[] + { + new Auth( + "USER-PASSWORD_IN_CONNECTIONSTRING", + "resources/configs/auth/password.conf", + NatsOpts.Default, + urlAuth: "a:b"), + }; + yield return new object[] { new Auth( @@ -84,15 +102,22 @@ public async Task UserCredentialAuthTest(Auth auth) var name = auth.Name; var serverConfig = auth.ServerConfig; var clientOpts = auth.ClientOpts; + var useAuthInUrl = !string.IsNullOrEmpty(auth.UrlAuth); _output.WriteLine($"AUTH TEST {name}"); - var serverOpts = new NatsServerOptsBuilder() + var serverOptsBuilder = new NatsServerOptsBuilder() .UseTransport(_transportType) - .AddServerConfig(serverConfig) - .Build(); + .AddServerConfig(serverConfig); + + if (useAuthInUrl) + { + serverOptsBuilder.WithClientUrlAuthentication(auth.UrlAuth); + } - await using var server = NatsServer.Start(_output, serverOpts, clientOpts); + var serverOpts = serverOptsBuilder.Build(); + + await using var server = NatsServer.Start(_output, serverOpts, clientOpts, useAuthInUrl); var subject = Guid.NewGuid().ToString("N"); @@ -104,8 +129,8 @@ public async Task UserCredentialAuthTest(Auth auth) Assert.Contains("Authorization Violation", natsException.GetBaseException().Message); } - await using var subConnection = server.CreateClientConnection(clientOpts); - await using var pubConnection = server.CreateClientConnection(clientOpts); + await using var subConnection = server.CreateClientConnection(clientOpts, useAuthInUrl: useAuthInUrl); + await using var pubConnection = server.CreateClientConnection(clientOpts, useAuthInUrl: useAuthInUrl); var signalComplete1 = new WaitSignal(); var signalComplete2 = new WaitSignal(); @@ -141,7 +166,7 @@ await Retry.Until( await disconnectSignal2; _output.WriteLine("START NEW SERVER"); - await using var newServer = NatsServer.Start(_output, serverOpts, clientOpts); + await using var newServer = NatsServer.Start(_output, serverOpts, clientOpts, useAuthInUrl); await subConnection.ConnectAsync(); // wait open again await pubConnection.ConnectAsync(); // wait open again @@ -162,11 +187,12 @@ await Retry.Until( public class Auth { - public Auth(string name, string serverConfig, NatsOpts clientOpts) + public Auth(string name, string serverConfig, NatsOpts clientOpts, string urlAuth = null) { Name = name; ServerConfig = serverConfig; ClientOpts = clientOpts; + UrlAuth = urlAuth; } public string Name { get; } @@ -175,6 +201,8 @@ public Auth(string name, string serverConfig, NatsOpts clientOpts) public NatsOpts ClientOpts { get; } + public string UrlAuth { get; } + public override string ToString() => Name; } } diff --git a/tests/NATS.Client.TestUtilities/NatsServer.cs b/tests/NATS.Client.TestUtilities/NatsServer.cs index 83cb90001..682a3fa79 100644 --- a/tests/NATS.Client.TestUtilities/NatsServer.cs +++ b/tests/NATS.Client.TestUtilities/NatsServer.cs @@ -85,6 +85,22 @@ private NatsServer(ITestOutputHelper outputHelper, NatsServerOpts opts) _ => throw new ArgumentOutOfRangeException(), }; + public string ClientUrlWithAuth + { + get + { + if (!string.IsNullOrEmpty(Opts.ClientUrlUserName)) + { + var uriBuilder = new UriBuilder(ClientUrl); + uriBuilder.UserName = Opts.ClientUrlUserName; + uriBuilder.Password = Opts.ClientUrlPassword; + return uriBuilder.ToString().TrimEnd('/'); + } + + return ClientUrl; + } + } + public int ConnectionPort { get @@ -134,7 +150,7 @@ public static NatsServer StartWithTrace(ITestOutputHelper outputHelper) public static NatsServer Start(ITestOutputHelper outputHelper, TransportType transportType) => Start(outputHelper, new NatsServerOptsBuilder().UseTransport(transportType).Build()); - public static NatsServer Start(ITestOutputHelper outputHelper, NatsServerOpts opts, NatsOpts? clientOpts = default) + public static NatsServer Start(ITestOutputHelper outputHelper, NatsServerOpts opts, NatsOpts? clientOpts = default, bool useAuthInUrl = false) { NatsServer? server = null; NatsConnection? nats = null; @@ -144,7 +160,7 @@ public static NatsServer Start(ITestOutputHelper outputHelper, NatsServerOpts op { server = new NatsServer(outputHelper, opts); server.StartServerProcess(); - nats = server.CreateClientConnection(clientOpts ?? NatsOpts.Default, reTryCount: 3); + nats = server.CreateClientConnection(clientOpts ?? GetDefaultClientOpts(server), reTryCount: 3, useAuthInUrl: useAuthInUrl); #pragma warning disable CA2012 return server; } @@ -162,6 +178,11 @@ public static NatsServer Start(ITestOutputHelper outputHelper, NatsServerOpts op throw new Exception("Can't start nats-server and connect to it"); } + private static NatsOpts GetDefaultClientOpts(NatsServer server) + { + return NatsOpts.Default/* with { Url = server.ClientUrl }*/; + } + public void StartServerProcess() { _cancellationTokenSource = new CancellationTokenSource(); @@ -335,13 +356,13 @@ public async ValueTask DisposeAsync() return (client, proxy); } - public NatsConnection CreateClientConnection(NatsOpts? options = default, int reTryCount = 10, bool ignoreAuthorizationException = false, bool testLogger = true) + public NatsConnection CreateClientConnection(NatsOpts? options = default, int reTryCount = 10, bool ignoreAuthorizationException = false, bool testLogger = true, bool useAuthInUrl = false) { for (var i = 0; i < reTryCount; i++) { try { - var nats = new NatsConnection(ClientOpts(options ?? NatsOpts.Default, testLogger: testLogger)); + var nats = new NatsConnection(ClientOpts(options ?? GetDefaultClientOpts(this), testLogger: testLogger, useAuthInUrl: useAuthInUrl)); try { @@ -369,14 +390,14 @@ public NatsConnection CreateClientConnection(NatsOpts? options = default, int re throw new Exception("Can't create a connection to nats-server"); } - public NatsConnectionPool CreatePooledClientConnection() => CreatePooledClientConnection(NatsOpts.Default); + public NatsConnectionPool CreatePooledClientConnection() => CreatePooledClientConnection(GetDefaultClientOpts(this)); public NatsConnectionPool CreatePooledClientConnection(NatsOpts opts) { return new NatsConnectionPool(4, ClientOpts(opts)); } - public NatsOpts ClientOpts(NatsOpts opts, bool testLogger = true) + public NatsOpts ClientOpts(NatsOpts opts, bool testLogger = true, bool useAuthInUrl = false) { var natsTlsOpts = Opts.EnableTls ? opts.TlsOpts with @@ -392,7 +413,7 @@ public NatsOpts ClientOpts(NatsOpts opts, bool testLogger = true) { LoggerFactory = testLogger ? _loggerFactory : opts.LoggerFactory, TlsOpts = natsTlsOpts, - Url = ClientUrl, + Url = useAuthInUrl ? ClientUrlWithAuth : ClientUrl, }; } @@ -431,6 +452,15 @@ private static (string configFileName, string config, string cmd) GetCmd(NatsSer var cmd = $"{NatsServerPath} -c {configFileName}"; + //var cmdBuilder = new StringBuilder($"{NatsServerPath} -c {configFileName}"); + + //if (!string.IsNullOrEmpty(opts.UserName) && !string.IsNullOrEmpty(opts.Password)) + //{ + // cmdBuilder.Append($" --user {opts.UserName} --pass {opts.Password}"); + //} + + //var cmd = cmdBuilder.ToString(); + return (configFileName, config, cmd); } @@ -464,7 +494,7 @@ public class NatsCluster : IAsyncDisposable { private readonly ITestOutputHelper _outputHelper; - public NatsCluster(ITestOutputHelper outputHelper, TransportType transportType, Action? configure = default) + public NatsCluster(ITestOutputHelper outputHelper, TransportType transportType, Action? configure = default, bool useAuthInUrl = false) { _outputHelper = outputHelper; @@ -516,13 +546,13 @@ public NatsCluster(ITestOutputHelper outputHelper, TransportType transportType, } _outputHelper.WriteLine($"Starting server 1..."); - Server1 = NatsServer.Start(outputHelper, opts1); + Server1 = NatsServer.Start(outputHelper, opts1, useAuthInUrl: useAuthInUrl); _outputHelper.WriteLine($"Starting server 2..."); - Server2 = NatsServer.Start(outputHelper, opts2); + Server2 = NatsServer.Start(outputHelper, opts2, useAuthInUrl: useAuthInUrl); _outputHelper.WriteLine($"Starting server 3..."); - Server3 = NatsServer.Start(outputHelper, opts3); + Server3 = NatsServer.Start(outputHelper, opts3, useAuthInUrl: useAuthInUrl); } public NatsServer Server1 { get; } diff --git a/tests/NATS.Client.TestUtilities/NatsServerOpts.cs b/tests/NATS.Client.TestUtilities/NatsServerOpts.cs index 43c69ba26..2f2b6fc3a 100644 --- a/tests/NATS.Client.TestUtilities/NatsServerOpts.cs +++ b/tests/NATS.Client.TestUtilities/NatsServerOpts.cs @@ -31,6 +31,8 @@ public sealed class NatsServerOptsBuilder private bool _serverDisposeReturnsPorts; private bool _enableClustering; private bool _trace; + private string? _clientUrlUserName; + private string? _clientUrlPassword; public NatsServerOpts Build() => new() { @@ -40,6 +42,8 @@ public sealed class NatsServerOptsBuilder TlsVerify = _tlsVerify, EnableJetStream = _enableJetStream, ServerName = _serverName, + ClientUrlUserName = _clientUrlUserName, + ClientUrlPassword = _clientUrlPassword, TlsServerCertFile = _tlsServerCertFile, TlsServerKeyFile = _tlsServerKeyFile, TlsClientCertFile = _tlsClientCertFile, @@ -110,6 +114,21 @@ public NatsServerOptsBuilder WithServerName(string serverName) return this; } + public NatsServerOptsBuilder WithClientUrlAuthentication(string userName, string password) + { + _clientUrlUserName = userName; + _clientUrlPassword = password; + return this; + } + + public NatsServerOptsBuilder WithClientUrlAuthentication(string authInfo) + { + var infoParts = authInfo.Split(':'); + _clientUrlUserName = infoParts.FirstOrDefault(); + _clientUrlPassword = infoParts.ElementAtOrDefault(1); + return this; + } + public NatsServerOptsBuilder UseJetStream() { _enableJetStream = true; @@ -176,6 +195,10 @@ public NatsServerOpts() public bool ServerDisposeReturnsPorts { get; init; } = true; + public string? ClientUrlUserName { get; set; } + + public string? ClientUrlPassword { get; set; } + public string? TlsClientCertFile { get; init; } public string? TlsClientKeyFile { get; init; } From 3ec4b8aa2b6dec61bd395d0f6be634c57c21f761 Mon Sep 17 00:00:00 2001 From: Mrxx99 Date: Sat, 2 Nov 2024 17:08:25 +0100 Subject: [PATCH 02/14] formatting --- src/NATS.Client.Core/NatsConnection.cs | 4 +--- tests/NATS.Client.TestUtilities/NatsServer.cs | 9 --------- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index 834446c0c..bcae5b6c8 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -289,7 +289,6 @@ internal ValueTask UnsubscribeAsync(int sid) return default; } - private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) { var first = true; @@ -326,14 +325,13 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) { AuthOpts = opts.AuthOpts with { - Token = uriBuilder.UserName, + Token = uriBuilder.UserName, }, }; uriBuilder.UserName = "***"; // to redact the token from logs } } - } maskedUris.Add(uriBuilder.ToString().TrimEnd('/')); diff --git a/tests/NATS.Client.TestUtilities/NatsServer.cs b/tests/NATS.Client.TestUtilities/NatsServer.cs index 682a3fa79..7709d0079 100644 --- a/tests/NATS.Client.TestUtilities/NatsServer.cs +++ b/tests/NATS.Client.TestUtilities/NatsServer.cs @@ -452,15 +452,6 @@ private static (string configFileName, string config, string cmd) GetCmd(NatsSer var cmd = $"{NatsServerPath} -c {configFileName}"; - //var cmdBuilder = new StringBuilder($"{NatsServerPath} -c {configFileName}"); - - //if (!string.IsNullOrEmpty(opts.UserName) && !string.IsNullOrEmpty(opts.Password)) - //{ - // cmdBuilder.Append($" --user {opts.UserName} --pass {opts.Password}"); - //} - - //var cmd = cmdBuilder.ToString(); - return (configFileName, config, cmd); } From 87a4f86c3074bc44d971365a8d47792c85422226 Mon Sep 17 00:00:00 2001 From: Mrxx99 Date: Sat, 2 Nov 2024 17:43:51 +0100 Subject: [PATCH 03/14] undo not needed change --- tests/NATS.Client.TestUtilities/NatsServer.cs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/NATS.Client.TestUtilities/NatsServer.cs b/tests/NATS.Client.TestUtilities/NatsServer.cs index 7709d0079..3de63b5d0 100644 --- a/tests/NATS.Client.TestUtilities/NatsServer.cs +++ b/tests/NATS.Client.TestUtilities/NatsServer.cs @@ -160,7 +160,7 @@ public static NatsServer Start(ITestOutputHelper outputHelper, NatsServerOpts op { server = new NatsServer(outputHelper, opts); server.StartServerProcess(); - nats = server.CreateClientConnection(clientOpts ?? GetDefaultClientOpts(server), reTryCount: 3, useAuthInUrl: useAuthInUrl); + nats = server.CreateClientConnection(clientOpts ?? NatsOpts.Default, reTryCount: 3, useAuthInUrl: useAuthInUrl); #pragma warning disable CA2012 return server; } @@ -178,11 +178,6 @@ public static NatsServer Start(ITestOutputHelper outputHelper, NatsServerOpts op throw new Exception("Can't start nats-server and connect to it"); } - private static NatsOpts GetDefaultClientOpts(NatsServer server) - { - return NatsOpts.Default/* with { Url = server.ClientUrl }*/; - } - public void StartServerProcess() { _cancellationTokenSource = new CancellationTokenSource(); @@ -362,7 +357,7 @@ public NatsConnection CreateClientConnection(NatsOpts? options = default, int re { try { - var nats = new NatsConnection(ClientOpts(options ?? GetDefaultClientOpts(this), testLogger: testLogger, useAuthInUrl: useAuthInUrl)); + var nats = new NatsConnection(ClientOpts(options ?? NatsOpts.Default, testLogger: testLogger, useAuthInUrl: useAuthInUrl)); try { @@ -390,7 +385,7 @@ public NatsConnection CreateClientConnection(NatsOpts? options = default, int re throw new Exception("Can't create a connection to nats-server"); } - public NatsConnectionPool CreatePooledClientConnection() => CreatePooledClientConnection(GetDefaultClientOpts(this)); + public NatsConnectionPool CreatePooledClientConnection() => CreatePooledClientConnection(NatsOpts.Default); public NatsConnectionPool CreatePooledClientConnection(NatsOpts opts) { From 4e0f9481aa70f7ebca635787aa7bc8162817715a Mon Sep 17 00:00:00 2001 From: Mrxx99 Date: Sat, 2 Nov 2024 17:47:15 +0100 Subject: [PATCH 04/14] address review --- src/NATS.Client.Core/NatsConnection.cs | 24 ++++++++++++++---------- src/NATS.Client.Core/NatsOpts.cs | 4 ++-- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index bcae5b6c8..d46085079 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -1,6 +1,5 @@ using System.Buffers; using System.Diagnostics; -using System.Runtime.CompilerServices; using System.Threading.Channels; using Microsoft.Extensions.Logging; using NATS.Client.Core.Commands; @@ -302,12 +301,12 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) if (uriBuilder.UserName is { Length: > 0 } username) { - if (first) + if (uriBuilder.Password is { Length: > 0 } password) { - first = false; - - if (uriBuilder.Password is { Length: > 0 } password) + if (first) { + first = false; + opts = opts with { AuthOpts = opts.AuthOpts with @@ -316,11 +315,16 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) Password = uriBuilder.Password, }, }; - - uriBuilder.Password = "***"; // to redact the password from logs } - else + + uriBuilder.Password = "***"; // to redact the password from logs + } + else + { + if (first) { + first = false; + opts = opts with { AuthOpts = opts.AuthOpts with @@ -328,9 +332,9 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) Token = uriBuilder.UserName, }, }; - - uriBuilder.UserName = "***"; // to redact the token from logs } + + uriBuilder.UserName = "***"; // to redact the token from logs } } diff --git a/src/NATS.Client.Core/NatsOpts.cs b/src/NATS.Client.Core/NatsOpts.cs index 2c2245b2a..7353b3e84 100644 --- a/src/NATS.Client.Core/NatsOpts.cs +++ b/src/NATS.Client.Core/NatsOpts.cs @@ -117,10 +117,10 @@ public sealed record NatsOpts /// public BoundedChannelFullMode SubPendingChannelFullMode { get; init; } = BoundedChannelFullMode.DropNewest; - internal NatsUri[] GetSeedUris() + internal NatsUri[] GetSeedUris(bool suppressRandomization = false) { var urls = Url.Split(','); - return NoRandomize + return NoRandomize || suppressRandomization ? urls.Select(x => new NatsUri(x, true)).Distinct().ToArray() : urls.Select(x => new NatsUri(x, true)).OrderBy(_ => Guid.NewGuid()).Distinct().ToArray(); } From 13d43f20122f4b2f6526ee486d2b76db7b49dd7e Mon Sep 17 00:00:00 2001 From: Mrxx99 Date: Sat, 2 Nov 2024 17:48:22 +0100 Subject: [PATCH 05/14] use first uri to get auth information --- src/NATS.Client.Core/NatsConnection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index d46085079..129caf192 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -292,7 +292,7 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) { var first = true; - var natsUris = opts.GetSeedUris(); + var natsUris = opts.GetSeedUris(suppressRandomization: true); var maskedUris = new List(natsUris.Length); foreach (var natsUri in natsUris) From 894faf3dfec370c81bdd1984d24a930f55f035a1 Mon Sep 17 00:00:00 2001 From: Mrxx99 Date: Sat, 2 Nov 2024 20:30:02 +0100 Subject: [PATCH 06/14] set username on multiple uris to the actual used one + better redaction --- src/NATS.Client.Core/NatsConnection.cs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index 129caf192..94cffb24f 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -295,6 +295,9 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) var natsUris = opts.GetSeedUris(suppressRandomization: true); var maskedUris = new List(natsUris.Length); + var usesPasswordInUrl = false; + var usesTokenInUrl = false; + foreach (var natsUri in natsUris) { var uriBuilder = new UriBuilder(natsUri.Uri); @@ -306,6 +309,7 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) if (first) { first = false; + usesPasswordInUrl = true; opts = opts with { @@ -324,6 +328,7 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) if (first) { first = false; + usesTokenInUrl = true; opts = opts with { @@ -338,6 +343,16 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) } } + if (usesPasswordInUrl) + { + uriBuilder.UserName = opts.AuthOpts.Username; // show actual used user name in logs + } + else if (usesTokenInUrl) + { + uriBuilder.UserName = "***"; // to redact the token from logs + uriBuilder.Password = null; // when token is used remove password + } + maskedUris.Add(uriBuilder.ToString().TrimEnd('/')); } From e47ed27df37b63921c30e885bbcd789cc7c5180a Mon Sep 17 00:00:00 2001 From: Ziya Suzen Date: Sat, 2 Nov 2024 21:41:13 +0000 Subject: [PATCH 07/14] Add tests and docs --- src/NATS.Client.Core/NatsConnection.cs | 14 ++--- src/NATS.Client.Core/NatsOpts.cs | 22 ++++++++ .../NatsConnectionTest.Auth.cs | 6 +-- .../OptsUrlTests.cs | 53 +++++++++++++++++++ 4 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index 94cffb24f..23287d7fa 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -302,9 +302,9 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) { var uriBuilder = new UriBuilder(natsUri.Uri); - if (uriBuilder.UserName is { Length: > 0 } username) + if (uriBuilder.UserName is { Length: > 0 }) { - if (uriBuilder.Password is { Length: > 0 } password) + if (uriBuilder.Password is { Length: > 0 }) { if (first) { @@ -317,11 +317,10 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) { Username = uriBuilder.UserName, Password = uriBuilder.Password, + Token = null, // override token in case it was set }, }; } - - uriBuilder.Password = "***"; // to redact the password from logs } else { @@ -335,17 +334,18 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) AuthOpts = opts.AuthOpts with { Token = uriBuilder.UserName, + Username = null, // override user-password in case it was set + Password = null, }, }; } - - uriBuilder.UserName = "***"; // to redact the token from logs } } if (usesPasswordInUrl) { - uriBuilder.UserName = opts.AuthOpts.Username; // show actual used user name in logs + uriBuilder.UserName = opts.AuthOpts.Username; // show actual used username in logs + uriBuilder.Password = "***"; // to redact the password from logs } else if (usesTokenInUrl) { diff --git a/src/NATS.Client.Core/NatsOpts.cs b/src/NATS.Client.Core/NatsOpts.cs index 7353b3e84..b2af398bd 100644 --- a/src/NATS.Client.Core/NatsOpts.cs +++ b/src/NATS.Client.Core/NatsOpts.cs @@ -14,6 +14,28 @@ public sealed record NatsOpts { public static readonly NatsOpts Default = new(); + /// + /// NATS server URL to connect to. (default: nats://localhost:4222) + /// + /// + /// + /// You can set more than one server as seed servers in a comma-separated list. + /// The client will randomly select a server from the list to connect to unless + /// (which is false by default) is set to true. + /// + /// + /// User-password or token authentication can be set in the URL. + /// For example, nats://derek:s3cr3t@localhost:4222 or nats://token@localhost:4222. + /// You can also set the username and password or token separately using ; + /// however, if both are set, the URL will take precedence. + /// You should URL-encode the username and password or token if they contain special characters. + /// + /// + /// If multiple servers are specified and user-password or token authentication is used in the URL, + /// only the credentials in the first server URL will be used; credentials in the remaining server + /// URLs will be ignored. + /// + /// public string Url { get; init; } = "nats://localhost:4222"; public string Name { get; init; } = "NATS .NET Client"; diff --git a/tests/NATS.Client.Core.Tests/NatsConnectionTest.Auth.cs b/tests/NATS.Client.Core.Tests/NatsConnectionTest.Auth.cs index 7e2baa12e..11c6ac559 100644 --- a/tests/NATS.Client.Core.Tests/NatsConnectionTest.Auth.cs +++ b/tests/NATS.Client.Core.Tests/NatsConnectionTest.Auth.cs @@ -112,7 +112,7 @@ public async Task UserCredentialAuthTest(Auth auth) if (useAuthInUrl) { - serverOptsBuilder.WithClientUrlAuthentication(auth.UrlAuth); + serverOptsBuilder.WithClientUrlAuthentication(auth.UrlAuth!); } var serverOpts = serverOptsBuilder.Build(); @@ -187,7 +187,7 @@ await Retry.Until( public class Auth { - public Auth(string name, string serverConfig, NatsOpts clientOpts, string urlAuth = null) + public Auth(string name, string serverConfig, NatsOpts clientOpts, string? urlAuth = null) { Name = name; ServerConfig = serverConfig; @@ -201,7 +201,7 @@ public Auth(string name, string serverConfig, NatsOpts clientOpts, string urlAut public NatsOpts ClientOpts { get; } - public string UrlAuth { get; } + public string? UrlAuth { get; } public override string ToString() => Name; } diff --git a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs new file mode 100644 index 000000000..69bf68926 --- /dev/null +++ b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs @@ -0,0 +1,53 @@ +namespace NATS.Client.Core.Tests; + +public class OptsUrlTests +{ + [Fact] + public void Default_URL() + { + var opts = new NatsConnection().Opts; + Assert.Equal("nats://localhost:4222", opts.Url); + } + + [Theory] + [InlineData("host1", "nats://host1:4222", null, null, null)] + [InlineData("host1:1234", "nats://host1:1234", null, null, null)] + [InlineData("tls://host1", "tls://host1:4222", null, null, null)] + [InlineData("u:p@host1:1234", "nats://u:***@host1:1234", "u", "p", null)] + [InlineData("t@host1:1234", "nats://***@host1:1234", null, null, "t")] + [InlineData("host1,host2", "nats://host1:4222,nats://host2:4222", null, null, null)] + [InlineData("u:p@host1,host2", "nats://u:***@host1:4222,nats://u:***@host2:4222", "u", "p", null)] + [InlineData("u:p@host1,x@host2", "nats://u:***@host1:4222,nats://u:***@host2:4222", "u", "p", null)] + [InlineData("t@host1,x:x@host2", "nats://***@host1:4222,nats://***@host2:4222", null, null, "t")] + [InlineData("u:p@host1,host2,host3", "nats://u:***@host1:4222,nats://u:***@host2:4222,nats://u:***@host3:4222", "u", "p", null)] + [InlineData("t@host1,@host2,host3", "nats://***@host1:4222,nats://***@host2:4222,nats://***@host3:4222", null, null, "t")] + public void URL_parts(string url, string expected, string? user, string? pass, string? token) + { + var opts = new NatsConnection(new NatsOpts { Url = url }).Opts; + Assert.Equal(expected, opts.Url); + Assert.Equal(user, opts.AuthOpts.Username); + Assert.Equal(pass, opts.AuthOpts.Password); + Assert.Equal(token, opts.AuthOpts.Token); + } + + [Theory] + [InlineData("u:p@host1:1234", "nats://u:***@host1:1234", "u", "p", null)] + [InlineData("t@host1:1234", "nats://***@host1:1234", null, null, "t")] + public void URL_should_override_auth_options(string url, string expected, string? user, string? pass, string? token) + { + var opts = new NatsConnection(new NatsOpts + { + Url = url, + AuthOpts = new NatsAuthOpts + { + Username = "should override username", + Password = "should override password", + Token = "should override token", + }, + }).Opts; + Assert.Equal(expected, opts.Url); + Assert.Equal(user, opts.AuthOpts.Username); + Assert.Equal(pass, opts.AuthOpts.Password); + Assert.Equal(token, opts.AuthOpts.Token); + } +} From 4780cf861970d2c27d5c2ac092616120b010b325 Mon Sep 17 00:00:00 2001 From: Ziya Suzen Date: Sat, 2 Nov 2024 21:51:58 +0000 Subject: [PATCH 08/14] Add docs to the client --- src/NATS.Client.Simplified/NatsClient.cs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/NATS.Client.Simplified/NatsClient.cs b/src/NATS.Client.Simplified/NatsClient.cs index 87de0d6c0..5cc9dc383 100644 --- a/src/NATS.Client.Simplified/NatsClient.cs +++ b/src/NATS.Client.Simplified/NatsClient.cs @@ -11,9 +11,25 @@ public class NatsClient : INatsClient /// /// Initializes a new instance of the class. /// - /// NATS server URL - /// Client name - /// Credentials filepath + /// NATS server URL to connect to. (default: nats://localhost:4222) + /// Client name. (default: NATS .NET Client) + /// Credentials filepath. + /// + /// + /// You can set more than one server as seed servers in a comma-separated list in the . + /// The client will randomly select a server from the list to connect. + /// + /// + /// User-password or token authentication can be set in the . + /// For example, nats://derek:s3cr3t@localhost:4222 or nats://token@localhost:4222. + /// You should URL-encode the username and password or token if they contain special characters. + /// + /// + /// If multiple servers are specified and user-password or token authentication is used in the , + /// only the credentials in the first server URL will be used; credentials in the remaining server + /// URLs will be ignored. + /// + /// public NatsClient( string url = "nats://localhost:4222", string name = "NATS .NET Client", From 27b33ac2560dbdaeed9ea5809fc5a8c15bf23c68 Mon Sep 17 00:00:00 2001 From: Ziya Suzen Date: Sat, 2 Nov 2024 21:57:12 +0000 Subject: [PATCH 09/14] Format --- tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs index 69bf68926..a19591b20 100644 --- a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs +++ b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs @@ -1,4 +1,4 @@ -namespace NATS.Client.Core.Tests; +namespace NATS.Client.Core.Tests; public class OptsUrlTests { From 50240b3a7af97a6a49519992bdaaa1fba905a68f Mon Sep 17 00:00:00 2001 From: Ziya Suzen Date: Sun, 3 Nov 2024 06:56:42 +0000 Subject: [PATCH 10/14] Implement URL encoding for user credentials --- src/NATS.Client.Core/NatsConnection.cs | 8 ++-- .../OptsUrlTests.cs | 40 +++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index 23287d7fa..fa1b673a6 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -315,8 +315,8 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) { AuthOpts = opts.AuthOpts with { - Username = uriBuilder.UserName, - Password = uriBuilder.Password, + Username = Uri.UnescapeDataString(uriBuilder.UserName), + Password = Uri.UnescapeDataString(uriBuilder.Password), Token = null, // override token in case it was set }, }; @@ -333,7 +333,7 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) { AuthOpts = opts.AuthOpts with { - Token = uriBuilder.UserName, + Token = Uri.UnescapeDataString(uriBuilder.UserName), Username = null, // override user-password in case it was set Password = null, }, @@ -344,7 +344,7 @@ private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) if (usesPasswordInUrl) { - uriBuilder.UserName = opts.AuthOpts.Username; // show actual used username in logs + uriBuilder.UserName = Uri.EscapeDataString(opts.AuthOpts.Username!); // show actual used username in logs uriBuilder.Password = "***"; // to redact the password from logs } else if (usesTokenInUrl) diff --git a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs index a19591b20..9b38e4341 100644 --- a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs +++ b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs @@ -50,4 +50,44 @@ public void URL_should_override_auth_options(string url, string expected, string Assert.Equal(pass, opts.AuthOpts.Password); Assert.Equal(token, opts.AuthOpts.Token); } + + [Fact] + public void URL_escape_user_password() + { + var opts = new NatsConnection(new NatsOpts { Url = "nats://u%2C:p%2C@host1,host2" }).Opts; + Assert.Equal("nats://u%2C:***@host1:4222,nats://u%2C:***@host2:4222", opts.Url); + Assert.Equal("u,", opts.AuthOpts.Username); + Assert.Equal("p,", opts.AuthOpts.Password); + Assert.Null(opts.AuthOpts.Token); + + var uris = opts.GetSeedUris(true); + uris[0].Uri.Scheme.Should().Be("nats"); + uris[0].Uri.Host.Should().Be("host1"); + uris[0].Uri.Port.Should().Be(4222); + uris[0].Uri.UserInfo.Should().Be("u%2C:***"); + uris[1].Uri.Scheme.Should().Be("nats"); + uris[1].Uri.Host.Should().Be("host2"); + uris[1].Uri.Port.Should().Be(4222); + uris[1].Uri.UserInfo.Should().Be("u%2C:***"); + } + + [Fact] + public void URL_escape_token() + { + var opts = new NatsConnection(new NatsOpts { Url = "nats://t%2C@host1,nats://t%2C@host2" }).Opts; + Assert.Equal("nats://***@host1:4222,nats://***@host2:4222", opts.Url); + Assert.Null(opts.AuthOpts.Username); + Assert.Null(opts.AuthOpts.Password); + Assert.Equal("t,", opts.AuthOpts.Token); + + var uris = opts.GetSeedUris(true); + uris[0].Uri.Scheme.Should().Be("nats"); + uris[0].Uri.Host.Should().Be("host1"); + uris[0].Uri.Port.Should().Be(4222); + uris[0].Uri.UserInfo.Should().Be("***"); + uris[1].Uri.Scheme.Should().Be("nats"); + uris[1].Uri.Host.Should().Be("host2"); + uris[1].Uri.Port.Should().Be(4222); + uris[1].Uri.UserInfo.Should().Be("***"); + } } From ee3adf77adfc14fd4ff14e710727205b64925374 Mon Sep 17 00:00:00 2001 From: Ziya Suzen Date: Sun, 3 Nov 2024 12:16:30 +0000 Subject: [PATCH 11/14] Refactor authentication precedence and URL redaction handling Redact passwords and tokens at the `NatsUri` level to preserve the original URIs, especially when used in WebSocket connections. This change aims to maintain backward compatibility and prevent disruptions in WebSocket connections that may rely on tokens or user-password pairs for proxy-level authentication. --- src/NATS.Client.Core/Internal/NatsUri.cs | 18 +++- src/NATS.Client.Core/NatsConnection.cs | 87 ++++++------------- src/NATS.Client.Core/NatsOpts.cs | 2 +- .../OptsUrlTests.cs | 68 ++++++++++----- 4 files changed, 93 insertions(+), 82 deletions(-) diff --git a/src/NATS.Client.Core/Internal/NatsUri.cs b/src/NATS.Client.Core/Internal/NatsUri.cs index 64d3ea4c2..979d59604 100644 --- a/src/NATS.Client.Core/Internal/NatsUri.cs +++ b/src/NATS.Client.Core/Internal/NatsUri.cs @@ -4,6 +4,8 @@ internal sealed class NatsUri : IEquatable { public const string DefaultScheme = "nats"; + private readonly Uri _redactedUri; + public NatsUri(string urlString, bool isSeed, string defaultScheme = DefaultScheme) { IsSeed = isSeed; @@ -38,6 +40,20 @@ public NatsUri(string urlString, bool isSeed, string defaultScheme = DefaultSche } Uri = uriBuilder.Uri; + + if (uriBuilder.UserName is { Length: > 0 }) + { + if (uriBuilder.Password is { Length: > 0 }) + { + uriBuilder.Password = "***"; + } + else + { + uriBuilder.UserName = "***"; + } + } + + _redactedUri = uriBuilder.Uri; } public Uri Uri { get; } @@ -65,7 +81,7 @@ public NatsUri CloneWith(string host, int? port = default) public override string ToString() { - return IsWebSocket && Uri.AbsolutePath != "/" ? Uri.ToString() : Uri.ToString().Trim('/'); + return IsWebSocket && Uri.AbsolutePath != "/" ? _redactedUri.ToString() : _redactedUri.ToString().Trim('/'); } public override int GetHashCode() => Uri.GetHashCode(); diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index fa1b673a6..a596fa32e 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -290,75 +290,42 @@ internal ValueTask UnsubscribeAsync(int sid) private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) { - var first = true; - - var natsUris = opts.GetSeedUris(suppressRandomization: true); - var maskedUris = new List(natsUris.Length); + // Setting credentials in options takes precedence over URL credentials + if (opts.AuthOpts.Username is { Length: > 0 } || opts.AuthOpts.Password is { Length: > 0 } || opts.AuthOpts.Token is { Length: > 0 }) + { + return opts; + } - var usesPasswordInUrl = false; - var usesTokenInUrl = false; + var natsUri = opts.GetSeedUris(suppressRandomization: true).First(); + var uriBuilder = new UriBuilder(natsUri.Uri); - foreach (var natsUri in natsUris) + if (uriBuilder.UserName is not { Length: > 0 }) { - var uriBuilder = new UriBuilder(natsUri.Uri); + return opts; + } - if (uriBuilder.UserName is { Length: > 0 }) + if (uriBuilder.Password is { Length: > 0 }) + { + opts = opts with { - if (uriBuilder.Password is { Length: > 0 }) + AuthOpts = opts.AuthOpts with { - if (first) - { - first = false; - usesPasswordInUrl = true; - - opts = opts with - { - AuthOpts = opts.AuthOpts with - { - Username = Uri.UnescapeDataString(uriBuilder.UserName), - Password = Uri.UnescapeDataString(uriBuilder.Password), - Token = null, // override token in case it was set - }, - }; - } - } - else - { - if (first) - { - first = false; - usesTokenInUrl = true; - - opts = opts with - { - AuthOpts = opts.AuthOpts with - { - Token = Uri.UnescapeDataString(uriBuilder.UserName), - Username = null, // override user-password in case it was set - Password = null, - }, - }; - } - } - } - - if (usesPasswordInUrl) - { - uriBuilder.UserName = Uri.EscapeDataString(opts.AuthOpts.Username!); // show actual used username in logs - uriBuilder.Password = "***"; // to redact the password from logs - } - else if (usesTokenInUrl) + Username = Uri.UnescapeDataString(uriBuilder.UserName), + Password = Uri.UnescapeDataString(uriBuilder.Password), + }, + }; + } + else + { + opts = opts with { - uriBuilder.UserName = "***"; // to redact the token from logs - uriBuilder.Password = null; // when token is used remove password - } - - maskedUris.Add(uriBuilder.ToString().TrimEnd('/')); + AuthOpts = opts.AuthOpts with + { + Token = Uri.UnescapeDataString(uriBuilder.UserName), + }, + }; } - var combinedUri = string.Join(",", maskedUris); - opts = opts with { Url = combinedUri }; - return opts; } diff --git a/src/NATS.Client.Core/NatsOpts.cs b/src/NATS.Client.Core/NatsOpts.cs index b2af398bd..f4213ccc4 100644 --- a/src/NATS.Client.Core/NatsOpts.cs +++ b/src/NATS.Client.Core/NatsOpts.cs @@ -27,7 +27,7 @@ public sealed record NatsOpts /// User-password or token authentication can be set in the URL. /// For example, nats://derek:s3cr3t@localhost:4222 or nats://token@localhost:4222. /// You can also set the username and password or token separately using ; - /// however, if both are set, the URL will take precedence. + /// however, if both are set, the will take precedence. /// You should URL-encode the username and password or token if they contain special characters. /// /// diff --git a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs index 9b38e4341..397d3e720 100644 --- a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs +++ b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs @@ -9,6 +9,22 @@ public void Default_URL() Assert.Equal("nats://localhost:4222", opts.Url); } + [Fact] + public void Redact_URL_user_password() + { + var natsUri = new NatsUri("u:p@host", true); + Assert.Equal("nats://u:***@host:4222", natsUri.ToString()); + Assert.Equal("u:p", natsUri.Uri.UserInfo); + } + + [Fact] + public void Redact_URL_token() + { + var natsUri = new NatsUri("t@host", true); + Assert.Equal("nats://***@host:4222", natsUri.ToString()); + Assert.Equal("t", natsUri.Uri.UserInfo); + } + [Theory] [InlineData("host1", "nats://host1:4222", null, null, null)] [InlineData("host1:1234", "nats://host1:1234", null, null, null)] @@ -16,15 +32,15 @@ public void Default_URL() [InlineData("u:p@host1:1234", "nats://u:***@host1:1234", "u", "p", null)] [InlineData("t@host1:1234", "nats://***@host1:1234", null, null, "t")] [InlineData("host1,host2", "nats://host1:4222,nats://host2:4222", null, null, null)] - [InlineData("u:p@host1,host2", "nats://u:***@host1:4222,nats://u:***@host2:4222", "u", "p", null)] - [InlineData("u:p@host1,x@host2", "nats://u:***@host1:4222,nats://u:***@host2:4222", "u", "p", null)] - [InlineData("t@host1,x:x@host2", "nats://***@host1:4222,nats://***@host2:4222", null, null, "t")] - [InlineData("u:p@host1,host2,host3", "nats://u:***@host1:4222,nats://u:***@host2:4222,nats://u:***@host3:4222", "u", "p", null)] - [InlineData("t@host1,@host2,host3", "nats://***@host1:4222,nats://***@host2:4222,nats://***@host3:4222", null, null, "t")] + [InlineData("u:p@host1,host2", "nats://u:***@host1:4222,nats://host2:4222", "u", "p", null)] + [InlineData("u:p@host1,x@host2", "nats://u:***@host1:4222,nats://***@host2:4222", "u", "p", null)] + [InlineData("t@host1,x:x@host2", "nats://***@host1:4222,nats://x:***@host2:4222", null, null, "t")] + [InlineData("u:p@host1,host2,host3", "nats://u:***@host1:4222,nats://host2:4222,nats://host3:4222", "u", "p", null)] + [InlineData("t@host1,@host2,host3", "nats://***@host1:4222,nats://host2:4222,nats://host3:4222", null, null, "t")] public void URL_parts(string url, string expected, string? user, string? pass, string? token) { var opts = new NatsConnection(new NatsOpts { Url = url }).Opts; - Assert.Equal(expected, opts.Url); + Assert.Equal(expected, GetUrisAsRedactedString(opts)); Assert.Equal(user, opts.AuthOpts.Username); Assert.Equal(pass, opts.AuthOpts.Password); Assert.Equal(token, opts.AuthOpts.Token); @@ -33,29 +49,29 @@ public void URL_parts(string url, string expected, string? user, string? pass, s [Theory] [InlineData("u:p@host1:1234", "nats://u:***@host1:1234", "u", "p", null)] [InlineData("t@host1:1234", "nats://***@host1:1234", null, null, "t")] - public void URL_should_override_auth_options(string url, string expected, string? user, string? pass, string? token) + public void URL_should_not_override_auth_options(string url, string expected, string? user, string? pass, string? token) { var opts = new NatsConnection(new NatsOpts { Url = url, AuthOpts = new NatsAuthOpts { - Username = "should override username", - Password = "should override password", - Token = "should override token", + Username = "shouldn't override username", + Password = "shouldn't override password", + Token = "shouldn't override token", }, }).Opts; - Assert.Equal(expected, opts.Url); - Assert.Equal(user, opts.AuthOpts.Username); - Assert.Equal(pass, opts.AuthOpts.Password); - Assert.Equal(token, opts.AuthOpts.Token); + Assert.Equal(expected, GetUrisAsRedactedString(opts)); + Assert.Equal("shouldn't override username", opts.AuthOpts.Username); + Assert.Equal("shouldn't override password", opts.AuthOpts.Password); + Assert.Equal("shouldn't override token", opts.AuthOpts.Token); } [Fact] public void URL_escape_user_password() { var opts = new NatsConnection(new NatsOpts { Url = "nats://u%2C:p%2C@host1,host2" }).Opts; - Assert.Equal("nats://u%2C:***@host1:4222,nats://u%2C:***@host2:4222", opts.Url); + Assert.Equal("nats://u%2C:***@host1:4222,nats://host2:4222", GetUrisAsRedactedString(opts)); Assert.Equal("u,", opts.AuthOpts.Username); Assert.Equal("p,", opts.AuthOpts.Password); Assert.Null(opts.AuthOpts.Token); @@ -64,18 +80,18 @@ public void URL_escape_user_password() uris[0].Uri.Scheme.Should().Be("nats"); uris[0].Uri.Host.Should().Be("host1"); uris[0].Uri.Port.Should().Be(4222); - uris[0].Uri.UserInfo.Should().Be("u%2C:***"); + uris[0].Uri.UserInfo.Should().Be("u%2C:p%2C"); uris[1].Uri.Scheme.Should().Be("nats"); uris[1].Uri.Host.Should().Be("host2"); uris[1].Uri.Port.Should().Be(4222); - uris[1].Uri.UserInfo.Should().Be("u%2C:***"); + uris[1].Uri.UserInfo.Should().Be(string.Empty); } [Fact] public void URL_escape_token() { var opts = new NatsConnection(new NatsOpts { Url = "nats://t%2C@host1,nats://t%2C@host2" }).Opts; - Assert.Equal("nats://***@host1:4222,nats://***@host2:4222", opts.Url); + Assert.Equal("nats://***@host1:4222,nats://***@host2:4222", GetUrisAsRedactedString(opts)); Assert.Null(opts.AuthOpts.Username); Assert.Null(opts.AuthOpts.Password); Assert.Equal("t,", opts.AuthOpts.Token); @@ -84,10 +100,22 @@ public void URL_escape_token() uris[0].Uri.Scheme.Should().Be("nats"); uris[0].Uri.Host.Should().Be("host1"); uris[0].Uri.Port.Should().Be(4222); - uris[0].Uri.UserInfo.Should().Be("***"); + uris[0].Uri.UserInfo.Should().Be("t%2C"); uris[1].Uri.Scheme.Should().Be("nats"); uris[1].Uri.Host.Should().Be("host2"); uris[1].Uri.Port.Should().Be(4222); - uris[1].Uri.UserInfo.Should().Be("***"); + uris[1].Uri.UserInfo.Should().Be("t%2C"); } + + [Fact] + public void Keep_URL_wss_path_and_query_string() + { + var opts = new NatsConnection(new NatsOpts { Url = "wss://t%2C@host1/path1/path2?q1=1" }).Opts; + Assert.Equal("wss://***@host1/path1/path2?q1=1", GetUrisAsRedactedString(opts)); + Assert.Null(opts.AuthOpts.Username); + Assert.Null(opts.AuthOpts.Password); + Assert.Equal("t,", opts.AuthOpts.Token); + } + + private static string GetUrisAsRedactedString(NatsOpts opts) => string.Join(",", opts.GetSeedUris(true).Select(u => u.ToString())); } From 26f64b154e3e3a40f8c809ea0a75c31230834c1b Mon Sep 17 00:00:00 2001 From: Ziya Suzen Date: Sun, 3 Nov 2024 12:22:34 +0000 Subject: [PATCH 12/14] Remove unused test params --- tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs index 397d3e720..e4144d179 100644 --- a/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs +++ b/tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs @@ -47,9 +47,9 @@ public void URL_parts(string url, string expected, string? user, string? pass, s } [Theory] - [InlineData("u:p@host1:1234", "nats://u:***@host1:1234", "u", "p", null)] - [InlineData("t@host1:1234", "nats://***@host1:1234", null, null, "t")] - public void URL_should_not_override_auth_options(string url, string expected, string? user, string? pass, string? token) + [InlineData("u:p@host1:1234", "nats://u:***@host1:1234")] + [InlineData("t@host1:1234", "nats://***@host1:1234")] + public void URL_should_not_override_auth_options(string url, string expected) { var opts = new NatsConnection(new NatsOpts { From 63b3bf29e53dfcb6a9c7f7f43360696a364215a0 Mon Sep 17 00:00:00 2001 From: Ziya Suzen Date: Mon, 4 Nov 2024 04:13:07 +0000 Subject: [PATCH 13/14] Refactor redacted URI handling in NatsUri class Modified the NatsUri class to store the redacted URI as a string instead of a Uri object. This change simplifies the ToString method and ensures that sensitive information like user credentials is consistently redacted for logging purposes. Redacted string is created once in the ctor and avoiding Uri.ToString() calls being made everytime it's needed. --- src/NATS.Client.Core/Internal/NatsUri.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/NATS.Client.Core/Internal/NatsUri.cs b/src/NATS.Client.Core/Internal/NatsUri.cs index 979d59604..02144d50c 100644 --- a/src/NATS.Client.Core/Internal/NatsUri.cs +++ b/src/NATS.Client.Core/Internal/NatsUri.cs @@ -4,7 +4,7 @@ internal sealed class NatsUri : IEquatable { public const string DefaultScheme = "nats"; - private readonly Uri _redactedUri; + private readonly string _redacted; public NatsUri(string urlString, bool isSeed, string defaultScheme = DefaultScheme) { @@ -41,6 +41,7 @@ public NatsUri(string urlString, bool isSeed, string defaultScheme = DefaultSche Uri = uriBuilder.Uri; + // Redact user/password or token from the URI string for logging if (uriBuilder.UserName is { Length: > 0 }) { if (uriBuilder.Password is { Length: > 0 }) @@ -53,7 +54,7 @@ public NatsUri(string urlString, bool isSeed, string defaultScheme = DefaultSche } } - _redactedUri = uriBuilder.Uri; + _redacted = IsWebSocket && Uri.AbsolutePath != "/" ? uriBuilder.Uri.ToString() : uriBuilder.Uri.ToString().Trim('/'); } public Uri Uri { get; } @@ -79,10 +80,7 @@ public NatsUri CloneWith(string host, int? port = default) return new NatsUri(newUri, IsSeed); } - public override string ToString() - { - return IsWebSocket && Uri.AbsolutePath != "/" ? _redactedUri.ToString() : _redactedUri.ToString().Trim('/'); - } + public override string ToString() => _redacted; public override int GetHashCode() => Uri.GetHashCode(); From a9f9d49cc911501495a0e777560f06e9f063aef4 Mon Sep 17 00:00:00 2001 From: Ziya Suzen Date: Mon, 4 Nov 2024 15:58:52 +0000 Subject: [PATCH 14/14] Move read URL credentials into NatsOpts --- src/NATS.Client.Core/NatsConnection.cs | 43 +------------------------- src/NATS.Client.Core/NatsOpts.cs | 39 +++++++++++++++++++++++ 2 files changed, 40 insertions(+), 42 deletions(-) diff --git a/src/NATS.Client.Core/NatsConnection.cs b/src/NATS.Client.Core/NatsConnection.cs index a596fa32e..0601ea6d7 100644 --- a/src/NATS.Client.Core/NatsConnection.cs +++ b/src/NATS.Client.Core/NatsConnection.cs @@ -75,7 +75,7 @@ public NatsConnection() public NatsConnection(NatsOpts opts) { _logger = opts.LoggerFactory.CreateLogger(); - Opts = ReadUserInfoFromConnectionString(opts); + Opts = opts.ReadUserInfoFromConnectionString(); ConnectionState = NatsConnectionState.Closed; _waitForOpenConnection = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _disposedCancellationTokenSource = new CancellationTokenSource(); @@ -288,47 +288,6 @@ internal ValueTask UnsubscribeAsync(int sid) return default; } - private static NatsOpts ReadUserInfoFromConnectionString(NatsOpts opts) - { - // Setting credentials in options takes precedence over URL credentials - if (opts.AuthOpts.Username is { Length: > 0 } || opts.AuthOpts.Password is { Length: > 0 } || opts.AuthOpts.Token is { Length: > 0 }) - { - return opts; - } - - var natsUri = opts.GetSeedUris(suppressRandomization: true).First(); - var uriBuilder = new UriBuilder(natsUri.Uri); - - if (uriBuilder.UserName is not { Length: > 0 }) - { - return opts; - } - - if (uriBuilder.Password is { Length: > 0 }) - { - opts = opts with - { - AuthOpts = opts.AuthOpts with - { - Username = Uri.UnescapeDataString(uriBuilder.UserName), - Password = Uri.UnescapeDataString(uriBuilder.Password), - }, - }; - } - else - { - opts = opts with - { - AuthOpts = opts.AuthOpts with - { - Token = Uri.UnescapeDataString(uriBuilder.UserName), - }, - }; - } - - return opts; - } - private async ValueTask InitialConnectAsync() { Debug.Assert(ConnectionState == NatsConnectionState.Connecting, "Connection state"); diff --git a/src/NATS.Client.Core/NatsOpts.cs b/src/NATS.Client.Core/NatsOpts.cs index f4213ccc4..5dc7b2083 100644 --- a/src/NATS.Client.Core/NatsOpts.cs +++ b/src/NATS.Client.Core/NatsOpts.cs @@ -146,4 +146,43 @@ internal NatsUri[] GetSeedUris(bool suppressRandomization = false) ? urls.Select(x => new NatsUri(x, true)).Distinct().ToArray() : urls.Select(x => new NatsUri(x, true)).OrderBy(_ => Guid.NewGuid()).Distinct().ToArray(); } + + internal NatsOpts ReadUserInfoFromConnectionString() + { + // Setting credentials in options takes precedence over URL credentials + if (AuthOpts.Username is { Length: > 0 } || AuthOpts.Password is { Length: > 0 } || AuthOpts.Token is { Length: > 0 }) + { + return this; + } + + var natsUri = GetSeedUris(suppressRandomization: true).First(); + var uriBuilder = new UriBuilder(natsUri.Uri); + + if (uriBuilder.UserName is not { Length: > 0 }) + { + return this; + } + + if (uriBuilder.Password is { Length: > 0 }) + { + return this with + { + AuthOpts = AuthOpts with + { + Username = Uri.UnescapeDataString(uriBuilder.UserName), + Password = Uri.UnescapeDataString(uriBuilder.Password), + }, + }; + } + else + { + return this with + { + AuthOpts = AuthOpts with + { + Token = Uri.UnescapeDataString(uriBuilder.UserName), + }, + }; + } + } }