Runtime: Add ClaimsIdentity.NameIdentifier property.

[Bartmax]

ClaimsIdentity currently expose the Name property but feels like it missing the NameIdentifier property. Since Claim is the recommended way to work with authentication and the most common property that developers needs to work with when dealing with users is the UserId, known as the subject or NameIdentifier claim, it's a great opportunity to make a better api for developers to access that value.

Rationale and Usage

This is certainly a nice-to-have, but it is a long-standing issue in various versions of ASP.NET Identity where there were always questions and workarounds in how to get the user id (a pretty basic need), the use of different Type for the primary key made methods (extension or not) hard to deal with from usage and for library to implement them.

Having the NameIdentifier claim as a property (which is already on the ClaimsIdentity object) would make developers access the userId value without the need of a dependency (like UserManager) in a consistent and reliable way.

Note: This is not about Identity specific, I'm using it as an example.

Historically the Name property on ClaimsIdentity were used many times to have the User Identifier since the id is normally more helpful than the Name, unfortunately one must pick one or the other while both are useful and serve different purposes and also feels like a hack to have a property that doesn't really represent it's intent.

Examples (without this change)

For example, using Identity, to get the the NameIdentifier Claim right you have 2 options with it's respective problems:
1 - Get the claim as it were any other claim
string userId = User.FindFirstValue(ClaimTypes.NameIdentifier);
configuring another claim for the Identifier (like "sub" in the case of OIDC) will break existing code and needs to be updated to User.FindFirstValue("claimType");

2 - Using the UserManager (similar for any other developer/library custom implementation)
string userId = Microsoft.AspNetCore.Identity.UserManager<ApplicationUser>().GetUserId(User);
This won't break if the claimtype changes but requires to have a dependency on the UserManager (or any custom implementation) everywhere you would need to get the userid which shouldn't be needed at all.

Example (with this change)

User.Identity.NameIdentifier

Proposed API

```c#
public ClaimsIdentity(IIdentity identity, IEnumerable claims, string authenticationType, string nameType, string roleType, string nameIdentifierType)
public const string DefaultNameIdentifierClaimType = ClaimTypes.NameIdentifier;
public string NameIdentifierClaimType { get; }
public virtual string NameIdentifier { get; }

# Detail
Adding the `NameIdentifier` and `NameIdentifierClaimType` property to `ClaimsIdentity` solve both problems, you can change the `ClaimIdentifierType` like this:
```c#
services.Configure<IdentityOptions>(options =>
{
    options.ClaimsIdentity.UserIdClaimType = OpenIdConnectConstants.Claims.Subject;
});

and code using ClaimIdentity.NameIdentifier will return the right thing.
No dependency, library, or custom code needed.

Open Questions

Should NameIdentifier be the right name for this property? I'm not aware where the name came from nor the history behind this, while NameIdentifier sounds good may there's opportunity to make it even better.

[karelz]

Can you please submit formal API proposal - see the example there. It simplifies discussion with API reviewers who are not expert in the area. Thanks!

[Bartmax]

@karelz updated with what I think (or at least looks like) a formal API proposal.

I'm not very used to this type of issues so I hope 🤞 it's fine this time.

[kevinchalet]

/cc @brentschmaltz as I believe he still owns the claims stuff.

[karelz]

cc @ianhays

[brentschmaltz]

@Bartmax to ensure I understand, in the same spirit as the mechanics behind: ClaimsIdentity.Name, we would have ClaimsIdentity.NameIdentifier (once we agreed upon a the right name). For OIDC, it could be 'sub'.

ClaimsPrincipal.NameIdentifier would be thorny as a ClaimsPrincipal contains multiple ClaimIdenties. This is the reason you don't see ClaimsPrincipal.Name.

@PinpointTownes thank's for pinging me.

[Bartmax]

@brentschmaltz my apologies, I made a mistake on the examples, I didn't mean User.NameIdentifier but User.Identity.NameIdentifier and ClaimsPrincipal.NameIdentifier should be ClaimsIdentity.NameIdentifier.

So to clarify, the proposal is to add NameIdentifier property in the same way Name property works, but for the Identifier/Sub.

Which it also affects (i think) the IIdentity interface and it should also add the NameIdentifier property.

Should I update my first comment to avoid misunderstandings? and also include the IIdentity changes?
Thanks for your patience :)

[brentschmaltz]

@Bartmax thanks that helps, no need to apologize. You can update your first comment if you want.

It would be a breaking change to add a property to IIdentity since IIdentity is an interface.

So let's consider adding a property to ClaimsIdentity.
I believe 2.0 is locked, so 2.1+ would be the next possible release. I would like to see what others think.

[Richard004]

It is 2018 and I have just bumped into this issue with Microsoft.AspNetCore.Identity Version="2.1.6".

I am refactoring from AspNet.Identity to Oidc and found ClaimsIdentity.RoleClaimType useful to distinguish between 'role' and 'http://schemas.microsoft.com/ws/2008/06/identity/claims/role'

Therefore, it seems to me now useful option to have ClaimsIdentity.NameIdentifierClaimType that would analogically distinguish 'sub' and 'http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier'

I am thinking about a good workaround for now.
But please consider this an up vote. Kind regards.

[ericsampson]

@brentschmaltz would it be possible to consider this along with the JsonWebTokenHandler changes? It would be extremely handy. This whole area has gotten very messy with the increased use of OIDC/JWTs.
ClaimsIdentity.NameIdentifier
and perhaps
ClaimsIdentity.NameIdentifierClaimType

Thanks!

[brentschmaltz]

@ericsampson could you sum up what you would be looking for?

[ericsampson]

@brentschmaltz I'm not an expert in this area, and I'm a little unsure about the back and forth between the OP and you in the first few comments on this issue (e.g. is the proposal in the top message incorporating your feedback)

Maybe I'll try restating what I've observed, and go from there:

In our Startup.cs, we're doing the following to disable the legacy claims mapping and preserve the OIDC claim names:
JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear();

later on we do the following to find the userId from the Http request context:
string userId = User.FindFirst(JwtRegisteredClaimNames.Sub)?.Value

However, this just feels 'wrong'.
It feels like we should be doing something more like (pseudo-code):

// Startup.cs
JwtSecurityTokenHandler.DefaultInboundClaimTypeMap.Clear(); // presumably not necessary with JsonWebTokenHandler 
services.Configure<IdentityOptions>(options =>
{
    options.ClaimsIdentity.UserIdClaimType = JwtRegisteredClaimNames.Sub;
});

// later 
string userId = User.SOMETHINGMOREGENERICHERE()`

[brentschmaltz]

@ericsampson i think i get it.
Code in startup would define the 'claimtype'.
All code in controllers could just use some friendly api.