Skip to content

Conversation

@tamkendigital
Copy link
Contributor

@tamkendigital tamkendigital commented Nov 22, 2025

Fix: "Manage" Button SSO and Magic Link Support

Problem

The "Manage" button in the "My Sites" widget was not properly redirecting to the admin panel when SSO (Single Sign-On) was enabled. Additionally, when magic links were used for custom domains, the magic link URL was generated on the home URL instead of the admin URL, causing users to be redirected to the front-end page instead of the admin panel after login.

Issues Identified:

  1. Missing SSO Support: The "Manage" button did not include SSO parameters, preventing automatic login when clicking the button.

  2. Magic Link Path Issue: When wu_get_admin_url() generated a magic link for custom domains, it created URLs like: https://site.online/?wu_magic_token=... Instead of: https://site.online/wp-admin/?wu_magic_token=... This caused users to be logged in but redirected to the home page instead of the admin panel.

  3. Incompatibility with WP Hide: The solution needed to respect custom admin paths set by plugins like WP Hide.

Solution

Modified the get_manage_url() method in class-my-sites-element.php to:

  1. Use wu_get_admin_url() to automatically handle magic link generation when needed for custom domains.

  2. Detect Magic Links: Check if the generated URL contains wu_magic_token parameter.

  3. Fix Magic Link Path: When a magic link is detected:

    • Parse the magic link URL to extract the base URL and query string
    • Get the admin path using get_admin_url() (respects WP Hide and other plugins)
    • Reconstruct the URL with the admin path: https://site.com/wp-admin/?wu_magic_token=...
  4. Add SSO Support: When magic link is not used:

    • Use wp_login_url() with redirect_to parameter pointing to the admin URL
    • Add SSO parameter (sso=login) to the login URL
    • This ensures automatic SSO login flow
  5. Backward Compatibility: Apply the wp_ultimo_manage_url filter for compatibility with older versions and custom implementations.

Implementation Details

For wp_admin Type:

  • Uses wu_get_admin_url() to get admin URL with automatic magic link support
  • If magic link exists: Adds admin path directly to the URL
  • If magic link doesn't exist: Uses wp_login_url() with SSO parameter
  • Applies wp_ultimo_manage_url filter

For default Type (Frontend Context):

  • Same logic as wp_admin type when not in admin context
  • Ensures "Manage" button goes directly to admin, not to front-end URL

For custom_page Type:

  • No changes (unchanged behavior)

Technical Notes

  • Magic Link Detection: Uses strpos() to check for wu_magic_token in the URL
  • Admin Path Resolution: Uses parse_url(get_admin_url(), PHP_URL_PATH) to get the admin path, which respects WordPress filters and plugins like WP Hide
  • Site Context Switching: Uses switch_to_blog() and restore_current_blog() to get correct URLs for the target site
  • Fallback: Defaults to /wp-admin if admin path cannot be determined

Testing

Tested scenarios:

  • ✅ SSO login with standard domains
  • ✅ Magic link login with custom domains
  • ✅ Compatibility with WP Hide plugin (custom admin paths)
  • ✅ Backward compatibility with wp_ultimo_manage_url filter
  • ✅ Both wp_admin and default types

Files Modified

  • /inc/ui/class-my-sites-element.php - Modified get_manage_url() method

Related Functions

  • wu_get_admin_url() - Generates admin URLs with magic link support
  • wp_login_url() - WordPress function to get login URL with redirect
  • get_admin_url() - WordPress function to get admin URL (respects filters)
  • \WP_Ultimo\SSO\SSO::with_sso() - Adds SSO parameters to URLs

Example URLs Generated

With Magic Link:

https://site.online/wp-admin/?wu_magic_token=cacf33253e6a4a3f8f82f4d7944f09757ec5e4953dd5ea095a2bb2dacfe8ec84

With SSO (no magic link):

https://site.online/wp-login.php?redirect_to=https://site.online/wp-admin&sso=login

Compatibility

  • ✅ WordPress Multisite
  • ✅ WP Hide plugin (custom admin paths)
  • ✅ Custom domains with magic links
  • ✅ Standard domains with SSO
  • ✅ Backward compatible with existing filters

Summary by CodeRabbit

