Conceal password in activity traces (#439)

DarkWanderer · Feb 18, 2024 · b58d851 · b58d851
1 parent 4818468
commit b58d851
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 45 deletions.
diff --git a/ClickHouse.Client.Tests/ConnectionTests.cs b/ClickHouse.Client.Tests/ConnectionTests.cs
@@ -179,5 +179,17 @@ public void ChangeDatabaseShouldChangeDatabase()
         Assert.AreEqual("default", conn.Database);
     }
 
+    [Test]
+    public void ShouldExcludePasswordFromRedactedConnectionString()
+    {
+        const string MOCK = "verysecurepassword";
+        using var conn = TestUtilities.GetTestClickHouseConnection();
+        var builder = conn.ConnectionStringBuilder;
+        builder.Password = MOCK;
+        conn.ConnectionStringBuilder = builder;
+        Assert.That(conn.ConnectionString, Contains.Substring($"Password={MOCK}"));
+        Assert.That(conn.RedactedConnectionString, Is.Not.Contains($"Password={MOCK}"));
+    }
+
     private static string[] GetColumnNames(DataTable table) => table.Columns.Cast<DataColumn>().Select(dc => dc.ColumnName).ToArray();
 }
diff --git a/ClickHouse.Client/ADO/ClickHouseConnection.cs b/ClickHouse.Client/ADO/ClickHouseConnection.cs
@@ -119,48 +119,8 @@ public ClickHouseConnection(string connectionString, IHttpClientFactory httpClie
     /// </summary>
     public sealed override string ConnectionString
     {
-        get
-        {
-            var builder = new ClickHouseConnectionStringBuilder
-            {
-                Database = database,
-                Username = username,
-                Password = password,
-                Host = serverUri?.Host,
-                Port = (ushort)serverUri?.Port,
-                Compression = UseCompression,
-                UseSession = session != null,
-                Timeout = timeout,
-                UseServerTimezone = useServerTimezone,
-                UseCustomDecimals = useCustomDecimals,
-            };
-
-            foreach (var kvp in CustomSettings)
-                builder[CustomSettingPrefix + kvp.Key] = kvp.Value;
-
-            return builder.ToString();
-        }
-
-        set
-        {
-            var builder = new ClickHouseConnectionStringBuilder() { ConnectionString = value };
-            database = builder.Database;
-            username = builder.Username;
-            password = builder.Password;
-            serverUri = new UriBuilder(builder.Protocol, builder.Host, builder.Port).Uri;
-            UseCompression = builder.Compression;
-            session = builder.UseSession ? builder.SessionId ?? Guid.NewGuid().ToString() : null;
-            timeout = builder.Timeout;
-            useServerTimezone = builder.UseServerTimezone;
-            useCustomDecimals = builder.UseCustomDecimals;
-
-            foreach (var key in builder.Keys.Cast<string>().Where(k => k.StartsWith(CustomSettingPrefix, true, CultureInfo.InvariantCulture)))
-            {
-                CustomSettings.Set(key.Replace(CustomSettingPrefix, string.Empty), builder[key]);
-            }
-
-            ResetHttpClientFactory();
-        }
+        get => ConnectionStringBuilder.ToString();
+        set => ConnectionStringBuilder = new ClickHouseConnectionStringBuilder() { ConnectionString = value };
     }
 
     public IDictionary<string, object> CustomSettings => customSettings;
@@ -173,6 +133,16 @@ public sealed override string ConnectionString
 
     internal Uri ServerUri => serverUri;
 
+    internal string RedactedConnectionString
+    {
+        get
+        {
+            var builder = ConnectionStringBuilder;
+            builder.Password = "****";
+            return builder.ToString();
+        }
+    }
+
     public string ServerTimezone => serverTimezone;
 
     public override string DataSource { get; }
@@ -374,6 +344,52 @@ internal void AddDefaultHttpHeaders(HttpRequestHeaders headers)
         }
     }
 
+    internal ClickHouseConnectionStringBuilder ConnectionStringBuilder
+    {
+        get
+        {
+            var builder = new ClickHouseConnectionStringBuilder
+            {
+                Database = database,
+                Username = username,
+                Password = password,
+                Host = serverUri?.Host,
+                Port = (ushort)serverUri?.Port,
+                Compression = UseCompression,
+                UseSession = session != null,
+                Timeout = timeout,
+                UseServerTimezone = useServerTimezone,
+                UseCustomDecimals = useCustomDecimals,
+            };
+
+            foreach (var kvp in CustomSettings)
+                builder[CustomSettingPrefix + kvp.Key] = kvp.Value;
+
+            return builder;
+        }
+
+        set
+        {
+            var builder = value;
+            database = builder.Database;
+            username = builder.Username;
+            password = builder.Password;
+            serverUri = new UriBuilder(builder.Protocol, builder.Host, builder.Port).Uri;
+            UseCompression = builder.Compression;
+            session = builder.UseSession ? builder.SessionId ?? Guid.NewGuid().ToString() : null;
+            timeout = builder.Timeout;
+            useServerTimezone = builder.UseServerTimezone;
+            useCustomDecimals = builder.UseCustomDecimals;
+
+            foreach (var key in builder.Keys.Cast<string>().Where(k => k.StartsWith(CustomSettingPrefix, true, CultureInfo.InvariantCulture)))
+            {
+                CustomSettings.Set(key.Replace(CustomSettingPrefix, string.Empty), builder[key]);
+            }
+
+            ResetHttpClientFactory();
+        }
+    }
+
     protected override DbTransaction BeginDbTransaction(IsolationLevel isolationLevel) => throw new NotSupportedException();
 
     protected override DbCommand CreateDbCommand() => CreateCommand();

diff --git a/ClickHouse.Client/Diagnostic/ActivitySourceHelper.cs b/ClickHouse.Client/Diagnostic/ActivitySourceHelper.cs
@@ -42,7 +42,7 @@ internal static Activity StartActivity(this ClickHouseConnection connection, str
             activity.SetTag(TagThreadId, Environment.CurrentManagedThreadId.ToString(CultureInfo.InvariantCulture));
             activity.SetTag(TagDbSystem, "clickhouse");
         }
-        activity.SetTag(TagDbConnectionString, connection.ConnectionString);
+        activity.SetTag(TagDbConnectionString, connection.RedactedConnectionString);
         activity.SetTag(TagDbName, connection.Database);
         activity.SetTag(TagUser, connection.Username);
         activity.SetTag(TagService, $"{connection.ServerUri.Host}:{connection.ServerUri.Port}");

diff --git a/tests/users.d/test.yml b/tests/users.d/test.yml
@@ -1,2 +1,7 @@
-test:
-  password: test1234
+users:
+  test:
+    password: test1234
+    grants:
+      query:
+        - GRANT ALL ON test.*
+        - GRANT SELECT ON system.*