-
-
Notifications
You must be signed in to change notification settings - Fork 79
fix: cookie-less cross-domain SSO login flow #1084
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -645,10 +645,26 @@ public function handle_already_logged_in_on_login_page(): void { | |||||
| return; | ||||||
| } | ||||||
|
|
||||||
| // Check if this is an SSO flow (return_url param present) | ||||||
| // Check if this is an SSO flow (sso param or return_url param present) | ||||||
| $sso_action = $this->input('sso', ''); | ||||||
| $return_url = $this->input('return_url', ''); | ||||||
|
|
||||||
| // Also extract return_url from redirect_to if present | ||||||
| if ( empty($return_url) ) { | ||||||
| $redirect_to = $this->input('redirect_to', ''); | ||||||
| if ( $redirect_to ) { | ||||||
| $parsed = wp_parse_url($redirect_to, PHP_URL_QUERY); | ||||||
| if ( $parsed ) { | ||||||
| parse_str($parsed, $query_params); | ||||||
| if ( ! empty($query_params['return_url']) ) { | ||||||
| $return_url = $query_params['return_url']; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing URL sanitization on the extracted
🛡️ Proposed fix- $return_url = $query_params['return_url'];
+ $return_url = esc_url_raw($query_params['return_url']);As per coding guidelines: "Use 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Check for SSO flow - either sso param or return_url pointing to different domain | ||||||
| if ( empty($sso_action) && empty($return_url) ) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoded
'sso'parameter name bypasses thewu_sso_get_url_pathfilterEvery other call site in this file retrieves the SSO URL path via
$this->get_url_path(), which wrapsapply_filters('wu_sso_get_url_path', 'sso', ...). Using a raw'sso'literal here means that ifwu_sso_get_url_pathis filtered to a different value,$sso_actionwill always be empty and this branch of the SSO detection silently degrades to relying solely onreturn_url.🐛 Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents