Skip to content

Commit

Permalink
SNOW-937188 connection string validation fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-mhofman committed May 9, 2024
1 parent 1963f28 commit 8ac7f18
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 76 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using System.Net;
using NUnit.Framework;
using Snowflake.Data.Client;
using Snowflake.Data.Core;
using Snowflake.Data.Tests.Util;


namespace Snowflake.Data.Tests.UnitTests
{
[TestFixture]
public class AuthenticationPropertiesValidatorTest
{
private const string _necessaryNonAuthProperties = "account=a;";

[TestCase("authenticator=snowflake;user=test;password=test", null)]
[TestCase("authenticator=Snowflake;user=test", "test")]
[TestCase("authenticator=ExternalBrowser", null)]
[TestCase("authenticator=snowflake_jwt;user=test;private_key_file=key.file", null)]
[TestCase("authenticator=SNOWFLAKE_JWT;user=test;private_key=key", null)]
[TestCase("authenticator=Snowflake_jwt;user=test;private_key=key;private_key_pwd=test", null)]
[TestCase("authenticator=oauth;token=value", null)]
[TestCase("AUTHENTICATOR=HTTPS://SOMETHING.OKTA.COM;USER=TEST;PASSWORD=TEST", null)]
[TestCase("authenticator=https://something.oktapreview.com;user=test;password=test", null)]
[TestCase("authenticator=https://vanity.url/snowflake/okta;USER=TEST;PASSWORD=TEST", null)]
public void TestAuthPropertiesValid(string connectionString, string password)
{
// Arrange
var securePassword = string.IsNullOrEmpty(password) ? null : new NetworkCredential(string.Empty, password).SecurePassword;

// Act/Assert
Assert.DoesNotThrow(() => SFSessionProperties.ParseConnectionString(_necessaryNonAuthProperties + connectionString, securePassword));
}

[TestCase("authenticator=snowflake;", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property PASSWORD is not provided.")]
[TestCase("authenticator=snowflake;", "test", SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property USER is not provided")]
[TestCase("authenticator=snowflake;user=;password=", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property PASSWORD is not provided.")]
[TestCase("authenticator=snowflake;user=;", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property PASSWORD is not provided")]
[TestCase("authenticator=snowflake;user=;", "test", SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property USER is not provided")]
[TestCase("authenticator=snowflake_jwt;private_key_file=", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property USER is not provided")]
[TestCase("authenticator=snowflake_jwt;private_key=", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property USER is not provided")]
[TestCase("authenticator=snowflake_jwt;", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property USER is not provided")]
[TestCase("authenticator=oauth;TOKen=", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property TOKEN is not provided")]
[TestCase("authenticator=oauth;", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property TOKEN is not provided")]
[TestCase("authenticator=okta;user=;password=", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property PASSWORD is not provided")]
[TestCase("authenticator=okta;user=", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property PASSWORD is not provided")]
[TestCase("authenticator=okta;password=", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property PASSWORD is not provided")]
[TestCase("authenticator=okta;", null, SFError.MISSING_CONNECTION_PROPERTY, "Error: Required property PASSWORD is not provided")]
[TestCase("authenticator=unknown;", null, SFError.UNKNOWN_AUTHENTICATOR, "Unknown authenticator")]
[TestCase("authenticator=http://unknown.okta.com;", null, SFError.UNKNOWN_AUTHENTICATOR, "Unknown authenticator")]
[TestCase("authenticator=https://unknown;", null, SFError.UNKNOWN_AUTHENTICATOR, "Unknown authenticator")]
public void TestAuthPropertiesInvalid(string connectionString, string password, SFError expectedError, string expectedErrorMessage)
{
// Arrange
var securePassword = string.IsNullOrEmpty(password) ? null : new NetworkCredential(string.Empty, password).SecurePassword;

// Act
var exception = Assert.Throws<SnowflakeDbException>(() => SFSessionProperties.ParseConnectionString(_necessaryNonAuthProperties + connectionString, securePassword));

// Assert
SnowflakeDbExceptionAssert.HasErrorCode(exception, expectedError);
Assert.That(exception.Message.Contains(expectedErrorMessage), $"Expecting:\n\t{exception.Message}\nto contain:\n\t{expectedErrorMessage}");
}
}
}
8 changes: 4 additions & 4 deletions Snowflake.Data.Tests/Util/SnowflakeDbExceptionAssert.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ public static class SnowflakeDbExceptionAssert
{
public static void HasErrorCode(SnowflakeDbException exception, SFError sfError)
{
Assert.AreEqual(exception.ErrorCode, sfError.GetAttribute<SFErrorAttr>().errorCode);
Assert.AreEqual(sfError.GetAttribute<SFErrorAttr>().errorCode, exception.ErrorCode);
}

public static void HasErrorCode(Exception exception, SFError sfError)
{
Assert.NotNull(exception);
switch (exception)
{
case SnowflakeDbException snowflakeDbException:
Assert.AreEqual(snowflakeDbException.ErrorCode, sfError.GetAttribute<SFErrorAttr>().errorCode);
Assert.AreEqual(sfError.GetAttribute<SFErrorAttr>().errorCode, snowflakeDbException.ErrorCode);
break;
default:
Assert.Fail(exception.GetType() + " type is not " + typeof(SnowflakeDbException));
Expand All @@ -45,7 +45,7 @@ public static void HasHttpErrorCodeInExceptionChain(Exception exception, HttpSta
return he.Message.Contains(((int)expected).ToString());
#else
return he.StatusCode == expected;
#endif
#endif
default:
return false;
}
Expand Down

This file was deleted.

2 changes: 1 addition & 1 deletion Snowflake.Data/Core/Authenticator/IAuthenticator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ internal static IAuthenticator GetAuthenticator(SFSession session)
else if (type.Equals(KeyPairAuthenticator.AUTH_NAME, StringComparison.InvariantCultureIgnoreCase))
{
// Get private key path or private key from connection settings
if (!session.properties.TryGetValue(SFSessionProperty.PRIVATE_KEY_FILE, out var pkPath) &&
if ((!session.properties.TryGetValue(SFSessionProperty.PRIVATE_KEY_FILE, out var pkPath) || string.IsNullOrEmpty(pkPath)) &&
!session.properties.TryGetValue(SFSessionProperty.PRIVATE_KEY, out var pkContent))
{
// There is no PRIVATE_KEY_FILE defined, can't authenticate with key-pair
Expand Down
38 changes: 33 additions & 5 deletions Snowflake.Data/Core/Session/SFSessionProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ internal static SFSessionProperties ParseConnectionString(string connectionStrin
properties[SFSessionProperty.PASSWORD] = new NetworkCredential(string.Empty, password).Password;
}

checkSessionProperties(properties);
ValidateAuthenticator(properties);
CheckSessionProperties(properties);
ValidateFileTransferMaxBytesInMemoryProperty(properties);
ValidateAccountDomain(properties);

Expand Down Expand Up @@ -283,6 +284,21 @@ internal static SFSessionProperties ParseConnectionString(string connectionStrin
return properties;
}

private static void ValidateAuthenticator(SFSessionProperties properties)
{
var knownAuthenticators = new[] { BasicAuthenticator.AUTH_NAME, OktaAuthenticator.AUTH_NAME, OAuthAuthenticator.AUTH_NAME, KeyPairAuthenticator.AUTH_NAME };
if (properties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator))
{
authenticator = authenticator.ToLower();
if (!knownAuthenticators.Contains(authenticator) && !(authenticator.Contains("okta") && authenticator.StartsWith("https://")))
{
var error = $"Unknown authenticator: {authenticator}";
logger.Error(error);
throw new SnowflakeDbException(SFError.UNKNOWN_AUTHENTICATOR, authenticator);
}
}
}

private static string BuildConnectionStringWithoutSecrets(ref string[] keys, ref string[] values)
{
var count = keys.Length;
Expand Down Expand Up @@ -368,25 +384,31 @@ private static bool IsAccountRegexMatched(string account) =>
.Select(regex => Regex.Match(account, regex, RegexOptions.IgnoreCase))
.All(match => match.Success);

private static void checkSessionProperties(SFSessionProperties properties)
private static void CheckSessionProperties(SFSessionProperties properties)
{
foreach (SFSessionProperty sessionProperty in Enum.GetValues(typeof(SFSessionProperty)))
{
// if required property, check if exists in the dictionary
if (IsRequired(sessionProperty, properties) &&
!properties.ContainsKey(sessionProperty))
{
SnowflakeDbException e = new SnowflakeDbException(SFError.MISSING_CONNECTION_PROPERTY,
sessionProperty);
SnowflakeDbException e = new SnowflakeDbException(SFError.MISSING_CONNECTION_PROPERTY, sessionProperty);
logger.Error("Missing connection property", e);
throw e;
}

if (IsRequired(sessionProperty, properties) && string.IsNullOrEmpty(properties[sessionProperty]))
{
SnowflakeDbException e = new SnowflakeDbException(SFError.MISSING_CONNECTION_PROPERTY, sessionProperty);
logger.Error("Empty connection property", e);
throw e;
}

// add default value to the map
string defaultVal = sessionProperty.GetAttribute<SFSessionPropertyAttr>().defaultValue;
if (defaultVal != null && !properties.ContainsKey(sessionProperty))
{
logger.Debug($"Sesssion property {sessionProperty} set to default value: {defaultVal}");
logger.Debug($"Session property {sessionProperty} set to default value: {defaultVal}");
properties.Add(sessionProperty, defaultVal);
}
}
Expand Down Expand Up @@ -450,6 +472,12 @@ private static bool IsRequired(SFSessionProperty sessionProperty, SFSessionPrope
return !authenticatorDefined || !authenticatorsWithoutUsername
.Any(auth => auth.Equals(authenticator, StringComparison.OrdinalIgnoreCase));
}
else if (sessionProperty.Equals(SFSessionProperty.TOKEN))
{
var authenticatorDefined = properties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator);

return !authenticatorDefined || authenticator.Equals(OAuthAuthenticator.AUTH_NAME);
}
else
{
return sessionProperty.GetAttribute<SFSessionPropertyAttr>().required;
Expand Down
1 change: 0 additions & 1 deletion Snowflake.Data/Core/Session/SessionPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ private static Tuple<ConnectionPoolConfig, string> ExtractConfig(string connecti
try
{
var properties = SFSessionProperties.ParseConnectionString(connectionString, password);
AuthenticationPropertiesValidator.Validate(properties);
var extractedProperties = SFSessionHttpClientProperties.ExtractAndValidate(properties);
return Tuple.Create(extractedProperties.BuildConnectionPoolConfig(), properties.ConnectionStringWithoutSecrets);
}
Expand Down

0 comments on commit 8ac7f18

Please sign in to comment.