From 8e4bfb27430e2741a1250a9be58439f79802ae58 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 24 Feb 2020 08:58:22 -0800 Subject: [PATCH 1/4] Initial test changes --- .../ManualTests/DataCommon/DataTestUtility.cs | 73 ++++++++++++++++++- .../ConnectivityTests/AADConnectionTest.cs | 27 ++----- .../tests/ManualTests/config.default.json | 2 +- 3 files changed, 77 insertions(+), 25 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index abbfa72bb0..9dd757ea03 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -11,9 +11,11 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Security; using System.Text; using System.Threading; using System.Threading.Tasks; +using Microsoft.Identity.Client; using Newtonsoft.Json; using Xunit; @@ -26,8 +28,9 @@ public static class DataTestUtility public static readonly string TCPConnectionStringHGSVBS = null; public static readonly string TCPConnectionStringAASVBS = null; public static readonly string TCPConnectionStringAASSGX = null; - public static readonly string AADAccessToken = null; + public static readonly string AADAuthorityURL = null; public static readonly string AADPasswordConnectionString = null; + public static readonly string AADAccessToken = null; public static readonly string AKVBaseUrl = null; public static readonly string AKVUrl = null; public static readonly string AKVClientId = null; @@ -60,7 +63,7 @@ private class Config public string TCPConnectionStringHGSVBS = null; public string TCPConnectionStringAASVBS = null; public string TCPConnectionStringAASSGX = null; - public string AADAccessToken = null; + public string AADAuthorityURL = null; public string AADPasswordConnectionString = null; public string AzureKeyVaultURL = null; public string AzureKeyVaultClientId = null; @@ -83,13 +86,20 @@ static DataTestUtility() TCPConnectionStringHGSVBS = c.TCPConnectionStringHGSVBS; TCPConnectionStringAASVBS = c.TCPConnectionStringAASVBS; TCPConnectionStringAASSGX = c.TCPConnectionStringAASSGX; - AADAccessToken = c.AADAccessToken; + AADAuthorityURL = c.AADAuthorityURL; AADPasswordConnectionString = c.AADPasswordConnectionString; SupportsLocalDb = c.SupportsLocalDb; SupportsIntegratedSecurity = c.SupportsIntegratedSecurity; SupportsFileStream = c.SupportsFileStream; EnclaveEnabled = c.EnclaveEnabled; + if (IsAADPasswordConnStrSetup()) + { + string username = RetrieveValueFromConnStr(AADPasswordConnectionString, new string[] { "User ID", "UID" }); + string password = RetrieveValueFromConnStr(AADPasswordConnectionString, new string[] { "Password", "PWD" }); + AADAccessToken = GenerateAccessToken(AADAuthorityURL, username, password); + } + string url = c.AzureKeyVaultURL; Uri AKVBaseUri = null; if (!string.IsNullOrEmpty(url) && Uri.TryCreate(url, UriKind.Absolute, out AKVBaseUri)) @@ -134,6 +144,41 @@ static DataTestUtility() } } + private static string GenerateAccessToken(string authorityURL, string aADAuthUserID, string aADAuthPassword) + { + return AcquireTokenAsync(authorityURL, aADAuthUserID, aADAuthPassword).Result; + } + + private static Task AcquireTokenAsync(string authorityURL, string userID, string password) => Task.Run(() => + { + // The below properties are set specific to test configurations. + string scope = "https://database.windows.net/.default"; + string applicationName = "Microsoft Data SqlClient Manual Tests"; + string clientVersion = "1.0.0.0"; + string adoClientId = "4d079b4c-cab7-4b7c-a115-8fd51b6f8239"; + + IPublicClientApplication app = PublicClientApplicationBuilder.Create(adoClientId) + .WithAuthority(authorityURL) + .WithClientName(applicationName) + .WithClientVersion(clientVersion) + .Build(); + AuthenticationResult result; + string[] scopes = new string[] { scope }; + + // Note: CorrelationId, which existed in ADAL, can not be set in MSAL (yet?). + // parameter.ConnectionId was passed as the CorrelationId in ADAL to aid support in troubleshooting. + // If/When MSAL adds CorrelationId support, it should be passed from parameters here, too. + + SecureString securePassword = new SecureString(); + + foreach (char c in password) + securePassword.AppendChar(c); + securePassword.MakeReadOnly(); + result = app.AcquireTokenByUsernamePassword(scopes, userID, securePassword).ExecuteAsync().Result; + + return result.AccessToken; + }); + public static bool IsDatabasePresent(string name) { AvailableDatabases = AvailableDatabases ?? new Dictionary(); @@ -519,5 +564,27 @@ public static string GetValueString(object paramValue) return paramValue.ToString(); } + + public static string RetrieveValueFromConnStr(string connStr, string[] keywords) + { + // tokenize connection string and retrieve value for a specific key. + string res = ""; + string[] keys = connStr.Split(';'); + foreach (var key in keys) + { + foreach (var keyword in keywords) + { + if (!string.IsNullOrEmpty(key.Trim())) + { + if (key.Trim().ToLower().StartsWith(keyword.Trim().ToLower())) + { + res = key.Substring(key.IndexOf('=') + 1).Trim(); + break; + } + } + } + } + return res; + } } } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs index 490137495c..ba6967fa86 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs @@ -48,32 +48,17 @@ private static string RemoveKeysInConnStr(string connStr, string[] keysToRemove) return res; } - private static string RetrieveValueFromConnStr(string connStr, string keyword) - { - // tokenize connection string and retrieve value for a specific key. - string res = ""; - string[] keys = connStr.Split(';'); - foreach (var key in keys) - { - if (!string.IsNullOrEmpty(key.Trim())) - { - if (key.Trim().ToLower().StartsWith(keyword.Trim().ToLower())) - { - res = key.Substring(key.IndexOf('=') + 1).Trim(); - break; - } - } - } - return res; - } - private static bool IsAccessTokenSetup() => DataTestUtility.IsAccessTokenSetup(); private static bool IsAADConnStringsSetup() => DataTestUtility.IsAADPasswordConnStrSetup(); [ConditionalFact(nameof(IsAccessTokenSetup), nameof(IsAADConnStringsSetup))] public static void AccessTokenTest() { - using (SqlConnection connection = new SqlConnection(DataTestUtility.TCPConnectionString)) + // Remove cred info and add invalid token + string[] credKeys = { "User ID", "Password", "UID", "PWD", "Authentication" }; + string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); + + using (SqlConnection connection = new SqlConnection(connStr)) { connection.AccessToken = DataTestUtility.GetAccessToken(); connection.Open(); @@ -227,7 +212,7 @@ public static void testADPasswordAuthentication() )) { string customerId = (string)sqlCommand.ExecuteScalar(); - string expected = RetrieveValueFromConnStr(DataTestUtility.AADPasswordConnectionString, "User ID"); + string expected = DataTestUtility.RetrieveValueFromConnStr(DataTestUtility.AADPasswordConnectionString, new string[] { "User ID", "UID" }); Assert.Equal(expected, customerId); } } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/config.default.json b/src/Microsoft.Data.SqlClient/tests/ManualTests/config.default.json index 27218a2f5e..263291d462 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/config.default.json +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/config.default.json @@ -5,7 +5,7 @@ "TCPConnectionStringAASVBS": "", "TCPConnectionStringAASSGX": "", "EnclaveEnabled": false, - "AADAccessToken": "", + "AADAuthorityURL": "", "AADPasswordConnectionString": "", "AzureKeyVaultURL": "", "AzureKeyVaultClientId": "", From 737ef2675cd2837a2e51076ee237365b411dbe85 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 24 Feb 2020 13:09:03 -0800 Subject: [PATCH 2/4] Fix access token behavior in connection pool --- .../Data/SqlClient/SqlConnectionPoolKey.cs | 2 +- .../Data/SqlClient/SqlConnectionPoolKey.cs | 2 +- .../ManualTests/DataCommon/DataTestUtility.cs | 41 +++++++++++-- .../ConnectionPoolTest/ConnectionPoolTest.cs | 38 ++++++++++++ .../ConnectivityTests/AADConnectionTest.cs | 61 ++++++------------- 5 files changed, 94 insertions(+), 50 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs index 6e8b98fee9..3b538405ba 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs @@ -63,7 +63,7 @@ internal string AccessToken public override bool Equals(object obj) { SqlConnectionPoolKey key = obj as SqlConnectionPoolKey; - return (key != null && _credential == key._credential && ConnectionString == key.ConnectionString && Object.ReferenceEquals(_accessToken, key._accessToken)); + return (key != null && _credential == key._credential && ConnectionString == key.ConnectionString && _accessToken == key._accessToken); } public override int GetHashCode() diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs index aea42e0b20..986966481e 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs @@ -110,7 +110,7 @@ public override bool Equals(object obj) return (key != null && _credential == key._credential && ConnectionString == key.ConnectionString && - Object.ReferenceEquals(_accessToken, key._accessToken) && + _accessToken == key._accessToken && _serverCertificateValidationCallback == key._serverCertificateValidationCallback && _clientCertificateRetrievalCallback == key._clientCertificateRetrievalCallback && _originalNetworkAddressInfo == key._originalNetworkAddressInfo); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 9dd757ea03..606a91e458 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -93,7 +93,7 @@ static DataTestUtility() SupportsFileStream = c.SupportsFileStream; EnclaveEnabled = c.EnclaveEnabled; - if (IsAADPasswordConnStrSetup()) + if (IsAADPasswordConnStrSetup() && IsAADAuthorityURLSetup()) { string username = RetrieveValueFromConnStr(AADPasswordConnectionString, new string[] { "User ID", "UID" }); string password = RetrieveValueFromConnStr(AADPasswordConnectionString, new string[] { "Password", "PWD" }); @@ -152,7 +152,7 @@ private static string GenerateAccessToken(string authorityURL, string aADAuthUse private static Task AcquireTokenAsync(string authorityURL, string userID, string password) => Task.Run(() => { // The below properties are set specific to test configurations. - string scope = "https://database.windows.net/.default"; + string scope = "https://database.windows.net//.default"; string applicationName = "Microsoft Data SqlClient Manual Tests"; string clientVersion = "1.0.0.0"; string adoClientId = "4d079b4c-cab7-4b7c-a115-8fd51b6f8239"; @@ -216,6 +216,11 @@ public static bool IsAADPasswordConnStrSetup() return !string.IsNullOrEmpty(AADPasswordConnectionString); } + public static bool IsAADAuthorityURLSetup() + { + return !string.IsNullOrEmpty(AADAuthorityURL); + } + public static bool IsNotAzureServer() { return AreConnStringsSetup() ? !DataTestUtility.IsAzureSqlServer(new SqlConnectionStringBuilder((DataTestUtility.TCPConnectionString)).DataSource) : true; @@ -293,10 +298,11 @@ public static string GetUniqueNameForSqlServer(string prefix) public static string GetAccessToken() { - return AADAccessToken; + // Creates a new Object Reference of Access Token - See GitHub Issue 438 + return (null != AADAccessToken) ? new string(AADAccessToken.ToCharArray()) : null; } - public static bool IsAccessTokenSetup() => string.IsNullOrEmpty(GetAccessToken()) ? false : true; + public static bool IsAccessTokenSetup() => !string.IsNullOrEmpty(GetAccessToken()); public static bool IsFileStreamSetup() => SupportsFileStream; @@ -565,6 +571,33 @@ public static string GetValueString(object paramValue) return paramValue.ToString(); } + public static string RemoveKeysInConnStr(string connStr, string[] keysToRemove) + { + // tokenize connection string and remove input keys. + string res = ""; + string[] keys = connStr.Split(';'); + foreach (var key in keys) + { + if (!string.IsNullOrEmpty(key.Trim())) + { + bool removeKey = false; + foreach (var keyToRemove in keysToRemove) + { + if (key.Trim().ToLower().StartsWith(keyToRemove.Trim().ToLower())) + { + removeKey = true; + break; + } + } + if (!removeKey) + { + res += key + ";"; + } + } + } + return res; + } + public static string RetrieveValueFromConnStr(string connStr, string[] keywords) { // tokenize connection string and retrieve value for a specific key. diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs index 5324039403..872d5d385c 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectionPoolTest/ConnectionPoolTest.cs @@ -64,8 +64,46 @@ private static void BasicConnectionPoolingTest(string connectionString) connection3.Close(); connectionPool.Cleanup(); + SqlConnection connection4 = new SqlConnection(connectionString); + connection4.Open(); + Assert.True(internalConnection.IsInternalConnectionOf(connection4), "New connection does not use same internal connection"); + Assert.True(connectionPool.ContainsConnection(connection4), "New connection is in a different pool"); + connection4.Close(); + } + + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.IsAADPasswordConnStrSetup), nameof(DataTestUtility.IsAADAuthorityURLSetup))] + public static void AccessTokenConnectionPoolingTest() + { + // Remove cred info and add invalid token + string[] credKeys = { "User ID", "Password", "UID", "PWD", "Authentication" }; + string connectionString = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); + SqlConnection connection = new SqlConnection(connectionString); + connection.AccessToken = DataTestUtility.GetAccessToken(); + connection.Open(); + InternalConnectionWrapper internalConnection = new InternalConnectionWrapper(connection); + ConnectionPoolWrapper connectionPool = new ConnectionPoolWrapper(connection); + connection.Close(); + + SqlConnection connection2 = new SqlConnection(connectionString); + connection2.AccessToken = DataTestUtility.GetAccessToken(); + connection2.Open(); + Assert.True(internalConnection.IsInternalConnectionOf(connection2), "New connection does not use same internal connection"); + Assert.True(connectionPool.ContainsConnection(connection2), "New connection is in a different pool"); + connection2.Close(); + + SqlConnection connection3 = new SqlConnection(connectionString + ";App=SqlConnectionPoolUnitTest;"); + connection3.AccessToken = DataTestUtility.GetAccessToken(); + connection3.Open(); + Assert.False(internalConnection.IsInternalConnectionOf(connection3), "Connection with different connection string uses same internal connection"); + Assert.False(connectionPool.ContainsConnection(connection3), "Connection with different connection string uses same connection pool"); + connection3.Close(); + + connectionPool.Cleanup(); + + SqlConnection connection4 = new SqlConnection(connectionString); + connection4.AccessToken = DataTestUtility.GetAccessToken(); connection4.Open(); Assert.True(internalConnection.IsInternalConnectionOf(connection4), "New connection does not use same internal connection"); Assert.True(connectionPool.ContainsConnection(connection4), "New connection is in a different pool"); diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs index ba6967fa86..0459fbc5df 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs @@ -21,33 +21,6 @@ private static void ConnectAndDisconnect(string connectionString, SqlCredential } } - private static string RemoveKeysInConnStr(string connStr, string[] keysToRemove) - { - // tokenize connection string and remove input keys. - string res = ""; - string[] keys = connStr.Split(';'); - foreach (var key in keys) - { - if (!string.IsNullOrEmpty(key.Trim())) - { - bool removeKey = false; - foreach (var keyToRemove in keysToRemove) - { - if (key.Trim().ToLower().StartsWith(keyToRemove.Trim().ToLower())) - { - removeKey = true; - break; - } - } - if (!removeKey) - { - res += key + ";"; - } - } - } - return res; - } - private static bool IsAccessTokenSetup() => DataTestUtility.IsAccessTokenSetup(); private static bool IsAADConnStringsSetup() => DataTestUtility.IsAADPasswordConnStrSetup(); @@ -56,7 +29,7 @@ public static void AccessTokenTest() { // Remove cred info and add invalid token string[] credKeys = { "User ID", "Password", "UID", "PWD", "Authentication" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); using (SqlConnection connection = new SqlConnection(connStr)) { @@ -70,7 +43,7 @@ public static void InvalidAccessTokenTest() { // Remove cred info and add invalid token string[] credKeys = { "User ID", "Password", "UID", "PWD", "Authentication" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); using (SqlConnection connection = new SqlConnection(connStr)) { @@ -87,7 +60,7 @@ public static void AccessTokenWithAuthType() { // Remove cred info and add invalid token string[] credKeys = { "User ID", "Password", "UID", "PWD" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); using (SqlConnection connection = new SqlConnection(connStr)) { @@ -104,7 +77,7 @@ public static void AccessTokenWithCred() { // Remove cred info and add invalid token string[] credKeys = { "Authentication" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); using (SqlConnection connection = new SqlConnection(connStr)) { @@ -121,7 +94,7 @@ public static void AccessTokenTestWithEmptyToken() { // Remove cred info and add invalid token string[] credKeys = { "User ID", "Password", "UID", "PWD", "Authentication" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); using (SqlConnection connection = new SqlConnection(connStr)) { @@ -138,7 +111,7 @@ public static void AccessTokenTestWithIntegratedSecurityTrue() { // Remove cred info and add invalid token string[] credKeys = { "User ID", "Password", "UID", "PWD", "Authentication" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) + "Integrated Security=True;"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) + "Integrated Security=True;"; using (SqlConnection connection = new SqlConnection(connStr)) { @@ -154,7 +127,7 @@ public static void InvalidAuthTypeTest() { // Remove cred info and add invalid token string[] credKeys = { "Authentication" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) + "Authentication=Active Directory Pass;"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) + "Authentication=Active Directory Pass;"; ArgumentException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); @@ -162,7 +135,7 @@ public static void InvalidAuthTypeTest() Assert.Contains(expectedMessage, e.Message); } - [ConditionalFact(nameof(IsAccessTokenSetup), nameof(IsAADConnStringsSetup))] + [ConditionalFact(nameof(IsAADConnStringsSetup))] public static void AADPasswordWithIntegratedSecurityTrue() { string connStr = DataTestUtility.AADPasswordConnectionString + "Integrated Security=True;"; @@ -177,7 +150,7 @@ public static void AADPasswordWithIntegratedSecurityTrue() public static void AADPasswordWithWrongPassword() { string[] credKeys = { "Password", "PWD" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) + "Password=TestPassword;"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) + "Password=TestPassword;"; AggregateException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); @@ -228,7 +201,7 @@ public static void ActiveDirectoryPasswordWithNoAuthType() { // connection fails with expected error message. string[] AuthKey = { "Authentication" }; - string connStrWithNoAuthType = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, AuthKey); + string connStrWithNoAuthType = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, AuthKey); SqlException e = Assert.Throws(() => ConnectAndDisconnect(connStrWithNoAuthType)); string expectedMessage = "Cannot open server \"microsoft.com\" requested by the login. The login failed."; @@ -241,7 +214,7 @@ public static void IntegratedAuthWithCred() { // connection fails with expected error message. string[] AuthKey = { "Authentication" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, AuthKey) + "Authentication=Active Directory Integrated;"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, AuthKey) + "Authentication=Active Directory Integrated;"; ArgumentException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); string expectedMessage = "Cannot use 'Authentication=Active Directory Integrated' with 'User ID', 'UID', 'Password' or 'PWD' connection string keywords."; @@ -254,7 +227,7 @@ public static void MFAAuthWithCred() { // connection fails with expected error message. string[] AuthKey = { "Authentication" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, AuthKey) + "Authentication=Active Directory Interactive;"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, AuthKey) + "Authentication=Active Directory Interactive;"; ArgumentException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); string expectedMessage = "Cannot use 'Authentication=Active Directory Interactive' with 'User ID', 'UID', 'Password' or 'PWD' connection string keywords."; @@ -266,7 +239,7 @@ public static void EmptyPasswordInConnStrAADPassword() { // connection fails with expected error message. string[] pwdKey = { "Password", "PWD" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, pwdKey) + "Password=;"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, pwdKey) + "Password=;"; AggregateException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); string expectedMessage = "ID3242: The security token could not be authenticated or authorized."; @@ -279,7 +252,7 @@ public static void EmptyCredInConnStrAADPasswordNetFx() { // connection fails with expected error message. string[] removeKeys = { "User ID", "Password", "UID", "PWD" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=; Password=;"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=; Password=;"; AggregateException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); string expectedMessage = "Failed to get user name"; @@ -293,7 +266,7 @@ public static void EmptyCredInConnStrAADPasswordNetCore() { // connection fails with expected error message. string[] removeKeys = { "User ID", "Password", "UID", "PWD" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=; Password=;"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=; Password=;"; AggregateException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); string expectedMessage = "Could not identify the user logged into the OS"; @@ -306,7 +279,7 @@ public static void AADPasswordWithInvalidUser() { // connection fails with expected error message. string[] removeKeys = { "User ID", "UID" }; - string connStr = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=testdotnet@microsoft.com"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=testdotnet@microsoft.com"; AggregateException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); string expectedMessage = "ID3242: The security token could not be authenticated or authorized."; @@ -321,7 +294,7 @@ public static void NoCredentialsActiveDirectoryPassword() // connection fails with expected error message. string[] credKeys = { "User ID", "Password", "UID", "PWD" }; - string connStrWithNoCred = RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); + string connStrWithNoCred = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys); InvalidOperationException e = Assert.Throws(() => ConnectAndDisconnect(connStrWithNoCred)); string expectedMessage = "Either Credential or both 'User ID' and 'Password' (or 'UID' and 'PWD') connection string keywords must be specified, if 'Authentication=Active Directory Password'."; From b5f10144c21a6059c192f7e1d71d56fdb6aa4db5 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 24 Feb 2020 15:03:19 -0800 Subject: [PATCH 3/4] Compare ordinals for strings --- .../src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs | 5 ++++- .../src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs index 3b538405ba..ef44a133a3 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs @@ -63,7 +63,10 @@ internal string AccessToken public override bool Equals(object obj) { SqlConnectionPoolKey key = obj as SqlConnectionPoolKey; - return (key != null && _credential == key._credential && ConnectionString == key.ConnectionString && _accessToken == key._accessToken); + return (key != null + && _credential == key._credential + && string.CompareOrdinal(ConnectionString, key.ConnectionString) == 0 + && string.CompareOrdinal(_accessToken, key._accessToken) == 0); } public override int GetHashCode() diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs index 986966481e..e165839cb1 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs @@ -109,8 +109,8 @@ public override bool Equals(object obj) return (key != null && _credential == key._credential && - ConnectionString == key.ConnectionString && - _accessToken == key._accessToken && + string.CompareOrdinal(ConnectionString, key.ConnectionString) == 0 && + string.CompareOrdinal(_accessToken, key._accessToken) == 0 && _serverCertificateValidationCallback == key._serverCertificateValidationCallback && _clientCertificateRetrievalCallback == key._clientCertificateRetrievalCallback && _originalNetworkAddressInfo == key._originalNetworkAddressInfo); From 38aa9fa6181190de145ec01b1b8f937722322a08 Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Mon, 24 Feb 2020 15:05:28 -0800 Subject: [PATCH 4/4] Access token only --- .../src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs | 2 +- .../netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs index ef44a133a3..f9a43f0c02 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs @@ -65,7 +65,7 @@ public override bool Equals(object obj) SqlConnectionPoolKey key = obj as SqlConnectionPoolKey; return (key != null && _credential == key._credential - && string.CompareOrdinal(ConnectionString, key.ConnectionString) == 0 + && ConnectionString == key.ConnectionString && string.CompareOrdinal(_accessToken, key._accessToken) == 0); } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs index e165839cb1..46b77f8fb2 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlConnectionPoolKey.cs @@ -109,7 +109,7 @@ public override bool Equals(object obj) return (key != null && _credential == key._credential && - string.CompareOrdinal(ConnectionString, key.ConnectionString) == 0 && + ConnectionString == key.ConnectionString && string.CompareOrdinal(_accessToken, key._accessToken) == 0 && _serverCertificateValidationCallback == key._serverCertificateValidationCallback && _clientCertificateRetrievalCallback == key._clientCertificateRetrievalCallback &&