Skip to content
This repository has been archived by the owner on Feb 24, 2018. It is now read-only.

Usernames and emails are case sensitive (Bug) #658

Open
ssahni opened this issue Jan 19, 2018 · 7 comments
Open

Usernames and emails are case sensitive (Bug) #658

ssahni opened this issue Jan 19, 2018 · 7 comments

Comments

@ssahni
Copy link

ssahni commented Jan 19, 2018

Hello,

There is a severe issue right now that is basically a deal breaker for my company and most likely many others. As previously noted in issue #524, a user should not be able to create an account with the same username or email address. Please see the RFC for Domain names, RFC for addr-spec, and Email address.

Reproduce:

  1. Create user with email Test@Test.com
  2. Create user with email test@test.com
  3. Check Userpool, both users are created

The above steps happens with usernames as well.

test@test.com === Test@Test.com
my_username === MY_USERNAME

This poses potential security risks and is a major liability (even with lowercasing usernames and emails prior to creating an account). The most obvious being that a malicious user can create an account of another user in an attempt to mimic that other user. There is nothing stopping the malicious intent of even an intermediate skilled party from executing a request to create an account outside the context of an application; i.e. a cURL request.

Additionally, the workaround of creating a Lambda trigger to do a check and fail at pre-signup is, quite frankly, a waste of resources and money.

Overall though, AWS Cognito is fantastic! Keep up the good work.

Thank you

@ldgarcia
Copy link

I have confirmed this test, registering the same email with case variations.

This poses potential security risks and is a major liability (even with lowercasing usernames and emails prior to creating an account). (...) There is nothing stopping the malicious intent of even an intermediate skilled party from executing a request to create an account outside the context of an application; i.e. a cURL request.

😱 Yes, if someone exploits your app and gets access to the cognito client configuration values, they can submit a malicious signup request.

@kickthedragon
Copy link

kickthedragon commented Jan 31, 2018

I'd actually argue these complaints / statements are subjective since Amazon provides you with the tools and hooks to really do whatever you like. Lambda functions really aren't a waist of resources at all and are extremely cheap. It didn't take me much time at all to get it to function the way i wanted it to.

I setup a custom attribute called "storage" to act as a data storage attribute to transfer data in between lambda functions (Pre signup and Confirmation). On confirmation it sets the preferred_username to the username the user would like, while i store all usernames in lowercase. I also check to make sure a user doesn't try to spoof a username submission with capital letters. You could just let AWS generate random usernames since the preferred_username is what really matters to the user.

NOTE: I'd also recommend checking the origin of the request to only allow the domain(s) you want to allow signups from here. Its currently not in this code below since i'm testing and will add the check later.

var AWS = require('aws-sdk');
var cognito = new AWS.CognitoIdentityServiceProvider();

exports.handler = (event, context, callback) => {
		
		const modifiedEvent = event;
		if(event.triggerSource === "PreSignUp_SignUp") {
		
                    if(event.userName.toLowerCase() != event.userName) {
				callback('Invalid username input');
		    }		
			var emailParams = {
				UserPoolId: process.env.USER_POOL_ID,
				Filter: "email = \""+event.request.userAttributes.email+"\"",
			};
	
			cognito.listUsers(emailParams, (err, emailResults) => {
				if (err) {
					callback(err);
				}
				if(emailResults.Users.length > 0) {
					callback('User with that email already exists');
				}
					modifiedEvent.response.autoConfirmUser = true;
					callback(null, modifiedEvent);
				return;
			});
		} 
		
	if (event.triggerSource === "PostConfirmation_ConfirmSignUp") {
		let params = {
			UserAttributes: [{
				Name: 'preferred_username',
				Value: event.request.userAttributes['custom:storage']
				}],
			UserPoolId: process.env.USER_POOL_ID,
			Username: event.userName
		};
			cognito.adminUpdateUserAttributes(params, (err, output) => {
				if (err) {
					callback(err);
				}
				callback(null, modifiedEvent);
				return;
			});
	}
};

This seems like a perfectly secure and good solution to me and didn't take much time to figure out at all. :)

@ldgarcia
Copy link

ldgarcia commented Feb 4, 2018

I think is it sensible to expect case-insensitive comparison as a given.

As the local-part of email addresses are, in fact - case sensitive, it is important to store and compare email addresses correctly. To normalize an email address input, you would convert the domain part ONLY to lowercase.
Unfortunately this does and will make input harder to normalize and correctly match to a users intent. It is reasonable to only accept one unique capitalization of an otherwise identical address, however in this case it is critical to:
Store the user-part as provided and verified by user verification
Perform comparisons by lowercase(provided)==lowercase(persisted)

^ https://www.owasp.org/index.php/Input_Validation_Cheat_Sheet#Email_Address_Validation

NOTE: I'd also recommend checking the origin of the request to only allow the domain(s) you want to allow signups from here. Its currently not in this code below since i'm testing and will add the check later.

What about mobile apps?

@tn0
Copy link

tn0 commented Feb 9, 2018

Not only that you can create 2 accounts with the same E-Mail Address. Also the login with the E-Mail Address ist case sensitive.

Create User Foo.Bar@example.com
Try to login as foo.bar@example.com gives an "User does not exist" error

So many user can't login to the system anymore.

But I think this is an problem on Backend and not on the JS Library

@ssahni
Copy link
Author

ssahni commented Feb 9, 2018

This is most likely not a trivial change for the Cognito team to make, which is unfortunate but understandable. In case anyone else stumbles upon this issue and needs a workaround, you can setup a Lambda Pre Sign-Up trigger in Cognito. Additionally, in your app, make sure to lowercase before signing users up or logging users in. I've seen < 2ms execution times with 128MB allocated for Lambda memory. Cost should be very low and I suspect almost negligible unless you have millions and millions of new users signing up a month. Which in that case you wouldn't care about the cost of the Lambda function :-). Not counting the free tier, if you had 1,000,000 new user sign ups in one month, then the total cost for the Lambda function would be $0.20 (20 cents).

The code for that lambda would look something like this:

if (event.triggerSource === 'PreSignUp_SignUp') {
  const newEvent = event;
  newEvent.userName = event.userName.toLowerCase();
  newEvent.request.userAttributes.email = event.request.userAttributes.email.toLowerCase();
  context.done(null, newEvent);
  return;
}

@kitkittan
Copy link

@ssahni I've tried your code and also the same code in Python, but the user is still saved with the initial email. When I log the result that should be returned it's correct, but Cognito seams to ignore any changes to the initial event object. I can only throw exceptions.

I ended up checking if the email is equal to its lowercase form.

@ssahni
Copy link
Author

ssahni commented Feb 16, 2018

@kitkittan Here is a more complete version of the relevant code from my function. Make sure you get the case of userName correct. That tripped me up initially. Hope it helps.

exports.handler = async (event, context, callback) => {
  context.callbackWaitsForEmptyEventLoop = false;

  // Force usernames and emails to lowercase prior to signing the user up
  if (event.triggerSource === 'PreSignUp_SignUp') {
    const newEvent = event;
    newEvent.userName = event.userName.toLowerCase();
    newEvent.request.userAttributes.email = event.request.userAttributes.email.toLowerCase();
    context.done(null, newEvent);
    return;
  }
}

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

5 participants