Opened 9 years ago
Closed 9 years ago
#34065 closed defect (bug) (fixed)
Upgrading from single site to multisite fails when the Network Admin Email doesn't match an existing user.
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 4.4 | Priority: | normal |
Severity: | normal | Version: | 4.4 |
Component: | Networks and Sites | Keywords: | has-patch |
Focuses: | administration, multisite | Cc: |
Description
While the Network Admin Email is a freeform input field, it appears to only work if used with the email of an existing user. In the current state, the network's main site isn't configured correctly if set up with an email that doesn't exist for a user.
The function populate_network
is using the network admin email to retrieve a user and with that user it's setting:
- The super admins for the main site
- The admin_email
- The admin_user_id
- Setting the 'source_domain' in that user's meta.
- Setting the 'primary_blog' in that user's meta.
All of those except admin_email
require an existing user. To fix we should check whether the email exists for an existing user, if not, bail with an error. This is similar to what happens if the field is submitted with something that doesn't pass email validation.
We could also consider changing the input field to use autocomplete, and only allow selecting of email addresses for users that exist, as it appears that on initial upgrade, Network Admin Email should be an email for an existing user. We could also update the field to indicate that requirement.
Steps to reproduce:
- Install WordPress.
- Add
define( 'WP_ALLOW_MULTISITE', true );
to wp-config.php - Navigate to
/wp-admin/network.php
- Leave the defaults, but change the 'Network Admin Email' to an email address that isn't used for any user already in the database.
- Click 'Install'.
Actual Results
The page reloads with 6 notices in schema.php, see screenshot below. Also because of how the $site_user
data is used, the sitemeta for the main site becomes invalid.
Questions:
- Even after the initial upgrade, is Network Admin Email required to be associated with an existing user?
- If not, is there another way to upgrade to multisite and not require the Network Admin Email to be associated with a user? Is that something worth pursuing or should we just fix the messaging and enforce it?
Attachments (2)
Change History (12)
#2
@
9 years ago
- Milestone changed from Awaiting Review to 4.4
Hey @jjeaton, nice catch.
Even if the email address is invalid, we still add the user performing the network setup as a super admin. So the only thing that really gets broken here is the site meta. At least there's that. :)
I like 34065.diff in that it cuts straight to the chase and prevents the installation. It would probably be nice for that screen to only allow the selection of an existing user one day—or really just not show that info and use the current user. We do pre-fill the email address, which is good.
I'd be okay adding the catch for 4.4, but if anybody has other ideas, let's discuss.
Also amusing to note that we check is_email()
after we've already attempted to get a user with it.
#3
follow-up:
↓ 5
@
9 years ago
@jeremyfelt I like this option:
or really just not show that info and use the current user
While maybe there would be a case where one admin was upgrading the site to multisite and wanted another admin account to be the admin_email user, I'm not sure how common that would be.
In addition to the existing patch, would we want to add a description to the field that mentions it should correspond to an existing user?
This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.
9 years ago
#5
in reply to:
↑ 3
@
9 years ago
- Keywords needs-patch added; has-patch removed
Replying to jjeaton:
While maybe there would be a case where one admin was upgrading the site to multisite and wanted another admin account to be the admin_email user, I'm not sure how common that would be.
It turns out this is much more common than I thought. This email is often a catch-all box for notifications and acts as a way to separate the admin's email from that attached to emails that users get when registering.
We came up with this during office hours yesterday as a good start:
- Check for valid
$site_user
based on the entered email. - If not valid, assign the provided email to
admin_email
, but ensure the current user is set for everything else. - Update messaging on the initial screen to match. Related: #34293.
#6
@
9 years ago
- Keywords has-patch added; needs-patch removed
Refreshed patch 34065.2.diff addresses all three items.
- The is_email() check now happens before attempting to get a user based on the email.
- The provided email is always used for
admin_email
, and if it doesn't match an existing user,$site_user
is set to the current user. - The field description on
network.php
has been updated to read: "This address is used for admin purposes, like site notifications. Registration emails will be sent from this address."
Added patch 34065.diff which bails out of the multisite upgrade process if the network admin email doesn't exist for an existing user.