Expose private UpdatePasswordHash with IUserPasswordStore overload to derived classes

[slaneyrw]

Is your feature request related to a problem? Please describe.

Content...

• I have a derived class of UserManager.
• I have an alternative implementation of PasswordHasher that requires an Async call ( Argon2 ).
• I also have additional processes I need to perform between when a password is hashed and the user is updated.

Unfortunately the placed where both of these processes are performed is in the private overload of UpdatePasswordHash that has the IUserPasswordStore parameter.

To resolve my issue I have to replace all 6 methods that hash passwords that, with the exception of UpdatePasswordHash, are identical to the implementation in the UserManager.

• AddPasswordAsync
• ChangePasswordAsync
• CheckPasswordAsync (ReHash flow)
• CreatePasswordAsync
• RemovePasswordAsync

The original hashing method also calls another private method UpdateSecurityStampInternal that I need to also call.

Incidently GenerateNewAuthenticatorKey is virtual, but UpdateSecurityStampInternal directly uses the it's default target of NewSecurityStamp. What's the point of making it virtual if UpdateSecurityStampInternal doesn't use it.

Describe the solution you'd like

Make the private UpdatePasswordHash with IUserPasswordStore overload protected
Make the private UpdateSecurityStampInternal method protected

Additional context

[slaneyrw]

Can we please plan this for v5, at least making IPasswordHasher methods async ( every method above it in the callchain is async, just not the last step)

[SamuelJohnsonMedia]

Would it be better to just make IPasswordHasher methods async?

[AnthonyMastrean]

In order to comply with DISA STIG, we're implementing password history (to prevent recent password reuse, etc.) by extending the various parts of Identity and the UserManager and have this exact same problem with the private update passsword hash and update security stamp methods.

[slaneyrw]

Just checked v6.0 preview... still synchronous methods. Please consider updating the hasher to be Async for the next LTS release.

Everything calling the hashing system is async.. seems a shame to not to complete it end to end.

[halter73]

I have an alternative implementation of PasswordHasher that requires an Async call ( Argon2 ).

From the looks of it, this means that making the IUserPasswordStore overload of UpdatePasswordHash protected would be insufficient without also making IPasswordHasher async as many people in the follow up comments have suggested.

I don't know why Argon2 requires an async call, especially with OWASP's recommended degree of parallelism being one. Even if you need more parallelism, it's not obvious why you couldn't use the current thread as one of the hashing threads. This would reduce the amount of dispatching.

Things like implementing password history to prevent password reuse seem like they should be done by the IPasswordValidator not the IPasswordHasher, and the validator is already async. And if you're in the uncommon situation where you do have such a need, overriding 4 or 5 CRUD methods to manage password doesn't seem too onerous.

The original hashing method also calls another private method UpdateSecurityStampInternal that I need to also call.

Why not just call the already-public UserManager.UpdateSecurityStampAsync(TUser user) method? If you want to really optimize it avoid DB round trips, you could call IUserSecurityStampStore.SetSecurityStampAsync(TUser user, string stamp) instead.

Incidently GenerateNewAuthenticatorKey is virtual, but UpdateSecurityStampInternal directly uses the it's default target of NewSecurityStamp. What's the point of making it virtual if UpdateSecurityStampInternal doesn't use it.

The short answer is that the security stamp is not an authenticator key. It just happens that the default implementation for generating an authenticator key and security stamp is shared.

GenerateNewAuthenticatorKey and GetAuthenticatorKeyAsync are virtual so you can do things like encrypt the key at rest. A security stamp generally doesn't need the same level of secrecy as a key. So long as any new values are unique and unpredictable, it should serve its purpose of invalidating stale credentials.