From e6f0addcb4e8e64e163372ce210ec4cf7f060a59 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 22 Aug 2023 13:10:14 -0400 Subject: [PATCH] More strongly-type multiple fields and return types (#2242) Some of these, like JsonClaimSet._jsonClaims, help to avoid allocation by ensuring that when the collection is enumerated, we're using its struct enumerator. In other cases, like with CreateClaimFromObject, we reduce the overhead of calls to the object. And other cases are just good hygiene. --- .../Json/JsonClaimSet.cs | 14 +++++++------- .../JwtTokenUtilities.cs | 6 +++--- .../OpenIdConnectProtocolValidator.cs | 2 +- .../AuthenticationProtocolMessage.cs | 2 +- .../Saml2/Saml2Evidence.cs | 6 +++--- .../AsymmetricSignatureProvider.cs | 6 ++---- .../BaseConfigurationManager.cs | 2 +- src/Microsoft.IdentityModel.Tokens/JsonWebKey.cs | 6 +++--- .../JsonWebKeySet.cs | 2 +- .../TokenUtilities.cs | 2 +- .../TokenValidationResult.cs | 4 ++-- src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs | 2 +- .../JwtSecurityTokenHandler.cs | 4 ++-- .../ConfigurationManagerTests.cs | 10 +++++----- 14 files changed, 33 insertions(+), 35 deletions(-) diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs index 5f95a34a41..2862fd258f 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs @@ -22,10 +22,10 @@ internal class JsonClaimSet internal static JsonClaimSet Empty { get; } = new JsonClaimSet("{}"u8.ToArray()); internal object _claimsLock = new(); - internal IDictionary _jsonClaims; - private IList _claims; + internal readonly Dictionary _jsonClaims; + private List _claims; - internal JsonClaimSet(IDictionary jsonClaims) + internal JsonClaimSet(Dictionary jsonClaims) { _jsonClaims = jsonClaims; } @@ -34,7 +34,7 @@ internal JsonClaimSet(byte[] jsonUtf8Bytes) _jsonClaims = JwtTokenUtilities.CreateClaimsDictionary(jsonUtf8Bytes, jsonUtf8Bytes.Length); } - internal IList Claims(string issuer) + internal List Claims(string issuer) { if (_claims == null) lock (_claimsLock) @@ -43,16 +43,16 @@ internal IList Claims(string issuer) return _claims; } - internal IList CreateClaims(string issuer) + internal List CreateClaims(string issuer) { - IList claims = new List(); + var claims = new List(); foreach (KeyValuePair kvp in _jsonClaims) CreateClaimFromObject(claims, kvp.Key, kvp.Value, issuer); return claims; } - internal static void CreateClaimFromObject(IList claims, string claimType, object value, string issuer) + internal static void CreateClaimFromObject(List claims, string claimType, object value, string issuer) { // Json.net recognized DateTime by default. if (value is string str) diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs b/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs index 568cbbfcb3..771375d194 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs @@ -533,7 +533,7 @@ internal static string GetStringClaimValueType(string str) // We need a shared model for adding claims from object for JsonWebToken and JwtSecurityToken // From getting ClaimValueTypes to setting object types - internal static IDictionary CreateClaimsDictionary(byte[] bytes, int length) + internal static Dictionary CreateClaimsDictionary(byte[] bytes, int length) { Dictionary claims = new(); Span utf8Span = bytes; @@ -575,9 +575,9 @@ internal static IDictionary CreateClaimsDictionary(byte[] bytes, /// /// /// - internal static IDictionary ParseJsonBytes(string rawString, int startIndex, int length) + internal static Dictionary ParseJsonBytes(string rawString, int startIndex, int length) { - return Base64UrlEncoding.Decode>(rawString, startIndex, length, CreateClaimsDictionary); + return Base64UrlEncoding.Decode(rawString, startIndex, length, CreateClaimsDictionary); } } } diff --git a/src/Microsoft.IdentityModel.Protocols.OpenIdConnect/OpenIdConnectProtocolValidator.cs b/src/Microsoft.IdentityModel.Protocols.OpenIdConnect/OpenIdConnectProtocolValidator.cs index b7384698f5..7ada8ec1b9 100644 --- a/src/Microsoft.IdentityModel.Protocols.OpenIdConnect/OpenIdConnectProtocolValidator.cs +++ b/src/Microsoft.IdentityModel.Protocols.OpenIdConnect/OpenIdConnectProtocolValidator.cs @@ -27,7 +27,7 @@ namespace Microsoft.IdentityModel.Protocols.OpenIdConnect /// public class OpenIdConnectProtocolValidator { - private IDictionary _hashAlgorithmMap = + private readonly Dictionary _hashAlgorithmMap = new Dictionary { { SecurityAlgorithms.EcdsaSha256, "SHA256" }, diff --git a/src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs b/src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs index 96fa9eb75e..dd9f0892fb 100644 --- a/src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs +++ b/src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs @@ -20,7 +20,7 @@ public abstract class AuthenticationProtocolMessage private string _scriptButtonText = "Submit"; private string _scriptDisabledText = "Script is disabled. Click Submit to continue."; - private IDictionary _parameters = new Dictionary(); + private readonly Dictionary _parameters = new Dictionary(); private string _issuerAddress = string.Empty; /// diff --git a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2Evidence.cs b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2Evidence.cs index 7282ba3ea7..dfd5b4e6f0 100644 --- a/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2Evidence.cs +++ b/src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2Evidence.cs @@ -18,9 +18,9 @@ namespace Microsoft.IdentityModel.Tokens.Saml2 /// public class Saml2Evidence { - private ICollection _assertionIdReferences = new List(); - private ICollection _assertions = new List(); - private AbsoluteUriCollection _assertionUriReferences = new AbsoluteUriCollection(); + private readonly List _assertionIdReferences = new List(); + private readonly List _assertions = new List(); + private readonly AbsoluteUriCollection _assertionUriReferences = new AbsoluteUriCollection(); /// /// Initializes a new instance of class. diff --git a/src/Microsoft.IdentityModel.Tokens/AsymmetricSignatureProvider.cs b/src/Microsoft.IdentityModel.Tokens/AsymmetricSignatureProvider.cs index 0bfa7b3d4e..e4c2f3534b 100644 --- a/src/Microsoft.IdentityModel.Tokens/AsymmetricSignatureProvider.cs +++ b/src/Microsoft.IdentityModel.Tokens/AsymmetricSignatureProvider.cs @@ -16,9 +16,8 @@ public class AsymmetricSignatureProvider : SignatureProvider private DisposableObjectPool _asymmetricAdapterObjectPool; private CryptoProviderFactory _cryptoProviderFactory; private bool _disposed; - private Lazy _keySizeIsValid; - private IReadOnlyDictionary _minimumAsymmetricKeySizeInBitsForSigningMap; - private IReadOnlyDictionary _minimumAsymmetricKeySizeInBitsForVerifyingMap; + private Dictionary _minimumAsymmetricKeySizeInBitsForSigningMap; + private Dictionary _minimumAsymmetricKeySizeInBitsForVerifyingMap; /// /// Mapping from algorithm to minimum .KeySize when creating signatures. @@ -131,7 +130,6 @@ public AsymmetricSignatureProvider(SecurityKey key, string algorithm, bool willC throw LogHelper.LogExceptionMessage(new NotSupportedException(LogHelper.FormatInvariant(LogMessages.IDX10634, LogHelper.MarkAsNonPII((algorithm)), key))); WillCreateSignatures = willCreateSignatures; - _keySizeIsValid = new Lazy(ValidKeySize); _asymmetricAdapterObjectPool = new DisposableObjectPool(CreateAsymmetricAdapter, _cryptoProviderFactory.SignatureProviderObjectPoolCacheSize); } diff --git a/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs b/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs index 2ae21b827f..042f69ba5c 100644 --- a/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs +++ b/src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs @@ -94,7 +94,7 @@ public virtual Task GetBaseConfigurationAsync(CancellationTok /// Gets all valid last known good configurations. /// /// A collection of all valid last known good configurations. - internal ICollection GetValidLkgConfigurations() + internal BaseConfiguration[] GetValidLkgConfigurations() { return _lastKnownGoodConfigurationCache.ToArray().Where(x => x.Value.Value > DateTime.UtcNow).Select(x => x.Key).ToArray(); } diff --git a/src/Microsoft.IdentityModel.Tokens/JsonWebKey.cs b/src/Microsoft.IdentityModel.Tokens/JsonWebKey.cs index b9504122a1..40f98ebc20 100644 --- a/src/Microsoft.IdentityModel.Tokens/JsonWebKey.cs +++ b/src/Microsoft.IdentityModel.Tokens/JsonWebKey.cs @@ -19,9 +19,9 @@ public class JsonWebKey : SecurityKey { internal const string ClassName = "Microsoft.IdentityModel.Tokens.JsonWebKey"; private Dictionary _additionalData; - private IList _keyOps; - private IList _oth; - private IList _x5c; + private List _keyOps; + private List _oth; + private List _x5c; private string _kid; /// diff --git a/src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs b/src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs index fa46dc58c0..be3d3306ba 100644 --- a/src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs +++ b/src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs @@ -19,7 +19,7 @@ namespace Microsoft.IdentityModel.Tokens public class JsonWebKeySet { internal const string ClassName = "Microsoft.IdentityModel.Tokens.JsonWebKeySet"; - private IDictionary _additionalData; + private Dictionary _additionalData; /// /// Returns a new instance of . diff --git a/src/Microsoft.IdentityModel.Tokens/TokenUtilities.cs b/src/Microsoft.IdentityModel.Tokens/TokenUtilities.cs index 8f75b45125..fc1b29f4c2 100644 --- a/src/Microsoft.IdentityModel.Tokens/TokenUtilities.cs +++ b/src/Microsoft.IdentityModel.Tokens/TokenUtilities.cs @@ -41,7 +41,7 @@ internal class TokenUtilities /// /// A list of claims. /// A Dictionary representing claims. - internal static IDictionary CreateDictionaryFromClaims(IEnumerable claims) + internal static Dictionary CreateDictionaryFromClaims(IEnumerable claims) { var payload = new Dictionary(); diff --git a/src/Microsoft.IdentityModel.Tokens/TokenValidationResult.cs b/src/Microsoft.IdentityModel.Tokens/TokenValidationResult.cs index 347e087c09..6714c71b9e 100644 --- a/src/Microsoft.IdentityModel.Tokens/TokenValidationResult.cs +++ b/src/Microsoft.IdentityModel.Tokens/TokenValidationResult.cs @@ -35,8 +35,8 @@ public class TokenValidationResult private volatile bool _claimsIdentityInitialized; private object _claimsIdentitySyncObj; private ClaimsIdentity _claimsIdentity; - private IDictionary _claims; - private IDictionary _propertyBag; + private Dictionary _claims; + private Dictionary _propertyBag; /// /// Creates an instance of diff --git a/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs b/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs index 80a6bed605..f78738d453 100644 --- a/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs +++ b/src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs @@ -663,7 +663,7 @@ private static bool TryConvertToInt(object value, ref int outVal) #pragma warning restore CS0162 // Unreachable code detected } - internal IList GetIListClaims(string claimType) + internal List GetIListClaims(string claimType) { List claimValues = new List(); diff --git a/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs b/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs index 220eab73f0..8819fa8630 100644 --- a/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs +++ b/src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs @@ -32,7 +32,7 @@ public class JwtSecurityTokenHandler : SecurityTokenHandler private const string _namespace = "http://schemas.xmlsoap.org/ws/2005/05/identity/claimproperties"; private const string _className = "System.IdentityModel.Tokens.Jwt.JwtSecurityTokenHandler"; private IDictionary _outboundClaimTypeMap; - private IDictionary _outboundAlgorithmMap = null; + private Dictionary _outboundAlgorithmMap = null; private static string _shortClaimType = _namespace + "/ShortTypeName"; private bool _mapInboundClaims = DefaultMapInboundClaims; @@ -741,7 +741,7 @@ private IEnumerable OutboundClaimTypeTransform(IEnumerable claims) } } - private IDictionary OutboundClaimTypeTransform(IDictionary claimCollection) + private Dictionary OutboundClaimTypeTransform(IDictionary claimCollection) { var claims = new Dictionary(); diff --git a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs index 0196bd7103..404318e6f5 100644 --- a/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs +++ b/test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs @@ -443,19 +443,19 @@ public void TestConfigurationComparer() configWithSameKidDiffKeyMaterial.SigningKeys.Add(new SymmetricSecurityKey(KeyingMaterial.DefaultSymmetricSecurityKey_128.Key) { KeyId = KeyingMaterial.DefaultSymmetricSecurityKey_256.KeyId }); var configurationManager = new MockConfigurationManager(config, config); - IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Count, 1, context); + IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Length, 1, context); configurationManager.LastKnownGoodConfiguration = configWithSameKeysDiffOrder; - IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Count, 1, context); + IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Length, 1, context); configurationManager.LastKnownGoodConfiguration = configWithOverlappingKey; - IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Count, 2, context); + IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Length, 2, context); configurationManager.LastKnownGoodConfiguration = configWithOverlappingKeyDiffissuer; - IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Count, 3, context); + IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Length, 3, context); configurationManager.LastKnownGoodConfiguration = configWithSameKidDiffKeyMaterial; - IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Count, 4, context); + IdentityComparer.AreEqual(configurationManager.GetValidLkgConfigurations().Length, 4, context); TestUtilities.AssertFailIfErrors(context); }