AspNetIdentity icon indicating copy to clipboard operation
AspNetIdentity copied to clipboard

TKey should be reference type, or compared against `default`

Open Bouke opened this issue 3 years ago • 0 comments

Various places in the SignInManager compare the userId against null. This is a problem for value types, for example Guid. I cannot use Guid? in my TUser as it needs to implement IEquatable<TKey>.

public override Guid ConvertIdFromString(string id) => Guid.Parse(id);

This returns Guid.Empty if id == null. However Guid.Empty != null and various places now assume a valid user. I see two possible fixes for this:

  1. Require TKey to be a reference type: null checks can remain as-is.
  2. Replace null checks with default checks: Guid.Empty == default.

Some places where using a value type is a problem:

https://github.com/aspnet/AspNetIdentity/blob/b7826741279450c58b230ece98bd04b4815beabf/src/Microsoft.AspNet.Identity.Owin/SignInManager.cs#L125 https://github.com/aspnet/AspNetIdentity/blob/b7826741279450c58b230ece98bd04b4815beabf/src/Microsoft.AspNet.Identity.Owin/SignInManager.cs#L170 https://github.com/aspnet/AspNetIdentity/blob/b7826741279450c58b230ece98bd04b4815beabf/src/Microsoft.AspNet.Identity.Owin/SignInManager.cs#L156

Bouke avatar Oct 13 '21 09:10 Bouke