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

@@ -4,6 +4,8 @@ internal sealed class NatsUri : IEquatable<NatsUri>
 {
     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();

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<string>(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;
     }
 

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, <c>nats://derek:s3cr3t@localhost:4222</c> or <c>nats://token@localhost:4222</c>.
     /// You can also set the username and password or token separately using <see cref="AuthOpts"/>;
-    /// however, if both are set, the URL will take precedence.
+    /// however, if both are set, the <see cref="AuthOpts"/> will take precedence.
     /// You should URL-encode the username and password or token if they contain special characters.
     /// </para>
     /// <para>

tests/NATS.Client.CoreUnit.Tests/OptsUrlTests.cs

@@ -9,22 +9,38 @@ 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)]
     [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")]
+    [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()));
 }