Avoid extra enumerator and Concat iterator in GetAllSigningKeys (#2166)

A single iterator method can just iterate through both sources.
AzureAD · Jul 28, 2023 · 89417e5 · 89417e5
1 parent 5f1248b
commit 89417e5
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 40 deletions.
diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs b/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebTokenHandler.cs
@@ -1715,7 +1715,7 @@ private static JsonWebToken ValidateSignature(JsonWebToken jwtToken, TokenValida
                 // 1. User specified delegate: IssuerSigningKeyResolver returned null
                 // 2. ResolveIssuerSigningKey returned null
                 // Try all the keys. This is the degenerate case, not concerned about perf.
-                keys = TokenUtilities.GetAllSigningKeys(validationParameters, configuration);
+                keys = TokenUtilities.GetAllSigningKeys(configuration, validationParameters);
             }
 
             // keep track of exceptions thrown, keys that were tried
@@ -1751,7 +1751,7 @@ private static JsonWebToken ValidateSignature(JsonWebToken jwtToken, TokenValida
             }
 
             // Get information on where keys used during token validation came from for debugging purposes.
-            var keysInTokenValidationParameters = TokenUtilities.GetAllSigningKeys(validationParameters);
+            var keysInTokenValidationParameters = TokenUtilities.GetAllSigningKeys(validationParameters: validationParameters);
             var keysInConfiguration = TokenUtilities.GetAllSigningKeys(configuration);
             var numKeysInTokenValidationParameters = keysInTokenValidationParameters.Count();
             var numKeysInConfiguration = keysInConfiguration.Count();

diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlSecurityTokenHandler.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlSecurityTokenHandler.cs
@@ -1042,7 +1042,7 @@ private SamlSecurityToken ValidateSignature(SamlSecurityToken samlToken, string
                 // 1. User specified delegate: IssuerSigningKeyResolver returned null
                 // 2. ResolveIssuerSigningKey returned null
                 // Try all the keys. This is the degenerate case, not concerned about perf.
-                keys = TokenUtilities.GetAllSigningKeys(validationParameters);
+                keys = TokenUtilities.GetAllSigningKeys(validationParameters: validationParameters);
             }
 
             // keep track of exceptions thrown, keys that were tried

diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlTokenUtilities.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml/SamlTokenUtilities.cs
@@ -75,7 +75,7 @@ internal static IEnumerable<SecurityKey> GetKeysForTokenSignatureValidation(stri
                     keyMatched = false;
                     if (validationParameters.TryAllIssuerSigningKeys)
                     {
-                        return TokenUtilities.GetAllSigningKeys(validationParameters);
+                        return TokenUtilities.GetAllSigningKeys(validationParameters: validationParameters);
                     }
                 }
             }

diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.cs
@@ -423,7 +423,7 @@ private Saml2SecurityToken ValidateSignature(Saml2SecurityToken samlToken, strin
                 // 1. User specified delegate: IssuerSigningKeyResolver returned null
                 // 2. ResolveIssuerSigningKey returned null
                 // Try all the keys. This is the degenerate case, not concerned about perf.
-                keys = TokenUtilities.GetAllSigningKeys(validationParameters);
+                keys = TokenUtilities.GetAllSigningKeys(validationParameters: validationParameters);
             }
 
             // keep track of exceptions thrown, keys that were tried

diff --git a/src/Microsoft.IdentityModel.Tokens/TokenUtilities.cs b/src/Microsoft.IdentityModel.Tokens/TokenUtilities.cs
@@ -112,47 +112,38 @@ internal static object GetClaimValueUsingValueType(Claim claim)
         }
 
         /// <summary>
-        /// Returns all <see cref="SecurityKey"/> provided in validationParameters.
+        /// Returns all <see cref="SecurityKey"/> provided in <paramref name="configuration"/> and <paramref name="validationParameters"/>.
         /// </summary>
+        /// <param name="configuration">The <see cref="BaseConfiguration"/> that contains signing keys used for validation.</param>
         /// <param name="validationParameters">A <see cref="TokenValidationParameters"/> required for validation.</param>
