Skip to content

Commit

Permalink
SNOW-990111: Add easy logging improvements (#849)
Browse files Browse the repository at this point in the history
### Description
Modify easy logging based on list of security improvements

### Checklist
- [x] Code compiles correctly
- [x] Code is formatted according to [Coding
Conventions](../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
  • Loading branch information
sfc-gh-ext-simba-lf authored Feb 5, 2024
1 parent e517022 commit bd5d8da
Show file tree
Hide file tree
Showing 11 changed files with 689 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

using System;
using System.IO;
using System.Runtime.InteropServices;
using Mono.Unix;
using Moq;
using NUnit.Framework;
using Snowflake.Data.Configuration;
Expand All @@ -17,13 +19,16 @@ public class EasyLoggingConfigFinderTest
private const string InputConfigFilePath = "input_config.json";
private const string EnvironmentalConfigFilePath = "environmental_config.json";
private const string HomeDirectory = "/home/user";
private static readonly string s_driverConfigFilePath = Path.Combine(".", EasyLoggingConfigFinder.ClientConfigFileName);
private const string DriverDirectory = ".";
private static readonly string s_driverConfigFilePath = Path.Combine(DriverDirectory, EasyLoggingConfigFinder.ClientConfigFileName);
private static readonly string s_homeConfigFilePath = Path.Combine(HomeDirectory, EasyLoggingConfigFinder.ClientConfigFileName);
private static readonly string s_tempConfigFilePath = Path.Combine(Path.GetTempPath(), EasyLoggingConfigFinder.ClientConfigFileName);

[ThreadStatic]
private static Mock<FileOperations> t_fileOperations;

[ThreadStatic]
private static Mock<UnixOperations> t_unixOperations;

[ThreadStatic]
private static Mock<EnvironmentOperations> t_environmentOperations;

Expand All @@ -34,8 +39,9 @@ public class EasyLoggingConfigFinderTest
public void Setup()
{
t_fileOperations = new Mock<FileOperations>();
t_unixOperations = new Mock<UnixOperations>();
t_environmentOperations = new Mock<EnvironmentOperations>();
t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_environmentOperations.Object);
t_finder = new EasyLoggingConfigFinder(t_fileOperations.Object, t_unixOperations.Object, t_environmentOperations.Object);
MockHomeDirectory();
}

Expand All @@ -46,7 +52,6 @@ public void TestThatTakesFilePathFromTheInput()
MockFileFromEnvironmentalVariable();
MockFileOnDriverPath();
MockFileOnHomePath();
MockFileOnTempPath();

// act
var filePath = t_finder.FindConfigFilePath(InputConfigFilePath);
Expand All @@ -65,7 +70,6 @@ public void TestThatTakesFilePathFromEnvironmentVariableIfInputNotPresent(
MockFileFromEnvironmentalVariable();
MockFileOnDriverPath();
MockFileOnHomePath();
MockFileOnTempPath();

// act
var filePath = t_finder.FindConfigFilePath(inputFilePath);
Expand All @@ -80,7 +84,6 @@ public void TestThatTakesFilePathFromDriverLocationWhenNoInputParameterNorEnviro
// arrange
MockFileOnDriverPath();
MockFileOnHomePath();
MockFileOnTempPath();

// act
var filePath = t_finder.FindConfigFilePath(null);
Expand All @@ -94,7 +97,6 @@ public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarN
{
// arrange
MockFileOnHomePath();
MockFileOnTempPath();

// act
var filePath = t_finder.FindConfigFilePath(null);
Expand All @@ -104,16 +106,36 @@ public void TestThatTakesFilePathFromHomeLocationWhenNoInputParamEnvironmentVarN
}

[Test]
public void TestThatTakesFilePathFromTempDirectoryWhenNoOtherWaysPossible()
public void TestThatTakesFilePathFromHomeDirectoryWhenNoOtherWaysPossible()
{
// arrange
MockFileOnTempPath();
MockFileOnHomePath();

// act
var filePath = t_finder.FindConfigFilePath(null);


// assert
Assert.AreEqual(s_homeConfigFilePath, filePath);
}

[Test]
public void TestThatConfigFileIsNotUsedIfOthersCanModifyTheConfigFile()
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Assert.Ignore("skip test on Windows");
}

// arrange
MockFileOnHomePath();
MockHasFlagReturnsTrue();

// act
var thrown = Assert.Throws<Exception>(() => t_finder.FindConfigFilePath(null));

// assert
Assert.AreEqual(s_tempConfigFilePath, filePath);
Assert.IsNotNull(thrown);
Assert.AreEqual(thrown.Message, $"Error due to other users having permission to modify the config file: {s_homeConfigFilePath}");
}

