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

Remove Support for legacy Password Hashers #447

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

0x46616c6b
Copy link
Member

@0x46616c6b 0x46616c6b commented Apr 12, 2023

We had built in a special case to cover your need to migrate old passwords to more modern hash algorithms. This is done and we can remove the special handling for old password hash algorithms.

@codecov
Copy link

codecov bot commented Apr 12, 2023

Codecov Report

Merging #447 (e582148) into main (953be62) will decrease coverage by 0.48%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #447      +/-   ##
============================================
- Coverage     36.01%   35.53%   -0.48%     
+ Complexity     1137     1121      -16     
============================================
  Files           185      183       -2     
  Lines          4584     4547      -37     
============================================
- Hits           1651     1616      -35     
+ Misses         2933     2931       -2     
Impacted Files Coverage Δ
src/Entity/User.php 80.95% <100.00%> (-2.39%) ⬇️
src/EventListener/LoginListener.php 88.00% <100.00%> (-1.66%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@0x46616c6b 0x46616c6b force-pushed the remove-legacy-password-hash-support branch from 5cacbf5 to e582148 Compare April 12, 2023 17:09
@t2d
Copy link
Contributor

t2d commented Apr 12, 2023

Don't you think future migrations to newer password hashing algorithms will eventually become necessary?

@0x46616c6b
Copy link
Member Author

In future I would maybe add a new implementation but for now I would remove it, remove the complexity and reevaluate if it is necessary. We also never did another migration for quite a while now and therefor I prefer the simplicity

@doobry-systemli
Copy link
Contributor

Code changes look good to me, though I wonder what happens if installations still have old passwords in their database 🤔

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.

3 participants