Aspnetcore: Unhandled Exception when more than one emails exist in the database

[pliolis]

https://github.com/dotnet/aspnetcore/blob/0437117cfb5bc91a34878fb962076bb05f4b8273/src/Identity/EntityFrameworkCore/src/UserStore.cs#L639

If there are more than one users with the same email, trying to create new user with the same email the userManager.CreateAsync(...) fails with an unhandled error because of SingleOrDefault usage.
Maybe this row must be change to FirstOrDefault to avoid this error.
Of Course the exception is thrown when options.User.RequireUniqueEmail is set to True.

Expected behavior:
the result of [await UserManager.CreateAsync(user, password)] must have result.success set to false and an "DuplicateEmail" error must be added to result.Errors IEnumerable.

[blowdart]

We expect you to check if the email address is registered using FindByEmailAsync() before attempting to register a new user, as the templates do

[pliolis]

Calling FindByEmailAsync() before attempting to register a new user will raise the same unhandled exception I described above because the method return SingleOrDefault()...

FindByEmailAsync() supposed to return user or null (not to thrown an exception in case of user existance)

...the documentation of this method
///

/// Gets the user, if any, associated with the specified, normalized email address.
///

/// The normalized email address to return the user for.
/// The used to propagate notifications that the operation should be canceled.
///
/// The task object containing the results of the asynchronous lookup operation, the user if any associated with the specified normalized email address.
///
public override Task FindByEmailAsync(string normalizedEmail, CancellationToken cancellationToken = default(CancellationToken))

[blowdart]

So you have two users with the same email address already, despite performing the check? If you're using find by email we're rather expecting uniqueness.

[pliolis]

Hi Barry,
Yes you're right, but this is because on our system first we had the default value for options.User.RequireUniqueEmail (default is false) and then we decided to set the value to true. In the meantime we have users with the same email address because the email was not unique from the beginning.

We use the template as it is for user registration and we get http status 500 for this case

[blowdart]

@HaoK What's the best way to go here, to search for count, rather than individual user?

[HaoK]

Well from our perspective the database is in an invalid state if RequireUniqueEmail is true and you have duplicates... FindByEmail when called is expecting Email to be a key for the user, so SingleOrDefault seems correct here.

@ajcvickers thoughts?

[pliolis]

So, if I let the default value for options.User.RequireUniqueEmail first, I cannot change this property in the future ?

[pliolis]

The username is the key, the email is an attribute I think

[HaoK]

You can change it, but you need to clean up your data and get rid of duplicate emails

[pliolis]

Of course this is a solution. But my question is if it is ok to return unhandled exception a method (FindByEmail) when there are users (more than one) with this email. What if options.User.RequireUniqueEmail is set to false and inside the code someone calls FindByEmail(), the code will crash?

[HaoK]

Right that's explicitly called out in the doc comments as well: https://github.com/dotnet/aspnetcore/blob/master/src/Identity/Extensions.Core/src/UserManager.cs#L1410

[HaoK]

Don't call that method if emails are not unique

[ajcvickers]

Concur with @HaoK

[pliolis]

You can change it, but you need to clean up your data and get rid of duplicate emails
Hi @HaoK, cleaning up the database it is not so easy because I have to choose between deleting users and changing the email to something else that I don't know.

[HaoK]

Right so don't call FindByEmail since its not unique per user.

[pliolis]

neither CreateUserAsync() because it throws unhandled exception

[HaoK]

Right, generally identity isn't going to work well if you configure it to expect emails are unique when that's not actually the case.