Skip to content

fix: reintegrate PasswordStrength (#4111) - #4291

Merged
svartkanin merged 1 commit into
archlinux:masterfrom
singhh-piyush:fix-unused-password-enum
Mar 18, 2026
Merged

fix: reintegrate PasswordStrength (#4111)#4291
svartkanin merged 1 commit into
archlinux:masterfrom
singhh-piyush:fix-unused-password-enum

Conversation

@singhh-piyush

Copy link
Copy Markdown
Contributor

PR Description:

Removes PasswordStrength enum and associated logic from lib/models/users.py. This is dead code following the refactor in 88b91ae.

Changes:

  • lib/models/users.py: Deleted PasswordStrength class and methods (value, color, strength, _check_password_strength).
  • Imports: Removed Enum, auto, and tr from users.py.
  • lib/models/__init__.py: Removed PasswordStrength from imports and __all__.

Tests and Checks

  • I have tested the code!

Verification:

  • Tests: pytest tests/ passed (13/13).
  • Linting: Passed ruff, mypy, flake8, and pylint.
  • Audit: grep -r "PasswordStrength" archinstall/ returned 0 results.
  • Style: Tab indentation preserved.

@singhh-piyush
singhh-piyush requested a review from Torxed as a code owner March 6, 2026 20:28
@singhh-piyush
singhh-piyush force-pushed the fix-unused-password-enum branch from 5de29ec to 20576ab Compare March 6, 2026 21:30
@singhh-piyush

Copy link
Copy Markdown
Contributor Author

PR Description

Restores the PasswordStrength validation logic that was removed during the commit 88b91ae. Based on feedback, instead of removing the dead code I integrated it into the password prompt flow.

Changes:

  • archinstall/lib/menu/util.py: Imported PasswordStrength and warn (leveraging the existing tr import). Integrated the strength() check immediately after the initial password input. It now emits a localized CLI warning for VERY_WEAK, WEAK, and MODERATE passwords before proceeding to the confirmation prompt.
  • tests/test_password_strength.py: Added a new test suite with 7 cases covering all enum tiers and edge cases (empty strings, digit-only, etc.).

Tests and Checks

  • I have tested the code locally.

Verification:

  • Tests: pytest tests/ passed (20/20, 0 regressions).
  • Linting: Passed ruff, mypy, flake8, and pylint via local pre-commit hooks.
  • Style: Tab indentation enforced and verified.

@singhh-piyush
singhh-piyush force-pushed the fix-unused-password-enum branch 2 times, most recently from 5ce42c4 to bb2a849 Compare March 6, 2026 21:47
@singhh-piyush
singhh-piyush marked this pull request as draft March 6, 2026 22:01
@singhh-piyush
singhh-piyush force-pushed the fix-unused-password-enum branch 3 times, most recently from 374096d to d4f916f Compare March 6, 2026 22:14
@singhh-piyush

singhh-piyush commented Mar 6, 2026

Copy link
Copy Markdown
Contributor Author

Logic and tests for PasswordStrength integrated. I've reverted all locale changes to keep this PR focused on the requested fix .

Technical Note: The translation-check CI is currently failing on master independent of this PR. A duplicate msgid in locales/hi/LC_MESSAGES/base.po (line ~1491) causes locales_generator.sh to abort mid-run, leaving the committed locale files out of sync with the generator's output. I've verified this reproduces on a clean checkout of master with no other changes.

All logic has been verified locally with pytest - 20/20 passed, including 7 new cases covering every PasswordStrength tier and edge cases.

@singhh-piyush
singhh-piyush marked this pull request as ready for review March 6, 2026 22:22
@singhh-piyush
singhh-piyush force-pushed the fix-unused-password-enum branch from d4f916f to c9d01da Compare March 6, 2026 23:25
Comment thread archinstall/lib/menu/util.py Outdated
@singhh-piyush
singhh-piyush force-pushed the fix-unused-password-enum branch from c9d01da to fcaff8b Compare March 7, 2026 14:23
@singhh-piyush

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback! I integrated PasswordStrength into the password prompt as a non-blocking hint. The hint renders beneath the input field and updates on each keystroke with color based on strength tier.

  • VERY_WEAK / WEAK - red
  • Screenshot_20260307_162118
  • MODERATE - yellow
  • Screenshot_20260307_162155
  • STRONG - green
  • Screenshot_20260307_162212

Files changed

  • util.py - password_hint closure, wired to first Input() call only
  • components.py - #input-info label, on_input_changed handler, CSS classes
  • helpers.py - type hint updated to tuple[str, str] | None

Tests

Tested locally in a QEMU VM running the Arch ISO. Screenshots attached.

Password Result
abc Too weak - too short (red)
abcdefg Weak - add uppercase, lowercase, numbers and symbols (red)
Abcdefgh12345 Moderate - increase length (yellow)
Abcdefgh12345!@# Strong (green)

pytest tests/ - 20/20 passed. mypy, ruff, flake8, pylint - clean.

@singhh-piyush
singhh-piyush requested a review from svartkanin March 7, 2026 14:33
@CelestifyX

Copy link
Copy Markdown

My personal opinion is that adding this functionality if it forces you to make a more complex password makes no sense at all, but if it’s just a warning and then it will skip this password, then that’s a different matter.

@singhh-piyush

Copy link
Copy Markdown
Contributor Author

My personal opinion is that adding this functionality if it forces you to make a more complex password makes no sense at all, but if it’s just a warning and then it will skip this password, then that’s a different matter.

Hey, it's a non-blocking warning only. You can still proceed with any password regardless of strength, more of a hint/warning for the user.

i added this to address issue #4111 where PasswordStrength became dead code after get_password() was moved in commit 88b91ae. Rather than deleting the enum, this restores its original intent as a user-facing hint.

@singhh-piyush singhh-piyush changed the title fix: remove unused PasswordStrength enum (#4111) fix: reintegrate PasswordStrength (#4111) Mar 7, 2026
@singhh-piyush

Copy link
Copy Markdown
Contributor Author

hey @svartkanin , you were right, warn() was bypassing the TUI entirely and only hitting the logs. Thanks for the feedback, I really appreciate it

I made changes as suggested. Extended the Input widget with an info_callback. Feedback now renders below the field as the user enters a password, with no interference with the validate attribute.

Could you take a look at the PR when you get a chance?

Solves issue #4111

@svartkanin

Copy link
Copy Markdown
Collaborator

I'll try getting around to test this this week

Comment thread archinstall/lib/menu/util.py Outdated
Comment thread archinstall/tui/ui/components.py Outdated
Comment thread archinstall/tui/ui/components.py Outdated
@singhh-piyush
singhh-piyush force-pushed the fix-unused-password-enum branch 2 times, most recently from 79b6e20 to 122e9f2 Compare March 15, 2026 21:31
@singhh-piyush
singhh-piyush force-pushed the fix-unused-password-enum branch from 122e9f2 to 33b0131 Compare March 15, 2026 21:34
@singhh-piyush singhh-piyush reopened this Mar 15, 2026
@singhh-piyush

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback. Sorry for the messy commit history. I was working on my local archinstall at the same time, testing some BTRFS formatting changes and some of those unrelated changes got added into this branch by accident. The PR is now clean.

All three points from the review addressed:

  • Simplified to Password strength: Weak / Moderate / Strong as suggested.

  • renamed to generic .input-hint-msg-error, .input-hint-msg-warning, .input-hint-msg-info.

  • replaced tuple[str, str] with InputInfo dataclass and InputInfoType enum as suggested.


Regarding VERY_WEAK - it is already defined in the existing PasswordStrength enum in users.py and is returned by the strength() classifier. I used it in password_hint to handle all four possible return values from that function. On the UI side, VERY_WEAK and WEAK are collapsed into the same display string 'Password strength: Weak', so the user only sees three levels

Tested locally in a QEMU VM.
Screenshot_20260315_232005
Screenshot_20260315_232037
Screenshot_20260315_232046

@svartkanin

Copy link
Copy Markdown
Collaborator

Could you rebase master, there was a bigger change merged which unfortunately conflicted with your branch

@singhh-piyush
singhh-piyush force-pushed the fix-unused-password-enum branch from 511f709 to 8e3099e Compare March 16, 2026 19:31
@singhh-piyush

Copy link
Copy Markdown
Contributor Author

Could you rebase master, there was a bigger change merged which unfortunately conflicted with your branch

Done, rebased on top of master and the are conflicts resolved.

@svartkanin
svartkanin merged commit e74a72f into archlinux:master Mar 18, 2026
9 checks passed
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