diff --git a/src/Umbraco.Core/Security/IUmbracoMemberTypeMembershipProvider.cs b/src/Umbraco.Core/Security/IUmbracoMemberTypeMembershipProvider.cs deleted file mode 100644 index bd165334f1..0000000000 --- a/src/Umbraco.Core/Security/IUmbracoMemberTypeMembershipProvider.cs +++ /dev/null @@ -1,21 +0,0 @@ -using System.Collections.Specialized; - -namespace Umbraco.Core.Security -{ - /// - /// An interface for exposing the content type properties for storing membership data in when - /// a membership provider's data is backed by an Umbraco content type. - /// - public interface IUmbracoMemberTypeMembershipProvider - { - - string LockPropertyTypeAlias { get; } - string LastLockedOutPropertyTypeAlias { get; } - string FailedPasswordAttemptsPropertyTypeAlias { get; } - string ApprovedPropertyTypeAlias { get; } - string CommentPropertyTypeAlias { get; } - string LastLoginPropertyTypeAlias { get; } - string LastPasswordChangedPropertyTypeAlias { get; } - - } -} diff --git a/src/Umbraco.Core/Security/PasswordSecurity.cs b/src/Umbraco.Core/Security/PasswordSecurity.cs index cedceecd3d..3e5d65dfd7 100644 --- a/src/Umbraco.Core/Security/PasswordSecurity.cs +++ b/src/Umbraco.Core/Security/PasswordSecurity.cs @@ -55,6 +55,9 @@ namespace Umbraco.Core.Security /// public string HashPasswordForStorage(string password) { + if (string.IsNullOrWhiteSpace(password)) + throw new ArgumentException("password cannot be empty", nameof(password)); + string salt; var hashed = HashNewPassword(password, out salt); return FormatPasswordForStorage(hashed, salt); diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 79a486b5f1..e50ed4153b 100755 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -737,7 +737,6 @@ - diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js index 825b4d1d44..a3017548a4 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js @@ -304,33 +304,6 @@ } saveModel.memberGroups = selectedGroups; - //turn the dictionary into an array of pairs - var memberProviderPropAliases = _.pairs(displayModel.fieldConfig); - _.each(displayModel.tabs, function (tab) { - _.each(tab.properties, function (prop) { - var foundAlias = _.find(memberProviderPropAliases, function (item) { - return prop.alias === item[1]; - }); - if (foundAlias) { - //we know the current property matches an alias, now we need to determine which membership provider property it was for - // by looking at the key - switch (foundAlias[0]) { - case "umbracoMemberLockedOut": - saveModel.isLockedOut = Object.toBoolean(prop.value); - break; - case "umbracoMemberApproved": - saveModel.isApproved = Object.toBoolean(prop.value); - break; - case "umbracoMemberComments": - saveModel.comments = prop.value; - break; - } - } - }); - }); - - - return saveModel; }, diff --git a/src/Umbraco.Web/Editors/Filters/MemberSaveModelValidator.cs b/src/Umbraco.Web/Editors/Filters/MemberSaveModelValidator.cs index 68068b373d..f0d9b1f1a9 100644 --- a/src/Umbraco.Web/Editors/Filters/MemberSaveModelValidator.cs +++ b/src/Umbraco.Web/Editors/Filters/MemberSaveModelValidator.cs @@ -58,9 +58,6 @@ namespace Umbraco.Web.Editors.Filters $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}email"); } - //default provider! - var membershipProvider = MembershipProviderExtensions.GetMembersMembershipProvider(); - var validEmail = ValidateUniqueEmail(model); if (validEmail == false) { diff --git a/src/Umbraco.Web/Editors/MemberController.cs b/src/Umbraco.Web/Editors/MemberController.cs index 01ff756990..a270460075 100644 --- a/src/Umbraco.Web/Editors/MemberController.cs +++ b/src/Umbraco.Web/Editors/MemberController.cs @@ -53,9 +53,7 @@ namespace Umbraco.Web.Editors private readonly IMemberPasswordConfiguration _passwordConfig; private readonly PropertyEditorCollection _propertyEditors; private PasswordSecurity _passwordSecurity; - private PasswordChanger _passwordChanger; private PasswordSecurity PasswordSecurity => _passwordSecurity ?? (_passwordSecurity = new PasswordSecurity(_passwordConfig)); - private PasswordChanger PasswordChanger => _passwordChanger ?? (_passwordChanger = new PasswordChanger(Logger)); public PagedResult GetPagedResults( int pageNumber = 1, @@ -281,7 +279,10 @@ namespace Umbraco.Web.Editors throw new InvalidOperationException($"No member type found with alias {contentItem.ContentTypeAlias}"); var member = new Member(contentItem.Name, contentItem.Email, contentItem.Username, memberType, true) { - CreatorId = Security.CurrentUser.Id + CreatorId = Security.CurrentUser.Id, + RawPasswordValue = PasswordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword), + Comments = contentItem.Comments, + IsApproved = contentItem.IsApproved }; return member; @@ -298,9 +299,10 @@ namespace Umbraco.Web.Editors { contentItem.PersistedContent.WriterId = Security.CurrentUser.Id; - //if the user doesn't have access to sensitive values, then we need to check if any of the built in member property types - //have been marked as sensitive. If that is the case we cannot change these persisted values no matter what value has been posted. - //There's only 3 special ones we need to deal with that are part of the MemberSave instance + // If the user doesn't have access to sensitive values, then we need to check if any of the built in member property types + // have been marked as sensitive. If that is the case we cannot change these persisted values no matter what value has been posted. + // There's only 3 special ones we need to deal with that are part of the MemberSave instance: Comments, IsApproved, IsLockedOut + // but we will take care of this in a generic way below so that it works for all props. if (!Security.CurrentUser.HasAccessToSensitiveData()) { var memberType = Services.MemberTypeService.Get(contentItem.PersistedContent.ContentTypeId); @@ -310,29 +312,25 @@ namespace Umbraco.Web.Editors foreach (var sensitiveProperty in sensitiveProperties) { - //if found, change the value of the contentItem model to the persisted value so it remains unchanged - switch (sensitiveProperty.Alias) + var destProp = contentItem.Properties.FirstOrDefault(x => x.Alias == sensitiveProperty.Alias); + if (destProp != null) { - case Constants.Conventions.Member.Comments: - contentItem.Comments = contentItem.PersistedContent.Comments; - break; - case Constants.Conventions.Member.IsApproved: - contentItem.IsApproved = contentItem.PersistedContent.IsApproved; - break; - case Constants.Conventions.Member.IsLockedOut: - contentItem.IsLockedOut = contentItem.PersistedContent.IsLockedOut; - break; + //if found, change the value of the contentItem model to the persisted value so it remains unchanged + var origValue = contentItem.PersistedContent.GetValue(sensitiveProperty.Alias); + destProp.Value = origValue; } } } + var isLockedOut = contentItem.IsLockedOut; + //if they were locked but now they are trying to be unlocked - if (contentItem.PersistedContent.IsLockedOut && contentItem.IsLockedOut == false) + if (contentItem.PersistedContent.IsLockedOut && isLockedOut == false) { contentItem.PersistedContent.IsLockedOut = false; contentItem.PersistedContent.FailedPasswordAttempts = 0; } - else if (!contentItem.PersistedContent.IsLockedOut && contentItem.IsLockedOut) + else if (!contentItem.PersistedContent.IsLockedOut && isLockedOut) { //NOTE: This should not ever happen unless someone is mucking around with the request data. //An admin cannot simply lock a user, they get locked out by password attempts, but an admin can un-approve them @@ -343,8 +341,8 @@ namespace Umbraco.Web.Editors if (contentItem.Password == null) return; - // change the password - contentItem.PersistedContent.RawPasswordValue = PasswordChanger.ChangePassword(contentItem.Password, PasswordSecurity); + // set the password + contentItem.PersistedContent.RawPasswordValue = PasswordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword); } private static void UpdateName(MemberSave memberSave) diff --git a/src/Umbraco.Web/Editors/PasswordChanger.cs b/src/Umbraco.Web/Editors/PasswordChanger.cs index 6b4227557b..6da4e364e1 100644 --- a/src/Umbraco.Web/Editors/PasswordChanger.cs +++ b/src/Umbraco.Web/Editors/PasswordChanger.cs @@ -101,14 +101,5 @@ namespace Umbraco.Web.Editors return Attempt.Succeed(new PasswordChangedModel()); } - public string ChangePassword(ChangingPasswordModel passwordModel, PasswordSecurity passwordSecurity) - { - if (passwordModel.NewPassword.IsNullOrWhiteSpace()) - throw new InvalidOperationException("No password value"); - - return passwordSecurity.HashPasswordForStorage(passwordModel.NewPassword); - } - - } } diff --git a/src/Umbraco.Web/Models/ContentEditing/MemberDisplay.cs b/src/Umbraco.Web/Models/ContentEditing/MemberDisplay.cs index 3391447535..f4200c4963 100644 --- a/src/Umbraco.Web/Models/ContentEditing/MemberDisplay.cs +++ b/src/Umbraco.Web/Models/ContentEditing/MemberDisplay.cs @@ -1,8 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Runtime.Serialization; -using Umbraco.Core.Models; -using Umbraco.Core.Models.Membership; +using System.Runtime.Serialization; namespace Umbraco.Web.Models.ContentEditing { @@ -12,24 +8,11 @@ namespace Umbraco.Web.Models.ContentEditing [DataContract(Name = "content", Namespace = "")] public class MemberDisplay : ListViewAwareContentItemDisplayBase { - public MemberDisplay() - { - MemberProviderFieldMapping = new Dictionary(); - } - [DataMember(Name = "username")] public string Username { get; set; } [DataMember(Name = "email")] public string Email { get; set; } - /// - /// This is used to indicate how to map the membership provider properties to the save model, this mapping - /// will change if a developer has opted to have custom member property aliases specified in their membership provider config, - /// or if we are editing a member that is not an Umbraco member (custom provider) - /// - [DataMember(Name = "fieldConfig")] - public IDictionary MemberProviderFieldMapping { get; set; } - } } diff --git a/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs b/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs index 19b41535a4..10774de1a4 100644 --- a/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs +++ b/src/Umbraco.Web/Models/ContentEditing/MemberSave.cs @@ -1,12 +1,14 @@ using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using System.Linq; using System.Runtime.Serialization; using Newtonsoft.Json.Linq; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.Models.Validation; using Umbraco.Web.WebApi.Filters; +using Umbraco.Core; namespace Umbraco.Web.Models.ContentEditing { @@ -29,16 +31,27 @@ namespace Umbraco.Web.Models.ContentEditing [DataMember(Name = "memberGroups")] public IEnumerable Groups { get; set; } - [DataMember(Name = "comments")] - public string Comments { get; set; } + /// + /// Returns the value from the Comments property + /// + public string Comments => GetPropertyValue(Constants.Conventions.Member.Comments); - [DataMember(Name = "isLockedOut")] - public bool IsLockedOut { get; set; } + /// + /// Returns the value from the IsLockedOut property + /// + public bool IsLockedOut => GetPropertyValue(Constants.Conventions.Member.IsLockedOut); - [DataMember(Name = "isApproved")] - public bool IsApproved { get; set; } - + /// + /// Returns the value from the IsApproved property + /// + public bool IsApproved => GetPropertyValue(Constants.Conventions.Member.IsApproved); - // TODO: Need to add question / answer support + private T GetPropertyValue(string alias) + { + var prop = Properties.FirstOrDefault(x => x.Alias == alias); + if (prop == null) return default; + var converted = prop.Value.TryConvertTo(); + return converted.ResultOr(default); + } } } diff --git a/src/Umbraco.Web/Models/Mapping/MemberMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/MemberMapDefinition.cs index 9b17ac9d70..37f86ae61e 100644 --- a/src/Umbraco.Web/Models/Mapping/MemberMapDefinition.cs +++ b/src/Umbraco.Web/Models/Mapping/MemberMapDefinition.cs @@ -1,16 +1,7 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using Umbraco.Core; +using Umbraco.Core; using Umbraco.Core.Mapping; using Umbraco.Core.Models; -using Umbraco.Core.Models.Membership; -using Umbraco.Core.Security; -using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; -using Umbraco.Core.Services.Implement; -using UserProfile = Umbraco.Web.Models.ContentEditing.UserProfile; -using Umbraco.Web.Security; namespace Umbraco.Web.Models.Mapping { @@ -49,7 +40,6 @@ namespace Umbraco.Web.Models.Mapping target.Icon = source.ContentType.Icon; target.Id = source.Id; target.Key = source.Key; - target.MemberProviderFieldMapping = GetMemberProviderFieldMapping(); target.Name = source.Name; target.Owner = _commonMapper.GetOwner(source, context); target.ParentId = source.ParentId; @@ -101,18 +91,5 @@ namespace Umbraco.Web.Models.Mapping target.Properties = context.MapEnumerable(source.Properties); } - private static IDictionary GetMemberProviderFieldMapping() - { - var provider = MembershipProviderExtensions.GetMembersMembershipProvider(); - - var umbracoProvider = (IUmbracoMemberTypeMembershipProvider)provider; - - return new Dictionary - { - {Constants.Conventions.Member.IsLockedOut, umbracoProvider.LockPropertyTypeAlias}, - {Constants.Conventions.Member.IsApproved, umbracoProvider.ApprovedPropertyTypeAlias}, - {Constants.Conventions.Member.Comments, umbracoProvider.CommentPropertyTypeAlias} - }; - } } } diff --git a/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs b/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs index 1a52816d14..616ad81fa4 100644 --- a/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/MemberTabsAndPropertiesMapper.cs @@ -5,13 +5,12 @@ using Umbraco.Core; using Umbraco.Core.Mapping; using Umbraco.Core.Composing; using Umbraco.Core.Models; -using Umbraco.Core.Models.Membership; -using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Web.Models.ContentEditing; using Umbraco.Core.Dictionary; using Umbraco.Web.Security; using Umbraco.Web.Security.Providers; +using Umbraco.Core.Configuration; namespace Umbraco.Web.Models.Mapping { @@ -30,8 +29,9 @@ namespace Umbraco.Web.Models.Mapping private readonly IMemberTypeService _memberTypeService; private readonly IMemberService _memberService; private readonly IMemberGroupService _memberGroupService; + private readonly IMemberPasswordConfiguration _memberPasswordConfiguration; - public MemberTabsAndPropertiesMapper(ICultureDictionary cultureDictionary, IUmbracoContextAccessor umbracoContextAccessor, ILocalizedTextService localizedTextService, IMemberTypeService memberTypeService, IMemberService memberService, IMemberGroupService memberGroupService) + public MemberTabsAndPropertiesMapper(ICultureDictionary cultureDictionary, IUmbracoContextAccessor umbracoContextAccessor, ILocalizedTextService localizedTextService, IMemberTypeService memberTypeService, IMemberService memberService, IMemberGroupService memberGroupService, IMemberPasswordConfiguration memberPasswordConfiguration) : base(cultureDictionary, localizedTextService) { _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); @@ -39,13 +39,13 @@ namespace Umbraco.Web.Models.Mapping _memberTypeService = memberTypeService ?? throw new ArgumentNullException(nameof(memberTypeService)); _memberService = memberService ?? throw new ArgumentNullException(nameof(memberService)); _memberGroupService = memberGroupService ?? throw new ArgumentNullException(nameof(memberGroupService)); + _memberPasswordConfiguration = memberPasswordConfiguration; } /// /// Overridden to deal with custom member properties and permissions. public override IEnumerable> Map(IMember source, MapperContext context) { - var provider = MembershipProviderExtensions.GetMembersMembershipProvider(); var memberType = _memberTypeService.Get(source.ContentTypeId); @@ -56,12 +56,10 @@ namespace Umbraco.Web.Models.Mapping var resolved = base.Map(source, context); - var umbracoProvider = (IUmbracoMemberTypeMembershipProvider)provider; - // This is kind of a hack because a developer is supposed to be allowed to set their property editor - would have been much easier // if we just had all of the membership provider fields on the member table :( // TODO: But is there a way to map the IMember.IsLockedOut to the property ? i dunno. - var isLockedOutProperty = resolved.SelectMany(x => x.Properties).FirstOrDefault(x => x.Alias == umbracoProvider.LockPropertyTypeAlias); + var isLockedOutProperty = resolved.SelectMany(x => x.Properties).FirstOrDefault(x => x.Alias == Constants.Conventions.Member.IsLockedOut); if (isLockedOutProperty?.Value != null && isLockedOutProperty.Value.ToString() != "1") { isLockedOutProperty.View = "readonlyvalue"; @@ -97,7 +95,6 @@ namespace Umbraco.Web.Models.Mapping protected override IEnumerable GetCustomGenericProperties(IContentBase content) { var member = (IMember)content; - var membersProvider = MembershipProviderExtensions.GetMembersMembershipProvider(); var genericProperties = new List { @@ -137,7 +134,7 @@ namespace Umbraco.Web.Models.Mapping // TODO: Hard coding this because the changepassword doesn't necessarily need to be a resolvable (real) property editor View = "changepassword", // initialize the dictionary with the configuration from the default membership provider - Config = GetPasswordConfig(membersProvider, member) + Config = GetPasswordConfig(member) }, new ContentPropertyDisplay { @@ -152,9 +149,9 @@ namespace Umbraco.Web.Models.Mapping return genericProperties; } - private Dictionary GetPasswordConfig(MembersMembershipProvider membersProvider, IMember member) + private Dictionary GetPasswordConfig(IMember member) { - var result = new Dictionary(membersProvider.PasswordConfiguration.GetConfiguration(true)) + var result = new Dictionary(_memberPasswordConfiguration.GetConfiguration(true)) { // the password change toggle will only be displayed if there is already a password assigned. {"hasPassword", member.RawPasswordValue.IsNullOrWhiteSpace() == false} diff --git a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs index 4d188b0073..37c8bd37d7 100644 --- a/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs +++ b/src/Umbraco.Web/Security/Providers/MembersMembershipProvider.cs @@ -15,7 +15,7 @@ namespace Umbraco.Web.Security.Providers /// /// Custom Membership Provider for Umbraco Members (User authentication for Frontend applications NOT umbraco CMS) /// - public class MembersMembershipProvider : UmbracoMembershipProvider, IUmbracoMemberTypeMembershipProvider + public class MembersMembershipProvider : UmbracoMembershipProvider { public MembersMembershipProvider() : this(Current.Services.MemberService, Current.Services.MemberTypeService, Current.UmbracoVersion)