Repository navigation
Conversation
assert_password_is_secure has been an empty stub since 8edd271 removed Data::Password::passwdqc, so no password strength check has run since. Restore the pre-passwdqc logic from e9adcde: a minimum length check plus the optional character class rule keyed off the existing password_complexity param. Add a password_check hook so a stricter policy can be supplied by an extension rather than by a core dependency. Remove the six passwdqc_* params, their checkers and their help text, along with t/903-passwdqc-conf.t, since nothing reads them. Update two test fixture passwords that did not satisfy the bmo complexity rule, which is enabled in the CI answers file and had been unenforced while the stub was empty.
|
Verified the full webservice suite against this PR. With both commits cherry-picked onto current main (04d962a), run-tests-in-docker.sh webservices passes: 25 files, 7828 tests, 0 failures. On main without this PR the same run fails with 198 failures in webservice_bug_update.t (the secure_mail crash) and 6 in webservice_user_create.t (the empty password check). Once this lands, the webservice job in ci.yml can be re-enabled. I can do that here or as a follow-up, whichever you prefer. |
mrenvoize
left a comment
mrenvoize
left a comment
There was a problem hiding this comment.
Requesting changes, mainly to split this PR. The first commit (guarding the secure_mail call at Bugzilla/Bug.pm:1112) is correct, and I found no encrypted-mail regression: core has no other secure_mail call sites and the SecureMail mailer is untouched. I'd approve that on its own.
The second commit is not described and does something different: it reinstates a native password policy that Harmony deliberately removed in 8edd271 (#80).
- It enforces a minimum length of 8 on all installs (
Bugzilla.pm:245, called fromUser.pm:349,token.cgi:125/419,userprefs.cgi:102,reset_password.cgi:69). - It deletes the
passwdqc_*params andt/903with no replacement test. - The
password_checkhook (Bugzilla.pm:259-260) can clear a core rejection.
Please move the password policy to its own PR with a description, tests and an upgrade note, and keep this PR to the guard.
Bugzilla/Bug.pm calls $group->secure_mail unconditionally when a bug is
made public. That method only exists because SecureMail's Extension.pm
installs it into Bugzilla::Group at load time, and SecureMail ships
disabled, so any install without it dies with:
Can't locate object method "secure_mail" via package "Bugzilla::Group"
This surfaced in the webservice QA suite. The "groups: remove Master"
test died there, leaving the test bug group-restricted, which cascaded
into 198 failures across webservice_bug_update.t. Guarding the call
takes that file from 198 failures to zero (921 passing).
Six failures remain in webservice_user_create.t from an unrelated cause
(assert_password_is_secure is an empty stub), so the webservice job in
ci.yml stays commented out for now.