[Identity] Fix token refresh issue for MSA accounts (Azure#15959)

* [Identity] Fix token refresh issue for MSA accounts * fixing tests MockMsalPublicClient * fix to allow explicitly specifying tenantId
heaths · Oct 14, 2020 · a69a529 · a69a529
1 parent 2d44823
commit a69a529
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 2 deletions.
diff --git a/sdk/identity/Azure.Identity/src/DeviceCodeCredential.cs b/sdk/identity/Azure.Identity/src/DeviceCodeCredential.cs
@@ -198,7 +198,7 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
                 {
                     try
                     {
-                        AuthenticationResult result = await Client.AcquireTokenSilentAsync(requestContext.Scopes, (AuthenticationAccount)Record, async, cancellationToken).ConfigureAwait(false);
+                        AuthenticationResult result = await Client.AcquireTokenSilentAsync(requestContext.Scopes, Record, async, cancellationToken).ConfigureAwait(false);
 
                         return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn));
                     }

diff --git a/sdk/identity/Azure.Identity/src/InteractiveBrowserCredential.cs b/sdk/identity/Azure.Identity/src/InteractiveBrowserCredential.cs
@@ -181,7 +181,7 @@ private async ValueTask<AccessToken> GetTokenImplAsync(bool async, TokenRequestC
                 {
                     try
                     {
-                        AuthenticationResult result = await Client.AcquireTokenSilentAsync(requestContext.Scopes, (AuthenticationAccount)Record, async, cancellationToken).ConfigureAwait(false);
+                        AuthenticationResult result = await Client.AcquireTokenSilentAsync(requestContext.Scopes, Record, async, cancellationToken).ConfigureAwait(false);
 
                         return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn));
                     }

diff --git a/sdk/identity/Azure.Identity/src/MsalPublicClient.cs b/sdk/identity/Azure.Identity/src/MsalPublicClient.cs
@@ -52,6 +52,17 @@ public virtual async ValueTask<AuthenticationResult> AcquireTokenSilentAsync(str
             IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
             return await client.AcquireTokenSilent(scopes, account).ExecuteAsync(async, cancellationToken).ConfigureAwait(false);
         }
+        public virtual async ValueTask<AuthenticationResult> AcquireTokenSilentAsync(string[] scopes, AuthenticationRecord record, bool async, CancellationToken cancellationToken)
+        {
+            IPublicClientApplication client = await GetClientAsync(async, cancellationToken).ConfigureAwait(false);
+
+            // if the user specified a TenantId when they created the client we want to authenticate to that tenant.
+            // otherwise we should authenticate with the tenant specified by the authentication record since that's the tenant the
+            // user authenticated to originally.
+            return await client.AcquireTokenSilent(scopes, (AuthenticationAccount)record)
+                .WithAuthority(Pipeline.AuthorityHost.AbsoluteUri, TenantId ?? record.TenantId)
+                .ExecuteAsync(async, cancellationToken).ConfigureAwait(false);
+        }
 
         public virtual async ValueTask<AuthenticationResult> AcquireTokenInteractiveAsync(string[] scopes, Prompt prompt, bool async, CancellationToken cancellationToken)
         {

diff --git a/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialLiveTests.cs b/sdk/identity/Azure.Identity/tests/InteractiveBrowserCredentialLiveTests.cs
@@ -85,5 +85,23 @@ public async Task AuthenticateWithSharedTokenCacheAsync()
 
             Assert.NotNull(token.Token);
         }
+
+        [Test]
+        [Ignore("This test is an integration test which can only be run with user interaction")]
+        // This test should be run with an MSA account to validate that the refresh for MSA accounts works properly
+        public async Task AuthenticateWithMSAWithSubsequentSilentRefresh()
+        {
+            var cred = new InteractiveBrowserCredential();
+
+            // this should pop browser
+            var authRecord = await cred.AuthenticateAsync();
+
+            Assert.NotNull(authRecord);
+
+            // this should not pop browser
+            AccessToken token = await cred.GetTokenAsync(new TokenRequestContext(new string[] { "https://vault.azure.net/.default" })).ConfigureAwait(false);
+
+            Assert.NotNull(token.Token);
+        }
     }
 }
diff --git a/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs b/sdk/identity/Azure.Identity/tests/Mock/MockMsalPublicClient.cs
@@ -73,6 +73,18 @@ public override ValueTask<AuthenticationResult> AcquireTokenSilentAsync(string[]
             throw new NotImplementedException();
         }
 
+        public override ValueTask<AuthenticationResult> AcquireTokenSilentAsync(string[] scopes, AuthenticationRecord record, bool async, CancellationToken cancellationToken)
+        {
+            Func<string[], AuthenticationResult> factory = SilentAuthFactory ?? AuthFactory;
+
+            if (factory != null)
+            {
+                return new ValueTask<AuthenticationResult>(factory(scopes));
+            }
+
+            throw new NotImplementedException();
+        }
+
         public override ValueTask<AuthenticationResult> AcquireTokenWithDeviceCodeAsync(string[] scopes, Func<DeviceCodeResult, Task> deviceCodeCallback, bool async, CancellationToken cancellationToken)
         {
             Func<string[], AuthenticationResult> factory = DeviceCodeAuthFactory ?? AuthFactory;