Skip to content

Commit

Permalink
Applying new PR suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-jmartinezramirez committed Oct 18, 2024
1 parent 8dbab0f commit d199909
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 44 deletions.
35 changes: 20 additions & 15 deletions Snowflake.Data.Tests/IntegrationTests/SFConnectionIT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,25 @@
* 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.Core.Tools;
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;
using Snowflake.Data.Core.CredentialManager;
using Snowflake.Data.Core.CredentialManager.Infrastructure;

[TestFixture]
class SFConnectionIT : SFBaseTest
Expand Down Expand Up @@ -2279,11 +2276,15 @@ public void TestUseMultiplePoolsConnectionPoolByDefault()
[Ignore("This test requires manual interaction and therefore cannot be run in CI")]
public void TestMFATokenCachingWithPasscodeFromConnectionString()
{
// Use a connection with MFA enabled and set value of encode from mfa authenticator in the passcode property.
// ACCOUNT PARAMETER ALLOW_CLIENT_MFA_CACHING should be set to true in the account.
// On Mac/Linux OS default credential manager is in memory so please uncomment following line to use file based credential manager
// SnowflakeCredentialManagerFactory.SetCredentialManager(SnowflakeCredentialManagerFileImpl.Instance);
using (SnowflakeDbConnection conn = new SnowflakeDbConnection())
{
conn.ConnectionString
= ConnectionString
+ ";authenticator=username_password_mfa;application=DuoTest;Passcode=123456;";
+ ";authenticator=username_password_mfa;application=DuoTest;minPoolSize=0;";


// Authenticate to retrieve and store the token if doesn't exist or invalid
Expand All @@ -2297,6 +2298,10 @@ public void TestMFATokenCachingWithPasscodeFromConnectionString()
[Ignore("Requires manual steps and environment with mfa authentication enrolled")] // to enroll to mfa authentication edit your user profile
public void TestMfaWithPasswordConnectionUsingPasscodeWithSecureString()
{
// Use a connection with MFA enabled and set value of encode from mfa authenticator in the passcode property.
// ACCOUNT PARAMETER ALLOW_CLIENT_MFA_CACHING should be set to true in the account.
// On Mac/Linux OS default credential manager is in memory so please uncomment following line to use file based credential manager
// SnowflakeCredentialManagerFactory.SetCredentialManager(SnowflakeCredentialManagerFileImpl.Instance);
// arrange
using (SnowflakeDbConnection conn = new SnowflakeDbConnection())
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public class SFFileCredentialManagerTest : SFBaseCredentialManagerTest
[SetUp]
public void SetUp()
{
_credentialManager = SnowflakeCredentialManagerFileImpl.Instance;
_credentialManager = SFCredentialManagerFileImpl.Instance;
}
}

Expand All @@ -133,15 +133,15 @@ class SFCredentialManagerTest

private const string CustomJsonDir = "testdirectory";

private static readonly string s_customJsonPath = Path.Combine(CustomJsonDir, SnowflakeCredentialManagerFileImpl.CredentialCacheFileName);
private static readonly string s_customJsonPath = Path.Combine(CustomJsonDir, SFCredentialManagerFileImpl.CredentialCacheFileName);

[SetUp] public void SetUp()
{
t_fileOperations = new Mock<FileOperations>();
t_directoryOperations = new Mock<DirectoryOperations>();
t_unixOperations = new Mock<UnixOperations>();
t_environmentOperations = new Mock<EnvironmentOperations>();
SnowflakeCredentialManagerFactory.SetCredentialManager(SFCredentialManagerInMemoryImpl.Instance);
SnowflakeCredentialManagerFactory.UseInMemoryCredentialManager();
}

[TearDown] public void TearDown()
Expand Down Expand Up @@ -173,13 +173,26 @@ public void TestUsingDefaultCredentialManager()
public void TestSettingCustomCredentialManager()
{
// arrange
SnowflakeCredentialManagerFactory.SetCredentialManager(SnowflakeCredentialManagerFileImpl.Instance);
SnowflakeCredentialManagerFactory.SetCredentialManager(SFCredentialManagerFileImpl.Instance);

// act
_credentialManager = SnowflakeCredentialManagerFactory.GetCredentialManager();

// assert
Assert.IsInstanceOf<SnowflakeCredentialManagerFileImpl>(_credentialManager);
Assert.IsInstanceOf<SFCredentialManagerFileImpl>(_credentialManager);
}

[Test]
public void TestUseFileImplCredentialManager()
{
// arrange
SnowflakeCredentialManagerFactory.UseFileCredentialManager();

// act
_credentialManager = SnowflakeCredentialManagerFactory.GetCredentialManager();

// assert
Assert.IsInstanceOf<SFCredentialManagerFileImpl>(_credentialManager);
}

[Test]
Expand All @@ -199,9 +212,9 @@ public void TestThatThrowsErrorWhenCacheFileIsNotCreated()
FilePermissions.S_IRUSR | FilePermissions.S_IWUSR | FilePermissions.S_IXUSR))
.Returns(-1);
t_environmentOperations
.Setup(e => e.GetEnvironmentVariable(SnowflakeCredentialManagerFileImpl.CredentialCacheDirectoryEnvironmentName))
.Setup(e => e.GetEnvironmentVariable(SFCredentialManagerFileImpl.CredentialCacheDirectoryEnvironmentName))
.Returns(CustomJsonDir);
SnowflakeCredentialManagerFactory.SetCredentialManager(new SnowflakeCredentialManagerFileImpl(t_fileOperations.Object, t_directoryOperations.Object, t_unixOperations.Object, t_environmentOperations.Object));
SnowflakeCredentialManagerFactory.SetCredentialManager(new SFCredentialManagerFileImpl(t_fileOperations.Object, t_directoryOperations.Object, t_unixOperations.Object, t_environmentOperations.Object));
_credentialManager = SnowflakeCredentialManagerFactory.GetCredentialManager();

