Skip to content

fix: harden cookie-less SSO token redirects#1086

Merged
superdav42 merged 3 commits into
mainfrom
fix/harden-cookie-less-sso
May 4, 2026
Merged

fix: harden cookie-less SSO token redirects#1086
superdav42 merged 3 commits into
mainfrom
fix/harden-cookie-less-sso

Conversation

@superdav42

@superdav42 superdav42 commented May 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Moves cookie-less SSO token consumption into Ultimate Multisite so local mapped subsites can authenticate without the multi-tenancy setting.
  • Handles custom /login/ pages via template_redirect, preserves direct cross-domain redirect_to admin URLs, and decorates cross-domain redirects with wu_sso_token.
  • Hardens tokens with audience host binding and one-time jti transients.

Verification

  • php -l inc/sso/class-sso.php
  • cd site && composer dump-autoload --optimize --no-scripts
  • Manual token redirect check: generated a token for vidanuevanaz.org and confirmed HTTP/2 302 to /wp/wp-admin/ with WordPress auth cookies set for .vidanuevanaz.org.

Notes

Browser-agent navigation to the local TLS domains currently fails with net::ERR_SSL_PROTOCOL_ERROR even after launching a custom headless Chrome with Caddy CA trust and host resolver rules. Curl verifies the TLS endpoints and token flow locally.


aidevops.sh v3.14.51 plugin for OpenCode v1.14.33 with claude-sonnet-4-6 spent 16h 4m on this as a headless worker.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved Single Sign-On redirect handling to ensure users are directed to the correct destination across multiple domains after authentication.
    • Fixed redirect behavior for already-logged-in users visiting the login page.
    • Enhanced token-based authentication flow to properly preserve redirect destinations.

When user is already logged in on main site and visits login page
with SSO params, redirect them directly to the subsite with a
verification token instead of showing 'already logged in' message.

- Check for 'sso' param in addition to 'return_url'
- Extract return_url from redirect_to query params if present
- Handle WP_Error user object in handle_login_redirect
@coderabbitai

coderabbitai Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

SSO redirect handling now uses a centralized get_sso_redirect_to() helper to compute correct cross-domain redirect_to values. Multiple handlers were updated to derive redirect destinations from this helper rather than raw request values, ensuring users return to intended destinations after SSO token consumption.

Changes

SSO Redirect Logic

Layer / File(s) Summary
New Centralization Helper
inc/sso/class-sso.php (lines 659–679)
New private get_sso_redirect_to($return_url) helper enforces cross-domain rules: honors explicit cross-domain redirect_to from request; otherwise, if return_url is cross-domain, forces /wp/wp-admin/ target; else falls back to admin_url().
Auth Redirect Handler
inc/sso/class-sso.php (lines 398–404)
handle_auth_redirect() now constructs login URL with explicit redirect_to query argument (derived from $redirect_after or admin_url()) rather than passing $redirect_after directly to wp_login_url().
Main Site Logged-In Handler
inc/sso/class-sso.php (lines 519–522)
handle_main_site_logged_in_user() derives redirect_to via get_sso_redirect_to($return_url) instead of raw request value.
Login Redirect Handler
inc/sso/class-sso.php (lines 568–573)
handle_login_redirect() computes redirect_to via get_sso_redirect_to($return_url) and injects it into the cookie-less token URL via add_query_arg().
Already Logged-In Handler
inc/sso/class-sso.php (line 824)
handle_already_logged_in_on_login_page() appends redirect_to (computed via get_sso_redirect_to($return_url)) to the cookie-less redirect URL.
Code Formatting
inc/sso/class-sso.php (line 736)
Minor alignment change in validate_sso_token() $hmac assignment (no logic change).

Sequence Diagram

sequenceDiagram
    actor User
    participant Client as Browser/<br>Client
    participant SSO as SSO Handler
    participant Auth as WordPress<br>Auth
    participant Helper as get_sso_redirect_to()

    User->>Client: Visit cross-domain resource<br/>(return_url + redirect_to)
    Client->>SSO: Request with return_url param
    
    alt User logged in on main site
        SSO->>Helper: get_sso_redirect_to(return_url)
        Helper-->>SSO: Resolved redirect_to<br/>(cross-domain aware)
        SSO->>Client: Redirect to login with<br/>redirect_to param
    else User not logged in
        SSO->>Auth: Build login URL
        Auth-->>SSO: Login URL ready
        SSO->>Client: Redirect to login
    end
    
    Client->>Auth: Submit credentials
    Auth->>SSO: handle_login_redirect()
    SSO->>Helper: Compute redirect_to
    Helper-->>SSO: Final redirect_to value
    SSO->>Client: Return cookie-less token URL<br/>+ redirect_to param
    Client->>SSO: Consume token via<br/>redirect_to destination
    SSO->>User: Redirect to intended<br/>destination (admin, etc)
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly Related PRs

Poem

🐰 A rabbit hops through domains with grace,
Redirects now land in the right place!
No more lost tokens or wayward streams,
Cross-domain SSO achieves its dreams.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: harden cookie-less SSO token redirects' directly reflects the core change: improving security of SSO token redirects by binding audience to host and using one-time jti transients, with proper cross-domain redirect_to handling.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/harden-cookie-less-sso

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@superdav42

