Bug fixes (#11716)

* - Fix #11595: [BUG] Azure.Identity DefaultAzureCredential in Visual Studio fails when MFA required - Fix #11371: [BUG] AzureConfigurationBuilder fails to build connection strings when deployed to App Service - Remove some dead code * Rename methods (CR feedback) * Rename to FailWrapAndThrow
Azure · May 20, 2020 · bff953f · bff953f
1 parent 2e9be67
commit bff953f
Show file tree

Hide file tree

Showing 17 changed files with 37 additions and 148 deletions.
diff --git a/sdk/identity/Azure.Identity/src/AuthorizationCodeCredential.cs b/sdk/identity/Azure.Identity/src/AuthorizationCodeCredential.cs
@@ -117,15 +117,9 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
 
                 return scope.Succeeded(token);
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
     }

diff --git a/sdk/identity/Azure.Identity/src/AzureCliCredential.cs b/sdk/identity/Azure.Identity/src/AzureCliCredential.cs
@@ -89,14 +89,9 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
                 AccessToken token = await RequestCliAccessTokenAsync(async, requestContext.Scopes, cancellationToken).ConfigureAwait(false);
                 return scope.Succeeded(token);
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 

diff --git a/sdk/identity/Azure.Identity/src/ClientCertificateCredential.cs b/sdk/identity/Azure.Identity/src/ClientCertificateCredential.cs
@@ -145,15 +145,9 @@ public override AccessToken GetToken(TokenRequestContext requestContext, Cancell
                 X509Certificate2 cert = ClientCertificateProvider.GetCertificateAsync(false, cancellationToken).EnsureCompleted();
                 return scope.Succeeded(_client.Authenticate(TenantId, ClientId, cert, requestContext.Scopes, cancellationToken));
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 
@@ -172,15 +166,9 @@ public override async ValueTask<AccessToken> GetTokenAsync(TokenRequestContext r
                 X509Certificate2 cert = await ClientCertificateProvider.GetCertificateAsync(true, cancellationToken).ConfigureAwait(false);
                 return scope.Succeeded(await _client.AuthenticateAsync(TenantId, ClientId, cert, requestContext.Scopes, cancellationToken).ConfigureAwait(false));
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 

diff --git a/sdk/identity/Azure.Identity/src/ClientSecretCredential.cs b/sdk/identity/Azure.Identity/src/ClientSecretCredential.cs
@@ -95,15 +95,9 @@ public override async ValueTask<AccessToken> GetTokenAsync(TokenRequestContext r
             {
                 return scope.Succeeded(await _client.AuthenticateAsync(TenantId, ClientId, ClientSecret, requestContext.Scopes, cancellationToken).ConfigureAwait(false));
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 
@@ -121,15 +115,9 @@ public override AccessToken GetToken(TokenRequestContext requestContext, Cancell
             {
                 return scope.Succeeded(_client.Authenticate(TenantId, ClientId, ClientSecret, requestContext.Scopes, cancellationToken));
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
     }

diff --git a/sdk/identity/Azure.Identity/src/CredentialDiagnosticScope.cs b/sdk/identity/Azure.Identity/src/CredentialDiagnosticScope.cs
@@ -2,8 +2,10 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Runtime.ExceptionServices;
 using Azure.Core;
 using Azure.Core.Pipeline;
+using Microsoft.Identity.Client;
 
 namespace Azure.Identity
 {
@@ -34,27 +36,26 @@ public AccessToken Succeeded(AccessToken token)
             return token;
         }
 
-        public AuthenticationFailedException FailAndWrap(Exception ex)
+        public Exception FailWrapAndThrow(Exception ex)
         {
-            if (!(ex is AuthenticationFailedException))
+            if (ex is OperationCanceledException || ex is AuthenticationFailedException)
             {
-                ex = new AuthenticationFailedException($"{_name.Substring(0, _name.IndexOf('.'))} authentication failed.", ex);
+                var info = ExceptionDispatchInfo.Capture(ex);
+                RegisterFailed(ex);
+                info.Throw();
             }
 
-            return (AuthenticationFailedException)Failed(ex);
+            ex = new AuthenticationFailedException($"{_name.Substring(0, _name.IndexOf('.'))} authentication failed.", ex);
+            RegisterFailed(ex);
+            throw ex;
         }
 
-
-        public Exception Failed(Exception ex)
+        private void RegisterFailed(Exception ex)
         {
             AzureIdentityEventSource.Singleton.GetTokenFailed(_name, _context, ex);
-
             _scope.Failed(ex);
-
-            return ex;
         }
 
-
         public void Dispose()
         {
             _scope.Dispose();

diff --git a/sdk/identity/Azure.Identity/src/DefaultAzureCredential.cs b/sdk/identity/Azure.Identity/src/DefaultAzureCredential.cs
@@ -112,20 +112,14 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
 
                 return scope.Succeeded(token);
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-                throw;
-            }
             catch (Exception e) when (!(e is CredentialUnavailableException))
             {
-               throw scope.FailAndWrap(new AuthenticationFailedException(UnhandledExceptionMessage, e));
+               throw scope.FailWrapAndThrow(new AuthenticationFailedException(UnhandledExceptionMessage, e));
             }
         }
 
         private async ValueTask<AccessToken> GetTokenFromSourcesAsync(bool async, TokenRequestContext requestContext, CancellationToken cancellationToken)
         {
-
             int i;
 
             List<CredentialUnavailableException> exceptions = new List<CredentialUnavailableException>();

diff --git a/sdk/identity/Azure.Identity/src/DeviceCodeCredential.cs b/sdk/identity/Azure.Identity/src/DeviceCodeCredential.cs
@@ -167,15 +167,9 @@ private async Task<AuthenticationRecord> AuthenticateImplAsync(bool async, Token
 
                 return _record;
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 
@@ -208,15 +202,9 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
 
                 return scope.Succeeded(await GetTokenViaDeviceCodeAsync(requestContext.Scopes, async, cancellationToken).ConfigureAwait(false));
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 

diff --git a/sdk/identity/Azure.Identity/src/EnvironmentCredential.cs b/sdk/identity/Azure.Identity/src/EnvironmentCredential.cs
@@ -125,7 +125,7 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
 
             if (_credential is null)
             {
-                throw scope.Failed(new CredentialUnavailableException(UnavailbleErrorMessage));
+                throw scope.FailWrapAndThrow(new CredentialUnavailableException(UnavailbleErrorMessage));
             }
 
             try
@@ -136,15 +136,9 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
 
                 return scope.Succeeded(token);
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                 throw scope.FailAndWrap(e);
+                 throw scope.FailWrapAndThrow(e);
             }
         }
     }

diff --git a/sdk/identity/Azure.Identity/src/InteractiveBrowserCredential.cs b/sdk/identity/Azure.Identity/src/InteractiveBrowserCredential.cs
@@ -161,15 +161,9 @@ private async Task<AuthenticationRecord> AuthenticateImplAsync(bool async, Token
 
                 return _record;
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 
@@ -202,15 +196,9 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
 
                 return scope.Succeeded(await GetTokenViaBrowserLoginAsync(requestContext.Scopes, async, cancellationToken).ConfigureAwait(false));
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 

diff --git a/sdk/identity/Azure.Identity/src/ManagedIdentityCredential.cs b/sdk/identity/Azure.Identity/src/ManagedIdentityCredential.cs
@@ -91,15 +91,9 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
 
                 return scope.Succeeded(result);
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-
-                throw;
-            }
             catch (Exception e)
             {
-               throw scope.FailAndWrap(e);
+               throw scope.FailWrapAndThrow(e);
             }
         }
     }

diff --git a/sdk/identity/Azure.Identity/src/SharedTokenCacheCredential.cs b/sdk/identity/Azure.Identity/src/SharedTokenCacheCredential.cs
@@ -116,16 +116,11 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
             }
             catch (MsalUiRequiredException)
             {
-                throw scope.Failed(new CredentialUnavailableException($"{nameof(SharedTokenCacheCredential)} authentication unavailable. Token acquisition failed for user {_username}. Ensure that you have authenticated with a developer tool that supports Azure single sign on."));
-            }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-                throw;
+                throw scope.FailWrapAndThrow(new CredentialUnavailableException($"{nameof(SharedTokenCacheCredential)} authentication unavailable. Token acquisition failed for user {_username}. Ensure that you have authenticated with a developer tool that supports Azure single sign on."));
             }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 

