-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Review: Allow Duplicate Email for Members #16202
Review: Allow Duplicate Email for Members #16202
Conversation
Hi there @jasont0101, thank you for this contribution! 👍 While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:
Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution. If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request. Thanks, from your friendly Umbraco GitHub bot 🤖 🙂 |
Hey @jasont0101, thanks a lot for creating a second PR for this one, sorry it's taken so long to get it reviewed. Have you addressed all of @ronaldbarendse's comments on this PR? I just want to make sure we don't lose those conversations 😄 Someone on the core collabs team will review this soon! |
@georgebid & @ronaldbarendse Yes, I incorporated Ronald's input from the prior PR (#14784). |
@jasont0101 I'm hoping you could answer a question that may help to unblock this issue for us. In your opening message for this pull request, you said: "While there are other way to resolve this issue...with out having to work out complicated alternative contact email options." Did your alternative approach involve appending +1 to a member's email address or similar? I'm really keen to understand the other ways you've managed to overcome this issue. We're unlikely to be in a position to upgrade to v13 in the immediate future, so any information you can share would be useful. Feel free to drop me an email at seo@connect.org.uk to discuss. |
@connectisl Sorry for the very slow reply. The only thing I can think of for handling this without being able to allow members to have duplicate emails is to add a new property to the member for a Contact Email, then make the email in the email field unique in some way. The email field would just be ignored in your code in favor of the new Contact Email field. This could be a fair bit of refactor work depending on your code. One of our clients did something similar as they are also reluctant to upgrade to v13 as well. There upgrade to v10 from v8 was very painful. Having done a few v10 to v13 upgrades now I have found that they are fairly painless. The change over from .Net Framework to .Net Core as well as some of Umbraco's updates was the worst part. I am hoping v14 will turn out to not be that bad. Being fearful of upgrades is a sign that there is an architectural issue with their project. Unfortunately, this is not an isolated issue and some of our clients whos sites, which we inherited, have gotten so complicated that they are fearful of upgrades. Over the years with Umbraco I have learned that upgrades are just part of the Umbraco life cycle and need to factored in to my designs. I will get off of my soap box now. :-) |
I have had the same problem, migrating an Umbraco 7 project to Umbraco 13. I was able to create more than one member with the same email address using the MemberService, rather than doing it in the backoffice. However I had a problem that I could not set a password for these duplicate members. I think that this is something to do with the Microsoft dotnet identity logic. I think that this logic still uses the email address when handling the password hash. Please make sure that the password setting logic is also addressed in this pull request. |
@TimPickin Unfortunately, I don't think Umbraco is going to get this merged in anytime soon, if ever. I was at Code Garden and talked to them about this an they are just too busy for this one. I believe they are focused on v14 and since I also have been busy and not able to make this update in v14, there is just no desire to merge this in, in my belief. I have updated this a few times and it goes stale because of their back log and priorities. I also believe they do not understand the problem. I suspect from their point of view, why would you want duplicate emails. There is an obvious push to have the email be the user's login name so duplicate email should not be allowed. If you want a secondary email, they might say, just added a new email property field to the member and customize the frontend login to use that property. If this is their position, I am just guessing, I can see their point and why they might not want to make this a priority. If they ignore it long enough, it will go away because everyone will be forced to move to a work around. |
I have the same problem in developing an Umbraco 13 site, in my case it is because email addresses are considered PII that are not allowed by company policy to be contained in the Umbraco website's database. For logins I'll be creating a unique ID for each Member and using that as the UserName, and logging them in via that - but the site is not expected to email these Members through Umbraco, the use of Umbraco Membership is only for managing role-based Access Permissions. My scenario would also be fulfilled if emails were simply an optional property of a Member. |
…nfused with users and members).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @jasont0101, I appreciate your input on this over several months.
I've had a look at it today and all seems to work as expected. I made a few minor updates and tested the following scenarios:
- Creating and updating members via the back-office.
- Registering members through a front-end implementation.
- Logging in as those members.
- Login via the delivery API authorization.
All looks to be working as expected with the different values of the new MemberRequireUniqueEmail
setting.
fixes #15676
Prerequisites
Description
This is a redo of this PR #14784. That PR went stale and the Contrib branch advanced to v14, so this is work for v13.
Oddly have had two project in a row that the client had the business need to allow member to have duplicate email address. While there are other way to resolve this issue, want to add (re-add?) this feature to Umbraco so that it was easy to enable with out having to work out complicated alternative contact email options. So, added the option to allow member accounts to be created with duplicate emails.
The current default behavior is that members must have unique emails. While it was possible to override the ASP.NET Identity provider value for RequireUniqueEmail in startup.cs or program.cs, validators still checked if the email existed and prevented the member from being added if the email was a duplicate.
Add a new appsettings.json option to the Umbraco:CMS:Security section, RequireUniqueEmailForMembers with a default value of true. This now sets the value for options.User.RequireUniqueEmail in the ConfigureMemberIdentityOptions class.
Also added a new method to the MemberService, GetMembersByEmail.The GetByEmail method will only return FirstOrDefault. This new method allows for the retrieval of all member with the same email.
This option only allows duplicate emails for members and not users. Usernames must still be unique.
TEST
This item has been added to our backlog AB#47446