dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.55k stars 4.54k forks source link

[API Proposal]: Add Claim.HasProperties to avoid allocating Properties Dictionary #101922

Open eerhardt opened 2 months ago

eerhardt commented 2 months ago

Background and motivation

System.Security.Claims.Claim will lazy load its Properties dictionary:

https://github.com/dotnet/runtime/blob/84b33395057737db3ea342a5151feb6b90c1b6f6/src/libraries/System.Security.Claims/src/System/Security/Claims/Claim.cs#L344

However, if you are writing code to inspect the Claim and checking if there are any Properties, the only way to check is to allocate this Dictionary just to see that its Count == 0. For example, here is some code from the JWT handling code in Microsoft.IdentityModel.JsonWebTokens:

            foreach (Claim jwtClaim in jwtToken.Claims)
            {
                ...

                if (jwtClaim.Properties.Count == 0)
                {
                    identity.AddClaim(new Claim(claimType, jwtClaim.Value, jwtClaim.ValueType, issuer, issuer, identity));
                }
                else
                {
                    Claim claim = new Claim(claimType, jwtClaim.Value, jwtClaim.ValueType, issuer, issuer, identity);

                    foreach (var kv in jwtClaim.Properties)
                        claim.Properties[kv.Key] = kv.Value;

                    identity.AddClaim(claim);
                }

Just doing that first check allocates the Dictionary on the Claim and then basically just throws it away (since the Claim has no Properties).

API Proposal

namespace System.Security.Claims;

public class Claim
{
+    public bool HasProperties { get; }
}

API Usage

foreach (Claim jwtClaim in jwtToken.Claims)
{
    ...

    Claim claim = new Claim(claimType, jwtClaim.Value, jwtClaim.ValueType, issuer, issuer, identity);
    if (jwtClaim.HasProperties)
    {
        foreach (var kv in jwtClaim.Properties)
            claim.Properties[kv.Key] = kv.Value;
    }
    identity.AddClaim(claim);

Alternative Designs

No response

Risks

No response

dotnet-policy-service[bot] commented 2 months ago

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones See info in area-owners.md if you want to be subscribed.

jmprieur commented 2 months ago

@brentschmaltz @keegan-caruso FYI