WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 16 months ago

Last modified 16 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 2 years ago.
Trunk patch, edit of wp-login.php
38744-fixes.diff (2.1 KB) - added by santilinwp 23 months ago.
Patch that works-for-me
38744-fixes.2.diff (1.9 KB) - added by santilinwp 23 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 21 months ago.
Added Unit Test for this Patch.
38744.2.diff (3.5 KB) - added by cafenoirdesign 16 months ago.
38744.3.diff (1.0 KB) - added by cafenoirdesign 16 months ago.

Download all attachments as: .zip

Change History (28)

#1 @wpkuf
4 years ago

  • Component changed from General to Login and Registration

#2 @wpkuf
4 years ago

  • Keywords needs-testing added

#4 @desrosj
2 years 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
2 years 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
2 years ago

Trunk patch, edit of wp-login.php

#6 @bibliofille
2 years 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
2 years 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
23 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
23 months ago

Patch that works-for-me

@santilinwp
23 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
23 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
23 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 23 months ago by santilinwp (previous) (diff)

#11 @desrosj
21 months ago

#46644 was marked as a duplicate.

@nsubugak
21 months ago

Added Unit Test for this Patch.

#12 @nsubugak
21 months ago

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

#13 @adhitya03
20 months ago

  • Keywords has-patch added

#14 @sncoker
16 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
16 months ago

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

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

#16 @cafenoirdesign
16 months ago

  • Keywords needs-refresh removed

Refreshed the patch, tests are passing.

#17 @whyisjake
16 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
16 months ago

Cleaned up the commenting a little.

#19 @SergeyBiryukov
16 months ago

  • Milestone changed from Future Release to 5.4

#20 @whyisjake
16 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
16 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
16 months ago

@SergeyBiryukov 🙇‍♂️

Note: See TracTickets for help on using tickets.