Repository navigation
fix(auth): store passwords with bcrypt and migrate legacy MD5 digests #1432 - #1445
Open
borealisalastor-byte wants to merge 9 commits into
Open
borealisalastor-byte wants to merge 9 commits into
borealisalastor-byte wants to merge 9 commits into
Conversation
…se#1432 Storage moves to bcrypt in the shared helpers; legacy unsalted MD5 digests stay verifiable only for rows created before the migration and are rewritten on the next successful login.
authenticate() verifies through the shared helper and rewrites a verified legacy digest with bcrypt through the request-scoped session.
The login flow verifies through the shared helper and the change-pwd endpoint stores a bcrypt hash.
The model default factory now returns a bcrypt hash of the configured default password.
Resetting a user writes the bcrypt default hash and the self-service update stores bcrypt instead of MD5.
The guard keeps its assertions but stubs get_password_hash and verify_password instead of the retired MD5 helpers.
The MCP integration fixture creates its user through get_password_hash instead of md5pwd.
Covers bcrypt storage, legacy digest compatibility, the login upgrade path and the repository-wide ban on MD5 password writes.
The security guide states that only bcrypt stores passwords and what the legacy MD5 helpers are still allowed to do.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cause
sys_user.passwordwas stored as an unsalted MD5 digest (common.core.security.md5pwd). As reported in #1432, one dump of the table allows offline cracking of every account, and equal passwords are equal in the table. The digest was also used at every write site: user creation defaults, admin reset, self-service change and the login flow.Fix
get_password_hash) everywhere: the model default, admin reset, login password change, profile password change;verify_passwordkeeps accepting a legacy unsalted MD5 digest, so an existing deployment can still log in, andauthenticate()rewrites a verified digest with bcrypt through the request-scoped session, so each row upgrades itself on the next successful login;md5pwd/verify_md5pwd/default_md5_pwdremain only as legacy helpers used by that compatibility path and are documented as such; no production module writes an MD5 password any more.Tests
backend/tests/test_password_hash_migration.py: bcrypt storage (never a 32-hex digest), verification of bcrypt and legacy digests, the login upgrade path (rewrite once on success, never on a failed or non-legacy login) and a repository scan that fails if any module underbackend/appsorbackend/commonhashes a password with the legacy helpers again;backend/tests/test_initial_password_policy.pyandbackend/tests/test_mcp_parameters_integration.pyfollow the new helpers instead of the retired MD5 ones.Verification
python -m pytest -q tests/test_password_hash_migration.py tests/test_initial_password_policy.py-> 28 passed, run inbackend/on a checkout ofmainat a0b6145 with passlib 1.7.4 and bcrypt 4.0.1;default_pwd_hash/is_legacy_md5_pwd, and the oldauthenticatecomparedverify_md5pwd; the old storage helpers are gone from every call site after this change;uv run pytest -qsuite was not run here (the environment has no access to the private xpack index), so untouched suites are not covered by this PR;api/login.py(ruff W292 on the current file), which the repository'send-of-file-fixerhook applies as well.Fixes #1432