WordPress.org

Make WordPress Core

Opened 18 months ago

Last modified 16 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: has-patch 3.6-early
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 18 months ago.

Download all attachments as: .zip

Change History (12)

johnjamesjacoby18 months ago

comment:1 nacin18 months ago

  • Milestone changed from 3.5 to Awaiting Review

comment:2 johnjamesjacoby18 months 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 18 months ago by johnjamesjacoby (previous) (diff)

comment:3 scribu18 months ago

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

comment:4 follow-up: miqrogroove18 months 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 SergeyBiryukov18 months ago

  • Keywords 3.6-early added

comment:6 in reply to: ↑ 4 johnjamesjacoby18 months 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.

Version 0, edited 18 months ago by johnjamesjacoby (next)

comment:7 ryan18 months 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 miqrogroove18 months 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: ryan18 months 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: johnjamesjacoby16 months 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 rmccue16 months 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).

Note: See TracTickets for help on using tickets.