[Fix 3.1] Handle NRE on Azure federated authentication (#1695)

Porting #1625 to 3.1-servicing.
dotnet · Aug 9, 2022 · d272da8 · d272da8
1 parent db1c825
commit d272da8
Show file tree

Hide file tree

Showing 20 changed files with 149 additions and 37 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
@@ -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 TimeoutException(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/...Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/...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("<sc.SqlInternalConnectionTds.GetFedAuthToken.MsalServiceException error:> Timeout: {0}", serviceException.ErrorCode);
+                            throw SQL.ActiveDirectoryTokenRetrievingTimeout(Enum.GetName(typeof(SqlAuthenticationMethod), ConnectionOptions.Authentication), serviceException.ErrorCode, serviceException);
                         }
                     }
+                    else
+                    {
+                        SqlClientEventSource.Log.TryTraceEvent("<sc.SqlInternalConnectionTds.GetFedAuthToken.MsalServiceException error:> {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("<sc.SqlInternalConnectionTds.GetFedAuthToken.MSALException error:> {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("<sc.SqlInternalConnectionTds.GetFedAuthToken|ADV> {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
@@ -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
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx b/src/Microsoft.Data.SqlClient/netcore/src/Resources/Strings.resx
@@ -1932,4 +1932,7 @@
   <data name="SqlRetryLogic_InvalidMinMaxPair" xml:space="preserve">
     <value>'{0}' is not less than '{1}'; '{2}' cannot be greater than '{3}'.</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>Connection timed out while retrieving an access token using '{0}' authentication method. Last error: {1}: {2}</value>
+  </data>
 </root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/Common/AdapterUtil.cs
@@ -23,6 +23,7 @@
 using System.Xml;
 using Microsoft.Data.SqlClient;
 using Microsoft.Data.SqlClient.Server;
+using Microsoft.Identity.Client;
 using Microsoft.Win32;
 using SysES = System.EnterpriseServices;
 using SysTx = System.Transactions;
@@ -230,9 +231,9 @@ static internal InvalidOperationException InvalidOperation(string error)
             TraceExceptionAsReturnValue(e);
             return e;
         }
-        static internal TimeoutException TimeoutException(string error)
+        static internal TimeoutException TimeoutException(string error, Exception inner = null)
         {
-            TimeoutException e = new TimeoutException(error);
+            TimeoutException e = new TimeoutException(error, inner);
             TraceExceptionAsReturnValue(e);
             return e;
         }
@@ -525,6 +526,32 @@ static internal ArgumentException MustBeReadOnly(string argumentName)
             return Argument(StringsHelper.GetString(Strings.ADP_MustBeReadOnly, argumentName));
         }
 
+        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);
+        }
+
         // IDbCommand.CommandType
         static internal ArgumentOutOfRangeException InvalidCommandType(CommandType value)
         {

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
@@ -105,6 +105,8 @@ public void AssertUnrecoverableStateCountIsCorrect()
 
     sealed internal 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 re-route limit
         internal const int _maxNumberOfRedirectRoute = 10;
@@ -2857,7 +2859,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)
@@ -2876,9 +2878,15 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
                         }
                         else
                         {
-                            break;
+                            SqlClientEventSource.Log.TryTraceEvent("<sc.SqlInternalConnectionTds.GetFedAuthToken.MsalServiceException error:> Timeout: {0}", serviceException.ErrorCode);
+                            throw SQL.ActiveDirectoryTokenRetrievingTimeout(Enum.GetName(typeof(SqlAuthenticationMethod), ConnectionOptions.Authentication), serviceException.ErrorCode, serviceException);
                         }
                     }
+                    else
+                    {
+                        SqlClientEventSource.Log.TryTraceEvent("<sc.SqlInternalConnectionTds.GetFedAuthToken.MsalServiceException error:> {0}", serviceException.ErrorCode);
+                        throw ADP.CreateSqlException(serviceException, ConnectionOptions, this, username);
+                    }
                 }
                 // Deal with normal MsalExceptions.
                 catch (MsalException msalException)
@@ -2889,21 +2897,7 @@ internal SqlFedAuthToken GetFedAuthToken(SqlFedAuthInfo fedAuthInfo)
                     {
                         SqlClientEventSource.Log.TryTraceEvent("<sc.SqlInternalConnectionTds.GetFedAuthToken.MSALException error:> {0}", msalException.ErrorCode);
 
-                        // Error[0]
-                        SqlErrorCollection sqlErs = new SqlErrorCollection();
-                        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("<sc.SqlInternalConnectionTds.GetFedAuthToken|ADV> {0}, sleeping {1}[Milliseconds]", ObjectID, sleepInterval);

diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlUtil.cs
@@ -514,6 +514,11 @@ static internal 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/netfx/src/Resources/Strings.Designer.cs b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.Designer.cs
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.de.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.de.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>Nicht negative Zahl erforderlich.</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>Timeout bei der Verbindung beim Abrufen eines Zugriffstokens mithilfe der Authentifizierungsmethode "{0}". Letzter Fehler: {1}: {2}</value>
+  </data>
 </root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.es.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.es.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>Número no negativo requerido.</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>Se agotó el tiempo de espera de la conexión al recuperar un token de acceso mediante el método de autenticación "{0}". Último error: {1}: {2}</value>
+  </data>
 </root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.fr.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.fr.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>Nombre non négatif obligatoire.</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>La connexion a expiré lors de la récupération d’un jeton d’accès à l’aide de '{0}' méthode d’authentification. Dernière erreur : {1} : {2}</value>
+  </data>
 </root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.it.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.it.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>Numero non negativo obbligatorio.</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>Timeout della connessione durante il recupero di un token di accesso tramite il metodo di autenticazione '{0}'. Ultimo errore: {1}: {2}</value>
+  </data>
 </root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.ja.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.ja.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>負でない数値が必要です。</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>認証方法 '{0}' によるアクセス トークンの取得中に接続がタイムアウトしました。前回のエラー: {1}: {2}</value>
+  </data>
 </root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.ko.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.ko.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>음수가 아닌 수가 필요합니다.</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>'{0}' 인증 방법을 사용하여 액세스 토큰을 검색하는 동안 연결 시간이 초과되었습니다. 마지막 오류: {1}: {2}</value>
+  </data>
 </root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.pt-BR.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.pt-BR.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>É necessário um número não negativo.</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>A conexão expirou ao recuperar um token de acesso usando o método de autenticação '{0}'. Último erro: {1}: {2}</value>
+  </data>
 </root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>Non-negative number required.</value>
   </data>
-</root>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>Connection timed out while retrieving an access token using '{0}' authentication method. Last error: {1}: {2}</value>
+  </data>
+</root>
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.ru.resx b/src/Microsoft.Data.SqlClient/netfx/src/Resources/Strings.ru.resx
@@ -4614,4 +4614,7 @@
   <data name="ArgumentOutOfRange_NeedNonNegNum" xml:space="preserve">
     <value>Требуется неотрицательное число.</value>
   </data>
+  <data name="AAD_Token_Retrieving_Timeout" xml:space="preserve">
+    <value>Истекло время ожидания подключения при получении маркера доступа с помощью метода проверки подлинности "{0}". Последняя ошибка: {1}: {2}</value>
+  </data>
 </root>