Reduce string allocations by comparing issuers in-place (#2597)

* compare issuers without string allocations

* add changelog details for this bug

* comment update

* use spans

* fix concat error

* change request

* Delay creation of issuer until necessary + remove unecessary indexOf call

* Add unit tests for new methods

* Forgot to push - update internal method

* Change requests

* Delay calling indexOf

* change requests

* Add additional test case for IsValidIssuer

CHANGELOG.md

@@ -1,5 +1,11 @@
 See the [releases](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/releases) for details on bug fixes and added features.
 
+
+Pending Next Release
+=====
+### Bug Fixes:
+- Reduced allocations in `AadIssuerValidator` by not using `string.Replace` where appropriate. See issue [#2595](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/2595) and PR [#2597](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/2597) for more details.
+
 7.6.0
 =====
 ### New Features:

src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs

@@ -13,7 +13,6 @@
 using Microsoft.IdentityModel.Protocols;
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 using Microsoft.IdentityModel.Tokens;
-using static Microsoft.IdentityModel.Validators.AadIssuerValidator;
 
 namespace Microsoft.IdentityModel.Validators
 {
@@ -383,18 +382,31 @@ private ConfigurationManager<OpenIdConnectConfiguration> CreateConfigManager(
             }
         }
 
-        private static bool IsValidIssuer(string validIssuerTemplate, string tenantId, string actualIssuer)
+        internal static bool IsValidIssuer(string validIssuerTemplate, string tenantId, string actualIssuer)
         {
-            if (string.IsNullOrEmpty(validIssuerTemplate))
+            if (string.IsNullOrEmpty(validIssuerTemplate) || string.IsNullOrEmpty(actualIssuer) || string.IsNullOrEmpty(tenantId))
                 return false;
 
-            if (validIssuerTemplate.Contains(TenantIdTemplate))
+            ReadOnlySpan<char> validIssuerTemplateSpan = validIssuerTemplate.AsSpan();
+            ReadOnlySpan<char> actualIssuerSpan = actualIssuer.AsSpan();
+            int indexOfTenantIdTemplate = validIssuerTemplate.IndexOf(TenantIdTemplate, StringComparison.Ordinal);
+
+            if (indexOfTenantIdTemplate >= 0 && actualIssuer.Length > indexOfTenantIdTemplate)
             {
-                return validIssuerTemplate.Replace(TenantIdTemplate, tenantId) == actualIssuer;
+                // ensure the first part of the validIssuerTemplate matches the first part of actualIssuer
+                if (!validIssuerTemplateSpan.Slice(0, indexOfTenantIdTemplate).SequenceEqual(actualIssuerSpan.Slice(0, indexOfTenantIdTemplate)))
+                    return false;
+
+                // ensure that actualIssuer contains the tenantId from indexOfTenantIdTemplate for the length of tenantId.Length
+                if (!actualIssuerSpan.Slice(indexOfTenantIdTemplate, tenantId.Length).SequenceEqual(tenantId.AsSpan()))
+                    return false;
+
+                // ensure the second halves are equal
+                return validIssuerTemplateSpan.Slice(indexOfTenantIdTemplate + TenantIdTemplate.Length).SequenceEqual(actualIssuerSpan.Slice(indexOfTenantIdTemplate + tenantId.Length));
             }
             else
             {
-                return validIssuerTemplate == actualIssuer;
+                return validIssuerTemplateSpan.SequenceEqual(actualIssuerSpan);
             }
         }
 

src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs

@@ -76,31 +76,36 @@ internal static bool ValidateIssuerSigningKey(SecurityKey securityKey, SecurityT
 #if NET6_0_OR_GREATER
                 if (!string.IsNullOrEmpty(tokenIssuer) && !tokenIssuer.Contains(tenantIdFromToken, StringComparison.Ordinal))
                     throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogHelper.FormatInvariant(LogMessages.IDX40004, LogHelper.MarkAsNonPII(tokenIssuer), LogHelper.MarkAsNonPII(tenantIdFromToken))));
-
-                // creating an effectiveSigningKeyIssuer is required as signingKeyIssuer might contain {tenantid}
-                var effectiveSigningKeyIssuer = signingKeyIssuer.Replace(AadIssuerValidator.TenantIdTemplate, tenantIdFromToken, StringComparison.Ordinal);
-                var v2TokenIssuer = openIdConnectConfiguration.Issuer?.Replace(AadIssuerValidator.TenantIdTemplate, tenantIdFromToken, StringComparison.Ordinal);
 #else
                 if (!string.IsNullOrEmpty(tokenIssuer) && !tokenIssuer.Contains(tenantIdFromToken))
                     throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogHelper.FormatInvariant(LogMessages.IDX40004, LogHelper.MarkAsNonPII(tokenIssuer), LogHelper.MarkAsNonPII(tenantIdFromToken))));
