Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/wp-admin/css/list-tables.css
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,18 @@ p.pagenav {
margin-top: 1px;
}

.column-email .duplicate-email {
margin: 0.3em 0 0;
color: #b32d2e;
}

.column-email .duplicate-email .dashicons {
font-size: 16px;
width: 16px;
height: 16px;
vertical-align: text-bottom;
}

.row-actions {
color: #646970;
font-size: 13px;
Expand Down
81 changes: 81 additions & 0 deletions src/wp-admin/includes/class-wp-users-list-table.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ class WP_Users_List_Table extends WP_List_Table {
*/
public $is_site_users;

/**
* IDs of users on the current page whose email address is also used by another
* user, ignoring letter case. Keys are user IDs.
*
* @since 7.2.0
* @var true[]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @var true[]
* @var array<int, true>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This being said, I'm not sure an associative array is needed here. It could just be list<int>, without any mapping.

*/
protected $duplicate_email_user_ids = array();

/**
* Constructor.
*
Expand Down Expand Up @@ -411,11 +420,77 @@ public function display_rows() {
$post_counts = count_many_users_posts( array_keys( $this->items ) );
}

$this->duplicate_email_user_ids = $this->get_duplicate_email_user_ids( array_keys( $this->items ) );

foreach ( $this->items as $userid => $user_object ) {
echo "\n\t" . $this->single_row( $user_object, '', '', isset( $post_counts ) ? $post_counts[ $userid ] : 0 );
}
}

/**
* Finds which of the given users share an email address with another user,
* ignoring letter case.
*
* Many mailbox providers treat `abc@example.com` and `ABc@example.com` as the
* same mailbox, so such accounts are likely duplicates.
*
* @since 7.2.0
*
* @global wpdb $wpdb WordPress database abstraction object.
*
* @param int[] $user_ids IDs of the users to check.
* @return true[] Array keyed by the IDs of users whose email address is shared.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @return true[] Array keyed by the IDs of users whose email address is shared.
* @return array<int, true> Array keyed by the IDs of users whose email address is shared.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But note above, how this could just return list<int>. An associative array seems unnecessary.

*/
protected function get_duplicate_email_user_ids( $user_ids ) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected function get_duplicate_email_user_ids( $user_ids ) {
protected function get_duplicate_email_user_ids( array $user_ids ): array {

global $wpdb;

$user_ids = array_filter( array_map( 'intval', $user_ids ) );
if ( empty( $user_ids ) ) {
return array();
}

$emails = array();
foreach ( $user_ids as $user_id ) {
$user = get_userdata( $user_id );
if ( $user && '' !== $user->user_email ) {
$emails[ strtolower( $user->user_email ) ] = true;
}
}

if ( empty( $emails ) ) {
return array();
}

$emails = array_keys( $emails );
$placeholders = implode( ',', array_fill( 0, count( $emails ), '%s' ) );

/*
* All users whose email matches, ignoring case, the email of a user on this page.
* With the default case-insensitive collation a plain comparison already ignores
* letter case and can use the user_email index; LOWER() is only needed otherwise.
*/
$email_column = _wp_is_user_email_case_sensitive() ? 'LOWER(user_email)' : 'user_email';

$matches = $wpdb->get_results(
// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare
$wpdb->prepare( "SELECT ID, user_email FROM $wpdb->users WHERE $email_column IN ($placeholders)", $emails )
);

$users_by_email = array();
foreach ( $matches as $match ) {
$users_by_email[ strtolower( $match->user_email ) ][] = (int) $match->ID;
}

$duplicates = array();
foreach ( $users_by_email as $ids ) {
if ( count( $ids ) > 1 ) {
$duplicates += array_fill_keys( $ids, true );
}
}

return array_intersect_key( $duplicates, array_flip( $user_ids ) );
}

/**
* Generates HTML for a single row on the users.php admin panel.
*
Expand Down Expand Up @@ -599,6 +674,12 @@ public function single_row( $user_object, $style = '', $role = '', $numposts = 0
break;
case 'email':
$row .= "<a href='" . esc_url( "mailto:$email" ) . "'>$email</a>";
if ( isset( $this->duplicate_email_user_ids[ $user_object->ID ] ) ) {
$row .= sprintf(
'<p class="duplicate-email"><span class="dashicons dashicons-warning" aria-hidden="true"></span> %s</p>',
__( 'Another user has this email address, possibly with different letter case.' )
);
}
break;
case 'role':
$row .= esc_html( $roles_list );
Expand Down
76 changes: 76 additions & 0 deletions src/wp-includes/user.php
Original file line number Diff line number Diff line change
Expand Up @@ -2134,16 +2134,42 @@ function username_exists( $username ) {
* Conditional Tags} article in the Theme Developer Handbook.
*
* @since 2.1.0
* @since 7.2.0 The comparison is case-insensitive regardless of the database collation.
*
* @global wpdb $wpdb WordPress database abstraction object.
*
* @param string $email The email to check for existence.
* @return int|false The user ID on success, false on failure.
*/
function email_exists( $email ) {
global $wpdb;

$user = get_user_by( 'email', $email );
if ( $user ) {
$user_id = $user->ID;
} else {
$user_id = false;

/*
* Most mailbox providers treat the local part of an address as case-insensitive.
* The default users table collation already compares emails that way, so the
* lookup above has covered it. Only when the column compares case-sensitively
* (or the collation is unknown) fall back to an explicit case-insensitive lookup.
* That lookup cannot use the user_email index, so it is avoided where possible.
*/
$trimmed_email = trim( (string) $email );
if ( '' !== $trimmed_email && _wp_is_user_email_case_sensitive() ) {
$found_id = $wpdb->get_var(
$wpdb->prepare(
"SELECT ID FROM $wpdb->users WHERE LOWER(user_email) = LOWER(%s) LIMIT 1",
$trimmed_email
)
);

if ( $found_id ) {
$user_id = (int) $found_id;
}
}
}

/**
Expand All @@ -2158,6 +2184,56 @@ function email_exists( $email ) {
return apply_filters( 'email_exists', $user_id, $email );
}

/**
* Determines whether the database compares user email addresses case-sensitively.
*
* The default collation of the users table is case-insensitive, in which case
* a plain comparison already matches email addresses that differ only in letter
* case, and can use the user_email index. Case-sensitive collations such as
* `*_bin` or `*_cs` need an explicit LOWER() comparison instead.
*
* When the collation cannot be determined, for example on a non-MySQL database,
* the comparison is assumed to be case-sensitive.
*
* @since 7.2.0
* @access private
*
* @global wpdb $wpdb WordPress database abstraction object.
*
* @return bool True if user email comparisons are case-sensitive, false otherwise.
*/
function _wp_is_user_email_case_sensitive() {
global $wpdb;

static $is_case_sensitive = array();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this an array when it only ever has one item in it?


if ( ! isset( $is_case_sensitive[ $wpdb->users ] ) ) {
$result = true;

if ( $wpdb->is_mysql ) {
$column = $wpdb->get_row( "SHOW FULL COLUMNS FROM $wpdb->users LIKE 'user_email'" );

if ( $column && ! empty( $column->Collation ) ) {
$result = ! str_ends_with( strtolower( $column->Collation ), '_ci' );
}
}

$is_case_sensitive[ $wpdb->users ] = $result;
}

/**
* Filters whether user email comparisons are case-sensitive in the database.
*
* When true, email lookups that need to ignore letter case use LOWER(), which
* cannot use the user_email index.
*
* @since 7.2.0
*
* @param bool $is_case_sensitive Whether the user_email column compares case-sensitively.
*/
return (bool) apply_filters( 'wp_is_user_email_case_sensitive', $is_case_sensitive[ $wpdb->users ] );
}

/**
* Checks whether a username is valid.
*
Expand Down
88 changes: 88 additions & 0 deletions tests/phpunit/tests/admin/wpUsersListTable.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,92 @@ public function test_get_views_should_return_views_by_default() {

$this->assertSame( $expected, $this->table->get_views() );
}

/**
* @ticket 66238
*
* @covers WP_Users_List_Table::get_duplicate_email_user_ids
*
* @dataProvider data_email_case_sensitivity
*
* @param bool $is_case_sensitive Whether the database compares user emails case-sensitively.
*/
public function test_get_duplicate_email_user_ids_flags_case_variants( $is_case_sensitive ) {
global $wpdb;

$original = self::factory()->user->create( array( 'user_email' => 'abc@example.com' ) );
$variant = self::factory()->user->create( array( 'user_email' => 'placeholder@example.com' ) );
$unique = self::factory()->user->create( array( 'user_email' => 'unique@example.com' ) );

// Duplicates can no longer be created through the API, so simulate legacy data.
$wpdb->update( $wpdb->users, array( 'user_email' => 'ABc@example.com' ), array( 'ID' => $variant ) );
clean_user_cache( $variant );

add_filter( 'wp_is_user_email_case_sensitive', $is_case_sensitive ? '__return_true' : '__return_false' );

$method = new ReflectionMethod( $this->table, 'get_duplicate_email_user_ids' );
if ( PHP_VERSION_ID < 80100 ) {
$method->setAccessible( true );
}

$queries = array();
$collect = static function ( $query ) use ( &$queries ) {
$queries[] = $query;
return $query;
};
add_filter( 'query', $collect );

$flagged = $method->invoke( $this->table, array( $original, $unique ) );

remove_filter( 'query', $collect );

$this->assertSame( array( $original ), array_keys( $flagged ), 'Only the user sharing an address should be flagged, even when the other account is not on the page.' );

$lower_queries = preg_grep( '/LOWER\(user_email\)/', $queries );
if ( $is_case_sensitive ) {
$this->assertNotEmpty( $lower_queries, 'A case-sensitive collation should use a LOWER() comparison.' );
} else {
$this->assertEmpty( $lower_queries, 'A case-insensitive collation should not use LOWER(), so that the index can be used.' );
}
}

/**
* Data provider.
*
* @return array[]
*/
public function data_email_case_sensitivity() {
return array(
'case-insensitive collation' => array( false ),
'case-sensitive collation' => array( true ),
);
}

/**
* @ticket 66238
*
* @covers WP_Users_List_Table::display_rows
*/
public function test_display_rows_highlights_duplicate_email() {
global $wpdb;

$original = self::factory()->user->create( array( 'user_email' => 'abc@example.com' ) );
$variant = self::factory()->user->create( array( 'user_email' => 'placeholder@example.com' ) );
$unique = self::factory()->user->create( array( 'user_email' => 'unique@example.com' ) );

$wpdb->update( $wpdb->users, array( 'user_email' => 'ABc@example.com' ), array( 'ID' => $variant ) );
clean_user_cache( $variant );

$this->table->items = array(
$original => get_userdata( $original ),
$unique => get_userdata( $unique ),
);

ob_start();
$this->table->display_rows();
$output = ob_get_clean();

$this->assertSame( 1, substr_count( $output, 'class="duplicate-email"' ), 'Exactly one row should be highlighted.' );
$this->assertMatchesRegularExpression( "#<tr id='user-{$original}'.*?class=\"duplicate-email\".*?</tr>#s", $output, 'The duplicate user row should be highlighted.' );
}
}
Loading
Loading