Logging Improvement for issuer/audience validation (#1945)

AzureAD · Oct 7, 2022 · a8b18ca · a8b18ca
1 parent 4d6d4f4
commit a8b18ca
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 17 deletions.
diff --git 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;
         }
 
         /// <summary>
@@ -270,6 +272,7 @@ public TokenValidationParameters()
             ValidateIssuerSigningKey = false;
             ValidateLifetime = true;
             ValidateTokenReplay = false;
+            LogAllPolicyFailuresAsError = true;
         }
 
         /// <summary>
@@ -409,6 +412,11 @@ public virtual ClaimsIdentity CreateClaimsIdentity(SecurityToken securityToken,
         /// </summary>
         public CryptoProviderFactory CryptoProviderFactory { get; set; }
 
+        /// <summary>
+        /// Gets or sets a string that helps with setting breakpoints when debugging.
+        /// </summary>
+        public string DebugId { get; set; }
+
         /// <summary>
         /// Gets or sets a boolean that controls if a '/' is significant at the end of the audience.
         /// The default is <c>true</c>.
@@ -825,5 +833,16 @@ public string RoleClaimType
         /// The default is <c>null</c>.
         /// </summary>
         public IEnumerable<string> ValidTypes { get; set; }
+
+        /// <summary>
+        /// Gets or sets a <see cref="bool"/> that will decide if cause of a policy failure needs to be logged as an error.
+        /// Default value is <c>true</c> for backward compatibility of the behavior.
+        /// If set to false, exceptions are logged as Information and then thrown.
+        /// </summary>
+        /// <remarks>
+        /// 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. 
+        /// </remarks>
+        [DefaultValue(true)]
+        public bool LogAllPolicyFailuresAsError { get; set; }
     }
 }
diff --git a/src/Microsoft.IdentityModel.Tokens/Validators.cs b/src/Microsoft.IdentityModel.Tokens/Validators.cs
@@ -105,12 +105,17 @@ public static void ValidateAudience(IEnumerable<string> 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<string> audiences, TokenValidationParameters validationParameters, IEnumerable<string> 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);
         }
 
         /// <summary>

diff --git 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