Release Notes

Bug Fixes

  • Improved admin URL routing in the My Sites element to correctly handle magic-link authentication and retain query parameters during navigation.
  • Enhanced Single Sign-On (SSO) integration for more reliable, seamless access to site admin areas.
  • Standardized admin routing across authentication methods to ensure consistent behavior while preserving backward compatibility.

✏️ Tip: You can customize this high-level summary in your review settings.

# Fix: "Manage" Button SSO and Magic Link Support

## Problem

The "Manage" button in the "My Sites" widget was not properly redirecting to the admin panel when SSO (Single Sign-On) was enabled. Additionally, when magic links were used for custom domains, the magic link URL was generated on the home URL instead of the admin URL, causing users to be redirected to the front-end page instead of the admin panel after login.

### Issues Identified:

1. **Missing SSO Support**: The "Manage" button did not include SSO parameters, preventing automatic login when clicking the button.

2. **Magic Link Path Issue**: When `wu_get_admin_url()` generated a magic link for custom domains, it created URLs like:
   ```
   https://site.online/?wu_magic_token=...
   ```
   Instead of:
   ```
   https://site.online/wp-admin/?wu_magic_token=...
   ```
   This caused users to be logged in but redirected to the home page instead of the admin panel.

3. **Incompatibility with WP Hide**: The solution needed to respect custom admin paths set by plugins like WP Hide.

## Solution

Modified the `get_manage_url()` method in `class-my-sites-element.php` to:

1. **Use `wu_get_admin_url()`** to automatically handle magic link generation when needed for custom domains.

2. **Detect Magic Links**: Check if the generated URL contains `wu_magic_token` parameter.

3. **Fix Magic Link Path**: When a magic link is detected:
   - Parse the magic link URL to extract the base URL and query string
   - Get the admin path using `get_admin_url()` (respects WP Hide and other plugins)
   - Reconstruct the URL with the admin path: `https://site.com/wp-admin/?wu_magic_token=...`

4. **Add SSO Support**: When magic link is not used:
   - Use `wp_login_url()` with `redirect_to` parameter pointing to the admin URL
   - Add SSO parameter (`sso=login`) to the login URL
   - This ensures automatic SSO login flow

5. **Backward Compatibility**: Apply the `wp_ultimo_manage_url` filter for compatibility with older versions and custom implementations.

## Implementation Details

### For `wp_admin` Type:
- Uses `wu_get_admin_url()` to get admin URL with automatic magic link support
- If magic link exists: Adds admin path directly to the URL
- If magic link doesn't exist: Uses `wp_login_url()` with SSO parameter
- Applies `wp_ultimo_manage_url` filter

### For `default` Type (Frontend Context):
- Same logic as `wp_admin` type when not in admin context
- Ensures "Manage" button goes directly to admin, not to front-end URL

### For `custom_page` Type:
- No changes (unchanged behavior)

## Technical Notes

- **Magic Link Detection**: Uses `strpos()` to check for `wu_magic_token` in the URL
- **Admin Path Resolution**: Uses `parse_url(get_admin_url(), PHP_URL_PATH)` to get the admin path, which respects WordPress filters and plugins like WP Hide
- **Site Context Switching**: Uses `switch_to_blog()` and `restore_current_blog()` to get correct URLs for the target site
- **Fallback**: Defaults to `/wp-admin` if admin path cannot be determined

## Testing

Tested scenarios:
- ✅ SSO login with standard domains
- ✅ Magic link login with custom domains
- ✅ Compatibility with WP Hide plugin (custom admin paths)
- ✅ Backward compatibility with `wp_ultimo_manage_url` filter
- ✅ Both `wp_admin` and `default` types

## Files Modified

- `/inc/ui/class-my-sites-element.php` - Modified `get_manage_url()` method

## Related Functions

- `wu_get_admin_url()` - Generates admin URLs with magic link support
- `wp_login_url()` - WordPress function to get login URL with redirect
- `get_admin_url()` - WordPress function to get admin URL (respects filters)
- `\WP_Ultimo\SSO\SSO::with_sso()` - Adds SSO parameters to URLs

## Example URLs Generated

