WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 2 years ago

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

Download all attachments as: .zip

Change History (28)

#1 @wpkuf
5 years ago

  • Component changed from General to Login and Registration

#2 @wpkuf
5 years ago

  • Keywords needs-testing added

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

Trunk patch, edit of wp-login.php

#6 @bibliofille
3 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
3 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
3 years 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
3 years ago

Patch that works-for-me

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

#11 @desrosj
2 years ago

#46644 was marked as a duplicate.

@nsubugak
2 years ago

Added Unit Test for this Patch.

#12 @nsubugak
2 years ago

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

#13 @adhitya03
2 years ago

  • Keywords has-patch added

#14 @sncoker
2 years 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
2 years ago

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

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

#16 @cafenoirdesign
2 years ago

  • Keywords needs-refresh removed

Refreshed the patch, tests are passing.

#17 @whyisjake
2 years 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
2 years ago

Cleaned up the commenting a little.

#19 @SergeyBiryukov
2 years ago

  • Milestone changed from Future Release to 5.4

#20 @whyisjake
2 years 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
2 years 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
2 years ago

@SergeyBiryukov 🙇‍♂️

Note: See TracTickets for help on using tickets.