Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More strongly-type multiple fields and return types #2242

Merged
merged 1 commit into from
Aug 22, 2023
Merged

More strongly-type multiple fields and return types #2242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/Microsoft.IdentityModel.JsonWebTokens/Json/JsonClaimSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +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;
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 @@ -34,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 @@ -43,16 +43,16 @@ internal IList<Claim> Claims(string issuer)
return _claims;
}

internal IList<Claim> CreateClaims(string issuer)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephentoub Notice you changed all Interfaces to Concrete Subtypes. Just curious what is the benefit or advantage of this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the advantages were noted in the PR description.

For example, if you have:

IList<T> list = ...;
foreach (T item in list) { ... }

that allocates an IEnumerator<T> object. If you instead have:

List<T> list = ...;
foreach (T item in list) { ... }

it doesn't, instead using the List<T>.Enumerator struct for enumeration.

But even just simple accesses, e.g.

IList<T> list = ...;
return list[2];

That involves interface dispatch, which is not only slower than a regular method invocation:

List<T> list = ...;
return list[2];

but without a technology like dynamic PGO, it can't be inlined.

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
6 changes: 3 additions & 3 deletions src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/Microsoft.IdentityModel.Protocols.OpenIdConnect/OpenIdConnectProtocolValidator.cs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/Microsoft.IdentityModel.Protocols/AuthenticationProtocolMessage.cs
View file
Open in desktop
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
6 changes: 3 additions & 3 deletions src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2Evidence.cs
View file
Open in desktop
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
6 changes: 2 additions & 4 deletions src/Microsoft.IdentityModel.Tokens/AsymmetricSignatureProvider.cs
View file
Open in desktop
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
2 changes: 1 addition & 1 deletion src/Microsoft.IdentityModel.Tokens/BaseConfigurationManager.cs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
4 changes: 2 additions & 2 deletions src/System.IdentityModel.Tokens.Jwt/JwtSecurityTokenHandler.cs
View file
Open in desktop
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
10 changes: 5 additions & 5 deletions test/Microsoft.IdentityModel.Protocols.OpenIdConnect.Tests/ConfigurationManagerTests.cs
View file
Open in desktop
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