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

It is possible to create 2 users with the same email address #351

Open
davitol opened this issue Aug 6, 2019 · 16 comments
Open

It is possible to create 2 users with the same email address #351

davitol opened this issue Aug 6, 2019 · 16 comments
Assignees
Milestone

Comments

@davitol
Copy link
Contributor

davitol commented Aug 6, 2019

Steps to reproduce

  1. Create a guest user sharing a file using an email mail@mymail.com
  2. As admin go to users page and create a new regular user using mail@mymail.com

Expected behavior

An error message should be shown, mail should not be sent and the user should not be create

Actual behavior

Mail is sent and the user can be created

Tested with oC 10.3.0 prealpha and Guests 0.8.2. RC1

@phil-davis
Copy link
Contributor

In the title, I guess that you mean "with the same email address"

@davitol
Copy link
Contributor Author

davitol commented Aug 6, 2019

In the title, I guess that you mean "with the same email address"

Yes, you are right. editing the title

@davitol davitol changed the title It is possible to create 2 users with the same password It is possible to create 2 users with the same email Aug 6, 2019
@davitol davitol changed the title It is possible to create 2 users with the same email It is possible to create 2 users with the same email address Aug 6, 2019
@patrickjahns patrickjahns modified the milestones: qa, development Aug 6, 2019
@patrickjahns
Copy link
Contributor

Moving to development - it's not a regression and will be addressed later

@individual-it
Copy link
Member

individual-it commented Aug 12, 2019

@davitol should the QA-team do something here? If not maybe lets take off the QA-team tag

@sharidas
Copy link
Contributor

Created PR: owncloud/core#36050

@sharidas
Copy link
Contributor

This issue depends on the decision whether:

@cortho
Copy link
Contributor

cortho commented Aug 20, 2019

In this case, the guest app should not depend on the behavior in core. My suggestion: if an email address already exists, this address cannot be used for a guest account.
So the same email address for multiple accounts would be still possible but not when creating a new guest user.

Edit: In 2017 @PVince81 wrote "duplicate email addresses are forbidden" - see owncloud/core#27626 (comment)

@sharidas
Copy link
Contributor

the guest app should not depend on the behavior in core. My suggestion: if an email address already exists, this address cannot be used for a guest account.

I have verified that guest app actually takes care of the user creation from the UI part. That is say if I have created 2 users user1 and user2 both have same email address foo@bar.com. Now I try to share a folder to the guest user, with email address foo@bar.com, the drop down in the User and Groups of the share tab will show foo@bar.com as federated user, and user1 and user2 and not a guest user. The guest user will not be listed, where user can click it and create one. So imho, the query is again with respect to core.

@sharidas
Copy link
Contributor

Aah now I got the reference. There is a PR owncloud/core#27662 which actually allows the core to have multiple email address. So I would say we can close this issue? If every one here agrees then I can also close my PR too. Let me know the opinion here @davitol @individual-it @phil-davis

@cortho
Copy link
Contributor

cortho commented Aug 22, 2019

I'd recommend not to close this issue. IMO guest users' email addresses should be unique and from a user's point of view it wouldn't make sense to create a guest user who is already a regular ownCloud user.

@phil-davis
Copy link
Contributor

phil-davis commented Aug 22, 2019

IMO, when creating a guest user, the email address should not be allowed to be one that is already in use by a regular user. That should be easy to implement (might already be like that).

The more challenging is the reverse. When:

  • creating a new regular user
  • changing the email address of a regular user
  • syncing regular users from an external backend like LDAP

What should happen if the email address already matches the email address of a guest user?

@PVince81
Copy link
Contributor

I thought the user:sync process would already fail in such cases ? cc @jvillafanez might know

@sharidas
Copy link
Contributor

when creating a guest user, the email address should not be allowed to be one that is already in use by a regular user.

At least from the webUI part, a guest user cannot be created if an oC instance has same email assigned to existing user(s).

@jvillafanez
Copy link
Member

I thought the user:sync process would already fail in such cases ? cc @jvillafanez might know

I'm not aware of any email check during the user:sync. It's the userid the one that needs to be unique. There were some changes aiming for LDAP in case an LDAP userid collides with another LDAP userid, but I think the userid was just ignored or overwritten.

What should happen if the email address already matches the email address of a guest user?

This is why I'd rather either force unique emails at DB level, or allow duplicates regadless of where the user comes from. I don't think there is an easy way to go around this, and I wouldn't want to implement and maintain complex logic that can easily fail and cause problems.

@micbar
Copy link
Contributor

micbar commented Oct 21, 2019

@pmaier1 @jvillafanez I suspect that this can bee closed.

@micbar micbar modified the milestones: development, backlog Oct 21, 2019
@jvillafanez
Copy link
Member

It isn't clear to me what to do:

  • keeping the current behaviour could be problematic (It is possible to create 2 users with the same email address #351 (comment))
  • ensuring the email is unique via DB constraint could cause bugs (we need to investigate what could be broken by this change)
  • preventing new guest user to use an email in use. This works only if the guest is created last, there is no guarantee if users are synced afterward and any of them has an email being used by a guest.

I guess the easiest solution is "do nothing", but we probably need to ensure there is no risk involved.

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

No branches or pull requests

9 participants