**With Magic Link:**
```
https://site.online/wp-admin/?wu_magic_token=cacf33253e6a4a3f8f82f4d7944f09757ec5e4953dd5ea095a2bb2dacfe8ec84
```

**With SSO (no magic link):**
```
https://site.online/wp-login.php?redirect_to=https://site.online/wp-admin&sso=login
```

## Compatibility

- ✅ WordPress Multisite
- ✅ WP Hide plugin (custom admin paths)
- ✅ Custom domains with magic links
- ✅ Standard domains with SSO
- ✅ Backward compatible with existing filters
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 22, 2025

📝 Walkthrough

Walkthrough

The get_manage_url method was extended to handle magic-link (wu_magic_token) detection and WP Ultimo SSO flows for wp_admin and default types, reconstructing admin URLs or routing through SSO as appropriate while preserving query strings and applying the wp_ultimo_manage_url filter.

Changes

Cohort / File(s) Summary
WP Admin & manage URL logic
inc/ui/class-my-sites-element.php
Rewrote get_manage_url() branches for wp_admin and default (non-admin) flows: detect wu_magic_token and reconstruct admin URL (preserve query string); when SSO is available/enabled, switch to target site to build SSO-based login redirect to admin; apply wp_ultimo_manage_url filter for backward compatibility.

Sequence Diagram

sequenceDiagram
    participant Client
    participant MySites as My_Sites_Element::get_manage_url
    participant WuAdmin as wu_get_admin_url
    participant SiteSwitch as switch_to_blog
    participant SSO as WP_Ultimo_SSO
    participant Filter as wp_ultimo_manage_url

    Client->>MySites: request manage URL
    MySites->>WuAdmin: get base admin URL
    alt URL contains wu_magic_token
        MySites->>MySites: parse base + query
        MySites->>SiteSwitch: switch to target site
        SiteSwitch->>MySites: provide correct admin path
        MySites->>MySites: reconstruct admin URL + original query
    else no magic token and SSO enabled
        MySites->>SiteSwitch: switch to target site
        SiteSwitch->>SSO: request SSO login/admin path
        SSO->>MySites: return login redirect URL to admin
    else fallback (no magic, no SSO)
        MySites->>WuAdmin: use standard admin URL
    end
    MySites->>Filter: apply wp_ultimo_manage_url
    Filter->>Client: return final URL
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through tokens, paths, and keys,
Rebuilt the doors and tied the breeze,
When SSO sings I guide the way,
Queries kept safe so links can stay,
A rabbit's tweak — a smoother day.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main changes: adding SSO support and magic link handling to the get_manage_url method, which are the core objectives of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec60b2f and 1b559c6.

📒 Files selected for processing (1)
  • inc/ui/class-my-sites-element.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/ui/class-my-sites-element.php (3)
inc/functions/url.php (1)
  • wu_get_admin_url (107-129)
inc/sso/class-sso.php (3)
  • SSO (39-1113)
  • is_enabled (92-111)
  • get_url_path (1076-1087)
inc/functions/site.php (1)
  • wu_get_site (37-40)

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (4)
inc/ui/class-my-sites-element.php (4)

396-412: Clarify and date the “custom modification” header.

The docblock and inline “CUSTOM MODIFICATION” comments are clear, but the @modified_date is left as 2025-01-XX. Please update this to a concrete date and confirm this header style matches the project’s conventions, or simplify it if not needed.


414-459: Harden magic-link/SSO URL handling and reduce duplication.

The wp_admin and default (! is_admin()) branches share almost identical magic-link and SSO logic; keeping two copies risks future drift. Also, the URL handling can be made more robust:

  • For magic links, you assume wp_parse_url($admin_url) (and later parse_url(get_admin_url(), PHP_URL_PATH)) always returns an array with scheme and host. A filtered or malformed URL could make this false or omit keys, leading to PHP notices/fatal errors.
  • You’re manually reconstructing SSO URLs using $sso->get_url_path() and add_query_arg. The \WP_Ultimo\SSO\SSO::with_sso($url) helper already encapsulates this behavior and the is_enabled() check.

Consider extracting a small private helper on this class (or reusing existing helpers) to encapsulate:

private function build_manage_admin_url( int $site_id ): string {
    $admin_url = wu_get_admin_url( $site_id );

    // Magic-link handling...

    // SSO handling using \WP_Ultimo\SSO\SSO::with_sso( $login_url ) if available...

    // wp_ultimo_manage_url filter...
}

Then call it from both branches. Also, when parsing URLs:

  • Prefer wp_parse_url() everywhere instead of parse_url() for consistency with WordPress.
  • Guard against false or missing keys before indexing.

Example adjustment for the admin-path extraction:

- switch_to_blog($site_id);
- $admin_path = parse_url(get_admin_url(), PHP_URL_PATH);
- restore_current_blog();
+ switch_to_blog($site_id);
+ $admin_url_for_path = get_admin_url();
+ restore_current_blog();
+
+ $admin_path = wp_parse_url($admin_url_for_path, PHP_URL_PATH) ?: '';
+ if ($admin_path === '') {
+     $admin_path = '/wp-admin';
+ }

This keeps behavior consistent while making the code safer and easier to maintain.

Also applies to: 485-533


485-545: Confirm intentional behavior change for default (“Same Page”) manage type.

For the default type, when ! is_admin(), you now always send users directly to the site’s admin URL (with magic-link/SSO support) and bypass the previous \WP_Ultimo\Current::get_manage_url() behavior. Given the field label ('default' => 'Same Page') this is a semantic change:

  • Front-end uses of the My Sites element that relied on “Same Page” navigation (with a site hash in the URL) will now jump straight to admin.
  • In admin context (is_admin()), you still delegate to Current::get_manage_url, preserving the old behavior there.

If this is the intended new contract, consider:

  • Updating the field label/description to match the new behavior, or
  • Documenting this as a breaking/behavioral change in the changelog/upgrade notes.

If not intended, you may want to keep the old “Same Page” behavior for default type and introduce the new SSO/magic-link logic only for 'wp_admin'.


396-549: Address coding-standards warnings (trailing whitespace, alignment, parse_url).

Static analysis is flagging:

  • Trailing whitespace on several lines in this block.
  • Misaligned = signs in some assignments.
  • Use of parse_url() instead of wp_parse_url() for the admin-path extraction.

These aren’t runtime bugs but will likely keep code-quality checks red. Cleaning them up (and switching fully to wp_parse_url()) should make CI happy and keep the file consistent with the rest of the project.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0df6d71 and ec60b2f.