// act
Expand All @@ -228,9 +241,9 @@ public void TestThatThrowsErrorWhenCacheFileCanBeAccessedByOthers()
.Setup(u => u.GetFilePermissions(s_customJsonPath))
.Returns(FileAccessPermissions.AllPermissions);
t_environmentOperations
.Setup(e => e.GetEnvironmentVariable(SnowflakeCredentialManagerFileImpl.CredentialCacheDirectoryEnvironmentName))
.Setup(e => e.GetEnvironmentVariable(SFCredentialManagerFileImpl.CredentialCacheDirectoryEnvironmentName))
.Returns(CustomJsonDir);
SnowflakeCredentialManagerFactory.SetCredentialManager(new SnowflakeCredentialManagerFileImpl(t_fileOperations.Object, t_directoryOperations.Object, t_unixOperations.Object, t_environmentOperations.Object));
SnowflakeCredentialManagerFactory.SetCredentialManager(new SFCredentialManagerFileImpl(t_fileOperations.Object, t_directoryOperations.Object, t_unixOperations.Object, t_environmentOperations.Object));
_credentialManager = SnowflakeCredentialManagerFactory.GetCredentialManager();

// act
Expand All @@ -257,14 +270,14 @@ public void TestThatJsonFileIsCheckedIfAlreadyExists()
.Setup(u => u.GetFilePermissions(s_customJsonPath))
.Returns(FileAccessPermissions.UserReadWriteExecute);
t_environmentOperations
.Setup(e => e.GetEnvironmentVariable(SnowflakeCredentialManagerFileImpl.CredentialCacheDirectoryEnvironmentName))
.Setup(e => e.GetEnvironmentVariable(SFCredentialManagerFileImpl.CredentialCacheDirectoryEnvironmentName))
.Returns(CustomJsonDir);
t_fileOperations
.SetupSequence(f => f.Exists(s_customJsonPath))
.Returns(false)
.Returns(true);

