WP allows creating username that is already used email address

[phillipburger]

As reported in Support Forum (​https://wordpress.org/support/topic/wp-allows-creating-username-that-is-already-used-email-address/) it seems I can create a user with wp_create_user where the user's "username" is set as a value that is an existing Email Address for another user in the WordPress system.

(I have not submitted a bug here before, so let me know if more info is needed on this and what to do next).

[pbiron]

Welcome to trac!!!

Nice catch...in all my years building WP sites I never thought to use email addresses as usernames :-)

In the support topic you reference you say,

I have since added a check in my code to stop this

I'm not sure how you implemented that check, but I would suggest you do so using the ​username_exists filter, as follows:

<?php
add_filter( 'username_exists', 'my_username_exists_filter_func', 10, 2 );
function my_username_exists_filter_func( $user_id, $username ) {
        if ( $user_id ) {
                return $user_id;
        }

        return get_user_by( 'email', $username );
}

This will cause ​wp_create_user() to return a WP_Error and the new user will not be created.

[phillipburger]

Thanks! Yeah, I guess when you allow end users to input information, they will find the bugs.

I was using a check similar to the example on ​https://developer.wordpress.org/reference/functions/wp_create_user/#user-contributed-notes which checks:

1. does the username entered exist as a username
2. does the email entered exist as an email address

but it forgot to check the other 2 ways of:

3. does the username exist as an email address (the problem I had in this case)
4. does the email address exist as a username (a reverse problem that would cause the same issue)

I just over coded my side to do checks of username and email address both in username_exists() and email_exists() and then make sure all 4 checks brought back false before processing.

I have been using filters already but I did not think of it here.

Let me know any more info needed.

Replying to pbiron:

Welcome to trac!!!

Nice catch...in all my years building WP sites I never thought to use email addresses as usernames :-)

In the support topic you reference you say,

I have since added a check in my code to stop this

I'm not sure how you implemented that check, but I would suggest you do so using the ​username_exists filter, as follows:

<?php
add_filter( 'username_exists', 'my_username_exists_filter_func', 10, 2 );
function my_username_exists_filter_func( $user_id, $username ) {
        if ( $user_id ) {
                return $user_id;
        }

        return get_user_by( 'email', $username );
}

This will cause ​wp_create_user() to return a WP_Error and the new user will not be created.

[phillipburger]

As I thought more about this one, it might actually be a bigger problem. Lets say that a user (maybe the only admin of a site) has an account like:

username: my_admin_username (clearly a poor username)
email: websiteadmin@…

If a bad person happens to know that email address and they have a way to register a new user on that website and they decide to create an account with:

username: websiteadmin@…
email: mypersonalemail@…

The problem I realized is, hopefully the admin knows their username and does not always log in with email address because the lost email and other login dialogs that use "username or email address" seem to check the email address against username first - so that admin user may be locked out of the site.

Thanks again, hope this helps.

[subrataemfluence]

Good find!!
Adding additional guard for checking the existence of username supplied in email field in core functionality should resolve the issue.

Possible solution:

File name: includes/user.php.
Function: edit_user

Addition:

<?php
if ( ! $update && email_exists( $user->user_login ) ) {
   $errors->add( 'user_login', __( '<strong>ERROR</strong>: This username is invalid because it is already in use as email address of another account.' ) );
}

File: wp-includes/user.php
Function: register_new_user

Addition:

<?php
if ( $user_email == '' ) {
   ...
} elseif ( ! is_email( $user_email ) ) {
   ...
} elseif ( email_exists( $user_email ) ) {
   ...
} elseif ( email_exists( $sanitized_user_login ) ) {
   $errors->add( 'email_exists', __( '<strong>ERROR</strong>: This username is invalid because it is already in use as email address of another account.' ) );
}

I think adding the check into the core itself will prevent additional filter and functions.

Please let me know if this makes some sense.

[phillipburger]

As much input as possible is great, I think this seems to make sense when a new username comes in to make sure it is not already existing email address, but the other way too - what if a new email address is entered and it is already a username - that needs to be verified as well.

And as this is fixed, what should be a solution if these situations already exist in a deployment? Do they need to be flagged for updates somehow?

Replying to subrataemfluence:

Good find!!
Adding additional guard for checking the existence of username supplied in email field in core functionality should resolve the issue.

Possible solution:

File name: includes/user.php.
Function: edit_user

Addition:

<?php
if ( ! $update && email_exists( $user->user_login ) ) {
   $errors->add( 'user_login', __( '<strong>ERROR</strong>: This username is invalid because it is already in use as email address of another account.' ) );
}

File: wp-includes/user.php
Function: register_new_user

Addition:

<?php
if ( $user_email == '' ) {
   ...
} elseif ( ! is_email( $user_email ) ) {
   ...
} elseif ( email_exists( $user_email ) ) {
   ...
} elseif ( email_exists( $sanitized_user_login ) ) {
   $errors->add( 'email_exists', __( '<strong>ERROR</strong>: This username is invalid because it is already in use as email address of another account.' ) );
}

I think adding the check into the core itself will prevent additional filter and functions.

Please let me know if this makes some sense.

[subrataemfluence]

Uploading two proposed patches. Please let me know if they make sense!

[subrataemfluence]

File path: /wp-admin/includes/user.php

[subrataemfluence]

File path: /wp-includes/user.php

[phillipburger]

To me the solutions look good, but I definitely want some other people to input here since I have never been involved in any changes to core WP.

Also, what are thoughts on situations where this already exists in a deployment that is upgraded to a version where this is included? The usernames that match email may already be in place, should there be some place that has a small warning about potential overlaps and allow the admin to correct the situation? I know that could be hard since usernames are not editable and typically an email address is not going to be something someone will change on an account.

Last thought for now - because these checks are only checking for existing data in the user portion of the database, if someone creates an account where username and email address are the same - that is still acceptable and will pass these checks, right?

[phillipburger]

What happens next with this process? Who actually does the review and decides if it goes into Core?

[phillipburger]

Does anyone know what the next step is on tickets like this?

[garrett-eclipse]

Thanks @phillipburger & @subrataemfluence for working on this.

I came across this ticket while reviewing #44683 and definitely find it odd that emails can be used as usernames.

The initial checks being worked on here are a great help and will help reduce confusion and potential issues in other areas like the Privacy tools which has a single input for username or email which brings light to this issue all the more.

In my opinion usernames should block the use of emails as usernames. It creates potential issues as have been noted here.
Barring simply prohibiting the use of emails as usernames then they should be restricted to be an exact match of the users email and there should be constraints put into the profile update and user edits to ensure it's a match or that the username is not an email.

I've marked this for additional feedback but would love to see emails being prohibited as usernames or at least forced to be an exact match.