Skip to content

Commit

Permalink
Applying PR suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-jmartinezramirez committed Sep 3, 2024
1 parent 3c1b4e7 commit 7123341
Show file tree
Hide file tree
Showing 13 changed files with 233 additions and 216 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ private static void MockHomeDirectoryReturnsNull()
private static void MockFileFromEnvironmentalVariable()
{
t_environmentOperations
.Setup(e => e.GetEnvironmentVariable(EasyLoggingConfigFinder.ClientConfigEnvironmentName, null))
.Setup(e => e.GetEnvironmentVariable(EasyLoggingConfigFinder.ClientConfigEnvironmentName))
.Returns(EnvironmentalConfigFilePath);
}

Expand Down
22 changes: 11 additions & 11 deletions Snowflake.Data.Tests/UnitTests/SnowflakeDbConnectionTest.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@


using System;
using System.IO;
using Mono.Unix;

namespace Snowflake.Data.Tests.UnitTests
{
using Core;
Expand All @@ -16,37 +20,33 @@ public void TestFillConnectionStringFromTomlConfig()
// Arrange
var mockFileOperations = new Mock<FileOperations>();
var mockEnvironmentOperations = new Mock<EnvironmentOperations>();
mockEnvironmentOperations.Setup(e => e.GetEnvironmentVariable(It.IsAny<string>(), It.IsAny<string>()))
.Returns((string v, string d) => d);
mockEnvironmentOperations.Setup(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile))
.Returns($"{Path.DirectorySeparatorChar}home");
mockFileOperations.Setup(f => f.Exists(It.IsAny<string>())).Returns(true);
mockFileOperations.Setup(f => f.ReadAllText(It.IsAny<string>()))
mockFileOperations.Setup(f => f.ReadAllText(It.IsAny<string>(), It.IsAny<Action<UnixStream>>()))
.Returns("[default]\naccount=\"testaccount\"\nuser=\"testuser\"\npassword=\"testpassword\"\n");
var tomlConnectionBuilder = new SnowflakeTomlConnectionBuilder(mockFileOperations.Object, mockEnvironmentOperations.Object);
var tomlConnectionBuilder = new TomlConnectionBuilder(mockFileOperations.Object, mockEnvironmentOperations.Object);

// Act
using (var conn = new SnowflakeDbConnection(tomlConnectionBuilder))
{
conn.ConnectionString = "account=user1account;user=user1;password=user1password;";
conn.FillConnectionStringFromTomlConfigIfNotSet();
// Assert
Assert.AreNotEqual("account=testaccount;user=testuser;password=testpassword;", conn.ConnectionString);
Assert.AreNotEqual("account=testaccount;user=testuser;password=testpassword;", conn.ConnectionString);
Assert.AreEqual("account=testaccount;user=testuser;password=testpassword;", conn.ConnectionString);
}
}

[Test]
public void TestFillConnectionStringFromTomlConfigShouldNotBeExecutedIfAlreadySetConnectionString()
public void TestTomlConfigurationDoesNotOverrideExistingConnectionString()
{
// Arrange
var connectionTest = "account=user1account;user=user1;password=user1password;";
var mockFileOperations = new Mock<FileOperations>();
var mockEnvironmentOperations = new Mock<EnvironmentOperations>();
mockEnvironmentOperations.Setup(e => e.GetEnvironmentVariable(It.IsAny<string>(), It.IsAny<string>()))
.Returns((string v, string d) => d);
mockFileOperations.Setup(f => f.Exists(It.IsAny<string>())).Returns(true);
mockFileOperations.Setup(f => f.ReadAllText(It.IsAny<string>()))
.Returns("[default]\naccount=\"testaccount\"\nuser=\"testuser\"\npassword=\"testpassword\"\n");
var tomlConnectionBuilder = new SnowflakeTomlConnectionBuilder(mockFileOperations.Object, mockEnvironmentOperations.Object);
var tomlConnectionBuilder = new TomlConnectionBuilder(mockFileOperations.Object, mockEnvironmentOperations.Object);

// Act
using (var conn = new SnowflakeDbConnection(tomlConnectionBuilder))
Expand Down
147 changes: 52 additions & 95 deletions Snowflake.Data.Tests/UnitTests/SnowflakeTomlConnectionBuilderTest.cs

Large diffs are not rendered by default.

24 changes: 20 additions & 4 deletions Snowflake.Data.Tests/UnitTests/Tools/FileOperationsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
*/


using System;

namespace Snowflake.Data.Tests.Tools
{
using System.IO;
Expand Down Expand Up @@ -49,7 +51,7 @@ public void TestReadAllTextOnWindows()
var filePath = CreateConfigTempFile(s_workingDirectory, content);

// act
var result = s_fileOperations.ReadAllText(filePath);
var result = s_fileOperations.ReadAllText(filePath, GetTestFileValidation());

// assert
Assert.AreEqual(content, result);
Expand All @@ -69,14 +71,14 @@ public void TestReadAllTextCheckingPermissions()
Syscall.chmod(filePath, (FilePermissions)filePermissions);

// act
var result = s_fileOperations.ReadAllText(filePath);
var result = s_fileOperations.ReadAllText(filePath, GetTestFileValidation());

// assert
Assert.AreEqual(content, result);
}

[Test]
public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadAllText()
public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadConfigurationFile()
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Expand All @@ -89,8 +91,22 @@ public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadAllText()
Syscall.chmod(filePath, (FilePermissions)filePermissions);

// act and assert
Assert.Throws<SecurityException>(() => s_fileOperations.ReadAllText(filePath),
Assert.Throws<SecurityException>(() => s_fileOperations.ReadAllText(filePath, GetTestFileValidation()),
"Attempting to read a file with too broad permissions assigned");
}

private Action<UnixStream> 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");
};
}
}
}
13 changes: 7 additions & 6 deletions Snowflake.Data.Tests/UnitTests/Tools/UnixOperationsTest.cs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
using System.Collections.Generic;
using System.IO;
using System.Runtime.InteropServices;
using System.Security;
using Mono.Unix;
using Mono.Unix.Native;
using NUnit.Framework;
using Snowflake.Data.Core;
using Snowflake.Data.Core.Tools;
using static Snowflake.Data.Tests.UnitTests.Configuration.EasyLoggingConfigGenerator;

