From 089e707b5f039fda93985b67f3e2f9e4ab741216 Mon Sep 17 00:00:00 2001 From: Ignacio Inglese Date: Mon, 4 Nov 2024 21:55:10 +0000 Subject: [PATCH] Addressed PR feedback --- ...lSecurityTokenHandler.ValidateSignature.cs | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlSecurityTokenHandler.ValidateSignature.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlSecurityTokenHandler.ValidateSignature.cs index 99e50582c3..196ed69069 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlSecurityTokenHandler.ValidateSignature.cs +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlSecurityTokenHandler.ValidateSignature.cs @@ -78,8 +78,8 @@ internal static ValidationResult ValidateSignature( } bool canMatchKey = samlToken.Assertion.Signature.KeyInfo != null; - List errors = new(); - StringBuilder keysAttempted = new(); + List? errors = null; + StringBuilder? keysAttempted = null; if (keys is not null) { @@ -95,7 +95,7 @@ internal static ValidationResult ValidateSignature( if (!algorithmValidationResult.IsValid) { - errors.Add(algorithmValidationResult.UnwrapError()); + (errors ??= new()).Add(algorithmValidationResult.UnwrapError()); } else { @@ -112,11 +112,11 @@ internal static ValidationResult ValidateSignature( } else { - errors.Add(validationError.AddStackFrame(new StackFrame())); + (errors ??= new()).Add(validationError.AddStackFrame(new StackFrame())); } } - keysAttempted.Append(key.ToString()); + (keysAttempted ??= new()).Append(key.ToString()); if (canMatchKey && !keyMatched && key.KeyId is not null && samlToken.Assertion.Signature.KeyInfo is not null) keyMatched = samlToken.Assertion.Signature.KeyInfo.MatchesKey(key); } @@ -126,7 +126,7 @@ internal static ValidationResult ValidateSignature( return new XmlValidationError( new MessageDetail( TokenLogMessages.IDX10514, - keysAttempted.ToString(), + keysAttempted?.ToString(), samlToken.Assertion.Signature.KeyInfo, GetErrorStrings(errors), samlToken), @@ -134,11 +134,11 @@ internal static ValidationResult ValidateSignature( typeof(SecurityTokenInvalidSignatureException), new StackFrame(true)); - if (keysAttempted.Length > 0) + if ((keysAttempted?.Length ?? 0) > 0) return new XmlValidationError( new MessageDetail( TokenLogMessages.IDX10512, - keysAttempted.ToString(), + keysAttempted!.ToString(), GetErrorStrings(errors), samlToken), ValidationFailureType.SignatureValidationFailed, @@ -152,12 +152,20 @@ internal static ValidationResult ValidateSignature( new StackFrame(true)); } - private static string GetErrorStrings(List errors) + private static string GetErrorStrings(List? errors) { + // This method is called if there are errors in the signature validation process. + // This check is there to account for the optional parameter. + if (errors is null) + return string.Empty; + + if (errors.Count == 1) + return errors[0].MessageDetail.Message; + StringBuilder sb = new(); for (int i = 0; i < errors.Count; i++) { - sb.AppendLine(errors[i].ToString()); + sb.AppendLine(errors[i].MessageDetail.Message); } return sb.ToString();