UserManager now blocks null security stamps

[HaoK]

Fixes #1016

cc @divega @blowdart

[HaoK]

Note we usually take care of modifying the security stamp, but this guards against some edge cases where they mucked with the entity directly, or end up with a null security stamp via other means outside of our APIs

[kevinchalet]

So, in the "rare" case where a security stamp would be missing, the user wouldn't be able to update his profile, right?

[HaoK]

Yes, basically its a developer error to end up with a user with a security stamp missing, you won't be able to save a user with no security stamp anymore.

[kevinchalet]

its a developer error to end up with a user with a security stamp missing

My point is that it shouldn't be returned to the end user, as there's nothing he can do to fix it.
Is there a particular reason you prefer returning an error instead of generating a new security stamp on the fly?

[HaoK]

Ah yes, your point about this being a pretty useless message to the user is valid. I guess this behavior also shouldn't be optional based on the user validator, this check should be explicit in the user manager and throw an exception instead.

Its not really safe to automatically generate a new security stamp, that would just result in really hard to repro cases of users that are suddenly signed out.

[HaoK]

@divega @blowdart do you guys agree that a missing security stamp on a user when the feature is enabled should result in a runtime exception instead of a 'recoverable' identity error.

[kevinchalet]

Its not really safe to automatically generate a new security stamp, that would just result in really hard to repro cases of users that are suddenly signed out.

Well, if a user has a null security stamp at some point, he will be unable to log in at all, as it will crash when creating the ClaimsPrincipal

[blowdart]

Sure, throw. It's unexpected enough to be an exceptional event.

[HaoK]

Updated moving null stamp check to unoverridable user manager code. @divega look reasonable?

[kevinchalet]

No love for UserStore.SetSecurityStampAsync? If null stamps are illegal, maybe it should throw an ArgumentException when it's null or empty.

https://github.com/aspnet/Identity/blob/dev/src/Microsoft.AspNetCore.Identity.EntityFrameworkCore/UserStore.cs#L1242

[HaoK]

Sure that's valid defense in depth, but its very hard to call that today using the identity APIs as its only called in the UpdateSecurityStamp internal method which never passes in null, but it doesn't hurt to add

[HaoK]

4dd38e8