Call AcquireTokenSilent after AuthenticationRecord has been fetched (Azure#23361)

* Call AcquireTokenSilent after AuthenticationRecord has been fetched
* handle MsalUiRequiredException

Author: christothes

sdk/identity/Azure.Identity/src/AzureIdentityEventSource.cs

@@ -30,6 +30,7 @@ internal sealed class AzureIdentityEventSource : AzureEventSource
         private const int DefaultAzureCredentialCredentialSelectedEvent = 13;
         private const int ProcessRunnerErrorEvent = 14;
         private const int ProcessRunnerInfoEvent = 15;
+        private const int UsernamePasswordCredentialAcquireTokenSilentFailedEvent = 16;
 
         private AzureIdentityEventSource() : base(EventSourceName) { }
 
@@ -260,5 +261,23 @@ public void LogProcessRunnerInformational(string message)
         {
             WriteEvent(ProcessRunnerInfoEvent, message);
         }
+
+        [NonEvent]
+        public void UsernamePasswordCredentialAcquireTokenSilentFailed(Exception e)
+        {
+            if (IsEnabled(EventLevel.Informational, EventKeywords.All))
+            {
+                UsernamePasswordCredentialAcquireTokenSilentFailed(FormatException(e));
+            }
+        }
+
+        [Event(
+            UsernamePasswordCredentialAcquireTokenSilentFailedEvent,
+            Level = EventLevel.Informational,
+            Message = "UsernamePasswordCredential failed to acquire token silently. Error: {1}")]
+        public void UsernamePasswordCredentialAcquireTokenSilentFailed(string error)
+        {
+            WriteEvent(UsernamePasswordCredentialAcquireTokenSilentFailedEvent, error);
+        }
     }
 }

sdk/identity/Azure.Identity/src/UsernamePasswordCredential.cs

@@ -33,8 +33,7 @@ public class UsernamePasswordCredential : TokenCredential
         /// Protected constructor for mocking
         /// </summary>
         protected UsernamePasswordCredential()
-        {
-        }
+        { }
 
         /// <summary>
         /// Creates an instance of the <see cref="UsernamePasswordCredential"/> with the details needed to authenticate against Azure Active Directory with a simple username
@@ -46,8 +45,7 @@ protected UsernamePasswordCredential()
         /// <param name="clientId">The client (application) ID of an App Registration in the tenant.</param>
         public UsernamePasswordCredential(string username, string password, string tenantId, string clientId)
             : this(username, password, tenantId, clientId, (TokenCredentialOptions)null)
-        {
-        }
+        { }
 
         /// <summary>
         /// Creates an instance of the <see cref="UsernamePasswordCredential"/> with the details needed to authenticate against Azure Active Directory with a simple username
