From d5237fc0a9384260ea7da75997288f81fb99c6b0 Mon Sep 17 00:00:00 2001 From: Juan Martinez Ramirez Date: Tue, 3 Sep 2024 15:11:38 -0600 Subject: [PATCH] Applying additional PR suggestions --- .../IntegrationTests/SFConnectionIT.cs | 35 ++--- .../SFConnectionWithTomlIT.cs | 134 ++++++++++++++++++ .../Snowflake.Data.Tests.csproj | 1 + .../SnowflakeTomlConnectionBuilderTest.cs | 34 +++++ .../UnitTests/Tools/FileOperationsTest.cs | 21 +-- .../UnitTests/Tools/UnixOperationsTest.cs | 23 ++- Snowflake.Data/Core/TomlConnectionBuilder.cs | 5 +- doc/Connecting.md | 8 ++ 8 files changed, 214 insertions(+), 47 deletions(-) create mode 100644 Snowflake.Data.Tests/IntegrationTests/SFConnectionWithTomlIT.cs diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs index db9582aae..c387cdc59 100644 --- a/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs +++ b/Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs @@ -2,25 +2,24 @@ * Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved. */ +using System; +using System.Data; using System.Data.Common; +using System.Diagnostics; using System.Net; +using System.Runtime.InteropServices; +using System.Threading; +using System.Threading.Tasks; +using NUnit.Framework; +using Snowflake.Data.Client; +using Snowflake.Data.Core; using Snowflake.Data.Core.Session; +using Snowflake.Data.Log; +using Snowflake.Data.Tests.Mock; using Snowflake.Data.Tests.Util; namespace Snowflake.Data.Tests.IntegrationTests { - using NUnit.Framework; - using Snowflake.Data.Client; - using System.Data; - using System; - using Snowflake.Data.Core; - using System.Threading.Tasks; - using System.Threading; - using Snowflake.Data.Log; - using System.Diagnostics; - using Snowflake.Data.Tests.Mock; - using System.Runtime.InteropServices; - using System.Net.Http; [TestFixture] class SFConnectionIT : SFBaseTest @@ -2262,18 +2261,6 @@ public void TestConnectStringWithQueryTag() } } - - [Test] - [IgnoreOnCI("This test requires a valid connection string in the configuration file.")] - public void TestLocalDefaultConnectStringReadFromToml() - { - using (var conn = new SnowflakeDbConnection()) - { - conn.Open(); - Assert.AreEqual(ConnectionState.Open, conn.State); - } - } - [Test] public void TestUseMultiplePoolsConnectionPoolByDefault() { diff --git a/Snowflake.Data.Tests/IntegrationTests/SFConnectionWithTomlIT.cs b/Snowflake.Data.Tests/IntegrationTests/SFConnectionWithTomlIT.cs new file mode 100644 index 000000000..a24db761e --- /dev/null +++ b/Snowflake.Data.Tests/IntegrationTests/SFConnectionWithTomlIT.cs @@ -0,0 +1,134 @@ +/* + * Copyright (c) 2024 Snowflake Computing Inc. All rights reserved. + */ + +using System; +using System.Data; +using System.IO; +using System.Runtime.InteropServices; +using Mono.Unix.Native; +using NUnit.Framework; +using Snowflake.Data.Client; +using Snowflake.Data.Core; +using Snowflake.Data.Log; +using Tomlyn; +using Tomlyn.Model; + +namespace Snowflake.Data.Tests.IntegrationTests +{ + + [TestFixture, NonParallelizable] + class SFConnectionWithTomlIT : SFBaseTest + { + private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger(); + + private static readonly string s_workingDirectory = Path.Combine(Path.GetTempPath(), "tomlconfig", Path.GetRandomFileName()); + + + [SetUp] + public new void BeforeTest() + { + if (!Directory.Exists(s_workingDirectory)) + { + Directory.CreateDirectory(s_workingDirectory); + } + var snowflakeHome = Environment.GetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome); + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome, s_workingDirectory); + } + + [TearDown] + public new void AfterTest() + { + Directory.Delete(s_workingDirectory, true); + } + + public static void CreateTomlConfigBaseOnConnectionString(string connectionString) + { + var tomlModel = new TomlTable(); + var properties = SFSessionProperties.ParseConnectionString(connectionString, null); + + var defaultTomlTable = new TomlTable(); + tomlModel.Add("default", defaultTomlTable); + + foreach (var property in properties) + { + defaultTomlTable.Add(property.Key.ToString(), property.Value); + } + + var filePath = Path.Combine(s_workingDirectory, "connections.toml"); + using (var writer = File.CreateText(filePath)) + { + writer.Write(Toml.FromModel(tomlModel)); + } + + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + return; + + Syscall.chmod(filePath, FilePermissions.S_IRUSR | FilePermissions.S_IWUSR); + + } + + [Test] + public void TestLocalDefaultConnectStringReadFromToml() + { + var snowflakeHome = Environment.GetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome); + CreateTomlConfigBaseOnConnectionString(ConnectionString); + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome, s_workingDirectory); + try + { + using (var conn = new SnowflakeDbConnection()) + { + conn.Open(); + Assert.AreEqual(ConnectionState.Open, conn.State); + } + } + finally + { + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome, snowflakeHome); + } + } + + [Test] + public void TestThrowExceptionIfTomlNotFoundWithOtherConnectionString() + { + var snowflakeHome = Environment.GetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome); + var connectionName = Environment.GetEnvironmentVariable(TomlConnectionBuilder.SnowflakeDefaultConnectionName); + CreateTomlConfigBaseOnConnectionString(ConnectionString); + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome, s_workingDirectory); + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeDefaultConnectionName, "notfoundconnection"); + try + { + using (var conn = new SnowflakeDbConnection()) + { + Assert.Throws(() => conn.Open(), "Unable to connect. Specified connection name does not exist in connections.toml"); + } + } + finally + { + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome, snowflakeHome); + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeDefaultConnectionName, connectionName); + } + } + + [Test] + public void TestThrowExceptionIfTomlFromNotFoundFromDbConnection() + { + var snowflakeHome = Environment.GetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome); + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome, s_workingDirectory); + try + { + using (var conn = new SnowflakeDbConnection()) + { + Assert.Throws(() => conn.Open(), "Error: Required property ACCOUNT is not provided"); + } + } + finally + { + Environment.SetEnvironmentVariable(TomlConnectionBuilder.SnowflakeHome, snowflakeHome); + } + } + } + +} + + diff --git a/Snowflake.Data.Tests/Snowflake.Data.Tests.csproj b/Snowflake.Data.Tests/Snowflake.Data.Tests.csproj index cc895154e..86da12b20 100644 --- a/Snowflake.Data.Tests/Snowflake.Data.Tests.csproj +++ b/Snowflake.Data.Tests/Snowflake.Data.Tests.csproj @@ -19,6 +19,7 @@ + diff --git a/Snowflake.Data.Tests/UnitTests/SnowflakeTomlConnectionBuilderTest.cs b/Snowflake.Data.Tests/UnitTests/SnowflakeTomlConnectionBuilderTest.cs index c19863028..370ae8cec 100644 --- a/Snowflake.Data.Tests/UnitTests/SnowflakeTomlConnectionBuilderTest.cs +++ b/Snowflake.Data.Tests/UnitTests/SnowflakeTomlConnectionBuilderTest.cs @@ -476,6 +476,40 @@ public void TestConnectionWithOauthAuthenticatorShouldNotIncludeTokenIfNullOrEmp // Assert Assert.AreEqual($"account=testaccountname;authenticator=oauth;", connectionString); } + + [Test] + [TestCase("\\\"password;default\\\"", "password;default")] + [TestCase("\\\"\\\"\\\"password;default\\\"", "\"password;default")] + [TestCase("p\\\"assworddefault", "p\"assworddefault")] + [TestCase("password\\\"default", "password\"default")] + [TestCase("password\'default", "password\'default")] + [TestCase("password=default", "password=default")] + [TestCase("\\\"pa=ss\\\"\\\"word;def\'ault\\\"", "pa=ss\"word;def\'ault")] + public void TestConnectionMapPropertiesWithSpecialCharacters(string passwordValueWithSpecialCharacter, string expectedValue) + { + // Arrange + var mockFileOperations = new Mock(); + var mockEnvironmentOperations = new Mock(); + mockEnvironmentOperations.Setup(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile)) + .Returns($"{Path.DirectorySeparatorChar}home"); + mockFileOperations.Setup(f => f.Exists(It.IsAny())).Returns(true); + mockFileOperations.Setup(f => f.ReadAllText(It.Is(p => p.Contains(".snowflake")), It.IsAny>())) + .Returns($@" +[default] +account = ""defaultaccountname"" +user = ""defaultusername"" +password = ""{passwordValueWithSpecialCharacter}"" +"); + + var reader = new TomlConnectionBuilder(mockFileOperations.Object, mockEnvironmentOperations.Object); + + // Act + var connectionString = reader.GetConnectionStringFromToml(); + var properties = SFSessionProperties.ParseConnectionString(connectionString, null); + + // Assert + Assert.AreEqual(expectedValue, properties[SFSessionProperty.PASSWORD]); + } } } diff --git a/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs b/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs index d390908d9..0a1ee6359 100644 --- a/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs @@ -4,6 +4,7 @@ using System; +using Snowflake.Data.Core; namespace Snowflake.Data.Tests.Tools { @@ -51,7 +52,7 @@ public void TestReadAllTextOnWindows() var filePath = CreateConfigTempFile(s_workingDirectory, content); // act - var result = s_fileOperations.ReadAllText(filePath, GetTestFileValidation()); + var result = s_fileOperations.ReadAllText(filePath, TomlConnectionBuilder.GetFileValidations()); // assert Assert.AreEqual(content, result); @@ -71,7 +72,7 @@ public void TestReadAllTextCheckingPermissions() Syscall.chmod(filePath, (FilePermissions)filePermissions); // act - var result = s_fileOperations.ReadAllText(filePath, GetTestFileValidation()); + var result = s_fileOperations.ReadAllText(filePath, TomlConnectionBuilder.GetFileValidations()); // assert Assert.AreEqual(content, result); @@ -91,22 +92,8 @@ public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadConfiguration Syscall.chmod(filePath, (FilePermissions)filePermissions); // act and assert - Assert.Throws(() => s_fileOperations.ReadAllText(filePath, GetTestFileValidation()), + Assert.Throws(() => s_fileOperations.ReadAllText(filePath, TomlConnectionBuilder.GetFileValidations()), "Attempting to read a file with too broad permissions assigned"); } - - private Action GetTestFileValidation() - { - return stream => - { - const FileAccessPermissions forbiddenPermissions = FileAccessPermissions.OtherReadWriteExecute | FileAccessPermissions.GroupReadWriteExecute; - if (stream.OwnerUser.UserId != Syscall.geteuid()) - throw new SecurityException("Attempting to read a file not owned by the effective user of the current process"); - if (stream.OwnerGroup.GroupId != Syscall.getegid()) - throw new SecurityException("Attempting to read a file not owned by the effective group of the current process"); - if ((stream.FileAccessPermissions & forbiddenPermissions) != 0) - throw new SecurityException("Attempting to read a file with too broad permissions assigned"); - }; - } } } diff --git a/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs b/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs index e47a12540..417e004f9 100644 --- a/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs +++ b/Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs @@ -103,9 +103,9 @@ public void TestReadAllTextCheckingPermissions() } [Test] - [TestCase(FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.OtherReadWriteExecute)] - [TestCase(FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.GroupReadWriteExecute)] - public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadAllText(FileAccessPermissions filePermissions) + public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadAllText([ValueSource(nameof(UserReadWritePermissions))] FilePermissions userPermissions, + [ValueSource(nameof(GroupOrOthersWritablePermissions))] FilePermissions groupOrOthersWritablePermissions, + [ValueSource(nameof(GroupOrOthersReadablePermissions))] FilePermissions groupOrOthersReadablePermissions) { if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) { @@ -113,7 +113,9 @@ public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadAllText(FileA } var content = "random text"; var filePath = CreateConfigTempFile(s_workingDirectory, content); - Syscall.chmod(filePath, (FilePermissions)filePermissions); + + var filePermissions = userPermissions | groupOrOthersWritablePermissions | groupOrOthersReadablePermissions; + Syscall.chmod(filePath, filePermissions); // act and assert Assert.Throws(() => s_unixOperations.ReadAllText(filePath, TomlConnectionBuilder.GetFileValidations()), "Attempting to read a file with too broad permissions assigned"); @@ -149,5 +151,18 @@ public static IEnumerable OtherNotWritablePermissions() yield return FilePermissions.S_IXOTH; yield return FilePermissions.S_IROTH | FilePermissions.S_IXOTH; } + + public static IEnumerable UserReadWritePermissions() + { + yield return FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR; + } + + public static IEnumerable GroupOrOthersReadablePermissions() + { + yield return 0; + yield return FilePermissions.S_IRGRP; + yield return FilePermissions.S_IROTH; + yield return FilePermissions.S_IRGRP | FilePermissions.S_IROTH; + } } } diff --git a/Snowflake.Data/Core/TomlConnectionBuilder.cs b/Snowflake.Data/Core/TomlConnectionBuilder.cs index 2af5f8dae..3a8ae6269 100644 --- a/Snowflake.Data/Core/TomlConnectionBuilder.cs +++ b/Snowflake.Data/Core/TomlConnectionBuilder.cs @@ -36,7 +36,7 @@ internal class TomlConnectionBuilder private readonly FileOperations _fileOperations; private readonly EnvironmentOperations _environmentOperations; - internal static readonly TomlConnectionBuilder Instance = new TomlConnectionBuilder(); + public static readonly TomlConnectionBuilder Instance = new TomlConnectionBuilder(); private TomlConnectionBuilder() : this(FileOperations.Instance, EnvironmentOperations.Instance) { @@ -118,7 +118,8 @@ private TomlTable GetTomlTableFromConfig(string tomlPath, string connectionName) } var connectionExists = toml.TryGetValue(connectionName, out var connection); - // In the case where the connection name is the default connection name and does not exist, we will not use the toml builder feature. + // Avoid handling error when default connection does not exist, user could not want to use toml configuration and forgot to provide the + // connection string, this error should be thrown later when the undefined connection string is used. if (!connectionExists && connectionName != DefaultConnectionName) { throw new Exception("Specified connection name does not exist in connections.toml"); diff --git a/doc/Connecting.md b/doc/Connecting.md index ef9cd5874..0999d6a58 100644 --- a/doc/Connecting.md +++ b/doc/Connecting.md @@ -356,6 +356,14 @@ The following examples show how you can include different types of special chara user = "fakeuser" password = "fake\"password" ``` + - In case that double quote is use with other character that requires be wrap with double quoted it shoud use \\"\\" for a ": + + ```toml + [default] + host = "fakeaccount.snowflakecomputing.com" + user = "fakeuser" + password = "\";fake\"\"password\"" + ``` - To include a semicolon (;):