From 739697d0732d48d13e88836e66d98cd1ab74ceb5 Mon Sep 17 00:00:00 2001 From: David Engel Date: Fri, 5 Aug 2022 16:49:43 -0700 Subject: [PATCH] [Fix 3.1] Handle NRE on Azure federated authentication Porting #1625 to 3.1-servicing. --- .../src/Microsoft/Data/Common/AdapterUtil.cs | 33 ++++++++++++++++ .../SqlClient/SqlInternalConnectionTds.cs | 29 ++++++-------- .../src/Microsoft/Data/SqlClient/SqlUtil.cs | 4 ++ .../netcore/src/Resources/Strings.Designer.cs | 38 ++++++++++++------- .../netcore/src/Resources/Strings.resx | 3 ++ 5 files changed, 77 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/Common/AdapterUtil.cs index 19a9d2447b..de0d9288c8 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/Common/AdapterUtil.cs @@ -15,6 +15,7 @@ using System.Threading.Tasks; using System.Transactions; using Microsoft.Data.SqlClient; +using Microsoft.Identity.Client; namespace Microsoft.Data.Common { @@ -321,6 +322,38 @@ static internal InvalidOperationException InvalidDataDirectory() InvalidOperationException e = new InvalidOperationException(Strings.ADP_InvalidDataDirectory); return e; } + internal static TimeoutException TimeoutException(string error, Exception inner = null) + { + TimeoutException e = new(error, inner); + TraceExceptionAsReturnValue(e); + return e; + } + + internal static Exception CreateSqlException(MsalException msalException, SqlConnectionString connectionOptions, SqlInternalConnectionTds sender, string username) + { + // Error[0] + 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] + string errorMessage1 = StringsHelper.GetString(Strings.SQL_MSALInnerException, msalException.ErrorCode); + sqlErs.Add(new SqlError(0, (byte)0x00, (byte)TdsEnums.MIN_ERROR_CLASS, + connectionOptions.DataSource, errorMessage1, + ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0)); + + // Error[2] + if (!string.IsNullOrEmpty(msalException.Message)) + { + sqlErs.Add(new SqlError(0, (byte)0x00, (byte)TdsEnums.MIN_ERROR_CLASS, + connectionOptions.DataSource, msalException.Message, + ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0)); + } + return SqlException.CreateException(sqlErs, "", sender); + } // // Generic Data Provider Collection 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 d4c1f7435f..e4a9647a7e 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 @@ -103,6 +103,9 @@ public void AssertUnrecoverableStateCountIsCorrect() internal sealed class SqlInternalConnectionTds : SqlInternalConnection, IDisposable { + // https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/wiki/retry-after#simple-retry-for-errors-with-http-error-codes-500-600 + internal const int MsalHttpRetryStatusCode = 429; + // CONNECTION AND STATE VARIABLES private readonly SqlConnectionPoolGroupProviderInfo _poolGroupProviderInfo; // will only be null when called for ChangePassword, or creating SSE User Instance private TdsParser _parser; @@ -2421,7 +2424,7 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) // Deal with Msal service exceptions first, retry if 429 received. catch (MsalServiceException serviceException) { - if (429 == serviceException.StatusCode) + if (serviceException.StatusCode == MsalHttpRetryStatusCode) { RetryConditionHeaderValue retryAfter = serviceException.Headers.RetryAfter; if (retryAfter.Delta.HasValue) @@ -2440,9 +2443,15 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) } else { - break; + SqlClientEventSource.Log.TryTraceEvent(" Timeout: {0}", serviceException.ErrorCode); + throw SQL.ActiveDirectoryTokenRetrievingTimeout(Enum.GetName(typeof(SqlAuthenticationMethod), ConnectionOptions.Authentication), serviceException.ErrorCode, serviceException); } } + else + { + SqlClientEventSource.Log.TryTraceEvent(" {0}", serviceException.ErrorCode); + throw ADP.CreateSqlException(serviceException, ConnectionOptions, this, username); + } } // Deal with normal MsalExceptions. catch (MsalException msalException) @@ -2453,21 +2462,7 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo) { SqlClientEventSource.Log.TryTraceEvent(" {0}", msalException.ErrorCode); - // Error[0] - 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] - string errorMessage1 = StringsHelper.GetString(Strings.SQL_MSALInnerException, msalException.ErrorCode); - sqlErs.Add(new SqlError(0, (byte)0x00, (byte)TdsEnums.MIN_ERROR_CLASS, ConnectionOptions.DataSource, errorMessage1, ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0)); - - // Error[2] - if (!string.IsNullOrEmpty(msalException.Message)) - { - sqlErs.Add(new SqlError(0, (byte)0x00, (byte)TdsEnums.MIN_ERROR_CLASS, ConnectionOptions.DataSource, msalException.Message, ActiveDirectoryAuthentication.MSALGetAccessTokenFunctionName, 0)); - } - SqlException exc = SqlException.CreateException(sqlErs, "", this); - throw exc; + throw ADP.CreateSqlException(msalException, ConnectionOptions, this, username); } SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, sleeping {1}[Milliseconds]", ObjectID, sleepInterval); diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs index 0ede264635..c6b4500850 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs @@ -486,6 +486,10 @@ internal static Exception ActiveDirectoryDeviceFlowTimeout() return ADP.TimeoutException(Strings.SQL_Timeout_Active_Directory_DeviceFlow_Authentication); } + internal static Exception ActiveDirectoryTokenRetrievingTimeout(string authenticaton, string errorCode, Exception exception) + { + return ADP.TimeoutException(StringsHelper.GetString(Strings.AAD_Token_Retrieving_Timeout, authenticaton, errorCode, exception?.Message), exception); + } // // SQL.DataCommand 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 0503b86149..c05612b717 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.Designer.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.Designer.cs @@ -60,6 +60,15 @@ internal Strings() { } } + /// + /// Looks up a localized string similar to Connection timed out while retrieving an access token using '{0}' authentication method. Last error: {1}: {2}. + /// + internal static string AAD_Token_Retrieving_Timeout { + get { + return ResourceManager.GetString("AAD_Token_Retrieving_Timeout", resourceCulture); + } + } + /// /// Looks up a localized string similar to Specified parameter name '{0}' is not valid.. /// @@ -2796,15 +2805,6 @@ internal static string SQL_KerberosTicketMissingError { } } - /// - /// Looks up a localized string similar to Cannot use 'Authentication={0}' with 'Password' or 'PWD' connection string keywords.. - /// - internal static string SQL_NonInteractiveWithPassword { - get { - return ResourceManager.GetString("SQL_NonInteractiveWithPassword", resourceCulture); - } - } - /// /// Looks up a localized string similar to The connection does not support MultipleActiveResultSets.. /// @@ -2868,6 +2868,15 @@ internal static string SQL_NonCharColumn { } } + /// + /// Looks up a localized string similar to Cannot use 'Authentication={0}' with 'Password' or 'PWD' connection string keywords.. + /// + internal static string SQL_NonInteractiveWithPassword { + get { + return ResourceManager.GetString("SQL_NonInteractiveWithPassword", resourceCulture); + } + } + /// /// Looks up a localized string similar to SSE Instance re-direction is not supported for non-local user instances.. /// @@ -4442,12 +4451,15 @@ internal static string TCE_DbConnectionString_AttestationProtocol { return ResourceManager.GetString("TCE_DbConnectionString_AttestationProtocol", resourceCulture); } } - + /// - /// Looks up a localized string similar to Specifies an IP address preference when connecting to SQL instances. + /// Looks up a localized string similar to Specifies an IP address preference when connecting to SQL instances.. /// - internal static string TCE_DbConnectionString_IPAddressPreference - => ResourceManager.GetString("TCE_DbConnectionString_IPAddressPreference", resourceCulture); + internal static string TCE_DbConnectionString_IPAddressPreference { + get { + return ResourceManager.GetString("TCE_DbConnectionString_IPAddressPreference", resourceCulture); + } + } /// /// Looks up a localized string similar to Decryption failed. The last 10 bytes of the encrypted column encryption key are: '{0}'. The first 10 bytes of ciphertext are: '{1}'.. diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx b/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx index 7e6d4c5ac2..457d06a36f 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx +++ b/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx @@ -1932,4 +1932,7 @@ '{0}' is not less than '{1}'; '{2}' cannot be greater than '{3}'. + + Connection timed out while retrieving an access token using '{0}' authentication method. Last error: {1}: {2} + \ No newline at end of file