diff --git a/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs b/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs index fa6b589762..2bf25c8a03 100644 --- a/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs +++ b/src/Microsoft.IdentityModel.Tokens/TokenValidationParameters.cs @@ -61,7 +61,7 @@ protected TokenValidationParameters(TokenValidationParameters other) IssuerSigningKey = other.IssuerSigningKey; IssuerSigningKeyResolver = other.IssuerSigningKeyResolver; IssuerSigningKeyResolverUsingConfiguration = other.IssuerSigningKeyResolverUsingConfiguration; - IssuerSigningKeys = other.IssuerSigningKeys; + IssuerSigningKeys = other.IssuerSigningKeys is not null ? new List(other.IssuerSigningKeys) : null; IssuerSigningKeyValidator = other.IssuerSigningKeyValidator; IssuerSigningKeyValidatorUsingConfiguration = other.IssuerSigningKeyValidatorUsingConfiguration; IssuerValidator = other.IssuerValidator; @@ -72,7 +72,12 @@ protected TokenValidationParameters(TokenValidationParameters other) LogValidationExceptions = other.LogValidationExceptions; NameClaimType = other.NameClaimType; NameClaimTypeRetriever = other.NameClaimTypeRetriever; - PropertyBag = other.PropertyBag; + PropertyBag = other.PropertyBag switch + { + null => null, + Dictionary dictionary => new Dictionary(dictionary, dictionary.Comparer), + _ => new Dictionary(other.PropertyBag) + }; TryReadJwtClaim = other.TryReadJwtClaim; RefreshBeforeValidation = other.RefreshBeforeValidation; RequireAudience = other.RequireAudience; @@ -86,7 +91,7 @@ protected TokenValidationParameters(TokenValidationParameters other) SignatureValidatorUsingConfiguration = other.SignatureValidatorUsingConfiguration; TokenDecryptionKey = other.TokenDecryptionKey; TokenDecryptionKeyResolver = other.TokenDecryptionKeyResolver; - TokenDecryptionKeys = other.TokenDecryptionKeys; + TokenDecryptionKeys = other.TokenDecryptionKeys is not null ? new List(other.TokenDecryptionKeys) : null; TokenReader = other.TokenReader; TokenReplayCache = other.TokenReplayCache; TokenReplayValidator = other.TokenReplayValidator; @@ -105,12 +110,12 @@ protected TokenValidationParameters(TokenValidationParameters other) ValidateSignatureLast = other.ValidateSignatureLast; ValidateTokenReplay = other.ValidateTokenReplay; ValidateWithLKG = other.ValidateWithLKG; - ValidAlgorithms = other.ValidAlgorithms; + ValidAlgorithms = other.ValidAlgorithms is not null ? new List(other.ValidAlgorithms) : null; ValidAudience = other.ValidAudience; - ValidAudiences = other.ValidAudiences; + ValidAudiences = other.ValidAudiences is not null ? new List(other.ValidAudiences) : null; ValidIssuer = other.ValidIssuer; - ValidIssuers = other.ValidIssuers; - ValidTypes = other.ValidTypes; + ValidIssuers = other.ValidIssuers is not null ? new List(other.ValidIssuers) : null; + ValidTypes = other.ValidTypes is not null ? new List(other.ValidTypes) : null; } /// @@ -204,11 +209,11 @@ public TimeSpan ClockSkew /// /// Returns a new instance of with values copied from this object. /// - /// A new object copied from this object - /// This is a shallow Clone. + /// A new object copied from this object. + /// This clone creates new instances for collection/dictionary properties but does not deep-clone referenced objects (for example, instances) or delegates. public virtual TokenValidationParameters Clone() { - return new(this) + return new TokenValidationParameters(this) { IsClone = true }; diff --git a/test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs b/test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs index c728e0afa4..579cb2d6a1 100644 --- a/test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs +++ b/test/Microsoft.IdentityModel.Tokens.Tests/TokenValidationParametersTests.cs @@ -277,14 +277,30 @@ public void Clone() TokenValidationParameters validationParametersClone = validationParameters.Clone(); IdentityComparer.AreEqual(validationParametersClone, validationParameters, compareContext); if (validationParameters.IsClone) - compareContext.AddDiff("if (validationParameters.IsClone), IsCone should be false"); + compareContext.AddDiff("if (validationParameters.IsClone), IsClone should be false"); if (!validationParametersClone.IsClone) - compareContext.AddDiff("if (!validationParametersClone.IsClone), IsCone should be true"); + compareContext.AddDiff("if (!validationParametersClone.IsClone), IsClone should be true"); if (validationParametersClone.InstancePropertyBag.Count != 0) compareContext.AddDiff("validationParametersClone.InstancePropertyBag.Count != 0), should be empty."); + // Ensure Clone() makes independent copies of mutable collections. + if (object.ReferenceEquals(validationParameters.IssuerSigningKeys, validationParametersClone.IssuerSigningKeys)) + compareContext.AddDiff("IssuerSigningKeys should not be the same reference after Clone()."); + + int issuerSigningKeysCloneCount = ((ICollection)validationParametersClone.IssuerSigningKeys).Count; + ((List)validationParameters.IssuerSigningKeys).Add(KeyingMaterial.RsaSecurityKey_1024); + if (((ICollection)validationParametersClone.IssuerSigningKeys).Count != issuerSigningKeysCloneCount) + compareContext.AddDiff("IssuerSigningKeys in the clone should not change when the original collection is mutated."); + + if (object.ReferenceEquals(validationParameters.PropertyBag, validationParametersClone.PropertyBag)) + compareContext.AddDiff("PropertyBag should not be the same reference after Clone()."); + + validationParameters.PropertyBag["NewKey"] = obj; + if (validationParametersClone.PropertyBag.ContainsKey("NewKey")) + compareContext.AddDiff("PropertyBag in the clone should not change when the original dictionary is mutated."); + TestUtilities.AssertFailIfErrors(compareContext); }