wp_update_user() allows using existing user_email, user_login, and user_nicename

[danielbachhuber]

When updating a user with wp_update_user() (which, in turn, uses wp_insert_user()), it's possible to set the user_email or user_nicename to those of an existing user. It's also possible to supply user_login, which gets silently discarded.

Each of these values need to be considered unique per user. If the values were supplied and not unique, I'd expect WP_Error to be returned.

Here's the relevant faulty logic:

if ( ! $update && username_exists( $user_login ) ) {
	return new WP_Error( 'existing_user_login', __( 'Sorry, that username already exists!' ) );
}

if ( ! $update && ! defined( 'WP_IMPORTING' ) && email_exists( $user_email ) ) {
	return new WP_Error( 'existing_user_email', __( 'Sorry, that email address is already used!' ) );
}

Looks like this was introduced in r12778.

Discovered in:

• ​https://github.com/WP-API/WP-API/pull/705
• ​https://github.com/WP-API/WP-API/pull/707

[danielbachhuber]

It would be nice to have a nicename_exists() function, similar to username_exists()

[rittesh.patel]

if ( ! $update && ! defined( 'WP_IMPORTING' ) && email_exists( $user_email ) )

Here $update is playing a critical role. In case of new user, $update will be false hence email_exist condition will be checked resulting in an error saying Sorry, that email address is already used!.

But, when we update an existing user, email_exist will not be checked at all since the very first check for $update will fail itself.

And anyhow email_exist condition should be checked for all the cases. Hence I've removed $update. I'm not sure about WP_IMPORTING hence kept it as it is.

Dev suggestions will be appreciated if I'm missing anything.

[danielbachhuber]

Any patch should have copious unit tests to even be considered.

[rittesh.patel]

Patch updated

[rittesh.patel]

test case for 30647.diff patch

[rittesh.patel]

Find the updated patch and test case for that patch in attachment.

The old version of patch was throwing error for test_update_user check when checked with phpunit, it wasn't updating any detail of user at all when ! $update removed from if condition.

Following is the current code to check for email exist and throw WP_Error accordingly

( ! $update && ! defined( 'WP_IMPORTING' ) && email_exists( $user_email ) )

I have updated it to the following

( ( ! $update || ( ! empty( $old_user_data ) && $user_email !== $old_user_data->user_email ) ) && ! defined( 'WP_IMPORTING' ) && email_exists( $user_email ) )

It will simply first check whether these is update or insert, if it's insert just check for the email exist and if it's update than first check whether email has been changed or not and then check accordingly for email exist.

[SergeyBiryukov]

Replying to danielbachhuber:

It would be nice to have a nicename_exists() function, similar to username_exists()

Related: #31106

[DrewAPicture]

30647.2.diff​ combines the unit tests diff with the patch and cleans up the syntax. Unit tests pass.

Moving for commit consideration.

[wonderboymusic]

In 31963:

When updating the email address for an existing user, make sure the email address is not already in use.

Adds unit tests.

Props rittesh.patel, DrewAPicture.
Fixes #30647.