Skip to content

Commit

Permalink
More strongly-type multiple fields and return types (#2242)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stephentoub authored and Brent Schmaltz committed Sep 6, 2023
1 parent 034ec06 commit e46348a
Show file tree
Hide file tree
Showing 14 changed files with 33 additions and 36 deletions.
15 changes: 7 additions & 8 deletions src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@ internal class JsonClaimSet

internal static JsonClaimSet Empty { get; } = new JsonClaimSet("{}"u8.ToArray());
internal object _claimsLock = new();
internal IDictionary<string, object> _jsonClaims;
private IList<Claim> _claims;
private readonly object _claimsLock = new object();
internal readonly Dictionary<string, object> _jsonClaims;
private List<Claim> _claims;

internal JsonClaimSet(IDictionary<string, object> jsonClaims)
internal JsonClaimSet(Dictionary<string, object> jsonClaims)
{
_jsonClaims = jsonClaims;
}
Expand All @@ -35,7 +34,7 @@ internal JsonClaimSet(byte[] jsonUtf8Bytes)
_jsonClaims = JwtTokenUtilities.CreateClaimsDictionary(jsonUtf8Bytes, jsonUtf8Bytes.Length);
}

internal IList<Claim> Claims(string issuer)
internal List<Claim> Claims(string issuer)
{
if (_claims == null)
lock (_claimsLock)
Expand All @@ -44,16 +43,16 @@ internal IList<Claim> Claims(string issuer)
return _claims;
}

internal IList<Claim> CreateClaims(string issuer)
internal List<Claim> CreateClaims(string issuer)
{
IList<Claim> claims = new List<Claim>();
var claims = new List<Claim>();
foreach (KeyValuePair<string, object> kvp in _jsonClaims)
CreateClaimFromObject(claims, kvp.Key, kvp.Value, issuer);

return claims;
}

