-
-
Notifications
You must be signed in to change notification settings - Fork 67
fix(login): show proper error for invalid password reset links #314
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: main
Are you sure you want to change the base?
Conversation
When using the Ultimate Multisite login element for custom login pages, invalid or expired password reset links now display proper error messages instead of silently redirecting. Changes: - Add WP_Error check after check_password_reset_key() in Login_Form_Element - Redirect to lost password page with appropriate error code when key is invalid - Clear the invalid reset cookie before redirecting - Fix hash_equals bug that compared $_POST['rp_key'] to itself - Add 'invalid_key' and 'expired_key' error codes to match WordPress core 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Adds a password strength meter that enforces minimum medium strength when resetting passwords via the custom login page. Changes: - Add wu-password-reset.js for client-side strength checking - Dynamically create strength meter element below password field - Disable submit button until password meets minimum strength - Add server-side password strength validation - Remove password hint text (replaced by strength meter) - Add field-password.php template with meter support Strength levels: - Very weak/Weak (red): Not allowed - Medium (yellow): Minimum required - Strong (green): Full strength 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Extract password strength checking logic into a reusable utility to eliminate duplication between checkout and password reset forms. Changes: - Create shared wu-password-strength.js utility for password validation - Update wu-password-reset.js to use shared utility (reduced from 122 to 46 lines) - Remove hardcoded password strength strings in favor of WordPress's pwsL10n - Remove custom localization in class-login-form-element.php - Update script dependencies to include the shared utility This refactoring ensures consistency across forms and leverages WordPress's built-in localized password strength strings instead of duplicating them.
📝 WalkthroughWalkthroughAdds client-side password strength meter and visibility toggle, integrates them into password fields and reset pages, and enforces a minimum password strength server-side during password reset; also registers/enqueues related scripts and updates templates to include meter and toggle elements. Changes
Sequence DiagramsequenceDiagram
autonumber
participant User
participant Browser
participant WU_PS as WU_PasswordStrength
participant WP_Core as WP.passwordStrength
participant Backend
User->>Browser: type password (pass1 / pass2)
Browser->>WU_PS: input events
WU_PS->>WP_Core: wp.passwordStrength.meter(pass1, disallowedList, pass2)
WP_Core-->>WU_PS: strength score
WU_PS->>Browser: update UI (label, classes) and validity state
alt client strength >= min
Browser->>User: enable submit
User->>Browser: submit form
Browser->>Backend: POST password + token
Backend->>Backend: calculate_password_strength(password)
alt server strength >= min
Backend-->>Browser: success (password updated)
else
Backend-->>Browser: error (password_too_weak)
end
else client strength < min
Browser->>User: block/disable submit
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
views/admin-pages/fields/field-password.php (1)
30-36: Consider whether outputting password value is necessary.The template outputs
value="<?php echo esc_attr($field->value); ?>"for the password input. While properly escaped, echoing a password value into HTML is typically avoided since password fields are usually left empty on page load. If this is intentional (e.g., for specific form flows), this is fine; otherwise, consider removing the value attribute.inc/checkout/class-checkout-pages.php (1)
266-282: Address unused$userparameter (static analysis hint).The
$userparameter is required by thevalidate_password_resethook signature but isn't used. Add a@SuppressWarningsannotation or prefix with underscore to indicate it's intentionally unused.🔎 Proposed fix
- public function validate_password_strength($errors, $user): void { + public function validate_password_strength($errors, $_user): void {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
assets/js/wu-password-reset.jsassets/js/wu-password-strength.jsinc/checkout/class-checkout-pages.phpinc/ui/class-login-form-element.phpviews/admin-pages/fields/field-password.php
🧰 Additional context used
🧬 Code graph analysis (4)
inc/checkout/class-checkout-pages.php (1)
inc/functions/helper.php (1)
wu_request(132-137)
views/admin-pages/fields/field-password.php (2)
inc/ui/class-field.php (1)
print_wrapper_html_attributes(477-484)inc/functions/template.php (1)
wu_get_template(20-73)
inc/ui/class-login-form-element.php (1)
inc/functions/helper.php (1)
wu_get_version(21-24)
assets/js/wu-password-strength.js (1)
assets/js/checkout.js (2)
pass1(764-764)strength(780-780)
🪛 GitHub Check: Code Quality Checks
inc/checkout/class-checkout-pages.php
[warning] 266-266:
The method parameter $user is never used
🪛 PHPMD (2.15.0)
inc/checkout/class-checkout-pages.php
266-266: Avoid unused parameters such as '$user'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (11)
assets/js/wu-password-reset.js (1)
1-45: LGTM!Clean implementation that properly guards against missing dependencies, uses the shared utility correctly, and prevents form submission when the password doesn't meet strength requirements. The IIFE pattern with strict mode is a good practice.
views/admin-pages/fields/field-password.php (1)
1-64: LGTM!The template is well-structured with proper escaping, conditional strength meter rendering, and follows existing template patterns. The use of partials for title and description promotes consistency.
inc/checkout/class-checkout-pages.php (2)
209-211: Good addition of error codes.Adding
invalid_key,expired_key, andpassword_too_weakerror codes properly extends the error message system. The underscore variants match the codes used in the Login_Form_Element redirect.
65-66: LGTM!Priority 5 ensures password strength validation runs before the error handler at default priority, which is the correct ordering.
assets/js/wu-password-strength.js (3)
113-122: Good backward compatibility handling.Supporting both
userInputDisallowedList(new) anduserInputBlacklist(deprecated) ensures compatibility across WordPress versions. This is a well-implemented pattern.
99-106: Correct usage of password strength meter.This correctly passes
pass2as the third argument towp.passwordStrength.meter()for mismatch detection. Note: The existingcheckout.js(line 779) has a bug where it passespass1twice instead ofpass1andpass2- consider fixing that separately.
1-241: Well-designed reusable utility.Clean implementation with proper encapsulation, sensible defaults, backward compatibility, and good separation of concerns between strength checking, UI updates, and validity tracking. The callback pattern for validity changes enables flexible integration.
inc/ui/class-login-form-element.php (4)
304-326: LGTM!Script enqueuing is properly conditional on
is_reset_password_page(), includes correct dependencies, and useswu_get_version()for cache busting. The dependency chain (wu-password-resetdepends onwu-password-strengthwhich depends onpassword-strength-meter) is correctly specified.
631-658: Good error handling for invalid/expired reset keys.This properly checks for
WP_Errorfromcheck_password_reset_key(), extracts the error code, clears the invalid cookie, and redirects to the lost password page with the appropriate error. This fixes the silent redirect issue mentioned in the PR objectives.
660-662: Security fix: Correct hash_equals comparison.This fixes the bug where
$_POST['rp_key']was compared to itself. Now it correctly compares the cookie-derived$rp_keywith the POST-submitted value using timing-safehash_equals().
669-716: LGTM!The field configuration is well done:
meter: trueon pass1 enables the strength indicatorautocomplete: new-passwordfollows HTML spec for password creation fields- Field structure aligns with the new password template and JS utility expectations
| protected function calculate_password_strength(string $password): int { | ||
|
|
||
| $strength = 0; | ||
| $length = strlen($password); | ||
|
|
||
| // Minimum length check | ||
| if ($length < 6) { | ||
| return 0; | ||
| } | ||
|
|
||
| // Length scoring | ||
| if ($length >= 8) { | ||
| ++$strength; | ||
| } | ||
|
|
||
| if ($length >= 12) { | ||
| ++$strength; | ||
| } | ||
|
|
||
| // Character variety scoring | ||
| if (preg_match('/[a-z]/', $password)) { | ||
| ++$strength; | ||
| } | ||
|
|
||
| if (preg_match('/[A-Z]/', $password)) { | ||
| ++$strength; | ||
| } | ||
|
|
||
| if (preg_match('/[0-9]/', $password)) { | ||
| ++$strength; | ||
| } | ||
|
|
||
| if (preg_match('/[^a-zA-Z0-9]/', $password)) { | ||
| ++$strength; | ||
| } | ||
|
|
||
| // Map to WordPress strength levels (0-4) | ||
| if ($strength <= 1) { | ||
| return 1; // Very weak | ||
| } | ||
|
|
||
| if ($strength <= 2) { | ||
| return 2; // Weak | ||
| } | ||
|
|
||
| if ($strength <= 4) { | ||
| return 3; // Medium | ||
| } | ||
|
|
||
| return 4; // Strong | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
zxcvbn PHP library port password strength
💡 Result:
Yes — zxcvbn has several PHP ports you can use. Popular options and quick usage:
- michaelokgb / zxcvbn-php — Composer package (michaelokgb/zxcvbn-php). Install with composer and call Zxcvbn::passwordStrength(...). [1]
- fernleafsystems / zxcvbn-php — another actively published Packagist port with same API shape. [2]
- mkopinsky / zxcvbn-php — a commonly used fork/packagist distribution (older updates tracked). [3]
- Integrations: Symfony validator wrapper (Locastic/ZxcvbnPasswordValidator) if you need framework validation. [4]
- Upstream resources and port list: zxcvbn project docs list zxcvbn-php ports and related ports in other languages. [5]
Example (composer + usage):
- composer require michaelokgb/zxcvbn-php. [1]
- PHP:
- use ZxcvbnPhp\Zxcbn;
- $z = new Zxcbn(); $result = $z->passwordStrength('hunter2', ['username@example.com']); $score = $result['score']; [1]
Notes:
- Most PHP ports aim to be compatible with Dropbox’s JS zxcvbn algorithms, but check the package README/version for exact compatibility and PHP version support before use. [5][3]
Sources:
[1] michaelokgb/zxcvbn-php (Packagist/GitHub).
[2] fernleafsystems/zxcvbn-php (Packagist).
[3] mkopinsky/zxcvbn-php (Packagist/README).
[4] Locastic/ZxcvbnPasswordValidator (GitHub).
[5] zxcvbn docs (ports list).
🏁 Script executed:
# Search for where calculate_password_strength is used in the codebase
rg "calculate_password_strength" -B 2 -A 2
# Search for password validation and strength checks
rg "password.*strength|strength.*password" -i --type php
# Look for wp.passwordStrength references
rg "passwordStrength|password_strength" -B 2 -A 2Repository: Multisite-Ultimate/ultimate-multisite
Length of output: 27478
🏁 Script executed:
# Search for where the backend enforces minimum password strength
rg "return.*strength|strength.*>|strength.*<" -B 3 -A 1 --type php
# Look for password validation in checkout process
rg "validate.*password|check.*password" -i --type phpRepository: Multisite-Ultimate/ultimate-multisite
Length of output: 8374
Server-side strength calculation diverges from frontend zxcvbn algorithm, risking UX issues.
The frontend uses WordPress's wp.passwordStrength.meter() (zxcvbn-based), while the backend implements simple regex heuristics. The same password can receive different strength scores—a user may see a passing score on the client but receive a rejection server-side, creating user confusion.
Use a PHP port of zxcvbn for backend validation (e.g., michaelokgb/zxcvbn-php or fernleafsystems/zxcvbn-php on Packagist) to ensure consistent password strength evaluation across client and server.
🤖 Prompt for AI Agents
In inc/checkout/class-checkout-pages.php around lines 292 to 342, the
server-side password strength uses simple regex heuristics that differ from the
frontend zxcvbn-based meter; replace this custom logic with a PHP zxcvbn library
to guarantee consistent scores. Install and require a zxcvbn PHP package (e.g.,
composer require michaelokgb/zxcvbn-php or fernleafsystems/zxcvbn-php), update
calculate_password_strength to call the zxcvbn analyzer to get the
entropy/score, then map the zxcvbn score (0-4) directly to the WordPress
strength levels used by the client and return that value; include a minimal
fallback that logs a warning and returns a conservative low score if the
dependency is missing.
Add eye icon to show/hide password on all password fields: - Checkout signup form (password + password confirmation) - Login form - Password reset page (new password + confirm password) Uses WordPress core approach with dashicons (visibility/hidden icons) and accessible aria-label attributes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
assets/js/wu-password-toggle.jsinc/checkout/class-checkout.phpinc/class-scripts.phpinc/ui/class-login-form-element.phpviews/admin-pages/fields/field-password.phpviews/checkout/fields/field-password.php
🚧 Files skipped from review as they are similar to previous changes (1)
- views/admin-pages/fields/field-password.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/ui/class-login-form-element.php (1)
inc/functions/helper.php (1)
wu_get_version(21-24)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
🔇 Additional comments (7)
inc/class-scripts.php (1)
153-157: LGTM!The password toggle script registration is properly implemented with the correct dependency on
wp-i18nfor internationalization support.inc/checkout/class-checkout.php (1)
2567-2572: LGTM!The script and style enqueuing is properly implemented to support the password toggle functionality on checkout pages. The addition of dashicons and the wu-password-toggle script dependency, along with proper translation setup, ensures the feature works correctly.
views/checkout/fields/field-password.php (1)
26-39: LGTM!The password field UI enhancement is well implemented with proper accessibility attributes, progressive enhancement (hide-if-no-js), and appropriate styling to prevent text overlap with the toggle button.
assets/js/wu-password-toggle.js (1)
11-68: LGTM!The password toggle implementation follows JavaScript best practices with proper use of IIFE pattern, internationalization via wp.i18n, accessibility updates (aria-label), and safe guards for missing elements. The toggle functionality is clean and well-structured.
inc/ui/class-login-form-element.php (3)
305-340: LGTM!The script enqueuing is properly implemented with conditional loading of password strength scripts only on the reset password page, and correct dependency chains. Translation setup for the password toggle ensures internationalized UI strings.
645-672: Excellent improvement addressing the PR objective.This change properly handles invalid or expired password reset links by checking for WP_Error from
check_password_reset_key(), redirecting to the lost password page with the appropriate error code, and clearing the invalid cookie. This replaces the previous silent failure behavior with proper user feedback.
684-730: LGTM!The password field updates are well-implemented:
meter: trueenables the password strength indicator for better UXautocomplete: 'new-password'follows modern best practices and helps password managers correctly identify these as new password fields
| exit; | ||
| } | ||
|
|
||
| if (isset($_POST['pass1']) && isset($_POST['rp_key']) && ! hash_equals($rp_key, wp_unslash($_POST['rp_key']))) { // phpcs:ignore WordPress.Security.NonceVerification |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical security fix verified.
This change corrects a critical bug where hash_equals() was comparing $_POST['rp_key'] to itself (always returning true), which would bypass the password reset key validation. The fix properly compares the key from the cookie ($rp_key) with the submitted form value, as intended. This directly addresses the issue mentioned in the PR description.
Note: While this is now fixed, this was a serious security vulnerability that would have allowed password reset bypasses.
🤖 Prompt for AI Agents
In inc/ui/class-login-form-element.php around line 674, the hash_equals call was
comparing $_POST['rp_key'] to itself (always true) which bypassed reset-key
validation; change the comparison so hash_equals($rp_key,
wp_unslash($_POST['rp_key'])) is used (keep the existing isset checks and
phpcs:ignore comment), ensuring the cookie-derived $rp_key is compared to the
submitted, unslashed value for a proper timing-safe validation.
- Use event delegation for click handling to support Vue-rendered fields - Fix button positioning with inline styles (wu-right-2 class not available) - Position toggle button at right edge of password input - Add padding-right to input to prevent text overlap with button 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
assets/js/wu-password-toggle.jsviews/admin-pages/fields/field-password.phpviews/checkout/fields/field-password.php
🚧 Files skipped from review as they are similar to previous changes (2)
- views/admin-pages/fields/field-password.php
- views/checkout/fields/field-password.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: PHP 8.5
- GitHub Check: Build Plugin for Testing
- GitHub Check: PHP 8.3
- GitHub Check: PHP 7.4
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
🔇 Additional comments (3)
assets/js/wu-password-toggle.js (3)
38-52: LGTM! Well-implemented toggle logic.The password visibility toggle correctly:
- Synchronizes all state changes (data attribute, ARIA label, input type, icon classes)
- Uses appropriate accessibility attributes (
aria-label)- Follows WordPress conventions with dashicons
55-56: LGTM! Appropriate use of event delegation.Document-level event delegation is the correct approach for handling dynamically added password fields (e.g., via Vue or other dynamic rendering), and the
closest()check in the handler ensures clicks are properly scoped to toggle buttons.
1-57: PR objectives don't mention password visibility toggle functionality.The PR title and objectives focus exclusively on "fix(login): show proper error for invalid password reset links" and mention backend error handling (WP_Error checks, hash_equals bug, redirects). However, this file implements client-side password visibility toggle functionality, which isn't mentioned in the PR objectives at all.
While the AI summary mentions "password visibility toggle," the PR description should explain why password UX improvements (visibility toggle, strength meter mentioned in AI summary) are included alongside password reset error fixes. This helps reviewers understand the scope and rationale for combining these changes.
| (function() { | ||
| 'use strict'; | ||
|
|
||
| var __ = wp.i18n.__; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add defensive check for wp.i18n availability.
While the script declares wp-i18n as a dependency, directly accessing wp.i18n.__ without an existence check can cause a runtime error if the dependency fails to load due to network issues, script errors, or load-order problems.
🔎 Proposed fix with defensive fallback
- var __ = wp.i18n.__;
+ var __ = (wp && wp.i18n && wp.i18n.__) ? wp.i18n.__ : function(text) { return text; };This provides a graceful fallback that returns untranslated strings rather than breaking the entire toggle functionality.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var __ = wp.i18n.__; | |
| var __ = (wp && wp.i18n && wp.i18n.__) ? wp.i18n.__ : function(text) { return text; }; |
🤖 Prompt for AI Agents
In assets/js/wu-password-toggle.js around line 14, the code directly uses
wp.i18n.__ which can throw if wp or wp.i18n isn't present; update the line to
first defensively check for window.wp and wp.i18n and set __ to a safe fallback
(identity function that returns the input string) when unavailable so the script
continues to work without translations; ensure subsequent uses of __ remain
unchanged and the fallback is documented with a short comment.
When using the Ultimate Multisite login element for custom login pages,
invalid or expired password reset links now display proper error messages
instead of silently redirecting.
Changes:
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.