-
Notifications
You must be signed in to change notification settings - Fork 289
security: switch password storage to password_hash() with legacy MD5 migration #685
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
base: master
Are you sure you want to change the base?
security: switch password storage to password_hash() with legacy MD5 migration #685
Conversation
663029d to
ff8ebf2
Compare
b128707 to
f661475
Compare
f661475 to
d2d7bfd
Compare
|
Marking this PR as draft because the test pipeline is not working yet. |
d2d7bfd to
11eb588
Compare
|
CI/CD Pipeline is currently failing because this PR also reactivates the First-Login-Wizard if default credentials are set. However, test/features/login.feature currently assumes that the dashboard will load after login. @RussH seems like there is a bigger refactor needed. Alternatively we can initially just deactivate the restored First-Login-Wizard and revisit this later if you are okay with this. |
|
Since my plan is to rebuild the installer later (including a proper user setup flow instead of creating an admin account with predefined credentials) once a few prerequisite PRs have been merged, the most pragmatic and targeted short-term approach for this PR was to disable the First-Login Wizard for now by commenting it out. This keeps the test suite deterministic again and avoids spending time on a partial fix in an area that is going to be refactored anyway. |
|
yes, happy to disable it for now |
Summary
This pull request replaces the legacy MD5-based password handling with PHP's
password_hash()andpassword_verify()functions using PASSWORD_DEFAULT and ensures that newly set or changed passwords are always stored using the modern hashing API.For existing installations, legacy MD5 hashes are still accepted at login but, on successful authentication, the plaintext password is rehashed using PASSWORD_DEFAULT and the stored hash is updated. For modern hashes,
password_needs_rehash()is used after a successful login to transparently upgrade hashes if PASSWORD_DEFAULT changes in the future.For new installations, the base schema now defines
user.passwordasVARCHAR(255), the installer’s schema upgrade path adds anALTER TABLEstep so existing databases are migrated to the same definition and the Security.MD documentation has been updated to reflect that OpenCATS now usespassword_hash()andpassword_verify()with PASSWORD_DEFAULT for password storage.Motivation
MD5 is no longer considered appropriate for storing user passwords, and modern PHP provides a well-established and actively maintained API for password hashing via
password_hash()andpassword_verify(), with PASSWORD_DEFAULT acting as a safe, forward-compatible algorithm selector that can adopt stronger algorithms over time without requiring further application changes.Integrating
password_needs_rehash()independently of the MD5 compatibility logic ensures that future changes to PASSWORD_DEFAULT or its options can be adopted at runtime without another invasive refactor and increasing the password column toVARCHAR(255)while updating Security.MD keeps schema and documentation aligned with current best practices and avoids further schema changes when PASSWORD_DEFAULT eventually moves to a stronger or longer hash format.