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
sfc-gh-jmartinezramirez committed Oct 21, 2024
1 parent be6f19d commit a9cec2f
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 47 deletions.
36 changes: 8 additions & 28 deletions Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerMFATest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class ConnectionPoolManagerMFATest
{
private readonly ConnectionPoolManager _connectionPoolManager = new ConnectionPoolManager();
private const string ConnectionStringMFACache = "db=D1;warehouse=W1;account=A1;user=U1;password=P1;role=R1;minPoolSize=2;passcode=12345;authenticator=username_password_mfa";
private const string ConnectionStringMFABasicWithoutPasscode = "db=D2;warehouse=W2;account=A2;user=U2;password=P2;role=R2;minPoolSize=3;";
private static PoolConfig s_poolConfig;
private static MockLoginMFATokenCacheRestRequester s_restRequester;

Expand Down Expand Up @@ -82,41 +81,22 @@ public void TestPoolManagerReturnsSessionPoolForGivenConnectionStringUsingMFA()
}

[Test]
public void TestPoolManagerShouldOnlyUsePasscodeAsArgumentForFirstSessionWhenNotUsingMFAAuthenticator()
public void TestPoolManagerShouldThrowExceptionIfForcePoolingWithPasscodeNotUsingMFATokenCacheAuthenticator()
{
// Arrange
const string TestPasscode = "123456";
s_restRequester.LoginResponses.Enqueue(new LoginResponseData()
{
authResponseSessionInfo = new SessionInfo()
});
s_restRequester.LoginResponses.Enqueue(new LoginResponseData()
{
authResponseSessionInfo = new SessionInfo()
});
s_restRequester.LoginResponses.Enqueue(new LoginResponseData()
{
authResponseSessionInfo = new SessionInfo()
});

// Act
_connectionPoolManager.GetSession(ConnectionStringMFABasicWithoutPasscode, null, SecureStringHelper.Encode(TestPasscode));

// Assert
Awaiter.WaitUntilConditionOrTimeout(() => s_restRequester.LoginRequests.Count == 3, TimeSpan.FromSeconds(15));
Assert.AreEqual(3, s_restRequester.LoginRequests.Count);
var request = s_restRequester.LoginRequests.ToList();
Assert.AreEqual(1, request.Count(r => r.data.extAuthnDuoMethod == "passcode" && r.data.passcode == TestPasscode));
Assert.AreEqual(2, request.Count(r => r.data.extAuthnDuoMethod == "push" && r.data.passcode == null));
var connectionString = "db=D1;warehouse=W1;account=A1;user=U1;password=P1;role=R1;minPoolSize=2;passcode=12345;POOLINGENABLED=true";
// Act and assert
var thrown = Assert.Throws<SnowflakeDbException>(() =>_connectionPoolManager.GetSession(connectionString, null,null));
Assert.That(thrown.Message, Does.Contain("Passcode with MinPoolSize feature of connection pool allowed only for username_password_mfa authentication"));
}

[Test]
public void TestPoolManagerShouldThrowExceptionIfForcePoolingWithPasscodeNotUsingMFATokenCacheAuthenticator()
public void TestPoolManagerShouldThrowExceptionIfForcePoolingWithPasscodeAsSecureStringNotUsingMFATokenCacheAuthenticator()
{
// Arrange
var connectionString = "db=D1;warehouse=W1;account=A1;user=U1;password=P1;role=R1;minPoolSize=2;passcode=12345;POOLINGENABLED=true";
var connectionString = "db=D1;warehouse=W1;account=A1;user=U1;password=P1;role=R1;minPoolSize=2;POOLINGENABLED=true";
// Act and assert
var thrown = Assert.Throws<Exception>(() =>_connectionPoolManager.GetSession(connectionString, null,null));
var thrown = Assert.Throws<SnowflakeDbException>(() =>_connectionPoolManager.GetSession(connectionString, null,SecureStringHelper.Encode("12345")));
Assert.That(thrown.Message, Does.Contain("Passcode with MinPoolSize feature of connection pool allowed only for username_password_mfa authentication"));
}

Expand Down
22 changes: 10 additions & 12 deletions Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,19 @@
* Copyright (c) 2024 Snowflake Computing Inc. All rights reserved.
*/

using Snowflake.Data.Core;
using System;
using System.IO;
using System.Runtime.InteropServices;
using Mono.Unix;
using Mono.Unix.Native;
using Moq;
using NUnit.Framework;
using Snowflake.Data.Client;
using Snowflake.Data.Core.CredentialManager.Infrastructure;
using Snowflake.Data.Core.Tools;

