aws-amplify / amplify-js

A declarative JavaScript library for application development using cloud services.
https://docs.amplify.aws/lib/q/platform/js
Apache License 2.0
9.42k stars 2.12k forks source link

Usernames and emails are case sensitive (Bug) #610

Closed DocCaliban closed 6 years ago

DocCaliban commented 6 years ago

Usernames and emails are case sensitive

Repro: Create user with email Test@Test.com then Create user with email test@test.com Results: In the Userpool, both users are created

Repro: Create user with username Dave then Create user with username dave Results: In the Userpool, both users are created

I have read all the comments in cognito issue's 524 and 658, as well as a bunch of comments in the forums about using a lambda to resolve the issue myself, but the reality is that I shouldn't have to.

This is just horrible behavior, and it's something that both Firebase and Auth0 don't have problems with. There is no world where emails or usernames should be case sensitive. It's a burden to users and a burden to developers.

yuntuowang commented 6 years ago

Hi @DocCaliban, thanks for pointing out this issue. We already track on it and we are working on the fix. Thanks!

DocCaliban commented 6 years ago

So this is resolved?

bhao-speedline commented 6 years ago

It seems this hasn't been resolved yet.

buggy commented 6 years ago

The problem is caused by Cognito User Pools and needs to be fixed there. The best that could be done inside aws-amplify is to always lower case email and username fields before calling Cognito.

bhao-speedline commented 6 years ago

@buggy Thanks! I'm aware of that, but didn't notice that this is NOT the repository for Cognito. Sorry about that.

buggy commented 6 years ago

@bhao-speedline It's good to discuss this. Even if the problem is caused by Cognito User Pools there are a large number of people using Amplify who are going to suffer. It probably makes sense to add a feature that automatically lower cases certain fields inside Amplify before calling Cognito User Pools.

EssyTech commented 6 years ago

I was just finishing up my implementation to convert all of our company applications to cognito and this issue came up in some tests. What is being done, as of now, and is there going to be a solution to this?

ZebGirouard commented 6 years ago

As someone mentioned in a related issue, this is most likely a Sophie's Choice for the AWS Amplify team to implement. For so long, these emails/usernames have been case sensitive, and people are now relying on that behavior.

I honestly can't imagine what AWS could do to keep those numerous legacy users happy, and the rest of us happy too besides creating a LogInCaseInsensitive function which is really no different from me putting .toLowerCase() on all my Amplify Cognito functions. Which I guess everyone following these issues has already done, including me.

@yuntuowang , am I wrong? It sounds like this was a priority about a year ago, but all the relevant issues are closed or archived.

EssyTech commented 6 years ago

Thank you for the quick reply. However, we are currently implementing the cognito solution using the supplied cognito auth server by aws and not using amplify at this time, I just realized I posted this on the amplify issue area as I was searching this issue on google. I will have to look into amplify for long term implementation and find out a way (maybe through lambda triggers) to implement this solution on aws supplied auth server. Our users are used to case insensitve usernames and this will increase our support call volume in some cases, if we do not find a solution to this.

DocCaliban commented 6 years ago

people are now relying on that behavior. I honestly can't imagine what AWS could do to keep those numerous legacy users happy, and the rest of us happy.

Sorry, but who would rely on this? No one wants case sensitive email addresses or usernames, it's only going to lead to confusion.

@EssyTech I ended up just implementing toLower() myself everywhere i needed to, which is not ideal, but it works. I was hoping that they would actually try to fix it though, instead of making excuses.

valonhaliti commented 5 years ago

We're using Cognito's hosted UI sign in page, I added PreSignUp and PreAuthentication triggers which lowercase the email but it is not working, since users are being created (are being inserted in userpool) without being lowercased, and the PreAuthentication isn't being triggered before checking if that email exists in userpool. As I can see this can't be done using Cognito's host UI?

shlomiken commented 5 years ago

We're using Cognito's hosted UI sign in page, I added PreSignUp and PreAuthentication triggers which lowercase the email but it is not working, since users are being created (are being inserted in userpool) without being lowercased, and the PreAuthentication isn't being triggered before checking if that email exists in userpool. As I can see this can't be done using Cognito's host UI?

we are having the same issue , it cannot be worked around with the hosted UI, we are now going to talk with AWS about this. the whole point of us using cognito was to avoid writing our own UI for authentication.

joaoricardo000 commented 5 years ago

Hey @shlomiken, any updates from AWS on this? We're about to add a lambda function just to normalize this... seems so stupid...

shlomiken commented 5 years ago

as far as i tried (and its a lot) lambda would not help you , cause you can't change the input of the function. since AWS response was so bad on this - we have decided to move to auth0.

rampageservices commented 5 years ago

This was closed with no answer.

genio commented 5 years ago

Is there a place to actually see if AWS is working on this? I realize this is the repo for the Amplify library, but this is INSANE and needs to be fixed. I've been hitting every resource I can find for over 2 years and can't get any headway on this.

l3wy commented 5 years ago

