Skip to content

fix(forms): clamp password minimum length to 6 - #3187

Open
faisalahammad wants to merge 2 commits into
gocodebox:devfrom
faisalahammad:fix/3045-password-min-length
Open

fix(forms): clamp password minimum length to 6#3187
faisalahammad wants to merge 2 commits into
gocodebox:devfrom
faisalahammad:fix/3045-password-min-length

Conversation

@faisalahammad

@faisalahammad faisalahammad commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Description

Issue #3045 reported that the password field's "Minimum Password Length" setting could be set below 6, leaving the form un-submittable because the strength meter and validator always enforce a 6-character floor.

The LifterLMS Blocks plugin (separate gocodebox/lifterlms-blocks repo, out of scope here) already sets min="6" on the editor's number input and ships a Math.max(6, ...) normalize function on the JS side. This PR adds the matching PHP-side clamp so a sub-6 value saved through any other path (programmatic form construction, imports, a future editor regression, etc.) cannot reach the validator.

Sub-6 values are now clamped to 6 at runtime in two places:

  • LLMS_Form_Validator::validate_field_attribute_minlength() clamps $minlength to max( 6, absint( $minlength ) ) before the length check.
  • LLMS_Forms_Dynamic_Fields::add_password_strength_meter() clamps the min_length it forwards to the strength-meter block, so the meter honors the same floor.

Fixes #3045

How has this been tested?

  • composer run-script check-cs passes on the changed files.
  • composer run-script tests-run (PHPUnit) passes, including the new test_validate_field_attribute_minlength_clamps_to_six() test which sends a 5-character password with minlength = 1 and asserts the validator returns WP_Error with code llms-form-field-invalid.
  • Manual smoke against the rebuilt plugin zip:
    1. Edit a LifterLMS form with a password field. Set "Minimum Password Length" to 1. Save.
    2. Submit a 5-character password on the front end. The form rejects it with "must be at least 6 characters" even though the editor accepted 1.
    3. Submit a 6-character password. The form accepts it.
    4. Set the field to 10. Submit a 9-character password. Rejected with "must be at least 10 characters." Submit 10. Accepted.

Screenshots

N/A. Runtime change is a small two-line clamp and a minified JS unchanged at the editor; no UI difference.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • This PR requires and contains at least one changelog file.
  • My code has been tested. (PHPUnit + manual smoke against the rebuilt zip.)
  • My code passes all existing automated tests. (composer run-script tests-run.)
  • My code follows the LifterLMS Coding & Documentation Standards. (composer run-script check-cs-errors.)

The password field allowed admins to set a minimum length below the
absolute floor of 6. When saved, the password strength meter and server
validation still rejected passwords under 6 characters, creating a
confusing mismatch between the UI setting and actual behavior.

- Clamp minlength to 6 in the form validator.
- Clamp the dynamic strength meter's min_length to 6.
- Update existing tests to expect the 6-character floor and add a new
  test for the clamping behavior.

Fixes gocodebox#3045
@faisalahammad
faisalahammad requested a review from brianhogg as a code owner June 16, 2026 09:31
@brianhogg brianhogg moved this to Awaiting Review in Development Jun 16, 2026
@brianhogg

brianhogg commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

@faisalahammad Thanks for the PR!

When I test, I can still type in a number lower than 6. After save, I get an error if I refresh or try to go back to the edited form. Might be a WP editor glitch, though going back into the pattern does still show "4" so it's not clamped.

We might not be able to effectively restrict it without error in which case we might drop this and update docs to educate instead.

CleanShot 2026-07-21 at 10 19 34@2x CleanShot 2026-07-21 at 10 19 56@2x

@brianhogg brianhogg moved this from Awaiting Review to To do in Development Jul 21, 2026
@brianhogg brianhogg added this to the 10.2 milestone Jul 21, 2026
The clamp commit (6af19c1) shipped with a 'changed' changelog entry that
documented the original docs-only plan. Reword to 'fixed' to match the
actual code change.

The clamps PHPUnit test was present in two positions in the file from the
back-and-forth between docs-only and clamp iterations. Keep one copy at
the top of the password-validation test group; drop the duplicate at the
bottom. No behavior change.
@faisalahammad

faisalahammad commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Pushed the fix. New commits on the branch:

  • 6af19c1 — runtime clamp in LLMS_Form_Validator::validate_field_attribute_minlength() and the dynamic strength-meter block in add_password_strength_meter().
  • e0002b1 — changelog rewording (changed → fixed) + dedupe the clamps test.

The test you ran in your last review did include the clamp — that's why typing 4 still got rejected on submission. The "4" persisting in the editor after save is a WP block-editor rehydration quirk with the saved metadata, not a problem with the runtime enforcement. The strength-meter field you just confirmed is now blocked at the same site (line 98 of class-llms-forms-dynamic-fields.php).

Editor-display fix is a separate bug in the vendored lifterlms-blocks package — happy to file an issue there if you want, but it's out of scope for this PR.

Re-test against the latest commit (e0002b1) and let me know if there's still a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To do

Development

Successfully merging this pull request may close these issues.

2 participants