From a9380abd942c9e54566f2213ad6d956ecf69c8ea Mon Sep 17 00:00:00 2001 From: sruthikeerthi <73967733+sruke@users.noreply.github.com> Date: Thu, 19 Sep 2024 13:06:05 -0700 Subject: [PATCH] Uses spans for issuer comparison and fixes prior index out of range error (#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) --- .../AadIssuerValidator/AadIssuerValidator.cs | 30 ++++- .../AadIssuerValidatorTests.cs | 121 ++++++++++++++++++ 2 files changed, 146 insertions(+), 5 deletions(-) create mode 100644 test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs diff --git a/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs b/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs index a20c1ac4f4..dd95322e06 100644 --- a/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs +++ b/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs @@ -382,18 +382,38 @@ private ConfigurationManager 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 issuerTemplateSpan = issuerTemplate.AsSpan(); + ReadOnlySpan 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 new file mode 100644 index 0000000000..4fc6183144 --- /dev/null +++ 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 AadIssuerValidationTestCases() + { + var theoryData = new TheoryData + { + // 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; } + } +}