From a8b18ca94d51810fdbbff1b507ceb7c630980dd6 Mon Sep 17 00:00:00 2001 From: sruthikeerthi <73967733+sruke@users.noreply.github.com> Date: Fri, 7 Oct 2022 11:38:54 -0700 Subject: [PATCH] Logging Improvement for issuer/audience validation (#1945) --- .../TokenValidationParameters.cs | 19 ++++++++++ .../Validators.cs | 35 ++++++++++++------- .../TokenValidationParametersTests.cs | 12 ++++--- 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs b/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs index d16e2520da..1682344261 100644 --- a/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs +++ b/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs @@ -210,6 +210,7 @@ protected TokenValidationParameters(TokenValidationParameters other) ClockSkew = other.ClockSkew; ConfigurationManager = other.ConfigurationManager; CryptoProviderFactory = other.CryptoProviderFactory; + DebugId = other.DebugId; IgnoreTrailingSlashWhenValidatingAudience = other.IgnoreTrailingSlashWhenValidatingAudience; IssuerSigningKey = other.IssuerSigningKey; IssuerSigningKeyResolver = other.IssuerSigningKeyResolver; @@ -252,6 +253,7 @@ protected TokenValidationParameters(TokenValidationParameters other) ValidIssuer = other.ValidIssuer; ValidIssuers = other.ValidIssuers; ValidTypes = other.ValidTypes; + LogAllPolicyFailuresAsError = other.LogAllPolicyFailuresAsError; } /// @@ -270,6 +272,7 @@ public TokenValidationParameters() ValidateIssuerSigningKey = false; ValidateLifetime = true; ValidateTokenReplay = false; + LogAllPolicyFailuresAsError = true; } /// @@ -409,6 +412,11 @@ public virtual ClaimsIdentity CreateClaimsIdentity(SecurityToken securityToken, /// public CryptoProviderFactory CryptoProviderFactory { get; set; } + /// + /// Gets or sets a string that helps with setting breakpoints when debugging. + /// + public string DebugId { get; set; } + /// /// Gets or sets a boolean that controls if a '/' is significant at the end of the audience. /// The default is true. @@ -825,5 +833,16 @@ public string RoleClaimType /// The default is null. /// public IEnumerable ValidTypes { get; set; } + + /// + /// Gets or sets a that will decide if cause of a policy failure needs to be logged as an error. + /// Default value is true for backward compatibility of the behavior. + /// If set to false, exceptions are logged as Information and then thrown. + /// + /// + /// When multiple polices are defined, all of them are tried until one succeeds and setting this property to false will reduce the noise in the logs. + /// + [DefaultValue(true)] + public bool LogAllPolicyFailuresAsError { get; set; } } } diff --git a/src/Microsoft.IdentityModel.Tokens/Validators.cs b/src/Microsoft.IdentityModel.Tokens/Validators.cs index 499907e0b2..4601354ce8 100644 --- a/src/Microsoft.IdentityModel.Tokens/Validators.cs +++ b/src/Microsoft.IdentityModel.Tokens/Validators.cs @@ -105,12 +105,17 @@ public static void ValidateAudience(IEnumerable audiences, SecurityToken if (AudienceIsValid(audiences, validationParameters, validationParametersAudiences)) return; - throw LogHelper.LogExceptionMessage( - new SecurityTokenInvalidAudienceException(LogHelper.FormatInvariant(LogMessages.IDX10214, - LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(audiences)), - LogHelper.MarkAsNonPII(validationParameters.ValidAudience ?? "null"), - LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(validationParameters.ValidAudiences)))) - { InvalidAudience = Utility.SerializeAsSingleCommaDelimitedString(audiences) }); + SecurityTokenInvalidAudienceException ex = new SecurityTokenInvalidAudienceException( + LogHelper.FormatInvariant(LogMessages.IDX10214, + LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(audiences)), + LogHelper.MarkAsNonPII(validationParameters.ValidAudience ?? "null"), + LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(validationParameters.ValidAudiences)))) + { InvalidAudience = Utility.SerializeAsSingleCommaDelimitedString(audiences) }; + + if (!validationParameters.LogAllPolicyFailuresAsError) + throw ex; + + throw LogHelper.LogExceptionMessage(ex); } private static bool AudienceIsValid(IEnumerable audiences, TokenValidationParameters validationParameters, IEnumerable validationParametersAudiences) @@ -261,12 +266,18 @@ internal static string ValidateIssuer(string issuer, SecurityToken securityToken } } - throw LogHelper.LogExceptionMessage( - new SecurityTokenInvalidIssuerException(LogHelper.FormatInvariant(LogMessages.IDX10205, - LogHelper.MarkAsNonPII(issuer), - LogHelper.MarkAsNonPII(validationParameters.ValidIssuer ?? "null"), - LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(validationParameters.ValidIssuers)), - LogHelper.MarkAsNonPII(configuration?.Issuer))) { InvalidIssuer = issuer }); + SecurityTokenInvalidIssuerException ex = new SecurityTokenInvalidIssuerException( + LogHelper.FormatInvariant(LogMessages.IDX10205, + LogHelper.MarkAsNonPII(issuer), + LogHelper.MarkAsNonPII(validationParameters.ValidIssuer ?? "null"), + LogHelper.MarkAsNonPII(Utility.SerializeAsSingleCommaDelimitedString(validationParameters.ValidIssuers)), + LogHelper.MarkAsNonPII(configuration?.Issuer))) + { InvalidIssuer = issuer }; + + if (!validationParameters.LogAllPolicyFailuresAsError) + throw ex; + + throw LogHelper.LogExceptionMessage(ex); } /// diff --git a/test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs b/test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs index a0d0c26760..96a2183520 100644 --- a/test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs +++ b/test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs @@ -21,8 +21,8 @@ public void Publics() TokenValidationParameters validationParameters = new TokenValidationParameters(); Type type = typeof(TokenValidationParameters); PropertyInfo[] properties = type.GetProperties(); - if (properties.Length != 52) - Assert.True(false, "Number of properties has changed from 52 to: " + properties.Length + ", adjust tests"); + if (properties.Length != 54) + Assert.True(false, "Number of properties has changed from 54 to: " + properties.Length + ", adjust tests"); TokenValidationParameters actorValidationParameters = new TokenValidationParameters(); SecurityKey issuerSigningKey = KeyingMaterial.DefaultX509Key_2048_Public; @@ -82,7 +82,8 @@ public void Publics() ValidAudiences = validAudiences, ValidIssuer = validIssuer, ValidIssuers = validIssuers, - ValidTypes = validTypes + ValidTypes = validTypes, + LogAllPolicyFailuresAsError = true }; Assert.True(object.ReferenceEquals(actorValidationParameters, validationParametersInline.ActorValidationParameters)); @@ -120,6 +121,7 @@ public void Publics() validationParametersSets.ValidIssuer = validIssuer; validationParametersSets.ValidIssuers = validIssuers; validationParametersSets.ValidTypes = validTypes; + validationParametersSets.LogAllPolicyFailuresAsError = true; var compareContext = new CompareContext(); IdentityComparer.AreEqual(validationParametersInline, validationParametersSets, compareContext); @@ -140,8 +142,8 @@ public void GetSets() TokenValidationParameters validationParameters = new TokenValidationParameters(); Type type = typeof(TokenValidationParameters); PropertyInfo[] properties = type.GetProperties(); - if (properties.Length != 52) - Assert.True(false, "Number of public fields has changed from 52 to: " + properties.Length + ", adjust tests"); + if (properties.Length != 54) + Assert.True(false, "Number of public fields has changed from 54 to: " + properties.Length + ", adjust tests"); GetSetContext context = new GetSetContext