[Test]
Expand All @@ -139,13 +161,28 @@ public void TestThatDoesNotFailWhenSearchForOneOfDirectoriesFails()
Assert.IsNull(filePath);
t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once);
}

[Test]
public void TestThatDoesNotFailWhenOneOfDirectoriesNotDefined()
public void TestThatDoesNotFailWhenHomeDirectoryReturnsNull()
{
// arrange
MockHomeDirectoryReturnsNull();

// act
var filePath = t_finder.FindConfigFilePath(null);

// assert
Assert.IsNull(filePath);
t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once);
}

[Test]
public void TestThatDoesNotFailWhenHomeDirectoryDoesNotExist()
{
// arrange
MockFileOnHomePath();
MockFileOnHomePathDoesNotExist();

// act
var filePath = t_finder.FindConfigFilePath(null);

Expand All @@ -154,6 +191,14 @@ public void TestThatDoesNotFailWhenOneOfDirectoriesNotDefined()
t_environmentOperations.Verify(e => e.GetFolderPath(Environment.SpecialFolder.UserProfile), Times.Once);
}

private static void MockHasFlagReturnsTrue()
{
t_unixOperations
.Setup(f => f.CheckFileHasPermissions(s_homeConfigFilePath,
It.Is<FileAccessPermissions>(p => p.Equals(FileAccessPermissions.GroupWrite | FileAccessPermissions.OtherWrite))))
.Returns(true);
}

private static void MockHomeDirectory()
{
t_environmentOperations
Expand All @@ -168,6 +213,13 @@ private static void MockHomeDirectoryFails()
.Throws(() => new Exception("No home directory"));
}

private static void MockFileOnHomePathDoesNotExist()
{
t_fileOperations
.Setup(f => f.Exists(s_homeConfigFilePath))
.Returns(false);
}

private static void MockHomeDirectoryReturnsNull()
{
t_environmentOperations
Expand Down Expand Up @@ -195,12 +247,5 @@ private static void MockFileOnHomePath()
.Setup(f => f.Exists(s_homeConfigFilePath))
.Returns(true);
}

private static void MockFileOnTempPath()
{
t_fileOperations
.Setup(f => f.Exists(s_tempConfigFilePath))
.Returns(true);
}
}
}
35 changes: 34 additions & 1 deletion Snowflake.Data.Tests/UnitTests/Logger/EasyLoggerManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using NUnit.Framework;
using Snowflake.Data.Configuration;
using Snowflake.Data.Core;
Expand All @@ -21,7 +22,7 @@ public class EasyLoggerManagerTest
private const string WarnMessage = "Easy logging Warn message";
private const string ErrorMessage = "Easy logging Error message";
private const string FatalMessage = "Easy logging Fatal message";
private static readonly string s_logsDirectory = Path.GetTempPath();
private static readonly string s_logsDirectory = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);

[ThreadStatic]
private static string t_directoryLogPath;
Expand Down Expand Up @@ -92,6 +93,38 @@ public void TestThatLogsToProperFileWithProperLogLevelOnly()
Assert.That(logLines, Has.Exactly(1).Matches<string>(s => s.Contains(FatalMessage)));
}

[Test]
public void TestThatOnlyUnknownFieldsAreLogged()
{
// arrange
string expectedFakeLogField = "fake_log_field";
string ConfigWithUnknownFields = $@"{{
""common"": {{
""LOG_LEVEL"": ""warn"",
""lOg_PaTh"": ""path"",
""{expectedFakeLogField}_1"": ""abc"",
""{expectedFakeLogField}_2"": ""123""
}}
}}";
var configFilePath = Guid.NewGuid().ToString() + ".json";
using (var writer = File.CreateText(configFilePath))
{
writer.Write(ConfigWithUnknownFields);
}
EasyLoggerManager.Instance.ReconfigureEasyLogging(EasyLoggingLogLevel.Warn, t_directoryLogPath);
var parser = new EasyLoggingConfigParser();

// act
parser.Parse(configFilePath);

// assert
var logLines = File.ReadLines(FindLogFilePath(t_directoryLogPath));
Assert.That(logLines, Has.Exactly(2).Matches<string>(s => s.Contains($"Unknown field from config: {expectedFakeLogField}")));

// cleanup
File.Delete(configFilePath);
}

private static string RandomLogsDirectoryPath()
{
var randomName = Path.GetRandomFileName();
Expand Down
Loading

0 comments on commit bd5d8da

Please sign in to comment.