Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proof of concept members password hasher #10017

Conversation

scottbrady91
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Description

Password hasher for members that supports the legacy hashing algorithm and the ASP.NET Identity hashing algorithm. The hashing algorithm is determined by hash format (base64 or not) and the ASP.NET Identity marker byte.

Since LegacyPasswordSecurity didn't seem 100% testable I wrote some integration tests using known password hashes.

Let me know if there are any other requirements that I am not aware of.

@scottbrady91
Copy link
Contributor Author

@bergmania @Shazwazza

@bergmania
Copy link
Member

Looks good @scottbrady91.

Not sure where, but I think we should update the users' algorithm to the new format and a new hash, if we see it is the legacy? What do you think @Shazwazza.

@Shazwazza
Copy link
Contributor

Not sure where, but I think we should update the users' algorithm to the new format and a new hash, if we see it is the legacy? What do you think @Shazwazza.

We already do this and is handled by BackOfficePasswordHasher. It is handled differently for users because we store the current hashing algorithm with them. Ideally we do this for members too but that requires DB changes, etc... for members (which we want to do for other reasons anyways)

@scottbrady91
Copy link
Contributor Author

By returning SuccessRehashNeeded, the UserManager will automatically update the user's password hash: https://github.com/dotnet/aspnetcore/blob/main/src/Identity/Extensions.Core/src/UserManager.cs#L710

@Shazwazza
Copy link
Contributor

By returning SuccessRehashNeeded, the UserManager will automatically update the user's password hash:

ah yep, that's also how the user stuff works too. I'll get this merged and updated. Thanks!

@Shazwazza Shazwazza changed the base branch from netcore/dev to netcore/task/password-roll-forward-10369 April 16, 2021 00:04
@Shazwazza
Copy link
Contributor

Merged in commit 8b8bf47
Not sure why that isn't reflecting here.

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants