diff --git a/CHANGELOG.md b/CHANGELOG.md index ec96da3605..79b39aa066 100644 --- a/CHANGELOG.md +++ b/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: diff --git a/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs b/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs index 323718cc84..445671d134 100644 --- a/src/Microsoft.IdentityModel.Validators/AadIssuerValidator/AadIssuerValidator.cs +++ b/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 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 validIssuerTemplateSpan = validIssuerTemplate.AsSpan(); + ReadOnlySpan 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); } } diff --git a/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs b/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs index d745bf124c..5335284168 100644 --- a/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs +++ b/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 + } + /// /// Validates the issuer signing key certificate. /// diff --git a/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs b/test/Microsoft.IdentityModel.Validators.Tests/AadIssuerValidatorTests.cs new file mode 100644 index 0000000000..4e05268f1f --- /dev/null +++ b/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); + } + } +} diff --git a/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs b/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs index e768a2372f..b979f7c49e 100644 --- a/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs +++ b/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs @@ -325,6 +325,21 @@ public static TheoryData 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();