Allow wildcarded domains in multisite limited email domains

[djcp]

Here at blogs.law.harvard.edu, we want to allow all harvard.edu subdomains to create blogs in our multisite install. There are hundreds of domains and it would be difficult to get a complete list because of the complexity of our DNS infrastructure.

I propose allowing the inclusion of a single prefix wildcard character in the limited email domains feature. If a limited email domain contains a "*", we would create a regex and match that specific entry via a wildcard. So "*.harvard.edu" would match "cyber.law.harvard.edu", "fas.harvard.edu", etc. To match the root TLD, you'd just manually enter "harvard.edu".

We have a variant of this applied as a core hack to our wordpress install at ​http://blogs.law.harvard.edu and it's been working fine for years. I will package it up as a patch if there's interest. Thoughts?

I don't think it'd make sense to allow embedded wildcards (dom*ain.org).

Seems sensible. A patch would be great.

So if a limited domain begins with "*.", we nick off those characters and check it against the right side of the user's email domain. If a limited domain doesn't begin with "*.", we just check it normally while iterating through the limited domains.

This should be fully backwards compatible, we've just expanded out the in_array to inspect each limited_domain value, making those prefixed with "*." match their subdomains.

So here at Harvard, our allowed domain list looks like:

harvard.edu
hbs.edu
radcliffe.edu
*.harvard.edu
*.hbs.edu
*.radcliffe.edu

It's important that we keep wildcarding to subdomains and not actual domain names. Otherwise, if ibm.com was using this feature with a wildcard thusly:

*ibm.com

I could register "notibm.com" and exploit their multisite install. Since we require wildcarding on the subdomain level ("*.ibm.com"), that's not possible.

I suppose you could do something stupid like enter "*.com" and allow anyone with a .com address to register in your multisite install, but c'mon. You can't fix stupid.

I also added the "*" to the limited_email_domain regex in wp-admin/network/edit.php.

Replying to djcp:

I suppose you could do something stupid like enter "*.com" and allow anyone with a .com address to register in your multisite install, but c'mon. You can't fix stupid.

:-) I like your thinking. (That said, I see a legitimate use case in *.gov or *.edu for certain themed networks, for example.)

We use tabs, not spaces, and if you can re-upload it with a diff or patch extension, it'll be presented better in Trac.

Done, sorry about that. I can't see a way to remove the old .txt file (not that familiar with trac).

Cool. (Not a problem, also. I killed the attachment.)

Patch looks great, at a glance. All I see are coding standards improvements to be addressed. Marking for 3.2-early.

• foreach ( and if ( rather than foreach( and if(. Same goes for opening braces, space before.
• substr( $limited_domain, 0, 2 ) == '*.' should probably be 0 === strpos( $limited_domain, '*.' ).
• The direct domain match can be made into an elseif, instead of the nested if.
• Once a match is found, you should break; out of the foreach; no need to keep checking.
• No need for the braces around the one-line if at the bottom.
• $matched_domain == false should be ! $matched_domain.

+1, yes please. As an extra, anyway you could make the same in the DISallowed email field? So super admins can block all *.info or *.ru domains from signing up (for example).

Replying to nacin:

Cool. (Not a problem, also. I killed the attachment.)

Patch looks great, at a glance. All I see are coding standards improvements to be addressed. Marking for 3.2-early.

As this is my first contribution to the wordpress core, I'm a little unsure of standards. Sorry! Should I make those change and re-upload?

Replying to andrea_r:

+1, yes please. As an extra, anyway you could make the same in the DISallowed email field? So super admins can block all *.info or *.ru domains from signing up (for example).

That seems like a fine idea but my time is a little tight right now.

Replying to djcp:

Replying to nacin:
As this is my first contribution to the wordpress core, I'm a little unsure of standards. Sorry! Should I make those change and re-upload?

Not a problem. Congrats on your first patch :-) If you can do that, that'd be great. Otherwise, I'll clean it up in about a month when 3.2 development gets underway.

If there's no patch on this ticket come 3.2 for that Andrea mentions, I'll port this over and get it handled there as well.

Replying to djcp:

Replying to nacin:

Cool. (Not a problem, also. I killed the attachment.)

Patch looks great, at a glance. All I see are coding standards improvements to be addressed. Marking for 3.2-early.

As this is my first contribution to the wordpress core, I'm a little unsure of standards. Sorry! Should I make those change and re-upload?

​WordPress Coding Standards

You don't have to fix it for this patch. Patch looks good for the 3.2 cycle.

I wonder how many people use the limited email domains feature - didn't even realise it existed.

I also wonder if this would not be better served by better hooks and a plugin?

I am a sad panda. This didn't make it into 3.2 beta 1, and I think this has real utility for academia and any large institution that wants to create easy to manage multisite installs. Any chance this could get reviewed? It's highly unlikely to cause any backwards compatibility issues.

Refreshed for 3.3, adjusted as per nacin's comments.

Replying to andrea_r:

As an extra, anyway you could make the same in the DISallowed email field? So super admins can block all *.info or *.ru domains from signing up (for example).

is_email_address_unsafe() seems to support regular expressions:

if (
	strstr( $email_domain, $banned_domain ) ||
	(
		strstr( $banned_domain, '/' ) &&
		preg_match( $banned_domain, $email_domain )
	)
)
return true;

According to comments on #14695, it's possible to use something like /.*\.info/.

We're at freeze for 3.3.

Leads/committers, please weigh in on commit vs punt.

As always an incremental improvement over the course of several releases is better than not improving anything until something can be made perfect 3-4 releases from now. If this patch is good, let's get it in? If not, please leave specific feedback about what makes it not commit-worthy so the contributors can learn and/or revise for next round.

I still think a better solution here is a filter which allows for a plugin to implement this or any other rule for allowed domains rather than complicating the core code in this case.

We should just augment this line of code with an extra apply_filters call:

if ( in_array( $emaildomain, $limited_email_domains ) == false )

Tend to agree that a filter would be better. A filter on the response, that passes in the emaildomain. That can do all the wildcard regexes it wants.

Patch uploaded with suggested filter solution.

Since "Banned Email Domains" field already supports regexps, wouldn't it make sense to do the same in "Limited Email Registrations" for consistency? Done in 15706.2.patch​.

Related: #21570

Related: #21730