Multisite Subfolders and bunk /wp-admin areas

[MadtownLems]

On a multisite installation with subfolders, navigating to:

www.mysite.com/i-dont-exist/wp-admin

loads a dashboard and maintains the i-dont-exist in the url. That part exists if i navigate to themes and/or plugin pages. However, I'm actually altering the base site.
This can be quite confusing if someone makes a typo in a sitename and they end up changing the base site (like happened to us) :(

[MadtownLems]

My desired behavior, by the way, would be sending them to their actual dashboard with a message "the site i-dont-exist doesn't exist"

[ddean]

proposal to handle the redirect and display an error message

[jeremyfelt]

Moving to 3.7 for discussion. Haven't tested patch yet, but we have one.

[nacin]

The use of all_admin_notices is clever, but I think we could probably ditch the notice — just silently do the redirect to the right place.

Need to make sure this properly handles the network admin (which always operates from the root of the network's URL), when WordPress is used in a subdirectory (i.e. something/wp-admin/users.php is actually /wordpress/wp-admin/users.php), and the user admin (wp-admin/user). I didn't identify any problems in particular, but this also looks like a great opportunity for unit tests.

The regexes themselves could be cleaned up a little. We only really need to worry about path comparisons here. Something like:

$site_admin_url = parse_url( admin_url(), PHP_URL_PATH );
preg_match( '.*/wp-admin', $_SERVER['REQUEST_URI'], $requested_admin_url );
$site_admin_url === trailingslashit( $requested_admin_url );

[MadtownLems]

just silently do the redirect to the right place.

I think the admin notice is critical to the solution here.

If I go to mysite.com/typo/wp-admin and only _silently_ get sent to the main site's dashboard, it's not that much better than continuing to display /typo/ in the URL. A typical user will think they're at the intended subsite's dashboard and start making changes, not realizing they're on the root site's dashboard.

[ddean]

Updated with nacin's simplified comparison and moved function call so it works on 3.6

[ocean90]

Needs some tests.

[Denis-de-Bernardy]

@ocean90:

Changes to the original patch:

• reformatted the code somewhat
• fixed the invalid comments
• added doc-blocks
• prevented the plugin screen's "unknown error" from firing by changing the semantics of the extra $_GET param

Tests done:

• Getting redirected as super-admin works as expected: you end up on the live site with an error message
• Getting redirected as a non-super-admin "nearly" works as expected: 17376.diff keeps you at the same url, while 17376.2.diff​ redirects you to the dev site first. In both cases, you're then greeted with large error message because your permissions don't allow you to visit the site's admin area. (The first with a url mismatch, the second with a correct url.)

I predict that the first behavior (17367.diff) is in fact better, in the sense that it allows end-users to see and fix their typos in the url. It might make sense to enforce the same kind of thing for super-admins. If you'd rather that, I can change the patch accordingly by using wp_die() instead of redirecting.

The wording of the error could possibly be improved. (Please suggest if you've anything better.)

[jeremyfelt]

To lock down desired behavior here:

1. The request comes in for http://validnetwork.com/invalid-site/wp-admin/
2. WordPress establishes that the network - $current_site - is valid.
3. WordPress establishes that the requested path does not match a current site and fills $current_blog in with the network's primary site data.
4. Redirect to the primary site's admin UR - http://validnetwork.com/wp-admin/
5. Show some sort of message indicating that the request was invalid and we redirected.

Is that what we're looking for?

[Denis-de-Bernardy]

Replying to jeremyfelt:

I'd argue it should be this instead:

1. The request comes in for http://validnetwork.com/invalid-site/wp-admin/
2. WordPress establishes that the network - $current_site - is valid.
3. WordPress establishes that the requested path does not match a current site and fills $current_blog in with the network's primary site data.
4. Check that the user can access the main site's admin area. If not, fail with an error message that says permission denied.
5. If yes, redirect to the primary site's admin UR - http://validnetwork.com/wp-admin/
6. Show some sort of message indicating that the request was invalid and we redirected.

The above is what my first patch does.

[jeremyfelt]

Replying to Denis-de-Bernardy:

4. Check that the user can access the main site's admin area. If not, fail with an error message that says permission denied.

Yup, that makes sense.

[jeremyfelt]

#25802 was marked as a duplicate.

[jeremyfelt]

I wonder if we can take advantage of get_site_by_path() and/or get_network_by_path() to figure out where to redirect now. I haven't completely tested 17376.diff​ yet, but the path positions seem pretty hard coded at first glance. We'll need to make sure this works for foo.bar/path1/path2/wp-admin/ as well.

[jeremyfelt]

17376.3.diff takes a different approach from other patches on this ticket and watches for admin requests during get_site_by_path(). If is_admin() returns true, it alters the search from multiple possible path segments to one possible path segment.

For foo.bar/site2 on a network where only foo.bar/ and foo.bar/site1/ exist:

• foo.bar/site2/ will search both site2/ and / as paths
• foo.bar/site2/wp-admin/ will search only site2/ as a path.

When a site is not found, the patch redirects to the network's primary site wp-admin/ path. At that point, if the user does not have access, we already throw a fairly helpful message showing the current sites that the user does have access to.

If the user does have access, the redirect is silent. I'm not sure that we should worry about additional admin notices for this scenario.

Anything outside the range of possible path segments—foo.bar/site2/page1/wp-admin/ will result in a 404 for the primary site, the current behavior.

[nacin]

17376.4.diff​ is a bit of an adjustment from jeremyfelt's approach, and refreshed after recent commits to #27003.

I'm not sure I want to add is_admin() to get_site_by_path() as it would be adding global state to a general utility function. Instead, we can simply ignore its results for is_admin() when $path !== $current_blog->path. (I'm not sure if strcasecmp() is needed here; it probably isn't.)

[jeremyfelt]

Replying to nacin:

I'm not sure I want to add is_admin() to get_site_by_path() as it would be adding global state to a general utility function.

Agreed, that felt ugly.

(I'm not sure if strcasecmp() is needed here; it probably isn't.)

I think we're clear in core. I'm not sure if it's probable that a sunrise is passing different cases that could get caught up here, so it may be worth leaving in.

+1 for the 17376.4.diff​, works in my testing.

[nacin]

If /typo/ means should actually send them to / instead, then any of this will work.

However, given /foo/typo/, /foo/ might be a valid site and / might be the network. At that point, should we send them to /wp-admin/, or /foo/wp-admin/? 17376.5.diff​ is an alternative that could handle this. I'm really not sure anymore and would rather we punt this, finalize it in the coming weeks, and get it right in 4.0.

[jeremyfelt]

Agreed. Let's pick this up again in 4.0.

[jeremyfelt]

Current status with valid sites at http://foo.org/ and http://foo.org/bar/

With 17376.5.diff

• http://foo.org/wp-admin/ loads admin of main site.
• http://foo.org/bar/wp-admin/ loads admin of bar site.
• http://foo.org/typo/wp-admin/ redirects to http://foo.org/
• http://foo.org/bar/typo/wp-admin/ loads a 404 on bar site.

With 17376.4.diff

• http://foo.org/wp-admin/ loads admin of main site.
• http://foo.org/bar/wp-admin/ loads admin of bar site.
• http://foo.org/typo/wp-admin/ redirects to http://foo.org/wp-admin/
• http://foo.org/bar/typo/wp-admin/ loads a 404 on bar site.

I like the simplicity of both patches in that we aren't trying to process a lot to figure out where the user is trying to go. I like the behavior of 17376.5.diff the best as a mistyped path/wp-admin/ loads a front end view. This forces another attempt rather than requiring the reading of an admin notice.

We're probably ok with http://foo.org/bar/typo/wp-admin/ loading a 404 instead of redirecting in the case where /bar/ is a valid path, not that WordPress is served from in /bar/.

I'm betting we can find a way to write tests for this in multisite/bootstrap.php. :)

[DrewAPicture]

@jeremyfelt: Looks like we still need unit tests here. My inclination is to punt for 4.2, but I'll let make that call.

[jeremyfelt]

Agreed. We really need unit tests for this to be sane.

[jeremyfelt]

#34844 was marked as a duplicate.

[jeremyfelt]

Replying to jeremyfelt:

Agreed. We really need unit tests for this to be sane.

I want to get a testable bootstrap going in #34941. I think we have a better chance of testing for this bug after that.

[SergeyBiryukov]

#48182 was marked as a duplicate.

[ridinhighspeeds]

I recently posted #48182 as our multisite network was recently under a heavy brute force attack.. The attacks were hitting all types of url's, not the standard www.mysite.com/wp-admin url. I assume this helps the attacker avoid getting noticed by typical security plugins, as those plugins probably monitor the www.mysite.com/wp-admin, not www.mysite.com/i-dont-exist/wp-admin or www.mysite.com/i-dont-exist-222/wp-admin etc...

[jeremyfelt]

#53504 was marked as a duplicate.