diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index d506f9ba11..42bfa3bda1 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -2352,7 +2352,9 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) } else { - _fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken(); + // We use Task.Run here in all places to execute task synchronously in the same context. + // Fixes block-over-async deadlock possibilities https://github.com/dotnet/SqlClient/issues/1209 + _fedAuthToken = Task.Run(async () => await authProvider.AcquireTokenAsync(authParamsBuilder)).GetAwaiter().GetResult().ToSqlFedAuthToken(); _activeDirectoryAuthTimeoutRetryHelper.CachedToken = _fedAuthToken; } break; @@ -2368,7 +2370,7 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) else { authParamsBuilder.WithUserId(ConnectionOptions.UserID); - _fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken(); + _fedAuthToken = Task.Run(async () => await authProvider.AcquireTokenAsync(authParamsBuilder)).GetAwaiter().GetResult().ToSqlFedAuthToken(); _activeDirectoryAuthTimeoutRetryHelper.CachedToken = _fedAuthToken; } break; @@ -2384,13 +2386,13 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) { username = _credential.UserId; authParamsBuilder.WithUserId(username).WithPassword(_credential.Password); - _fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken(); + _fedAuthToken = Task.Run(async () => await authProvider.AcquireTokenAsync(authParamsBuilder)).GetAwaiter().GetResult().ToSqlFedAuthToken(); } else { username = ConnectionOptions.UserID; authParamsBuilder.WithUserId(username).WithPassword(ConnectionOptions.Password); - _fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken(); + _fedAuthToken = Task.Run(async () => await authProvider.AcquireTokenAsync(authParamsBuilder)).GetAwaiter().GetResult().ToSqlFedAuthToken(); } _activeDirectoryAuthTimeoutRetryHelper.CachedToken = _fedAuthToken; } @@ -2444,8 +2446,9 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) || _timeout.MillisecondsRemaining <= sleepInterval) { SqlClientEventSource.Log.TryTraceEvent(" {0}", msalException.ErrorCode); + // Error[0] - SqlErrorCollection sqlErs = new SqlErrorCollection(); + SqlErrorCollection sqlErs = new(); sqlErs.Add(new SqlError(0, (byte)0x00, (byte)TdsEnums.MIN_ERROR_CLASS, ConnectionOptions.DataSource, StringsHelper.GetString(Strings.SQL_MSALFailure, username, ConnectionOptions.Authentication.ToString("G")), ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0)); // Error[1] @@ -2467,6 +2470,11 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) Thread.Sleep(sleepInterval); sleepInterval *= 2; } + // All other exceptions from MSAL/Azure Identity APIs + catch (Exception e) + { + throw SqlException.CreateException(new() { new(0, (byte)0x00, (byte)TdsEnums.FATAL_ERROR_CLASS, ConnectionOptions.DataSource, e.Message, ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0) }, "", this, e); + } } Debug.Assert(_fedAuthToken != null, "fedAuthToken should not be null."); @@ -2632,7 +2640,7 @@ internal void OnFeatureExtAck(int featureId, byte[] data) Debug.Assert(_tceVersionSupported <= TdsEnums.MAX_SUPPORTED_TCE_VERSION, "Client support TCE version 2"); _parser.IsColumnEncryptionSupported = true; _parser.TceVersionSupported = _tceVersionSupported; - _parser.AreEnclaveRetriesSupported = _tceVersionSupported == 3; + _parser.AreEnclaveRetriesSupported = _tceVersionSupported == 3; if (data.Length > 1) { diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.Designer.cs b/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.Designer.cs index 291bead3d3..824deb1601 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.Designer.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.Designer.cs @@ -2833,7 +2833,7 @@ internal static string SQL_MSALFailure { } /// - /// Looks up a localized string similar to Error code 0x{0}; state {1}. + /// Looks up a localized string similar to Error code 0x{0}.. /// internal static string SQL_MSALInnerException { get { diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx b/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx index 2ae63630be..857ef5cddc 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx +++ b/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx @@ -1348,7 +1348,7 @@ Failed to authenticate the user {0} in Active Directory (Authentication={1}). - Error code 0x{0}; state {1} + Error code 0x{0} Internal connection fatal error. Error state: {0}. diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/DbConnectionStringCommon.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/DbConnectionStringCommon.cs index 49ebbb1ebf..0e1a941d06 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/DbConnectionStringCommon.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/DbConnectionStringCommon.cs @@ -705,7 +705,7 @@ internal static string ColumnEncryptionSettingToString(SqlConnectionColumnEncryp internal static bool IsValidAuthenticationTypeValue(SqlAuthenticationMethod value) { - Debug.Assert(Enum.GetNames(typeof(SqlAuthenticationMethod)).Length == 9, "SqlAuthenticationMethod enum has changed, update needed"); + Debug.Assert(Enum.GetNames(typeof(SqlAuthenticationMethod)).Length == 10, "SqlAuthenticationMethod enum has changed, update needed"); return value == SqlAuthenticationMethod.SqlPassword || value == SqlAuthenticationMethod.ActiveDirectoryPassword || value == SqlAuthenticationMethod.ActiveDirectoryIntegrated diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index b9b0a217e4..68695ae984 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -2790,7 +2790,9 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) } else { - fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken(); + // We use Task.Run here in all places to execute task synchronously in the same context. + // Fixes block-over-async deadlock possibilities https://github.com/dotnet/SqlClient/issues/1209 + fedAuthToken = Task.Run(async () => await authProvider.AcquireTokenAsync(authParamsBuilder)).GetAwaiter().GetResult().ToSqlFedAuthToken(); _activeDirectoryAuthTimeoutRetryHelper.CachedToken = fedAuthToken; } break; @@ -2806,7 +2808,7 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) else { authParamsBuilder.WithUserId(ConnectionOptions.UserID); - fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken(); + fedAuthToken = Task.Run(async () => await authProvider.AcquireTokenAsync(authParamsBuilder)).GetAwaiter().GetResult().ToSqlFedAuthToken(); _activeDirectoryAuthTimeoutRetryHelper.CachedToken = fedAuthToken; } break; @@ -2822,13 +2824,13 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) { username = _credential.UserId; authParamsBuilder.WithUserId(username).WithPassword(_credential.Password); - fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken(); + fedAuthToken = Task.Run(async () => await authProvider.AcquireTokenAsync(authParamsBuilder)).GetAwaiter().GetResult().ToSqlFedAuthToken(); } else { username = ConnectionOptions.UserID; authParamsBuilder.WithUserId(username).WithPassword(ConnectionOptions.Password); - fedAuthToken = authProvider.AcquireTokenAsync(authParamsBuilder).Result.ToSqlFedAuthToken(); + fedAuthToken = Task.Run(async () => await authProvider.AcquireTokenAsync(authParamsBuilder)).GetAwaiter().GetResult().ToSqlFedAuthToken(); } _activeDirectoryAuthTimeoutRetryHelper.CachedToken = fedAuthToken; } @@ -2906,6 +2908,11 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) Thread.Sleep(sleepInterval); sleepInterval *= 2; } + // All other exceptions from MSAL/Azure Identity APIs + catch (Exception e) + { + throw SqlException.CreateException(new() { new(0, (byte)0x00, (byte)TdsEnums.FATAL_ERROR_CLASS, ConnectionOptions.DataSource, e.Message, ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0) }, "", this, e); + } } Debug.Assert(fedAuthToken != null, "fedAuthToken should not be null."); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.Designer.cs b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.Designer.cs index 2644b0a713..72f7fa5243 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.Designer.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.Designer.cs @@ -9658,7 +9658,7 @@ internal static string SQL_MSALFailure { } /// - /// Looks up a localized string similar to Error code 0x{0}; state {1}. + /// Looks up a localized string similar to Error code 0x{0}.. /// internal static string SQL_MSALInnerException { get { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.resx index 893c22f01c..0d9e331900 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.resx +++ b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.resx @@ -2563,7 +2563,7 @@ Failed to authenticate the user {0} in Active Directory (Authentication={1}). - Error code 0x{0}; state {1} + Error code 0x{0} ChangePassword requires SQL Server 9.0 or later. diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs index 1f6ad4ddea..e3a1fea22d 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/DataCommon/DataTestUtility.cs @@ -728,6 +728,29 @@ public static string GetValueString(object paramValue) return paramValue.ToString(); } + public static string FetchKeyInConnStr(string connStr, string[] keys) + { + // tokenize connection string and find matching key + if (connStr != null && keys != null) + { + string[] connProps = connStr.Split(';'); + foreach (string cp in connProps) + { + if (!string.IsNullOrEmpty(cp.Trim())) + { + foreach (var key in keys) + { + if (cp.Trim().ToLower().StartsWith(key.Trim().ToLower())) + { + return cp.Substring(cp.IndexOf('=') + 1); + } + } + } + } + } + return null; + } + public static string RemoveKeysInConnStr(string connStr, string[] keysToRemove) { // tokenize connection string and remove input keys. 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 5723958636..e9849cccd9 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/ConnectivityTests/AADConnectionTest.cs @@ -161,10 +161,10 @@ public static void AADPasswordWithWrongPassword() string[] credKeys = { "Password", "PWD" }; string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) + "Password=TestPassword;"; - AggregateException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); + SqlException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); string expectedMessage = "ID3242: The security token could not be authenticated or authorized."; - Assert.Contains(expectedMessage, e.InnerException.InnerException.Message); + Assert.Contains(expectedMessage, e.Message); } @@ -241,7 +241,11 @@ public static void EmptyPasswordInConnStrAADPassword() // connection fails with expected error message. string[] pwdKey = { "Password", "PWD" }; string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, pwdKey) + "Password=;"; - Assert.Throws(() => ConnectAndDisconnect(connStr)); + SqlException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); + + string user = DataTestUtility.FetchKeyInConnStr(DataTestUtility.AADPasswordConnectionString, new string[] { "User Id", "UID" }); + string expectedMessage = string.Format("Failed to authenticate the user {0} in Active Directory (Authentication=ActiveDirectoryPassword).", user); + Assert.Contains(expectedMessage, e.Message); } [PlatformSpecific(TestPlatforms.Windows)] @@ -251,7 +255,10 @@ public static void EmptyCredInConnStrAADPassword() // connection fails with expected error message. string[] removeKeys = { "User ID", "Password", "UID", "PWD" }; string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=; Password=;"; - Assert.Throws(() => ConnectAndDisconnect(connStr)); + SqlException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); + + string expectedMessage = "Failed to authenticate the user in Active Directory (Authentication=ActiveDirectoryPassword)."; + Assert.Contains(expectedMessage, e.Message); } [PlatformSpecific(TestPlatforms.AnyUnix)] @@ -261,7 +268,10 @@ public static void EmptyCredInConnStrAADPasswordAnyUnix() // connection fails with expected error message. string[] removeKeys = { "User ID", "Password", "UID", "PWD" }; string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=; Password=;"; - Assert.Throws(() => ConnectAndDisconnect(connStr)); + SqlException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); + + string expectedMessage = "MSAL cannot determine the username (UPN) of the currently logged in user.For Integrated Windows Authentication and Username/Password flows, please use .WithUsername() before calling ExecuteAsync()."; + Assert.Contains(expectedMessage, e.Message); } [ConditionalFact(nameof(IsAADConnStringsSetup))] @@ -269,8 +279,12 @@ public static void AADPasswordWithInvalidUser() { // connection fails with expected error message. string[] removeKeys = { "User ID", "UID" }; - string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + "User ID=testdotnet@microsoft.com"; - Assert.Throws(() => ConnectAndDisconnect(connStr)); + string user = "testdotnet@domain.com"; + string connStr = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, removeKeys) + $"User ID={user}"; + SqlException e = Assert.Throws(() => ConnectAndDisconnect(connStr)); + + string expectedMessage = string.Format("Failed to authenticate the user {0} in Active Directory (Authentication=ActiveDirectoryPassword).", user); + Assert.Contains(expectedMessage, e.Message); } [ConditionalFact(nameof(IsAADConnStringsSetup))] @@ -386,7 +400,7 @@ public static void ActiveDirectoryManagedIdentityWithInvalidUserIdMustFail(strin string connStrWithNoCred = DataTestUtility.RemoveKeysInConnStr(DataTestUtility.AADPasswordConnectionString, credKeys) + $"Authentication=Active Directory Managed Identity; User Id={userId}"; - AggregateException e = Assert.Throws(() => ConnectAndDisconnect(connStrWithNoCred)); + SqlException e = Assert.Throws(() => ConnectAndDisconnect(connStrWithNoCred)); string expectedMessage = "ManagedIdentityCredential authentication unavailable"; Assert.Contains(expectedMessage, e.GetBaseException().Message);