diff --git a/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs b/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs index da4f823c70..fb92edd47d 100644 --- a/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs +++ b/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs @@ -1,77 +1,179 @@ -using System; +using System; +using System.Collections.Generic; +using System.ComponentModel; +using Umbraco.Core.Models.Entities; namespace Umbraco.Core.Members { /// /// An Umbraco member user type + /// TODO: use of identity classes in future /// - public class UmbracoMembersIdentityUser - //: IRememberBeingDirty - //TODO: use of identity classes - //: IdentityUser, IdentityUserClaim>, + public class UmbracoMembersIdentityUser : IRememberBeingDirty { - private bool _hasIdentity; private int _id; - 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; } + private string _passwordHash; + + private string _passwordConfig; /// - /// Returns true if an Id has been set on this object + /// Gets or sets the member name + /// + public string Name { get; set; } + + /// + /// Gets or sets the member email + /// + public string Email { get; set; } + + /// + /// Gets or sets the member username + /// + public string UserName { get; set; } + + /// + /// Gets or sets the alias of the member type + /// + public string MemberTypeAlias { get; set; } + + /// + /// Gets or sets a value indicating whether the member is locked out + /// + public bool IsLockedOut { get; set; } + + /// + /// Gets a value indicating whether an Id has been set on this object /// This will be false if the object is new and not persisted to the database /// - public bool HasIdentity => _hasIdentity; + public bool HasIdentity { get; private set; } + /// + /// Gets or sets the member Id + /// public int Id { get => _id; set { _id = value; - _hasIdentity = true; + HasIdentity = true; } } - //TODO: track - public string PasswordHash { get; set; } + /// + /// Gets or sets the salted/hashed form of the user password + /// + public string PasswordHash + { + get => _passwordHash; + set => BeingDirty.SetPropertyValueAndDetectChanges(value, ref _passwordHash, nameof(PasswordHash)); + } - //TODO: config - public string PasswordConfig { get; set; } + /// + /// Gets or sets the password config + /// + public string PasswordConfig + { + get => _passwordConfig; + set => BeingDirty.SetPropertyValueAndDetectChanges(value, ref _passwordConfig, nameof(PasswordConfig)); + } - internal bool IsApproved; + /// + /// Gets or sets a value indicating whether member Is Approved + /// + public bool IsApproved { get; set; } - //TODO: needed? + /// + /// Gets the for change tracking + /// + protected BeingDirty BeingDirty { get; } = new BeingDirty(); + + // TODO: implement as per base identity user //public bool LoginsChanged; //public bool RolesChanged; - + /// + /// Create a new identity member + /// + /// The member username + /// The member email + /// The member type alias + /// The member name + /// TODO: confirm The password (may be null in some instances) + /// Throws is username is null or whitespace + /// The identity member user public static UmbracoMembersIdentityUser CreateNew( string username, string email, string memberTypeAlias, - string name) + string name, + string password = null) { - if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); + if (string.IsNullOrWhiteSpace(username)) + { + throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); + } - //no groups/roles yet + // no groups/roles yet var member = new UmbracoMembersIdentityUser { UserName = username, Email = email, Name = name, MemberTypeAlias = memberTypeAlias, - Id = 0, //TODO: is this meant to be 0 in this circumstance? - //false by default unless specifically set - _hasIdentity = false + Id = 0, // TODO: is this meant to be 0 in this circumstance? + // false by default unless specifically set + HasIdentity = false }; - //TODO: do we use this? - //member.EnableChangeTracking(); + member.EnableChangeTracking(); return member; } + + /// + public event PropertyChangedEventHandler PropertyChanged + { + add => BeingDirty.PropertyChanged += value; + + remove => BeingDirty.PropertyChanged -= value; + } + + /// + + public bool IsDirty() => BeingDirty.IsDirty(); + + /// + public bool IsPropertyDirty(string propName) => BeingDirty.IsPropertyDirty(propName); + + /// + public IEnumerable GetDirtyProperties() => BeingDirty.GetDirtyProperties(); + + /// + + public void ResetDirtyProperties() => BeingDirty.ResetDirtyProperties(); + + /// + + public void DisableChangeTracking() => BeingDirty.DisableChangeTracking(); + + /// + + public void EnableChangeTracking() => BeingDirty.EnableChangeTracking(); + + /// + public bool WasDirty() => BeingDirty.WasDirty(); + + /// + public bool WasPropertyDirty(string propertyName) => BeingDirty.WasPropertyDirty(propertyName); + + /// + public void ResetWereDirtyProperties() => BeingDirty.ResetWereDirtyProperties(); + + /// + public void ResetDirtyProperties(bool rememberDirty) => BeingDirty.ResetDirtyProperties(rememberDirty); + + /// + public IEnumerable GetWereDirtyProperties() => BeingDirty.GetWereDirtyProperties(); } } diff --git a/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs b/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs index 02c1436a44..8eedb67af6 100644 --- a/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs +++ b/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs @@ -1,4 +1,5 @@ -using System; +using System; +using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; using Umbraco.Core.Members; @@ -12,11 +13,10 @@ namespace Umbraco.Infrastructure.Members public interface IUmbracoMembersUserManager : IDisposable where TUser : UmbracoMembersIdentityUser { /// - /// Creates the specified in the backing store with a password, - /// as an asynchronous operation. + /// Creates the specified user in the backing store with given password, as an asynchronous operation. /// /// The member to create. - /// The password to add + /// The new password /// /// The that represents the asynchronous operation, containing the /// of the operation. @@ -26,7 +26,7 @@ namespace Umbraco.Infrastructure.Members /// /// Helper method to generate a password for a user based on the current password validator /// - /// + /// Returns the generated password string GeneratePassword(); /// @@ -51,5 +51,12 @@ namespace Umbraco.Infrastructure.Members /// the specified matches the one store for the , /// otherwise false. Task CheckPasswordAsync(TUser memberUser, string password); + + /// + /// Method to validate the password without an identity user + /// + /// The password to validate + /// The result of the validation + Task> ValidatePassword(string password); } } diff --git a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs index 1637eb2ff7..3ab230a76e 100644 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs +++ b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs @@ -1,14 +1,14 @@ -using Microsoft.AspNetCore.Identity; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Umbraco.Core.Configuration; +using Umbraco.Core.Configuration.Models; using Umbraco.Core.Members; using Umbraco.Core.Security; -using System.Threading; -using Umbraco.Core.Configuration.Models; namespace Umbraco.Infrastructure.Members @@ -18,6 +18,7 @@ namespace Umbraco.Infrastructure.Members /// public class UmbracoMembersUserManager : UmbracoMembersUserManager, IUmbracoMembersUserManager { + /// public UmbracoMembersUserManager( IUserStore store, IOptions optionsAccessor, @@ -34,13 +35,28 @@ namespace Umbraco.Infrastructure.Members } } + /// + /// Manager for the member identity user + /// + /// The identity user public class UmbracoMembersUserManager : UserManager where T : UmbracoMembersIdentityUser { - public IPasswordConfiguration PasswordConfiguration { get; protected set; } - private PasswordGenerator _passwordGenerator; + /// + /// Initializes a new instance of the class. + /// + /// The members store + /// The identity options accessor + /// The password hasher + /// The user validators + /// The password validators + /// The keep lookup normalizer + /// The error display messages + /// The service provider + /// The logger + /// The password configuration public UmbracoMembersUserManager( IUserStore store, IOptions optionsAccessor, @@ -51,14 +67,17 @@ namespace Umbraco.Infrastructure.Members IdentityErrorDescriber errors, IServiceProvider services, ILogger> logger, - IOptions passwordConfiguration) : - base(store, optionsAccessor, passwordHasher, userValidators, passwordValidators, keyNormalizer, errors, services, logger) - { + IOptions passwordConfiguration) : base(store, optionsAccessor, passwordHasher, userValidators, passwordValidators, keyNormalizer, errors, services, logger) => PasswordConfiguration = passwordConfiguration.Value ?? throw new ArgumentNullException(nameof(passwordConfiguration)); - } + /// - /// Replace the underlying options property with our own strongly typed version + /// Gets or sets the password configuration + /// + public IPasswordConfiguration PasswordConfiguration { get; protected set; } + + /// + /// gets or sets the underlying options property with our own strongly typed version /// public new UmbracoMembersIdentityOptions Options { @@ -67,24 +86,24 @@ namespace Umbraco.Infrastructure.Members } /// - /// Gets/sets the default Umbraco member user password checker + /// Gets or sets the default Umbraco member user password checker /// public IUmbracoMembersUserPasswordChecker UmbracoMembersUserPasswordChecker { get; set; } - /// - /// [TODO: from BackOfficeUserManager duplicated, could be shared] - /// Override to determine how to hash the password - /// - /// - /// - /// - /// - /// - /// This method is called anytime the password needs to be hashed for storage (i.e. including when reset password is used) - /// + /// + /// TODO: from BackOfficeUserManager duplicated, could be shared + /// Override to determine how to hash the password + /// + /// The member to validate + /// The new password + /// Whether to validate the password + /// The identity result of updating the password hash + /// + /// This method is called anytime the password needs to be hashed for storage (i.e. including when reset password is used) + /// protected override async Task UpdatePasswordHash(T memberUser, string newPassword, bool validatePassword) { - //memberUser.LastPasswordChangeDateUtc = DateTime.UtcNow; + // memberUser.LastPasswordChangeDateUtc = DateTime.UtcNow; if (validatePassword) { @@ -95,8 +114,10 @@ namespace Umbraco.Infrastructure.Members } } - var passwordStore = Store as IUserPasswordStore; - if (passwordStore == null) throw new NotSupportedException("The current user store does not implement " + typeof(IUserPasswordStore<>)); + if (!(Store is IUserPasswordStore passwordStore)) + { + throw new NotSupportedException("The current user store does not implement " + typeof(IUserPasswordStore<>)); + } var hash = newPassword != null ? PasswordHasher.HashPassword(memberUser, newPassword) : null; await passwordStore.SetPasswordHashAsync(memberUser, hash, CancellationToken); @@ -104,13 +125,13 @@ namespace Umbraco.Infrastructure.Members return IdentityResult.Success; } - ///TODO: duplicated code from backofficeusermanager, could be shared? + /// TODO: duplicated code from backofficeusermanager, could be shared /// /// Logic used to validate a username and password /// - /// - /// - /// + /// The member to validate + /// The password to validate + /// Whether the password is the correct password for this member /// /// By default this uses the standard ASP.Net Identity approach which is: /// * Get password store @@ -136,68 +157,88 @@ namespace Umbraco.Infrastructure.Members return false; } - //if the result indicates to not fallback to the default, then return true if the credentials are valid + // if the result indicates to not fallback to the default, then return true if the credentials are valid if (result != UmbracoMembersUserPasswordCheckerResult.FallbackToDefaultChecker) { return result == UmbracoMembersUserPasswordCheckerResult.ValidCredentials; } } - //we cannot proceed if the user passed in does not have an identity + // we cannot proceed if the user passed in does not have an identity if (member.HasIdentity == false) + { return false; + } - //use the default behavior + // use the default behavior return await base.CheckPasswordAsync(member, password); } - ///[TODO: from BackOfficeUserManager duplicated, could be shared] + /// TODO: from BackOfficeUserManager duplicated, could be shared /// /// This is copied from the underlying .NET base class since they decided to not expose it /// - /// - /// + /// The user to update the security stamp for + /// Task returns private async Task UpdateSecurityStampInternal(T user) { - if (SupportsUserSecurityStamp == false) return; + if (SupportsUserSecurityStamp == false) + { + return; + } + await GetSecurityStore().SetSecurityStampAsync(user, NewSecurityStamp(), CancellationToken.None); } - ///[TODO: from BackOfficeUserManager duplicated, could be shared] + /// TODO: from BackOfficeUserManager duplicated, could be shared /// /// This is copied from the underlying .NET base class since they decided to not expose it /// - /// + /// Return a user security stamp private IUserSecurityStampStore GetSecurityStore() { - var store = Store as IUserSecurityStampStore; - if (store == null) throw new NotSupportedException("The current user store does not implement " + typeof(IUserSecurityStampStore<>)); + if (!(Store is IUserSecurityStampStore store)) + { + throw new NotSupportedException("The current user store does not implement " + typeof(IUserSecurityStampStore<>)); + } + return store; } - ///[TODO: from BackOfficeUserManager duplicated, could be shared] + /// TODO: from BackOfficeUserManager duplicated, could be shared /// /// This is copied from the underlying .NET base class since they decided to not expose it /// - /// - private static string NewSecurityStamp() - { - return Guid.NewGuid().ToString(); - } + /// Returns a new security stamp + private static string NewSecurityStamp() => Guid.NewGuid().ToString(); - ///[TODO: from BackOfficeUserManager duplicated, could be shared] /// + /// TODO: from BackOfficeUserManager duplicated, could be shared /// Helper method to generate a password for a member based on the current password validator /// - /// + /// The generated password public string GeneratePassword() { - if (_passwordGenerator == null) - { - _passwordGenerator = new PasswordGenerator(PasswordConfiguration); - } + _passwordGenerator ??= new PasswordGenerator(PasswordConfiguration); string password = _passwordGenerator.GeneratePassword(); return password; } + + /// + /// Helper method to validate a password based on the current password validator + /// + /// The password to update + /// The validated password + public async Task> ValidatePassword(string password) + { + var passwordValidators = new List(); + foreach(IPasswordValidator validator in PasswordValidators) + { + IdentityResult result = await validator.ValidateAsync(this, null, password); + passwordValidators.Add(result); + } + + return passwordValidators; + } } } diff --git a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs index 0d8b296c05..d68b21dc1b 100644 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs +++ b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs @@ -1,12 +1,15 @@ -using Microsoft.AspNetCore.Identity; using System; using System.Data; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; using Umbraco.Core; +using Umbraco.Core.BackOffice; using Umbraco.Core.Mapping; using Umbraco.Core.Members; using Umbraco.Core.Models; +using Umbraco.Core.Models.Identity; +using Umbraco.Core.Scoping; using Umbraco.Core.Services; namespace Umbraco.Infrastructure.Members @@ -25,40 +28,57 @@ namespace Umbraco.Infrastructure.Members //IUserTwoFactorStore //IUserSessionStore { - private bool _disposed = false; + private readonly bool _disposed = false; private readonly IMemberService _memberService; private readonly UmbracoMapper _mapper; + private readonly IScopeProvider _scopeProvider; - public UmbracoMembersUserStore(IMemberService memberService, UmbracoMapper mapper) + /// + /// Initializes a new instance of the class for the members identity store + /// + /// The member service + /// The mapper for properties + /// The scope provider + public UmbracoMembersUserStore(IMemberService memberService, UmbracoMapper mapper, IScopeProvider scopeProvider) { _memberService = memberService; _mapper = mapper; + _scopeProvider = scopeProvider; } + /// + /// Create the member as an identity user + /// + /// The identity user` for a member + /// The cancellation token + /// The identity result public Task CreateAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - if (user == null) throw new ArgumentNullException(nameof(user)); + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } - //create member - //TODO: are we keeping this method, e.g. the Member Service? The user service creates it directly, but this gets the membertype + // create member + // TODO: are we keeping this method, e.g. the Member Service? + // The user service creates it directly, but this way we get the member type by alias first IMember member = _memberService.CreateMember( user.UserName, user.Email, user.Name.IsNullOrWhiteSpace() ? user.UserName : user.Name, - user.MemberTypeAlias.IsNullOrWhiteSpace() ? - Constants.Security.DefaultMemberTypeAlias : user.MemberTypeAlias); + user.MemberTypeAlias.IsNullOrWhiteSpace() ? Constants.Security.DefaultMemberTypeAlias : user.MemberTypeAlias); UpdateMemberProperties(member, user); - //TODO: do we want to accept empty passwords here - if third-party for example? In other method if so? + // TODO: do we want to accept empty passwords here - if third-party for example? + // In other method if so? _memberService.Save(member); - //re-assign id + // re-assign id user.Id = member.Id; - // TODO: do we need this? // TODO: [from backofficeuser] we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. //bool isLoginsPropertyDirty = member.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.Logins)); @@ -72,18 +92,21 @@ namespace Umbraco.Infrastructure.Members // x.UserData))); //} - if (!member.HasIdentity) throw new DataException("Could not create the user, check logs for details"); + if (!member.HasIdentity) + { + throw new DataException("Could not create the member, check logs for details"); + } return Task.FromResult(IdentityResult.Success); - //TODO: confirm + // TODO: confirm and implement //if (memberUser.LoginsChanged) //{ // var logins = await GetLoginsAsync(memberUser); // _externalLoginStore.SaveUserLogins(member.Id, logins); //} - //TODO: confirm + // TODO: confirm and implement //if (memberUser.RolesChanged) //{ //IMembershipRoleService memberRoleService = _memberService; @@ -102,24 +125,23 @@ namespace Umbraco.Infrastructure.Members private bool UpdateMemberProperties(IMember member, UmbracoMembersIdentityUser memberIdentityUser) { - //[Comments as per BackOfficeUserStore & identity package] var anythingChanged = false; - //don't assign anything if nothing has changed as this will trigger the track changes of the model - if ( - //memberIdentityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Name)) && + + // don't assign anything if nothing has changed as this will trigger the track changes of the model + if (memberIdentityUser.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.Name)) && member.Name != memberIdentityUser.Name && memberIdentityUser.Name.IsNullOrWhiteSpace() == false) { anythingChanged = true; member.Name = memberIdentityUser.Name; } - if ( - //memberIdentityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Email)) && + + if (memberIdentityUser.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.Email)) && member.Email != memberIdentityUser.Email && memberIdentityUser.Email.IsNullOrWhiteSpace() == false) { anythingChanged = true; member.Email = memberIdentityUser.Email; } - + if (member.IsLockedOut != memberIdentityUser.IsLockedOut) { anythingChanged = true; @@ -127,22 +149,20 @@ namespace Umbraco.Infrastructure.Members if (member.IsLockedOut) { - //need to set the last lockout date + // need to set the last lockout date member.LastLockoutDate = DateTime.Now; } } - if ( - //memberIdentityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.UserName)) && + + if (memberIdentityUser.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.UserName)) && member.Username != memberIdentityUser.UserName && memberIdentityUser.UserName.IsNullOrWhiteSpace() == false) { anythingChanged = true; member.Username = memberIdentityUser.UserName; } - - if ( - //member.IsPropertyDirty(nameof(BackOfficeIdentityUser.PasswordHash))&& - member.RawPasswordValue != memberIdentityUser.PasswordHash - && memberIdentityUser.PasswordHash.IsNullOrWhiteSpace() == false) + + if (memberIdentityUser.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.PasswordHash)) + && member.RawPasswordValue != memberIdentityUser.PasswordHash && memberIdentityUser.PasswordHash.IsNullOrWhiteSpace() == false) { anythingChanged = true; member.RawPasswordValue = memberIdentityUser.PasswordHash; @@ -151,7 +171,7 @@ namespace Umbraco.Infrastructure.Members // TODO: Roles // [Comment] Same comment as per BackOfficeUserStore: Fix this for Groups too - //if (identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Roles)) || identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Groups))) + //if (identityUser.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.Roles)) || identityUser.IsPropertyDirty(nameof(BackOfficeIdentityUser.Groups))) //{ // var userGroupAliases = member.Groups.Select(x => x.Alias).ToArray(); @@ -182,9 +202,7 @@ namespace Umbraco.Infrastructure.Members // } //} - //TODO: reset all changes - //memberIdentityUser.ResetDirtyProperties(false); - + memberIdentityUser.ResetDirtyProperties(false); return anythingChanged; } @@ -200,16 +218,17 @@ namespace Umbraco.Infrastructure.Members public async Task FindByNameAsync(string normalizedUserName, CancellationToken cancellationToken) { - //TODO: confirm logic + // TODO: confirm logic cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - var member = _memberService.GetByUsername(normalizedUserName); + + IMember member = _memberService.GetByUsername(normalizedUserName); if (member == null) { return null; } - var result = _mapper.Map(member); + UmbracoMembersIdentityUser result = _mapper.Map(member); return await Task.FromResult(result); } @@ -223,64 +242,147 @@ namespace Umbraco.Infrastructure.Members { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - if (user == null) throw new ArgumentNullException(nameof(user)); + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } return Task.FromResult(user.Id.ToString()); } public Task GetUserNameAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) { - //TODO: unit tests for and implement all bodies + // TODO: unit tests for and implement all bodies cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - if (user == null) throw new ArgumentNullException(nameof(user)); + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } return Task.FromResult(user.UserName); } - public Task SetNormalizedUserNameAsync(UmbracoMembersIdentityUser user, string normalizedName, CancellationToken cancellationToken) - { - return SetUserNameAsync(user, normalizedName, cancellationToken); throw new NotImplementedException(); - } + /// + /// Sets the normalized user name + /// + /// The member identity user + /// The normalized member name + /// The cancellation token + /// A task once complete + public Task SetNormalizedUserNameAsync(UmbracoMembersIdentityUser user, string normalizedName, CancellationToken cancellationToken) => SetUserNameAsync(user, normalizedName, cancellationToken); + /// + /// Sets the user name as an async operation + /// + /// The member identity user + /// The member user name + /// The cancellation token + /// A task once complete public Task SetUserNameAsync(UmbracoMembersIdentityUser user, string userName, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - if (user == null) throw new ArgumentNullException(nameof(user)); + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } user.UserName = userName; return Task.CompletedTask; } - public Task UpdateAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) + /// + /// Update the user asynchronously + /// + /// The member identity user + /// The cancellation token + /// An identity result task + public Task UpdateAsync(UmbracoMembersIdentityUser member, CancellationToken cancellationToken) { - throw new NotImplementedException(); + cancellationToken.ThrowIfCancellationRequested(); + ThrowIfDisposed(); + if (member == null) + { + throw new ArgumentNullException(nameof(member)); + } + + Attempt asInt = member.Id.TryConvertTo(); + if (asInt == false) + { + throw new InvalidOperationException("The member id must be an integer to work with the Umbraco"); + } + + using (IScope scope = _scopeProvider.CreateScope()) + { + IMember found = _memberService.GetById(asInt.Result); + if (found != null) + { + // we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. + var isLoginsPropertyDirty = member.IsPropertyDirty(nameof(BackOfficeIdentityUser.Logins)); + + if (UpdateMemberProperties(found, member)) + { + _memberService.Save(found); + } + + //if (isLoginsPropertyDirty) + //{ + // _externalLoginService.Save( + // found.Id, + // member.Logins.Select(x => new ExternalLogin( + // x.LoginProvider, + // x.ProviderKey, + // x.UserData))); + //} + } + + scope.Complete(); + } + + return Task.FromResult(IdentityResult.Success); } private void ThrowIfDisposed() { if (_disposed) + { throw new ObjectDisposedException(GetType().Name); + } } - ///TODO: All from BackOfficeUserStore - same. Can we share? + /// TODO: All from BackOfficeUserStore - same. Can we share? /// /// Set the user password hash /// - /// - /// - /// + /// The identity member user + /// The password hash + /// The cancellation token + /// Throws if the properties are null + /// Returns asynchronously public Task SetPasswordHashAsync(UmbracoMembersIdentityUser user, string passwordHash, CancellationToken cancellationToken = default(CancellationToken)) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - if (user == null) throw new ArgumentNullException(nameof(user)); - if (passwordHash == null) throw new ArgumentNullException(nameof(passwordHash)); - if (string.IsNullOrEmpty(passwordHash)) throw new ArgumentException("Value can't be empty.", nameof(passwordHash)); + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + + if (passwordHash == null) + { + throw new ArgumentNullException(nameof(passwordHash)); + } + + if (string.IsNullOrEmpty(passwordHash)) + { + throw new ArgumentException("Value can't be empty.", nameof(passwordHash)); + } user.PasswordHash = passwordHash; - user.PasswordConfig = null; // Clear this so that it's reset at the repository level + + // Clear this so that it's reset at the repository level + user.PasswordConfig = null; return Task.CompletedTask; } @@ -290,12 +392,16 @@ namespace Umbraco.Infrastructure.Members /// /// /// + /// /// public Task GetPasswordHashAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken = default(CancellationToken)) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - if (user == null) throw new ArgumentNullException(nameof(user)); + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } return Task.FromResult(user.PasswordHash); } @@ -303,17 +409,19 @@ namespace Umbraco.Infrastructure.Members /// /// Returns true if a user has a password set /// - /// - /// - /// + /// The identity user + /// The cancellation token + /// True if the user has a password public Task HasPasswordAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken = default(CancellationToken)) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); - if (user == null) throw new ArgumentNullException(nameof(user)); + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } return Task.FromResult(string.IsNullOrEmpty(user.PasswordHash) == false); } - } } diff --git a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs index 4ae0458910..24c15957e2 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MemberService.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using Microsoft.Extensions.Logging; @@ -109,19 +109,20 @@ namespace Umbraco.Core.Services.Implement /// Email of the Member to create /// Name of the Member to create /// Alias of the MemberType the Member should be based on + /// Thrown when a member type for the given alias isn't found /// public IMember CreateMember(string username, string email, string name, string memberTypeAlias) { - var memberType = GetMemberType(memberTypeAlias); + IMemberType memberType = GetMemberType(memberTypeAlias); if (memberType == null) + { throw new ArgumentException("No member type with that alias.", nameof(memberTypeAlias)); + } var member = new Member(name, email.ToLower().Trim(), username, memberType); - using (var scope = ScopeProvider.CreateScope()) - { - CreateMember(scope, member, 0, false); - scope.Complete(); - } + using IScope scope = ScopeProvider.CreateScope(); + CreateMember(scope, member, 0, false); + scope.Complete(); return member; } @@ -312,7 +313,9 @@ namespace Umbraco.Core.Services.Implement // if saving is cancelled, media remains without an identity var saveEventArgs = new SaveEventArgs(member); if (scope.Events.DispatchCancelable(Saving, this, saveEventArgs)) + { return; + } _memberRepository.Save(member); @@ -321,7 +324,9 @@ namespace Umbraco.Core.Services.Implement } if (withIdentity == false) + { return; + } Audit(AuditType.New, member.CreatorId, member.Id, $"Member '{member.Name}' was created with Id {member.Id}"); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs index 59334d763c..5f8a143329 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; using System.Linq; @@ -10,6 +10,7 @@ using NUnit.Framework; using Umbraco.Core.Mapping; using Umbraco.Core.Members; using Umbraco.Core.Models; +using Umbraco.Core.Scoping; using Umbraco.Core.Services; using Umbraco.Infrastructure.Members; using Umbraco.Tests.UnitTests.Umbraco.Core.ShortStringHelper; @@ -26,7 +27,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members _mockMemberService = new Mock(); return new UmbracoMembersUserStore( _mockMemberService.Object, - new UmbracoMapper(new MapDefinitionCollection(new List()))); + new UmbracoMapper(new MapDefinitionCollection(new List())), + new Mock().Object); } [Test] diff --git a/src/Umbraco.Web.BackOffice/Controllers/ContentControllerBase.cs b/src/Umbraco.Web.BackOffice/Controllers/ContentControllerBase.cs index aef6abdd5e..ec8f09bc17 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/ContentControllerBase.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/ContentControllerBase.cs @@ -1,15 +1,10 @@ using System; using System.Linq; -using System.Net; -using System.Net.Http; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Umbraco.Core; -using Umbraco.Core.Cache; -using Umbraco.Core.Configuration; using Umbraco.Core.Dictionary; using Umbraco.Core.Events; -using Umbraco.Core.Mapping; using Umbraco.Core.Models; using Umbraco.Core.Models.Editors; using Umbraco.Core.PropertyEditors; @@ -54,14 +49,22 @@ namespace Umbraco.Web.BackOffice.Controllers _serializer = serializer; } + /// + /// Handles if the content for the specified ID isn't found + /// + /// The content ID to find + /// Whether to throw an exception + /// The error response protected NotFoundObjectResult HandleContentNotFound(object id, bool throwException = true) { ModelState.AddModelError("id", $"content with id: {id} was not found"); - var errorResponse = NotFound(ModelState); + NotFoundObjectResult errorResponse = NotFound(ModelState); + if (throwException) { throw new HttpResponseException(errorResponse); } + return errorResponse; } @@ -78,7 +81,7 @@ namespace Umbraco.Web.BackOffice.Controllers where TSaved : IContentSave { // map the property values - foreach (var propertyDto in dto.Properties) + foreach (ContentPropertyDto propertyDto in dto.Properties) { // get the property editor if (propertyDto.PropertyEditor == null) @@ -89,48 +92,63 @@ namespace Umbraco.Web.BackOffice.Controllers // get the value editor // nothing to save/map if it is readonly - var valueEditor = propertyDto.PropertyEditor.GetValueEditor(); - if (valueEditor.IsReadOnly) continue; + IDataValueEditor valueEditor = propertyDto.PropertyEditor.GetValueEditor(); + if (valueEditor.IsReadOnly) + { + continue; + } // get the property - var property = contentItem.PersistedContent.Properties[propertyDto.Alias]; + IProperty property = contentItem.PersistedContent.Properties[propertyDto.Alias]; // prepare files, if any matching property and culture - var files = contentItem.UploadedFiles + ContentPropertyFile[] files = contentItem.UploadedFiles .Where(x => x.PropertyAlias == propertyDto.Alias && x.Culture == propertyDto.Culture && x.Segment == propertyDto.Segment) .ToArray(); - foreach (var file in files) + foreach (ContentPropertyFile file in files) + { file.FileName = file.FileName.ToSafeFileName(ShortStringHelper); + } // create the property data for the property editor var data = new ContentPropertyData(propertyDto.Value, propertyDto.DataType.Configuration) { ContentKey = contentItem.PersistedContent.Key, PropertyTypeKey = property.PropertyType.Key, - Files = files + Files = files }; // let the editor convert the value that was received, deal with files, etc - var value = valueEditor.FromEditor(data, getPropertyValue(contentItem, property)); + object value = valueEditor.FromEditor(data, getPropertyValue(contentItem, property)); // set the value - tags are special - var tagAttribute = propertyDto.PropertyEditor.GetTagAttribute(); + TagsPropertyEditorAttribute tagAttribute = propertyDto.PropertyEditor.GetTagAttribute(); if (tagAttribute != null) { - var tagConfiguration = ConfigurationEditor.ConfigurationAs(propertyDto.DataType.Configuration); - if (tagConfiguration.Delimiter == default) tagConfiguration.Delimiter = tagAttribute.Delimiter; + TagConfiguration tagConfiguration = ConfigurationEditor.ConfigurationAs(propertyDto.DataType.Configuration); + if (tagConfiguration.Delimiter == default) + { + tagConfiguration.Delimiter = tagAttribute.Delimiter; + } + var tagCulture = property.PropertyType.VariesByCulture() ? culture : null; property.SetTagsValue(_serializer, value, tagConfiguration, tagCulture); } else + { savePropertyValue(contentItem, property, value); + } } } + /// + /// Handles if the state is invalid + /// + /// The model state to display protected virtual void HandleInvalidModelState(IErrorModel display) { - //lastly, if it is not valid, add the model state to the outgoing object and throw a 403 + // lastly, if it is not valid, add the model state to the outgoing object and throw a 403 if (!ModelState.IsValid) { display.Errors = ModelState.ToErrorDictionary(); @@ -151,38 +169,45 @@ namespace Umbraco.Web.BackOffice.Controllers /// protected TPersisted GetObjectFromRequest(Func getFromService) { - //checks if the request contains the key and the item is not null, if that is the case, return it from the request, otherwise return + // checks if the request contains the key and the item is not null, if that is the case, return it from the request, otherwise return // it from the callback return HttpContext.Items.ContainsKey(typeof(TPersisted).ToString()) && HttpContext.Items[typeof(TPersisted).ToString()] != null - ? (TPersisted) HttpContext.Items[typeof (TPersisted).ToString()] + ? (TPersisted)HttpContext.Items[typeof(TPersisted).ToString()] : getFromService(); } /// /// Returns true if the action passed in means we need to create something new /// - /// - /// - internal static bool IsCreatingAction(ContentSaveAction action) - { - return (action.ToString().EndsWith("New")); - } + /// The content action + /// Returns true if this is a creating action + internal static bool IsCreatingAction(ContentSaveAction action) => action.ToString().EndsWith("New"); - protected void AddCancelMessage(INotificationModel display, - string header = "speechBubbles/operationCancelledHeader", - string message = "speechBubbles/operationCancelledText", - bool localizeHeader = true, + /// + /// Adds a cancelled message to the display + /// + /// + /// + /// + /// + /// + /// + /// + protected void AddCancelMessage(INotificationModel display, string header = "speechBubbles/operationCancelledHeader", string message = "speechBubbles/operationCancelledText", bool localizeHeader = true, bool localizeMessage = true, string[] headerParams = null, string[] messageParams = null) { - //if there's already a default event message, don't add our default one - var msgs = EventMessages; - if (msgs != null && msgs.GetOrDefault().GetAll().Any(x => x.IsDefaultEventMessage)) return; + // if there's already a default event message, don't add our default one + IEventMessagesFactory messages = EventMessages; + if (messages != null && messages.GetOrDefault().GetAll().Any(x => x.IsDefaultEventMessage)) + { + return; + } display.AddWarningNotification( localizeHeader ? LocalizedTextService.Localize(header, headerParams) : header, - localizeMessage ? LocalizedTextService.Localize(message, messageParams): message); + localizeMessage ? LocalizedTextService.Localize(message, messageParams) : message); } } } diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index 629e2282ff..efaed903f2 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; @@ -7,8 +7,8 @@ using System.Net.Http; using System.Net.Mime; using System.Text; using System.Threading.Tasks; -using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Identity; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; using Umbraco.Core; @@ -17,6 +17,7 @@ using Umbraco.Core.Events; using Umbraco.Core.Mapping; using Umbraco.Core.Models; using Umbraco.Core.Models.ContentEditing; +using Umbraco.Core.Models.Membership; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Security; using Umbraco.Core.Serialization; @@ -54,9 +55,25 @@ namespace Umbraco.Web.BackOffice.Controllers private readonly IUmbracoMembersUserManager _memberManager; private readonly IDataTypeService _dataTypeService; private readonly ILocalizedTextService _localizedTextService; - private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; private readonly IJsonSerializer _jsonSerializer; + /// + /// Initializes a new instance of the class. + /// + /// The culture dictionary + /// The logger factory + /// The string helper + /// The event messages factory + /// The entry point for localizing key services + /// The property editors + /// The mapper + /// The member service + /// The member type service + /// The member manager + /// The data-type service + /// The back office security accessor + /// The JSON serializer public MemberController( ICultureDictionary cultureDictionary, ILoggerFactory loggerFactory, @@ -69,7 +86,7 @@ namespace Umbraco.Web.BackOffice.Controllers IMemberTypeService memberTypeService, IUmbracoMembersUserManager memberManager, IDataTypeService dataTypeService, - IBackOfficeSecurityAccessor backofficeSecurityAccessor, + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IJsonSerializer jsonSerializer) : base(cultureDictionary, loggerFactory, shortStringHelper, eventMessages, localizedTextService, jsonSerializer) { @@ -80,10 +97,21 @@ namespace Umbraco.Web.BackOffice.Controllers _memberManager = memberManager; _dataTypeService = dataTypeService; _localizedTextService = localizedTextService; - _backofficeSecurityAccessor = backofficeSecurityAccessor; + _backOfficeSecurityAccessor = backOfficeSecurityAccessor; _jsonSerializer = jsonSerializer; } + /// + /// The paginated list of members + /// + /// The page number to display + /// The size of the page + /// The ordering of the member list + /// The direction of the member list + /// The system field to order by + /// The current filter for the list + /// The member type + /// The paged result of members public PagedResult GetPagedResults( int pageNumber = 1, int pageSize = 100, @@ -123,11 +151,11 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// Returns a display node with a list view to render members /// - /// - /// + /// The member type to list + /// The member list for display public MemberListDisplay GetListNodeDisplay(string listName) { - var foundType = _memberTypeService.Get(listName); + IMemberType foundType = _memberTypeService.Get(listName); var name = foundType != null ? foundType.Name : listName; var apps = new List @@ -159,25 +187,26 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// Gets the content json for the member /// - /// - /// + /// The Guid key of the member + /// The member for display [OutgoingEditorModelEvent] public MemberDisplay GetByKey(Guid key) { - //TODO: this is not finding the key currently + // TODO: this is not finding the key currently IMember foundMember = _memberService.GetByKey(key); if (foundMember == null) { HandleContentNotFound(key); } + return _umbracoMapper.Map(foundMember); } /// /// Gets an empty content item for the /// - /// - /// + /// The content type + /// The empty member for display [OutgoingEditorModelEvent] public MemberDisplay GetEmpty(string contentTypeAlias = null) { @@ -202,91 +231,109 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// Saves member /// - /// + /// The content item to save as a member + /// The resulting member display object [FileUploadCleanupFilter] [OutgoingEditorModelEvent] [MemberSaveValidation] - public async Task> PostSave( - [ModelBinder(typeof(MemberBinder))] - MemberSave contentItem) + public async Task> PostSave([ModelBinder(typeof(MemberBinder))] MemberSave contentItem) { - if (contentItem == null) throw new ArgumentNullException(nameof(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: + // If we've reached here it means: // * Our model has been bound // * and validated // * any file attachments have been saved to their temporary location for us to use // * we have a reference to the DTO object and the persisted object // * Permissions are valid - //map the properties to the persisted entity + // map the properties to the persisted entity MapPropertyValues(contentItem); - UmbracoMembersIdentityUser identityMember = ValidateMemberData(contentItem); + ValidateMemberData(contentItem); - //Unlike content/media - if there are errors for a member, we do NOT proceed to save them, we cannot so return the errors + // 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) { - var forDisplay = _umbracoMapper.Map(contentItem.PersistedContent); + MemberDisplay forDisplay = _umbracoMapper.Map(contentItem.PersistedContent); forDisplay.Errors = ModelState.ToErrorDictionary(); throw HttpResponseException.CreateValidationErrorResponse(forDisplay); } - //We're gonna look up the current roles now because the below code can cause + IMemberType memberType = _memberTypeService.Get(contentItem.ContentTypeAlias); + if (memberType == null) + { + throw new InvalidOperationException($"No member type found with alias {contentItem.ContentTypeAlias}"); + } + + // Create the member with the MemberManager + var identityMember = UmbracoMembersIdentityUser.CreateNew( + contentItem.Username, + contentItem.Email, + memberType.Alias, + contentItem.Name); + + // We're gonna look up the current roles now because the below code can cause // 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 currentRoles = _memberService.GetAllRoles(contentItem.PersistedContent.Username); - //find the ones to remove and remove them - IEnumerable roles = currRoles.ToList(); - var rolesToRemove = roles.Except(contentItem.Groups).ToArray(); + // find the ones to remove and remove them + IEnumerable roles = currentRoles.ToList(); + string[] rolesToRemove = roles.Except(contentItem.Groups).ToArray(); - //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. + // 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: - await CreateMemberAsync(contentItem, identityMember); + IdentityResult identityResult = await CreateMemberAsync(contentItem, identityMember); break; default: - //we don't support anything else for members + // we don't support anything else for members throw new HttpResponseException(HttpStatusCode.NotFound); } - //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 + // 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 - //Now let's do the role provider stuff - now that we've saved the content item (that is important since + // 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()) { _memberService.DissociateRoles(new[] { contentItem.PersistedContent.Username }, rolesToRemove); } - //find the ones to add and add them - var toAdd = contentItem.Groups.Except(roles).ToArray(); + + // find the ones to add and add them + string[] toAdd = contentItem.Groups.Except(roles).ToArray(); if (toAdd.Any()) { - //add the ones submitted + // add the ones submitted _memberService.AssignRoles(new[] { contentItem.PersistedContent.Username }, toAdd); } - //return the updated model - var display = _umbracoMapper.Map(contentItem.PersistedContent); + // return the updated model + MemberDisplay display = _umbracoMapper.Map(contentItem.PersistedContent); - //lastly, if it is not valid, add the model state to the outgoing object and throw a 403 + // lastly, if it is not valid, add the model state to the outgoing object and throw a 403 HandleInvalidModelState(display); - var localizedTextService = _localizedTextService; - //put the correct messages in + ILocalizedTextService localizedTextService = _localizedTextService; + + // put the correct messages in switch (contentItem.Action) { case ContentSaveAction.Save: @@ -303,77 +350,64 @@ namespace Umbraco.Web.BackOffice.Controllers /// /// Maps the property values to the persisted entity /// - /// + /// The member content item to map properties from private void MapPropertyValues(MemberSave contentItem) { - //Don't update the name if it is empty + // 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 + // map the custom properties - this will already be set for new entities in our member binder contentItem.PersistedContent.Email = contentItem.Email; contentItem.PersistedContent.Username = contentItem.Username; - //use the base method to map the rest of the properties - base.MapPropertyValuesForPersistence( + // use the base method to map the rest of the properties + MapPropertyValuesForPersistence( contentItem, contentItem.PropertyCollectionDto, - (save, property) => property.GetValue(), //get prop val - (save, property, v) => property.SetValue(v), //set prop val + (save, property) => property.GetValue(), // get prop val + (save, property, v) => property.SetValue(v), // set prop val 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 + /// + /// All member password processing and creation is done via the identity manager /// - /// - /// - /// - private async Task CreateMemberAsync(MemberSave contentItem, UmbracoMembersIdentityUser identityMember) + /// Member content data + /// The identity member to update + /// The identity result of the created member + private async Task CreateMemberAsync(MemberSave contentItem, UmbracoMembersIdentityUser identityMember) { - //var memberType = _memberTypeService.Get(contentItem.ContentTypeAlias); - //if (memberType == null) - // throw new InvalidOperationException($"No member type found with alias {contentItem.ContentTypeAlias}"); - //var member = new Member(contentItem.Name, contentItem.Email, contentItem.Username, memberType, true) - //{ - // CreatorId = _backofficeSecurityAccessor.BackofficeSecurity.CurrentUser.Id, - // RawPasswordValue = _passwordSecurity.HashPasswordForStorage(contentItem.Password.NewPassword), - // Comments = contentItem.Comments, - // IsApproved = contentItem.IsApproved - //}; - - //return member; - 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 + // now re-look the member back up which will now exist IMember member = _memberService.GetByEmail(contentItem.Email); - member.CreatorId = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; + member.CreatorId = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; + // should this be removed since we've moved passwords out? - //since the back office user is creating this member, they will be set to approved + member.RawPasswordValue = identityMember.PasswordHash; + member.Comments = contentItem.Comments; + + // 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 + // map the save info over onto the user member = _umbracoMapper.Map(contentItem, member); contentItem.PersistedContent = member; + return created; } - private UmbracoMembersIdentityUser ValidateMemberData(MemberSave contentItem) + private void ValidateMemberData(MemberSave 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( @@ -399,14 +433,13 @@ namespace Umbraco.Web.BackOffice.Controllers if (contentItem.Password != null && !contentItem.Password.NewPassword.IsNullOrWhiteSpace()) { - //TODO: check password - //var validPassword = await _memberManager.CheckPasswordAsync(null, contentItem.Password.NewPassword); - //if (!validPassword) - //{ - // ModelState.AddPropertyError( - // new ValidationResult("Invalid password", new[] { "value" }), - // $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password"); - //} + Task> result = _memberManager.ValidatePassword(contentItem.Password.NewPassword); + if (result.Result.Exists(x => x.Succeeded == false)) + { + ModelState.AddPropertyError( + new ValidationResult($"Invalid password: {MapErrors(result.Result)}", new[] { "value" }), + $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password"); + } } else { @@ -414,48 +447,48 @@ namespace Umbraco.Web.BackOffice.Controllers new ValidationResult("Password cannot be empty", new[] { "value" }), $"{Constants.PropertyEditors.InternalGenericPropertiesPrefix}password"); } + } - // 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; - return identityMember; + private string MapErrors(List result) + { + var sb = new StringBuilder(); + IEnumerable errors = result.Where(x => x.Succeeded == false); + + foreach (IdentityResult error in errors) + { + sb.AppendLine(error.Errors.ToErrorMessage()); + } + + return sb.ToString(); } /// /// Update the member security data - /// - /// - /// /// If the password has been reset then this method will return the reset/generated password, otherwise will return null. - /// - private async void UpdateMemberData(MemberSave memberSave) + /// + /// The member to save + private void UpdateMemberData(MemberSave memberSave) { - //TODO: optimise based on new member manager - memberSave.PersistedContent.WriterId = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Id; + memberSave.PersistedContent.WriterId = _backOfficeSecurityAccessor.BackOfficeSecurity.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: Comments, IsApproved, IsLockedOut // but we will take care of this in a generic way below so that it works for all props. - if (!_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.HasAccessToSensitiveData()) + if (!_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.HasAccessToSensitiveData()) { - var memberType = _memberTypeService.Get(memberSave.PersistedContent.ContentTypeId); + IMemberType memberType = _memberTypeService.Get(memberSave.PersistedContent.ContentTypeId); var sensitiveProperties = memberType .PropertyTypes.Where(x => memberType.IsSensitiveProperty(x.Alias)) .ToList(); - foreach (var sensitiveProperty in sensitiveProperties) + foreach (IPropertyType sensitiveProperty in sensitiveProperties) { - var destProp = memberSave.Properties.FirstOrDefault(x => x.Alias == sensitiveProperty.Alias); + ContentPropertyBasic destProp = memberSave.Properties.FirstOrDefault(x => x.Alias == sensitiveProperty.Alias); if (destProp != null) { - //if found, change the value of the contentItem model to the persisted value so it remains unchanged - var origValue = memberSave.PersistedContent.GetValue(sensitiveProperty.Alias); + // if found, change the value of the contentItem model to the persisted value so it remains unchanged + object origValue = memberSave.PersistedContent.GetValue(sensitiveProperty.Alias); destProp.Value = origValue; } } @@ -463,7 +496,7 @@ namespace Umbraco.Web.BackOffice.Controllers var isLockedOut = memberSave.IsLockedOut; - //if they were locked but now they are trying to be unlocked + // if they were locked but now they are trying to be unlocked if (memberSave.PersistedContent.IsLockedOut && isLockedOut == false) { memberSave.PersistedContent.IsLockedOut = false; @@ -471,34 +504,34 @@ namespace Umbraco.Web.BackOffice.Controllers } else if (!memberSave.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 + // 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 ModelState.AddModelError("custom", "An admin cannot lock a user"); } - //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(); + // no password changes then exit ? + if (memberSave.Password != null) + { + // set the password + memberSave.PersistedContent.RawPasswordValue = _memberManager.GeneratePassword(); + } } /// /// Permanently deletes a member /// - /// - /// + /// Guid of the member to delete + /// The result of the deletion /// [HttpPost] public IActionResult DeleteByKey(Guid key) { - var foundMember = _memberService.GetByKey(key); + IMember foundMember = _memberService.GetByKey(key); if (foundMember == null) { return HandleContentNotFound(key, false); } + _memberService.Delete(foundMember); return Ok(); @@ -512,19 +545,23 @@ namespace Umbraco.Web.BackOffice.Controllers [HttpGet] public IActionResult ExportMemberData(Guid key) { - var currentUser = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser; + IUser currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser; if (currentUser.HasAccessToSensitiveData() == false) { return Forbid(); } - var member = ((MemberService)_memberService).ExportMember(key); - if (member is null) throw new NullReferenceException("No member found with key " + key); + MemberExportModel member = ((MemberService)_memberService).ExportMember(key); + if (member is null) + { + throw new NullReferenceException("No member found with key " + key); + } var json = _jsonSerializer.Serialize(member); var fileName = $"{member.Name}_{member.Email}.txt"; + // Set custom header so umbRequestHelper.downloadFile can save the correct filename HttpContext.Response.Headers.Add("x-filename", fileName); diff --git a/src/umbraco.sln.DotSettings b/src/umbraco.sln.DotSettings index 2f99fe6350..6fb927035e 100644 --- a/src/umbraco.sln.DotSettings +++ b/src/umbraco.sln.DotSettings @@ -5,5 +5,6 @@ HINT False Default + True True True