WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 6 months ago

#22367 new defect (bug)

Usernames with "@" char are assumed email addresses, causing incorrect look-up in several places

Reported by: johnjamesjacoby Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Users Keywords: needs-patch
Focuses: Cc:

Description

Problem

Usernames containing the "@" character are mistakenly assumed to be email addresses when:

  • wp-login.php - Resetting passwords
  • /wp-admin/user-new.php - Adding an existing user to a site, in multisite
  • /wp-includes/user.php - Searching for a user

Duplicate

  • Create a user with the login "@testing"
  • Verify the account, etc...

Bug in Search

  • Visit: wp-admin/network/users.php - attempt to search for: "@testing"
  • Result: no users found
  • What should happen: find the user

Bug in Add New

  • Visit: wp-admin/wp-admin/user-new.php - attempt to add: "@testing"
  • Result: no users found
  • What should happen: add the user

Bug in Reset Password

  • Visit: wp-login.php - attempt to reset password for: "@testing"
  • Result: retrieve_password() accidentally succeeds, because strpos() check returns 0, which is the correct position of the "@" character. If the username was "testing@" this test would fail

Solution

The attached patch fixes these bugs by using is_email() instead of an strpos() for an @ character.

Attachments (1)

22367.patch (1.4 KB) - added by johnjamesjacoby 2 years ago.

Download all attachments as: .zip

Change History (15)

@johnjamesjacoby2 years ago

comment:1 @nacin2 years ago

  • Milestone changed from 3.5 to Awaiting Review

comment:2 @johnjamesjacoby2 years ago

Reminder that as it stands now, users with "@" in their logins:

  • Cannot reset their passwords (unless @ is the first char).
  • Cannot be added to existing sites.
  • Cannot be found in network user searches.

Whether or not user_login should be allowed to have "@" at all is debatable.

Sad to see this get punted to Awaiting Review. It's an easy win, and fixes several similar, existing, user-facing issues with only 3 lines changed.

Last edited 2 years ago by johnjamesjacoby (previous) (diff)

comment:3 @scribu2 years ago

If it's not a regression from 3.4, it's not that urgent.

comment:4 follow-up: @miqrogroove2 years ago

Some of the above descriptions need to be qualified with the option of simply using e-mail addresses instead of usernames. It sounds like the features are broken, but not unusable.

I encourage setting a 3.6 milestone.

comment:5 @SergeyBiryukov2 years ago

  • Keywords 3.6-early added

comment:6 in reply to: ↑ 4 @johnjamesjacoby2 years ago

Replying to scribu:

If it's not a regression from 3.4, it's not that urgent.

All of these bugs are critical to actual users of the software.

Replying to miqrogroove:

Some of the above descriptions need to be qualified with the option of simply using e-mail addresses instead of usernames.

Painting the house doesn't fix the holes.

Last edited 2 years ago by johnjamesjacoby (previous) (diff)

comment:7 @ryan2 years ago

This probably puts too much faith in is_email(), which has its problems. Maybe look for @ in a position other than 0?

comment:8 @miqrogroove2 years ago

Or fix is_email(). If you need a solution for that, I know Google offers a free one that might be GPL compatible.

comment:9 follow-up: @ryan2 years ago

#17491 for is_email().

It is possible to use a valid email address as a username in single site. Which is more common, a non-email username with @ or an email username?

comment:10 in reply to: ↑ 9 ; follow-up: @johnjamesjacoby2 years ago

Replying to ryan:

It is possible to use a valid email address as a username in single site.

I have a hunch LDAP plugins support and/or enable this.

Which is more common, a non-email username with @ or an email username?

I have no stats, but seems moot since we've allowed users with @'s for this long.

comment:11 in reply to: ↑ 10 @rmccue2 years ago

Replying to johnjamesjacoby:

Replying to ryan:

It is possible to use a valid email address as a username in single site.

I have a hunch LDAP plugins support and/or enable this.

You're definitely correct on that. I've experienced this issue when working with LDAP integration and it required workarounds (basically, replacing @ with another character when syncing from LDAP).

comment:12 @DrewAPicture12 months ago

#28157 was marked as a duplicate.

comment:13 @dd327 months ago

Perhaps the best option here is to optimize for what we think it is, but fall back to login search:

$user = false;
if ( $search contains '@' )
  $user = get_user_by( 'email', $search );
if ( ! $user )
  $user = get_user_by( 'login', $search );

comment:14 @johnbillion6 months ago

  • Keywords needs-patch added; has-patch 3.6-early removed
Note: See TracTickets for help on using tickets.