-
-                // creating an effectiveSigningKeyIssuer is required as signingKeyIssuer might contain {tenantid}
-                var effectiveSigningKeyIssuer = signingKeyIssuer.Replace(AadIssuerValidator.TenantIdTemplate, tenantIdFromToken);
-                var v2TokenIssuer = openIdConnectConfiguration.Issuer?.Replace(AadIssuerValidator.TenantIdTemplate, tenantIdFromToken);
 #endif
-
-                // comparing effectiveSigningKeyIssuer with v2TokenIssuer is required as well because of the following scenario:
+                // comparing effectiveSigningKeyIssuer with v2TokenIssuer is required because of the following scenario:
                 // 1. service trusts /common/v2.0 endpoint 
                 // 2. service receieves a v1 token that has issuer like sts.windows.net
                 // 3. signing key issuers will never match sts.windows.net as v1 endpoint doesn't have issuers attached to keys
                 // v2TokenIssuer is the representation of Token.Issuer (if it was a v2 issuer)
-                if (effectiveSigningKeyIssuer != tokenIssuer && effectiveSigningKeyIssuer != v2TokenIssuer)
+                if (!AadIssuerValidator.IsValidIssuer(signingKeyIssuer, tenantIdFromToken, tokenIssuer)
+                    && !AadIssuerValidator.IsValidIssuer(signingKeyIssuer, tenantIdFromToken, openIdConnectConfiguration.Issuer))
+                {
+                    int templateStartIndex = signingKeyIssuer.IndexOf(AadIssuerValidator.TenantIdTemplate, StringComparison.Ordinal);
+                    string effectiveSigningKeyIssuer = templateStartIndex > -1 ? CreateIssuer(signingKeyIssuer, AadIssuerValidator.TenantIdTemplate, tenantIdFromToken, templateStartIndex) : signingKeyIssuer;
                     throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogHelper.FormatInvariant(LogMessages.IDX40005, LogHelper.MarkAsNonPII(tokenIssuer), LogHelper.MarkAsNonPII(effectiveSigningKeyIssuer))));
+                }
             }
 
             return true;
         }
 
+        internal static string CreateIssuer(string issuer, string tenantIdTemplate, string tenantId, int templateStartIndex)
+        {
+#if NET6_0_OR_GREATER
+            return string.Concat(issuer.AsSpan(0, templateStartIndex), tenantId, issuer.AsSpan(templateStartIndex + tenantIdTemplate.Length, issuer.Length - tenantIdTemplate.Length - templateStartIndex));
+#else
+            return string.Concat(issuer.Substring(0, templateStartIndex), tenantId, issuer.Substring(templateStartIndex + tenantIdTemplate.Length));
+#endif
+        }
+
         /// <summary>
         /// Validates the issuer signing key certificate.
         /// </summary>

test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs

@@ -0,0 +1,24 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+using Xunit;
+
+namespace Microsoft.IdentityModel.Validators.Tests
+{
+    public class AadIssuerValidatorTests
+    {
+        [Theory]
+        [InlineData(ValidatorConstants.AadInstance + AadIssuerValidator.TenantIdTemplate, ValidatorConstants.AadInstance + ValidatorConstants.TenantIdAsGuid, true)]
+        [InlineData(ValidatorConstants.AadInstancePPE + AadIssuerValidator.TenantIdTemplate, ValidatorConstants.AadInstance + ValidatorConstants.TenantIdAsGuid, false)]
+        [InlineData(ValidatorConstants.AadInstance + AadIssuerValidator.TenantIdTemplate, ValidatorConstants.AadInstance + ValidatorConstants.B2CTenantAsGuid, false)]
+        [InlineData("", ValidatorConstants.AadInstance + ValidatorConstants.TenantIdAsGuid, false)]
+        [InlineData(ValidatorConstants.AadInstance + AadIssuerValidator.TenantIdTemplate, "", false)]
+        public static void IsValidIssuer_CanValidateTemplatedIssuers(string templatedIssuer, string issuer, bool expectedResult)
+        {
+            // act
+            var result = AadIssuerValidator.IsValidIssuer(templatedIssuer, ValidatorConstants.TenantIdAsGuid, issuer);
+
+            // assert
+            Assert.Equal(expectedResult, result);
+        }
+    }
+}

test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs

@@ -325,6 +325,21 @@ public static TheoryData<AadSigningKeyIssuerTheoryData> ValidateIssuerSigningKey
             }
         }
 
+        [Fact]
+        public static void CreateIssuer_ReturnsExpectedIssuer()
+        {
+            // arrange
+            var issuerTemplate = "{tenantId}";
+            var issuer = ValidatorConstants.AadInstance + issuerTemplate;
+            int templateStartIndex = issuer.IndexOf(issuerTemplate);
+
+            // act
+            var result = AadTokenValidationParametersExtension.CreateIssuer(issuer, issuerTemplate, ValidatorConstants.TenantIdAsGuid, templateStartIndex);
+
+            // assert
+            Assert.Equal(ValidatorConstants.AadInstance + ValidatorConstants.TenantIdAsGuid, result);
+        }
+
         private static OpenIdConnectConfiguration GetConfigurationMock()
         {
             var config = new OpenIdConnectConfiguration();