-        /// <returns>Returns all <see cref="SecurityKey"/> provided in validationParameters.</returns>
-        internal static IEnumerable<SecurityKey> GetAllSigningKeys(TokenValidationParameters validationParameters)
+        /// <returns>Returns all <see cref="SecurityKey"/> provided in provided in <paramref name="configuration"/> and <paramref name="validationParameters"/>.</returns>
+        internal static IEnumerable<SecurityKey> GetAllSigningKeys(BaseConfiguration configuration = null, TokenValidationParameters validationParameters = null)
         {
-            LogHelper.LogInformation(TokenLogMessages.IDX10243);
-            if (validationParameters.IssuerSigningKey != null)
-                yield return validationParameters.IssuerSigningKey;
-
-            if (validationParameters.IssuerSigningKeys != null)
-                foreach (SecurityKey key in validationParameters.IssuerSigningKeys)
-                    yield return key;
-        }
+            if (configuration is not null)
+            {
+                if (validationParameters is not null)
+                {
+                    LogHelper.LogInformation(TokenLogMessages.IDX10264);
+                }
 
+                LogHelper.LogInformation(TokenLogMessages.IDX10265);
 
-        /// <summary>
-        /// Returns all <see cref="SecurityKey"/> provided in <paramref name="configuration"/>.
-        /// </summary>
-        /// <param name="configuration">The <see cref="BaseConfiguration"/> that contains signing keys used for validation.</param>
-        /// <returns>Returns all <see cref="SecurityKey"/> provided in provided in <paramref name="configuration"/>.</returns>
-        internal static IEnumerable<SecurityKey> GetAllSigningKeys(BaseConfiguration configuration)
-        {
-            LogHelper.LogInformation(TokenLogMessages.IDX10265);
+                if (configuration?.SigningKeys != null)
+                    foreach (SecurityKey key in configuration.SigningKeys)
+                        yield return key;
+            }
 
-            if (configuration?.SigningKeys != null)
-                foreach (SecurityKey key in configuration.SigningKeys)
-                    yield return key;
-        }
+            if (validationParameters is not null)
+            {
+                LogHelper.LogInformation(TokenLogMessages.IDX10243);
 
-        /// <summary>
-        /// Returns all <see cref="SecurityKey"/> provided in <paramref name="configuration"/> and <paramref name="validationParameters"/>.
-        /// </summary>
-        /// <param name="configuration">The <see cref="BaseConfiguration"/> that contains signing keys used for validation.</param>
-        /// <param name="validationParameters">A <see cref="TokenValidationParameters"/> required for validation.</param>
-        /// <returns>Returns all <see cref="SecurityKey"/> provided in provided in <paramref name="configuration"/> and <paramref name="validationParameters"/>.</returns>
-        internal static IEnumerable<SecurityKey> GetAllSigningKeys(TokenValidationParameters validationParameters, BaseConfiguration configuration)
-        {
-            LogHelper.LogInformation(TokenLogMessages.IDX10264);
+                if (validationParameters.IssuerSigningKey != null)
+                    yield return validationParameters.IssuerSigningKey;
 
-            return GetAllSigningKeys(configuration).Concat(GetAllSigningKeys(validationParameters));
+                if (validationParameters.IssuerSigningKeys != null)
+                    foreach (SecurityKey key in validationParameters.IssuerSigningKeys)
+                        yield return key;
+            }
         }
 
         /// <summary>

diff --git a/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs b/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs
@@ -1343,7 +1343,7 @@ private JwtSecurityToken ValidateSignature(string token, JwtSecurityToken jwtTok
                 // 1. User specified delegate: IssuerSigningKeyResolver returned null
                 // 2. ResolveIssuerSigningKey returned null
                 // Try all the keys. This is the degenerate case, not concerned about perf.
-                keys = TokenUtilities.GetAllSigningKeys(validationParameters, configuration);
+                keys = TokenUtilities.GetAllSigningKeys(configuration, validationParameters);
             }
 
             // keep track of exceptions thrown, keys that were tried
@@ -1390,7 +1390,7 @@ private JwtSecurityToken ValidateSignature(string token, JwtSecurityToken jwtTok
             }
 
             // Get information on where keys used during token validation came from for debugging purposes.
-            var keysInTokenValidationParameters = TokenUtilities.GetAllSigningKeys(validationParameters);
+            var keysInTokenValidationParameters = TokenUtilities.GetAllSigningKeys(validationParameters: validationParameters);
             var keysInConfiguration = TokenUtilities.GetAllSigningKeys(configuration);
             var numKeysInTokenValidationParameters = keysInTokenValidationParameters.Count();
             var numKeysInConfiguration = keysInConfiguration.Count();