namespace Snowflake.Data.Tests.Tools
{
using System.Security;

[TestFixture, NonParallelizable]
public class UnixOperationsTest
{
Expand Down Expand Up @@ -96,26 +96,27 @@ public void TestReadAllTextCheckingPermissions()
Syscall.chmod(filePath, (FilePermissions)filePermissions);

// act
var result = s_unixOperations.ReadAllText(filePath);
var result = s_unixOperations.ReadAllText(filePath, TomlConnectionBuilder.GetFileValidations());

// assert
Assert.AreEqual(content, result);
}

[Test]
public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadAllText()
[TestCase(FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.OtherReadWriteExecute)]
[TestCase(FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.GroupReadWriteExecute)]
public void TestShouldThrowExceptionIfOtherPermissionsIsSetWhenReadAllText(FileAccessPermissions filePermissions)
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Assert.Ignore("skip test on Windows");
}
var content = "random text";
var filePath = CreateConfigTempFile(s_workingDirectory, content);
var filePermissions = FileAccessPermissions.UserReadWriteExecute | FileAccessPermissions.OtherReadWriteExecute;
Syscall.chmod(filePath, (FilePermissions)filePermissions);

// act and assert
Assert.Throws<SecurityException>(() => s_unixOperations.ReadAllText(filePath), "Attempting to read a file with too broad permissions assigned");
Assert.Throws<SecurityException>(() => s_unixOperations.ReadAllText(filePath, TomlConnectionBuilder.GetFileValidations()), "Attempting to read a file with too broad permissions assigned");
}

public static IEnumerable<FilePermissions> UserPermissions()
Expand Down
24 changes: 12 additions & 12 deletions Snowflake.Data/Client/SnowflakeDbConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
* Copyright (c) 2012-2021 Snowflake Computing Inc. All rights reserved.
*/

using System;
using System.Data;
using System.Data.Common;
using System.Security;
using System.Threading;
using System.Threading.Tasks;
using Snowflake.Data.Core;
using Snowflake.Data.Log;

namespace Snowflake.Data.Client
{
using System;
using System.Data.Common;
using Snowflake.Data.Core;
using System.Security;
using System.Threading.Tasks;
using System.Data;
using System.Threading;
using Snowflake.Data.Log;

[System.ComponentModel.DesignerCategory("Code")]
public class SnowflakeDbConnection : DbConnection
{
Expand All @@ -37,7 +37,7 @@ public class SnowflakeDbConnection : DbConnection
// Will fix that in a separated PR though as it's a different issue
private static Boolean _isArrayBindStageCreated;

private readonly SnowflakeTomlConnectionBuilder _tomlConnectionBuilder;
private readonly TomlConnectionBuilder _tomlConnectionBuilder;

protected enum TransactionRollbackStatus
{
Expand All @@ -46,7 +46,7 @@ protected enum TransactionRollbackStatus
Failure
}

public SnowflakeDbConnection() : this(new SnowflakeTomlConnectionBuilder())
public SnowflakeDbConnection() : this(TomlConnectionBuilder.Instance)
{
}

Expand All @@ -55,7 +55,7 @@ public SnowflakeDbConnection(string connectionString) : this()
ConnectionString = connectionString;
}

internal SnowflakeDbConnection(SnowflakeTomlConnectionBuilder tomlConnectionBuilder)
internal SnowflakeDbConnection(TomlConnectionBuilder tomlConnectionBuilder)
{
_tomlConnectionBuilder = tomlConnectionBuilder;
_connectionState = ConnectionState.Closed;
Expand Down
12 changes: 0 additions & 12 deletions Snowflake.Data/Core/EnvironmentVariables.cs

This file was deleted.

Loading

0 comments on commit 7123341

Please sign in to comment.