Removes the remaining bits of membership providers stuff from being used in the back office, simplifies how properties are mapped and removes the legacy weirdness of mapping membership provider properties

This commit is contained in:
Shannon
2019-12-04 12:50:05 +11:00
parent e7c85daa2e
commit 29908dee44
12 changed files with 54 additions and 144 deletions

View File

@@ -1,21 +0,0 @@
using System.Collections.Specialized;
namespace Umbraco.Core.Security
{
/// <summary>
/// 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.
/// </summary>
public interface IUmbracoMemberTypeMembershipProvider
{
string LockPropertyTypeAlias { get; }
string LastLockedOutPropertyTypeAlias { get; }
string FailedPasswordAttemptsPropertyTypeAlias { get; }
string ApprovedPropertyTypeAlias { get; }
string CommentPropertyTypeAlias { get; }
string LastLoginPropertyTypeAlias { get; }
string LastPasswordChangedPropertyTypeAlias { get; }
}
}

View File

@@ -55,6 +55,9 @@ namespace Umbraco.Core.Security
/// <returns></returns>
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);

View File

@@ -737,7 +737,6 @@
<Compile Include="Security\BackOfficeUserValidator.cs" />
<Compile Include="Security\ContentPermissionsHelper.cs" />
<Compile Include="Security\EmailService.cs" />
<Compile Include="Security\IUmbracoMemberTypeMembershipProvider.cs" />
<Compile Include="Security\IUserAwarePasswordHasher.cs" />
<Compile Include="Security\IUserSessionStore.cs" />
<Compile Include="Security\MachineKeyGenerator.cs" />

View File

@@ -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;
},

View File

@@ -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)
{

View File

@@ -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<MemberBasic> 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)

View File

@@ -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);
}
}
}

View File

@@ -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<ContentPropertyDisplay>
{
public MemberDisplay()
{
MemberProviderFieldMapping = new Dictionary<string, string>();
}
[DataMember(Name = "username")]
public string Username { get; set; }
[DataMember(Name = "email")]
public string Email { get; set; }
/// <summary>
/// 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)
/// </summary>
[DataMember(Name = "fieldConfig")]
public IDictionary<string, string> MemberProviderFieldMapping { get; set; }
}
}

View File

@@ -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<string> Groups { get; set; }
[DataMember(Name = "comments")]
public string Comments { get; set; }
/// <summary>
/// Returns the value from the Comments property
/// </summary>
public string Comments => GetPropertyValue<string>(Constants.Conventions.Member.Comments);
[DataMember(Name = "isLockedOut")]
public bool IsLockedOut { get; set; }
/// <summary>
/// Returns the value from the IsLockedOut property
/// </summary>
public bool IsLockedOut => GetPropertyValue<bool>(Constants.Conventions.Member.IsLockedOut);
[DataMember(Name = "isApproved")]
public bool IsApproved { get; set; }
/// <summary>
/// Returns the value from the IsApproved property
/// </summary>
public bool IsApproved => GetPropertyValue<bool>(Constants.Conventions.Member.IsApproved);
// TODO: Need to add question / answer support
private T GetPropertyValue<T>(string alias)
{
var prop = Properties.FirstOrDefault(x => x.Alias == alias);
if (prop == null) return default;
var converted = prop.Value.TryConvertTo<T>();
return converted.ResultOr(default);
}
}
}

View File

@@ -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<IProperty, ContentPropertyDto>(source.Properties);
}
private static IDictionary<string, string> GetMemberProviderFieldMapping()
{
var provider = MembershipProviderExtensions.GetMembersMembershipProvider();
var umbracoProvider = (IUmbracoMemberTypeMembershipProvider)provider;
return new Dictionary<string, string>
{
{Constants.Conventions.Member.IsLockedOut, umbracoProvider.LockPropertyTypeAlias},
{Constants.Conventions.Member.IsApproved, umbracoProvider.ApprovedPropertyTypeAlias},
{Constants.Conventions.Member.Comments, umbracoProvider.CommentPropertyTypeAlias}
};
}
}
}

View File

@@ -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;
}
/// <inheritdoc />
/// <remarks>Overridden to deal with custom member properties and permissions.</remarks>
public override IEnumerable<Tab<ContentPropertyDisplay>> 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<ContentPropertyDisplay> GetCustomGenericProperties(IContentBase content)
{
var member = (IMember)content;
var membersProvider = MembershipProviderExtensions.GetMembersMembershipProvider();
var genericProperties = new List<ContentPropertyDisplay>
{
@@ -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<string, object> GetPasswordConfig(MembersMembershipProvider membersProvider, IMember member)
private Dictionary<string, object> GetPasswordConfig(IMember member)
{
var result = new Dictionary<string, object>(membersProvider.PasswordConfiguration.GetConfiguration(true))
var result = new Dictionary<string, object>(_memberPasswordConfiguration.GetConfiguration(true))
{
// the password change toggle will only be displayed if there is already a password assigned.
{"hasPassword", member.RawPasswordValue.IsNullOrWhiteSpace() == false}

View File

@@ -15,7 +15,7 @@ namespace Umbraco.Web.Security.Providers
/// <summary>
/// Custom Membership Provider for Umbraco Members (User authentication for Frontend applications NOT umbraco CMS)
/// </summary>
public class MembersMembershipProvider : UmbracoMembershipProvider<IMembershipMemberService, IMember>, IUmbracoMemberTypeMembershipProvider
public class MembersMembershipProvider : UmbracoMembershipProvider<IMembershipMemberService, IMember>
{
public MembersMembershipProvider()
: this(Current.Services.MemberService, Current.Services.MemberTypeService, Current.UmbracoVersion)