[URGENT] fix(checkout-pages): rewrite password reset URL on subsites (#1168)#1169
Conversation
…-Multisite#1168) Before this patch, replace_reset_password_link() returned early on subsites with `if ( ! is_main_site()) return $message;`. As a result the password reset email always pointed to the main network site's wp-login.php, even for users who registered on a subsite or on a mapped custom domain. This is a serious UX problem for networks where subsite owners run their own brand: 1. The end customer (the subsite owner's customer) gets a reset link on the network owner's domain, not on the brand they signed up with. 2. On mapped custom domains the email link goes to a completely different domain, which looks like phishing and breaks trust. 3. After resetting, the user lands on the main site's account area and never sees the subsite they were trying to access. This change makes replace_reset_password_link() subsite-aware: - On the main site: behaviour is unchanged. The wp-login.php URL is rewritten to the configured custom login page. - On a subsite: the URL is rewritten to home_url('/') of the current subsite with the same action=rp / key / login / wp_lang query args, so existing handlers (WooCommerce my-account, BuddyPress, custom themes, wp-login fallback) can pick up the request on the subsite's own domain. The hook registration was also moved out of the `is_main_site()` branch so the filter actually runs on subsites — without that move the new branch in the function would never execute. A new filter `wu_subsite_password_reset_url` lets integrations override the destination URL while keeping it on the subsite's domain (e.g. point to the WooCommerce reset-password endpoint). Tested on 8 subsites in a staging network including a custom mapped domain (.cl TLD): all of them now produce a reset URL on the correct subsite host. Main site behaviour verified unchanged.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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 (1)
📝 WalkthroughWalkthroughThe ChangesSubsite-Aware Password Reset Links
🎯 3 (Moderate) | ⏱️ ~22 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inc/checkout/class-checkout-pages.php`:
- Around line 423-451: The filter wu_subsite_password_reset_url should be
applied to the base endpoint before you append the reset query parameters;
currently apply_filters is called after add_query_arg which causes integrations
that return a clean path (e.g., "/my-account/lost-password/") to lose
action/key/login/wp_lang. Fix by calling
apply_filters('wu_subsite_password_reset_url', $subsite_base, $key, $user_login,
$user_data) (or similar) to let integrations override the endpoint, then build
$new_url = add_query_arg([
'action'=>'rp','key'=>$key,'login'=>rawurlencode($user_login),'wp_lang'=>$locale,
], $filtered_base) and finally call set_url_scheme($new_url, null).
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 411faef3-7ebd-41a1-a78a-481eb529f43d
📒 Files selected for processing (1)
inc/checkout/class-checkout-pages.php
| $subsite_base = home_url('/'); | ||
|
|
||
| $new_url = add_query_arg( | ||
| [ | ||
| 'action' => 'rp', | ||
| 'key' => $key, | ||
| 'login' => rawurlencode($user_login), | ||
| 'wp_lang' => $locale, | ||
| ], | ||
| $subsite_base | ||
| ); | ||
|
|
||
| /** | ||
| * Filter the subsite-aware password reset URL. | ||
| * | ||
| * Allows integrations (WooCommerce, BuddyPress, custom themes) | ||
| * to override the destination URL while keeping it on the | ||
| * subsite's own domain. | ||
| * | ||
| * @since 2.10.2 | ||
| * | ||
| * @param string $new_url The default subsite reset URL. | ||
| * @param string $key The reset key. | ||
| * @param string $user_login The user login. | ||
| * @param WP_User $user_data The user data. | ||
| */ | ||
| $new_url = apply_filters('wu_subsite_password_reset_url', $new_url, $key, $user_login, $user_data); | ||
|
|
||
| $new_url = set_url_scheme($new_url, null); |
There was a problem hiding this comment.
Apply wu_subsite_password_reset_url before appending the reset params.
The new hook currently receives the fully-built reset URL. If an integration returns a clean endpoint like /my-account/lost-password/, it will accidentally drop action, key, login, and wp_lang, and the emailed link stops working. Filter the base endpoint first, then append the standard reset query args afterward.
Suggested change
- $subsite_base = home_url('/');
-
- $new_url = add_query_arg(
- [
- 'action' => 'rp',
- 'key' => $key,
- 'login' => rawurlencode($user_login),
- 'wp_lang' => $locale,
- ],
- $subsite_base
- );
+ $subsite_base = apply_filters('wu_subsite_password_reset_url', home_url('/'), $key, $user_login, $user_data);
+
+ $new_url = add_query_arg(
+ [
+ 'action' => 'rp',
+ 'key' => $key,
+ 'login' => rawurlencode($user_login),
+ 'wp_lang' => $locale,
+ ],
+ $subsite_base
+ );
/**
- * Filter the subsite-aware password reset URL.
+ * Filter the subsite-aware password reset endpoint.
*
* Allows integrations (WooCommerce, BuddyPress, custom themes)
- * to override the destination URL while keeping it on the
+ * to override the destination endpoint while keeping it on the
* subsite's own domain.
*
* `@since` 2.10.2
*
- * `@param` string $new_url The default subsite reset URL.
+ * `@param` string $subsite_base The default subsite reset endpoint.
* `@param` string $key The reset key.
* `@param` string $user_login The user login.
* `@param` WP_User $user_data The user data.
*/
- $new_url = apply_filters('wu_subsite_password_reset_url', $new_url, $key, $user_login, $user_data);
-
$new_url = set_url_scheme($new_url, null);🧰 Tools
🪛 GitHub Check: Code Quality Checks
[warning] 449-449:
@param WP_Ultimo\Checkout\WP_User $user_data does not accept actual type of parameter: array.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inc/checkout/class-checkout-pages.php` around lines 423 - 451, The filter
wu_subsite_password_reset_url should be applied to the base endpoint before you
append the reset query parameters; currently apply_filters is called after
add_query_arg which causes integrations that return a clean path (e.g.,
"/my-account/lost-password/") to lose action/key/login/wp_lang. Fix by calling
apply_filters('wu_subsite_password_reset_url', $subsite_base, $key, $user_login,
$user_data) (or similar) to let integrations override the endpoint, then build
$new_url = add_query_arg([
'action'=>'rp','key'=>$key,'login'=>rawurlencode($user_login),'wp_lang'=>$locale,
], $filtered_base) and finally call set_url_scheme($new_url, null).
superdav42
left a comment
There was a problem hiding this comment.
Auto-approved by pulse runner @superdav42 — author @kenedytorcatt confirmed collaborator, pre-merge gates passed.
…-Multisite#1168) While reproducing the original password-reset URL bug we found 3 more email hooks with the same architectural problem: the URL is built with network_site_url('wp-login.php'), so on subsites it always points to the main network site. UM was not handling any of them. Same root cause, same fix shape: - wp_new_user_notification_email: when an admin creates a user on a subsite, the 'set your password' link in the welcome email pointed to the main site. New users got bounced off the subsite they were just registered on. - retrieve_password_notification_email: when a user requests a reset, the admin notification firing alongside the user email contained a wp-login.php URL on the main site, so subsite admins got links pointing somewhere they cannot act on. - new_user_email_content: when a user changes their email from their profile on a subsite, the confirmation link pointed to the main site, so the confirm-email step failed for users editing their profile on a subsite. All three are addressed with the same approach: rewrite any wp-login.php URL embedded in the email body so it stays on the current site (custom login page on main, home_url on subsites). The shared helper rewrite_subsite_aware_login_url() centralizes the rewrite logic so all four filters behave consistently. Tested across 5 subsites in staging: 20/20 PASS (4 hooks x 5 sites). Main site regression: all 4 still rewrite to the configured custom login page, no behaviour change there.
PR extended — covering 3 more auth-email hooks with the same root causeHi David, after the initial fix I went back and audited every auth-related email WordPress fires from a subsite, to see whether the same problem existed elsewhere. It does. Pushed an additional commit ( What I foundThe original report (zuletadia / yariglam.cl) was about the "I forgot my password" flow only. But the underlying cause —
I reproduced all four on UM 2.10.1 in our staging network. Sample output across 5 subsites before the extension: So a real user life-cycle on a subsite (be created → set password → maybe change password later → maybe change email later) was hitting cross-domain URLs at every single step, not just the password-reset one. The user is sent away from the subsite they are actually using on every auth touchpoint. Why the extensionThree reasons I think it belongs in the same PR rather than a follow-up:
What changed in the extension commit (
|
superdav42
left a comment
There was a problem hiding this comment.
Auto-approved by pulse runner @superdav42 — author @kenedytorcatt confirmed collaborator, pre-merge gates passed.
) (#1172) Locks in the subsite-aware behaviour added in #1169 so the bug cannot silently regress again. The original GH#1168 bug was masked for years because an existing test test_replace_reset_password_link_returns_early_on_subsite was actively asserting the buggy behaviour as a contract. Replacing that test and adding nine new tests covers all four hooks introduced in #1169: - replace_reset_password_link: subsite host is preserved, wp-login.php is removed from the URL, all standard query args (action, key, login, wp_lang) are kept, and the wu_subsite_password_reset_url integration filter actually fires. - retrieve_password_message hook is registered on subsites (not just main site). - rewrite_new_user_notification_email: welcome email links stay on the subsite for users created on a subsite. - rewrite_password_notification_email: admin notification email stays on the subsite. - rewrite_email_change_content: profile email-change confirmation links stay on the subsite. Each test includes an empty-input safety case where applicable. Verification: reverted inc/checkout/class-checkout-pages.php to its pre-#1169 state and re-ran the new tests; 9/13 fail (4 errors from the new methods being absent, 5 failures from the subsite host assertions), proving the tests catch the regression. Ref #1168
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Cannot open a stacked PR on a forked repository. |
…1169 (subsite password reset) (#1198) Two Cypress specs + matching WP-CLI fixtures that lock in the behaviour of two recently merged fixes so future refactors of the SSO bootstrap chain or the password reset rewrite can't silently regress them. ## 066-sso-bootstrap-race.spec.js (guards PR #1185) Verifies: - `calculate_secret_from_date('')` does NOT throw and returns a hash. - Two consecutive calls with empty input return the SAME hash (deterministic fallback — important so SSO state stays consistent across requests during a bootstrap window). - `convert_bearer_into_auth_cookies()` does NOT throw when `$current_blog` exists with an empty `registered` property. Driven via fixture `setup-sso-bootstrap-race.php` which calls both methods through the live `SSO::get_instance()` singleton and emits JSON. ## 011-password-reset-subsite-domain.spec.js (guards PR #1169) Verifies: - The URL produced by `retrieve_password_message` on a subsite uses the subsite host (or at least no longer points at `/wp-login.php` on a different host). - The reset query args `action / key / login / wp_lang` are preserved so WooCommerce my-account, BuddyPress, custom themes, and the default wp-login fallback can still pick the request up. - The new `wu_subsite_password_reset_url` filter (added by #1169) is reachable for integration overrides. Driven via fixture `setup-password-reset-subsite.php` which creates a test subsite, switches into it, and applies the filter chain directly with a synthetic raw message — no SMTP / Mailpit / cron dependency. Co-authored-by: Kenedy Torcatt <kursopro7@gmail.com>
…1280) * chore: upgrade planning templates * ci(e2e): run the 9 cypress specs that existed but were never invoked The e2e workflow listed only 6 of the 16 spec files in tests/e2e/cypress/integration/. The other 10 were committed and syntactically valid but never executed in CI, so any regression they guarded against could slip through unnoticed. Audit findings (16 specs total): Already in CI (6, unchanged): - 000-setup.spec.js - 010-manual-checkout-flow.spec.js - 020-free-trial-flow.spec.js - 030-stripe-checkout-flow.spec.js (gated, STRIPE_TEST_SK_KEY) - 040-stripe-renewal-flow.spec.js (gated, STRIPE_TEST_SK_KEY) - 065-sso-redirect-loop.spec.js Added to CI in this change (9): - login.spec.js (sanity) - mail.spec.js (sanity) - wizard.spec.js (drives Setup Wizard UI) - 011-password-reset-subsite-domain.spec.js (regression: PR #1169) - 030-modal-form-error-handling.spec.js (UI: AJAX error handling) - 050-password-strength-enforcement.spec.js (UI: zxcvbn scoring) - 060-sso-cross-domain.spec.js (SSO: domain mapping) - 066-sso-bootstrap-race.spec.js (regression: PR #1185) - 035-paypal-checkout-flow.spec.js (gated, new PAYPAL_SANDBOX_*) Intentionally excluded (1): - installation.spec.js — single ‘cy.visit(/wp-admin/)’ call with no assertions; doesn’t actually test anything. Recommend deletion in a follow-up; not removed here to keep this change ci-only. Why wizard.spec.js matters specifically: 000-setup.spec.js bypasses the Setup Wizard UI by calling the installer + setup-finished flag directly. The path Setup_Wizard_Admin_Page::handle_save_settings -> Settings::save_settings -> Field::set_value -> validate_textarea_field was therefore never exercised in CI. The recent fix/textarea-array-coercion fix added unit-test coverage for the validator itself; running wizard.spec.js now covers the integration path. Step organisation (after the existing ‘Run Setup Test’): 1. Sanity Tests — login, mail (smoke) 2. Wizard Test — wizard 3. Regression & UI — 011, 030, 050, 066 4. Checkout Tests — 010, 020 (existing) 5. SSO Tests — 060 (new), 065 (was: only 065) 6. Stripe Tests — 030, 040 (existing, gated) 7. PayPal Tests — 035 (new, gated on PAYPAL_SANDBOX_*) All new groups follow the existing pattern: ‘set +e’ + per-spec loop + aggregated failure count + final exit 1 if any failed, so a single flake doesn’t mask other failures. PayPal step mirrors the Stripe gating: if PAYPAL_SANDBOX_CLIENT_SECRET is unavailable (typical for fork PRs) the step prints the same diagnostic message Stripe uses and exits 0. The PAYPAL_SANDBOX_* repo secrets are not currently configured upstream, so PayPal will skip until they are added; Stripe continues to run. Verification performed in this change: - YAML parse: python3 -c 'import yaml; yaml.safe_load(open(path))' OK - Per-spec node --check on all 9 added specs OK - All referenced fixture .php files exist in tests/e2e/cypress/fixtures - Inventory diff: workflow now references 15/16 specs Local Cypress execution is not attempted in this commit because each matrix variant requires a fresh wp-env (Docker download + WP install + plugin activation + multi-spec run) and would take longer than the CI run that gates the merge. CI is the canonical verification surface for this change. * ci(e2e): disable custom login page after wizard spec The wizard's Default Content step creates a Login page and points Ultimate Multisite's custom-login feature at it, which redirects /wp-login.php -> /login/. The custom page has no #rememberme checkbox, so cy.loginByForm() fails in every subsequent spec's session-setup hook. 000-setup.spec.js already calls setup-disable-custom-login.php defensively in its before() hook for the same reason; this CI step runs the same fixture right after wizard.spec.js so the regression, checkout, SSO, and gated gateway groups inherit a clean login state. Observed on PR #1280 cypress matrix run 26456938227 where 030-modal-form-error-handling failed in its session-setup hook with 'Expected to find element: #rememberme, but never found it', cascading into every subsequent step being skipped.
Fixes #1168.
Summary
replace_reset_password_link()returned early on subsites, so the password reset email always pointed to the main network site'swp-login.php. End customers of subsite owners (especially on mapped custom domains) were taken to a brand they didn't recognize and frequently couldn't complete the reset. This PR makes the function subsite-aware.Why this is urgent
This is breaking real users in production right now. We run a multisite SaaS where every customer has their own mapped custom domain. Today a customer's customer (
zuletadiaonyariglam.cl) tried to reset her password and received a link tohttps://kursopro.com/wp-login.php— a domain she doesn't recognize. She didn't trust it. Every subsite owner in any UM-based network is hitting this whenever one of their users requests a password reset.Reproduced on 8 subsites in our staging network, including subdomains and a
.clmapped TLD. 8/8 broken before patch.Root cause
Two issues in
inc/checkout/class-checkout-pages.php:Hook never runs on subsites. The
add_filter('retrieve_password_message', ...)registration is wrapped insideif (is_main_site())(around line 67), so on a subsite the filter is not even attached.Function returns early on subsites. Even if the filter ran,
replace_reset_password_link()hadif ( ! is_main_site()) return $message;at the top (line 354), so it never rewrote the URL.Combined, WordPress's default
retrieve_password()builds the URL withnetwork_site_url('wp-login.php'), which on every subsite resolves to the main site'swp-login.php. UM never gets a chance to fix it.Solution
This PR:
Moves
add_filter('retrieve_password_message', ...)out of theis_main_site()branch and into theif ($use_custom_login)block, so the filter runs on all sites in the network when custom login is enabled.Rewrites
replace_reset_password_link()to be subsite-aware:home_url('/')of the current subsite, with the sameaction=rp / key / login / wp_langquery args. Existing handlers (WooCommerce my-account, BuddyPress, custom themes, even default wp-login fallback) pick up the request on the subsite's own domain and complete the reset there. The user never leaves the brand they signed up on.Adds a
wu_subsite_password_reset_urlfilter so integrations can point at a specific endpoint (e.g. the WooCommercelost-passwordendpoint, a custom page, etc.) without having to re-implement the rewrite themselves.The diff is intentionally minimal and self-contained: one method body refactored, one hook registration moved by ~10 lines, one new filter introduced. No new files. No changes to
filter_lostpassword_url()orfilter_login_url()— those were already correct.What I changed
inc/checkout/class-checkout-pages.php:add_filter('retrieve_password_message', [$this, 'replace_reset_password_link'], 10, 4);from insideif (is_main_site())to theif ($use_custom_login)block (so it runs on all sites).if ( ! is_main_site()) return $message;guard fromreplace_reset_password_link().home_url('/')of the current subsite.wu_subsite_password_reset_urlfilter for integration overrides.@since 2.10.2for the new behaviour.Test plan
-lsyntax check passes.retrieve_password()flow tested on 5 subsites with non-network-admin users: PASS (the failures we saw in our test runs were unrelated rate-limit / SMTP issues, not the patch).yariglam.cl, .cl TLD): URL correctly rewritten tohttp://yariglam.cl/?action=rp&key=...&login=zuletadia&wp_lang=es_ES./login).action=rp / key / login / wp_langquery args.Backwards compatibility
enable_custom_login_page = 0: no change at all. The hook registration is still gated on this setting.enable_custom_login_page = 1(the affected setup): main site behaviour is exactly the same as before. Only the subsite branch is new — and on subsites the previous behaviour was already broken (URL pointed to the main site), so there is nothing useful to preserve.wu_subsite_password_reset_urlfilter to direct the URL at a specific endpoint.Suggested release
Please consider a 2.10.2 patch release shortly after merge — every UM-based network with custom login enabled is impacted. Happy to iterate on review feedback ASAP.
Thanks David, and sorry for the urgency — we have customers losing trust over this every time someone forgets their password.
Summary by CodeRabbit
Bug Fixes
New Features