📒 Files selected for processing (1)
  • inc/ui/class-my-sites-element.php (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
inc/ui/class-my-sites-element.php (4)
inc/functions/url.php (1)
  • wu_get_admin_url (107-129)
inc/sso/class-sso.php (3)
  • SSO (39-1113)
  • is_enabled (92-111)
  • get_url_path (1076-1087)
inc/functions/site.php (1)
  • wu_get_site (37-40)
inc/class-current.php (1)
  • Current (20-427)
🪛 GitHub Check: Code Quality Checks
inc/ui/class-my-sites-element.php

[failure] 466-466:
Whitespace found at end of line


[failure] 460-460:
Whitespace found at end of line


[failure] 455-455:
Whitespace found at end of line


[warning] 453-453:
Equals sign not aligned with surrounding assignments; expected 8 spaces but found 1 space


[warning] 451-451:
Equals sign not aligned with surrounding assignments; expected 9 spaces but found 1 space


[warning] 443-443:
Equals sign not aligned with surrounding assignments; expected 4 spaces but found 1 space


[failure] 440-440:
Whitespace found at end of line


[failure] 435-435:
Whitespace found at end of line


[warning] 433-433:
parse_url() is discouraged because of inconsistency in the output across PHP versions; use wp_parse_url() instead.


[failure] 430-430:
Whitespace found at end of line


[warning] 426-426:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space


[failure] 421-421:
Whitespace found at end of line


[failure] 417-417:
Whitespace found at end of line


[warning] 525-525:
Equals sign not aligned with surrounding assignments; expected 9 spaces but found 1 space


[warning] 517-517:
Equals sign not aligned with surrounding assignments; expected 4 spaces but found 1 space


[warning] 507-507:
parse_url() is discouraged because of inconsistency in the output across PHP versions; use wp_parse_url() instead.


[warning] 500-500:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space


[failure] 495-495:
Whitespace found at end of line


[failure] 491-491:
Whitespace found at end of line

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: PHP 8.4
  • GitHub Check: cypress (8.2, chrome)
  • GitHub Check: cypress (8.1, chrome)

Comment on lines 425 to 444
$parsed_url = wp_parse_url($admin_url);
$base_url = $parsed_url['scheme'] . '://' . $parsed_url['host'];
if (isset($parsed_url['port'])) {
$base_url .= ':' . $parsed_url['port'];
}

// Get admin path (respects WP Hide and other plugins)
switch_to_blog($site_id);
$admin_path = parse_url(get_admin_url(), PHP_URL_PATH);
restore_current_blog();

// Ensure admin path exists, default to /wp-admin if not found
if (empty($admin_path)) {
$admin_path = '/wp-admin';
}

// Build new URL with admin path
$query_string = isset($parsed_url['query']) ? '?' . $parsed_url['query'] : '';
$admin_url = $base_url . $admin_path . $query_string;
} elseif (class_exists('\WP_Ultimo\SSO\SSO')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Align with WordPress URL helpers and guard against parsing failures.

Both magic-link branches construct $base_url and $admin_path using wp_parse_url() and parse_url() without error checking. To better align with WordPress practices and avoid subtle PHP-version differences:

  • Replace parse_url(get_admin_url(), PHP_URL_PATH) with wp_parse_url(get_admin_url(), PHP_URL_PATH).
  • Add simple guards when working with $parsed_url to avoid notices if parsing fails or components are missing.

For example:

- $parsed_url = wp_parse_url($admin_url);
- $base_url = $parsed_url['scheme'] . '://' . $parsed_url['host'];
+ $parsed_url = wp_parse_url($admin_url);
+ if (! is_array($parsed_url) || empty($parsed_url['scheme']) || empty($parsed_url['host'])) {
+     return $admin_url; // Fallback safely if URL is unexpected.
+ }
+ $base_url = $parsed_url['scheme'] . '://' . $parsed_url['host'];

This keeps behavior the same in normal cases but fails safely if filters or environment quirks ever produce non-standard URLs.

Also applies to: 498-517


I'll verify the review comment by examining the actual code and validating the suggestions against WordPress practices.


🏁 Script executed:

#!/bin/bash
# Locate and examine the file at the specified lines
if [ -f "inc/ui/class-my-sites-element.php" ]; then
  echo "=== Lines 425-444 ==="
  sed -n '425,444p' "inc/ui/class-my-sites-element.php"
  echo ""
  echo "=== Lines 498-517 ==="
  sed -n '498,517p' "inc/ui/class-my-sites-element.php"
else
  echo "File not found at expected path"
  # Try to find it
  find . -name "class-my-sites-element.php" 2>/dev/null | head -5
fi

Length of output: 1567


🏁 Script executed:

#!/bin/bash
# Search for wp_parse_url usage patterns in the codebase to understand conventions
echo "=== wp_parse_url usage in codebase ==="
rg -n "wp_parse_url" -A 3 -B 1 --type php | head -40

Length of output: 2849


🏁 Script executed:

#!/bin/bash
# Search for parse_url usage to see if it's mixed with wp_parse_url
echo "=== parse_url usage patterns ==="
rg -n "parse_url\(" inc/ui/class-my-sites-element.php -A 2 -B 1 --type php

Length of output: 769


🏁 Script executed:

#!/bin/bash
# Check WordPress source or documentation within repo for wp_parse_url definition
echo "=== Searching for wp_parse_url definition ==="
rg -n "function wp_parse_url" --type php -A 5

Length of output: 45


🌐 Web query:

WordPress wp_parse_url function PHP_URL_PATH constant support

💡 Result:

Yes — wp_parse_url accepts PHP's component constants (including PHP_URL_PATH) and will return the path when you pass PHP_URL_PATH. The function is a wrapper around parse_url() and maps PHP_URL_PATH to the 'path' key (introduced in WP 4.7). [1][2]

Sources:

  • wp_parse_url() docs. [1]
  • Internal mapping of PHP_URL_* constants to keys (includes PHP_URL_PATH). [2]

Replace parse_url() with wp_parse_url() and add guards for missing URL components.

Both locations (lines 433, 507) use parse_url() instead of wp_parse_url(), which is inconsistent with the preceding call on the same URL. Additionally, the code accesses $parsed_url['scheme'] and $parsed_url['host'] without checking if parsing succeeded, risking notices if the URL is malformed or filters produce unexpected results.

Replace parse_url(get_admin_url(), PHP_URL_PATH) with wp_parse_url(get_admin_url(), PHP_URL_PATH) at lines 433 and 507, and add guards immediately after the first wp_parse_url() call:

$parsed_url = wp_parse_url($admin_url);
if (! is_array($parsed_url) || empty($parsed_url['scheme']) || empty($parsed_url['host'])) {
    return $admin_url; // Fallback safely if URL is unexpected.
}
🧰 Tools
🪛 GitHub Check: Code Quality Checks

[warning] 443-443:
Equals sign not aligned with surrounding assignments; expected 4 spaces but found 1 space


[failure] 440-440:
Whitespace found at end of line


[failure] 435-435:
Whitespace found at end of line


[warning] 433-433:
parse_url() is discouraged because of inconsistency in the output across PHP versions; use wp_parse_url() instead.


[failure] 430-430:
Whitespace found at end of line


[warning] 426-426:
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space

🤖 Prompt for AI Agents
In inc/ui/class-my-sites-element.php around lines 425 to 444 (and also update
the other occurrence at ~507), replace the raw PHP parse_url() calls with
wp_parse_url() and add a guard after the first wp_parse_url($admin_url) call to
ensure parsing succeeded and required components exist; if wp_parse_url doesn't
return an array or the scheme/host are empty, return the original $admin_url as
a safe fallback. Also update the two other parse_url(get_admin_url(),
PHP_URL_PATH) uses to wp_parse_url(get_admin_url(), PHP_URL_PATH) so all URL
parsing is consistent and protected by the same guard.

@superdav42
Copy link
Collaborator

@tamkendigital I think most of these changes should actually go into wu_get_admin_url() the proper url will be used in more places.

Copy link
Collaborator

@superdav42 superdav42 left a comment

Choose a reason for hiding this comment

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

I think most of the code changes should actually be in wu_get_admin_url() so the correct admin url will be used in more places besides the my sites element

public function get_manage_url($site_id, $type = 'default', $custom_page_id = 0) {

if ('wp_admin' === $type) {
return wu_get_admin_url($site_id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think most of the code changes should actually be in wu_get_admin_url() so the correct admin url will be used in more places besides the my sites element

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
inc/ui/class-my-sites-element.php (1)

404-533: Extract duplicated magic-link and SSO logic into a private helper method.

Lines 405-458 and 479-532 contain nearly identical logic (~70 lines duplicated): both detect magic links via strpos(), reconstruct URLs with proper admin paths by parsing and switching blog context, handle SSO flow when enabled, and apply the same filter. This duplication violates DRY and creates a maintenance burden—any bug fix or enhancement must be applied twice.

🔎 Proposed refactor: Extract to helper method

Add a private helper method:

/**
 * Processes admin URL with magic link and SSO support.
 *
 * @param int $site_id The site ID.
 * @return string The processed admin URL.
 */
private function process_admin_url_with_magic_link_and_sso($site_id) {
    $admin_url = wu_get_admin_url($site_id);
    
    $has_magic_link = (strpos($admin_url, 'wu_magic_token') !== false);
    
    if ($has_magic_link) {
        $parsed_url = wp_parse_url($admin_url);
        if (! is_array($parsed_url) || empty($parsed_url['scheme']) || empty($parsed_url['host'])) {
            return $admin_url;
        }
        
        $base_url = $parsed_url['scheme'] . '://' . $parsed_url['host'];
        if (isset($parsed_url['port'])) {
            $base_url .= ':' . $parsed_url['port'];
        }
        
        switch_to_blog($site_id);
        $admin_path = wp_parse_url(get_admin_url(), PHP_URL_PATH);
        restore_current_blog();
        
        if (empty($admin_path)) {
            $admin_path = '/wp-admin';
        }
        
        $query_string = isset($parsed_url['query']) ? '?' . $parsed_url['query'] : '';
        $admin_url = $base_url . $admin_path . $query_string;
    } elseif (class_exists('\WP_Ultimo\SSO\SSO')) {
        $sso = \WP_Ultimo\SSO\SSO::get_instance();
        if ($sso && $sso->is_enabled()) {
            switch_to_blog($site_id);
            $sso_path = $sso->get_url_path();
            $actual_admin_url = get_admin_url($site_id);
            $login_url = wp_login_url($actual_admin_url);
            restore_current_blog();
            
            $admin_url = add_query_arg($sso_path, 'login', $login_url);
        }
    }
    
    $site = wu_get_site($site_id);
    if ($site) {
        $admin_url = apply_filters('wp_ultimo_manage_url', $admin_url, $site);
    }
    
    return $admin_url;
}

Then simplify both branches:

 	if ('wp_admin' === $type) {
-		// Use wu_get_admin_url() to get admin URL with magic link support if needed
-		// ... (50+ lines of duplicated logic)
-		return $admin_url;
+		return $this->process_admin_url_with_magic_link_and_sso($site_id);
 	}
 	
 	// ...
 	
 	if ( ! is_admin()) {
-		// Use wu_get_admin_url() to get admin URL with magic link support if needed
-		// ... (50+ lines of duplicated logic)
-		return $admin_url;
+		return $this->process_admin_url_with_magic_link_and_sso($site_id);
 	}
♻️ Duplicate comments (2)
inc/ui/class-my-sites-element.php (2)

404-459: Add guards for URL parsing failures (as previously flagged).

The past review comment correctly identified that wp_parse_url() calls at lines 416-420 lack guards against parsing failures or missing components. If wp_parse_url() returns false or the URL lacks scheme/host, accessing $parsed_url['scheme'] and $parsed_url['host'] will trigger PHP notices.

🔎 Proposed fix: Add guards after wp_parse_url
 			$admin_url = wu_get_admin_url($site_id);
 
 			// Check if magic link is already used (contains wu_magic_token)
 			// If magic link exists, add admin path directly to the URL
 			$has_magic_link = (strpos($admin_url, 'wu_magic_token') !== false);
 
 			if ($has_magic_link) {
 				// Magic link is generated on home URL, we need to add admin path
 				// Parse the URL to get the base URL and query string
 				$parsed_url = wp_parse_url($admin_url);
+				if (! is_array($parsed_url) || empty($parsed_url['scheme']) || empty($parsed_url['host'])) {
+					return $admin_url; // Fallback safely if URL parsing fails
+				}
 				$base_url   = $parsed_url['scheme'] . '://' . $parsed_url['host'];

476-533: Add guards for URL parsing failures (as previously flagged).

The same issue from the previous review comment applies here: wp_parse_url() at lines 490-494 lacks guards against parsing failures. If parsing returns false or the URL is missing required components, accessing array keys will trigger PHP notices.

🔎 Proposed fix: Add guards after wp_parse_url
 			$admin_url = wu_get_admin_url($site_id);
 
 			// Check if magic link is already used (contains wu_magic_token)
 			// If magic link exists, add admin path directly to the URL
 			$has_magic_link = (strpos($admin_url, 'wu_magic_token') !== false);
 
 			if ($has_magic_link) {
 				// Magic link is generated on home URL, we need to add admin path
 				// Parse the URL to get the base URL and query string
 				$parsed_url = wp_parse_url($admin_url);
+				if (! is_array($parsed_url) || empty($parsed_url['scheme']) || empty($parsed_url['host'])) {
+					return $admin_url; // Fallback safely if URL parsing fails
+				}
 				$base_url   = $parsed_url['scheme'] . '://' . $parsed_url['host'];
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec60b2f and 1b559c6.

📒 Files selected for processing (1)
  • inc/ui/class-my-sites-element.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/ui/class-my-sites-element.php (3)
inc/functions/url.php (1)
  • wu_get_admin_url (107-129)
inc/sso/class-sso.php (3)
  • SSO (39-1113)
  • is_enabled (92-111)
  • get_url_path (1076-1087)
inc/functions/site.php (1)
  • wu_get_site (37-40)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants