fix(sso): admin auth-redirect preempts incoming cookie-less token - infinite customize.php <-> /login/ loop (site customizer down)#1636
Conversation
…s token handle_auth_redirect() kicks off an SSO round-trip for any wp-admin request that is not logged in and not on the main domain. When the request ALREADY carries a valid ?wu_sso_token= (the cookie-less flow that panel/iframe integrations use to open a subsite's customizer or admin), that kick-off fires BEFORE handle_cookie_less_sso_token() (init:4) can consume the token: the request is 302'd to the main /login/, which mints yet another token and sends the browser back - an infinite customize.php <-> /login/ loop (~2 rounds/sec in production access logs). Front-end URLs are unaffected (no kick-off there), which is why the bug only surfaces on wp-admin targets like the customizer. Real-world impact (kursopro.com, 2026-07-14): the site customizer opened through the panel was down for every subsite - the embedding UI just shows a generic error while the iframe loops. Fix: mirror the existing $sso_path short-circuit - when wu_sso_token is present in the current request, return true so the auth redirect is bypassed and the init:4 receiver consumes the token. Verified end-to-end in production: token -> 302 clean URL + auth cookies -> HTTP 200 customizer.
📝 WalkthroughWalkthroughAdds an early return to ChangesSSO redirect handling
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: |
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inc/sso/class-sso.php`:
- Line 393: Add the required empty line immediately before the block comment
near the affected code in class-sso.php, without changing the comment or
surrounding logic.
- Around line 403-404: Update the SSO token guard to accept only string values
before bypassing authentication, without casting arrays or other types to
strings. In handle_cookie_less_sso_token(), add the same non-string early return
before invoking validate_sso_token(), while preserving existing handling for
valid string tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| if ($this->input($sso_path) && $this->input($sso_path) !== 'done') { | ||
| return true; | ||
| } | ||
| /* |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required blank line before the block comment.
This line triggers the static-analysis warning for a missing empty line before the new comment.
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[warning] 393-393:
Empty line required before block comment
🤖 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/sso/class-sso.php` at line 393, Add the required empty line immediately
before the block comment near the affected code in class-sso.php, without
changing the comment or surrounding logic.
Source: Linters/SAST tools
| if ('' !== (string) $this->input('wu_sso_token', '')) { | ||
| return true; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject non-string SSO tokens before bypassing authentication.
wu_sso_token[]=... is parsed as an array. The cast makes this condition true, but handle_cookie_less_sso_token() later passes the same array to validate_sso_token(string), causing a PHP TypeError instead of normal authentication handling. Validate the token type consistently in both the guard and consumer.
Proposed fix
- if ('' !== (string) $this->input('wu_sso_token', '')) {
+ $wu_sso_token = $this->input('wu_sso_token', '');
+
+ if (is_string($wu_sso_token) && '' !== $wu_sso_token) {
return true;
}Also make handle_cookie_less_sso_token() return early for non-string values before calling validate_sso_token().
🤖 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/sso/class-sso.php` around lines 403 - 404, Update the SSO token guard to
accept only string values before bypassing authentication, without casting
arrays or other types to strings. In handle_cookie_less_sso_token(), add the
same non-string early return before invoking validate_sso_token(), while
preserving existing handling for valid string tokens.
superdav42
left a comment
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.
2 participants
Real-world impact (kursopro.com, 2026-07-14)
The site customizer opened through our panel was down for every subsite. The embedding UI shows a generic error while the iframe silently loops. Access log (~2 rounds per second):
The bug
handle_auth_redirect()kicks off an SSO round-trip for any wp-admin request that is not logged in and not on the main domain. But when the request already carries a valid?wu_sso_token=(the cookie-less flow), the kick-off fires beforehandle_cookie_less_sso_token()(init:4) can consume it - so the token is never consumed, the main site mints another one, and the loop never converges. Thewu_sso_redirect_attemptsbreaker only degrades it to a login form inside the iframe, which the embedding UI renders as an error.Front-end URLs are unaffected (no kick-off there) - we verified a fresh token on
subsite/?wu_sso_token=...consumes perfectly (auth cookies set), while the same flow onwp-admin/customize.phpnever even reaches the receiver (zero lines in the sso log, not even the already-used error for a burned token).The fix
Mirror the existing
$sso_pathshort-circuit right above: whenwu_sso_tokenis present in the current request, returntrueso the auth redirect is bypassed and the init:4 receiver consumes the token.1 file, +13 lines (comment-heavy), no behavior change for requests without a token.
Verified end-to-end in production
After deploying this patch:
customize.php?wu_sso_token=FRESH-> 302 to the clean URL with auth cookies set -> following with cookies -> HTTP 200, customizer opens.Related: #1624 (merged - thanks for the fast turnaround!), Ultimate-Multisite/ultimate-multisite-woocommerce#118, Ultimate-Multisite/ultimate-multisite-woocommerce#119.
Summary by CodeRabbit