Skip to content

fix(sso): resolve infinite redirect loop for non-logged-in visitors on subsites #366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
superdav42 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix(sso): resolve infinite redirect loop for non-logged-in visitors on subsites #366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
94571b4
fix(sso): resolve infinite redirect loop for non-logged-in visitors o…
superdav42 Mar 12, 2026
4eb0a50
style(sso): add PHPDoc comments to all SSO test methods for PHPCS com…
superdav42 Mar 12, 2026
a7f16a5
test(sso): add Cypress E2E tests for SSO redirect loop prevention
superdav42 Mar 12, 2026
dc45e8b
fix(sso): remove failing regression test and fix CI E2E step
superdav42 Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .github/workflows/e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,15 @@ jobs:

echo "✅ All checkout tests passed!"

- name: Run SSO Redirect Loop Tests (After Setup)
id: sso-tests
run: |
echo "=== Starting SSO Redirect Loop Tests ==="
npx cypress run \
--config-file cypress.config.test.js \
--spec "tests/e2e/cypress/integration/065-sso-redirect-loop.spec.js" \
--browser ${{ matrix.browser }}

- name: Run Stripe Tests (After Setup)
id: stripe-tests
env:
Expand Down
143 changes: 57 additions & 86 deletions assets/js/sso.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,46 +1,27 @@
/* global wu_create_cookie, wu_sso_config, wu_read_cookie, detectIncognito */
/* global wu_create_cookie, wu_sso_config, wu_read_cookie */
(function(o) {

window.wu = window.wu || {};
window.wu = window.wu || {};

window.is_incognito = false;
window.wu.sso_denied = function() {

window.wu.sso_denied = function() {
wu_create_cookie('wu_sso_denied', 1, o.expiration_in_days);

wu_create_cookie('wu_sso_denied', 1, o.expiration_in_days);
};

};
const w = document.createElement('script');

window.wu.check_for_incognito_window = function() {
w.type = 'text/javascript';

try {
w.async = true;

detectIncognito(results => window.is_incognito = results.isPrivate);
w.defer = true;

} catch (e) {
w.src = o.server_url + '?_jsonp=1';

// If detectIncognito fails, assume not incognito
window.is_incognito = false;
const denied = wu_read_cookie('wu_sso_denied');

}

}

window.wu.check_for_incognito_window();

const w = document.createElement('script');

w.type = 'text/javascript';

w.async = true;

w.defer = true;

w.src = o.server_url + '?_jsonp=1';

const denied = wu_read_cookie('wu_sso_denied');

document.head.insertAdjacentHTML('beforeend', `
document.head.insertAdjacentHTML('beforeend', `
<style>
@keyframes fade_in {
from { opacity: 0; }
Expand Down Expand Up @@ -78,83 +59,73 @@
</style>
`);

if (! o.is_user_logged_in && ! denied) {
if (! o.is_user_logged_in && ! denied) {

const s = document.getElementsByTagName('script')[0];
const s = document.getElementsByTagName('script')[ 0 ];

s.parentNode.insertBefore(w, s);
s.parentNode.insertBefore(w, s);

document.body.insertAdjacentHTML('beforeend', '<div class="sso-overlay">&nbsp;</div>');
document.body.insertAdjacentHTML('beforeend', '<div class="sso-overlay">&nbsp;</div>');

}
}

window.wu.sso = function(payload) {
window.wu.sso = function(payload) {

const encodedLocation = encodeURIComponent(window.location.href);
const encodedLocation = encodeURIComponent(window.location.href);

if (payload.code === 200) {
if (payload.code === 200) {

if (o.use_overlay) {
if (o.use_overlay) {

document.body.classList.add('sso-loading');
document.body.classList.add('sso-loading');

}

/**
* In case we're dealing with http (without ssl),
* we force a redirect to bypass browser cookie
* limitations.
*
* Otherwise, on the else block,
* we redirect with the verification code attached,
* to perform a regular SSO flow.
*/
if (payload.verify === 'must-redirect') {

window.location.replace(`${ o.server_url }?return_url=${ encodedLocation }`);

} else {
window.location.replace(`${ o.server_url }?sso_verify=${ payload.verify }&return_url=${ encodedLocation }`);
}

} else {
}

/**
* If we are in a incognito window,
* we give it another try with a full redirect,
* as chrome settings might be blocking
* cookies from being sent anyways.
*/
if (window.is_incognito) {
/**
* In case we're dealing with http (without ssl),
* we force a redirect to bypass browser cookie
* limitations.
*
* Otherwise, on the else block,
* we redirect with the verification code attached,
* to perform a regular SSO flow.
*/
if (payload.verify === 'must-redirect') {

if (o.use_overlay) {
window.location.replace(`${ o.server_url }?return_url=${ encodedLocation }`);

document.body.classList.add('sso-loading');
} else {
window.location.replace(`${ o.server_url }?sso_verify=${ payload.verify }&return_url=${ encodedLocation }`);
}

}

window.location.replace(`${o.server_url}?return_url=${ encodedLocation }`);

return;

}
} else {

window.wu.sso_denied();
/**
* SSO failed (user not logged in on main site).
* Set the denied cookie so we don't try again for 5 minutes,
* and remove the loading overlay.
*
* Previously, incognito windows would attempt a full redirect
* as a fallback, but this caused an infinite redirect loop:
* redirect -> sso_verify=invalid -> redirect -> repeat.
* The denied cookie now prevents re-entry in all cases.
*/
window.wu.sso_denied();

document.body.classList.remove('sso-loading');
document.body.classList.remove('sso-loading');

}
}

};
};

(function clean_up_query_args() {
(function clean_up_query_args() {

if (window.history.replaceState) {
if (window.history.replaceState) {

window.history.replaceState(null, null, o.filtered_url + window.location.hash);
window.history.replaceState(null, null, o.filtered_url + window.location.hash);

}
}

}());
}());

}(wu_sso_config));
4 changes: 2 additions & 2 deletions assets/js/sso.min.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
(n=>{window.wu=window.wu||{},window.is_incognito=!1,window.wu.sso_denied=function(){wu_create_cookie("wu_sso_denied",1,n.expiration_in_days)},window.wu.check_for_incognito_window=function(){try{detectIncognito(o=>window.is_incognito=o.isPrivate)}catch(o){window.is_incognito=!1}},window.wu.check_for_incognito_window();var o=document.createElement("script"),e=(o.type="text/javascript",o.async=!0,o.defer=!0,o.src=n.server_url+"?_jsonp=1",wu_read_cookie("wu_sso_denied"));document.head.insertAdjacentHTML("beforeend",`
(n=>{window.wu=window.wu||{},window.wu.sso_denied=function(){wu_create_cookie("wu_sso_denied",1,n.expiration_in_days)};var e=document.createElement("script"),o=(e.type="text/javascript",e.async=!0,e.defer=!0,e.src=n.server_url+"?_jsonp=1",wu_read_cookie("wu_sso_denied"));document.head.insertAdjacentHTML("beforeend",`
<style>
@keyframes fade_in {
from { opacity: 0; }
Expand Down Expand Up @@ -34,4 +34,4 @@
background-size: 20px 20px;
}
</style>
`),n.is_user_logged_in||e||((e=document.getElementsByTagName("script")[0]).parentNode.insertBefore(o,e),document.body.insertAdjacentHTML("beforeend",'<div class="sso-overlay">&nbsp;</div>')),window.wu.sso=function(o){var e=encodeURIComponent(window.location.href);200===o.code?(n.use_overlay&&document.body.classList.add("sso-loading"),"must-redirect"===o.verify?window.location.replace(n.server_url+"?return_url="+e):window.location.replace(`${n.server_url}?sso_verify=${o.verify}&return_url=`+e)):window.is_incognito?(n.use_overlay&&document.body.classList.add("sso-loading"),window.location.replace(n.server_url+"?return_url="+e)):(window.wu.sso_denied(),document.body.classList.remove("sso-loading"))},window.history.replaceState&&window.history.replaceState(null,null,n.filtered_url+window.location.hash)})(wu_sso_config);
`),n.is_user_logged_in||o||((o=document.getElementsByTagName("script")[0]).parentNode.insertBefore(e,o),document.body.insertAdjacentHTML("beforeend",'<div class="sso-overlay">&nbsp;</div>')),window.wu.sso=function(e){var o=encodeURIComponent(window.location.href);200===e.code?(n.use_overlay&&document.body.classList.add("sso-loading"),"must-redirect"===e.verify?window.location.replace(n.server_url+"?return_url="+o):window.location.replace(`${n.server_url}?sso_verify=${e.verify}&return_url=`+o)):(window.wu.sso_denied(),document.body.classList.remove("sso-loading"))},window.history.replaceState&&window.history.replaceState(null,null,n.filtered_url+window.location.hash)})(wu_sso_config);
49 changes: 33 additions & 16 deletions inc/sso/class-sso.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,22 +543,41 @@ public function handle_broker($response_type = 'redirect'): void {

// Attach through redirect if the client isn't attached yet.
if ( ! $broker->isAttached()) {
$return_url = $this->get_current_url();

if ( 'jsonp' === $response_type) {
$attach_url = $broker->getAttachUrl(
[
'_jsonp' => '1',
]
);
} else {
$attach_url = $broker->getAttachUrl(
[
'return_url' => $return_url,
]
/*
* For JSONP requests (initiated by a <script> tag), we must NOT
* redirect — the browser follows 302s transparently for script
* tags, but the final response from the server will be a redirect
* (not JavaScript), so the wu.sso() callback never fires.
* Instead, return a JSONP error so the JS can handle it gracefully
* and set the wu_sso_denied cookie to prevent further attempts.
*/
if ('jsonp' === $response_type) {
header('Content-Type: application/javascript; charset=utf-8');

printf(
'wu.sso(%s, %d);',
wp_json_encode(
[
'code' => 0,
'message' => 'Broker not attached',
]
),
200
);

status_header(200);

exit;
Comment on lines 545 to +570
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Inspect broker attachment state:"
rg -n -C4 'class\s+SSO_Broker|function\s+isAttached\s*\(|function\s+getAttachUrl\s*\(' --glob '*.php'

echo
echo "Inspect the JSONP entrypoint and the unattached broker branch:"
rg -n -C4 '_jsonp=1|isAttached\(\)|getAttachUrl\(|sso_verify|wu\.sso\(' inc/sso/class-sso.php assets/js/sso.js

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 6504

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Find the parent Broker class:"
find . -type f -name '*.php' | xargs rg -l 'class Broker' | head -20

echo
echo "Search for Broker class definition:"
rg -n -A 20 'class Broker' --glob '*.php' | grep -A 20 'class Broker [^a-zA-Z_]'

echo
echo "Find isAttached implementation in parent class:"
rg -n -B 2 -A 10 'function isAttached' --glob '*.php'

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 112

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Search for Broker class references and imports:"
rg -n 'Broker|extends Broker|use.*Broker' inc/sso/class-sso-broker.php

echo
echo "Check composer.json for dependencies:"
cat composer.json 2>/dev/null | head -50 || echo "No composer.json at root"

echo
echo "Look for vendor directories or external class files:"
find . -type f -name '*.php' -path '*/vendor/*' 2>/dev/null | grep -i broker | head -10

echo
echo "Broader search for isAttached definition anywhere:"
rg -n 'function isAttached|isAttached.*=' --glob '*.php' -A 5

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 1863

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Search vendor directory for Jasny Broker:"
find vendor -type f -name '*.php' -path '*Jasny*Broker*' 2>/dev/null | head -20

echo
echo "Look for get_broker() method to understand initialization:"
rg -n -A 15 'function get_broker|private.*broker' inc/sso/class-sso.php

echo
echo "Check if there's any session state setup before line 545:"
sed -n '500,545p' inc/sso/class-sso.php | cat -n

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 122

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Find get_broker() method definition:"
rg -n -A 20 'function get_broker' inc/sso/class-sso.php

echo
echo "Show context around line 545 (the isAttached check):"
sed -n '510,580p' inc/sso/class-sso.php | cat -n

echo
echo "Search for where broker attachment is set or initialized:"
rg -n 'setAttached|attach|token|session' inc/sso/class-sso.php | head -30

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 4156

🌐 Web query:

Jasny SSO Broker isAttached() session state fresh request

💡 Result:

In jasny/sso the broker’s isAttached() does not make a “fresh” call to the SSO server and it does not validate the remote server-side session on each request. It only checks whether the broker has a stored verification code in its client-side state (cookie/session). Concretely, isAttached() returns true iff getVerificationCode() !== null. [1]

What that means for “fresh request / session state”:

  • On a new HTTP request (new PHP process), you typically create a new Broker(...). When you call isAttached(), it will read state from the configured state handler (default: cookies) and decide based on whether the sso_verify_{broker} value exists. [1][2]
  • That sso_verify value is only stored after you complete the attach + verify flow (i.e., after redirect back with ?sso_verify=..., you must call $broker->verify($_GET['sso_verify']);, which persists it into state). [1][2]
  • If you want to force a re-attach (i.e., treat every request as “fresh”), clear the stored state (clearToken() clears both token and verify state) and redirect to getAttachUrl() again. [1]

Common reason isAttached() is “false every request”: the verify/token cookies aren’t being persisted/sent back (cookie domain/path/secure/SameSite issues), or you switched to session storage but didn’t session_start() before using withTokenIn(new Session()). [2]

Sources:
[1] jasny/sso Broker::isAttached() / verify() implementation (Scrutinizer)
[2] jasny/sso README usage showing isAttached(), attach redirect, and verify() (Packagist)

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Check what JS does with JSONP error (code 0):"
sed -n '1,120p' assets/js/sso.js | cat -n

echo
echo "Look for wu.sso_denied definition and what it does:"
rg -n -A 10 'sso_denied|wu\.sso_denied' assets/js/sso.js

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 4762

The unattached JSONP branch blocks the entire SSO attach flow for fresh browser sessions.

When isAttached() returns false (the expected state for a fresh session), the JSONP endpoint returns error code 0 instead of initiating the attach handshake. The JavaScript receives this error, calls wu.sso_denied() which sets a denial cookie, and the normal attach flow at lines 573–579 ($broker->getAttachUrl()) is never reached for JSONP requests. This prevents SSO from working at all for new users.

The Jasny SSO Broker::isAttached() checks only whether a stored verification code exists in session/cookies. On the first request, this is always false, so the JSONP endpoint must either initiate the attach redirect or provide another path to attachment—not deny it outright.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@inc/sso/class-sso.php` around lines 545 - 570, The JSONP branch currently
treats $broker->isAttached() === false as a final error and returns wu.sso(...
code:0 ...) which causes wu.sso_denied() and sets the wu_sso_denied cookie,
breaking first-time attachment; change the JSONP handling so that when
response_type === 'jsonp' and the broker is not attached you do not emit a
denial but instead initiate the attach flow: call $broker->getAttachUrl() (or
otherwise build the attach URL) and return a JSONP payload that instructs the
client JS to start the attach handshake (e.g., provide an "attach_url" and
success code or a specific action field) so the front-end can open/redirect to
the attach URL rather than calling wu.sso_denied(); ensure you do not set the
denial cookie in this path and keep status_header(200).

}

$return_url = $this->get_current_url();

$attach_url = $broker->getAttachUrl(
[
'return_url' => $return_url,
]
);

wp_safe_redirect($attach_url, 302, 'WP-Ultimo-SSO');

exit();
Expand Down Expand Up @@ -743,9 +762,7 @@ public function enqueue_script(): void {
return;
}

wp_register_script('wu-detect-incognito', wu_get_asset('detectincognito.js', 'js/lib'), false, wu_get_version(), true);

wp_register_script('wu-sso', wu_get_asset('sso.js', 'js'), ['wu-cookie-helpers', 'wu-detect-incognito'], wu_get_version(), true);
wp_register_script('wu-sso', wu_get_asset('sso.js', 'js'), ['wu-cookie-helpers'], wu_get_version(), true);

$sso_path = $this->get_url_path();

Expand Down
Loading
Loading