From 3f0e7ab315789d5f92cd3740b6de8a9fbe9ced40 Mon Sep 17 00:00:00 2001 From: emmagarland Date: Tue, 8 Dec 2020 01:57:14 +0000 Subject: [PATCH] Merged and updated according to shared latest work, renamed to Members instead of UmbracoMembers. Tests currently red, fixing next. Empty appsettings again. --- .../IUmbracoMembersUserPasswordChecker.cs | 19 -- .../Members/UmbracoMembersIdentityUser.cs | 178 ----------- ...UmbracoMembersUserPasswordCheckerResult.cs | 12 - .../Models/Mapping/MemberMapDefinition.cs | 21 +- .../MembersUserPasswordCheckerResult.cs | 12 + src/Umbraco.Core/Umbraco.Core.csproj | 2 +- .../CoreMappingProfiles.cs | 1 + .../Members/IUmbracoMembersUserManager.cs | 62 ---- .../Members/UmbracoMembersIdentityOptions.cs | 11 - .../Members/UmbracoMembersUserManager.cs | 244 --------------- .../Security/IBackOfficeUserManager.cs | 1 + .../Security/IMembersUserManager.cs | 14 + .../Security/IMembersUserPasswordChecker.cs | 20 ++ .../Security/IUmbracoUserManager.cs | 18 +- .../Security/IdentityMapDefinition.cs | 16 +- .../MembersIdentityBuilder.cs} | 15 +- .../Security/MembersIdentityOptions.cs | 11 + .../Security/MembersIdentityUser.cs | 139 +++++++++ .../MembersUserStore.cs} | 82 ++--- .../Umbraco.Infrastructure.csproj | 4 + ...MembersServiceCollectionExtensionsTests.cs | 24 +- .../UmbracoMemberIdentityUserManagerTests.cs | 69 +++-- .../UmbracoMemberIdentityUserStoreTests.cs | 14 +- .../Controllers/MemberControllerUnitTests.cs | 15 +- .../Controllers/MemberController.cs | 187 ++++++------ .../UmbracoMemberIdentityBuilderExtensions.cs | 4 +- ...oMembersUserServiceCollectionExtensions.cs | 20 +- .../Security/MembersUserManager.cs | 286 ++++++++++++++++++ src/Umbraco.Web.UI.NetCore/appsettings.json | 4 +- 29 files changed, 738 insertions(+), 767 deletions(-) delete mode 100644 src/Umbraco.Core/Members/IUmbracoMembersUserPasswordChecker.cs delete mode 100644 src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs delete mode 100644 src/Umbraco.Core/Members/UmbracoMembersUserPasswordCheckerResult.cs create mode 100644 src/Umbraco.Core/Security/MembersUserPasswordCheckerResult.cs delete mode 100644 src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs delete mode 100644 src/Umbraco.Infrastructure/Members/UmbracoMembersIdentityOptions.cs delete mode 100644 src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs create mode 100644 src/Umbraco.Infrastructure/Security/IMembersUserManager.cs create mode 100644 src/Umbraco.Infrastructure/Security/IMembersUserPasswordChecker.cs rename src/Umbraco.Infrastructure/{Members/UmbracoMembersIdentityBuilder.cs => Security/MembersIdentityBuilder.cs} (65%) create mode 100644 src/Umbraco.Infrastructure/Security/MembersIdentityOptions.cs create mode 100644 src/Umbraco.Infrastructure/Security/MembersIdentityUser.cs rename src/Umbraco.Infrastructure/{Members/UmbracoMembersUserStore.cs => Security/MembersUserStore.cs} (79%) rename src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/{Members => Security}/UmbracoMemberIdentityUserManagerTests.cs (65%) rename src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/{Members => Security}/UmbracoMemberIdentityUserStoreTests.cs (88%) create mode 100644 src/Umbraco.Web.Common/Security/MembersUserManager.cs diff --git a/src/Umbraco.Core/Members/IUmbracoMembersUserPasswordChecker.cs b/src/Umbraco.Core/Members/IUmbracoMembersUserPasswordChecker.cs deleted file mode 100644 index b361ca3121..0000000000 --- a/src/Umbraco.Core/Members/IUmbracoMembersUserPasswordChecker.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System.Threading.Tasks; - -namespace Umbraco.Core.Members -{ - /// - /// Used by the UmbracoMembersUserManager to check the username/password which allows for developers to more easily - /// set the logic for this procedure. - /// - public interface IUmbracoMembersUserPasswordChecker - { - /// - /// Checks a password for a member - /// - /// - /// - /// - Task CheckPasswordAsync(UmbracoMembersIdentityUser member, string password); - } -} diff --git a/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs b/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs deleted file mode 100644 index fd4687c664..0000000000 --- a/src/Umbraco.Core/Members/UmbracoMembersIdentityUser.cs +++ /dev/null @@ -1,178 +0,0 @@ -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 - { - private int _id; - - private string _passwordHash; - - private string _passwordConfig; - - /// - /// 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 { get; private set; } - - /// - /// Gets or sets the member Id - /// - public int Id - { - get => _id; - set - { - _id = value; - HasIdentity = true; - } - } - - /// - /// Gets or sets the salted/hashed form of the user password - /// - public string PasswordHash - { - get => _passwordHash; - set => BeingDirty.SetPropertyValueAndDetectChanges(value, ref _passwordHash, nameof(PasswordHash)); - } - - /// - /// Gets or sets the password config - /// - public string PasswordConfig - { - get => _passwordConfig; - set => BeingDirty.SetPropertyValueAndDetectChanges(value, ref _passwordConfig, nameof(PasswordConfig)); - } - - /// - /// Gets or sets a value indicating whether member Is Approved - /// - public bool IsApproved { get; set; } - - /// - /// 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) - { - if (string.IsNullOrWhiteSpace(username)) - { - throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); - } - - // 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 - }; - - 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.Core/Members/UmbracoMembersUserPasswordCheckerResult.cs b/src/Umbraco.Core/Members/UmbracoMembersUserPasswordCheckerResult.cs deleted file mode 100644 index 8432b7b0bf..0000000000 --- a/src/Umbraco.Core/Members/UmbracoMembersUserPasswordCheckerResult.cs +++ /dev/null @@ -1,12 +0,0 @@ -namespace Umbraco.Core.Members -{ - /// - /// The result returned from the IUmbracoMembersUserPasswordChecker - /// - public enum UmbracoMembersUserPasswordCheckerResult - { - ValidCredentials, - InvalidCredentials, - FallbackToDefaultChecker - } -} diff --git a/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs b/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs index bc8589e8ef..60fe4daace 100644 --- a/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs +++ b/src/Umbraco.Core/Models/Mapping/MemberMapDefinition.cs @@ -1,23 +1,18 @@ -using Umbraco.Core.Mapping; -using Umbraco.Core.Models; +using Umbraco.Core.Mapping; using Umbraco.Web.Models.ContentEditing; -namespace Umbraco.Web.Models.Mapping +namespace Umbraco.Core.Models.Mapping { + /// public class MemberMapDefinition : IMapDefinition { - public MemberMapDefinition() - { - } - - public void DefineMaps(UmbracoMapper mapper) - { - mapper.Define(Map); - } + /// + public void DefineMaps(UmbracoMapper mapper) => mapper.Define(Map); // mappers private static void Map(MemberSave source, IMember target, MapperContext context) { + // TODO: ensure all properties are mapped as required target.IsApproved = source.IsApproved; target.Name = source.Name; target.Email = source.Email; @@ -25,10 +20,6 @@ namespace Umbraco.Web.Models.Mapping target.Username = source.Username; target.Id = (int)(long)source.Id; target.Comments = source.Comments; - target.IsApproved = source.IsApproved; - - //TODO: ensure all properties are mapped as required - } } } diff --git a/src/Umbraco.Core/Security/MembersUserPasswordCheckerResult.cs b/src/Umbraco.Core/Security/MembersUserPasswordCheckerResult.cs new file mode 100644 index 0000000000..3212609ed9 --- /dev/null +++ b/src/Umbraco.Core/Security/MembersUserPasswordCheckerResult.cs @@ -0,0 +1,12 @@ +namespace Umbraco.Core.Security +{ + /// + /// The result returned from the IMembersUserPasswordChecker + /// + public enum MembersUserPasswordCheckerResult + { + ValidCredentials, + InvalidCredentials, + FallbackToDefaultChecker + } +} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 300dedc1c6..28695d1673 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -1,4 +1,4 @@ - + netstandard2.0 diff --git a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/CoreMappingProfiles.cs b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/CoreMappingProfiles.cs index 05cd6a74c8..72af10e31c 100644 --- a/src/Umbraco.Infrastructure/Composing/CompositionExtensions/CoreMappingProfiles.cs +++ b/src/Umbraco.Infrastructure/Composing/CompositionExtensions/CoreMappingProfiles.cs @@ -1,6 +1,7 @@ using Microsoft.Extensions.DependencyInjection; using Umbraco.Core.DependencyInjection; using Umbraco.Core.Mapping; +using Umbraco.Core.Models.Mapping; using Umbraco.Core.Security; using Umbraco.Web.Models.Mapping; diff --git a/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs b/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs deleted file mode 100644 index 8eedb67af6..0000000000 --- a/src/Umbraco.Infrastructure/Members/IUmbracoMembersUserManager.cs +++ /dev/null @@ -1,62 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Identity; -using Umbraco.Core.Members; - -namespace Umbraco.Infrastructure.Members -{ - public interface IUmbracoMembersUserManager : IUmbracoMembersUserManager - { - } - - public interface IUmbracoMembersUserManager : IDisposable where TUser : UmbracoMembersIdentityUser - { - /// - /// Creates the specified user in the backing store with given password, as an asynchronous operation. - /// - /// The member to create. - /// The new password - /// - /// The that represents the asynchronous operation, containing the - /// of the operation. - /// - Task CreateAsync(TUser memberUser, string password); - - /// - /// Helper method to generate a password for a user based on the current password validator - /// - /// Returns the generated password - string GeneratePassword(); - - /// - /// Adds the to the specified only if the user - /// does not already have a password. - /// - /// The member whose password should be set. - /// The password to set. - /// - /// The that represents the asynchronous operation, containing the - /// of the operation. - /// - Task AddPasswordAsync(TUser memberUser, string password); - - /// - /// Returns a flag indicating whether the given is valid for the - /// specified . - /// - /// The user whose password should be validated. - /// The password to validate - /// The that represents the asynchronous operation, containing true if - /// 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/UmbracoMembersIdentityOptions.cs b/src/Umbraco.Infrastructure/Members/UmbracoMembersIdentityOptions.cs deleted file mode 100644 index e72b2e3aba..0000000000 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersIdentityOptions.cs +++ /dev/null @@ -1,11 +0,0 @@ -using Microsoft.AspNetCore.Identity; - -namespace Umbraco.Infrastructure.Members -{ - /// - /// Identity options specifically for the Umbraco members identity implementation - /// - public class UmbracoMembersIdentityOptions : IdentityOptions - { - } -} diff --git a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs b/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs deleted file mode 100644 index 3ab230a76e..0000000000 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserManager.cs +++ /dev/null @@ -1,244 +0,0 @@ -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; - - -namespace Umbraco.Infrastructure.Members -{ - /// - /// A manager for the Umbraco members identity implementation - /// - public class UmbracoMembersUserManager : UmbracoMembersUserManager, IUmbracoMembersUserManager - { - /// - public UmbracoMembersUserManager( - IUserStore store, - IOptions optionsAccessor, - IPasswordHasher passwordHasher, - IEnumerable> userValidators, - IEnumerable> passwordValidators, - ILookupNormalizer keyNormalizer, - IdentityErrorDescriber errors, - IServiceProvider services, - ILogger> logger, - IOptions passwordConfiguration) : - base(store, optionsAccessor, passwordHasher, userValidators, passwordValidators, keyNormalizer, errors, services, logger, passwordConfiguration) - { - } - } - - /// - /// Manager for the member identity user - /// - /// The identity user - public class UmbracoMembersUserManager : UserManager - where T : UmbracoMembersIdentityUser - { - 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, - IPasswordHasher passwordHasher, - IEnumerable> userValidators, - IEnumerable> passwordValidators, - ILookupNormalizer keyNormalizer, - IdentityErrorDescriber errors, - IServiceProvider services, - ILogger> logger, - IOptions passwordConfiguration) : base(store, optionsAccessor, passwordHasher, userValidators, passwordValidators, keyNormalizer, errors, services, logger) => - PasswordConfiguration = passwordConfiguration.Value ?? throw new ArgumentNullException(nameof(passwordConfiguration)); - - - /// - /// 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 - { - get => (UmbracoMembersIdentityOptions)base.Options; - set => base.Options = value; - } - - /// - /// 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 - /// - /// 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; - - if (validatePassword) - { - IdentityResult validate = await ValidatePasswordAsync(memberUser, newPassword); - if (!validate.Succeeded) - { - return validate; - } - } - - 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); - await UpdateSecurityStampInternal(memberUser); - return IdentityResult.Success; - } - - /// 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 - /// * Call VerifyPasswordAsync with the password store + user + password - /// * Uses the PasswordHasher.VerifyHashedPassword to compare the stored password - /// - /// In some cases people want simple custom control over the username/password check, for simplicity - /// sake, developers would like the users to simply validate against an LDAP directory but the user - /// data remains stored inside of Umbraco. - /// See: http://issues.umbraco.org/issue/U4-7032 for the use cases. - /// - /// We've allowed this check to be overridden with a simple callback so that developers don't actually - /// have to implement/override this class. - /// - public override async Task CheckPasswordAsync(T member, string password) - { - if (UmbracoMembersUserPasswordChecker != null) - { - UmbracoMembersUserPasswordCheckerResult result = await UmbracoMembersUserPasswordChecker.CheckPasswordAsync(member, password); - - if (member.HasIdentity == false) - { - return false; - } - - // 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 - if (member.HasIdentity == false) - { - return false; - } - - // use the default behavior - return await base.CheckPasswordAsync(member, password); - } - - /// 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; - } - - await GetSecurityStore().SetSecurityStampAsync(user, NewSecurityStamp(), CancellationToken.None); - } - - /// 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() - { - 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 - /// - /// This is copied from the underlying .NET base class since they decided to not expose it - /// - /// Returns a new security stamp - private static string NewSecurityStamp() => Guid.NewGuid().ToString(); - - /// - /// 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() - { - _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/Security/IBackOfficeUserManager.cs b/src/Umbraco.Infrastructure/Security/IBackOfficeUserManager.cs index 4235195bb1..cc0a63142b 100644 --- a/src/Umbraco.Infrastructure/Security/IBackOfficeUserManager.cs +++ b/src/Umbraco.Infrastructure/Security/IBackOfficeUserManager.cs @@ -1,4 +1,5 @@ using Umbraco.Core.Security; +using Umbraco.Infrastructure.Security; namespace Umbraco.Core.Security { diff --git a/src/Umbraco.Infrastructure/Security/IMembersUserManager.cs b/src/Umbraco.Infrastructure/Security/IMembersUserManager.cs new file mode 100644 index 0000000000..a5b0579cb7 --- /dev/null +++ b/src/Umbraco.Infrastructure/Security/IMembersUserManager.cs @@ -0,0 +1,14 @@ +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Umbraco.Core.Security; + +namespace Umbraco.Infrastructure.Security +{ + /// + /// The user manager for members + /// + public interface IMembersUserManager : IUmbracoUserManager + { + } +} diff --git a/src/Umbraco.Infrastructure/Security/IMembersUserPasswordChecker.cs b/src/Umbraco.Infrastructure/Security/IMembersUserPasswordChecker.cs new file mode 100644 index 0000000000..969b4feb79 --- /dev/null +++ b/src/Umbraco.Infrastructure/Security/IMembersUserPasswordChecker.cs @@ -0,0 +1,20 @@ +using System.Threading.Tasks; +using Umbraco.Core.Security; + +namespace Umbraco.Infrastructure.Security +{ + /// + /// Used by the MembersUserManager to check the username/password which allows for developers to more easily + /// set the logic for this procedure. + /// + public interface IMembersUserPasswordChecker + { + /// + /// Checks a password for a member + /// + /// + /// TODO: what should our implementation be for members? + /// + Task CheckPasswordAsync(MembersIdentityUser user, string password); + } +} diff --git a/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs b/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs index 4bec4c9c7a..30b11cc8a8 100644 --- a/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs +++ b/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs @@ -4,11 +4,12 @@ using System.Security.Claims; using System.Security.Principal; using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; +using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.Membership; using Umbraco.Core.Security; using Umbraco.Web.Models.ContentEditing; -namespace Umbraco.Core.Security +namespace Umbraco.Infrastructure.Security { /// @@ -16,7 +17,7 @@ namespace Umbraco.Core.Security /// /// The type of user public interface IUmbracoUserManager : IDisposable - where TUser : BackOfficeIdentityUser + where TUser : UmbracoIdentityUser { /// /// Gets the user id of a user @@ -223,6 +224,19 @@ namespace Umbraco.Core.Security /// Task CreateAsync(TUser user); + + /// + /// Creates the specified in the backing store with a password, + /// as an asynchronous operation. + /// + /// The user to create. + /// The password to add to the user. + /// + /// The that represents the asynchronous operation, containing the + /// of the operation. + /// + Task CreateAsync(TUser user, string password); + /// /// Generate a password for a user based on the current password validator /// diff --git a/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs b/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs index 4f5a11e887..00239b21eb 100644 --- a/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs +++ b/src/Umbraco.Infrastructure/Security/IdentityMapDefinition.cs @@ -3,10 +3,10 @@ using Microsoft.Extensions.Options; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; using Umbraco.Core.Mapping; -using Umbraco.Core.Members; using Umbraco.Core.Models; using Umbraco.Core.Models.Membership; using Umbraco.Core.Services; +using Umbraco.Infrastructure.Security; namespace Umbraco.Core.Security { @@ -39,10 +39,10 @@ namespace Umbraco.Core.Security target.EnableChangeTracking(); }); - mapper.Define( + mapper.Define( (source, context) => { - var target = new UmbracoMembersIdentityUser(); + var target = new MembersIdentityUser(source.Id); target.DisableChangeTracking(); return target; }, @@ -91,20 +91,20 @@ namespace Umbraco.Core.Security //target.Roles =; } - private void Map(IMember source, UmbracoMembersIdentityUser target) + private void Map(IMember source, MembersIdentityUser target) { target.Email = source.Email; target.UserName = source.Username; - //target.LastPasswordChangeDateUtc = source.LastPasswordChangeDate.ToUniversalTime(); - //target.LastLoginDateUtc = source.LastLoginDate.ToUniversalTime(); + target.LastPasswordChangeDateUtc = source.LastPasswordChangeDate.ToUniversalTime(); + target.LastLoginDateUtc = source.LastLoginDate.ToUniversalTime(); //target.EmailConfirmed = source.EmailConfirmedDate.HasValue; target.Name = source.Name; - //target.AccessFailedCount = source.FailedPasswordAttempts; + target.AccessFailedCount = source.FailedPasswordAttempts; target.PasswordHash = GetPasswordHash(source.RawPasswordValue); target.PasswordConfig = source.PasswordConfiguration; target.IsApproved = source.IsApproved; //target.SecurityStamp = source.SecurityStamp; - //target.LockoutEndDateUtc = source.IsLockedOut ? DateTime.MaxValue.ToUniversalTime() : (DateTime?)null; + target.LockoutEnd = source.IsLockedOut ? DateTime.MaxValue.ToUniversalTime() : (DateTime?)null; } private static string GetPasswordHash(string storedPass) diff --git a/src/Umbraco.Infrastructure/Members/UmbracoMembersIdentityBuilder.cs b/src/Umbraco.Infrastructure/Security/MembersIdentityBuilder.cs similarity index 65% rename from src/Umbraco.Infrastructure/Members/UmbracoMembersIdentityBuilder.cs rename to src/Umbraco.Infrastructure/Security/MembersIdentityBuilder.cs index e195dc925c..553cca6a17 100644 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersIdentityBuilder.cs +++ b/src/Umbraco.Infrastructure/Security/MembersIdentityBuilder.cs @@ -1,24 +1,23 @@ -using System; +using System; using System.Reflection; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; -using Umbraco.Core.Members; -namespace Umbraco.Infrastructure.Members +namespace Umbraco.Infrastructure.Security { - public class UmbracoMembersIdentityBuilder : IdentityBuilder + public class MembersIdentityBuilder : IdentityBuilder { - public UmbracoMembersIdentityBuilder(IServiceCollection services) : base(typeof(UmbracoMembersIdentityUser), services) + public MembersIdentityBuilder(IServiceCollection services) : base(typeof(MembersIdentityUser), services) { } - public UmbracoMembersIdentityBuilder(Type role, IServiceCollection services) : base(typeof(UmbracoMembersIdentityUser), role, services) + public MembersIdentityBuilder(Type role, IServiceCollection services) : base(typeof(MembersIdentityUser), role, services) { } /// - /// Adds a token provider for the . + /// Adds a token provider for the . /// /// The name of the provider to add. /// The type of the to add. @@ -29,7 +28,7 @@ namespace Umbraco.Infrastructure.Members { throw new InvalidOperationException($"Invalid Type for TokenProvider: {provider.FullName}"); } - Services.Configure(options => + Services.Configure(options => { options.Tokens.ProviderMap[providerName] = new TokenProviderDescriptor(provider); }); diff --git a/src/Umbraco.Infrastructure/Security/MembersIdentityOptions.cs b/src/Umbraco.Infrastructure/Security/MembersIdentityOptions.cs new file mode 100644 index 0000000000..0510096bb2 --- /dev/null +++ b/src/Umbraco.Infrastructure/Security/MembersIdentityOptions.cs @@ -0,0 +1,11 @@ +using Microsoft.AspNetCore.Identity; + +namespace Umbraco.Infrastructure.Security +{ + /// + /// Identity options specifically for the Umbraco members identity implementation + /// + public class MembersIdentityOptions : IdentityOptions + { + } +} diff --git a/src/Umbraco.Infrastructure/Security/MembersIdentityUser.cs b/src/Umbraco.Infrastructure/Security/MembersIdentityUser.cs new file mode 100644 index 0000000000..4e13e6839d --- /dev/null +++ b/src/Umbraco.Infrastructure/Security/MembersIdentityUser.cs @@ -0,0 +1,139 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.AspNetCore.Identity; +using Umbraco.Core; +using Umbraco.Core.Models.Identity; +using Umbraco.Core.Models.Membership; + +namespace Umbraco.Infrastructure.Security +{ + /// + /// The identity user used for the member + /// + public class MembersIdentityUser : UmbracoIdentityUser + { + private string _name; + private string _passwordConfig; + private IReadOnlyCollection _groups; + + // TODO: reused from backoffice - share? + // Custom comparer for enumerables + private static readonly DelegateEqualityComparer> s_groupsComparer = new DelegateEqualityComparer>( + (groups, enumerable) => groups.Select(x => x.Alias).UnsortedSequenceEqual(enumerable.Select(x => x.Alias)), + groups => groups.GetHashCode()); + + + /// + /// Used to construct a new instance without an identity + /// + public static MembersIdentityUser CreateNew(string username, string email, string memberTypeAlias, + string name = null) + { + if (string.IsNullOrWhiteSpace(username)) + { + throw new ArgumentException("Value cannot be null or whitespace.", nameof(username)); + } + + var user = new MembersIdentityUser(); + user.DisableChangeTracking(); + user.UserName = username; + user.Email = email; + user.MemberTypeAlias = memberTypeAlias; + // TODO: confirm if should be approved + user.IsApproved = true; + user.Id = null; + user.HasIdentity = false; + user._name = name; + user.EnableChangeTracking(); + return user; + } + + /// + /// Initializes a new instance of the class. + /// + public MembersIdentityUser(int userId) + { + // use the property setters - they do more than just setting a field + Id = UserIdToString(userId); + } + + public MembersIdentityUser() + { + } + + /// + /// Gets or sets the member's real name + /// + public string Name + { + get => _name; + set => BeingDirty.SetPropertyValueAndDetectChanges(value, ref _name, nameof(Name)); + } + + /// + /// Gets or sets the password config + /// + public string PasswordConfig + { + get => _passwordConfig; + set => BeingDirty.SetPropertyValueAndDetectChanges(value, ref _passwordConfig, nameof(PasswordConfig)); + } + + /// + /// Gets or sets the user groups + /// TBC: how to implement for members? + /// + public IReadOnlyCollection Groups + { + get => _groups; + set + { + _groups = value.Where(x => x.Alias != null).ToArray(); + + var roles = new List>(); + foreach (IdentityUserRole identityUserRole in _groups.Select(x => new IdentityUserRole + { + RoleId = x.Alias, + UserId = Id?.ToString() + })) + { + roles.Add(identityUserRole); + } + + // now reset the collection + Roles = roles; + + BeingDirty.SetPropertyValueAndDetectChanges(value, ref _groups, nameof(Groups), s_groupsComparer); + } + } + + /// + /// Gets a value indicating whether the member is locked out + /// + public bool IsLockedOut + { + get + { + var isLocked = LockoutEnd.HasValue && LockoutEnd.Value.ToLocalTime() >= DateTime.Now; + return isLocked; + } + } + + /// + /// Gets or sets a value indicating the member is approved + /// + public bool IsApproved { get; set; } + + /// + /// Gets or sets the alias of the member type + /// + public string MemberTypeAlias { get; set; } + + private static string UserIdToString(int userId) => string.Intern(userId.ToString()); + + // TODO: implement as per base identity user + //public bool LoginsChanged; + //public bool RolesChanged; + } +} diff --git a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs b/src/Umbraco.Infrastructure/Security/MembersUserStore.cs similarity index 79% rename from src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs rename to src/Umbraco.Infrastructure/Security/MembersUserStore.cs index 024511f8e4..79a2949a08 100644 --- a/src/Umbraco.Infrastructure/Members/UmbracoMembersUserStore.cs +++ b/src/Umbraco.Infrastructure/Security/MembersUserStore.cs @@ -5,26 +5,25 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Identity; using Umbraco.Core; using Umbraco.Core.Mapping; -using Umbraco.Core.Members; using Umbraco.Core.Models; using Umbraco.Core.Scoping; using Umbraco.Core.Services; -namespace Umbraco.Infrastructure.Members +namespace Umbraco.Infrastructure.Security { /// /// A custom user store that uses Umbraco member data /// - public class UmbracoMembersUserStore : DisposableObjectSlim, + public class MembersUserStore : DisposableObjectSlim, //IUserStore, - IUserPasswordStore - //IUserEmailStore - //IUserLoginStore - //IUserRoleStore, - //IUserSecurityStampStore - //IUserLockoutStore - //IUserTwoFactorStore - //IUserSessionStore + IUserPasswordStore + //IUserEmailStore + //IUserLoginStore + //IUserRoleStore, + //IUserSecurityStampStore + //IUserLockoutStore + //IUserTwoFactorStore + //IUserSessionStore { private readonly bool _disposed = false; private readonly IMemberService _memberService; @@ -32,12 +31,12 @@ namespace Umbraco.Infrastructure.Members private readonly IScopeProvider _scopeProvider; /// - /// Initializes a new instance of the class for the members identity store + /// 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) + public MembersUserStore(IMemberService memberService, UmbracoMapper mapper, IScopeProvider scopeProvider) { _memberService = memberService; _mapper = mapper; @@ -47,10 +46,10 @@ namespace Umbraco.Infrastructure.Members /// /// Create the member as an identity user /// - /// The identity user` for a member + /// The identity user for a member /// The cancellation token /// The identity result - public Task CreateAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) + public Task CreateAsync(MembersIdentityUser user, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); @@ -62,23 +61,23 @@ namespace Umbraco.Infrastructure.Members // 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( + IMember memberEntity = _memberService.CreateMember( user.UserName, user.Email, user.Name.IsNullOrWhiteSpace() ? user.UserName : user.Name, user.MemberTypeAlias.IsNullOrWhiteSpace() ? Constants.Security.DefaultMemberTypeAlias : user.MemberTypeAlias); - UpdateMemberProperties(member, user); + UpdateMemberProperties(memberEntity, user); // TODO: do we want to accept empty passwords here - if third-party for example? // In other method if so? - _memberService.Save(member); + _memberService.Save(memberEntity); // re-assign id - user.Id = member.Id; + user.Id = UserIdToString(memberEntity.Id); - // 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)); + // [from backofficeuser] we have to remember whether Logins property is dirty, since the UpdateMemberProperties will reset it. + bool isLoginsPropertyDirty = memberEntity.IsPropertyDirty(nameof(MembersIdentityUser.Logins)); //if (isLoginsPropertyDirty) //{ @@ -90,7 +89,7 @@ namespace Umbraco.Infrastructure.Members // x.UserData))); //} - if (!member.HasIdentity) + if (!memberEntity.HasIdentity) { throw new DataException("Could not create the member, check logs for details"); } @@ -121,19 +120,19 @@ namespace Umbraco.Infrastructure.Members //} } - private bool UpdateMemberProperties(IMember member, UmbracoMembersIdentityUser memberIdentityUser) + private bool UpdateMemberProperties(IMember member, MembersIdentityUser memberIdentityUser) { 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(UmbracoMembersIdentityUser.Name)) && + if (memberIdentityUser.IsPropertyDirty(nameof(MembersIdentityUser.Name)) && member.Name != memberIdentityUser.Name && memberIdentityUser.Name.IsNullOrWhiteSpace() == false) { anythingChanged = true; member.Name = memberIdentityUser.Name; } - if (memberIdentityUser.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.Email)) && + if (memberIdentityUser.IsPropertyDirty(nameof(MembersIdentityUser.Email)) && member.Email != memberIdentityUser.Email && memberIdentityUser.Email.IsNullOrWhiteSpace() == false) { anythingChanged = true; @@ -152,14 +151,14 @@ namespace Umbraco.Infrastructure.Members } } - if (memberIdentityUser.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.UserName)) && + if (memberIdentityUser.IsPropertyDirty(nameof(MembersIdentityUser.UserName)) && member.Username != memberIdentityUser.UserName && memberIdentityUser.UserName.IsNullOrWhiteSpace() == false) { anythingChanged = true; member.Username = memberIdentityUser.UserName; } - - if (memberIdentityUser.IsPropertyDirty(nameof(UmbracoMembersIdentityUser.PasswordHash)) + + if (memberIdentityUser.IsPropertyDirty(nameof(MembersIdentityUser.PasswordHash)) && member.RawPasswordValue != memberIdentityUser.PasswordHash && memberIdentityUser.PasswordHash.IsNullOrWhiteSpace() == false) { anythingChanged = true; @@ -204,17 +203,17 @@ namespace Umbraco.Infrastructure.Members return anythingChanged; } - public Task DeleteAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) + public Task DeleteAsync(MembersIdentityUser user, CancellationToken cancellationToken) { throw new NotImplementedException(); } - public Task FindByIdAsync(string userId, CancellationToken cancellationToken) + public Task FindByIdAsync(string userId, CancellationToken cancellationToken) { throw new NotImplementedException(); } - public async Task FindByNameAsync(string normalizedUserName, CancellationToken cancellationToken) + public async Task FindByNameAsync(string normalizedUserName, CancellationToken cancellationToken) { // TODO: confirm logic cancellationToken.ThrowIfCancellationRequested(); @@ -226,17 +225,17 @@ namespace Umbraco.Infrastructure.Members return null; } - UmbracoMembersIdentityUser result = _mapper.Map(member); + MembersIdentityUser result = _mapper.Map(member); return await Task.FromResult(result); } - public Task GetNormalizedUserNameAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) + public Task GetNormalizedUserNameAsync(MembersIdentityUser user, CancellationToken cancellationToken) { throw new NotImplementedException(); } - public Task GetUserIdAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) + public Task GetUserIdAsync(MembersIdentityUser user, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); @@ -248,7 +247,7 @@ namespace Umbraco.Infrastructure.Members return Task.FromResult(user.Id.ToString()); } - public Task GetUserNameAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken) + public Task GetUserNameAsync(MembersIdentityUser user, CancellationToken cancellationToken) { // TODO: unit tests for and implement all bodies cancellationToken.ThrowIfCancellationRequested(); @@ -268,7 +267,7 @@ namespace Umbraco.Infrastructure.Members /// The normalized member name /// The cancellation token /// A task once complete - public Task SetNormalizedUserNameAsync(UmbracoMembersIdentityUser user, string normalizedName, CancellationToken cancellationToken) => SetUserNameAsync(user, normalizedName, cancellationToken); + public Task SetNormalizedUserNameAsync(MembersIdentityUser user, string normalizedName, CancellationToken cancellationToken) => SetUserNameAsync(user, normalizedName, cancellationToken); /// /// Sets the user name as an async operation @@ -277,7 +276,7 @@ namespace Umbraco.Infrastructure.Members /// The member user name /// The cancellation token /// A task once complete - public Task SetUserNameAsync(UmbracoMembersIdentityUser user, string userName, CancellationToken cancellationToken) + public Task SetUserNameAsync(MembersIdentityUser user, string userName, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); @@ -296,7 +295,7 @@ namespace Umbraco.Infrastructure.Members /// The member identity user /// The cancellation token /// An identity result task - public Task UpdateAsync(UmbracoMembersIdentityUser member, CancellationToken cancellationToken) + public Task UpdateAsync(MembersIdentityUser member, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); @@ -358,7 +357,7 @@ namespace Umbraco.Infrastructure.Members /// The cancellation token /// Throws if the properties are null /// Returns asynchronously - public Task SetPasswordHashAsync(UmbracoMembersIdentityUser user, string passwordHash, CancellationToken cancellationToken = default(CancellationToken)) + public Task SetPasswordHashAsync(MembersIdentityUser user, string passwordHash, CancellationToken cancellationToken = default(CancellationToken)) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); @@ -392,7 +391,7 @@ namespace Umbraco.Infrastructure.Members /// /// /// - public Task GetPasswordHashAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken = default(CancellationToken)) + public Task GetPasswordHashAsync(MembersIdentityUser user, CancellationToken cancellationToken = default(CancellationToken)) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); @@ -410,7 +409,7 @@ namespace Umbraco.Infrastructure.Members /// The identity user /// The cancellation token /// True if the user has a password - public Task HasPasswordAsync(UmbracoMembersIdentityUser user, CancellationToken cancellationToken = default(CancellationToken)) + public Task HasPasswordAsync(MembersIdentityUser user, CancellationToken cancellationToken = default(CancellationToken)) { cancellationToken.ThrowIfCancellationRequested(); ThrowIfDisposed(); @@ -421,5 +420,6 @@ namespace Umbraco.Infrastructure.Members return Task.FromResult(string.IsNullOrEmpty(user.PasswordHash) == false); } + private static string UserIdToString(int userId) => string.Intern(userId.ToString()); } } diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index b9368da89b..373d7df0e0 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -102,4 +102,8 @@ + + + + diff --git a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/UmbracoMembersServiceCollectionExtensionsTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/UmbracoMembersServiceCollectionExtensionsTests.cs index 0432ffa7db..691d3e0e29 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/UmbracoMembersServiceCollectionExtensionsTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/UmbracoMembersServiceCollectionExtensionsTests.cs @@ -1,10 +1,9 @@ -using System; +using System; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; using NUnit.Framework; -using Umbraco.Core.Members; using Umbraco.Extensions; -using Umbraco.Infrastructure.Members; +using Umbraco.Infrastructure.Security; using Umbraco.Tests.Integration.Testing; namespace Umbraco.Tests.Integration.Umbraco.Web.BackOffice @@ -13,27 +12,18 @@ namespace Umbraco.Tests.Integration.Umbraco.Web.BackOffice public class UmbracoMembersServiceCollectionExtensionsTests : UmbracoIntegrationTest { [Test] - public void AddUmbracoMembersIdentity_ExpectUmbracoMembersUserStoreResolvable() + public void AddUmbracoMembersIdentity_ExpectMembersUserStoreResolvable() { - var userStore = Services.GetService>(); + IUserStore userStore = Services.GetService>(); Assert.IsNotNull(userStore); - Assert.AreEqual(typeof(UmbracoMembersUserStore), userStore.GetType()); + Assert.AreEqual(typeof(MembersUserStore), userStore.GetType()); } - //[Test] - //public void AddUmbracoMembersIdentity_ExpectUmbracoMembersClaimsPrincipalFactoryResolvable() - //{ - // var principalFactory = Services.GetService>(); - - // Assert.IsNotNull(principalFactory); - // Assert.AreEqual(typeof(UmbracoMembersClaimsPrincipalFactory), principalFactory.GetType()); - //} - [Test] - public void AddUmbracoMembersIdentity_ExpectUmbracomMembersUserManagerResolvable() + public void AddUmbracoMembersIdentity_ExpectUmbracoMembersUserManagerResolvable() { - var userManager = Services.GetService(); + IMembersUserManager userManager = Services.GetService(); Assert.NotNull(userManager); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserManagerTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/UmbracoMemberIdentityUserManagerTests.cs similarity index 65% rename from src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserManagerTests.cs rename to src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/UmbracoMemberIdentityUserManagerTests.cs index 0645590b38..c5aea4d1a3 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserManagerTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/UmbracoMemberIdentityUserManagerTests.cs @@ -1,52 +1,55 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; using NUnit.Framework; using Umbraco.Core.Configuration.Models; -using Umbraco.Core.Members; -using Umbraco.Infrastructure.Members; +using Umbraco.Core.Security; +using Umbraco.Infrastructure.Security; +using Umbraco.Net; +using Umbraco.Web.Common.Security; -namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members +namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Security { [TestFixture] public class UmbracoMemberIdentityUserManagerTests { - private Mock> _mockMemberStore; - private Mock> _mockIdentityOptions; - private Mock> _mockPasswordHasher; - private Mock> _mockUserValidators; - private Mock>> _mockPasswordValidators; + private Mock> _mockMemberStore; + private Mock> _mockIdentityOptions; + private Mock> _mockPasswordHasher; + private Mock> _mockUserValidators; + private Mock>> _mockPasswordValidators; private Mock _mockNormalizer; private IdentityErrorDescriber _mockErrorDescriber; private Mock _mockServiceProviders; - private Mock>> _mockLogger; + private Mock>> _mockLogger; private Mock> _mockPasswordConfiguration; - public UmbracoMembersUserManager CreateSut() + public MembersUserManager CreateSut() { - _mockMemberStore = new Mock>(); - _mockIdentityOptions = new Mock>(); + _mockMemberStore = new Mock>(); + _mockIdentityOptions = new Mock>(); - var idOptions = new UmbracoMembersIdentityOptions { Lockout = { AllowedForNewUsers = false } }; + var idOptions = new MembersIdentityOptions { Lockout = { AllowedForNewUsers = false } }; _mockIdentityOptions.Setup(o => o.Value).Returns(idOptions); - _mockPasswordHasher = new Mock>(); + _mockPasswordHasher = new Mock>(); - var userValidators = new List>(); - _mockUserValidators = new Mock>(); - var validator = new Mock>(); + var userValidators = new List>(); + _mockUserValidators = new Mock>(); + var validator = new Mock>(); userValidators.Add(validator.Object); - _mockPasswordValidators = new Mock>>(); + _mockPasswordValidators = new Mock>>(); _mockNormalizer = new Mock(); _mockErrorDescriber = new IdentityErrorDescriber(); _mockServiceProviders = new Mock(); - _mockLogger = new Mock>>(); + _mockLogger = new Mock>>(); _mockPasswordConfiguration = new Mock>(); _mockPasswordConfiguration.Setup(x => x.Value).Returns(() => new MemberPasswordConfigurationSettings() @@ -54,26 +57,28 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members }); - var pwdValidators = new List> + var pwdValidators = new List> { - new PasswordValidator() + new PasswordValidator() }; - var userManager = new UmbracoMembersUserManager( + var userManager = new MembersUserManager( + new Mock().Object, _mockMemberStore.Object, _mockIdentityOptions.Object, _mockPasswordHasher.Object, userValidators, pwdValidators, - new UpperInvariantLookupNormalizer(), - new IdentityErrorDescriber(), + new BackOfficeLookupNormalizer(), + new BackOfficeIdentityErrorDescriber(), _mockServiceProviders.Object, - new Mock>>().Object, + new Mock().Object, + new Mock>>().Object, _mockPasswordConfiguration.Object); validator.Setup(v => v.ValidateAsync( userManager, - It.IsAny())) + It.IsAny())) .Returns(Task.FromResult(IdentityResult.Success)).Verifiable(); return userManager; @@ -83,8 +88,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members public async Task GivenICreateUser_AndTheIdentityResultFailed_ThenIShouldGetAFailedResultAsync() { //arrange - UmbracoMembersUserManager sut = CreateSut(); - UmbracoMembersIdentityUser fakeUser = new UmbracoMembersIdentityUser() + MembersUserManager sut = CreateSut(); + MembersIdentityUser fakeUser = new MembersIdentityUser() { PasswordConfig = "testConfig" }; @@ -116,7 +121,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members public async Task GivenICreateUser_AndTheUserIsNull_ThenIShouldGetAFailedResultAsync() { //arrange - UmbracoMembersUserManager sut = CreateSut(); + MembersUserManager sut = CreateSut(); CancellationToken fakeCancellationToken = new CancellationToken() { }; IdentityError[] identityErrors = { @@ -144,8 +149,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members public async Task GivenICreateANewUser_AndTheUserIsPopulatedCorrectly_ThenIShouldGetASuccessResultAsync() { //arrange - UmbracoMembersUserManager sut = CreateSut(); - UmbracoMembersIdentityUser fakeUser = new UmbracoMembersIdentityUser() + MembersUserManager sut = CreateSut(); + MembersIdentityUser fakeUser = new MembersIdentityUser() { PasswordConfig = "testConfig" }; diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/UmbracoMemberIdentityUserStoreTests.cs similarity index 88% rename from src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs rename to src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/UmbracoMemberIdentityUserStoreTests.cs index 5f8a143329..c1a39e7b57 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Members/UmbracoMemberIdentityUserStoreTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Security/UmbracoMemberIdentityUserStoreTests.cs @@ -1,5 +1,4 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -8,11 +7,10 @@ using Microsoft.AspNetCore.Identity; using Moq; 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.Infrastructure.Security; using Umbraco.Tests.UnitTests.Umbraco.Core.ShortStringHelper; namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members @@ -22,10 +20,10 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members { private Mock _mockMemberService; - public UmbracoMembersUserStore CreateSut() + public MembersUserStore CreateSut() { _mockMemberService = new Mock(); - return new UmbracoMembersUserStore( + return new MembersUserStore( _mockMemberService.Object, new UmbracoMapper(new MapDefinitionCollection(new List())), new Mock().Object); @@ -35,7 +33,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members public void GivenICreateUser_AndTheUserIsNull_ThenIShouldGetAFailedResultAsync() { //arrange - UmbracoMembersUserStore sut = CreateSut(); + MembersUserStore sut = CreateSut(); CancellationToken fakeCancellationToken = new CancellationToken(){}; //act @@ -50,8 +48,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Infrastructure.Members public async Task GivenICreateANewUser_AndTheUserIsPopulatedCorrectly_ThenIShouldGetASuccessResultAsync() { //arrange - UmbracoMembersUserStore sut = CreateSut(); - UmbracoMembersIdentityUser fakeUser = new UmbracoMembersIdentityUser() { }; + MembersUserStore sut = CreateSut(); + MembersIdentityUser fakeUser = new MembersIdentityUser() { }; CancellationToken fakeCancellationToken = new CancellationToken() { }; IMemberType fakeMemberType = new MemberType(new MockShortStringHelper(), 77); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs index 9924084bca..aa8fea1d78 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -11,14 +11,13 @@ using Umbraco.Core.Cache; using Umbraco.Core.Dictionary; using Umbraco.Core.Events; using Umbraco.Core.Mapping; -using Umbraco.Core.Members; using Umbraco.Core.Models; using Umbraco.Core.PropertyEditors; using Umbraco.Core.Security; using Umbraco.Core.Serialization; using Umbraco.Core.Services; using Umbraco.Core.Strings; -using Umbraco.Infrastructure.Members; +using Umbraco.Infrastructure.Security; using Umbraco.Tests.UnitTests.AutoFixture; using Umbraco.Tests.UnitTests.Umbraco.Core.ShortStringHelper; using Umbraco.Web; @@ -72,7 +71,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers [Test] [AutoMoqData] public async Task PostSaveMember_SaveNew_WhenAllIsSetupCorrectly_ExpectSuccessResponse( - [Frozen] IUmbracoMembersUserManager umbracoMembersUserManager, + [Frozen] IMembersUserManager umbracoMembersUserManager, IMemberTypeService memberTypeService, IDataTypeService dataTypeService, IMemberService memberService, @@ -101,7 +100,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers [Test] [AutoMoqData] public async Task PostSaveMember_Save_WhenAllIsSetupCorrectly_ExpectSuccessResponse( - [Frozen] IUmbracoMembersUserManager umbracoMembersUserManager, + [Frozen] IMembersUserManager umbracoMembersUserManager, IMemberTypeService memberTypeService, IDataTypeService dataTypeService, IMemberService memberService, @@ -130,7 +129,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers [Test] [AutoMoqData] public async Task PostSaveMember_SaveNew_WhenMemberEmailAlreadyExists_ExpectSuccessResponse( - [Frozen] IUmbracoMembersUserManager umbracoMembersUserManager, + [Frozen] IMembersUserManager umbracoMembersUserManager, IMemberTypeService memberTypeService, IDataTypeService dataTypeService, IMemberService memberService, @@ -159,7 +158,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers /// /// Setup all standard member data for test /// - private Member SetupMemberTestData(IUmbracoMembersUserManager umbracoMembersUserManager, + private Member SetupMemberTestData(IMembersUserManager umbracoMembersUserManager, IMemberTypeService memberTypeService, MapDefinitionCollection memberMapDefinition, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, @@ -170,7 +169,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers ContentSaveAction contentAction) { Mock.Get(umbracoMembersUserManager) - .Setup(x => x.CreateAsync(It.IsAny(), It.IsAny())) + .Setup(x => x.CreateAsync(It.IsAny())) .ReturnsAsync(() => IdentityResult.Success); Mock.Get(memberTypeService).Setup(x => x.GetDefault()).Returns("fakeAlias"); Mock.Get(backOfficeSecurityAccessor).Setup(x => x.BackOfficeSecurity).Returns(backOfficeSecurity); @@ -205,7 +204,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers UmbracoMapper mapper, IMemberService memberService, IMemberTypeService memberTypeService, - IUmbracoMembersUserManager membersUserManager, + IMembersUserManager membersUserManager, IDataTypeService dataTypeService, PropertyEditorCollection propertyEditorCollection, IBackOfficeSecurityAccessor backOfficeSecurityAccessor) => diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index ec0ea75986..b3f6e528cf 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -25,7 +25,7 @@ using Umbraco.Core.Services; using Umbraco.Core.Services.Implement; using Umbraco.Core.Strings; using Umbraco.Extensions; -using Umbraco.Infrastructure.Members; +using Umbraco.Infrastructure.Security; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.BackOffice.ModelBinders; using Umbraco.Web.Common.Attributes; @@ -35,7 +35,6 @@ using Umbraco.Web.Common.Filters; using Umbraco.Web.ContentApps; using Umbraco.Web.Models.ContentEditing; using Constants = Umbraco.Core.Constants; -using UmbracoMembersIdentityUser = Umbraco.Core.Members.UmbracoMembersIdentityUser; namespace Umbraco.Web.BackOffice.Controllers { @@ -52,7 +51,7 @@ namespace Umbraco.Web.BackOffice.Controllers private readonly UmbracoMapper _umbracoMapper; private readonly IMemberService _memberService; private readonly IMemberTypeService _memberTypeService; - private readonly IUmbracoMembersUserManager _memberManager; + private readonly IMembersUserManager _memberManager; private readonly IDataTypeService _dataTypeService; private readonly ILocalizedTextService _localizedTextService; private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; @@ -84,7 +83,7 @@ namespace Umbraco.Web.BackOffice.Controllers UmbracoMapper umbracoMapper, IMemberService memberService, IMemberTypeService memberTypeService, - IUmbracoMembersUserManager memberManager, + IMembersUserManager memberManager, IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, IJsonSerializer jsonSerializer) @@ -243,10 +242,11 @@ namespace Umbraco.Web.BackOffice.Controllers throw new ArgumentNullException(nameof(contentItem)); } - if (ModelState.IsValid == false) - { - throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); - } + // TODO: this causes an issue when trying to correct an invalid model + //if (ModelState.IsValid == false) + //{ + // throw new HttpResponseException(HttpStatusCode.BadRequest, ModelState); + //} // If we've reached here it means: // * Our model has been bound @@ -268,19 +268,6 @@ namespace Umbraco.Web.BackOffice.Controllers throw HttpResponseException.CreateValidationErrorResponse(forDisplay); } - 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 @@ -296,10 +283,10 @@ namespace Umbraco.Web.BackOffice.Controllers switch (contentItem.Action) { case ContentSaveAction.Save: - UpdateMemberData(contentItem); + await UpdateMemberDataAsync(contentItem); break; case ContentSaveAction.SaveNew: - IdentityResult identityResult = await CreateMemberAsync(contentItem, identityMember); + await CreateMemberAsync(contentItem); break; default: // we don't support anything else for members @@ -378,11 +365,24 @@ namespace Umbraco.Web.BackOffice.Controllers /// All member password processing and creation is done via the identity manager /// /// Member content data - /// The identity member to update /// The identity result of the created member - private async Task CreateMemberAsync(MemberSave contentItem, UmbracoMembersIdentityUser identityMember) + private async Task CreateMemberAsync(MemberSave contentItem) { + 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 = MembersIdentityUser.CreateNew( + contentItem.Username, + contentItem.Email, + memberType.Alias, + contentItem.Name); + IdentityResult created = await _memberManager.CreateAsync(identityMember, contentItem.Password.NewPassword); + if (created.Succeeded == false) { throw HttpResponseException.CreateNotificationValidationErrorResponse(created.Errors.ToErrorMessage()); @@ -406,6 +406,73 @@ namespace Umbraco.Web.BackOffice.Controllers return created; } + /// + /// Update the member security data + /// If the password has been reset then this method will return the reset/generated password, otherwise will return null. + /// + /// The member to save + private async Task UpdateMemberDataAsync(MemberSave contentItem) + { + MembersIdentityUser identityMember = await _memberManager.FindByIdAsync(((int)contentItem.Id).ToString()); + if (identityMember == null) + { + } + + IdentityResult updatedResult = await _memberManager.UpdateAsync(identityMember); + + if (updatedResult.Succeeded == false) + { + throw HttpResponseException.CreateNotificationValidationErrorResponse(updatedResult.Errors.ToErrorMessage()); + } + + contentItem.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()) + { + IMemberType memberType = _memberTypeService.Get(contentItem.PersistedContent.ContentTypeId); + var sensitiveProperties = memberType + .PropertyTypes.Where(x => memberType.IsSensitiveProperty(x.Alias)) + .ToList(); + + foreach (IPropertyType sensitiveProperty in sensitiveProperties) + { + ContentPropertyBasic destProp = contentItem.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 + object origValue = contentItem.PersistedContent.GetValue(sensitiveProperty.Alias); + destProp.Value = origValue; + } + } + } + + bool isLockedOut = contentItem.IsLockedOut; + + // if they were locked but now they are trying to be unlocked + if (contentItem.PersistedContent.IsLockedOut && isLockedOut == false) + { + contentItem.PersistedContent.IsLockedOut = false; + contentItem.PersistedContent.FailedPasswordAttempts = 0; + } + 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 + ModelState.AddModelError("custom", "An admin cannot lock a user"); + } + + // no password changes then exit ? + if (contentItem.Password != null) + { + // TODO: set the password using Identity core + contentItem.PersistedContent.RawPasswordValue = _memberManager.GeneratePassword(); + } + } + private void ValidateMemberData(MemberSave contentItem) { if (contentItem.Name.IsNullOrWhiteSpace()) @@ -433,13 +500,14 @@ namespace Umbraco.Web.BackOffice.Controllers if (contentItem.Password != null && !contentItem.Password.NewPassword.IsNullOrWhiteSpace()) { - 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"); - } + //TODO: should we validate the password here, in advance? or when saving the identity user + //Task> result = _memberManager.ValidatePasswordAsync(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"); + //} } } @@ -456,61 +524,6 @@ namespace Umbraco.Web.BackOffice.Controllers 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. - /// - /// The member to save - private void UpdateMemberData(MemberSave memberSave) - { - 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()) - { - IMemberType memberType = _memberTypeService.Get(memberSave.PersistedContent.ContentTypeId); - var sensitiveProperties = memberType - .PropertyTypes.Where(x => memberType.IsSensitiveProperty(x.Alias)) - .ToList(); - - foreach (IPropertyType sensitiveProperty in sensitiveProperties) - { - 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 - object origValue = memberSave.PersistedContent.GetValue(sensitiveProperty.Alias); - destProp.Value = origValue; - } - } - } - - var isLockedOut = memberSave.IsLockedOut; - - // if they were locked but now they are trying to be unlocked - if (memberSave.PersistedContent.IsLockedOut && isLockedOut == false) - { - memberSave.PersistedContent.IsLockedOut = false; - memberSave.PersistedContent.FailedPasswordAttempts = 0; - } - 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 - ModelState.AddModelError("custom", "An admin cannot lock a user"); - } - - // no password changes then exit ? - if (memberSave.Password != null) - { - // TODO: set the password - memberSave.PersistedContent.RawPasswordValue = _memberManager.GeneratePassword(); - } - } - /// /// Permanently deletes a member /// diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoMemberIdentityBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoMemberIdentityBuilderExtensions.cs index bc8d2bcbfb..888d963891 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoMemberIdentityBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoMemberIdentityBuilderExtensions.cs @@ -1,6 +1,6 @@ using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; -using Umbraco.Core.Members; +using Umbraco.Infrastructure.Security; namespace Umbraco.Extensions { @@ -13,7 +13,7 @@ namespace Umbraco.Extensions /// The type of the user manager to add. /// /// The current instance. - public static IdentityBuilder AddUserManager(this IdentityBuilder identityBuilder) where TUserManager : UserManager, TInterface + public static IdentityBuilder AddUserManager(this IdentityBuilder identityBuilder) where TUserManager : UserManager, TInterface { identityBuilder.AddUserManager(); identityBuilder.Services.AddScoped(typeof(TInterface), typeof(TUserManager)); diff --git a/src/Umbraco.Web.BackOffice/Extensions/UmbracoMembersUserServiceCollectionExtensions.cs b/src/Umbraco.Web.BackOffice/Extensions/UmbracoMembersUserServiceCollectionExtensions.cs index 6687c9e5be..d898e217cd 100644 --- a/src/Umbraco.Web.BackOffice/Extensions/UmbracoMembersUserServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Extensions/UmbracoMembersUserServiceCollectionExtensions.cs @@ -1,11 +1,11 @@ -using Microsoft.AspNetCore.Identity; +using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; -using Umbraco.Core.Members; using Umbraco.Core.Security; using Umbraco.Core.Serialization; -using Umbraco.Infrastructure.Members; +using Umbraco.Infrastructure.Security; using Umbraco.Web.BackOffice.Security; +using Umbraco.Web.Common.Security; namespace Umbraco.Extensions { @@ -19,17 +19,17 @@ namespace Umbraco.Extensions { services.BuildUmbracoMembersIdentity() .AddDefaultTokenProviders() - .AddUserStore() - .AddUserManager(); + .AddUserStore() + .AddUserManager(); } - private static UmbracoMembersIdentityBuilder BuildUmbracoMembersIdentity(this IServiceCollection services) + private static MembersIdentityBuilder BuildUmbracoMembersIdentity(this IServiceCollection services) { // Services used by Umbraco members identity - services.TryAddScoped, UserValidator>(); - services.TryAddScoped, PasswordValidator>(); - services.TryAddScoped, PasswordHasher>(); - return new UmbracoMembersIdentityBuilder(services); + services.TryAddScoped, UserValidator>(); + services.TryAddScoped, PasswordValidator>(); + services.TryAddScoped, PasswordHasher>(); + return new MembersIdentityBuilder(services); } } } diff --git a/src/Umbraco.Web.Common/Security/MembersUserManager.cs b/src/Umbraco.Web.Common/Security/MembersUserManager.cs new file mode 100644 index 0000000000..1a8c3e509a --- /dev/null +++ b/src/Umbraco.Web.Common/Security/MembersUserManager.cs @@ -0,0 +1,286 @@ +using System; +using System.Collections.Generic; +using System.Security.Principal; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Umbraco.Core; +using Umbraco.Core.Configuration.Models; +using Umbraco.Core.Models.Membership; +using Umbraco.Core.Security; +using Umbraco.Extensions; +using Umbraco.Infrastructure.Security; +using Umbraco.Net; +using Umbraco.Web.Models.ContentEditing; + + +namespace Umbraco.Web.Common.Security +{ + public class MembersUserManager : UmbracoUserManager, IMembersUserManager + { + private readonly IHttpContextAccessor _httpContextAccessor; + + public MembersUserManager( + IIpResolver ipResolver, + IUserStore store, + IOptions optionsAccessor, + IPasswordHasher passwordHasher, + IEnumerable> userValidators, + IEnumerable> passwordValidators, + //TODO: do we need members versions of this? + BackOfficeLookupNormalizer keyNormalizer, + BackOfficeIdentityErrorDescriber errors, + IServiceProvider services, + IHttpContextAccessor httpContextAccessor, + ILogger> logger, + IOptions passwordConfiguration) + : base(ipResolver, store, optionsAccessor, passwordHasher, userValidators, passwordValidators, keyNormalizer, errors, services, logger, passwordConfiguration) + { + _httpContextAccessor = httpContextAccessor; + } + + /// + /// Gets or sets the default members user password checker + /// + public IMembersUserPasswordChecker MembersUserPasswordChecker { get; set; } + // TODO: as per backoffice: This isn't a good way to set this, it needs to be injected + + /// + /// + /// By default this uses the standard ASP.Net Identity approach which is: + /// * Get password store + /// * Call VerifyPasswordAsync with the password store + user + password + /// * Uses the PasswordHasher.VerifyHashedPassword to compare the stored password + /// + /// In some cases people want simple custom control over the username/password check, for simplicity + /// sake, developers would like the users to simply validate against an LDAP directory but the user + /// data remains stored inside of Umbraco. + /// See: http://issues.umbraco.org/issue/U4-7032 for the use cases. + /// + /// We've allowed this check to be overridden with a simple callback so that developers don't actually + /// have to implement/override this class. + /// + public override async Task CheckPasswordAsync(MembersIdentityUser user, string password) + { + if (MembersUserPasswordChecker != null) + { + MembersUserPasswordCheckerResult result = await MembersUserPasswordChecker.CheckPasswordAsync(user, password); + + if (user.HasIdentity == false) + { + return false; + } + + // if the result indicates to not fallback to the default, then return true if the credentials are valid + if (result != MembersUserPasswordCheckerResult.FallbackToDefaultChecker) + { + return result == MembersUserPasswordCheckerResult.ValidCredentials; + } + } + + // use the default behavior + return await base.CheckPasswordAsync(user, password); + } + + /// + /// Override to check the user approval value as well as the user lock out date, by default this only checks the user's locked out date + /// + /// The user + /// True if the user is locked out, else false + /// + /// In the ASP.NET Identity world, there is only one value for being locked out, in Umbraco we have 2 so when checking this for Umbraco we need to check both values + /// + public override async Task IsLockedOutAsync(MembersIdentityUser user) + { + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + + if (user.IsApproved == false) + { + return true; + } + + return await base.IsLockedOutAsync(user); + } + + public override async Task AccessFailedAsync(MembersIdentityUser user) + { + IdentityResult result = await base.AccessFailedAsync(user); + + // Slightly confusing: this will return a Success if we successfully update the AccessFailed count + if (result.Succeeded) + { + RaiseLoginFailedEvent(_httpContextAccessor.HttpContext?.User, user.Id); + } + + return result; + } + + public override async Task ChangePasswordWithResetAsync(string userId, string token, string newPassword) + { + IdentityResult result = await base.ChangePasswordWithResetAsync(userId, token, newPassword); + if (result.Succeeded) + { + RaisePasswordChangedEvent(_httpContextAccessor.HttpContext?.User, userId); + } + + return result; + } + + public override async Task ChangePasswordAsync(MembersIdentityUser user, string currentPassword, string newPassword) + { + IdentityResult result = await base.ChangePasswordAsync(user, currentPassword, newPassword); + if (result.Succeeded) + { + RaisePasswordChangedEvent(_httpContextAccessor.HttpContext?.User, user.Id); + } + + return result; + } + + /// + public override async Task SetLockoutEndDateAsync(MembersIdentityUser user, DateTimeOffset? lockoutEnd) + { + if (user == null) + { + throw new ArgumentNullException(nameof(user)); + } + + IdentityResult result = await base.SetLockoutEndDateAsync(user, lockoutEnd); + + // The way we unlock is by setting the lockoutEnd date to the current datetime + if (result.Succeeded && lockoutEnd >= DateTimeOffset.UtcNow) + { + RaiseAccountLockedEvent(_httpContextAccessor.HttpContext?.User, user.Id); + } + else + { + RaiseAccountUnlockedEvent(_httpContextAccessor.HttpContext?.User, user.Id); + + // Resets the login attempt fails back to 0 when unlock is clicked + await ResetAccessFailedCountAsync(user); + } + + return result; + } + + /// + public override async Task ResetAccessFailedCountAsync(MembersIdentityUser user) + { + IdentityResult result = await base.ResetAccessFailedCountAsync(user); + + // raise the event now that it's reset + RaiseResetAccessFailedCountEvent(_httpContextAccessor.HttpContext?.User, user.Id); + + return result; + } + + private string GetCurrentUserId(IPrincipal currentUser) + { + UmbracoBackOfficeIdentity umbIdentity = currentUser?.GetUmbracoIdentity(); + var currentUserId = umbIdentity?.GetUserId() ?? Core.Constants.Security.SuperUserIdAsString; + return currentUserId; + } + + private IdentityAuditEventArgs CreateArgs(AuditEvent auditEvent, IPrincipal currentUser, string affectedUserId, string affectedUsername) + { + var currentUserId = GetCurrentUserId(currentUser); + var ip = IpResolver.GetCurrentRequestIpAddress(); + return new IdentityAuditEventArgs(auditEvent, ip, currentUserId, string.Empty, affectedUserId, affectedUsername); + } + + private IdentityAuditEventArgs CreateArgs(AuditEvent auditEvent, BackOfficeIdentityUser currentUser, string affectedUserId, string affectedUsername) + { + var currentUserId = currentUser.Id; + var ip = IpResolver.GetCurrentRequestIpAddress(); + return new IdentityAuditEventArgs(auditEvent, ip, currentUserId, string.Empty, affectedUserId, affectedUsername); + } + + // TODO: Review where these are raised and see if they can be simplified and either done in the this usermanager or the signin manager, + // lastly we'll resort to the authentication controller but we should try to remove all instances of that occuring + public void RaiseAccountLockedEvent(IPrincipal currentUser, string userId) => OnAccountLocked(CreateArgs(AuditEvent.AccountLocked, currentUser, userId, string.Empty)); + + public void RaiseAccountUnlockedEvent(IPrincipal currentUser, string userId) => OnAccountUnlocked(CreateArgs(AuditEvent.AccountUnlocked, currentUser, userId, string.Empty)); + + public void RaiseForgotPasswordRequestedEvent(IPrincipal currentUser, string userId) => OnForgotPasswordRequested(CreateArgs(AuditEvent.ForgotPasswordRequested, currentUser, userId, string.Empty)); + + public void RaiseForgotPasswordChangedSuccessEvent(IPrincipal currentUser, string userId) => OnForgotPasswordChangedSuccess(CreateArgs(AuditEvent.ForgotPasswordChangedSuccess, currentUser, userId, string.Empty)); + + public void RaiseLoginFailedEvent(IPrincipal currentUser, string userId) => OnLoginFailed(CreateArgs(AuditEvent.LoginFailed, currentUser, userId, string.Empty)); + + public void RaiseLoginRequiresVerificationEvent(IPrincipal currentUser, string userId) => OnLoginRequiresVerification(CreateArgs(AuditEvent.LoginRequiresVerification, currentUser, userId, string.Empty)); + + public void RaiseLoginSuccessEvent(IPrincipal currentUser, string userId) => OnLoginSuccess(CreateArgs(AuditEvent.LoginSucces, currentUser, userId, string.Empty)); + + public SignOutAuditEventArgs RaiseLogoutSuccessEvent(IPrincipal currentUser, string userId) + { + var currentUserId = GetCurrentUserId(currentUser); + var args = new SignOutAuditEventArgs(AuditEvent.LogoutSuccess, IpResolver.GetCurrentRequestIpAddress(), performingUser: currentUserId, affectedUser: userId); + OnLogoutSuccess(args); + return args; + } + + public void RaisePasswordChangedEvent(IPrincipal currentUser, string userId) => OnPasswordChanged(CreateArgs(AuditEvent.LogoutSuccess, currentUser, userId, string.Empty)); + + public void RaiseResetAccessFailedCountEvent(IPrincipal currentUser, string userId) => OnResetAccessFailedCount(CreateArgs(AuditEvent.ResetAccessFailedCount, currentUser, userId, string.Empty)); + + public UserInviteEventArgs RaiseSendingUserInvite(IPrincipal currentUser, UserInvite invite, IUser createdUser) + { + var currentUserId = GetCurrentUserId(currentUser); + var ip = IpResolver.GetCurrentRequestIpAddress(); + var args = new UserInviteEventArgs(ip, currentUserId, invite, createdUser); + OnSendingUserInvite(args); + return args; + } + + public bool HasSendingUserInviteEventHandler => SendingUserInvite != null; + + // TODO: These static events are problematic. Moving forward we don't want static events at all but we cannot + // have non-static events here because the user manager is a Scoped instance not a singleton + // so we'll have to deal with this a diff way i.e. refactoring how events are done entirely + public static event EventHandler AccountLocked; + public static event EventHandler AccountUnlocked; + public static event EventHandler ForgotPasswordRequested; + public static event EventHandler ForgotPasswordChangedSuccess; + public static event EventHandler LoginFailed; + public static event EventHandler LoginRequiresVerification; + public static event EventHandler LoginSuccess; + public static event EventHandler LogoutSuccess; + public static event EventHandler PasswordChanged; + public static event EventHandler PasswordReset; + public static event EventHandler ResetAccessFailedCount; + + /// + /// Raised when a user is invited + /// + public static event EventHandler SendingUserInvite; // this event really has nothing to do with the user manager but was the most convenient place to put it + + protected virtual void OnAccountLocked(IdentityAuditEventArgs e) => AccountLocked?.Invoke(this, e); + + protected virtual void OnSendingUserInvite(UserInviteEventArgs e) => SendingUserInvite?.Invoke(this, e); + + protected virtual void OnAccountUnlocked(IdentityAuditEventArgs e) => AccountUnlocked?.Invoke(this, e); + + protected virtual void OnForgotPasswordRequested(IdentityAuditEventArgs e) => ForgotPasswordRequested?.Invoke(this, e); + + protected virtual void OnForgotPasswordChangedSuccess(IdentityAuditEventArgs e) => ForgotPasswordChangedSuccess?.Invoke(this, e); + + protected virtual void OnLoginFailed(IdentityAuditEventArgs e) => LoginFailed?.Invoke(this, e); + + protected virtual void OnLoginRequiresVerification(IdentityAuditEventArgs e) => LoginRequiresVerification?.Invoke(this, e); + + protected virtual void OnLoginSuccess(IdentityAuditEventArgs e) => LoginSuccess?.Invoke(this, e); + + protected virtual void OnLogoutSuccess(SignOutAuditEventArgs e) => LogoutSuccess?.Invoke(this, e); + + protected virtual void OnPasswordChanged(IdentityAuditEventArgs e) => PasswordChanged?.Invoke(this, e); + + protected virtual void OnPasswordReset(IdentityAuditEventArgs e) => PasswordReset?.Invoke(this, e); + + protected virtual void OnResetAccessFailedCount(IdentityAuditEventArgs e) => ResetAccessFailedCount?.Invoke(this, e); + } +} diff --git a/src/Umbraco.Web.UI.NetCore/appsettings.json b/src/Umbraco.Web.UI.NetCore/appsettings.json index 43ae07d5d6..29682186f6 100644 --- a/src/Umbraco.Web.UI.NetCore/appsettings.json +++ b/src/Umbraco.Web.UI.NetCore/appsettings.json @@ -1,6 +1,6 @@ { "ConnectionStrings": { - "umbracoDbDSN": "Server=(LocalDB)\\Umbraco;Database=NetCore;Integrated Security=true" + "umbracoDbDSN": "" }, "Serilog": { "MinimumLevel": { @@ -71,4 +71,4 @@ } } } -} \ No newline at end of file +}