diff --git a/sdk/identity/Azure.Identity/src/UsernamePasswordCredential.cs b/sdk/identity/Azure.Identity/src/UsernamePasswordCredential.cs
@@ -114,7 +114,7 @@ private async Task<AccessToken> GetTokenImplAsync(bool async, TokenRequestContex
             }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
     }

diff --git a/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs b/sdk/identity/Azure.Identity/src/VisualStudioCodeCredential.cs
@@ -36,15 +36,15 @@ protected VisualStudioCodeCredential() : this(default, default) {}
         public VisualStudioCodeCredential(string tenantId, TokenCredentialOptions options) : this(tenantId, CredentialPipeline.GetInstance(options), default, default) {}
 
         internal VisualStudioCodeCredential(string tenantId, CredentialPipeline pipeline, IFileSystemService fileSystem, IVisualStudioCodeAdapter vscAdapter)
-            : this(tenantId, pipeline, pipeline.CreateMsalPublicClient(ClientId), fileSystem, vscAdapter)
+            : this(tenantId, pipeline, default, fileSystem, vscAdapter)
         {
         }
 
         internal VisualStudioCodeCredential(string tenantId, CredentialPipeline pipeline, MsalPublicClient client, IFileSystemService fileSystem, IVisualStudioCodeAdapter vscAdapter)
         {
             _tenantId = tenantId ?? "common";
             _pipeline = pipeline;
-            _client = client;
+            _client = client ?? pipeline.CreateMsalPublicClient(ClientId);
             _fileSystem = fileSystem ?? FileSystemService.Default;
             _vscAdapter = vscAdapter ?? GetVscAdapter();
         }
