fix: keep password reset on subsite domain (#291)#380
Conversation
When a user clicked 'Lost your password?' from a subsite login form, the lostpassword_url filter was routing them to the main network site's login page (or wp-login.php), causing a 404 when login URL obfuscation was enabled. Root cause: the lostpassword_url filter reused filter_login_url() which always returns the main site's custom login page URL, regardless of which site the user is on. Fix: - Add filter_lostpassword_url() that on subsites returns the current page URL with ?action=lostpassword, keeping users on their own domain - On the main site, delegate to filter_login_url() (existing behaviour) - Update the 'Lost your password?' link in Login_Form_Element to use wp_lostpassword_url() so it benefits from the new filter The lostpassword form already sets redirect_to to the subsite page URL, so the post-submission redirect back to the subsite was already correct. The bug was only in the link that takes users to the lostpassword form.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes fix password reset link handling for multisite installations by introducing a dedicated filter method in the Checkout Pages class that constructs lost-password URLs for subsites, preventing unwanted redirects to the main network site's login page. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: |
PHPCS requires an empty line before block comments. Moving the explanatory comment for the wp_lostpassword_url() change outside the array definition to comply with the coding standard.
🔨 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/checkout/class-checkout-pages.php`:
- Around line 644-682: The filter_lostpassword_url method drops the $redirect
parameter for subsites; when building $subsite_lostpassword_url (using
wu_get_current_url(), remove_query_arg and add_query_arg) ensure you preserve
and append the redirect target by adding a redirect_to query arg when $redirect
is non-empty (or pass $redirect through to add_query_arg alongside the existing
action param), so callers that provide $redirect receive the redirect_to param
on subsites just like the main-site branch does via filter_login_url.
In `@inc/ui/class-login-form-element.php`:
- Around line 828-835: The PHPCS failure is caused by missing blank line before
the block comment above the 'content' array entry in
class-login-form-element.php; open the code around the 'content' => sprintf(...
wp_lostpassword_url() ...) line and insert a single empty line immediately
before the /* block comment so there is a blank line separating the previous
code and the comment, then keep the comment and the 'content' assignment
unchanged to satisfy the “Empty line required before block comment” rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d770178d-ae80-40e8-8cb7-69256e092453
📒 Files selected for processing (2)
inc/checkout/class-checkout-pages.phpinc/ui/class-login-form-element.php
When filter_lostpassword_url() is called with a non-empty $redirect argument, include it as redirect_to in the subsite URL so callers receive the same behaviour as on the main site via filter_login_url(). Addresses CodeRabbit review finding.
🔨 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: |
1 participant
Summary
Fixes the password reset redirect bug where clicking "Lost your password?" from a subsite login form redirected users to the main network site's
wp-login.php(or custom login page), causing a 404 when login URL obfuscation was enabled.Root Cause
The
lostpassword_urlfilter was reusingfilter_login_url(), which always returns the main site's custom login page URL regardless of which site the user is on. On a subsite, this sent users tovoyagosites.com/wp-login.php?action=lostpasswordinstead of staying onmoonshottravel.voyagosites.com.Changes
inc/checkout/class-checkout-pages.phpadd_filter('lostpassword_url', [$this, 'filter_login_url'], ...)with a dedicatedfilter_lostpassword_url()methodfilter_login_url()(existing behaviour unchanged)?action=lostpassword, keeping users on their own domain throughout the entire password reset flowinc/ui/class-login-form-element.phpwp_lostpassword_url()instead ofadd_query_arg('action', 'lostpassword')so it benefits from the new filterFlow After Fix
moonshottravel.voyagosites.com/login/clicks "Lost your password?"wp_lostpassword_url()→filter_lostpassword_url()→ returnsmoonshottravel.voyagosites.com/login/?action=lostpassword✓redirect_toto the subsite URL)lostpassword_redirectfilter returns the subsite URL with?checkemail=confirm✓Notes
The lostpassword form's
redirect_tohidden field was already correctly set to the subsite URL, so the post-submission redirect was already working. The bug was only in the "Lost your password?" link that takes users to the lostpassword form view.Closes #291
Summary by CodeRabbit
Release Notes