fix(sso): restore front-end auto-SSO via main-site magic-link redirect#1297
fix(sso): restore front-end auto-SSO via main-site magic-link redirect#1297superdav42 wants to merge 10 commits into
Conversation
Visiting a subsite front-end page while logged in to the main site was not re-authenticating the visitor on the subsite even in browsers that permit third-party cookies. Two interacting regressions blocked the flow: 1. #1185 turned the JSONP unattached branch in handle_broker() into an immediate '{code:0, message:"Broker not attached"}' response, which triggered sso_denied() in sso.js and set wu_sso_denied for 5 minutes on the very first subsite page-load — there was no longer any path that ever surfaced the main-site session to the broker. 2. The redirect branch still called $broker->getAttachUrl() which targets /sso-grant. After the cookie-less rework in #1084 the dispatcher in handle_requests() unconditionally appends '_grant' on main, so /sso-grant produces 'wu_sso_handle_sso_grant_grant' — an action with no listener — and the request silently 404'd on the main site, leaving the user logged out on the subsite. This change replaces both branches with a redirect to the main-site /sso endpoint that handle_server() already supports. handle_server() checks the first-party main-site auth cookie (so third-party-cookie posture is irrelevant), issues an HMAC-signed wu_sso_token via add_cookie_less_sso_token(), and 302s back to the subsite where handle_cookie_less_sso_token() consumes the token at init priority 4 and sets the broker-side auth cookie: GET <broker>/sso?_jsonp=1 -> 200 must-redirect JSONP GET <broker>/sso?return_url=<original> -> 302 GET <main>/sso?sso=login&return_url=<original> &redirect_to=<original> -> 302 GET <broker>/?wu_sso_token=<hmac> &redirect_to=<original> -> 302 GET <broker>/ -> 200, logged in For the JSONP branch (which cannot follow a 302 to HTML in a <script> tag), the response now returns 'verify: must-redirect' which the already-present sso.js branch (assets/js/sso.js#L93) handles by performing a top-level navigation. The non-JSONP path of handle_broker is then re-entered as a normal page load and performs the redirect to $main_sso_url. The return_url passed forward is the explicit ?return_url= query param when present (set by sso.js on the must-redirect navigation) and falls back to get_current_url() only on the direct first hit. The /sso path is rejected as a return target so a stray legacy URL cannot restart the same nested-/sso ERR_TOO_MANY_REDIRECTS loop. redirect_to is also passed explicitly so get_sso_redirect_to() does not append /wp/wp-admin/ for front-end visitors. Verified end-to-end in a real browser against a multisite with mapped subsite domains: logged-in main-site visitor lands logged-in on the subsite front-end page they originally requested.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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: |
|
Follow-up issues filed for the two items in the "Out of scope" section above:
aidevops.sh v3.20.0 plugin for OpenCode v1.15.11 with claude-opus-4-7 spent 1h 11m and 87,936 tokens on this with the user in an interactive session. |
🔨 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: |
|
Performance Test Results Performance test results for 5c4e3ec are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
🔨 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: |
🔨 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: |
🔨 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: |
🔨 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: |
|
CLAIM_RELEASED reason=worker_failed runner=superdav42 ts=2026-05-28T04:39:04Z aidevops_version=3.20.4 opencode_version=1.15.11 |
🔨 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: |
🔨 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: |
🔨 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: |
🔨 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: |
MERGE_SUMMARYStatus: BLOCKED — Cypress still fails on the target SSO spec after pushed fixes. Changes pushed to
Verification:
Current blocker:
|
|
CLAIM_RELEASED reason=worker_complete runner=superdav42 ts=2026-05-28T15:38:53Z aidevops_version=3.20.4 opencode_version=1.15.11 |
|
Closing in favour of #1309, which carries the minimal subset of this PR that the post-#1301 / post-#1302 codebase still needs. When this PR was opened, two regressions interacted:
(2) has since been fixed independently:
With those landed, the only remaining defect is the JSONP denial payload (1). #1309 fixes exactly that one line and adds source-code regression tests asserting If the checkout-form skip and wp-env Cypress fixture rework are still wanted, they should each be opened as separate PRs scoped to their own bug. |
…1309) The unattached-broker JSONP branch in handle_broker() returned '{code: 0, message: "Broker not attached"}', which sso.js treats as a denial (assets/js/sso.js lines 101-117). That set the wu_sso_denied cookie for 5 minutes on the very first subsite front-end page-load, silently disabling auto-SSO before it could complete a single round-trip — even when the user was in fact logged in on the main site. The denial payload is the wrong signal here: the broker not being attached on the JSONP probe is the expected state on the first hit, not a failure. Return 'verify: must-redirect' instead so the already-existing sso.js branch (assets/js/sso.js lines 93-95) performs a top-level navigation to '<broker>/sso?return_url=...'. That request re-enters handle_broker() as a regular page load and falls through to the non-JSONP redirect, which after #1301 reaches the cookie-less server handler via getAttachUrl() and completes the SSO round-trip. This is the minimal subset of #1297 that the post-#1301/#1302 codebase still needs. The /sso-grant routing fix (#1301) and the scoped redirect-loop counter (#1302) already shipped, so the rest of #1297's PHP changes are no longer required. Regression tests in tests/WP_Ultimo/SSO/SSO_Test.php: - test_handle_broker_source_jsonp_unattached_returns_must_redirect: locks in the new payload and forbids the legacy 'Broker not attached' denial. - test_sso_js_handles_must_redirect_verify: confirms sso.js still recognises the must-redirect verify value.
|
CLAIM_RELEASED reason=process_exit runner=superdav42 ts=2026-06-02T22:43:21Z aidevops_version=3.20.10 opencode_version=1.15.13 exit=0 session_count=1 |
1 participant
Summary
Visiting a subsite front-end page while logged in to the main site was
not re-authenticating the visitor on the subsite — even in browsers
that permit third-party cookies. Two interacting regressions blocked
the flow:
fix(sso): guard against unpopulated $current_blog during early bootstrap #1185 turned the JSONP unattached branch in
handle_broker()intoan immediate
{code: 0, message: "Broker not attached"}response,which calls
sso_denied()insso.jsand setswu_sso_deniedfor5 minutes on the very first subsite page-load. After that there is
no path that ever surfaces the main-site session to the broker.
The redirect branch still called
$broker->getAttachUrl()whichtargets
/sso-grant. After the cookie-less rework in fix: cookie-less cross-domain SSO login flow #1084 thedispatcher in
handle_requests()unconditionally appends_granton main, so
/sso-grantproduceswu_sso_handle_sso_grant_grant— an action with no listener — and the request silently 404s on
main, leaving the user logged out on the subsite.
What this changes
Both branches now redirect to the main-site
/ssoendpoint thathandle_server()already supports under the cookie-less rework.handle_server()checks the first-party main-site auth cookie (sothird-party-cookie posture is irrelevant), issues an HMAC-signed
wu_sso_tokenviaadd_cookie_less_sso_token(), and 302s back to thesubsite where
handle_cookie_less_sso_token()consumes the token atinitpriority 4 and sets the broker-side auth cookie.Resulting request chain (verified in browser):
JSONP branch
The JSONP
<script>tag cannot follow a 302 to HTML, so instead ofsilently failing the broker returns
verify: 'must-redirect'. Thealready-present branch in
assets/js/sso.js(lines 93-95) handles thisby performing a top-level
window.location.replace()too.server_url + '?return_url=<encoded current>', which re-entershandle_broker()via the non-JSONP path on the same request andredirects to
$main_sso_url.Return-URL handling
return_urlis taken from the explicit?return_url=query param when present (set by
sso.json the must-redirectnavigation) and falls back to
get_current_url()only on the directfirst hit.
/ssopath itself is rejected as a return target — withoutthis guard the value
/sso?return_url=…would feed back intoreturn_urlon each iteration and the URL would grow on every hop,producing an
ERR_TOO_MANY_REDIRECTS.redirect_tois passed explicitly soget_sso_redirect_to()doesnot append
/wp/wp-admin/(its fallback when only a cross-domainreturn_urlis supplied) and therefore does not send front-endvisitors to the subsite admin.
Tests / verification
php -lclean on the modified file.with mapped subsite domains: a main-site-logged-in visitor lands
logged-in on the subsite front-end page they originally requested.
wu_sso_deniedisunaffected by the previous broken behaviour because the new code
path no longer sets that cookie on its own.
Out of scope
Two related issues observed while diagnosing this, left for follow-up:
/sso-grantrouting is dead. The dispatcher inhandle_requests()rewrites the action tosso_grantand thenappends
_grant, so the only URL that reacheshandle_server()is/sso(not/sso-grant). TheSSO_Broker::getAttachUrl()methodstill points at
/sso-grant. After this PR no caller uses that URL,but it should either be removed or the dispatcher should normalise
so the two URLs do the same thing.
should_skip_redirect_due_to_loop()from GH#1199: fix: cap SSO redirects on mapped domains #1203 incrementswu_sso_redirect_attemptson every logged-outhandle_auth_redirectcall regardless of whether the previous request was itself part of
an SSO loop. It can burn the counter on benign repeat visits to
wp-admin from a logged-out user. Consider scoping it to requests
whose
Refereris the main-site/sso*endpoint.aidevops.sh v3.20.0 plugin for OpenCode v1.15.11 with claude-opus-4-7 spent 1h 2m and 81,144 tokens on this with the user in an interactive session.