From 04d6533722411686259d30d361038642be2d7b9f Mon Sep 17 00:00:00 2001 From: Krzysztof Nozderko Date: Tue, 14 May 2024 15:12:40 +0000 Subject: [PATCH 1/5] SNOW-1417631 Disable default pooling for JWT token with key provided in a file --- .../UnitTests/ConnectionCacheManagerTest.cs | 46 +++++++++++++++++++ .../UnitTests/SFSessionPropertyTest.cs | 36 ++++++++++++--- .../Core/Session/SFSessionProperty.cs | 35 +++++++++----- 3 files changed, 98 insertions(+), 19 deletions(-) create mode 100644 Snowflake.Data.Tests/UnitTests/ConnectionCacheManagerTest.cs diff --git a/Snowflake.Data.Tests/UnitTests/ConnectionCacheManagerTest.cs b/Snowflake.Data.Tests/UnitTests/ConnectionCacheManagerTest.cs new file mode 100644 index 000000000..589565ddf --- /dev/null +++ b/Snowflake.Data.Tests/UnitTests/ConnectionCacheManagerTest.cs @@ -0,0 +1,46 @@ +using NUnit.Framework; +using Snowflake.Data.Client; +using Snowflake.Data.Core.Session; +using Snowflake.Data.Tests.Util; + +namespace Snowflake.Data.Tests.UnitTests +{ + [TestFixture, NonParallelizable] + public class ConnectionCacheManagerTest + { + private readonly ConnectionCacheManager _connectionCacheManager = new ConnectionCacheManager(); + private const string ConnectionString = "db=D1;warehouse=W1;account=A1;user=U1;password=P1;role=R1;minPoolSize=1;"; + private static PoolConfig s_poolConfig; + + [OneTimeSetUp] + public static void BeforeAllTests() + { + s_poolConfig = new PoolConfig(); + SnowflakeDbConnectionPool.SetConnectionPoolVersion(ConnectionPoolType.SingleConnectionCache); + SessionPool.SessionFactory = new MockSessionFactory(); + } + + [OneTimeTearDown] + public static void AfterAllTests() + { + s_poolConfig.Reset(); + SessionPool.SessionFactory = new SessionFactory(); + } + + [SetUp] + public void BeforeEach() + { + _connectionCacheManager.ClearAllPools(); + } + + [Test] + public void TestEnablePoolingRegardlessOfConnectionStringProperty() + { + // act + var pool = _connectionCacheManager.GetPool(ConnectionString + "poolingEnabled=false"); + + // assert + Assert.IsTrue(pool.GetPooling()); + } + } +} diff --git a/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs b/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs index 71af2b1c9..0df5086dc 100644 --- a/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs +++ b/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs @@ -156,15 +156,37 @@ public void TestPoolingEnabledForExternalBrowserAuthenticator(string connectionP } [Test] - [TestCase(BasicAuthenticator.AUTH_NAME, "true")] - [TestCase(KeyPairAuthenticator.AUTH_NAME, "true")] - [TestCase(OAuthAuthenticator.AUTH_NAME, "true")] - [TestCase(OktaAuthenticator.AUTH_NAME, "true")] - [TestCase(ExternalBrowserAuthenticator.AUTH_NAME, "false")] - public void TestDefaultPoolingEnabledForAuthenticator(string authenticator, string expectedPoolingEnabled) + [TestCase(BasicAuthenticator.AUTH_NAME, "", "true")] + [TestCase(KeyPairAuthenticator.AUTH_NAME, "", "true")] + [TestCase(KeyPairAuthenticator.AUTH_NAME, ";PRIVATE_KEY_FILE=/some/file.txt", "false")] + [TestCase(OAuthAuthenticator.AUTH_NAME, "", "true")] + [TestCase(OktaAuthenticator.AUTH_NAME, "", "true")] + [TestCase(ExternalBrowserAuthenticator.AUTH_NAME, "", "false")] + public void TestDefaultPoolingEnabledForAuthenticator(string authenticator, string additionalConnectionStringPart, string expectedPoolingEnabled) { // arrange - var connectionString = $"ACCOUNT=test;USER=test;PASSWORD=test;TOKEN=test;AUTHENTICATOR={authenticator}"; + var connectionString = $"ACCOUNT=test;USER=test;PASSWORD=test;TOKEN=test;AUTHENTICATOR={authenticator}{additionalConnectionStringPart}"; + + // act + var properties = SFSessionProperties.ParseConnectionString(connectionString, null); + + // assert + Assert.AreEqual(expectedPoolingEnabled, properties[SFSessionProperty.POOLINGENABLED]); + } + + [Test] + [TestCase("PRIVATE_KEY_FILE=/some/file.txt", "false")] + [TestCase("PRIVATE_KEY=topSecret", "true")] + [TestCase("PRIVATE_KEY_FILE=/some/file.txt;PRIVATE_KEY=topSecret", "false")] + [TestCase("PRIVATE_KEY_FILE=;PRIVATE_KEY=topSecret", "true")] + [TestCase("PRIVATE_KEY_FILE=/some/file.txt;poolingEnabled=true", "true")] + [TestCase("PRIVATE_KEY_FILE=/some/file.txt;poolingEnabled=false", "false")] + [TestCase("PRIVATE_KEY=topSecret;poolingEnabled=true", "true")] + [TestCase("PRIVATE_KEY=topSecret;poolingEnabled=false", "false")] + public void TestPoolingEnabledForJwtTokenAuthenticator(string connectionParam, string expectedPoolingEnabled) + { + // arrange + var connectionString = $"ACCOUNT=test;USER=testuser;AUTHENTICATOR=snowflake_jwt;{connectionParam}"; // act var properties = SFSessionProperties.ParseConnectionString(connectionString, null); diff --git a/Snowflake.Data/Core/Session/SFSessionProperty.cs b/Snowflake.Data/Core/Session/SFSessionProperty.cs index c2e3777b6..858fe58fc 100644 --- a/Snowflake.Data/Core/Session/SFSessionProperty.cs +++ b/Snowflake.Data/Core/Session/SFSessionProperty.cs @@ -254,7 +254,7 @@ internal static SFSessionProperties ParseConnectionString(string connectionStrin } ValidateAuthenticator(properties); - DisableConnectionPoolingForExternalBrowser(properties); + DisableConnectionPoolingWhenSecretsPassedExternally(properties); CheckSessionProperties(properties); ValidateFileTransferMaxBytesInMemoryProperty(properties); ValidateAccountDomain(properties); @@ -308,23 +308,34 @@ private static void ValidateAuthenticator(SFSessionProperties properties) } } - private static void DisableConnectionPoolingForExternalBrowser(SFSessionProperties properties) + private static void DisableConnectionPoolingWhenSecretsPassedExternally(SFSessionProperties properties) { if (properties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator)) { authenticator = authenticator.ToLower(); - if (authenticator.Equals(ExternalBrowserAuthenticator.AUTH_NAME)) + if (ExternalBrowserAuthenticator.AUTH_NAME.Equals(authenticator)) { - if (!properties.TryGetValue(SFSessionProperty.POOLINGENABLED, out var poolingEnabledStr)) - { - properties.Add(SFSessionProperty.POOLINGENABLED, "false"); - logger.Info("Connection pooling is disabled for external browser authentication"); - } - else if (Boolean.TryParse(poolingEnabledStr, out var poolingEnabled) && poolingEnabled) - { - logger.Warn("Connection pooling is enabled for external browser authentication"); - } + DisableConnectionPooling(properties, "external browser"); } + else if (KeyPairAuthenticator.AUTH_NAME.Equals(authenticator) + && properties.TryGetValue(SFSessionProperty.PRIVATE_KEY_FILE, out var privateKeyFile) + && !string.IsNullOrEmpty(privateKeyFile)) + { + DisableConnectionPooling(properties, "jwt token with private key in a file"); + } + } + } + + private static void DisableConnectionPooling(SFSessionProperties properties, string authenticationDescription) + { + if (!properties.TryGetValue(SFSessionProperty.POOLINGENABLED, out var poolingEnabledStr)) + { + properties.Add(SFSessionProperty.POOLINGENABLED, "false"); + logger.Info($"Connection pooling is disabled for {authenticationDescription} authentication"); + } + else if (Boolean.TryParse(poolingEnabledStr, out var poolingEnabled) && poolingEnabled) + { + logger.Warn($"Connection pooling is enabled for {authenticationDescription} authentication"); } } From e3a87d133b1a4644f0354d8bbb18854bdce53dd5 Mon Sep 17 00:00:00 2001 From: Krzysztof Nozderko Date: Wed, 15 May 2024 12:15:54 +0000 Subject: [PATCH 2/5] Execute disabling pooling for specific authenticators for new pool only --- .../UnitTests/SFSessionPropertyTest.cs | 58 +------------------ .../ConnectionPoolConfigExtractorTest.cs | 28 +++++++-- .../Session/SFSessionHttpClientProperties.cs | 26 +++++++++ .../Core/Session/SFSessionProperty.cs | 36 ++---------- Snowflake.Data/Core/Session/SessionPool.cs | 3 +- 5 files changed, 56 insertions(+), 95 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs b/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs index 0df5086dc..40c7551f8 100644 --- a/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs +++ b/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs @@ -139,62 +139,6 @@ public void TestValidateSupportEscapedQuotesInsideValuesForObjectProperties(stri Assert.AreEqual(expectedValue, properties[sessionProperty]); } - [Test] - [TestCase("", "false")] - [TestCase("poolingEnabled=true", "true")] - [TestCase("poolingEnabled=false", "false")] - public void TestPoolingEnabledForExternalBrowserAuthenticator(string connectionParam, string expectedPoolingEnabled) - { - // arrange - var connectionString = $"ACCOUNT=test;AUTHENTICATOR=externalbrowser;{connectionParam}"; - - // act - var properties = SFSessionProperties.ParseConnectionString(connectionString, null); - - // assert - Assert.AreEqual(expectedPoolingEnabled, properties[SFSessionProperty.POOLINGENABLED]); - } - - [Test] - [TestCase(BasicAuthenticator.AUTH_NAME, "", "true")] - [TestCase(KeyPairAuthenticator.AUTH_NAME, "", "true")] - [TestCase(KeyPairAuthenticator.AUTH_NAME, ";PRIVATE_KEY_FILE=/some/file.txt", "false")] - [TestCase(OAuthAuthenticator.AUTH_NAME, "", "true")] - [TestCase(OktaAuthenticator.AUTH_NAME, "", "true")] - [TestCase(ExternalBrowserAuthenticator.AUTH_NAME, "", "false")] - public void TestDefaultPoolingEnabledForAuthenticator(string authenticator, string additionalConnectionStringPart, string expectedPoolingEnabled) - { - // arrange - var connectionString = $"ACCOUNT=test;USER=test;PASSWORD=test;TOKEN=test;AUTHENTICATOR={authenticator}{additionalConnectionStringPart}"; - - // act - var properties = SFSessionProperties.ParseConnectionString(connectionString, null); - - // assert - Assert.AreEqual(expectedPoolingEnabled, properties[SFSessionProperty.POOLINGENABLED]); - } - - [Test] - [TestCase("PRIVATE_KEY_FILE=/some/file.txt", "false")] - [TestCase("PRIVATE_KEY=topSecret", "true")] - [TestCase("PRIVATE_KEY_FILE=/some/file.txt;PRIVATE_KEY=topSecret", "false")] - [TestCase("PRIVATE_KEY_FILE=;PRIVATE_KEY=topSecret", "true")] - [TestCase("PRIVATE_KEY_FILE=/some/file.txt;poolingEnabled=true", "true")] - [TestCase("PRIVATE_KEY_FILE=/some/file.txt;poolingEnabled=false", "false")] - [TestCase("PRIVATE_KEY=topSecret;poolingEnabled=true", "true")] - [TestCase("PRIVATE_KEY=topSecret;poolingEnabled=false", "false")] - public void TestPoolingEnabledForJwtTokenAuthenticator(string connectionParam, string expectedPoolingEnabled) - { - // arrange - var connectionString = $"ACCOUNT=test;USER=testuser;AUTHENTICATOR=snowflake_jwt;{connectionParam}"; - - // act - var properties = SFSessionProperties.ParseConnectionString(connectionString, null); - - // assert - Assert.AreEqual(expectedPoolingEnabled, properties[SFSessionProperty.POOLINGENABLED]); - } - public static IEnumerable ConnectionStringTestCases() { string defAccount = "testaccount"; @@ -285,7 +229,7 @@ public static IEnumerable ConnectionStringTestCases() { SFSessionProperty.CHANGEDSESSION, DefaultValue(SFSessionProperty.CHANGEDSESSION) }, { SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) }, { SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) }, - { SFSessionProperty.POOLINGENABLED, "false" } // connection pooling is disabled for external browser authentication + { SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) } } }; var testCaseWithProxySettings = new TestCase() diff --git a/Snowflake.Data.Tests/UnitTests/Session/ConnectionPoolConfigExtractorTest.cs b/Snowflake.Data.Tests/UnitTests/Session/ConnectionPoolConfigExtractorTest.cs index 1f1c18758..e27a12866 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/ConnectionPoolConfigExtractorTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/ConnectionPoolConfigExtractorTest.cs @@ -219,6 +219,26 @@ public void TestExtractPoolingEnabled(string propertyValue, bool poolingEnabled) Assert.AreEqual(poolingEnabled, result.PoolingEnabled); } + [Test] + [TestCase("account=test;user=test;password=test;", true)] + [TestCase("authenticator=externalbrowser;account=test;user=test;", false)] + [TestCase("authenticator=externalbrowser;account=test;user=test;poolingEnabled=true;", true)] + [TestCase("authenticator=externalbrowser;account=test;user=test;poolingEnabled=false;", false)] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key_file=/some/file.key", false)] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key_file=/some/file.key;poolingEnabled=true;", true)] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key_file=/some/file.key;poolingEnabled=false;", false)] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key=secretKey", true)] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key=secretKey;poolingEnabled=true;", true)] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key=secretKey;poolingEnabled=false;", false)] + public void TestDisablePoolingDefaultWhenSecretsProvidedExternally(string connectionString, bool poolingEnabled) + { + // act + var result = ExtractConnectionPoolConfig(connectionString); + + // assert + Assert.AreEqual(poolingEnabled, result.PoolingEnabled); + } + [Test] [TestCase("wrong_value")] [TestCase("15")] @@ -252,12 +272,8 @@ public void TestExtractChangedSessionBehaviour(string propertyValue, ChangedSess Assert.AreEqual(expectedChangedSession, result.ChangedSession); } - private ConnectionPoolConfig ExtractConnectionPoolConfig(string connectionString) - { - var properties = SFSessionProperties.ParseConnectionString(connectionString, null); - var extractedProperties = SFSessionHttpClientProperties.ExtractAndValidate(properties); - return extractedProperties.BuildConnectionPoolConfig(); - } + private ConnectionPoolConfig ExtractConnectionPoolConfig(string connectionString) => + SessionPool.ExtractConfig(connectionString, null).Item1; public class TimeoutTestCase { diff --git a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs index b79c332c7..8800804d7 100644 --- a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs +++ b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs @@ -47,6 +47,32 @@ public static SFSessionHttpClientProperties ExtractAndValidate(SFSessionProperti return extractedProperties; } + public void DisablePoolingDefaultIfSecretsProvidedExternally(SFSessionProperties properties) + { + var authenticator = properties[SFSessionProperty.AUTHENTICATOR].ToLower(); + if (ExternalBrowserAuthenticator.AUTH_NAME.Equals(authenticator)) + { + DisablePoolingIfNotExplicitlyEnabled(properties, "external browser"); + + } else if (KeyPairAuthenticator.AUTH_NAME.Equals(authenticator) && !string.IsNullOrEmpty(properties.GetValueOrDefault(SFSessionProperty.PRIVATE_KEY_FILE))) + { + DisablePoolingIfNotExplicitlyEnabled(properties, "jwt token with private key in a file"); + } + } + + private void DisablePoolingIfNotExplicitlyEnabled(SFSessionProperties properties, string authenticationDescription) + { + if (!properties.IsPoolingEnabledValueProvided && _poolingEnabled) + { + _poolingEnabled = false; + s_logger.Info($"Disabling connection pooling for {authenticationDescription} authentication"); + } + else if (properties.IsPoolingEnabledValueProvided && _poolingEnabled) + { + s_logger.Warn($"Connection pooling is enabled for {authenticationDescription} authentication which is not recommended"); + } + } + private void CheckPropertiesAreValid() { try diff --git a/Snowflake.Data/Core/Session/SFSessionProperty.cs b/Snowflake.Data/Core/Session/SFSessionProperty.cs index 858fe58fc..cea067025 100644 --- a/Snowflake.Data/Core/Session/SFSessionProperty.cs +++ b/Snowflake.Data/Core/Session/SFSessionProperty.cs @@ -128,6 +128,8 @@ class SFSessionProperties : Dictionary internal string ConnectionStringWithoutSecrets { get; set; } + internal bool IsPoolingEnabledValueProvided { get; set; } + // Connection string properties to obfuscate in the log private static readonly List s_secretProps = Enum.GetValues(typeof(SFSessionProperty)) .Cast() @@ -254,7 +256,7 @@ internal static SFSessionProperties ParseConnectionString(string connectionStrin } ValidateAuthenticator(properties); - DisableConnectionPoolingWhenSecretsPassedExternally(properties); + properties.IsPoolingEnabledValueProvided = CheckPoolingEnabledValueProvided(properties); CheckSessionProperties(properties); ValidateFileTransferMaxBytesInMemoryProperty(properties); ValidateAccountDomain(properties); @@ -308,36 +310,8 @@ private static void ValidateAuthenticator(SFSessionProperties properties) } } - private static void DisableConnectionPoolingWhenSecretsPassedExternally(SFSessionProperties properties) - { - if (properties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator)) - { - authenticator = authenticator.ToLower(); - if (ExternalBrowserAuthenticator.AUTH_NAME.Equals(authenticator)) - { - DisableConnectionPooling(properties, "external browser"); - } - else if (KeyPairAuthenticator.AUTH_NAME.Equals(authenticator) - && properties.TryGetValue(SFSessionProperty.PRIVATE_KEY_FILE, out var privateKeyFile) - && !string.IsNullOrEmpty(privateKeyFile)) - { - DisableConnectionPooling(properties, "jwt token with private key in a file"); - } - } - } - - private static void DisableConnectionPooling(SFSessionProperties properties, string authenticationDescription) - { - if (!properties.TryGetValue(SFSessionProperty.POOLINGENABLED, out var poolingEnabledStr)) - { - properties.Add(SFSessionProperty.POOLINGENABLED, "false"); - logger.Info($"Connection pooling is disabled for {authenticationDescription} authentication"); - } - else if (Boolean.TryParse(poolingEnabledStr, out var poolingEnabled) && poolingEnabled) - { - logger.Warn($"Connection pooling is enabled for {authenticationDescription} authentication"); - } - } + private static bool CheckPoolingEnabledValueProvided(SFSessionProperties properties) => + properties.TryGetValue(SFSessionProperty.POOLINGENABLED, out var poolingEnabledStr) && !string.IsNullOrEmpty(poolingEnabledStr); private static string BuildConnectionStringWithoutSecrets(ref string[] keys, ref string[] values) { diff --git a/Snowflake.Data/Core/Session/SessionPool.cs b/Snowflake.Data/Core/Session/SessionPool.cs index e0d72bc4d..c28db40c0 100644 --- a/Snowflake.Data/Core/Session/SessionPool.cs +++ b/Snowflake.Data/Core/Session/SessionPool.cs @@ -103,12 +103,13 @@ private void CleanExpiredSessions() } } - private static Tuple ExtractConfig(string connectionString, SecureString password) + internal static Tuple ExtractConfig(string connectionString, SecureString password) { try { var properties = SFSessionProperties.ParseConnectionString(connectionString, password); var extractedProperties = SFSessionHttpClientProperties.ExtractAndValidate(properties); + extractedProperties.DisablePoolingDefaultIfSecretsProvidedExternally(properties); return Tuple.Create(extractedProperties.BuildConnectionPoolConfig(), properties.ConnectionStringWithoutSecrets); } catch (Exception exception) From 280b60736ce3f5d4bc1406a72360c86277572b97 Mon Sep 17 00:00:00 2001 From: Krzysztof Nozderko Date: Wed, 15 May 2024 13:11:26 +0000 Subject: [PATCH 3/5] Fix framework problem --- Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs index 8800804d7..ed13ffeed 100644 --- a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs +++ b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs @@ -54,7 +54,7 @@ public void DisablePoolingDefaultIfSecretsProvidedExternally(SFSessionProperties { DisablePoolingIfNotExplicitlyEnabled(properties, "external browser"); - } else if (KeyPairAuthenticator.AUTH_NAME.Equals(authenticator) && !string.IsNullOrEmpty(properties.GetValueOrDefault(SFSessionProperty.PRIVATE_KEY_FILE))) + } else if (KeyPairAuthenticator.AUTH_NAME.Equals(authenticator) && !string.IsNullOrEmpty(properties.GetValueOrDefault(SFSessionProperty.PRIVATE_KEY_FILE, null))) { DisablePoolingIfNotExplicitlyEnabled(properties, "jwt token with private key in a file"); } From db479e64fafa4c0be24acae4e654ef65825649f8 Mon Sep 17 00:00:00 2001 From: Krzysztof Nozderko Date: Wed, 15 May 2024 13:52:18 +0000 Subject: [PATCH 4/5] fix for .net framework --- .../Core/Session/SFSessionHttpClientProperties.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs index ed13ffeed..6252084ec 100644 --- a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs +++ b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs @@ -54,9 +54,11 @@ public void DisablePoolingDefaultIfSecretsProvidedExternally(SFSessionProperties { DisablePoolingIfNotExplicitlyEnabled(properties, "external browser"); - } else if (KeyPairAuthenticator.AUTH_NAME.Equals(authenticator) && !string.IsNullOrEmpty(properties.GetValueOrDefault(SFSessionProperty.PRIVATE_KEY_FILE, null))) + } else if (KeyPairAuthenticator.AUTH_NAME.Equals(authenticator) + && properties.TryGetValue(SFSessionProperty.PRIVATE_KEY_FILE, out var privateKeyFile) + && !string.IsNullOrEmpty(privateKeyFile)) { - DisablePoolingIfNotExplicitlyEnabled(properties, "jwt token with private key in a file"); + DisablePoolingIfNotExplicitlyEnabled(properties, "key pair with private key in a file"); } } From edb50c280004a3d3a28047b961d5c685c9009066 Mon Sep 17 00:00:00 2001 From: Krzysztof Nozderko Date: Wed, 15 May 2024 14:37:26 +0000 Subject: [PATCH 5/5] Accept pooling for key files with password --- .../UnitTests/ConnectionPoolManagerTest.cs | 12 ++++++++++++ .../Session/ConnectionPoolConfigExtractorTest.cs | 2 ++ .../Core/Session/SFSessionHttpClientProperties.cs | 4 ++-- Snowflake.Data/Core/Session/SFSessionProperty.cs | 6 +++--- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerTest.cs b/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerTest.cs index b7d29c1b1..70efa47fb 100644 --- a/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerTest.cs @@ -211,6 +211,18 @@ public void TestGetPoolingOnManagerLevelAlwaysTrue() Assert.IsFalse(sessionPool2.GetPooling()); } + [Test] + [TestCase("authenticator=externalbrowser;account=test;user=test;")] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key_file=/some/file.key")] + public void TestDisabledPoolingWhenSecretesProvidedExternally(string connectionString) + { + // act + var pool = _connectionPoolManager.GetPool(connectionString, null); + + // assert + Assert.IsFalse(pool.GetPooling()); + } + [Test] public void TestGetTimeoutOnManagerLevelWhenNotAllPoolsEqual() { diff --git a/Snowflake.Data.Tests/UnitTests/Session/ConnectionPoolConfigExtractorTest.cs b/Snowflake.Data.Tests/UnitTests/Session/ConnectionPoolConfigExtractorTest.cs index e27a12866..0cc61f28b 100644 --- a/Snowflake.Data.Tests/UnitTests/Session/ConnectionPoolConfigExtractorTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Session/ConnectionPoolConfigExtractorTest.cs @@ -230,6 +230,8 @@ public void TestExtractPoolingEnabled(string propertyValue, bool poolingEnabled) [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key=secretKey", true)] [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key=secretKey;poolingEnabled=true;", true)] [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key=secretKey;poolingEnabled=false;", false)] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key_file=/some/file.key;private_key_pwd=secretPwd", true)] + [TestCase("authenticator=snowflake_jwt;account=test;user=test;private_key_file=/some/file.key;private_key_pwd=", false)] public void TestDisablePoolingDefaultWhenSecretsProvidedExternally(string connectionString, bool poolingEnabled) { // act diff --git a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs index 6252084ec..2ba4709e7 100644 --- a/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs +++ b/Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs @@ -55,8 +55,8 @@ public void DisablePoolingDefaultIfSecretsProvidedExternally(SFSessionProperties DisablePoolingIfNotExplicitlyEnabled(properties, "external browser"); } else if (KeyPairAuthenticator.AUTH_NAME.Equals(authenticator) - && properties.TryGetValue(SFSessionProperty.PRIVATE_KEY_FILE, out var privateKeyFile) - && !string.IsNullOrEmpty(privateKeyFile)) + && properties.IsNonEmptyValueProvided(SFSessionProperty.PRIVATE_KEY_FILE) + && !properties.IsNonEmptyValueProvided(SFSessionProperty.PRIVATE_KEY_PWD)) { DisablePoolingIfNotExplicitlyEnabled(properties, "key pair with private key in a file"); } diff --git a/Snowflake.Data/Core/Session/SFSessionProperty.cs b/Snowflake.Data/Core/Session/SFSessionProperty.cs index cea067025..12b650ce6 100644 --- a/Snowflake.Data/Core/Session/SFSessionProperty.cs +++ b/Snowflake.Data/Core/Session/SFSessionProperty.cs @@ -256,7 +256,7 @@ internal static SFSessionProperties ParseConnectionString(string connectionStrin } ValidateAuthenticator(properties); - properties.IsPoolingEnabledValueProvided = CheckPoolingEnabledValueProvided(properties); + properties.IsPoolingEnabledValueProvided = properties.IsNonEmptyValueProvided(SFSessionProperty.POOLINGENABLED); CheckSessionProperties(properties); ValidateFileTransferMaxBytesInMemoryProperty(properties); ValidateAccountDomain(properties); @@ -310,8 +310,8 @@ private static void ValidateAuthenticator(SFSessionProperties properties) } } - private static bool CheckPoolingEnabledValueProvided(SFSessionProperties properties) => - properties.TryGetValue(SFSessionProperty.POOLINGENABLED, out var poolingEnabledStr) && !string.IsNullOrEmpty(poolingEnabledStr); + internal bool IsNonEmptyValueProvided(SFSessionProperty property) => + TryGetValue(property, out var propertyValueStr) && !string.IsNullOrEmpty(propertyValueStr); private static string BuildConnectionStringWithoutSecrets(ref string[] keys, ref string[] values) {