Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Onboard Id Web to Threading Analyzers #3041

Merged
merged 19 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions benchmark/TokenAcquisitionBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
7 changes: 7 additions & 0 deletions src/Microsoft.Identity.Web.Azure/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
@@ -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")]
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
westin-m marked this conversation as resolved.
Show resolved Hide resolved
{
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.
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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)")]
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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?
Expand Down
Loading
Loading