SnowflakeCredentialManagerFactory.SetCredentialManager(new SnowflakeCredentialManagerFileImpl(t_fileOperations.Object, t_directoryOperations.Object, t_unixOperations.Object, t_environmentOperations.Object));
SnowflakeCredentialManagerFactory.SetCredentialManager(new SFCredentialManagerFileImpl(t_fileOperations.Object, t_directoryOperations.Object, t_unixOperations.Object, t_environmentOperations.Object));
_credentialManager = SnowflakeCredentialManagerFactory.GetCredentialManager();

// act
Expand Down
25 changes: 24 additions & 1 deletion Snowflake.Data/Client/SnowflakeCredentialManagerFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,33 @@ public static void UseDefaultCredentialManager()
}

public static void SetCredentialManager(ISnowflakeCredentialManager customCredentialManager)
{
SetCredentialManager(customCredentialManager, true);
}

public static void UseInMemoryCredentialManager()
{
SetCredentialManager(SFCredentialManagerInMemoryImpl.Instance, false);
}

public static void UseFileCredentialManager()
{
SetCredentialManager(SFCredentialManagerFileImpl.Instance, false);
}

public static void UseWindowsCredentialManager()
{
SetCredentialManager(SFCredentialManagerWindowsNativeImpl.Instance, false);
}

Check warning on line 53 in Snowflake.Data/Client/SnowflakeCredentialManagerFactory.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Client/SnowflakeCredentialManagerFactory.cs#L51-L53

Added lines #L51 - L53 were not covered by tests

