fix(login): show proper error for invalid password reset links #312
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>
📝 WalkthroughWalkthroughAdds client- and server-side password strength enforcement to the password-reset flow, new underscore-prefixed reset-key error codes, UI changes to render a strength meter and related assets, and server validation that emits a Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant FrontendJS as "wu-password-reset.js"
participant Server as "WP (Checkout_Pages)"
rect rgba(135,206,235,0.12)
Note over Browser,FrontendJS: User opens reset-password page
Browser->>FrontendJS: page load
FrontendJS-->>Browser: injects strength UI, minStrength, labels
end
rect rgba(144,238,144,0.08)
Note over Browser,FrontendJS: User types new password
Browser->>FrontendJS: keyup/input events
FrontendJS->>FrontendJS: evaluate strength (wp.passwordStrength.meter)
alt strength < minStrength
FrontendJS->>Browser: show "too weak", disable submit
else strength >= minStrength
FrontendJS->>Browser: show strength label, enable submit
end
end
rect rgba(255,228,196,0.10)
Note over Browser,Server: Form submission
Browser->>Server: POST reset form (rp_key,user_login,password...)
Server->>Server: check_password_reset_key()
alt key invalid/expired
Server->>Browser: redirect to lost-password?error=invalid_key/expired_key (clear cookie)
else key valid
Server->>Server: validate_password_strength()
alt password weak
Server->>Browser: return error (`password_too_weak`) and re-render form
else password ok
Server->>Server: complete reset, authenticate/redirect success
end
end
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: |
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>
🔨 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
views/admin-pages/fields/field-password.php (1)
38-44: Consider making the strength meter ID dynamic.The hardcoded
id="pass-strength-result"on Line 40 could cause conflicts if multiple password fields with meters exist on the same page. While this is unlikely in the password reset flow, making the ID dynamic (e.g.,pass-strength-result-<?php echo esc_attr($field->id); ?>) would improve reusability.However, this would also require updating the JavaScript in
wu-password-reset.jsto use a dynamic selector.inc/checkout/class-checkout-pages.php (1)
284-342: Consider alignment between client and server password strength algorithms.The server-side strength calculation uses a simplified algorithm based on length and character variety, while the client-side JavaScript uses WordPress's
wp.passwordStrength.meter(which relies on the zxcvbn library). This could lead to edge cases where:
- A password passes client-side validation but fails server-side (or vice versa)
- Users receive inconsistent feedback about password strength
While both enforce the same minimum threshold (strength level 3), the scoring mechanisms differ significantly. For better consistency, consider:
- Using a PHP port of zxcvbn for server-side validation, or
- Documenting this known limitation
- Ensuring the simplified algorithm is calibrated to be slightly more permissive than zxcvbn to avoid false rejections
The current implementation is functional but may lead to user confusion in edge cases.
Do you want me to search for PHP implementations of zxcvbn that could provide consistency between client and server validation?
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
assets/js/wu-password-reset.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)
views/admin-pages/fields/field-password.php (1)
inc/ui/class-field.php (1)
print_wrapper_html_attributes(477-484)
inc/checkout/class-checkout-pages.php (2)
inc/functions/helper.php (1)
wu_request(132-137)assets/js/checkout.js (1)
strength(780-780)
assets/js/wu-password-reset.js (1)
assets/js/checkout.js (2)
pass1(764-764)strength(780-780)
inc/ui/class-login-form-element.php (1)
inc/functions/helper.php (1)
wu_get_version(21-24)
🪛 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 (10)
assets/js/wu-password-reset.js (3)
1-21: LGTM!The setup and configuration are well-structured. The IIFE pattern properly isolates the code, the i18n fallback ensures graceful degradation, and the default minStrength of 3 (medium) correctly aligns with the server-side validation threshold.
82-119: LGTM!The initialization logic is well-implemented with proper event binding, initial state management, and form submission prevention for weak passwords. The progressive enhancement approach (checking if elements exist before binding) is good practice.
47-47: The third parameter is correct aspass2. This is a password reset form with both password and confirmation fields, where the third parameter is used for password matching validation (not disallowed list checking, which is handled by the second parameter). The switch statement correctly includes case 5 fori18n.mismatch. Thecheckout.jsusage passespass1twice only because that form has a single password field without confirmation, making the approaches both valid for their respective contexts.Likely an incorrect or invalid review comment.
views/admin-pages/fields/field-password.php (1)
1-37: LGTM!The template structure follows WordPress best practices with proper ABSPATH protection, consistent output escaping, and reuse of existing partial templates. The field attributes are correctly configured.
inc/checkout/class-checkout-pages.php (3)
65-65: LGTM!The hook registration at priority 5 ensures password strength validation runs before error handling (priority 10), which is the correct order.
209-211: LGTM!The new error codes follow a consistent underscore-separated naming convention and provide clear, user-friendly messages. The
password_too_weakmessage correctly uses the plugin's text domain.
255-282: LGTM!The password strength validation correctly enforces a minimum strength of "medium" (level 3), matching the client-side validation. The early return for empty passwords is appropriate as WordPress core handles that case separately.
Note: The static analysis warning about the unused
$userparameter is a false positive—the parameter is required by thevalidate_password_resethook signature.inc/ui/class-login-form-element.php (3)
304-330: LGTM!The script registration is well-structured with proper conditional loading for the reset password page only. The dependencies are correct, and the localized strings align with both the JavaScript implementation and server-side validation (min_strength: 3).
635-662: Excellent fix for invalid password reset links!This properly addresses the issue described in the PR objectives by:
- Checking if
check_password_reset_key()returns aWP_Error- Redirecting to the lost password page with the appropriate error code
- Clearing the invalid reset cookie before redirecting
- Preventing the silent failure that occurred previously
The implementation is secure and provides clear user feedback.
664-720: LGTM!The form field configuration is properly structured with:
- Correct
hash_equals()comparison on Line 664 (comparing$rp_keyfrom cookie with$_POST['rp_key'])—this appears to be the bug fix mentioned in the PR summary- Strength meter enabled on the password field (
meter => trueon Line 679)- Proper
autocomplete="new-password"attributes for browser password managers (Lines 683, 694)- All required hidden fields for the reset flow
Summary
hash_equalscompared the same value to itself (always true)Changes
is_wp_error()check aftercheck_password_reset_key()inLogin_Form_Elementinvalid_key/expired_key)invalid_key,expired_key) to error messages arrayTest plan
?action=rp&key=invalid&login=user)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.