@@ -60,8 +58,7 @@ public UsernamePasswordCredential(string username, string password, string tenan
         /// <param name="options">The client options for the newly created UsernamePasswordCredential</param>
         public UsernamePasswordCredential(string username, string password, string tenantId, string clientId, TokenCredentialOptions options)
             : this(username, password, tenantId, clientId, options, null, null)
-        {
-        }
+        { }
 
         /// <summary>
         /// Creates an instance of the <see cref="UsernamePasswordCredential"/> with the details needed to authenticate against Azure Active Directory with a simple username
@@ -74,10 +71,16 @@ public UsernamePasswordCredential(string username, string password, string tenan
         /// <param name="options">The client options for the newly created UsernamePasswordCredential</param>
         public UsernamePasswordCredential(string username, string password, string tenantId, string clientId, UsernamePasswordCredentialOptions options)
             : this(username, password, tenantId, clientId, options, null, null)
-        {
-        }
-
-        internal UsernamePasswordCredential(string username, string password, string tenantId, string clientId, TokenCredentialOptions options, CredentialPipeline pipeline, MsalPublicClient client)
+        { }
+
+        internal UsernamePasswordCredential(
+            string username,
+            string password,
+            string tenantId,
+            string clientId,
+            TokenCredentialOptions options,
+            CredentialPipeline pipeline,
+            MsalPublicClient client)
         {
             Argument.AssertNotNull(username, nameof(username));
             Argument.AssertNotNull(password, nameof(password));
@@ -126,7 +129,8 @@ public virtual async Task<AuthenticationRecord> AuthenticateAsync(CancellationTo
         /// <returns>The <see cref="AuthenticationRecord"/> of the authenticated account.</returns>
         public virtual AuthenticationRecord Authenticate(TokenRequestContext requestContext, CancellationToken cancellationToken = default)
         {
-            return AuthenticateImplAsync(false, requestContext, cancellationToken).EnsureCompleted();
+            AuthenticateImplAsync(false, requestContext, cancellationToken).EnsureCompleted();
+            return _record;
         }
 
         /// <summary>
@@ -137,7 +141,8 @@ public virtual AuthenticationRecord Authenticate(TokenRequestContext requestCont
         /// <returns>The <see cref="AuthenticationRecord"/> of the authenticated account.</returns>
         public virtual async Task<AuthenticationRecord> AuthenticateAsync(TokenRequestContext requestContext, CancellationToken cancellationToken = default)
         {
-            return await AuthenticateImplAsync(true, requestContext, cancellationToken).ConfigureAwait(false);
+            await AuthenticateImplAsync(true, requestContext, cancellationToken).ConfigureAwait(false);
+            return _record;
         }
 
         /// <summary>
@@ -166,15 +171,19 @@ public override async ValueTask<AccessToken> GetTokenAsync(TokenRequestContext r
             return await GetTokenImplAsync(true, requestContext, cancellationToken).ConfigureAwait(false);
         }
 
-        private async Task<AuthenticationRecord> AuthenticateImplAsync(bool async, TokenRequestContext requestContext, CancellationToken cancellationToken)
+        private async Task<AuthenticationResult> AuthenticateImplAsync(bool async, TokenRequestContext requestContext, CancellationToken cancellationToken)
         {
             using CredentialDiagnosticScope scope = _pipeline.StartGetTokenScope($"{nameof(UsernamePasswordCredential)}.{nameof(Authenticate)}", requestContext);
-
             try
             {
-                scope.Succeeded(await GetTokenImplAsync(async, requestContext, cancellationToken).ConfigureAwait(false));
+                var tenantId = TenantIdResolver.Resolve(_tenantId, requestContext, _allowMultiTenantAuthentication);
 
-                return _record;
+                AuthenticationResult result = await Client
+                    .AcquireTokenByUsernamePasswordAsync(requestContext.Scopes, requestContext.Claims, _username, _password, tenantId, async, cancellationToken)
+                    .ConfigureAwait(false);
+
+                _record = new AuthenticationRecord(result, _clientId);
+                return result;
             }
             catch (Exception e)
             {
@@ -185,17 +194,25 @@ private async Task<AuthenticationRecord> AuthenticateImplAsync(bool async, Token
         private async Task<AccessToken> GetTokenImplAsync(bool async, TokenRequestContext requestContext, CancellationToken cancellationToken)
         {
             using CredentialDiagnosticScope scope = _pipeline.StartGetTokenScope("UsernamePasswordCredential.GetToken", requestContext);
-
             try
             {
-                var tenantId = TenantIdResolver.Resolve(_tenantId, requestContext, _allowMultiTenantAuthentication);
-
-                AuthenticationResult result = await Client
-                    .AcquireTokenByUsernamePasswordAsync(requestContext.Scopes, requestContext.Claims, _username, _password, tenantId, async, cancellationToken)
-                    .ConfigureAwait(false);
-
-                _record = new AuthenticationRecord(result, _clientId);
-
+                AuthenticationResult result;
+                if (_record != null)
+                {
+                    var tenantId = TenantIdResolver.Resolve(_tenantId, requestContext, _allowMultiTenantAuthentication);
+                    try
+                    {
+                        result = await Client.AcquireTokenSilentAsync(requestContext.Scopes, requestContext.Claims, _record, tenantId, async, cancellationToken)
+                            .ConfigureAwait(false);
+                        return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn));
+                    }
+                    catch (MsalUiRequiredException msalEx)
+                    {
+                        AzureIdentityEventSource.Singleton.UsernamePasswordCredentialAcquireTokenSilentFailed(msalEx);
+                        // fall through so that AuthenticateImplAsync is called.
+                    }
+                }
+                result = await AuthenticateImplAsync(async, requestContext, cancellationToken).ConfigureAwait(false);
                 return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn));
             }
             catch (Exception e)

sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs

@@ -25,7 +25,7 @@ internal class MockMsalPublicClient : MsalPublicClient
 
         public Func<string[], string, AuthenticationResult> SilentAuthFactory { get; set; }
 
-        public Func<string[], IAccount, string, bool, CancellationToken, AuthenticationResult> ExtendedSilentAuthFactory { get; set; }
+        public Func<string[], string, IAccount, string, bool, CancellationToken, AuthenticationResult> ExtendedSilentAuthFactory { get; set; }
 
         public Func<DeviceCodeInfo, CancellationToken, AuthenticationResult> DeviceCodeAuthFactory { get; set; }
 
@@ -96,7 +96,7 @@ protected override ValueTask<AuthenticationResult> AcquireTokenSilentCoreAsync(
         {
             if (ExtendedSilentAuthFactory != null)
             {
-                return new ValueTask<AuthenticationResult>(ExtendedSilentAuthFactory(scopes, account, tenantId, async, cancellationToken));
+                return new ValueTask<AuthenticationResult>(ExtendedSilentAuthFactory(scopes, claims, account, tenantId, async, cancellationToken));
             }
 
             Func<string[], string, AuthenticationResult> factory = SilentAuthFactory ?? AuthFactory;

sdk/identity/Azure.Identity/tests/SharedTokenCacheCredentialTests.cs

@@ -51,7 +51,7 @@ public async Task VerifyAuthenticationRecordOption()
             var mockMsalClient = new MockMsalPublicClient
             {
                 Accounts = new List<IAccount> { new MockAccount("[email protected]") },
-                ExtendedSilentAuthFactory = (_, account, _, _, _) =>
+                ExtendedSilentAuthFactory = (_, _, account, _, _, _) =>
                 {
                     Assert.AreEqual(expectedUsername, account.Username);
 
@@ -625,7 +625,7 @@ public void TestSetup()
                 Guid.NewGuid(),
                 null,
                 "Bearer");
-            mockMsal.ExtendedSilentAuthFactory = (_, _, tenant, _, _) =>
+            mockMsal.ExtendedSilentAuthFactory = (_, _, _, tenant, _, _) =>
             {
                 Assert.AreEqual(expectedTenantId, tenant, "TenantId passed to msal should match");
                 return result;

sdk/identity/Azure.Identity/tests/UsernamePasswordCredentialTests.cs

@@ -23,6 +23,8 @@ public class UsernamePasswordCredentialTests : ClientTestBase
         private MockMsalPublicClient mockMsal;
         private DeviceCodeResult deviceCodeResult;
         private string expectedTenantId;
+        private bool interactiveCalled;
+        private bool silentCalled;
 
         public UsernamePasswordCredentialTests(bool isAsync) : base(isAsync)
         { }
@@ -60,7 +62,14 @@ public void RespectsIsPIILoggingEnabled([Values(true, false)] bool isLoggingPIIE
             var clientId = Guid.NewGuid().ToString();
             var tenantId = Guid.NewGuid().ToString();
 
-            var credential = new UsernamePasswordCredential(username, password, clientId, tenantId, new TokenCredentialOptions{ IsLoggingPIIEnabled = isLoggingPIIEnabled}, default, null);
+            var credential = new UsernamePasswordCredential(
+                username,
+                password,
+                clientId,
+                tenantId,
+                new TokenCredentialOptions { IsLoggingPIIEnabled = isLoggingPIIEnabled },
+                default,
+                null);
 
             Assert.NotNull(credential.Client);
             Assert.AreEqual(isLoggingPIIEnabled, credential.Client.LogPII);
@@ -82,8 +91,37 @@ public async Task UsesTenantIdHint([Values(null, TenantIdHint)] string tenantId,
             Assert.AreEqual(expiresOn, token.ExpiresOn);
         }
 
+        [Test]
+        public async Task CallsGetAzquireTokenSilentAfterFirstTokenAcquired(
+            [Values(null, TenantIdHint)] string tenantId,
+            [Values(true)] bool allowMultiTenantAuthentication)
+        {
+            TestSetup();
+            var options = new UsernamePasswordCredentialOptions { AllowMultiTenantAuthentication = allowMultiTenantAuthentication };
+            var context = new TokenRequestContext(new[] { Scope }, tenantId: tenantId);
+            expectedTenantId = TenantIdResolver.Resolve(TenantId, context, options.AllowMultiTenantAuthentication);
+
+            var credential = InstrumentClient(new UsernamePasswordCredential("user", "password", TenantId, ClientId, options, null, mockMsal));
+
+            AccessToken token = await credential.GetTokenAsync(context);
+
+            Assert.AreEqual(expectedToken, token.Token);
+            Assert.AreEqual(expiresOn, token.ExpiresOn);
+            Assert.True(interactiveCalled);
+            Assert.False(silentCalled);
+
+            // Second call should acquireSilent
+            token = await credential.GetTokenAsync(context);
+
+            Assert.AreEqual(expectedToken, token.Token);
+            Assert.AreEqual(expiresOn, token.ExpiresOn);
+            Assert.True(silentCalled);
+        }
+
         public void TestSetup()
         {
+            interactiveCalled = false;
+            silentCalled = false;
             expectedTenantId = null;
             expectedCode = Guid.NewGuid().ToString();
             expectedToken = Guid.NewGuid().ToString();
@@ -106,6 +144,14 @@ public void TestSetup()
                 "Bearer");
             mockMsal.UserPassAuthFactory = (_, tenant) =>
             {
+                interactiveCalled = true;
+                Assert.AreEqual(expectedTenantId, tenant, "TenantId passed to msal should match");
+                return result;
+            };
+
+            mockMsal.SilentAuthFactory = (_, tenant) =>
+            {
+                silentCalled = true;
                 Assert.AreEqual(expectedTenantId, tenant, "TenantId passed to msal should match");
                 return result;
             };