Skip to content

Commit

Permalink
Execute disabling pooling for specific authenticators for new pool only
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-knozderko committed May 15, 2024
1 parent 04d6533 commit e3a87d1
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 95 deletions.
58 changes: 1 addition & 57 deletions Snowflake.Data.Tests/UnitTests/SFSessionPropertyTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<TestCase> ConnectionStringTestCases()
{
string defAccount = "testaccount";
Expand Down Expand Up @@ -285,7 +229,7 @@ public static IEnumerable<TestCase> 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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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
{
Expand Down
26 changes: 26 additions & 0 deletions Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)))

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, GCP)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AZURE)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AWS)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, AZURE)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net471, AWS)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)

Check failure on line 57 in Snowflake.Data/Core/Session/SFSessionHttpClientProperties.cs

View workflow job for this annotation

GitHub Actions / Tests on Windows (net472, GCP)

'SFSessionProperties' does not contain a definition for 'GetValueOrDefault' and no accessible extension method 'GetValueOrDefault' accepting a first argument of type 'SFSessionProperties' could be found (are you missing a using directive or an assembly reference?)
{
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
Expand Down
36 changes: 5 additions & 31 deletions Snowflake.Data/Core/Session/SFSessionProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ class SFSessionProperties : Dictionary<SFSessionProperty, String>

internal string ConnectionStringWithoutSecrets { get; set; }

internal bool IsPoolingEnabledValueProvided { get; set; }

// Connection string properties to obfuscate in the log
private static readonly List<string> s_secretProps = Enum.GetValues(typeof(SFSessionProperty))
.Cast<SFSessionProperty>()
Expand Down Expand Up @@ -254,7 +256,7 @@ internal static SFSessionProperties ParseConnectionString(string connectionStrin
}

ValidateAuthenticator(properties);
DisableConnectionPoolingWhenSecretsPassedExternally(properties);
properties.IsPoolingEnabledValueProvided = CheckPoolingEnabledValueProvided(properties);
CheckSessionProperties(properties);
ValidateFileTransferMaxBytesInMemoryProperty(properties);
ValidateAccountDomain(properties);
Expand Down Expand Up @@ -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)
{
Expand Down
3 changes: 2 additions & 1 deletion Snowflake.Data/Core/Session/SessionPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,13 @@ private void CleanExpiredSessions()
}
}

private static Tuple<ConnectionPoolConfig, string> ExtractConfig(string connectionString, SecureString password)
internal static Tuple<ConnectionPoolConfig, string> 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)
Expand Down

0 comments on commit e3a87d1

Please sign in to comment.