Copy link
Copy Markdown
Collaborator Author

DISPATCH_CLAIM nonce=c97c6395a5a24215946ab950ab65f8cd runner=superdav42 ts=2026-05-04T21:02:16Z max_age_s=1800 version=3.14.51 opencode_version=1.14.33

@superdav42 superdav42 added status:queued Worker dispatched, not yet started origin:worker Auto-created by pulse labelless backfill (t2112) labels May 4, 2026
@superdav42 superdav42 self-assigned this May 4, 2026
@superdav42

Copy link
Copy Markdown
Collaborator Author

Dispatching worker (deterministic).

@superdav42

Copy link
Copy Markdown
Collaborator Author

Maintainer review needed: this PR is DIRTY (merge conflicts with the default branch) and does not meet the auto-rebase or auto-close criteria.

Reason: dirty-not-auto-resolvable

Options:

  • Rebase manually: git fetch origin && git rebase origin/$(git symbolic-ref --short refs/remotes/origin/HEAD | sed 's|origin/||') (or --strategy-option=union when TODO.md is the culprit).
  • Close as superseded: gh pr close 1086 --delete-branch.
  • Opt out of future sweeps: add the do-not-close label.

This comment is posted once per cooldown window (1800s) so the sweep stays quiet.

Triggered by pulse-dirty-pr-sweep.sh (t2350 / GH#19948).


aidevops.sh v3.14.51 automated scan.

@superdav42

Copy link
Copy Markdown
Collaborator Author

CLAIM_RELEASED reason=clean runner=dave ts=2026-05-04T21:10:17Z aidevops_version=3.14.51 opencode_version=1.14.33 exit=0 session_count=1

@superdav42 superdav42 added status:available Task is available for claiming and removed status:queued Worker dispatched, not yet started labels May 4, 2026
@superdav42

Copy link
Copy Markdown
Collaborator Author

DISPATCH_CLAIM nonce=2831b9c002a64964bcfcc8521add0f69 runner=superdav42 ts=2026-05-04T21:10:54Z max_age_s=1800 version=3.14.51 opencode_version=1.14.33

@superdav42 superdav42 added status:queued Worker dispatched, not yet started and removed status:available Task is available for claiming labels May 4, 2026
@superdav42

Copy link
Copy Markdown
Collaborator Author

Dispatching worker (deterministic).

@superdav42

Copy link
Copy Markdown
Collaborator Author

CLAIM_RELEASED reason=clean runner=dave ts=2026-05-04T21:16:03Z aidevops_version=3.14.51 opencode_version=1.14.33 exit=0 session_count=1

@superdav42 superdav42 added status:available Task is available for claiming and removed status:queued Worker dispatched, not yet started labels May 4, 2026
@superdav42

Copy link
Copy Markdown
Collaborator Author

ORPHAN_REATTACHED reason=pulse_restart worker_pid=1192512 ts=2026-05-04T21:18:14Z

Worker PID 1192512 survived a previous pulse-wrapper.sh restart. It has been reattached to the new pulse instance's dispatch ledger and will continue under normal watchdog supervision. No action needed.

@superdav42

Copy link
Copy Markdown
Collaborator Author

DISPATCH_CLAIM nonce=5117614f0a1df4c723b1772b0db7a1f8 runner=superdav42 ts=2026-05-04T21:21:07Z max_age_s=1800 version=3.14.51 opencode_version=1.14.33

@superdav42 superdav42 added status:queued Worker dispatched, not yet started and removed status:available Task is available for claiming labels May 4, 2026
@superdav42

Copy link
Copy Markdown
Collaborator Author

Dispatching worker (deterministic).

@superdav42

Copy link
Copy Markdown
Collaborator Author

CLAIM_RELEASED reason=clean runner=dave ts=2026-05-04T21:26:21Z aidevops_version=3.14.51 opencode_version=1.14.33 exit=0 session_count=1

@superdav42 superdav42 added status:available Task is available for claiming and removed status:queued Worker dispatched, not yet started labels May 4, 2026
@superdav42

Copy link
Copy Markdown
Collaborator Author

ORPHAN_REATTACHED reason=pulse_restart worker_pid=1263562 ts=2026-05-04T21:28:36Z

Worker PID 1263562 survived a previous pulse-wrapper.sh restart. It has been reattached to the new pulse instance's dispatch ledger and will continue under normal watchdog supervision. No action needed.

@superdav42

Copy link
Copy Markdown
Collaborator Author

DISPATCH_CLAIM nonce=aa6e37b2cb867e8b17b6b2bb096d9226 runner=superdav42 ts=2026-05-04T21:31:12Z max_age_s=1800 version=3.14.51 opencode_version=1.14.33

@superdav42 superdav42 added status:queued Worker dispatched, not yet started and removed status:available Task is available for claiming labels May 4, 2026
@superdav42

Copy link
Copy Markdown
Collaborator Author

Dispatching worker (deterministic).

@superdav42

Copy link
Copy Markdown
Collaborator Author

CLAIM_RELEASED reason=clean runner=dave ts=2026-05-04T21:36:55Z aidevops_version=3.14.51 opencode_version=1.14.33 exit=0 session_count=1

@superdav42 superdav42 added status:available Task is available for claiming and removed status:queued Worker dispatched, not yet started labels May 4, 2026
Resolves conflicts with #1088 which landed an overlapping cookie-less SSO
hardening on main. Kept this branch's redirect_to wiring and the
get_sso_redirect_to() helper so the originally requested admin URL
survives the cross-domain bounce instead of always landing on /wp-admin/.

Conflicts:
- inc/sso/class-sso.php (5 conflict regions, all in cookie-less SSO logic)
@superdav42

Copy link
Copy Markdown
Collaborator Author

Conflict resolution

Rebased/merged main onto this branch to clear the 5 conflicts in inc/sso/class-sso.php. The conflicts came from #1088 (David Stone, merged a few hours ago) which landed an overlapping cookie-less SSO hardening with the same goals: HMAC + audience binding + one-time jti.

What I kept from this branch (vs. main / #1088)

This branch retains a small but functional delta over main:

  1. get_sso_redirect_to() helper — resolves the final post-auth landing URL: prefers a cross-domain redirect_to from the request, otherwise derives <return_url>/wp/wp-admin/, otherwise falls back to admin_url().
  2. redirect_to is propagated as a separate query arg on the cross-domain bounceadd_query_arg('redirect_to', $redirect_to, $url) after add_cookie_less_sso_token() in:
    • handle_main_site_logged_in_user()
    • handle_login_redirect()
    • handle_already_logged_in_on_login_page()
  3. Login redirect uses wp_login_url() + explicit redirect_to query arg instead of wp_login_url($redirect_after), so the value survives at the top level rather than nested-encoded inside wp_login_url's redirect_to.

The reason this matters: handle_cookie_less_sso_token() reads $this->input('redirect_to', admin_url()) on the target site to decide where to land the user post-auth. Without (1)+(2), the user always lands on the subsite's /wp/wp-admin/ regardless of what they originally requested (e.g. /wp-admin/edit.php?post=123).

Verification

  • php -l inc/sso/class-sso.php — no syntax errors
  • vendor/bin/phpstan analyse inc/sso/class-sso.php --memory-limit=1G — no errors
  • vendor/bin/phpunit --filter SSO — 316 tests, 1 error / 13 failures; identical baseline on main, so this merge introduces no test regressions
  • vendor/bin/phpcs — pre-existing environment issue (configured WordPress.Arrays.ArrayDeclaration.* sniffs unavailable in this PHPCS version), unrelated to this change

Reviewer question

Given #1088 already shipped the core hardening (HMAC + aud + jti), do you still want this PR's redirect_to propagation on top? If yes, this branch is ready to merge. If no, this can be closed — the security hardening is already on main.


aidevops.sh v3.14.51 plugin for OpenCode v1.14.33 with claude-sonnet-4-6 spent 16h 48m on this as a headless worker.

@superdav42

Copy link
Copy Markdown
Collaborator Author

ORPHAN_REATTACHED reason=pulse_restart worker_pid=1329002 ts=2026-05-04T21:39:13Z

Worker PID 1329002 survived a previous pulse-wrapper.sh restart. It has been reattached to the new pulse instance's dispatch ledger and will continue under normal watchdog supervision. No action needed.

@superdav42 superdav42 added the consolidation-in-progress Another runner is creating a consolidation child issue (cross-runner advisory lock) label May 4, 2026
@superdav42 superdav42 added needs-consolidation Issue held from dispatch pending comment consolidation and removed consolidation-in-progress Another runner is creating a consolidation child issue (cross-runner advisory lock) labels May 4, 2026
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

🔨 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!
Playground support for multisite is very limitied, hopefully it will get better in the future.

🚀 Launch in Playground

Login credentials: admin / password

@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

Performance Test Results

Performance test results for c749156 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: /

Run DB Queries Memory Before Template Template WP Total LCP TTFB LCP - TTFB
0 41 (+1 / +2% ) 37.78 MB 809.50 ms (-39.50 ms / -5% ) 173.00 ms (+15.00 ms / +9% ) 1046.50 ms (-25.00 ms / -2% ) 1972.00 ms 1885.20 ms 88.40 ms
1 56 49.10 MB 929.50 ms (+19.50 ms / +2% ) 145.00 ms (-4.00 ms / -3% ) 1076.50 ms 2052.00 ms 1974.10 ms 80.55 ms

@superdav42 superdav42 merged commit 9b01184 into main May 4, 2026
9 of 11 checks passed
@superdav42

Copy link
Copy Markdown
Collaborator Author

Worker failed: orphan worktree detected (crash_type=no_work, 0 commits). Cleared for re-dispatch.


aidevops.sh v3.14.52 automated scan.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-consolidation Issue held from dispatch pending comment consolidation origin:worker Auto-created by pulse labelless backfill (t2112) status:available Task is available for claiming

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant