SNOW-1373131 disable connection pooling for external browser authenti…

…cation (#947) ### Description Disable connection pooling for external browser authentication ### Checklist - [x] Code compiles correctly - [x] Code is formatted according to [Coding Conventions](../blob/master/CodingConventions.md) - [x] Created tests which fail without the change (if possible) - [x] All tests passing (`dotnet test`) - [x] Extended the README / documentation, if necessary - [x] Provide JIRA issue id (if possible) or GitHub issue id in PR name
snowflakedb · May 14, 2024 · fe51400 · fe51400
1 parent 97cceaf
commit fe51400
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 2 deletions.
diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
@@ -914,6 +914,30 @@ public void TestSSOConnectionWithUser()
                     + ";authenticator=externalbrowser;[email protected]";
                 conn.Open();
                 Assert.AreEqual(ConnectionState.Open, conn.State);
+
+                // connection pooling is disabled for external browser by default
+                Assert.AreEqual(false, SnowflakeDbConnectionPool.GetPool(conn.ConnectionString).GetPooling());
+                using (IDbCommand command = conn.CreateCommand())
+                {
+                    command.CommandText = "SELECT CURRENT_USER()";
+                    Assert.AreEqual("QA", command.ExecuteScalar().ToString());
+                }
+            }
+        }
+
+        [Test]
+        [Ignore("This test requires manual interaction and therefore cannot be run in CI")]
+        public void TestSSOConnectionWithPoolingEnabled()
+        {
+            // Use external browser to log in using proper password for [email protected]
+            using (IDbConnection conn = new SnowflakeDbConnection())
+            {
+                conn.ConnectionString
+                    = ConnectionStringWithoutAuth
+                      + ";authenticator=externalbrowser;[email protected];POOLINGENABLED=TRUE";
+                conn.Open();
+                Assert.AreEqual(ConnectionState.Open, conn.State);
+                Assert.AreEqual(true, SnowflakeDbConnectionPool.GetPool(conn.ConnectionString).GetPooling());
                 using (IDbCommand command = conn.CreateCommand())
                 {
                     command.CommandText = "SELECT CURRENT_USER()";

diff --git a/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs b/Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs
@@ -139,6 +139,40 @@ 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(OAuthAuthenticator.AUTH_NAME, "true")]
+        [TestCase(OktaAuthenticator.AUTH_NAME, "true")]
+        [TestCase(ExternalBrowserAuthenticator.AUTH_NAME, "false")]
+        public void TestDefaultPoolingEnabledForAuthenticator(string authenticator, string expectedPoolingEnabled)
+        {
+            // arrange
+            var connectionString = $"ACCOUNT=test;USER=test;PASSWORD=test;TOKEN=test;AUTHENTICATOR={authenticator}";
+
+            // act
+            var properties = SFSessionProperties.ParseConnectionString(connectionString, null);
+
+            // assert
+            Assert.AreEqual(expectedPoolingEnabled, properties[SFSessionProperty.POOLINGENABLED]);
+        }
+
         public static IEnumerable<TestCase> ConnectionStringTestCases()
         {
             string defAccount = "testaccount";
@@ -229,7 +263,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
                     { SFSessionProperty.CHANGEDSESSION, DefaultValue(SFSessionProperty.CHANGEDSESSION) },
                     { SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT, DefaultValue(SFSessionProperty.WAITINGFORIDLESESSIONTIMEOUT) },
                     { SFSessionProperty.EXPIRATIONTIMEOUT, DefaultValue(SFSessionProperty.EXPIRATIONTIMEOUT) },
-                    { SFSessionProperty.POOLINGENABLED, DefaultValue(SFSessionProperty.POOLINGENABLED) }
+                    { SFSessionProperty.POOLINGENABLED, "false" } // connection pooling is disabled for external browser authentication
                 }
             };
             var testCaseWithProxySettings = new TestCase()
@@ -616,6 +650,7 @@ public static IEnumerable<TestCase> ConnectionStringTestCases()
 
         private static string DefaultValue(SFSessionProperty property) =>
             property.GetAttribute<SFSessionPropertyAttr>().defaultValue;
+
         internal class TestCase
         {
             public string ConnectionString { get; set; }

diff --git a/Snowflake.Data/Core/Session/SFSessionProperty.cs b/Snowflake.Data/Core/Session/SFSessionProperty.cs
@@ -254,6 +254,7 @@ internal static SFSessionProperties ParseConnectionString(string connectionStrin
             }
 
             ValidateAuthenticator(properties);
+            DisableConnectionPoolingForExternalBrowser(properties);
             CheckSessionProperties(properties);
             ValidateFileTransferMaxBytesInMemoryProperty(properties);
             ValidateAccountDomain(properties);
@@ -287,7 +288,14 @@ internal static SFSessionProperties ParseConnectionString(string connectionStrin
 
         private static void ValidateAuthenticator(SFSessionProperties properties)
         {
-            var knownAuthenticators = new[] { BasicAuthenticator.AUTH_NAME, OktaAuthenticator.AUTH_NAME, OAuthAuthenticator.AUTH_NAME, KeyPairAuthenticator.AUTH_NAME, ExternalBrowserAuthenticator.AUTH_NAME };
+            var knownAuthenticators = new[] {
+                BasicAuthenticator.AUTH_NAME,
+                OktaAuthenticator.AUTH_NAME,
+                OAuthAuthenticator.AUTH_NAME,
+                KeyPairAuthenticator.AUTH_NAME,
+                ExternalBrowserAuthenticator.AUTH_NAME
+            };
+
             if (properties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator))
             {
                 authenticator = authenticator.ToLower();
@@ -300,6 +308,26 @@ private static void ValidateAuthenticator(SFSessionProperties properties)
             }
         }
 
+        private static void DisableConnectionPoolingForExternalBrowser(SFSessionProperties properties)
+        {
+            if (properties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator))
+            {
+                authenticator = authenticator.ToLower();
+                if (authenticator.Equals(ExternalBrowserAuthenticator.AUTH_NAME))
+                {
+                    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");
+                    }
+                }
+            }
+        }
+
         private static string BuildConnectionStringWithoutSecrets(ref string[] keys, ref string[] values)
         {
             var count = keys.Length;