WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 5 months ago

Last modified 5 months ago

#38744 closed defect (bug) (fixed)

Can't login with email address that contains an apostrophe

Reported by: wpkuf Owned by: whyisjake
Milestone: 5.4 Priority: normal
Severity: normal Version:
Component: Login and Registration Keywords: good-first-bug dev-feedback has-unit-tests has-patch
Focuses: Cc:

Description


  • Test case :
  1. Create a user with email address that contains an apostrophe. For example you can create this user from the Dashboard. Example : testemailaddress'@…
  2. Try to login submitting this email address and valid password on /wp-login.php page.

Expected result

User is logged in

Actual result

User isn't logged in. Error invalid_username

If WordPress allows to create user with such email, then it should also allow to login with it. It seems to be correct.

So the question is to allow to login with similar emails or restrict them at all.

Attachments (6)

38744.diff (464 bytes) - added by bibliofille 12 months ago.
Trunk patch, edit of wp-login.php
38744-fixes.diff (2.1 KB) - added by santilinwp 11 months ago.
Patch that works-for-me
38744-fixes.2.diff (1.9 KB) - added by santilinwp 11 months ago.
Please don't use the former one, this one is well formatted and in the other one there was an unnoticed change to .gitignore
38744-fixes.3.diff.patch (3.4 KB) - added by nsubugak 9 months ago.
Added Unit Test for this Patch.
38744.2.diff (3.5 KB) - added by cafenoirdesign 5 months ago.
38744.3.diff (1.0 KB) - added by cafenoirdesign 5 months ago.

Download all attachments as: .zip

Change History (28)

#1 @wpkuf
3 years ago

  • Component changed from General to Login and Registration

#2 @wpkuf
3 years ago

  • Keywords needs-testing added

#4 @desrosj
12 months ago

  • Keywords needs-patch good-first-bug added; needs-testing removed
  • Milestone changed from Awaiting Review to Future Release
  • Version 4.6.1 deleted

This looks to still be an issue. The registration forms (both in the admin and wp-login.php) and password reset form both allow apostrophes in emails. The login form does not.

#5 @socalchristina
12 months ago

I'd like to try to help out on this one @wpkuf. I'll read through the history and review the source then get started. I'd also like to know if @desrosj and @zodiac1978 have any additional input or comments.

Looking forward to contributing.

@bibliofille
12 months ago

Trunk patch, edit of wp-login.php

#6 @bibliofille
12 months ago

Newbie contributor here! Not sure if I'm on the right track, but made some adjustments to wp-login.php. Dug through the suggested related issue https://core.trac.wordpress.org/ticket/34483 and took a stab at it.

#7 @socalchristina
12 months ago

Hi @bibliofille! I'm a newbie too. I just returned from work and was about to get started on this. Nice work!

#8 @santilinwp
11 months ago

  • Keywords has-patch needs-testing needs-unit-tests added; needs-patch removed
  • Resolution set to worksforme
  • Status changed from new to closed

I have found the using of $_POST['user_login'] without wp_unslash in a number of places, so I have gone through all the uses I could find and fixed it.

I have not found any tests of the login process as I am a newbie. I would appreciate if someone could point me out to them.

I have attached the diff with three changes. After applying them, I can now login with an email name with an ' in it.

@santilinwp
11 months ago

Patch that works-for-me

@santilinwp
11 months ago

Please don't use the former one, this one is well formatted and in the other one there was an unnoticed change to .gitignore

#9 follow-up: @SergeyBiryukov
11 months ago

  • Resolution worksforme deleted
  • Status changed from closed to reopened

Hi @santilinwp, thanks for the patch! Tickets should only be closed once the change is committed to WordPress source.

#10 in reply to: ↑ 9 @santilinwp
11 months ago

Replying to SergeyBiryukov:

Hi @santilinwp, thanks for the patch! Tickets should only be closed once the change is committed to WordPress source.

Ok, sorry, somehow I understood I had to change the status :)

Last edited 11 months ago by santilinwp (previous) (diff)

#11 @desrosj
10 months ago

#46644 was marked as a duplicate.

@nsubugak
9 months ago

Added Unit Test for this Patch.

#12 @nsubugak
9 months ago

  • Keywords dev-feedback has-unit-tests added; has-patch needs-testing needs-unit-tests removed

#13 @adhitya03
8 months ago

  • Keywords has-patch added

#14 @sncoker
5 months ago

  • Keywords needs-refresh added

At #WCUS Contributor Day. The latest patch no longer applies cleanly to trunk. Marking this as needs-refresh.

#15 @whyisjake
5 months ago

  • Owner set to whyisjake
  • Status changed from reopened to accepted

Thanks for flagging @sncoker. Working on this with @cafenoirdesign.

#16 @cafenoirdesign
5 months ago

  • Keywords needs-refresh removed

Refreshed the patch, tests are passing.

#17 @whyisjake
5 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 46640:

Login and Registration: Allow email logins to be more flexible.

Allows a login to have an apostorphe. Which would normally be created as a mistake, but this allows the login to happen.

Fixes #38744
Props wpkuf, desrosj, socalchristina, bibliofille, santilinwp, nsubugak, sncoker, cafenoirdesign, whyisjake.

#18 @cafenoirdesign
5 months ago

Cleaned up the commenting a little.

#19 @SergeyBiryukov
5 months ago

  • Milestone changed from Future Release to 5.4

#20 @whyisjake
5 months ago

In 46643:

Coding Standards: Clean up the tests around test_that_you_can_login_with_an_email_that_has_apostrophe.

Let's use the proper coding standards for the comments.

Fixes #38744.
Props cafenoirdesign.

#21 @SergeyBiryukov
5 months ago

In 46650:

Login and Registration: Simplify the test for wp_signon() added in [46640].

Make sure it actually tests the change in behavior, previously it passed both before and after the patch.

Add wp_unslash() to the last remaining instance of $_POST['user_login'] that didn't have it.

See #38744.

#22 @whyisjake
5 months ago

@SergeyBiryukov 🙇‍♂️

Note: See TracTickets for help on using tickets.