Onboard Id Web to Threading Analyzers (#3041)

* first set of changes

* Add async suffix to methods that return an awaitable type

* next set of analyzer warning cleanup

* update tests

* more async renames

* PR feedback

* undo public api changes

* pr feedback

* pr feedback

* fix api issues, xunit warnings

* Update AcquireTokenForAppIntegrationTests.cs

remove configureawait from theory test

Author: westin-m

Directory.Build.props

@@ -72,6 +72,10 @@
     <DelaySign>false</DelaySign>
   </PropertyGroup>
 
+  <ItemGroup>
+    <PackageReference Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.11.20" PrivateAssets="all" />
+  </ItemGroup>
+
   <PropertyGroup Condition="'$(TargetFramework)' == 'net472' Or '$(TargetFramework)' == 'net462' Or '$(TargetFramework)' == 'netstandard2.0'">
     <LangVersion>12</LangVersion>
   </PropertyGroup>

benchmark/TokenAcquisitionBenchmark.cs

@@ -34,15 +34,15 @@ static TokenAcquisitionBenchmark()
         //}
 
         [Benchmark]
-        public async Task CreateAuthorizationHeader()
+        public async Task CreateAuthorizationHeaderAsync()
         {
             // Get the authorization request creator service
             IAuthorizationHeaderProvider authorizationHeaderProvider = s_serviceProvider!.GetRequiredService<IAuthorizationHeaderProvider>();
             await authorizationHeaderProvider.CreateAuthorizationHeaderForAppAsync("https://graph.microsoft.com/.default").ConfigureAwait(false);
         }
 
         [Benchmark]
-        public async Task GetTokenAcquirer()
+        public async Task GetTokenAcquirerAsync()
         {
             // Get the token acquisition service
             ITokenAcquirer tokenAcquirer = s_tokenAcquirerFactory!.GetTokenAcquirer();

src/Microsoft.Identity.Web.Azure/GlobalSuppressions.cs

@@ -0,0 +1,7 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Diagnostics.CodeAnalysis;
+
+[assembly: SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "This method has an async counterpart.", Scope = "member", Target = "~M:Microsoft.Identity.Web.TokenAcquirerAppTokenCredential.GetToken(Azure.Core.TokenRequestContext,System.Threading.CancellationToken)~Azure.Core.AccessToken")]
+[assembly: SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "This method has an async counterpart.", Scope = "member", Target = "~M:Microsoft.Identity.Web.TokenAcquirerTokenCredential.GetToken(Azure.Core.TokenRequestContext,System.Threading.CancellationToken)~Azure.Core.AccessToken")]

src/Microsoft.Identity.Web.Certificate/DefaultCertificateLoader.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Security.Cryptography.X509Certificates;
+using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Microsoft.Identity.Abstractions;
 
@@ -74,6 +75,29 @@ public static string? UserAssignedManagedIdentityClientId
 
             return certDescription?.Certificate;
         }
+        
+        /// <summary>
+        /// Load the first certificate from the certificate description list.
+        /// </summary>
+        /// <param name="certificateDescriptions">Description of the certificates.</param>
+        /// <returns>First certificate in the certificate description list.</returns>
+        public static async Task<X509Certificate2?> LoadFirstCertificateAsync(IEnumerable<CertificateDescription> certificateDescriptions)
+        {
+            DefaultCertificateLoader defaultCertificateLoader = new(null);
+            CertificateDescription? certDescription = null;
+            foreach (var c in certificateDescriptions)
+            {
+                await defaultCertificateLoader.LoadCredentialsIfNeededAsync(c).ConfigureAwait(false);
+                if (c.Certificate != null)
+                {
+                    certDescription = c;
+                    break;
+                }
+            };
+
+            return certDescription?.Certificate;
+        }
+
 
         /// <summary>
         /// Load all the certificates from the certificate description list.
@@ -97,7 +121,7 @@ public static string? UserAssignedManagedIdentityClientId
             {
                 foreach (var certDescription in certificateDescriptions)
                 {
-                    LoadCredentialsIfNeededAsync(certDescription).GetAwaiter().GetResult();
+                    _ = LoadCredentialsIfNeededAsync(certDescription);
                     if (certDescription.Certificate != null)
                     {
                         yield return certDescription.Certificate;
@@ -148,5 +172,14 @@ public void LoadIfNeeded(CertificateDescription certificateDescription)
         {
             LoadCredentialsIfNeededAsync(certificateDescription).GetAwaiter().GetResult();
         }
+        
+        /// <summary>
+        /// Load the certificate from the description, if needed.
+        /// </summary>
+        /// <param name="certificateDescription">Description of the certificate.</param>
+        public async Task LoadIfNeededAsync(CertificateDescription certificateDescription)
+        {
+            await LoadCredentialsIfNeededAsync(certificateDescription);
+        }
     }
 }

src/Microsoft.Identity.Web.Certificate/GlobalSuppressions.cs

@@ -0,0 +1,7 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System.Diagnostics.CodeAnalysis;
+
+[assembly: SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "This method has an async counterpart.", Scope = "member", Target = "~M:Microsoft.Identity.Web.DefaultCertificateLoader.LoadFirstCertificate(System.Collections.Generic.IEnumerable{Microsoft.Identity.Web.CertificateDescription})~System.Security.Cryptography.X509Certificates.X509Certificate2")]
+[assembly: SuppressMessage("Usage", "VSTHRD002:Avoid problematic synchronous waits", Justification = "This method has an async counterpart.", Scope = "member", Target = "~M:Microsoft.Identity.Web.DefaultCertificateLoader.LoadIfNeeded(Microsoft.Identity.Web.CertificateDescription)")]

