Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Email is case-sensitive #881

Closed
x00x70 opened this issue May 9, 2018 · 10 comments

Comments

@x00x70
Copy link
Contributor

commented May 9, 2018

Currently it is possible to create to user accounts with the same email address, as long as the email addresses have different cases. For example, a user can register with "bob@example.com" and another user can register with "BOB@example.com" and this is allowed by the userfrosting application. I have tested this with PostgreSQL - but I assume this will be the case with all databases as bob@hotmail.com != BOB@hotmail.com for database string uniqueness.

I suggest the email address is either converted to lowercase at user input points, e.g. registration, the login form, password reset etc. and the email is converted to lowercase when storing in the database.

I originally found this to be an issue, when a user signed up, using an email with the first letter capitalized, however they then could not login with their lowercase email.

@alexweissman

This comment has been minimized.

Copy link
Member

commented May 9, 2018

I assume this will be the case with all databases as bob@hotmail.com != BOB@hotmail.com for database string uniqueness

I tested this with my local MariaDB installation, and it correctly gave me the "email address already in use" when I used variances of an email address with different capitalizations. So, I believe this is an issue specific to Postgres.

I don't like the solution of normalizing the user's email address to be all lowercase. Users might have a specific way of expressing their email address that makes sense to them (recall the whole expertsexchange vs ExpertsExchange confusion) that we should try to preserve.

Clearly though, we need some solution if Postgres behaves this way.

@lcharette

This comment has been minimized.

Copy link
Member

commented May 9, 2018

They could be stored in their user defined form in the db, but compared using lowercase. MySQL/MariaDB does support the Lower() function. Not sure if other db support this.

Maybe Eloquent can help also with some sort of mutator/acessor magic ?

@x00x70

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2018

Related to this : laravel/framework#9430

@lcharette

This comment has been minimized.

Copy link
Member

commented May 9, 2018

Hum... So if I understand correctly, another way to fix this might be to store both original and lowercase email?

this makes me wonder, could we have the same issue with username ?

@JohnSlaughter

This comment has been minimized.

Copy link

commented May 9, 2018

They need to be made lowercase. Either in the DB or in PHP, but email addresses should NEVER be case sensitive. (Passwords, yes, but never email.)

The same applies to domains. GoDaddy.com still goes to godaddy.com because we should be smart enough to know they are identical. Email addresses should be too.

@sinfuljosh

This comment has been minimized.

Copy link

commented Jun 12, 2018

Regarding the php method, if that route was used then there would be no changes on the scheme or in the model.
You would need to apply php’s strtolower function to both the value pulled from the database, and the value submitted by the user prior to comparing.

Once validated the the actual value existing in the database would be used when getting the users email.

The only issue I see here is that strtolower by itself does not handle multi byte characters, any character like those that include accents would be passed through with whichever case they were submitted as.

There is an mb_strtolower that accounts for that , but from reading it appears to be a big performance impact.

http://php.net/manual/en/function.strtolower.php

@Silic0nS0ldier

This comment has been minimized.

Copy link
Member

commented Jan 15, 2019

Plenty of systems out there that process unicode inconsistently, so I think its best to not put the normalisation onto the database for this. It is also worth looking into how email services (or web browsers) see unicode as any normalisation made could potentially produce a different email address.

@amosfolz

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

It seems the consensus is to use PHP to accomplish this. Is there an existing class it would make sense to add a method to?

Right now findUnique is used to check if a username or email already exists.

As we need to do the same thing to both the data submission from user and the database records, would it be better to use two methods or try to combine everything into one?

Maybe a findUniqueCaseInsensitive method inside the Model class, and then another method somewhere else for the form submission? (or possibly a transformation)?

@x00x70

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

given that this has so far just been a postgres issue - we could just put in a DB fix for postgres setup's using the CITEXT field. https://bambielli.com/til/2016-12-28-postgres-extensions-citext/

I don't think this would be an optimal solution, but it would at least give consistent behaviour between postgres and mariaDB....

@lcharette lcharette added this to the 4.3.0 milestone Jul 6, 2019

@lcharette

This comment has been minimized.

Copy link
Member

commented Jul 13, 2019

Done in PR #1012

@lcharette lcharette closed this Jul 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.