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 IDs are case-sensitive, allowing multiple accounts with one handle #5643

Closed
divyam-goel opened this issue Feb 23, 2019 · 14 comments · Fixed by #5728

Comments

@divyam-goel
Copy link
Contributor

@divyam-goel divyam-goel commented Feb 23, 2019

Describe the bug
Currently, email id is stored in the database as raw string i.e. as entered by the user and then during auth the user is expected to enter the exact case-sensitive email id. This can lead to creation of multiple accounts with one email id and is a major bug.

To Reproduce
Steps to reproduce the behavior:

  1. Go to login
  2. Go to create account
  3. Enter email id and password
  4. Click Register
  5. Again go to Create Account
  6. Enter the same email id and password, however this time
    change the casing of some letters of email
  7. Click Register
  8. You would have successfully created two different accounts with the same email id

Expected behavior
When we try to create the second account, it should give an error: Email already registered.

Additional context
I am working on it.

@divyam-goel

This comment has been minimized.

Copy link
Contributor Author

@divyam-goel divyam-goel commented Feb 24, 2019

@iamareebjamal @CosmicCoder96 this change will have to be made at multiple places. Essentially everytime we take email id as input from the user we will have to lowercase it for consistency in the database. Can you suggest what all places should I look into to fix the issue?

@iamareebjamal

This comment has been minimized.

Copy link
Member

@iamareebjamal iamareebjamal commented Feb 24, 2019

The only place it needs to change is registration

@divyam-goel

This comment has been minimized.

Copy link
Contributor Author

@divyam-goel divyam-goel commented Feb 24, 2019

@iamareebjamal I will have to make the following changes if we want to allow the users to enter the 'email' in any 'case'.

Changes for Email consistency:

app/api/users.py

UserList - before_create_object (change)
UserDetail - before_update_object (change)
is_email_available (change)

app/api/events.py

EventList - before_create_object (add)
EventDetail - before_update_object (change)

app/api/speakers.py

SpeakerListPost - before_create_object (add)
SpeakerDetail - before_update_object (change)

app/api/auth.py

resend_verification_email (change)
reset_password - method:POST (change)

app/api/user_emails.py

UserEmailListPost - before_create_object (add)

app/api/role_invite.py

RoleInvitePost - before_create_object (add)

app/api/helpers/jwt.py

jwt_authenticate (change)

Note: The add/change written in the bracket refers to whether I will be changing the method or adding it.

Can you please review this once and give me a go ahead if you are satisfied with the proposed changes?

@iamareebjamal

This comment has been minimized.

Copy link
Member

@iamareebjamal iamareebjamal commented Feb 24, 2019

Just create an index on email fields to be unique lower

CREATE INDEX idx_groups_email ON groups lower(email);
@divyam-goel

This comment has been minimized.

Copy link
Contributor Author

@divyam-goel divyam-goel commented Feb 24, 2019

This is a good solution however, I feel it will miss some cases. For example: while logging in, if the user entered the email in a different case then the might fail. Moreover, the data in database won't be as consistent, I think it's better if all the emails are stored in lowercase. Therefore, I think the best way to solve this issue might be that whenever we take email as input from the user we should convert it to lowercase before we do any further processing on it.

@iamareebjamal

This comment has been minimized.

Copy link
Member

@iamareebjamal iamareebjamal commented Feb 24, 2019

while logging in, if the user entered the email in a different case then the might fail.

Why will it fail?

Therefore, I think the best way to solve this issue might be that whenever we take email as input from the user we should convert it to lowercase before we do any further processing on it.

How does your solution work on pre-existing data? And ensures on database level that data will not be inconsistent

@divyam-goel

This comment has been minimized.

Copy link
Contributor Author

@divyam-goel divyam-goel commented Feb 26, 2019

I will explain what I am trying to say with an example:

Example

During Registration

email entered by the user: "EXAMPLE@EXAMPLE.COM"

Database

email stored in the database: "EXAMPLE@EXAMPLE.COM"
email in the index: "example@example.com"

During Logging In

email entered by the user: "EXAMPLE@example.com"

Problem

The email id entered by the user during logging in, does not match the email in database or the lowercase email on which index is constructed.

Additional Problem

Also, while coming up with a solution we will also have to keep in mind that there might be existing mixed-case emails in the database.

Solution

Every time user enters email, we convert it to lowercase and then store it in the database or do any further processing on it (like querying the database).

Auxiliary Solution

We will also construct the index (as you mentioned) on lowercase emails to avoid the clashed with existing mixed-case emails.

@iamareebjamal

This comment has been minimized.

Copy link
Member

@iamareebjamal iamareebjamal commented Feb 26, 2019

We need a solution which works for previous emails and prevents inconsistency on database level

@divyam-goel

This comment has been minimized.

Copy link
Contributor Author

@divyam-goel divyam-goel commented Feb 26, 2019

My plan was to use both the things mentioned below.

Solution

Every time user enters email, we convert it to lowercase and then store it in the database or do any further processing on it (like querying the database).

This will prevent future inconsistency.

Auxiliary Solution

We will also construct the index (as you mentioned) on lowercase emails to avoid the clashed with existing mixed-case emails.

This will make sure the system works for previously existing emails in database.
I think this will solve the issue.

@iamareebjamal

This comment has been minimized.

Copy link
Member

@iamareebjamal iamareebjamal commented Feb 26, 2019

OK

@uds5501

This comment has been minimized.

Copy link
Contributor

@uds5501 uds5501 commented Mar 10, 2019

@divyam-goel if you have not invested much time on this enhancement, can I take over?

@prateekj117

This comment has been minimized.

Copy link
Member

@prateekj117 prateekj117 commented Mar 27, 2019

@divyam-goel @uds5501 Anyone working on this issue?

@uds5501

This comment has been minimized.

Copy link
Contributor

@uds5501 uds5501 commented Mar 27, 2019

@prateekj117 No, I am not, you should probably take over

@prateekj117

This comment has been minimized.

Copy link
Member

@prateekj117 prateekj117 commented Mar 27, 2019

@uds5501 Ok.

@prateekj117 prateekj117 mentioned this issue Mar 28, 2019
3 of 6 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.