Revert PR2597 to reduce allocations (#2649)

Co-authored-by: Sruthi Keerthi Rangavajhula (from Dev Box) <srangavajhul@microsoft.com>
AzureAD · Jun 19, 2024 · caf123f · caf123f
1 parent bd2bf8e
commit caf123f
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 73 deletions.
diff --git a/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs b/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs
@@ -13,6 +13,7 @@
 using Microsoft.IdentityModel.Protocols;
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 using Microsoft.IdentityModel.Tokens;
+using static Microsoft.IdentityModel.Validators.AadIssuerValidator;
 
 namespace Microsoft.IdentityModel.Validators
 {
@@ -382,31 +383,18 @@ private static string CreateV1Authority(string aadV2Authority, string suffixToRe
             }
         }
 
-        internal static bool IsValidIssuer(string validIssuerTemplate, string tenantId, string actualIssuer)
+        private static bool IsValidIssuer(string validIssuerTemplate, string tenantId, string actualIssuer)
         {
-            if (string.IsNullOrEmpty(validIssuerTemplate) || string.IsNullOrEmpty(actualIssuer) || string.IsNullOrEmpty(tenantId))
+            if (string.IsNullOrEmpty(validIssuerTemplate))
                 return false;
 
-            ReadOnlySpan<char> validIssuerTemplateSpan = validIssuerTemplate.AsSpan();
-            ReadOnlySpan<char> actualIssuerSpan = actualIssuer.AsSpan();
-            int indexOfTenantIdTemplate = validIssuerTemplate.IndexOf(TenantIdTemplate, StringComparison.Ordinal);
-
-            if (indexOfTenantIdTemplate >= 0 && actualIssuer.Length > indexOfTenantIdTemplate)
+            if (validIssuerTemplate.Contains(TenantIdTemplate))
             {
-                // 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));
+                return validIssuerTemplate.Replace(TenantIdTemplate, tenantId) == actualIssuer;
             }
             else
             {
-                return validIssuerTemplateSpan.SequenceEqual(actualIssuerSpan);
+                return validIssuerTemplate == actualIssuer;
             }
         }
 

diff --git a/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs b/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs
@@ -76,36 +76,31 @@ 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 because of the following scenario:
+
+                // comparing effectiveSigningKeyIssuer with v2TokenIssuer is required as well 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 (!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;
+                if (effectiveSigningKeyIssuer != tokenIssuer && effectiveSigningKeyIssuer != v2TokenIssuer)
                     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>

diff --git a/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs b/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs
diff --git a/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs b/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs
@@ -325,21 +325,6 @@ 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();