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

[4.0] Remove missing phpass in favour of Laravel support hash #577

Open
wants to merge 1 commit into
base: 4.0
Choose a base branch
from

Conversation

TheoKouzelis
Copy link

Phpass has been deleted. But upon investigating the package, it seems it's main purpose is to support strong hasing in php versions less than 5.5 https://www.openwall.com/phpass/. Since this package requires php 7.2. I don't think the dependency is required.

Phpass has been deleted. But upon investigating the package, it seems
it's main purpose is to support strong hasing in php versions less than
5.5 https://www.openwall.com/phpass/. Since this package requires
php 7.2. I don't think the dependency is required.
@TheoKouzelis
Copy link
Author

Thinking about it, this will probably invalidate existing hashes stored in databases.

@bumbummen99
Copy link
Contributor

bumbummen99 commented Sep 14, 2021

"phpass was released in 2005 when a typical web host ran PHP 4 and a typical web app used raw MD5. In 2007 and on major web apps moved to phpass, which was an important step forward (bringing web apps' password hashing on par with Unix systems'). phpass API might also have inspired the password_hash() / password_verify() API included in PHP 5.5+. "

I can not make any argument defending to keep phpass. Should be removed immediately. Versions that have native functions are already EOL and 7 will be EOL soon too:
https://www.php.net/supported-versions.php

@jgrossi
Copy link
Member

jgrossi commented Sep 16, 2021

@bumbummen99 completely agree, but invalidating all hashes stored in the databases would be a big impact for now.

gonna keep all PRs from @TheoKouzelis opened and maybe adding a breaking change or something when merging it. for now I've changed to bordoni/phpass.

I've tagged it under v5.1.0 along with removing travis-ci in favor of github actions.

let's keep these PRs and merge soon 👍

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.

None yet

3 participants