src/Microsoft.Identity.Web.Certificate/InternalAPI.Unshipped.txt

@@ -51,6 +51,6 @@ static Microsoft.Identity.Web.CertificateLoaderHelper.DetermineX509KeyStorageFla
 static Microsoft.Identity.Web.CertificateLoaderHelper.DetermineX509KeyStorageFlag(Microsoft.Identity.Abstractions.CredentialDescription! credentialDescription) -> System.Security.Cryptography.X509Certificates.X509KeyStorageFlags
 static Microsoft.Identity.Web.CertificateLoaderHelper.FindCertificateByCriterium(System.Security.Cryptography.X509Certificates.X509Store! x509Store, System.Security.Cryptography.X509Certificates.X509FindType identifierCriterium, string! certificateIdentifier) -> System.Security.Cryptography.X509Certificates.X509Certificate2?
 static Microsoft.Identity.Web.CertificateLoaderHelper.ParseStoreLocationAndName(string! storeDescription, ref System.Security.Cryptography.X509Certificates.StoreLocation certificateStoreLocation, ref System.Security.Cryptography.X509Certificates.StoreName certificateStoreName) -> void
-static Microsoft.Identity.Web.KeyVaultCertificateLoader.LoadFromKeyVault(string! keyVaultUrl, string! certificateName, string? managedIdentityClientId, System.Security.Cryptography.X509Certificates.X509KeyStorageFlags x509KeyStorageFlags) -> System.Threading.Tasks.Task<System.Security.Cryptography.X509Certificates.X509Certificate2?>!
+static Microsoft.Identity.Web.KeyVaultCertificateLoader.LoadFromKeyVaultAsync(string! keyVaultUrl, string! certificateName, string? managedIdentityClientId, System.Security.Cryptography.X509Certificates.X509KeyStorageFlags x509KeyStorageFlags) -> System.Threading.Tasks.Task<System.Security.Cryptography.X509Certificates.X509Certificate2?>!
 static Microsoft.Identity.Web.KeyVaultCertificateLoader.UserAssignedManagedIdentityClientId.get -> string?
 static Microsoft.Identity.Web.KeyVaultCertificateLoader.UserAssignedManagedIdentityClientId.set -> void

src/Microsoft.Identity.Web.Certificate/KeyVaultCertificateLoader.cs

@@ -18,7 +18,7 @@ internal sealed class KeyVaultCertificateLoader : ICredentialSourceLoader
 
         public async Task LoadIfNeededAsync(CredentialDescription credentialDescription, CredentialSourceLoaderParameters? _)
         {
-            credentialDescription.Certificate = await LoadFromKeyVault(
+            credentialDescription.Certificate = await LoadFromKeyVaultAsync(
                             credentialDescription.KeyVaultUrl!,
                             credentialDescription.KeyVaultCertificateName!,
                             credentialDescription.ManagedIdentityClientId ?? UserAssignedManagedIdentityClientId ?? Environment.GetEnvironmentVariable("AZURE_CLIENT_ID"),
@@ -39,7 +39,7 @@ public async Task LoadIfNeededAsync(CredentialDescription credentialDescription,
         /// <remarks>This code is inspired by Heath Stewart's code in:
         /// https://github.com/heaths/azsdk-sample-getcert/blob/master/Program.cs#L46-L82.
         /// </remarks>
-        internal static Task<X509Certificate2?> LoadFromKeyVault(
+        internal static async Task<X509Certificate2?> LoadFromKeyVaultAsync(
             string keyVaultUrl,
             string certificateName,
             string? managedIdentityClientId,
@@ -83,25 +83,25 @@ public async Task LoadIfNeededAsync(CredentialDescription credentialDescription,
             CertificateClient certificateClient = new(keyVaultUri, credential);
             SecretClient secretClient = new(keyVaultUri, credential);
 
-            KeyVaultCertificateWithPolicy certificate = certificateClient.GetCertificate(certificateName);
+            KeyVaultCertificateWithPolicy certificate = await certificateClient.GetCertificateAsync(certificateName);
 
             if (certificate.Properties.NotBefore == null || certificate.Properties.ExpiresOn == null)
             {
-                return Task.FromResult<X509Certificate2?>(null);
+                return null;
             }
 
             if (DateTimeOffset.UtcNow < certificate.Properties.NotBefore || DateTimeOffset.UtcNow > certificate.Properties.ExpiresOn)
             {
-                return Task.FromResult<X509Certificate2?>(null);
+                return null;
             }
 
             // Return a certificate with only the public key if the private key is not exportable.
             if (certificate.Policy?.Exportable != true)
             {
-                return Task.FromResult<X509Certificate2?>(new X509Certificate2(
+                return new X509Certificate2(
                     certificate.Cer,
                     (string?)null,
-                    x509KeyStorageFlags));
+                    x509KeyStorageFlags);
             }
 
             // Parse the secret ID and version to retrieve the private key.
@@ -118,13 +118,13 @@ public async Task LoadIfNeededAsync(CredentialDescription credentialDescription,
             string secretName = segments[1];
             string secretVersion = segments[2];
 
-            KeyVaultSecret secret = secretClient.GetSecret(secretName, secretVersion);
+            KeyVaultSecret secret = await secretClient.GetSecretAsync(secretName, secretVersion);
 
             // For PEM, you'll need to extract the base64-encoded message body.
             // .NET 5.0 preview introduces the System.Security.Cryptography.PemEncoding class to make this easier.
             if (CertificateConstants.MediaTypePksc12.Equals(secret.Properties.ContentType, StringComparison.OrdinalIgnoreCase))
             {
-                return Task.FromResult<X509Certificate2?>(Base64EncodedCertificateLoader.LoadFromBase64Encoded(secret.Value, x509KeyStorageFlags));
+                return Base64EncodedCertificateLoader.LoadFromBase64Encoded(secret.Value, x509KeyStorageFlags);
             }
 
             throw new NotSupportedException(

src/Microsoft.Identity.Web.Certificate/PublicAPI.Unshipped.txt

@@ -18,6 +18,7 @@ Microsoft.Identity.Web.DefaultCertificateLoader
 Microsoft.Identity.Web.DefaultCertificateLoader.DefaultCertificateLoader() -> void
 Microsoft.Identity.Web.DefaultCertificateLoader.DefaultCertificateLoader(Microsoft.Extensions.Logging.ILogger<Microsoft.Identity.Web.DefaultCertificateLoader!>? logger) -> void
 Microsoft.Identity.Web.DefaultCertificateLoader.LoadIfNeeded(Microsoft.Identity.Web.CertificateDescription! certificateDescription) -> void
+Microsoft.Identity.Web.DefaultCertificateLoader.LoadIfNeededAsync(Microsoft.Identity.Web.CertificateDescription! certificateDescription) -> System.Threading.Tasks.Task!
 Microsoft.Identity.Web.DefaultCredentialsLoader
 Microsoft.Identity.Web.DefaultCredentialsLoader.CredentialSourceLoaders.get -> System.Collections.Generic.IDictionary<Microsoft.Identity.Abstractions.CredentialSource, Microsoft.Identity.Abstractions.ICredentialSourceLoader!>!
 Microsoft.Identity.Web.DefaultCredentialsLoader.DefaultCredentialsLoader() -> void
@@ -36,6 +37,7 @@ static Microsoft.Identity.Web.CertificateDescription.FromStoreWithDistinguishedN
 static Microsoft.Identity.Web.CertificateDescription.FromStoreWithThumbprint(string! certificateThumbprint, System.Security.Cryptography.X509Certificates.StoreLocation certificateStoreLocation = System.Security.Cryptography.X509Certificates.StoreLocation.CurrentUser, System.Security.Cryptography.X509Certificates.StoreName certificateStoreName = System.Security.Cryptography.X509Certificates.StoreName.My) -> Microsoft.Identity.Web.CertificateDescription!
 static Microsoft.Identity.Web.DefaultCertificateLoader.LoadAllCertificates(System.Collections.Generic.IEnumerable<Microsoft.Identity.Web.CertificateDescription!>! certificateDescriptions) -> System.Collections.Generic.IEnumerable<System.Security.Cryptography.X509Certificates.X509Certificate2?>!
 static Microsoft.Identity.Web.DefaultCertificateLoader.LoadFirstCertificate(System.Collections.Generic.IEnumerable<Microsoft.Identity.Web.CertificateDescription!>! certificateDescriptions) -> System.Security.Cryptography.X509Certificates.X509Certificate2?
+static Microsoft.Identity.Web.DefaultCertificateLoader.LoadFirstCertificateAsync(System.Collections.Generic.IEnumerable<Microsoft.Identity.Web.CertificateDescription!>! certificateDescriptions) -> System.Threading.Tasks.Task<System.Security.Cryptography.X509Certificates.X509Certificate2?>!
 static Microsoft.Identity.Web.DefaultCertificateLoader.ResetCertificates(System.Collections.Generic.IEnumerable<Microsoft.Identity.Abstractions.CredentialDescription!>? credentialDescription) -> void
 static Microsoft.Identity.Web.DefaultCertificateLoader.ResetCertificates(System.Collections.Generic.IEnumerable<Microsoft.Identity.Web.CertificateDescription!>? certificateDescriptions) -> void
 static Microsoft.Identity.Web.DefaultCertificateLoader.UserAssignedManagedIdentityClientId.get -> string?