diff --git a/src/Umbraco.Core/Configuration/MemberPasswordConfiguration.cs b/src/Umbraco.Core/Configuration/MemberPasswordConfiguration.cs index 8e7cd97f35..b56a6e7272 100644 --- a/src/Umbraco.Core/Configuration/MemberPasswordConfiguration.cs +++ b/src/Umbraco.Core/Configuration/MemberPasswordConfiguration.cs @@ -3,7 +3,7 @@ namespace Umbraco.Core.Configuration { /// - /// The password configuration for back office users + /// The password configuration for members /// public class MemberPasswordConfiguration : PasswordConfiguration, IMemberPasswordConfiguration { diff --git a/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs b/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs index f3d5661926..89ede9d4a6 100644 --- a/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs +++ b/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs @@ -13,14 +13,13 @@ namespace Umbraco.Core.Members //: IdentityUser, IdentityUserClaim>, { private bool _hasIdentity; + private int _id; - public int Id { get; set; } public string Name { get; set; } public string Email { get; set; } public string UserName { get; set; } public string MemberTypeAlias { get; set; } public bool IsLockedOut { get; set; } - public string RawPasswordValue { get; set; } public DateTime LastPasswordChangeDateUtc { get; set; } @@ -30,6 +29,16 @@ namespace Umbraco.Core.Members /// public bool HasIdentity => _hasIdentity; + public int Id + { + get => _id; + set + { + _id = value; + _hasIdentity = true; + } + } + //TODO: track public string PasswordHash { get; set; } @@ -48,7 +57,11 @@ namespace Umbraco.Core.Members //public bool RolesChanged; - public static UmbracoMembersIdentityUser CreateNew(string username, string email, string name = null) + public static UmbracoMembersIdentityUser CreateNew( + string username, + string email, + string memberTypeAlias, + string name) { if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); @@ -58,7 +71,9 @@ namespace Umbraco.Core.Members UserName = username, Email = email, Name = name, - Id = 0, //TODO + MemberTypeAlias = memberTypeAlias, + Id = 0, //TODO: is this meant to be 0 in this circumstance? + //false by default unless specifically set _hasIdentity = false }; diff --git a/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs b/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs index dad995bdac..bc8589e8ef 100644 --- a/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs +++ b/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs @@ -24,7 +24,11 @@ namespace Umbraco.Web.Models.Mapping target.Key = source.Key; target.Username = source.Username; target.Id = (int)(long)source.Id; - //TODO: map more properties as required + target.Comments = source.Comments; + target.IsApproved = source.IsApproved; + + //TODO: ensure all properties are mapped as required + } } } diff --git a/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs b/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs index 81b11f5141..02c1436a44 100644 --- a/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs +++ b/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs @@ -12,15 +12,16 @@ namespace Umbraco.Infrastructure.Members public interface IUmbracoMembersUserManager : IDisposable where TUser : UmbracoMembersIdentityUser { /// - /// Creates the specified in the backing store with no password, + /// Creates the specified in the backing store with a password, /// as an asynchronous operation. /// /// The member to create. + /// The password to add /// /// The that represents the asynchronous operation, containing the /// of the operation. /// - Task CreateAsync(TUser memberUser); + Task CreateAsync(TUser memberUser, string password); /// /// Helper method to generate a password for a user based on the current password validator diff --git a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs index 717d054120..ce6e20210e 100644 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs +++ b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs @@ -107,7 +107,7 @@ namespace Umbraco.Infrastructure.Members return IdentityResult.Success; } - ///TODO: duplicated code + ///TODO: duplicated code from backofficeusermanager /// /// Logic used to validate a username and password /// diff --git a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs index aedf1e851d..ba7d5d9324 100644 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs +++ b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs @@ -52,19 +52,7 @@ namespace Umbraco.Infrastructure.Members UpdateMemberProperties(member, user); - if (member.RawPasswordValue.IsNullOrWhiteSpace()) - { - // [Comments from Identity package and BackOfficeUser - can/should we share this functionality] - // the password must be 'something' it could be empty if authenticating - // with an external provider so we'll just generate one and prefix it, the - // prefix will help us determine if the password hasn't actually been specified yet. - //this will hash the guid with a salt so should be nicely random - var aspHasher = new PasswordHasher(); - var emptyPasswordValue = - Constants.Security.EmptyPasswordPrefix + - aspHasher.HashPassword(user, Guid.NewGuid().ToString("N")); - member.RawPasswordValue = emptyPasswordValue; - } + //TODO: do we want to accept empty passwords here - if thirdparty for example? In other method if so? _memberService.Save(member); diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index 1e1ae5ff0f..a6d440d505 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -11,13 +11,10 @@ using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; using Umbraco.Core; -using Umbraco.Core.Configuration.Models; using Umbraco.Core.Dictionary; using Umbraco.Core.Events; using Umbraco.Core.Mapping; -using Umbraco.Core.Members; using Umbraco.Core.Models; using Umbraco.Core.Models.ContentEditing; using Umbraco.Core.Models.Membership; @@ -38,6 +35,7 @@ using Umbraco.Web.Common.Filters; using Umbraco.Web.ContentApps; using Umbraco.Web.Models.ContentEditing; using Constants = Umbraco.Core.Constants; +using UmbracoMembersIdentityUser = Umbraco.Core.Members.UmbracoMembersIdentityUser; namespace Umbraco.Web.BackOffice.Controllers { @@ -213,6 +211,12 @@ namespace Umbraco.Web.BackOffice.Controllers [ModelBinder(typeof(MemberBinder))] MemberSave contentItem) { + if (contentItem == null) throw new ArgumentNullException(nameof(contentItem)); + + if (ModelState.IsValid == false) + { + throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); + } //If we've reached here it means: // * Our model has been bound @@ -224,7 +228,44 @@ namespace Umbraco.Web.BackOffice.Controllers //map the properties to the persisted entity MapPropertyValues(contentItem); - await ValidateMemberDataAsync(contentItem); + var memberType = _memberTypeService.Get(contentItem.ContentTypeAlias); + if (memberType == null) + { + throw new InvalidOperationException($"No member type found with alias {contentItem.ContentTypeAlias}"); + } + + if (contentItem.Name.IsNullOrWhiteSpace()) + { + ModelState.AddPropertyError( + new ValidationResult("Invalid user name", new[] { "value" }), + $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}login"); + } + + IMember byUsername = _memberService.GetByUsername(contentItem.Username); + if (byUsername != null && byUsername.Key != contentItem.Key) + { + ModelState.AddPropertyError( + new ValidationResult("Username is already in use", new[] { "value" }), + $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}login"); + } + + IMember byEmail = _memberService.GetByEmail(contentItem.Email); + if (byEmail != null && byEmail.Key != contentItem.Key) + { + ModelState.AddPropertyError( + new ValidationResult("Email address is already in use", new[] { "value" }), + $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}email"); + } + + // Create the member with the MemberManager + var identityMember = UmbracoMembersIdentityUser.CreateNew( + contentItem.Username, + contentItem.Email, + memberType.Alias, + contentItem.Name); + + //TODO: confirm where to do this + identityMember.RawPasswordValue = contentItem.Password.NewPassword; //Unlike content/media - if there are errors for a member, we do NOT proceed to save them, we cannot so return the errors if (ModelState.IsValid == false) @@ -238,21 +279,21 @@ namespace Umbraco.Web.BackOffice.Controllers // events to be raised and developers could be manually adding roles to members in // their handlers. If we don't look this up now there's a chance we'll just end up // removing the roles they've assigned. - var currRoles = _memberService.GetAllRoles(contentItem.PersistedContent.Username); + IEnumerable currRoles = _memberService.GetAllRoles(contentItem.PersistedContent.Username); //find the ones to remove and remove them IEnumerable roles = currRoles.ToList(); var rolesToRemove = roles.Except(contentItem.Groups).ToArray(); - //Depending on the action we need to first do a create or update using the membership provider - // this ensures that passwords are formatted correctly and also performs the validation on the provider itself. + //Depending on the action we need to first do a create or update using the membership manager + //this ensures that passwords are formatted correctly and also performs the validation on the provider itself. switch (contentItem.Action) { case ContentSaveAction.Save: UpdateMemberData(contentItem); break; case ContentSaveAction.SaveNew: - contentItem.PersistedContent = await CreateMemberData(contentItem); + await CreateMemberAsync(contentItem, identityMember); break; default: //we don't support anything else for members @@ -262,9 +303,6 @@ namespace Umbraco.Web.BackOffice.Controllers //TODO: There's 3 things saved here and we should do this all in one transaction, which we can do here by wrapping in a scope // but it would be nicer to have this taken care of within the Save method itself - //TODO: create/save the IMember: this is now saved in CreateAsync, remove once logic is clarified - //_memberService.Save(contentItem.PersistedContent); - //Now let's do the role provider stuff - now that we've saved the content item (that is important since // if we are changing the username, it must be persisted before looking up the member roles). if (rolesToRemove.Any()) @@ -300,13 +338,46 @@ namespace Umbraco.Web.BackOffice.Controllers return display; } + /// + /// Create the member + /// + /// + /// + /// + + private async Task CreateMemberAsync(MemberSave contentItem, UmbracoMembersIdentityUser identityMember) + { + IdentityResult created = await _memberManager.CreateAsync(identityMember, contentItem.Password.NewPassword); + if (created.Succeeded == false) + { + throw HttpResponseException.CreateNotificationValidationErrorResponse(created.Errors.ToErrorMessage()); + } + + //now re-look the member back up which will now exist + IMember member = _memberService.GetByEmail(contentItem.Email); + + member.CreatorId = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; + member.RawPasswordValue = identityMember.RawPasswordValue; + + //since the back office user is creating this member, they will be set to approved + member.IsApproved = true; + + //map the save info over onto the user + member = _umbracoMapper.Map(contentItem, member); + contentItem.PersistedContent = member; + } + /// /// Maps the property values to the persisted entity /// /// private void MapPropertyValues(MemberSave contentItem) { - UpdateName(contentItem); + //Don't update the name if it is empty + if (contentItem.Name.IsNullOrWhiteSpace() == false) + { + contentItem.PersistedContent.Name = contentItem.Name; + } //map the custom properties - this will already be set for new entities in our member binder contentItem.PersistedContent.Email = contentItem.Email; @@ -321,83 +392,9 @@ namespace Umbraco.Web.BackOffice.Controllers null); // member are all invariant } - /// /// Create a member from the supplied member content data /// All member password processing and creation is done via the aspnet identity MemberUserManager - /// - /// - /// - private async Task CreateMemberData(MemberSave memberSave) - { - if (memberSave == null) throw new ArgumentNullException("memberSave"); - if (ModelState.IsValid == false) - { - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); - } - //TODO: check if unique - - IMemberType memberType = _memberTypeService.Get(memberSave.ContentTypeAlias); - if (memberType == null) - { - throw new InvalidOperationException($"No member type found with alias {memberSave.ContentTypeAlias}"); - } - - // Create the member with the UserManager - // The 'empty' (special) password format is applied without us having to duplicate that logic - UmbracoMembersIdentityUser identityMember = UmbracoMembersIdentityUser.CreateNew( - memberSave.Username, - memberSave.Email, - memberSave.Name); - - //TODO: confirm - identityMember.MemberTypeAlias = memberType.Alias; - - IdentityResult created = await _memberManager.CreateAsync(identityMember); - if (created.Succeeded == false) - { - throw HttpResponseException.CreateNotificationValidationErrorResponse(created.Errors.ToErrorMessage()); - } - - //string resetPassword; - //string password = _memberManager.GeneratePassword(); - - //IdentityResult result = await _memberManager.AddPasswordAsync(identityMember, password); - //if (result.Succeeded == false) - //{ - // throw HttpResponseException.CreateNotificationValidationErrorResponse(created.Errors.ToErrorMessage()); - //} - - //resetPassword = password; - - //now re-look the member back up which will now exist - IMember member = _memberService.GetByEmail(memberSave.Email); - - //TODO: previous implementation - //IMember member = new Member( - // memberSave.Name, - // memberSave.Email, - // memberSave.Username, - // memberType, - // true) - //{ - // CreatorId = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id, - // RawPasswordValue = _memberManager.GeneratePassword(), - // Comments = memberSave.Comments, - // IsApproved = memberSave.IsApproved - //}; - - - //since the back office user is creating this member, they will be set to approved - member.IsApproved = true; - member.CreatorId = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; - member.Comments = memberSave.Comments; - member.IsApproved = memberSave.IsApproved; - - //map the save info over onto the user - member = _umbracoMapper.Map(memberSave, member); - return member; - } /// /// Update the member security data @@ -406,7 +403,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// If the password has been reset then this method will return the reset/generated password, otherwise will return null. /// - private void UpdateMemberData(MemberSave memberSave) + private async void UpdateMemberData(MemberSave memberSave) { //TODO: optimise based on new member manager memberSave.PersistedContent.WriterId = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; @@ -452,66 +449,12 @@ namespace Umbraco.Web.BackOffice.Controllers //no password changes then exit ? if (memberSave.Password == null) return; + //TODO: update member password functionality in manager// set the password // set the password memberSave.PersistedContent.RawPasswordValue = _memberManager.GeneratePassword(); } - private static void UpdateName(MemberSave memberSave) - { - //Don't update the name if it is empty - if (memberSave.Name.IsNullOrWhiteSpace() == false) - { - memberSave.PersistedContent.Name = memberSave.Name; - } - } - - // TODO: This logic should be pulled into the service layer - private async Task ValidateMemberDataAsync(MemberSave contentItem) - { - if (contentItem.Name.IsNullOrWhiteSpace()) - { - ModelState.AddPropertyError( - new ValidationResult("Invalid user name", new[] { "value" }), - $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}login"); - return false; - } - - if (contentItem.Password != null && !contentItem.Password.NewPassword.IsNullOrWhiteSpace()) - { - //TODO: implement as per backoffice user - //var validPassword = await _memberManager.CheckPasswordAsync(null, contentItem.Password.NewPassword); - //if (!validPassword) - //{ - // ModelState.AddPropertyError( - // new ValidationResult("Invalid password: TODO", new[] { "value" }), - // $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password"); - // return false; - //} - return true; - } - - var byUsername = _memberService.GetByUsername(contentItem.Username); - if (byUsername != null && byUsername.Key != contentItem.Key) - { - ModelState.AddPropertyError( - new ValidationResult("Username is already in use", new[] { "value" }), - $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}login"); - return false; - } - - var byEmail = _memberService.GetByEmail(contentItem.Email); - if (byEmail != null && byEmail.Key != contentItem.Key) - { - ModelState.AddPropertyError( - new ValidationResult("Email address is already in use", new[] { "value" }), - $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}email"); - return false; - } - - return true; - } - /// /// Permanently deletes a member ///