WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 4 months ago

#20459 new enhancement

Super admin should be able to bypass banned/limited domains when creating users

Reported by: boonebgorges Owned by:
Milestone: Priority: normal
Severity: minor Version:
Component: Users Keywords: needs-patch 2nd-opinion
Focuses: administration, multisite Cc:
PR Number:

Description

The function wpmu_validate_user_signup() is run whenever a new user is created, either through self-registration (wp-signup.php) or through manual user creation by an admin. wpmu_validate_user_signup() does two different kinds of validation:
(1) validation that is more or less technically required by WP, like spaces in usernames, email/login uniqueness, etc.
(2) checks against some admin-set membership restrictions, namely, email domain whitelist (limited_email_domains) and blacklist (is_email_address_unsafe() and banned_email_domains).

The second kind of validation is problematic in the following use case: An MS install might restrict open membership based on email domains, but the admin might occasionally want to make exceptions to the rule and manually create an account. Currently, there are two ways to bypass the built-in checks: to temporarily remove the domain restrictions at Network Admin > Settings, or to filter 'wpmu_validate_user_signup' and remove the error messages.

Having to manually change settings for this purpose is pretty hackish. The filter method works, but my experience (from consulting with a fairly large number of MS network admins) is that this is a pretty common use case, so it seems like it should be supported by default.

So I'm proposing that the domain checks be skipped when is_super_admin(). Patch attached.

Attachments (2)

20459.patch (1.2 KB) - added by boonebgorges 8 years ago.
20459.2.patch (1.3 KB) - added by mordauk 6 years ago.
Check manage_network_users capability instead of is_super_admin()

Download all attachments as: .zip

Change History (10)

@boonebgorges
8 years ago

#1 @jeremyfelt
6 years ago

  • Component changed from Network Admin to Users
  • Focuses administration added

@mordauk
6 years ago

Check manage_network_users capability instead of is_super_admin()

#2 @mordauk
6 years ago

I've run into this before as well.

20459.2.patch is a refreshed patch, it also uses ! current_user_can( 'manage_network_users' ) instead of ! is_super_admin().

#3 follow-up: @boonebgorges
5 years ago

  • Keywords needs-patch 2nd-opinion added; has-patch removed

This issue just came up again with a client, so I'd like to work toward a solution at some point in the near future.

I do feel just a tad uneasy about silently ignoring the whitelist, even for 'manage_network_users' users. Would it expand the scope of this ticket too much to do something like this: prefetch the domain whitelist + blacklist into a JS variable; on blur, do a quick compare to see if the entered email address is allowed; if so, show some sort of warning, but continue to let the user create the account. This would allow super admins to bypass the check, but only after being reminded that they're doing so.

#4 @boonebgorges
4 years ago

#32511 was marked as a duplicate.

#5 in reply to: ↑ 3 @jeremyfelt
4 years ago

  • Milestone changed from Awaiting Review to Future Release

+1 in general.

Replying to boonebgorges:

Would it expand the scope of this ticket too much to do something like this: prefetch the domain whitelist + blacklist into a JS variable; on blur, do a quick compare to see if the entered email address is allowed; if so, show some sort of warning, but continue to let the user create the account. This would allow super admins to bypass the check, but only after being reminded that they're doing so.

And this would be slick.

#6 @boonebgorges
4 years ago

In 32638:

Improve tests for is_email_address_unsafe().

  • Move to a separate file for better organization and method names.
  • Use a dataProvider when appropriate, for better readability.
  • Add a test for splitting the banned domain list on line breaks.

See #20459, #21730.

#7 @boonebgorges
4 years ago

I started working on a patch that provides some admin feedback when entering a blacklisted/non-whitelisted email address. But it's pretty cumbersome to do it at the moment. Email addresses are subject to the 'limited_email_domains' as well as the 'banned_email_domains' lists. But the interfaces for dealing with these lists are really inconsistent. See #21730. Reproducing this logic in JavaScript is unappealing. And it seems like a mistake to do it on the server until #21730 has been resolved. I also wonder whether, if we're going to be in the business of validating against domain white/blacklists, whether we should be doing full-fledged validation: checking that it's a well-formed email address, checking that the email address is not already in use. And if we're going to do this for email addresses, maybe we should be doing the same for user names.

Maybe we can take a crack at overhauling email domain settings and user-new validation as a larger project in 4.4.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


4 years ago

Note: See TracTickets for help on using tickets.