Fixes #2251

.github/workflows/codeql-analysis.yml

@@ -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/[email protected]
-      with:
-        dotnet-version: 6.0.301
-
+
     - name: Setup .NET 7.0.x
       uses: actions/[email protected]
       with:
@@ -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)"
 

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

@@ -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
@@ -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).
@@ -50,7 +65,7 @@ public static string? UserAssignedManagedIdentityClientId
         /// <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();
@@ -67,12 +82,22 @@ public static string? UserAssignedManagedIdentityClientId
         /// <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;

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

@@ -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
@@ -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.

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

@@ -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)
@@ -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;
                 }
             }

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

@@ -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;
                 }

src/Microsoft.Identity.Web.Certificateless/AzureIdentityForKubernetesClientAssertion.Logger.cs

@@ -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}. ");
+        }
+    }
+}