namespace Snowflake.Data.Tests.UnitTests.CredentialManager
{
using Mono.Unix;
using Mono.Unix.Native;
using Moq;
using NUnit.Framework;
using Snowflake.Data.Client;
using Snowflake.Data.Core.CredentialManager.Infrastructure;
using Snowflake.Data.Core.Tools;
using System;
using System.IO;
using System.Runtime.InteropServices;

public abstract class SFBaseCredentialManagerTest
{
protected ISnowflakeCredentialManager _credentialManager;
Expand Down
10 changes: 5 additions & 5 deletions Snowflake.Data/Core/Session/SessionPool.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ internal SFSession GetSession(string connStr, SecureString password, SecureStrin
{
s_logger.Debug("SessionPool::GetSession" + PoolIdentification());
var sessionProperties = SFSessionProperties.ParseConnectionString(connStr, password);
ValidateMinPoolSizeWithPasscode(sessionProperties);
ValidateMinPoolSizeWithPasscode(sessionProperties, passcode);
if (!GetPooling())
return NewNonPoolingSession(connStr, password, passcode);
var isMfaAuthentication = sessionProperties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator) && authenticator == MFACacheAuthenticator.AUTH_NAME;
Expand All @@ -158,10 +158,10 @@ internal SFSession GetSession(string connStr, SecureString password, SecureStrin
return session;
}

private void ValidateMinPoolSizeWithPasscode(SFSessionProperties sessionProperties)
private void ValidateMinPoolSizeWithPasscode(SFSessionProperties sessionProperties, SecureString passcode)
{
if (!GetPooling() || !IsMultiplePoolsVersion() || _poolConfig.MinPoolSize == 0) return;
var isUsingPasscode = (sessionProperties.IsNonEmptyValueProvided(SFSessionProperty.PASSCODE) ||
var isUsingPasscode = passcode != null || (sessionProperties.IsNonEmptyValueProvided(SFSessionProperty.PASSCODE) ||
(sessionProperties.TryGetValue(SFSessionProperty.PASSCODEINPASSWORD, out var passcodeInPasswordValue) &&
bool.TryParse(passcodeInPasswordValue, out var isPasscodeinPassword) && isPasscodeinPassword));
var isMfaAuthenticator = sessionProperties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator) &&
Expand All @@ -170,15 +170,15 @@ private void ValidateMinPoolSizeWithPasscode(SFSessionProperties sessionProperti
{
const string ErrorMessage = "Passcode with MinPoolSize feature of connection pool allowed only for username_password_mfa authentication";
s_logger.Error(ErrorMessage + PoolIdentification());
throw new Exception(ErrorMessage);
throw new SnowflakeDbException(SFError.INTERNAL_ERROR, ErrorMessage);
}
}

internal async Task<SFSession> GetSessionAsync(string connStr, SecureString password, SecureString passcode, CancellationToken cancellationToken)
{
s_logger.Debug("SessionPool::GetSessionAsync" + PoolIdentification());
var sessionProperties = SFSessionProperties.ParseConnectionString(connStr, password);
ValidateMinPoolSizeWithPasscode(sessionProperties);
ValidateMinPoolSizeWithPasscode(sessionProperties, passcode);
if (!GetPooling())
return await NewNonPoolingSessionAsync(connStr, password, passcode, cancellationToken).ConfigureAwait(false);
var isMfaAuthentication = sessionProperties.TryGetValue(SFSessionProperty.AUTHENTICATOR, out var authenticator) && authenticator == MFACacheAuthenticator.AUTH_NAME;
Expand Down
5 changes: 3 additions & 2 deletions Snowflake.Data/Core/Tools/StringUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ internal static string ToSha256Hash(this string text)
if (string.IsNullOrEmpty(text))
return string.Empty;

Check warning on line 15 in Snowflake.Data/Core/Tools/StringUtils.cs

View check run for this annotation

Codecov / codecov/patch

Snowflake.Data/Core/Tools/StringUtils.cs#L15 

Added line #L15 was not covered by tests

using (var sha = new SHA256Managed())
using (var sha256Encoder = SHA256.Create())
{
return BitConverter.ToString(sha.ComputeHash(System.Text.Encoding.UTF8.GetBytes(text))).Replace("-", string.Empty);
var sha256Hash = sha256Encoder.ComputeHash(System.Text.Encoding.UTF8.GetBytes(text));
return BitConverter.ToString(sha256Hash).Replace("-", string.Empty);
}
}
}
Expand Down

0 comments on commit a9cec2f

Please sign in to comment.