internal static void SetCredentialManager(ISnowflakeCredentialManager customCredentialManager, bool isCustomCredential)
{
lock (credentialManagerLock)
{
s_logger.Info($"Setting the custom credential manager: {customCredentialManager.GetType().Name}");
var customText = isCustomCredential ? "custom " : string.Empty;
s_logger.Info(customCredentialManager == null
? $"Clearing the {customText}credential manager:"
: $"Setting the {customText}credential manager: {customCredentialManager?.GetType()?.Name}");
s_customCredentialManager = customCredentialManager;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@
using System.Runtime.InteropServices;
using KeyTokenDict = System.Collections.Generic.Dictionary<string, string>;

namespace Snowflake.Data.Core
namespace Snowflake.Data.Core.CredentialManager.Infrastructure
{
public class SnowflakeCredentialManagerFileImpl : ISnowflakeCredentialManager
internal class SFCredentialManagerFileImpl : ISnowflakeCredentialManager
{
internal const string CredentialCacheDirectoryEnvironmentName = "SF_TEMPORARY_CREDENTIAL_CACHE_DIR";

internal const string CredentialCacheDirName = ".snowflake";

internal const string CredentialCacheFileName = "temporary_credential.json";

private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger<SnowflakeCredentialManagerFileImpl>();
private static readonly SFLogger s_logger = SFLoggerFactory.GetLogger<SFCredentialManagerFileImpl>();

private readonly string _jsonCacheDirectory;

Expand All @@ -37,9 +37,9 @@ public class SnowflakeCredentialManagerFileImpl : ISnowflakeCredentialManager

private readonly EnvironmentOperations _environmentOperations;

public static readonly SnowflakeCredentialManagerFileImpl Instance = new SnowflakeCredentialManagerFileImpl(FileOperations.Instance, DirectoryOperations.Instance, UnixOperations.Instance, EnvironmentOperations.Instance);
public static readonly SFCredentialManagerFileImpl Instance = new SFCredentialManagerFileImpl(FileOperations.Instance, DirectoryOperations.Instance, UnixOperations.Instance, EnvironmentOperations.Instance);

internal SnowflakeCredentialManagerFileImpl(FileOperations fileOperations, DirectoryOperations directoryOperations, UnixOperations unixOperations, EnvironmentOperations environmentOperations)
internal SFCredentialManagerFileImpl(FileOperations fileOperations, DirectoryOperations directoryOperations, UnixOperations unixOperations, EnvironmentOperations environmentOperations)
{
_fileOperations = fileOperations;
_directoryOperations = directoryOperations;
Expand Down
2 changes: 0 additions & 2 deletions Snowflake.Data/Core/Session/SFSessionProperty.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ internal enum SFSessionProperty
POOLINGENABLED,
[SFSessionPropertyAttr(required = false, defaultValue = "false")]
DISABLE_SAML_URL_CHECK,
[SFSessionPropertyAttr(required = false, defaultValue = "false")]
CLIENT_REQUEST_MFA_TOKEN,
[SFSessionPropertyAttr(required = false, IsSecret = true)]
PASSCODE,
[SFSessionPropertyAttr(required = false, defaultValue = "false")]
Expand Down
9 changes: 4 additions & 5 deletions Snowflake.Data/Core/Session/SessionPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ internal async Task<SFSession> GetSessionAsync(string connStr, SecureString pass
if (!GetPooling())
return await NewNonPoolingSessionAsync(connStr, password, passcode, cancellationToken).ConfigureAwait(false);
var isMfaAuthentication = sessionProperties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator) && authenticator == MFACacheAuthenticator.AUTH_NAME;
var sessionOrCreateTokens = GetIdleSession(connStr, isMfaAuthentication ? 1 : Int32.MaxValue);
var sessionOrCreateTokens = GetIdleSession(connStr, isMfaAuthentication ? 1 : int.MaxValue);
WarnAboutOverridenConfig();

if (sessionOrCreateTokens.Session != null)
Expand Down Expand Up @@ -254,10 +254,9 @@ private SessionOrCreationTokens GetIdleSession(string connStr, int maxSessions)
return new SessionOrCreationTokens(session);
}
s_logger.Debug("SessionPool::GetIdleSession - no thread was waiting for a session, but could not find any idle session available in the pool" + PoolIdentification());
var sessionsCount = Math.Min(maxSessions, AllowedNumberOfNewSessionCreations(1));
var sessionsCount = AllowedNumberOfNewSessionCreations(1, maxSessions);
if (sessionsCount > 0)
{
s_logger.Debug($"SessionPool::GetIdleSession - register creation of {sessionsCount} sessions" + PoolIdentification());
// there is no need to wait for a session since we can create new ones
return new SessionOrCreationTokens(RegisterSessionCreations(sessionsCount));
}
Expand All @@ -277,7 +276,7 @@ private List<SessionCreationToken> RegisterSessionCreations(int sessionsCount) =
.Select(_ => _sessionCreationTokenCounter.NewToken())
.ToList();

private int AllowedNumberOfNewSessionCreations(int atLeastCount)
private int AllowedNumberOfNewSessionCreations(int atLeastCount, int maxSessionsLimit = int.MaxValue)
{
// we are expecting to create atLeast 1 session in case of opening a connection (atLeastCount = 1)
// but we have no expectations when closing a connection (atLeastCount = 0)
Expand All @@ -292,7 +291,7 @@ private int AllowedNumberOfNewSessionCreations(int atLeastCount)
{
var maxSessionsToCreate = _poolConfig.MaxPoolSize - currentSize;
var sessionsNeeded = Math.Max(_poolConfig.MinPoolSize - currentSize, atLeastCount);
var sessionsToCreate = Math.Min(sessionsNeeded, maxSessionsToCreate);
var sessionsToCreate = Math.Min(maxSessionsLimit, Math.Min(sessionsNeeded, maxSessionsToCreate));
s_logger.Debug($"SessionPool - allowed to create {sessionsToCreate} sessions, current pool size is {currentSize} out of {_poolConfig.MaxPoolSize}" + PoolIdentification());
return sessionsToCreate;
}
Expand Down
10 changes: 5 additions & 5 deletions Snowflake.Data/Core/Tools/UnixOperations.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@
*/


using System.IO;
using System.Security;
using System.Text;
using Mono.Unix;
using Mono.Unix.Native;

namespace Snowflake.Data.Core.Tools
{
using System.IO;
using System.Security;
using System.Text;
using Mono.Unix;
using Mono.Unix.Native;

internal class UnixOperations
{
Expand Down

0 comments on commit d199909

Please sign in to comment.