wp_insert_user allows the new user to have a username equal to an already registered email

[buutqn]

Scenario:

User A is an administrator, has username = 'admin' and email = 'admin@…';
User B registers (open to register wp install) with username = 'admin@…' and email = 'test@…';

In my case, i use both users, one to admin the website, and the other to simulate an customer user. Both users had same password.

When i tried to log in with administrator user by email, i end up logged in the as user B.

Then i changed User B password to not conflict, and it works as it should, if i set user A password it gets logged in as user A and if i use User B password it gets logged in as User B.

I don't think this is a security issue, but allowing an user to be registered with another users email as username could be annoying and confuse some website administrators for scams or something else.

To solve this issue, i just filtered username_exists filter and checked if the username was existing email.

public function __construct() {
    add_filter("username_exists", array($this, "username_exists"));
}

public function username_exists($user_id, $username) {
    if (email_exists($username)) {
        $user = get_user_by("email", $username);
        if ($user->exists()) {
            return $user->ID;
        }
    }
    return $user_id;
}

[dunhakdis]

I was able to reproduce the same behaviour. I think it should throw a WP_Error if the supplied username already exists as an email of an existing user.

The same behaviour can be reproduced with wp-login.php?action=register form.

Added a patch.

[dunhakdis]

Added check to determine if username supplied already exists as an email of an existing user.

---
Trac ticket: https://core.trac.wordpress.org/ticket/57394

@mukeshpanchal27 Thanks. I've applied your suggestions as commit. Cheers

[dunhakdis]

Updated diff from @mukeshpanchal27 comments

[roytanck]

A possible privacy concern could be that this would allow a bad actor to try entering email addresses until it hits the new error message (assuming an open registration form). They would then know that that address is in use.

[dunhakdis]

I think that is also true for the default WordPress registration form with or without these changes.

Hi @adamsilverstein, @JosephGabito I added a unit test for this fix.

[ajayver]

Created a unit test that attempts to create a new user with admin email as a username.

[dunhakdis]

@ajayver , Thanks for adding a unit-test.

Trac ticket: https://core.trac.wordpress.org/ticket/57394

[ajayver]

This patch includes both the fix and unit tests and the code standards fix.

Fixed the failing code standards issue.

[adamsilverstein]

In 55358:

Login and Registration: prevent registering with username that matches previous user email.

When registering a new user, check that no existing user has an email matching the username.

Prevents a login name collision when one user registers with the email address user@… and a second user tries to register with the username user@….

Props buutqn, dunhakdis, roytanck, ajayver.
Fixes #57394.

Trac ticket: https://core.trac.wordpress.org/ticket/57394

[SergeyBiryukov]

In 55360:

Users: Correct the error code in wp_insert_user() when login matches an existing email.

Move the test next to the other tests for user_login.

Follow-up to [55358].

See #57394.

This patch improves the logic around the username checks added in WP 6.2, to make sure the check doesn't cause issues when updating a user.

Trac ticket: https://core.trac.wordpress.org/ticket/57967

Thanks @costdev, applied and updated

[audrasjb]

Reopening this ticket as changesets [55358] and [55360] are probably going to be reverted, due to an issue reported in ticket #57967.

Wondering if this PR should be repurposed or a new one opened to implement the logic discussed in ​https://github.com/WordPress/wordpress-develop/pull/4257#discussion_r1144203722 and the following comments.

[hellofromTonya]

@audrasjb are you still planning to commit the reverts?

[audrasjb]

In 55584:

Login and Registration: Revert [55358] and [55360].

This reverts the changes implemented in [55358] and [55360].

Changeset [55358] was committed to prevent login name collision when one user registers with the email address user@example.com and a second user tries to register with the username user@example.com. However, it also introduced a potential backward compatibility issues for plugins that use wp_update_user(). When updating an existing user, it throws an existing_user_email_as_login error if the email address is also used for the user login, due to the code introduced in [55358].

This changeset removes the new scenario added in [55358] and [55360], restoring the wp_insert_user() function back to its previous state.

Props polevaultweb, audrasjb, costdev, peterwilsoncc, hellofromTonya, SergeyBiryukov, azaozz.
See #57967, #57394.

[hellofromTonya]

Marking for double committer sign-off.

[hellofromTonya]

[55584] is ready for backport to 6.2 branch.

[audrasjb]

In 55585:

Login and Registration: Revert [55358] and [55360].

This reverts the changes implemented in [55358] and [55360].

Changeset [55358] was committed to prevent login name collision when one user registers with the email address user@example.com and a second user tries to register with the username user@example.com. However, it also introduced a potential backward compatibility issues for plugins that use wp_update_user(). When updating an existing user, it throws an existing_user_email_as_login error if the email address is also used for the user login, due to the code introduced in [55358].

This changeset removes the new scenario added in [55358] and [55360], restoring the wp_insert_user() function back to its previous state.

Props polevaultweb, audrasjb, costdev, peterwilsoncc, hellofromTonya, SergeyBiryukov, azaozz.
Reviewed by hellofromTonya.
Merges [55584] to the 6.2 branch.
See #57967, #57394.

[hellofromTonya]

The changesets have been reverted. Additional considerations are needed to address the issue raised #57967 and potentially other scenarios.

With 6.2 Dry Run and final release next week, it's too late in the cycle to continue working on this. Moving to 6.3.

[agnieszkaszuba]

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/ticket/57394

Environment

• OS: macOS 13.0.1
• Web Server: Nginx
• PHP: 7.4.24
• WordPress: 6.3-alpha-55505-src
• Browser: Chrome 109.0.5414.119 (Official Build) (x86_64)
• Theme: Twenty Twenty-Three
• Active Plugins: -

Actual Results

• ✅ Issue resolved with patch.

Additional Notes

Supplemental Artifacts