Uses spans for issuer comparison and fixes prior index out of range e…

…rror (#2826) * Use spans to compare token issuer with templated issuer and fix prior index out of range exception * Address feedback --------- Co-authored-by: Sruthi Keerthi Rangavajhula (from Dev Box) <[email protected]>
AzureAD · Sep 19, 2024 · a9380ab · a9380ab
1 parent 5f437df
commit a9380ab
Show file tree

Hide file tree

Showing 2 changed files with 146 additions and 5 deletions.
diff --git a/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs b/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs
@@ -382,18 +382,38 @@ private ConfigurationManager<OpenIdConnectConfiguration> CreateConfigManager(
             }
         }
 
-        private static bool IsValidIssuer(string validIssuerTemplate, string tenantId, string actualIssuer)
+        internal static bool IsValidIssuer(string issuerTemplate, string tenantId, string tokenIssuer)
         {
-            if (string.IsNullOrEmpty(validIssuerTemplate))
+            if (string.IsNullOrEmpty(issuerTemplate) || string.IsNullOrEmpty(tenantId) || string.IsNullOrEmpty(tokenIssuer))
                 return false;
 
-            if (validIssuerTemplate.Contains(TenantIdTemplate))
+            ReadOnlySpan<char> issuerTemplateSpan = issuerTemplate.AsSpan();
+            ReadOnlySpan<char> tokenIssuerSpan = tokenIssuer.AsSpan();
+            int templateTenantIdPosition = issuerTemplate.IndexOf(TenantIdTemplate, StringComparison.Ordinal);
+
+            // If the template contains the tenantIdTemplate, ensure the actual issuer matches the template with the tenantId replaced.
+            if (templateTenantIdPosition >= 0 && tokenIssuer.Length > templateTenantIdPosition)
             {
-                return validIssuerTemplate.Replace(TenantIdTemplate, tenantId) == actualIssuer;
+                // Ensure the prefix of the issuer template matches the token issuer's prefix
+                if (!issuerTemplateSpan.Slice(0, templateTenantIdPosition).SequenceEqual(tokenIssuerSpan.Slice(0, templateTenantIdPosition)))
+                    return false;
+
+                // Ensure tokenIssuer is atleast as long as issuerTemplate with tenantIdTemplate replaced
+                if (tokenIssuer.Length <= templateTenantIdPosition + tenantId.Length)
+                    return false;
+
+                // Ensure the tenant ID in the token issuer matches the expected tenant ID
+                if (!tokenIssuerSpan.Slice(templateTenantIdPosition, tenantId.Length).SequenceEqual(tenantId.AsSpan()))
+                    return false;
+
+                // Ensure the suffixes of both issuer template and token issuer match
+                return issuerTemplateSpan.Slice(templateTenantIdPosition + TenantIdTemplate.Length)
+                    .SequenceEqual(tokenIssuerSpan.Slice(templateTenantIdPosition + tenantId.Length));
             }
             else
             {
-                return validIssuerTemplate == actualIssuer;
+                // If no tenant ID template exists, directly compare issuerTemplate and tokenIssuer
+                return issuerTemplateSpan.SequenceEqual(tokenIssuerSpan);
             }
         }
 

diff --git a/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs b/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs
@@ -0,0 +1,121 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using Microsoft.IdentityModel.TestUtils;
+using Xunit;
+
+namespace Microsoft.IdentityModel.Validators.Tests
+{
+    public class AadIssuerValidatorTests
+    {
+        [Theory, MemberData(nameof(AadIssuerValidationTestCases))]
+        public static void IsValidIssuer_ValidatesIssuersCorrectly(AadIssuerValidatorTheoryData theoryData)
+        {
+            // Act
+            var validationResult = AadIssuerValidator.IsValidIssuer(
+                theoryData.TemplatedIssuer,
+                theoryData.TenantIdClaim,
+                theoryData.TokenIssuer);
+
+            // Assert
+            Assert.Equal(theoryData.ExpectedResult, validationResult);
+        }
+
+        public static TheoryData<AadIssuerValidatorTheoryData> AadIssuerValidationTestCases()
+        {
+            var theoryData = new TheoryData<AadIssuerValidatorTheoryData>
+            {
+                // Success cases
+                new AadIssuerValidatorTheoryData("V1_Template_Matches_V1_Issuer_Success")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuerV1CommonAuthority,
+                    TokenIssuer = ValidatorConstants.V1Issuer,
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = true,
+                },
+                new AadIssuerValidatorTheoryData("V2_Template_Matches_V2_Issuer_Success")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuerV2CommonAuthority,
+                    TokenIssuer = ValidatorConstants.AadIssuer,
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = true,
+                },
+                new AadIssuerValidatorTheoryData("IssuerTemplate_WithTenantId_TokenIssuer_Match_Success")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuer,
+                    TokenIssuer = ValidatorConstants.AadIssuer,
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = true,
+                },
+
+                // Failure cases
+                new AadIssuerValidatorTheoryData("V1_Template_With_V2_Issuer_Failure")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuerV1CommonAuthority,
+                    TokenIssuer = ValidatorConstants.AadIssuer,
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = false,
+                },
+                new AadIssuerValidatorTheoryData("V2_Template_With_V1_Issuer_Failure")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuerV2CommonAuthority,
+                    TokenIssuer = ValidatorConstants.V1Issuer,
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = false,
+                },
+                new AadIssuerValidatorTheoryData("Null_TokenIssuer_Failure")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuerV1CommonAuthority,
+                    TokenIssuer = "",
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = false,
+                },
+                new AadIssuerValidatorTheoryData("Null_TenantId_Failure")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuerV1CommonAuthority,
+                    TokenIssuer = ValidatorConstants.AadIssuer,
+                    TenantIdClaim = "",
+                    ExpectedResult = false,
+                },
+                new AadIssuerValidatorTheoryData("PPE_Template_With_V1_Issuer_Failure")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadInstancePPE + "/" + AadIssuerValidator.TenantIdTemplate,
+                    TokenIssuer =  ValidatorConstants.AadInstance + "/" + ValidatorConstants.TenantIdAsGuid,
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = false,
+                },
+                new AadIssuerValidatorTheoryData("Malformed_V2_TokenIssuer_Failure")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuerV2CommonAuthority,
+                    TokenIssuer = "https://login.microsoftonline.com/{tenantid}/v2.0",
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = false,
+                },
+                new AadIssuerValidatorTheoryData("IssuerTemplate_WithTenantId_TokenIssuer_NoMatch_Failure")
+                {
+                    TemplatedIssuer = ValidatorConstants.AadIssuerPPE,
+                    TokenIssuer = ValidatorConstants.AadIssuer,
+                    TenantIdClaim = ValidatorConstants.TenantIdAsGuid,
+                    ExpectedResult = false,
+                },
+            };
+
+            return theoryData;
+        }
+    }
+
+    public class AadIssuerValidatorTheoryData : TheoryDataBase
+    {
+        public AadIssuerValidatorTheoryData() {}
+
+        public AadIssuerValidatorTheoryData(string testId) : base(testId) { }
+
+        public string TemplatedIssuer { get; set; }
+
+        public string TokenIssuer { get; set; }
+
+        public string TenantIdClaim { get; set; }
+
+        public bool ExpectedResult { get; set; }
+    }
+}