Make ConfidentialClientApplicationBuilderExtension.WithClientCredenti…

…als async (#2567)

* Make ConfidentialClientApplicationBuilderExtension.WithClientCredentials fully async

* Remove breaking changes in ITokenAcquisition

---------

Co-authored-by: jennyf19 <jeferrie@microsoft.com>

src/Microsoft.Identity.Web.TokenAcquisition/AspNetCore/TokenAcquisition-AspnetCore.cs

@@ -53,13 +53,12 @@ internal class TokenAcquisitionAspNetCore : TokenAcquisition, ITokenAcquisitionI
         /// <param name="msalServiceException">The <see cref="MsalUiRequiredException"/> that triggered the challenge.</param>
         /// <param name="httpResponse">The <see cref="HttpResponse"/> to update.</param>
         /// if called from a web app, and JwtBearerDefault.AuthenticationScheme if called from a web API.
-        public Task ReplyForbiddenWithWwwAuthenticateHeaderAsync(
+        public async Task ReplyForbiddenWithWwwAuthenticateHeaderAsync(
             IEnumerable<string> scopes,
             MsalUiRequiredException msalServiceException,
             HttpResponse? httpResponse = null)
         {
-            ReplyForbiddenWithWwwAuthenticateHeader(scopes, msalServiceException, null, httpResponse);
-            return Task.CompletedTask;
+            await ReplyForbiddenWithWwwAuthenticateHeaderAsync(scopes, msalServiceException, null, httpResponse);
         }
 
         /// <summary>
@@ -77,6 +76,28 @@ internal class TokenAcquisitionAspNetCore : TokenAcquisition, ITokenAcquisitionI
             MsalUiRequiredException msalServiceException,
             string? authenticationScheme = JwtBearerDefaults.AuthenticationScheme,
             HttpResponse? httpResponse = null)
+        {
+            ReplyForbiddenWithWwwAuthenticateHeaderAsync(scopes, msalServiceException, authenticationScheme, httpResponse)
+                .GetAwaiter()
+                .GetResult();
+        }
+
+        /// <summary>
+        /// Used in web APIs (no user interaction).
+        /// Replies to the client through the HTTP response by sending a 403 (forbidden) and populating the 'WWW-Authenticate' header so that
+        /// the client, in turn, can trigger a user interaction so that the user consents to more scopes.
+        /// </summary>
+        /// <param name="scopes">Scopes to consent to.</param>
+        /// <param name="msalServiceException">The <see cref="MsalUiRequiredException"/> that triggered the challenge.</param>
+        /// <param name="authenticationScheme">Authentication scheme. If null, will use OpenIdConnectDefault.AuthenticationScheme
+        /// if called from a web app, and JwtBearerDefault.AuthenticationScheme if called from a web API.</param>
+        /// <param name="httpResponse">The <see cref="HttpResponse"/> to update.</param>
+        /// if called from a web app, and JwtBearerDefault.AuthenticationScheme if called from a web API.
+        private async Task ReplyForbiddenWithWwwAuthenticateHeaderAsync(
+            IEnumerable<string> scopes,
+            MsalUiRequiredException msalServiceException,
+            string? authenticationScheme = JwtBearerDefaults.AuthenticationScheme,
+            HttpResponse? httpResponse = null)
         {
             // A user interaction is required, but we are in a web API, and therefore,
             // we need to report back to the client through a 'WWW-Authenticate' header https://tools.ietf.org/html/rfc6750#section-3.1
@@ -88,7 +109,7 @@ internal class TokenAcquisitionAspNetCore : TokenAcquisition, ITokenAcquisitionI
 
             MergedOptions mergedOptions = _tokenAcquisitionHost.GetOptions(authenticationScheme, out _);
 
-            var application = GetOrBuildConfidentialClientApplication(mergedOptions);
+            var application = await GetOrBuildConfidentialClientApplicationAsync(mergedOptions);
 
             string consentUrl = $"{application.Authority}/oauth2/v2.0/authorize?client_id={mergedOptions.ClientId}"
                 + $"&response_type=code&redirect_uri={application.AppConfig.RedirectUri}"

src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Microsoft.Identity.Abstractions;
 using Microsoft.Identity.Client;
@@ -12,12 +13,24 @@ namespace Microsoft.Identity.Web
 {
     internal static partial class ConfidentialClientApplicationBuilderExtension
     {
+        [Obsolete(IDWebErrorMessage.WithClientCredentialsIsObsolete, false)]
         public static ConfidentialClientApplicationBuilder WithClientCredentials(
             this ConfidentialClientApplicationBuilder builder,
             IEnumerable<CredentialDescription> clientCredentials,
             ILogger logger,
             ICredentialsLoader credentialsLoader,
             CredentialSourceLoaderParameters credentialSourceLoaderParameters)
+        {
+            return WithClientCredentialsAsync(builder, clientCredentials, logger, credentialsLoader,
+                credentialSourceLoaderParameters).GetAwaiter().GetResult();
+        }
+
+        public static async Task<ConfidentialClientApplicationBuilder> WithClientCredentialsAsync(
+            this ConfidentialClientApplicationBuilder builder,
+            IEnumerable<CredentialDescription> clientCredentials,
+            ILogger logger,
+            ICredentialsLoader credentialsLoader,
+            CredentialSourceLoaderParameters credentialSourceLoaderParameters)
         {
             foreach (var credential in clientCredentials)
             {
@@ -27,7 +40,7 @@ internal static partial class ConfidentialClientApplicationBuilderExtension
                     string errorMessage = string.Empty;
                     try
                     {
-                        credentialsLoader.LoadCredentialsIfNeededAsync(credential, credentialSourceLoaderParameters).GetAwaiter().GetResult();
+                        await credentialsLoader.LoadCredentialsIfNeededAsync(credential, credentialSourceLoaderParameters);
                     }
                     catch(Exception ex)
                     {

src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs

@@ -67,8 +67,9 @@ internal static class IDWebErrorMessage
         // Obsolete messages IDW10800 = "IDW10800:"
         public const string AadIssuerValidatorGetIssuerValidatorIsObsolete = "IDW10800: Use MicrosoftIdentityIssuerValidatorFactory.GetAadIssuerValidator. See https://aka.ms/ms-id-web/1.2.0. ";
         public const string InitializeAsyncIsObsolete = "IDW10801: Use Initialize instead. See https://aka.ms/ms-id-web/1.9.0. ";
-        public const string ReplyForbiddenWithWwwAuthenticateHeaderAsyncIsObsolete = "IDW10802: Use ReplyForbiddenWithWwwAuthenticateHeader instead. See https://aka.ms/ms-id-web/1.9.0. ";
         public const string FromStoreWithThumprintIsObsolete = "IDW10803: Use FromStoreWithThumbprint instead, due to spelling error. ";
         public const string AadIssuerValidatorIsObsolete = "IDW10804: Use MicrosoftIdentityIssuerValidator. ";
+
+        public const string WithClientCredentialsIsObsolete = "Use WithClientCredentialsAsync instead.";
     }
 }

src/Microsoft.Identity.Web.TokenAcquisition/ITokenAcquisition.cs

@@ -279,7 +279,6 @@ public interface ITokenAcquisition
         /// <param name="msalServiceException"><see cref="MsalUiRequiredException"/> triggering the challenge.</param>
         /// <param name="httpResponse">The <see cref="HttpResponse"/> to update.</param>
         /// <returns>A <see cref="Task"/> representing the asynchronous operation.</returns>
-        [Obsolete(IDWebErrorMessage.ReplyForbiddenWithWwwAuthenticateHeaderAsyncIsObsolete, false)]
         Task ReplyForbiddenWithWwwAuthenticateHeaderAsync(
         IEnumerable<string> scopes,
         MsalUiRequiredException msalServiceException,

src/Microsoft.Identity.Web.TokenAcquisition/TokenAcquisition.cs

@@ -44,7 +44,7 @@ class OAuthConstants
 #endif
         protected readonly IMsalTokenCacheProvider _tokenCacheProvider;
 
-        private readonly object _applicationSyncObj = new();
+        private SemaphoreSlim _applicationSync = new (1, 1);
 
         /// <summary>
         ///  Please call GetOrBuildConfidentialClientApplication instead of accessing _applicationsByAuthorityClientId directly.
@@ -118,7 +118,7 @@ class OAuthConstants
             IConfidentialClientApplication? application = null;
             try
             {
-                application = GetOrBuildConfidentialClientApplication(mergedOptions);
+                application = await GetOrBuildConfidentialClientApplicationAsync(mergedOptions);
 
                 // Do not share the access token with ASP.NET Core otherwise ASP.NET will cache it and will not send the OAuth 2.0 request in
                 // case a further call to AcquireTokenByAuthorizationCodeAsync in the future is required for incremental consent (getting a code requesting more scopes)
@@ -239,7 +239,7 @@ private static string GetApplicationKey(MergedOptions mergedOptions)
 
             user ??= await _tokenAcquisitionHost.GetAuthenticatedUserAsync(user).ConfigureAwait(false);
 
-            var application = GetOrBuildConfidentialClientApplication(mergedOptions);
+            var application = await GetOrBuildConfidentialClientApplicationAsync(mergedOptions);
 
             try
             {
@@ -378,7 +378,7 @@ private void LogAuthResult(AuthenticationResult? authenticationResult)
             }
 
             // Use MSAL to get the right token to call the API
-            var application = GetOrBuildConfidentialClientApplication(mergedOptions);
+            var application = await GetOrBuildConfidentialClientApplicationAsync(mergedOptions);
 
             AcquireTokenForClientParameterBuilder builder = application
                    .AcquireTokenForClient(new[] { scope }.Except(_scopesRequestedByMsal))
@@ -558,7 +558,7 @@ private void LogAuthResult(AuthenticationResult? authenticationResult)
             {
                 MergedOptions mergedOptions = _tokenAcquisitionHost.GetOptions(authenticationScheme, out _);
 
-                IConfidentialClientApplication app = GetOrBuildConfidentialClientApplication(mergedOptions);
+                IConfidentialClientApplication app = await GetOrBuildConfidentialClientApplicationAsync(mergedOptions);
 
                 if (mergedOptions.IsB2C)
                 {
@@ -592,25 +592,36 @@ private bool IsInvalidClientCertificateOrSignedAssertionError(MsalServiceExcepti
                 || exMsal.Message.Contains(Constants.CertificateHasBeenRevoked));
 #endif
         }
-
-        internal /* for testing */ IConfidentialClientApplication GetOrBuildConfidentialClientApplication(
+        
+        internal /* for testing */ async Task<IConfidentialClientApplication> GetOrBuildConfidentialClientApplicationAsync(
            MergedOptions mergedOptions)
         {
             if (!_applicationsByAuthorityClientId.TryGetValue(GetApplicationKey(mergedOptions), out IConfidentialClientApplication? application) || application == null)
             {
-                lock (_applicationSyncObj)
+                await _applicationSync.WaitAsync();
+
+                try
                 {
-                    application = BuildConfidentialClientApplication(mergedOptions);
-                    _applicationsByAuthorityClientId.TryAdd(GetApplicationKey(mergedOptions), application);
+                    if (!_applicationsByAuthorityClientId.TryGetValue(GetApplicationKey(mergedOptions), out application) ||
+                            application == null)
+                    {
+                        application = await BuildConfidentialClientApplicationAsync(mergedOptions);
+                        _applicationsByAuthorityClientId[GetApplicationKey(mergedOptions)] = application;
+                    }
                 }
+                finally
+                {
+                    _applicationSync.Release();
+                }           
             }
+
             return application;
         }
 
         /// <summary>
         /// Creates an MSAL confidential client application.
         /// </summary>
-        private IConfidentialClientApplication BuildConfidentialClientApplication(MergedOptions mergedOptions)
+        private async Task<IConfidentialClientApplication> BuildConfidentialClientApplicationAsync(MergedOptions mergedOptions)
         {
             string? currentUri = _tokenAcquisitionHost.GetCurrentRedirectUri(mergedOptions);
             mergedOptions.PrepareAuthorityInstanceForMsal();
@@ -652,7 +663,7 @@ private IConfidentialClientApplication BuildConfidentialClientApplication(Merged
 
                 try
                 {
-                    builder.WithClientCredentials(
+                    await builder.WithClientCredentialsAsync(
                         mergedOptions.ClientCredentials!,
                         _logger,
                         _credentialsLoader,

tests/Microsoft.Identity.Web.Test/TokenAcquisitionAuthorityTests.cs

@@ -6,6 +6,7 @@
 using System.Globalization;
 using System.Net.Http;
 using System.Threading;
+using System.Threading.Tasks;
 using Microsoft.AspNetCore.Authentication.JwtBearer;
 using Microsoft.AspNetCore.Authentication.OpenIdConnect;
 using Microsoft.Extensions.Caching.Memory;
@@ -89,7 +90,7 @@ public void VerifyCorrectSchemeTests(string scheme)
         [InlineData(TC.B2CLoginMicrosoft)]
         [InlineData(TC.B2CInstance, true)]
         [InlineData(TC.B2CLoginMicrosoft, true)]
-        public void VerifyCorrectAuthorityUsedInTokenAcquisition_B2CAuthorityTests(
+        public async Task VerifyCorrectAuthorityUsedInTokenAcquisition_B2CAuthorityTestsAsync(
             string authorityInstance,
             bool withTfp = false)
         {
@@ -125,7 +126,7 @@ public void VerifyCorrectSchemeTests(string scheme)
 
             InitializeTokenAcquisitionObjects();
 
-            IConfidentialClientApplication app = _tokenAcquisition.GetOrBuildConfidentialClientApplication(mergedOptions);
+            IConfidentialClientApplication app = await _tokenAcquisition.GetOrBuildConfidentialClientApplicationAsync(mergedOptions);
 
             string expectedAuthority = string.Format(
                 CultureInfo.InvariantCulture,
@@ -140,7 +141,7 @@ public void VerifyCorrectSchemeTests(string scheme)
         [Theory]
         [InlineData("https://localhost:1234")]
         [InlineData("")]
-        public void VerifyCorrectRedirectUriAsync(
+        public async Task VerifyCorrectRedirectUriAsync(
             string redirectUri)
         {
             _microsoftIdentityOptionsMonitor = new TestOptionsMonitor<MicrosoftIdentityOptions>(new MicrosoftIdentityOptions
@@ -164,7 +165,7 @@ public void VerifyCorrectSchemeTests(string scheme)
 
             InitializeTokenAcquisitionObjects();
 
-            IConfidentialClientApplication app = _tokenAcquisition.GetOrBuildConfidentialClientApplication(mergedOptions);
+            IConfidentialClientApplication app = await _tokenAcquisition.GetOrBuildConfidentialClientApplicationAsync(mergedOptions);
 
             if (!string.IsNullOrEmpty(redirectUri))
             {