@@ -76,14 +76,13 @@ private async ValueTask<AccessToken> GetTokenImplAsync(TokenRequestContext reque
                 var result = await _client.AcquireTokenByRefreshToken(requestContext.Scopes, storedCredentials, cloudInstance, tenant, async, cancellationToken).ConfigureAwait(false);
                 return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn));
             }
-            catch (OperationCanceledException e)
+            catch (MsalUiRequiredException e)
             {
-                scope.Failed(e);
-                throw;
+                throw scope.FailWrapAndThrow(new CredentialUnavailableException($"{nameof(VisualStudioCodeCredential)} authentication unavailable. Token acquisition failed. Ensure that you have authenticated in VSCode Azure Account.", e));
             }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 

diff --git a/sdk/identity/Azure.Identity/src/VisualStudioCredential.cs b/sdk/identity/Azure.Identity/src/VisualStudioCredential.cs
@@ -75,14 +75,9 @@ private async ValueTask<AccessToken> GetTokenImplAsync(TokenRequestContext reque
                 var accessToken = await RunProcessesAsync(processStartInfos, async, cancellationToken).ConfigureAwait(false);
                 return scope.Succeeded(accessToken);
             }
-            catch (OperationCanceledException e)
-            {
-                scope.Failed(e);
-                throw;
-            }
             catch (Exception e)
             {
-                throw scope.FailAndWrap(e);
+                throw scope.FailWrapAndThrow(e);
             }
         }
 
@@ -120,7 +115,7 @@ private async Task<AccessToken> RunProcessesAsync(List<ProcessStartInfo> process
                 }
                 catch (JsonException exception)
                 {
-                    exceptions.Add(new CredentialUnavailableException($"Process \"{processStartInfo.FileName}\" has invalid output: {output}.", exception));
+                    exceptions.Add(new CredentialUnavailableException($"Process \"{processStartInfo.FileName}\" has non-json output: {output}.", exception));
                 }
                 catch (Exception exception)
                 {

diff --git a/sdk/identity/Azure.Identity/tests/DefaultAzureCredentialTests.cs b/sdk/identity/Azure.Identity/tests/DefaultAzureCredentialTests.cs
@@ -570,27 +570,5 @@ public async Task ValidateSelectedCredentialCaching([Values(typeof(EnvironmentCr
 
             Assert.AreEqual(calledCredentials[0], availableCredential);
         }
-
-        internal class PartialMockDefaultAzureCredentialFactory : DefaultAzureCredentialFactory
-        {
-            private EnvironmentCredential _environmentCredential;
-            private ManagedIdentityCredential _managedIdentityCredential;
-
-            public PartialMockDefaultAzureCredentialFactory(CredentialPipeline pipeline=null, EnvironmentCredential environmentCredential = null, ManagedIdentityCredential managedIdentityCredential = null) : base(pipeline ?? CredentialPipeline.GetInstance(null))
-            {
-                _environmentCredential = environmentCredential;
-                _managedIdentityCredential = managedIdentityCredential;
-            }
-
-            public override TokenCredential CreateEnvironmentCredential()
-            {
-                return _environmentCredential ?? base.CreateEnvironmentCredential();
-            }
-
-            public override TokenCredential CreateManagedIdentityCredential(string clientId)
-            {
-                return _managedIdentityCredential ?? base.CreateManagedIdentityCredential(clientId);
-            }
-        }
     }
 }
-Original file line number
+Diff line change
@@ Expand Up @@
                 }
                 catch (Exception e)
                 {
-                    throw scope.FailAndWrap(e);
+                    throw scope.FailWrapAndThrow(e);
                 }
             }
         }
@@ Expand Down @@