From a9cec2fdf95749dc4b3699822dd07236fa550f82 Mon Sep 17 00:00:00 2001 From: Juan Martinez Ramirez Date: Mon, 21 Oct 2024 15:31:26 -0600 Subject: [PATCH] Applying PR suggestions --- .../UnitTests/ConnectionPoolManagerMFATest.cs | 36 +++++-------------- .../SFCredentialManagerTest.cs | 22 ++++++------ Snowflake.Data/Core/Session/SessionPool.cs | 10 +++--- Snowflake.Data/Core/Tools/StringUtils.cs | 5 +-- 4 files changed, 26 insertions(+), 47 deletions(-) diff --git a/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerMFATest.cs b/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerMFATest.cs index c7970fe6e..a739e759e 100644 --- a/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerMFATest.cs +++ b/Snowflake.Data.Tests/UnitTests/ConnectionPoolManagerMFATest.cs @@ -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; @@ -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(() =>_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(() =>_connectionPoolManager.GetSession(connectionString, null,null)); + var thrown = Assert.Throws(() =>_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")); } diff --git a/Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerTest.cs b/Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerTest.cs index 663e059fc..f0aeb944e 100644 --- a/Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerTest.cs +++ b/Snowflake.Data.Tests/UnitTests/CredentialManager/SFCredentialManagerTest.cs @@ -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; diff --git a/Snowflake.Data/Core/Session/SessionPool.cs b/Snowflake.Data/Core/Session/SessionPool.cs index 8ba301b4b..95e5842ba 100644 --- a/Snowflake.Data/Core/Session/SessionPool.cs +++ b/Snowflake.Data/Core/Session/SessionPool.cs @@ -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; @@ -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) && @@ -170,7 +170,7 @@ 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); } } @@ -178,7 +178,7 @@ internal async Task GetSessionAsync(string connStr, SecureString pass { 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; diff --git a/Snowflake.Data/Core/Tools/StringUtils.cs b/Snowflake.Data/Core/Tools/StringUtils.cs index 329a3bf27..47cb8d953 100644 --- a/Snowflake.Data/Core/Tools/StringUtils.cs +++ b/Snowflake.Data/Core/Tools/StringUtils.cs @@ -14,9 +14,10 @@ internal static string ToSha256Hash(this string text) if (string.IsNullOrEmpty(text)) return string.Empty; - 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); } } }