WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#30598 closed defect (bug) (fixed)

Multisite Subdomain doesn't properly redirect users logging in to the primary site

Reported by: Ipstenu Owned by: jeremyfelt
Milestone: 4.5 Priority: normal
Severity: normal Version:
Component: Networks and Sites Keywords: has-patch
Focuses: multisite Cc:

Description

In this scenario, you go to example.com/wp-login.php and log in while you are not a member of that site. You are a member of any other site.

On Subfolders

Upon successful login, you are sent to example.com/yoursite/wp-admin

On Subdomains

Upon successful login, you get the Access Denied page (see #17121 )

What Was Expected

You should have gone to either yoursite.example.com/wp-admin or the user dashboard page.

(FWIW, this always happened on Subdomains as far as I recall, to the point that I just assumed that was normal!)

Attachments (3)

30598-error.2.png (45.9 KB) - added by RMarks 3 years ago.
Error message shown when logging into network dashboard.
30598.diff (1.5 KB) - added by ShinichiN 3 years ago.
jeremyfelt's method.
30598.2.diff (469 bytes) - added by jeremyfelt 3 years ago.

Download all attachments as: .zip

Change History (13)

#1 @jeremyfelt
4 years ago

  • Milestone changed from Awaiting Review to Future Release

This happens because of the wp_safe_redirect() call used to manage the redirect request. The user's subdomain does not match the allowed host from the original request and the redirect is sanitized back to the standard admin URL.

It seems like we could split the redirect handling here. If $redirect_to is actually provided by the login page as $_POST data, use wp_safe_redirect(). If the URL is core generated (e.g. get_dashboard_url()), use wp_redirect().

Not sure if there are other repercussions to think of here.

@RMarks
3 years ago

Error message shown when logging into network dashboard.

#2 @RMarks
3 years ago

  • Keywords reporter-feedback added

Please see my screen shot above. I think the error describes well what has happened and provides the user with a path forward.

@Ipstenu can you look at this and see if this still needs attention?

If so, I'm leaving a note that Jeremy's suggested code should go in wp-login.php at/before line 826.

#3 @Ipstenu
3 years ago

Yes, it should be corrected for parity. The two (subdomain and subfolder) should act the same.

I don't care which way we go, as long as they stop being different.

#4 @RMarks
3 years ago

  • Keywords reporter-feedback removed

I did some additional testing and wanted to clarify a detail in this ticket's description.

On Subfolders

Upon successful login, you are sent to example.com/yourFirstSite/wp-admin

In the event the user is a member of more than one site, the user is redirected to the admin of the site with the lowest blog ID.

Also, here is another scenario in case it's useful:
If you go to example.com/site2/wp-login.php and log in while you are not a member of that site, you will see the Access Denied message.

#5 @jeremyfelt
3 years ago

  • Keywords needs-patch added
  • Milestone changed from Future Release to 4.5

In either subdomain or subdirectory, I could expect this to happen on the main network site:

  • If authenticating at example.org without direct access, redirect to my primary site.
  • If no primary site is assigned (possible?), redirect to user dashboard. (My sites?)

For other sites on the network:

  • If authenticating at othersite.example.org without direct access, show "Access denied".
  • If authenticating at example.org/othersite/ without direct access, show "Access denied".

@ShinichiN
3 years ago

jeremyfelt's method.

#6 @ShinichiN
3 years ago

I'm wondering if it is better to use the allowed_redirect_hosts filter in wp_validate_redirect() and add the subdomain host name to the $allowed_hosts array.

<?php
/**
 * Filter the whitelist of hosts to redirect to.
 *
 * @since 2.3.0
 *
 * @param array       $hosts An array of allowed hosts.
 * @param bool|string $host  The parsed host; empty if not isset.
 */
$allowed_hosts = (array) apply_filters( 'allowed_redirect_hosts', array($wpp['host']), isset($lp['host']) ? $lp['host'] : '' );

#7 @ShinichiN
3 years ago

  • Keywords has-patch added; needs-patch removed

#8 @mikeschroder
3 years ago

  • Owner set to jeremyfelt
  • Status changed from new to assigned

@jeremyfelt, what's your take on this? Is it too late in the cycle to make the change, or safe enough to proceed?

@jeremyfelt
3 years ago

#9 @jeremyfelt
3 years ago

This is what happens in wp-login.php when a user logs in and a redirect_to query var is not provided:

  • If the user has a network account, but has no active site, redirect to user_admin_url().
  • If the user does not have read capabilities on this site, redirect to the user's active site via get_dashboard_url().
  • If the user has read capabilities for the site, but cannot edit posts, redirect to the user's profile page on that site via admin_url( 'profile.php' ). In single site, it's also possible that home_url() is used here.

Right now, we use wp_safe_redirect() for any redirect in wp-login.php because we also handle the unsafe redirect_to query var data. When doing this, a subdomain will not be found as a safe host (and likely shouldn't), and the redirect will go to an unexpected location, showing access denied.

In the above list, we can instead use wp_redirect() for each condition because they do not involve unsafe data from the request or referrer. I think we can solve without adding complexity. 30598.2.diff does this

#10 @jeremyfelt
3 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 36867:

Multisite: Handle redirect to a user's subdomain properly during login

wp-login.php uses wp_safe_redirect() for all redirects, even those that do not involve unsafe data from the request or referer.

When a user of a subdomain site attempts to login to a network site they do not have access to, the host in the redirect URL is treated as unsafe by wp_safe_redirect() as it has no immediate awareness as to which hosts are valid on the network. On a subdirectoy network, everything works as expected because the host is the same.

In this specific block of wp-login.php, all URLs are generated by WordPress and we can use wp_redirect() to handle the redirects. Users authenticating via other network sites will now be redirected properly. Hosts passed via the redirect_to query var will continue to be handled by wp_safe_redirect().

Fixes #30598.

Note: See TracTickets for help on using tickets.