From aed3c1675f05eef081fbbf69e5ca096a305dfb7e Mon Sep 17 00:00:00 2001 From: Bogdan Gavril Date: Tue, 24 Sep 2024 01:44:00 +0100 Subject: [PATCH] Fix for 3000 - improve error messages when credentials fail to load (#3003) * Fix for 3000 - logging for loading credentials into MSAL and minor for MSI+FIC * revert msal bump * Address comment * Fix a nullable warning * Updating a few files used to test --------- Co-authored-by: Jean-Marc Prieur --- .../DefaultCredentialsLoader.Logger.cs | 30 +++ .../DefaultCredentialsLoader.cs | 33 ++- ...ignedAssertionFilePathCredentialsLoader.cs | 5 +- ...tionFromManagedIdentityCredentialLoader.cs | 18 +- .../ManagedIdentityClientAssertion.cs | 97 ++++++++- ...lientApplicationBuilderExtension.Logger.cs | 57 +++++ ...entialClientApplicationBuilderExtension.cs | 73 +++++-- .../IDWebErrorMessage.cs | 3 +- .../LoggingEventId.cs | 3 + .../Pages/ClientCredentials.cshtml | 7 + .../Pages/ClientCredentials.cshtml.cs | 54 +++++ .../appsettings.json | 4 +- .../DaemonConsoleCallingDownstreamApi.csproj | 1 + .../Program.cs | 6 + .../WithClientCredentialsTests.cs | 199 ++++++++++++++++++ ...ignedAssertionFilePathCredentialsLoader.cs | 5 +- 16 files changed, 552 insertions(+), 43 deletions(-) create mode 100644 src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.Logger.cs create mode 100644 tests/DevApps/WebAppCallsMicrosoftGraph/Pages/ClientCredentials.cshtml create mode 100644 tests/DevApps/WebAppCallsMicrosoftGraph/Pages/ClientCredentials.cshtml.cs create mode 100644 tests/Microsoft.Identity.Web.Test/Certificates/WithClientCredentialsTests.cs diff --git a/src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.Logger.cs b/src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.Logger.cs new file mode 100644 index 000000000..aa6686779 --- /dev/null +++ b/src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.Logger.cs @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.Extensions.Logging; +using Microsoft.Identity.Abstractions; +using System; + +namespace Microsoft.Identity.Web +{ + // Log messages for DefaultCredentialsLoader + public partial class DefaultCredentialsLoader + { + /// + /// Logging infrastructure + /// + private static class Logger + { + private static readonly Action s_credentialLoadingFailure = + LoggerMessage.Define( + LogLevel.Information, + new EventId( + 7, + nameof(CredentialLoadingFailure)), + "Failed to load credential {id} from source {sourceType}. Will it be skipped in the future ? {skip}."); + + public static void CredentialLoadingFailure(ILogger logger, CredentialDescription cd, Exception? ex) + => s_credentialLoadingFailure(logger, cd.Id, cd.SourceType.ToString(), cd.Skip, ex); + } + } +} diff --git a/src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.cs b/src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.cs index 118beb256..6f4dea6de 100644 --- a/src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.cs +++ b/src/Microsoft.Identity.Web.Certificate/DefaultCredentialsLoader.cs @@ -1,11 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Identity.Abstractions; namespace Microsoft.Identity.Web @@ -13,9 +15,9 @@ namespace Microsoft.Identity.Web /// /// Default credentials loader. /// - public class DefaultCredentialsLoader : ICredentialsLoader + public partial class DefaultCredentialsLoader : ICredentialsLoader { - ILogger? _logger; + private readonly ILogger _logger; private readonly ConcurrentDictionary _loadingSemaphores = new ConcurrentDictionary(); /// @@ -24,7 +26,8 @@ public class DefaultCredentialsLoader : ICredentialsLoader /// public DefaultCredentialsLoader(ILogger? logger) { - _logger = logger; + _logger = logger ?? new NullLogger(); + CredentialSourceLoaders = new Dictionary { { CredentialSource.KeyVault, new KeyVaultCertificateLoader() }, @@ -32,7 +35,7 @@ public DefaultCredentialsLoader(ILogger? logger) { CredentialSource.StoreWithThumbprint, new StoreWithThumbprintCertificateLoader() }, { CredentialSource.StoreWithDistinguishedName, new StoreWithDistinguishedNameCertificateLoader() }, { CredentialSource.Base64Encoded, new Base64EncodedCertificateLoader() }, - { CredentialSource.SignedAssertionFromManagedIdentity, new SignedAssertionFromManagedIdentityCredentialLoader() }, + { CredentialSource.SignedAssertionFromManagedIdentity, new SignedAssertionFromManagedIdentityCredentialLoader(_logger) }, { CredentialSource.SignedAssertionFilePath, new SignedAssertionFilePathCredentialsLoader(_logger) } }; } @@ -51,7 +54,8 @@ public DefaultCredentialsLoader() : this(null) public IDictionary CredentialSourceLoaders { get; } /// - /// Load the credentials from the description, if needed. + /// Load the credentials from the description, if needed. + /// Important: Ignores SKIP flag, propagates exceptions. public async Task LoadCredentialsIfNeededAsync(CredentialDescription credentialDescription, CredentialSourceLoaderParameters? parameters = null) { _ = Throws.IfNull(credentialDescription); @@ -69,7 +73,17 @@ public async Task LoadCredentialsIfNeededAsync(CredentialDescription credentialD if (credentialDescription.CachedValue == null) { if (CredentialSourceLoaders.TryGetValue(credentialDescription.SourceType, out ICredentialSourceLoader? loader)) - await loader.LoadIfNeededAsync(credentialDescription, parameters); + { + try + { + await loader.LoadIfNeededAsync(credentialDescription, parameters); + } + catch (Exception ex) + { + Logger.CredentialLoadingFailure(_logger, credentialDescription, ex); + throw; + } + } } } finally @@ -81,11 +95,15 @@ public async Task LoadCredentialsIfNeededAsync(CredentialDescription credentialD } /// - public async Task LoadFirstValidCredentialsAsync(IEnumerable credentialDescriptions, CredentialSourceLoaderParameters? parameters = null) + /// Loads first valid credential which is not marked as Skipped. + public async Task LoadFirstValidCredentialsAsync( + IEnumerable credentialDescriptions, + CredentialSourceLoaderParameters? parameters = null) { foreach (var credentialDescription in credentialDescriptions) { await LoadCredentialsIfNeededAsync(credentialDescription, parameters); + if (!credentialDescription.Skip) { return credentialDescription; @@ -107,6 +125,5 @@ public void ResetCredentials(IEnumerable credentialDescri } } } - } } diff --git a/src/Microsoft.Identity.Web.Certificate/SignedAssertionFilePathCredentialsLoader.cs b/src/Microsoft.Identity.Web.Certificate/SignedAssertionFilePathCredentialsLoader.cs index b2f3f2c84..82756afdb 100644 --- a/src/Microsoft.Identity.Web.Certificate/SignedAssertionFilePathCredentialsLoader.cs +++ b/src/Microsoft.Identity.Web.Certificate/SignedAssertionFilePathCredentialsLoader.cs @@ -36,13 +36,14 @@ 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 signedAssertion!.GetSignedAssertionAsync(null); + _ = await signedAssertion!.GetSignedAssertionAsync(null); credentialDescription.CachedValue = signedAssertion; } catch (Exception) { credentialDescription.Skip = true; - } + throw; + } } } } diff --git a/src/Microsoft.Identity.Web.Certificate/SignedAssertionFromManagedIdentityCredentialLoader.cs b/src/Microsoft.Identity.Web.Certificate/SignedAssertionFromManagedIdentityCredentialLoader.cs index 005d59ac0..2c0a71a02 100644 --- a/src/Microsoft.Identity.Web.Certificate/SignedAssertionFromManagedIdentityCredentialLoader.cs +++ b/src/Microsoft.Identity.Web.Certificate/SignedAssertionFromManagedIdentityCredentialLoader.cs @@ -4,12 +4,21 @@ using System.Threading; using System.Threading.Tasks; using Azure.Identity; +using Microsoft.Extensions.Logging; using Microsoft.Identity.Abstractions; +using Microsoft.Identity.Client; namespace Microsoft.Identity.Web { internal class SignedAssertionFromManagedIdentityCredentialLoader : ICredentialSourceLoader { + private readonly ILogger _logger; + + public SignedAssertionFromManagedIdentityCredentialLoader(ILogger logger) + { + _logger = logger; + } + public CredentialSource CredentialSource => CredentialSource.SignedAssertionFromManagedIdentity; public async Task LoadIfNeededAsync(CredentialDescription credentialDescription, CredentialSourceLoaderParameters? credentialSourceLoaderParameters) @@ -19,16 +28,19 @@ public async Task LoadIfNeededAsync(CredentialDescription credentialDescription, ManagedIdentityClientAssertion? managedIdentityClientAssertion = credentialDescription.CachedValue as ManagedIdentityClientAssertion; if (credentialDescription.CachedValue == null) { - managedIdentityClientAssertion = new ManagedIdentityClientAssertion(credentialDescription.ManagedIdentityClientId, credentialDescription.TokenExchangeUrl); + managedIdentityClientAssertion = new ManagedIdentityClientAssertion( + credentialDescription.ManagedIdentityClientId, + credentialDescription.TokenExchangeUrl, + _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 managedIdentityClientAssertion!.GetSignedAssertionAsync(null); + _ = await managedIdentityClientAssertion!.GetSignedAssertionAsync(null); credentialDescription.CachedValue = managedIdentityClientAssertion; } - catch (AuthenticationFailedException) + catch (MsalServiceException) { credentialDescription.Skip = true; throw; diff --git a/src/Microsoft.Identity.Web.Certificateless/ManagedIdentityClientAssertion.cs b/src/Microsoft.Identity.Web.Certificateless/ManagedIdentityClientAssertion.cs index 2cc13d1e5..0f9568a47 100644 --- a/src/Microsoft.Identity.Web.Certificateless/ManagedIdentityClientAssertion.cs +++ b/src/Microsoft.Identity.Web.Certificateless/ManagedIdentityClientAssertion.cs @@ -1,10 +1,13 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +using System; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; using Microsoft.Identity.Client; using Microsoft.Identity.Client.AppConfig; +using Microsoft.Identity.Client.Extensibility; using Microsoft.Identity.Web.Certificateless; namespace Microsoft.Identity.Web @@ -16,21 +19,16 @@ public class ManagedIdentityClientAssertion : ClientAssertionProviderBase { IManagedIdentityApplication _managedIdentityApplication; private readonly string _tokenExchangeUrl; + private readonly ILogger? _logger; /// /// See https://aka.ms/ms-id-web/certificateless. /// /// Optional ClientId of the Managed Identity - public ManagedIdentityClientAssertion(string? managedIdentityClientId) + public ManagedIdentityClientAssertion(string? managedIdentityClientId) : + this(managedIdentityClientId, tokenExchangeUrl: null, logger: null) { - var id = ManagedIdentityId.SystemAssigned; - if (!string.IsNullOrEmpty(managedIdentityClientId)) - { - id = ManagedIdentityId.WithUserAssignedClientId(managedIdentityClientId); - } - _managedIdentityApplication = ManagedIdentityApplicationBuilder.Create(id).Build(); - _tokenExchangeUrl = CertificatelessConstants.DefaultTokenExchangeUrl; } /// @@ -39,9 +37,38 @@ public ManagedIdentityClientAssertion(string? managedIdentityClientId) /// Optional ClientId of the Managed Identity /// Optional audience of the token to be requested from Managed Identity. Default value is "api://AzureADTokenExchange". /// This value is different on clouds other than Azure Public - public ManagedIdentityClientAssertion(string? managedIdentityClientId, string? tokenExchangeUrl) : this (managedIdentityClientId) + public ManagedIdentityClientAssertion(string? managedIdentityClientId, string? tokenExchangeUrl) : + this(managedIdentityClientId, tokenExchangeUrl, null) + { + } + + /// + /// See https://aka.ms/ms-id-web/certificateless. + /// + /// Optional ClientId of the Managed Identity + /// Optional audience of the token to be requested from Managed Identity. Default value is "api://AzureADTokenExchange". + /// This value is different on clouds other than Azure Public + /// A logger + public ManagedIdentityClientAssertion(string? managedIdentityClientId, string? tokenExchangeUrl, ILogger? logger) { _tokenExchangeUrl = tokenExchangeUrl ?? CertificatelessConstants.DefaultTokenExchangeUrl; + _logger = logger; + + var id = ManagedIdentityId.SystemAssigned; + if (!string.IsNullOrEmpty(managedIdentityClientId)) + { + id = ManagedIdentityId.WithUserAssignedClientId(managedIdentityClientId); + } + + var builder = ManagedIdentityApplicationBuilder.Create(id); + if (_logger != null) + { + builder = builder.WithLogging(Log, ConvertMicrosoftExtensionsLogLevelToMsal(_logger), enablePiiLogging: false); + _logger.LogInformation($"ManagedIdentityClientAssertion with tokenExchangeUrl={_tokenExchangeUrl}"); + } + + _managedIdentityApplication = builder + .Build(); } /// @@ -58,5 +85,57 @@ protected override async Task GetClientAssertionAsync(Assertion return new ClientAssertion(result.AccessToken, result.ExpiresOn); } + + private void Log( + Client.LogLevel level, + string message, + bool containsPii) + { + switch (level) + { + case Client.LogLevel.Always: + _logger.LogInformation(message); + break; + case Client.LogLevel.Error: + _logger.LogError(message); + break; + case Client.LogLevel.Warning: + _logger.LogWarning(message); + break; + case Client.LogLevel.Info: + _logger.LogInformation(message); + break; + case Client.LogLevel.Verbose: + _logger.LogDebug(message); + break; + } + } + + private Client.LogLevel? ConvertMicrosoftExtensionsLogLevelToMsal(ILogger logger) + { + if (logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Debug) + || logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Trace)) + { + return Client.LogLevel.Verbose; + } + else if (logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Information)) + { + return Client.LogLevel.Info; + } + else if (logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Warning)) + { + return Client.LogLevel.Warning; + } + else if (logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Error) + || logger.IsEnabled(Microsoft.Extensions.Logging.LogLevel.Critical)) + { + return Client.LogLevel.Error; + } + else + { + return null; + } + } + } } diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.Logger.cs b/src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.Logger.cs index e0dfe3cb1..477143708 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.Logger.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.Logger.cs @@ -3,6 +3,7 @@ using System; using Microsoft.Extensions.Logging; +using Microsoft.Identity.Abstractions; namespace Microsoft.Identity.Web { @@ -40,6 +41,62 @@ internal static class Logger LoggingEventId.UsingCertThumbprint, "[MsIdWeb] Using certificate Thumbprint={certThumbprint} as client credentials. "); + private static readonly Action s_credentialAttempt = + LoggerMessage.Define( + LogLevel.Information, + LoggingEventId.CredentialLoadAttempt, + "[MsIdWeb] Attempting to load the credential from the CredentialDescription with Id={Id} and Skip={Skip} . "); + + private static readonly Action s_credentialAttemptFailed = + LoggerMessage.Define( + LogLevel.Information, + LoggingEventId.CredentialLoadAttemptFailed, + "[MsIdWeb] Loading the credential from CredentialDescription Id={Id} failed. Will the credential be re-attempted? - {Skip}."); + + /// + /// Logger for attempting to use a CredentialDescription with MSAL + /// + /// + /// + /// + public static void AttemptToLoadCredentialsFailed( + ILogger logger, + CredentialDescription certificateDescription, + Exception ex) => + s_credentialAttemptFailed( + logger, + certificateDescription.Id, + certificateDescription.Skip.ToString(), + ex); + + /// + /// Logger for attempting to use a CredentialDescription with MSAL + /// + /// + /// + public static void AttemptToLoadCredentials( + ILogger logger, + CredentialDescription certificateDescription) => + s_credentialAttempt( + logger, + certificateDescription.Id, + certificateDescription.Skip.ToString(), + default!); + + /// + /// Logger for attempting to use a CredentialDescription with MSAL + /// + /// + /// + public static void FailedToLoadCredentials( + ILogger logger, + CredentialDescription certificateDescription) => + s_credentialAttemptFailed( + logger, + certificateDescription.Id, + certificateDescription.Skip.ToString(), + default!); + /// /// Logger for handling information specific to ConfidentialClientApplicationBuilderExtension. /// diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.cs b/src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.cs index a7690832c..b95db70d4 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/ConfidentialClientApplicationBuilderExtension.cs @@ -30,23 +30,61 @@ public static async Task WithClientCredent IEnumerable clientCredentials, ILogger logger, ICredentialsLoader credentialsLoader, - CredentialSourceLoaderParameters credentialSourceLoaderParameters) + CredentialSourceLoaderParameters? credentialSourceLoaderParameters) + { + var credential = await LoadCredentialForMsalOrFailAsync( + clientCredentials, + logger, + credentialsLoader, + credentialSourceLoaderParameters) + .ConfigureAwait(false); + + if (credential == null) + { + return builder; + } + + switch (credential.CredentialType) + { + case CredentialType.SignedAssertion: + return builder.WithClientAssertion((credential.CachedValue as ClientAssertionProviderBase)!.GetSignedAssertionAsync); + case CredentialType.Certificate: + return builder.WithCertificate(credential.Certificate); + case CredentialType.Secret: + return builder.WithClientSecret(credential.ClientSecret); + default: + throw new NotImplementedException(); + + } + } + + internal /* for test */ async static Task LoadCredentialForMsalOrFailAsync( + IEnumerable clientCredentials, + ILogger logger, + ICredentialsLoader credentialsLoader, + CredentialSourceLoaderParameters? credentialSourceLoaderParameters) { - foreach (var credential in clientCredentials) + string errorMessage = "\n"; + + foreach (CredentialDescription credential in clientCredentials) { + Logger.AttemptToLoadCredentials(logger, credential); + if (!credential.Skip) { - // Load the credentials - string errorMessage = string.Empty; + // Load the credentials and record error messages in case we need to fail at the end try { + await credentialsLoader.LoadCredentialsIfNeededAsync(credential, credentialSourceLoaderParameters); } - catch(Exception ex) + catch (Exception ex) { - errorMessage = ex.Message; + Logger.AttemptToLoadCredentialsFailed(logger, credential, ex); + errorMessage += $"Credential {credential.Id} failed because: {ex} \n"; } + if (credential.CredentialType == CredentialType.SignedAssertion) { if (credential.SourceType == CredentialSource.SignedAssertionFromManagedIdentity) @@ -58,7 +96,7 @@ public static async Task WithClientCredent else { Logger.UsingManagedIdentity(logger); - return builder.WithClientAssertion((credential.CachedValue as ManagedIdentityClientAssertion)!.GetSignedAssertionAsync); + return credential; } } if (credential.SourceType == CredentialSource.SignedAssertionFilePath) @@ -66,7 +104,7 @@ public static async Task WithClientCredent if (!credential.Skip) { Logger.UsingPodIdentityFile(logger, credential.SignedAssertionFileDiskPath ?? "not found"); - return builder.WithClientAssertion((credential.CachedValue as AzureIdentityForKubernetesClientAssertion)!.GetSignedAssertionAsync); + return credential; } } if (credential.SourceType == CredentialSource.SignedAssertionFromVault) @@ -74,35 +112,38 @@ public static async Task WithClientCredent if (!credential.Skip) { Logger.UsingSignedAssertionFromVault(logger, credential.KeyVaultUrl ?? "undefined"); - return builder.WithClientAssertion((credential.CachedValue as ClientAssertionProviderBase)!.GetSignedAssertionAsync); + return credential; } } } if (credential.CredentialType == CredentialType.Certificate) { - if (credential.Certificate !=null) + if (credential.Certificate != null) { Logger.UsingCertThumbprint(logger, credential.Certificate.Thumbprint); - return builder.WithCertificate(credential.Certificate); + return credential; } } if (credential.CredentialType == CredentialType.Secret) { - return builder.WithClientSecret(credential.ClientSecret); + return credential; } } } - if (clientCredentials.Any(c => c.CredentialType == CredentialType.Certificate)) + if (clientCredentials.Any(c => c.CredentialType == CredentialType.Certificate || c.CredentialType == CredentialType.SignedAssertion)) { throw new ArgumentException( - IDWebErrorMessage.ClientCertificatesHaveExpiredOrCannotBeLoaded, - nameof(clientCredentials)); + IDWebErrorMessage.ClientCertificatesHaveExpiredOrCannotBeLoaded + errorMessage, + nameof(clientCredentials)); } - return builder; + logger.LogInformation($"No client credential could be used. Secret may have been defined elsewhere. " + + $"Count {(clientCredentials != null ? clientCredentials.Count() : 0)} "); + + return null; } } } diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs b/src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs index 4f8d8f14d..fc989d5b6 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/IDWebErrorMessage.cs @@ -19,7 +19,8 @@ internal static class IDWebErrorMessage public const string ConfigurationOptionRequired = "IDW10106: The '{0}' option must be provided. "; public const string ScopesNotConfiguredInConfigurationOrViaDelegate = "IDW10107: Scopes need to be passed-in either by configuration or by the delegate overriding it. "; public const string MissingRequiredScopesForAuthorizationFilter = "IDW10108: RequiredScope Attribute does not contain a value. The scopes need to be set on the controller, the page or action. See https://aka.ms/ms-id-web/required-scope-attribute. "; - public const string ClientCertificatesHaveExpiredOrCannotBeLoaded = "IDW10109: All client certificates passed to the configuration have expired or can't be loaded. "; + public const string ClientCertificatesHaveExpiredOrCannotBeLoaded = "IDW10109: No credential could be loaded. This can happen when certificates passed to the configuration have expired or can't be loaded and the code isn't running on Azure to be able to use Managed Identity, Pod Identity etc. Details: "; + public const string ClientSecretAndCredentialsCannotBeCombined = "IDW10110: ClientSecret top level configuration cannot be combined with ClientCredentials. Instead, add a new entry in the ClientCredentials array describing the secret."; // Authorization IDW10200 = "IDW10200:" public const string NeitherScopeOrRolesClaimFoundInToken = "IDW10201: Neither scope nor roles claim was found in the bearer token. Authentication scheme used: '{0}'. "; diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/LoggingEventId.cs b/src/Microsoft.Identity.Web.TokenAcquisition/LoggingEventId.cs index 57f3b4c17..746132727 100644 --- a/src/Microsoft.Identity.Web.TokenAcquisition/LoggingEventId.cs +++ b/src/Microsoft.Identity.Web.TokenAcquisition/LoggingEventId.cs @@ -25,6 +25,9 @@ internal static class LoggingEventId public static readonly EventId UsingPodIdentityFile = new EventId(402, "UsingPodIdentityFile"); public static readonly EventId UsingCertThumbprint = new EventId(403, "UsingCertThumbprint"); public static readonly EventId UsingSignedAssertionFromVault = new EventId(404, "UsingSignedAssertionFromVault"); + public static readonly EventId CredentialLoadAttempt = new EventId(405, "CredentialLoadAttempt"); + public static readonly EventId CredentialLoadAttemptFailed = new EventId(406, "CredentialLoadAttemptFailed"); + #pragma warning restore IDE1006 // Naming Styles } } diff --git a/tests/DevApps/WebAppCallsMicrosoftGraph/Pages/ClientCredentials.cshtml b/tests/DevApps/WebAppCallsMicrosoftGraph/Pages/ClientCredentials.cshtml new file mode 100644 index 000000000..d7f1f9ca9 --- /dev/null +++ b/tests/DevApps/WebAppCallsMicrosoftGraph/Pages/ClientCredentials.cshtml @@ -0,0 +1,7 @@ +@page +@model WebAppCallsMicrosoftGraph.Pages.ClientCredentialsModel +@{ + ViewData["Title"] = "Client Credentials"; + + @ViewData["json"] +} diff --git a/tests/DevApps/WebAppCallsMicrosoftGraph/Pages/ClientCredentials.cshtml.cs b/tests/DevApps/WebAppCallsMicrosoftGraph/Pages/ClientCredentials.cshtml.cs new file mode 100644 index 000000000..ac939a405 --- /dev/null +++ b/tests/DevApps/WebAppCallsMicrosoftGraph/Pages/ClientCredentials.cshtml.cs @@ -0,0 +1,54 @@ +using System; +using System.Security.Cryptography.X509Certificates; +using System.Text.Json; +using System.Text.Json.Serialization; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Microsoft.Identity.Abstractions; +using Microsoft.Identity.Web; + +namespace WebAppCallsMicrosoftGraph.Pages +{ + public class ClientCredentialsModel : PageModel + { + public ClientCredentialsModel(IOptionsMonitor optionsMonitor) + { + OptionsMonitor = optionsMonitor; + } + + class CertificateJsonConverter : JsonConverter + { + public override X509Certificate2 Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + return null; + } + + public override void Write(Utf8JsonWriter writer, X509Certificate2 value, JsonSerializerOptions options) + { + writer.WriteStartObject(); + writer.WriteString("Thumbprint", value.Thumbprint); + writer.WriteEndObject(); + } + } + + public IOptionsMonitor OptionsMonitor { get; } + + public void OnGet() + { + JsonSerializerOptions jsonSerializerOptions = new JsonSerializerOptions() + { + WriteIndented = true, + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull + }; + jsonSerializerOptions.Converters.Add(new CertificateJsonConverter()); + + + MicrosoftIdentityOptions options = OptionsMonitor.Get(OpenIdConnectDefaults.AuthenticationScheme); + string json = JsonSerializer.Serialize(options.ClientCredentials, jsonSerializerOptions); + ViewData["json"] = json; + } + } +} diff --git a/tests/DevApps/WebAppCallsMicrosoftGraph/appsettings.json b/tests/DevApps/WebAppCallsMicrosoftGraph/appsettings.json index e79e1c74b..3e1e8f8e4 100644 --- a/tests/DevApps/WebAppCallsMicrosoftGraph/appsettings.json +++ b/tests/DevApps/WebAppCallsMicrosoftGraph/appsettings.json @@ -45,10 +45,10 @@ "Logging": { "LogLevel": { - "Default": "Information", + "Default": "Warning", "Microsoft": "Warning", "Microsoft.Hosting.Lifetime": "Information", - "Microsoft.Identity.Web": "Information" + "Microsoft.Identity.Web": "None" } }, "AllowedHosts": "*" diff --git a/tests/DevApps/daemon-app/daemon-console-calling-downstreamApi/DaemonConsoleCallingDownstreamApi.csproj b/tests/DevApps/daemon-app/daemon-console-calling-downstreamApi/DaemonConsoleCallingDownstreamApi.csproj index 32933c13d..910db86af 100644 --- a/tests/DevApps/daemon-app/daemon-console-calling-downstreamApi/DaemonConsoleCallingDownstreamApi.csproj +++ b/tests/DevApps/daemon-app/daemon-console-calling-downstreamApi/DaemonConsoleCallingDownstreamApi.csproj @@ -10,6 +10,7 @@ + diff --git a/tests/DevApps/daemon-app/daemon-console-calling-downstreamApi/Program.cs b/tests/DevApps/daemon-app/daemon-console-calling-downstreamApi/Program.cs index eb667ad7e..60c7299c7 100644 --- a/tests/DevApps/daemon-app/daemon-console-calling-downstreamApi/Program.cs +++ b/tests/DevApps/daemon-app/daemon-console-calling-downstreamApi/Program.cs @@ -1,9 +1,15 @@ using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Identity.Abstractions; using Microsoft.Identity.Web; using WebApi; +// simple console logger +//var loggerFactory = LoggerFactory.Create(builder => builder.AddConsole()); + var tokenAcquirerFactory = TokenAcquirerFactory.GetDefaultInstance(); +tokenAcquirerFactory.Services.AddLogging(builder => builder.AddConsole()); + tokenAcquirerFactory.Services.AddDownstreamApi("MyApi", tokenAcquirerFactory.Configuration.GetSection("MyWebApi")); var sp = tokenAcquirerFactory.Build(); diff --git a/tests/Microsoft.Identity.Web.Test/Certificates/WithClientCredentialsTests.cs b/tests/Microsoft.Identity.Web.Test/Certificates/WithClientCredentialsTests.cs new file mode 100644 index 000000000..6a5ffedd1 --- /dev/null +++ b/tests/Microsoft.Identity.Web.Test/Certificates/WithClientCredentialsTests.cs @@ -0,0 +1,199 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Security.Cryptography.X509Certificates; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Microsoft.Identity.Abstractions; +using Microsoft.Identity.Web.Test.Common; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using Xunit; + +namespace Microsoft.Identity.Web.Test.Certificates +{ + public class WithClientCredentialsTests + { + [Fact] + public async Task FicFails_CertificateFallback() + { + // Arrange + ILogger logger = Substitute.For(); + ICredentialsLoader credLoader = Substitute.For(); + + CredentialDescription[] credentialDescriptions = new[] + { + new CredentialDescription + { + SourceType = CredentialSource.SignedAssertionFromManagedIdentity, + }, + + new CredentialDescription + { + SourceType = CredentialSource.KeyVault, + KeyVaultUrl = "https://bogus.net", + KeyVaultCertificateName = "Self-Signed-5-5-22" + } + }; + + // Mock the credential loader to fail to load FIC but load the cert + credLoader.LoadCredentialsIfNeededAsync(Arg.Any(), Arg.Any()) + .Returns(args => + { + + var cd = (args[0] as CredentialDescription)!; + + if (cd.CredentialType == CredentialType.SignedAssertion) + { + cd.Skip = true; // mimic the credential loader + return Task.FromException(new Exception($"Failed to load credential with ID {cd.Id}")); + } + else + { + cd.Certificate = Base64EncodedCertificateLoader.LoadFromBase64Encoded( + TestConstants.CertificateX5c, X509KeyStorageFlags.DefaultKeySet); + return Task.CompletedTask; + } + }); + + // Act +#pragma warning disable CS8600 // Converting null literal or possible null value to non-nullable type. + CredentialDescription cd = await ConfidentialClientApplicationBuilderExtension.LoadCredentialForMsalOrFailAsync( + credentialDescriptions, + logger, + credLoader, + null).ConfigureAwait(false); +#pragma warning restore CS8600 // Converting null literal or possible null value to non-nullable type. + + Assert.Equal(credentialDescriptions[1], cd); + } + + #region Test around failure to load creds + [Fact] + public async Task FailsForFic_ReturnsMeaningfulMessage() + { + + var ficCredential = new CredentialDescription + { + SourceType = CredentialSource.SignedAssertionFromManagedIdentity, + ManagedIdentityClientId = "9a192b78-6580-4f8a-aace-f36ffea4f7be" + }; + + await RunFailToLoadLogicAsync(new[] { ficCredential }); + } + + [Fact] + public async Task FailsForCerts_ReturnsMeaningfulMessage() + { + + var certCredential1 = new CredentialDescription + { + SourceType = CredentialSource.KeyVault, + KeyVaultUrl = "https://bogus.net", + KeyVaultCertificateName = "Self-Signed-5-5-22" + }; + var certCredential2 = new CredentialDescription + { + SourceType = CredentialSource.StoreWithThumbprint, + CertificateThumbprint = "x5t", + CertificateStorePath = "CurrentUser/My" + }; + + + await RunFailToLoadLogicAsync(new[] { certCredential1, certCredential2 }); + } + + [Fact] + public async Task FailsForFicAndCert_ReturnsMeaningfulMessage() + { + var ficCredential = new CredentialDescription + { + SourceType = CredentialSource.SignedAssertionFromManagedIdentity, + ManagedIdentityClientId = "9a192b78-6580-4f8a-aace-f36ffea4f7be" + }; + + var certCredential = new CredentialDescription + { + SourceType = CredentialSource.KeyVault, + KeyVaultUrl = "https://bogus.net", + KeyVaultCertificateName = "Self-Signed-5-5-22" + }; + + await RunFailToLoadLogicAsync(new[] { ficCredential, certCredential }); + } + + [Fact] + public async Task FailsForCertAndFic_ReturnsMeaningfulMessage() + { + var certCredential = new CredentialDescription + { + SourceType = CredentialSource.KeyVault, + KeyVaultUrl = "https://bogus.net", + KeyVaultCertificateName = "Self-Signed-5-5-22" + }; + + var ficCredential = new CredentialDescription + { + SourceType = CredentialSource.SignedAssertionFromManagedIdentity, + ManagedIdentityClientId = "9a192b78-6580-4f8a-aace-f36ffea4f7be" + }; + + await RunFailToLoadLogicAsync(new[] { ficCredential, certCredential }); + } + + [Fact] + public async Task FailsForPodAndCert_ReturnsMeaningfulMessage() + { + var certCredential = new CredentialDescription + { + SourceType = CredentialSource.KeyVault, + KeyVaultUrl = "https://bogus.net", + KeyVaultCertificateName = "Self-Signed-5-5-22" + }; + + var ficCredential = new CredentialDescription + { + SourceType = CredentialSource.SignedAssertionFilePath, + }; + + await RunFailToLoadLogicAsync(new[] { ficCredential, certCredential }); + } + + private static async Task RunFailToLoadLogicAsync(IEnumerable credentialDescriptions) + { + // Arrange + var logger = Substitute.For(); + ICredentialsLoader credLoader = Substitute.For(); + + // Mock the credential loader to fail to load any certificate + credLoader.LoadCredentialsIfNeededAsync(Arg.Any(), Arg.Any()) + .Returns(args => + { + + var cd = (args[0] as CredentialDescription)!; + cd.Skip = true; // mimic the credential loader + + return Task.FromException(new Exception($"Failed to load credential with ID {cd.Id}")); + }); + + + // Act + var ex = await Assert.ThrowsAsync(() => ConfidentialClientApplicationBuilderExtension.LoadCredentialForMsalOrFailAsync( + credentialDescriptions, + logger, + credLoader, + null)); + + // Assert + foreach (var cd in credentialDescriptions) + { + Assert.True(ex.Message.Contains($"Failed to load credential with ID {cd.Id}", StringComparison.OrdinalIgnoreCase)); + } + } + #endregion + + } +} diff --git a/tests/Microsoft.Identity.Web.Test/SignedAssertionFilePathCredentialsLoader.cs b/tests/Microsoft.Identity.Web.Test/SignedAssertionFilePathCredentialsLoader.cs index da560144e..b7bf56f5a 100644 --- a/tests/Microsoft.Identity.Web.Test/SignedAssertionFilePathCredentialsLoader.cs +++ b/tests/Microsoft.Identity.Web.Test/SignedAssertionFilePathCredentialsLoader.cs @@ -74,10 +74,11 @@ public async Task GetClientAssertion_WhenSignedAssertionFileDoesNotExist_ThrowsF { SourceType = CredentialSource.SignedAssertionFilePath, }; - await _signedAssertionFilePathCredentialsLoader.LoadIfNeededAsync(credentialDescription, null); // Act & Assert - Assert.Null(credentialDescription.CachedValue); + await Assert.ThrowsAsync(() => _signedAssertionFilePathCredentialsLoader.LoadIfNeededAsync( + credentialDescription, null)); } + } }