Throw error message when a user with email address as uid exists

[sharidas]

When admin creates user we should check the users
with username as email address exists.

Signed-off-by: Sujith H sharidasan@owncloud.com

Description

Lets consider a scenario where a user with username as email address exist ( for instance a guest user). And admin tries to create another user say user1 with the email address of guest user. The user should not be created. And an error message should be displayed. This PR addresses the same. I have made the error message same as for:
a) when the user exists and admin tries to create the same user.
b) The user names differ, but one of the username is same as the email address of new user to be created.

Related Issue

• Fixes <issue_link>

Motivation and Context

A check to forbid the user creation when a user is created with same email address of a guest user.

How Has This Been Tested?

• Create a guest user with a valid email address
• From the webUI create another user with different uid and with same email address of guest.
• Error is thrown in the webUI.
• Also this has been tested using occ command and provisioning api.

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

Open tasks:

• Backport (if applicable set "backport-request" label and remove when the backport was done)

[sharidas]

While looking into the UsersController.php there is a room for improvement to use UserCreateService. I was thinking to make it as a separate PR/task. Let me know if that's ok or not.

[jvillafanez]

I'm not sure if this is a good idea.

Users have an uid and an email (among other things). Whether the guests app uses the email as uid is irrelevant because for core is just another uid, what is important is that the guests app also sets the email as email.
Now, the question is, should 2 different users have the same email? I don't remember if there is already a DB constraint preventing multiple emails. In any case, I expect core to be handling this case already, no matter if the same email is duplicated or not.

[codecov]

Codecov Report

Merging #36050 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #36050      +/-   ##
==========================================
- Coverage   54.01%   53.85%   -0.16%     
==========================================
  Files          63       63              
  Lines        7404     7377      -27     
  Branches     1309     1301       -8     
==========================================
- Hits         3999     3973      -26     
  Misses       3019     3019              
+ Partials      386      385       -1

Flag	Coverage Δ
#javascript	53.85% <ø> (-0.16%)	⬇️

Impacted Files	Coverage Δ
core/js/shareitemmodel.js	80% <0%> (-0.93%)	⬇️
core/js/sharedialogshareelistview.js	80.48% <0%> (-0.66%)	⬇️
core/js/sharedialoglinkshareview.js	83.46% <0%> (-0.13%)	⬇️
core/js/sharedialogmailview.js	62.37% <0%> (ø)	⬆️
core/js/shareconfigmodel.js	72.41% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4941f02...0b1456d. Read the comment docs.

[sharidas]

In any case, I expect core to be handling this case already, no matter if the same email is duplicated or not.

If I understand the comment correctly you mean to say the core might be handling case of same email address already. But as per my observation the core does not. The core allows to have same email address for different users. In fact I wonder why can't we allow multiple users with same email address. Or say what would cause wrong if 2 different users have same email address. Though I know that allowing multiple users having same email address would contradict to this PR ( that's fine ).

[jvillafanez]

If we want to prevent duplicate emails, a DB constraint is the way to go. The fact that there is a DB index which allows duplicated values in the email means that core is fine allowing duplicated values for whatever reason.
The fix, if we want it, is to include that constraint in the DB ensuring a proper migration in case duplicates already exist in the DB.

If I understand the comment correctly you mean to say the core might be handling case of same email address already. But as per my observation the core does not.

The solution, as said, is to put a constraint in the DB. If core hasn't made that change yet, I guess core is fine having duplicated emails for whatever reason.
My point here is that there is no bug if core allows it and handles the situation correctly.

In fact I wonder why can't we allow multiple users with same email address. Or say what would cause wrong if 2 different users have same email address

Emails are intended to be personal and private. Admins use email to comunicate with the users. This can involve password resets, activity traces, and other personal information that you don't want anyone to see. Obviously, if the user uses public or shared email accounts, it's his responsability

[sharidas]

The fix, if we want it, is to include that constraint in the DB ensuring a proper migration in case duplicates already exist in the DB.

Totally agree with you. The only problem is when we touch the DB, we need to have migrations for the same.

My point here is that there is no bug if core allows it and handles the situation correctly.

Agreed.

[sharidas]

Modified the PR with following changes:

• Added migration to remove the duplicate email address with NULL and then mark the email column of accounts to unique. This would prevent duplicate email addresses. The draw back I see here is that the migration is setting duplicate email addresses to NULL. Meaning say there are 2 email addresses found with foo@bar.com then both are set to NULL. But then the admin/users have to take care to set the email address back.
• The warning message is thrown to the UI saying duplicate email address found.

[GrendelOnCampus]

I guess, this is not an good idea. Different users may have the same mail-address for testing purposes.