From f13540453f1cd34e9ada84cd3126e6fced87d096 Mon Sep 17 00:00:00 2001 From: "Sruthi Keerthi Rangavajhula (from Dev Box)" Date: Sun, 15 Sep 2024 17:41:33 -0700 Subject: [PATCH 1/2] Use spans to compare token issuer with templated issuer and fix prior index out of range exception --- .../AadIssuerValidator/AadIssuerValidator.cs | 30 ++++- .../AadIssuerValidatorTests.cs | 107 ++++++++++++++++++ 2 files changed, 132 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..6cc7b7758b --- /dev/null +++ b/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs @@ -0,0 +1,107 @@ +// 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, + }, + + // 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, + } + }; + + 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; } + } +} From 90fa8c2ace27e3b736558d1bcb67cde7187ee48a Mon Sep 17 00:00:00 2001 From: "Sruthi Keerthi Rangavajhula (from Dev Box)" Date: Thu, 19 Sep 2024 09:35:52 -0700 Subject: [PATCH 2/2] Address feedback --- .../AadIssuerValidatorTests.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs b/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs index 6cc7b7758b..4fc6183144 100644 --- a/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs +++ b/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs @@ -40,6 +40,13 @@ public static TheoryData AadIssuerValidationTestCa 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") @@ -83,7 +90,14 @@ public static TheoryData AadIssuerValidationTestCa 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;