Skip to content

quality-debt: inc/sso/class-sso.php — PR #1084 review feedback (high) #1104

Description

@superdav42

Unactioned Review Feedback

Source PR: #1084
File: inc/sso/class-sso.php
Reviewers: coderabbit
Findings: 2
Max severity: high


HIGH: coderabbit (coderabbitai[bot])

File: inc/sso/class-sso.php:649
⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Hardcoded 'sso' parameter name bypasses the wu_sso_get_url_path filter

Every other call site in this file retrieves the SSO URL path via $this->get_url_path(), which wraps apply_filters('wu_sso_get_url_path', 'sso', ...). Using a raw 'sso' literal here means that if wu_sso_get_url_path is filtered to a different value, $sso_action will always be empty and this branch of the SSO detection silently degrades to relying solely on return_url.

🐛 Proposed fix
-		$sso_action = $this->input('sso', '');
+		$sso_action = $this->input($this->get_url_path(), '');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

		$sso_action = $this->input($this->get_url_path(), '');
🤖 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 649, The code uses a hardcoded 'sso' literal
when calling $this->input which bypasses the wu_sso_get_url_path filter; change
the call that sets $sso_action to use $this->get_url_path() instead of the
literal so the filtered URL path is respected (i.e. replace $this->input('sso',
'') with $this->input($this->get_url_path(), '')); ensure this change is applied
wherever $sso_action is set so apply_filters('wu_sso_get_url_path', 'sso', ...)
can take effect.

View comment


HIGH: coderabbit (coderabbitai[bot])

File: inc/sso/class-sso.php:660
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Missing URL sanitization on the extracted return_url

$query_params['return_url'] is derived from user-controlled redirect_to input via parse_str but is assigned without any sanitization. Per coding guidelines, wu_clean() or a WordPress sanitization function must be used. For a value treated as a URL, esc_url_raw() is the right choice.

🛡️ Proposed fix
-					$return_url = $query_params['return_url'];
+					$return_url = esc_url_raw($query_params['return_url']);

As per coding guidelines: "Use wu_clean() or WordPress sanitization functions for input sanitization".

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

						$return_url = esc_url_raw($query_params['return_url']);
🤖 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 660, The extracted $query_params['return_url']
is assigned to $return_url without sanitization; update the code in the SSO
handling (where parse_str populates $query_params) to sanitize the value before
assignment by running it through a URL sanitizer (use esc_url_raw() per
guidelines or wu_clean() as an alternative) and ensure you check
isset($query_params['return_url']) and provide a safe fallback if missing/empty.

View comment



Auto-generated by quality-feedback-helper.sh scan-merged. Review each finding and either fix the code or dismiss with a reason.


aidevops.sh v3.14.59 automated scan.

Metadata

Metadata

Assignees

Labels

origin:workerAuto-created by pulse labelless backfill (t2112)priority:highHigh severity — significant quality issuequality-debtUnactioned review feedback from merged PRssource:review-feedbackAuto-created by quality-feedback-helper.shstatus:in-reviewPR open, awaiting review/merge

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions