Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<p align="center">
<a href="https://github.com/superdav42/wp-multisite-waas/actions/workflows/tests.yml"><img src="https://github.com/superdav42/wp-multisite-waas/actions/workflows/tests.yml/badge.svg" alt="Unit & Integration Tests"></a>
<a href="https://github.com/superdav42/wp-multisite-waas/actions/workflows/e2e.yml"><img src="https://github.com/superdav42/wp-multisite-waas/actions/workflows/e2e.yml/badge.svg" alt="E2E Tests"></a>
<a href="https://img.shields.io/coderabbit/prs/github/superdav42/wp-multisite-waas?utm_source=oss&utm_medium=github&utm_campaign=superdav42%2Fwp-multisite-waas&labelColor=171717&color=FF570A&link=https%3A%2F%2Fcoderabbit.ai&label=CodeRabbit+Reviews"><img src="https://img.shields.io/coderabbit/prs/github/superdav42/wp-multisite-waas?utm_source=oss&utm_medium=github&utm_campaign=superdav42%2Fwp-multisite-waas&labelColor=171717&color=FF570A&link=https%3A%2F%2Fcoderabbit.ai&label=CodeRabbit+Reviews" alt="E2E Tests"></a>
<a href="https://img.shields.io/coderabbit/prs/github/Multisite-Ultimate/multisite-ultimate?utm_source=oss&utm_medium=github&utm_campaign=Multisite-Ultimate%2Fmultisite-ultimate&labelColor=171717&color=FF570A&link=https%3A%2F%2Fcoderabbit.ai&label=CodeRabbit+Reviews"><img src="https://img.shields.io/coderabbit/prs/github/Multisite-Ultimate/multisite-ultimate?utm_source=oss&utm_medium=github&utm_campaign=Multisite-Ultimate%2Fmultisite-ultimate&labelColor=171717&color=FF570A&link=https%3A%2F%2Fcoderabbit.ai&label=CodeRabbit+Reviews" alt="Code Rabbit"></a>
</p>

## 🌟 Overview
Expand Down
2 changes: 1 addition & 1 deletion assets/js/domain-logs.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
return_ascii: 'no',
},
success(response) {
$('#content').html(response.data.contents);
$('#content').text(response.data.contents);

if (typeof callback !== 'undefined') {
callback();
Expand Down
20 changes: 20 additions & 0 deletions inc/class-domain-mapping.php
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ public function startup(): void {
*/
add_filter('pre_get_site_by_path', [$this, 'check_domain_mapping'], 10, 2);

add_action('ms_site_not_found', [$this, 'verify_dns_mapping'], 5, 3);

/*
* When a site gets delete, clean up the mapped domains
*/
Expand Down Expand Up @@ -240,6 +242,24 @@ public function get_www_and_nowww_versions($domain) {
return [$nowww, $www];
}

public function verify_dns_mapping($current_site, $domain, $path) {

// Nonce functions are unavailable and the wp_hash is basically the same.
if (isset($_REQUEST['async_check_dns_nonce'])) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended
// This is very early in the request we need these to use wp_hash.
require_once ABSPATH . WPINC . '/l10n.php';
require_once ABSPATH . WPINC . '/pluggable.php';
if (hash_equals(wp_hash($domain), $_REQUEST['async_check_dns_nonce'])) { // phpcs:ignore WordPress.Security.NonceVerification.Recommended, WordPress.Security.ValidatedSanitizedInput.MissingUnslash
$domains = $this->get_www_and_nowww_versions($domain);

$mapping = Domain::get_by_domain($domains);
if ($mapping) {
wp_send_json($mapping->to_array());
}
}
}
}

/**
* Checks if we have a site associated with the domain being accessed
*
Expand Down
6 changes: 5 additions & 1 deletion inc/limits/class-customer-user-role-limits.php
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ public function update_site_user_roles($membership_id): void {
$role = $membership->get_limitations()->customer_user_role->get_limit();

foreach ($sites as $site) {
add_user_to_blog($site->get_id(), $customer->get_user_id(), $role);
// only add user to blog if they are not already a member, or we are downgrading their role.
// Without this check the user could lose additional roles added manually or with hooks.
if ('administrator' !== $role || ! is_user_member_of_blog($customer->get_user_id(), $site->get_id())) {
add_user_to_blog($site->get_id(), $customer->get_user_id(), $role);
}
Comment on lines +138 to +142
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid role update when customer has no WP user_id.

Prevent accidental calls with a falsy user_id; minor perf win by not repeating get_user_id().

Apply this diff:

- // only add user to blog if they are not already a member, or we are downgrading their role.
- // Without this check the user could lose additional roles added manually or with hooks.
- if ('administrator' !== $role || ! is_user_member_of_blog($customer->get_user_id(), $site->get_id())) {
-     add_user_to_blog($site->get_id(), $customer->get_user_id(), $role);
- }
+ // only add user to blog if they are not already a member, or we are downgrading their role.
+ // Without this check the user could lose additional roles added manually or with hooks.
+ $uid = (int) $customer->get_user_id();
+ if ($uid && ('administrator' !== $role || ! is_user_member_of_blog($uid, $site->get_id()))) {
+     add_user_to_blog($site->get_id(), $uid, $role);
+ }
📝 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.

Suggested change
// only add user to blog if they are not already a member, or we are downgrading their role.
// Without this check the user could lose additional roles added manually or with hooks.
if ('administrator' !== $role || ! is_user_member_of_blog($customer->get_user_id(), $site->get_id())) {
add_user_to_blog($site->get_id(), $customer->get_user_id(), $role);
}
// only add user to blog if they are not already a member, or we are downgrading their role.
// Without this check the user could lose additional roles added manually or with hooks.
$uid = (int) $customer->get_user_id();
if ( $uid && ( 'administrator' !== $role || ! is_user_member_of_blog( $uid, $site->get_id() ) ) ) {
add_user_to_blog( $site->get_id(), $uid, $role );
}
🤖 Prompt for AI Agents
In inc/limits/class-customer-user-role-limits.php around lines 138 to 142, avoid
calling add_user_to_blog or is_user_member_of_blog when the customer has no WP
user_id and avoid repeated get_user_id() calls: retrieve the customer user_id
once into a local variable, return/skip early if it's falsy, then perform the
existing check using that cached user_id and only call add_user_to_blog when
appropriate.

}
}
}
Expand Down
143 changes: 143 additions & 0 deletions inc/managers/class-domain-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use Psr\Log\LogLevel;
use WP_Ultimo\Domain_Mapping\Helper;
use WP_Ultimo\Models\Domain;

// Exit if accessed directly
defined('ABSPATH') || exit;
Expand Down Expand Up @@ -904,4 +905,146 @@ public function load_integrations(): void {
*/
do_action('wp_ultimo_host_providers_load');
}

/**
* Verify domain ownership using a loopback request.
*
* This method attempts to verify a domain by making an loopback request with a
* a specific parameter that is used by Domain_Mapper::verify_dns_mapping().
*
* @since 2.4.4
*
* @param Domain $domain The domain object to verify.
* @return bool True if verification succeeds, false otherwise.
*/
public function verify_domain_with_loopback_request(Domain $domain): bool {

$domain_url = $domain->get_domain();
$domain_id = $domain->get_id();

$endpoint_path = '/';

// Test protocols in order of preference: HTTPS with SSL verify, HTTPS without SSL verify, HTTP
$protocols_to_test = [
[
'url' => "https://{$domain_url}{$endpoint_path}",
/** This filter is documented in wp-includes/class-wp-http-streams.php */
'sslverify' => apply_filters('https_local_ssl_verify', false),
'label' => 'HTTPS with SSL verification',
],
[
'url' => "http://{$domain_url}{$endpoint_path}",
'label' => 'HTTP',
],
];

foreach ($protocols_to_test as $protocol_config) {
wu_log_add(
"domain-{$domain_url}",
sprintf(
/* translators: %1$s: Protocol label (HTTPS with SSL verification, HTTPS without SSL verification, HTTP), %2$s: URL being tested */
__('Testing domain verification via Loopback using %1$s: %2$s', 'multisite-ultimate'),
$protocol_config['label'],
$protocol_config['url']
)
);

// Make API request with basic auth
$response = wp_remote_get(
$protocol_config['url'],
[
'timeout' => 10,
'redirection' => 0,
'sslverify' => $protocol_config['sslverify'],
'body' => ['async_check_dns_nonce' => wp_hash($domain_url)],
]
);

// Check for connection errors
if (is_wp_error($response)) {
wu_log_add(
"domain-{$domain_url}",
sprintf(
/* translators: %1$s: Protocol label (HTTPS with SSL verification, HTTPS without SSL verification, HTTP), %2$s: Error Message */
__('Failed to connect via %1$s: %2$s', 'multisite-ultimate'),
$protocol_config['label'],
$response->get_error_message()
),
LogLevel::WARNING
);
continue;
}

$response_code = wp_remote_retrieve_response_code($response);
$body = wp_remote_retrieve_body($response);

// Check HTTP status
if (200 !== $response_code) {
wu_log_add(
"domain-{$domain_url}",
sprintf(
/* translators: %1$s: Protocol label (HTTPS with SSL verification, HTTPS without SSL verification, HTTP), %2$s: HTTP Response Code */
__('Loopback request via %1$s returned HTTP %2$d', 'multisite-ultimate'),
$protocol_config['label'],
$response_code
),
LogLevel::WARNING
);
continue;
}

// Try to decode JSON response
$data = json_decode($body, true);

if (json_last_error() !== JSON_ERROR_NONE) {
wu_log_add(
"domain-{$domain_url}",
sprintf(
/* translators: %1$s: Protocol label (HTTPS with SSL verification, HTTPS without SSL verification, HTTP), %2$s: Json error, %3$s part of the response */
__('Loopback response via %1$s is not valid JSON: %2$s : %3$s', 'multisite-ultimate'),
$protocol_config['label'],
json_last_error_msg(),
substr($body, 0, 100)
),
LogLevel::WARNING
);
continue;
}

// Check if we got a valid domain object back
if (isset($data['id']) && (int) $data['id'] === $domain_id) {
wu_log_add(
"domain-{$domain_url}",
sprintf(
/* translators: %1$s: Protocol label (HTTPS with SSL verification, HTTPS without SSL verification, HTTP), %2$s: Domain ID number */
__('Domain verification successful via Loopback using %1$s. Domain ID %2$d confirmed.', 'multisite-ultimate'),
$protocol_config['label'],
$domain_id
)
);

return true;
}

wu_log_add(
"domain-{$domain_url}",
sprintf(
/* translators: %1$s: Protocol label (HTTPS with SSL verification, HTTPS without SSL verification, HTTP), %2$s: Domain ID number, %3$s Domain ID number */
__('Loopback response via %1$s did not contain expected domain ID. Expected: %2$d, Got: %3$s', 'multisite-ultimate'),
$protocol_config['label'],
$domain_id,
isset($data['id']) ? $data['id'] : 'null'
),
LogLevel::WARNING
);
}

wu_log_add(
"domain-{$domain_url}",
__('Domain verification failed via loopback on all protocols (HTTPS with SSL, HTTPS without SSL, HTTP).', 'multisite-ultimate'),
LogLevel::ERROR
);

return false;
}
}
10 changes: 8 additions & 2 deletions inc/models/class-domain.php
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,12 @@ public function has_correct_dns() {

$domain_url = $this->get_domain();

$domain_manager = \WP_Ultimo\Managers\Domain_Manager::get_instance();

if ($domain_manager->verify_domain_with_loopback_request($this)) {
return true;
}

Comment on lines +400 to +405
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Loopback short-circuit is fine; ensure TLS verification is actually enforced

This path depends on Domain_Manager::verify_domain_with_loopback_request(). In that method, the “HTTPS with SSL verification” variant currently uses sslverify tied to https_local_ssl_verify with a default false. That disables certificate verification by default and invites MITM.

Proposed fix in inc/managers/class-domain-manager.php:

- 'sslverify' => apply_filters('https_local_ssl_verify', false),
+ 'sslverify' => apply_filters('https_local_ssl_verify', true),

Also ensure the log label matches behavior (i.e., only claim “with SSL verification” when sslverify is true).

$network_ip_address = Helper::get_network_public_ip();

$results = \WP_Ultimo\Managers\Domain_Manager::dns_get_record($domain_url);
Expand Down Expand Up @@ -600,7 +606,7 @@ public static function get_by_site($site) {
* @since 2.0.0
*
* @param array|string $domains Domain names to search for.
* @return object
* @return static
*/
public static function get_by_domain($domains) {

Expand Down Expand Up @@ -633,7 +639,7 @@ public static function get_by_domain($domains) {
$placeholders_in = implode(',', $placeholders);

// Prepare the query
$query = "SELECT * FROM {$wpdb->wu_dmtable} WHERE domain IN ($placeholders_in) AND active = 1 ORDER BY primary_domain DESC, active DESC, secure DESC LIMIT 1";
$query = "SELECT * FROM {$wpdb->wu_dmtable} WHERE domain IN ($placeholders_in) ORDER BY primary_domain DESC, active DESC, secure DESC LIMIT 1";

$query = $wpdb->prepare($query, $domains); // phpcs:ignore

Expand Down
6 changes: 5 additions & 1 deletion inc/models/class-site.php
Original file line number Diff line number Diff line change
Expand Up @@ -1679,7 +1679,11 @@ public function save() {

$user_id = $customer->get_user_id();

add_user_to_blog($this->get_id(), $user_id, $role);
// only add user to blog if they are not already a member, or we are downgrading their role.
// Without this check the user could lose additional roles added manually or with hooks.
if ('administrator' !== $role || ! is_user_member_of_blog($user_id, $this->get_id())) {
add_user_to_blog($this->get_id(), $user_id, $role);
}
Comment on lines +1682 to +1686
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Guard add_user_to_blog when user_id is empty.

If get_user_id() returns 0/false, add_user_to_blog may misbehave. Add a truthy check.

Apply this diff:

- if ('administrator' !== $role || ! is_user_member_of_blog($user_id, $this->get_id())) {
-     add_user_to_blog($this->get_id(), $user_id, $role);
- }
+ if ($user_id && ('administrator' !== $role || ! is_user_member_of_blog($user_id, $this->get_id()))) {
+     add_user_to_blog($this->get_id(), $user_id, $role);
+ }
📝 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.

Suggested change
// only add user to blog if they are not already a member, or we are downgrading their role.
// Without this check the user could lose additional roles added manually or with hooks.
if ('administrator' !== $role || ! is_user_member_of_blog($user_id, $this->get_id())) {
add_user_to_blog($this->get_id(), $user_id, $role);
}
// only add user to blog if they are not already a member, or we are downgrading their role.
// Without this check the user could lose additional roles added manually or with hooks.
if ( $user_id && ( 'administrator' !== $role || ! is_user_member_of_blog( $user_id, $this->get_id() ) ) ) {
add_user_to_blog( $this->get_id(), $user_id, $role );
}
🤖 Prompt for AI Agents
In inc/models/class-site.php around lines 1682 to 1686, add a truthy check for
$user_id before calling is_user_member_of_blog() or add_user_to_blog(), because
get_user_id() can return 0/false; modify the conditional to first ensure
$user_id is truthy (e.g. $user_id) and then perform the existing role/member
checks so add_user_to_blog is only invoked when a valid user ID exists.

} elseif ($this->get_type() !== Site_Type::CUSTOMER_OWNED && $original_customer_id) {
$user_id = wu_get_customer($original_customer_id)->get_user_id();

Expand Down
41 changes: 11 additions & 30 deletions inc/objects/class-limitations.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Limitations {
*/
public function __construct($modules_data = []) {

$this->build_modules($modules_data);
$this->raw_module_data = $modules_data;
}

/**
Expand All @@ -83,16 +83,10 @@ public function __get($name) {
$module = wu_get_isset($this->modules, $name, false);

if (false === $module) {
$repo = self::repository();

$class_name = wu_get_isset($repo, $name, false);

if (class_exists($class_name)) {
$module = new $class_name($this->raw_module_data[ $name ] ?? []);

$module = self::build($this->raw_module_data[ $name ] ?? [], $name);
if ($module) {
$this->modules[ $name ] = $module;

return $module;
return $this->modules[ $name ];
}
}

Expand Down Expand Up @@ -121,7 +115,7 @@ public function __serialize() {
* @return void
*/
public function __unserialize($modules_data) {
$this->build_modules($modules_data);
$this->raw_module_data = $modules_data;
}

/**
Expand All @@ -136,14 +130,6 @@ public function build_modules($modules_data) {

$this->raw_module_data = $modules_data;

foreach ($modules_data as $type => $data) {
$module = self::build($data, $type);

if ($module) {
$this->modules[ $type ] = $module;
}
}

return $this;
}

Expand Down Expand Up @@ -181,7 +167,7 @@ public static function build($data, $module_name) {
*/
public function exists($module) {

return wu_get_isset($this->modules, $module, false);
return (bool) wu_get_isset($this->raw_module_data, $module, false);
}

/**
Expand All @@ -192,15 +178,13 @@ public function exists($module) {
*/
public function has_limitations() {

$has_limitations = false;

foreach ($this->modules as $module) {
if ($module->is_enabled()) {
foreach ($this->raw_module_data as $module) {
if ($module['enabled']) {
return true;
}
}

return $has_limitations;
return false;
}

/**
Expand All @@ -213,9 +197,7 @@ public function has_limitations() {
*/
public function is_module_enabled($module_name) {

$module = $this->{$module_name};

return $module ? $module->is_enabled() : false;
return $this->raw_module_data[ $module_name ]['enabled'] ?? false;
}

/**
Expand Down Expand Up @@ -374,8 +356,7 @@ protected function merge_recursive(array &$array1, array &$array2, $should_sum =
* @since 2.0.0
*/
public function to_array(): array {

return array_map(fn($module) => method_exists($module, 'to_array') ? $module->to_array() : (array) $module, $this->modules);
return $this->raw_module_data;
}

/**
Expand Down
Loading
Loading