Ok.. I was just working on this because of the paste bug (which you'll run across if you submit an app to Apple).. and realized I could handle the username/email issue there as well... here's some code for those of you using ionic/angular:

@Directive({selector: '[AuthFix]'})
export class AuthFix {

    keyupEvent: KeyboardEvent = new KeyboardEvent("keyup");

    paste: boolean = false;

    upperReg: RegExp = /[A-Z]/;

    @HostListener('paste') onPaste() {
        this.paste = true;
    }

    @HostListener('input', ['$event.target']) onInput(target: any) {

        if (target.type == "email") {
            if (this.upperReg.test(target.value)) {
                target.value = target.value.toLowerCase();
                this.paste = true; //ok, so it's not true, but it flows better this way
            }
        }

        if (this.paste) {
            target.parentElement.dispatchEvent(this.keyupEvent);
            this.paste = false;
        }

    }

}

Just throw AuthFix into your amplify-authenticator tag:

<amplify-authenticator [signUpConfig]="signUpConfig" [usernameAttributes]="usernameAttributes" framework="ionic" AuthFix></amplify-authenticator>

This should handle it everywhere in the amplify authenticator code .. As they're typing it'll swap in the lowercase letters on the email type fields... I'm not using username, so not sure if it's as easy to identify by type or something... unfortunately the way the fields are built I think the names are generally useless (ion-input-1 for example) .. but maybe they're consistent across forms, I didn't check.

chiragpurohit71085 commented 4 years ago

HI

How can I make user's input in lower case before sending it to cognito in React.js? I tried to search here

https://aws-amplify.github.io/docs/js/authentication

but not find anything related to it. Whether it is required to create custom UI to achieve what I am looking for?

I am not using lambda function

https://docs.aws.amazon.com/cognito/latest/developerguide/cognito-user-identity-pools-working-with-aws-lambda-triggers.html#aws-lambda-triggers-pre-token-generation-example-1

elorzafe commented 4 years ago

@DocCaliban @chiragpurohit71085 @l3wy @genio @rampageservices @shlomiken @EssyTech @valonhaliti @bhao-speedline @joaoricardo000 from today new User Pools can be created with case insensitivity for username input More info here

genio commented 4 years ago

Any effort being made to update existing pools to be case insensitive?

ahnliu commented 4 years ago

Bump on Genios comment. We are in a live environment, and users are complaining that they can't login, because they didn't realize that their email addresses were case sensitive. (So they try to login with bobsmith@test.com, when they registered originally with bobSmith@test.com).

Great to see that 4 hours ago, Amazon came out with a fix for this. But is there a migration path? If not, it's actually a pretty non-trivial problem to figure out how to convert existing users with capitalizations in their usernames to lowercase, without asking them to create a new account. Any thoughts?

elorzafe commented 4 years ago

I think that could be more problematic in case you have collisions, maybe in that case is better to migrate the user to a new User Pool

genio commented 4 years ago

"better to migrate"... That's not really doable, so it's not a viable work-around.

You can't backup/restore pools and you can't migrate pools. (yes, I know you can setup a lambda function to get users in a one-by-one method, but that's not exactly the same thing)

sarahcec commented 4 years ago

Hi all, I'm the product manager for Cognito. We weren't able to implement case insensitivity on existing user pools because there are so many ways to solve the collision problem. If I have two usernames in my pool that differ only in case, I might want to delete one, or ask the user to reregister, or prompt them to log into the other one. Depending on what logic you want to use, you can craft a migration lambda trigger that will migrate the users to a new pool as they log in so you can handle collisions properly. We're working on a blog post now with explicit instructions on how to do that. It should be out soon.

DocCaliban commented 4 years ago

I gotta say it was a long time coming, but I am glad it was finally fixed and that you are creating some documentation around it as well, even if' it's just a blog post.

genio commented 4 years ago

Again, this version of the term "migrating" isn't really a valid work-around or a real migration. Repeating the documentation you guys already have out there for this type of migration path won't change the fact that you can't really migrate a pool.

claym commented 4 years ago

@elorzafe @sarahcec How do you enable case insensitivity in an amplify project?

I created a new environment with amplify 4.13.4 on an existing project, and the case insensitivity was disabled and I was not able to change it.

elorzafe commented 4 years ago

As mentioned here

We don't have this feature available on CLI but it's scheduled for release. For the time being you can enable it in the Cloudformation template more information can be found at https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-cognito-userpool.html under UsernameConfiguration

jingxizhang commented 4 years ago

I am using amplify version 4.18.1. I created a new Cognito user pool (using email for sign-in) but I still experience the case-sensitive problem. If I signed up with all lower case email address and later signed in with a capital email address, the cognito GUI will show "User does not exist".

Albert-Gao commented 4 years ago

I think the root of cause is cognito stands too long such that, this change would break the existing app. So it will never be applied until a new version will be announced.

A costy-workaround is to provide a pre-auth lambda trigger

The migration is just too much for a simple requirement like this...And introduce several async steps (which could fail and you need to try-catch,,,,, just can not think any further, my head) just for case-sensitivity does not worth it.

LionelB5 commented 4 years ago

So is the official response on this issue from Amazon that we need to implement a custom auth lambda trigger?

As @Albert-Gao has mentioned, this is just too much for such a simple requirement. It also looks like this ticket has been closed multiple times without resolution (since being opened in 2018), which is a bit disheartening.

Are the team planning on taking a look at this anytime soon? I guess this would require collaboration between the Amplify/Cognito teams. Your help is appreciated.

flyandi commented 4 years ago

There is a new option in the userpool config:

image

But how would we configure that in amplify-cli?

justifiedfalsebeliefs commented 3 years ago

There is a new option in the userpool config:

image

But how would we configure that in amplify-cli?

This setting cannot be changed in the Cognito console after the user pool is created. A low-hanging fruit would be for the Amplify team to add the ability to flip this check during the cli-add auth flow. This wouldn't help migrating users, but would allow a new Cognito user pool to be provisioned that is case-insensitive.

Edit: Note, I discovered this thread which discusses a workaround, and acknowledges this is coming in a future release: https://github.com/aws-amplify/amplify-cli/issues/3494

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.