Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

emails are incorrectly case sensitive in account create #455

Closed
edwindotcom opened this issue Dec 19, 2013 · 14 comments
Closed

emails are incorrectly case sensitive in account create #455

edwindotcom opened this issue Dec 19, 2013 · 14 comments

Comments

@edwindotcom
Copy link

  1. create an account with user@hotmail.com
  2. attempt to create acct with user@HOTmail.com or User@hotmail.com or user@hotmail.coM

actual: user is created, you get UUID

expected: you should get
{"code":400,"error":"Bad Request","message":"Account already exists","info":"https://github.com/mozilla/fxa-auth-server/blob/master/docs/api.md#response-format","errno":101,"email":"ed@hotmail.com","log":[{"op":"Account.create","rid":"1387491430481-45014-59836"}

@dannycoates
Copy link
Contributor

Since email is used on the client as part of the key stretching it will be important for all clients to normalize the email before stretching so that user input isn't case dependent. The server might be able to verify whether the email was normalized, but normalization can't only be done on the server. Since that's the case I think we need to be very specific about how its done especially since we allow unicode characters. I know it gets complicated quickly.

@warner @ncalexan any insights?

@ncalexan
Copy link
Member

ncalexan commented Jan 3, 2014

Ugh, this is gnarly. I can think of at least the following options:

1a. specify the unicode normalization procedure;
1b. and specify the email normalization procedure (like a.b+blah@gmail.com getting mapped to ab@gmail.com);
2. have the key-stretching not include the email;
3. have the server normalize and return the normalized email address for future use.

1 is hard. 2 weakens security. 3. might require server round-trips, might enable account lockouts, and might confuse users when they see different emails in different places. Gnarly.

1 might be right. Looking at what's easy to do on Android, I see http://docs.oracle.com/javase/7/docs/api/java/lang/String.html#toLowerCase%28%29 and http://docs.oracle.com/javase/7/docs/api/java/util/Locale.html; but http://docs.oracle.com/javase/tutorial/i18n/text/normalizerapi.html looks better.

Looking for Javascript, I see http://norbertlindenberg.com/2013/10/javascript-internationalization/Internationalizing%20JavaScript%20Applications.pdf which points to https://github.com/walling/unorm.

My uneducated vote is for using this standard normalization procedure and not normalizing email at all. Surely mozillians.org has the same issue; we should reach out to them.

@ncalexan
Copy link
Member

ncalexan commented Jan 3, 2014

mozillians.org uses Persona. Perhaps we can do the same as Persona, and just document what it does. It might just call some Node.js normalizing function, though :(

@warner
Copy link
Contributor

warner commented Jan 3, 2014

Ugh. If we really had to, we could have the server return a per-normalized-email salt before the client did any stretching, but it'd add a roundtrip, and would give a malicious server an extra attack (give the client a fixed salt for which they had a pre-computed dictionary, bypassing the benefits of a salt).

/me hates normalization, we hates it, yess, yesss.

If we do go for normalization, I'd vote against normalizing extension addresses away. Even just for testing, I use them all the time, and I expect them to map to different accounts. I suspect we've got plenty of "normal" users who expect the same.

Can we get away without this? What about the following:

  • use original un-normalized uSEr@doMAin for salt
  • store both un-normalized and normalized addresses in the DB
  • permit only one account per normalized address
  • upon conflict (creating an account that already exists, or logging into a non-existent account for which the normalized form of the address does exist), return an error that says "case matters, please use X instead", and have the client type it in again?

@warner
Copy link
Contributor

warner commented Jan 3, 2014

Chatting in the office just now, we came up with the following variant on my proposal:

  • if a client submits address Y and receives the "case matters, please use X instead" error, it normalizes both X and Y and makes sure they map to the same thing
  • if so, it automatically re-tries with X
  • if not, it shows Y and X to the user: something like "you tried to use Y, I don't have an account by that name, but there is one called X. Want to try that instead?", and re-displays the email-entry field for them (probably with Y still filled in)
  • if re-trying with X still fails, show the usual "unknown account" error

The norm(X)!=norm(Y) case could happen if the client's (old?) rules for normalization no longer match the server's.

@dannycoates
Copy link
Contributor

As of #466 we have a functional solution (#463) in master.

@edwindotcom
Copy link
Author

@warner just to clarify, when you say 'client' you mean Fx Desktop/Android/FxOS/Web will get error codes from auth server so that clients resolve the issue. If we ever have to message the user with alertnate case emails -- that that's a UX 💣

We have to be weary about phishing attacks also, where you might sign in as JoeUSER@domain.com and you get asked to enter you password from someone who owns JOEuser@domain.com

finally, persona respects case upon input and always presents you with the email string as you entered it: so JoeUser@Domain.com should always display that way, but the client/front will lower() the string upon db insert.

@edwindotcom
Copy link
Author

tracking in bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=957681

@dannycoates
Copy link
Contributor

finally, persona respects case upon input and always presents you with the email string as you entered it: so JoeUser@Domain.com should always display that way, but the client/front will lower() the string upon db insert.

Which is how this works as well.

@warner
Copy link
Contributor

warner commented Jan 24, 2014

In the meeting today, we just decided that a "wrong-case login-attempt" (i.e. you submit an email address which doesn't exist verbatim in the database, but there is an account whose case-folded address matches the case-folded submitted address) should return a new error code, instead of being presented as "wrong password". The new error code should be something like "unknown account (but try this case instead)". @dannycoates will allocate the errno and update the server code+docs.

@edwindotcom
Copy link
Author

i logged a bug for clients to handle incorrect case error: https://bugzilla.mozilla.org/show_bug.cgi?id=963835

@SamPenrose
Copy link

@edmoz Thanks for the headsup. This bit confused me: "so JoeUser@Domain.com should always display that way, but the client/front will lower() the string upon db insert", specifically "client/front". The rest of this discussion makes it sound like:

  1. FxOS does NOT normalize the email (but the server may return an error on normalization-match, with FxOS has to handle)

Alternately:

  1. FxOS has to present both versions of the email to the server and handle them on return

which would be a significant change. Can you clarify?

@ncalexan
Copy link
Member

@SamPenrose: Android client handles 4xx/120 (alternate email provided) by retrying once with the new email and existing password. See https://bugzilla.mozilla.org/show_bug.cgi?id=963160. It is tricky to thread a possibly alternate email through to the right places, in particular I missed unwrapkB the first time through.

@pdehaan
Copy link
Contributor

pdehaan commented Feb 4, 2014

We're handling this client side. CLosing.

@pdehaan pdehaan closed this as completed Feb 4, 2014
rfk pushed a commit that referenced this issue Oct 24, 2018
fix(config): expose more environment variables for config
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants