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

usernames in sign up + sign in should not case sensitive #65

Closed
ryw opened this issue Feb 2, 2014 · 11 comments
Closed

usernames in sign up + sign in should not case sensitive #65

ryw opened this issue Feb 2, 2014 · 11 comments
Assignees
Labels
Milestone

Comments

@ryw
Copy link
Contributor

ryw commented Feb 2, 2014

lowercase the form input before sending it to be validated

@ryw ryw added the bug label Feb 8, 2014
@ryw ryw added this to the Next release milestone Feb 8, 2014
@ryw ryw assigned queso Feb 8, 2014
@queso
Copy link
Contributor

queso commented Feb 13, 2014

Fixed in master. Will be part of 0.5.4.

@queso queso closed this as completed Feb 13, 2014
@calebl
Copy link
Contributor

calebl commented Feb 18, 2014

will this be released soon? still on 0.5.3 in atmosphere

@queso
Copy link
Contributor

queso commented Feb 18, 2014

We are prepping for a release tonight. I've bumped the version and I am ready to tag and release it once the docs are up to date.

@queso
Copy link
Contributor

queso commented Feb 20, 2014

I think this issue has a whole host of corner cases we didn't consider:

  • Do we just call toLowerCase on everything that we use for sign in?
  • Are we destroying a users creativity by not storing their username the way they typed it?
  • Turns out email is actually case sensitive in the RFC, at least the first part before the @ symbol.
  • This is really an accounts-password issue, should we be dealing with this in our package?

@ryw
Copy link
Contributor Author

ryw commented Feb 20, 2014

I'd say we should deal with it in our package the way we want it to work.

While emails are case sensitive in the spec, I don't think that's really followed in practice, is it?

I'm OK w storing usernames with their capitalization, but when comparing we would need to lowerCase both sides of the expression.

@calebl
Copy link
Contributor

calebl commented Feb 20, 2014

is the comparison evening happening in accounts-entry? this same issue is currently getting tracks accounts-password: meteor/meteor#550

@bshardi
Copy link
Contributor

bshardi commented Feb 26, 2014

Having read both threads, I would lean toward making an option to make the user name case insensitive and always making the email case insensitive. As far as the production work I have done, both user name and email are toLowerCase.

I believe that doing the work on the server is a much more elegant solution that the one I submitted. I do think the fields need to be stored as lowerCase because the way meteor works by default if you only make the test case insensitive you may still get two different users who's logins resolve only to one user. Unless i am missing something.

Good catch on this tho.

As far as should accounts-entry deal with it. I don't think so. If there is a clear way to make the data save on the server side as the developer wants, then no, ignore the issue and undo my changes.

@bshardi
Copy link
Contributor

bshardi commented Feb 26, 2014

Added the following server side,

Accounts.onCreateUser(function(options, user){
    if (user.emails && user.emails.length > 0) {
        for (var i = 0; i < user.emails.length; i += 1){
            user.emails[i].address = user.emails[i].address.toLowerCase();
        }
    }
    if (user.username)
        user.username = user.username.toLowerCase();
    if (options.profile) 
        user.profile = options.profile;
    return user;
});

This gave me lowercased username and email addresses. However it broke the following...

  • After creating a new user, the signUp uses either Meteor.loginWithPassword with the email or username submitted from the form which is mixed case and thus fails.
  • The login will also fail with email or username if the user enters mixed case.

At this point if I have not confused the issue too much, the problem does need to be addressed on both server and client. If the client interface takes care of it all, the server may not need to.

If Meteor.loginWithPassword took care of the comparison it might not be an issue for you.

queso added a commit that referenced this issue Mar 8, 2014
Take Two on Case Sensitive Username and Email - #65
@cinjon
Copy link

cinjon commented Mar 21, 2014

Is there a strategy for solving this going forward?

@bshardi
Copy link
Contributor

bshardi commented Mar 22, 2014

This Issue has been taken care of in the Pull Request #101. I added two new client options for setting the username and email to lowercase.

@SachaG
Copy link

SachaG commented Aug 23, 2014

I might be wrong but it seems like usernames now default to being automatically converted to lowercase? I don't mind case-insensitive sign-in, but now if I enter "Sacha" as a username it then becomes "sacha" instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants