Skip to content

Commit

Permalink
Fixes #2251 (#2252)
Browse files Browse the repository at this point in the history
* Fixes ##2251
and the same issue for WIF for MSI
and adds loggin in WIF for AKS

* Adding unit tests for:
- AzureIdentityForKubernetesClientAssertion
-  SignedAssertionFilePathCredentialsLoader

* Update codeql-analysis.yml

* Addressing PR feedback
  • Loading branch information
jmprieur committed May 22, 2023
1 parent eb3c0b8 commit 82b1bdf
Show file tree
Hide file tree
Showing 13 changed files with 402 additions and 44 deletions.
10 changes: 1 addition & 9 deletions .github/workflows/codeql-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,7 @@ jobs:
# ✏️ If the Autobuild fails above, remove it and uncomment the following three lines
# and modify them (or add more) to build your code if your project
# uses a compiled language

- name: Setup .NET 6.0.301
uses: actions/setup-dotnet@v2.1.0
with:
dotnet-version: 6.0.301


- name: Setup .NET 7.0.x
uses: actions/setup-dotnet@v2.1.0
with:
Expand All @@ -59,9 +54,6 @@ jobs:
- name: Setup wasm-tools
run: dotnet workload install wasm-tools

- name: Build with .NET 6
run: dotnet test Microsoft.Identity.Web.sln -f net6.0 -p:FROM_GITHUB_ACTION=true --configuration Release --filter "(FullyQualifiedName!~Microsoft.Identity.Web.Test.Integration)&(FullyQualifiedName!~WebAppUiTests)&(FullyQualifiedName!~IntegrationTests)"

- name: Build with .NET 7
run: dotnet test Microsoft.Identity.Web.sln -f net7.0 -p:FROM_GITHUB_ACTION=true --configuration Release --filter "(FullyQualifiedName!~Microsoft.Identity.Web.Test.Integration)&(FullyQualifiedName!~WebAppUiTests)&(FullyQualifiedName=IntegrationTests)"

Expand Down
31 changes: 28 additions & 3 deletions src/Microsoft.Identity.Web.Certificate/DefaultCertificateLoader.cs
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 Microsoft.Extensions.Logging;
using Microsoft.Identity.Abstractions;

namespace Microsoft.Identity.Web
Expand All @@ -26,6 +27,20 @@ namespace Microsoft.Identity.Web
/// </summary>
public class DefaultCertificateLoader : DefaultCredentialsLoader, ICertificateLoader
{
/// <summary>
/// Constructor with a logger.
/// </summary>
/// <param name="logger"></param>
public DefaultCertificateLoader(ILogger<DefaultCertificateLoader>? logger) : base(logger)
{
}

/// <summary>
/// Default constuctor.
/// </summary>
public DefaultCertificateLoader() : this(null)
{
}

/// <summary>
/// This default is overridable at the level of the credential description (for the certificate from KeyVault).
Expand All @@ -50,7 +65,7 @@ public class DefaultCertificateLoader : DefaultCredentialsLoader, ICertificateLo
/// <returns>First certificate in the certificate description list.</returns>
public static X509Certificate2? LoadFirstCertificate(IEnumerable<CertificateDescription> certificateDescriptions)
{
DefaultCertificateLoader defaultCertificateLoader = new();
DefaultCertificateLoader defaultCertificateLoader = new(null);
CertificateDescription? certDescription = certificateDescriptions.FirstOrDefault(c =>
{
defaultCertificateLoader.LoadCredentialsIfNeededAsync(c).GetAwaiter().GetResult();
Expand All @@ -67,12 +82,22 @@ public class DefaultCertificateLoader : DefaultCredentialsLoader, ICertificateLo
/// <returns>All the certificates in the certificate description list.</returns>
public static IEnumerable<X509Certificate2?> LoadAllCertificates(IEnumerable<CertificateDescription> certificateDescriptions)
{
DefaultCertificateLoader defaultCertificateLoader = new();
DefaultCertificateLoader defaultCertificateLoader = new(null);
return defaultCertificateLoader.LoadCertificates(certificateDescriptions);
}

/// <summary>
/// Load the certificates from the certificate description list.
/// </summary>
/// <param name="certificateDescriptions"></param>
/// <returns>a collection of certificates</returns>
private IEnumerable<X509Certificate2?> LoadCertificates(IEnumerable<CertificateDescription> certificateDescriptions)
{
if (certificateDescriptions != null)
{
foreach (var certDescription in certificateDescriptions)
{
defaultCertificateLoader.LoadCredentialsIfNeededAsync(certDescription).GetAwaiter().GetResult();
LoadCredentialsIfNeededAsync(certDescription).GetAwaiter().GetResult();
if (certDescription.Certificate != null)
{
yield return certDescription.Certificate;
Expand Down
41 changes: 31 additions & 10 deletions src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.Identity.Abstractions;

namespace Microsoft.Identity.Web
Expand All @@ -12,20 +14,39 @@ namespace Microsoft.Identity.Web
/// </summary>
public class DefaultCredentialsLoader : ICredentialsLoader
{
ILogger<DefaultCredentialsLoader>? _logger;

/// <summary>
/// Constructor with a logger
/// </summary>
/// <param name="logger"></param>
public DefaultCredentialsLoader(ILogger<DefaultCredentialsLoader>? logger)
{
_logger = logger;
CredentialSourceLoaders = new Dictionary<CredentialSource, ICredentialSourceLoader>
{
{ CredentialSource.KeyVault, new KeyVaultCertificateLoader() },
{ CredentialSource.Path, new FromPathCertificateLoader() },
{ CredentialSource.StoreWithThumbprint, new StoreWithThumbprintCertificateLoader() },
{ CredentialSource.StoreWithDistinguishedName, new StoreWithDistinguishedNameCertificateLoader() },
{ CredentialSource.Base64Encoded, new Base64EncodedCertificateLoader() },
{ CredentialSource.SignedAssertionFromManagedIdentity, new SignedAssertionFromManagedIdentityCredentialLoader() },
{ CredentialSource.SignedAssertionFilePath, new SignedAssertionFilePathCredentialsLoader(_logger) }
};
}

/// <summary>
/// Default constructor (for backward compatibility)
/// </summary>
public DefaultCredentialsLoader() : this(null)
{
}

/// <summary>
/// Dictionary of credential loaders per credential source. The application can add more to
/// process additional credential sources(like dSMS).
/// </summary>
public IDictionary<CredentialSource, ICredentialSourceLoader> CredentialSourceLoaders { get; } = new Dictionary<CredentialSource, ICredentialSourceLoader>
{
{ CredentialSource.KeyVault, new KeyVaultCertificateLoader() },
{ CredentialSource.Path, new FromPathCertificateLoader() },
{ CredentialSource.StoreWithThumbprint, new StoreWithThumbprintCertificateLoader() },
{ CredentialSource.StoreWithDistinguishedName, new StoreWithDistinguishedNameCertificateLoader() },
{ CredentialSource.Base64Encoded, new Base64EncodedCertificateLoader() },
{ CredentialSource.SignedAssertionFromManagedIdentity, new SignedAssertionFromManagedIdentityCredentialLoader() },
{ CredentialSource.SignedAssertionFilePath, new SignedAssertionFilePathCredentialsLoader() },
};
public IDictionary<CredentialSource, ICredentialSourceLoader> CredentialSourceLoaders { get; }

/// <inheritdoc/>
/// Load the credentials from the description, if needed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,22 @@
using System.Threading;
using Microsoft.Identity.Abstractions;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;

namespace Microsoft.Identity.Web
{
internal class SignedAssertionFilePathCredentialsLoader : ICredentialSourceLoader
{
ILogger? _logger;

/// <summary>
/// Constructor
/// </summary>
/// <param name="logger">Optional logger.</param>
public SignedAssertionFilePathCredentialsLoader(ILogger? logger)
{
_logger = logger;
}
public CredentialSource CredentialSource => CredentialSource.SignedAssertionFilePath;

public async Task LoadIfNeededAsync(CredentialDescription credentialDescription, CredentialSourceLoaderParameters? credentialSourceLoaderParameters)
Expand All @@ -19,17 +30,17 @@ public async Task LoadIfNeededAsync(CredentialDescription credentialDescription,
AzureIdentityForKubernetesClientAssertion? signedAssertion = credentialDescription.CachedValue as AzureIdentityForKubernetesClientAssertion;
if (credentialDescription.CachedValue == null)
{
signedAssertion = new AzureIdentityForKubernetesClientAssertion(credentialDescription.SignedAssertionFileDiskPath);
signedAssertion = new AzureIdentityForKubernetesClientAssertion(credentialDescription.SignedAssertionFileDiskPath, _logger);
}
try
{
// Given that managed identity can be not available locally, we need to try to get a
// signed assertion, and if it fails, move to the next credentials
_= await signedAssertion!.GetSignedAssertion(CancellationToken.None);
credentialDescription.CachedValue = signedAssertion;
}
catch (Exception)
{
credentialDescription.CachedValue = signedAssertion;
credentialDescription.Skip = true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ public async Task LoadIfNeededAsync(CredentialDescription credentialDescription,
// Given that managed identity can be not available locally, we need to try to get a
// signed assertion, and if it fails, move to the next credentials
_= await managedIdentityClientAssertion!.GetSignedAssertion(CancellationToken.None);
credentialDescription.CachedValue = managedIdentityClientAssertion;
}
catch (AuthenticationFailedException)
{
credentialDescription.CachedValue = managedIdentityClientAssertion;
credentialDescription.Skip = true;
throw;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using Microsoft.Extensions.Logging;

namespace Microsoft.Identity.Web
{
public partial class AzureIdentityForKubernetesClientAssertion
{
/*
// High performance logger messages (before generation).
#pragma warning disable SYSLIB1009 // Logging methods must be static
[LoggerMessage(EventId = 1, Level = LogLevel.Information, Message = "SignedAssertionFileDiskPath not provided. Falling back to the content of the AZURE_FEDERATED_TOKEN_FILE environment variable. ")]
partial void SignedAssertionFileDiskPathNotProvided(ILogger logger);
[LoggerMessage(EventId = 2, Level = LogLevel.Information, Message = "The `{environmentVariableName}` environment variable not provided. ")]
partial void SignedAssertionEnvironmentVariableNotProvided(ILogger logger, string environmentVariableName);
[LoggerMessage(EventId = 3, Level = LogLevel.Error, Message = "The environment variable AZURE_FEDERATED_TOKEN_FILE or AZURE_ACCESS_TOKEN_FILE or the 'SignedAssertionFileDiskPath' must be set to the path of the file containing the signed assertion. ")]
partial void NoSignedAssertionParameterProvided(ILogger logger);
[LoggerMessage(EventId = 4, Level = LogLevel.Error, Message = "The file `{filePath}` containing the signed assertion was not found. ")]
partial void FileAssertionPathNotFound(ILogger logger, string filePath);
[LoggerMessage(EventId = 5, Level = LogLevel.Information, Message = "Successfully read the signed assertion for `{filePath}`. Expires at {expiry}. ")]
partial void SuccessFullyReadSignedAssertion(ILogger logger, string filePath, DateTime expiry);
[LoggerMessage(EventId = 6, Level = LogLevel.Error, Message = "The file `{filePath} does not contain a valid signed assertion. {message}. ")]
partial void FileDoesNotContainValidAssertion(ILogger logger, string filePath, string message);
#pragma warning restore SYSLIB1009 // Logging methods must be static
*/

/// <summary>
/// Performant logging messages.
/// </summary>
static class Logger
{
public static void SignedAssertionFileDiskPathNotProvided(ILogger? logger)
{
if (logger != null && logger.IsEnabled(LogLevel.Information))
{
__SignedAssertionFileDiskPathNotProvidedCallback(logger, null);
}
}

public static void SignedAssertionEnvironmentVariableNotProvided(ILogger? logger, string environmentVariableName)
{
if (logger != null && logger.IsEnabled(LogLevel.Information))
{
__SignedAssertionEnvironmentVariableNotProvidedCallback(logger, environmentVariableName, null);
}
}

public static void NoSignedAssertionParameterProvided(ILogger? logger)
{
if (logger != null && logger.IsEnabled(LogLevel.Error))
{
__NoSignedAssertionParameterProvidedCallback(logger, null);
}
}

public static void FileAssertionPathNotFound(ILogger? logger, string filePath)
{
if (logger != null && logger.IsEnabled(LogLevel.Error))
{
__FileAssertionPathNotFoundCallback(logger, filePath, null);
}
}

public static void SuccessFullyReadSignedAssertion(ILogger? logger, string filePath, DateTime expiry)
{
if (logger != null && logger.IsEnabled(LogLevel.Information))
{
__SuccessFullyReadSignedAssertionCallback(logger, filePath, expiry, null);
}
}

public static void FileDoesNotContainValidAssertion(ILogger? logger, string filePath, string message)
{
if (logger != null && logger.IsEnabled(LogLevel.Error))
{
__FileDoesNotContainValidAssertionCallback(logger, filePath, message, null);
}
}

private static readonly Action<ILogger, Exception?> __SignedAssertionFileDiskPathNotProvidedCallback =
LoggerMessage.Define(LogLevel.Information, new EventId(1, nameof(SignedAssertionFileDiskPathNotProvided)), "SignedAssertionFileDiskPath not provided. Falling back to the content of the AZURE_FEDERATED_TOKEN_FILE environment variable. ");
private static readonly Action<ILogger, string, Exception?> __SignedAssertionEnvironmentVariableNotProvidedCallback =
LoggerMessage.Define<string>(LogLevel.Information, new EventId(2, nameof(SignedAssertionEnvironmentVariableNotProvided)), "The `{environmentVariableName}` environment variable not provided. ");

private static readonly Action<ILogger, Exception?> __NoSignedAssertionParameterProvidedCallback =
LoggerMessage.Define(LogLevel.Error, new EventId(3, nameof(NoSignedAssertionParameterProvided)), "The environment variable AZURE_FEDERATED_TOKEN_FILE or AZURE_ACCESS_TOKEN_FILE or the 'SignedAssertionFileDiskPath' must be set to the path of the file containing the signed assertion. ");

private static readonly Action<ILogger, string, Exception?> __FileAssertionPathNotFoundCallback =
LoggerMessage.Define<string>(LogLevel.Error, new EventId(4, nameof(FileAssertionPathNotFound)), "The file `{filePath}` containing the signed assertion was not found. ");

private static readonly Action<ILogger, string, DateTime, Exception?> __SuccessFullyReadSignedAssertionCallback =
LoggerMessage.Define<string, DateTime>(LogLevel.Information, new EventId(5, nameof(SuccessFullyReadSignedAssertion)), "Successfully read the signed assertion for `{filePath}`. Expires at {expiry}. ");

private static readonly Action<ILogger, string, string, Exception?> __FileDoesNotContainValidAssertionCallback =
LoggerMessage.Define<string, string>(LogLevel.Error, new EventId(6, nameof(FileDoesNotContainValidAssertion)), "The file `{filePath} does not contain a valid signed assertion. {message}. ");
}
}
}

0 comments on commit 82b1bdf

Please sign in to comment.