internal static void CreateClaimFromObject(IList<Claim> claims, string claimType, object value, string issuer)
internal static void CreateClaimFromObject(List<Claim> claims, string claimType, object value, string issuer)
{
// Json.net recognized DateTime by default.
if (value is string str)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, object> CreateClaimsDictionary(byte[] bytes, int length)
internal static Dictionary<string, object> CreateClaimsDictionary(byte[] bytes, int length)
{
Dictionary<string, object> claims = new();
Span<byte> utf8Span = bytes;
Expand Down Expand Up @@ -575,9 +575,9 @@ internal static IDictionary<string, object> CreateClaimsDictionary(byte[] bytes,
/// <param name="startIndex"></param>
/// <param name="length"></param>
/// <returns></returns>
internal static IDictionary<string,object> ParseJsonBytes(string rawString, int startIndex, int length)
internal static Dictionary<string,object> ParseJsonBytes(string rawString, int startIndex, int length)
{
return Base64UrlEncoding.Decode<IDictionary<string,object>>(rawString, startIndex, length, CreateClaimsDictionary);
return Base64UrlEncoding.Decode(rawString, startIndex, length, CreateClaimsDictionary);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace Microsoft.IdentityModel.Protocols.OpenIdConnect
/// </summary>
public class OpenIdConnectProtocolValidator
{
private IDictionary<string, string> _hashAlgorithmMap =
private readonly Dictionary<string, string> _hashAlgorithmMap =
new Dictionary<string, string>
{
{ SecurityAlgorithms.EcdsaSha256, "SHA256" },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public abstract class AuthenticationProtocolMessage
private string _scriptButtonText = "Submit";
private string _scriptDisabledText = "Script is disabled. Click Submit to continue.";

private IDictionary<string, string> _parameters = new Dictionary<string, string>();
private readonly Dictionary<string, string> _parameters = new Dictionary<string, string>();
private string _issuerAddress = string.Empty;

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ namespace Microsoft.IdentityModel.Tokens.Saml2
/// </remarks>
public class Saml2Evidence
{
private ICollection<Saml2Id> _assertionIdReferences = new List<Saml2Id>();
private ICollection<Saml2Assertion> _assertions = new List<Saml2Assertion>();
private AbsoluteUriCollection _assertionUriReferences = new AbsoluteUriCollection();
private readonly List<Saml2Id> _assertionIdReferences = new List<Saml2Id>();
private readonly List<Saml2Assertion> _assertions = new List<Saml2Assertion>();
private readonly AbsoluteUriCollection _assertionUriReferences = new AbsoluteUriCollection();

/// <summary>
/// Initializes a new instance of <see cref="Saml2Evidence"/> class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,8 @@ public class AsymmetricSignatureProvider : SignatureProvider
private DisposableObjectPool<AsymmetricAdapter> _asymmetricAdapterObjectPool;
private CryptoProviderFactory _cryptoProviderFactory;
private bool _disposed;
private Lazy<bool> _keySizeIsValid;
private IReadOnlyDictionary<string, int> _minimumAsymmetricKeySizeInBitsForSigningMap;
private IReadOnlyDictionary<string, int> _minimumAsymmetricKeySizeInBitsForVerifyingMap;
private Dictionary<string, int> _minimumAsymmetricKeySizeInBitsForSigningMap;
private Dictionary<string, int> _minimumAsymmetricKeySizeInBitsForVerifyingMap;

/// <summary>
/// Mapping from algorithm to minimum <see cref="AsymmetricSecurityKey"/>.KeySize when creating signatures.
Expand Down Expand Up @@ -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<bool>(ValidKeySize);
_asymmetricAdapterObjectPool = new DisposableObjectPool<AsymmetricAdapter>(CreateAsymmetricAdapter, _cryptoProviderFactory.SignatureProviderObjectPoolCacheSize);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public virtual Task<BaseConfiguration> GetBaseConfigurationAsync(CancellationTok
/// Gets all valid last known good configurations.
/// </summary>
/// <returns>A collection of all valid last known good configurations.</returns>
internal ICollection<BaseConfiguration> GetValidLkgConfigurations()
internal BaseConfiguration[] GetValidLkgConfigurations()
{
return _lastKnownGoodConfigurationCache.ToArray().Where(x => x.Value.Value > DateTime.UtcNow).Select(x => x.Key).ToArray();
}
Expand Down
6 changes: 3 additions & 3 deletions src/Microsoft.IdentityModel.Tokens/JsonWebKey.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ public class JsonWebKey : SecurityKey
{
internal const string ClassName = "Microsoft.IdentityModel.Tokens.JsonWebKey";
private Dictionary<string, object> _additionalData;
private IList<string> _keyOps;
private IList<string> _oth;
private IList<string> _x5c;
private List<string> _keyOps;
private List<string> _oth;
private List<string> _x5c;
private string _kid;

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.IdentityModel.Tokens/JsonWebKeySet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Microsoft.IdentityModel.Tokens
public class JsonWebKeySet
{
internal const string ClassName = "Microsoft.IdentityModel.Tokens.JsonWebKeySet";
private IDictionary<string, object> _additionalData;
private Dictionary<string, object> _additionalData;

/// <summary>
/// Returns a new instance of <see cref="JsonWebKeySet"/>.
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.IdentityModel.Tokens/TokenUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ internal class TokenUtilities
/// </summary>
/// <param name="claims"> A list of claims.</param>
/// <returns> A Dictionary representing claims.</returns>
internal static IDictionary<string, object> CreateDictionaryFromClaims(IEnumerable<Claim> claims)
internal static Dictionary<string, object> CreateDictionaryFromClaims(IEnumerable<Claim> claims)
{
var payload = new Dictionary<string, object>();

Expand Down
4 changes: 2 additions & 2 deletions src/Microsoft.IdentityModel.Tokens/TokenValidationResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public class TokenValidationResult
private volatile bool _claimsIdentityInitialized;
private object _claimsIdentitySyncObj;
private ClaimsIdentity _claimsIdentity;
private IDictionary<string, object> _claims;
private IDictionary<string, object> _propertyBag;
private Dictionary<string, object> _claims;
private Dictionary<string, object> _propertyBag;

/// <summary>
/// Creates an instance of <see cref="TokenValidationResult"/>
Expand Down
2 changes: 1 addition & 1 deletion src/System.IdentityModel.Tokens.Jwt/JwtPayload.cs
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,7 @@ private static bool TryConvertToInt(object value, ref int outVal)
#pragma warning restore CS0162 // Unreachable code detected
}

internal IList<string> GetIListClaims(string claimType)
internal List<string> GetIListClaims(string claimType)
{
List<string> claimValues = new List<string>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> _outboundClaimTypeMap;
private IDictionary<string, string> _outboundAlgorithmMap = null;
private Dictionary<string, string> _outboundAlgorithmMap = null;
private static string _shortClaimType = _namespace + "/ShortTypeName";
private bool _mapInboundClaims = DefaultMapInboundClaims;

Expand Down Expand Up @@ -741,7 +741,7 @@ private IEnumerable<Claim> OutboundClaimTypeTransform(IEnumerable<Claim> claims)
}
}

private IDictionary<string, object> OutboundClaimTypeTransform(IDictionary<string, object> claimCollection)
private Dictionary<string, object> OutboundClaimTypeTransform(IDictionary<string, object> claimCollection)
{
var claims = new Dictionary<string, object>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<OpenIdConnectConfiguration>(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);
}
Expand Down

0 comments on commit e46348a

Please sign in to comment.